Skip to content

selftests/bpf: Fix zero batch_hash on CPU 0 after batch_gen wraparound#12141

Open
kernel-patches-daemon-bpf[bot] wants to merge 1 commit into
bpf-next_basefrom
series/1098273=>bpf-next
Open

selftests/bpf: Fix zero batch_hash on CPU 0 after batch_gen wraparound#12141
kernel-patches-daemon-bpf[bot] wants to merge 1 commit into
bpf-next_basefrom
series/1098273=>bpf-next

Conversation

@kernel-patches-daemon-bpf
Copy link
Copy Markdown

Pull request for series with
subject: selftests/bpf: Fix zero batch_hash on CPU 0 after batch_gen wraparound
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1098273

@kernel-patches-daemon-bpf
Copy link
Copy Markdown
Author

Upstream branch: 3db0419
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1098273
version: 1

@kernel-patches-review-bot
Copy link
Copy Markdown

commit 60c59313ab10927c9bcddae4c0f323338b43cc60
Author: Muhammad Bilal <meatuni001@gmail.com>

selftests/bpf: Fix zero batch_hash on CPU 0 after batch_gen wraparound

This commit addresses an issue where batch_hash becomes zero after
u32 wraparound on CPU 0 by setting bit 0 before multiplication.

Fixes: fa747e9f843b ("selftests/bpf: Fix cold_lru producing zero batch_hash in XDP LB benchmark")

> diff --git a/tools/testing/selftests/bpf/progs/xdp_lb_bench.c b/tools/testing/selftests/bpf/progs/xdp_lb_bench.c
> index 13777b3dcac8..816880c1cd5e 100644
> --- a/tools/testing/selftests/bpf/progs/xdp_lb_bench.c
> +++ b/tools/testing/selftests/bpf/progs/xdp_lb_bench.c
> @@ -618,7 +618,7 @@ int xdp_lb_bench(struct xdp_md *xdp)
>  		__u32 *saddr = data + saddr_off;
>
>  		batch_gen++;
> -		batch_hash = (batch_gen + bpf_get_smp_processor_id()) * KNUTH_HASH_MULT;
> +		batch_hash = ((batch_gen + bpf_get_smp_processor_id()) | 1) * KNUTH_HASH_MULT;
                         ^^^^

Does forcing the LSB to 1 with '| 1' inadvertently destroy the lowest
bit of entropy from the batch generator?

Since batch_gen increments sequentially by 1 per batch execution,
forcing the least significant bit to 1 maps pairs of adjacent values
to the same number.

For example, if the CPU ID evaluates to 0, both batch_gen = 2 and
batch_gen = 3 will yield 3 before the multiplication:

    batch_gen = 2, CPU = 0: (2 + 0) | 1 = 3
    batch_gen = 3, CPU = 0: (3 + 0) | 1 = 3

Could this cause 50% of consecutive batches to generate the exact same
batch_hash as the previous batch, causing those iterations to hit the
warm LRU cache entries that were just populated and defeating the
purpose of the cold_lru benchmark?

This concern was raised in Puranjay's v1 patch where he used XOR instead
of addition, and he acknowledged the issue and switched to plain addition
in v2. This patch reintroduces the same '| 1' approach with addition, but
the entropy-loss problem remains.

Referenced discussion:
https://lore.kernel.org/bpf/2afdea40f0c5a11c2337efafe6f460899d85bc28650e04d5e0b9ceb35f3ad9fc@mail.kernel.org/

>  		if ((void *)(saddr + 1) <= data_end)
>  			*saddr ^= batch_hash;
>  	}



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

In-Reply-To-Subject: selftests/bpf: Fix zero batch_hash on CPU 0 after batch_gen wraparound
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26188863722

@kernel-patches-daemon-bpf
Copy link
Copy Markdown
Author

@kernel-patches-daemon-bpf
Copy link
Copy Markdown
Author

Upstream branch: b1fcdf9
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1098273
version: 1

commit fa747e9 ("selftests/bpf: Fix cold_lru producing zero
batch_hash in XDP LB benchmark") claims the addition ensures the
multiplier input is "always >= 1". This invariant does not hold after
wraparound.

batch_gen is __u32. After 2^32 increments it wraps to 0. On CPU 0,
bpf_get_smp_processor_id() returns 0:

    batch_gen = 0  (after u32 wraparound)
    batch_hash = (0 + 0) * KNUTH_HASH_MULT = 0
    *saddr ^= 0  ->  no-op, cold_lru miss counter stays 0

Setting bit 0 before multiplying guarantees a non-zero odd result for
all possible values of batch_gen and cpu_id, including after wraparound:

    (any_value | 1) >= 1  always, since bit 0 is always set

Fixes: fa747e9 ("selftests/bpf: Fix cold_lru producing zero batch_hash in XDP LB benchmark")
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants