:p
atchew
Login
On the TCP side, the min/max RTO can be configured via the route rto_min option, the TCP_BPF_RTO_MIN and TCP_RTO_MIN_US socket options, and the tcp_rto_min_us / tcp_rto_max_ms sysctls, in that order of precedence. MPTCP did not honour any of these because its retransmit logic still used the hard-coded TCP_RTO_MIN / TCP_RTO_MAX constants. Replace the constants with the tcp_rto_min() / tcp_rto_max() helpers in the three MPTCP-level retransmit paths: - mptcp_set_datafin_timeout(): both the backoff cap computation and the resulting timer_ival now follow the configured values. Guard against a pathological rto_min >= rto_max (e.g. via BPF or racing sysctl writers) which would otherwise feed ilog2(0). - __mptcp_set_timeout(): the fallback when no subflow timeout is available now uses tcp_rto_min(). - __mptcp_init_sock(): MPTCP does not invoke tcp_init_sock() on the msk, so inet_csk(sk)->icsk_rto_min and icsk_rto_max remain zero by default. Seed them from the per-netns sysctls before using them, then set the initial timer_ival via tcp_rto_min(). The remaining uses of TCP_RTO_MAX in net/mptcp/ctrl.c (ADD_ADDR default add_addr_timeout) and net/mptcp/subflow.c (mptcp_subflow_fail() 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 Signed-off-by: Kalpan Jani <kalpan.jani@mpiricsoftware.com> --- net/mptcp/protocol.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index XXXXXXX..XXXXXXX 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -XXX,XX +XXX,XX @@ 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 = tcp_rto_min(sk); + u32 rto_max = tcp_rto_max(sk); u32 retransmits; 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) @@ -XXX,XX +XXX,XX @@ static void mptcp_worker(struct work_struct *work) static void __mptcp_init_sock(struct sock *sk) { struct mptcp_sock *msk = mptcp_sk(sk); + struct inet_connection_sock *icsk = inet_csk(sk); + struct net *net = sock_net(sk); INIT_LIST_HEAD(&msk->conn_list); INIT_LIST_HEAD(&msk->join_list); @@ -XXX,XX +XXX,XX @@ 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 = tcp_rto_min(sk); msk->scaling_ratio = TCP_DEFAULT_SCALING_RATIO; msk->backlog_len = 0; mptcp_init_rtt_est(msk); -- 2.43.0
On the TCP side, the min/max RTO can be configured via the route rto_min option, the TCP_BPF_RTO_MIN and TCP_RTO_MIN_US socket options, and the tcp_rto_min_us / tcp_rto_max_ms sysctls, in that order of precedence. MPTCP did not honour any of these because its retransmit logic still used the hard-coded TCP_RTO_MIN / TCP_RTO_MAX constants. Replace the constants with the tcp_rto_min() / tcp_rto_max() helpers in the three MPTCP-level retransmit paths: - mptcp_set_datafin_timeout(): both the backoff cap computation and the resulting timer_ival now follow the configured values. Guard against a pathological rto_min >= rto_max (e.g. via BPF or racing sysctl writers) which would otherwise feed ilog2(0). - __mptcp_set_timeout(): the fallback when no subflow timeout is available now uses tcp_rto_min(). - __mptcp_init_sock(): MPTCP does not invoke tcp_init_sock() on the msk, so inet_csk(sk)->icsk_rto_min and icsk_rto_max remain zero by default. Seed them from the per-netns sysctls before using them. The initial timer_ival reads icsk_rto_min directly rather than going through tcp_rto_min(sk): at socket init time sk_dst_cache is not yet under RCU/lock protection, and the dst lookup inside tcp_rto_min() would otherwise trip lockdep_rcu_suspicious() via __sk_dst_get() (reported by the mptcp CI on v1). The remaining uses of TCP_RTO_MAX in net/mptcp/ctrl.c (ADD_ADDR default add_addr_timeout) and net/mptcp/subflow.c (mptcp_subflow_fail() 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 Signed-off-by: Kalpan Jani <kalpan.jani@mpiricsoftware.com> --- Link to v1: https://lore.kernel.org/mptcp/20260610101123.765958-1-kalpan.jani@mpiricsoftware.com/ Changes since v1: - __mptcp_init_sock(): seed icsk_rto_min / icsk_rto_max from the per-netns sysctls so that the helpers return 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 | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index XXXXXXX..XXXXXXX 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -XXX,XX +XXX,XX @@ 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 = tcp_rto_min(sk); + u32 rto_max = tcp_rto_max(sk); u32 retransmits; 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 : tcp_rto_min(sk); } static long mptcp_timeout_from_subflow(const struct mptcp_subflow_context *subflow) @@ -XXX,XX +XXX,XX @@ static void mptcp_worker(struct work_struct *work) static void __mptcp_init_sock(struct sock *sk) { struct mptcp_sock *msk = mptcp_sk(sk); + struct inet_connection_sock *icsk = inet_csk(sk); + struct net *net = sock_net(sk); INIT_LIST_HEAD(&msk->conn_list); INIT_LIST_HEAD(&msk->join_list); @@ -XXX,XX +XXX,XX @@ 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