2011-05-18 21:09:31

by David Decotigny

[permalink] [raw]
Subject: [PATCH] forcedeth: Improve stats counters

From: Mandeep Baines <[email protected]>

Rx byte count was off; instead use the hardware's count. Tx packet
count was counting pre-TSO packets; instead count on-the-wire packets.
Report hardware dropped frame count as rx_fifo_errors.

- The count of transmitted packets reported by the forcedeth driver
reports pre-TSO (TCP Segmentation Offload) packet counts and not the
count of the number of packets sent on the wire. This change fixes
the forcedeth driver to report the correct count. Fixed the code by
copying the count stored in the NIC H/W to the value reported by the
driver.

- Count rx_drop_frame errors as rx_fifo_errors:
We see a lot of rx_drop_frame errors if we disable the rx bottom-halves
for too long. Normally, rx_fifo_errors would be counted in this case.
The rx_drop_frame error count is private to forcedeth and is not
reported by ifconfig or sysfs. The rx_fifo_errors count is currently
unused in the forcedeth driver. It is reported by ifconfig as overruns.
This change reports rx_drop_frame errors as rx_fifo_errors.


Signed-off-by: David Decotigny <[email protected]>
---
drivers/net/forcedeth.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index d09e8b0..895471d 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -1684,6 +1684,7 @@ static void nv_get_hw_stats(struct net_device *dev)
np->estats.tx_pause += readl(base + NvRegTxPause);
np->estats.rx_pause += readl(base + NvRegRxPause);
np->estats.rx_drop_frame += readl(base + NvRegRxDropFrame);
+ np->estats.rx_errors_total += np->estats.rx_drop_frame;
}

if (np->driver_data & DEV_HAS_STATISTICS_V3) {
@@ -1708,11 +1709,14 @@ static struct net_device_stats *nv_get_stats(struct net_device *dev)
nv_get_hw_stats(dev);

/* copy to net_device stats */
+ dev->stats.tx_packets = np->estats.tx_packets;
+ dev->stats.rx_bytes = np->estats.rx_bytes;
dev->stats.tx_bytes = np->estats.tx_bytes;
dev->stats.tx_fifo_errors = np->estats.tx_fifo_errors;
dev->stats.tx_carrier_errors = np->estats.tx_carrier_errors;
dev->stats.rx_crc_errors = np->estats.rx_crc_errors;
dev->stats.rx_over_errors = np->estats.rx_over_errors;
+ dev->stats.rx_fifo_errors = np->estats.rx_drop_frame;
dev->stats.rx_errors = np->estats.rx_errors_total;
dev->stats.tx_errors = np->estats.tx_errors_total;
}
--
1.7.3.1


2011-05-18 21:09:41

by David Decotigny

[permalink] [raw]
Subject: [PATCH] forcedeth: new ethtool stat "tx_timeout" to account for tx_timeouts

From: Sameer Nanda <[email protected]>

This change publishes a new ethtool stats: tx_timeout that counts the
number of times the tx_timeout callback was triggered.


Signed-off-by: David Decotigny <[email protected]>
---
drivers/net/forcedeth.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index 895471d..112dc0b 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -632,6 +632,7 @@ static const struct nv_ethtool_str nv_estats_str[] = {
{ "rx_packets" },
{ "rx_errors_total" },
{ "tx_errors_total" },
+ { "tx_timeout" },

/* version 2 stats */
{ "tx_deferral" },
@@ -672,6 +673,7 @@ struct nv_ethtool_stats {
u64 rx_packets;
u64 rx_errors_total;
u64 tx_errors_total;
+ u64 tx_timeout;

/* version 2 stats */
u64 tx_deferral;
@@ -2526,6 +2528,8 @@ static void nv_tx_timeout(struct net_device *dev)

spin_lock_irq(&np->lock);

+ np->estats.tx_timeout++;
+
/* 1) stop tx engine */
nv_stop_tx(dev);

--
1.7.3.1

2011-05-18 21:14:34

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] forcedeth: new ethtool stat "tx_timeout" to account for tx_timeouts


When submitting multiple patches in a patch set, NUMBER THEM.

Otherwise there is no unambiguous way to figure out what order
I should apply these patches.

2011-05-18 21:20:46

by David Decotigny

[permalink] [raw]
Subject: Re: [PATCH] forcedeth: new ethtool stat "tx_timeout" to account for tx_timeouts

Hi David,

They should be independent and applicable in any order, except for the
two 'PATCH x/2' which have to go together.

FYI, here is the order in which I'm applying them:
1/ forcedeth: Improve stats counters
2/ forcedeth: new ethtool stat "tx_timeout" to account for tx_timeouts
3/ [PATCH 1/2] forcedeth: make module parameters readable in /sys/module
4/ [PATCH 2/2] forcedeth: allow to silence tx_timeout debug messages
5/ forcedeth: Acknowledge only interrupts that are being processed
6/ forcedeth: Add messages to indicate using MSI or MSI-X
7/ forcedeth: Fix a race during rmmod of forcedeth

Sorry for that.
Regards,

--
David Decotigny



On Wed, May 18, 2011 at 2:14 PM, David Miller <[email protected]> wrote:
>
> When submitting multiple patches in a patch set, NUMBER THEM.
>
> Otherwise there is no unambiguous way to figure out what order
> I should apply these patches.
>

2011-05-18 21:26:45

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] forcedeth: new ethtool stat "tx_timeout" to account for tx_timeouts

From: David Decotigny <[email protected]>
Date: Wed, 18 May 2011 14:20:21 -0700

> Hi David,
>
> They should be independent and applicable in any order, except for the
> two 'PATCH x/2' which have to go together.

That doesn't matter.

When you are sending a set of patches, always number them.

Please resubmit your changes with my feedback incorporated, I'm
not applying what you sent until everything is fixed up.

Thanks.