diff mbox

[net] tcp: fix RTO calculated from cached RTT

Message ID 1379382260-29303-1-git-send-email-ncardwell@google.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Neal Cardwell Sept. 17, 2013, 1:44 a.m. UTC
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(-)

Comments

Eric Dumazet Sept. 17, 2013, 12:21 p.m. UTC | #1
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 ;)



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yuchung Cheng Sept. 17, 2013, 2:32 p.m. UTC | #2
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
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Sept. 17, 2013, 11:08 p.m. UTC | #3
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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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,