From: Eduard Zingerman <eddyz87@gmail.com>
To: Geliang Tang <geliang@kernel.org>,
Andrii Nakryiko <andrii@kernel.org>,
Mykola Lysenko <mykolal@fb.com>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Martin KaFai Lau <martin.lau@linux.dev>,
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>, Shuah Khan <shuah@kernel.org>
Cc: Geliang Tang <tanggeliang@kylinos.cn>,
bpf@vger.kernel.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH bpf-next v4 6/9] selftests/bpf: Invoke attach_reuseport out of make_server
Date: Mon, 24 Jun 2024 17:59:49 -0700 [thread overview]
Message-ID: <003488efcca0c1205d3091242b2a106bc971f8a1.camel@gmail.com> (raw)
In-Reply-To: <f846835a758f2e531026083253eeb747205bd0dd.1719203293.git.tanggeliang@kylinos.cn>
On Mon, 2024-06-24 at 12:33 +0800, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> In order to facilitate subsequent commits to drop make_server(), this patch
> invokes attach_reuseport() out of make_server(), right after invoking
> make_server() if the passed "reuseport_prog" argument is not NULL.
>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> .../selftests/bpf/prog_tests/sk_lookup.c | 21 +++++++++----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c
> index de2466547efe..d87dfcf5db07 100644
Tbh, I don't like this refactoring for sk_lookup, it does not seem to
make the code clearer or shorter (e.g. 1414 LOC vs 1409).
If anything, it looks like reuseport_prog, callbacks setup and
start_server_str could be hidden insider make_server().
> --- a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c
> @@ -204,15 +204,6 @@ static int make_server(int sotype, const char *ip, int port,
> }
> }
>
> - /* Late attach reuseport prog so we can have one init path */
> - if (reuseport_prog) {
> - err = attach_reuseport(fd, reuseport_prog);
> - if (CHECK(err, "attach_reuseport", "failed\n")) {
> - log_err("failed to attach reuseport prog");
> - goto fail;
> - }
> - }
> -
> return fd;
> fail:
> close(fd);
> @@ -610,7 +601,8 @@ static void run_lookup_prog(const struct test *t)
> server_fds[i] = make_server(t->sotype, t->listen_at.ip,
> t->listen_at.port,
> t->reuseport_prog);
> - if (server_fds[i] < 0)
> + if (server_fds[i] < 0 ||
> + attach_reuseport(server_fds[i], t->reuseport_prog))
> goto close;
>
> err = update_lookup_map(t->sock_map, i, server_fds[i]);
> @@ -636,7 +628,8 @@ static void run_lookup_prog(const struct test *t)
> reuse_conn_fd = make_server(t->sotype, t->listen_at.ip,
> t->listen_at.port,
> t->reuseport_prog);
> - if (reuse_conn_fd < 0)
> + if (reuse_conn_fd < 0 ||
> + attach_reuseport(reuse_conn_fd, t->reuseport_prog))
> goto close;
>
> /* Connect the extra socket to itself */
> @@ -878,6 +871,9 @@ static void drop_on_lookup(const struct test *t)
> if (server_fd < 0)
> goto detach;
>
> + if (attach_reuseport(server_fd, t->reuseport_prog))
> + goto close_srv;
> +
> client_fd = make_socket(t->sotype, t->connect_to.ip,
> t->connect_to.port, &dst);
> if (client_fd < 0)
> @@ -992,6 +988,9 @@ static void drop_on_reuseport(const struct test *t)
> if (server1 < 0)
> goto detach;
>
> + if (attach_reuseport(server1, t->reuseport_prog))
> + goto close_srv1;
> +
> err = update_lookup_map(t->sock_map, SERVER_A, server1);
> if (err)
> goto close_srv1;
next prev parent reply other threads:[~2024-06-25 0:59 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-24 4:33 [PATCH bpf-next v4 0/9] use network helpers, part 8 Geliang Tang
2024-06-24 4:33 ` [PATCH bpf-next v4 1/9] selftests/bpf: Add backlog for network_helper_opts Geliang Tang
2024-06-24 4:33 ` [PATCH bpf-next v4 2/9] selftests/bpf: Use start_server_str in sockmap_ktls Geliang Tang
2024-06-24 4:33 ` [PATCH bpf-next v4 3/9] selftests/bpf: Use connect_to_fd " Geliang Tang
2024-06-24 4:33 ` [PATCH bpf-next v4 4/9] selftests/bpf: Use make_sockaddr " Geliang Tang
2024-06-24 21:21 ` Eduard Zingerman
2024-06-24 4:33 ` [PATCH bpf-next v4 5/9] selftests/bpf: Close fd in error path in drop_on_reuseport Geliang Tang
2024-06-24 21:32 ` Eduard Zingerman
2024-06-24 4:33 ` [PATCH bpf-next v4 6/9] selftests/bpf: Invoke attach_reuseport out of make_server Geliang Tang
2024-06-25 0:59 ` Eduard Zingerman [this message]
2024-06-24 4:33 ` [PATCH bpf-next v4 7/9] selftests/bpf: Use start_server_str in sk_lookup Geliang Tang
2024-06-24 4:33 ` [PATCH bpf-next v4 8/9] selftests/bpf: Use connect_to_fd " Geliang Tang
2024-06-24 4:33 ` [PATCH bpf-next v4 9/9] selftests/bpf: Use connect_to_addr " Geliang Tang
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=003488efcca0c1205d3091242b2a106bc971f8a1.camel@gmail.com \
--to=eddyz87@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=geliang@kernel.org \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kpsingh@kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=martin.lau@linux.dev \
--cc=mykolal@fb.com \
--cc=sdf@google.com \
--cc=shuah@kernel.org \
--cc=song@kernel.org \
--cc=tanggeliang@kylinos.cn \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox