Skip to content

Commit

Permalink
Merge branch 'selftests-bpf-update-multiple-prog_tests-to-use-assert_…
Browse files Browse the repository at this point in the history 
…-macros'

Yuran Pereira says:

====================
selftests/bpf: Update multiple prog_tests to use ASSERT_ macros

Multiple files/programs in `tools/testing/selftests/bpf/prog_tests/` still
heavily use the `CHECK` macro, even when better `ASSERT_` alternatives are
available.

As it was already pointed out by Yonghong Song [1] in the bpf selftests the use
of the ASSERT_* series of macros is preferred over the CHECK macro.

This patchset replaces the usage of `CHECK(` macros to the equivalent `ASSERT_`
family of macros in the following prog_tests:
- bind_perm.c
- bpf_obj_id.c
- bpf_tcp_ca.c
- vmlinux.c

[1] https://lore.kernel.org/lkml/0a142924-633c-44e6-9a92-2dc019656bf2@linux.dev

Changes in v3:
- Addressed the following points mentioned by Yonghong Song
- Improved `bpf_map_lookup_elem` assertion in bpf_tcp_ca.
- Replaced assertion introduced in v2 with one that checks `thread_ret`
  instead of `pthread_join`. This ensures that `server`'s return value
  (thread_ret) is the one being checked, as oposed to `pthread_join`'s
  return value, since the latter one is less likely to fail.

Changes in v2:
- Fixed pthread_join assertion that broke the previous test

Previous version:
v2 - https://lore.kernel.org/lkml/GV1PR10MB6563AECF8E94798A1E5B36A4E8B6A@GV1PR10MB6563.EURPRD10.PROD.OUTLOOK.COM
v1 - https://lore.kernel.org/lkml/GV1PR10MB6563FCFF1C5DEBE84FEA985FE8B0A@GV1PR10MB6563.EURPRD10.PROD.OUTLOOK.COM
====================

Link: https://lore.kernel.org/r/GV1PR10MB6563BEFEA4269E1DDBC264B1E8BBA@GV1PR10MB6563.EURPRD10.PROD.OUTLOOK.COM
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
  • Loading branch information
Andrii Nakryiko committed Nov 21, 2023
2 parents 57b97ec + 3ece0e8 commit 3cbbf91
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 169 deletions.
6 changes: 2 additions & 4 deletions tools/testing/selftests/bpf/prog_tests/bind_perm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
#include "cap_helpers.h"
#include "bind_perm.skel.h"

static int duration;

