Commit 3cbbf919 authored by Andrii Nakryiko's avatar Andrii Nakryiko

Merge branch 'selftests-bpf-update-multiple-prog_tests-to-use-assert_-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.COMSigned-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
parents 57b97ecb 3ece0e85
...@@ -9,8 +9,6 @@ ...@@ -9,8 +9,6 @@
#include "cap_helpers.h" #include "cap_helpers.h"
#include "bind_perm.skel.h" #include "bind_perm.skel.h"
static int duration;
static int create_netns(void) static int create_netns(void)
{ {
if (!ASSERT_OK(unshare(CLONE_NEWNET), "create netns")) if (!ASSERT_OK(unshare(CLONE_NEWNET), "create netns"))
...@@ -27,7 +25,7 @@ void try_bind(int family, int port, int expected_errno) ...@@ -27,7 +25,7 @@ void try_bind(int family, int port, int expected_errno)
int fd = -1; int fd = -1;
fd = socket(family, SOCK_STREAM, 0); fd = socket(family, SOCK_STREAM, 0);
if (CHECK(fd < 0, "fd", "errno %d", errno)) if (!ASSERT_GE(fd, 0, "socket"))
goto close_socket; goto close_socket;
if (family == AF_INET) { if (family == AF_INET) {
...@@ -60,7 +58,7 @@ void test_bind_perm(void) ...@@ -60,7 +58,7 @@ void test_bind_perm(void)
return; return;
cgroup_fd = test__join_cgroup("/bind_perm"); 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; return;
skel = bind_perm__open_and_load(); skel = bind_perm__open_and_load();
......
...@@ -20,15 +20,14 @@ ...@@ -20,15 +20,14 @@
static const unsigned int total_bytes = 10 * 1024 * 1024; static const unsigned int total_bytes = 10 * 1024 * 1024;
static int expected_stg = 0xeB9F; static int expected_stg = 0xeB9F;
static int stop, duration; static int stop;
static int settcpca(int fd, const char *tcp_ca) static int settcpca(int fd, const char *tcp_ca)
{ {
int err; int err;
err = setsockopt(fd, IPPROTO_TCP, TCP_CONGESTION, tcp_ca, strlen(tcp_ca)); err = setsockopt(fd, IPPROTO_TCP, TCP_CONGESTION, tcp_ca, strlen(tcp_ca));
if (CHECK(err == -1, "setsockopt(fd, TCP_CONGESTION)", "errno:%d\n", if (!ASSERT_NEQ(err, -1, "setsockopt"))
errno))
return -1; return -1;
return 0; return 0;
...@@ -65,8 +64,7 @@ static void *server(void *arg) ...@@ -65,8 +64,7 @@ static void *server(void *arg)
bytes += nr_sent; bytes += nr_sent;
} }
CHECK(bytes != total_bytes, "send", "%zd != %u nr_sent:%zd errno:%d\n", ASSERT_EQ(bytes, total_bytes, "send");
bytes, total_bytes, nr_sent, errno);
done: done:
if (fd >= 0) if (fd >= 0)
...@@ -92,10 +90,11 @@ static void do_test(const char *tcp_ca, const struct bpf_map *sk_stg_map) ...@@ -92,10 +90,11 @@ static void do_test(const char *tcp_ca, const struct bpf_map *sk_stg_map)
WRITE_ONCE(stop, 0); WRITE_ONCE(stop, 0);
lfd = socket(AF_INET6, SOCK_STREAM, 0); lfd = socket(AF_INET6, SOCK_STREAM, 0);
if (CHECK(lfd == -1, "socket", "errno:%d\n", errno)) if (!ASSERT_NEQ(lfd, -1, "socket"))
return; return;
fd = socket(AF_INET6, SOCK_STREAM, 0); fd = socket(AF_INET6, SOCK_STREAM, 0);
if (CHECK(fd == -1, "socket", "errno:%d\n", errno)) { if (!ASSERT_NEQ(fd, -1, "socket")) {
close(lfd); close(lfd);
return; return;
} }
...@@ -108,26 +107,27 @@ static void do_test(const char *tcp_ca, const struct bpf_map *sk_stg_map) ...@@ -108,26 +107,27 @@ static void do_test(const char *tcp_ca, const struct bpf_map *sk_stg_map)
sa6.sin6_family = AF_INET6; sa6.sin6_family = AF_INET6;
sa6.sin6_addr = in6addr_loopback; sa6.sin6_addr = in6addr_loopback;
err = bind(lfd, (struct sockaddr *)&sa6, addrlen); err = bind(lfd, (struct sockaddr *)&sa6, addrlen);
if (CHECK(err == -1, "bind", "errno:%d\n", errno)) if (!ASSERT_NEQ(err, -1, "bind"))
goto done; goto done;
err = getsockname(lfd, (struct sockaddr *)&sa6, &addrlen); err = getsockname(lfd, (struct sockaddr *)&sa6, &addrlen);
if (CHECK(err == -1, "getsockname", "errno:%d\n", errno)) if (!ASSERT_NEQ(err, -1, "getsockname"))
goto done; goto done;
err = listen(lfd, 1); err = listen(lfd, 1);
if (CHECK(err == -1, "listen", "errno:%d\n", errno)) if (!ASSERT_NEQ(err, -1, "listen"))
goto done; goto done;
if (sk_stg_map) { if (sk_stg_map) {
err = bpf_map_update_elem(bpf_map__fd(sk_stg_map), &fd, err = bpf_map_update_elem(bpf_map__fd(sk_stg_map), &fd,
&expected_stg, BPF_NOEXIST); &expected_stg, BPF_NOEXIST);
if (CHECK(err, "bpf_map_update_elem(sk_stg_map)", if (!ASSERT_OK(err, "bpf_map_update_elem(sk_stg_map)"))
"err:%d errno:%d\n", err, errno))
goto done; goto done;
} }
/* connect to server */ /* connect to server */
err = connect(fd, (struct sockaddr *)&sa6, addrlen); err = connect(fd, (struct sockaddr *)&sa6, addrlen);
if (CHECK(err == -1, "connect", "errno:%d\n", errno)) if (!ASSERT_NEQ(err, -1, "connect"))
goto done; goto done;
if (sk_stg_map) { if (sk_stg_map) {
...@@ -135,14 +135,13 @@ static void do_test(const char *tcp_ca, const struct bpf_map *sk_stg_map) ...@@ -135,14 +135,13 @@ static void do_test(const char *tcp_ca, const struct bpf_map *sk_stg_map)
err = bpf_map_lookup_elem(bpf_map__fd(sk_stg_map), &fd, err = bpf_map_lookup_elem(bpf_map__fd(sk_stg_map), &fd,
&tmp_stg); &tmp_stg);
if (CHECK(!err || errno != ENOENT, if (!ASSERT_ERR(err, "bpf_map_lookup_elem(sk_stg_map)") ||
"bpf_map_lookup_elem(sk_stg_map)", !ASSERT_EQ(errno, ENOENT, "bpf_map_lookup_elem(sk_stg_map)"))
"err:%d errno:%d\n", err, errno))
goto done; goto done;
} }
err = pthread_create(&srv_thread, NULL, server, (void *)(long)lfd); err = pthread_create(&srv_thread, NULL, server, (void *)(long)lfd);
if (CHECK(err != 0, "pthread_create", "err:%d errno:%d\n", err, errno)) if (!ASSERT_OK(err, "pthread_create"))
goto done; goto done;
/* recv total_bytes */ /* recv total_bytes */
...@@ -156,13 +155,12 @@ static void do_test(const char *tcp_ca, const struct bpf_map *sk_stg_map) ...@@ -156,13 +155,12 @@ static void do_test(const char *tcp_ca, const struct bpf_map *sk_stg_map)
bytes += nr_recv; bytes += nr_recv;
} }
CHECK(bytes != total_bytes, "recv", "%zd != %u nr_recv:%zd errno:%d\n", ASSERT_EQ(bytes, total_bytes, "recv");
bytes, total_bytes, nr_recv, errno);
WRITE_ONCE(stop, 1); WRITE_ONCE(stop, 1);
pthread_join(srv_thread, &thread_ret); pthread_join(srv_thread, &thread_ret);
CHECK(IS_ERR(thread_ret), "pthread_join", "thread_ret:%ld", ASSERT_OK(IS_ERR(thread_ret), "thread_ret");
PTR_ERR(thread_ret));
done: done:
close(lfd); close(lfd);
close(fd); close(fd);
...@@ -174,7 +172,7 @@ static void test_cubic(void) ...@@ -174,7 +172,7 @@ static void test_cubic(void)
struct bpf_link *link; struct bpf_link *link;
cubic_skel = bpf_cubic__open_and_load(); cubic_skel = bpf_cubic__open_and_load();
if (CHECK(!cubic_skel, "bpf_cubic__open_and_load", "failed\n")) if (!ASSERT_OK_PTR(cubic_skel, "bpf_cubic__open_and_load"))
return; return;
link = bpf_map__attach_struct_ops(cubic_skel->maps.cubic); link = bpf_map__attach_struct_ops(cubic_skel->maps.cubic);
...@@ -197,7 +195,7 @@ static void test_dctcp(void) ...@@ -197,7 +195,7 @@ static void test_dctcp(void)
struct bpf_link *link; struct bpf_link *link;
dctcp_skel = bpf_dctcp__open_and_load(); dctcp_skel = bpf_dctcp__open_and_load();
if (CHECK(!dctcp_skel, "bpf_dctcp__open_and_load", "failed\n")) if (!ASSERT_OK_PTR(dctcp_skel, "bpf_dctcp__open_and_load"))
return; return;
link = bpf_map__attach_struct_ops(dctcp_skel->maps.dctcp); link = bpf_map__attach_struct_ops(dctcp_skel->maps.dctcp);
...@@ -207,9 +205,7 @@ static void test_dctcp(void) ...@@ -207,9 +205,7 @@ static void test_dctcp(void)
} }
do_test("bpf_dctcp", dctcp_skel->maps.sk_stg_map); do_test("bpf_dctcp", dctcp_skel->maps.sk_stg_map);
CHECK(dctcp_skel->bss->stg_result != expected_stg, ASSERT_EQ(dctcp_skel->bss->stg_result, expected_stg, "stg_result");
"Unexpected stg_result", "stg_result (%x) != expected_stg (%x)\n",
dctcp_skel->bss->stg_result, expected_stg);
bpf_link__destroy(link); bpf_link__destroy(link);
bpf_dctcp__destroy(dctcp_skel); bpf_dctcp__destroy(dctcp_skel);
......
...@@ -16,27 +16,27 @@ static void nsleep() ...@@ -16,27 +16,27 @@ static void nsleep()
void test_vmlinux(void) void test_vmlinux(void)
{ {
int duration = 0, err; int err;
struct test_vmlinux* skel; struct test_vmlinux* skel;
struct test_vmlinux__bss *bss; struct test_vmlinux__bss *bss;
skel = test_vmlinux__open_and_load(); skel = test_vmlinux__open_and_load();
if (CHECK(!skel, "skel_open", "failed to open skeleton\n")) if (!ASSERT_OK_PTR(skel, "test_vmlinux__open_and_load"))
return; return;
bss = skel->bss; bss = skel->bss;
err = test_vmlinux__attach(skel); err = test_vmlinux__attach(skel);
if (CHECK(err, "skel_attach", "skeleton attach failed: %d\n", err)) if (!ASSERT_OK(err, "test_vmlinux__attach"))
goto cleanup; goto cleanup;
/* trigger everything */ /* trigger everything */
nsleep(); nsleep();
CHECK(!bss->tp_called, "tp", "not called\n"); ASSERT_TRUE(bss->tp_called, "tp");
CHECK(!bss->raw_tp_called, "raw_tp", "not called\n"); ASSERT_TRUE(bss->raw_tp_called, "raw_tp");
CHECK(!bss->tp_btf_called, "tp_btf", "not called\n"); ASSERT_TRUE(bss->tp_btf_called, "tp_btf");
CHECK(!bss->kprobe_called, "kprobe", "not called\n"); ASSERT_TRUE(bss->kprobe_called, "kprobe");
CHECK(!bss->fentry_called, "fentry", "not called\n"); ASSERT_TRUE(bss->fentry_called, "fentry");
cleanup: cleanup:
test_vmlinux__destroy(skel); test_vmlinux__destroy(skel);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment