[PATCH mptcp-next v4] mptcp: honour configured min/max RTO in retransmit paths

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 <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]/
Signed-off-by: Kalpan Jani <[email protected]>
---
Link to v1: https://lore.kernel.org/mptcp/[email protected]/
Link to v2: https://lore.kernel.org/mptcp/[email protected]/
Link to v3: https://lore.kernel.org/mptcp/[email protected]/

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