Re: [PATCH bpf-next v12 07/10] selftests/bpf: Add tests to verify global percpu data

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The test `test_global_percpu_data` fails because `bpf_map__set_value_size()` clears the `.percpu` map's BTF type ID when it encounters a non-array as the last variable in the datasec.
--

--- Patch [7]: [PATCH bpf-next v12 07/10] selftests/bpf: Add tests to verify global percpu data ---
commit 0224e1ae61dc47271ec8a72ac29b073935a25d31
Author: Leon Hwang <[email protected]>

selftests/bpf: Add tests to verify global percpu data

This patch adds a selftest to verify global percpu data map initialization
and modification using APIs like bpf_map__set_value_size() and
bpf_map_lookup_elem_flags(). It includes both light and standard skeleton
tests.

> 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..06d163a022dcc 100644
> --- a/tools/testing/selftests/bpf/prog_tests/global_data_init.c
> +++ b/tools/testing/selftests/bpf/prog_tests/global_data_init.c
[ ... ]
> +static void test_global_percpu_data_init(void)
> +{
[ ... ]
> +	err = bpf_map__set_value_size(map, desired_sz);
> +	if (!ASSERT_OK(err, "bpf_map__set_value_size"))
> +		goto out;
> +	if (!ASSERT_EQ(bpf_map__value_size(map), desired_sz, "percpu value size"))
> +		goto out;
> +	if (!ASSERT_NEQ(bpf_map__btf_value_type_id(map), 0, "percpu BTF value type"))
> +		goto out;

[Severity: Medium]
Will this test fail here because bpf_map__set_value_size() clears the BTF
type ID? 

When bpf_map__set_value_size() is called, it attempts to resize the BTF
datasec. This requires the last variable in the datasec to be an array.
Because `struct_data` is defined last in the `.percpu` section in the BPF
program, map_btf_datasec_resize() returns -EINVAL and clears the BTF type
ID to 0, causing this assertion to fail.

> diff --git a/tools/testing/selftests/bpf/progs/test_global_percpu_data.c b/tools/testing/selftests/bpf/progs/test_global_percpu_data.c
> new file mode 100644
> index 0000000000000..416841cd35692
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_global_percpu_data.c
[ ... ]
> +int nums[7] SEC(".percpu");
> +bool set SEC(".percpu") = false;
> +struct {
> +	char set;
> +	int i;
> +	int nums[7];
> +} struct_data SEC(".percpu") = {
> +	.set = 0,
> +	.i = -1,
> +};

[Severity: Medium]
Does placing this struct at the end of the `.percpu` section break the
BTF resizing logic?

Could the variables in the `.percpu` section be reordered so that an array
like `nums` or `cpu_id` is the last variable?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7