All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH mptcp-next 0/3] mptcp: improve ADD_ADDR timeout handling
@ 2025-07-29  8:22 Geliang Tang
  2025-07-29  8:22 ` [PATCH mptcp-next 1/3] mptcp: rename add_addr_timeout to add_addr_timeout_max Geliang Tang
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Geliang Tang @ 2025-07-29  8:22 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

This series optimizes MPTCP's ADD_ADDR retransmission timeout:

1. Renames sysctl to add_addr_timeout_max for clearer semantics
2. Makes timeout adaptive to subflow RTT
3. Updates selftests for the rename

The adaptive timeout improves subflow establishment responsiveness
while maintaining reliability through the max timeout cap. The rename
provides better configuration clarity.

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

Geliang Tang (3):
  mptcp: rename add_addr_timeout to add_addr_timeout_max
  mptcp: make ADD_ADDR timeout adaptive to subflow RTT
  selftests: mptcp: update for add_addr_timeout_max rename

 Documentation/networking/mptcp-sysctl.rst     |  6 ++---
 net/mptcp/ctrl.c                              | 12 ++++-----
 net/mptcp/pm.c                                | 27 ++++++++++++++++---
 net/mptcp/protocol.h                          |  2 +-
 .../testing/selftests/net/mptcp/mptcp_join.sh |  8 +++---
 5 files changed, 38 insertions(+), 17 deletions(-)

-- 
2.48.1


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

* [PATCH mptcp-next 1/3] mptcp: rename add_addr_timeout to add_addr_timeout_max
  2025-07-29  8:22 [PATCH mptcp-next 0/3] mptcp: improve ADD_ADDR timeout handling Geliang Tang
@ 2025-07-29  8:22 ` Geliang Tang
  2025-07-30  9:22   ` Matthieu Baerts
  2025-07-29  8:22 ` [PATCH mptcp-next 2/3] mptcp: make ADD_ADDR timeout adaptive to subflow RTT Geliang Tang
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Geliang Tang @ 2025-07-29  8:22 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

This renames the MPTCP sysctl variable to better reflect its purpose as
the maximum ADD_ADDR retransmission timeout.

The patch includes updates to:
 - Documentation/networking/mptcp-sysctl.rst
 - net/mptcp/ctrl.c
 - net/mptcp/pm.c
 - net/mptcp/protocol.h

All references to the old variable name have been updated to use the new
naming convention.

