On Thu, Apr 8, 2021 at 12:57 PM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote:
>
> Extend the fexit_bpf2bpf test to check that the info for the bpf_link
> returned by the kernel matches the expected values.
>
> While we're updating the test, change existing uses of CHEC() to use the
> much easier to read ASSERT_*() macros.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>
> ---
Just a minor nit below. Looks good, thanks.
Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> .../selftests/bpf/prog_tests/fexit_bpf2bpf.c | 50 +++++++++++++++----
> 1 file changed, 39 insertions(+), 11 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
> index 5c0448910426..019a46d8e98e 100644
> --- a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
> @@ -57,11 +57,13 @@ static void test_fexit_bpf2bpf_common(const char *obj_file,
> bool run_prog,
> test_cb cb)
> {
> + __u32 duration = 0, retval, tgt_prog_id, info_len;
if not CHECK() is used, duration shouldn't be needed anymore
> struct bpf_object *obj = NULL, *tgt_obj;
> + struct bpf_prog_info prog_info = {};
> struct bpf_program **prog = NULL;
> struct bpf_link **link = NULL;
> - __u32 duration = 0, retval;
> int err, tgt_fd, i;
> + struct btf *btf;
>
> err = bpf_prog_load(target_obj_file, BPF_PROG_TYPE_UNSPEC,
> &tgt_obj, &tgt_fd);
> @@ -72,28 +74,55 @@ static void test_fexit_bpf2bpf_common(const char *obj_file,
> .attach_prog_fd = tgt_fd,
> );
>
> + info_len = sizeof(prog_info);
> + err = bpf_obj_get_info_by_fd(tgt_fd, &prog_info, &info_len);
> + if (!ASSERT_OK(err, "tgt_fd_get_info"))
> + goto close_prog;
> +
> + tgt_prog_id = prog_info.id;
> + btf = bpf_object__btf(tgt_obj);
> +
> link = calloc(sizeof(struct bpf_link *), prog_cnt);
> prog = calloc(sizeof(struct bpf_program *), prog_cnt);
> - if (CHECK(!link || !prog, "alloc_memory", "failed to alloc memory"))
> + if (!ASSERT_OK_PTR(link, "link_ptr") || !ASSERT_OK_PTR(prog, "prog_ptr"))
nit: can you split them into two independent ifs now? Just one extra
`goto close_prog` is no big deal, but reads nicer
> goto close_prog;
>
> obj = bpf_object__open_file(obj_file, &opts);
> - if (CHECK(IS_ERR_OR_NULL(obj), "obj_open",
> - "failed to open %s: %ld\n", obj_file,
> - PTR_ERR(obj)))
> + if (!ASSERT_OK_PTR(obj, "obj_open"))
> goto close_prog;
>
> err = bpf_object__load(obj);
> - if (CHECK(err, "obj_load", "err %d\n", err))
> + if (!ASSERT_OK(err, "obj_load"))
> goto close_prog;
>
[...]
