* [PATCH mptcp-next v3 0/2] Add TCP_MAXSEG socket option support
@ 2025-04-30 5:49 Geliang Tang
2025-04-30 5:49 ` [PATCH mptcp-next v3 1/2] tcp: add tcp_sock_set_maxseg Geliang Tang
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Geliang Tang @ 2025-04-30 5:49 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
v3:
- drop "EXPORT_SYMBOL(tcp_sock_set_maxseg)" as Matt suggested.
- use mptcp_getsockopt_first_sf_only instead of mptcp_put_int_option.
- drop selftest
- add a packetdrill test.
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 (2):
tcp: add tcp_sock_set_maxseg
mptcp: add TCP_MAXSEG sockopt support
include/linux/tcp.h | 1 +
net/ipv4/tcp.c | 23 ++++++++++++++---------
net/mptcp/protocol.h | 1 +
net/mptcp/sockopt.c | 25 +++++++++++++++++++++++++
4 files changed, 41 insertions(+), 9 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH mptcp-next v3 1/2] tcp: add tcp_sock_set_maxseg 2025-04-30 5:49 [PATCH mptcp-next v3 0/2] Add TCP_MAXSEG socket option support Geliang Tang @ 2025-04-30 5:49 ` Geliang Tang 2025-04-30 5:49 ` [PATCH mptcp-next v3 2/2] mptcp: add TCP_MAXSEG sockopt support Geliang Tang 2025-04-30 5:49 ` [PATCH mptcp-next] mptcp: validate maxseg sockopt Geliang Tang 2 siblings, 0 replies; 12+ messages in thread From: Geliang Tang @ 2025-04-30 5:49 UTC (permalink / raw) To: mptcp; +Cc: Geliang Tang, Matthieu Baerts From: Geliang Tang <tanggeliang@kylinos.cn> Add a helper tcp_sock_set_maxseg() to directly set the TCP_MAXSEG sockopt from kernel space. This new helper will be used in the following patch. Acked-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn> --- include/linux/tcp.h | 1 + net/ipv4/tcp.c | 23 ++++++++++++++--------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/include/linux/tcp.h b/include/linux/tcp.h index a8af71623ba7..41206c3f289f 100644 --- a/include/linux/tcp.h +++ b/include/linux/tcp.h @@ -624,6 +624,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 e1bbb070e734..e1f00faed760 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -3726,6 +3726,19 @@ 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; +} + /* * Socket option code for TCP. */ @@ -3858,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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH mptcp-next v3 2/2] mptcp: add TCP_MAXSEG sockopt support 2025-04-30 5:49 [PATCH mptcp-next v3 0/2] Add TCP_MAXSEG socket option support Geliang Tang 2025-04-30 5:49 ` [PATCH mptcp-next v3 1/2] tcp: add tcp_sock_set_maxseg Geliang Tang @ 2025-04-30 5:49 ` Geliang Tang 2025-04-30 8:55 ` Matthieu Baerts 2025-04-30 9:06 ` Matthieu Baerts 2025-04-30 5:49 ` [PATCH mptcp-next] mptcp: validate maxseg sockopt Geliang Tang 2 siblings, 2 replies; 12+ messages in thread From: Geliang Tang @ 2025-04-30 5:49 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. 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 | 25 +++++++++++++++++++++++++ 2 files changed, 26 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 afa54fba51e2..5a002dd05cc9 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; } @@ -1406,6 +1427,9 @@ 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_first_sf_only(msk, SOL_TCP, optname, + optval, optlen); } return -EOPNOTSUPP; } @@ -1552,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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH mptcp-next v3 2/2] mptcp: add TCP_MAXSEG sockopt support 2025-04-30 5:49 ` [PATCH mptcp-next v3 2/2] mptcp: add TCP_MAXSEG sockopt support Geliang Tang @ 2025-04-30 8:55 ` Matthieu Baerts 2025-05-06 2:36 ` Geliang Tang 2025-04-30 9:06 ` Matthieu Baerts 1 sibling, 1 reply; 12+ messages in thread From: Matthieu Baerts @ 2025-04-30 8:55 UTC (permalink / raw) To: Geliang Tang, mptcp; +Cc: Geliang Tang Hi Geliang, On 30/04/2025 07:49, 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. Might be good to add: The get part looks at the value on the first subflow, to be as closed as possible to TCP. Only one value can be returned for the cached MSS, so this can come only from one subflow. The rest looks good to me: Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> I can do the modification when applying the patch. Note that I will not send them upstream before the addition of the packetdrill test in our other repo. Cheers, Matt -- Sponsored by the NGI0 Core fund. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH mptcp-next v3 2/2] mptcp: add TCP_MAXSEG sockopt support 2025-04-30 8:55 ` Matthieu Baerts @ 2025-05-06 2:36 ` Geliang Tang 0 siblings, 0 replies; 12+ messages in thread From: Geliang Tang @ 2025-05-06 2:36 UTC (permalink / raw) To: Matthieu Baerts, mptcp; +Cc: Geliang Tang Hi Matt, On Wed, 2025-04-30 at 10:55 +0200, Matthieu Baerts wrote: > Hi Geliang, > > On 30/04/2025 07:49, 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. > > Might be good to add: > > The get part looks at the value on the first subflow, to be as > closed > as possible to TCP. Only one value can be returned for the cached > MSS, > so this can come only from one subflow. > > The rest looks good to me: > > Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> > > I can do the modification when applying the patch. > > Note that I will not send them upstream before the addition of the > packetdrill test in our other repo. Thanks for applying these patches for me. They can be WIP on the export branch until I add the packetdrill test, which I have already started working on. Thanks, -Geliang > > Cheers, > Matt ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH mptcp-next v3 2/2] mptcp: add TCP_MAXSEG sockopt support 2025-04-30 5:49 ` [PATCH mptcp-next v3 2/2] mptcp: add TCP_MAXSEG sockopt support Geliang Tang 2025-04-30 8:55 ` Matthieu Baerts @ 2025-04-30 9:06 ` Matthieu Baerts 1 sibling, 0 replies; 12+ messages in thread From: Matthieu Baerts @ 2025-04-30 9:06 UTC (permalink / raw) To: Geliang Tang, mptcp; +Cc: Geliang Tang On 30/04/2025 07:49, 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 | 25 +++++++++++++++++++++++++ > 2 files changed, 26 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 afa54fba51e2..5a002dd05cc9 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, Checkpatch was complaining about this line because we were over 80 chars. I just did a modification to avoid that, and while at it, I also use 'mptcp_setsockopt_all_sf', similar to 'first_sf_only' and what was suggested there: https://patchwork.kernel.org/project/mptcp/patch/20230707-mptcp-unify-sockopt-issue-353-v1-5-693e15c06646@tessares.net/ Cheers, Matt -- Sponsored by the NGI0 Core fund. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH mptcp-next] mptcp: validate maxseg sockopt 2025-04-30 5:49 [PATCH mptcp-next v3 0/2] Add TCP_MAXSEG socket option support Geliang Tang 2025-04-30 5:49 ` [PATCH mptcp-next v3 1/2] tcp: add tcp_sock_set_maxseg Geliang Tang 2025-04-30 5:49 ` [PATCH mptcp-next v3 2/2] mptcp: add TCP_MAXSEG sockopt support Geliang Tang @ 2025-04-30 5:49 ` Geliang Tang 2025-04-30 7:15 ` MPTCP CI ` (2 more replies) 2 siblings, 3 replies; 12+ messages in thread From: Geliang Tang @ 2025-04-30 5:49 UTC (permalink / raw) To: mptcp; +Cc: Geliang Tang This patch validates the newly added TCP_MAXSEG sockopt. Signed-off-by: Geliang Tang <geliang@kernel.org> --- gtests/net/mptcp/sockopts/sockopt_maxseg.pkt | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 gtests/net/mptcp/sockopts/sockopt_maxseg.pkt diff --git a/gtests/net/mptcp/sockopts/sockopt_maxseg.pkt b/gtests/net/mptcp/sockopts/sockopt_maxseg.pkt new file mode 100644 index 0000000..62492db --- /dev/null +++ b/gtests/net/mptcp/sockopts/sockopt_maxseg.pkt @@ -0,0 +1,20 @@ +--tolerance_usecs=100000 +`../common/defaults.sh` + +0.0 socket(..., SOCK_STREAM, IPPROTO_MPTCP) = 3 + ++0 bind(3, ..., ...) = 0 ++0 listen(3, 1) = 0 + ++0 setsockopt(3, SOL_TCP, TCP_MAXSEG, [1000], 4) = 0 ++0 getsockopt(3, SOL_TCP, TCP_MAXSEG, [1000], [4]) = 0 + ++0 < S 0:0(0) win 8000 <mss 1000, sackOK, nop, nop, nop, wscale 0, mpcapable v1 flags[flag_h] nokey> ++0 > S. 0:0(0) ack 1 <mss 1000, nop, nop, sackOK, nop, wscale 8, mpcapable v1 flags[flag_h] key[skey]> ++0.01 < . 1:1(0) ack 1 win 8000 <mpcapable v1 flags[flag_h] key[ckey=2, skey]> ++0 accept(3, ..., ...) = 4 + ++0 write(4, ..., 100) = 100 + ++0 > P. 1:101(100) ack 1 <dss dack4=1 dsn8=1 ssn=1 dll=100 nocs, nop, nop> ++0.01 < . 1:1(0) ack 101 win 256 <dss dack4=101 nocs> -- 2.43.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH mptcp-next] mptcp: validate maxseg sockopt 2025-04-30 5:49 ` [PATCH mptcp-next] mptcp: validate maxseg sockopt Geliang Tang @ 2025-04-30 7:15 ` MPTCP CI 2025-04-30 7:31 ` MPTCP CI 2025-04-30 9:42 ` Matthieu Baerts 2 siblings, 0 replies; 12+ messages in thread From: MPTCP CI @ 2025-04-30 7:15 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/14747840104 Initiator: Patchew Applier Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/34632815fba5 Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=958366 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] 12+ messages in thread
* Re: [PATCH mptcp-next] mptcp: validate maxseg sockopt 2025-04-30 5:49 ` [PATCH mptcp-next] mptcp: validate maxseg sockopt Geliang Tang 2025-04-30 7:15 ` MPTCP CI @ 2025-04-30 7:31 ` MPTCP CI 2025-04-30 9:42 ` Matthieu Baerts 2 siblings, 0 replies; 12+ messages in thread From: MPTCP CI @ 2025-04-30 7:31 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/14747844787 Initiator: Patchew Applier Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/c9ed2af6416c Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=958366 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] 12+ messages in thread
* Re: [PATCH mptcp-next] mptcp: validate maxseg sockopt 2025-04-30 5:49 ` [PATCH mptcp-next] mptcp: validate maxseg sockopt Geliang Tang 2025-04-30 7:15 ` MPTCP CI 2025-04-30 7:31 ` MPTCP CI @ 2025-04-30 9:42 ` Matthieu Baerts 2025-05-08 7:21 ` Geliang Tang 2 siblings, 1 reply; 12+ messages in thread From: Matthieu Baerts @ 2025-04-30 9:42 UTC (permalink / raw) To: Geliang Tang, mptcp Hi Geliang, On 30/04/2025 07:49, Geliang Tang wrote: > This patch validates the newly added TCP_MAXSEG sockopt. If you don't mind and have access, do you mind creating a PR on GitHub instead please? https://github.com/multipath-tcp/packetdrill When you send them here, the CI might think it is a kernel patch and try to apply it (which can work with new files). > Signed-off-by: Geliang Tang <geliang@kernel.org> > --- > gtests/net/mptcp/sockopts/sockopt_maxseg.pkt | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > create mode 100644 gtests/net/mptcp/sockopts/sockopt_maxseg.pkt > > diff --git a/gtests/net/mptcp/sockopts/sockopt_maxseg.pkt b/gtests/net/mptcp/sockopts/sockopt_maxseg.pkt > new file mode 100644 > index 0000000..62492db > --- /dev/null > +++ b/gtests/net/mptcp/sockopts/sockopt_maxseg.pkt > @@ -0,0 +1,20 @@ > +--tolerance_usecs=100000 > +`../common/defaults.sh` > + > +0.0 socket(..., SOCK_STREAM, IPPROTO_MPTCP) = 3 > + > ++0 bind(3, ..., ...) = 0 > ++0 listen(3, 1) = 0 > + Here, similar to other sockopt tests, can you check the default value please? It should be the same as the one for "plain" TCP, 536 I suppose (min mss). > ++0 setsockopt(3, SOL_TCP, TCP_MAXSEG, [1000], 4) = 0 > ++0 getsockopt(3, SOL_TCP, TCP_MAXSEG, [1000], [4]) = 0 Can you move this block before the bind()/listen()? It seems to make more sense to set that before the listen(), no? > ++0 < S 0:0(0) win 8000 <mss 1000, sackOK, nop, nop, nop, wscale 0, mpcapable v1 flags[flag_h] nokey> Just to avoid confusions, in the inject packet here, please set something else, e.g. 1460. > ++0 > S. 0:0(0) ack 1 <mss 1000, nop, nop, sackOK, nop, wscale 8, mpcapable v1 flags[flag_h] key[skey]> > ++0.01 < . 1:1(0) ack 1 win 8000 <mpcapable v1 flags[flag_h] key[ckey=2, skey]> > ++0 accept(3, ..., ...) = 4 Can you do a getsockopt(4, ..., TCP_MAXSEG) here to check the value? It should be similar to what you would get with "plain" TCP (is it 988? or -20B reserved for MPTCP options?). It might be better to introduce a new test doing the same thing with "plain" TCP to compare if you don't mind. > ++0 write(4, ..., 100) = 100 Here, please write more than the MSS you set, e.g. 1250B to see how the stack will split that in two packets > + > ++0 > P. 1:101(100) ack 1 <dss dack4=1 dsn8=1 ssn=1 dll=100 nocs, nop, nop> > ++0.01 < . 1:1(0) ack 101 win 256 <dss dack4=101 nocs> Here, can you change TCP_MAXSEG on the accepted socket (fd = 4)? e.g. set it to 800, then do a getsockopt(), then write(4, ..., 1250) and see if you have 2 packets of the expected size. Then, if you don't mind, please also create a new subflow by injecting an MP_JOIN with a MSS of 1460, and check that the SYN+ACK+MP_JOIN generated by the kernel has a MSS of 1000 as expected. For an example, check mptcp/mp_join/mp_join_server.pkt. Cheers, Matt -- Sponsored by the NGI0 Core fund. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH mptcp-next] mptcp: validate maxseg sockopt 2025-04-30 9:42 ` Matthieu Baerts @ 2025-05-08 7:21 ` Geliang Tang 2025-05-08 9:05 ` Matthieu Baerts 0 siblings, 1 reply; 12+ messages in thread From: Geliang Tang @ 2025-05-08 7:21 UTC (permalink / raw) To: Matthieu Baerts, mptcp Hi Matt, Thanks for the review. On Wed, 2025-04-30 at 11:42 +0200, Matthieu Baerts wrote: > Hi Geliang, > > On 30/04/2025 07:49, Geliang Tang wrote: > > This patch validates the newly added TCP_MAXSEG sockopt. > > If you don't mind and have access, do you mind creating a PR on > GitHub > instead please? Here's the PR: https://github.com/multipath-tcp/packetdrill/pull/161 > > https://github.com/multipath-tcp/packetdrill > > When you send them here, the CI might think it is a kernel patch and > try > to apply it (which can work with new files). > > > Signed-off-by: Geliang Tang <geliang@kernel.org> > > --- > > gtests/net/mptcp/sockopts/sockopt_maxseg.pkt | 20 > > ++++++++++++++++++++ > > 1 file changed, 20 insertions(+) > > create mode 100644 gtests/net/mptcp/sockopts/sockopt_maxseg.pkt > > > > diff --git a/gtests/net/mptcp/sockopts/sockopt_maxseg.pkt > > b/gtests/net/mptcp/sockopts/sockopt_maxseg.pkt > > new file mode 100644 > > index 0000000..62492db > > --- /dev/null > > +++ b/gtests/net/mptcp/sockopts/sockopt_maxseg.pkt > > @@ -0,0 +1,20 @@ > > +--tolerance_usecs=100000 > > +`../common/defaults.sh` > > + > > +0.0 socket(..., SOCK_STREAM, IPPROTO_MPTCP) = 3 > > + > > ++0 bind(3, ..., ...) = 0 > > ++0 listen(3, 1) = 0 > > + > > Here, similar to other sockopt tests, can you check the default value > please? It should be the same as the one for "plain" TCP, 536 I > suppose > (min mss). > > > ++0 setsockopt(3, SOL_TCP, TCP_MAXSEG, [1000], 4) = 0 > > ++0 getsockopt(3, SOL_TCP, TCP_MAXSEG, [1000], [4]) = 0 > > Can you move this block before the bind()/listen()? It seems to make > more sense to set that before the listen(), no? > > > ++0 < S 0:0(0) win 8000 <mss 1000, sackOK, nop, > > nop, nop, wscale 0, mpcapable v1 flags[flag_h] nokey> > > Just to avoid confusions, in the inject packet here, please set > something else, e.g. 1460. > > > ++0 > S. 0:0(0) ack 1 <mss 1000, nop, nop, > > sackOK, nop, wscale 8, mpcapable v1 flags[flag_h] key[skey]> > > ++0.01 < . 1:1(0) ack 1 win > > 8000 <mpcapable v1 > > flags[flag_h] key[ckey=2, skey]> > > ++0 accept(3, ..., ...) = 4 > > Can you do a getsockopt(4, ..., TCP_MAXSEG) here to check the value? > It > should be similar to what you would get with "plain" TCP (is it 988? > or > -20B reserved for MPTCP options?). > > It might be better to introduce a new test doing the same thing with > "plain" TCP to compare if you don't mind. > > > ++0 write(4, ..., 100) = 100 > > Here, please write more than the MSS you set, e.g. 1250B to see how > the > stack will split that in two packets > > > + > > ++0 > P. 1:101(100) ack 1 <dss dack4=1 dsn8=1 > > ssn=1 dll=100 nocs, nop, nop> > > ++0.01 < . 1:1(0) ack 101 win 256 <dss dack4=101 nocs> > Here, can you change TCP_MAXSEG on the accepted socket (fd = 4)? > > e.g. set it to 800, then do a getsockopt(), then write(4, ..., 1250) > and > see if you have 2 packets of the expected size. > In this PR, I almost addressed all your comments above ... > Then, if you don't mind, please also create a new subflow by > injecting > an MP_JOIN with a MSS of 1460, and check that the SYN+ACK+MP_JOIN > generated by the kernel has a MSS of 1000 as expected. For an > example, > check mptcp/mp_join/mp_join_server.pkt. But I still have some problems when creating a new subflow, which will be added later. Thanks, -Geliang > > Cheers, > Matt ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH mptcp-next] mptcp: validate maxseg sockopt 2025-05-08 7:21 ` Geliang Tang @ 2025-05-08 9:05 ` Matthieu Baerts 0 siblings, 0 replies; 12+ messages in thread From: Matthieu Baerts @ 2025-05-08 9:05 UTC (permalink / raw) To: Geliang Tang; +Cc: mptcp Hi Geliang, On 08/05/2025 09:21, Geliang Tang wrote: > Hi Matt, > > Thanks for the review. > > On Wed, 2025-04-30 at 11:42 +0200, Matthieu Baerts wrote: >> Hi Geliang, >> >> On 30/04/2025 07:49, Geliang Tang wrote: >>> This patch validates the newly added TCP_MAXSEG sockopt. >> >> If you don't mind and have access, do you mind creating a PR on >> GitHub >> instead please? > > Here's the PR: > > https://github.com/multipath-tcp/packetdrill/pull/161 Thanks, I just did a review: https://github.com/multipath-tcp/packetdrill/pull/161/files (...) > In this PR, I almost addressed all your comments above ... > >> Then, if you don't mind, please also create a new subflow by >> injecting >> an MP_JOIN with a MSS of 1460, and check that the SYN+ACK+MP_JOIN >> generated by the kernel has a MSS of 1000 as expected. For an >> example, >> check mptcp/mp_join/mp_join_server.pkt. > > But I still have some problems when creating a new subflow, which will > be added later. Do not hesitate to share this WIP code and the error you have on the PR. I also added a few pointers there. Cheers, Matt -- Sponsored by the NGI0 Core fund. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-05-08 9:05 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-04-30 5:49 [PATCH mptcp-next v3 0/2] Add TCP_MAXSEG socket option support Geliang Tang 2025-04-30 5:49 ` [PATCH mptcp-next v3 1/2] tcp: add tcp_sock_set_maxseg Geliang Tang 2025-04-30 5:49 ` [PATCH mptcp-next v3 2/2] mptcp: add TCP_MAXSEG sockopt support Geliang Tang 2025-04-30 8:55 ` Matthieu Baerts 2025-05-06 2:36 ` Geliang Tang 2025-04-30 9:06 ` Matthieu Baerts 2025-04-30 5:49 ` [PATCH mptcp-next] mptcp: validate maxseg sockopt Geliang Tang 2025-04-30 7:15 ` MPTCP CI 2025-04-30 7:31 ` MPTCP CI 2025-04-30 9:42 ` Matthieu Baerts 2025-05-08 7:21 ` Geliang Tang 2025-05-08 9:05 ` Matthieu Baerts
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.