* [PATCH bpf-next v2 0/2] cleanups for sockmap_listen
@ 2023-10-05 7:21 Geliang Tang
2023-10-05 7:21 ` [PATCH bpf-next v2 1/2] selftests/bpf: Enable CONFIG_VSOCKETS in config Geliang Tang
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Geliang Tang @ 2023-10-05 7:21 UTC (permalink / raw)
To: Andrii Nakryiko, Mykola Lysenko, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa
Cc: Geliang Tang, bpf, linux-kselftest
v2:
- rename c0/c1 to cli0/cli1, p0/p1 to peer0/perr1 as Daniel suggested.
Two cleanups for sockmap_listen selftests: enable a kconfig and add a
new helper.
Geliang Tang (2):
selftests/bpf: Enable CONFIG_VSOCKETS in config
selftests/bpf: Add pairs_redir_to_connected helper
tools/testing/selftests/bpf/config | 1 +
.../selftests/bpf/prog_tests/sockmap_listen.c | 159 ++++--------------
2 files changed, 36 insertions(+), 124 deletions(-)
--
2.35.3
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH bpf-next v2 1/2] selftests/bpf: Enable CONFIG_VSOCKETS in config 2023-10-05 7:21 [PATCH bpf-next v2 0/2] cleanups for sockmap_listen Geliang Tang @ 2023-10-05 7:21 ` Geliang Tang 2023-10-05 7:21 ` [PATCH bpf-next v2 2/2] selftests/bpf: Add pairs_redir_to_connected helper Geliang Tang 2023-10-06 5:10 ` [PATCH bpf-next v2 0/2] cleanups for sockmap_listen patchwork-bot+netdevbpf 2 siblings, 0 replies; 6+ messages in thread From: Geliang Tang @ 2023-10-05 7:21 UTC (permalink / raw) To: Andrii Nakryiko, Mykola Lysenko, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa Cc: Geliang Tang, bpf, linux-kselftest CONFIG_VSOCKETS is required by BPF selftests, otherwise we get errors like this: ./test_progs:socket_loopback_reuseport:386: socket: Address family not supported by protocol socket_loopback_reuseport:FAIL:386 ./test_progs:vsock_unix_redir_connectible:1496: vsock_socketpair_connectible() failed vsock_unix_redir_connectible:FAIL:1496 So this patch enables it in tools/testing/selftests/bpf/config. Signed-off-by: Geliang Tang <geliang.tang@suse.com> --- tools/testing/selftests/bpf/config | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config index e41eb33b2704..02dd4409200e 100644 --- a/tools/testing/selftests/bpf/config +++ b/tools/testing/selftests/bpf/config @@ -84,3 +84,4 @@ CONFIG_USERFAULTFD=y CONFIG_VXLAN=y CONFIG_XDP_SOCKETS=y CONFIG_XFRM_INTERFACE=y +CONFIG_VSOCKETS=y -- 2.35.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf-next v2 2/2] selftests/bpf: Add pairs_redir_to_connected helper 2023-10-05 7:21 [PATCH bpf-next v2 0/2] cleanups for sockmap_listen Geliang Tang 2023-10-05 7:21 ` [PATCH bpf-next v2 1/2] selftests/bpf: Enable CONFIG_VSOCKETS in config Geliang Tang @ 2023-10-05 7:21 ` Geliang Tang 2023-10-06 5:18 ` Martin KaFai Lau 2023-10-06 5:10 ` [PATCH bpf-next v2 0/2] cleanups for sockmap_listen patchwork-bot+netdevbpf 2 siblings, 1 reply; 6+ messages in thread From: Geliang Tang @ 2023-10-05 7:21 UTC (permalink / raw) To: Andrii Nakryiko, Mykola Lysenko, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa Cc: Geliang Tang, bpf, linux-kselftest Extract duplicate code from these four functions unix_redir_to_connected() udp_redir_to_connected() inet_unix_redir_to_connected() unix_inet_redir_to_connected() to create a new helper pairs_redir_to_connected(). Create the different socketpairs in these four functions, then pass the socketpairs info to the new common helper to do the connections. Signed-off-by: Geliang Tang <geliang.tang@suse.com> --- .../selftests/bpf/prog_tests/sockmap_listen.c | 159 ++++-------------- 1 file changed, 35 insertions(+), 124 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c index 8df8cbb447f1..74cbce11e22a 100644 --- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c @@ -1336,32 +1336,22 @@ static void test_redir(struct test_sockmap_listen *skel, struct bpf_map *map, } } -static void unix_redir_to_connected(int sotype, int sock_mapfd, - int verd_mapfd, enum redir_mode mode) +static void pairs_redir_to_connected(int cli0, int peer0, int cli1, int peer1, + 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); - if (socketpair(AF_UNIX, sotype | SOCK_NONBLOCK, 0, sfd)) - return; - c0 = sfd[0], p0 = sfd[1]; - - if (socketpair(AF_UNIX, sotype | SOCK_NONBLOCK, 0, sfd)) - goto close0; - c1 = sfd[0], p1 = sfd[1]; - - err = add_to_sockmap(sock_mapfd, p0, p1); + err = add_to_sockmap(sock_mapfd, peer0, peer1); if (err) goto close; - n = write(c1, "a", 1); + n = write(cli1, "a", 1); if (n < 0) FAIL_ERRNO("%s: write", log_prefix); if (n == 0) @@ -1376,16 +1366,34 @@ static void unix_redir_to_connected(int sotype, int sock_mapfd, if (pass != 1) FAIL("%s: want pass count 1, have %d", log_prefix, pass); - n = recv_timeout(mode == REDIR_INGRESS ? p0 : c0, &b, 1, 0, IO_TIMEOUT_SEC); + n = recv_timeout(mode == REDIR_INGRESS ? peer0 : cli0, &b, 1, 0, IO_TIMEOUT_SEC); if (n < 0) FAIL_ERRNO("%s: recv_timeout", log_prefix); if (n == 0) FAIL("%s: incomplete recv", log_prefix); close: - xclose(c1); - xclose(p1); -close0: + xclose(cli1); + xclose(peer1); +} + +static void unix_redir_to_connected(int sotype, int sock_mapfd, + int verd_mapfd, enum redir_mode mode) +{ + int c0, c1, p0, p1; + int sfd[2]; + + if (socketpair(AF_UNIX, sotype | SOCK_NONBLOCK, 0, sfd)) + return; + c0 = sfd[0], p0 = sfd[1]; + + if (socketpair(AF_UNIX, sotype | SOCK_NONBLOCK, 0, sfd)) + goto close; + c1 = sfd[0], p1 = sfd[1]; + + pairs_redir_to_connected(c0, p0, c1, p1, sock_mapfd, verd_mapfd, mode); + +close: xclose(c0); xclose(p0); } @@ -1661,51 +1669,19 @@ static int inet_socketpair(int family, int type, int *s, int *c) static void 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; - u32 key; - char b; - - zero_verdict_count(verd_mapfd); + int err; err = inet_socketpair(family, SOCK_DGRAM, &p0, &c0); if (err) return; err = inet_socketpair(family, SOCK_DGRAM, &p1, &c1); if (err) - goto close_cli0; - - 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); + goto close; - n = recv_timeout(mode == REDIR_INGRESS ? p0 : c0, &b, 1, 0, IO_TIMEOUT_SEC); - if (n < 0) - FAIL_ERRNO("%s: recv_timeout", log_prefix); - if (n == 0) - FAIL("%s: incomplete recv", log_prefix); + pairs_redir_to_connected(c0, p0, c1, p1, sock_mapfd, verd_mapfd, mode); -close_cli1: - xclose(c1); - xclose(p1); -close_cli0: +close: xclose(c0); xclose(p0); } @@ -1747,15 +1723,9 @@ static void test_udp_redir(struct test_sockmap_listen *skel, struct bpf_map *map static void inet_unix_redir_to_connected(int family, int type, 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); + int err; if (socketpair(AF_UNIX, SOCK_DGRAM | SOCK_NONBLOCK, 0, sfd)) return; @@ -1765,34 +1735,8 @@ static void inet_unix_redir_to_connected(int family, int type, int sock_mapfd, 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; + pairs_redir_to_connected(c0, p0, c1, p1, sock_mapfd, verd_mapfd, mode); - 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); - - n = recv_timeout(mode == REDIR_INGRESS ? p0 : c0, &b, 1, 0, IO_TIMEOUT_SEC); - if (n < 0) - FAIL_ERRNO("%s: recv_timeout", log_prefix); - if (n == 0) - FAIL("%s: incomplete recv", log_prefix); - -close_cli1: - xclose(c1); - xclose(p1); close: xclose(c0); xclose(p0); @@ -1827,56 +1771,23 @@ static void inet_unix_skb_redir_to_connected(struct test_sockmap_listen *skel, static void unix_inet_redir_to_connected(int family, int type, 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); + int err; err = inet_socketpair(family, SOCK_DGRAM, &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; + c1 = sfd[0], p1 = sfd[1]; - 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 = recv_timeout(mode == REDIR_INGRESS ? p0 : c0, &b, 1, 0, IO_TIMEOUT_SEC); - if (n < 0) - FAIL_ERRNO("%s: recv_timeout", log_prefix); - if (n == 0) - FAIL("%s: incomplete recv", log_prefix); + pairs_redir_to_connected(c0, p0, c1, p1, sock_mapfd, verd_mapfd, mode); close: - xclose(c1); - xclose(p1); -close_cli0: xclose(c0); xclose(p0); - } static void unix_inet_skb_redir_to_connected(struct test_sockmap_listen *skel, -- 2.35.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v2 2/2] selftests/bpf: Add pairs_redir_to_connected helper 2023-10-05 7:21 ` [PATCH bpf-next v2 2/2] selftests/bpf: Add pairs_redir_to_connected helper Geliang Tang @ 2023-10-06 5:18 ` Martin KaFai Lau 2023-10-13 10:40 ` Jakub Sitnicki 0 siblings, 1 reply; 6+ messages in thread From: Martin KaFai Lau @ 2023-10-06 5:18 UTC (permalink / raw) To: Geliang Tang Cc: bpf, linux-kselftest, Andrii Nakryiko, Mykola Lysenko, Alexei Starovoitov, Daniel Borkmann, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa On 10/5/23 12:21 AM, Geliang Tang wrote: > --- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c > +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c > @@ -1336,32 +1336,22 @@ static void test_redir(struct test_sockmap_listen *skel, struct bpf_map *map, > } > } > > -static void unix_redir_to_connected(int sotype, int sock_mapfd, > - int verd_mapfd, enum redir_mode mode) > +static void pairs_redir_to_connected(int cli0, int peer0, int cli1, int peer1, > + 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); > > - if (socketpair(AF_UNIX, sotype | SOCK_NONBLOCK, 0, sfd)) > - return; > - c0 = sfd[0], p0 = sfd[1]; > - > - if (socketpair(AF_UNIX, sotype | SOCK_NONBLOCK, 0, sfd)) > - goto close0; > - c1 = sfd[0], p1 = sfd[1]; > - > - err = add_to_sockmap(sock_mapfd, p0, p1); > + err = add_to_sockmap(sock_mapfd, peer0, peer1); > if (err) > goto close; > > - n = write(c1, "a", 1); > + n = write(cli1, "a", 1); > if (n < 0) > FAIL_ERRNO("%s: write", log_prefix); > if (n == 0) > @@ -1376,16 +1366,34 @@ static void unix_redir_to_connected(int sotype, int sock_mapfd, > if (pass != 1) > FAIL("%s: want pass count 1, have %d", log_prefix, pass); > > - n = recv_timeout(mode == REDIR_INGRESS ? p0 : c0, &b, 1, 0, IO_TIMEOUT_SEC); > + n = recv_timeout(mode == REDIR_INGRESS ? peer0 : cli0, &b, 1, 0, IO_TIMEOUT_SEC); > if (n < 0) > FAIL_ERRNO("%s: recv_timeout", log_prefix); > if (n == 0) > FAIL("%s: incomplete recv", log_prefix); > > close: > - xclose(c1); > - xclose(p1); > -close0: > + xclose(cli1); > + xclose(peer1); > +} > + > +static void unix_redir_to_connected(int sotype, int sock_mapfd, > + int verd_mapfd, enum redir_mode mode) > +{ > + int c0, c1, p0, p1; > + int sfd[2]; > + > + if (socketpair(AF_UNIX, sotype | SOCK_NONBLOCK, 0, sfd)) > + return; > + c0 = sfd[0], p0 = sfd[1]; > + > + if (socketpair(AF_UNIX, sotype | SOCK_NONBLOCK, 0, sfd)) > + goto close; > + c1 = sfd[0], p1 = sfd[1]; > + > + pairs_redir_to_connected(c0, p0, c1, p1, sock_mapfd, verd_mapfd, mode); > + > +close: > xclose(c0); > xclose(p0); > } > @@ -1661,51 +1669,19 @@ static int inet_socketpair(int family, int type, int *s, int *c) > static void 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; > - u32 key; > - char b; > - > - zero_verdict_count(verd_mapfd); > + int err; > > err = inet_socketpair(family, SOCK_DGRAM, &p0, &c0); > if (err) > return; > err = inet_socketpair(family, SOCK_DGRAM, &p1, &c1); > if (err) > - goto close_cli0; > - > - 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); > + goto close; > > - n = recv_timeout(mode == REDIR_INGRESS ? p0 : c0, &b, 1, 0, IO_TIMEOUT_SEC); > - if (n < 0) > - FAIL_ERRNO("%s: recv_timeout", log_prefix); > - if (n == 0) > - FAIL("%s: incomplete recv", log_prefix); > + pairs_redir_to_connected(c0, p0, c1, p1, sock_mapfd, verd_mapfd, mode); > > -close_cli1: > - xclose(c1); > - xclose(p1); > -close_cli0: > +close: > xclose(c0); > xclose(p0); Patch 1 is applied. Thanks. In patch 2, the xclose() here is confusing after this change. It is also inconsistent from how other tests in sockmap_listen.c is doing it. c0/p0 and c1/p1 are opened here but only c0/p0 is closed here and c1/p1 is closed in the pairs_redir_to_connected() above instead. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v2 2/2] selftests/bpf: Add pairs_redir_to_connected helper 2023-10-06 5:18 ` Martin KaFai Lau @ 2023-10-13 10:40 ` Jakub Sitnicki 0 siblings, 0 replies; 6+ messages in thread From: Jakub Sitnicki @ 2023-10-13 10:40 UTC (permalink / raw) To: Martin KaFai Lau Cc: Geliang Tang, bpf, linux-kselftest, Andrii Nakryiko, Mykola Lysenko, Alexei Starovoitov, Daniel Borkmann, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa On Thu, Oct 05, 2023 at 10:18 PM -07, Martin KaFai Lau wrote: > On 10/5/23 12:21 AM, Geliang Tang wrote: >> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c >> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c >> @@ -1336,32 +1336,22 @@ static void test_redir(struct test_sockmap_listen *skel, struct bpf_map *map, >> } >> } >> -static void unix_redir_to_connected(int sotype, int sock_mapfd, >> - int verd_mapfd, enum redir_mode mode) >> +static void pairs_redir_to_connected(int cli0, int peer0, int cli1, int peer1, >> + 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); >> - if (socketpair(AF_UNIX, sotype | SOCK_NONBLOCK, 0, sfd)) >> - return; >> - c0 = sfd[0], p0 = sfd[1]; >> - >> - if (socketpair(AF_UNIX, sotype | SOCK_NONBLOCK, 0, sfd)) >> - goto close0; >> - c1 = sfd[0], p1 = sfd[1]; >> - >> - err = add_to_sockmap(sock_mapfd, p0, p1); >> + err = add_to_sockmap(sock_mapfd, peer0, peer1); >> if (err) >> goto close; >> - n = write(c1, "a", 1); >> + n = write(cli1, "a", 1); >> if (n < 0) >> FAIL_ERRNO("%s: write", log_prefix); >> if (n == 0) >> @@ -1376,16 +1366,34 @@ static void unix_redir_to_connected(int sotype, int sock_mapfd, >> if (pass != 1) >> FAIL("%s: want pass count 1, have %d", log_prefix, pass); >> - n = recv_timeout(mode == REDIR_INGRESS ? p0 : c0, &b, 1, 0, >> IO_TIMEOUT_SEC); >> + n = recv_timeout(mode == REDIR_INGRESS ? peer0 : cli0, &b, 1, 0, IO_TIMEOUT_SEC); >> if (n < 0) >> FAIL_ERRNO("%s: recv_timeout", log_prefix); >> if (n == 0) >> FAIL("%s: incomplete recv", log_prefix); >> close: >> - xclose(c1); >> - xclose(p1); >> -close0: >> + xclose(cli1); >> + xclose(peer1); >> +} >> + >> +static void unix_redir_to_connected(int sotype, int sock_mapfd, >> + int verd_mapfd, enum redir_mode mode) >> +{ >> + int c0, c1, p0, p1; >> + int sfd[2]; >> + >> + if (socketpair(AF_UNIX, sotype | SOCK_NONBLOCK, 0, sfd)) >> + return; >> + c0 = sfd[0], p0 = sfd[1]; >> + >> + if (socketpair(AF_UNIX, sotype | SOCK_NONBLOCK, 0, sfd)) >> + goto close; >> + c1 = sfd[0], p1 = sfd[1]; >> + >> + pairs_redir_to_connected(c0, p0, c1, p1, sock_mapfd, verd_mapfd, mode); >> + >> +close: >> xclose(c0); >> xclose(p0); >> } >> @@ -1661,51 +1669,19 @@ static int inet_socketpair(int family, int type, int *s, int *c) >> static void 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; >> - u32 key; >> - char b; >> - >> - zero_verdict_count(verd_mapfd); >> + int err; >> err = inet_socketpair(family, SOCK_DGRAM, &p0, &c0); >> if (err) >> return; >> err = inet_socketpair(family, SOCK_DGRAM, &p1, &c1); >> if (err) >> - goto close_cli0; >> - >> - 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); >> + goto close; >> - n = recv_timeout(mode == REDIR_INGRESS ? p0 : c0, &b, 1, 0, >> IO_TIMEOUT_SEC); >> - if (n < 0) >> - FAIL_ERRNO("%s: recv_timeout", log_prefix); >> - if (n == 0) >> - FAIL("%s: incomplete recv", log_prefix); >> + pairs_redir_to_connected(c0, p0, c1, p1, sock_mapfd, verd_mapfd, mode); >> -close_cli1: >> - xclose(c1); >> - xclose(p1); >> -close_cli0: >> +close: >> xclose(c0); >> xclose(p0); > > Patch 1 is applied. Thanks. > > In patch 2, the xclose() here is confusing after this change. It is also > inconsistent from how other tests in sockmap_listen.c is doing it. c0/p0 and > c1/p1 are opened here but only c0/p0 is closed here and c1/p1 is closed in the > pairs_redir_to_connected() above instead. I agree with Martin. The refactoring idea itself is nice. But, I also find it surprising that newly extracted pairs_redir_to_connected helper takes ownership of cli1 & peer1 FDs. For me, ideally, we would start using the __cleanup__ attribute, like we do now in the kernel code [1], to auto-close FDs on return. That would make error recovery and resource release less verbose. [1] https://elixir.bootlin.com/linux/v6.5/source/include/linux/cleanup.h ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v2 0/2] cleanups for sockmap_listen 2023-10-05 7:21 [PATCH bpf-next v2 0/2] cleanups for sockmap_listen Geliang Tang 2023-10-05 7:21 ` [PATCH bpf-next v2 1/2] selftests/bpf: Enable CONFIG_VSOCKETS in config Geliang Tang 2023-10-05 7:21 ` [PATCH bpf-next v2 2/2] selftests/bpf: Add pairs_redir_to_connected helper Geliang Tang @ 2023-10-06 5:10 ` patchwork-bot+netdevbpf 2 siblings, 0 replies; 6+ messages in thread From: patchwork-bot+netdevbpf @ 2023-10-06 5:10 UTC (permalink / raw) To: Geliang Tang Cc: andrii, mykolal, ast, daniel, martin.lau, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, bpf, linux-kselftest Hello: This series was applied to bpf/bpf-next.git (master) by Martin KaFai Lau <martin.lau@kernel.org>: On Thu, 5 Oct 2023 15:21:50 +0800 you wrote: > v2: > - rename c0/c1 to cli0/cli1, p0/p1 to peer0/perr1 as Daniel suggested. > > Two cleanups for sockmap_listen selftests: enable a kconfig and add a > new helper. > > Geliang Tang (2): > selftests/bpf: Enable CONFIG_VSOCKETS in config > selftests/bpf: Add pairs_redir_to_connected helper > > [...] Here is the summary with links: - [bpf-next,v2,1/2] selftests/bpf: Enable CONFIG_VSOCKETS in config https://git.kernel.org/bpf/bpf-next/c/d549854bc58f - [bpf-next,v2,2/2] selftests/bpf: Add pairs_redir_to_connected helper (no matching commit) You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-10-13 10:52 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-05 7:21 [PATCH bpf-next v2 0/2] cleanups for sockmap_listen Geliang Tang 2023-10-05 7:21 ` [PATCH bpf-next v2 1/2] selftests/bpf: Enable CONFIG_VSOCKETS in config Geliang Tang 2023-10-05 7:21 ` [PATCH bpf-next v2 2/2] selftests/bpf: Add pairs_redir_to_connected helper Geliang Tang 2023-10-06 5:18 ` Martin KaFai Lau 2023-10-13 10:40 ` Jakub Sitnicki 2023-10-06 5:10 ` [PATCH bpf-next v2 0/2] cleanups for sockmap_listen patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox