* [PATCH] tcp_nv: fix division by zero in tcpnv_acked()
@ 2017-11-01 13:32 Konstantin Khlebnikov
2017-11-01 13:47 ` Lawrence Brakmo
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Konstantin Khlebnikov @ 2017-11-01 13:32 UTC (permalink / raw)
To: netdev, David S. Miller
Cc: Lawrence Brakmo, Alexey Kuznetsov, Hideaki YOSHIFUJI,
linux-kernel
Average RTT could become zero. This happened in real life at least twice.
This patch treats zero as 1us.
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
net/ipv4/tcp_nv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_nv.c b/net/ipv4/tcp_nv.c
index 1ff73982e28c..125fc1450b01 100644
--- a/net/ipv4/tcp_nv.c
+++ b/net/ipv4/tcp_nv.c
@@ -252,7 +252,7 @@ static void tcpnv_acked(struct sock *sk, const struct ack_sample *sample)
/* rate in 100's bits per second */
rate64 = ((u64)sample->in_flight) * 8000000;
- rate = (u32)div64_u64(rate64, (u64)(avg_rtt * 100));
+ rate = (u32)div64_u64(rate64, (u64)(avg_rtt ?: 1) * 100);
/* Remember the maximum rate seen during this RTT
* Note: It may be more than one RTT. This function should be
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] tcp_nv: fix division by zero in tcpnv_acked()
2017-11-01 13:32 [PATCH] tcp_nv: fix division by zero in tcpnv_acked() Konstantin Khlebnikov
@ 2017-11-01 13:47 ` Lawrence Brakmo
2017-11-01 14:13 ` Stephen Hemminger
2017-11-01 14:17 ` Eric Dumazet
2017-11-02 7:17 ` David Miller
2 siblings, 1 reply; 5+ messages in thread
From: Lawrence Brakmo @ 2017-11-01 13:47 UTC (permalink / raw)
To: Konstantin Khlebnikov, netdev@vger.kernel.org, David S. Miller
Cc: Alexey Kuznetsov, Hideaki YOSHIFUJI, linux-kernel@vger.kernel.org
Thank you for finding and fixing this.
On 11/1/17, 6:32 AM, "Konstantin Khlebnikov" <khlebnikov@yandex-team.ru> wrote:
Average RTT could become zero. This happened in real life at least twice.
This patch treats zero as 1us.
Signed-off-by: Konstantin Khlebnikov khlebnikov@yandex-team.ru
Acked-by: Lawrence Brakmo <Brakmo@fb.com>
---
net/ipv4/tcp_nv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_nv.c b/net/ipv4/tcp_nv.c
index 1ff73982e28c..125fc1450b01 100644
--- a/net/ipv4/tcp_nv.c
+++ b/net/ipv4/tcp_nv.c
@@ -252,7 +252,7 @@ static void tcpnv_acked(struct sock *sk, const struct ack_sample *sample)
/* rate in 100's bits per second */
rate64 = ((u64)sample->in_flight) * 8000000;
- rate = (u32)div64_u64(rate64, (u64)(avg_rtt * 100));
+ rate = (u32)div64_u64(rate64, (u64)(avg_rtt ?: 1) * 100);
/* Remember the maximum rate seen during this RTT
* Note: It may be more than one RTT. This function should be
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] tcp_nv: fix division by zero in tcpnv_acked()
2017-11-01 13:47 ` Lawrence Brakmo
@ 2017-11-01 14:13 ` Stephen Hemminger
0 siblings, 0 replies; 5+ messages in thread
From: Stephen Hemminger @ 2017-11-01 14:13 UTC (permalink / raw)
To: Lawrence Brakmo
Cc: Konstantin Khlebnikov, netdev@vger.kernel.org, David S. Miller,
Alexey Kuznetsov, Hideaki YOSHIFUJI, linux-kernel@vger.kernel.org
On Wed, 1 Nov 2017 13:47:17 +0000
Lawrence Brakmo <brakmo@fb.com> wrote:
> Thank you for finding and fixing this.
>
> On 11/1/17, 6:32 AM, "Konstantin Khlebnikov" <khlebnikov@yandex-team.ru> wrote:
>
> Average RTT could become zero. This happened in real life at least twice.
> This patch treats zero as 1us.
>
> Signed-off-by: Konstantin Khlebnikov khlebnikov@yandex-team.ru
> Acked-by: Lawrence Brakmo <Brakmo@fb.com>
> ---
> net/ipv4/tcp_nv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_nv.c b/net/ipv4/tcp_nv.c
> index 1ff73982e28c..125fc1450b01 100644
> --- a/net/ipv4/tcp_nv.c
> +++ b/net/ipv4/tcp_nv.c
> @@ -252,7 +252,7 @@ static void tcpnv_acked(struct sock *sk, const struct ack_sample *sample)
>
> /* rate in 100's bits per second */
> rate64 = ((u64)sample->in_flight) * 8000000;
> - rate = (u32)div64_u64(rate64, (u64)(avg_rtt * 100));
> + rate = (u32)div64_u64(rate64, (u64)(avg_rtt ?: 1) * 100);
Why is this code using expensive 64 bit by 64 bit divide when avg_rtt should never be bigger
than 32 bits?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tcp_nv: fix division by zero in tcpnv_acked()
2017-11-01 13:32 [PATCH] tcp_nv: fix division by zero in tcpnv_acked() Konstantin Khlebnikov
2017-11-01 13:47 ` Lawrence Brakmo
@ 2017-11-01 14:17 ` Eric Dumazet
2017-11-02 7:17 ` David Miller
2 siblings, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2017-11-01 14:17 UTC (permalink / raw)
To: Konstantin Khlebnikov
Cc: netdev, David S. Miller, Lawrence Brakmo, Alexey Kuznetsov,
Hideaki YOSHIFUJI, linux-kernel
On Wed, 2017-11-01 at 16:32 +0300, Konstantin Khlebnikov wrote:
> Average RTT could become zero. This happened in real life at least twice.
> This patch treats zero as 1us.
>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> ---
> net/ipv4/tcp_nv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_nv.c b/net/ipv4/tcp_nv.c
> index 1ff73982e28c..125fc1450b01 100644
> --- a/net/ipv4/tcp_nv.c
> +++ b/net/ipv4/tcp_nv.c
> @@ -252,7 +252,7 @@ static void tcpnv_acked(struct sock *sk, const struct ack_sample *sample)
>
> /* rate in 100's bits per second */
> rate64 = ((u64)sample->in_flight) * 8000000;
> - rate = (u32)div64_u64(rate64, (u64)(avg_rtt * 100));
> + rate = (u32)div64_u64(rate64, (u64)(avg_rtt ?: 1) * 100);
>
> /* Remember the maximum rate seen during this RTT
> * Note: It may be more than one RTT. This function should be
>
BTW, this div64_u64() could be replaced by a u32 divide (aka do_div()),
slightly less expensive.
rate64 = ((u64)sample->in_flight) * 80000;
do_div(rate64, avg_rtt ?: 1);
rate = (u32)rate64;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tcp_nv: fix division by zero in tcpnv_acked()
2017-11-01 13:32 [PATCH] tcp_nv: fix division by zero in tcpnv_acked() Konstantin Khlebnikov
2017-11-01 13:47 ` Lawrence Brakmo
2017-11-01 14:17 ` Eric Dumazet
@ 2017-11-02 7:17 ` David Miller
2 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2017-11-02 7:17 UTC (permalink / raw)
To: khlebnikov; +Cc: netdev, brakmo, kuznet, yoshfuji, linux-kernel
From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Date: Wed, 01 Nov 2017 16:32:15 +0300
> Average RTT could become zero. This happened in real life at least twice.
> This patch treats zero as 1us.
>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Applied and queued up for -stable, thank you.
Indeed, as others have mentioned, a cheaper divide should be used
but that is a separate issue from this fix and therefore should be
done in a separate patch.
Thanks again.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-11-02 7:17 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-01 13:32 [PATCH] tcp_nv: fix division by zero in tcpnv_acked() Konstantin Khlebnikov
2017-11-01 13:47 ` Lawrence Brakmo
2017-11-01 14:13 ` Stephen Hemminger
2017-11-01 14:17 ` Eric Dumazet
2017-11-02 7:17 ` David Miller
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).