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

[PATCH bpf-next v3] 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 1000 times 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.

enable_stats run 100 times with 32 processes calling dd with bs=1 in
the background, on a bpf-next kernel with KASAN enabled, x86_64 VM with
32 vCPUs, before and after this patch:

            before                                after
  outcome   3 of 100 failed (check_run_cnt_valid)   0 of 100 failed

Fixes: 31a9f7fe9337 ("bpf: Add selftest for BPF_ENABLE_STATS")
Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>
---
V2 -> V3:
- Run the program 1000 times and keep getting the info as in v1, as the
  loop of v2 never ends if the kernel counts the runs but not their
  time (Alexei)
- Add before/after data

V1 -> V2:
- Run the program until its run time shows, as a single run can be
  shorter than the resolution of the clock (Sashiko)

v2: https://lore.kernel.org/r/20261009-selftests-bpf-enable-stats-inflight-v2-1-691ee379e511@gmail.com
v1: https://lore.kernel.org/r/20261008-selftests-bpf-enable-stats-inflight-v1-1-0cb92ee2c6d3@gmail.com
---
 tools/testing/selftests/bpf/prog_tests/enable_stats.c | 15 +++++++++------
 tools/testing/selftests/bpf/progs/test_enable_stats.c |  2 +-
 2 files changed, 10 insertions(+), 7 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..5b9cc119d072d 100644
--- a/tools/testing/selftests/bpf/prog_tests/enable_stats.c
+++ b/tools/testing/selftests/bpf/prog_tests/enable_stats.c
@@ -5,10 +5,11 @@
 void test_enable_stats(void)
 {
 	struct test_enable_stats *skel;
-	int stats_fd, err, prog_fd;
+	int stats_fd, err, prog_fd, i;
 	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,13 +21,15 @@ void test_enable_stats(void)
 		return;
 	}
 
-	err = test_enable_stats__attach(skel);
-	if (CHECK(err, "attach_raw_tp", "err %d\n", err))
+	prog_fd = bpf_program__fd(skel->progs.test_enable_stats);
+	for (i = 0; i < 1000; i++) {
+		err = bpf_prog_test_run_opts(prog_fd, &topts);
+		if (err)
+			break;
+	}
+	if (CHECK(err, "test_run", "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",
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 v3] 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] selftests/bpf: deprecated CHECK() macro used in test_enable_stats()
--

commit 149e144806feffc72e780046d3d7759e157ee137
Author: Qiliang Yuan <odys.yuan@gmail.com>

selftests/bpf: Run the enable_stats program with BPF_PROG_TEST_RUN

This commit replaces attaching the enable_stats BPF program to sys_enter
with running it 1000 times via BPF_PROG_TEST_RUN so that concurrent
syscalls on other CPUs do not race with reading run_cnt and count.

> diff --git a/tools/testing/selftests/bpf/prog_tests/enable_stats.c b/tools/testing/selftests/bpf/prog_tests/enable_stats.c
> index 75f85d0fe74a4..5b9cc119d072d 100644
> --- a/tools/testing/selftests/bpf/prog_tests/enable_stats.c
> +++ b/tools/testing/selftests/bpf/prog_tests/enable_stats.c
> @@ -5,10 +5,11 @@
>  void test_enable_stats(void)
>  {
>  	struct test_enable_stats *skel;
> -	int stats_fd, err, prog_fd;
> +	int stats_fd, err, prog_fd, i;
>  	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,13 +21,15 @@ void test_enable_stats(void)
>  		return;
>  	}
>  
> -	err = test_enable_stats__attach(skel);
> -	if (CHECK(err, "attach_raw_tp", "err %d\n", err))
> +	prog_fd = bpf_program__fd(skel->progs.test_enable_stats);
> +	for (i = 0; i < 1000; i++) {
> +		err = bpf_prog_test_run_opts(prog_fd, &topts);
> +		if (err)
> +			break;
> +	}
> +	if (CHECK(err, "test_run", "err %d\n", err))
>  		goto cleanup;

[Severity: Low]
Since CHECK() is deprecated in BPF selftests, should this new check in
test_enable_stats() (and perhaps the surrounding checks in this function)
use ASSERT_OK(err, "test_run") instead?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019-selftests-bpf-enable-stats-inflight-v3-1-e7728f5b061a@gmail.com?part=1