diff mbox series

[bpf-next,v5,11/11] selftests/bpf: add test cases for redirection between udp and unix

Message ID 20210704190252.11866-12-xiyou.wangcong@gmail.com
State New
Headers show
Series sockmap: add sockmap support for unix datagram socket | expand

Commit Message

Cong Wang July 4, 2021, 7:02 p.m. UTC 
From: Cong Wang <cong.wang@bytedance.com>

Add two test cases to ensure redirection between udp and unix
work bidirectionally.

Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jakub Sitnicki <jakub@cloudflare.com>
Cc: Lorenz Bauer <lmb@cloudflare.com>
Signed-off-by: Cong Wang <cong.wang@bytedance.com>
---
 .../selftests/bpf/prog_tests/sockmap_listen.c | 170 ++++++++++++++++++
 1 file changed, 170 insertions(+)

Comments

Andrii Nakryiko Aug. 5, 2021, 10:43 p.m. UTC | #1 
On Sun, Jul 4, 2021 at 12:05 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>

> From: Cong Wang <cong.wang@bytedance.com>

>

> Add two test cases to ensure redirection between udp and unix

> work bidirectionally.

>

> Cc: John Fastabend <john.fastabend@gmail.com>

> Cc: Daniel Borkmann <daniel@iogearbox.net>

> Cc: Jakub Sitnicki <jakub@cloudflare.com>

> Cc: Lorenz Bauer <lmb@cloudflare.com>

> Signed-off-by: Cong Wang <cong.wang@bytedance.com>

> ---

>  .../selftests/bpf/prog_tests/sockmap_listen.c | 170 ++++++++++++++++++

>  1 file changed, 170 insertions(+)

>


[...]

> +       n = write(c1, "a", 1);

> +       if (n < 0)

> +               FAIL_ERRNO("%s: write", log_prefix);

> +       if (n == 0)

> +               FAIL("%s: incomplete write", log_prefix);

> +       if (n < 1)

> +               goto close;

> +

> +       key = SK_PASS;

> +       err = xbpf_map_lookup_elem(verd_mapfd, &key, &pass);

> +       if (err)

> +               goto close;

> +       if (pass != 1)

> +               FAIL("%s: want pass count 1, have %d", log_prefix, pass);

> +

> +       n = read(mode == REDIR_INGRESS ? p0 : c0, &b, 1);

> +       if (n < 0)

> +               FAIL_ERRNO("%s: read", log_prefix);


Hey Cong,

This test is pretty flaky and quite frequently fails in our CIs (e.g., [0]):

./test_progs-no_alu32:unix_udp_redir_to_connected:1949: egress: read:
Resource temporarily unavailable
  unix_udp_redir_to_connected:FAIL:1949

Please send a fix to make it more reliable. Thanks!


  [0] https://github.com/anakryiko/libbpf/runs/3249152533?check_suite_focus=true


> +       if (n == 0)

> +               FAIL("%s: incomplete read", log_prefix);

> +

> +close:

> +       xclose(c1);

> +       xclose(p1);

> +close_cli0:

> +       xclose(c0);

> +       xclose(p0);

> +

> +}

> +


[...]
Cong Wang Aug. 6, 2021, 2:34 a.m. UTC | #2 
On Thu, Aug 5, 2021 at 3:43 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>

> On Sun, Jul 4, 2021 at 12:05 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:

> >

> > From: Cong Wang <cong.wang@bytedance.com>

> >

> > Add two test cases to ensure redirection between udp and unix

> > work bidirectionally.

> >

> > Cc: John Fastabend <john.fastabend@gmail.com>

> > Cc: Daniel Borkmann <daniel@iogearbox.net>

> > Cc: Jakub Sitnicki <jakub@cloudflare.com>

> > Cc: Lorenz Bauer <lmb@cloudflare.com>

> > Signed-off-by: Cong Wang <cong.wang@bytedance.com>

> > ---

> >  .../selftests/bpf/prog_tests/sockmap_listen.c | 170 ++++++++++++++++++

> >  1 file changed, 170 insertions(+)

> >

>

> [...]

>

> > +       n = write(c1, "a", 1);

> > +       if (n < 0)

> > +               FAIL_ERRNO("%s: write", log_prefix);

> > +       if (n == 0)

> > +               FAIL("%s: incomplete write", log_prefix);

> > +       if (n < 1)

> > +               goto close;

> > +

> > +       key = SK_PASS;

> > +       err = xbpf_map_lookup_elem(verd_mapfd, &key, &pass);

> > +       if (err)

> > +               goto close;

> > +       if (pass != 1)

> > +               FAIL("%s: want pass count 1, have %d", log_prefix, pass);

> > +

> > +       n = read(mode == REDIR_INGRESS ? p0 : c0, &b, 1);

> > +       if (n < 0)

> > +               FAIL_ERRNO("%s: read", log_prefix);

>

> Hey Cong,

>

> This test is pretty flaky and quite frequently fails in our CIs (e.g., [0]):

>

> ./test_progs-no_alu32:unix_udp_redir_to_connected:1949: egress: read:

> Resource temporarily unavailable

>   unix_udp_redir_to_connected:FAIL:1949

>

> Please send a fix to make it more reliable. Thanks!


Using an unbound number of retries makes this reliable, but you
dislike it. ;) But, there must be some reason for the delay of
packet delivery, I guess it might be related to workqueue scheduling,
let me check it tomorrow.

Thanks.
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
index b6464be89f1a..a9f1bf9d5dff 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
@@ -1825,6 +1825,175 @@  static void test_udp_redir(struct test_sockmap_listen *skel, struct bpf_map *map
 	udp_skb_redir_to_connected(skel, map, family);
 }
 
