Re: [PATCH bpf-next v5 5/5] selftest/bpf: The test cses of BPF cookie for fentry/fexit/fmod_ret.

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Tue, Apr 12, 2022 at 9:56 AM Kui-Feng Lee <kuifeng@fb.com> wrote:
>
> Make sure BPF cookies are correct for fentry/fexit/fmod_ret.
>
> Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
> ---
>  .../selftests/bpf/prog_tests/bpf_cookie.c     | 52 +++++++++++++++++++
>  .../selftests/bpf/progs/test_bpf_cookie.c     | 24 +++++++++
>  2 files changed, 76 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> index 923a6139b2d8..7f05056c66d4 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> @@ -410,6 +410,56 @@ static void pe_subtest(struct test_bpf_cookie *skel)
>         bpf_link__destroy(link);
>  }
>
> +static void tracing_subtest(struct test_bpf_cookie *skel)
> +{
> +       __u64 cookie;
> +       int prog_fd;
> +       int fentry_fd = -1, fexit_fd = -1, fmod_ret_fd = -1;
> +

unnecessary empty line

> +       LIBBPF_OPTS(bpf_test_run_opts, opts, .repeat = 1);

.repeat = 1 is not necessary, I think, .repeat = 0 is equivalent to that

> +       LIBBPF_OPTS(bpf_link_create_opts, link_opts);
> +
> +       skel->bss->fentry_res = 0;
> +       skel->bss->fexit_res = 0;
> +
> +       cookie = 0x100000;

nit: make this value bigger to make sure higher 32 bits of u64 are
preserved properly. Maybe 0x1000000010000000 (and similarly with 2 and
3)

> +       prog_fd = bpf_program__fd(skel->progs.fentry_test1);
> +       link_opts.tracing.bpf_cookie = cookie;
> +       fentry_fd = bpf_link_create(prog_fd, 0, BPF_TRACE_FENTRY, &link_opts);
> +

ASSERT_GE?

> +       cookie = 0x200000;
> +       prog_fd = bpf_program__fd(skel->progs.fexit_test1);
> +       link_opts.tracing.bpf_cookie = cookie;
> +       fexit_fd = bpf_link_create(prog_fd, 0, BPF_TRACE_FEXIT, &link_opts);
> +       if (!ASSERT_GE(fexit_fd, 0, "fexit.open"))
> +               goto cleanup;
> +

[...]