Re: [PATCH bpf-next 5/7] selftests/bpf: Add XDP load-balancer BPF program

Puranjay Mohan
2026-04-28T09:30:52Z

On Tue, Apr 28, 2026 at 2:05 AM <sashiko-bot@kernel.org> wrote:
>
> Sashiko AI review found 1 potential issue(s):
> - [Medium] The `cold_lru` logic inside the benchmark loop's `reset` block toggles the IP source address back and forth, breaking the benchmark's LRU cache miss measurement.
> --
>
> commit 295ed705f4fdd57c01615b43803c47313840bbf1
> Author: Puranjay Mohan <puranjay@kernel.org>
>
> selftests/bpf: Add XDP load-balancer BPF program
>
> Add the BPF datapath for the XDP load-balancer benchmark, a
> simplified L4 load-balancer inspired by katran.
>
> > diff --git a/tools/testing/selftests/bpf/progs/xdp_lb_bench.c b/tools/testing/selftests/bpf/progs/xdp_lb_bench.c
> > new file mode 100644
> > index 000000000000..b9fd848c035d
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/xdp_lb_bench.c
> [ ... ]
> > +SEC("xdp")
> > +int xdp_lb_bench(struct xdp_md *xdp)
> > +{
> [ ... ]
> > + if (cold_lru) {
> > + __u32 *saddr = data + saddr_off;
> > +
> > + batch_gen++;
> > + batch_hash = (batch_gen ^ bpf_get_smp_processor_id()) * KNUTH_HASH_MULT;
> > + if ((void *)(saddr + 1) <= data_end)
> > + *saddr ^= batch_hash;
> > + }
> > +
> > + return BENCH_BPF_LOOP(
> > + process_packet(xdp),
> > + ({
> > + if (__bench_result == XDP_TX) {
> > + if (strip_encap(xdp, &saved_eth))
> > + return XDP_DROP;
> > + if (rand_state)
> > + randomize_src(xdp, saddr_off, &rand_state);
> > + }
> > + if (cold_lru) {
> > + void *d = (void *)(long)xdp->data;
> > + void *de = (void *)(long)xdp->data_end;
> > + __u32 *__sa = d + saddr_off;
> > +
> > + if ((void *)(__sa + 1) <= de)
> > + *__sa ^= batch_hash;
>
> Does this code toggle the source IP back and forth on alternating iterations?
>
> Because batch_hash is computed outside the loop and remains constant, XORing
> the source IP on every reset block will cause it to alternate between two
> values.
>
> Won't this allow the LRU cache to easily cache both addresses, leading to a
> near 100% cache hit rate and defeating the purpose of measuring cold_lru cache
> misses?

All cold_lru scenarios also have flow_mask set, which means
randomize_src() runs in the reset block on every iteration, producing
a different source IP each time via an xorshift PRNG. The cold_lru XOR
is an additional per-batch perturbation, not the sole source of flow
diversity. With flow_mask = 0xFFFFFF (16M flow space) and a 100K LRU,
most iterations genuinely miss.