[PATCH bpf-next v2] selftests/bpf: Run the enable_stats program with BPF_PROG_TEST_RUN

[PATCH bpf-next v2] selftests/bpf: Run the enable_stats program with BPF_PROG_TEST_RUN

[Qiliang Yuan]

enable_stats attaches its program to sys_enter, so every syscall of the
system runs it. Once the program is detached, the test compares its
run_cnt to the number of runs the program counted itself.

Detaching doesn't wait for the runs already started on other CPUs, and
each of them increments count before run_cnt. With other tests making
syscalls in parallel, the test reads run_cnt one run behind count now
and then:

  test_enable_stats:FAIL:check_run_cnt_valid invalid run_cnt stats

Run the program with BPF_PROG_TEST_RUN instead of attaching it, so that
nothing else runs it and both counts are final when the test reads
them. As a single run can be shorter than the resolution of the clock,
run it until its run time shows, unless its runs aren't counted at all.

Fixes: 31a9f7fe9337 ("bpf: Add selftest for BPF_ENABLE_STATS")
Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>
---
V1 -> V2:
- Run the program until its run time shows, as a single run can be
  shorter than the resolution of the clock (Sashiko)

v1: https://lore.kernel.org/r/20261008-selftests-bpf-enable-stats-inflight-v1-1-0cb92ee2c6d3@gmail.com
---
 .../selftests/bpf/prog_tests/enable_stats.c        | 26 +++++++++++++---------
 .../selftests/bpf/progs/test_enable_stats.c        |  2 +-
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/enable_stats.c b/tools/testing/selftests/bpf/prog_tests/enable_stats.c
index 75f85d0fe74a4..691a82a4d6d20 100644
--- a/tools/testing/selftests/bpf/prog_tests/enable_stats.c
+++ b/tools/testing/selftests/bpf/prog_tests/enable_stats.c
@@ -9,6 +9,7 @@ void test_enable_stats(void)
 	struct bpf_prog_info info;
 	__u32 info_len = sizeof(info);
 	int duration = 0;
+	LIBBPF_OPTS(bpf_test_run_opts, topts);
 
 	skel = test_enable_stats__open_and_load();
 	if (CHECK(!skel, "skel_open_and_load", "skeleton open/load failed\n"))
@@ -20,18 +21,23 @@ void test_enable_stats(void)
 		return;
 	}
 
-	err = test_enable_stats__attach(skel);
-	if (CHECK(err, "attach_raw_tp", "err %d\n", err))
-		goto cleanup;
-
-	test_enable_stats__detach(skel);
-
 	prog_fd = bpf_program__fd(skel->progs.test_enable_stats);
-	memset(&info, 0, info_len);
-	err = bpf_prog_get_info_by_fd(prog_fd, &info, &info_len);
-	if (CHECK(err, "get_prog_info",
-		  "failed to get bpf_prog_info for fd %d\n", prog_fd))
+
+	/*
+	 * A single run can be too short for the clock to tell. Run the program
+	 * until its run time shows, unless its runs aren't counted at all.
+	 */
+	do {
+		err = bpf_prog_test_run_opts(prog_fd, &topts);
+		if (err)
+			break;
+
+		memset(&info, 0, info_len);
+		err = bpf_prog_get_info_by_fd(prog_fd, &info, &info_len);
+	} while (!err && info.run_cnt && !info.run_time_ns);
+	if (CHECK(err, "run_and_get_prog_info", "err %d\n", err))
 		goto cleanup;
+
 	if (CHECK(info.run_time_ns == 0, "check_stats_enabled",
 		  "failed to enable run_time_ns stats\n"))
 		goto cleanup;
diff --git a/tools/testing/selftests/bpf/progs/test_enable_stats.c b/tools/testing/selftests/bpf/progs/test_enable_stats.c
index 1705097d01d7b..e28d674fc77ac 100644
--- a/tools/testing/selftests/bpf/progs/test_enable_stats.c
+++ b/tools/testing/selftests/bpf/progs/test_enable_stats.c
@@ -10,7 +10,7 @@ char _license[] SEC("license") = "GPL";
 
 __u64 count = 0;
 
-SEC("raw_tracepoint/sys_enter")
+SEC("raw_tp")
 int test_enable_stats(void *ctx)
 {
 	__sync_fetch_and_add(&count, 1);

---
base-commit: e1d84a37cba984388988d2f1ddc84561413f0db2
change-id: 20261008-selftests-bpf-enable-stats-inflight-edfb36a280f3

Best regards,
-- 
Qiliang Yuan <odys.yuan@gmail.com>

Re: [PATCH bpf-next v2] selftests/bpf: Run the enable_stats program with BPF_PROG_TEST_RUN

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] bpf: selftests: use of deprecated CHECK macro instead of ASSERT_*
--

commit 2195ab7af2e709db01f39a1419b8eb0dd3b3c741
Author: Qiliang Yuan <odys.yuan@gmail.com>

selftests/bpf: Run the enable_stats program with BPF_PROG_TEST_RUN
    
This commit changes the test_enable_stats test to run its program with
BPF_PROG_TEST_RUN rather than attaching it to sys_enter. This ensures that
counts are final and avoids race conditions with parallel tests.

Link: https://lore.kernel.org/r/20261008-selftests-bpf-enable-stats-inflight-v1-1-0cb92ee2c6d3@gmail.com

> diff --git a/tools/testing/selftests/bpf/prog_tests/enable_stats.c b/tools/testing/selftests/bpf/prog_tests/enable_stats.c
> index 75f85d0fe74a4..691a82a4d6d20 100644
> --- a/tools/testing/selftests/bpf/prog_tests/enable_stats.c
> +++ b/tools/testing/selftests/bpf/prog_tests/enable_stats.c
[ ... ]
> @@ -20,18 +21,23 @@ void test_enable_stats(void)
[ ... ]
> +	do {
> +		err = bpf_prog_test_run_opts(prog_fd, &topts);
> +		if (err)
> +			break;
> +
> +		memset(&info, 0, info_len);
> +		err = bpf_prog_get_info_by_fd(prog_fd, &info, &info_len);
> +	} while (!err && info.run_cnt && !info.run_time_ns);
> +	if (CHECK(err, "run_and_get_prog_info", "err %d\n", err))

[Severity: Low]
Does this newly added error path use the deprecated CHECK() macro? The BPF
subsystem selftest guidelines prefer using modern assertions like ASSERT_OK()
for new code instead of CHECK().

>  		goto cleanup;

[Severity: Low]
This problem wasn't introduced by this patch, but while updating the
test_enable_stats() logic, would it be worth migrating the existing
CHECK(info.run_cnt != skel->bss->count, ...) to a modern assertion like
ASSERT_NEQ() to help reduce technical debt?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-selftests-bpf-enable-stats-inflight-v2-1-691ee379e511@gmail.com?part=1