> On 11/3/21 6:07 PM, Di Zhu wrote:
> > Add test for querying progs attached to sockmap. we use an existing
> > libbpf query interface to query prog cnt before and after progs
> > attaching to sockmap and check whether the queried prog id is right.
> >
> > Signed-off-by: Di Zhu <zhudi2@xxxxxxxxxx>
> > ---
> > .../selftests/bpf/prog_tests/sockmap_basic.c | 75 +++++++++++++++++++
> > .../bpf/progs/test_sockmap_progs_query.c | 24 ++++++
> > 2 files changed, 99 insertions(+)
> > create mode 100644
> tools/testing/selftests/bpf/progs/test_sockmap_progs_query.c
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> > index 1352ec104149..de8f91d91240 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> > @@ -8,6 +8,7 @@
> > #include "test_sockmap_update.skel.h"
> > #include "test_sockmap_invalid_update.skel.h"
> > #include "test_sockmap_skb_verdict_attach.skel.h"
> > +#include "test_sockmap_progs_query.skel.h"
> > #include "bpf_iter_sockmap.skel.h"
> >
> > #define TCP_REPAIR 19 /* TCP sock is under repair right now */
> > @@ -315,6 +316,74 @@ static void test_sockmap_skb_verdict_attach(enum
> bpf_attach_type first,
> > test_sockmap_skb_verdict_attach__destroy(skel);
> > }
> >
> > +static __u32 query_prog_id(int prog_fd)
> > +{
> > + struct bpf_prog_info info = {};
> > + __u32 info_len = sizeof(info);
> > + int err;
> > +
> > + err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> > + if (CHECK_FAIL(err || info_len != sizeof(info))) {
> > + perror("bpf_obj_get_info_by_fd");
>
> Please use ASSERT_* macros. These macros are defined in test_progs.h.
> We may have some old files still using CHECK* which are not converted
> to ASSERT* yet. But for new contributions, we would like to use
> ASSERT* from start. You can check other prog_tests/*.c files for
> examples.
>
> For the above example, you can do
> if (!ASSERT_OK(err, "bpf_obj_get_info_by_fd") ||
> !ASSERT_EQ(info_len, sizeof(info), "bpf_obj_get_info_by_fd"))
> return 0;
I refer to the implementation of the old codes, I will modify it
Thanks,
> > + return 0;
> > + }
> > +
> > + return info.id;
> [...]
> > + err = bpf_prog_query(map_fd, attach_type, 0 /* query flags */,
> > + &attach_flags, prog_ids, &prog_cnt);
> > + if (CHECK(err, "bpf_prog_query", "failed\n"))
> > + goto out;
>
> In this case, you can use
> if (!ASSERT_OK(err, "bpf_prog_query"))
> goto out;
>
> Please also change below other CHECK usages.
>
> > +
> > + if (CHECK(attach_flags != 0, "bpf_prog_query",
> > + "wrong attach_flags on query: %u", attach_flags))
> > + goto out;
> > +
> > + if (CHECK(prog_cnt != 0, "bpf_prog_query",
> > + "wrong program count on query: %u", prog_cnt))
> > + goto out;
> > +
> > + err = bpf_prog_attach(verdict_fd, map_fd, attach_type, 0);
> > + if (CHECK(err, "bpf_prog_attach", "failed\n"))
> > + goto out;
> > +
> > + prog_cnt = 1;
> > + err = bpf_prog_query(map_fd, attach_type, 0 /* query flags */,
> > + &attach_flags, prog_ids, &prog_cnt);
> > +
> > + CHECK(err, "bpf_prog_query", "failed\n");
> > + CHECK(attach_flags != 0, "bpf_prog_query attach_flags",
> > + "wrong attach_flags on query: %u", attach_flags);
> > + CHECK(prog_cnt != 1, "bpf_prog_query prog_cnt",
> > + "wrong program count on query: %u", prog_cnt);
> > + CHECK(prog_ids[0] != query_prog_id(verdict_fd), "bpf_prog_query",
> > + "wrong prog_ids on query: %u", prog_ids[0]);
> [...]
