bpf/tests/scapy: warn ctx LEN < expected len scapy

We are unable to easily write in the Scapy assert MAP when the ctx
len < than the expected length, as in order to unroll loops we are
expecting a const value.

Therefore, store the expected len and the actual ctx len, so that
bpftest can issue a warning and a useful tip to further debug the
issue in absence of the packet diff.

Added test coverage.

Signed-off-by: Marc Suñé <marc.sune@isovalent.com>

diff --git a/bpf/tests/_scapy_selftest.c b/bpf/tests/_scapy_selftest.c
index 5df6dd92ea94240ec0aabe8aef55ffadfd140f55..1c7a9478681ba25ed29718a3d27beb186a8ddde3 100644 (file)
--- a/bpf/tests/_scapy_selftest.c
+++ b/bpf/tests/_scapy_selftest.c
@@ -18,9 +18,11 @@
 
 #define ASSERT1_FAIL "assert1_fail"
 #define ASSERT2_FAIL "assert2_fail"
+#define ASSERT3_FAIL "assert3_fail"
 
 #define LEN_SST_EXP sizeof(BUF(SST_EXP))
 #define LEN_SST_NOT_EXP sizeof(BUF(SST_NOT_EXP))
+#define LEN_SST_NOT_EXP_PAD sizeof(BUF(SST_NOT_EXP_PAD))
 
 /**
  * These are here so that test_fail_now() that returns is captured
@@ -56,7 +58,20 @@ int force_assert_fail_off2(struct __ctx_buff *ctx)
        return TEST_PASS;
 }
 
-static struct scapy_assert null_entry = {0};
+static __always_inline
+int force_assert_fail_ctx_smaller_exp(struct __ctx_buff *ctx)
+{
+       test_init();
+
+       BUF_DECL(SST_NOT_EXP_PAD, sst_rep_pad);
+
+       ASSERT_CTX_BUF_OFF2(ASSERT3_FAIL, "Ether", ctx, 0, SST_NOT_EXP_PAD,
+                           BUF(SST_NOT_EXP_PAD), LEN_SST_NOT_EXP_PAD,
+                           LEN_SST_NOT_EXP_PAD);
+       fake_test_end();
+
+       return TEST_PASS;
+}
 
 PKTGEN("tc", "1_basic_test")
 int pktgen_scapy_basic_test(struct __ctx_buff *ctx)
@@ -83,6 +98,7 @@ int check_scapy_basic_test(struct __ctx_buff *ctx)
 
        BUF_DECL(SST_EXP, sst_req);
        BUF_DECL(SST_NOT_EXP, sst_rep);
+       BUF_DECL(SST_NOT_EXP_PAD, sst_rep_pad);
 
        ASSERT_CTX_BUF_OFF(ASSERT1, "Ether", ctx, 0, SST_EXP,
                           LEN_SST_EXP);
@@ -94,8 +110,10 @@ int check_scapy_basic_test(struct __ctx_buff *ctx)
        assert(rc == TEST_FAIL);
        rc = force_assert_fail_off2(ctx);
        assert(rc == TEST_FAIL);
+       rc = force_assert_fail_ctx_smaller_exp(ctx);
+       assert(rc == TEST_FAIL);
 
-       assert(scapy_assert_map_cnt == 2);
+       assert(scapy_assert_map_cnt == 3);
 
        {
                /* ASSERT_CTX_BUF_OFF */
@@ -108,9 +126,12 @@ int check_scapy_basic_test(struct __ctx_buff *ctx)
                                    LEN_SST_EXP) == 0);
                assert(memcmp(entry->name, ASSERT1_FAIL,
                              sizeof(ASSERT1_FAIL)) == 0);
-               assert(entry->len == LEN_SST_NOT_EXP);
-               rc = map_update_elem(&scapy_assert_map, &id, &null_entry,
-                                    BPF_ANY);
+               assert(entry->exp_len == LEN_SST_NOT_EXP);
+               assert(entry->got_len == LEN_SST_EXP);
+               assert(entry->exp_len == entry->got_len);
+
+               rc = map_update_elem(&scapy_assert_map, &id,
+                                    &__scapy_null_assert, BPF_ANY);
                assert(rc == 0);
        }
        {
@@ -124,8 +145,28 @@ int check_scapy_basic_test(struct __ctx_buff *ctx)
                                    LEN_SST_EXP) == 0);
                assert(memcmp(entry->name, ASSERT2_FAIL,
                              sizeof(ASSERT2_FAIL)) == 0);
