The MPTCP-level retransmit timers (DATA_FIN retransmissions and the
fallback timeout) used the hard-coded TCP_RTO_MIN / TCP_RTO_MAX
constants, ignoring the tcp_rto_min_us and tcp_rto_max_ms sysctls.
Make them follow the sysctls instead: seed icsk_rto_min / icsk_rto_max
on the MPTCP socket from the per-netns sysctls in __mptcp_init_sock()
-- the msk does not go through tcp_init_sock(), so these fields would
otherwise stay zero -- and read them directly where the constants were
used:
- mptcp_set_datafin_timeout(): both the backoff cap computation and
the resulting timer_ival. The two sysctls are validated
independently, so rto_min > rto_max is a valid configuration; keep
a max_t() guard so ilog2() is never called with 0.
- __mptcp_set_timeout(): the fallback when no subflow timeout is
available.
The icsk fields are read directly instead of using the
tcp_rto_min()/tcp_rto_max() helpers: the MPTCP socket does not perform
routing lookups in these paths, so the rto_min route metric checked by
tcp_rto_min() can never apply here. The TCP_RTO_MIN_US /
TCP_RTO_MAX_MS socket options are not supported by MPTCP setsockopt()
either; this can be revisited if they get supported on MPTCP sockets.
The remaining uses of TCP_RTO_MAX in net/mptcp/ctrl.c (default
add_addr_timeout) and net/mptcp/subflow.c (MP_FAIL timeout) are
intentionally left unchanged: they use the constant as a default
duration, not as an RTO bound on a retransmit timer.
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/618
Reported-by: Li Xiasong <lixiasong1@huawei.com>
Closes: https://lore.kernel.org/all/95552642-7b60-410b-9953-70e0b31a90e1@huawei.com/
Signed-off-by: Kalpan Jani <kalpan.jani@mpiricsoftware.com>
---
Link to v1: https://lore.kernel.org/mptcp/20260610101123.765958-1-kalpan.jani@mpiricsoftware.com/
Link to v2: https://lore.kernel.org/mptcp/20260611064937.422416-1-kalpan.jani@mpiricsoftware.com/
Link to v3: https://lore.kernel.org/mptcp/20260617114508.253716-1-kalpan.jani@mpiricsoftware.com/
Changes since v3:
- read icsk_rto_min / icsk_rto_max directly instead of using the
tcp_rto_min() / tcp_rto_max() helpers: the route metric checked by
the helpers cannot apply on the msk (no routing lookup in these
paths). This also removes the v2/v3 special case in
__mptcp_init_sock() that avoided the dst lookup.
- drop the rto_min == 0 floor: without the route metric, rto_min
comes from tcp_rto_min_us which has a floor of 1us, and
usecs_to_jiffies() rounds up.
- rewrite the commit message: only the sysctls are honoured; the
route metric and the TCP_RTO_MIN_US / TCP_RTO_MAX_MS socket
options are explicitly out of scope.
- a Packetdrill patch extending mptcp/dss/dss_fin_retrans_*
will be sent separately to validate mptcp_set_datafin_timeout().
Changes since v2:
- mptcp_set_datafin_timeout(): guard the division against a zero
rto_min. Thanks to Li Xiasong for spotting it.
- __mptcp_init_sock(): order the local declarations longest-first
(reverse christmas tree).
Changes since v1:
- __mptcp_init_sock(): seed icsk_rto_min / icsk_rto_max from the
per-netns sysctls so the fields hold meaningful values on the msk
(MPTCP does not call tcp_init_sock() on the msk).
- __mptcp_init_sock(): use icsk->icsk_rto_min directly for the initial
timer_ival instead of tcp_rto_min(sk), to avoid a
lockdep_rcu_suspicious() splat from __sk_dst_get() at socket init
time. Reported by the mptcp CI on v1.
- mptcp_set_datafin_timeout(): add an ilog2(0) shift-safety guard for
the rto_min >= rto_max corner case.
net/mptcp/protocol.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 373406cff72e..aab78daa8bdf 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -570,17 +570,23 @@ static bool mptcp_pending_data_fin(struct sock *sk, u64 *seq)
static void mptcp_set_datafin_timeout(struct sock *sk)
{
struct inet_connection_sock *icsk = inet_csk(sk);
+ u32 rto_min = READ_ONCE(icsk->icsk_rto_min);
+ u32 rto_max = READ_ONCE(icsk->icsk_rto_max);
u32 retransmits;
+ /* The sysctls are validated independently: rto_min > rto_max is
+ * possible, guard against ilog2(0).
+ */
retransmits = min_t(u32, icsk->icsk_retransmits,
- ilog2(TCP_RTO_MAX / TCP_RTO_MIN));
+ ilog2(max_t(u32, rto_max / rto_min, 1)));
- mptcp_sk(sk)->timer_ival = TCP_RTO_MIN << retransmits;
+ mptcp_sk(sk)->timer_ival = rto_min << retransmits;
}
static void __mptcp_set_timeout(struct sock *sk, long tout)
{
- mptcp_sk(sk)->timer_ival = tout > 0 ? tout : TCP_RTO_MIN;
+ mptcp_sk(sk)->timer_ival = tout > 0 ? tout :
+ READ_ONCE(inet_csk(sk)->icsk_rto_min);
}
static long mptcp_timeout_from_subflow(const struct mptcp_subflow_context *subflow)
@@ -3196,7 +3202,9 @@ static void mptcp_worker(struct work_struct *work)
static void __mptcp_init_sock(struct sock *sk)
{
+ struct inet_connection_sock *icsk = inet_csk(sk);
struct mptcp_sock *msk = mptcp_sk(sk);
+ struct net *net = sock_net(sk);
INIT_LIST_HEAD(&msk->conn_list);
INIT_LIST_HEAD(&msk->join_list);
@@ -3205,7 +3213,13 @@ static void __mptcp_init_sock(struct sock *sk)
INIT_WORK(&msk->work, mptcp_worker);
msk->out_of_order_queue = RB_ROOT;
msk->first_pending = NULL;
- msk->timer_ival = TCP_RTO_MIN;
+
+ /* msk does not go through tcp_init_sock(); seed RTO bounds. */
+ icsk->icsk_rto_min =
+ usecs_to_jiffies(READ_ONCE(net->ipv4.sysctl_tcp_rto_min_us));
+ icsk->icsk_rto_max =
+ msecs_to_jiffies(READ_ONCE(net->ipv4.sysctl_tcp_rto_max_ms));
+ msk->timer_ival = icsk->icsk_rto_min;
msk->scaling_ratio = TCP_DEFAULT_SCALING_RATIO;
msk->backlog_len = 0;
mptcp_init_rtt_est(msk);
--
2.43.0
Hi Kalpan, On 05/08/2026 08:39, Kalpan Jani wrote: > The MPTCP-level retransmit timers (DATA_FIN retransmissions and the > fallback timeout) used the hard-coded TCP_RTO_MIN / TCP_RTO_MAX > constants, ignoring the tcp_rto_min_us and tcp_rto_max_ms sysctls. > > Make them follow the sysctls instead: seed icsk_rto_min / icsk_rto_max > on the MPTCP socket from the per-netns sysctls in __mptcp_init_sock() > -- the msk does not go through tcp_init_sock(), so these fields would > otherwise stay zero -- and read them directly where the constants were > used: > > - mptcp_set_datafin_timeout(): both the backoff cap computation and > the resulting timer_ival. The two sysctls are validated > independently, so rto_min > rto_max is a valid configuration; keep > a max_t() guard so ilog2() is never called with 0. > > - __mptcp_set_timeout(): the fallback when no subflow timeout is > available. > > The icsk fields are read directly instead of using the > tcp_rto_min()/tcp_rto_max() helpers: the MPTCP socket does not perform > routing lookups in these paths, so the rto_min route metric checked by > tcp_rto_min() can never apply here. The TCP_RTO_MIN_US / > TCP_RTO_MAX_MS socket options are not supported by MPTCP setsockopt() > either; this can be revisited if they get supported on MPTCP sockets. > > The remaining uses of TCP_RTO_MAX in net/mptcp/ctrl.c (default > add_addr_timeout) and net/mptcp/subflow.c (MP_FAIL timeout) are > intentionally left unchanged: they use the constant as a default > duration, not as an RTO bound on a retransmit timer. Now in our tree: New patches for t/upstream: - 3dcc47c54bb6: mptcp: honour configured min/max RTO in retransmit paths - Results: 570d3b306aec..0715f5ba093a (export) Tests are now in progress: - export: https://github.com/multipath-tcp/mptcp_net-next/commit/810cb9f0a83e758a5e310b4dc7248b5d7cec76bc/checks Cheers, Matt -- Sponsored by the NGI0 Core fund.
Hi Kalpan, > The MPTCP-level retransmit timers (DATA_FIN retransmissions and the > fallback timeout) used the hard-coded TCP_RTO_MIN / TCP_RTO_MAX > constants, ignoring the tcp_rto_min_us and tcp_rto_max_ms sysctls. > > Make them follow the sysctls instead: seed icsk_rto_min / icsk_rto_max > on the MPTCP socket from the per-netns sysctls in __mptcp_init_sock() > -- the msk does not go through tcp_init_sock(), so these fields would > otherwise stay zero -- and read them directly where the constants were > used: > > - mptcp_set_datafin_timeout(): both the backoff cap computation and > the resulting timer_ival. The two sysctls are validated > independently, so rto_min > rto_max is a valid configuration; keep > a max_t() guard so ilog2() is never called with 0. > > - __mptcp_set_timeout(): the fallback when no subflow timeout is > available. > > The icsk fields are read directly instead of using the > tcp_rto_min()/tcp_rto_max() helpers: the MPTCP socket does not perform > routing lookups in these paths, so the rto_min route metric checked by > tcp_rto_min() can never apply here. The TCP_RTO_MIN_US / > TCP_RTO_MAX_MS socket options are not supported by MPTCP setsockopt() > either; this can be revisited if they get supported on MPTCP sockets. > > The remaining uses of TCP_RTO_MAX in net/mptcp/ctrl.c (default > add_addr_timeout) and net/mptcp/subflow.c (MP_FAIL timeout) are > intentionally left unchanged: they use the constant as a default > duration, not as an RTO bound on a retransmit timer. > > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/618 > Reported-by: Li Xiasong <lixiasong1@huawei.com> > Closes: https://lore.kernel.org/all/95552642-7b60-410b-9953-70e0b31a90e1@huawei.com/ I guess we can remove these two lines because it was about fixes related to the route checks that are no longer used, and this might be confusing for others, right? If yes, no need to send a new version, I can remove them when applying the patch. The rest looks good to me: Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> Also, usually there is no Reported-by tags when addressing review comments: you can either use 'Suggested-by' when the whole idea was given by someone else, mention a person in the commit message if a part of the idea is coming from someone else, or only in the changelog. BTW, thank you, Li, for the previous review! -- Matthieu Baerts (NGI0) <matttbe@kernel.org>
Hi Matt, Thank you for the review! > I guess we can remove these two lines because it was about fixes related > to the route checks that are no longer used, and this might be confusing > for others, right? Yes, please drop both lines when applying. > Also, usually there is no Reported-by tags when addressing review > comments: (...) Understood, I will make sure to follow this next time. And thanks again to Li for the reviews on the earlier versions. Cheers, Kalpan Jani From: Matthieu Baerts (NGI0) <matttbe@kernel.org> To: "Kalpan Jani"<kalpan.jani@mpiricsoftware.com> Cc: <mptcp@lists.linux.dev>, <matttbe@kernel.org>, <martineau@kernel.org>, <pabeni@redhat.com>, <shardul.b@mpiricsoftware.com>, <janak@mpiric.us>, <kalpanjani009@gmail.com>, "Li Xiasong"<lixiasong1@huawei.com> Date: Wed, 05 Aug 2026 14:56:42 +0530 Subject: Re: [PATCH mptcp-next v4] mptcp: honour configured min/max RTO in retransmit paths > Hi Kalpan, > > > The MPTCP-level retransmit timers (DATA_FIN retransmissions and the > > fallback timeout) used the hard-coded TCP_RTO_MIN / TCP_RTO_MAX > > constants, ignoring the tcp_rto_min_us and tcp_rto_max_ms sysctls. > > > > Make them follow the sysctls instead: seed icsk_rto_min / icsk_rto_max > > on the MPTCP socket from the per-netns sysctls in __mptcp_init_sock() > > -- the msk does not go through tcp_init_sock(), so these fields would > > otherwise stay zero -- and read them directly where the constants were > > used: > > > > - mptcp_set_datafin_timeout(): both the backoff cap computation and > > the resulting timer_ival. The two sysctls are validated > > independently, so rto_min > rto_max is a valid configuration; keep > > a max_t() guard so ilog2() is never called with 0. > > > > - __mptcp_set_timeout(): the fallback when no subflow timeout is > > available. > > > > The icsk fields are read directly instead of using the > > tcp_rto_min()/tcp_rto_max() helpers: the MPTCP socket does not perform > > routing lookups in these paths, so the rto_min route metric checked by > > tcp_rto_min() can never apply here. The TCP_RTO_MIN_US / > > TCP_RTO_MAX_MS socket options are not supported by MPTCP setsockopt() > > either; this can be revisited if they get supported on MPTCP sockets. > > > > The remaining uses of TCP_RTO_MAX in net/mptcp/ctrl.c (default > > add_addr_timeout) and net/mptcp/subflow.c (MP_FAIL timeout) are > > intentionally left unchanged: they use the constant as a default > > duration, not as an RTO bound on a retransmit timer. > > > > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/618 > > Reported-by: Li Xiasong <lixiasong1@huawei.com> > > Closes: https://lore.kernel.org/all/95552642-7b60-410b-9953-70e0b31a90e1@huawei.com/ > > I guess we can remove these two lines because it was about fixes related > to the route checks that are no longer used, and this might be confusing > for others, right? > > If yes, no need to send a new version, I can remove them when applying > the patch. The rest looks good to me: > > Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> > > Also, usually there is no Reported-by tags when addressing review > comments: you can either use 'Suggested-by' when the whole idea was > given by someone else, mention a person in the commit message if a part > of the idea is coming from someone else, or only in the changelog. > > BTW, thank you, Li, for the previous review! > > -- > Matthieu Baerts (NGI0) <matttbe@kernel.org> >
Hi Kalpan,
Thank you for your modifications, that's great!
Our CI did some validations and here is its report:
- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): 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/30983430746
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/f8bc9a73a07f
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1140556
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)
© 2016 - 2026 Red Hat, Inc.