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

On Tue, Apr 28, 2026 at 2:05 AM <[email protected]> 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 <[email protected]>
>
> 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.