mptcp.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH mptcp-next] mptcp: set ADD_ADDR retrans timeout to 1sec
@ 2025-07-17 23:15 Matthieu Baerts (NGI0)
  2025-07-18  0:59 ` MPTCP CI
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Matthieu Baerts (NGI0) @ 2025-07-17 23:15 UTC (permalink / raw)
  To: mptcp; +Cc: Matthieu Baerts (NGI0)

It doesn't really make sense to wait 2 minutes -- the previous default
value -- to retransmit the ADD_ADDR if the echo has not been received in
between: that's way too long to react.

Switching to 1 second by default, which seems like a good default value,
and can be increased for particular cases.

Later on, we could eventually use a timeout based on the sRTT, with an
exponential backoff, which would be more reactive [1].

Note: the documentation has been updated accordingly, and the selftests
are keeping the previous timeout when using a broadcast IP not to have
retransmissions in this case, which would cause troubles with the
different counters depending on how fast the test will run.

Link: https://github.com/multipath-tcp/mptcp_net-next/issues/576 [1]
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 Documentation/networking/mptcp-sysctl.rst       | 5 ++---
 net/mptcp/ctrl.c                                | 2 +-
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 3 +++
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/Documentation/networking/mptcp-sysctl.rst b/Documentation/networking/mptcp-sysctl.rst
index 5bfab01eff5a9db89e1484787953241c16e147cf..14059d2cac1b3467d6a8c9c4362f5a0b427b726d 100644
--- a/Documentation/networking/mptcp-sysctl.rst
+++ b/Documentation/networking/mptcp-sysctl.rst
@@ -12,10 +12,9 @@ add_addr_timeout - INTEGER (seconds)
 	resent to an MPTCP peer that has not acknowledged a previous
 	ADD_ADDR message.
 
-	The default value matches TCP_RTO_MAX. This is a per-namespace
-	sysctl.
+	This is a per-namespace sysctl.
 
-	Default: 120
+	Default: 1
 
 allow_join_initial_addr_port - BOOLEAN
 	Allow peers to send join requests to the IP address and port number used
diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index fed40dae5583a35914da2cb1c52f37830d72705e..92873457c47ea6b04137cece0200317a94b13e0d 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -97,7 +97,7 @@ const char *mptcp_get_scheduler(const struct net *net)
 static void mptcp_pernet_set_defaults(struct mptcp_pernet *pernet)
 {
 	pernet->mptcp_enabled = 1;
-	pernet->add_addr_timeout = TCP_RTO_MAX;
+	pernet->add_addr_timeout = HZ;	/* 1 second */
 	pernet->blackhole_timeout = 3600;
 	pernet->syn_retrans_before_tcp_fallback = 2;
 	atomic_set(&pernet->active_disable_times, 0);
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index b8af65373b3ada96472347171924ad3a6cf14777..87a9d022d31160a8ff6185b3a117a73841512f9a 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -2429,6 +2429,7 @@ remove_tests()
 		pm_nl_set_limits $ns1 3 3
 		pm_nl_add_endpoint $ns1 10.0.12.1 flags signal
 		# broadcast IP: no packet for this address will be received on ns1
+		ip netns exec ${ns1} sysctl -q net.mptcp.add_addr_timeout=120
 		pm_nl_add_endpoint $ns1 224.0.0.1 flags signal
 		pm_nl_add_endpoint $ns1 10.0.3.1 flags signal
 		pm_nl_set_limits $ns2 2 2
@@ -3846,6 +3847,7 @@ endpoint_tests()
 		pm_nl_set_limits $ns2 3 3
 		pm_nl_add_endpoint $ns1 10.0.2.1 id 1 flags signal
 		# broadcast IP: no packet for this address will be received on ns1
+		ip netns exec ${ns1} sysctl -q net.mptcp.add_addr_timeout=120
 		pm_nl_add_endpoint $ns1 224.0.0.1 id 2 flags signal
 		pm_nl_add_endpoint $ns1 10.0.1.1 id 42 flags signal
 		{ test_linkfail=4 speed=5 \
@@ -3919,6 +3921,7 @@ endpoint_tests()
 		pm_nl_set_limits $ns1 0 2
 		pm_nl_set_limits $ns2 1 2
 		# broadcast IP: no packet for this address will be received on ns1
+		ip netns exec ${ns1} sysctl -q net.mptcp.add_addr_timeout=120
 		pm_nl_add_endpoint $ns1 224.0.0.1 id 2 flags signal
 		pm_nl_add_endpoint $ns2 10.0.3.2 id 3 flags subflow
 		{ test_linkfail=4 speed=20 \

---
base-commit: d196b09457190bc11ffca2867d5c12beb5a4759e
change-id: 20250718-mptcp-add_addr_timeout_1-2b57726cf3cf

Best regards,
-- 
Matthieu Baerts (NGI0) <matttbe@kernel.org>


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

* Re: [PATCH mptcp-next] mptcp: set ADD_ADDR retrans timeout to 1sec
  2025-07-17 23:15 [PATCH mptcp-next] mptcp: set ADD_ADDR retrans timeout to 1sec Matthieu Baerts (NGI0)
@ 2025-07-18  0:59 ` MPTCP CI
  2025-07-22 14:19 ` MPTCP CI
  2025-08-14  0:20 ` Mat Martineau
  2 siblings, 0 replies; 5+ messages in thread
From: MPTCP CI @ 2025-07-18  0:59 UTC (permalink / raw)
  To: Matthieu Baerts; +Cc: mptcp

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Unstable: 1 failed test(s): packetdrill_sockopts 🔴
- KVM Validation: debug: Unstable: 1 failed test(s): selftest_mptcp_connect 🔴
- 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/16358285418

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


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] 5+ messages in thread

* Re: [PATCH mptcp-next] mptcp: set ADD_ADDR retrans timeout to 1sec
  2025-07-17 23:15 [PATCH mptcp-next] mptcp: set ADD_ADDR retrans timeout to 1sec Matthieu Baerts (NGI0)
  2025-07-18  0:59 ` MPTCP CI
@ 2025-07-22 14:19 ` MPTCP CI
  2025-08-14  0:20 ` Mat Martineau
  2 siblings, 0 replies; 5+ messages in thread
From: MPTCP CI @ 2025-07-22 14:19 UTC (permalink / raw)
  To: Matthieu Baerts; +Cc: mptcp

Hi Matthieu,

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/16444985243

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


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] 5+ messages in thread

* Re: [PATCH mptcp-next] mptcp: set ADD_ADDR retrans timeout to 1sec
  2025-07-17 23:15 [PATCH mptcp-next] mptcp: set ADD_ADDR retrans timeout to 1sec Matthieu Baerts (NGI0)
  2025-07-18  0:59 ` MPTCP CI
  2025-07-22 14:19 ` MPTCP CI
@ 2025-08-14  0:20 ` Mat Martineau
  2025-08-15 15:35   ` Matthieu Baerts
  2 siblings, 1 reply; 5+ messages in thread
From: Mat Martineau @ 2025-08-14  0:20 UTC (permalink / raw)
  To: Matthieu Baerts (NGI0); +Cc: mptcp

On Fri, 18 Jul 2025, Matthieu Baerts (NGI0) wrote:

> It doesn't really make sense to wait 2 minutes -- the previous default
> value -- to retransmit the ADD_ADDR if the echo has not been received in
> between: that's way too long to react.
>
> Switching to 1 second by default, which seems like a good default value,
> and can be increased for particular cases.
>
> Later on, we could eventually use a timeout based on the sRTT, with an
> exponential backoff, which would be more reactive [1].
>
> Note: the documentation has been updated accordingly, and the selftests
> are keeping the previous timeout when using a broadcast IP not to have
> retransmissions in this case, which would cause troubles with the
> different counters depending on how fast the test will run.
>
> Link: https://github.com/multipath-tcp/mptcp_net-next/issues/576 [1]
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> Documentation/networking/mptcp-sysctl.rst       | 5 ++---
> net/mptcp/ctrl.c                                | 2 +-
> tools/testing/selftests/net/mptcp/mptcp_join.sh | 3 +++
> 3 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/networking/mptcp-sysctl.rst b/Documentation/networking/mptcp-sysctl.rst
> index 5bfab01eff5a9db89e1484787953241c16e147cf..14059d2cac1b3467d6a8c9c4362f5a0b427b726d 100644
> --- a/Documentation/networking/mptcp-sysctl.rst
> +++ b/Documentation/networking/mptcp-sysctl.rst
> @@ -12,10 +12,9 @@ add_addr_timeout - INTEGER (seconds)
> 	resent to an MPTCP peer that has not acknowledged a previous
> 	ADD_ADDR message.
>
> -	The default value matches TCP_RTO_MAX. This is a per-namespace
> -	sysctl.
> +	This is a per-namespace sysctl.
>
> -	Default: 120
> +	Default: 1

Hi Matthieu -

I definitely agree that TCP_RTO_MAX is too large. After reading Paolo's 
feedback, I'm thinking 1 second might be a little aggressive. It seems 
like satellite links and heavily loaded mobile networks (plus a little 
buffer bloat) can lead to round trip latencies that can be hundreds of 
milliseconds. With exponential backoff, I think a slightly higher number 
would work. I noticed TCP_TIMEOUT_FALLBACK is 3 seconds, that might be a 
good starting point?

- Mat

>
> allow_join_initial_addr_port - BOOLEAN
> 	Allow peers to send join requests to the IP address and port number used
> diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
> index fed40dae5583a35914da2cb1c52f37830d72705e..92873457c47ea6b04137cece0200317a94b13e0d 100644
> --- a/net/mptcp/ctrl.c
> +++ b/net/mptcp/ctrl.c
> @@ -97,7 +97,7 @@ const char *mptcp_get_scheduler(const struct net *net)
> static void mptcp_pernet_set_defaults(struct mptcp_pernet *pernet)
> {
> 	pernet->mptcp_enabled = 1;
> -	pernet->add_addr_timeout = TCP_RTO_MAX;
> +	pernet->add_addr_timeout = HZ;	/* 1 second */
> 	pernet->blackhole_timeout = 3600;
> 	pernet->syn_retrans_before_tcp_fallback = 2;
> 	atomic_set(&pernet->active_disable_times, 0);
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index b8af65373b3ada96472347171924ad3a6cf14777..87a9d022d31160a8ff6185b3a117a73841512f9a 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -2429,6 +2429,7 @@ remove_tests()
> 		pm_nl_set_limits $ns1 3 3
> 		pm_nl_add_endpoint $ns1 10.0.12.1 flags signal
> 		# broadcast IP: no packet for this address will be received on ns1
> +		ip netns exec ${ns1} sysctl -q net.mptcp.add_addr_timeout=120
> 		pm_nl_add_endpoint $ns1 224.0.0.1 flags signal
> 		pm_nl_add_endpoint $ns1 10.0.3.1 flags signal
> 		pm_nl_set_limits $ns2 2 2
> @@ -3846,6 +3847,7 @@ endpoint_tests()
> 		pm_nl_set_limits $ns2 3 3
> 		pm_nl_add_endpoint $ns1 10.0.2.1 id 1 flags signal
> 		# broadcast IP: no packet for this address will be received on ns1
> +		ip netns exec ${ns1} sysctl -q net.mptcp.add_addr_timeout=120
> 		pm_nl_add_endpoint $ns1 224.0.0.1 id 2 flags signal
> 		pm_nl_add_endpoint $ns1 10.0.1.1 id 42 flags signal
> 		{ test_linkfail=4 speed=5 \
> @@ -3919,6 +3921,7 @@ endpoint_tests()
> 		pm_nl_set_limits $ns1 0 2
> 		pm_nl_set_limits $ns2 1 2
> 		# broadcast IP: no packet for this address will be received on ns1
> +		ip netns exec ${ns1} sysctl -q net.mptcp.add_addr_timeout=120
> 		pm_nl_add_endpoint $ns1 224.0.0.1 id 2 flags signal
> 		pm_nl_add_endpoint $ns2 10.0.3.2 id 3 flags subflow
> 		{ test_linkfail=4 speed=20 \
>
> ---
> base-commit: d196b09457190bc11ffca2867d5c12beb5a4759e
> change-id: 20250718-mptcp-add_addr_timeout_1-2b57726cf3cf
>
> Best regards,
> -- 
> Matthieu Baerts (NGI0) <matttbe@kernel.org>
>
>
>

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

* Re: [PATCH mptcp-next] mptcp: set ADD_ADDR retrans timeout to 1sec
  2025-08-14  0:20 ` Mat Martineau
@ 2025-08-15 15:35   ` Matthieu Baerts
  0 siblings, 0 replies; 5+ messages in thread
From: Matthieu Baerts @ 2025-08-15 15:35 UTC (permalink / raw)
  To: Mat Martineau; +Cc: mptcp

Hi Mat,

Thank you for your review!

On 14/08/2025 02:20, Mat Martineau wrote:
> On Fri, 18 Jul 2025, Matthieu Baerts (NGI0) wrote:
> 
>> It doesn't really make sense to wait 2 minutes -- the previous default
>> value -- to retransmit the ADD_ADDR if the echo has not been received in
>> between: that's way too long to react.
>>
>> Switching to 1 second by default, which seems like a good default value,
>> and can be increased for particular cases.
>>
>> Later on, we could eventually use a timeout based on the sRTT, with an
>> exponential backoff, which would be more reactive [1].
>>
>> Note: the documentation has been updated accordingly, and the selftests
>> are keeping the previous timeout when using a broadcast IP not to have
>> retransmissions in this case, which would cause troubles with the
>> different counters depending on how fast the test will run.
>>
>> Link: https://github.com/multipath-tcp/mptcp_net-next/issues/576 [1]
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>> Documentation/networking/mptcp-sysctl.rst       | 5 ++---
>> net/mptcp/ctrl.c                                | 2 +-
>> tools/testing/selftests/net/mptcp/mptcp_join.sh | 3 +++
>> 3 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/networking/mptcp-sysctl.rst b/
>> Documentation/networking/mptcp-sysctl.rst
>> index
>> 5bfab01eff5a9db89e1484787953241c16e147cf..14059d2cac1b3467d6a8c9c4362f5a0b427b726d 100644
>> --- a/Documentation/networking/mptcp-sysctl.rst
>> +++ b/Documentation/networking/mptcp-sysctl.rst
>> @@ -12,10 +12,9 @@ add_addr_timeout - INTEGER (seconds)
>>     resent to an MPTCP peer that has not acknowledged a previous
>>     ADD_ADDR message.
>>
>> -    The default value matches TCP_RTO_MAX. This is a per-namespace
>> -    sysctl.
>> +    This is a per-namespace sysctl.
>>
>> -    Default: 120
>> +    Default: 1
> 
> Hi Matthieu -
> 
> I definitely agree that TCP_RTO_MAX is too large. After reading Paolo's
> feedback, I'm thinking 1 second might be a little aggressive. It seems
> like satellite links and heavily loaded mobile networks (plus a little
> buffer bloat) can lead to round trip latencies that can be hundreds of
> milliseconds. With exponential backoff, I think a slightly higher number
> would work. I noticed TCP_TIMEOUT_FALLBACK is 3 seconds, that might be a
> good starting point?
Indeed, probably setting 3 seconds seems better. But I think this
modification is no longer needed now that we have Geliang's series:
"mptcp: improve ADD_ADDR timeout handling". So I'm going to archive this
patch.

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


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

end of thread, other threads:[~2025-08-15 15:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-17 23:15 [PATCH mptcp-next] mptcp: set ADD_ADDR retrans timeout to 1sec Matthieu Baerts (NGI0)
2025-07-18  0:59 ` MPTCP CI
2025-07-22 14:19 ` MPTCP CI
2025-08-14  0:20 ` Mat Martineau
2025-08-15 15:35   ` Matthieu Baerts

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).