net/mptcp/sockopt.c | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-)
Hello!
I have been working on adding support for IP_TOS for setsockopt(). I
am attaching the packetdrill test and the patch but it still won't set the TOS value
for the subflows that are created after the setsockopt() call.
https://github.com/multipath-tcp/mptcp_net-next/issues/220#issuecomment-947863717
Thanks,
Poorva
This patch adds support for IP_TOS for setsockopt(.. ,SOL_IP, ..)
https://github.com/multipath-tcp/mptcp_net-next/issues/220
Currently, any new subflows that are created after the setsockopt()
call are not being updated with the set IP_TOS value.
Signed-off-by: Poorva Sonparote <psonparo@redhat.com>
---
net/mptcp/sockopt.c | 39 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 8137cc3a4296..f5ee49660902 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -598,6 +598,40 @@ static int mptcp_setsockopt_sol_tcp_congestion(struct mptcp_sock *msk, sockptr_t
return ret;
}
+static int mptcp_setsockopt_sol_ip_set_tos(struct mptcp_sock *msk, int optname,
+ sockptr_t optval, unsigned int optlen)
+{
+ struct mptcp_subflow_context *subflow;
+ struct sock *sk = (struct sock *)msk;
+ int ret, err;
+ int val = 0;
+
+ err = ip_setsockopt(sk, SOL_IP, optname, optval, optlen);
+ val = inet_sk(sk)->tos;
+
+ if (err != 0)
+ return err;
+ lock_sock(sk);
+ mptcp_for_each_subflow(msk, subflow) {
+ struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+
+ ip_sock_set_tos(ssk, val);
+ }
+ release_sock(sk);
+ return ret;
+}
+
+static int mptcp_setsockopt_sol_ip(struct mptcp_sock *msk, int optname,
+ sockptr_t optval, unsigned int optlen)
+{
+ switch (optname) {
+ case IP_TOS:
+ return mptcp_setsockopt_sol_ip_set_tos(msk, optname, optval, optlen);
+ }
+
+ return -EOPNOTSUPP;
+}
+
static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
sockptr_t optval, unsigned int optlen)
{
@@ -637,12 +671,14 @@ int mptcp_setsockopt(struct sock *sk, int level, int optname,
if (ssk)
return tcp_setsockopt(ssk, level, optname, optval, optlen);
+ if (level == SOL_IP)
+ return mptcp_setsockopt_sol_ip(msk, optname, optval, optlen);
+
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;
}
@@ -1000,6 +1036,7 @@ static void sync_socket_options(struct mptcp_sock *msk, struct sock *ssk)
ssk->sk_priority = sk->sk_priority;
ssk->sk_bound_dev_if = sk->sk_bound_dev_if;
ssk->sk_incoming_cpu = sk->sk_incoming_cpu;
+ ip_sock_set_tos(ssk, inet_sk(sk)->tos);
if (sk->sk_userlocks & tx_rx_locks) {
ssk->sk_userlocks |= sk->sk_userlocks & tx_rx_locks;
--
2.18.2
On Wed, 20 Oct 2021, Poorva Sonparote wrote: > Hello! > > I have been working on adding support for IP_TOS for setsockopt(). I > am attaching the packetdrill test and the patch but it still won't set the TOS value > for the subflows that are created after the setsockopt() call. > > https://github.com/multipath-tcp/mptcp_net-next/issues/220#issuecomment-947863717 > > Thanks, > Poorva > > > > > This patch adds support for IP_TOS for setsockopt(.. ,SOL_IP, ..) > https://github.com/multipath-tcp/mptcp_net-next/issues/220 > > Currently, any new subflows that are created after the setsockopt() > call are not being updated with the set IP_TOS value. > Hi Poorva, Thanks for the patch! A 'Closes:' tag with the github issue link is also helpful here. > Signed-off-by: Poorva Sonparote <psonparo@redhat.com> > --- > net/mptcp/sockopt.c | 39 ++++++++++++++++++++++++++++++++++++++- > 1 file changed, 38 insertions(+), 1 deletion(-) > If you have more commentary that isn't part of the commit message you want to be permanently stored in git, this space (after the '---' line above but before the 'diff --git ...' below) is a good for that! 'git am' will drop this text when applying. This is especially not a big deal with RFC patches like this one, since they don't end up getting merged anyway. > diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c > index 8137cc3a4296..f5ee49660902 100644 > --- a/net/mptcp/sockopt.c > +++ b/net/mptcp/sockopt.c > @@ -598,6 +598,40 @@ static int mptcp_setsockopt_sol_tcp_congestion(struct mptcp_sock *msk, sockptr_t > return ret; > } > > +static int mptcp_setsockopt_sol_ip_set_tos(struct mptcp_sock *msk, int optname, > + sockptr_t optval, unsigned int optlen) > +{ > + struct mptcp_subflow_context *subflow; > + struct sock *sk = (struct sock *)msk; > + int ret, err; > + int val = 0; > + > + err = ip_setsockopt(sk, SOL_IP, optname, optval, optlen); > + val = inet_sk(sk)->tos; > + > + if (err != 0) > + return err; Minor - need a blank line here. > + lock_sock(sk); > + mptcp_for_each_subflow(msk, subflow) { > + struct sock *ssk = mptcp_subflow_tcp_sock(subflow); > + > + ip_sock_set_tos(ssk, val); > + } > + release_sock(sk); and here > + return ret; > +} > + > +static int mptcp_setsockopt_sol_ip(struct mptcp_sock *msk, int optname, > + sockptr_t optval, unsigned int optlen) > +{ > + switch (optname) { > + case IP_TOS: > + return mptcp_setsockopt_sol_ip_set_tos(msk, optname, optval, optlen); > + } > + > + return -EOPNOTSUPP; > +} > + > static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname, > sockptr_t optval, unsigned int optlen) > { > @@ -637,12 +671,14 @@ int mptcp_setsockopt(struct sock *sk, int level, int optname, > if (ssk) > return tcp_setsockopt(ssk, level, optname, optval, optlen); > > + if (level == SOL_IP) > + return mptcp_setsockopt_sol_ip(msk, optname, optval, optlen); > + > 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); > - Looks like an accidental line deletion here. > return -EOPNOTSUPP; > } > > @@ -1000,6 +1036,7 @@ static void sync_socket_options(struct mptcp_sock *msk, struct sock *ssk) > ssk->sk_priority = sk->sk_priority; > ssk->sk_bound_dev_if = sk->sk_bound_dev_if; > ssk->sk_incoming_cpu = sk->sk_incoming_cpu; > + ip_sock_set_tos(ssk, inet_sk(sk)->tos); I think this does sync the tos value to new subflows, right? Since the ssk is already locked, use __ip_sock_set_tos(). Maybe you ran in to a deadlock here that interfered with the syncing. > > if (sk->sk_userlocks & tx_rx_locks) { > ssk->sk_userlocks |= sk->sk_userlocks & tx_rx_locks; > -- > 2.18.2 > > > -- Mat Martineau Intel
© 2016 - 2024 Red Hat, Inc.