* [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
* 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
* [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 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
* 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
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