netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/3] Series to improve setsockopt() TCP_USER_TIMEOUT accuracy
@ 2018-07-17  4:15 Jon Maxwell
  2018-07-17  4:56 ` Eric Dumazet
  0 siblings, 1 reply; 3+ messages in thread
From: Jon Maxwell @ 2018-07-17  4:15 UTC (permalink / raw)
  To: davem
  Cc: edumazet, ncardwell, David.Laight, kuznet, yoshfuji, netdev,
	linux-kernel, jmaxwell

This is a patch series based on:

https://patchwork.kernel.org/patch/10516195/

Every time the TCP retransmission timer fires. It checks to see if there is a 
timeout before scheduling the next retransmit timer. The retransmit interval 
between each retransmission increases exponentially. The issue is that in order 
for the timeout to occur the retransmit timer needs to fire again. If the user 
timeout check happens after the 9th retransmit for example. It needs to wait for 
the 10th retransmit timer to fire in order to evaluate whether a timeout has 
occurred or not. If the interval is large enough then the timeout will be 
inaccurate.

For example with a TCP_USER_TIMEOUT of 10 seconds without patch:

1st retransmit:

22:25:18.973488 IP host1.49310 > host2.search-agent: Flags [.]

Last retransmit:

22:25:26.205499 IP host1.49310 > host2.search-agent: Flags [.]

Timeout:

send: Connection timed out
Sun Jul  1 22:25:34 EDT 2018

We can see that last retransmit took ~7 seconds. Which pushed the total 
timeout to ~15 seconds instead of the expected 10 seconds. This gets more 
inaccurate the larger the TCP_USER_TIMEOUT value. As the interval increases.

Add tcp_clamp_rto_to_user_timeout() to determine if the user rto has expired.
Or whether the rto interval needs to be recalculated. Use the original interval
if user rto is not set. 

Test results with the patch is the expected 10 second timeout:

1st retransmit:

01:37:59.022555 IP host1.49310 > host2.search-agent: Flags [.]

Last retransmit:

01:38:06.486558 IP host1.49310 > host2.search-agent: Flags [.]

Timeout:

send: Connection timed out
Mon Jul  2 01:38:09 EDT 2018

Jon Maxwell (3):
  [PATCH net-next 1/3] tcp: convert icsk_user_timeout from jiffies to msecs
  [PATCH net-next v1 2/3] tcp: convert icsk_user_timeout from jiffies to msecs
  [PATCH net-next v1 3/3] tcp: convert icsk_user_timeout from jiffies to msecs

 net/ipv4/tcp.c       |  4 ++--
 net/ipv4/tcp_timer.c | 51 ++++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 40 insertions(+), 15 deletions(-)

-- 
2.13.6

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

* Re: [PATCH net-next 0/3] Series to improve setsockopt() TCP_USER_TIMEOUT accuracy
  2018-07-17  4:15 [PATCH net-next 0/3] Series to improve setsockopt() TCP_USER_TIMEOUT accuracy Jon Maxwell
@ 2018-07-17  4:56 ` Eric Dumazet
  2018-07-17  5:40   ` Jon Maxwell
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2018-07-17  4:56 UTC (permalink / raw)
  To: Jon Maxwell, davem
  Cc: edumazet, ncardwell, David.Laight, kuznet, yoshfuji, netdev,
	linux-kernel, jmaxwell



On 07/16/2018 09:15 PM, Jon Maxwell wrote:
> This is a patch series based on:
>
> Jon Maxwell (3):
>   [PATCH net-next 1/3] tcp: convert icsk_user_timeout from jiffies to msecs
>   [PATCH net-next v1 2/3] tcp: convert icsk_user_timeout from jiffies to msecs
>   [PATCH net-next v1 3/3] tcp: convert icsk_user_timeout from jiffies to msecs

This would have been nice to use meaningful titles for each patch,
instead of copy/pasting the first one ?

Thanks !

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

* Re: [PATCH net-next 0/3] Series to improve setsockopt() TCP_USER_TIMEOUT accuracy
  2018-07-17  4:56 ` Eric Dumazet
@ 2018-07-17  5:40   ` Jon Maxwell
  0 siblings, 0 replies; 3+ messages in thread
From: Jon Maxwell @ 2018-07-17  5:40 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Jon Maxwell, Miller, David, edumazet, ncardwell, David.Laight,
	kuznet, yoshfuji, netdev, linux-kernel

On Tue, Jul 17, 2018 at 2:56 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
> On 07/16/2018 09:15 PM, Jon Maxwell wrote:
>> This is a patch series based on:
>>
>> Jon Maxwell (3):
>>   [PATCH net-next 1/3] tcp: convert icsk_user_timeout from jiffies to msecs
>>   [PATCH net-next v1 2/3] tcp: convert icsk_user_timeout from jiffies to msecs
>>   [PATCH net-next v1 3/3] tcp: convert icsk_user_timeout from jiffies to msecs
>
> This would have been nice to use meaningful titles for each patch,
> instead of copy/pasting the first one ?
>
> Thanks !
>
>
>

This is my 1st time doing a patch series. Let me rebase and resubmit
each with a more descriptive title.

But 1st do I still need to put the patch number in the series? Even if
the title differs?

1) e.g:

Jon Maxwell (3):
   [PATCH net-next 1/3] tcp: convert icsk_user_timeout from jiffies to msecs
   [PATCH net-next 2/3] tcp: Add tcp_retransmit_stamp() helper
   [PATCH net-next 3/3] tcp: Add  tcp_clamp_rto_to_user_timeout() to
improve accuracy

2) or:

Jon Maxwell (3):
   [PATCH net-next] tcp: convert icsk_user_timeout from jiffies to msecs
   [PATCH net-next] tcp: Add tcp_retransmit_time() helper
   [PATCH net-next] tcp: Add  tcp_clamp_rto_to_user_timeout() to
improve TCP_USER_TIMEOUT accuracy

Which is preferred (1) or (2)? Are the above titles descriptive enough?

Regards

Jon

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

end of thread, other threads:[~2018-07-17  5:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-17  4:15 [PATCH net-next 0/3] Series to improve setsockopt() TCP_USER_TIMEOUT accuracy Jon Maxwell
2018-07-17  4:56 ` Eric Dumazet
2018-07-17  5:40   ` Jon Maxwell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).