Re: [PATCH bpf-next 2/2 v3] selftests/bpf: Test LINK_DETACH for perf link

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `test_perf_link_detach` test verification is brittle and likely to falsely pass, as a single call to `burn_cpu()` is too short to guarantee a perf event tick.
--

commit 80a1776d4374467cf1d037c1e22a1252e27c885c
Author: Florian Lehner <dev@der-flo.net>

selftests/bpf: Test LINK_DETACH for perf link

This commit adds a new test, test_perf_link_detach(), to verify the
BPF_LINK_DETACH syscall path for software perf events. It creates a link,
verifies execution, detaches it, and checks that the program is no longer
invoked.

> diff --git a/tools/testing/selftests/bpf/prog_tests/perf_link.c b/tools/testing/selftests/bpf/prog_tests/perf_link.c
> index 9e3a0d217af8..b75112c1b67d 100644
> --- a/tools/testing/selftests/bpf/prog_tests/perf_link.c
> +++ b/tools/testing/selftests/bpf/prog_tests/perf_link.c

[ ... ]

> +void test_perf_link_detach(void)
> +{
> +	struct test_perf_link *skel = NULL;
> +	int pfd = -1, link_fd = -1, err;
> +	int run_cnt_before, run_cnt_after;
> +	__u64 timeout_time_ns;

[ ... ]

> +	/* detach via BPF_LINK_DETACH - BPF program should no longer be executed */
> +	err = bpf_link_detach(link_fd);
> +	if (!ASSERT_OK(err, "link_detach"))
> +		goto cleanup;
> +
> +	/* make sure there are no stragglers */
> +	kern_sync_rcu();
> +
> +	run_cnt_before = skel->bss->run_cnt;
> +	burn_cpu();
> +	run_cnt_after = skel->bss->run_cnt;
> +
> +	ASSERT_EQ(run_cnt_before, run_cnt_after, "run_cnt_detached");

Is a single call to burn_cpu() sufficient to guarantee a perf event tick here?

The software CPU clock perf event is configured in perf_link_setup() to sample
at 1000 Hz, meaning it triggers approximately once every 1 millisecond of CPU
time.

The burn_cpu() function executes a fixed loop of 1,000,000 iterations. On
modern CPUs, this loop often takes significantly less than 1 millisecond to
complete.

Because a single burn_cpu() invocation takes less than the 1ms interval, is
it possible that no timer interrupt occurs during this window?

If so, run_cnt_after would equal run_cnt_before even if the detach operation
failed and the program was still attached, causing the test to falsely pass.
Could this be changed to use a time-based loop (similar to the initial
execution check) for several milliseconds to ensure correctness?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260501160901.224134-1-dev@der-flo.net?part=2