Re: [PATCH bpf-next] selftests/bpf: Drop the on_enter recursion check of task_local_storage

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] bpf: incomplete fix for flaky task_local_storage recursion test
--

commit 93ecb8817313103c1cdb556cb288695d3d3c939c
Author: Qiliang Yuan <odys.yuan@gmail.com>

selftests/bpf: Drop the on_enter recursion check of task_local_storage

This commit removes the recursion misses check for the on_enter program
in the task_local_storage test to fix test flakiness. It prevents failures
caused by preemption-induced spurious misses when other tasks trigger
sys_enter on the same CPU.

> diff --git a/tools/testing/selftests/bpf/prog_tests/task_local_storage.c b/tools/testing/selftests/bpf/prog_tests/task_local_storage.c
> index 5b2b56cc3a4f3..c03ea94325d9e 100644
> --- a/tools/testing/selftests/bpf/prog_tests/task_local_storage.c
> +++ b/tools/testing/selftests/bpf/prog_tests/task_local_storage.c
> @@ -138,12 +138,6 @@ static void test_recursion(void)
>  	ASSERT_OK(err, "get prog info");
>  	ASSERT_EQ(info.recursion_misses, 2, "on_update prog recursion");

[Severity: Low]
Is this exact-match assertion susceptible to the same preemption-induced
spurious misses that this patch aims to fix for on_enter?

The on_update program is an fentry program attached to the globally shared
bpf_local_storage_update function, which also executes in a preemptible
context (rcu_read_lock_dont_migrate()).

If a task is preempted while executing on_update, its prog->active recursion
counter remains elevated on that CPU.

If the preempting task (such as another selftest thread running in parallel)
calls bpf_local_storage_update, the fentry trampoline will check the elevated
recursion context, increment the program's global recursion_misses counter,
and abort before the test_pid filter inside the program is evaluated.

This spurious miss could randomly push info.recursion_misses above 2, leaving
the test flaky.

Would it be safer to relax this check to
ASSERT_GE(info.recursion_misses, 2, "on_update prog recursion")?

> -	prog_fd = bpf_program__fd(skel->progs.on_enter);
> -	memset(&info, 0, sizeof(info));
> -	err = bpf_prog_get_info_by_fd(prog_fd, &info, &info_len);
> -	ASSERT_OK(err, "get prog info");
> -	ASSERT_EQ(info.recursion_misses, 0, "on_enter prog recursion");
> -
>  out:
>  	close(task_fd);
>  	task_ls_recursion__destroy(skel);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-selftests-bpf-task-ls-recursion-v1-1-c3a3e3c41f8c@gmail.com?part=1