Re: [PATCH] selftests/bpf: Add valid flag to bpf_cookie selftest's res

On Wed, Sep 4, 2024 at 9:10 PM Yuan Chen chenyuan_fl@163.com wrote:

From: Yuan Chen chenyuan@kylinos.cn

This patch identifies whether a test item is valid by adding a valid flag to res.

When we test the bpf_cookies/perf_event sub-test item of test_progs, there is a probability failure of the test item. In fact, this is not a problem, because the corresponding perf event is not collected. This should not output the test failure, and it is more reasonable to output SKIP. Therefore, add a valid identifier to res to distinguish whether the test item is valid, and skip the test item if it is invalid.

Signed-off-by: Yuan Chen chenyuan@kylinos.cn

.../testing/selftests/bpf/prog_tests/bpf_cookie.c | 15 +++++++++++++++ .../testing/selftests/bpf/progs/test_bpf_cookie.c | 2 ++ 2 files changed, 17 insertions(+)

I'm not following the proposal. We expect BPF program to fire, and if it fires, then we should get a valid BPF cookie value. If perf event didn't fire, then it's flakiness in the test setup, but adding this SKIP behavior for such a case is just papering over the real issue, no?

diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c index 070c52c312e5..e5bf4b385501 100644 --- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c +++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c @@ -456,6 +456,7 @@ static void pe_subtest(struct test_bpf_cookie *skel) if (!ASSERT_GE(pfd, 0, "perf_fd")) goto cleanup;

•   skel->bss->res_valid = false;
    opts.bpf_cookie = 0x100000;
    link = bpf_program__attach_perf_event_opts(skel->progs.handle_pe, pfd, &opts);
    if (!ASSERT_OK_PTR(link, "link1"))
  

@@ -463,6 +464,12 @@ static void pe_subtest(struct test_bpf_cookie *skel)

    burn_cpu(); /* trigger BPF prog */

•   if (!skel->bss->res_valid) {
  

•           printf("%s:SKIP:the corresponding perf event was not sampled.\n",
  

•                   __func__);
  

•           test__skip();
  

•           goto cleanup;
  

•   }
    ASSERT_EQ(skel->bss->pe_res, 0x100000, "pe_res1");
  
    /* prevent bpf_link__destroy() closing pfd itself */
  

@@ -474,6 +481,7 @@ static void pe_subtest(struct test_bpf_cookie *skel) link = NULL; kern_sync_rcu(); skel->bss->pe_res = 0;

•   skel->bss->res_valid = false;
  
    opts.bpf_cookie = 0x200000;
    link = bpf_program__attach_perf_event_opts(skel->progs.handle_pe, pfd, &opts);
  

@@ -482,6 +490,13 @@ static void pe_subtest(struct test_bpf_cookie *skel)

    burn_cpu(); /* trigger BPF prog */

•   if (!skel->bss->res_valid) {
  

•           printf("%s:SKIP:the corresponding perf event was not sampled.\n",
  

•                   __func__);
  

•           test__skip();
  

•           goto cleanup;
  

•   }
  

•   ASSERT_EQ(skel->bss->pe_res, 0x200000, "pe_res2");
  
  

cleanup: diff --git a/tools/testing/selftests/bpf/progs/test_bpf_cookie.c b/tools/testing/selftests/bpf/progs/test_bpf_cookie.c index c83142b55f47..28d0ae6810d9 100644 --- a/tools/testing/selftests/bpf/progs/test_bpf_cookie.c +++ b/tools/testing/selftests/bpf/progs/test_bpf_cookie.c @@ -7,6 +7,7 @@ #include <errno.h>

int my_tid; +bool res_valid;

__u64 kprobe_res; __u64 kprobe_multi_res; @@ -27,6 +28,7 @@ static void update(void *ctx, __u64 *res) if (my_tid != (u32)bpf_get_current_pid_tgid()) return;

•   res_valid = true;
    *res |= bpf_get_attach_cookie(ctx);
  

}

-- 2.46.0