From: Jakub Sitnicki <jakub@cloudflare.com>
To: Lorenz Bauer <lmb@cloudflare.com>
Cc: ast@kernel.org, daniel@iogearbox.net, john.fastabend@gmail.com,
netdev@vger.kernel.org, bpf@vger.kernel.org,
kernel-team@cloudflare.com
Subject: Re: [PATCH bpf-next 6/7] selftests: bpf: add tests for UDP sockets in sockmap
Date: Thu, 27 Feb 2020 12:49:09 +0100 [thread overview]
Message-ID: <877e08cksq.fsf@cloudflare.com> (raw)
In-Reply-To: <20200225135636.5768-7-lmb@cloudflare.com>
On Tue, Feb 25, 2020 at 02:56 PM CET, Lorenz Bauer wrote:
> Expand the TCP sockmap test suite to also check UDP sockets.
>
> Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
> ---
> .../selftests/bpf/prog_tests/sockmap_listen.c | 92 +++++++++++++------
> 1 file changed, 63 insertions(+), 29 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> index 4ba41dd26d6b..72e578a5a5d2 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> @@ -330,7 +330,7 @@ static void test_insert_bound(int family, int sotype, int mapfd)
> xclose(s);
> }
>
> -static void test_insert_listening(int family, int sotype, int mapfd)
> +static void test_insert(int family, int sotype, int mapfd)
> {
> u64 value;
> u32 key;
> @@ -467,7 +467,7 @@ static void test_lookup_32_bit_value(int family, int sotype, int mapfd)
> xclose(s);
> }
>
> -static void test_update_listening(int family, int sotype, int mapfd)
> +static void test_update_existing(int family, int sotype, int mapfd)
> {
> int s1, s2;
> u64 value;
> @@ -1302,11 +1302,15 @@ static void test_reuseport_mixed_groups(int family, int sotype, int sock_map,
> xclose(s1);
> }
>
> -#define TEST(fn) \
> +#define TEST_SOTYPE(fn, sotype) \
> { \
> - fn, #fn \
> + fn, #fn, sotype \
> }
>
> +#define TEST(fn) TEST_SOTYPE(fn, 0)
> +#define TEST_STREAM(fn) TEST_SOTYPE(fn, SOCK_STREAM)
> +#define TEST_DGRAM(fn) TEST_SOTYPE(fn, SOCK_DGRAM)
> +
An alternative would be to use __VA_ARGS__ and designated intializers,
as I did recently in e0360423d020 ("selftests/bpf: Run SYN cookies with
reuseport BPF test only for TCP"). TEST_DGRAM is unused at the moment,
so that's something to consider.
> static void test_ops_cleanup(const struct bpf_map *map)
> {
> const struct bpf_map_def *def;
> @@ -1353,18 +1357,31 @@ static const char *map_type_str(const struct bpf_map *map)
> }
> }
>
> +static const char *sotype_str(int sotype)
> +{
> + switch (sotype) {
> + case SOCK_DGRAM:
> + return "UDP";
> + case SOCK_STREAM:
> + return "TCP";
> + default:
> + return "unknown";
> + }
> +}
> +
> static void test_ops(struct test_sockmap_listen *skel, struct bpf_map *map,
> int family, int sotype)
> {
> const struct op_test {
> void (*fn)(int family, int sotype, int mapfd);
> const char *name;
> + int sotype;
> } tests[] = {
> /* insert */
> TEST(test_insert_invalid),
> TEST(test_insert_opened),
> - TEST(test_insert_bound),
> - TEST(test_insert_listening),
> + TEST_STREAM(test_insert_bound),
> + TEST(test_insert),
> /* delete */
> TEST(test_delete_after_insert),
> TEST(test_delete_after_close),
> @@ -1373,28 +1390,33 @@ static void test_ops(struct test_sockmap_listen *skel, struct bpf_map *map,
> TEST(test_lookup_after_delete),
> TEST(test_lookup_32_bit_value),
> /* update */
> - TEST(test_update_listening),
> + TEST(test_update_existing),
> /* races with insert/delete */
> - TEST(test_destroy_orphan_child),
> - TEST(test_syn_recv_insert_delete),
> - TEST(test_race_insert_listen),
> + TEST_STREAM(test_destroy_orphan_child),
> + TEST_STREAM(test_syn_recv_insert_delete),
> + TEST_STREAM(test_race_insert_listen),
> /* child clone */
> - TEST(test_clone_after_delete),
> - TEST(test_accept_after_delete),
> - TEST(test_accept_before_delete),
> + TEST_STREAM(test_clone_after_delete),
> + TEST_STREAM(test_accept_after_delete),
> + TEST_STREAM(test_accept_before_delete),
> };
> - const char *family_name, *map_name;
> + const char *family_name, *map_name, *sotype_name;
> const struct op_test *t;
> char s[MAX_TEST_NAME];
> int map_fd;
>
> family_name = family_str(family);
> map_name = map_type_str(map);
> + sotype_name = sotype_str(sotype);
> map_fd = bpf_map__fd(map);
>
> +
> for (t = tests; t < tests + ARRAY_SIZE(tests); t++) {
> - snprintf(s, sizeof(s), "%s %s %s", map_name, family_name,
> - t->name);
> + snprintf(s, sizeof(s), "%s %s %s %s", map_name, family_name,
> + sotype_name, t->name);
> +
> + if (t->sotype != 0 && t->sotype != sotype)
> + continue;
>
> if (!test__start_subtest(s))
> continue;
> @@ -1411,22 +1433,28 @@ static void test_redir(struct test_sockmap_listen *skel, struct bpf_map *map,
> void (*fn)(struct test_sockmap_listen *skel,
> struct bpf_map *map, int family, int sotype);
> const char *name;
> + int sotype;
> } tests[] = {
> - TEST(test_skb_redir_to_connected),
> - TEST(test_skb_redir_to_listening),
> - TEST(test_msg_redir_to_connected),
> - TEST(test_msg_redir_to_listening),
> + TEST_STREAM(test_skb_redir_to_connected),
> + TEST_STREAM(test_skb_redir_to_listening),
> + TEST_STREAM(test_msg_redir_to_connected),
> + TEST_STREAM(test_msg_redir_to_listening),
> };
> - const char *family_name, *map_name;
> + const char *family_name, *map_name, *sotype_name;
> const struct redir_test *t;
> char s[MAX_TEST_NAME];
>
> family_name = family_str(family);
> map_name = map_type_str(map);
> + sotype_name = sotype_str(sotype);
>
> for (t = tests; t < tests + ARRAY_SIZE(tests); t++) {
> - snprintf(s, sizeof(s), "%s %s %s", map_name, family_name,
> - t->name);
> + snprintf(s, sizeof(s), "%s %s %s %s", map_name, family_name,
> + sotype_name, t->name);
> +
> + if (t->sotype != 0 && t->sotype != sotype)
> + continue;
> +
> if (!test__start_subtest(s))
> continue;
>
test_redir() doesn't get called with SOCK_DGRAM, because redirection
with sockmap and UDP is not supported yet, so perhaps no need to touch
this function.
> @@ -1441,26 +1469,31 @@ static void test_reuseport(struct test_sockmap_listen *skel,
> void (*fn)(int family, int sotype, int socket_map,
> int verdict_map, int reuseport_prog);
> const char *name;
> + int sotype;
> } tests[] = {
> - TEST(test_reuseport_select_listening),
> - TEST(test_reuseport_select_connected),
> - TEST(test_reuseport_mixed_groups),
> + TEST_STREAM(test_reuseport_select_listening),
> + TEST_STREAM(test_reuseport_select_connected),
> + TEST_STREAM(test_reuseport_mixed_groups),
> };
> int socket_map, verdict_map, reuseport_prog;
> - const char *family_name, *map_name;
> + const char *family_name, *map_name, *sotype_name;
> const struct reuseport_test *t;
> char s[MAX_TEST_NAME];
>
> family_name = family_str(family);
> map_name = map_type_str(map);
> + sotype_name = sotype_str(sotype);
>
> socket_map = bpf_map__fd(map);
> verdict_map = bpf_map__fd(skel->maps.verdict_map);
> reuseport_prog = bpf_program__fd(skel->progs.prog_reuseport);
>
> for (t = tests; t < tests + ARRAY_SIZE(tests); t++) {
> - snprintf(s, sizeof(s), "%s %s %s", map_name, family_name,
> - t->name);
> + snprintf(s, sizeof(s), "%s %s %s %s", map_name, family_name,
> + sotype_name, t->name);
> +
> + if (t->sotype != 0 && t->sotype != sotype)
> + continue;
>
> if (!test__start_subtest(s))
> continue;
> @@ -1473,6 +1506,7 @@ static void run_tests(struct test_sockmap_listen *skel, struct bpf_map *map,
> int family)
> {
> test_ops(skel, map, family, SOCK_STREAM);
> + test_ops(skel, map, family, SOCK_DGRAM);
> test_redir(skel, map, family, SOCK_STREAM);
> test_reuseport(skel, map, family, SOCK_STREAM);
> }
Looks like we can enable reuseport tests with not too much effort.
I managed to get them working with the changes below.
---
.../selftests/bpf/prog_tests/sockmap_listen.c | 102 +++++++++++++++---
1 file changed, 87 insertions(+), 15 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
index 72e578a5a5d2..6850994fc4d5 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
@@ -108,6 +108,22 @@
__ret; \
})
+#define xsend(fd, buf, len, flags) \
+ ({ \
+ ssize_t __ret = send((fd), (buf), (len), (flags)); \
+ if (__ret == -1) \
+ FAIL_ERRNO("send"); \
+ __ret; \
+ })
+
+#define xrecv(fd, buf, len, flags) \
+ ({ \
+ ssize_t __ret = recv((fd), (buf), (len), (flags)); \
+ if (__ret == -1) \
+ FAIL_ERRNO("recv"); \
+ __ret; \
+ })
+
#define xsocket(family, sotype, flags) \
({ \
int __ret = socket(family, sotype, flags); \
@@ -1116,7 +1132,7 @@ static void test_reuseport_select_listening(int family, int sotype,
{
struct sockaddr_storage addr;
unsigned int pass;
- int s, c, p, err;
+ int s, c, err;
socklen_t len;
u64 value;
u32 key;
@@ -1145,19 +1161,33 @@ static void test_reuseport_select_listening(int family, int sotype,
if (err)
goto close_cli;
- p = xaccept(s, NULL, NULL);
- if (p < 0)
- goto close_cli;
+ if (sotype == SOCK_STREAM) {
+ int p;
+
+ p = xaccept(s, NULL, NULL);
+ if (p < 0)
+ goto close_cli;
+ xclose(p);
+ } else {
+ char b = 'a';
+ ssize_t n;
+
+ n = xsend(c, &b, sizeof(b), 0);
+ if (n == -1)
+ goto close_cli;
+
+ n = xrecv(s, &b, sizeof(b), 0);
+ if (n == -1)
+ goto close_cli;
+ }
key = SK_PASS;
err = xbpf_map_lookup_elem(verd_map, &key, &pass);
if (err)
- goto close_peer;
+ goto close_cli;
if (pass != 1)
FAIL("want pass count 1, have %d", pass);
-close_peer:
- xclose(p);
close_cli:
xclose(c);
close_srv:
@@ -1201,9 +1231,24 @@ static void test_reuseport_select_connected(int family, int sotype,
if (err)
goto close_cli0;
- p0 = xaccept(s, NULL, NULL);
- if (err)
- goto close_cli0;
+ if (sotype == SOCK_STREAM) {
+ p0 = xaccept(s, NULL, NULL);
+ if (p0 < 0)
+ goto close_cli0;
+ } else {
+ p0 = xsocket(family, sotype, 0);
+ if (p0 < 0)
+ goto close_cli0;
+
+ len = sizeof(addr);
+ err = xgetsockname(c0, sockaddr(&addr), &len);
+ if (err)
+ goto close_cli0;
+
+ err = xconnect(p0, sockaddr(&addr), len);
+ if (err)
+ goto close_cli0;
+ }
/* Update sock_map[0] to redirect to a connected socket */
key = 0;
@@ -1216,8 +1261,24 @@ static void test_reuseport_select_connected(int family, int sotype,
if (c1 < 0)
goto close_peer0;
+ len = sizeof(addr);
+ err = xgetsockname(s, sockaddr(&addr), &len);
+ if (err)
+ goto close_srv;
+
errno = 0;
err = connect(c1, sockaddr(&addr), len);
+ if (sotype == SOCK_DGRAM) {
+ char b = 'a';
+ ssize_t n;
+
+ n = xsend(c1, &b, sizeof(b), 0);
+ if (n == -1)
+ goto close_cli1;
+
+ n = recv(c1, &b, sizeof(b), 0);
+ err = n == -1;
+ }
if (!err || errno != ECONNREFUSED)
FAIL_ERRNO("connect: expected ECONNREFUSED");
@@ -1281,7 +1342,18 @@ static void test_reuseport_mixed_groups(int family, int sotype, int sock_map,
goto close_srv2;
err = connect(c, sockaddr(&addr), len);
- if (err && errno != ECONNREFUSED) {
+ if (sotype == SOCK_DGRAM) {
+ char b = 'a';
+ ssize_t n;
+
+ n = xsend(c, &b, sizeof(b), 0);
+ if (n == -1)
+ goto close_cli;
+
+ n = recv(c, &b, sizeof(b), 0);
+ err = n == -1;
+ }
+ if (!err || errno != ECONNREFUSED) {
FAIL_ERRNO("connect: expected ECONNREFUSED");
goto close_cli;
}
@@ -1410,7 +1482,6 @@ static void test_ops(struct test_sockmap_listen *skel, struct bpf_map *map,
sotype_name = sotype_str(sotype);
map_fd = bpf_map__fd(map);
-
for (t = tests; t < tests + ARRAY_SIZE(tests); t++) {
snprintf(s, sizeof(s), "%s %s %s %s", map_name, family_name,
sotype_name, t->name);
@@ -1471,9 +1542,9 @@ static void test_reuseport(struct test_sockmap_listen *skel,
const char *name;
int sotype;
} tests[] = {
- TEST_STREAM(test_reuseport_select_listening),
- TEST_STREAM(test_reuseport_select_connected),
- TEST_STREAM(test_reuseport_mixed_groups),
+ TEST(test_reuseport_select_listening),
+ TEST(test_reuseport_select_connected),
+ TEST(test_reuseport_mixed_groups),
};
int socket_map, verdict_map, reuseport_prog;
const char *family_name, *map_name, *sotype_name;
@@ -1509,6 +1580,7 @@ static void run_tests(struct test_sockmap_listen *skel, struct bpf_map *map,
test_ops(skel, map, family, SOCK_DGRAM);
test_redir(skel, map, family, SOCK_STREAM);
test_reuseport(skel, map, family, SOCK_STREAM);
+ test_reuseport(skel, map, family, SOCK_DGRAM);
}
void test_sockmap_listen(void)
--
2.24.1
next prev parent reply other threads:[~2020-02-27 11:49 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-25 13:56 [PATCH bpf-next 0/7] bpf: sockmap, sockhash: support storing UDP sockets Lorenz Bauer
2020-02-25 13:56 ` [PATCH bpf-next 1/7] bpf: sockmap: only check ULP for TCP sockets Lorenz Bauer
2020-02-25 16:45 ` Song Liu
2020-02-25 13:56 ` [PATCH bpf-next 2/7] bpf: sockmap: move generic sockmap hooks from BPF TCP Lorenz Bauer
2020-02-25 17:22 ` Song Liu
2020-02-25 22:15 ` kbuild test robot
2020-02-25 13:56 ` [PATCH bpf-next 3/7] skmsg: introduce sk_psock_hooks Lorenz Bauer
2020-02-26 14:57 ` Jakub Sitnicki
2020-02-26 18:37 ` Martin KaFai Lau
2020-02-28 10:48 ` Lorenz Bauer
2020-02-27 9:27 ` Jakub Sitnicki
2020-02-27 9:40 ` Jakub Sitnicki
2020-02-25 13:56 ` [PATCH bpf-next 4/7] bpf: sockmap: allow UDP sockets Lorenz Bauer
2020-02-25 23:36 ` kbuild test robot
2020-02-26 18:47 ` Martin KaFai Lau
2020-02-25 13:56 ` [PATCH bpf-next 5/7] selftests: bpf: don't listen() on " Lorenz Bauer
2020-02-25 13:56 ` [PATCH bpf-next 6/7] selftests: bpf: add tests for UDP sockets in sockmap Lorenz Bauer
2020-02-27 11:49 ` Jakub Sitnicki [this message]
2020-02-27 12:02 ` Lorenz Bauer
2020-02-25 13:56 ` [PATCH bpf-next 7/7] selftests: bpf: enable UDP sockmap reuseport tests Lorenz Bauer
2020-02-26 13:12 ` Jakub Sitnicki
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=877e08cksq.fsf@cloudflare.com \
--to=jakub@cloudflare.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=john.fastabend@gmail.com \
--cc=kernel-team@cloudflare.com \
--cc=lmb@cloudflare.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.