net/mptcp/sockopt.c | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-)
mptcp_setsockopt_sol_tcp_defer() was doing the same thing as
mptcp_setsockopt_first_sf_only() except for the returned code in case of
error.
Ignoring the error is needed to mimic how TCP_DEFER_ACCEPT is handled
when used with "plain" TCP sockets.
The specific function for TCP_DEFER_ACCEPT can be replaced by the new
mptcp_setsockopt_first_sf_only() helper and errors can be ignored to
stay compatible with TCP. A bit of cleanup.
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Suggested-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
net/mptcp/sockopt.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 1857281a0dd5..f85e9bbfe86f 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -758,18 +758,6 @@ static int mptcp_setsockopt_v4(struct mptcp_sock *msk, int optname,
return -EOPNOTSUPP;
}
-static int mptcp_setsockopt_sol_tcp_defer(struct mptcp_sock *msk, sockptr_t optval,
- unsigned int optlen)
-{
- struct socket *listener;
-
- listener = __mptcp_nmpc_socket(msk);
- if (!listener)
- return 0; /* TCP_DEFER_ACCEPT does not fail */
-
- return tcp_setsockopt(listener->sk, SOL_TCP, TCP_DEFER_ACCEPT, optval, optlen);
-}
-
static int mptcp_setsockopt_first_sf_only(struct mptcp_sock *msk, int level, int optname,
sockptr_t optval, unsigned int optlen)
{
@@ -810,7 +798,9 @@ static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
case TCP_NODELAY:
return mptcp_setsockopt_sol_tcp_nodelay(msk, optval, optlen);
case TCP_DEFER_ACCEPT:
- return mptcp_setsockopt_sol_tcp_defer(msk, optval, optlen);
+ /* See tcp.c: TCP_DEFER_ACCEPT does not fail */
+ mptcp_setsockopt_first_sf_only(msk, SOL_TCP, optname, optval, optlen);
+ return 0;
case TCP_FASTOPEN_CONNECT:
case TCP_FASTOPEN_NO_COOKIE:
return mptcp_setsockopt_first_sf_only(msk, SOL_TCP, optname,
base-commit: 6711fbb5e6c8541f84685542127455c18145d559
--
2.37.2
Hi Paolo, Mat, On 28/09/2022 11:18, Matthieu Baerts wrote: > mptcp_setsockopt_sol_tcp_defer() was doing the same thing as > mptcp_setsockopt_first_sf_only() except for the returned code in case of > error. > > Ignoring the error is needed to mimic how TCP_DEFER_ACCEPT is handled > when used with "plain" TCP sockets. > > The specific function for TCP_DEFER_ACCEPT can be replaced by the new > mptcp_setsockopt_first_sf_only() helper and errors can be ignored to > stay compatible with TCP. A bit of cleanup. Thank you both for your review! I just applied this patch in our tree (feat. for net-next) without Paolo's Suggested-by tag. New patches for t/upstream: - db52578cbc69: mptcp: sockopt: use new helper for TCP_DEFER_ACCEPT - Results: 6ae98b87e288..a34025ee429c (export) Tests are now in progress: https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20220930T081708 Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net
On Wed, 28 Sep 2022, Matthieu Baerts wrote: > mptcp_setsockopt_sol_tcp_defer() was doing the same thing as > mptcp_setsockopt_first_sf_only() except for the returned code in case of > error. > > Ignoring the error is needed to mimic how TCP_DEFER_ACCEPT is handled > when used with "plain" TCP sockets. > > The specific function for TCP_DEFER_ACCEPT can be replaced by the new > mptcp_setsockopt_first_sf_only() helper and errors can be ignored to > stay compatible with TCP. A bit of cleanup. > > Suggested-by: Paolo Abeni <pabeni@redhat.com> > Suggested-by: Mat Martineau <mathew.j.martineau@linux.intel.com> > Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net> Looks good to me, thanks! Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com> > --- > net/mptcp/sockopt.c | 16 +++------------- > 1 file changed, 3 insertions(+), 13 deletions(-) > > diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c > index 1857281a0dd5..f85e9bbfe86f 100644 > --- a/net/mptcp/sockopt.c > +++ b/net/mptcp/sockopt.c > @@ -758,18 +758,6 @@ static int mptcp_setsockopt_v4(struct mptcp_sock *msk, int optname, > return -EOPNOTSUPP; > } > > -static int mptcp_setsockopt_sol_tcp_defer(struct mptcp_sock *msk, sockptr_t optval, > - unsigned int optlen) > -{ > - struct socket *listener; > - > - listener = __mptcp_nmpc_socket(msk); > - if (!listener) > - return 0; /* TCP_DEFER_ACCEPT does not fail */ > - > - return tcp_setsockopt(listener->sk, SOL_TCP, TCP_DEFER_ACCEPT, optval, optlen); > -} > - > static int mptcp_setsockopt_first_sf_only(struct mptcp_sock *msk, int level, int optname, > sockptr_t optval, unsigned int optlen) > { > @@ -810,7 +798,9 @@ static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname, > case TCP_NODELAY: > return mptcp_setsockopt_sol_tcp_nodelay(msk, optval, optlen); > case TCP_DEFER_ACCEPT: > - return mptcp_setsockopt_sol_tcp_defer(msk, optval, optlen); > + /* See tcp.c: TCP_DEFER_ACCEPT does not fail */ > + mptcp_setsockopt_first_sf_only(msk, SOL_TCP, optname, optval, optlen); > + return 0; > case TCP_FASTOPEN_CONNECT: > case TCP_FASTOPEN_NO_COOKIE: > return mptcp_setsockopt_first_sf_only(msk, SOL_TCP, optname, > > base-commit: 6711fbb5e6c8541f84685542127455c18145d559 > -- > 2.37.2 > > -- Mat Martineau Intel
Hi Matthieu, Thank you for your modifications, that's great! Our CI did some validations and here is its report: - KVM Validation: normal: - Success! ✅: - Task: https://cirrus-ci.com/task/4607251246743552 - Summary: https://api.cirrus-ci.com/v1/artifact/task/4607251246743552/summary/summary.txt - KVM Validation: debug: - Success! ✅: - Task: https://cirrus-ci.com/task/5733151153586176 - Summary: https://api.cirrus-ci.com/v1/artifact/task/5733151153586176/summary/summary.txt Initiator: Patchew Applier Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/3d4592725a0d If there are some issues, you can reproduce them using the same environment as the one used by the CI thanks to a docker image, e.g.: $ cd [kernel source code] $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \ --pull always mptcp/mptcp-upstream-virtme-docker:latest \ auto-debug For more details: https://github.com/multipath-tcp/mptcp-upstream-virtme-docker Please note that despite all the efforts that have been already done to have a stable tests suite when executed on a public CI like here, it is possible some reported issues are not due to your modifications. Still, do not hesitate to help us improve that ;-) Cheers, MPTCP GH Action bot Bot operated by Matthieu Baerts (Tessares)
On Wed, 2022-09-28 at 11:18 +0200, Matthieu Baerts wrote: > mptcp_setsockopt_sol_tcp_defer() was doing the same thing as > mptcp_setsockopt_first_sf_only() except for the returned code in case of > error. > > Ignoring the error is needed to mimic how TCP_DEFER_ACCEPT is handled > when used with "plain" TCP sockets. > > The specific function for TCP_DEFER_ACCEPT can be replaced by the new > mptcp_setsockopt_first_sf_only() helper and errors can be ignored to > stay compatible with TCP. A bit of cleanup. > > Suggested-by: Paolo Abeni <pabeni@redhat.com> You can/should drop the above tag ;) > Suggested-by: Mat Martineau <mathew.j.martineau@linux.intel.com> > Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net> > --- > net/mptcp/sockopt.c | 16 +++------------- > 1 file changed, 3 insertions(+), 13 deletions(-) > > diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c > index 1857281a0dd5..f85e9bbfe86f 100644 > --- a/net/mptcp/sockopt.c > +++ b/net/mptcp/sockopt.c > @@ -758,18 +758,6 @@ static int mptcp_setsockopt_v4(struct mptcp_sock *msk, int optname, > return -EOPNOTSUPP; > } > > -static int mptcp_setsockopt_sol_tcp_defer(struct mptcp_sock *msk, sockptr_t optval, > - unsigned int optlen) > -{ > - struct socket *listener; > - > - listener = __mptcp_nmpc_socket(msk); > - if (!listener) > - return 0; /* TCP_DEFER_ACCEPT does not fail */ > - > - return tcp_setsockopt(listener->sk, SOL_TCP, TCP_DEFER_ACCEPT, optval, optlen); > -} > - > static int mptcp_setsockopt_first_sf_only(struct mptcp_sock *msk, int level, int optname, > sockptr_t optval, unsigned int optlen) > { > @@ -810,7 +798,9 @@ static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname, > case TCP_NODELAY: > return mptcp_setsockopt_sol_tcp_nodelay(msk, optval, optlen); > case TCP_DEFER_ACCEPT: > - return mptcp_setsockopt_sol_tcp_defer(msk, optval, optlen); > + /* See tcp.c: TCP_DEFER_ACCEPT does not fail */ > + mptcp_setsockopt_first_sf_only(msk, SOL_TCP, optname, optval, optlen); > + return 0; > case TCP_FASTOPEN_CONNECT: > case TCP_FASTOPEN_NO_COOKIE: > return mptcp_setsockopt_first_sf_only(msk, SOL_TCP, optname, > > base-commit: 6711fbb5e6c8541f84685542127455c18145d559 Nice! LGTM! Acked-by: Paolo Abeni <pabeni@redhat.com>
Hi Paolo, On 28/09/2022 11:46, Paolo Abeni wrote: > On Wed, 2022-09-28 at 11:18 +0200, Matthieu Baerts wrote: >> mptcp_setsockopt_sol_tcp_defer() was doing the same thing as >> mptcp_setsockopt_first_sf_only() except for the returned code in case of >> error. >> >> Ignoring the error is needed to mimic how TCP_DEFER_ACCEPT is handled >> when used with "plain" TCP sockets. >> >> The specific function for TCP_DEFER_ACCEPT can be replaced by the new >> mptcp_setsockopt_first_sf_only() helper and errors can be ignored to >> stay compatible with TCP. A bit of cleanup. >> >> Suggested-by: Paolo Abeni <pabeni@redhat.com> > > You can/should drop the above tag ;) You suggested to remove mptcp_setsockopt_sol_tcp_defer() but I understand your request :) Thank you for the review! I can wait for Mat to review it as he suggested the whole code :-) Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net
© 2016 - 2024 Red Hat, Inc.