[PATCH mptcp net-next v2] Support for IP_TOS for MPTCP setsockopt()

[PATCH mptcp net-next v2] Support for IP_TOS for MPTCP setsockopt()

[Poorva Sonparote]

SOL_IP provides a way to configure network layer attributes in a
socket. This patch adds support for IP_TOS for setsockopt(.. ,SOL_IP, ..)
https://github.com/multipath-tcp/mptcp_net-next/issues/220

Support for SOL_IP is added in mptcp_setsockopt() and IP_TOS is handled in
a private function. The idea here is to take in the value passed for IP_TOS
and set it to the current subflow, open subflows as well new subflows that
might be created after the initial call to setsockopt(). This sync is done
using sync_socket_options(.., ssk) and setting the value of tos using
 __ip_sock_set_tos(ssk,..).
To set the value for all subflows associated, __ip_sock_set_tos(ssk,..) is
exposed in ip.h

The patch has been tested using the packetdrill script here -
https://github.com/multipath-tcp/mptcp_net-next/issues/220#issuecomment-947863717
It has not been selftested because there's no support for IP_TOS in
getsockopt() yet.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/220
Signed-off-by: Poorva Sonparote <psonparo@redhat.com>
---
Thanks Mat and Matthieu for the feedback! Kindly let me know if any changes
are required.

Notes:
    - Fixed formatting errors in ip.h and sockopt.c
    - Edited the commit message to add in more details.
    - The patch cannot be seltested because there's no
      support for IP_TOS in getsockopt() yet.
    - Changed the function name mptcp_setsockopt_sol_ip(..)
     to mptcp_setsockopt_v4(..).
    - Similarly, changed the function name mptcp_setsockopt_sol_ip_set_tos(..)
     to mptcp_setsockopt_v4_set_tos(..) for uniformity.

 include/net/ip.h       |  2 +-
 net/ipv4/ip_sockglue.c |  2 +-
 net/mptcp/sockopt.c    | 41 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/include/net/ip.h b/include/net/ip.h
index cf229a531194..e9e050f5af99 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -762,7 +762,6 @@ void ip_icmp_error(struct sock *sk, struct sk_buff *skb, int err, __be16 port,
 		   u32 info, u8 *payload);
 void ip_local_error(struct sock *sk, int err, __be32 daddr, __be16 dport,
 		    u32 info);
-
 static inline void ip_cmsg_recv(struct msghdr *msg, struct sk_buff *skb)
 {
 	ip_cmsg_recv_offset(msg, skb->sk, skb, 0, 0);
@@ -789,5 +788,6 @@ int ip_sock_set_mtu_discover(struct sock *sk, int val);
 void ip_sock_set_pktinfo(struct sock *sk);
 void ip_sock_set_recverr(struct sock *sk);
 void ip_sock_set_tos(struct sock *sk, int val);
+void  __ip_sock_set_tos(struct sock *sk, int val);
 
 #endif	/* _IP_H */
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index b297bb28556e..7fd83f14daae 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -576,7 +576,7 @@ int ip_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len)
 	return err;
 }
 
