| Message ID | 20210317163828.27406-9-fw@strlen.de |
|---|---|
| State | Superseded, archived |
| Delegated to: | Paolo Abeni |
| Headers | show |
| Series | initial SOL_SOCKET support | expand |
On Wed, 2021-03-17 at 17:38 +0100, Florian Westphal wrote: > Make this interact with the initial subflow only, as that is the one > exposed to userspace (connect, bind). I'm wondering if we should instead propagate the CG algo to all subflows ??! I see this is possibly subject to personal taste - and mine is notoriously not good ;) Do you have strong preference here? Thanks! Paolo
On Wed, 2021-03-17 at 17:38 +0100, Florian Westphal wrote: > @@ -585,6 +625,47 @@ int mptcp_setsockopt(struct sock *sk, int level, int optname, > if (level == SOL_IPV6) > return mptcp_setsockopt_v6(msk, optname, optval, optlen); > > + if (level == SOL_TCP) > + return mptcp_setsockopt_sol_tcp(msk, optname, optval, optlen); > + > + return -EOPNOTSUPP; > +} Side notes not really related to this patch, but to the code near here;) I think we could call __mptcp_check_fallback() before acquiring the lock, so that in the non fallback case we could avoid acquring the msk lock. I think we also need to sync also sk_reuseport/sk_reuse/sk_ipv6only ?!? I don't see where msk->setsockopt_seq is incremented. I expected to find that on successful mptcp_setsockopt() but I missed the relevant code ?!? /P
On Wed, 17 Mar 2021, Florian Westphal wrote: > Make this interact with the initial subflow only, as that is the one > exposed to userspace (connect, bind). I would apply this one to all subflows. It's easier to explain to users that they're setting the congestion algorithm for all subflows rather than having one configurable subflow and all the rest are stuck on the default. Later on we will need to figure out a way to set certain options (like congestion?) on specific subflows, since each subflow could be using 5G or Wi-Fi or wired connections that might benefit from different configurations. -- Mat Martineau Intel
Hi Florian, Mat, On 20/03/2021 00:41, Mat Martineau wrote: > > On Wed, 17 Mar 2021, Florian Westphal wrote: > >> Make this interact with the initial subflow only, as that is the one >> exposed to userspace (connect, bind). > > I would apply this one to all subflows. It's easier to explain to users > that they're setting the congestion algorithm for all subflows rather > than having one configurable subflow and all the rest are stuck on the > default. Indeed, I think by default, we should apply a maximum of settings to all subflows. > Later on we will need to figure out a way to set certain options (like > congestion?) on specific subflows, since each subflow could be using 5G > or Wi-Fi or wired connections that might benefit from different > configurations. If we want a different configuration per subflow, I think it makes more sense to use a BPF program to set different sockopt per path, no? Note that for TCP CC and other options, we can also have a different value per destination IP by using "ip route" with options, e.g. congctl. That's another way to configure a different CC per subflow. Cheers, Matt
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c index 3ba31f4a73e3..c1ffc3603b4f 100644 --- a/net/mptcp/sockopt.c +++ b/net/mptcp/sockopt.c @@ -556,6 +556,46 @@ static bool mptcp_supported_sockopt(int level, int optname) return false; } +static int mptcp_setsockopt_first_sf_only(struct mptcp_sock *msk, int level, int optname, + sockptr_t optval, unsigned int optlen) +{ + struct sock *sk = (struct sock *)msk; + struct socket *ssock; + int ret = -EINVAL; + struct sock *ssk; + + lock_sock(sk); + ssk = msk->first; + if (ssk) + goto do_sockopt; + + ssock = __mptcp_nmpc_socket(msk); + if (!ssock) + goto out; + + ssk = ssock->sk; +do_sockopt: + ret = tcp_setsockopt(ssk, level, optname, optval, optlen); +out: + + release_sock(sk); + return ret; +} + +static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname, + sockptr_t optval, unsigned int optlen) +{ + switch (optname) { + case TCP_ULP: + return -EOPNOTSUPP; + case TCP_CONGESTION: + return mptcp_setsockopt_first_sf_only(msk, SOL_TCP, optname, + optval, optlen); + } + + return -EOPNOTSUPP; +} + int mptcp_setsockopt(struct sock *sk, int level, int optname, sockptr_t optval, unsigned int optlen) { @@ -585,6 +625,47 @@ int mptcp_setsockopt(struct sock *sk, int level, int optname, if (level == SOL_IPV6) return mptcp_setsockopt_v6(msk, optname, optval, optlen); + if (level == SOL_TCP) + return mptcp_setsockopt_sol_tcp(msk, optname, optval, optlen); + + return -EOPNOTSUPP; +} + +static int mptcp_getsockopt_first_sf_only(struct mptcp_sock *msk, int level, int optname, + char __user *optval, int __user *optlen) +{ + struct sock *sk = (struct sock *)msk; + struct socket *ssock; + int ret = -EINVAL; + struct sock *ssk; + + lock_sock(sk); + ssk = msk->first; + if (ssk) { + ret = tcp_getsockopt(ssk, level, optname, optval, optlen); + goto out; + } + + ssock = __mptcp_nmpc_socket(msk); + if (!ssock) + goto out; + + ret = tcp_getsockopt(ssock->sk, level, optname, optval, optlen); + +out: + release_sock(sk); + return ret; +} + +static int mptcp_getsockopt_sol_tcp(struct mptcp_sock *msk, int optname, + char __user *optval, int __user *optlen) +{ + switch (optname) { + case TCP_ULP: + case TCP_CONGESTION: + return mptcp_getsockopt_first_sf_only(msk, SOL_TCP, optname, + optval, optlen); + } return -EOPNOTSUPP; } @@ -608,6 +689,8 @@ int mptcp_getsockopt(struct sock *sk, int level, int optname, if (ssk) return tcp_getsockopt(ssk, level, optname, optval, option); + if (level == SOL_TCP) + return mptcp_getsockopt_sol_tcp(msk, optname, optval, option); return -EOPNOTSUPP; }
Make this interact with the initial subflow only, as that is the one exposed to userspace (connect, bind). Signed-off-by: Florian Westphal <fw@strlen.de> --- net/mptcp/sockopt.c | 83 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+)