Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v3 0/9] forcedeth: stats & debug enhancements
From: David Decotigny @ 2011-11-15  0:11 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: David S. Miller, Ian Campbell, Eric Dumazet, Jeff Kirsher,
	Ben Hutchings, Jiri Pirko, Joe Perches, Szymon Janc,
	Richard Jones, Ayaz Abdulla, David Decotigny

These changes implement the ndo_get_stats64 API and add a few more
stats and debugging features for forcedeth. They also ensure that
stats updates are correct in SMP systems, 32 or 64-bits.

Changes since v2:
  - patch 1/9 is the cherry-pick of 898bdf2cb43e ("forcedeth: fix
    stats on hardware without extended stats support")
  - removed patch 5/10 "stats for rx_packets based on hardware
    registers" because packets&bytes stats are updated in software
    only (898bdf2cb43e)

Changes since v1:
  - patch 1/10 is the same as
    http://patchwork.ozlabs.org/patch/125017/ (targetting net)
  - other patches updated to take patch 1/10 into account
  - various commit message updates


Tested:
  ~150Mbps incoming TCP, ethtool -S in a loop, x86_64 16-way:
     tx_bytes: 5441989419
     rx_packets: 5439224
     tx_timeout: 0
     tx_packets: 5456705
     rx_bytes: 5566763850

Tested:
  pktgen + loopback report same RX/TX packets and bytes stats

Tested:
  tests above with Kconfig DEBUG_PAGEALLOC DEBUG_MUTEXES
  DEBUG_SPINLOCK LOCKUP_DETECTOR DEBUG_RT_MUTEXES DEBUG_LOCK_ALLOC
  PROVE_LOCKING DEBUG_ATOMIC_SLEEP DEBUG_STACK_USAGE DEBUG_KOBJECT
  DEBUG_VM DEBUG_LIST DEBUG_SG DEBUG_NOTIFIERS TEST_KSTRTOX
  STRICT_DEVMEM DEBUG_STACKOVERFLOW


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

David Decotigny (6):
  forcedeth: expose module parameters in /sys/module
  forcedeth: implement ndo_get_stats64() API
  forcedeth: account for dropped RX frames
  forcedeth: new ethtool stat counter for TX timeouts
  forcedeth: stats updated with a deferrable timer
  forcedeth: whitespace/indentation fixes

Mike Ditto (1):
  forcedeth: Add messages to indicate using MSI or MSI-X

Sameer Nanda (1):
  forcedeth: allow to silence "TX timeout" debug messages

david decotigny (1):
  forcedeth: fix stats on hardware without extended stats support

 drivers/net/ethernet/nvidia/forcedeth.c |  342 ++++++++++++++++++++++---------
 1 files changed, 246 insertions(+), 96 deletions(-)

-- 
1.7.3.1

^ permalink raw reply

* [PATCH net-next v3 7/9] forcedeth: new ethtool stat counter for TX timeouts
From: David Decotigny @ 2011-11-15  0:11 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: David S. Miller, Ian Campbell, Eric Dumazet, Jeff Kirsher,
	Ben Hutchings, Jiri Pirko, Joe Perches, Szymon Janc,
	Richard Jones, Ayaz Abdulla, David Decotigny
In-Reply-To: <cover.1321313729.git.david.decotigny@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 |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index 84e8d17..dd24035 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; /* should be ifconfig->rx_packets */
 	u64 rx_errors_total;
 	u64 tx_errors_total;
+	u64 tx_timeout;
 
 	/* version 2 stats */
 	u64 tx_deferral;
@@ -852,6 +854,7 @@ struct fe_priv {
 	struct nv_driver_stat stat_tx_packets; /* not always available in HW */
 	struct nv_driver_stat stat_tx_bytes;
 	struct nv_driver_stat stat_tx_dropped;
+	atomic_t stat_tx_timeout;  /* TX timeouts since last nv_update_stats */
 
 	/* msi/msi-x fields */
 	u32 msi_flags;
@@ -1746,6 +1749,7 @@ static void nv_update_stats(struct net_device *dev)
 	NV_DRIVER_STAT_UPDATE_TOTAL(&np->stat_tx_packets);
 	NV_DRIVER_STAT_UPDATE_TOTAL(&np->stat_tx_bytes);
 	NV_DRIVER_STAT_UPDATE_TOTAL(&np->stat_tx_dropped);
+	np->estats.tx_timeout += atomic_xchg(&np->stat_tx_timeout, 0);
 }
 
 /*
@@ -2639,6 +2643,8 @@ static void nv_tx_timeout(struct net_device *dev)
 		}
 	}
 
+	atomic_inc(&np->stat_tx_timeout);
+
 	spin_lock_irq(&np->lock);
 
 	/* 1) stop tx engine */
-- 
1.7.3.1

^ permalink raw reply related

* [PATCH net-next v3 3/9] forcedeth: allow to silence "TX timeout" debug messages
From: David Decotigny @ 2011-11-15  0:11 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: David S. Miller, Ian Campbell, Eric Dumazet, Jeff Kirsher,
	Ben Hutchings, Jiri Pirko, Joe Perches, Szymon Janc,
	Richard Jones, Ayaz Abdulla, Sameer Nanda, David Decotigny
In-Reply-To: <cover.1321313729.git.david.decotigny@google.com>

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 that only a single warning
will be reported.



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 fe17e42..9b917ff 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.
  */
@@ -2477,56 +2482,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));
+			}
 		}
 	}
 
@@ -6156,6 +6169,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

^ permalink raw reply related

* [PATCH net-next v3 2/9] forcedeth: Add messages to indicate using MSI or MSI-X
From: David Decotigny @ 2011-11-15  0:11 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: David S. Miller, Ian Campbell, Eric Dumazet, Jeff Kirsher,
	Ben Hutchings, Jiri Pirko, Joe Perches, Szymon Janc,
	Richard Jones, Ayaz Abdulla, Mike Ditto, David Decotigny
