Re: [PATCH bpf-next 1/2] bpf: selftests: get rid of CHECK macro in xdp_adjust_tail.c

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2448 bytes --]

> On Wed, Jan 19, 2022 at 8:58 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> >
> > Rely on ASSERT* macros and get rid of deprecated CHECK ones in
> > xdp_adjust_tail bpf selftest.
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> >  .../bpf/prog_tests/xdp_adjust_tail.c          | 62 +++++++------------
> >  1 file changed, 23 insertions(+), 39 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c
> > index 3f5a17c38be5..dffa21b35503 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c
> > @@ -11,22 +11,19 @@ static void test_xdp_adjust_tail_shrink(void)
> >         char buf[128];
> >
> >         err = bpf_prog_test_load(file, BPF_PROG_TYPE_XDP, &obj, &prog_fd);
> > -       if (CHECK_FAIL(err))
> > +       if (ASSERT_OK(err, "test_xdp_adjust_tail_shrink"))
> >                 return;
> >
> >         err = bpf_prog_test_run(prog_fd, 1, &pkt_v4, sizeof(pkt_v4),
> >                                 buf, &size, &retval, &duration);
> >
> > -       CHECK(err || retval != XDP_DROP,
> > -             "ipv4", "err %d errno %d retval %d size %d\n",
> > -             err, errno, retval, size);
> > +       ASSERT_OK(err || retval != XDP_DROP, "ipv4");
> 
> it's better to do such checks as two ASSERTS: ASSERT_OK(err) and
> ASSERT_EQ(retval, XDP_DROP). It will give much more meaningful error
> messages and I think is easier to follow.

ack, I will fix it in v2.

> 
> >
> >         expect_sz = sizeof(pkt_v6) - 20;  /* Test shrink with 20 bytes */
> >         err = bpf_prog_test_run(prog_fd, 1, &pkt_v6, sizeof(pkt_v6),
> >                                 buf, &size, &retval, &duration);
> > -       CHECK(err || retval != XDP_TX || size != expect_sz,
> > -             "ipv6", "err %d errno %d retval %d size %d expect-size %d\n",
> > -             err, errno, retval, size, expect_sz);
> > +       ASSERT_OK(err || retval != XDP_TX || size != expect_sz, "ipv6");
> 
> same as above, old CHECK printed all those values so it was ok-ish to
> combine checks. With ASSERT_XXX() let's do each error condition check
> separately. Same for all the rest below.

ack, I will fix it in v2.

Regards,
Lorenzo

> 
> > +
> >         bpf_object__close(obj);
> >  }
> >
> 
> [...]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]