Pull request: sfc-next 2013-06-19

Pull request: sfc-next 2013-06-19

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1903 bytes --]

The following changes since commit df09b36f2225b84571b3d5eda2e2683412320713:

  macvtap: enable multiqueue flag (2013-06-07 23:49:09 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-next.git master

for you to fetch changes up to 7585a2d50af2450f9acecb90a0c289d46310317a:

  sfc: Improve test for IOMMU in use (2013-06-19 21:44:19 +0100)

----------------------------------------------------------------
Alexandre Rames (1):
      sfc: Fix EEH with legacy interrupts.

Andy Lutomirski (1):
      sfc: Enable accelerated RFS on vlans

Ben Hutchings (6):
      sfc: Report software timestamping capabilities
      sfc: Define and set RX buffer flag for packets parsed as TCP
      sfc: Do not pass non-TCP packets into GRO code
      sfc: Prefetch RX skb header area
      sfc: Fix IRQ cleanup in case of a probe failure
      sfc: Improve test for IOMMU in use

Jon Cooper (3):
      sfc: Enable RX checksum offload for packets not handled by GRO
      sfc: Increase size of RX SKB header area
      sfc: Implement RX SKB cache to improve latency

 drivers/net/ethernet/sfc/efx.c        | 40 +++----------------
 drivers/net/ethernet/sfc/efx.h        |  1 +
 drivers/net/ethernet/sfc/ethtool.c    | 16 +++++++-
 drivers/net/ethernet/sfc/filter.c     | 15 ++++++-
 drivers/net/ethernet/sfc/net_driver.h |  6 +++
 drivers/net/ethernet/sfc/nic.c        | 74 ++++++++++++++++++++++++++++++-----
 drivers/net/ethernet/sfc/nic.h        |  4 +-
 drivers/net/ethernet/sfc/ptp.c        | 13 +++---
 drivers/net/ethernet/sfc/rx.c         | 63 +++++++++++++++++++++++++----
 9 files changed, 169 insertions(+), 63 deletions(-)

-- 
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.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[PATCH net-next 01/11] sfc: Fix EEH with legacy interrupts.

[Ben Hutchings]

From: Alexandre Rames <arames@solarflare.com>

PCI legacy interrupts are level-triggered, and we cannot mask them up
on an isolated device.  Instead, disable the IRQ at the controller
until we have recovered.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/ethernet/sfc/efx.c        |  7 ++++++-
 drivers/net/ethernet/sfc/efx.h        |  1 +
 drivers/net/ethernet/sfc/net_driver.h |  1 +
 drivers/net/ethernet/sfc/nic.c        | 10 ++++++++++
 4 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 46cc11d..787c9eb 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -23,6 +23,7 @@
 #include <linux/gfp.h>
 #include <linux/cpu_rmap.h>
 #include <linux/aer.h>
+#include <linux/interrupt.h>
 #include "net_driver.h"
 #include "efx.h"
 #include "nic.h"
@@ -1427,6 +1428,10 @@ static void efx_start_interrupts(struct efx_nic *efx, bool may_keep_eventq)
 
 	BUG_ON(efx->state == STATE_DISABLED);
 
+	if (efx->eeh_disabled_legacy_irq) {
+		enable_irq(efx->legacy_irq);
+		efx->eeh_disabled_legacy_irq = false;
+	}
 	if (efx->legacy_irq)
 		efx->legacy_irq_enabled = true;
 	efx_nic_enable_interrupts(efx);
@@ -2365,7 +2370,7 @@ out:
  * Returns 0 if the recovery mechanisms are unsuccessful.
  * Returns a non-zero value otherwise.
  */
-static int efx_try_recovery(struct efx_nic *efx)
+int efx_try_recovery(struct efx_nic *efx)
 {
 #ifdef CONFIG_EEH
 	/* A PCI error can occur and not be seen by EEH because nothing
diff --git a/drivers/net/ethernet/sfc/efx.h b/drivers/net/ethernet/sfc/efx.h
index 8372da2..bdb30bb 100644
--- a/drivers/net/ethernet/sfc/efx.h
+++ b/drivers/net/ethernet/sfc/efx.h
@@ -124,6 +124,7 @@ extern const struct ethtool_ops efx_ethtool_ops;
 extern int efx_reset(struct efx_nic *efx, enum reset_type method);
 extern void efx_reset_down(struct efx_nic *efx, enum reset_type method);
 extern int efx_reset_up(struct efx_nic *efx, enum reset_type method, bool ok);
+extern int efx_try_recovery(struct efx_nic *efx);
 
 /* Global */
 extern void efx_schedule_reset(struct efx_nic *efx, enum reset_type type);
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index 39d6bd77..942b6e6 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -787,6 +787,7 @@ struct efx_nic {
 	const struct efx_nic_type *type;
 	int legacy_irq;
 	bool legacy_irq_enabled;
+	bool eeh_disabled_legacy_irq;
 	struct workqueue_struct *workqueue;
 	char workqueue_name[16];
 	struct work_struct reset_work;
diff --git a/drivers/net/ethernet/sfc/nic.c b/drivers/net/ethernet/sfc/nic.c
index b0503cd..39432d3 100644
--- a/drivers/net/ethernet/sfc/nic.c
+++ b/drivers/net/ethernet/sfc/nic.c
@@ -1579,6 +1579,16 @@ static irqreturn_t efx_legacy_interrupt(int irq, void *dev_id)
 	efx_readd(efx, &reg, FR_BZ_INT_ISR0);
 	queues = EFX_EXTRACT_DWORD(reg, 0, 31);
 
+	/* Legacy interrupts are disabled too late by the EEH kernel
+	 * code. Disable them earlier.
+	 * If an EEH error occurred, the read will have returned all ones.
+	 */
+	if (EFX_DWORD_IS_ALL_ONES(reg) && efx_try_recovery(efx) &&
+	    !efx->eeh_disabled_legacy_irq) {
+		disable_irq_nosync(efx->legacy_irq);
+		efx->eeh_disabled_legacy_irq = true;
+	}
+
 	/* Handle non-event-queue sources */
 	if (queues & (1U << efx->irq_level)) {
 		syserr = EFX_OWORD_FIELD(*int_ker, FSF_AZ_NET_IVEC_FATAL_INT);


-- 
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.

[PATCH net-next 02/11] sfc: Enable RX checksum offload for packets not handled by GRO

[Ben Hutchings]

From: Jon Cooper <jcooper@solarflare.com>

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/ethernet/sfc/rx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
index a7dfe36..b915e09 100644
--- a/drivers/net/ethernet/sfc/rx.c
+++ b/drivers/net/ethernet/sfc/rx.c
@@ -598,6 +598,8 @@ static void efx_rx_deliver(struct efx_channel *channel, u8 *eh,
 
 	/* Set the SKB flags */
 	skb_checksum_none_assert(skb);
+	if (likely(rx_buf->flags & EFX_RX_PKT_CSUMMED))
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
 	if (channel->type->receive_skb)
 		if (channel->type->receive_skb(channel, skb))


-- 
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.

[PATCH net-next 03/11] sfc: Increase size of RX SKB header area

[Ben Hutchings]

From: Jon Cooper <jcooper@solarflare.com>

This allows the SKB to hold the headers without reallocation more often.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/ethernet/sfc/rx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
index b915e09..6efff3d 100644
--- a/drivers/net/ethernet/sfc/rx.c
+++ b/drivers/net/ethernet/sfc/rx.c
@@ -36,7 +36,7 @@
 #define EFX_RECYCLE_RING_SIZE_NOIOMMU (2 * EFX_RX_PREFERRED_BATCH)
 
 /* Size of buffer allocated for skb header area. */
-#define EFX_SKB_HEADERS  64u
+#define EFX_SKB_HEADERS  128u
 
 /* This is the percentage fill level below which new RX descriptors
  * will be added to the RX descriptor ring.


-- 
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.

Re: [PATCH net-next 03/11] sfc: Increase size of RX SKB header area

[Eric Dumazet]

On Wed, 2013-06-19 at 23:18 +0100, Ben Hutchings wrote:
> From: Jon Cooper <jcooper@solarflare.com>
> 
> This allows the SKB to hold the headers without reallocation more often.
> 
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
>  drivers/net/ethernet/sfc/rx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
> index b915e09..6efff3d 100644
> --- a/drivers/net/ethernet/sfc/rx.c
> +++ b/drivers/net/ethernet/sfc/rx.c
> @@ -36,7 +36,7 @@
>  #define EFX_RECYCLE_RING_SIZE_NOIOMMU (2 * EFX_RX_PREFERRED_BATCH)
>  
>  /* Size of buffer allocated for skb header area. */
> -#define EFX_SKB_HEADERS  64u
> +#define EFX_SKB_HEADERS  128u
>  

Yes, TCP with timestamps needs 66 bytes of headers on IPv4, and more on
IPv6...

[PATCH net-next 04/11] sfc: Report software timestamping capabilities

[Ben Hutchings]

The kernel can generate software receive timestamps and we should
report those for all ports regardless of hardware capabilities.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/ethernet/sfc/ethtool.c | 16 +++++++++++++++-
 drivers/net/ethernet/sfc/nic.h     |  4 ++--
 drivers/net/ethernet/sfc/ptp.c     | 13 +++++--------
 3 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/sfc/ethtool.c b/drivers/net/ethernet/sfc/ethtool.c
index 6e76817..1fc2145 100644
--- a/drivers/net/ethernet/sfc/ethtool.c
+++ b/drivers/net/ethernet/sfc/ethtool.c
@@ -1114,6 +1114,20 @@ static int efx_ethtool_set_rxfh_indir(struct net_device *net_dev,
 	return 0;
 }
 
+int efx_ethtool_get_ts_info(struct net_device *net_dev,
+			    struct ethtool_ts_info *ts_info)
+{
+	struct efx_nic *efx = netdev_priv(net_dev);
+
+	/* Software capabilities */
+	ts_info->so_timestamping = (SOF_TIMESTAMPING_RX_SOFTWARE |
+				    SOF_TIMESTAMPING_SOFTWARE);
+	ts_info->phc_index = -1;
+
+	efx_ptp_get_ts_info(efx, ts_info);
+	return 0;
+}
+
 static int efx_ethtool_get_module_eeprom(struct net_device *net_dev,
 					 struct ethtool_eeprom *ee,
 					 u8 *data)
@@ -1176,7 +1190,7 @@ const struct ethtool_ops efx_ethtool_ops = {
 	.get_rxfh_indir_size	= efx_ethtool_get_rxfh_indir_size,
 	.get_rxfh_indir		= efx_ethtool_get_rxfh_indir,
 	.set_rxfh_indir		= efx_ethtool_set_rxfh_indir,
-	.get_ts_info		= efx_ptp_get_ts_info,
+	.get_ts_info		= efx_ethtool_get_ts_info,
 	.get_module_info	= efx_ethtool_get_module_info,
 	.get_module_eeprom	= efx_ethtool_get_module_eeprom,
 };
diff --git a/drivers/net/ethernet/sfc/nic.h b/drivers/net/ethernet/sfc/nic.h
index 1b00033..d63c299 100644
--- a/drivers/net/ethernet/sfc/nic.h
+++ b/drivers/net/ethernet/sfc/nic.h
@@ -254,8 +254,8 @@ extern int efx_sriov_set_vf_spoofchk(struct net_device *net_dev, int vf,
 struct ethtool_ts_info;
 extern void efx_ptp_probe(struct efx_nic *efx);
 extern int efx_ptp_ioctl(struct efx_nic *efx, struct ifreq *ifr, int cmd);
-extern int efx_ptp_get_ts_info(struct net_device *net_dev,
-			       struct ethtool_ts_info *ts_info);
+extern void efx_ptp_get_ts_info(struct efx_nic *efx,
+				struct ethtool_ts_info *ts_info);
 extern bool efx_ptp_is_ptp_tx(struct efx_nic *efx, struct sk_buff *skb);
 extern int efx_ptp_tx(struct efx_nic *efx, struct sk_buff *skb);
 extern void efx_ptp_event(struct efx_nic *efx, efx_qword_t *ev);
diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 9a95abf..b495394 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -1203,18 +1203,16 @@ static int efx_ptp_ts_init(struct efx_nic *efx, struct hwtstamp_config *init)
 	return 0;
 }
 
-int
-efx_ptp_get_ts_info(struct net_device *net_dev, struct ethtool_ts_info *ts_info)
+void efx_ptp_get_ts_info(struct efx_nic *efx, struct ethtool_ts_info *ts_info)
 {
-	struct efx_nic *efx = netdev_priv(net_dev);
 	struct efx_ptp_data *ptp = efx->ptp_data;
 
 	if (!ptp)
-		return -EOPNOTSUPP;
+		return;
 
-	ts_info->so_timestamping = (SOF_TIMESTAMPING_TX_HARDWARE |
-				    SOF_TIMESTAMPING_RX_HARDWARE |
-				    SOF_TIMESTAMPING_RAW_HARDWARE);
+	ts_info->so_timestamping |= (SOF_TIMESTAMPING_TX_HARDWARE |
+				     SOF_TIMESTAMPING_RX_HARDWARE |
+				     SOF_TIMESTAMPING_RAW_HARDWARE);
 	ts_info->phc_index = ptp_clock_index(ptp->phc_clock);
 	ts_info->tx_types = 1 << HWTSTAMP_TX_OFF | 1 << HWTSTAMP_TX_ON;
 	ts_info->rx_filters = (1 << HWTSTAMP_FILTER_NONE |
@@ -1224,7 +1222,6 @@ efx_ptp_get_ts_info(struct net_device *net_dev, struct ethtool_ts_info *ts_info)
 			       1 << HWTSTAMP_FILTER_PTP_V2_L4_EVENT |
 			       1 << HWTSTAMP_FILTER_PTP_V2_L4_SYNC |
 			       1 << HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ);
-	return 0;
 }
 
 int efx_ptp_ioctl(struct efx_nic *efx, struct ifreq *ifr, int cmd)


-- 
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.

[PATCH net-next 05/11] sfc: Enable accelerated RFS on vlans

[Ben Hutchings]

From: Andy Lutomirski <luto@amacapital.net>

As far as I know, the hardware doesn't support matching on both IP
fields and vlan tag, but it can at least match on the IP fields.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/ethernet/sfc/filter.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/sfc/filter.c b/drivers/net/ethernet/sfc/filter.c
index 2397f0e..b74a60a 100644
--- a/drivers/net/ethernet/sfc/filter.c
+++ b/drivers/net/ethernet/sfc/filter.c
@@ -1185,8 +1185,21 @@ int efx_filter_rfs(struct net_device *net_dev, const struct sk_buff *skb,
 
 	nhoff = skb_network_offset(skb);
 
-	if (skb->protocol != htons(ETH_P_IP))
+	if (skb->protocol == htons(ETH_P_8021Q)) {
+		EFX_BUG_ON_PARANOID(skb_headlen(skb) <
+				    nhoff + sizeof(struct vlan_hdr));
+		if (((const struct vlan_hdr *)skb->data + nhoff)->
+		    h_vlan_encapsulated_proto != htons(ETH_P_IP))
+			return -EPROTONOSUPPORT;
+
+		/* This is IP over 802.1q VLAN.  We can't filter on the
+		 * IP 5-tuple and the vlan together, so just strip the
+		 * vlan header and filter on the IP part.
+		 */
+		nhoff += sizeof(struct vlan_hdr);
+	} else if (skb->protocol != htons(ETH_P_IP)) {
 		return -EPROTONOSUPPORT;
+	}
 
 	/* RFS must validate the IP header length before calling us */
 	EFX_BUG_ON_PARANOID(skb_headlen(skb) < nhoff + sizeof(*ip));


-- 
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.

[PATCH net-next 06/11] sfc: Define and set RX buffer flag for packets parsed as TCP

[Ben Hutchings]

This will be useful for shortcutting some software packet parsing.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/ethernet/sfc/net_driver.h |  1 +
 drivers/net/ethernet/sfc/nic.c        | 19 ++++++++++++++-----
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index 942b6e6..60da43e 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -243,6 +243,7 @@ struct efx_rx_buffer {
 #define EFX_RX_BUF_LAST_IN_PAGE	0x0001
 #define EFX_RX_PKT_CSUMMED	0x0002
 #define EFX_RX_PKT_DISCARD	0x0004
+#define EFX_RX_PKT_TCP		0x0040
 
 /**
  * struct efx_rx_page_state - Page-based rx buffer state
diff --git a/drivers/net/ethernet/sfc/nic.c b/drivers/net/ethernet/sfc/nic.c
index 39432d3..f2b864c 100644
--- a/drivers/net/ethernet/sfc/nic.c
+++ b/drivers/net/ethernet/sfc/nic.c
@@ -1080,12 +1080,21 @@ efx_handle_rx_event(struct efx_channel *channel, const efx_qword_t *event)
 	rx_ev_hdr_type = EFX_QWORD_FIELD(*event, FSF_AZ_RX_EV_HDR_TYPE);
 
 	if (likely(rx_ev_pkt_ok)) {
-		/* If packet is marked as OK and packet type is TCP/IP or
-		 * UDP/IP, then we can rely on the hardware checksum.
+		/* If packet is marked as OK then we can rely on the
+		 * hardware checksum and classification.
 		 */
-		flags = (rx_ev_hdr_type == FSE_CZ_RX_EV_HDR_TYPE_IPV4V6_TCP ||
-			 rx_ev_hdr_type == FSE_CZ_RX_EV_HDR_TYPE_IPV4V6_UDP) ?
-			EFX_RX_PKT_CSUMMED : 0;
+		flags = 0;
+		switch (rx_ev_hdr_type) {
+		case FSE_CZ_RX_EV_HDR_TYPE_IPV4V6_TCP:
+			flags |= EFX_RX_PKT_TCP;
+			/* fall through */
+		case FSE_CZ_RX_EV_HDR_TYPE_IPV4V6_UDP:
+			flags |= EFX_RX_PKT_CSUMMED;
+			/* fall through */
+		case FSE_CZ_RX_EV_HDR_TYPE_IPV4V6_OTHER:
+		case FSE_AZ_RX_EV_HDR_TYPE_OTHER:
+			break;
+		}
 	} else {
 		flags = efx_handle_rx_not_ok(rx_queue, event);
 	}


-- 
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.

[PATCH net-next 07/11] sfc: Do not pass non-TCP packets into GRO code

[Ben Hutchings]

GRO can handle non-TCP packets and pass them up without coalescing,
but it has to do some extra work to parse the packet which we can
bypass using the hardware parse result.  (Falcon won't mark TCP/IPv6
packets as TCP, but then it won't validate their checksums either, so
GRO can't coalesce them.)

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/ethernet/sfc/rx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
index 6efff3d..5c45118 100644
--- a/drivers/net/ethernet/sfc/rx.c
+++ b/drivers/net/ethernet/sfc/rx.c
@@ -629,7 +629,7 @@ void __efx_rx_packet(struct efx_channel *channel)
 	if (unlikely(!(efx->net_dev->features & NETIF_F_RXCSUM)))
 		rx_buf->flags &= ~EFX_RX_PKT_CSUMMED;
 
-	if (!channel->type->receive_skb)
+	if ((rx_buf->flags & EFX_RX_PKT_TCP) && !channel->type->receive_skb)
 		efx_rx_packet_gro(channel, rx_buf, channel->rx_pkt_n_frags, eh);
 	else
 		efx_rx_deliver(channel, eh, rx_buf, channel->rx_pkt_n_frags);


-- 
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.

Re: [PATCH net-next 07/11] sfc: Do not pass non-TCP packets into GRO code

[Eric Dumazet]

On Wed, 2013-06-19 at 23:19 +0100, Ben Hutchings wrote:
> GRO can handle non-TCP packets and pass them up without coalescing,
> but it has to do some extra work to parse the packet which we can
> bypass using the hardware parse result.  (Falcon won't mark TCP/IPv6
> packets as TCP, but then it won't validate their checksums either, so
> GRO can't coalesce them.)

Thats not exactly true. GRO can validate the checksums.

Re: [PATCH net-next 07/11] sfc: Do not pass non-TCP packets into GRO code

[Ben Hutchings]

On Wed, 2013-06-19 at 15:56 -0700, Eric Dumazet wrote:
> On Wed, 2013-06-19 at 23:19 +0100, Ben Hutchings wrote:
> > GRO can handle non-TCP packets and pass them up without coalescing,
> > but it has to do some extra work to parse the packet which we can
> > bypass using the hardware parse result.  (Falcon won't mark TCP/IPv6
> > packets as TCP, but then it won't validate their checksums either, so
> > GRO can't coalesce them.)
> 
> Thats not exactly true. GRO can validate the checksums.

OK, maybe that changed at some point.  The original conversions to use
napi_gro_receive() were all conditional on hardware checksum validation.

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.

[PATCH net-next 08/11] sfc: Implement RX SKB cache to improve latency

[Ben Hutchings]

From: Jon Cooper <jcooper@solarflare.com>

Allocate skbs in batches, instead of just after receiving each packet.

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

diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index 60da43e..d8e32e0 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -323,6 +323,10 @@ struct efx_rx_queue {
 	unsigned int recycle_count;
 	struct timer_list slow_fill;
 	unsigned int slow_fill_count;
+
+#define SKB_CACHE_SIZE 8
+	struct sk_buff *skb_cache[SKB_CACHE_SIZE];
+	unsigned skb_cache_next_unused;
 };
 
 /**
diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
index 5c45118..a28f2fe 100644
--- a/drivers/net/ethernet/sfc/rx.c
+++ b/drivers/net/ethernet/sfc/rx.c
@@ -104,6 +104,31 @@ void efx_rx_config_page_split(struct efx_nic *efx)
 					       efx->rx_bufs_per_page);
 }
 
+static void efx_refill_skb_cache(struct efx_rx_queue *rx_queue)
+{
+	int i;
+	for (i = 0; i < SKB_CACHE_SIZE; ++i) {
+		rx_queue->skb_cache[i] =
+			netdev_alloc_skb(rx_queue->efx->net_dev,
+					 EFX_SKB_HEADERS + EFX_PAGE_SKB_ALIGN);
+	}
+	rx_queue->skb_cache_next_unused = 0;
+}
+
+static void efx_refill_skb_cache_check(struct efx_rx_queue *rx_queue)
+{
+	if (rx_queue->skb_cache_next_unused == SKB_CACHE_SIZE)
+		efx_refill_skb_cache(rx_queue);
+}
+
+static void efx_fini_skb_cache(struct efx_rx_queue *rx_queue)
+{
+	/* free unused skbs in the cache */
+	while (rx_queue->skb_cache_next_unused < SKB_CACHE_SIZE)
+		dev_kfree_skb(
+			rx_queue->skb_cache[rx_queue->skb_cache_next_unused++]);
+}
+
 /* Check the RX page recycle ring for a page that can be reused. */
 static struct page *efx_reuse_page(struct efx_rx_queue *rx_queue)
 {
@@ -454,12 +479,20 @@ static struct sk_buff *efx_rx_mk_skb(struct efx_channel *channel,
 				     u8 *eh, int hdr_len)
 {
 	struct efx_nic *efx = channel->efx;
-	struct sk_buff *skb;
+	struct efx_rx_queue *rx_queue = efx_channel_get_rx_queue(channel);
+	struct sk_buff *skb = NULL;
 
 	/* Allocate an SKB to store the headers */
-	skb = netdev_alloc_skb(efx->net_dev, hdr_len + EFX_PAGE_SKB_ALIGN);
-	if (unlikely(skb == NULL))
-		return NULL;
+	if (hdr_len <= EFX_SKB_HEADERS &&
+	    likely(rx_queue->skb_cache_next_unused < SKB_CACHE_SIZE)) {
+		skb = rx_queue->skb_cache[rx_queue->skb_cache_next_unused++];
+	}
+	if (unlikely(!skb)) {
+		skb = netdev_alloc_skb(efx->net_dev,
+				       hdr_len + EFX_PAGE_SKB_ALIGN);
+		if (unlikely(skb == NULL))
+			return NULL;
+	}
 
 	EFX_BUG_ON_PARANOID(rx_buf->len < hdr_len);
 
@@ -580,6 +613,9 @@ void efx_rx_packet(struct efx_rx_queue *rx_queue, unsigned int index,
 	efx_rx_flush_packet(channel);
 	channel->rx_pkt_n_frags = n_frags;
 	channel->rx_pkt_index = index;
+
+	/* refill skb cache if needed */
+	efx_refill_skb_cache_check(rx_queue);
 }
 
 static void efx_rx_deliver(struct efx_channel *channel, u8 *eh,
@@ -729,6 +765,9 @@ void efx_init_rx_queue(struct efx_rx_queue *rx_queue)
 	/* Set up RX descriptor ring */
 	rx_queue->enabled = true;
 	efx_nic_init_rx(rx_queue);
+
+	/* fill SKB cache */
+	efx_refill_skb_cache(rx_queue);
 }
 
 void efx_fini_rx_queue(struct efx_rx_queue *rx_queue)
@@ -756,6 +795,8 @@ void efx_fini_rx_queue(struct efx_rx_queue *rx_queue)
 		}
 	}
 
+	efx_fini_skb_cache(rx_queue);
+
 	/* Unmap and release the pages in the recycle ring. Remove the ring. */
 	for (i = 0; i <= rx_queue->page_ptr_mask; i++) {
 		struct page *page = rx_queue->page_ring[i];


-- 
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.

[PATCH net-next 09/11] sfc: Prefetch RX skb header area

[Ben Hutchings]

When we pre-allocate skbs, the sk_buff structures themselves will be
cache-hot (having just been filled in) but the header area will be
cache-cold.  The skb cache is small and we're likely to use each skb
quite soon, so prefetch the start of the header area, where we will
copy the headers, and the skb_shared_info structure, where we will
store page fragments.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/ethernet/sfc/rx.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
index a28f2fe..0dc4a80 100644
--- a/drivers/net/ethernet/sfc/rx.c
+++ b/drivers/net/ethernet/sfc/rx.c
@@ -106,11 +106,17 @@ void efx_rx_config_page_split(struct efx_nic *efx)
 
 static void efx_refill_skb_cache(struct efx_rx_queue *rx_queue)
 {
+	struct sk_buff *skb;
 	int i;
+
 	for (i = 0; i < SKB_CACHE_SIZE; ++i) {
-		rx_queue->skb_cache[i] =
-			netdev_alloc_skb(rx_queue->efx->net_dev,
-					 EFX_SKB_HEADERS + EFX_PAGE_SKB_ALIGN);
+		skb = netdev_alloc_skb(rx_queue->efx->net_dev,
+				       EFX_SKB_HEADERS + EFX_PAGE_SKB_ALIGN);
+		rx_queue->skb_cache[i] = skb;
+		if (skb) {
+			prefetch(skb->data);
+			prefetch(skb_shinfo(skb));
+		}
 	}
 	rx_queue->skb_cache_next_unused = 0;
 }


-- 
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.

Re: [PATCH net-next 09/11] sfc: Prefetch RX skb header area

[Eric Dumazet]

On Wed, 2013-06-19 at 23:20 +0100, Ben Hutchings wrote:
> When we pre-allocate skbs, the sk_buff structures themselves will be
> cache-hot (having just been filled in) but the header area will be
> cache-cold.  The skb cache is small and we're likely to use each skb
> quite soon, so prefetch the start of the header area, where we will
> copy the headers, and the skb_shared_info structure, where we will
> store page fragments.
> 
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
>  drivers/net/ethernet/sfc/rx.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
> index a28f2fe..0dc4a80 100644
> --- a/drivers/net/ethernet/sfc/rx.c
> +++ b/drivers/net/ethernet/sfc/rx.c
> @@ -106,11 +106,17 @@ void efx_rx_config_page_split(struct efx_nic *efx)
>  
>  static void efx_refill_skb_cache(struct efx_rx_queue *rx_queue)
>  {
> +	struct sk_buff *skb;
>  	int i;
> +
>  	for (i = 0; i < SKB_CACHE_SIZE; ++i) {
> -		rx_queue->skb_cache[i] =
> -			netdev_alloc_skb(rx_queue->efx->net_dev,
> -					 EFX_SKB_HEADERS + EFX_PAGE_SKB_ALIGN);
> +		skb = netdev_alloc_skb(rx_queue->efx->net_dev,
> +				       EFX_SKB_HEADERS + EFX_PAGE_SKB_ALIGN);
> +		rx_queue->skb_cache[i] = skb;
> +		if (skb) {
> +			prefetch(skb->data);

This is really suspect.

This part will be dirtied anyway by the NIC once frame is received.

> +			prefetch(skb_shinfo(skb));
> +		}
>  	}
>  	rx_queue->skb_cache_next_unused = 0;
>  }
> 
> 

Quite frankly this cache sounds suspect to me.

Why is it needed only for sfc ?

Have you tried build_skb() instead ?

Re: [PATCH net-next 09/11] sfc: Prefetch RX skb header area

[Ben Hutchings]

On Wed, 2013-06-19 at 15:36 -0700, Eric Dumazet wrote:
> On Wed, 2013-06-19 at 23:20 +0100, Ben Hutchings wrote:
> > When we pre-allocate skbs, the sk_buff structures themselves will be
> > cache-hot (having just been filled in) but the header area will be
> > cache-cold.  The skb cache is small and we're likely to use each skb
> > quite soon, so prefetch the start of the header area, where we will
> > copy the headers, and the skb_shared_info structure, where we will
> > store page fragments.
> > 
> > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> > ---
> >  drivers/net/ethernet/sfc/rx.c | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
> > index a28f2fe..0dc4a80 100644
> > --- a/drivers/net/ethernet/sfc/rx.c
> > +++ b/drivers/net/ethernet/sfc/rx.c
> > @@ -106,11 +106,17 @@ void efx_rx_config_page_split(struct efx_nic *efx)
> >  
> >  static void efx_refill_skb_cache(struct efx_rx_queue *rx_queue)
> >  {
> > +	struct sk_buff *skb;
> >  	int i;
> > +
> >  	for (i = 0; i < SKB_CACHE_SIZE; ++i) {
> > -		rx_queue->skb_cache[i] =
> > -			netdev_alloc_skb(rx_queue->efx->net_dev,
> > -					 EFX_SKB_HEADERS + EFX_PAGE_SKB_ALIGN);
> > +		skb = netdev_alloc_skb(rx_queue->efx->net_dev,
> > +				       EFX_SKB_HEADERS + EFX_PAGE_SKB_ALIGN);
> > +		rx_queue->skb_cache[i] = skb;
> > +		if (skb) {
> > +			prefetch(skb->data);
> 
> This is really suspect.
> 
> This part will be dirtied anyway by the NIC once frame is received.

This is not used for DMA.

> > +			prefetch(skb_shinfo(skb));
> > +		}
> >  	}
> >  	rx_queue->skb_cache_next_unused = 0;
> >  }
> > 
> > 
> 
> Quite frankly this cache sounds suspect to me.
> 
> Why is it needed only for sfc ?
> 
> Have you tried build_skb() instead ?

You can't use build_skb() if you put multiple RX buffers in a page.  See
<1365805319.2791.18.camel@bwh-desktop.uk.solarflarecom.com>.

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.

Re: [PATCH net-next 09/11] sfc: Prefetch RX skb header area

[Eric Dumazet]

On Wed, 2013-06-19 at 23:39 +0100, Ben Hutchings wrote:

> You can't use build_skb() if you put multiple RX buffers in a page.  See
> <1365805319.2791.18.camel@bwh-desktop.uk.solarflarecom.com>.

Well, you use small skb (used to have 64 bytes headroom and you changed
it to 128) and attach a frag on it, so anyway build_skb() brings
nothing.

I suspect you have a win on workloads with flood of rx, but on moderate
load sk_buff will be cold, so the cache adds latencies.

My feeling is that this kind of tradeoff should not be per driver.

Re: [PATCH net-next 09/11] sfc: Prefetch RX skb header area

[Ben Hutchings]

On Wed, 2013-06-19 at 15:56 -0700, Eric Dumazet wrote:
> On Wed, 2013-06-19 at 23:39 +0100, Ben Hutchings wrote:
> 
> > You can't use build_skb() if you put multiple RX buffers in a page.  See
> > <1365805319.2791.18.camel@bwh-desktop.uk.solarflarecom.com>.
> 
> Well, you use small skb (used to have 64 bytes headroom and you changed
> it to 128) and attach a frag on it, so anyway build_skb() brings
> nothing.
> 
> I suspect you have a win on workloads with flood of rx, but on moderate
> load sk_buff will be cold, so the cache adds latencies.
> 
> My feeling is that this kind of tradeoff should not be per driver.

Jon's testing showed a significant latency improvement.

In Linux 3.10 sfc has reduced UDP performance (both latency and
throughput) as we removed the support for skb pre-allocation in
preparation for the implementation for DMA scatter and recycling of
IOMMU-mapped buffers.  The RX changes in this series are intended to
recover that, and our benchmark results suggest that we succeeded.

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.

[PATCH net-next 10/11] sfc: Fix IRQ cleanup in case of a probe failure

[Ben Hutchings]

The lifetime of an irq_cpu_rmap is odd: we have to allocate it before
installing IRQ handlers and free it before removing the IRQ handlers.
As a result of this asymmetry, it was omitted from some failure paths.

On another failure path, we could try to remove IRQ handlers we
had not yet installed.

Move the irq_cpu_rmap allocation and freeing alongside IRQ handler
installation and removal, in efx_nic_{init,fini}_interrupts().
Count the number of IRQ handlers successfully installed and only
remove those on the failure path.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/ethernet/sfc/efx.c | 33 -------------------------------
 drivers/net/ethernet/sfc/nic.c | 45 +++++++++++++++++++++++++++++++++++++-----
 2 files changed, 40 insertions(+), 38 deletions(-)

diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 787c9eb..e7284a2 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -21,7 +21,6 @@
 #include <linux/ethtool.h>
 #include <linux/topology.h>
 #include <linux/gfp.h>
-#include <linux/cpu_rmap.h>
 #include <linux/aer.h>
 #include <linux/interrupt.h>
 #include "net_driver.h"
@@ -1284,29 +1283,6 @@ static unsigned int efx_wanted_parallelism(struct efx_nic *efx)
 	return count;
 }
 
-static int
-efx_init_rx_cpu_rmap(struct efx_nic *efx, struct msix_entry *xentries)
-{
-#ifdef CONFIG_RFS_ACCEL
-	unsigned int i;
-	int rc;
-
-	efx->net_dev->rx_cpu_rmap = alloc_irq_cpu_rmap(efx->n_rx_channels);
-	if (!efx->net_dev->rx_cpu_rmap)
-		return -ENOMEM;
-	for (i = 0; i < efx->n_rx_channels; i++) {
-		rc = irq_cpu_rmap_add(efx->net_dev->rx_cpu_rmap,
-				      xentries[i].vector);
-		if (rc) {
-			free_irq_cpu_rmap(efx->net_dev->rx_cpu_rmap);
-			efx->net_dev->rx_cpu_rmap = NULL;
-			return rc;
-		}
-	}
-#endif
-	return 0;
-}
-
 /* Probe the number and type of interrupts we are able to obtain, and
  * the resulting numbers of channels and RX queues.
  */
@@ -1360,11 +1336,6 @@ static int efx_probe_interrupts(struct efx_nic *efx)
 				efx->n_tx_channels = n_channels;
 				efx->n_rx_channels = n_channels;
 			}
-			rc = efx_init_rx_cpu_rmap(efx, xentries);
-			if (rc) {
-				pci_disable_msix(efx->pci_dev);
-				return rc;
-			}
 			for (i = 0; i < efx->n_channels; i++)
 				efx_get_channel(efx, i)->irq =
 					xentries[i].vector;
@@ -2608,10 +2579,6 @@ static void efx_pci_remove_main(struct efx_nic *efx)
 	BUG_ON(efx->state == STATE_READY);
 	cancel_work_sync(&efx->reset_work);
 
-#ifdef CONFIG_RFS_ACCEL
-	free_irq_cpu_rmap(efx->net_dev->rx_cpu_rmap);
-	efx->net_dev->rx_cpu_rmap = NULL;
-#endif
 	efx_stop_interrupts(efx, false);
 	efx_nic_fini_interrupt(efx);
 	efx_fini_port(efx);
diff --git a/drivers/net/ethernet/sfc/nic.c b/drivers/net/ethernet/sfc/nic.c
index f2b864c..56ed3bc 100644
--- a/drivers/net/ethernet/sfc/nic.c
+++ b/drivers/net/ethernet/sfc/nic.c
@@ -14,6 +14,7 @@
 #include <linux/pci.h>
 #include <linux/module.h>
 #include <linux/seq_file.h>
+#include <linux/cpu_rmap.h>
 #include "net_driver.h"
 #include "bitfield.h"
 #include "efx.h"
@@ -1706,6 +1707,7 @@ void efx_nic_push_rx_indir_table(struct efx_nic *efx)
 int efx_nic_init_interrupt(struct efx_nic *efx)
 {
 	struct efx_channel *channel;
+	unsigned int n_irqs;
 	int rc;
 
 	if (!EFX_INT_MODE_USE_MSI(efx)) {
@@ -1726,7 +1728,19 @@ int efx_nic_init_interrupt(struct efx_nic *efx)
 		return 0;
 	}
 
+#ifdef CONFIG_RFS_ACCEL
+	if (efx->interrupt_mode == EFX_INT_MODE_MSIX) {
+		efx->net_dev->rx_cpu_rmap =
+			alloc_irq_cpu_rmap(efx->n_rx_channels);
+		if (!efx->net_dev->rx_cpu_rmap) {
+			rc = -ENOMEM;
+			goto fail1;
+		}
+	}
+#endif
+
 	/* Hook MSI or MSI-X interrupt */
+	n_irqs = 0;
 	efx_for_each_channel(channel, efx) {
 		rc = request_irq(channel->irq, efx_msi_interrupt,
 				 IRQF_PROBE_SHARED, /* Not shared */
@@ -1737,13 +1751,31 @@ int efx_nic_init_interrupt(struct efx_nic *efx)
 				  "failed to hook IRQ %d\n", channel->irq);
 			goto fail2;
 		}
+		++n_irqs;
+
+#ifdef CONFIG_RFS_ACCEL
+		if (efx->interrupt_mode == EFX_INT_MODE_MSIX &&
+		    channel->channel < efx->n_rx_channels) {
+			rc = irq_cpu_rmap_add(efx->net_dev->rx_cpu_rmap,
+					      channel->irq);
+			if (rc)
+				goto fail2;
+		}
+#endif
 	}
 
 	return 0;
 
  fail2:
-	efx_for_each_channel(channel, efx)
+#ifdef CONFIG_RFS_ACCEL
+	free_irq_cpu_rmap(efx->net_dev->rx_cpu_rmap);
+	efx->net_dev->rx_cpu_rmap = NULL;
+#endif
+	efx_for_each_channel(channel, efx) {
+		if (n_irqs-- == 0)
+			break;
 		free_irq(channel->irq, &efx->channel[channel->channel]);
+	}
  fail1:
 	return rc;
 }
@@ -1753,11 +1785,14 @@ void efx_nic_fini_interrupt(struct efx_nic *efx)
 	struct efx_channel *channel;
 	efx_oword_t reg;
 
+#ifdef CONFIG_RFS_ACCEL
+	free_irq_cpu_rmap(efx->net_dev->rx_cpu_rmap);
+	efx->net_dev->rx_cpu_rmap = NULL;
+#endif
+
 	/* Disable MSI/MSI-X interrupts */
-	efx_for_each_channel(channel, efx) {
-		if (channel->irq)
-			free_irq(channel->irq, &efx->channel[channel->channel]);
-	}
+	efx_for_each_channel(channel, efx)
+		free_irq(channel->irq, &efx->channel[channel->channel]);
 
 	/* ACK legacy interrupt */
 	if (efx_nic_rev(efx) >= EFX_REV_FALCON_B0)


-- 
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.

[PATCH net-next 11/11] sfc: Improve test for IOMMU in use

[Ben Hutchings]

The device::iommu_group field may be set even if no IOMMU is in use.
iommu_present() is still a better indicator, although it doesn't tell
us whether *our* device is affected.

Reported-by: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/ethernet/sfc/rx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
index 0dc4a80..c053cf5 100644
--- a/drivers/net/ethernet/sfc/rx.c
+++ b/drivers/net/ethernet/sfc/rx.c
@@ -719,7 +719,7 @@ static void efx_init_rx_recycle_ring(struct efx_nic *efx,
 #ifdef CONFIG_PPC64
 	bufs_in_recycle_ring = EFX_RECYCLE_RING_SIZE_IOMMU;
 #else
-	if (efx->pci_dev->dev.iommu_group)
+	if (iommu_present(&pci_bus_type))
 		bufs_in_recycle_ring = EFX_RECYCLE_RING_SIZE_IOMMU;
 	else
 		bufs_in_recycle_ring = EFX_RECYCLE_RING_SIZE_NOIOMMU;

-- 
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.

Re: Pull request: sfc-next 2013-06-19

[David Miller]

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 19 Jun 2013 23:16:09 +0100

> The following changes since commit df09b36f2225b84571b3d5eda2e2683412320713:
> 
>   macvtap: enable multiqueue flag (2013-06-07 23:49:09 -0700)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-next.git master
> 
> for you to fetch changes up to 7585a2d50af2450f9acecb90a0c289d46310317a:
> 
>   sfc: Improve test for IOMMU in use (2013-06-19 21:44:19 +0100)

I feel you have some issues to work out with Eric, and at a minimum
enhance some commit log messages, so I'm deferring on this series.