[PATCH mptcp-next v2 0/3] Add TCP_MAXSEG socket option support

[PATCH mptcp-next v2 0/3] Add TCP_MAXSEG socket option support

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

v2:
 - add a new helper tcp_sock_set_maxseg.
 - add maxseg member of struct mptcp_sock.
 - add mptcp_setsockopt_all_subflows helper.
 - invoke mptcp_setsockopt_all_subflows under the msk socket lock.
 - drop mptcp_getsockopt_sol_tcp_maxseg, use mptcp_put_int_option
   instead.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/515

Geliang Tang (3):
  tcp: add tcp_sock_set_maxseg
  mptcp: add TCP_MAXSEG sockopt support
  selftests: mptcp: sockopt: add TCP_MAXSEG sockopt tests

 include/linux/tcp.h                           |  1 +
 net/ipv4/tcp.c                                | 24 ++++++++++++-------
 net/mptcp/protocol.h                          |  1 +
 net/mptcp/sockopt.c                           | 24 +++++++++++++++++++
 .../selftests/net/mptcp/mptcp_sockopt.c       | 22 +++++++++++++++++
 5 files changed, 63 insertions(+), 9 deletions(-)

-- 
2.43.0

[PATCH mptcp-next v2 1/3] tcp: add tcp_sock_set_maxseg

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Add a helper tcp_sock_set_maxseg() to directly set the TCP_MAXSEG sockopt
from kernel space.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 include/linux/tcp.h |  1 +
 net/ipv4/tcp.c      | 24 +++++++++++++++---------
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 1669d95bb0f9..0041bc0aaee9 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -623,6 +623,7 @@ void tcp_sock_set_nodelay(struct sock *sk);
 void tcp_sock_set_quickack(struct sock *sk, int val);
 int tcp_sock_set_syncnt(struct sock *sk, int val);
 int tcp_sock_set_user_timeout(struct sock *sk, int val);
+int tcp_sock_set_maxseg(struct sock *sk, int val);
 
 static inline bool dst_tcp_usec_ts(const struct dst_entry *dst)
 {
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 4516f93f6cd2..402f4a9b3fd1 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3725,6 +3725,20 @@ int tcp_set_window_clamp(struct sock *sk, int val)
 	return 0;
 }
 
+int tcp_sock_set_maxseg(struct sock *sk, int val)
+{
+	/* Values greater than interface MTU won't take effect. However
+	 * at the point when this call is done we typically don't yet
+	 * know which interface is going to be used
+	 */
+	if (val && (val < TCP_MIN_MSS || val > MAX_TCP_WINDOW))
+		return -EINVAL;
+
+	tcp_sk(sk)->rx_opt.user_mss = val;
+	return 0;
+}
+EXPORT_SYMBOL(tcp_sock_set_maxseg);
+
 /*
  *	Socket option code for TCP.
  */
@@ -3857,15 +3871,7 @@ int do_tcp_setsockopt(struct sock *sk, int level, int optname,
 
 	switch (optname) {
 	case TCP_MAXSEG:
-		/* Values greater than interface MTU won't take effect. However
-		 * at the point when this call is done we typically don't yet
-		 * know which interface is going to be used
-		 */
-		if (val && (val < TCP_MIN_MSS || val > MAX_TCP_WINDOW)) {
-			err = -EINVAL;
-			break;
-		}
-		tp->rx_opt.user_mss = val;
+		tcp_sock_set_maxseg(sk, val);
 		break;
 
 	case TCP_NODELAY:
-- 
2.43.0

Re: [PATCH mptcp-next v2 1/3] tcp: add tcp_sock_set_maxseg

[Matthieu Baerts]

Hi Geliang,

On 23/04/2025 11:40, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Add a helper tcp_sock_set_maxseg() to directly set the TCP_MAXSEG sockopt
> from kernel space.

Probably clearer to add:

  This new helper will be used in the following patch.

> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  include/linux/tcp.h |  1 +
>  net/ipv4/tcp.c      | 24 +++++++++++++++---------
>  2 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index 1669d95bb0f9..0041bc0aaee9 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -623,6 +623,7 @@ void tcp_sock_set_nodelay(struct sock *sk);
>  void tcp_sock_set_quickack(struct sock *sk, int val);
>  int tcp_sock_set_syncnt(struct sock *sk, int val);
>  int tcp_sock_set_user_timeout(struct sock *sk, int val);
> +int tcp_sock_set_maxseg(struct sock *sk, int val);
>  
>  static inline bool dst_tcp_usec_ts(const struct dst_entry *dst)
>  {
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 4516f93f6cd2..402f4a9b3fd1 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -3725,6 +3725,20 @@ int tcp_set_window_clamp(struct sock *sk, int val)
>  	return 0;
>  }
>  
> +int tcp_sock_set_maxseg(struct sock *sk, int val)
> +{
> +	/* Values greater than interface MTU won't take effect. However
> +	 * at the point when this call is done we typically don't yet
> +	 * know which interface is going to be used
> +	 */
> +	if (val && (val < TCP_MIN_MSS || val > MAX_TCP_WINDOW))
> +		return -EINVAL;
> +
> +	tcp_sk(sk)->rx_opt.user_mss = val;
> +	return 0;
> +}
> +EXPORT_SYMBOL(tcp_sock_set_maxseg);

This export is not needed: it is only needed when used from a module.

The rest looks OK to me:

Acked-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

> +
>  /*
>   *	Socket option code for TCP.
>   */
> @@ -3857,15 +3871,7 @@ int do_tcp_setsockopt(struct sock *sk, int level, int optname,
>  
>  	switch (optname) {
>  	case TCP_MAXSEG:
> -		/* Values greater than interface MTU won't take effect. However
> -		 * at the point when this call is done we typically don't yet
> -		 * know which interface is going to be used
> -		 */
> -		if (val && (val < TCP_MIN_MSS || val > MAX_TCP_WINDOW)) {
> -			err = -EINVAL;
> -			break;
> -		}
> -		tp->rx_opt.user_mss = val;
> +		tcp_sock_set_maxseg(sk, val);
>  		break;
>  
>  	case TCP_NODELAY:

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

[PATCH mptcp-next v2 2/3] mptcp: add TCP_MAXSEG sockopt support

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

The TCP_MAXSEG socket option is currently not supported by MPTCP, mainly
because it has never been requested before. But there are still valid
use-cases, e.g. with HAProxy.

This patch adds its support in MPTCP by propagating the value to all
subflows.

Similar to mptcp_setsockopt_first_sf_only(), a generic helper
mptcp_setsockopt_all_subflows() is added to set sockopt for each
subflows of the mptcp socket.

Add a new member for struct mptcp_sock to store the TCP_MAXSEG value,
and return this value in getsockopt.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/515
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/protocol.h |  1 +
 net/mptcp/sockopt.c  | 24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 64aa091cb685..91aaed17fe56 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -327,6 +327,7 @@ struct mptcp_sock {
 	int		keepalive_cnt;
 	int		keepalive_idle;
 	int		keepalive_intvl;
+	int		maxseg;
 	struct work_struct work;
 	struct sk_buff  *ooo_last_skb;
 	struct rb_root  out_of_order_queue;
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 3caa0a9d3b38..a7992cbc5ed9 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -798,6 +798,22 @@ static int mptcp_setsockopt_first_sf_only(struct mptcp_sock *msk, int level, int
 	return ret;
 }
 
+static int mptcp_setsockopt_all_subflows(struct mptcp_sock *msk, int level, int optname,
+					 sockptr_t optval, unsigned int optlen)
+{
+	struct mptcp_subflow_context *subflow;
+
+	mptcp_for_each_subflow(msk, subflow) {
+		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+		int ret = 0;
+
+		ret = tcp_setsockopt(ssk, level, optname, optval, optlen);
+		if (ret)
+			return ret;
+	}
+	return 0;
+}
+
 static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 				    sockptr_t optval, unsigned int optlen)
 {
@@ -859,6 +875,11 @@ static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 						 &msk->keepalive_cnt,
 						 val);
 		break;
+	case TCP_MAXSEG:
+		msk->maxseg = val;
+		ret = mptcp_setsockopt_all_subflows(msk, SOL_TCP, optname,
+						    optval, optlen);
+		break;
 	default:
 		ret = -ENOPROTOOPT;
 	}
@@ -1407,6 +1428,8 @@ static int mptcp_getsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 		return mptcp_put_int_option(msk, optval, optlen, msk->notsent_lowat);
 	case TCP_IS_MPTCP:
 		return mptcp_put_int_option(msk, optval, optlen, 1);
+	case TCP_MAXSEG:
+		return mptcp_put_int_option(msk, optval, optlen, msk->maxseg);
 	}
 	return -EOPNOTSUPP;
 }
@@ -1553,6 +1576,7 @@ static void sync_socket_options(struct mptcp_sock *msk, struct sock *ssk)
 	tcp_sock_set_keepidle_locked(ssk, msk->keepalive_idle);
 	tcp_sock_set_keepintvl(ssk, msk->keepalive_intvl);
 	tcp_sock_set_keepcnt(ssk, msk->keepalive_cnt);
+	tcp_sock_set_maxseg(ssk, msk->maxseg);
 
 	inet_assign_bit(TRANSPARENT, ssk, inet_test_bit(TRANSPARENT, sk));
 	inet_assign_bit(FREEBIND, ssk, inet_test_bit(FREEBIND, sk));
-- 
2.43.0

Re: [PATCH mptcp-next v2 2/3] mptcp: add TCP_MAXSEG sockopt support

[Matthieu Baerts]

Hi Geliang,

On 23/04/2025 11:40, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> The TCP_MAXSEG socket option is currently not supported by MPTCP, mainly
> because it has never been requested before. But there are still valid
> use-cases, e.g. with HAProxy.
> 
> This patch adds its support in MPTCP by propagating the value to all
> subflows.
> 
> Similar to mptcp_setsockopt_first_sf_only(), a generic helper
> mptcp_setsockopt_all_subflows() is added to set sockopt for each
> subflows of the mptcp socket.
> 
> Add a new member for struct mptcp_sock to store the TCP_MAXSEG value,
> and return this value in getsockopt.
> 
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/515
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/protocol.h |  1 +
>  net/mptcp/sockopt.c  | 24 ++++++++++++++++++++++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 64aa091cb685..91aaed17fe56 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -327,6 +327,7 @@ struct mptcp_sock {
>  	int		keepalive_cnt;
>  	int		keepalive_idle;
>  	int		keepalive_intvl;
> +	int		maxseg;
>  	struct work_struct work;
>  	struct sk_buff  *ooo_last_skb;
>  	struct rb_root  out_of_order_queue;
> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> index 3caa0a9d3b38..a7992cbc5ed9 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c
> @@ -798,6 +798,22 @@ static int mptcp_setsockopt_first_sf_only(struct mptcp_sock *msk, int level, int
>  	return ret;
>  }
>  
> +static int mptcp_setsockopt_all_subflows(struct mptcp_sock *msk, int level, int optname,
> +					 sockptr_t optval, unsigned int optlen)
> +{
> +	struct mptcp_subflow_context *subflow;
> +
> +	mptcp_for_each_subflow(msk, subflow) {
> +		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> +		int ret = 0;
> +
> +		ret = tcp_setsockopt(ssk, level, optname, optval, optlen);
> +		if (ret)
> +			return ret;
> +	}
> +	return 0;
> +}
> +
>  static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
>  				    sockptr_t optval, unsigned int optlen)
>  {
> @@ -859,6 +875,11 @@ static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
>  						 &msk->keepalive_cnt,
>  						 val);
>  		break;
> +	case TCP_MAXSEG:
> +		msk->maxseg = val;
> +		ret = mptcp_setsockopt_all_subflows(msk, SOL_TCP, optname,
> +						    optval, optlen);
> +		break;
>  	default:
>  		ret = -ENOPROTOOPT;
>  	}
> @@ -1407,6 +1428,8 @@ static int mptcp_getsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
>  		return mptcp_put_int_option(msk, optval, optlen, msk->notsent_lowat);
>  	case TCP_IS_MPTCP:
>  		return mptcp_put_int_option(msk, optval, optlen, 1);
> +	case TCP_MAXSEG:
> +		return mptcp_put_int_option(msk, optval, optlen, msk->maxseg);