-static void __ip_sock_set_tos(struct sock *sk, int val)
+void __ip_sock_set_tos(struct sock *sk, int val)
 {
 	if (sk->sk_type == SOCK_STREAM) {
 		val &= ~INET_ECN_MASK;
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 8137cc3a4296..ded47f0483ed 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -598,6 +598,43 @@ static int mptcp_setsockopt_sol_tcp_congestion(struct mptcp_sock *msk, sockptr_t
 	return ret;
 }
 
+static int mptcp_setsockopt_v4_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, val;
+	int err;
+
+	err = ip_setsockopt(sk, SOL_IP, optname, optval, optlen);
+
+	if (err != 0)
+		return err;
+
+	lock_sock(sk);
+	sockopt_seq_inc(msk);
+	val = inet_sk(sk)->tos;
+	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_v4(struct mptcp_sock *msk, int optname,
+			       sockptr_t optval, unsigned int optlen)
+{
+	switch (optname) {
+	case IP_TOS:
+		return mptcp_setsockopt_v4_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,6 +674,9 @@ 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_v4(msk, optname, optval, optlen);
+
 	if (level == SOL_IPV6)
 		return mptcp_setsockopt_v6(msk, optname, optval, optlen);
 
@@ -1000,6 +1040,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

Re: [PATCH mptcp net-next v2] Support for IP_TOS for MPTCP setsockopt()

[Mat Martineau]

On Tue, 26 Oct 2021, Poorva Sonparote wrote:

> SOL_IP provides a way to configure network layer attributes in a
> socket. This patch adds support for IP_TOS for setsockopt(.. ,SOL_IP, ..)
> https://github.com/multipath-tcp/mptcp_net-next/issues/220
>
> Support for SOL_IP is added in mptcp_setsockopt() and IP_TOS is handled in
> a private function. The idea here is to take in the value passed for IP_TOS
> and set it to the current subflow, open subflows as well new subflows that
> might be created after the initial call to setsockopt(). This sync is done
> using sync_socket_options(.., ssk) and setting the value of tos using
> __ip_sock_set_tos(ssk,..).
> To set the value for all subflows associated, __ip_sock_set_tos(ssk,..) is
> exposed in ip.h
>
> The patch has been tested using the packetdrill script here -
> https://github.com/multipath-tcp/mptcp_net-next/issues/220#issuecomment-947863717
> It has not been selftested because there's no support for IP_TOS in
> getsockopt() yet.
>
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/220
> Signed-off-by: Poorva Sonparote <psonparo@redhat.com>
> ---
> Thanks Mat and Matthieu for the feedback! Kindly let me know if any changes
> are required.
>

Hi Poorva -

Thanks for the revision.

The main thing that still needs to be changed is that this should be two 
separate commits (and therefore multiple email messages): one commit for 
changes to expose __ip_sock_set_tos() and a separate commit for the 
changes in sockopt.c.

As with a lot of tasks in git, there are many ways to split a commit into 
multiple commits. I usually refer to the "SPLITTING COMMITS" section of 
the 'man git rebase' manual page. If you have more questions about this 
process, #mptcp on IRC is a good place to ask.


> Notes:
>    - Fixed formatting errors in ip.h and sockopt.c
>    - Edited the commit message to add in more details.
>    - The patch cannot be seltested because there's no
>      support for IP_TOS in getsockopt() yet.
>    - Changed the function name mptcp_setsockopt_sol_ip(..)
>     to mptcp_setsockopt_v4(..).
>    - Similarly, changed the function name mptcp_setsockopt_sol_ip_set_tos(..)
>     to mptcp_setsockopt_v4_set_tos(..) for uniformity.
>
> include/net/ip.h       |  2 +-
> net/ipv4/ip_sockglue.c |  2 +-
> net/mptcp/sockopt.c    | 41 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/ip.h b/include/net/ip.h
> index cf229a531194..e9e050f5af99 100644
> --- a/include/net/ip.h
> +++ b/include/net/ip.h
> @@ -762,7 +762,6 @@ void ip_icmp_error(struct sock *sk, struct sk_buff *skb, int err, __be16 port,
> 		   u32 info, u8 *payload);
> void ip_local_error(struct sock *sk, int err, __be32 daddr, __be16 dport,
> 		    u32 info);
> -

Still a deleted line to restore here.

Regards,

Mat


> static inline void ip_cmsg_recv(struct msghdr *msg, struct sk_buff *skb)
> {
> 	ip_cmsg_recv_offset(msg, skb->sk, skb, 0, 0);
> @@ -789,5 +788,6 @@ int ip_sock_set_mtu_discover(struct sock *sk, int val);
> void ip_sock_set_pktinfo(struct sock *sk);
> void ip_sock_set_recverr(struct sock *sk);
> void ip_sock_set_tos(struct sock *sk, int val);
> +void  __ip_sock_set_tos(struct sock *sk, int val);
>
> #endif	/* _IP_H */
> diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
> index b297bb28556e..7fd83f14daae 100644
> --- a/net/ipv4/ip_sockglue.c
> +++ b/net/ipv4/ip_sockglue.c
> @@ -576,7 +576,7 @@ int ip_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len)
> 	return err;
> }
>
> -static void __ip_sock_set_tos(struct sock *sk, int val)
> +void __ip_sock_set_tos(struct sock *sk, int val)
> {
> 	if (sk->sk_type == SOCK_STREAM) {
> 		val &= ~INET_ECN_MASK;
> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> index 8137cc3a4296..ded47f0483ed 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c
> @@ -598,6 +598,43 @@ static int mptcp_setsockopt_sol_tcp_congestion(struct mptcp_sock *msk, sockptr_t
> 	return ret;
> }
>
> +static int mptcp_setsockopt_v4_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, val;
> +	int err;
> +
> +	err = ip_setsockopt(sk, SOL_IP, optname, optval, optlen);
> +
> +	if (err != 0)
> +		return err;
> +
> +	lock_sock(sk);
> +	sockopt_seq_inc(msk);
> +	val = inet_sk(sk)->tos;
> +	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_v4(struct mptcp_sock *msk, int optname,
> +			       sockptr_t optval, unsigned int optlen)
> +{
> +	switch (optname) {
> +	case IP_TOS:
> +		return mptcp_setsockopt_v4_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,6 +674,9 @@ 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_v4(msk, optname, optval, optlen);
> +
> 	if (level == SOL_IPV6)
> 		return mptcp_setsockopt_v6(msk, optname, optval, optlen);
>
> @@ -1000,6 +1040,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
>
>

--
Mat Martineau
Intel

Re: [PATCH mptcp net-next v2] Support for IP_TOS for MPTCP setsockopt()

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2669 bytes --]

Hi Poorva,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Poorva-Sonparote/Support-for-IP_TOS-for-MPTCP-setsockopt/20211027-004553
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 36d935a0a67e4456bd84943319718448c9647675
config: riscv-buildonly-randconfig-r004-20211031 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 82ed106567063ea269c6d5669278b733e173a42f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/d5f0751cd6aa8cc9cbd7ce6cc11e39106127098e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Poorva-Sonparote/Support-for-IP_TOS-for-MPTCP-setsockopt/20211027-004553
        git checkout d5f0751cd6aa8cc9cbd7ce6cc11e39106127098e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=riscv 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> net/mptcp/sockopt.c:624:9: error: variable 'ret' is uninitialized when used here [-Werror,-Wuninitialized]
           return ret;
                  ^~~
   net/mptcp/sockopt.c:606:9: note: initialize the variable 'ret' to silence this warning
           int ret, val;
                  ^
                   = 0
   1 error generated.


vim +/ret +624 net/mptcp/sockopt.c

   600	
   601	static int mptcp_setsockopt_v4_set_tos(struct mptcp_sock *msk, int optname,
   602					       sockptr_t optval, unsigned int optlen)
   603	{
   604		struct mptcp_subflow_context *subflow;
   605		struct sock *sk = (struct sock *)msk;
   606		int ret, val;
   607		int err;
   608	
   609		err = ip_setsockopt(sk, SOL_IP, optname, optval, optlen);
   610	
   611		if (err != 0)
   612			return err;
   613	
   614		lock_sock(sk);
   615		sockopt_seq_inc(msk);
   616		val = inet_sk(sk)->tos;
   617		mptcp_for_each_subflow(msk, subflow) {
   618			struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
   619	
   620			__ip_sock_set_tos(ssk, val);
   621		}
   622		release_sock(sk);
   623	
 > 624		return ret;
   625	}
   626	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35962 bytes --]

Re: [PATCH mptcp net-next v2] Support for IP_TOS for MPTCP setsockopt()

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2741 bytes --]

Hi Poorva,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Poorva-Sonparote/Support-for-IP_TOS-for-MPTCP-setsockopt/20211027-004553
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 36d935a0a67e4456bd84943319718448c9647675
config: riscv-buildonly-randconfig-r004-20211031 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 82ed106567063ea269c6d5669278b733e173a42f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/d5f0751cd6aa8cc9cbd7ce6cc11e39106127098e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Poorva-Sonparote/Support-for-IP_TOS-for-MPTCP-setsockopt/20211027-004553
        git checkout d5f0751cd6aa8cc9cbd7ce6cc11e39106127098e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=riscv 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> net/mptcp/sockopt.c:624:9: error: variable 'ret' is uninitialized when used here [-Werror,-Wuninitialized]
           return ret;
                  ^~~
   net/mptcp/sockopt.c:606:9: note: initialize the variable 'ret' to silence this warning
           int ret, val;
                  ^
                   = 0
   1 error generated.


vim +/ret +624 net/mptcp/sockopt.c

   600	
   601	static int mptcp_setsockopt_v4_set_tos(struct mptcp_sock *msk, int optname,
   602					       sockptr_t optval, unsigned int optlen)
   603	{
   604		struct mptcp_subflow_context *subflow;
   605		struct sock *sk = (struct sock *)msk;
   606		int ret, val;
   607		int err;
   608	
   609		err = ip_setsockopt(sk, SOL_IP, optname, optval, optlen);
   610	
   611		if (err != 0)
   612			return err;
   613	
   614		lock_sock(sk);
   615		sockopt_seq_inc(msk);
   616		val = inet_sk(sk)->tos;
   617		mptcp_for_each_subflow(msk, subflow) {
   618			struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
   619	
   620			__ip_sock_set_tos(ssk, val);
   621		}
   622		release_sock(sk);
   623	
 > 624		return ret;
   625	}
   626	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 35962 bytes --]