[PATCH bpf-next 1/2] selftests/bpf: use raw_tp program for atomic test

[Andrii Nakryiko]

On Fri, Jan 21, 2022 at 5:57 AM Hou Tao <houtao1 at huawei.com> wrote:
>
> Now atomic tests will attach fentry program and run it through
> bpf_prog_test_run(), but attaching fentry program depends on bpf
> trampoline which is only available under x86-64. Considering many
> archs have atomic support, using raw_tp program instead.
>
> Signed-off-by: Hou Tao <houtao1 at huawei.com>
> ---

Nits about using generic ASSERT_TRUE instead of dedicated ASSERT_OK
checks, but otherwise LGTM

Acked-by: Andrii Nakryiko <andrii at kernel.org>


>  .../selftests/bpf/prog_tests/atomics.c        | 114 +++++-------------
>  tools/testing/selftests/bpf/progs/atomics.c   |  29 ++---
>  2 files changed, 44 insertions(+), 99 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/atomics.c b/tools/testing/selftests/bpf/prog_tests/atomics.c
> index 86b7d5d84eec..0de292c1ec02 100644
> --- a/tools/testing/selftests/bpf/prog_tests/atomics.c
> +++ b/tools/testing/selftests/bpf/prog_tests/atomics.c
> @@ -8,18 +8,13 @@ static void test_add(struct atomics_lskel *skel)
>  {
>         int err, prog_fd;
>         __u32 duration = 0, retval;
> -       int link_fd;
> -
> -       link_fd = atomics_lskel__add__attach(skel);
> -       if (!ASSERT_GT(link_fd, 0, "attach(add)"))
> -               return;
>
> +       /* No need to attach it, just run it directly */
>         prog_fd = skel->progs.add.prog_fd;
> -       err = bpf_prog_test_run(prog_fd, 1, NULL, 0,
> +       err = bpf_prog_test_run(prog_fd, 0, NULL, 0,
>                                 NULL, NULL, &retval, &duration);
> -       if (CHECK(err || retval, "test_run add",
> -                 "err %d errno %d retval %d duration %d\n", err, errno, retval, duration))
> -               goto cleanup;
> +       if (!ASSERT_TRUE(!err && !retval, "test_run add"))

please do this as two separate asserts: ASSERT_OK(err) and ASSERT_OK(retval)

> +               return;
>
>         ASSERT_EQ(skel->data->add64_value, 3, "add64_value");
>         ASSERT_EQ(skel->bss->add64_result, 1, "add64_result");
> @@ -31,28 +26,19 @@ static void test_add(struct atomics_lskel *skel)
>         ASSERT_EQ(skel->bss->add_stack_result, 1, "add_stack_result");
>
>         ASSERT_EQ(skel->data->add_noreturn_value, 3, "add_noreturn_value");
> -
> -cleanup:
> -       close(link_fd);
>  }
>
>  static void test_sub(struct atomics_lskel *skel)
>  {
>         int err, prog_fd;
>         __u32 duration = 0, retval;
> -       int link_fd;
> -
> -       link_fd = atomics_lskel__sub__attach(skel);
> -       if (!ASSERT_GT(link_fd, 0, "attach(sub)"))
> -               return;
>
> +       /* No need to attach it, just run it directly */
>         prog_fd = skel->progs.sub.prog_fd;
> -       err = bpf_prog_test_run(prog_fd, 1, NULL, 0,
> +       err = bpf_prog_test_run(prog_fd, 0, NULL, 0,
>                                 NULL, NULL, &retval, &duration);
> -       if (CHECK(err || retval, "test_run sub",
> -                 "err %d errno %d retval %d duration %d\n",
> -                 err, errno, retval, duration))
> -               goto cleanup;
> +       if (!ASSERT_TRUE(!err && !retval, "test_run sub"))

same as above, same below for all the CHECKs replaced with ASSERT_TRUE

> +               return;
>
>         ASSERT_EQ(skel->data->sub64_value, -1, "sub64_value");
>         ASSERT_EQ(skel->bss->sub64_result, 1, "sub64_result");
> @@ -64,27 +50,19 @@ static void test_sub(struct atomics_lskel *skel)
>         ASSERT_EQ(skel->bss->sub_stack_result, 1, "sub_stack_result");
>
>         ASSERT_EQ(skel->data->sub_noreturn_value, -1, "sub_noreturn_value");
> -
> -cleanup:
> -       close(link_fd);
>  }
>

[...]