[PATCH net v3 0/9] forcedeth: minor fixes for stats, rmmod, sparse

[PATCH net v3 0/9] forcedeth: minor fixes for stats, rmmod, sparse

[David Decotigny]

Changes since v2:
 - removed "Fix a race during rmmod of forcedeth" from the series
   (will look at it separately with original author)
 - added "remove unneeded stats updates" and "64-bit stats"
 - reordered patches

Changes since v1:
 - rebased on top of netdev tip
 - do not repeat name of device in netdev_dbg
 - do not completely mute TX timeout messages when debug_tx_timeout is
   not set
 - make debug_tx_timeout writable in /sys/module
 Note: I am re-submitting "expose module parameters in /sys/module" as
       it can be useful in production and I was assured it doesn't add
       much memory overhead by the sysfs maintainers.

Tested:
  16-way x86_64 SMP, dual forcedeth ->
  RX bytes:7244556582 (7.2 GB)  TX bytes:181904254 (181.9 MB)


############################################
# Patch Set Summary:

David Decotigny (4):
  forcedeth: expose module parameters in /sys/module
  forcedeth: remove unneeded stats updates
  forcedeth: 64-bit stats
  forcedeth: fix a few sparse warnings (variable shadowing)

Mandeep Baines (1):
  forcedeth: Improve stats counters

Mike Ditto (2):
  forcedeth: Add messages to indicate using MSI or MSI-X
  forcedeth: Acknowledge only interrupts that are being processed

Sameer Nanda (2):
  forcedeth: allow to silence tx_timeout debug messages
  forcedeth: new ethtool stat "tx_timeout" to account for tx_timeouts

 drivers/net/ethernet/nvidia/forcedeth.c |  269 ++++++++++++++++---------------
 1 files changed, 142 insertions(+), 127 deletions(-)

-- 
1.7.3.1

[PATCH net v3 1/9] forcedeth: Add messages to indicate using MSI or MSI-X

[David Decotigny]

From: Mike Ditto <mditto@google.com>

This adds a few debug messages to indicate whether PCIe interrupts are
signaled with MSI or MSI-X.



Signed-off-by: David Decotigny <david.decotigny@google.com>
---
 drivers/net/ethernet/nvidia/forcedeth.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index 1e37eb9..2f884fc 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -3737,6 +3737,7 @@ static int nv_request_irq(struct net_device *dev, int intr_test)
 				writel(0, base + NvRegMSIXMap0);
 				writel(0, base + NvRegMSIXMap1);
 			}
