diff mbox

[net-next] tcp: fix tcp_probe_timer() for TCP_USER_TIMEOUT

Message ID 1495388340.6465.50.camel@edumazet-glaptop3.roam.corp.google.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Eric Dumazet May 21, 2017, 5:39 p.m. UTC 
From: Eric Dumazet <edumazet@google.com>

TCP_USER_TIMEOUT is still converted to jiffies value in
icsk_user_timeout

So we need to make a conversion for the cases HZ != 1000 

Fixes: 9a568de4818d ("tcp: switch TCP TS option (RFC 7323) to 1ms clock")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/tcp_timer.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Soheil Hassas Yeganeh May 21, 2017, 5:46 p.m. UTC | #1 
On Sun, May 21, 2017 at 1:39 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> TCP_USER_TIMEOUT is still converted to jiffies value in
> icsk_user_timeout
>
> So we need to make a conversion for the cases HZ != 1000
>
> Fixes: 9a568de4818d ("tcp: switch TCP TS option (RFC 7323) to 1ms clock")
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Acked-by: Soheil Hassas Yeganeh <soheil@google.com>

Thank you for the fix, Eric!
David Miller May 21, 2017, 5:51 p.m. UTC | #2 
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 21 May 2017 10:39:00 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> TCP_USER_TIMEOUT is still converted to jiffies value in
> icsk_user_timeout
> 
> So we need to make a conversion for the cases HZ != 1000 
> 
> Fixes: 9a568de4818d ("tcp: switch TCP TS option (RFC 7323) to 1ms clock")
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied, thank Eric.

I kinda expected a few pieces of fallout from the 1ms changes :)
Eric Dumazet May 21, 2017, 6 p.m. UTC | #3 
On Sun, 2017-05-21 at 13:51 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Sun, 21 May 2017 10:39:00 -0700
> 
> > From: Eric Dumazet <edumazet@google.com>
> > 
> > TCP_USER_TIMEOUT is still converted to jiffies value in
> > icsk_user_timeout
> > 
> > So we need to make a conversion for the cases HZ != 1000 
> > 
> > Fixes: 9a568de4818d ("tcp: switch TCP TS option (RFC 7323) to 1ms clock")
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> 
> Applied, thank Eric.
> 
> I kinda expected a few pieces of fallout from the 1ms changes :)

Absolutely ;)

One last piece is in TCP_SYNCNT support.

I saw that retransmits_timed_out() could have a rounding error :

tcp_time_stamp(tcp_sk(sk)) - start_ts) ends up to 999 ms,
while the timeout is/was 1000 ms (And timer _was_ progammed with 1000
jiffies for HZ=1000 kernel)

So if user setup TCP_SYNCNT = 1 socket option, we sometime sends one
extra SYN packet.

I will send a fix later.
Eric Dumazet May 21, 2017, 6:01 p.m. UTC | #4 
On Sun, 2017-05-21 at 13:46 -0400, Soheil Hassas Yeganeh wrote:
> On Sun, May 21, 2017 at 1:39 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > From: Eric Dumazet <edumazet@google.com>
> >
> > TCP_USER_TIMEOUT is still converted to jiffies value in
> > icsk_user_timeout
> >
> > So we need to make a conversion for the cases HZ != 1000
> >
> > Fixes: 9a568de4818d ("tcp: switch TCP TS option (RFC 7323) to 1ms clock")
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> 
> Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
> 
> Thank you for the fix, Eric!

Thanks for reviewing !
diff mbox

Patch

diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 27a667bce8060e6b2290fe636c27a79d0d593b48..c4a35ba7f8ed0dac573c864900b081b4847927d8 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -341,7 +341,8 @@  static void tcp_probe_timer(struct sock *sk)
 	if (!start_ts)
 		tcp_send_head(sk)->skb_mstamp = tp->tcp_mstamp;
 	else if (icsk->icsk_user_timeout &&
-		 (s32)(tcp_time_stamp(tp) - start_ts) > icsk->icsk_user_timeout)
+		 (s32)(tcp_time_stamp(tp) - start_ts) >
+		 jiffies_to_msecs(icsk->icsk_user_timeout))
 		goto abort;
 
 	max_probes = sock_net(sk)->ipv4.sysctl_tcp_retries2;