> This isn't a bug, but would it be clearer to paste the full failure
> output or mark the elision? The summary line says six subtests failed
> (1/6 FAILED) while only the ipv6 one is shown, and test_progs.c runs
> three ASSERT_EQ calls per subtest ('test first port', 'test second port',
> 'test third port'), so 'test first port' would have failed too. And
> '1/0 PASSED' reports one passing test, which cannot come from a run whose
> only named test failed.Not a bug. That log is the test failing on the TEST_PACKET_LEN=99 case against the old (pre-fix) csum_partial(), not my patch. My change is what makes the test catch it; it passes with the fix applied. Madhav On Wed, Sep 16, 2026 at 12:13 PM <[email protected]> wrote: > > > diff --git a/tools/testing/selftests/bpf/network_helpers.h > > b/tools/testing/selftests/bpf/network_helpers.h > > index 75133119c04a3..878c9fc5c37ca 100644 > > --- a/tools/testing/selftests/bpf/network_helpers.h > > +++ b/tools/testing/selftests/bpf/network_helpers.h > > @@ -129,12 +129,23 @@ static __u16 csum_fold(__u32 csum) > > > > static __wsum csum_partial(const void *buf, int len, __wsum sum) > > { > > - __u16 *p = (__u16 *)buf; > > + const __u8 *p = buf; > > int num_u16 = len >> 1; > > int i; > > > > for (i = 0; i < num_u16; i++) > > - sum += p[i]; > > + sum += ((const __u16 *)p)[i]; > > + > > + /* > > + * RFC 1071: an odd-length buffer's trailing byte is paired with > > + * a zero pad byte to form the final 16-bit word. > > + */ > > + if (len & 1) { > > + __u16 tail = 0; > > + > > + __builtin_memcpy(&tail, p + len - 1, 1); > > + sum += tail; > > + } > > > > return sum; > > } > > diff --git > > a/tools/testing/selftests/bpf/prog_tests/flow_dissector_classification.c > > b/tools/testing/selftests/bpf/prog_tests/flow_dissector_classification.c > > index 80b153d3ddecf..421dfa6c4ea36 100644 > > --- a/tools/testing/selftests/bpf/prog_tests/flow_dissector_classification.c > > +++ b/tools/testing/selftests/bpf/prog_tests/flow_dissector_classification.c > > @@ -27,7 +27,7 @@ > > #define TEST_NAME_MAX_LEN (32 + SUBTEST_NAME_MAX_LEN) > > #define MAX_SOURCE_PORTS 3 > > #define TEST_PACKETS_COUNT 10 > > -#define TEST_PACKET_LEN 100 > > +#define TEST_PACKET_LEN 99 > > #define TEST_PACKET_PATTERN 'a' > > #define TEST_IPV4 "192.168.0.1/32" > > #define TEST_IPV6 "100::a/128" > > The commit message body includes: > > > v1 -> v2: > > - comment style: opening /* on its own line, per BPF selftests style > > - Retarget the Fixes tag to bcc00987bc56. commit f4504af68575 > > ("selftests/bpf: move ip checksum helper to network helpers") moved > > the helper, but sizeof(iphdr) is always a multiple of 32 bit words / > > 4 Bytes (iph->ihl counts in 4-byte words), so the odd-length path > > was never reachable through build_ip_csum(). csum_partial() first > > gets called with a length that isn't guaranteed even in > > bcc00987bc56, via build_udp_v4_csum()/build_udp_v6_csum(). > > - TEST_PACKET_LEN 100 -> 99 so an existing test catches this instead > > of the bug staying unexercised > > This isn't a bug, but could the 'v1 -> v2:' block move below the '---' > line so it stays out of the committed changelog? Documentation/process/ > submitting-patches.rst asks for inter-version changelogs to go below the > '---' separator so they are dropped when the patch is applied. The > content itself is useful review material, especially the reasoning for > retargeting the Fixes tag. > > The commit message also includes this test failure excerpt: > > > Without the fix and with TEST_PACKET_LEN=99, > > flow_dissector_classification > > fails under vmtest.sh: > > > > test_flow_dissector_classification:FAIL:test third port unexpected > > test third port: actual 0 != expected 10 > > #137/6 flow_dissector_classification/ipv6:FAIL > > #137 flow_dissector_classification:FAIL > > Summary: 1/0 PASSED, 0 SKIPPED, 1/6 FAILED > > This isn't a bug, but would it be clearer to paste the full failure > output or mark the elision? The summary line says six subtests failed > (1/6 FAILED) while only the ipv6 one is shown, and test_progs.c runs > three ASSERT_EQ calls per subtest ('test first port', 'test second port', > 'test third port'), so 'test first port' would have failed too. And > '1/0 PASSED' reports one passing test, which cannot come from a run whose > only named test failed. > > > --- > AI reviewed your patch. Please fix the bug or email reply why it's not a bug. > See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md > > CI run summary: https://github.com/kernel-patches/bpf/actions/runs/35091050280