static int create_netns(void)
{
if (!ASSERT_OK(unshare(CLONE_NEWNET), "create netns"))
Expand All @@ -27,7 +25,7 @@ void try_bind(int family, int port, int expected_errno)
int fd = -1;

fd = socket(family, SOCK_STREAM, 0);
if (CHECK(fd < 0, "fd", "errno %d", errno))
if (!ASSERT_GE(fd, 0, "socket"))
goto close_socket;

if (family == AF_INET) {
Expand Down Expand Up @@ -60,7 +58,7 @@ void test_bind_perm(void)
return;

cgroup_fd = test__join_cgroup("/bind_perm");
if (CHECK(cgroup_fd < 0, "cg-join", "errno %d", errno))
if (!ASSERT_GE(cgroup_fd, 0, "test__join_cgroup"))
return;

skel = bind_perm__open_and_load();
Expand Down
204 changes: 73 additions & 131 deletions tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,24 +25,24 @@ void serial_test_bpf_obj_id(void)
*/
__u32 map_ids[nr_iters + 1];
char jited_insns[128], xlated_insns[128], zeros[128], tp_name[128];
__u32 i, next_id, info_len, nr_id_found, duration = 0;
__u32 i, next_id, info_len, nr_id_found;
struct timespec real_time_ts, boot_time_ts;
int err = 0;
__u64 array_value;
uid_t my_uid = getuid();
time_t now, load_time;

err = bpf_prog_get_fd_by_id(0);
CHECK(err >= 0 || errno != ENOENT,
"get-fd-by-notexist-prog-id", "err %d errno %d\n", err, errno);
ASSERT_LT(err, 0, "bpf_prog_get_fd_by_id");
ASSERT_EQ(errno, ENOENT, "bpf_prog_get_fd_by_id");

err = bpf_map_get_fd_by_id(0);
CHECK(err >= 0 || errno != ENOENT,
"get-fd-by-notexist-map-id", "err %d errno %d\n", err, errno);
ASSERT_LT(err, 0, "bpf_map_get_fd_by_id");
ASSERT_EQ(errno, ENOENT, "bpf_map_get_fd_by_id");

err = bpf_link_get_fd_by_id(0);
CHECK(err >= 0 || errno != ENOENT,
"get-fd-by-notexist-link-id", "err %d errno %d\n", err, errno);
ASSERT_LT(err, 0, "bpf_map_get_fd_by_id");
ASSERT_EQ(errno, ENOENT, "bpf_map_get_fd_by_id");

/* Check bpf_map_get_info_by_fd() */
bzero(zeros, sizeof(zeros));
Expand All @@ -53,25 +53,26 @@ void serial_test_bpf_obj_id(void)
/* test_obj_id.o is a dumb prog. It should never fail
* to load.
*/
if (CHECK_FAIL(err))
if (!ASSERT_OK(err, "bpf_prog_test_load"))
continue;

/* Insert a magic value to the map */
map_fds[i] = bpf_find_map(__func__, objs[i], "test_map_id");
if (CHECK_FAIL(map_fds[i] < 0))
if (!ASSERT_GE(map_fds[i], 0, "bpf_find_map"))
goto done;

err = bpf_map_update_elem(map_fds[i], &array_key,
&array_magic_value, 0);
if (CHECK_FAIL(err))
if (!ASSERT_OK(err, "bpf_map_update_elem"))
goto done;

prog = bpf_object__find_program_by_name(objs[i],
"test_obj_id");
if (CHECK_FAIL(!prog))
prog = bpf_object__find_program_by_name(objs[i], "test_obj_id");
if (!ASSERT_OK_PTR(prog, "bpf_object__find_program_by_name"))
goto done;

links[i] = bpf_program__attach(prog);
err = libbpf_get_error(links[i]);
if (CHECK(err, "prog_attach", "prog #%d, err %d\n", i, err)) {
if (!ASSERT_OK(err, "bpf_program__attach")) {
links[i] = NULL;
goto done;
}
Expand All @@ -81,24 +82,14 @@ void serial_test_bpf_obj_id(void)
bzero(&map_infos[i], info_len);
err = bpf_map_get_info_by_fd(map_fds[i], &map_infos[i],
&info_len);
if (CHECK(err ||
map_infos[i].type != BPF_MAP_TYPE_ARRAY ||
map_infos[i].key_size != sizeof(__u32) ||
map_infos[i].value_size != sizeof(__u64) ||
map_infos[i].max_entries != 1 ||
map_infos[i].map_flags != 0 ||
info_len != sizeof(struct bpf_map_info) ||
strcmp((char *)map_infos[i].name, expected_map_name),
"get-map-info(fd)",
"err %d errno %d type %d(%d) info_len %u(%zu) key_size %u value_size %u max_entries %u map_flags %X name %s(%s)\n",
err, errno,
map_infos[i].type, BPF_MAP_TYPE_ARRAY,
info_len, sizeof(struct bpf_map_info),
map_infos[i].key_size,
map_infos[i].value_size,
map_infos[i].max_entries,
map_infos[i].map_flags,
map_infos[i].name, expected_map_name))
if (!ASSERT_OK(err, "bpf_map_get_info_by_fd") ||
!ASSERT_EQ(map_infos[i].type, BPF_MAP_TYPE_ARRAY, "map_type") ||
!ASSERT_EQ(map_infos[i].key_size, sizeof(__u32), "key_size") ||
!ASSERT_EQ(map_infos[i].value_size, sizeof(__u64), "value_size") ||
!ASSERT_EQ(map_infos[i].max_entries, 1, "max_entries") ||
!ASSERT_EQ(map_infos[i].map_flags, 0, "map_flags") ||
!ASSERT_EQ(info_len, sizeof(struct bpf_map_info), "map_info_len") ||
!ASSERT_STREQ((char *)map_infos[i].name, expected_map_name, "map_name"))
goto done;

/* Check getting prog info */
Expand All @@ -112,48 +103,34 @@ void serial_test_bpf_obj_id(void)
prog_infos[i].xlated_prog_len = sizeof(xlated_insns);
prog_infos[i].map_ids = ptr_to_u64(map_ids + i);
prog_infos[i].nr_map_ids = 2;

err = clock_gettime(CLOCK_REALTIME, &real_time_ts);
if (CHECK_FAIL(err))
if (!ASSERT_OK(err, "clock_gettime"))
goto done;

err = clock_gettime(CLOCK_BOOTTIME, &boot_time_ts);
if (CHECK_FAIL(err))
if (!ASSERT_OK(err, "clock_gettime"))
goto done;

err = bpf_prog_get_info_by_fd(prog_fds[i], &prog_infos[i],
&info_len);
load_time = (real_time_ts.tv_sec - boot_time_ts.tv_sec)
+ (prog_infos[i].load_time / nsec_per_sec);
if (CHECK(err ||
prog_infos[i].type != BPF_PROG_TYPE_RAW_TRACEPOINT ||
info_len != sizeof(struct bpf_prog_info) ||
(env.jit_enabled && !prog_infos[i].jited_prog_len) ||
(env.jit_enabled &&
!memcmp(jited_insns, zeros, sizeof(zeros))) ||
!prog_infos[i].xlated_prog_len ||
!memcmp(xlated_insns, zeros, sizeof(zeros)) ||
load_time < now - 60 || load_time > now + 60 ||
prog_infos[i].created_by_uid != my_uid ||
prog_infos[i].nr_map_ids != 1 ||
*(int *)(long)prog_infos[i].map_ids != map_infos[i].id ||
strcmp((char *)prog_infos[i].name, expected_prog_name),
"get-prog-info(fd)",
"err %d errno %d i %d type %d(%d) info_len %u(%zu) "
"jit_enabled %d jited_prog_len %u xlated_prog_len %u "
"jited_prog %d xlated_prog %d load_time %lu(%lu) "
"uid %u(%u) nr_map_ids %u(%u) map_id %u(%u) "
"name %s(%s)\n",
err, errno, i,
prog_infos[i].type, BPF_PROG_TYPE_SOCKET_FILTER,
info_len, sizeof(struct bpf_prog_info),
env.jit_enabled,
prog_infos[i].jited_prog_len,
prog_infos[i].xlated_prog_len,
!!memcmp(jited_insns, zeros, sizeof(zeros)),
!!memcmp(xlated_insns, zeros, sizeof(zeros)),
load_time, now,
prog_infos[i].created_by_uid, my_uid,
prog_infos[i].nr_map_ids, 1,
*(int *)(long)prog_infos[i].map_ids, map_infos[i].id,
prog_infos[i].name, expected_prog_name))

if (!ASSERT_OK(err, "bpf_prog_get_info_by_fd") ||
!ASSERT_EQ(prog_infos[i].type, BPF_PROG_TYPE_RAW_TRACEPOINT, "prog_type") ||
!ASSERT_EQ(info_len, sizeof(struct bpf_prog_info), "prog_info_len") ||
!ASSERT_FALSE((env.jit_enabled && !prog_infos[i].jited_prog_len), "jited_prog_len") ||
!ASSERT_FALSE((env.jit_enabled && !memcmp(jited_insns, zeros, sizeof(zeros))),
"jited_insns") ||
!ASSERT_NEQ(prog_infos[i].xlated_prog_len, 0, "xlated_prog_len") ||
!ASSERT_NEQ(memcmp(xlated_insns, zeros, sizeof(zeros)), 0, "xlated_insns") ||
!ASSERT_GE(load_time, (now - 60), "load_time") ||
!ASSERT_LE(load_time, (now + 60), "load_time") ||
!ASSERT_EQ(prog_infos[i].created_by_uid, my_uid, "created_by_uid") ||
!ASSERT_EQ(prog_infos[i].nr_map_ids, 1, "nr_map_ids") ||
!ASSERT_EQ(*(int *)(long)prog_infos[i].map_ids, map_infos[i].id, "map_ids") ||
!ASSERT_STREQ((char *)prog_infos[i].name, expected_prog_name, "prog_name"))
goto done;

/* Check getting link info */
Expand All @@ -163,25 +140,12 @@ void serial_test_bpf_obj_id(void)
link_infos[i].raw_tracepoint.tp_name_len = sizeof(tp_name);
err = bpf_link_get_info_by_fd(bpf_link__fd(links[i]),
&link_infos[i], &info_len);
if (CHECK(err ||
link_infos[i].type != BPF_LINK_TYPE_RAW_TRACEPOINT ||
link_infos[i].prog_id != prog_infos[i].id ||
link_infos[i].raw_tracepoint.tp_name != ptr_to_u64(&tp_name) ||
strcmp(u64_to_ptr(link_infos[i].raw_tracepoint.tp_name),
"sys_enter") ||
info_len != sizeof(struct bpf_link_info),
"get-link-info(fd)",
"err %d errno %d info_len %u(%zu) type %d(%d) id %d "
"prog_id %d (%d) tp_name %s(%s)\n",
err, errno,
info_len, sizeof(struct bpf_link_info),
link_infos[i].type, BPF_LINK_TYPE_RAW_TRACEPOINT,
link_infos[i].id,
link_infos[i].prog_id, prog_infos[i].id,
(const char *)u64_to_ptr(link_infos[i].raw_tracepoint.tp_name),
"sys_enter"))
if (!ASSERT_OK(err, "bpf_link_get_info_by_fd") ||
!ASSERT_EQ(link_infos[i].type, BPF_LINK_TYPE_RAW_TRACEPOINT, "link_type") ||
!ASSERT_EQ(link_infos[i].prog_id, prog_infos[i].id, "prog_id") ||
!ASSERT_EQ(link_infos[i].raw_tracepoint.tp_name, ptr_to_u64(&tp_name), "&tp_name") ||
!ASSERT_STREQ(u64_to_ptr(link_infos[i].raw_tracepoint.tp_name), "sys_enter", "tp_name"))
goto done;

}

/* Check bpf_prog_get_next_id() */
Expand All @@ -190,17 +154,15 @@ void serial_test_bpf_obj_id(void)
while (!bpf_prog_get_next_id(next_id, &next_id)) {
struct bpf_prog_info prog_info = {};
__u32 saved_map_id;
int prog_fd;
int prog_fd, cmp_res;

info_len = sizeof(prog_info);

prog_fd = bpf_prog_get_fd_by_id(next_id);
if (prog_fd < 0 && errno == ENOENT)
/* The bpf_prog is in the dead row */
continue;
if (CHECK(prog_fd < 0, "get-prog-fd(next_id)",
"prog_fd %d next_id %d errno %d\n",
prog_fd, next_id, errno))
if (!ASSERT_GE(prog_fd, 0, "bpf_prog_get_fd_by_id"))
break;

for (i = 0; i < nr_iters; i++)
Expand All @@ -218,9 +180,8 @@ void serial_test_bpf_obj_id(void)
*/
prog_info.nr_map_ids = 1;
err = bpf_prog_get_info_by_fd(prog_fd, &prog_info, &info_len);
if (CHECK(!err || errno != EFAULT,
"get-prog-fd-bad-nr-map-ids", "err %d errno %d(%d)",
err, errno, EFAULT))
if (!ASSERT_ERR(err, "bpf_prog_get_info_by_fd") ||
!ASSERT_EQ(errno, EFAULT, "bpf_prog_get_info_by_fd"))
break;
bzero(&prog_info, sizeof(prog_info));
info_len = sizeof(prog_info);
Expand All @@ -231,37 +192,30 @@ void serial_test_bpf_obj_id(void)
err = bpf_prog_get_info_by_fd(prog_fd, &prog_info, &info_len);
prog_infos[i].jited_prog_insns = 0;
prog_infos[i].xlated_prog_insns = 0;
CHECK(err || info_len != sizeof(struct bpf_prog_info) ||
memcmp(&prog_info, &prog_infos[i], info_len) ||
*(int *)(long)prog_info.map_ids != saved_map_id,
"get-prog-info(next_id->fd)",
"err %d errno %d info_len %u(%zu) memcmp %d map_id %u(%u)\n",
err, errno, info_len, sizeof(struct bpf_prog_info),
memcmp(&prog_info, &prog_infos[i], info_len),
*(int *)(long)prog_info.map_ids, saved_map_id);
cmp_res = memcmp(&prog_info, &prog_infos[i], info_len);

ASSERT_OK(err, "bpf_prog_get_info_by_fd");
ASSERT_EQ(info_len, sizeof(struct bpf_prog_info), "prog_info_len");
ASSERT_OK(cmp_res, "memcmp");
ASSERT_EQ(*(int *)(long)prog_info.map_ids, saved_map_id, "map_id");
close(prog_fd);
}
CHECK(nr_id_found != nr_iters,
"check total prog id found by get_next_id",
"nr_id_found %u(%u)\n",
nr_id_found, nr_iters);
ASSERT_EQ(nr_id_found, nr_iters, "prog_nr_id_found");

/* Check bpf_map_get_next_id() */
nr_id_found = 0;
next_id = 0;
while (!bpf_map_get_next_id(next_id, &next_id)) {
struct bpf_map_info map_info = {};
int map_fd;
int map_fd, cmp_res;

info_len = sizeof(map_info);

map_fd = bpf_map_get_fd_by_id(next_id);
if (map_fd < 0 && errno == ENOENT)
/* The bpf_map is in the dead row */
continue;
if (CHECK(map_fd < 0, "get-map-fd(next_id)",
"map_fd %d next_id %u errno %d\n",
map_fd, next_id, errno))
if (!ASSERT_GE(map_fd, 0, "bpf_map_get_fd_by_id"))
break;

for (i = 0; i < nr_iters; i++)
Expand All @@ -274,25 +228,19 @@ void serial_test_bpf_obj_id(void)
nr_id_found++;

err = bpf_map_lookup_elem(map_fd, &array_key, &array_value);
if (CHECK_FAIL(err))
if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
goto done;

err = bpf_map_get_info_by_fd(map_fd, &map_info, &info_len);
CHECK(err || info_len != sizeof(struct bpf_map_info) ||
memcmp(&map_info, &map_infos[i], info_len) ||
array_value != array_magic_value,
"check get-map-info(next_id->fd)",
"err %d errno %d info_len %u(%zu) memcmp %d array_value %llu(%llu)\n",
err, errno, info_len, sizeof(struct bpf_map_info),
memcmp(&map_info, &map_infos[i], info_len),
array_value, array_magic_value);
cmp_res = memcmp(&map_info, &map_infos[i], info_len);
ASSERT_OK(err, "bpf_map_get_info_by_fd");
ASSERT_EQ(info_len, sizeof(struct bpf_map_info), "info_len");
ASSERT_OK(cmp_res, "memcmp");
ASSERT_EQ(array_value, array_magic_value, "array_value");

close(map_fd);
}
CHECK(nr_id_found != nr_iters,
"check total map id found by get_next_id",
"nr_id_found %u(%u)\n",
nr_id_found, nr_iters);
ASSERT_EQ(nr_id_found, nr_iters, "map_nr_id_found");

/* Check bpf_link_get_next_id() */
nr_id_found = 0;
Expand All @@ -308,9 +256,7 @@ void serial_test_bpf_obj_id(void)
if (link_fd < 0 && errno == ENOENT)
/* The bpf_link is in the dead row */
continue;
if (CHECK(link_fd < 0, "get-link-fd(next_id)",
"link_fd %d next_id %u errno %d\n",
link_fd, next_id, errno))
if (!ASSERT_GE(link_fd, 0, "bpf_link_get_fd_by_id"))
break;

for (i = 0; i < nr_iters; i++)
Expand All @@ -325,17 +271,13 @@ void serial_test_bpf_obj_id(void)
err = bpf_link_get_info_by_fd(link_fd, &link_info, &info_len);
cmp_res = memcmp(&link_info, &link_infos[i],
offsetof(struct bpf_link_info, raw_tracepoint));
CHECK(err || info_len != sizeof(link_info) || cmp_res,
"check get-link-info(next_id->fd)",
"err %d errno %d info_len %u(%zu) memcmp %d\n",
err, errno, info_len, sizeof(struct bpf_link_info),
cmp_res);
ASSERT_OK(err, "bpf_link_get_info_by_fd");
ASSERT_EQ(info_len, sizeof(link_info), "info_len");
ASSERT_OK(cmp_res, "memcmp");

close(link_fd);
}
CHECK(nr_id_found != nr_iters,
"check total link id found by get_next_id",
"nr_id_found %u(%u)\n", nr_id_found, nr_iters);
ASSERT_EQ(nr_id_found, nr_iters, "link_nr_id_found");

done:
for (i = 0; i < nr_iters; i++) {
Expand Down
Loading

0 comments on commit 3cbbf91

Please sign in to comment.