+			netdev_info(dev, "MSI-X enabled\n");
 		}
 	}
 	if (ret != 0 && np->msi_flags & NV_MSI_CAPABLE) {
@@ -3758,6 +3759,7 @@ static int nv_request_irq(struct net_device *dev, int intr_test)
 			writel(0, base + NvRegMSIMap1);
 			/* enable msi vector 0 */
 			writel(NVREG_MSI_VECTOR_0_ENABLED, base + NvRegMSIIrqMask);
+			netdev_info(dev, "MSI enabled\n");
 		}
 	}
 	if (ret != 0) {
-- 
1.7.3.1

[PATCH net v3 2/9] forcedeth: Acknowledge only interrupts that are being processed

[David Decotigny]

From: Mike Ditto <mditto@google.com>

This is to avoid a race, accidentally acknowledging an interrupt that
we didn't notice and won't immediately process.  This is based solely
on code inspection; it is not known if there was an actual bug here.



Signed-off-by: David Decotigny <david.decotigny@google.com>
---
 drivers/net/ethernet/nvidia/forcedeth.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index 2f884fc..9aefee1 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -3398,7 +3398,8 @@ static irqreturn_t nv_nic_irq_tx(int foo, void *data)
 
 	for (i = 0;; i++) {
 		events = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQ_TX_ALL;
-		writel(NVREG_IRQ_TX_ALL, base + NvRegMSIXIrqStatus);
+		writel(events, base + NvRegMSIXIrqStatus);
+		netdev_dbg(dev, "tx irq events: %08x\n", events);
 		if (!(events & np->irqmask))
 			break;
 
@@ -3509,7 +3510,8 @@ static irqreturn_t nv_nic_irq_rx(int foo, void *data)
 
 	for (i = 0;; i++) {
 		events = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQ_RX_ALL;
-		writel(NVREG_IRQ_RX_ALL, base + NvRegMSIXIrqStatus);
+		writel(events, base + NvRegMSIXIrqStatus);
+		netdev_dbg(dev, "rx irq events: %08x\n", events);
 		if (!(events & np->irqmask))
 			break;
 
@@ -3553,7 +3555,8 @@ static irqreturn_t nv_nic_irq_other(int foo, void *data)
 
 	for (i = 0;; i++) {
 		events = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQ_OTHER;
-		writel(NVREG_IRQ_OTHER, base + NvRegMSIXIrqStatus);
+		writel(events, base + NvRegMSIXIrqStatus);
+		netdev_dbg(dev, "irq events: %08x\n", events);
 		if (!(events & np->irqmask))
 			break;
 
@@ -3617,10 +3620,10 @@ static irqreturn_t nv_nic_irq_test(int foo, void *data)
 
 	if (!(np->msi_flags & NV_MSI_X_ENABLED)) {
 		events = readl(base + NvRegIrqStatus) & NVREG_IRQSTAT_MASK;
-		writel(NVREG_IRQ_TIMER, base + NvRegIrqStatus);
+		writel(events & NVREG_IRQ_TIMER, base + NvRegIrqStatus);
 	} else {
 		events = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQSTAT_MASK;
-		writel(NVREG_IRQ_TIMER, base + NvRegMSIXIrqStatus);
+		writel(events & NVREG_IRQ_TIMER, base + NvRegMSIXIrqStatus);
 	}
 	pci_push(base);
 	if (!(events & NVREG_IRQ_TIMER))
-- 
1.7.3.1

[PATCH net v3 3/9] forcedeth: allow to silence tx_timeout debug messages

[David Decotigny]

From: Sameer Nanda <snanda@google.com>

This adds a new module parameter "debug_tx_timeout" to silence most
debug messages in case of TX timeout. These messages don't provide a
signal/noise ratio high enough for production systems and, with ~30kB
logged each time, they tend to add to a cascade effect if the system
is already under stress (memory pressure, disk, etc.).

By default, the parameter is clear, meaning the debug messages are not
displayed.



Signed-off-by: David Decotigny <david.decotigny@google.com>
---
 drivers/net/ethernet/nvidia/forcedeth.c |   98 ++++++++++++++++++-------------
 1 files changed, 57 insertions(+), 41 deletions(-)

diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index 9aefee1..3f66b74 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -892,6 +892,11 @@ enum {
 static int dma_64bit = NV_DMA_64BIT_ENABLED;
 
 /*
+ * Debug output control for tx_timeout
+ */
+static bool debug_tx_timeout = false;
+
+/*
  * Crossover Detection
  * Realtek 8201 phy + some OEM boards do not work properly.
  */
@@ -2475,56 +2480,64 @@ static void nv_tx_timeout(struct net_device *dev)
 	u32 status;
 	union ring_type put_tx;
 	int saved_tx_limit;
-	int i;
 
 	if (np->msi_flags & NV_MSI_X_ENABLED)
 		status = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQSTAT_MASK;
 	else
 		status = readl(base + NvRegIrqStatus) & NVREG_IRQSTAT_MASK;
 
-	netdev_info(dev, "Got tx_timeout. irq: %08x\n", status);
+	netdev_warn(dev, "Got tx_timeout. irq status: %08x\n", status);
 
-	netdev_info(dev, "Ring at %lx\n", (unsigned long)np->ring_addr);
-	netdev_info(dev, "Dumping tx registers\n");
-	for (i = 0; i <= np->register_size; i += 32) {
-		netdev_info(dev,
-			    "%3x: %08x %08x %08x %08x %08x %08x %08x %08x\n",
-			    i,
-			    readl(base + i + 0), readl(base + i + 4),
-			    readl(base + i + 8), readl(base + i + 12),
-			    readl(base + i + 16), readl(base + i + 20),
-			    readl(base + i + 24), readl(base + i + 28));
-	}
-	netdev_info(dev, "Dumping tx ring\n");
-	for (i = 0; i < np->tx_ring_size; i += 4) {
-		if (!nv_optimized(np)) {
-			netdev_info(dev,
-				    "%03x: %08x %08x // %08x %08x // %08x %08x // %08x %08x\n",
-				    i,
-				    le32_to_cpu(np->tx_ring.orig[i].buf),
-				    le32_to_cpu(np->tx_ring.orig[i].flaglen),
-				    le32_to_cpu(np->tx_ring.orig[i+1].buf),
-				    le32_to_cpu(np->tx_ring.orig[i+1].flaglen),
-				    le32_to_cpu(np->tx_ring.orig[i+2].buf),
-				    le32_to_cpu(np->tx_ring.orig[i+2].flaglen),
-				    le32_to_cpu(np->tx_ring.orig[i+3].buf),
-				    le32_to_cpu(np->tx_ring.orig[i+3].flaglen));
-		} else {
+	if (unlikely(debug_tx_timeout)) {
+		int i;
+
+		netdev_info(dev, "Ring at %lx\n", (unsigned long)np->ring_addr);
+		netdev_info(dev, "Dumping tx registers\n");
+		for (i = 0; i <= np->register_size; i += 32) {
 			netdev_info(dev,
-				    "%03x: %08x %08x %08x // %08x %08x %08x // %08x %08x %08x // %08x %08x %08x\n",
+				    "%3x: %08x %08x %08x %08x "
+				    "%08x %08x %08x %08x\n",
 				    i,
-				    le32_to_cpu(np->tx_ring.ex[i].bufhigh),
-				    le32_to_cpu(np->tx_ring.ex[i].buflow),
-				    le32_to_cpu(np->tx_ring.ex[i].flaglen),
-				    le32_to_cpu(np->tx_ring.ex[i+1].bufhigh),
-				    le32_to_cpu(np->tx_ring.ex[i+1].buflow),
-				    le32_to_cpu(np->tx_ring.ex[i+1].flaglen),
-				    le32_to_cpu(np->tx_ring.ex[i+2].bufhigh),
-				    le32_to_cpu(np->tx_ring.ex[i+2].buflow),
-				    le32_to_cpu(np->tx_ring.ex[i+2].flaglen),
-				    le32_to_cpu(np->tx_ring.ex[i+3].bufhigh),
-				    le32_to_cpu(np->tx_ring.ex[i+3].buflow),
-				    le32_to_cpu(np->tx_ring.ex[i+3].flaglen));
+				    readl(base + i + 0), readl(base + i + 4),
+				    readl(base + i + 8), readl(base + i + 12),
+				    readl(base + i + 16), readl(base + i + 20),
+				    readl(base + i + 24), readl(base + i + 28));
+		}
+		netdev_info(dev, "Dumping tx ring\n");
+		for (i = 0; i < np->tx_ring_size; i += 4) {
+			if (!nv_optimized(np)) {
+				netdev_info(dev,
+					    "%03x: %08x %08x // %08x %08x "
+					    "// %08x %08x // %08x %08x\n",
+					    i,
+					    le32_to_cpu(np->tx_ring.orig[i].buf),
+					    le32_to_cpu(np->tx_ring.orig[i].flaglen),
+					    le32_to_cpu(np->tx_ring.orig[i+1].buf),
+					    le32_to_cpu(np->tx_ring.orig[i+1].flaglen),
+					    le32_to_cpu(np->tx_ring.orig[i+2].buf),
+					    le32_to_cpu(np->tx_ring.orig[i+2].flaglen),
+					    le32_to_cpu(np->tx_ring.orig[i+3].buf),
+					    le32_to_cpu(np->tx_ring.orig[i+3].flaglen));
+			} else {
+				netdev_info(dev,
+					    "%03x: %08x %08x %08x "
+					    "// %08x %08x %08x "
+					    "// %08x %08x %08x "
+					    "// %08x %08x %08x\n",
+					    i,
+					    le32_to_cpu(np->tx_ring.ex[i].bufhigh),
+					    le32_to_cpu(np->tx_ring.ex[i].buflow),
+					    le32_to_cpu(np->tx_ring.ex[i].flaglen),
+					    le32_to_cpu(np->tx_ring.ex[i+1].bufhigh),
+					    le32_to_cpu(np->tx_ring.ex[i+1].buflow),
+					    le32_to_cpu(np->tx_ring.ex[i+1].flaglen),
+					    le32_to_cpu(np->tx_ring.ex[i+2].bufhigh),
+					    le32_to_cpu(np->tx_ring.ex[i+2].buflow),
+					    le32_to_cpu(np->tx_ring.ex[i+2].flaglen),
+					    le32_to_cpu(np->tx_ring.ex[i+3].bufhigh),
+					    le32_to_cpu(np->tx_ring.ex[i+3].buflow),
+					    le32_to_cpu(np->tx_ring.ex[i+3].flaglen));
+			}
 		}
 	}
 
@@ -6013,6 +6026,9 @@ module_param(phy_cross, int, 0);
 MODULE_PARM_DESC(phy_cross, "Phy crossover detection for Realtek 8201 phy is enabled by setting to 1 and disabled by setting to 0.");
 module_param(phy_power_down, int, 0);
 MODULE_PARM_DESC(phy_power_down, "Power down phy and disable link when interface is down (1), or leave phy powered up (0).");
+module_param(debug_tx_timeout, bool, 0);
+MODULE_PARM_DESC(debug_tx_timeout,
+		 "Dump tx related registers and ring when tx_timeout happens");
 
 MODULE_AUTHOR("Manfred Spraul <manfred@colorfullife.com>");
 MODULE_DESCRIPTION("Reverse Engineered nForce ethernet driver");
-- 
1.7.3.1

[PATCH net v3 4/9] forcedeth: expose module parameters in /sys/module

[David Decotigny]

In particular, debug_tx_timeout can be updated at runtime.



Signed-off-by: David Decotigny <david.decotigny@google.com>
---
 drivers/net/ethernet/nvidia/forcedeth.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index 3f66b74..295652d 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -6010,23 +6010,23 @@ static void __exit exit_nic(void)
 	pci_unregister_driver(&driver);
 }
 
-module_param(max_interrupt_work, int, 0);
+module_param(max_interrupt_work, int, S_IRUGO);
 MODULE_PARM_DESC(max_interrupt_work, "forcedeth maximum events handled per interrupt");
-module_param(optimization_mode, int, 0);
+module_param(optimization_mode, int, S_IRUGO);
 MODULE_PARM_DESC(optimization_mode, "In throughput mode (0), every tx & rx packet will generate an interrupt. In CPU mode (1), interrupts are controlled by a timer. In dynamic mode (2), the mode toggles between throughput and CPU mode based on network load.");
-module_param(poll_interval, int, 0);
+module_param(poll_interval, int, S_IRUGO);
 MODULE_PARM_DESC(poll_interval, "Interval determines how frequent timer interrupt is generated by [(time_in_micro_secs * 100) / (2^10)]. Min is 0 and Max is 65535.");
-module_param(msi, int, 0);
+module_param(msi, int, S_IRUGO);
 MODULE_PARM_DESC(msi, "MSI interrupts are enabled by setting to 1 and disabled by setting to 0.");
-module_param(msix, int, 0);
+module_param(msix, int, S_IRUGO);
 MODULE_PARM_DESC(msix, "MSIX interrupts are enabled by setting to 1 and disabled by setting to 0.");
-module_param(dma_64bit, int, 0);
+module_param(dma_64bit, int, S_IRUGO);
 MODULE_PARM_DESC(dma_64bit, "High DMA is enabled by setting to 1 and disabled by setting to 0.");
-module_param(phy_cross, int, 0);
+module_param(phy_cross, int, S_IRUGO);
 MODULE_PARM_DESC(phy_cross, "Phy crossover detection for Realtek 8201 phy is enabled by setting to 1 and disabled by setting to 0.");
-module_param(phy_power_down, int, 0);
+module_param(phy_power_down, int, S_IRUGO);
 MODULE_PARM_DESC(phy_power_down, "Power down phy and disable link when interface is down (1), or leave phy powered up (0).");
-module_param(debug_tx_timeout, bool, 0);
+module_param(debug_tx_timeout, bool, S_IRUGO|S_IWUSR);
 MODULE_PARM_DESC(debug_tx_timeout,
 		 "Dump tx related registers and ring when tx_timeout happens");
 
-- 
1.7.3.1

[PATCH net v3 5/9] forcedeth: remove unneeded stats updates

[David Decotigny]

Function ndo_get_stats() updates most of the stats from hardware
registers, making the manual updates un-needed. This change removes
these manual updates. Main exception is rx_missed_errors which needs
manual update. Another exception is rx_packets, still updated manually
in this commit to make sure this patch doesn't change behavior of
driver (this is addressed by a later commit in this series).



Signed-off-by: David Decotigny <david.decotigny@google.com>
---
 drivers/net/ethernet/nvidia/forcedeth.c |   35 +------------------------------
 1 files changed, 1 insertions(+), 34 deletions(-)

diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index 295652d..38d8391 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -2379,16 +2379,8 @@ static int nv_tx_done(struct net_device *dev, int limit)
 		if (np->desc_ver == DESC_VER_1) {
 			if (flags & NV_TX_LASTPACKET) {
 				if (flags & NV_TX_ERROR) {
-					if (flags & NV_TX_UNDERFLOW)
-						dev->stats.tx_fifo_errors++;
-					if (flags & NV_TX_CARRIERLOST)
-						dev->stats.tx_carrier_errors++;
 					if ((flags & NV_TX_RETRYERROR) && !(flags & NV_TX_RETRYCOUNT_MASK))
 						nv_legacybackoff_reseed(dev);
-					dev->stats.tx_errors++;
-				} else {
-					dev->stats.tx_packets++;
-					dev->stats.tx_bytes += np->get_tx_ctx->skb->len;
 				}
 				dev_kfree_skb_any(np->get_tx_ctx->skb);
 				np->get_tx_ctx->skb = NULL;
@@ -2397,16 +2389,8 @@ static int nv_tx_done(struct net_device *dev, int limit)
 		} else {
 			if (flags & NV_TX2_LASTPACKET) {
 				if (flags & NV_TX2_ERROR) {
-					if (flags & NV_TX2_UNDERFLOW)
-						dev->stats.tx_fifo_errors++;
-					if (flags & NV_TX2_CARRIERLOST)
-						dev->stats.tx_carrier_errors++;
 					if ((flags & NV_TX2_RETRYERROR) && !(flags & NV_TX2_RETRYCOUNT_MASK))
 						nv_legacybackoff_reseed(dev);
-					dev->stats.tx_errors++;
-				} else {
-					dev->stats.tx_packets++;
-					dev->stats.tx_bytes += np->get_tx_ctx->skb->len;
 				}
 				dev_kfree_skb_any(np->get_tx_ctx->skb);
 				np->get_tx_ctx->skb = NULL;
@@ -2439,9 +2423,7 @@ static int nv_tx_done_optimized(struct net_device *dev, int limit)
 		nv_unmap_txskb(np, np->get_tx_ctx);
 
 		if (flags & NV_TX2_LASTPACKET) {
-			if (!(flags & NV_TX2_ERROR))
-				dev->stats.tx_packets++;
-			else {
+			if (flags & NV_TX2_ERROR) {
 				if ((flags & NV_TX2_RETRYERROR) && !(flags & NV_TX2_RETRYCOUNT_MASK)) {
 					if (np->driver_data & DEV_HAS_GEAR_MODE)
 						nv_gear_backoff_reseed(dev);
@@ -2649,7 +2631,6 @@ static int nv_rx_process(struct net_device *dev, int limit)
 					if ((flags & NV_RX_ERROR_MASK) == NV_RX_ERROR4) {
 						len = nv_getlen(dev, skb->data, len);
 						if (len < 0) {
-							dev->stats.rx_errors++;
 							dev_kfree_skb(skb);
 							goto next_pkt;
 						}
@@ -2663,11 +2644,6 @@ static int nv_rx_process(struct net_device *dev, int limit)
 					else {
 						if (flags & NV_RX_MISSEDFRAME)
 							dev->stats.rx_missed_errors++;
-						if (flags & NV_RX_CRCERR)
-							dev->stats.rx_crc_errors++;
-						if (flags & NV_RX_OVERFLOW)
-							dev->stats.rx_over_errors++;
-						dev->stats.rx_errors++;
 						dev_kfree_skb(skb);
 						goto next_pkt;
 					}
@@ -2683,7 +2659,6 @@ static int nv_rx_process(struct net_device *dev, int limit)
 					if ((flags & NV_RX2_ERROR_MASK) == NV_RX2_ERROR4) {
 						len = nv_getlen(dev, skb->data, len);
 						if (len < 0) {
-							dev->stats.rx_errors++;
 							dev_kfree_skb(skb);
 							goto next_pkt;
 						}
@@ -2695,11 +2670,6 @@ static int nv_rx_process(struct net_device *dev, int limit)
 					}
 					/* the rest are hard errors */
 					else {
-						if (flags & NV_RX2_CRCERR)
-							dev->stats.rx_crc_errors++;
-						if (flags & NV_RX2_OVERFLOW)
-							dev->stats.rx_over_errors++;
-						dev->stats.rx_errors++;
 						dev_kfree_skb(skb);
 						goto next_pkt;
 					}
@@ -2717,7 +2687,6 @@ static int nv_rx_process(struct net_device *dev, int limit)
 		skb->protocol = eth_type_trans(skb, dev);
 		napi_gro_receive(&np->napi, skb);
 		dev->stats.rx_packets++;
-		dev->stats.rx_bytes += len;
 next_pkt:
 		if (unlikely(np->get_rx.orig++ == np->last_rx.orig))
 			np->get_rx.orig = np->first_rx.orig;
@@ -2800,9 +2769,7 @@ static int nv_rx_process_optimized(struct net_device *dev, int limit)
 				__vlan_hwaccel_put_tag(skb, vid);
 			}
 			napi_gro_receive(&np->napi, skb);
-
 			dev->stats.rx_packets++;
-			dev->stats.rx_bytes += len;
 		} else {
 			dev_kfree_skb(skb);
 		}
-- 
1.7.3.1

[PATCH net v3 6/9] 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 <david.decotigny@google.com>
---
 drivers/net/ethernet/nvidia/forcedeth.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index 38d8391..b26e7db 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -1687,6 +1687,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) {
@@ -1711,11 +1712,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 net v3 7/9] 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 <david.decotigny@google.com>
---
 drivers/net/ethernet/nvidia/forcedeth.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index b26e7db..90cdf26 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -633,6 +633,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" },
@@ -673,6 +674,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;
@@ -2529,6 +2531,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

[PATCH net v3 8/9] forcedeth: 64-bit stats

[David Decotigny]

This converts forcedeth stats to be 64-bits. It also improves
accounting for dropped rx frames.

Tested:
  16-way SMP x86_64 ->
  RX bytes:7244556582 (7.2 GB)  TX bytes:181904254 (181.9 MB)



Signed-off-by: David Decotigny <david.decotigny@google.com>
---
 drivers/net/ethernet/nvidia/forcedeth.c |   69 +++++++++++++++++++-----------
 1 files changed, 44 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index 90cdf26..08c512b 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -799,6 +799,8 @@ struct fe_priv {
 	struct timer_list stats_poll;
 	u32 nic_poll_irq;
 	int rx_ring_size;
+	unsigned long stats_rx_dropped;
+	unsigned long stats_rx_missed_errors;
 
 	/* media detection workaround.
 	 * Locking: Within irq hander or disable_irq+spin_lock(&np->lock);
@@ -821,6 +823,7 @@ struct fe_priv {
 	struct nv_skb_map *tx_change_owner;
 	struct nv_skb_map *tx_end_flip;
 	int tx_stop;
+	unsigned long stats_tx_dropped;
 
 	/* msi/msi-x fields */
 	u32 msi_flags;
@@ -1700,33 +1703,47 @@ static void nv_get_hw_stats(struct net_device *dev)
 }
 
 /*
- * nv_get_stats: dev->get_stats function
+ * nv_get_stats: dev->ndo_get_stats64 function
  * Get latest stats value from the nic.
  * Called with read_lock(&dev_base_lock) held for read -
  * only synchronized against unregister_netdevice.
  */
-static struct net_device_stats *nv_get_stats(struct net_device *dev)
+static struct rtnl_link_stats64*
+nv_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *storage)
 {
 	struct fe_priv *np = netdev_priv(dev);
 
 	/* If the nic supports hw counters then retrieve latest values */
-	if (np->driver_data & (DEV_HAS_STATISTICS_V1|DEV_HAS_STATISTICS_V2|DEV_HAS_STATISTICS_V3)) {
+	if (np->driver_data & (DEV_HAS_STATISTICS_V1
+			       | DEV_HAS_STATISTICS_V2
+			       | DEV_HAS_STATISTICS_V3)) {
 		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;
-	}
-
-	return &dev->stats;
+		/* generic stats */
+		storage->rx_packets = np->estats.rx_packets;
+		storage->tx_packets = np->estats.tx_packets;
+		storage->rx_bytes   = np->estats.rx_bytes;
+		storage->tx_bytes   = np->estats.tx_bytes;
+		storage->rx_errors  = np->estats.rx_errors_total;
+		storage->tx_errors  = np->estats.tx_errors_total;
+		storage->rx_dropped = np->stats_rx_dropped;
+		storage->tx_dropped = np->stats_tx_dropped;
+		storage->multicast  = np->estats.rx_multicast;
+
+		/* detailed rx_errors */
+		storage->rx_length_errors = np->estats.rx_length_error;
+		storage->rx_over_errors   = np->estats.rx_over_errors;
+		storage->rx_crc_errors    = np->estats.rx_crc_errors;
+		storage->rx_frame_errors  = np->estats.rx_frame_align_error;
+		storage->rx_fifo_errors   = np->estats.rx_drop_frame;
+		storage->rx_missed_errors = np->stats_rx_missed_errors;
+
+		/* detailed tx_errors */
+		storage->tx_carrier_errors = np->estats.tx_carrier_errors;
+		storage->tx_fifo_errors    = np->estats.tx_fifo_errors;
+	}
+
+	return storage;
 }
 
 /*
@@ -1759,8 +1776,10 @@ static int nv_alloc_rx(struct net_device *dev)
 				np->put_rx.orig = np->first_rx.orig;
 			if (unlikely(np->put_rx_ctx++ == np->last_rx_ctx))
 				np->put_rx_ctx = np->first_rx_ctx;
-		} else
+		} else {
+			np->stats_rx_dropped++;
 			return 1;
+		}
 	}
 	return 0;
 }
@@ -1791,8 +1810,10 @@ static int nv_alloc_rx_optimized(struct net_device *dev)
 				np->put_rx.ex = np->first_rx.ex;
 			if (unlikely(np->put_rx_ctx++ == np->last_rx_ctx))
 				np->put_rx_ctx = np->first_rx_ctx;
-		} else
+		} else {
+			np->stats_rx_dropped++;
 			return 1;
+		}
 	}
 	return 0;
 }
@@ -1928,7 +1949,7 @@ static void nv_drain_tx(struct net_device *dev)
 			np->tx_ring.ex[i].buflow = 0;
 		}
 		if (nv_release_txskb(np, &np->tx_skb[i]))
-			dev->stats.tx_dropped++;
+			np->stats_tx_dropped++;
 		np->tx_skb[i].dma = 0;
 		np->tx_skb[i].dma_len = 0;
 		np->tx_skb[i].dma_single = 0;
@@ -2651,7 +2672,7 @@ static int nv_rx_process(struct net_device *dev, int limit)
 					/* the rest are hard errors */
 					else {
 						if (flags & NV_RX_MISSEDFRAME)
-							dev->stats.rx_missed_errors++;
+							np->stats_rx_missed_errors++;
 						dev_kfree_skb(skb);
 						goto next_pkt;
 					}
@@ -2694,7 +2715,6 @@ static int nv_rx_process(struct net_device *dev, int limit)
 		skb_put(skb, len);
 		skb->protocol = eth_type_trans(skb, dev);
 		napi_gro_receive(&np->napi, skb);
-		dev->stats.rx_packets++;
 next_pkt:
 		if (unlikely(np->get_rx.orig++ == np->last_rx.orig))
 			np->get_rx.orig = np->first_rx.orig;
@@ -2777,7 +2797,6 @@ static int nv_rx_process_optimized(struct net_device *dev, int limit)
 				__vlan_hwaccel_put_tag(skb, vid);
 			}
 			napi_gro_receive(&np->napi, skb);
-			dev->stats.rx_packets++;
 		} else {
 			dev_kfree_skb(skb);
 		}
@@ -5199,7 +5218,7 @@ static int nv_close(struct net_device *dev)
 static const struct net_device_ops nv_netdev_ops = {
 	.ndo_open		= nv_open,
 	.ndo_stop		= nv_close,
-	.ndo_get_stats		= nv_get_stats,
+	.ndo_get_stats64	= nv_get_stats64,
 	.ndo_start_xmit		= nv_start_xmit,
 	.ndo_tx_timeout		= nv_tx_timeout,
 	.ndo_change_mtu		= nv_change_mtu,
@@ -5216,7 +5235,7 @@ static const struct net_device_ops nv_netdev_ops = {
 static const struct net_device_ops nv_netdev_ops_optimized = {
 	.ndo_open		= nv_open,
 	.ndo_stop		= nv_close,
-	.ndo_get_stats		= nv_get_stats,
+	.ndo_get_stats64	= nv_get_stats64,
 	.ndo_start_xmit		= nv_start_xmit_optimized,
 	.ndo_tx_timeout		= nv_tx_timeout,
 	.ndo_change_mtu		= nv_change_mtu,
-- 
1.7.3.1

[PATCH net v3 9/9] forcedeth: fix a few sparse warnings (variable shadowing)

[David Decotigny]

This fixes the following sparse warnings:
drivers/net/ethernet/nvidia/forcedeth.c:2113:7: warning: symbol 'size' shadows an earlier one
drivers/net/ethernet/nvidia/forcedeth.c:2102:6: originally declared here
drivers/net/ethernet/nvidia/forcedeth.c:2155:7: warning: symbol 'size' shadows an earlier one
drivers/net/ethernet/nvidia/forcedeth.c:2102:6: originally declared here
drivers/net/ethernet/nvidia/forcedeth.c:2227:7: warning: symbol 'size' shadows an earlier one
drivers/net/ethernet/nvidia/forcedeth.c:2215:6: originally declared here
drivers/net/ethernet/nvidia/forcedeth.c:2271:7: warning: symbol 'size' shadows an earlier one
drivers/net/ethernet/nvidia/forcedeth.c:2215:6: originally declared here
drivers/net/ethernet/nvidia/forcedeth.c:2986:20: warning: symbol 'addr' shadows an earlier one
drivers/net/ethernet/nvidia/forcedeth.c:2963:6: originally declared here



Signed-off-by: David Decotigny <david.decotigny@google.com>
---
 drivers/net/ethernet/nvidia/forcedeth.c |   34 +++++++++++++++---------------
 1 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index 08c512b..5ca9859 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -2131,10 +2131,10 @@ static netdev_tx_t nv_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	/* add fragments to entries count */
 	for (i = 0; i < fragments; i++) {
-		u32 size = skb_frag_size(&skb_shinfo(skb)->frags[i]);
+		u32 frag_size = skb_frag_size(&skb_shinfo(skb)->frags[i]);
 
-		entries += (size >> NV_TX2_TSO_MAX_SHIFT) +
-			   ((size & (NV_TX2_TSO_MAX_SIZE-1)) ? 1 : 0);
+		entries += (frag_size >> NV_TX2_TSO_MAX_SHIFT) +
+			   ((frag_size & (NV_TX2_TSO_MAX_SIZE-1)) ? 1 : 0);
 	}
 
 	spin_lock_irqsave(&np->lock, flags);
@@ -2173,13 +2173,13 @@ static netdev_tx_t nv_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	/* setup the fragments */
 	for (i = 0; i < fragments; i++) {
 		const skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
-		u32 size = skb_frag_size(frag);
+		u32 frag_size = skb_frag_size(frag);
 		offset = 0;
 
 		do {
 			prev_tx = put_tx;
 			prev_tx_ctx = np->put_tx_ctx;
-			bcnt = (size > NV_TX2_TSO_MAX_SIZE) ? NV_TX2_TSO_MAX_SIZE : size;
+			bcnt = (frag_size > NV_TX2_TSO_MAX_SIZE) ? NV_TX2_TSO_MAX_SIZE : frag_size;
 			np->put_tx_ctx->dma = skb_frag_dma_map(
 							&np->pci_dev->dev,
 							frag, offset,
@@ -2191,12 +2191,12 @@ static netdev_tx_t nv_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			put_tx->flaglen = cpu_to_le32((bcnt-1) | tx_flags);
 
 			offset += bcnt;
-			size -= bcnt;
+			frag_size -= bcnt;
 			if (unlikely(put_tx++ == np->last_tx.orig))
 				put_tx = np->first_tx.orig;
 			if (unlikely(np->put_tx_ctx++ == np->last_tx_ctx))
 				np->put_tx_ctx = np->first_tx_ctx;
-		} while (size);
+		} while (frag_size);
 	}
 
 	/* set last fragment flag  */
@@ -2245,10 +2245,10 @@ static netdev_tx_t nv_start_xmit_optimized(struct sk_buff *skb,
 
 	/* add fragments to entries count */
 	for (i = 0; i < fragments; i++) {
-		u32 size = skb_frag_size(&skb_shinfo(skb)->frags[i]);
+		u32 frag_size = skb_frag_size(&skb_shinfo(skb)->frags[i]);
 
-		entries += (size >> NV_TX2_TSO_MAX_SHIFT) +
-			   ((size & (NV_TX2_TSO_MAX_SIZE-1)) ? 1 : 0);
+		entries += (frag_size >> NV_TX2_TSO_MAX_SHIFT) +
+			   ((frag_size & (NV_TX2_TSO_MAX_SIZE-1)) ? 1 : 0);
 	}
 
 	spin_lock_irqsave(&np->lock, flags);
@@ -2289,13 +2289,13 @@ static netdev_tx_t nv_start_xmit_optimized(struct sk_buff *skb,
 	/* setup the fragments */
 	for (i = 0; i < fragments; i++) {
 		skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
-		u32 size = skb_frag_size(frag);
+		u32 frag_size = skb_frag_size(frag);
 		offset = 0;
 
 		do {
 			prev_tx = put_tx;
 			prev_tx_ctx = np->put_tx_ctx;
-			bcnt = (size > NV_TX2_TSO_MAX_SIZE) ? NV_TX2_TSO_MAX_SIZE : size;
+			bcnt = (frag_size > NV_TX2_TSO_MAX_SIZE) ? NV_TX2_TSO_MAX_SIZE : frag_size;
 			np->put_tx_ctx->dma = skb_frag_dma_map(
 							&np->pci_dev->dev,
 							frag, offset,
@@ -2308,12 +2308,12 @@ static netdev_tx_t nv_start_xmit_optimized(struct sk_buff *skb,
 			put_tx->flaglen = cpu_to_le32((bcnt-1) | tx_flags);
 
 			offset += bcnt;
-			size -= bcnt;
+			frag_size -= bcnt;
 			if (unlikely(put_tx++ == np->last_tx.ex))
 				put_tx = np->first_tx.ex;
 			if (unlikely(np->put_tx_ctx++ == np->last_tx_ctx))
 				np->put_tx_ctx = np->first_tx_ctx;
-		} while (size);
+		} while (frag_size);
 	}
 
 	/* set last fragment flag  */
@@ -2969,11 +2969,11 @@ static void nv_set_multicast(struct net_device *dev)
 				struct netdev_hw_addr *ha;
 
 				netdev_for_each_mc_addr(ha, dev) {
-					unsigned char *addr = ha->addr;
+					unsigned char *hw_addr = ha->addr;
 					u32 a, b;
 
-					a = le32_to_cpu(*(__le32 *) addr);
-					b = le16_to_cpu(*(__le16 *) (&addr[4]));
+					a = le32_to_cpu(*(__le32 *) hw_addr);
+					b = le16_to_cpu(*(__le16 *) (&hw_addr[4]));
 					alwaysOn[0] &= a;
 					alwaysOff[0] &= ~a;
 					alwaysOn[1] &= b;
-- 
1.7.3.1

Re: [PATCH net v3 0/9] forcedeth: minor fixes for stats, rmmod, sparse

[David Miller]

From: David Decotigny <david.decotigny@google.com>
Date: Fri,  4 Nov 2011 18:53:24 -0700

> Changes since v2:
>  - removed "Fix a race during rmmod of forcedeth" from the series
>    (will look at it separately with original author)
>  - added "remove unneeded stats updates" and "64-bit stats"
>  - reordered patches
> 
> Changes since v1:
>  - rebased on top of netdev tip
>  - do not repeat name of device in netdev_dbg
>  - do not completely mute TX timeout messages when debug_tx_timeout is
>    not set
>  - make debug_tx_timeout writable in /sys/module
>  Note: I am re-submitting "expose module parameters in /sys/module" as
>        it can be useful in production and I was assured it doesn't add
>        much memory overhead by the sysfs maintainers.

If you want me to apply any of this now you're going to have to split
out the pure bug fixes from the feature additions and submit the feature
bits later when net-next opens back up.

Because things like 64-bit stats and new stat counters are not
appropriate at this time in the development cycle.

Re: [PATCH net v3 8/9] forcedeth: 64-bit stats

[Eric Dumazet]

Le vendredi 04 novembre 2011 à 18:53 -0700, David Decotigny a écrit :
> This converts forcedeth stats to be 64-bits. It also improves
> accounting for dropped rx frames.
> 
> Tested:
>   16-way SMP x86_64 ->
>   RX bytes:7244556582 (7.2 GB)  TX bytes:181904254 (181.9 MB)
> 
> 

This changelog and patch title are misleading.

On a 32bit x86, stats are still 32bit wide after your patch.

On a 64bit x86_64, stats were already 64bit wide before your patch.

So the real thing is about not using the embedded netdevice dev->stats
structure, to reduce false sharing.

> 
> Signed-off-by: David Decotigny <david.decotigny@google.com>
> ---
>  drivers/net/ethernet/nvidia/forcedeth.c |   69 +++++++++++++++++++-----------
>  1 files changed, 44 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
> index 90cdf26..08c512b 100644
> --- a/drivers/net/ethernet/nvidia/forcedeth.c
> +++ b/drivers/net/ethernet/nvidia/forcedeth.c
> @@ -799,6 +799,8 @@ struct fe_priv {
>  	struct timer_list stats_poll;
>  	u32 nic_poll_irq;
>  	int rx_ring_size;
> +	unsigned long stats_rx_dropped;
> +	unsigned long stats_rx_missed_errors;
>  
>  	/* media detection workaround.
>  	 * Locking: Within irq hander or disable_irq+spin_lock(&np->lock);
> @@ -821,6 +823,7 @@ struct fe_priv {
>  	struct nv_skb_map *tx_change_owner;
>  	struct nv_skb_map *tx_end_flip;
>  	int tx_stop;
> +	unsigned long stats_tx_dropped;
>  
>  	/* msi/msi-x fields */
>  	u32 msi_flags;
> @@ -1700,33 +1703,47 @@ static void nv_get_hw_stats(struct net_device *dev)
>  }
>  
>  /*
> - * nv_get_stats: dev->get_stats function
> + * nv_get_stats: dev->ndo_get_stats64 function
>   * Get latest stats value from the nic.
>   * Called with read_lock(&dev_base_lock) held for read -
>   * only synchronized against unregister_netdevice.
>   */
> -static struct net_device_stats *nv_get_stats(struct net_device *dev)
> +static struct rtnl_link_stats64*
> +nv_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *storage)
>  {
>  	struct fe_priv *np = netdev_priv(dev);
>  
>  	/* If the nic supports hw counters then retrieve latest values */
> -	if (np->driver_data & (DEV_HAS_STATISTICS_V1|DEV_HAS_STATISTICS_V2|DEV_HAS_STATISTICS_V3)) {
> +	if (np->driver_data & (DEV_HAS_STATISTICS_V1
> +			       | DEV_HAS_STATISTICS_V2
> +			       | DEV_HAS_STATISTICS_V3)) {
>  		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;
> -	}
> -
> -	return &dev->stats;
> +		/* generic stats */
> +		storage->rx_packets = np->estats.rx_packets;
> +		storage->tx_packets = np->estats.tx_packets;
> +		storage->rx_bytes   = np->estats.rx_bytes;
> +		storage->tx_bytes   = np->estats.tx_bytes;
> +		storage->rx_errors  = np->estats.rx_errors_total;
> +		storage->tx_errors  = np->estats.tx_errors_total;
> +		storage->rx_dropped = np->stats_rx_dropped;
> +		storage->tx_dropped = np->stats_tx_dropped;
> +		storage->multicast  = np->estats.rx_multicast;
> +
> +		/* detailed rx_errors */
> +		storage->rx_length_errors = np->estats.rx_length_error;
> +		storage->rx_over_errors   = np->estats.rx_over_errors;
> +		storage->rx_crc_errors    = np->estats.rx_crc_errors;
> +		storage->rx_frame_errors  = np->estats.rx_frame_align_error;
> +		storage->rx_fifo_errors   = np->estats.rx_drop_frame;
> +		storage->rx_missed_errors = np->stats_rx_missed_errors;
> +
> +		/* detailed tx_errors */
> +		storage->tx_carrier_errors = np->estats.tx_carrier_errors;
> +		storage->tx_fifo_errors    = np->estats.tx_fifo_errors;
> +	}
> +
> +	return storage;
>  }
>  
>  /*
> @@ -1759,8 +1776,10 @@ static int nv_alloc_rx(struct net_device *dev)
>  				np->put_rx.orig = np->first_rx.orig;
>  			if (unlikely(np->put_rx_ctx++ == np->last_rx_ctx))
>  				np->put_rx_ctx = np->first_rx_ctx;
> -		} else
> +		} else {
> +			np->stats_rx_dropped++;
>  			return 1;
> +		}
>  	}
>  	return 0;
>  }
> @@ -1791,8 +1810,10 @@ static int nv_alloc_rx_optimized(struct net_device *dev)
>  				np->put_rx.ex = np->first_rx.ex;
>  			if (unlikely(np->put_rx_ctx++ == np->last_rx_ctx))
>  				np->put_rx_ctx = np->first_rx_ctx;
> -		} else
> +		} else {
> +			np->stats_rx_dropped++;
>  			return 1;
> +		}
>  	}
>  	return 0;
>  }
> @@ -1928,7 +1949,7 @@ static void nv_drain_tx(struct net_device *dev)
>  			np->tx_ring.ex[i].buflow = 0;
>  		}
>  		if (nv_release_txskb(np, &np->tx_skb[i]))
> -			dev->stats.tx_dropped++;
> +			np->stats_tx_dropped++;
>  		np->tx_skb[i].dma = 0;
>  		np->tx_skb[i].dma_len = 0;
>  		np->tx_skb[i].dma_single = 0;
> @@ -2651,7 +2672,7 @@ static int nv_rx_process(struct net_device *dev, int limit)
>  					/* the rest are hard errors */
>  					else {
>  						if (flags & NV_RX_MISSEDFRAME)
> -							dev->stats.rx_missed_errors++;
> +							np->stats_rx_missed_errors++;
>  						dev_kfree_skb(skb);
>  						goto next_pkt;
>  					}
> @@ -2694,7 +2715,6 @@ static int nv_rx_process(struct net_device *dev, int limit)
>  		skb_put(skb, len);
>  		skb->protocol = eth_type_trans(skb, dev);
>  		napi_gro_receive(&np->napi, skb);
> -		dev->stats.rx_packets++;
>  next_pkt:
>  		if (unlikely(np->get_rx.orig++ == np->last_rx.orig))
>  			np->get_rx.orig = np->first_rx.orig;
> @@ -2777,7 +2797,6 @@ static int nv_rx_process_optimized(struct net_device *dev, int limit)
>  				__vlan_hwaccel_put_tag(skb, vid);
>  			}
>  			napi_gro_receive(&np->napi, skb);
> -			dev->stats.rx_packets++;
>  		} else {
>  			dev_kfree_skb(skb);
>  		}
> @@ -5199,7 +5218,7 @@ static int nv_close(struct net_device *dev)
>  static const struct net_device_ops nv_netdev_ops = {
>  	.ndo_open		= nv_open,
>  	.ndo_stop		= nv_close,
> -	.ndo_get_stats		= nv_get_stats,
> +	.ndo_get_stats64	= nv_get_stats64,
>  	.ndo_start_xmit		= nv_start_xmit,
>  	.ndo_tx_timeout		= nv_tx_timeout,
>  	.ndo_change_mtu		= nv_change_mtu,
> @@ -5216,7 +5235,7 @@ static const struct net_device_ops nv_netdev_ops = {
>  static const struct net_device_ops nv_netdev_ops_optimized = {
>  	.ndo_open		= nv_open,
>  	.ndo_stop		= nv_close,
> -	.ndo_get_stats		= nv_get_stats,
> +	.ndo_get_stats64	= nv_get_stats64,
>  	.ndo_start_xmit		= nv_start_xmit_optimized,
>  	.ndo_tx_timeout		= nv_tx_timeout,
>  	.ndo_change_mtu		= nv_change_mtu,

Re: [PATCH net v3 8/9] forcedeth: 64-bit stats

[David Decotigny]

Thanks Eric, I will update this.

Please note that I am deferring 5 of the patches from this "v3"
patch-set for net-next: this patch is one of them.
Meanwhile, the most recent version of the remaining 4 patches is the
"v4" series I sent yesterday; they are limited to minor fixes.

Regards,

--
David Decotigny



On Sat, Nov 5, 2011 at 12:28 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le vendredi 04 novembre 2011 à 18:53 -0700, David Decotigny a écrit :
>> This converts forcedeth stats to be 64-bits. It also improves
>> accounting for dropped rx frames.
>>
>> Tested:
>>   16-way SMP x86_64 ->
>>   RX bytes:7244556582 (7.2 GB)  TX bytes:181904254 (181.9 MB)
>>
>>
>
> This changelog and patch title are misleading.
>
> On a 32bit x86, stats are still 32bit wide after your patch.
>
> On a 64bit x86_64, stats were already 64bit wide before your patch.
>
> So the real thing is about not using the embedded netdevice dev->stats
> structure, to reduce false sharing.
>
>>
>> Signed-off-by: David Decotigny <david.decotigny@google.com>
>> ---
>>  drivers/net/ethernet/nvidia/forcedeth.c |   69 +++++++++++++++++++-----------
>>  1 files changed, 44 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
>> index 90cdf26..08c512b 100644
>> --- a/drivers/net/ethernet/nvidia/forcedeth.c
>> +++ b/drivers/net/ethernet/nvidia/forcedeth.c
>> @@ -799,6 +799,8 @@ struct fe_priv {
>>       struct timer_list stats_poll;
>>       u32 nic_poll_irq;
>>       int rx_ring_size;
>> +     unsigned long stats_rx_dropped;
>> +     unsigned long stats_rx_missed_errors;
>>
>>       /* media detection workaround.
>>        * Locking: Within irq hander or disable_irq+spin_lock(&np->lock);
>> @@ -821,6 +823,7 @@ struct fe_priv {
>>       struct nv_skb_map *tx_change_owner;
>>       struct nv_skb_map *tx_end_flip;
>>       int tx_stop;
>> +     unsigned long stats_tx_dropped;
>>
>>       /* msi/msi-x fields */
>>       u32 msi_flags;
>> @@ -1700,33 +1703,47 @@ static void nv_get_hw_stats(struct net_device *dev)
>>  }
>>
>>  /*
>> - * nv_get_stats: dev->get_stats function
>> + * nv_get_stats: dev->ndo_get_stats64 function
>>   * Get latest stats value from the nic.
>>   * Called with read_lock(&dev_base_lock) held for read -
>>   * only synchronized against unregister_netdevice.
>>   */
>> -static struct net_device_stats *nv_get_stats(struct net_device *dev)
>> +static struct rtnl_link_stats64*
>> +nv_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *storage)
>>  {
>>       struct fe_priv *np = netdev_priv(dev);
>>
>>       /* If the nic supports hw counters then retrieve latest values */
>> -     if (np->driver_data & (DEV_HAS_STATISTICS_V1|DEV_HAS_STATISTICS_V2|DEV_HAS_STATISTICS_V3)) {
>> +     if (np->driver_data & (DEV_HAS_STATISTICS_V1
>> +                            | DEV_HAS_STATISTICS_V2
>> +                            | DEV_HAS_STATISTICS_V3)) {
>>               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;
>> -     }
>> -
>> -     return &dev->stats;
>> +             /* generic stats */
>> +             storage->rx_packets = np->estats.rx_packets;
>> +             storage->tx_packets = np->estats.tx_packets;
>> +             storage->rx_bytes   = np->estats.rx_bytes;
>> +             storage->tx_bytes   = np->estats.tx_bytes;
>> +             storage->rx_errors  = np->estats.rx_errors_total;
>> +             storage->tx_errors  = np->estats.tx_errors_total;
>> +             storage->rx_dropped = np->stats_rx_dropped;
>> +             storage->tx_dropped = np->stats_tx_dropped;
>> +             storage->multicast  = np->estats.rx_multicast;
>> +
>> +             /* detailed rx_errors */
>> +             storage->rx_length_errors = np->estats.rx_length_error;
>> +             storage->rx_over_errors   = np->estats.rx_over_errors;
>> +             storage->rx_crc_errors    = np->estats.rx_crc_errors;
>> +             storage->rx_frame_errors  = np->estats.rx_frame_align_error;
>> +             storage->rx_fifo_errors   = np->estats.rx_drop_frame;
>> +             storage->rx_missed_errors = np->stats_rx_missed_errors;
>> +
>> +             /* detailed tx_errors */
>> +             storage->tx_carrier_errors = np->estats.tx_carrier_errors;
>> +             storage->tx_fifo_errors    = np->estats.tx_fifo_errors;
>> +     }
>> +
>> +     return storage;
>>  }
>>
>>  /*
>> @@ -1759,8 +1776,10 @@ static int nv_alloc_rx(struct net_device *dev)
>>                               np->put_rx.orig = np->first_rx.orig;
>>                       if (unlikely(np->put_rx_ctx++ == np->last_rx_ctx))
>>                               np->put_rx_ctx = np->first_rx_ctx;
>> -             } else
>> +             } else {
>> +                     np->stats_rx_dropped++;
>>                       return 1;
>> +             }
>>       }
>>       return 0;
>>  }
>> @@ -1791,8 +1810,10 @@ static int nv_alloc_rx_optimized(struct net_device *dev)
>>                               np->put_rx.ex = np->first_rx.ex;
>>                       if (unlikely(np->put_rx_ctx++ == np->last_rx_ctx))
>>                               np->put_rx_ctx = np->first_rx_ctx;
>> -             } else
>> +             } else {
>> +                     np->stats_rx_dropped++;
>>                       return 1;
>> +             }
>>       }
>>       return 0;
>>  }
>> @@ -1928,7 +1949,7 @@ static void nv_drain_tx(struct net_device *dev)
>>                       np->tx_ring.ex[i].buflow = 0;
>>               }
>>               if (nv_release_txskb(np, &np->tx_skb[i]))
>> -                     dev->stats.tx_dropped++;
>> +                     np->stats_tx_dropped++;
>>               np->tx_skb[i].dma = 0;
>>               np->tx_skb[i].dma_len = 0;
>>               np->tx_skb[i].dma_single = 0;
>> @@ -2651,7 +2672,7 @@ static int nv_rx_process(struct net_device *dev, int limit)
>>                                       /* the rest are hard errors */
>>                                       else {
>>                                               if (flags & NV_RX_MISSEDFRAME)
>> -                                                     dev->stats.rx_missed_errors++;
>> +                                                     np->stats_rx_missed_errors++;
>>                                               dev_kfree_skb(skb);
>>                                               goto next_pkt;
>>                                       }
>> @@ -2694,7 +2715,6 @@ static int nv_rx_process(struct net_device *dev, int limit)
>>               skb_put(skb, len);
>>               skb->protocol = eth_type_trans(skb, dev);
>>               napi_gro_receive(&np->napi, skb);
>> -             dev->stats.rx_packets++;
>>  next_pkt:
>>               if (unlikely(np->get_rx.orig++ == np->last_rx.orig))
>>                       np->get_rx.orig = np->first_rx.orig;
>> @@ -2777,7 +2797,6 @@ static int nv_rx_process_optimized(struct net_device *dev, int limit)
>>                               __vlan_hwaccel_put_tag(skb, vid);
>>                       }
>>                       napi_gro_receive(&np->napi, skb);
>> -                     dev->stats.rx_packets++;
>>               } else {
>>                       dev_kfree_skb(skb);
>>               }
>> @@ -5199,7 +5218,7 @@ static int nv_close(struct net_device *dev)
>>  static const struct net_device_ops nv_netdev_ops = {
>>       .ndo_open               = nv_open,
>>       .ndo_stop               = nv_close,
>> -     .ndo_get_stats          = nv_get_stats,
>> +     .ndo_get_stats64        = nv_get_stats64,
>>       .ndo_start_xmit         = nv_start_xmit,
>>       .ndo_tx_timeout         = nv_tx_timeout,
>>       .ndo_change_mtu         = nv_change_mtu,
>> @@ -5216,7 +5235,7 @@ static const struct net_device_ops nv_netdev_ops = {
>>  static const struct net_device_ops nv_netdev_ops_optimized = {
>>       .ndo_open               = nv_open,
>>       .ndo_stop               = nv_close,
>> -     .ndo_get_stats          = nv_get_stats,
>> +     .ndo_get_stats64        = nv_get_stats64,
>>       .ndo_start_xmit         = nv_start_xmit_optimized,
>>       .ndo_tx_timeout         = nv_tx_timeout,
>>       .ndo_change_mtu         = nv_change_mtu,
>
>
>