Re: [PATCH bpf-next 6/7] selftests/bpf: Add XDP load-balancer benchmark driver

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [Low] Incorrect IPv4 checksum calculation for packets with IP options in test packet generator
--

commit cc1cb16090fdc833cb82a1c6a168d6cb11005220
Author: Puranjay Mohan <puranjay@kernel.org>
selftests/bpf: Add XDP load-balancer benchmark driver

This commit wires up the userspace side of the XDP load-balancer benchmark.
It covers 24 scenarios and validates correctness before benchmarking by
comparing the output packet byte-for-byte against an expected packet.

> diff --git a/tools/testing/selftests/bpf/benchs/bench_xdp_lb.c b/tools/testing/selftests/bpf/benchs/bench_xdp_lb.c
> new file mode 100644
> index 000000000000..0b6709a2b03c
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/benchs/bench_xdp_lb.c
[ ... ]
> +static void build_packet(int idx)
> +{
[ ... ]
> +	if (sc->is_v6) {
[ ... ]
> +	} else {
> +		struct iphdr iph = {};
> +		__u32 ip_off = off;
> +
> +		iph.version  = 4;
> +		iph.ihl      = sc->set_ip_options ? 6 : 5;
> +		iph.ttl      = 64;
> +		iph.protocol = sc->ip_proto;
> +		iph.saddr    = htonl(sc->src_addr);
> +		iph.daddr    = htonl(sc->vip_addr);
> +		iph.frag_off = sc->set_frag ? htons(IP_MF) : 0;
> +		off += sizeof(iph);
> +
> +		if (sc->set_ip_options) {
> +			/* NOP option padding (4 bytes = 1 word) */
> +			__u32 nop = htonl(0x01010101);
> +
> +			memcpy(p + off, &nop, sizeof(nop));
> +			off += sizeof(nop);
> +		}
> +
> +		build_l4(sc, p, &off);
> +
> +		memcpy(p + off, "bench___payload!", 16);
> +		off += 16;
> +
> +		iph.tot_len = htons(off - ip_off);
> +		iph.check   = ip_checksum(&iph, sizeof(iph));

Does this calculation exclude the appended IP options from the checksum?

When sc->set_ip_options is true, the header length is set to 6 words (24
bytes) and 4 bytes of NOP options are copied directly into the packet
buffer. However, ip_checksum() is only passed sizeof(iph), which is exactly
20 bytes, and computes the checksum over the local iph struct alone.

While this logically malformed packet might not cause the current tests
to fail because the BPF program validates iph->ihl == 5 before verifying
the IP checksum, could this mask bugs or produce false test failures if
the BPF program is later modified to validate L3 checksums?

> +		memcpy(p + ip_off, &iph, sizeof(iph));
> +	}
> +
> +	pkt_len[idx] = off;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260427232313.1582588-1-puranjay@kernel.org?part=6