Skip to content

Commit d2a0fc3

Browse files
Fred Chendavem330
authored andcommitted
tcp: fix wrong RTO timeout when received SACK reneging
This commit fix wrong RTO timeout when received SACK reneging. When an ACK arrived pointing to a SACK reneging, tcp_check_sack_reneging() will rearm the RTO timer for min(1/2*srtt, 10ms) into to the future. But since the commit 62d9f1a ("tcp: fix TLP timer not set when CA_STATE changes from DISORDER to OPEN") merged, the tcp_set_xmit_timer() is moved after tcp_fastretrans_alert()(which do the SACK reneging check), so the RTO timeout will be overwrited by tcp_set_xmit_timer() with icsk_rto instead of 1/2*srtt. Here is a packetdrill script to check this bug: 0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3 +0 bind(3, ..., ...) = 0 +0 listen(3, 1) = 0 // simulate srtt to 100ms +0 < S 0:0(0) win 32792 <mss 1000, sackOK,nop,nop,nop,wscale 7> +0 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK,nop,wscale 7> +.1 < . 1:1(0) ack 1 win 1024 +0 accept(3, ..., ...) = 4 +0 write(4, ..., 10000) = 10000 +0 > P. 1:10001(10000) ack 1 // inject sack +.1 < . 1:1(0) ack 1 win 257 <sack 1001:10001,nop,nop> +0 > . 1:1001(1000) ack 1 // inject sack reneging +.1 < . 1:1(0) ack 1001 win 257 <sack 9001:10001,nop,nop> // we expect rto fired in 1/2*srtt (50ms) +.05 > . 1001:2001(1000) ack 1 This fix remove the FLAG_SET_XMIT_TIMER from ack_flag when tcp_check_sack_reneging() set RTO timer with 1/2*srtt to avoid being overwrited later. Fixes: 62d9f1a ("tcp: fix TLP timer not set when CA_STATE changes from DISORDER to OPEN") Signed-off-by: Fred Chen <[email protected]> Reviewed-by: Neal Cardwell <[email protected]> Tested-by: Neal Cardwell <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent a40614f commit d2a0fc3

File tree

1 file changed

+5
-4
lines changed

1 file changed

+5
-4
lines changed

net/ipv4/tcp_input.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2207,16 +2207,17 @@ void tcp_enter_loss(struct sock *sk)
22072207
* restore sanity to the SACK scoreboard. If the apparent reneging
22082208
* persists until this RTO then we'll clear the SACK scoreboard.
22092209
*/
2210-
static bool tcp_check_sack_reneging(struct sock *sk, int flag)
2210+
static bool tcp_check_sack_reneging(struct sock *sk, int *ack_flag)
22112211
{
2212-
if (flag & FLAG_SACK_RENEGING &&
2213-
flag & FLAG_SND_UNA_ADVANCED) {
2212+
if (*ack_flag & FLAG_SACK_RENEGING &&
2213+
*ack_flag & FLAG_SND_UNA_ADVANCED) {
22142214
struct tcp_sock *tp = tcp_sk(sk);
22152215
unsigned long delay = max(usecs_to_jiffies(tp->srtt_us >> 4),
22162216
msecs_to_jiffies(10));
22172217

22182218
inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
22192219
delay, TCP_RTO_MAX);
2220+
*ack_flag &= ~FLAG_SET_XMIT_TIMER;
22202221
return true;
22212222
}
22222223
return false;
@@ -2986,7 +2987,7 @@ static void tcp_fastretrans_alert(struct sock *sk, const u32 prior_snd_una,
29862987
tp->prior_ssthresh = 0;
29872988

29882989
/* B. In all the states check for reneging SACKs. */
2989-
if (tcp_check_sack_reneging(sk, flag))
2990+
if (tcp_check_sack_reneging(sk, ack_flag))
29902991
return;
29912992

29922993
/* C. Check consistency of the current state. */

0 commit comments

Comments
 (0)