On 04.05.2017 21:27, David Arcari wrote:
On 05/04/2017 01:09 PM, Pavel Belous wrote:
On 04.05.2017 19:51, David Miller wrote:
From: Lino Sanfilippo <linosanfili...@gmx.de>
Date: Thu, 4 May 2017 18:48:12 +0200
Hi Pavel,
On 04.05.2017 18:33, Pavel Belous wrote:
From: Pavel Belous <pavel.bel...@aquantia.com>
This patch fixes the crash that happens when driver tries to collect statistics
from already released "aq_vec" object.
Fixes: 97bde5c4f909 ("net: ethernet: aquantia: Support for NIC-specific code")
Signed-off-by: Pavel Belous <pavel.bel...@aquantia.com>
---
drivers/net/ethernet/aquantia/atlantic/aq_nic.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index cdb0299..3a32573 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -755,7 +755,7 @@ void aq_nic_get_stats(struct aq_nic_s *self, u64 *data)
count = 0U;
for (i = 0U, aq_vec = self->aq_vec[0];
- self->aq_vecs > i; ++i, aq_vec = self->aq_vec[i]) {
+ aq_vec && self->aq_vecs > i; ++i, aq_vec = self->aq_vec[i]) {
data += count;
aq_vec_get_sw_stats(aq_vec, data, &count);
}
@@ -961,6 +961,7 @@ void aq_nic_free_hot_resources(struct aq_nic_s *self)
for (i = AQ_DIMOF(self->aq_vec); i--;) {
if (self->aq_vec[i])
aq_vec_free(self->aq_vec[i]);
+ self->aq_vec[i] = NULL;
}
err_exit:;
if the driver does not support statistics when the interface is down, would
not it be clearer
to check if netif_running() in get_stats() instead?
Yes, much cleaner.
Much better would be to have a cached software copy so that statistics
can be reported regardless of whether the device is down or not.
Thank you.
I will think about how to do it better.
It appears that the adapter is still reporting the cumulative hardware stats
even while its down. The user is just losing the per queue stats.
Although the loss of the per queue stats is not ideal, this patch still fixes a
crash.
It might be worthwhile to refactor this patch as a short term solution and then
subsequently produce a version that contains cached statistics. Assuming that
is amenable to everyone of course.
-DA
Yes, even adapter is in the down state user can still see statistics
from the HW.
For example (adapter is down):
$ ethtool -S enp2s0
NIC statistics:
InPackets: 3237727
InUCast: 3237214
InMCast: 391
InBCast: 122
InErrors: 0
OutPackets: 14157898
OutUCast: 14157089
OutMCast: 304
OutBCast: 505
InUCastOctects: 226714406
OutUCastOctects: 10463156
InMCastOctects: 58046
OutMCastOctects: 44817
InBCastOctects: 12857
OutBCastOctects: 41626
InOctects: 226785309
OutOctects: 10549599
InPacketsDma: 0
OutPacketsDma: 16
InOctetsDma: 0
OutOctetsDma: 2396
InDroppedDma: 0
Queue[0] InPackets: 0
Queue[0] OutPackets: 0
Queue[0] InJumboPackets: 0
Queue[0] InLroPackets: 0
Queue[0] InErrors: 0
Queue[1] InPackets: 0
Queue[1] OutPackets: 0
Queue[1] InJumboPackets: 0
Queue[1] InLroPackets: 0
Queue[1] InErrors: 0
Queue[2] InPackets: 0
Queue[2] OutPackets: 0
Queue[2] InJumboPackets: 0
Queue[2] InLroPackets: 0
Queue[2] InErrors: 0
Queue[3] InPackets: 0
Queue[3] OutPackets: 0
Queue[3] InJumboPackets: 0
Queue[3] InLroPackets: 0
Queue[3] InErrors: 0
Lino, David what do you think?
If you agree I can re-submit the patch (with fixed braces).
Regards,
Pavel
