[PATCH] forcedeth: Improve stats counters

[PATCH] forcedeth: Improve stats counters

[David Decotigny]

From: Mandeep Baines <msb@google.com>

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 <decot@google.com>
---
 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

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

[David Decotigny]

From: Sameer Nanda <snanda@google.com>

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 <decot@google.com>
---
 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

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

[David Miller]

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.

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

[David Decotigny]

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 <davem@davemloft.net> 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.
>

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

[David Miller]

From: David Decotigny <decot@google.com>
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.