The functionality remains unchanged.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 Documentation/networking/mptcp-sysctl.rst |  6 +++---
 net/mptcp/ctrl.c                          | 12 ++++++------
 net/mptcp/pm.c                            |  6 +++---
 net/mptcp/protocol.h                      |  2 +-
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/Documentation/networking/mptcp-sysctl.rst b/Documentation/networking/mptcp-sysctl.rst
index 5bfab01eff5a..1af35757a235 100644
--- a/Documentation/networking/mptcp-sysctl.rst
+++ b/Documentation/networking/mptcp-sysctl.rst
@@ -7,9 +7,9 @@ MPTCP Sysfs variables
 /proc/sys/net/mptcp/* Variables
 ===============================
 
-add_addr_timeout - INTEGER (seconds)
-	Set the timeout after which an ADD_ADDR control message will be
-	resent to an MPTCP peer that has not acknowledged a previous
+add_addr_timeout_max - INTEGER (seconds)
+	Set the maximum value of timeout after which an ADD_ADDR control message
+	will be 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
diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index fed40dae5583..9a408e96c07f 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -27,7 +27,7 @@ struct mptcp_pernet {
 	struct ctl_table_header *ctl_table_hdr;
 #endif
 
-	unsigned int add_addr_timeout;
+	unsigned int add_addr_timeout_max;
 	unsigned int blackhole_timeout;
 	unsigned int close_timeout;
 	unsigned int stale_loss_cnt;
@@ -52,9 +52,9 @@ int mptcp_is_enabled(const struct net *net)
 	return mptcp_get_pernet(net)->mptcp_enabled;
 }
 
-unsigned int mptcp_get_add_addr_timeout(const struct net *net)
+unsigned int mptcp_get_add_addr_timeout_max(const struct net *net)
 {
-	return mptcp_get_pernet(net)->add_addr_timeout;
+	return mptcp_get_pernet(net)->add_addr_timeout_max;
 }
 
 int mptcp_is_checksum_enabled(const struct net *net)
@@ -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_max = TCP_RTO_MAX;
 	pernet->blackhole_timeout = 3600;
 	pernet->syn_retrans_before_tcp_fallback = 2;
 	atomic_set(&pernet->active_disable_times, 0);
@@ -284,7 +284,7 @@ static struct ctl_table mptcp_sysctl_table[] = {
 		.extra2       = SYSCTL_ONE
 	},
 	{
-		.procname = "add_addr_timeout",
+		.procname = "add_addr_timeout_max",
 		.maxlen = sizeof(unsigned int),
 		.mode = 0644,
 		.proc_handler = proc_dointvec_jiffies,
@@ -377,7 +377,7 @@ static int mptcp_pernet_new_table(struct net *net, struct mptcp_pernet *pernet)
 	}
 
 	table[0].data = &pernet->mptcp_enabled;
-	table[1].data = &pernet->add_addr_timeout;
+	table[1].data = &pernet->add_addr_timeout_max;
 	table[2].data = &pernet->checksum_enabled;
 	table[3].data = &pernet->allow_join_initial_addr_port;
 	table[4].data = &pernet->stale_loss_cnt;
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 687dbb59d084..40e8ebe566e5 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -302,7 +302,7 @@ static void mptcp_pm_add_timer(struct timer_list *timer)
 
 	if (entry->retrans_times < ADD_ADDR_RETRANS_MAX)
 		sk_reset_timer(sk, timer,
-			       jiffies + mptcp_get_add_addr_timeout(sock_net(sk)));
+			       jiffies + mptcp_get_add_addr_timeout_max(sock_net(sk)));
 
 	spin_unlock_bh(&msk->pm.lock);
 
@@ -354,7 +354,7 @@ bool mptcp_pm_alloc_anno_list(struct mptcp_sock *msk,
 			return false;
 
 		sk_reset_timer(sk, &add_entry->add_timer,
-			       jiffies + mptcp_get_add_addr_timeout(net));
+			       jiffies + mptcp_get_add_addr_timeout_max(net));
 		return true;
 	}
 
@@ -370,7 +370,7 @@ bool mptcp_pm_alloc_anno_list(struct mptcp_sock *msk,
 
 	timer_setup(&add_entry->add_timer, mptcp_pm_add_timer, 0);
 	sk_reset_timer(sk, &add_entry->add_timer,
-		       jiffies + mptcp_get_add_addr_timeout(net));
+		       jiffies + mptcp_get_add_addr_timeout_max(net));
 
 	return true;
 }
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index db50b216143a..f2dd45148e63 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -708,7 +708,7 @@ mptcp_subflow_delegated_next(struct mptcp_delegated_action *delegated)
 }
 
 int mptcp_is_enabled(const struct net *net);
-unsigned int mptcp_get_add_addr_timeout(const struct net *net);
+unsigned int mptcp_get_add_addr_timeout_max(const struct net *net);
 int mptcp_is_checksum_enabled(const struct net *net);
 int mptcp_allow_join_id0(const struct net *net);
 unsigned int mptcp_stale_loss_cnt(const struct net *net);
-- 
2.48.1


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

* [PATCH mptcp-next 2/3] mptcp: make ADD_ADDR timeout adaptive to subflow RTT
  2025-07-29  8:22 [PATCH mptcp-next 0/3] mptcp: improve ADD_ADDR timeout handling Geliang Tang
  2025-07-29  8:22 ` [PATCH mptcp-next 1/3] mptcp: rename add_addr_timeout to add_addr_timeout_max Geliang Tang
@ 2025-07-29  8:22 ` Geliang Tang
  2025-07-30  9:26   ` Matthieu Baerts
  2025-07-29  8:22 ` [PATCH mptcp-next 3/3] selftests: mptcp: update for add_addr_timeout_max rename Geliang Tang
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Geliang Tang @ 2025-07-29  8:22 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch makes the ADD_ADDR retransmission timeout adaptive by using
the maximum subflow RTT, while still capping it at the configured max
value (add_addr_timeout_max). This improves responsiveness when
establishing new subflows.

The change:
 - Adds mptcp_get_add_addr_timeout() helper
 - Uses subflow RTT when available
 - Falls back to max timeout otherwise

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 40e8ebe566e5..bf4d92293e1e 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -268,6 +268,27 @@ int mptcp_pm_mp_prio_send_ack(struct mptcp_sock *msk,
 	return -EINVAL;
 }
 
+static unsigned int mptcp_get_add_addr_timeout(struct mptcp_sock *msk,
+					       const struct net *net)
+{
+	unsigned int timeout = mptcp_get_add_addr_timeout_max(net);
+	struct mptcp_subflow_context *subflow;
+	unsigned int srtt_us = 0;
+
+	mptcp_for_each_subflow(msk, subflow) {
+		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+		struct tcp_sock *tp = tcp_sk(ssk);
+
+		if (srtt_us < tp->srtt_us)
+			srtt_us = tp->srtt_us;
+	}
+
+	if (srtt_us && srtt_us < timeout)
+		timeout = srtt_us;
+
+	return timeout;
+}
+
 static void mptcp_pm_add_timer(struct timer_list *timer)
 {
 	struct mptcp_pm_add_entry *entry = timer_container_of(entry, timer,
@@ -302,7 +323,7 @@ static void mptcp_pm_add_timer(struct timer_list *timer)
 
 	if (entry->retrans_times < ADD_ADDR_RETRANS_MAX)
 		sk_reset_timer(sk, timer,
-			       jiffies + mptcp_get_add_addr_timeout_max(sock_net(sk)));
+			       jiffies + mptcp_get_add_addr_timeout(msk, sock_net(sk)));
 
 	spin_unlock_bh(&msk->pm.lock);
 
@@ -354,7 +375,7 @@ bool mptcp_pm_alloc_anno_list(struct mptcp_sock *msk,
 			return false;
 
 		sk_reset_timer(sk, &add_entry->add_timer,
-			       jiffies + mptcp_get_add_addr_timeout_max(net));
+			       jiffies + mptcp_get_add_addr_timeout(msk, net));
 		return true;
 	}
 
@@ -370,7 +391,7 @@ bool mptcp_pm_alloc_anno_list(struct mptcp_sock *msk,
 
 	timer_setup(&add_entry->add_timer, mptcp_pm_add_timer, 0);
 	sk_reset_timer(sk, &add_entry->add_timer,
-		       jiffies + mptcp_get_add_addr_timeout_max(net));
+		       jiffies + mptcp_get_add_addr_timeout(msk, net));
 
 	return true;
 }
-- 
2.48.1


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

* [PATCH mptcp-next 3/3] selftests: mptcp: update for add_addr_timeout_max rename
  2025-07-29  8:22 [PATCH mptcp-next 0/3] mptcp: improve ADD_ADDR timeout handling Geliang Tang
  2025-07-29  8:22 ` [PATCH mptcp-next 1/3] mptcp: rename add_addr_timeout to add_addr_timeout_max Geliang Tang
  2025-07-29  8:22 ` [PATCH mptcp-next 2/3] mptcp: make ADD_ADDR timeout adaptive to subflow RTT Geliang Tang
@ 2025-07-29  8:22 ` Geliang Tang
  2025-07-30  9:26   ` Matthieu Baerts
  2025-07-29 10:24 ` [PATCH mptcp-next 0/3] mptcp: improve ADD_ADDR timeout handling MPTCP CI
  2025-07-30  9:20 ` Matthieu Baerts
  4 siblings, 1 reply; 9+ messages in thread
From: Geliang Tang @ 2025-07-29  8:22 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

Update MPTCP selftests to use the renamed sysctl variable
add_addr_timeout_max. This aligns tests with the kernel change.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index b8af65373b3a..fec9dda192be 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -347,7 +347,7 @@ reset_with_add_addr_timeout()
 		tables="${ip6tables}"
 	fi
 
-	ip netns exec $ns1 sysctl -q net.mptcp.add_addr_timeout=1
+	ip netns exec $ns1 sysctl -q net.mptcp.add_addr_timeout_max=1
 
 	if ! ip netns exec $ns2 $tables -A OUTPUT -p tcp \
 			-m tcp --tcp-option 30 \
@@ -1556,7 +1556,7 @@ chk_add_nr()
 		rx=" server"
 	fi
 
-	timeout=$(ip netns exec ${ns_tx} sysctl -n net.mptcp.add_addr_timeout)
+	timeout=$(ip netns exec ${ns_tx} sysctl -n net.mptcp.add_addr_timeout_max)
 
 	print_check "add addr rx${rx}"
 	count=$(mptcp_lib_get_counter ${ns_rx} "MPTcpExtAddAddr")
@@ -1655,7 +1655,7 @@ chk_add_tx_nr()
 	local timeout
 	local count
 
-	timeout=$(ip netns exec $ns1 sysctl -n net.mptcp.add_addr_timeout)
+	timeout=$(ip netns exec $ns1 sysctl -n net.mptcp.add_addr_timeout_max)
 
 	print_check "add addr tx"
 	count=$(mptcp_lib_get_counter ${ns1} "MPTcpExtAddAddrTx")
@@ -2183,7 +2183,7 @@ signal_address_tests()
 		pm_nl_add_endpoint $ns2 10.0.4.2 flags signal
 
 		# the peer could possibly miss some addr notification, allow retransmission
-		ip netns exec $ns1 sysctl -q net.mptcp.add_addr_timeout=1
+		ip netns exec $ns1 sysctl -q net.mptcp.add_addr_timeout_max=1
 		speed=slow \
 			run_tests $ns1 $ns2 10.0.1.1
 
-- 
2.48.1


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

* Re: [PATCH mptcp-next 0/3] mptcp: improve ADD_ADDR timeout handling
  2025-07-29  8:22 [PATCH mptcp-next 0/3] mptcp: improve ADD_ADDR timeout handling Geliang Tang
                   ` (2 preceding siblings ...)
  2025-07-29  8:22 ` [PATCH mptcp-next 3/3] selftests: mptcp: update for add_addr_timeout_max rename Geliang Tang
@ 2025-07-29 10:24 ` MPTCP CI
  2025-07-30  9:20 ` Matthieu Baerts
  4 siblings, 0 replies; 9+ messages in thread
From: MPTCP CI @ 2025-07-29 10:24 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: Unstable: 2 failed test(s): packetdrill_add_addr selftest_mptcp_connect 🔴
- KVM Validation: debug: Unstable: 3 failed test(s): packetdrill_add_addr packetdrill_sockopts selftest_mptcp_connect_checksum 🔴
- 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/16591166379

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


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

* Re: [PATCH mptcp-next 0/3] mptcp: improve ADD_ADDR timeout handling
  2025-07-29  8:22 [PATCH mptcp-next 0/3] mptcp: improve ADD_ADDR timeout handling Geliang Tang
                   ` (3 preceding siblings ...)
  2025-07-29 10:24 ` [PATCH mptcp-next 0/3] mptcp: improve ADD_ADDR timeout handling MPTCP CI
@ 2025-07-30  9:20 ` Matthieu Baerts
  4 siblings, 0 replies; 9+ messages in thread
From: Matthieu Baerts @ 2025-07-30  9:20 UTC (permalink / raw)
  To: Geliang Tang, mptcp; +Cc: Geliang Tang

Hi Geliang,

On 29/07/2025 10:22, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This series optimizes MPTCP's ADD_ADDR retransmission timeout:
> 
> 1. Renames sysctl to add_addr_timeout_max for clearer semantics
> 2. Makes timeout adaptive to subflow RTT
> 3. Updates selftests for the rename
> 
> The adaptive timeout improves subflow establishment responsiveness
> while maintaining reliability through the max timeout cap. The rename
> provides better configuration clarity.

Thank you for working on that. I have some comments, please see the
individual patches.

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

Better to (also) put that in the last patch closing the ticket, so
GitHub can automatically close it when this patch will be applied.

> Geliang Tang (3):
>   mptcp: rename add_addr_timeout to add_addr_timeout_max
>   mptcp: make ADD_ADDR timeout adaptive to subflow RTT
>   selftests: mptcp: update for add_addr_timeout_max rename
> 
>  Documentation/networking/mptcp-sysctl.rst     |  6 ++---
>  net/mptcp/ctrl.c                              | 12 ++++-----
>  net/mptcp/pm.c                                | 27 ++++++++++++++++---
>  net/mptcp/protocol.h                          |  2 +-
>  .../testing/selftests/net/mptcp/mptcp_join.sh |  8 +++---
>  5 files changed, 38 insertions(+), 17 deletions(-)
> 

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


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

* Re: [PATCH mptcp-next 1/3] mptcp: rename add_addr_timeout to add_addr_timeout_max
  2025-07-29  8:22 ` [PATCH mptcp-next 1/3] mptcp: rename add_addr_timeout to add_addr_timeout_max Geliang Tang
@ 2025-07-30  9:22   ` Matthieu Baerts
  0 siblings, 0 replies; 9+ messages in thread
From: Matthieu Baerts @ 2025-07-30  9:22 UTC (permalink / raw)
  To: Geliang Tang, mptcp; +Cc: Geliang Tang

Hi Geliang,

On 29/07/2025 10:22, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This renames the MPTCP sysctl variable to better reflect its purpose as
> the maximum ADD_ADDR retransmission timeout.

I don't think we can rename a sysctl knob: it is sort of part of the API
as it is exposed. Either we deprecate it with a warning in the doc +
pr_warn() somewhere, or use it as a max value without renaming it (at
least not renaming the exposed variable: .procname).

I think it should be fine to use it as a max value because it was set to
TCP_RTO_MAX by default, and this timeout is in second. If you do that,
you can probably squash patch 1 and 2.

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


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

* Re: [PATCH mptcp-next 2/3] mptcp: make ADD_ADDR timeout adaptive to subflow RTT
  2025-07-29  8:22 ` [PATCH mptcp-next 2/3] mptcp: make ADD_ADDR timeout adaptive to subflow RTT Geliang Tang
@ 2025-07-30  9:26   ` Matthieu Baerts
  0 siblings, 0 replies; 9+ messages in thread
From: Matthieu Baerts @ 2025-07-30  9:26 UTC (permalink / raw)
  To: Geliang Tang, mptcp; +Cc: Geliang Tang

Hi Geliang,

On 29/07/2025 10:22, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch makes the ADD_ADDR retransmission timeout adaptive by using
> the maximum subflow RTT, while still capping it at the configured max
> value (add_addr_timeout_max). This improves responsiveness when
> establishing new subflows.
> 
> The change:
>  - Adds mptcp_get_add_addr_timeout() helper
>  - Uses subflow RTT when available
>  - Falls back to max timeout otherwise
> 

Can you add the Closes tag here, so GitHub will automatically close the
ticket when this patch will be applied?

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

> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/pm.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 40e8ebe566e5..bf4d92293e1e 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -268,6 +268,27 @@ int mptcp_pm_mp_prio_send_ack(struct mptcp_sock *msk,
>  	return -EINVAL;
>  }
>  
> +static unsigned int mptcp_get_add_addr_timeout(struct mptcp_sock *msk,
> +					       const struct net *net)
> +{
> +	unsigned int timeout = mptcp_get_add_addr_timeout_max(net);
> +	struct mptcp_subflow_context *subflow;
> +	unsigned int srtt_us = 0;
> +
> +	mptcp_for_each_subflow(msk, subflow) {
> +		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> +		struct tcp_sock *tp = tcp_sk(ssk);
> +
> +		if (srtt_us < tp->srtt_us)

(detail: it feels more logical do have "if (tp->srtt_us > srtt_us)" if
you look for the max, but the result is the same)

> +			srtt_us = tp->srtt_us;
> +	}
> +
> +	if (srtt_us && srtt_us < timeout)

I don't think you can do that: timeout is in jiffies, while srtt_us is
in µsec. You need to use "usecs_to_jiffies(srtt_us)"

> +		timeout = srtt_us;

Taking just srtt_us doesn't seem to be a good idea as the ADD_ADDR echo
is supposed to arrive around srtt_us: this will certainly cause too
aggressive ADD_ADDR retransmissions if timeout is around the time the
echo is supposed to arrive.

I think we should imitate TCP here: using srtt_us and rttvar_us. (Or
start with "srtt_us << 1"?). Or maybe better to use "icsk->icsk_rto"
instead, but I don't know if it will be set to a correct value when we
will look, no?

> +
> +	return timeout;
> +}
> +
>  static void mptcp_pm_add_timer(struct timer_list *timer)
>  {
>  	struct mptcp_pm_add_entry *entry = timer_container_of(entry, timer,
> @@ -302,7 +323,7 @@ static void mptcp_pm_add_timer(struct timer_list *timer)
>  
>  	if (entry->retrans_times < ADD_ADDR_RETRANS_MAX)
>  		sk_reset_timer(sk, timer,
> -			       jiffies + mptcp_get_add_addr_timeout_max(sock_net(sk)));
> +			       jiffies + mptcp_get_add_addr_timeout(msk, sock_net(sk)));

I wonder if we should increase the time after each retransmission,
similar to what is done in TCP (timeout << entry->retrans_times). We
could keep it linear, but because this time now depends on the measured
srtt, it might be better to have an exponential backoff. It will still
need to be bounded to the max. In this case, it might be easier to pass
entry->retrans_times to mptcp_get_add_addr_timeout())

>  
>  	spin_unlock_bh(&msk->pm.lock);
>  
> @@ -354,7 +375,7 @@ bool mptcp_pm_alloc_anno_list(struct mptcp_sock *msk,
>  			return false;
>  
>  		sk_reset_timer(sk, &add_entry->add_timer,
> -			       jiffies + mptcp_get_add_addr_timeout_max(net));
> +			       jiffies + mptcp_get_add_addr_timeout(msk, net));
>  		return true;
>  	}
>  
> @@ -370,7 +391,7 @@ bool mptcp_pm_alloc_anno_list(struct mptcp_sock *msk,
>  
>  	timer_setup(&add_entry->add_timer, mptcp_pm_add_timer, 0);
>  	sk_reset_timer(sk, &add_entry->add_timer,
> -		       jiffies + mptcp_get_add_addr_timeout_max(net));
> +		       jiffies + mptcp_get_add_addr_timeout(msk, net));
>  
>  	return true;
>  }

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


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

* Re: [PATCH mptcp-next 3/3] selftests: mptcp: update for add_addr_timeout_max rename
  2025-07-29  8:22 ` [PATCH mptcp-next 3/3] selftests: mptcp: update for add_addr_timeout_max rename Geliang Tang
@ 2025-07-30  9:26   ` Matthieu Baerts
  0 siblings, 0 replies; 9+ messages in thread
From: Matthieu Baerts @ 2025-07-30  9:26 UTC (permalink / raw)
  To: Geliang Tang, mptcp; +Cc: Geliang Tang

Hi Geliang,

On 29/07/2025 10:22, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Update MPTCP selftests to use the renamed sysctl variable
> add_addr_timeout_max. This aligns tests with the kernel change.

This should not be needed, see my comment on patch 1/3.

What you will probably need to adapt is the packetdrill tests dedicated
to the ADD_ADDR: mptcp/add_addr. These tests expect a retransmission
every second, because net.mptcp.add_addr_timeout sysctl knob is set to 1.

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


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

end of thread, other threads:[~2025-07-30  9:26 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-29  8:22 [PATCH mptcp-next 0/3] mptcp: improve ADD_ADDR timeout handling Geliang Tang
2025-07-29  8:22 ` [PATCH mptcp-next 1/3] mptcp: rename add_addr_timeout to add_addr_timeout_max Geliang Tang
2025-07-30  9:22   ` Matthieu Baerts
2025-07-29  8:22 ` [PATCH mptcp-next 2/3] mptcp: make ADD_ADDR timeout adaptive to subflow RTT Geliang Tang
2025-07-30  9:26   ` Matthieu Baerts
2025-07-29  8:22 ` [PATCH mptcp-next 3/3] selftests: mptcp: update for add_addr_timeout_max rename Geliang Tang
2025-07-30  9:26   ` Matthieu Baerts
2025-07-29 10:24 ` [PATCH mptcp-next 0/3] mptcp: improve ADD_ADDR timeout handling MPTCP CI
2025-07-30  9:20 ` 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.