* [PATCH net] tcp: fix RTO calculated from cached RTT
@ 2013-09-17 1:44 Neal Cardwell
2013-09-17 12:21 ` Eric Dumazet
2013-09-17 14:32 ` Yuchung Cheng
0 siblings, 2 replies; 4+ messages in thread
From: Neal Cardwell @ 2013-09-17 1:44 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Neal Cardwell, Eric Dumazet, Yuchung Cheng
Commit 1b7fdd2ab5852 ("tcp: do not use cached RTT for RTT estimation")
did not correctly account for the fact that crtt is the RTT shifted
left 3 bits. Fix the calculation to consistently reflect this fact.
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
---
net/ipv4/tcp_metrics.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c
index 4a22f3e..52f3c6b 100644
--- a/net/ipv4/tcp_metrics.c
+++ b/net/ipv4/tcp_metrics.c
@@ -502,7 +502,9 @@ reset:
* ACKs, wait for troubles.
*/
if (crtt > tp->srtt) {
- inet_csk(sk)->icsk_rto = crtt + max(crtt >> 2, tcp_rto_min(sk));
+ /* Set RTO like tcp_rtt_estimator(), but from cached RTT. */
+ crtt >>= 3;
+ inet_csk(sk)->icsk_rto = crtt + max(2 * crtt, tcp_rto_min(sk));
} else if (tp->srtt == 0) {
/* RFC6298: 5.7 We've failed to get a valid RTT sample from
* 3WHS. This is most likely due to retransmission,
--
1.8.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH net] tcp: fix RTO calculated from cached RTT
2013-09-17 1:44 [PATCH net] tcp: fix RTO calculated from cached RTT Neal Cardwell
@ 2013-09-17 12:21 ` Eric Dumazet
2013-09-17 14:32 ` Yuchung Cheng
1 sibling, 0 replies; 4+ messages in thread
From: Eric Dumazet @ 2013-09-17 12:21 UTC (permalink / raw)
To: Neal Cardwell; +Cc: David Miller, netdev, Eric Dumazet, Yuchung Cheng
On Mon, 2013-09-16 at 21:44 -0400, Neal Cardwell wrote:
> Commit 1b7fdd2ab5852 ("tcp: do not use cached RTT for RTT estimation")
> did not correctly account for the fact that crtt is the RTT shifted
> left 3 bits. Fix the calculation to consistently reflect this fact.
>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
> ---
> net/ipv4/tcp_metrics.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Acked-by: Eric Dumazet <edumazet@google.com>
Instead of rto = 3*crtt, we had rto = 10*crtt, which was indeed
suboptimal ;)
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net] tcp: fix RTO calculated from cached RTT
2013-09-17 1:44 [PATCH net] tcp: fix RTO calculated from cached RTT Neal Cardwell
2013-09-17 12:21 ` Eric Dumazet
@ 2013-09-17 14:32 ` Yuchung Cheng
2013-09-17 23:08 ` David Miller
1 sibling, 1 reply; 4+ messages in thread
From: Yuchung Cheng @ 2013-09-17 14:32 UTC (permalink / raw)
To: Neal Cardwell; +Cc: David Miller, netdev, Eric Dumazet
On Mon, Sep 16, 2013 at 6:44 PM, Neal Cardwell <ncardwell@google.com> wrote:
> Commit 1b7fdd2ab5852 ("tcp: do not use cached RTT for RTT estimation")
> did not correctly account for the fact that crtt is the RTT shifted
> left 3 bits. Fix the calculation to consistently reflect this fact.
>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
> ---
> net/ipv4/tcp_metrics.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Acked-By: Yuchung Cheng <ycheng@google.com>
Thanks for discovering and fixing it!
>
> diff --git a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c
> index 4a22f3e..52f3c6b 100644
> --- a/net/ipv4/tcp_metrics.c
> +++ b/net/ipv4/tcp_metrics.c
> @@ -502,7 +502,9 @@ reset:
> * ACKs, wait for troubles.
> */
> if (crtt > tp->srtt) {
> - inet_csk(sk)->icsk_rto = crtt + max(crtt >> 2, tcp_rto_min(sk));
> + /* Set RTO like tcp_rtt_estimator(), but from cached RTT. */
> + crtt >>= 3;
> + inet_csk(sk)->icsk_rto = crtt + max(2 * crtt, tcp_rto_min(sk));
> } else if (tp->srtt == 0) {
> /* RFC6298: 5.7 We've failed to get a valid RTT sample from
> * 3WHS. This is most likely due to retransmission,
> --
> 1.8.4
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net] tcp: fix RTO calculated from cached RTT
2013-09-17 14:32 ` Yuchung Cheng
@ 2013-09-17 23:08 ` David Miller
0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2013-09-17 23:08 UTC (permalink / raw)
To: ycheng; +Cc: ncardwell, netdev, edumazet
From: Yuchung Cheng <ycheng@google.com>
Date: Tue, 17 Sep 2013 07:32:38 -0700
> On Mon, Sep 16, 2013 at 6:44 PM, Neal Cardwell <ncardwell@google.com> wrote:
>> Commit 1b7fdd2ab5852 ("tcp: do not use cached RTT for RTT estimation")
>> did not correctly account for the fact that crtt is the RTT shifted
>> left 3 bits. Fix the calculation to consistently reflect this fact.
>>
>> Signed-off-by: Neal Cardwell <ncardwell@google.com>
>> Cc: Eric Dumazet <edumazet@google.com>
>> Cc: Yuchung Cheng <ycheng@google.com>
>> ---
>> net/ipv4/tcp_metrics.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
> Acked-By: Yuchung Cheng <ycheng@google.com>
>
> Thanks for discovering and fixing it!
Applied, thanks everyone.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-09-17 23:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-17 1:44 [PATCH net] tcp: fix RTO calculated from cached RTT Neal Cardwell
2013-09-17 12:21 ` Eric Dumazet
2013-09-17 14:32 ` Yuchung Cheng
2013-09-17 23:08 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox