Re: [PATCH bpf v6 3/3] selftests/bpf: add tests for percpu map flags combination

[Leon Hwang <leon.hwang@linux.dev>]

On 2026/10/8 20:07, Masoud Aghasi wrote:
> All possible combinations of (BPF_EXIST, BPF_NOEXIST) and
> (BPF_F_ALL_CPUS, BPF_F_CPU) flags are covered for all percpu map types.
> 
> As BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE does not support BPF_NOEXIST and
> also in order to reduce the amount of code duplicates, I added its
> new test scenarios to the existing cpu_flag_percpu_cgroup_storage test.
> 
> But for other percpu map types, I added new tests to be able to
> exercise all flag combinations and the edge cases such as percpu LRU
> unnecessary deletion issue.
> 
> Signed-off-by: Masoud Aghasi <maghasi@disroot.org>

Two concerns below, others lgtm.

Acked-by: Leon Hwang <leon.hwang@linux.dev>

> ---
>  .../selftests/bpf/prog_tests/percpu_alloc.c   | 254 ++++++++++++++++++
>  1 file changed, 254 insertions(+)
> 

[...]

> +
> +	if (map_type != BPF_MAP_TYPE_LRU_PERCPU_HASH)
> +		goto out;
> +
> +	/* Percpu LRU hash map should not delete old entries unnecessarily */
> +	flags = BPF_F_ALL_CPUS | BPF_NOEXIST;
> +	key = 2;
> +	err = bpf_map_update_elem(map_fd, &key, values, flags);
> +	if (!ASSERT_OK(err, "bpf_map_update_elem lru_percpu_hash all_cpus|noexist"))
> +		goto out;

Update values[0] before update_elem(), and verify_map_value_all_cpus()?

Also update values[0] before the below update_elem().

> +
> +	flags = BPF_F_ALL_CPUS | BPF_EXIST;
> +	err = bpf_map_update_elem(map_fd, &key, values, flags);
> +	if (!ASSERT_OK(err, "bpf_map_update_elem lru_percpu_hash all_cpus|exist"))
> +		goto out;
> +
> +	key = 0;

Why do you use 'k = 0' instead of '2'?

I think we should update and verify using the same key.

Thanks,
Leon


> +	verify_map_value_all_cpus(map_fd, &key, values, elem_sz, first_value, nr_cpus);
> +
> +out:
> +	close(map_fd);
> +	free(values);
> +}
> +

[...]