All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] tcp: make TCP_USER_TIMEOUT accurate for zero window probes
@ 2021-01-22 19:13 Enke Chen
  2021-01-22 20:41 ` Eric Dumazet
  2021-01-23  1:43 ` Jakub Kicinski
  0 siblings, 2 replies; 9+ messages in thread
From: Enke Chen @ 2021-01-22 19:13 UTC (permalink / raw)
  To: Eric Dumazet, David S. Miller, Jakub Kicinski, Alexey Kuznetsov,
	Hideaki YOSHIFUJI
  Cc: netdev, linux-kernel, Neal Cardwell, enkechen2020

From: Enke Chen <enchen@paloaltonetworks.com>

The TCP_USER_TIMEOUT is checked by the 0-window probe timer. As the
timer has backoff with a max interval of about two minutes, the
actual timeout for TCP_USER_TIMEOUT can be off by up to two minutes.

In this patch the TCP_USER_TIMEOUT is made more accurate by taking it
into account when computing the timer value for the 0-window probes.

This patch is similar to the one that made TCP_USER_TIMEOUT accurate for
RTOs in commit b701a99e431d ("tcp: Add tcp_clamp_rto_to_user_timeout()
helper to improve accuracy").

Signed-off-by: Enke Chen <enchen@paloaltonetworks.com>
Reviewed-by: Neal Cardwell <ncardwell@google.com>
---
 include/net/tcp.h     |  1 +
 net/ipv4/tcp_input.c  |  4 ++--
 net/ipv4/tcp_output.c |  2 ++
 net/ipv4/tcp_timer.c  | 18 ++++++++++++++++++
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 78d13c88720f..ca7e2c6cc663 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -630,6 +630,7 @@ static inline void tcp_clear_xmit_timers(struct sock *sk)
 
 unsigned int tcp_sync_mss(struct sock *sk, u32 pmtu);
 unsigned int tcp_current_mss(struct sock *sk);
+u32 tcp_clamp_probe0_to_user_timeout(const struct sock *sk, u32 when);
 
 /* Bound MSS / TSO packet size with the half of the window */
 static inline int tcp_bound_to_half_wnd(struct tcp_sock *tp, int pktsize)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index bafcab75f425..4923cdbea95a 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3392,8 +3392,8 @@ static void tcp_ack_probe(struct sock *sk)
 	} else {
 		unsigned long when = tcp_probe0_when(sk, TCP_RTO_MAX);
 
-		tcp_reset_xmit_timer(sk, ICSK_TIME_PROBE0,
-				     when, TCP_RTO_MAX);
+		when = tcp_clamp_probe0_to_user_timeout(sk, when);
+		tcp_reset_xmit_timer(sk, ICSK_TIME_PROBE0, when, TCP_RTO_MAX);
 	}
 }
 
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index ab458697881e..8478cf749821 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -4099,6 +4099,8 @@ void tcp_send_probe0(struct sock *sk)
 		 */
 		timeout = TCP_RESOURCE_PROBE_INTERVAL;
 	}
+
+	timeout = tcp_clamp_probe0_to_user_timeout(sk, timeout);
 	tcp_reset_xmit_timer(sk, ICSK_TIME_PROBE0, timeout, TCP_RTO_MAX);
 }
 
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 454732ecc8f3..90722e30ad90 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -40,6 +40,24 @@ static u32 tcp_clamp_rto_to_user_timeout(const struct sock *sk)
 	return min_t(u32, icsk->icsk_rto, msecs_to_jiffies(remaining));
 }
 
+u32 tcp_clamp_probe0_to_user_timeout(const struct sock *sk, u32 when)
+{
+	struct inet_connection_sock *icsk = inet_csk(sk);
+	u32 remaining;
+	s32 elapsed;
+
+	if (!icsk->icsk_user_timeout || !icsk->icsk_probes_tstamp)
+		return when;
+
+	elapsed = tcp_jiffies32 - icsk->icsk_probes_tstamp;
+	if (unlikely(elapsed < 0))
+		elapsed = 0;
+	remaining = msecs_to_jiffies(icsk->icsk_user_timeout) - elapsed;
+	remaining = max_t(u32, remaining, TCP_TIMEOUT_MIN);
+
+	return min_t(u32, remaining, when);
+}
+
 /**
  *  tcp_write_err() - close socket and save error info
  *  @sk:  The socket the error has appeared on.
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH net] tcp: make TCP_USER_TIMEOUT accurate for zero window probes
  2021-01-22 19:13 [PATCH net] tcp: make TCP_USER_TIMEOUT accurate for zero window probes Enke Chen
@ 2021-01-22 20:41 ` Eric Dumazet
  2021-01-23  1:43 ` Jakub Kicinski
  1 sibling, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2021-01-22 20:41 UTC (permalink / raw)
  To: Enke Chen
  Cc: David S. Miller, Jakub Kicinski, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, netdev, LKML, Neal Cardwell

On Fri, Jan 22, 2021 at 8:13 PM Enke Chen <enkechen2020@gmail.com> wrote:
>
> From: Enke Chen <enchen@paloaltonetworks.com>
>
> The TCP_USER_TIMEOUT is checked by the 0-window probe timer. As the
> timer has backoff with a max interval of about two minutes, the
> actual timeout for TCP_USER_TIMEOUT can be off by up to two minutes.
>
> In this patch the TCP_USER_TIMEOUT is made more accurate by taking it
> into account when computing the timer value for the 0-window probes.
>
> This patch is similar to the one that made TCP_USER_TIMEOUT accurate for
> RTOs in commit b701a99e431d ("tcp: Add tcp_clamp_rto_to_user_timeout()
> helper to improve accuracy").
>
> Signed-off-by: Enke Chen <enchen@paloaltonetworks.com>
> Reviewed-by: Neal Cardwell <ncardwell@google.com>
> ---

SGTM, thanks !

Signed-off-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net] tcp: make TCP_USER_TIMEOUT accurate for zero window probes
  2021-01-22 19:13 [PATCH net] tcp: make TCP_USER_TIMEOUT accurate for zero window probes Enke Chen
  2021-01-22 20:41 ` Eric Dumazet
@ 2021-01-23  1:43 ` Jakub Kicinski
  2021-01-23  2:28   ` Enke Chen
  1 sibling, 1 reply; 9+ messages in thread
From: Jakub Kicinski @ 2021-01-23  1:43 UTC (permalink / raw)
  To: Enke Chen
  Cc: Eric Dumazet, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, netdev, linux-kernel, Neal Cardwell

On Fri, 22 Jan 2021 11:13:06 -0800 Enke Chen wrote:
> From: Enke Chen <enchen@paloaltonetworks.com>
> 
> The TCP_USER_TIMEOUT is checked by the 0-window probe timer. As the
> timer has backoff with a max interval of about two minutes, the
> actual timeout for TCP_USER_TIMEOUT can be off by up to two minutes.
> 
> In this patch the TCP_USER_TIMEOUT is made more accurate by taking it
> into account when computing the timer value for the 0-window probes.
> 
> This patch is similar to the one that made TCP_USER_TIMEOUT accurate for
> RTOs in commit b701a99e431d ("tcp: Add tcp_clamp_rto_to_user_timeout()
> helper to improve accuracy").
> 
> Signed-off-by: Enke Chen <enchen@paloaltonetworks.com>
> Reviewed-by: Neal Cardwell <ncardwell@google.com>

This is targeting net, any guidance on Fixes / backporting?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net] tcp: make TCP_USER_TIMEOUT accurate for zero window probes
  2021-01-23  1:43 ` Jakub Kicinski
@ 2021-01-23  2:28   ` Enke Chen
  2021-01-23  2:34     ` Jakub Kicinski
  0 siblings, 1 reply; 9+ messages in thread
From: Enke Chen @ 2021-01-23  2:28 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Eric Dumazet, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, netdev, linux-kernel, Neal Cardwell,
	enkechen2020

Hi, Jakub:

In terms of backporting, this patch should go together with:

    9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window

Thanks.  -- Enke

On Fri, Jan 22, 2021 at 05:43:25PM -0800, Jakub Kicinski wrote:
> On Fri, 22 Jan 2021 11:13:06 -0800 Enke Chen wrote:
> > From: Enke Chen <enchen@paloaltonetworks.com>
> > 
> > The TCP_USER_TIMEOUT is checked by the 0-window probe timer. As the
> > timer has backoff with a max interval of about two minutes, the
> > actual timeout for TCP_USER_TIMEOUT can be off by up to two minutes.
> > 
> > In this patch the TCP_USER_TIMEOUT is made more accurate by taking it
> > into account when computing the timer value for the 0-window probes.
> > 
> > This patch is similar to the one that made TCP_USER_TIMEOUT accurate for
> > RTOs in commit b701a99e431d ("tcp: Add tcp_clamp_rto_to_user_timeout()
> > helper to improve accuracy").
> > 
> > Signed-off-by: Enke Chen <enchen@paloaltonetworks.com>
> > Reviewed-by: Neal Cardwell <ncardwell@google.com>
> 
> This is targeting net, any guidance on Fixes / backporting?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net] tcp: make TCP_USER_TIMEOUT accurate for zero window probes
  2021-01-23  2:28   ` Enke Chen
@ 2021-01-23  2:34     ` Jakub Kicinski
  2021-01-23  2:45       ` Enke Chen
  0 siblings, 1 reply; 9+ messages in thread
From: Jakub Kicinski @ 2021-01-23  2:34 UTC (permalink / raw)
  To: Enke Chen
  Cc: Eric Dumazet, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, netdev, linux-kernel, Neal Cardwell

On Fri, 22 Jan 2021 18:28:23 -0800 Enke Chen wrote:
> Hi, Jakub:
> 
> In terms of backporting, this patch should go together with:
> 
>     9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window

As in it:

Fixes: 9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window

or does it further fix the same issue, so:

Fixes: 9721e709fa68 ("tcp: simplify window probe aborting on USER_TIMEOUT")

?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net] tcp: make TCP_USER_TIMEOUT accurate for zero window probes
  2021-01-23  2:34     ` Jakub Kicinski
@ 2021-01-23  2:45       ` Enke Chen
  2021-01-24  0:19         ` Neal Cardwell
  0 siblings, 1 reply; 9+ messages in thread
From: Enke Chen @ 2021-01-23  2:45 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Eric Dumazet, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, netdev, linux-kernel, Neal Cardwell,
	enkechen2020

Hi, Jakub:

On Fri, Jan 22, 2021 at 06:34:24PM -0800, Jakub Kicinski wrote:
> On Fri, 22 Jan 2021 18:28:23 -0800 Enke Chen wrote:
> > Hi, Jakub:
> > 
> > In terms of backporting, this patch should go together with:
> > 
> >     9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window
> 
> As in it:
> 
> Fixes: 9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window
> 
> or does it further fix the same issue, so:
> 
> Fixes: 9721e709fa68 ("tcp: simplify window probe aborting on USER_TIMEOUT")
>
> ?

Let me clarify:

1) 9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window

   fixes the bug and makes it work.

2) The current patch makes the TCP_USER_TIMEOUT accurate for 0-window probes.
   It's independent.

With 1) and 2), the known issues with TCP_USER_TIMEOUT for 0-window probes
would be resolved.

Thanks.   -- Enke




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net] tcp: make TCP_USER_TIMEOUT accurate for zero window probes
  2021-01-23  2:45       ` Enke Chen
@ 2021-01-24  0:19         ` Neal Cardwell
  2021-01-24  0:56           ` Enke Chen
  0 siblings, 1 reply; 9+ messages in thread
From: Neal Cardwell @ 2021-01-24  0:19 UTC (permalink / raw)
  To: Enke Chen
  Cc: Jakub Kicinski, Eric Dumazet, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, Netdev, LKML

On Fri, Jan 22, 2021 at 9:45 PM Enke Chen <enkechen2020@gmail.com> wrote:
>
> Hi, Jakub:
>
> On Fri, Jan 22, 2021 at 06:34:24PM -0800, Jakub Kicinski wrote:
> > On Fri, 22 Jan 2021 18:28:23 -0800 Enke Chen wrote:
> > > Hi, Jakub:
> > >
> > > In terms of backporting, this patch should go together with:
> > >
> > >     9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window
> >
> > As in it:
> >
> > Fixes: 9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window
> >
> > or does it further fix the same issue, so:
> >
> > Fixes: 9721e709fa68 ("tcp: simplify window probe aborting on USER_TIMEOUT")
> >
> > ?
>
> Let me clarify:
>
> 1) 9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window
>
>    fixes the bug and makes it work.
>
> 2) The current patch makes the TCP_USER_TIMEOUT accurate for 0-window probes.
>    It's independent.

Patch (2) ("tcp: make TCP_USER_TIMEOUT accurate for zero window
probes") is indeed conceptually independent of (1) but its
implementation depends on the icsk_probes_tstamp field defined in (1),
so AFAICT (2) cannot be backported further back than (1).

Patch (1) fixes a bug in 5.1:
    Fixes: 9721e709fa68 ("tcp: simplify window probe aborting on USER_TIMEOUT")

So probably (1) and (2) should be backported as a pair, and only back
as far as 5.1. (That covers 2 LTS kernels, 5.4 and 5.10, so hopefully
that is good enough.)

neal

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net] tcp: make TCP_USER_TIMEOUT accurate for zero window probes
  2021-01-24  0:19         ` Neal Cardwell
@ 2021-01-24  0:56           ` Enke Chen
  2021-01-24  3:36             ` Jakub Kicinski
  0 siblings, 1 reply; 9+ messages in thread
From: Enke Chen @ 2021-01-24  0:56 UTC (permalink / raw)
  To: Neal Cardwell
  Cc: Jakub Kicinski, Eric Dumazet, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, Netdev, LKML, enkechen2020

Hi, Neal:

What you described is more accurate, and is correct.

Thanks.  -- Enke

On Sat, Jan 23, 2021 at 07:19:13PM -0500, Neal Cardwell wrote:
> On Fri, Jan 22, 2021 at 9:45 PM Enke Chen <enkechen2020@gmail.com> wrote:
> >
> > Hi, Jakub:
> >
> > On Fri, Jan 22, 2021 at 06:34:24PM -0800, Jakub Kicinski wrote:
> > > On Fri, 22 Jan 2021 18:28:23 -0800 Enke Chen wrote:
> > > > Hi, Jakub:
> > > >
> > > > In terms of backporting, this patch should go together with:
> > > >
> > > >     9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window
> > >
> > > As in it:
> > >
> > > Fixes: 9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window
> > >
> > > or does it further fix the same issue, so:
> > >
> > > Fixes: 9721e709fa68 ("tcp: simplify window probe aborting on USER_TIMEOUT")
> > >
> > > ?
> >
> > Let me clarify:
> >
> > 1) 9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window
> >
> >    fixes the bug and makes it work.
> >
> > 2) The current patch makes the TCP_USER_TIMEOUT accurate for 0-window probes.
> >    It's independent.
> 
> Patch (2) ("tcp: make TCP_USER_TIMEOUT accurate for zero window
> probes") is indeed conceptually independent of (1) but its
> implementation depends on the icsk_probes_tstamp field defined in (1),
> so AFAICT (2) cannot be backported further back than (1).
> 
> Patch (1) fixes a bug in 5.1:
>     Fixes: 9721e709fa68 ("tcp: simplify window probe aborting on USER_TIMEOUT")
> 
> So probably (1) and (2) should be backported as a pair, and only back
> as far as 5.1. (That covers 2 LTS kernels, 5.4 and 5.10, so hopefully
> that is good enough.)
> 
> neal

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net] tcp: make TCP_USER_TIMEOUT accurate for zero window probes
  2021-01-24  0:56           ` Enke Chen
@ 2021-01-24  3:36             ` Jakub Kicinski
  0 siblings, 0 replies; 9+ messages in thread
From: Jakub Kicinski @ 2021-01-24  3:36 UTC (permalink / raw)
  To: Enke Chen, Neal Cardwell, Eric Dumazet
  Cc: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Netdev, LKML

On Sat, 23 Jan 2021 16:56:43 -0800 Enke Chen wrote:
> On Sat, Jan 23, 2021 at 07:19:13PM -0500, Neal Cardwell wrote:
> > On Fri, Jan 22, 2021 at 9:45 PM Enke Chen <enkechen2020@gmail.com> wrote:  
> > > On Fri, Jan 22, 2021 at 06:34:24PM -0800, Jakub Kicinski wrote:  
> > > > On Fri, 22 Jan 2021 18:28:23 -0800 Enke Chen wrote:  
> > > > > In terms of backporting, this patch should go together with:
> > > > >
> > > > >     9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window  
> > > >
> > > > As in it:
> > > >
> > > > Fixes: 9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window
> > > >
> > > > or does it further fix the same issue, so:
> > > >
> > > > Fixes: 9721e709fa68 ("tcp: simplify window probe aborting on USER_TIMEOUT")
> > > >
> > > > ?  
> > >
> > > Let me clarify:
> > >
> > > 1) 9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window
> > >
> > >    fixes the bug and makes it work.
> > >
> > > 2) The current patch makes the TCP_USER_TIMEOUT accurate for 0-window probes.
> > >    It's independent.  
> > 
> > Patch (2) ("tcp: make TCP_USER_TIMEOUT accurate for zero window
> > probes") is indeed conceptually independent of (1) but its
> > implementation depends on the icsk_probes_tstamp field defined in (1),
> > so AFAICT (2) cannot be backported further back than (1).
> > 
> > Patch (1) fixes a bug in 5.1:
> >     Fixes: 9721e709fa68 ("tcp: simplify window probe aborting on USER_TIMEOUT")
> > 
> > So probably (1) and (2) should be backported as a pair, and only back
> > as far as 5.1. (That covers 2 LTS kernels, 5.4 and 5.10, so hopefully
> > that is good enough.)
> 
> What you described is more accurate, and is correct.

That makes it clear.

I added a Fixes tag, reworded the message slightly and applied, thanks!

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-01-24  3:37 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-22 19:13 [PATCH net] tcp: make TCP_USER_TIMEOUT accurate for zero window probes Enke Chen
2021-01-22 20:41 ` Eric Dumazet
2021-01-23  1:43 ` Jakub Kicinski
2021-01-23  2:28   ` Enke Chen
2021-01-23  2:34     ` Jakub Kicinski
2021-01-23  2:45       ` Enke Chen
2021-01-24  0:19         ` Neal Cardwell
2021-01-24  0:56           ` Enke Chen
2021-01-24  3:36             ` Jakub Kicinski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.