forcedeth: minor fixes for stats, rmmod, sparse

PATCH Superseded David Decotigny 3 participants 13 messages Nov 5, 2011

Subsystems (3)

Networking Network drivers Forcedeth gigabit Ethernet driver

lore

Share & tools

lore

David Decotigny

author

2011-11-05 01:53 UTC #0 lore raw

v3 cover forcedeth: minor fixes for stats, rmmod, sparse

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(-)

Signature · 1 line

1.7.3.1

David Decotigny

author

2011-11-05 01:53 UTC #1 lore raw

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

From: Mike Ditto <[email protected]>

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

Signed-off-by David Decotigny <[email protected]>

---
 drivers/net/ethernet/nvidia/forcedeth.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

drivers/net/ethernet/nvidia/forcedeth.c +2 -0

index 1e37eb9..2f884fc 100644
@@ -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) {

Signature · 1 line

1.7.3.1

David Decotigny

author

2011-11-05 01:53 UTC #2 lore raw

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

In reply to David Decotigny

From: Mike Ditto <[email protected]>

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 <[email protected]>

---
 drivers/net/ethernet/nvidia/forcedeth.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

drivers/net/ethernet/nvidia/forcedeth.c +8 -5

index 2f884fc..9aefee1 100644
@@ -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))

Signature · 1 line

1.7.3.1

David Decotigny

author

2011-11-05 01:53 UTC #3 lore raw

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

In reply to David Decotigny

From: Sameer Nanda <[email protected]>

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 <[email protected]>

---
 drivers/net/ethernet/nvidia/forcedeth.c |   98 ++++++++++++++++++-------------
 1 files changed, 57 insertions(+), 41 deletions(-)

drivers/net/ethernet/nvidia/forcedeth.c +57 ~~-41~~

index 9aefee1..3f66b74 100644
@@ -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 <[email protected]>");
 MODULE_DESCRIPTION("Reverse Engineered nForce ethernet driver");

Signature · 1 line

1.7.3.1

David Decotigny

author

2011-11-05 01:53 UTC #4 lore raw

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

In reply to David Decotigny

In particular, debug_tx_timeout can be updated at runtime.

Signed-off-by David Decotigny <[email protected]>

---
 drivers/net/ethernet/nvidia/forcedeth.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

drivers/net/ethernet/nvidia/forcedeth.c +9 -9

index 3f66b74..295652d 100644
@@ -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");

Signature · 1 line

1.7.3.1

David Decotigny

author

2011-11-05 01:53 UTC #5 lore raw

PATCH v3 5/9 forcedeth: remove unneeded stats updates

In reply to 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 <[email protected]>

---
 drivers/net/ethernet/nvidia/forcedeth.c |   35 +------------------------------
 1 files changed, 1 insertions(+), 34 deletions(-)

drivers/net/ethernet/nvidia/forcedeth.c +1 ~~-34~~

index 295652d..38d8391 100644
@@ -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);
 		}

Signature · 1 line

1.7.3.1

David Decotigny

author

2011-11-05 01:53 UTC #6 lore raw

PATCH v3 6/9 forcedeth: Improve stats counters

In reply to David Decotigny

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/ethernet/nvidia/forcedeth.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

drivers/net/ethernet/nvidia/forcedeth.c +4 -0

index 38d8391..b26e7db 100644
@@ -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;
 	}

Signature · 1 line

1.7.3.1

David Decotigny

author

2011-11-05 01:53 UTC #7 lore raw

PATCH v3 7/9 forcedeth: new ethtool stat "tx_timeout" to account for tx_timeouts

In reply to David Decotigny

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/ethernet/nvidia/forcedeth.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

drivers/net/ethernet/nvidia/forcedeth.c +4 -0

index b26e7db..90cdf26 100644
@@ -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);

Signature · 1 line

1.7.3.1

David Decotigny

author

2011-11-05 01:53 UTC #8 lore raw

PATCH v3 8/9 forcedeth: 64-bit stats

In reply to 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 <[email protected]>

---
 drivers/net/ethernet/nvidia/forcedeth.c |   69 +++++++++++++++++++-----------
 1 files changed, 44 insertions(+), 25 deletions(-)

drivers/net/ethernet/nvidia/forcedeth.c +44 ~~-25~~

index 90cdf26..08c512b 100644
@@ -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,

Signature · 1 line

1.7.3.1

Eric Dumazet

2011-11-05 07:28 UTC #9 lore raw

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

Quoted 8 lines · 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)

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.

Quoted 168 lines

Signed-off-by David Decotigny <[email protected]>

---
 drivers/net/ethernet/nvidia/forcedeth.c |   69 +++++++++++++++++++-----------
 1 files changed, 44 insertions(+), 25 deletions(-)

drivers/net/ethernet/nvidia/forcedeth.c +44 ~~-25~~

index 90cdf26..08c512b 100644
@@ -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,

David Decotigny

author

2011-11-05 16:34 UTC #10 lore raw

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

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

Quoted 190 lines · Eric Dumazet

Quoted 8 lines · 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)

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.

Quoted 168 lines

Signed-off-by David Decotigny <[email protected]>

---
drivers/net/ethernet/nvidia/forcedeth.c | 69 +++++++++++++++++++-----------

 1 files changed, 44 insertions(+), 25 deletions(-)

drivers/net/ethernet/nvidia/forcedeth.c +42 ~~-22~~

index 90cdf26..08c512b 100644
@@ -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;

drivers/net/ethernet/nvidia/forcedeth.c +2 -3

@@ -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,

David Decotigny

author

2011-11-05 01:53 UTC #11 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/net/ethernet/nvidia/forcedeth.c |   34 +++++++++++++++---------------
 1 files changed, 17 insertions(+), 17 deletions(-)

drivers/net/ethernet/nvidia/forcedeth.c +17 ~~-17~~

index 08c512b..5ca9859 100644
@@ -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;

Signature · 1 line

1.7.3.1

David Miller

maintainer

2011-11-05 02:27 UTC #12 lore raw

In reply to David Decotigny

From: David Decotigny <[email protected]>
Date: Fri, 4 Nov 2011 18:53:24 -0700

Quoted 6 lines

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

Quoted 9 lines

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.