I just checked what TCP is doing, and I see that this is different: with
TCP, the cached MSS is returned when the connection is not closed (and
something else for TCP_REPAIR). We should then do the same.

Should we then simply call mptcp_getsockopt_first_sf_only()?

>  	}
>  	return -EOPNOTSUPP;
>  }
> @@ -1553,6 +1576,7 @@ static void sync_socket_options(struct mptcp_sock *msk, struct sock *ssk)
>  	tcp_sock_set_keepidle_locked(ssk, msk->keepalive_idle);
>  	tcp_sock_set_keepintvl(ssk, msk->keepalive_intvl);
>  	tcp_sock_set_keepcnt(ssk, msk->keepalive_cnt);
> +	tcp_sock_set_maxseg(ssk, msk->maxseg);
>  
>  	inet_assign_bit(TRANSPARENT, ssk, inet_test_bit(TRANSPARENT, sk));
>  	inet_assign_bit(FREEBIND, ssk, inet_test_bit(FREEBIND, sk));

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

[PATCH mptcp-next v2 3/3] selftests: mptcp: sockopt: add TCP_MAXSEG sockopt tests

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch adds the TCP_MAXSEG sockopt tests in mptcp_sockopt.c. Since
in getsockopt TCP_MAXSEG, the "user_mss" value can be obtained only in
the LISTEN state (see do_tcp_getsockopt in net/ipv4/tcp.c), the test
items are added to server() instead of client().

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 .../selftests/net/mptcp/mptcp_sockopt.c       | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
index 9934a68df237..5ad1e9ed2fe6 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
@@ -697,6 +697,26 @@ static int xaccept(int s)
 	return fd;
 }
 
+static void test_tcp_maxseg_sockopt(int fd)
+{
+	int maxseg = 1000;
+	socklen_t s;
+	int r;
+
+	s = sizeof(maxseg);
+	r = setsockopt(fd, IPPROTO_TCP, TCP_MAXSEG, &maxseg, s);
+	if (r != 0)
+		die_perror("setsockopt TCP_MAXSEG");
+
+	maxseg = 0;
+	r = getsockopt(fd, IPPROTO_TCP, TCP_MAXSEG, &maxseg, &s);
+	if (r != -1 && errno != EINVAL)
+		die_perror("getsockopt TCP_MAXSEG did not indicate -EINVAL");
+
+	if (maxseg != 1000)
+		xerror("maxseg=%d", maxseg);
+}
+
 static int server(int pipefd)
 {
 	int fd = -1, r;
@@ -721,6 +741,8 @@ static int server(int pipefd)
 
 	process_one_client(r, pipefd);
 
+	test_tcp_maxseg_sockopt(fd);
+
 	return 0;
 }
 
-- 
2.43.0

Re: [PATCH mptcp-next v2 3/3] selftests: mptcp: sockopt: add TCP_MAXSEG sockopt tests

[Matthieu Baerts]

Hi Geliang

On 23/04/2025 11:40, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch adds the TCP_MAXSEG sockopt tests in mptcp_sockopt.c. Since
> in getsockopt TCP_MAXSEG, the "user_mss" value can be obtained only in
> the LISTEN state (see do_tcp_getsockopt in net/ipv4/tcp.c), the test
> items are added to server() instead of client().

I understand you wanted a way to test it, but the set/get is not enough,
and it looks "useless". I'm OK to apply the two other patches with the
small suggested modifications, but I will only send them when there will
be a packetdrill test.

Do not hesitate to imitate other packetdrill test, e.g.
mptcp/sockopts/sockopt_keepalive.pkt. In this case, you could set MAXSEG
(e.g. 1000), then send data (e.g. write(5000)) and check 4 packets are
sent, and the MSS is set to 1000. It might be good to change the value
again later, and see if the behaviour is the expected one
(setsockopt(MAXSEG, 1250) + write(2500)). If possible, after that, in
the same test, also create another subflow, and only check the MSS to
see if it has the expected value. To test that, feel free to look at:

https://github.com/multipath-tcp/mptcp-upstream-virtme-docker?tab=readme-ov-file#packetdrill

Note that the behaviour should be as closed as possible to TCP. Don't
hesitate to run the same kind of test with "plain" TCP.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-next v2 0/3] Add TCP_MAXSEG socket option support

[Matthieu Baerts]

Hi Geliang,

On 23/04/2025 11:40, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> v2:
>  - add a new helper tcp_sock_set_maxseg.
>  - add maxseg member of struct mptcp_sock.
>  - add mptcp_setsockopt_all_subflows helper.
>  - invoke mptcp_setsockopt_all_subflows under the msk socket lock.
>  - drop mptcp_getsockopt_sol_tcp_maxseg, use mptcp_put_int_option
>    instead.

Thank you for the update.

> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/515
> 
> Geliang Tang (3):
>   tcp: add tcp_sock_set_maxseg
>   mptcp: add TCP_MAXSEG sockopt support
>   selftests: mptcp: sockopt: add TCP_MAXSEG sockopt tests

The two first patches look almost OK to me, please see the individual
comments. I think we can drop the 3rd one.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.