From: Marc Suñé Date: Wed, 10 Dec 2025 11:08:34 +0000 (+0100) Subject: bpf/tests/scapy: warn ctx LEN < expected len scapy X-Git-Url: https://git.feebdaed.xyz/?a=commitdiff_plain;h=adc04d6b1b61bea8414edb7242c53358c3f3afef;p=0xmirror%2Fcilium.git 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ñé --- diff --git a/bpf/tests/_scapy_selftest.c b/bpf/tests/_scapy_selftest.c index 5df6dd92ea..1c7a947868 100644 --- 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 8b4b2c66ec..f0d19aacde 100644 --- 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 f292a9c70b..20366fc2d7 100644 --- 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 1a26fe5a78..e92287296e 100644 --- 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 883c580956..bb54ce352c 100644 --- 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) {