tcp: fix TLP timer not set when CA_STATE changes from DISORDER to OPEN

commit 62d9f1a6945ba69c125e548e72a36d203b30596e upstream.

Upon receiving a cumulative ACK that changes the congestion state from
Disorder to Open, the TLP timer is not set. If the sender is app-limited,
it can only wait for the RTO timer to expire and retransmit.

The reason for this is that the TLP timer is set before the congestion
state changes in tcp_ack(), so we delay the time point of calling
tcp_set_xmit_timer() until after tcp_fastretrans_alert() returns and
remove the FLAG_SET_XMIT_TIMER from ack_flag when the RACK reorder timer
is set.

This commit has two additional benefits:
1) Make sure to reset RTO according to RFC6298 when receiving ACK, to
avoid spurious RTO caused by RTO timer early expires.
2) Reduce the xmit timer reschedule once per ACK when the RACK reorder
timer is set.

Fixes: df92c8394e6e ("tcp: fix xmit timer to only be reset if data ACKed/SACKed")
Link: https://lore.kernel.org/netdev/1611311242-6675-1-git-send-email-yangpc@wangsu.com
Signed-off-by: Pengcheng Yang <yangpc@wangsu.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Link: https://lore.kernel.org/r/1611464834-23030-1-git-send-email-yangpc@wangsu.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 58e8e57787cf168a0b8cf40e5893e0f6c6cd262e..4602959b58a12abae26da1d12fe0cea7e2ee4a47 100644 (file)
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1969,7 +1969,7 @@ void tcp_v4_init(void);
 void tcp_init(void);
 
 /* tcp_recovery.c */
-extern void tcp_rack_mark_lost(struct sock *sk);
+extern bool tcp_rack_mark_lost(struct sock *sk);
 extern void tcp_rack_advance(struct tcp_sock *tp, u8 sacked, u32 end_seq,
                             u64 xmit_time);
 extern void tcp_rack_reo_timeout(struct sock *sk);

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index ec12a52edce2ccb29afff17340d81fa495a1ba42..eb062fab625796a6464d488cefe4247a0b655ae1 100644 (file)
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2803,7 +2803,8 @@ static void tcp_rack_identify_loss(struct sock *sk, int *ack_flag)
        if (sysctl_tcp_recovery & TCP_RACK_LOSS_DETECTION) {
                u32 prior_retrans = tp->retrans_out;
 
-               tcp_rack_mark_lost(sk);
+               if (tcp_rack_mark_lost(sk))
+                       *ack_flag &= ~FLAG_SET_XMIT_TIMER;
                if (prior_retrans > tp->retrans_out)
                        *ack_flag |= FLAG_LOST_RETRANS;
        }
@@ -3688,15 +3689,16 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
 
        if (tp->tlp_high_seq)
                tcp_process_tlp_ack(sk, ack, flag);
-       /* If needed, reset TLP/RTO timer; RACK may later override this. */
-       if (flag & FLAG_SET_XMIT_TIMER)
-               tcp_set_xmit_timer(sk);
 
        if (tcp_ack_is_dubious(sk, flag)) {
                is_dupack = !(flag & (FLAG_SND_UNA_ADVANCED | FLAG_NOT_DUP));
                tcp_fastretrans_alert(sk, acked, is_dupack, &flag, &rexmit);
        }
 
+       /* If needed, reset TLP/RTO timer when RACK doesn't set. */
+       if (flag & FLAG_SET_XMIT_TIMER)
+               tcp_set_xmit_timer(sk);
+
        if ((flag & FLAG_FORWARD_PROGRESS) || !(flag & FLAG_NOT_DUP))
                sk_dst_confirm(sk);
 

diff --git a/net/ipv4/tcp_recovery.c b/net/ipv4/tcp_recovery.c
index be8ef1e5dfef79e9d1bdfef4e2199d45cc5536a3..0409cd2c4918f1dc3a0b3318ce7a599b005ec41e 100644 (file)
--- a/net/ipv4/tcp_recovery.c
+++ b/net/ipv4/tcp_recovery.c
@@ -102,13 +102,13 @@ static void tcp_rack_detect_loss(struct sock *sk, u32 *reo_timeout)
        }
 }
 
-void tcp_rack_mark_lost(struct sock *sk)
+bool tcp_rack_mark_lost(struct sock *sk)
 {
        struct tcp_sock *tp = tcp_sk(sk);
        u32 timeout;
 
        if (!tp->rack.advanced)
-               return;
+               return false;
 
        /* Reset the advanced flag to avoid unnecessary queue scanning */
        tp->rack.advanced = 0;
@@ -118,6 +118,7 @@ void tcp_rack_mark_lost(struct sock *sk)
                inet_csk_reset_xmit_timer(sk, ICSK_TIME_REO_TIMEOUT,
                                          timeout, inet_csk(sk)->icsk_rto);
        }
+       return !!timeout;
 }
 
 /* Record the most recently (re)sent time among the (s)acked packets