Re: [PATCH bpf-next v5 6/9] selftests/bpf: Add tests to verify global percpu data

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unnecessary and incorrect roundup of `value_sz` for single-CPU map lookup makes test brittle.
--

commit b029e57f81ce93314bf885c351d6b7e8fe5bd177
Author: Leon Hwang <leon.hwang@linux.dev>

selftests/bpf: Add tests to verify global percpu data

This commit adds new test cases to verify global percpu data handling in the
BPF subsystem. It tests initialization, lookups, and direct data pointers
using both full and light skeletons.

> diff --git a/tools/testing/selftests/bpf/prog_tests/global_data_init.c b/tools/testing/selftests/bpf/prog_tests/global_data_init.c
> index 8466332d7406f..5abdb0b8b8b33 100644
> --- a/tools/testing/selftests/bpf/prog_tests/global_data_init.c
> +++ b/tools/testing/selftests/bpf/prog_tests/global_data_init.c

[ ... ]

> @@ -60,3 +63,196 @@ void test_global_data_init(void)
> +static void test_global_percpu_data_init(void)
> +{
> +	struct test_global_percpu_data__percpu *init_data, *data = NULL;
> +	struct test_global_percpu_data__percpu init_value = {};
> +	int key, prog_fd, err, num_cpus, num_online, i;
> +	struct test_global_percpu_data *skel = NULL;
> +	__u64 args[2] = {0x1234ULL, 0x5678ULL};
> +	size_t elem_sz, init_data_sz;
> +	struct bpf_map *map;
> +	bool *online;

[ ... ]

> +	elem_sz = roundup(sizeof(*data), 8);

[ ... ]

> +	/* run on every CPU */
> +	for (i = 0; i < num_online; i++) {
> +		__u64 flags;
> +
> +		if (!online[i])
> +			continue;
> +
> +		topts.cpu = i;
> +		topts.retval = 0;
> +		err = bpf_prog_test_run_opts(prog_fd, &topts);
> +		ASSERT_OK(err, "bpf_prog_test_run_opts");
> +		ASSERT_EQ(topts.retval, 0, "bpf_prog_test_run_opts retval");
> +
> +		flags = ((__u64) i << 32) | BPF_F_CPU;
> +		err = bpf_map__lookup_elem(map, &key, sizeof(key), data, elem_sz, flags);

[Severity: Medium]
When looking up a per-CPU map element for a specific CPU using the BPF_F_CPU
flag, libbpf's validate_map_op() strictly requires that value_sz exactly matches
map->def.value_size.

By rounding up sizeof(*data) to a multiple of 8 and passing it as elem_sz
here, doesn't this make the test brittle?

It currently passes because the underlying .percpu section happens to be
exactly 72 bytes (a multiple of 8). If the variables in the BPF program were
changed such that the section size was 76 bytes, the roundup would yield 80,
which wouldn't match map->def.value_size and would cause
bpf_map__lookup_elem() to fail with -EINVAL.

Could we use the un-rounded size here instead?

> +		if (!ASSERT_OK(err, "bpf_map__lookup_elem"))
> +			goto out;
> +
> +		ASSERT_EQ(data->data, 1, "data->data");

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608145113.65857-1-leon.hwang@linux.dev?part=6