Skip to contentSkip to conversation

tcp: refresh tcp_mstamp from timers callbacks

PATCH In review Eric Dumazet 5 participants 5 messages Dec 13, 2017

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 tcp: refresh tcp_mstamp from timers callbacks

From: Eric Dumazet <[email protected]>

Only the retransmit timer currently refreshes tcp_mstamp

We should do the same for delayed acks and keepalives.

Even if RFC 7323 does not request it, this is consistent to what linux did in the past, when TS values were based on jiffies.

Fixes 385e20706fac ("tcp: use tp->tcp_mstamp in output path")
Cc Soheil Hassas Yeganeh <[email protected]>
Cc Mike Maloney <[email protected]>
Cc Neal Cardwell <[email protected]>

---
 net/ipv4/tcp_timer.c |    2 ++

 1 file changed, 2 insertions(+)
net/ipv4/tcp_timer.c +2 -0
index 16df6dd44b988a128d97df3a7953437499a216e8..968fda1983762e6d7c078a28ccfcbd9066788daf 100644
@@ -264,6 +264,7 @@ void tcp_delack_timer_handler(struct sock *sk)
 			icsk->icsk_ack.pingpong = 0;
 			icsk->icsk_ack.ato      = TCP_ATO_MIN;
 		}
+		tcp_mstamp_refresh(tcp_sk(sk));
 		tcp_send_ack(sk);
 		__NET_INC_STATS(sock_net(sk), LINUX_MIB_DELAYEDACKS);
 	}
@@ -632,6 +633,7 @@ static void tcp_keepalive_timer (struct timer_list *t)
 		goto out;
 	}
 
+	tcp_mstamp_refresh(tp);
 	if (sk->sk_state == TCP_FIN_WAIT2 && sock_flag(sk, SOCK_DEAD)) {
 		if (tp->linger2 >= 0) {
 			const int tmo = tcp_fin_time(sk) - TCP_TIMEWAIT_LEN;
maintainer Acked-by on v1 1/1
#1 lore raw
Quoted 9 lines · Eric Dumazet

From: Eric Dumazet <[email protected]>

Only the retransmit timer currently refreshes tcp_mstamp

We should do the same for delayed acks and keepalives.

Even if RFC 7323 does not request it, this is consistent to what linux did in the past, when TS values were based on jiffies.

Quoted 6 lines
Fixes 385e20706fac ("tcp: use tp->tcp_mstamp in output path")
Cc Soheil Hassas Yeganeh <[email protected]>
Cc Mike Maloney <[email protected]>
Cc Neal Cardwell <[email protected]>

---

Thanks, Eric!

neal

#2 lore raw
Quoted 17 lines · Neal Cardwell
Quoted 9 lines · Eric Dumazet

From: Eric Dumazet <[email protected]>

Only the retransmit timer currently refreshes tcp_mstamp

We should do the same for delayed acks and keepalives.

Even if RFC 7323 does not request it, this is consistent to what linux did in the past, when TS values were based on jiffies.

Quoted 6 lines
Fixes 385e20706fac ("tcp: use tp->tcp_mstamp in output path")
Cc Soheil Hassas Yeganeh <[email protected]>
Cc Mike Maloney <[email protected]>
Cc Neal Cardwell <[email protected]>

---

Quoted 5 lines

Thanks, Eric!

neal

This is a very nice catch! Thank you Eric!

Acked-by
#3 lore raw

Thanks for the quick fix!

Quoted 27 lines · Soheil Hassas Yeganeh
Quoted 17 lines · Neal Cardwell
Quoted 9 lines · Eric Dumazet

From: Eric Dumazet <[email protected]>

Only the retransmit timer currently refreshes tcp_mstamp

We should do the same for delayed acks and keepalives.

Even if RFC 7323 does not request it, this is consistent to what linux did in the past, when TS values were based on jiffies.

Quoted 6 lines
Fixes 385e20706fac ("tcp: use tp->tcp_mstamp in output path")
Cc Soheil Hassas Yeganeh <[email protected]>
Cc Mike Maloney <[email protected]>
Cc Neal Cardwell <[email protected]>

---

Quoted 5 lines

Thanks, Eric!

neal

This is a very nice catch! Thank you Eric!

maintainer
#4 lore raw
In reply to Eric Dumazet

From: Eric Dumazet <[email protected]>
Date: Tue, 12 Dec 2017 18:22:52 -0800

Quoted 11 lines

From: Eric Dumazet <[email protected]>

Only the retransmit timer currently refreshes tcp_mstamp

We should do the same for delayed acks and keepalives.

Even if RFC 7323 does not request it, this is consistent to what linux did in the past, when TS values were based on jiffies.

Fixes 385e20706fac ("tcp: use tp->tcp_mstamp in output path")

Applied and queued up for -stable, thanks Eric.