From: Martin KaFai Lau <martin.lau@linux.dev>
To: Geliang Tang <geliang.tang@suse.com>
Cc: bpf@vger.kernel.org, linux-kselftest@vger.kernel.org,
Andrii Nakryiko <andrii@kernel.org>,
Mykola Lysenko <mykolal@fb.com>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Song Liu <song@kernel.org>,
Yonghong Song <yonghong.song@linux.dev>,
John Fastabend <john.fastabend@gmail.com>,
KP Singh <kpsingh@kernel.org>,
Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>,
Jiri Olsa <jolsa@kernel.org>
Subject: Re: [PATCH bpf-next v2 2/2] selftests/bpf: Add pairs_redir_to_connected helper
Date: Thu, 5 Oct 2023 22:18:21 -0700 [thread overview]
Message-ID: <1124e2ba-2856-41c7-713f-5a4b4ffd3ec5@linux.dev> (raw)
In-Reply-To: <10920edc470974553e66e2391400dfa81ec03d47.1696490003.git.geliang.tang@suse.com>
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.
next prev parent reply other threads:[~2023-10-06 5:18 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
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=1124e2ba-2856-41c7-713f-5a4b4ffd3ec5@linux.dev \
--to=martin.lau@linux.dev \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=geliang.tang@suse.com \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kpsingh@kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=mykolal@fb.com \
--cc=sdf@google.com \
--cc=song@kernel.org \
--cc=yonghong.song@linux.dev \
/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.