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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Sep 4, 2024 at 9:10 PM Yuan Chen <chenyuan_fl@xxxxxxx> wrote:
>
> From: Yuan Chen <chenyuan@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
>  .../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
>





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux