Re: [PATCH bpf-next v5 2/2] selftests: bpf: test BPF_PROG_QUERY for progs attached to sockmap

From: Yonghong Song
Date: Thu Nov 04 2021 - 01:52:06 EST




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;

+ 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]);
[...]