From: Martin KaFai Lau <martin.lau@linux.dev>
To: Geliang Tang <geliang@kernel.org>
Cc: Andrii Nakryiko <andrii@kernel.org>,
Eduard Zingerman <eddyz87@gmail.com>,
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>, Shuah Khan <shuah@kernel.org>,
Geliang Tang <tanggeliang@kylinos.cn>,
bpf@vger.kernel.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH bpf-next 4/6] selftests/bpf: Add setsockopt for network_helper_opts
Date: Tue, 30 Apr 2024 17:40:40 -0700 [thread overview]
Message-ID: <f5148f33-cb4f-4e84-8182-b68a9c038d3e@linux.dev> (raw)
In-Reply-To: <0f676d51126bf7c260a71cfb60df0d1acb23e552.1714014697.git.tanggeliang@kylinos.cn>
On 4/24/24 8:23 PM, Geliang Tang wrote:
> +static int setsockopt_reuseport(int fd, const void *optval, socklen_t optlen)
> +{
> + return setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, optval, optlen);
> }
>
[ ... ]
> void free_fds(int *fds, unsigned int nr_close_fds)
> diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h
> index c62b54daa914..540ecfc52bd7 100644
> --- a/tools/testing/selftests/bpf/network_helpers.h
> +++ b/tools/testing/selftests/bpf/network_helpers.h
> @@ -28,6 +28,9 @@ struct network_helper_opts {
> bool noconnect;
> int type;
> int proto;
> + int (*setsockopt)(int fd, const void *optval, socklen_t optlen);
> + const void *optval;
> + socklen_t optlen;
optval and optlen could be in the stack of the (*setsockopt) callback.
e.g. the "int on;" could be local to the setsockopt_reuseport() instead of
adding optval/len to the network_helper_opts. Passing one optval in
network_helper_opts could be less flexible when we want to do multiple
setsockopt() after socket().
Another nit I would like to make, rename this from (*setsockopt) to
(*post_socket_cb) because this callback could do more than setsockopt, e.g.
adding a sk local storage to a socket fd before bind(). Also, add a "const
struct post_socket_opts *opts" for future extension, Like:
struct post_socket_opts {};
int (*post_socket_cb)(int fd, const struct post_socket_opts *opts);
Patch 6 will need two setsockopt cb functions because of different optval but I
believe the tradeoff is worth it for this callback doing more than just one
setsockopt.
Patch 1 to 3 have been applied. Thanks.
next prev parent reply other threads:[~2024-05-01 0:40 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-25 3:23 [PATCH bpf-next 0/6] use network helpers, part 3 Geliang Tang
2024-04-25 3:23 ` [PATCH bpf-next 1/6] selftests/bpf: Add opts argument for __start_server Geliang Tang
2024-04-25 3:23 ` [PATCH bpf-next 2/6] selftests/bpf: Make start_mptcp_server static Geliang Tang
2024-04-25 3:23 ` [PATCH bpf-next 3/6] selftests/bpf: Drop start_server_proto helper Geliang Tang
2024-04-25 3:23 ` [PATCH bpf-next 4/6] selftests/bpf: Add setsockopt for network_helper_opts Geliang Tang
2024-05-01 0:40 ` Martin KaFai Lau [this message]
2024-04-25 3:23 ` [PATCH bpf-next 5/6] selftests/bpf: Use start_server_addr in sockopt_inherit Geliang Tang
2024-04-25 3:23 ` [PATCH bpf-next 6/6] selftests/bpf: Use start_server_addr in test_tcp_check_syncookie Geliang Tang
2024-05-01 0:20 ` [PATCH bpf-next 0/6] use network helpers, part 3 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=f5148f33-cb4f-4e84-8182-b68a9c038d3e@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=eddyz87@gmail.com \
--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=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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).