+static void udp_unix_redir_to_connected(int family, int sock_mapfd,
+					int verd_mapfd, enum redir_mode mode)
+{
+	const char *log_prefix = redir_mode_str(mode);
+	int c0, c1, p0, p1;
+	unsigned int pass;
+	int retries = 100;
+	int err, n;
+	int sfd[2];
+	u32 key;
+	char b;
+
+	zero_verdict_count(verd_mapfd);
+
+	if (socketpair(AF_UNIX, SOCK_DGRAM | SOCK_NONBLOCK, 0, sfd))
+		return;
+	c0 = sfd[0], p0 = sfd[1];
+
+	err = udp_socketpair(family, &p1, &c1);
+	if (err)
+		goto close;
+
+	err = add_to_sockmap(sock_mapfd, p0, p1);
+	if (err)
+		goto close_cli1;
+
+	n = write(c1, "a", 1);
+	if (n < 0)
+		FAIL_ERRNO("%s: write", log_prefix);
+	if (n == 0)
+		FAIL("%s: incomplete write", log_prefix);
+	if (n < 1)
+		goto close_cli1;
+
+	key = SK_PASS;
+	err = xbpf_map_lookup_elem(verd_mapfd, &key, &pass);
+	if (err)
+		goto close_cli1;
+	if (pass != 1)
+		FAIL("%s: want pass count 1, have %d", log_prefix, pass);
+
+again:
+	n = read(mode == REDIR_INGRESS ? p0 : c0, &b, 1);
+	if (n < 0) {
+		if (errno == EAGAIN && retries--)
+			goto again;
+		FAIL_ERRNO("%s: read", log_prefix);
+	}
+	if (n == 0)
+		FAIL("%s: incomplete read", log_prefix);
+
+close_cli1:
+	xclose(c1);
+	xclose(p1);
+close:
+	xclose(c0);
+	xclose(p0);
+}
+
+static void udp_unix_skb_redir_to_connected(struct test_sockmap_listen *skel,
+					    struct bpf_map *inner_map, int family)
+{
+	int verdict = bpf_program__fd(skel->progs.prog_skb_verdict);
+	int verdict_map = bpf_map__fd(skel->maps.verdict_map);
+	int sock_map = bpf_map__fd(inner_map);
+	int err;
+
+	err = xbpf_prog_attach(verdict, sock_map, BPF_SK_SKB_VERDICT, 0);
+	if (err)
+		return;
+
+	skel->bss->test_ingress = false;
+	udp_unix_redir_to_connected(family, sock_map, verdict_map, REDIR_EGRESS);
+	skel->bss->test_ingress = true;
+	udp_unix_redir_to_connected(family, sock_map, verdict_map, REDIR_INGRESS);
+
+	xbpf_prog_detach2(verdict, sock_map, BPF_SK_SKB_VERDICT);
+}
+
+static void unix_udp_redir_to_connected(int family, int sock_mapfd,
+					int verd_mapfd, enum redir_mode mode)
+{
+	const char *log_prefix = redir_mode_str(mode);
+	int c0, c1, p0, p1;
+	unsigned int pass;
+	int err, n;
+	int sfd[2];
+	u32 key;
+	char b;
+
+	zero_verdict_count(verd_mapfd);
+
+	err = udp_socketpair(family, &p0, &c0);
+	if (err)
+		return;
+
+	if (socketpair(AF_UNIX, SOCK_DGRAM | SOCK_NONBLOCK, 0, sfd))
+		goto close_cli0;
+	c1 = sfd[0], p1 = sfd[1];
+
+	err = add_to_sockmap(sock_mapfd, p0, p1);
+	if (err)
+		goto close;
+
+	n = write(c1, "a", 1);
+	if (n < 0)
+		FAIL_ERRNO("%s: write", log_prefix);
+	if (n == 0)
+		FAIL("%s: incomplete write", log_prefix);
+	if (n < 1)
+		goto close;
+
+	key = SK_PASS;
+	err = xbpf_map_lookup_elem(verd_mapfd, &key, &pass);
+	if (err)
+		goto close;
+	if (pass != 1)
+		FAIL("%s: want pass count 1, have %d", log_prefix, pass);
+
+	n = read(mode == REDIR_INGRESS ? p0 : c0, &b, 1);
+	if (n < 0)
+		FAIL_ERRNO("%s: read", log_prefix);
+	if (n == 0)
+		FAIL("%s: incomplete read", log_prefix);
+
+close:
+	xclose(c1);
+	xclose(p1);
+close_cli0:
+	xclose(c0);
+	xclose(p0);
+
+}
+
+static void unix_udp_skb_redir_to_connected(struct test_sockmap_listen *skel,
+					    struct bpf_map *inner_map, int family)
+{
+	int verdict = bpf_program__fd(skel->progs.prog_skb_verdict);
+	int verdict_map = bpf_map__fd(skel->maps.verdict_map);
+	int sock_map = bpf_map__fd(inner_map);
+	int err;
+
+	err = xbpf_prog_attach(verdict, sock_map, BPF_SK_SKB_VERDICT, 0);
+	if (err)
+		return;
+
+	skel->bss->test_ingress = false;
+	unix_udp_redir_to_connected(family, sock_map, verdict_map, REDIR_EGRESS);
+	skel->bss->test_ingress = true;
+	unix_udp_redir_to_connected(family, sock_map, verdict_map, REDIR_INGRESS);
+
+	xbpf_prog_detach2(verdict, sock_map, BPF_SK_SKB_VERDICT);
+}
+
+static void test_udp_unix_redir(struct test_sockmap_listen *skel, struct bpf_map *map,
+				int family)
+{
+	const char *family_name, *map_name;
+	char s[MAX_TEST_NAME];
+
+	family_name = family_str(family);
+	map_name = map_type_str(map);
+	snprintf(s, sizeof(s), "%s %s %s", map_name, family_name, __func__);
+	if (!test__start_subtest(s))
+		return;
+	udp_unix_skb_redir_to_connected(skel, map, family);
+	unix_udp_skb_redir_to_connected(skel, map, family);
+}
+
 static void run_tests(struct test_sockmap_listen *skel, struct bpf_map *map,
 		      int family)
 {
@@ -1834,6 +2003,7 @@  static void run_tests(struct test_sockmap_listen *skel, struct bpf_map *map,
 	test_reuseport(skel, map, family, SOCK_STREAM);
 	test_reuseport(skel, map, family, SOCK_DGRAM);
 	test_udp_redir(skel, map, family);
+	test_udp_unix_redir(skel, map, family);
 }
 
 void test_sockmap_listen(void)