From: Alexander Duyck <alexander.duyck@gmail.com>
To: bpf@vger.kernel.org
Cc: ast@kernel.org, daniel@iogearbox.net, kafai@fb.com,
john.fastabend@gmail.com, kernel-team@fb.com,
netdev@vger.kernel.org, edumazet@google.com, brakmo@fb.com,
andrii.nakryiko@gmail.com, alexanderduyck@fb.com
Subject: [bpf-next PATCH v2 3/5] selftests/bpf: Replace EXPECT_EQ with ASSERT_EQ and refactor verify_results
Date: Sat, 31 Oct 2020 11:52:24 -0700 [thread overview]
Message-ID: <160417034457.2823.10600750891200038944.stgit@localhost.localdomain> (raw)
In-Reply-To: <160416890683.710453.7723265174628409401.stgit@localhost.localdomain>
From: Alexander Duyck <alexanderduyck@fb.com>
There is already logic in test_progs.h for asserting that a value is
expected to be another value. So instead of reinventing it we should just
make use of ASSERT_EQ in tcpbpf_user.c. This will allow for better
debugging and integrates much more closely with the test_progs framework.
In addition we can refactor the code a bit to merge together the two
verify functions and tie them together into a single function. Doing this
helps to clean the code up a bit and makes it more readable as all the
verification is now done in one function.
Lastly we can relocate the verification to the end of the run_test since it
is logically part of the test itself. With this we can drop the need for a
return value from run_test since verification becomes the last step of the
call and then immediately following is the tear down of the test setup.
Signed-off-by: Alexander Duyck <alexanderduyck@fb.com>
---
.../testing/selftests/bpf/prog_tests/tcpbpf_user.c | 114 ++++++++------------
1 file changed, 44 insertions(+), 70 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c b/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c
index 17d4299435df..d96f4084d2f5 100644
--- a/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c
+++ b/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c
@@ -10,66 +10,58 @@
static __u32 duration;
-#define EXPECT_EQ(expected, actual, fmt) \
- do { \
- if ((expected) != (actual)) { \
- fprintf(stderr, " Value of: " #actual "\n" \
- " Actual: %" fmt "\n" \
- " Expected: %" fmt "\n", \
- (actual), (expected)); \
- ret--; \
- } \
- } while (0)
-
-int verify_result(const struct tcpbpf_globals *result)
-{
- __u32 expected_events;
- int ret = 0;
-
- expected_events = ((1 << BPF_SOCK_OPS_TIMEOUT_INIT) |
- (1 << BPF_SOCK_OPS_RWND_INIT) |
- (1 << BPF_SOCK_OPS_TCP_CONNECT_CB) |
- (1 << BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB) |
- (1 << BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB) |
- (1 << BPF_SOCK_OPS_NEEDS_ECN) |
- (1 << BPF_SOCK_OPS_STATE_CB) |
- (1 << BPF_SOCK_OPS_TCP_LISTEN_CB));
-
- EXPECT_EQ(expected_events, result->event_map, "#" PRIx32);
- EXPECT_EQ(501ULL, result->bytes_received, "llu");
- EXPECT_EQ(1002ULL, result->bytes_acked, "llu");
- EXPECT_EQ(1, result->data_segs_in, PRIu32);
- EXPECT_EQ(1, result->data_segs_out, PRIu32);
- EXPECT_EQ(0x80, result->bad_cb_test_rv, PRIu32);
- EXPECT_EQ(0, result->good_cb_test_rv, PRIu32);
- EXPECT_EQ(1, result->num_listen, PRIu32);
-
- /* 3 comes from one listening socket + both ends of the connection */
- EXPECT_EQ(3, result->num_close_events, PRIu32);
-
- return ret;
-}
-
-int verify_sockopt_result(int sock_map_fd)
+static void verify_result(int map_fd, int sock_map_fd)
{
+ __u32 expected_events = ((1 << BPF_SOCK_OPS_TIMEOUT_INIT) |
+ (1 << BPF_SOCK_OPS_RWND_INIT) |
+ (1 << BPF_SOCK_OPS_TCP_CONNECT_CB) |
+ (1 << BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB) |
+ (1 << BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB) |
+ (1 << BPF_SOCK_OPS_NEEDS_ECN) |
+ (1 << BPF_SOCK_OPS_STATE_CB) |
+ (1 << BPF_SOCK_OPS_TCP_LISTEN_CB));
+ struct tcpbpf_globals result = { 0 };
__u32 key = 0;
- int ret = 0;
int res;
int rv;
+ rv = bpf_map_lookup_elem(map_fd, &key, &result);
+ if (CHECK(rv, "bpf_map_lookup_elem(map_fd)", "err:%d errno:%d",
+ rv, errno))
+ return;
+
+ /* check global map */
+ CHECK(expected_events != result.event_map, "event_map",
+ "unexpected event_map: actual %#" PRIx32" != expected %#" PRIx32 "\n",
+ result.event_map, expected_events);
+
+ ASSERT_EQ(result.bytes_received, 501, "bytes_received");
+ ASSERT_EQ(result.bytes_acked, 1002, "bytes_acked");
+ ASSERT_EQ(result.data_segs_in, 1, "data_segs_in");
+ ASSERT_EQ(result.data_segs_out, 1, "data_segs_out");
+ ASSERT_EQ(result.bad_cb_test_rv, 0x80, "bad_cb_test_rv");
+ ASSERT_EQ(result.good_cb_test_rv, 0, "good_cb_test_rv");
+ ASSERT_EQ(result.num_listen, 1, "num_listen");
+
+ /* 3 comes from one listening socket + both ends of the connection */
+ ASSERT_EQ(result.num_close_events, 3, "num_close_events");
+
/* check setsockopt for SAVE_SYN */
+ key = 0;
rv = bpf_map_lookup_elem(sock_map_fd, &key, &res);
- EXPECT_EQ(0, rv, "d");
- EXPECT_EQ(0, res, "d");
- key = 1;
+ CHECK(rv, "bpf_map_lookup_elem(sock_map_fd)", "err:%d errno:%d",
+ rv, errno);
+ ASSERT_EQ(res, 0, "bpf_setsockopt(TCP_SAVE_SYN)");
+
/* check getsockopt for SAVED_SYN */
+ key = 1;
rv = bpf_map_lookup_elem(sock_map_fd, &key, &res);
- EXPECT_EQ(0, rv, "d");
- EXPECT_EQ(1, res, "d");
- return ret;
+ CHECK(rv, "bpf_map_lookup_elem(sock_map_fd)", "err:%d errno:%d",
+ rv, errno);
+ ASSERT_EQ(res, 1, "bpf_getsockopt(TCP_SAVED_SYN)");
}
-static int run_test(void)
+static void run_test(int map_fd, int sock_map_fd)
{
int listen_fd = -1, cli_fd = -1, accept_fd = -1;
char buf[1000];
@@ -135,18 +127,17 @@ static int run_test(void)
if (listen_fd != -1)
close(listen_fd);
- return err;
+ if (!err)
+ verify_result(map_fd, sock_map_fd);
}
void test_tcpbpf_user(void)
{
const char *file = "test_tcpbpf_kern.o";
int prog_fd, map_fd, sock_map_fd;
- struct tcpbpf_globals g = {0};
int error = EXIT_FAILURE;
struct bpf_object *obj;
int cg_fd = -1;
- __u32 key = 0;
int rv;
cg_fd = test__join_cgroup(CG_NAME);
@@ -173,24 +164,7 @@ void test_tcpbpf_user(void)
if (sock_map_fd < 0)
goto err;
- if (run_test())
- goto err;
-
- rv = bpf_map_lookup_elem(map_fd, &key, &g);
- if (rv != 0) {
- fprintf(stderr, "FAILED: bpf_map_lookup_elem returns %d\n", rv);
- goto err;
- }
-
- if (verify_result(&g)) {
- fprintf(stderr, "FAILED: Wrong stats\n");
- goto err;
- }
-
- if (verify_sockopt_result(sock_map_fd)) {
- fprintf(stderr, "FAILED: Wrong sockopt stats\n");
- goto err;
- }
+ run_test(map_fd, sock_map_fd);
error = 0;
err:
next prev parent reply other threads:[~2020-10-31 18:52 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <160416890683.710453.7723265174628409401.stgit@localhost.localdomain>
2020-10-31 18:52 ` [bpf-next PATCH v2 1/5] selftests/bpf: Move test_tcppbf_user into test_progs Alexander Duyck
2020-11-03 18:34 ` Andrii Nakryiko
2020-11-03 19:06 ` Alexander Duyck
2020-10-31 18:52 ` [bpf-next PATCH v2 2/5] selftests/bpf: Drop python client/server in favor of threads Alexander Duyck
2020-11-03 0:38 ` Martin KaFai Lau
2020-11-03 0:49 ` Alexander Duyck
2020-11-03 1:33 ` Martin KaFai Lau
2020-11-03 16:01 ` Alexander Duyck
2020-10-31 18:52 ` Alexander Duyck [this message]
2020-11-03 0:42 ` [bpf-next PATCH v2 3/5] selftests/bpf: Replace EXPECT_EQ with ASSERT_EQ and refactor verify_results Martin KaFai Lau
2020-11-03 0:56 ` Alexander Duyck
2020-11-03 1:40 ` Martin KaFai Lau
2020-11-03 18:38 ` Andrii Nakryiko
2020-10-31 18:52 ` [bpf-next PATCH v2 4/5] selftests/bpf: Migrate tcpbpf_user.c to use BPF skeleton Alexander Duyck
2020-11-03 0:55 ` Martin KaFai Lau
2020-11-03 15:44 ` Alexander Duyck
2020-10-31 18:52 ` [bpf-next PATCH v2 5/5] selftest/bpf: Use global variables instead of maps for test_tcpbpf_kern Alexander Duyck
2020-11-03 1:25 ` Martin KaFai Lau
2020-11-03 15:42 ` Alexander Duyck
2020-11-03 18:12 ` Martin KaFai Lau
2020-11-03 18:40 ` Andrii Nakryiko
2020-10-31 19:03 ` [bpf-next PATCH v2 0/5] selftests/bpf: Migrate test_tcpbpf_user to be a part of test_progs framework Alexander Duyck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=160417034457.2823.10600750891200038944.stgit@localhost.localdomain \
--to=alexander.duyck@gmail.com \
--cc=alexanderduyck@fb.com \
--cc=andrii.nakryiko@gmail.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=brakmo@fb.com \
--cc=daniel@iogearbox.net \
--cc=edumazet@google.com \
--cc=john.fastabend@gmail.com \
--cc=kafai@fb.com \
--cc=kernel-team@fb.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).