-               rc = map_update_elem(&scapy_assert_map, &id, &null_entry,
-                                    BPF_ANY);
+               assert(entry->exp_len == LEN_SST_NOT_EXP);
+               assert(entry->got_len == LEN_SST_EXP);
+               assert(entry->exp_len == entry->got_len);
+               rc = map_update_elem(&scapy_assert_map, &id,
+                                    &__scapy_null_assert, BPF_ANY);
+               assert(rc == 0);
+       }
+       {
+               /* len(CTX) < len(EXP) */
+               id = 2;
+               entry = map_lookup_elem(&scapy_assert_map, &id);
+               assert(entry);
+               assert(scapy_memcmp(entry->exp_buf, BUF(SST_NOT_EXP),
+                                   LEN_SST_NOT_EXP) == 0);
+               assert(scapy_memcmp(entry->got_buf, __scapy_null_assert.got_buf,
+                                   LEN_SST_EXP) == 0);
+               assert(memcmp(entry->name, ASSERT3_FAIL,
+                             sizeof(ASSERT3_FAIL)) == 0);
+               assert(entry->exp_len == LEN_SST_NOT_EXP_PAD);
+               assert(entry->got_len == LEN_SST_EXP);
+               rc = map_update_elem(&scapy_assert_map, &id,
+                                    &__scapy_null_assert, BPF_ANY);
                assert(rc == 0);
        }
 

diff --git a/bpf/tests/bpftest/bpf_test.go b/bpf/tests/bpftest/bpf_test.go
index 8b4b2c66ec2b2f477266cc75a06d8eebf2d47cb6..f0d19aacde3e4650069a4425f7d4d4354d7651f8 100644 (file)
--- a/bpf/tests/bpftest/bpf_test.go
+++ b/bpf/tests/bpftest/bpf_test.go
@@ -351,8 +351,9 @@ type ScapyAssert struct {
        File       [scapyMaxStrLen]byte
        LNum       [scapyMaxStrLen]byte
        FirstLayer [scapyMaxStrLen]byte
-       Len        uint16
+       ExpLen     uint16
        ExpBuf     [scapyMaxBuf]byte
+       GotLen     uint16
        GotBuf     [scapyMaxBuf]byte
        Pad        [2]byte
 }
@@ -363,9 +364,10 @@ func assertToJSONMap(a ScapyAssert) map[string]any {
                "file":        string(bytes.TrimRight(a.File[:], "\x00")),
                "linenum":     string(bytes.TrimRight(a.LNum[:], "\x00")),
                "first-layer": string(bytes.TrimRight(a.FirstLayer[:], "\x00")),
-               "len":         a.Len,
-               "exp-buf":     hex.EncodeToString(a.ExpBuf[:a.Len]),
-               "got-buf":     hex.EncodeToString(a.GotBuf[:a.Len]),
+               "exp-len":     a.ExpLen,
+               "exp-buf":     hex.EncodeToString(a.ExpBuf[:a.ExpLen]),
+               "got-len":     a.GotLen,
+               "got-buf":     hex.EncodeToString(a.GotBuf[:a.GotLen]),
        }
 }
 
@@ -389,10 +391,19 @@ func scapyParseAsserts(t *testing.T, scapyAssertMap *ebpf.Map) {
                if err != nil {
                        t.Fatalf("error while getting iterating over the assert map: %s", err)
                }
-               if aval.Len == 0 {
+
+               if aval.GotLen == 0 || aval.ExpLen == 0 {
                        break
                }
 
+               if aval.GotLen < aval.ExpLen {
+                       fName := string(bytes.TrimRight(aval.File[:], "\x00"))
+                       lNum := string(bytes.TrimRight(aval.LNum[:], "\x00"))
+
+                       t.Logf("  Warning: assert '%s' at %s:%s expected CTX to have at least '%d' bytes, but only had '%d'. Got buffer will be NULL and diff invalid!", aval.Name, fName, lNum, aval.ExpLen, aval.GotLen)
+                       t.Logf("  Tip: temporally lower the expected size in the assert to '%d' to parse the ctx contents in Scapy.", aval.GotLen)
+               }
+
                asserts = append(asserts, assertToJSONMap(aval))
                count++
        }

diff --git a/bpf/tests/scapy.h b/bpf/tests/scapy.h
index f292a9c70b0c13bdeff75083b54d6b9b27a70886..20366fc2d7e3267e172abdeebfee7088c85f445c 100644 (file)
--- a/bpf/tests/scapy.h
+++ b/bpf/tests/scapy.h
@@ -109,8 +109,9 @@ struct scapy_assert {
        char file[__SCAPY_MAX_STR_LEN];         /* Filename */
        char lnum[__SCAPY_MAX_STR_LEN];         /* Line number */
        char first_layer[__SCAPY_MAX_STR_LEN];  /* Scapy  layer (e.g. Ether) */
-       __u16 len;                              /* Buffer len (data compared) */
+       __u16 exp_len;                          /* Exp. len (compared len) */
        __u8 exp_buf[__SCAPY_MAX_BUF];          /* Expected buffer */
+       __u16 got_len;                          /* Got buffer len */
        __u8 got_buf[__SCAPY_MAX_BUF];          /* Got buffer */
        __u8 pad[2];
 };
@@ -129,6 +130,7 @@ static __u32 scapy_assert_map_cnt;
 
 /* Needs to be here not to blow up stack size */
 static struct scapy_assert __scapy_assert = {0};
+static struct scapy_assert __scapy_null_assert = {0};
 
 #ifndef __ASSERT_TRACE_FAIL_LEN
 #define __ASSERT_TRACE_FAIL_LEN(BUF_NAME, _BUF_LEN, LEN)               \
@@ -141,6 +143,9 @@ static struct scapy_assert __scapy_assert = {0};
        test_log("CTX and buffer '" BUF_NAME "' content mismatch ")
 #endif /* __ASSERT_TRACE_FAIL_BUF */
 
+#define __SCAPY_GET_CTX_LEN(__DATA, __DATA_END) \
+       (__u16)((unsigned long long)(__DATA_END) - (unsigned long long)(__DATA))
+
 static __always_inline
 bool __assert_map_add_failure(const char *name, const __u8 name_len,
                              const char *first_layer,
@@ -149,11 +154,17 @@ bool __assert_map_add_failure(const char *name, const __u8 name_len,
                              const __u16 len, void *data,
                              const void *data_end)
 {
-       __scapy_assert.len = len;
+       __scapy_assert.exp_len = len;
        scapy_memcpy(__scapy_assert.exp_buf, buf, len);
 
-       if (data + len <= data_end)
+       __scapy_assert.got_len = __SCAPY_GET_CTX_LEN(data, data_end);
+       if (data + len <= data_end) {
                scapy_memcpy(__scapy_assert.got_buf, data, len);
+       } else {
+               /* Clear previous assert content */
+               scapy_memcpy(__scapy_assert.got_buf,
+                            __scapy_null_assert.got_buf, len);
+       }
 
        scapy_strncpy(__scapy_assert.name, name, name_len);
        scapy_strncpy(__scapy_assert.file, __FILE__, sizeof(__FILE__));

diff --git a/bpf/tests/scapy/selftest_pkt_defs.py b/bpf/tests/scapy/selftest_pkt_defs.py
index 1a26fe5a78bb46f2f0ed4fd8ddea01169de90bbc..e92287296ee0a235077f7d7644cf0b45989822b3 100644 (file)
--- a/bpf/tests/scapy/selftest_pkt_defs.py
+++ b/bpf/tests/scapy/selftest_pkt_defs.py
@@ -17,3 +17,13 @@ sst_rep = (
     ARP(op="is-at", psrc=v4_svc_one, pdst=v4_ext_one, \
         hwsrc=mac_two, hwdst=mac_one)
 )
+
+# Padded reply to test
+sst_rep_pad = (
+    Ether(dst=mac_one, src=mac_two) /
+    ARP(op="is-at", psrc=v4_svc_one, pdst=v4_ext_one, \
+        hwsrc=mac_two, hwdst=mac_one) /
+    Raw("A"*8)
+)
+
+assert len(bytes(sst_rep_pad)) == (len(bytes(sst_rep)) + 8)

diff --git a/tools/legacyhguardcheck/main.go b/tools/legacyhguardcheck/main.go
index 883c58095608b25b0fa05c5e0f971c14a5e1aee1..bb54ce352c7b15c7a50f49d0fb3018595e4ddaa0 100644 (file)
--- a/tools/legacyhguardcheck/main.go
+++ b/tools/legacyhguardcheck/main.go
@@ -75,6 +75,7 @@ var exclude = []string{
        "bpf/tests/common.h",
        "bpf/tests/bpf_skb_255_tests.c",
        "bpf/tests/bpf_skb_511_tests.c",
+       "bpf/tests/scapy.h",
 }
 
 func checkFile(filePath string) (bool, error) {