On Fri, Jan 21, 2022 at 5:57 AM Hou Tao <houtao1@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
Nits about using generic ASSERT_TRUE instead of dedicated ASSERT_OK
checks, but otherwise LGTM
Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> .../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);
> }
>
[...]
