This fixes a counter problem on 32bit systems:
When the rx_bytes counter reached 2 GiB, it jumpd to (2^64 Bytes - 2GiB) Bytes.
rtnl_link_stats64 has __u64 type and atomic_long_read returns
atomic_long_t which is signed. Due to the conversation
we get an incorrect value on 32bit systems if the MSB of
the atomic_long_t value is set.
CC: Tom Parkin <redacted>
Fixes: 7b7c0719cd7a ("l2tp: avoid deadlock in l2tp stats update")
Signed-off-by: Dominik Heidler <redacted>
---
net/l2tp/l2tp_eth.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
From: Stephen Hemminger <stephen@networkplumber.org> Date: 2017-06-09 22:16:31
On Fri, 9 Jun 2017 16:29:47 +0200
Dominik Heidler [off-list ref] wrote:
quoted hunk
This fixes a counter problem on 32bit systems:
When the rx_bytes counter reached 2 GiB, it jumpd to (2^64 Bytes - 2GiB) Bytes.
rtnl_link_stats64 has __u64 type and atomic_long_read returns
atomic_long_t which is signed. Due to the conversation
we get an incorrect value on 32bit systems if the MSB of
the atomic_long_t value is set.
CC: Tom Parkin <redacted>
Fixes: 7b7c0719cd7a ("l2tp: avoid deadlock in l2tp stats update")
Signed-off-by: Dominik Heidler <redacted>
---
net/l2tp/l2tp_eth.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
This is not the right way to fix this.
1. shouldn't be using atomic's for network counters, look at other network devices.
2. should be using u64_stats_fetch api to handle 64 bit counters.
From: Eric Dumazet <hidden> Date: 2017-06-10 00:29:16
On Fri, 2017-06-09 at 15:16 -0700, Stephen Hemminger wrote:
On Fri, 9 Jun 2017 16:29:47 +0200
Dominik Heidler [off-list ref] wrote:
quoted
This fixes a counter problem on 32bit systems:
When the rx_bytes counter reached 2 GiB, it jumpd to (2^64 Bytes - 2GiB) Bytes.
rtnl_link_stats64 has __u64 type and atomic_long_read returns
atomic_long_t which is signed. Due to the conversation
we get an incorrect value on 32bit systems if the MSB of
the atomic_long_t value is set.
CC: Tom Parkin <redacted>
Fixes: 7b7c0719cd7a ("l2tp: avoid deadlock in l2tp stats update")
Signed-off-by: Dominik Heidler <redacted>
---
net/l2tp/l2tp_eth.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
This is not the right way to fix this.
1. shouldn't be using atomic's for network counters, look at other network devices.
2. should be using u64_stats_fetch api to handle 64 bit counters.
But they do not want 64bit counters, and not per cpu counters for a
driver handling few packets per second.
Just use native size of "unsigned long".
We use the same atomic_long_t for (struct netdev)->rx_dropped,
tx_dropped & rx_nohandler
So I guess same fix is needed in dev_get_stats()
From: David Miller <davem@davemloft.net> Date: 2017-06-10 20:13:39
From: Eric Dumazet <redacted>
Date: Fri, 09 Jun 2017 17:29:11 -0700
On Fri, 2017-06-09 at 15:16 -0700, Stephen Hemminger wrote:
quoted
On Fri, 9 Jun 2017 16:29:47 +0200
Dominik Heidler [off-list ref] wrote:
quoted
This fixes a counter problem on 32bit systems:
When the rx_bytes counter reached 2 GiB, it jumpd to (2^64 Bytes - 2GiB) Bytes.
rtnl_link_stats64 has __u64 type and atomic_long_read returns
atomic_long_t which is signed. Due to the conversation
we get an incorrect value on 32bit systems if the MSB of
the atomic_long_t value is set.
CC: Tom Parkin <redacted>
Fixes: 7b7c0719cd7a ("l2tp: avoid deadlock in l2tp stats update")
Signed-off-by: Dominik Heidler <redacted>
---
net/l2tp/l2tp_eth.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
This is not the right way to fix this.
1. shouldn't be using atomic's for network counters, look at other network devices.
2. should be using u64_stats_fetch api to handle 64 bit counters.
But they do not want 64bit counters, and not per cpu counters for a
driver handling few packets per second.
Just use native size of "unsigned long".
Ahh yeah, indeed. I've applied this l2tp patch, therefore.
We use the same atomic_long_t for (struct netdev)->rx_dropped,
tx_dropped & rx_nohandler
So I guess same fix is needed in dev_get_stats()
Looks like it, I'll apply a formal submission of this.
From: Eric Dumazet <hidden> Date: 2017-06-27 14:02:23
From: Eric Dumazet <redacted>
Similar to the fix provided by Dominik Heidler in commit
9b3dc0a17d73 ("l2tp: cast l2tp traffic counter to unsigned")
we need to take care of 32bit kernels in dev_get_stats().
When using atomic_long_read(), we add a 'long' to u64 and
might misinterpret high order bit, unless we cast to unsigned.
Fixes: caf586e5f23ce ("net: add a core netdev->rx_dropped counter")
Fixes: 015f0688f57ca ("net: net: add a core netdev->tx_dropped counter")
Fixes: 6e7333d315a76 ("net: add rx_nohandler stat counter")
Signed-off-by: Eric Dumazet <redacted>
Cc: Jarod Wilson <redacted>
---
Note: I will provide similar other fixes in networking tree.
net/core/dev.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
From: David Miller <davem@davemloft.net> Date: 2017-06-27 18:51:32
From: Eric Dumazet <redacted>
Date: Tue, 27 Jun 2017 07:02:20 -0700
From: Eric Dumazet <redacted>
Similar to the fix provided by Dominik Heidler in commit
9b3dc0a17d73 ("l2tp: cast l2tp traffic counter to unsigned")
we need to take care of 32bit kernels in dev_get_stats().
When using atomic_long_read(), we add a 'long' to u64 and
might misinterpret high order bit, unless we cast to unsigned.
Fixes: caf586e5f23ce ("net: add a core netdev->rx_dropped counter")
Fixes: 015f0688f57ca ("net: net: add a core netdev->tx_dropped counter")
Fixes: 6e7333d315a76 ("net: add rx_nohandler stat counter")
Signed-off-by: Eric Dumazet <redacted>
Cc: Jarod Wilson <redacted>