MPTCP Linux Development
 help / color / mirror / Atom feed
* [PATCH mptcp-next v1 0/2] Add TCP_MAXSEG socket option support
@ 2025-04-22  9:30 Geliang Tang
  2025-04-22  9:30 ` [PATCH mptcp-next v1 1/2] mptcp: add TCP_MAXSEG sockopt support Geliang Tang
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Geliang Tang @ 2025-04-22  9:30 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

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

Geliang Tang (2):
  mptcp: add TCP_MAXSEG sockopt support
  selftests: mptcp: sockopt: add TCP_MAXSEG sockopt tests

 net/mptcp/sockopt.c                           | 29 +++++++++++++++++++
 .../selftests/net/mptcp/mptcp_sockopt.c       | 22 ++++++++++++++
 2 files changed, 51 insertions(+)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH mptcp-next v1 1/2] mptcp: add TCP_MAXSEG sockopt support
  2025-04-22  9:30 [PATCH mptcp-next v1 0/2] Add TCP_MAXSEG socket option support Geliang Tang
@ 2025-04-22  9:30 ` Geliang Tang
  2025-04-22 11:18   ` Matthieu Baerts
  2025-04-22  9:30 ` [PATCH mptcp-next v1 2/2] selftests: mptcp: sockopt: add TCP_MAXSEG sockopt tests Geliang Tang
  2025-04-22 10:52 ` [PATCH mptcp-next v1 0/2] Add TCP_MAXSEG socket option support MPTCP CI
  2 siblings, 1 reply; 7+ messages in thread
From: Geliang Tang @ 2025-04-22  9:30 UTC (permalink / raw)
  To: mptcp; +Cc: 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.

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

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 3caa0a9d3b38..3014713b037c 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -798,6 +798,24 @@ static int mptcp_setsockopt_first_sf_only(struct mptcp_sock *msk, int level, int
 	return ret;
 }
 
+static int mptcp_setsockopt_sol_tcp_maxseg(struct mptcp_sock *msk,
+					   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;
+
+		ret = tcp_setsockopt(ssk, SOL_TCP, TCP_MAXSEG, 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)
 {
@@ -819,6 +837,8 @@ static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 	case TCP_FASTOPEN_NO_COOKIE:
 		return mptcp_setsockopt_first_sf_only(msk, SOL_TCP, optname,
 						      optval, optlen);
+	case TCP_MAXSEG:
+		return mptcp_setsockopt_sol_tcp_maxseg(msk, optval, optlen);
 	}
 
 	ret = mptcp_get_int_option(msk, optval, optlen, &val);
@@ -1368,6 +1388,13 @@ static int mptcp_put_int_option(struct mptcp_sock *msk, char __user *optval,
 	return 0;
 }
 
+static int mptcp_getsockopt_sol_tcp_maxseg(struct mptcp_sock *msk,
+					   char __user *optval,
+					   int __user *optlen)
+{
+	return tcp_getsockopt(msk->first, SOL_TCP, TCP_MAXSEG, optval, optlen);
+}
+
 static int mptcp_getsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 				    char __user *optval, int __user *optlen)
 {
@@ -1407,6 +1434,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_getsockopt_sol_tcp_maxseg(msk, optval, optlen);
 	}
 	return -EOPNOTSUPP;
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH mptcp-next v1 2/2] selftests: mptcp: sockopt: add TCP_MAXSEG sockopt tests
  2025-04-22  9:30 [PATCH mptcp-next v1 0/2] Add TCP_MAXSEG socket option support Geliang Tang
  2025-04-22  9:30 ` [PATCH mptcp-next v1 1/2] mptcp: add TCP_MAXSEG sockopt support Geliang Tang
@ 2025-04-22  9:30 ` Geliang Tang
  2025-04-22 11:20   ` Matthieu Baerts
  2025-04-22 10:52 ` [PATCH mptcp-next v1 0/2] Add TCP_MAXSEG socket option support MPTCP CI
  2 siblings, 1 reply; 7+ messages in thread
From: Geliang Tang @ 2025-04-22  9:30 UTC (permalink / raw)
  To: mptcp; +Cc: 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 926b0be87c99..e2043c0261bd 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
@@ -689,6 +689,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;
@@ -713,6 +733,8 @@ static int server(int pipefd)
 
 	process_one_client(r, pipefd);
 
+	test_tcp_maxseg_sockopt(fd);
+
 	return 0;
 }
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH mptcp-next v1 0/2] Add TCP_MAXSEG socket option support
  2025-04-22  9:30 [PATCH mptcp-next v1 0/2] Add TCP_MAXSEG socket option support Geliang Tang
  2025-04-22  9:30 ` [PATCH mptcp-next v1 1/2] mptcp: add TCP_MAXSEG sockopt support Geliang Tang
  2025-04-22  9:30 ` [PATCH mptcp-next v1 2/2] selftests: mptcp: sockopt: add TCP_MAXSEG sockopt tests Geliang Tang
@ 2025-04-22 10:52 ` MPTCP CI
  2 siblings, 0 replies; 7+ messages in thread
From: MPTCP CI @ 2025-04-22 10:52 UTC (permalink / raw)
  To: Geliang Tang; +Cc: mptcp

Hi Geliang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Success! ✅
- KVM Validation: debug: Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/14591855995

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/a917e8ca4810
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=955617


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH mptcp-next v1 1/2] mptcp: add TCP_MAXSEG sockopt support
  2025-04-22  9:30 ` [PATCH mptcp-next v1 1/2] mptcp: add TCP_MAXSEG sockopt support Geliang Tang
@ 2025-04-22 11:18   ` Matthieu Baerts
  0 siblings, 0 replies; 7+ messages in thread
From: Matthieu Baerts @ 2025-04-22 11:18 UTC (permalink / raw)
  To: Geliang Tang, mptcp; +Cc: Geliang Tang

Hi Geliang,

On 22/04/2025 11:30, 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.

Thank you for looking at that.

> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/515
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/sockopt.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> index 3caa0a9d3b38..3014713b037c 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c
> @@ -798,6 +798,24 @@ static int mptcp_setsockopt_first_sf_only(struct mptcp_sock *msk, int level, int
>  	return ret;
>  }
>  
> +static int mptcp_setsockopt_sol_tcp_maxseg(struct mptcp_sock *msk,
> +					   sockptr_t optval,
> +					   unsigned int optlen)

Might be good to have a generic helper, similar to
mptcp_setsockopt_first_sf_only(), no?

  mptcp_setsockopt_all_subflows()

> +{
> +	struct mptcp_subflow_context *subflow;
> +
> +	mptcp_for_each_subflow(msk, subflow) {

If there is no subflows (listening sockets?), you might need to call
mptcp_setsockopt_first_sf_only(), no?

> +		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> +		int ret;
> +
> +		ret = tcp_setsockopt(ssk, SOL_TCP, TCP_MAXSEG, optval, optlen);

No need to lock around this call?

> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
>  				    sockptr_t optval, unsigned int optlen)
>  {
> @@ -819,6 +837,8 @@ static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
>  	case TCP_FASTOPEN_NO_COOKIE:
>  		return mptcp_setsockopt_first_sf_only(msk, SOL_TCP, optname,
>  						      optval, optlen);
> +	case TCP_MAXSEG:
> +		return mptcp_setsockopt_sol_tcp_maxseg(msk, optval, optlen);
>  	}
>  
>  	ret = mptcp_get_int_option(msk, optval, optlen, &val);
> @@ -1368,6 +1388,13 @@ static int mptcp_put_int_option(struct mptcp_sock *msk, char __user *optval,
>  	return 0;
>  }
>  
> +static int mptcp_getsockopt_sol_tcp_maxseg(struct mptcp_sock *msk,
> +					   char __user *optval,
> +					   int __user *optlen)
> +{
> +	return tcp_getsockopt(msk->first, SOL_TCP, TCP_MAXSEG, optval, optlen);

Same here: no need to lock? Also, msk->first can be NULL for listening
sockets, no?

Anyway, I think it is best not to change the way the socket options are
handled today: the socket option is propagated to existing and future
subflows. Look how "msk->keepalive_intvl" is handled in sockopt.c for
example.

- setsockopt: store the value (in case of success) in the msk.
- getsockopt: return this value (mptcp_put_int_option())

WDYT?

> +}
> +
>  static int mptcp_getsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
>  				    char __user *optval, int __user *optlen)
>  {
> @@ -1407,6 +1434,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_getsockopt_sol_tcp_maxseg(msk, optval, optlen);
>  	}
>  	return -EOPNOTSUPP;
>  }

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH mptcp-next v1 2/2] selftests: mptcp: sockopt: add TCP_MAXSEG sockopt tests
  2025-04-22  9:30 ` [PATCH mptcp-next v1 2/2] selftests: mptcp: sockopt: add TCP_MAXSEG sockopt tests Geliang Tang
@ 2025-04-22 11:20   ` Matthieu Baerts
  2025-04-23  9:46     ` Geliang Tang
  0 siblings, 1 reply; 7+ messages in thread
From: Matthieu Baerts @ 2025-04-22 11:20 UTC (permalink / raw)
  To: Geliang Tang, mptcp; +Cc: Geliang Tang

Hi Geliang,

On 22/04/2025 11:30, 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 think it is better to use packetdrill to do such validation, similar
to what was done with other socket options. No?

> 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 926b0be87c99..e2043c0261bd 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> @@ -689,6 +689,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);
> +}

Here, you set and get the value, but you don't really check the
behaviour is the one we expect.

It is important to check the behaviour. It would be easier to do that
with packetdrill I think.

> +
>  static int server(int pipefd)
>  {
>  	int fd = -1, r;
> @@ -713,6 +733,8 @@ static int server(int pipefd)
>  
>  	process_one_client(r, pipefd);
>  
> +	test_tcp_maxseg_sockopt(fd);
> +
>  	return 0;
>  }
>  

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH mptcp-next v1 2/2] selftests: mptcp: sockopt: add TCP_MAXSEG sockopt tests
  2025-04-22 11:20   ` Matthieu Baerts
@ 2025-04-23  9:46     ` Geliang Tang
  0 siblings, 0 replies; 7+ messages in thread
From: Geliang Tang @ 2025-04-23  9:46 UTC (permalink / raw)
  To: Matthieu Baerts, mptcp; +Cc: Geliang Tang

Hi Matt,

Thanks for the review.

> Hi Geliang,
> 
> On 22/04/2025 11:30, 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 think it is better to use packetdrill to do such validation,
> similar
> to what was done with other socket options. No?
> 
> > 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 926b0be87c99..e2043c0261bd 100644
> > --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> > @@ -689,6 +689,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);
> > +}
> 
> Here, you set and get the value, but you don't really check the
> behaviour is the one we expect.
> 
> It is important to check the behaviour. It would be easier to do that
> with packetdrill I think.

I totally agree, but I want to verify the set and get values ​​in this
set first, I will add packetdrill tests for it later (once I learn how
to modify packetdrill).

Thanks,
-Geliang

> 
> > +
> >  static int server(int pipefd)
> >  {
> >  	int fd = -1, r;
> > @@ -713,6 +733,8 @@ static int server(int pipefd)
> >  
> >  	process_one_client(r, pipefd);
> >  
> > +	test_tcp_maxseg_sockopt(fd);
> > +
> >  	return 0;
> >  }
> >  
> 
> Cheers,
> Matt


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-04-23  9:50 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-22  9:30 [PATCH mptcp-next v1 0/2] Add TCP_MAXSEG socket option support Geliang Tang
2025-04-22  9:30 ` [PATCH mptcp-next v1 1/2] mptcp: add TCP_MAXSEG sockopt support Geliang Tang
2025-04-22 11:18   ` Matthieu Baerts
2025-04-22  9:30 ` [PATCH mptcp-next v1 2/2] selftests: mptcp: sockopt: add TCP_MAXSEG sockopt tests Geliang Tang
2025-04-22 11:20   ` Matthieu Baerts
2025-04-23  9:46     ` Geliang Tang
2025-04-22 10:52 ` [PATCH mptcp-next v1 0/2] Add TCP_MAXSEG socket option support MPTCP CI

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox