* [PATCH RFC] Support for SOL_IP for MPTCP setsockopt
@ 2021-10-20 19:56 Poorva Sonparote
2021-10-22 23:49 ` Mat Martineau
0 siblings, 1 reply; 3+ messages in thread
From: Poorva Sonparote @ 2021-10-20 19:56 UTC (permalink / raw)
To: mptcp; +Cc: dcaratti
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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH RFC] Support for SOL_IP for MPTCP setsockopt
2021-10-20 19:56 [PATCH RFC] Support for SOL_IP for MPTCP setsockopt Poorva Sonparote
@ 2021-10-22 23:49 ` Mat Martineau
[not found] ` <CADppAWAgBA4qaxA1CrbwhUpaoRkX-aZGfQ8EC=-KkM9eZOMztA@mail.gmail.com>
0 siblings, 1 reply; 3+ messages in thread
From: Mat Martineau @ 2021-10-22 23:49 UTC (permalink / raw)
To: Poorva Sonparote; +Cc: mptcp, dcaratti
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-10-25 13:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-10-20 19:56 [PATCH RFC] Support for SOL_IP for MPTCP setsockopt Poorva Sonparote
2021-10-22 23:49 ` Mat Martineau
[not found] ` <CADppAWAgBA4qaxA1CrbwhUpaoRkX-aZGfQ8EC=-KkM9eZOMztA@mail.gmail.com>
2021-10-25 13:52 ` Poorva Sonparote
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.