Re: [PATCH 00/12] sfc changes for 2.6.36

Previous thread: [PATCH] IP: Increment INADDRERRORS if routing for a packet is not successful by Christoph Lameter on Tuesday, June 1, 2010 - 2:13 pm. (28 messages)

Next thread: [PATCH net-next-2.6 1/2] qlcnic: NIC Partitioning - Add basic infrastructure support by Anirban Chakraborty on Tuesday, June 1, 2010 - 2:28 pm. (2 messages)
From: Ben Hutchings
Date: Tuesday, June 1, 2010 - 2:16 pm

Here's a first series of changes for 2.6.36.

The major feature here is RX buffer recycling, which improves
performance on networks with heavy multicast traffic.  Other than that,
there are various bug fixes and cleanup.

Ben.

Ben Hutchings (3):
  sfc: Rename struct efx_mcdi_phy_cfg to efx_mcdi_phy_data
  sfc: Only count bad packets in rx_errors
  sfc: Get port number from CS_PORT_NUM, not PCI function number

Steve Hodgson (9):
  sfc: Reschedule any resets scheduled inside efx_pm_freeze()
  sfc: Workaround flush failures on Falcon B0
  sfc: Synchronise link_advertising and wanted_fc on Siena
  sfc: Wait for the link to stay up before running loopback selftest
  sfc: Allow DRV_GEN events to be used outside of selftests
  sfc: Remove efx_rx_queue::add_lock
  sfc: Support only two rx buffers per page
  sfc: Recycle discarded rx buffers back onto the queue
  sfc: Allow shared pages to be recycled

 drivers/net/sfc/efx.c         |   69 +++-----
 drivers/net/sfc/efx.h         |    4 +-
 drivers/net/sfc/falcon.c      |    8 +-
 drivers/net/sfc/mcdi_phy.c    |   21 ++-
 drivers/net/sfc/net_driver.h  |   46 +++--
 drivers/net/sfc/nic.c         |   55 +++++--
 drivers/net/sfc/nic.h         |    4 +-
 drivers/net/sfc/rx.c          |  393 +++++++++++++++++++----------------------
 drivers/net/sfc/selftest.c    |   26 ++--
 drivers/net/sfc/siena.c       |    4 +
 drivers/net/sfc/workarounds.h |    2 +-
 11 files changed, 317 insertions(+), 315 deletions(-)

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

--

From: Ben Hutchings
Date: Tuesday, June 1, 2010 - 2:17 pm

Most of its members are constant capabilities, not configuration.  The
new name is also consistent with the name of the pointer to it in
struct efx_nic and the names of structures used by other PHY drivers.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/sfc/mcdi_phy.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/sfc/mcdi_phy.c b/drivers/net/sfc/mcdi_phy.c
index 6032c0e..5c23f22 100644
--- a/drivers/net/sfc/mcdi_phy.c
+++ b/drivers/net/sfc/mcdi_phy.c
@@ -20,7 +20,7 @@
 #include "nic.h"
 #include "selftest.h"
 
-struct efx_mcdi_phy_cfg {
+struct efx_mcdi_phy_data {
 	u32 flags;
 	u32 type;
 	u32 supported_cap;
@@ -35,7 +35,7 @@ struct efx_mcdi_phy_cfg {
 };
 
 static int
-efx_mcdi_get_phy_cfg(struct efx_nic *efx, struct efx_mcdi_phy_cfg *cfg)
+efx_mcdi_get_phy_cfg(struct efx_nic *efx, struct efx_mcdi_phy_data *cfg)
 {
 	u8 outbuf[MC_CMD_GET_PHY_CFG_OUT_LEN];
 	size_t outlen;
@@ -259,7 +259,7 @@ static u32 ethtool_to_mcdi_cap(u32 cap)
 
 static u32 efx_get_mcdi_phy_flags(struct efx_nic *efx)
 {
-	struct efx_mcdi_phy_cfg *phy_cfg = efx->phy_data;
+	struct efx_mcdi_phy_data *phy_cfg = efx->phy_data;
 	enum efx_phy_mode mode, supported;
 	u32 flags;
 
@@ -307,7 +307,7 @@ static u32 mcdi_to_ethtool_media(u32 media)
 
 static int efx_mcdi_phy_probe(struct efx_nic *efx)
 {
-	struct efx_mcdi_phy_cfg *phy_data;
+	struct efx_mcdi_phy_data *phy_data;
 	u8 outbuf[MC_CMD_GET_LINK_OUT_LEN];
 	u32 caps;
 	int rc;
@@ -405,7 +405,7 @@ fail:
 
 int efx_mcdi_phy_reconfigure(struct efx_nic *efx)
 {
-	struct efx_mcdi_phy_cfg *phy_cfg = efx->phy_data;
+	struct efx_mcdi_phy_data *phy_cfg = efx->phy_data;
 	u32 caps = (efx->link_advertising ?
 		    ethtool_to_mcdi_cap(efx->link_advertising) :
 		    phy_cfg->forced_cap);
@@ -446,7 +446,7 @@ void efx_mcdi_phy_decode_link(struct efx_nic *efx,
  */
 void efx_mcdi_phy_check_fcntl(struct efx_nic *efx, u32 lpa)
 {
-	struct efx_mcdi_phy_cfg ...
From: Ben Hutchings
Date: Tuesday, June 1, 2010 - 2:17 pm

From: Steve Hodgson <shodgson@solarflare.com>

efx_pm_freeze() sets efx->state = STATE_FINI, which means
efx_reset_work() will abort any scheduled resets.

efx_pm_thaw() should reschedule efx_reset_work() again,
since a freeze/thaw will not have reset the hardware.

This bug was spotted by inspection - there is no real world example of
this happening.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/sfc/efx.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c
index 1564605..0319000 100644
--- a/drivers/net/sfc/efx.c
+++ b/drivers/net/sfc/efx.c
@@ -1886,6 +1886,9 @@ static void efx_reset_work(struct work_struct *data)
 {
 	struct efx_nic *efx = container_of(data, struct efx_nic, reset_work);
 
+	if (efx->reset_pending == RESET_TYPE_NONE)
+		return;
+
 	/* If we're not RUNNING then don't reset. Leave the reset_pending
 	 * flag set so that efx_pci_probe_main will be retried */
 	if (efx->state != STATE_RUNNING) {
@@ -2332,6 +2335,9 @@ static int efx_pm_thaw(struct device *dev)
 
 	efx->type->resume_wol(efx);
 
+	/* Reschedule any quenched resets scheduled during efx_pm_freeze() */
+	queue_work(reset_workqueue, &efx->reset_work);
+
 	return 0;
 }
 
-- 
1.6.2.5


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

--

From: Ben Hutchings
Date: Tuesday, June 1, 2010 - 2:17 pm

From: Steve Hodgson <shodgson@solarflare.com>

Under certain conditions a PHY may backpressure Falcon B0
in such a way that flushes timeout. In normal circumstances
the phy poller would fix the PHY, and the flush could complete.

But efx_nic_flush_queues() is always called after efx_stop_all(),
so the poller has been stopped. Even if this weren't the case,
how long would we have to wait for the poller to fix this? And
several callers of efx_nic_flush_queues() are about to reset
the device anyway - so we don't need to do anything.

Work around this bug by scheduling a reset. Ensure that the
MAC is never rewired back into the datapath before the reset
runs (we already ignore all rx events anyway).

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/sfc/efx.c         |   13 +++++++++++--
 drivers/net/sfc/falcon.c      |    8 +++++---
 drivers/net/sfc/nic.c         |    3 ---
 drivers/net/sfc/workarounds.h |    2 +-
 4 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c
index 0319000..d1a1d32 100644
--- a/drivers/net/sfc/efx.c
+++ b/drivers/net/sfc/efx.c
@@ -27,6 +27,7 @@
 #include "nic.h"
 
 #include "mcdi.h"
+#include "workarounds.h"
 
 /**************************************************************************
  *
@@ -556,10 +557,18 @@ static void efx_fini_channels(struct efx_nic *efx)
 	BUG_ON(efx->port_enabled);
 
 	rc = efx_nic_flush_queues(efx);
-	if (rc)
+	if (rc && EFX_WORKAROUND_7803(efx)) {
+		/* Schedule a reset to recover from the flush failure. The
+		 * descriptor caches reference memory we're about to free,
+		 * but falcon_reconfigure_mac_wrapper() won't reconnect
+		 * the MACs because of the pending reset. */
+		EFX_ERR(efx, "Resetting to recover from flush failure\n");
+		efx_schedule_reset(efx, RESET_TYPE_ALL);
+	} else if (rc) {
 		EFX_ERR(efx, "failed to flush queues\n");
-	else
+	} else {
 		EFX_LOG(efx, "successfully flushed all queues\n");
+	}
 
 ...
From: Ben Hutchings
Date: Tuesday, June 1, 2010 - 2:18 pm

From: Steve Hodgson <shodgson@solarflare.com>

All of the ethtool code paths keep them in sync, but we need
to ensure they are sync'd at start of day. Matches the sft9001
driver.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/sfc/mcdi_phy.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/net/sfc/mcdi_phy.c b/drivers/net/sfc/mcdi_phy.c
index 5c23f22..86e43b1 100644
--- a/drivers/net/sfc/mcdi_phy.c
+++ b/drivers/net/sfc/mcdi_phy.c
@@ -395,6 +395,7 @@ static int efx_mcdi_phy_probe(struct efx_nic *efx)
 	efx->wanted_fc = EFX_FC_RX | EFX_FC_TX;
 	if (phy_data->supported_cap & (1 << MC_CMD_PHY_CAP_AN_LBN))
 		efx->wanted_fc |= EFX_FC_AUTO;
+	efx_link_set_wanted_fc(efx, efx->wanted_fc);
 
 	return 0;
 
-- 
1.6.2.5


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

--

From: Ben Hutchings
Date: Tuesday, June 1, 2010 - 2:18 pm

From: Steve Hodgson <shodgson@solarflare.com>

It's been observed that some phys (such as the qt2025c) can
do down-up-down-up transitions, presumably as pcs block lock
settles down.

The loopback selftest will start sending data immediately
after the link comes up. Work around this by waiting for
the link state to stay up for two consecutive polls, rather
than one.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/sfc/selftest.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/sfc/selftest.c b/drivers/net/sfc/selftest.c
index 371e86c..52ac14a 100644
--- a/drivers/net/sfc/selftest.c
+++ b/drivers/net/sfc/selftest.c
@@ -545,7 +545,7 @@ efx_test_loopback(struct efx_tx_queue *tx_queue,
 static int efx_wait_for_link(struct efx_nic *efx)
 {
 	struct efx_link_state *link_state = &efx->link_state;
-	int count;
+	int count, link_up_count = 0;
 	bool link_up;
 
 	for (count = 0; count < 40; count++) {
@@ -567,8 +567,12 @@ static int efx_wait_for_link(struct efx_nic *efx)
 			link_up = !efx->mac_op->check_fault(efx);
 		mutex_unlock(&efx->mac_lock);
 
-		if (link_up)
-			return 0;
+		if (link_up) {
+			if (++link_up_count == 2)
+				return 0;
+		} else {
+			link_up_count = 0;
+		}
 	}
 
 	return -ETIMEDOUT;
-- 
1.6.2.5


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

--

From: Ben Hutchings
Date: Tuesday, June 1, 2010 - 2:19 pm

From: Steve Hodgson <shodgson@solarflare.com>

Formerly, efx_test_eventq_irq() assumed it was the only user of
driver generated events. Allow it to interoperate with other users.

We can create more than 16 channels, so align event codes with
a multiple of 256 not 16.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/sfc/net_driver.h |    4 ++--
 drivers/net/sfc/nic.c        |   17 ++++++++++-------
 drivers/net/sfc/nic.h        |    3 +--
 drivers/net/sfc/selftest.c   |   16 +++++-----------
 4 files changed, 18 insertions(+), 22 deletions(-)

diff --git a/drivers/net/sfc/net_driver.h b/drivers/net/sfc/net_driver.h
index 2e6fd89..ee0ea01 100644
--- a/drivers/net/sfc/net_driver.h
+++ b/drivers/net/sfc/net_driver.h
@@ -336,7 +336,7 @@ enum efx_rx_alloc_method {
  * @eventq: Event queue buffer
  * @eventq_read_ptr: Event queue read pointer
  * @last_eventq_read_ptr: Last event queue read pointer value.
- * @eventq_magic: Event queue magic value for driver-generated test events
+ * @magic_count: Event queue test event count
  * @irq_count: Number of IRQs since last adaptive moderation decision
  * @irq_mod_score: IRQ moderation score
  * @rx_alloc_level: Watermark based heuristic counter for pushing descriptors
@@ -367,7 +367,7 @@ struct efx_channel {
 	struct efx_special_buffer eventq;
 	unsigned int eventq_read_ptr;
 	unsigned int last_eventq_read_ptr;
-	unsigned int eventq_magic;
+	unsigned int magic_count;
 
 	unsigned int irq_count;
 	unsigned int irq_mod_score;
diff --git a/drivers/net/sfc/nic.c b/drivers/net/sfc/nic.c
index ec0bb80..ca9cf1a 100644
--- a/drivers/net/sfc/nic.c
+++ b/drivers/net/sfc/nic.c
@@ -79,6 +79,10 @@ MODULE_PARM_DESC(rx_xon_thresh_bytes, "RX fifo XON threshold");
 /* Depth of RX flush request fifo */
 #define EFX_RX_FLUSH_COUNT 4
 
+/* Magic value for efx_generate_test_event() */
+#define EFX_CHANNEL_MAGIC(_channel)			\
+	(0x00010100 + (_channel)->channel)
+
 ...
From: Ben Hutchings
Date: Tuesday, June 1, 2010 - 2:19 pm

From: Steve Hodgson <shodgson@solarflare.com>

Ensure that efx_fast_push_rx_descriptors() must only run
from efx_process_channel() [NAPI], or when napi_disable()
has been executed.

Reimplement the slow fill by sending an event to the
channel, so that NAPI runs, and hanging the subsequent
fast fill off the event handler. Replace the sfc_refill
workqueue and delayed work items with a timer. We do
not need to stop this timer in efx_flush_all() because
it's safe to send the event always; receiving it will
be delayed until NAPI is restarted.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/sfc/efx.c        |   44 +++----------------
 drivers/net/sfc/efx.h        |    4 +-
 drivers/net/sfc/net_driver.h |   10 +---
 drivers/net/sfc/nic.c        |   49 +++++++++++++++++----
 drivers/net/sfc/nic.h        |    1 +
 drivers/net/sfc/rx.c         |   95 +++++++++--------------------------------
 6 files changed, 73 insertions(+), 130 deletions(-)

diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c
index d1a1d32..5d9ef05 100644
--- a/drivers/net/sfc/efx.c
+++ b/drivers/net/sfc/efx.c
@@ -93,13 +93,6 @@ const char *efx_reset_type_names[] = {
 
 #define EFX_MAX_MTU (9 * 1024)
 
-/* RX slow fill workqueue. If memory allocation fails in the fast path,
- * a work item is pushed onto this work queue to retry the allocation later,
- * to avoid the NIC being starved of RX buffers. Since this is a per cpu
- * workqueue, there is nothing to be gained in making it per NIC
- */
-static struct workqueue_struct *refill_workqueue;
-
 /* Reset workqueue. If any NIC has a hardware failure then a reset will be
  * queued onto this work queue. This is not a per-nic work queue, because
  * efx_reset_work() acquires the rtnl lock, so resets are naturally serialised.
@@ -516,11 +509,11 @@ static void efx_start_channel(struct efx_channel *channel)
 	channel->enabled = true;
 	smp_wmb();
 
-	napi_enable(&channel->napi_str);
-
-	/* Load up RX descriptors */
+	/* ...
From: Ben Hutchings
Date: Tuesday, June 1, 2010 - 2:20 pm

From: Steve Hodgson <shodgson@solarflare.com>

The cut-through design of the receive path means that packets that
fail to match the appropriate MAC filter are not discarded at the MAC
but are flagged in the completion event as 'to be discarded'.  On
networks with heavy multicast traffic, this can account for a
significant proportion of received packets, so it is worthwhile to
recycle the buffer immediately in this case rather than freeing it
and then reallocating it shortly after.

The only complication here is dealing with a page shared
between two receive buffers. In that case, we need to be
careful to free the dma mapping when both buffers have
been free'd by the kernel. This means that we can only
recycle such a page if both receive buffers are discarded.
Unfortunately, in an environment with 1500mtu,
rx_alloc_method=PAGE, and a mixture of discarded and
not-discarded frames hitting the same receive queue,
buffer recycling won't always be possible.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/sfc/rx.c |   90 +++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 79 insertions(+), 11 deletions(-)

diff --git a/drivers/net/sfc/rx.c b/drivers/net/sfc/rx.c
index 615a1fc..dfebd73 100644
--- a/drivers/net/sfc/rx.c
+++ b/drivers/net/sfc/rx.c
@@ -82,9 +82,10 @@ static unsigned int rx_refill_limit = 95;
  * RX maximum head room required.
  *
  * This must be at least 1 to prevent overflow and at least 2 to allow
- * pipelined receives.
+ * pipelined receives. Then a further 1 because efx_recycle_rx_buffer()
+ * might insert two buffers.
  */
-#define EFX_RXD_HEAD_ROOM 2
+#define EFX_RXD_HEAD_ROOM 3
 
 static inline unsigned int efx_rx_buf_offset(struct efx_rx_buffer *buf)
 {
@@ -250,6 +251,70 @@ static void efx_fini_rx_buffer(struct efx_rx_queue *rx_queue,
 	efx_free_rx_buffer(rx_queue->efx, rx_buf);
 }
 
+/* Attempt to resurrect the other receive buffer that used to share this page,
+ * which had previously been passed up to ...
From: Ben Hutchings
Date: Tuesday, June 1, 2010 - 2:20 pm

From: Steve Hodgson <shodgson@solarflare.com>

Insert a structure at the start of the shared page that
tracks the dma mapping refcnt. DMA into the next cache
line of the (shared) page (plus EFX_PAGE_IP_ALIGN).

When recycling a page, check the page refcnt. If the
page is otherwise unused, then resurrect the other
receive buffer that previously referenced the page.
Be careful not to overflow the receive ring, since we
can now resurrect n receive buffers in a row.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/sfc/efx.c        |    3 +-
 drivers/net/sfc/net_driver.h |   18 +++++++++
 drivers/net/sfc/rx.c         |   84 +++++++++++++++++++++---------------------
 3 files changed, 62 insertions(+), 43 deletions(-)

diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c
index 5d9ef05..aae3347 100644
--- a/drivers/net/sfc/efx.c
+++ b/drivers/net/sfc/efx.c
@@ -469,7 +469,8 @@ static void efx_init_channels(struct efx_nic *efx)
 	efx->rx_buffer_len = (max(EFX_PAGE_IP_ALIGN, NET_IP_ALIGN) +
 			      EFX_MAX_FRAME_LEN(efx->net_dev->mtu) +
 			      efx->type->rx_buffer_padding);
-	efx->rx_buffer_order = get_order(efx->rx_buffer_len);
+	efx->rx_buffer_order = get_order(efx->rx_buffer_len +
+					 sizeof(struct efx_rx_page_state));
 
 	/* Initialise the channels */
 	efx_for_each_channel(channel, efx) {
diff --git a/drivers/net/sfc/net_driver.h b/drivers/net/sfc/net_driver.h
index 59c8ecc..40c0d93 100644
--- a/drivers/net/sfc/net_driver.h
+++ b/drivers/net/sfc/net_driver.h
@@ -232,6 +232,24 @@ struct efx_rx_buffer {
 };
 
 /**
+ * struct efx_rx_page_state - Page-based rx buffer state
+ *
+ * Inserted at the start of every page allocated for receive buffers.
+ * Used to facilitate sharing dma mappings between recycled rx buffers
+ * and those passed up to the kernel.
+ *
+ * @refcnt: Number of struct efx_rx_buffer's referencing this page.
+ *	When refcnt falls to zero, the page is unmapped for dma
+ * @dma_addr: The dma address of this ...
From: Ben Hutchings
Date: Tuesday, June 1, 2010 - 2:21 pm

rx_errors is defined as 'bad packets received', but we are currently
including various overflow errors as well.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/sfc/efx.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c
index aae3347..26b0cc2 100644
--- a/drivers/net/sfc/efx.c
+++ b/drivers/net/sfc/efx.c
@@ -1518,11 +1518,8 @@ static struct net_device_stats *efx_net_stats(struct net_device *net_dev)
 	stats->tx_window_errors = mac_stats->tx_late_collision;
 
 	stats->rx_errors = (stats->rx_length_errors +
-			    stats->rx_over_errors +
 			    stats->rx_crc_errors +
 			    stats->rx_frame_errors +
-			    stats->rx_fifo_errors +
-			    stats->rx_missed_errors +
 			    mac_stats->rx_symbol_error);
 	stats->tx_errors = (stats->tx_window_errors +
 			    mac_stats->tx_bad);
-- 
1.6.2.5


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

--

From: Dmitry Kravkov
Date: Tuesday, June 8, 2010 - 2:20 am

Hi,

Different drivers include different errors in stats->rx_errors:

  bxn2	  bnx2x	  ixgbx	  sfc		  dnet	  atlx
----------------------------------------------------------------------
rx_length	rx_length	rx_length	rx_length	rx_length	rx_length
rx_over	rx_over							rx_over
rx_frame	rx_frame			rx_frame	rx_frame	rx_frame
rx_crc	rx_crc	rx_crc	rx_crc			rx_crc
		rx_fifo							rx_fifo
		rx_missed							rx_missed
						rx_symbol
								unsup_opcd
----------------------------------------------------------------------
Is there any documentation which defines what should this statistic include?

Thanks
Dmitry


-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On Behalf Of Ben Hutchings
Sent: Wednesday, June 02, 2010 12:21 AM
To: David Miller
Cc: netdev@vger.kernel.org; linux-net-drivers@solarflare.com
Subject: [PATCH 11/12] sfc: Only count bad packets in rx_errors

rx_errors is defined as 'bad packets received', but we are currently
including various overflow errors as well.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/sfc/efx.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c
index aae3347..26b0cc2 100644
--- a/drivers/net/sfc/efx.c
+++ b/drivers/net/sfc/efx.c
@@ -1518,11 +1518,8 @@ static struct net_device_stats *efx_net_stats(struct net_device *net_dev)
 	stats->tx_window_errors = mac_stats->tx_late_collision;
 
 	stats->rx_errors = (stats->rx_length_errors +
-			    stats->rx_over_errors +
 			    stats->rx_crc_errors +
 			    stats->rx_frame_errors +
-			    stats->rx_fifo_errors +
-			    stats->rx_missed_errors +
 			    mac_stats->rx_symbol_error);
 	stats->tx_errors = (stats->tx_window_errors +
 			    mac_stats->tx_bad);
-- 
1.6.2.5


-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that ...
From: Ben Hutchings
Date: Tuesday, June 8, 2010 - 11:09 am

Your table didn't line up when display here.  It appears you used

My references are the comments on the structure (in <linux/if_link.h>
and <linux/netdevice.h>) and an email by Donald Becker
<http://www.uwsg.iu.edu/hypermail/linux/net/0003.3/0154.html>.  The
comments could probably be expanded.

Note also that some of the statistics are combined as follows in
/proc/net/dev, which is what ifconfig uses:

Heading/subheading  Summed statistics
-----------------------------------------------------------------------------------------------
Receive/drop        rx_dropped, rx_missed_errors
Receive/frame       rx_length_errors, rx_over_errors, rx_crc_errors, rx_frame_errors
Transmit/carrier    tx_carrier_errors, tx_aborted_errors, tx_window_errors, tx_heartbeat_errors

Something that doesn't make sense to me is that rx_over_errors is
included under 'frame' rather than under 'drop'.

Ben.

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

--

From: Ben Hutchings
Date: Tuesday, June 1, 2010 - 2:32 pm

A single shared memory region used to communicate with firmware is
mapped into both PCI PFs of the SFC9020 and SFL9021.  Drivers must be
able to identify which port they are addressing in order to use the
correct sub-region.  Currently we use the PCI function number, but the
PCI address may be virtualised.  Use the CS_PORT_NUM register field
defined for just this purpose.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
David,

I think this may be a candidate for 2.6.35 and 2.6.{33,34}.y. If a
driver uses the wrong sub-region of shared memory it will horribly
confuse the firmware and any driver bound to the other function.

Ben.

 drivers/net/sfc/net_driver.h |    4 +++-
 drivers/net/sfc/siena.c      |    4 ++++
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/net/sfc/net_driver.h b/drivers/net/sfc/net_driver.h
index 40c0d93..6b2e440 100644
--- a/drivers/net/sfc/net_driver.h
+++ b/drivers/net/sfc/net_driver.h
@@ -649,6 +649,7 @@ union efx_multicast_hash {
  * struct efx_nic - an Efx NIC
  * @name: Device name (net device name or bus id before net device registered)
  * @pci_dev: The PCI device
+ * @port_num: Index of this host port within the controller
  * @type: Controller type attributes
  * @legacy_irq: IRQ number
  * @workqueue: Workqueue for port reconfigures and the HW monitor.
@@ -732,6 +733,7 @@ union efx_multicast_hash {
 struct efx_nic {
 	char name[IFNAMSIZ];
 	struct pci_dev *pci_dev;
+	unsigned port_num;
 	const struct efx_nic_type *type;
 	int legacy_irq;
 	struct workqueue_struct *workqueue;
@@ -834,7 +836,7 @@ static inline const char *efx_dev_name(struct efx_nic *efx)
 
 static inline unsigned int efx_port_num(struct efx_nic *efx)
 {
-	return PCI_FUNC(efx->pci_dev->devfn);
+	return efx->port_num;
 }
 
 /**
diff --git a/drivers/net/sfc/siena.c b/drivers/net/sfc/siena.c
index 727b422..7ecd255 100644
--- a/drivers/net/sfc/siena.c
+++ b/drivers/net/sfc/siena.c
@@ -206,6 +206,7 @@ static int ...
From: Ben Hutchings
Date: Tuesday, June 1, 2010 - 2:33 pm

From: Steve Hodgson <shodgson@solarflare.com>

- Pull the loop handling into efx_init_rx_buffers_(skb|page)
- Remove rx_queue->buf_page, and associated clean up code
- Remove unmap_addr, since unmap_addr is trivially calculable

This will allow us to recycle discarded buffers directly
from efx_rx_packet(), since will never be in the middle of
splitting a page.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/sfc/net_driver.h |   10 --
 drivers/net/sfc/rx.c         |  228 ++++++++++++++++++------------------------
 2 files changed, 96 insertions(+), 142 deletions(-)

diff --git a/drivers/net/sfc/net_driver.h b/drivers/net/sfc/net_driver.h
index 4539803..59c8ecc 100644
--- a/drivers/net/sfc/net_driver.h
+++ b/drivers/net/sfc/net_driver.h
@@ -222,7 +222,6 @@ struct efx_tx_queue {
  *	If both this and skb are %NULL, the buffer slot is currently free.
  * @data: Pointer to ethernet header
  * @len: Buffer length, in bytes.
- * @unmap_addr: DMA address to unmap
  */
 struct efx_rx_buffer {
 	dma_addr_t dma_addr;
@@ -230,7 +229,6 @@ struct efx_rx_buffer {
 	struct page *page;
 	char *data;
 	unsigned int len;
-	dma_addr_t unmap_addr;
 };
 
 /**
@@ -257,11 +255,6 @@ struct efx_rx_buffer {
  * @alloc_page_count: RX allocation strategy counter.
  * @alloc_skb_count: RX allocation strategy counter.
  * @slow_fill: Timer used to defer efx_nic_generate_fill_event().
- * @buf_page: Page for next RX buffer.
- *	We can use a single page for multiple RX buffers. This tracks
- *	the remaining space in the allocation.
- * @buf_dma_addr: Page's DMA address.
- * @buf_data: Page's host address.
  * @flushed: Use when handling queue flushing
  */
 struct efx_rx_queue {
@@ -284,9 +277,6 @@ struct efx_rx_queue {
 	struct timer_list slow_fill;
 	unsigned int slow_fill_count;
 
-	struct page *buf_page;
-	dma_addr_t buf_dma_addr;
-	char *buf_data;
 	enum efx_flush_state flushed;
 };
 
diff --git a/drivers/net/sfc/rx.c b/drivers/net/sfc/rx.c
index ...
From: David Miller
Date: Wednesday, June 2, 2010 - 2:21 am

From: Ben Hutchings <bhutchings@solarflare.com>

ALl applied, thanks Ben.
--

From: Ben Hutchings
Date: Wednesday, June 2, 2010 - 6:52 am

Did you apply patch 12 "sfc: Get port number from CS_PORT_NUM, not PCI
function number" to net-2.6?

Ben.

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

--

From: David Miller
Date: Wednesday, June 2, 2010 - 6:57 am

From: Ben Hutchings <bhutchings@solarflare.com>

When you mix bug fixes and cleanups, I'm going to apply it all to
net-next-2.6 You didn't even specify an intended destination in your
subject lines, so you leave it entirely open for interpretation and
my choice.

So, if you want a bug fix added to net-2.6, tossing it into a series
which is not wholly appropriate for net-next-2.6 is not that way to
accomplish that.

The way to do it is:

1) Submit the bug fix, explicitly state "[PATCH net-2.6 xxx] "
   in your subject line.

2) Prepare your net-next-2.6 changes on top of that, and do one
   of two things:
   a) Explicitly state in your patch series in the "[PATCH net-next-2.6 0/xxx] "
      posting "this depends upon the bug fixes posted in the series
      XXX posted earlier.
   b) Wait for the bug fix to reach net-next-2.6 when I do a merge,
      then post your series.

Otherwise you make life way to miserable and complicated for me,
the one who has to review and integrate all of your work.

Thanks.
--

From: Ben Hutchings
Date: Wednesday, June 2, 2010 - 8:04 am

Well, I included a comment in the message and left it to your judgement


Sorry.

Ben.

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

--

Previous thread: [PATCH] IP: Increment INADDRERRORS if routing for a packet is not successful by Christoph Lameter on Tuesday, June 1, 2010 - 2:13 pm. (28 messages)

Next thread: [PATCH net-next-2.6 1/2] qlcnic: NIC Partitioning - Add basic infrastructure support by Anirban Chakraborty on Tuesday, June 1, 2010 - 2:28 pm. (2 messages)