In-Reply-To: <cover.1321313729.git.david.decotigny@google.com>

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 374625b..fe17e42 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -3810,6 +3810,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) {
@@ -3831,6 +3832,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

^ permalink raw reply related

* [PATCH net-next v3 1/9] forcedeth: fix stats on hardware without extended stats support
From: David Decotigny @ 2011-11-15  0:11 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: David S. Miller, Ian Campbell, Eric Dumazet, Jeff Kirsher,
	Ben Hutchings, Jiri Pirko, Joe Perches, Szymon Janc,
	Richard Jones, Ayaz Abdulla, david decotigny
In-Reply-To: <cover.1321313729.git.david.decotigny@google.com>

From: david decotigny <david.decotigny@google.com>

This change makes sure that tx_packets/rx_bytes ifconfig counters are
updated even on NICs that don't provide hardware support for these
stats: they are now updated in software. For the sake of consistency,
we also now have tx_bytes updated in software (hardware counters
include ethernet CRC, and software doesn't account for it).

This reverts parts of:
 - "forcedeth: statistics optimization" (21828163b2)
 - "forcedeth: Improve stats counters" (0bdfea8ba8)
 - "forcedeth: remove unneeded stats updates" (4687f3f364)

Tested:
  pktgen + loopback (http://patchwork.ozlabs.org/patch/124698/)
  reports identical tx_packets/rx_packets and tx_bytes/rx_bytes.

Signed-off-by: David Decotigny <david.decotigny@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 898bdf2cb43eb0a962c397eb4dd1aec2c7211be2)


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

diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index e8a5ae3..374625b 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -609,7 +609,7 @@ struct nv_ethtool_str {
 };
 
 static const struct nv_ethtool_str nv_estats_str[] = {
-	{ "tx_bytes" },
+	{ "tx_bytes" }, /* includes Ethernet FCS CRC */
 	{ "tx_zero_rexmt" },
 	{ "tx_one_rexmt" },
 	{ "tx_many_rexmt" },
@@ -637,7 +637,7 @@ static const struct nv_ethtool_str nv_estats_str[] = {
 	/* version 2 stats */
 	{ "tx_deferral" },
 	{ "tx_packets" },
-	{ "rx_bytes" },
+	{ "rx_bytes" }, /* includes Ethernet FCS CRC */
 	{ "tx_pause" },
 	{ "rx_pause" },
 	{ "rx_drop_frame" },
@@ -649,7 +649,7 @@ static const struct nv_ethtool_str nv_estats_str[] = {
 };
 
 struct nv_ethtool_stats {
-	u64 tx_bytes;
+	u64 tx_bytes; /* should be ifconfig->tx_bytes + 4*tx_packets */
 	u64 tx_zero_rexmt;
 	u64 tx_one_rexmt;
 	u64 tx_many_rexmt;
@@ -670,14 +670,14 @@ struct nv_ethtool_stats {
 	u64 rx_unicast;
 	u64 rx_multicast;
 	u64 rx_broadcast;
-	u64 rx_packets;
+	u64 rx_packets; /* should be ifconfig->rx_packets */
 	u64 rx_errors_total;
 	u64 tx_errors_total;
 
 	/* version 2 stats */
 	u64 tx_deferral;
-	u64 tx_packets;
-	u64 rx_bytes;
+	u64 tx_packets; /* should be ifconfig->tx_packets */
+	u64 rx_bytes;   /* should be ifconfig->rx_bytes + 4*rx_packets */
 	u64 tx_pause;
 	u64 rx_pause;
 	u64 rx_drop_frame;
@@ -1706,10 +1706,17 @@ static struct net_device_stats *nv_get_stats(struct net_device *dev)
 	if (np->driver_data & (DEV_HAS_STATISTICS_V1|DEV_HAS_STATISTICS_V2|DEV_HAS_STATISTICS_V3)) {
 		nv_get_hw_stats(dev);
 
+		/*
+		 * Note: because HW stats are not always available and
+		 * for consistency reasons, the following ifconfig
+		 * stats are managed by software: rx_bytes, tx_bytes,
+		 * rx_packets and tx_packets. The related hardware
+		 * stats reported by ethtool should be equivalent to
+		 * these ifconfig stats, with 4 additional bytes per
+		 * packet (Ethernet FCS CRC).
+		 */
+
 		/* 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;
@@ -2380,6 +2387,9 @@ static int nv_tx_done(struct net_device *dev, int limit)
 				if (flags & NV_TX_ERROR) {
 					if ((flags & NV_TX_RETRYERROR) && !(flags & NV_TX_RETRYCOUNT_MASK))
 						nv_legacybackoff_reseed(dev);
+				} 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;
@@ -2390,6 +2400,9 @@ static int nv_tx_done(struct net_device *dev, int limit)
 				if (flags & NV_TX2_ERROR) {
 					if ((flags & NV_TX2_RETRYERROR) && !(flags & NV_TX2_RETRYCOUNT_MASK))
 						nv_legacybackoff_reseed(dev);
+				} 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;
@@ -2429,6 +2442,9 @@ static int nv_tx_done_optimized(struct net_device *dev, int limit)
 					else
 						nv_legacybackoff_reseed(dev);
 				}
+			} else {
+				dev->stats.tx_packets++;
+				dev->stats.tx_bytes += np->get_tx_ctx->skb->len;
 			}
 
 			dev_kfree_skb_any(np->get_tx_ctx->skb);
@@ -2678,6 +2694,7 @@ 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;
@@ -2761,6 +2778,7 @@ static int nv_rx_process_optimized(struct net_device *dev, int limit)
 			}
 			napi_gro_receive(&np->napi, skb);
 			dev->stats.rx_packets++;
+			dev->stats.rx_bytes += len;
 		} else {
 			dev_kfree_skb(skb);
 		}
-- 
1.7.3.1

^ permalink raw reply related

* Re: [RFC PATCH net-next] enable virtio_net to return bus_info in ethtool -i consistent with emulated NICs
From: Ben Hutchings @ 2011-11-15  0:10 UTC (permalink / raw)
  To: Rick Jones
  Cc: Rick Jones, netdev, Rusty Russell, Michael Tsirkin,
	virtualization
In-Reply-To: <4EC1ACF6.9060908@hp.com>

On Mon, 2011-11-14 at 16:06 -0800, Rick Jones wrote:
> On 11/14/2011 02:30 PM, Ben Hutchings wrote:
> > On Mon, 2011-11-14 at 13:52 -0800, Rick Jones wrote:
> >> From: Rick Jones<rick.jones2@hp.com>
> >>
> >> Add a new .bus_name to virtio_config_ops then modify virtio_net to
> >> call through to it in an ethtool .get_drvinfo routine to report
> >> bus_info in ethtool -i output which is consistent with other
> >> emulated NICs and the output of lspci.
> > [...]
> >> diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
> >> index 0dc30ff..3724d45 100644
> >> --- a/drivers/lguest/lguest_device.c
> >> +++ b/drivers/lguest/lguest_device.c
> >> @@ -381,6 +381,11 @@ error:
> >>   	return PTR_ERR(vqs[i]);
> >>   }
> >>
> >> +static const char *lg_bus_name(struct virtio_device *vdev)
> >> +{
> >> +	return "Not Implemented";
> >> +}
> > [...]
> >> +static const char *kvm_bus_name(struct virtio_device *vdev)
> >> +{
> >> +	return "Not Implemented";
> >> +}
> > [...]
> >
> > Please use the existing 'not implemented' value, which is the empty
> > string.   If you think ethtool should print some helpful message instead
> > of an empty string, please submit a patch for ethtool.
> 
> 
> One question - will those actually be called via an ethtool path?  In my 
> poking about through the virtio code, I got the impression those modules 
> were for "other than networking" sorts of things.

I don't know; I just assumed that was why you were adding them!  In
other contexts such as dev_printk() this string would make even less
sense.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [PATCH] net/packet: remove dead code and unneeded variable from prb_setup_retire_blk_timer()
From: Jesper Juhl @ 2011-11-15  0:08 UTC (permalink / raw)
  To: David Miller
  Cc: rmallon, netdev, linux-kernel, eric.dumazet, xiaosuo, greearb,
	loke.chetan, waltje, gw4pts, waltje, ross.biro, alan
In-Reply-To: <20111114.184736.1515821968538974735.davem@davemloft.net>

On Mon, 14 Nov 2011, David Miller wrote:

> From: Jesper Juhl <jj@chaosbits.net>
> Date: Tue, 15 Nov 2011 00:51:57 +0100 (CET)
> 
> > On Mon, 14 Nov 2011, David Miller wrote:
> > 
> >> From: Jesper Juhl <jj@chaosbits.net>
> >> Date: Tue, 15 Nov 2011 00:37:33 +0100 (CET)
> >> 
> >> > David: You may want to pass on this one. I obviously didn't think it 
> >> > through properly - sorry :-(
> >> 
> >> It's already in my tree and pushed out to kernel.org, what in the
> >> world do you think "applied" means?
> >> 
> > I'm sorry about that. I try to be careful with my patches, but sometimes 
> > mistakes slip through - I'm only human... I do my best, but I mess up 
> > sometimes and Ryan raised a good point that I had not considered when I 
> > prepared the patch - what would you have me do except reply to his mail as 
> > I did?
> 
> Send a patch to fix things back up.

Sure, I can do that. Please see below.

I'm sorry about thiss mess and the extra work it caused you - I'll be more 
careful in the future.


From: Jesper Juhl <jj@chaosbits.net>
Subject: [PATCH] Revert incorrect dead-code changes to prb_setup_retire_blk_timer

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 net/packet/af_packet.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index ab10e84..82a6f34 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -516,11 +516,13 @@ static void prb_init_blk_timer(struct packet_sock *po,
 
 static void prb_setup_retire_blk_timer(struct packet_sock *po, int tx_ring)
 {
+	struct tpacket_kbdq_core *pkc;
+
 	if (tx_ring)
 		BUG();
 
-	prb_init_blk_timer(po, &po->rx_ring.prb_bdqc,
-			   prb_retire_rx_blk_timer_expired);
+	pkc = tx_ring ? &po->tx_ring.prb_bdqc : &po->rx_ring.prb_bdqc;
+	prb_init_blk_timer(po, pkc, prb_retire_rx_blk_timer_expired);
 }
 
 static int prb_calc_retire_blk_tmo(struct packet_sock *po,
-- 
1.7.7.3

-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

^ permalink raw reply related

* Re: [RFC PATCH net-next] enable virtio_net to return bus_info in ethtool -i consistent with emulated NICs
From: Rick Jones @ 2011-11-15  0:06 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Rick Jones, netdev, Rusty Russell, Michael Tsirkin,
	virtualization
In-Reply-To: <1321309800.2827.22.camel@bwh-desktop>

On 11/14/2011 02:30 PM, Ben Hutchings wrote:
> On Mon, 2011-11-14 at 13:52 -0800, Rick Jones wrote:
>> From: Rick Jones<rick.jones2@hp.com>
>>
>> Add a new .bus_name to virtio_config_ops then modify virtio_net to
>> call through to it in an ethtool .get_drvinfo routine to report
>> bus_info in ethtool -i output which is consistent with other
>> emulated NICs and the output of lspci.
> [...]
>> diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
>> index 0dc30ff..3724d45 100644
>> --- a/drivers/lguest/lguest_device.c
>> +++ b/drivers/lguest/lguest_device.c
>> @@ -381,6 +381,11 @@ error:
>>   	return PTR_ERR(vqs[i]);
>>   }
>>
>> +static const char *lg_bus_name(struct virtio_device *vdev)
>> +{
>> +	return "Not Implemented";
>> +}
> [...]
>> +static const char *kvm_bus_name(struct virtio_device *vdev)
>> +{
>> +	return "Not Implemented";
>> +}
> [...]
>
> Please use the existing 'not implemented' value, which is the empty
> string.   If you think ethtool should print some helpful message instead
> of an empty string, please submit a patch for ethtool.


One question - will those actually be called via an ethtool path?  In my 
poking about through the virtio code, I got the impression those modules 
were for "other than networking" sorts of things.

rick

^ permalink raw reply

* Re: [PATCH] net/packet: remove dead code and unneeded variable from prb_setup_retire_blk_timer()
From: Ryan Mallon @ 2011-11-15  0:00 UTC (permalink / raw)
  To: Jesper Juhl
  Cc: David S. Miller, netdev, linux-kernel, Eric Dumazet, Changli Gao,
	Ben Greear, Chetan Loke, waltje, gw4pts, waltje, ross.biro, alan
In-Reply-To: <alpine.LNX.2.00.1111150046350.9919@swampdragon.chaosbits.net>

On 15/11/11 10:48, Jesper Juhl wrote:
> On Tue, 15 Nov 2011, Ryan Mallon wrote:
>
>> On 15/11/11 10:37, Jesper Juhl wrote:
>>
>>> On Mon, 14 Nov 2011, Ryan Mallon wrote:
>>>
>>>> On 14/11/11 08:55, Jesper Juhl wrote:
>>>>
>>>>> We test for 'tx_ring' being != zero and BUG() if that's the case. So after
>>>>> that check there is no way that 'tx_ring' could be anything _but_ zero, so
>>>>> testing it again is just dead code. Once that dead code is removed, the
>>>>> 'pkc' local variable becomes entirely redundant, so remove that as well.
>>>>
>>>> What if CONFIG_BUG=n?
>>>>
>>> Arrgh, I didn't consider that (should have, but didn't).. In that case 
>>> we'll have 
>>> #define BUG() do {} while(0)
>>> which is not going to work all that peachy with my change...
>>>
>>> David: You may want to pass on this one. I obviously didn't think it 
>>> through properly - sorry :-(
>>>
>>
>> It's something I've never been entirely clear on. How are you meant to
>> handle something which really is a fatal bug if CONFIG_BUG=n?
>>
>> I think there are several places in the kernel where the expectation is
>> that BUG() causes a panic that probably don't behave nicely at all
>> (though I guess that might be the expected behaviour) if CONFIG_BUG=n.
>>
> You have a point. Perhaps the proper solution would be to remove 
> CONFIG_BUG so that it is always enabled...?  As you say, if something is a 
> fatal bug there is no sane way to continue, so why do we even allow 
> people/systems to try???
> That way lies madness it would seem...
>
The Kconfig looks like this:

config BUG
        bool "BUG() support" if EXPERT
        default y
        help
          Disabling this option eliminates support for BUG and WARN, reducing
          the size of your kernel image and potentially quietly ignoring
          numerous fatal conditions. You should only consider disabling this
          option for embedded systems with no facilities for reporting errors.
          Just say Y.


So you can only disable it if you have CONFIG_EXPERT (read embedded I
think). It says that you should only disable it for embedded systems
where you have no error reporting facilities, but you probably still
want the system to hang/die though since you might have a watchdog which
can reset the machine if it hangs. With CONFIG_BUG=n you could
potentially hit a BUG() and manage to carry on for sometime even though
the system is now unstable. Maybe BUG() should at least drop in to an
infinite loop in the case of CONFIG_BUG=n. I might try and come up with
a patch later tonight.

~Ryan

^ permalink raw reply

* Re: [PATCH] net/packet: remove dead code and unneeded variable from prb_setup_retire_blk_timer()
From: Jesper Juhl @ 2011-11-14 23:51 UTC (permalink / raw)
  To: David Miller
  Cc: rmallon, netdev, linux-kernel, eric.dumazet, xiaosuo, greearb,
	loke.chetan, waltje, gw4pts, waltje, ross.biro, alan
In-Reply-To: <20111114.184325.1962580278359413683.davem@davemloft.net>

On Mon, 14 Nov 2011, David Miller wrote:

> From: Jesper Juhl <jj@chaosbits.net>
> Date: Tue, 15 Nov 2011 00:37:33 +0100 (CET)
> 
> > David: You may want to pass on this one. I obviously didn't think it 
> > through properly - sorry :-(
> 
> It's already in my tree and pushed out to kernel.org, what in the
> world do you think "applied" means?
> 
I'm sorry about that. I try to be careful with my patches, but sometimes 
mistakes slip through - I'm only human... I do my best, but I mess up 
sometimes and Ryan raised a good point that I had not considered when I 
prepared the patch - what would you have me do except reply to his mail as 
I did?

-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

^ permalink raw reply

* Re: [PATCH] net/packet: remove dead code and unneeded variable from prb_setup_retire_blk_timer()
From: Jesper Juhl @ 2011-11-14 23:48 UTC (permalink / raw)
  To: Ryan Mallon
  Cc: David S. Miller, netdev, linux-kernel, Eric Dumazet, Changli Gao,
	Ben Greear, Chetan Loke, waltje, gw4pts, waltje, ross.biro, alan
In-Reply-To: <4EC1A65E.5040607@gmail.com>

On Tue, 15 Nov 2011, Ryan Mallon wrote:

> On 15/11/11 10:37, Jesper Juhl wrote:
> 
> > On Mon, 14 Nov 2011, Ryan Mallon wrote:
> > 
> >> On 14/11/11 08:55, Jesper Juhl wrote:
> >>
> >>> We test for 'tx_ring' being != zero and BUG() if that's the case. So after
> >>> that check there is no way that 'tx_ring' could be anything _but_ zero, so
> >>> testing it again is just dead code. Once that dead code is removed, the
> >>> 'pkc' local variable becomes entirely redundant, so remove that as well.
> >>
> >>
> >> What if CONFIG_BUG=n?
> >>
> > 
> > Arrgh, I didn't consider that (should have, but didn't).. In that case 
> > we'll have 
> > #define BUG() do {} while(0)
> > which is not going to work all that peachy with my change...
> > 
> > David: You may want to pass on this one. I obviously didn't think it 
> > through properly - sorry :-(
> > 
> 
> 
> It's something I've never been entirely clear on. How are you meant to
> handle something which really is a fatal bug if CONFIG_BUG=n?
> 
> I think there are several places in the kernel where the expectation is
> that BUG() causes a panic that probably don't behave nicely at all
> (though I guess that might be the expected behaviour) if CONFIG_BUG=n.
> 
You have a point. Perhaps the proper solution would be to remove 
CONFIG_BUG so that it is always enabled...?  As you say, if something is a 
fatal bug there is no sane way to continue, so why do we even allow 
people/systems to try???
That way lies madness it would seem...


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

^ permalink raw reply

* Re: [PATCH] net/packet: remove dead code and unneeded variable from prb_setup_retire_blk_timer()
From: David Miller @ 2011-11-14 23:47 UTC (permalink / raw)
  To: jj
  Cc: rmallon, netdev, linux-kernel, eric.dumazet, xiaosuo, greearb,
	loke.chetan, waltje, gw4pts, waltje, ross.biro, alan
In-Reply-To: <alpine.LNX.2.00.1111150049500.9919@swampdragon.chaosbits.net>

From: Jesper Juhl <jj@chaosbits.net>
Date: Tue, 15 Nov 2011 00:51:57 +0100 (CET)

> On Mon, 14 Nov 2011, David Miller wrote:
> 
>> From: Jesper Juhl <jj@chaosbits.net>
>> Date: Tue, 15 Nov 2011 00:37:33 +0100 (CET)
>> 
>> > David: You may want to pass on this one. I obviously didn't think it 
>> > through properly - sorry :-(
>> 
>> It's already in my tree and pushed out to kernel.org, what in the
>> world do you think "applied" means?
>> 
> I'm sorry about that. I try to be careful with my patches, but sometimes 
> mistakes slip through - I'm only human... I do my best, but I mess up 
> sometimes and Ryan raised a good point that I had not considered when I 
> prepared the patch - what would you have me do except reply to his mail as 
> I did?

Send a patch to fix things back up.

^ permalink raw reply

* Re: [PATCH] net/packet: remove dead code and unneeded variable from prb_setup_retire_blk_timer()
From: David Miller @ 2011-11-14 23:43 UTC (permalink / raw)
  To: jj
  Cc: rmallon, netdev, linux-kernel, eric.dumazet, xiaosuo, greearb,
	loke.chetan, waltje, gw4pts, waltje, ross.biro, alan
In-Reply-To: <alpine.LNX.2.00.1111150034120.9919@swampdragon.chaosbits.net>

From: Jesper Juhl <jj@chaosbits.net>
Date: Tue, 15 Nov 2011 00:37:33 +0100 (CET)

> David: You may want to pass on this one. I obviously didn't think it 
> through properly - sorry :-(

It's already in my tree and pushed out to kernel.org, what in the
world do you think "applied" means?

^ permalink raw reply

* Re: [PATCH 5/5] net-next:asix: update VERSION and white space changes
From: David Miller @ 2011-11-14 23:42 UTC (permalink / raw)
  To: grundler; +Cc: netdev, linux-kernel, allan, freddy
In-Reply-To: <1321312921-7748-5-git-send-email-grundler@chromium.org>

From: Grant Grundler <grundler@chromium.org>
Date: Mon, 14 Nov 2011 15:22:01 -0800

> No functional changes. Three things targeted:
> o update VERSION to reflect previous changes.
> o fix up white space/formatting of some lines
> o define 150 to be AX_SWRESET_MDELAY (self documenting code)
> 
> Signed-off-by: Grant Grundler <grundler@chromium.org>

Why have such a wonderful set of real bug fixes that I can apply right
now and send off to Linus's tree, only to sabotage it with whitespace
changes at the end which are no appropriate?

Please, just submit this without the whitespace and other cleanup
stuff.  Restrict it to real bug fixes, and I'll push it around.

The whitespace and other bits can be submitted for net-next afterwards.

^ permalink raw reply

* Re: [PATCH] net/packet: remove dead code and unneeded variable from prb_setup_retire_blk_timer()
From: Ryan Mallon @ 2011-11-14 23:38 UTC (permalink / raw)
  To: Jesper Juhl
  Cc: David S. Miller, netdev, linux-kernel, Eric Dumazet, Changli Gao,
	Ben Greear, Chetan Loke, waltje, gw4pts, waltje, ross.biro, alan
In-Reply-To: <alpine.LNX.2.00.1111150034120.9919@swampdragon.chaosbits.net>

On 15/11/11 10:37, Jesper Juhl wrote:

> On Mon, 14 Nov 2011, Ryan Mallon wrote:
> 
>> On 14/11/11 08:55, Jesper Juhl wrote:
>>
>>> We test for 'tx_ring' being != zero and BUG() if that's the case. So after
>>> that check there is no way that 'tx_ring' could be anything _but_ zero, so
>>> testing it again is just dead code. Once that dead code is removed, the
>>> 'pkc' local variable becomes entirely redundant, so remove that as well.
>>
>>
>> What if CONFIG_BUG=n?
>>
> 
> Arrgh, I didn't consider that (should have, but didn't).. In that case 
> we'll have 
> #define BUG() do {} while(0)
> which is not going to work all that peachy with my change...
> 
> David: You may want to pass on this one. I obviously didn't think it 
> through properly - sorry :-(
> 


It's something I've never been entirely clear on. How are you meant to
handle something which really is a fatal bug if CONFIG_BUG=n?

I think there are several places in the kernel where the expectation is
that BUG() causes a panic that probably don't behave nicely at all
(though I guess that might be the expected behaviour) if CONFIG_BUG=n.

~Ryan

^ permalink raw reply

* Re: [PATCH] net/packet: remove dead code and unneeded variable from prb_setup_retire_blk_timer()
From: Jesper Juhl @ 2011-11-14 23:37 UTC (permalink / raw)
  To: Ryan Mallon, David S. Miller
  Cc: netdev, linux-kernel, Eric Dumazet, Changli Gao, Ben Greear,
	Chetan Loke, waltje, gw4pts, waltje, ross.biro, alan
In-Reply-To: <4EC0AA31.9040403@gmail.com>

On Mon, 14 Nov 2011, Ryan Mallon wrote:

> On 14/11/11 08:55, Jesper Juhl wrote:
> 
> > We test for 'tx_ring' being != zero and BUG() if that's the case. So after
> > that check there is no way that 'tx_ring' could be anything _but_ zero, so
> > testing it again is just dead code. Once that dead code is removed, the
> > 'pkc' local variable becomes entirely redundant, so remove that as well.
> 
> 
> What if CONFIG_BUG=n?
> 

Arrgh, I didn't consider that (should have, but didn't).. In that case 
we'll have 
#define BUG() do {} while(0)
which is not going to work all that peachy with my change...

David: You may want to pass on this one. I obviously didn't think it 
through properly - sorry :-(

-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

^ permalink raw reply

* [PATCH 5/5] net-next:asix: update VERSION and white space changes
From: Grant Grundler @ 2011-11-14 23:22 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-kernel, Allan Chou, Freddy Xin, Grant Grundler
In-Reply-To: <1321312921-7748-1-git-send-email-grundler@chromium.org>

No functional changes. Three things targeted:
o update VERSION to reflect previous changes.
o fix up white space/formatting of some lines
o define 150 to be AX_SWRESET_MDELAY (self documenting code)

Signed-off-by: Grant Grundler <grundler@chromium.org>
---
 drivers/net/usb/asix.c |   46 +++++++++++++++++++++-------------------------
 1 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/drivers/net/usb/asix.c b/drivers/net/usb/asix.c
index c07dd26..29598dc 100644
--- a/drivers/net/usb/asix.c
+++ b/drivers/net/usb/asix.c
@@ -36,7 +36,7 @@
 #include <linux/usb/usbnet.h>
 #include <linux/slab.h>
 
-#define DRIVER_VERSION "26-Sep-2011"
+#define DRIVER_VERSION "08-Nov-2011"
 #define DRIVER_NAME "asix"
 
 /* ASIX AX8817X based USB 2.0 Ethernet Devices */
@@ -94,6 +94,8 @@
 #define AX_SWRESET_IPRL			0x20
 #define AX_SWRESET_IPPD			0x40
 
+#define AX_SWRESET_MDELAY	150
+
 #define AX88772_IPG0_DEFAULT		0x15
 #define AX88772_IPG1_DEFAULT		0x0c
 #define AX88772_IPG2_DEFAULT		0x12
@@ -494,9 +496,9 @@ static int asix_sw_reset(struct usbnet *dev, u8 flags)
 {
 	int ret;
 
-        ret = asix_write_cmd(dev, AX_CMD_SW_RESET, flags, 0, 0, NULL);
+	ret = asix_write_cmd(dev, AX_CMD_SW_RESET, flags, 0, 0, NULL);
 	if (ret < 0)
-		netdev_err(dev->net, "Failed to send software reset: %02x\n", ret);
+		netdev_err(dev->net, "Failed to send SW_RESET: %02x\n", ret);
 
 	return ret;
 }
@@ -540,7 +542,6 @@ static u16 asix_read_medium_status(struct usbnet *dev)
 	}
 
 	return le16_to_cpu(v);
-
 }
 
 static int asix_write_medium_mode(struct usbnet *dev, u16 mode)
@@ -792,9 +793,9 @@ static int asix_set_mac_address(struct net_device *net, void *p)
 	return 0;
 }
 
-/* We need to override some ethtool_ops so we require our
-   own structure so we don't interfere with other usbnet
-   devices that may be connected at the same time. */
+/* We override some ethtool_ops to access per device data.
+ * Multiple usbnet devices may be connected at the same time.
+ */
 static const struct ethtool_ops ax88172_ethtool_ops = {
 	.get_drvinfo		= asix_get_drvinfo,
 	.get_link		= asix_get_link,
@@ -994,25 +995,20 @@ static int ax88772_reset(struct usbnet *dev)
 	if (ret < 0)
 		goto out;
 
-	msleep(150);
+	msleep(AX_SWRESET_MDELAY);
 
 	ret = asix_sw_reset(dev, AX_SWRESET_CLEAR);
 	if (ret < 0)
 		goto out;
 
-	msleep(150);
+	msleep(AX_SWRESET_MDELAY);
 
-	if (embd_phy) {
-		ret = asix_sw_reset(dev, AX_SWRESET_IPRL);
-		if (ret < 0)
-			goto out;
-	} else {
-		ret = asix_sw_reset(dev, AX_SWRESET_PRTE);
-		if (ret < 0)
-			goto out;
-	}
+	ret = asix_sw_reset(dev, embd_phy ? AX_SWRESET_IPRL : AX_SWRESET_PRTE);
+	if (ret < 0)
+		goto out;
+
+	msleep(AX_SWRESET_MDELAY);
 
-	msleep(150);
 	rx_ctl = asix_read_rx_ctl(dev);
 	dbg("RX_CTL is 0x%04x after software reset", rx_ctl);
 	ret = asix_write_rx_ctl(dev, 0x0000);
@@ -1026,13 +1022,13 @@ static int ax88772_reset(struct usbnet *dev)
 	if (ret < 0)
 		goto out;
 
-	msleep(150);
+	msleep(AX_SWRESET_MDELAY);
 
 	ret = asix_sw_reset(dev, AX_SWRESET_IPRL | AX_SWRESET_PRL);
 	if (ret < 0)
 		goto out;
 
-	msleep(150);
+	msleep(AX_SWRESET_MDELAY);
 
 	asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR, BMCR_RESET);
 	asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
@@ -1290,10 +1286,10 @@ static int ax88178_reset(struct usbnet *dev)
 	asix_write_cmd(dev, AX_CMD_SW_PHY_SELECT, 0, 0, 0, NULL);
 
 	asix_sw_reset(dev, 0);
-	msleep(150);
+	msleep(AX_SWRESET_MDELAY);
 
 	asix_sw_reset(dev, AX_SWRESET_PRL | AX_SWRESET_IPPD);
-	msleep(150);
+	msleep(AX_SWRESET_MDELAY);
 
 	asix_write_rx_ctl(dev, 0);
 
@@ -1672,13 +1668,13 @@ static struct usb_driver asix_driver = {
 
 static int __init asix_init(void)
 {
- 	return usb_register(&asix_driver);
+	return usb_register(&asix_driver);
 }
 module_init(asix_init);
 
 static void __exit asix_exit(void)
 {
- 	usb_deregister(&asix_driver);
+	usb_deregister(&asix_driver);
 }
 module_exit(asix_exit);
 
-- 
1.7.3.1

^ permalink raw reply related

* [PATCH 4/5] net-next:asix: more fixes for ax88178 phy init sequence
From: Grant Grundler @ 2011-11-14 23:22 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-kernel, Allan Chou, Freddy Xin, Grant Grundler
In-Reply-To: <1321312921-7748-1-git-send-email-grundler@chromium.org>

Now works on Samsung Series 5 (chromebook)

Two fixes here:
o use 0x7F mask for phymode
o read phyid *AFTER* phy is powered up (via GPIOs)

Signed-off-by: Allan Chou <allan@asix.com.tw>
Signed-off-by: Grant Grundler <grundler@chromium.org>
---
 drivers/net/usb/asix.c |   22 +++++++++++++++-------
 1 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/asix.c b/drivers/net/usb/asix.c
index 8462be5..c07dd26 100644
--- a/drivers/net/usb/asix.c
+++ b/drivers/net/usb/asix.c
@@ -1248,6 +1248,7 @@ static int ax88178_reset(struct usbnet *dev)
 	__le16 eeprom;
 	u8 status;
 	int gpio0 = 0;
+	u32 phyid;
 
 	asix_read_cmd(dev, AX_CMD_READ_GPIOS, 0, 0, 1, &status);
 	dbg("GPIO Status: 0x%04x", status);
@@ -1263,12 +1264,13 @@ static int ax88178_reset(struct usbnet *dev)
 		data->ledmode = 0;
 		gpio0 = 1;
 	} else {
-		data->phymode = le16_to_cpu(eeprom) & 7;
+		data->phymode = le16_to_cpu(eeprom) & 0x7F;
 		data->ledmode = le16_to_cpu(eeprom) >> 8;
 		gpio0 = (le16_to_cpu(eeprom) & 0x80) ? 0 : 1;
 	}
 	dbg("GPIO0: %d, PhyMode: %d", gpio0, data->phymode);
 
+	/* Power up external GigaPHY through AX88178 GPIO pin */
 	asix_write_gpio(dev, AX_GPIO_RSE | AX_GPIO_GPO_1 | AX_GPIO_GPO1EN, 40);
 	if ((le16_to_cpu(eeprom) >> 8) != 1) {
 		asix_write_gpio(dev, 0x003c, 30);
@@ -1280,6 +1282,13 @@ static int ax88178_reset(struct usbnet *dev)
 		asix_write_gpio(dev, AX_GPIO_GPO1EN | AX_GPIO_GPO_1, 30);
 	}
 
+	/* Read PHYID register *AFTER* powering up PHY */
+	phyid = asix_get_phyid(dev);
+	dbg("PHYID=0x%08x", phyid);
+
+	/* Set AX88178 to enable MII/GMII/RGMII interface for external PHY */
+	asix_write_cmd(dev, AX_CMD_SW_PHY_SELECT, 0, 0, 0, NULL);
+
 	asix_sw_reset(dev, 0);
 	msleep(150);
 
@@ -1424,7 +1433,6 @@ static int ax88178_bind(struct usbnet *dev, struct usb_interface *intf)
 {
 	int ret;
 	u8 buf[ETH_ALEN];
-	u32 phyid;
 	struct asix_data *data = (struct asix_data *)&dev->data;
 
 	data->eeprom_len = AX88772_EEPROM_LEN;
@@ -1451,12 +1459,12 @@ static int ax88178_bind(struct usbnet *dev, struct usb_interface *intf)
 	dev->net->netdev_ops = &ax88178_netdev_ops;
 	dev->net->ethtool_ops = &ax88178_ethtool_ops;
 
-	phyid = asix_get_phyid(dev);
-	dbg("PHYID=0x%08x", phyid);
+	/* Blink LEDS so users know driver saw dongle */
+	ax88178_sw_reset(dev, 0);
+	msleep(150);
 
-	ret = ax88178_reset(dev);
-	if (ret < 0)
-		return ret;
+	asix_sw_reset(dev, AX_SWRESET_PRL | AX_SWRESET_IPPD);
+	msleep(150);
 
 	/* Asix framing packs multiple eth frames into a 2K usb bulk transfer */
 	if (dev->driver_info->flags & FLAG_FRAMING_AX) {
-- 
1.7.3.1

^ permalink raw reply related

* [PATCH 3/5] net-next:asix: reduce AX88772 init time by about 2 seconds
From: Grant Grundler @ 2011-11-14 23:21 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-kernel, Allan Chou, Freddy Xin, Grant Grundler
In-Reply-To: <1321312921-7748-1-git-send-email-grundler@chromium.org>

ax88772_reset takes about 2 seconds and is called twice.
Once from ax88772_bind() directly and again indirectly from usbnet_open().
Reset the USB FW/Phy enough to blink the LEDs when inserted.

Signed-off-by: Allan Chou <allan@asix.com.tw>
Signed-off-by: Grant Grundler <grundler@chromium.org>
---
 drivers/net/usb/asix.c |   30 +++++++++++++++++++++++++-----
 1 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/net/usb/asix.c b/drivers/net/usb/asix.c
index b4675e8..8462be5 100644
--- a/drivers/net/usb/asix.c
+++ b/drivers/net/usb/asix.c
@@ -1083,7 +1083,7 @@ static const struct net_device_ops ax88772_netdev_ops = {
 
 static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 {
-	int ret;
+	int ret, embd_phy;
 	struct asix_data *data = (struct asix_data *)&dev->data;
 	u8 buf[ETH_ALEN];
 	u32 phyid;
@@ -1108,16 +1108,36 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 	dev->mii.reg_num_mask = 0x1f;
 	dev->mii.phy_id = asix_get_phy_addr(dev);
 
-	phyid = asix_get_phyid(dev);
-	dbg("PHYID=0x%08x", phyid);
-
 	dev->net->netdev_ops = &ax88772_netdev_ops;
 	dev->net->ethtool_ops = &ax88772_ethtool_ops;
 
-	ret = ax88772_reset(dev);
+	embd_phy = ((dev->mii.phy_id & 0x1f) == 0x10 ? 1 : 0);
+
+	/* Reset the PHY to normal operation mode */
+	ret = asix_write_cmd(dev, AX_CMD_SW_PHY_SELECT, embd_phy, 0, 0, NULL);
+	if (ret < 0) {
+		dbg("Select PHY #1 failed: %d", ret);
+		return ret;
+	}
+
+	ret = asix_sw_reset(dev, AX_SWRESET_IPPD | AX_SWRESET_PRL);
 	if (ret < 0)
 		return ret;
 
+	msleep(150);
+
+	ret = asix_sw_reset(dev, AX_SWRESET_CLEAR);
+	if (ret < 0)
+		return ret;
+
+	msleep(150);
+
+	ret = asix_sw_reset(dev, embd_phy ? AX_SWRESET_IPRL : AX_SWRESET_PRTE);
+
+	/* Read PHYID register *AFTER* the PHY was reset properly */
+	phyid = asix_get_phyid(dev);
+	dbg("PHYID=0x%08x", phyid);
+
 	/* Asix framing packs multiple eth frames into a 2K usb bulk transfer */
 	if (dev->driver_info->flags & FLAG_FRAMING_AX) {
 		/* hard_mtu  is still the default - the device does not support
-- 
1.7.3.1

^ permalink raw reply related

* [PATCH 2/5] net-next:asix: poll in asix_get_phyid in case phy not ready
From: Grant Grundler @ 2011-11-14 23:21 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-kernel, Allan Chou, Freddy Xin, Grant Grundler
In-Reply-To: <1321312921-7748-1-git-send-email-grundler@chromium.org>

Sometimes the phy isn't ready after reset...poll and pray it will be soon.

Signed-off-by: Freddy Xin <freddy@asix.com.tw>
Signed-off-by: Grant Grundler <grundler@chromium.org>
---
 drivers/net/usb/asix.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/asix.c b/drivers/net/usb/asix.c
index 873860d..b4675e8 100644
--- a/drivers/net/usb/asix.c
+++ b/drivers/net/usb/asix.c
@@ -652,9 +652,17 @@ static u32 asix_get_phyid(struct usbnet *dev)
 {
 	int phy_reg;
 	u32 phy_id;
+	int i;
 
-	phy_reg = asix_mdio_read(dev->net, dev->mii.phy_id, MII_PHYSID1);
-	if (phy_reg < 0)
+	/* Poll for the rare case the FW or phy isn't ready yet.  */
+	for (i = 0; i < 100; i++) {
+		phy_reg = asix_mdio_read(dev->net, dev->mii.phy_id, MII_PHYSID1);
+		if (phy_reg != 0 && phy_reg != 0xFFFF)
+			break;
+		mdelay(1);
+	}
+
+	if (phy_reg <= 0 || phy_reg == 0xFFFF)
 		return 0;
 
 	phy_id = (phy_reg & 0xffff) << 16;
-- 
1.7.3.1

^ permalink raw reply related

* [PATCH 1/5] net-next:asix: PHY_MODE_RTL8211CL should be 0xC
From: Grant Grundler @ 2011-11-14 23:21 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-kernel, Allan Chou, Freddy Xin, Grant Grundler

Use correct value for rtl phy support.
(rtl phy are in AX88178 devices like NWU220G and USB2-ET1000).

Signed-off-by: Allan Chou <allan@asix.com.tw>
Tested-by: Grant Grundler <grundler@chromium.org>
---
 drivers/net/usb/asix.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/usb/asix.c b/drivers/net/usb/asix.c
index e81e22e..873860d 100644
--- a/drivers/net/usb/asix.c
+++ b/drivers/net/usb/asix.c
@@ -163,7 +163,7 @@
 #define MARVELL_CTRL_TXDELAY	0x0002
 #define MARVELL_CTRL_RXDELAY	0x0080
 
-#define	PHY_MODE_RTL8211CL	0x0004
+#define	PHY_MODE_RTL8211CL	0x000C
 
 /* This structure cannot exceed sizeof(unsigned long [5]) AKA 20 bytes */
 struct asix_data {
-- 
1.7.3.1

^ permalink raw reply related

* Re: [PATCH] tcp: fixes for DSACK-based undo of cwnd reduction during fast recovery
From: Neal Cardwell @ 2011-11-14 23:03 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, ilpo.jarvinen, nanditad, ycheng, therbert
In-Reply-To: <20111114.152359.1131974895743731960.davem@davemloft.net>

On Mon, Nov 14, 2011 at 3:23 PM, David Miller <davem@davemloft.net> wrote:
> From: Neal Cardwell <ncardwell@google.com>
> Date: Tue,  8 Nov 2011 13:07:11 -0500
>
>> (2) When the ACK field is below snd_una (the "old_ack" goto label),
>> process any DSACKs and try to send out more packets based on
>> newly-SACKed packets.
>
> This seems dubious.
>
> An older ACK should not have more uptodate SACK information than a newer
> ACK.
>

I grant that it is a rare corner case, but it is possible if there is
reordering and packet loss in both directions.

In fact, AFAICT the code path to handle this corner case is already
there: AFAICT we call tcp_sacktag_write_queue() in the old_ack code
path exactly because it is possible for old ACKs to carry SACK ranges
that we haven't seen yet. If this is not possible then it seems we
should remove the old_ack code path.

Let me try to lay out a specific example:

Suppose a sender sends packets 1-12, and there is lots of loss and
reordering in both directions.

Suppose the receiver sends the following ACKs with SACK blocks, in the
following order (using packet numbers rather than sequence numbers):

(1) ack 1, sack <3-4>
(2) ack 1, sack <3-4 6>
(3) ack 1, sack <3-4 6 8>
(4) ack 1, sack <3-4 6 8 10>
(5) ack 1, sack <6 8 10 12>
(6) ack 2, sack <6 8 10 12>

Note that starting at ACK (5) the receiver is limited by the fact that
TCP options can only hold 4 SACK ranges, so the SACK for 3-4 "falls
off" the SACK block set.

Now suppose the ACKs are reordered, and some are dropped, so that the
sender receives the ACKs in this order:

(6) ack 2, sack <6 8 10 12>
(1) ack 1, sack <3-4>

The arrival of (6) advances snd_una to 2. Now the question is what to
do when (1), with an ACK field below snd_una, arrives at the
sender. The existing code tags packets 3-4 as SACKed, but does not use
this newly SACK-tagged data to send out new data. This commit proposes
to use the newly SACK-tagged 3-4 to send out new data.

neal

^ permalink raw reply

* Re: [PATCH] Net, libertas: Resolve memory leak in if_spi_host_to_card()
From: Dan Williams @ 2011-11-14 22:43 UTC (permalink / raw)
  To: Jesper Juhl
  Cc: libertas-dev, linux-wireless, netdev, linux-kernel,
	David S. Miller, Andrey Yurovsky, Colin McCabe, John W. Linville
In-Reply-To: <alpine.LNX.2.00.1111132211580.3368@swampdragon.chaosbits.net>

On Sun, 2011-11-13 at 22:14 +0100, Jesper Juhl wrote:
> If we hit the default case in the switch in if_spi_host_to_card() we'll leak
> the memory we allocated for 'packet'. This patch resolves the leak by freeing
> the allocated memory in that case.
> 
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>

Acked-by: Dan Williams <dcbw@redhat.com>

> ---
>  drivers/net/wireless/libertas/if_spi.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
>  Compile tested only due to lack of hardware.
> 
> diff --git a/drivers/net/wireless/libertas/if_spi.c b/drivers/net/wireless/libertas/if_spi.c
> index 11b69b3..728baa4 100644
> --- a/drivers/net/wireless/libertas/if_spi.c
> +++ b/drivers/net/wireless/libertas/if_spi.c
> @@ -995,6 +995,7 @@ static int if_spi_host_to_card(struct lbs_private *priv,
>  		spin_unlock_irqrestore(&card->buffer_lock, flags);
>  		break;
>  	default:
> +		kfree(packet);
>  		netdev_err(priv->dev, "can't transfer buffer of type %d\n",
>  			   type);
>  		err = -EINVAL;
> -- 
> 1.7.7.3
> 
> 

^ permalink raw reply

* Re: [RFC PATCH net-next] enable virtio_net to return bus_info in ethtool -i consistent with emulated NICs
From: Rick Jones @ 2011-11-14 22:40 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Rick Jones, netdev, Rusty Russell, Michael Tsirkin,
	virtualization
In-Reply-To: <1321309800.2827.22.camel@bwh-desktop>

On 11/14/2011 02:30 PM, Ben Hutchings wrote:
> On Mon, 2011-11-14 at 13:52 -0800, Rick Jones wrote:
>> From: Rick Jones<rick.jones2@hp.com>
>>
>> Add a new .bus_name to virtio_config_ops then modify virtio_net to
>> call through to it in an ethtool .get_drvinfo routine to report
>> bus_info in ethtool -i output which is consistent with other
>> emulated NICs and the output of lspci.
> [...]
>> diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
>> index 0dc30ff..3724d45 100644
>> --- a/drivers/lguest/lguest_device.c
>> +++ b/drivers/lguest/lguest_device.c
>> @@ -381,6 +381,11 @@ error:
>>   	return PTR_ERR(vqs[i]);
>>   }
>>
>> +static const char *lg_bus_name(struct virtio_device *vdev)
>> +{
>> +	return "Not Implemented";
>> +}
> [...]
>> +static const char *kvm_bus_name(struct virtio_device *vdev)
>> +{
>> +	return "Not Implemented";
>> +}
> [...]
>
> Please use the existing 'not implemented' value, which is the empty
> string.

Will do.

thanks,

rick

^ permalink raw reply

* Re: [RFC PATCH net-next] enable virtio_net to return bus_info in ethtool -i consistent with emulated NICs
From: Ben Hutchings @ 2011-11-14 22:30 UTC (permalink / raw)
  To: Rick Jones; +Cc: netdev, Rusty Russell, Michael Tsirkin, virtualization
In-Reply-To: <20111114215241.5B8BF2900307@tardy>

On Mon, 2011-11-14 at 13:52 -0800, Rick Jones wrote:
> From: Rick Jones <rick.jones2@hp.com>
> 
> Add a new .bus_name to virtio_config_ops then modify virtio_net to 
> call through to it in an ethtool .get_drvinfo routine to report
> bus_info in ethtool -i output which is consistent with other
> emulated NICs and the output of lspci.  
[...]
> diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
> index 0dc30ff..3724d45 100644
> --- a/drivers/lguest/lguest_device.c
> +++ b/drivers/lguest/lguest_device.c
> @@ -381,6 +381,11 @@ error:
>  	return PTR_ERR(vqs[i]);
>  }
>  
> +static const char *lg_bus_name(struct virtio_device *vdev)
> +{
> +	return "Not Implemented";
> +}
[...]
> +static const char *kvm_bus_name(struct virtio_device *vdev)
> +{
> +	return "Not Implemented";
> +}
[...]

Please use the existing 'not implemented' value, which is the empty
string.   If you think ethtool should print some helpful message instead
of an empty string, please submit a patch for ethtool.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox