Netdev List
 help / color / mirror / Atom feed
* [net-next 12/17] i40e: Drop redundant Rx descriptor processing code
From: Jeff Kirsher @ 2016-10-29  7:30 UTC (permalink / raw)
  To: davem
  Cc: Alexander Duyck, netdev, nhorman, sassmann, jogreene,
	guru.anbalagane, Jeff Kirsher
In-Reply-To: <1477726258-111563-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Alexander Duyck <alexander.h.duyck@intel.com>

This patch cleans up several pieces of redundant code in the Rx clean-up
paths.

The first bit is that hdr_addr and the status_err_len portions of the Rx
descriptor represent the same value.  As such there is no point in setting
them to 0 before setting them to 0.  I'm dropping the second spot where we
are updating the value to 0 so that we only have 1 write for this value
instead of 2.

The second piece is the checking for the DD bit in the packet.  We only
need to check for a non-zero value for the status_err_len because if the
device is done with the descriptor it will have written something back and
the DD is just one piece of it.  In addition I have moved the reading of
the Rx descriptor bits related to rx_ptype down so that they are actually
below the dma_rmb() call so that we are guaranteed that we don't have any
funky 64b on 32b calls causing any ordering issues.

Change-ID: I256e44a025d3c64a7224aaaec37c852bfcb1871b
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 18 ++++++------------
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 18 ++++++------------
 2 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 48e6533..daade4fe 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -1220,7 +1220,6 @@ bool i40e_alloc_rx_buffers(struct i40e_ring *rx_ring, u16 cleaned_count)
 		 * because each write-back erases this info.
 		 */
 		rx_desc->read.pkt_addr = cpu_to_le64(bi->dma + bi->page_offset);
-		rx_desc->read.hdr_addr = 0;
 
 		rx_desc++;
 		bi++;
@@ -1741,7 +1740,6 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
 	while (likely(total_rx_packets < budget)) {
 		union i40e_rx_desc *rx_desc;
 		struct sk_buff *skb;
-		u32 rx_status;
 		u16 vlan_tag;
 		u8 rx_ptype;
 		u64 qword;
@@ -1755,21 +1753,13 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
 
 		rx_desc = I40E_RX_DESC(rx_ring, rx_ring->next_to_clean);
 
-		qword = le64_to_cpu(rx_desc->wb.qword1.status_error_len);
-		rx_ptype = (qword & I40E_RXD_QW1_PTYPE_MASK) >>
-			   I40E_RXD_QW1_PTYPE_SHIFT;
-		rx_status = (qword & I40E_RXD_QW1_STATUS_MASK) >>
-			    I40E_RXD_QW1_STATUS_SHIFT;
-
-		if (!(rx_status & BIT(I40E_RX_DESC_STATUS_DD_SHIFT)))
-			break;
-
 		/* status_error_len will always be zero for unused descriptors
 		 * because it's cleared in cleanup, and overlaps with hdr_addr
 		 * which is always zero because packet split isn't used, if the
 		 * hardware wrote DD then it will be non-zero
 		 */
-		if (!rx_desc->wb.qword1.status_error_len)
+		if (!i40e_test_staterr(rx_desc,
+				       BIT(I40E_RX_DESC_STATUS_DD_SHIFT)))
 			break;
 
 		/* This memory barrier is needed to keep us from reading
@@ -1803,6 +1793,10 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
 		/* probably a little skewed due to removing CRC */
 		total_rx_bytes += skb->len;
 
+		qword = le64_to_cpu(rx_desc->wb.qword1.status_error_len);
+		rx_ptype = (qword & I40E_RXD_QW1_PTYPE_MASK) >>
+			   I40E_RXD_QW1_PTYPE_SHIFT;
+
 		/* populate checksum, VLAN, and protocol */
 		i40e_process_skb_fields(rx_ring, rx_desc, skb, rx_ptype);
 
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index dd8ad6b..e2d3622 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -705,7 +705,6 @@ bool i40evf_alloc_rx_buffers(struct i40e_ring *rx_ring, u16 cleaned_count)
 		 * because each write-back erases this info.
 		 */
 		rx_desc->read.pkt_addr = cpu_to_le64(bi->dma + bi->page_offset);
-		rx_desc->read.hdr_addr = 0;
 
 		rx_desc++;
 		bi++;
@@ -1209,7 +1208,6 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
 	while (likely(total_rx_packets < budget)) {
 		union i40e_rx_desc *rx_desc;
 		struct sk_buff *skb;
-		u32 rx_status;
 		u16 vlan_tag;
 		u8 rx_ptype;
 		u64 qword;
@@ -1223,21 +1221,13 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
 
 		rx_desc = I40E_RX_DESC(rx_ring, rx_ring->next_to_clean);
 
-		qword = le64_to_cpu(rx_desc->wb.qword1.status_error_len);
-		rx_ptype = (qword & I40E_RXD_QW1_PTYPE_MASK) >>
-			   I40E_RXD_QW1_PTYPE_SHIFT;
-		rx_status = (qword & I40E_RXD_QW1_STATUS_MASK) >>
-			    I40E_RXD_QW1_STATUS_SHIFT;
-
-		if (!(rx_status & BIT(I40E_RX_DESC_STATUS_DD_SHIFT)))
-			break;
-
 		/* status_error_len will always be zero for unused descriptors
 		 * because it's cleared in cleanup, and overlaps with hdr_addr
 		 * which is always zero because packet split isn't used, if the
 		 * hardware wrote DD then it will be non-zero
 		 */
-		if (!rx_desc->wb.qword1.status_error_len)
+		if (!i40e_test_staterr(rx_desc,
+				       BIT(I40E_RX_DESC_STATUS_DD_SHIFT)))
 			break;
 
 		/* This memory barrier is needed to keep us from reading
@@ -1271,6 +1261,10 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
 		/* probably a little skewed due to removing CRC */
 		total_rx_bytes += skb->len;
 
+		qword = le64_to_cpu(rx_desc->wb.qword1.status_error_len);
+		rx_ptype = (qword & I40E_RXD_QW1_PTYPE_MASK) >>
+			   I40E_RXD_QW1_PTYPE_SHIFT;
+
 		/* populate checksum, VLAN, and protocol */
 		i40evf_process_skb_fields(rx_ring, rx_desc, skb, rx_ptype);
 
-- 
2.7.4

^ permalink raw reply related

* [net-next 16/17] i40e: Fix bit logic error in failure case
From: Jeff Kirsher @ 2016-10-29  7:30 UTC (permalink / raw)
  To: davem
  Cc: David Ertman, netdev, nhorman, sassmann, jogreene,
	guru.anbalagane, Jeff Kirsher
In-Reply-To: <1477726258-111563-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: David Ertman <david.m.ertman@intel.com>

Patch a036244c0686 "i40e: Fix kernel panic on enable/disable LLDP"
introduced an error in bit logic.

Originally this bit manipulation was meant to clear two bits to indicate
that DCB was not enabled or capable. An "&" was incorrectly used instead
of an "|" bit operator to combine the two bitmasks into one.  This also
created a static checker error since the resultant code was a no-op.

This patch fixes the error by using the correct bit-wise operator.

Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 00c322d..0c2328d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -10970,7 +10970,7 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	err = i40e_init_pf_dcb(pf);
 	if (err) {
 		dev_info(&pdev->dev, "DCB init failed %d, disabled\n", err);
-		pf->flags &= ~(I40E_FLAG_DCB_CAPABLE & I40E_FLAG_DCB_ENABLED);
+		pf->flags &= ~(I40E_FLAG_DCB_CAPABLE | I40E_FLAG_DCB_ENABLED);
 		/* Continue without DCB enabled */
 	}
 #endif /* CONFIG_I40E_DCB */
-- 
2.7.4

^ permalink raw reply related

* [net-next 10/17] i40e: Make struct i40e_stats const
From: Jeff Kirsher @ 2016-10-29  7:30 UTC (permalink / raw)
  To: davem
  Cc: Joe Perches, netdev, nhorman, sassmann, jogreene, guru.anbalagane,
	Jeff Kirsher
In-Reply-To: <1477726258-111563-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Joe Perches <joe@perches.com>

Move some data to text

$ size drivers/net/ethernet/intel/i40e/i40e_ethtool.o*
   text	   data	    bss	    dec	    hex	filename
  25012	      0	     32	  25044	   61d4	drivers/net/ethernet/intel/i40e/i40e_ethtool.o.new
  22868	   2120	     32	  25020	   61bc	drivers/net/ethernet/intel/i40e/i40e_ethtool.o.old

Signed-off-by: Joe Perches <joe@perches.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 92bc884..3a1f91e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -104,7 +104,7 @@ static const struct i40e_stats i40e_gstrings_misc_stats[] = {
  * The PF_STATs are appended to the netdev stats only when ethtool -S
  * is queried on the base PF netdev, not on the VMDq or FCoE netdev.
  */
-static struct i40e_stats i40e_gstrings_stats[] = {
+static const struct i40e_stats i40e_gstrings_stats[] = {
 	I40E_PF_STAT("rx_bytes", stats.eth.rx_bytes),
 	I40E_PF_STAT("tx_bytes", stats.eth.tx_bytes),
 	I40E_PF_STAT("rx_unicast", stats.eth.rx_unicast),
-- 
2.7.4

^ permalink raw reply related

* [net-next 07/17] i40e: group base mode VF offload flags
From: Jeff Kirsher @ 2016-10-29  7:30 UTC (permalink / raw)
  To: davem
  Cc: Preethi Banala, netdev, nhorman, sassmann, jogreene,
	guru.anbalagane, Jeff Kirsher
In-Reply-To: <1477726258-111563-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Preethi Banala <preethi.banala@intel.com>

Group together the minimum set of offload capabilities that are always
supported by VF in base mode. This define would be used by PF to make
sure VF in base mode gets minimum of base capabilities .

Change-ID: Id5e8f22ba169c8f0a38d22fc36b2cb531c02582c
Signed-off-by: Preethi Banala <preethi.banala@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl.h   | 4 ++++
 drivers/net/ethernet/intel/i40evf/i40e_virtchnl.h | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl.h b/drivers/net/ethernet/intel/i40e/i40e_virtchnl.h
index f861d31..974ba2b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl.h
@@ -165,6 +165,10 @@ struct i40e_virtchnl_vsi_resource {
 #define I40E_VIRTCHNL_VF_OFFLOAD_RSS_PF		0X00080000
 #define I40E_VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM	0X00100000
 
+#define I40E_VF_BASE_MODE_OFFLOADS (I40E_VIRTCHNL_VF_OFFLOAD_L2 | \
+				    I40E_VIRTCHNL_VF_OFFLOAD_VLAN | \
+				    I40E_VIRTCHNL_VF_OFFLOAD_RSS_PF)
+
 struct i40e_virtchnl_vf_resource {
 	u16 num_vsis;
 	u16 num_queue_pairs;
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_virtchnl.h b/drivers/net/ethernet/intel/i40evf/i40e_virtchnl.h
index bd691ad..fc374f8 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_virtchnl.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_virtchnl.h
@@ -162,6 +162,10 @@ struct i40e_virtchnl_vsi_resource {
 #define I40E_VIRTCHNL_VF_OFFLOAD_RSS_PF		0X00080000
 #define I40E_VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM	0X00100000
 
+#define I40E_VF_BASE_MODE_OFFLOADS (I40E_VIRTCHNL_VF_OFFLOAD_L2 | \
+				    I40E_VIRTCHNL_VF_OFFLOAD_VLAN | \
+				    I40E_VIRTCHNL_VF_OFFLOAD_RSS_PF)
+
 struct i40e_virtchnl_vf_resource {
 	u16 num_vsis;
 	u16 num_queue_pairs;
-- 
2.7.4

^ permalink raw reply related

* [net-next 03/17] i40e: Bit test mask correction
From: Jeff Kirsher @ 2016-10-29  7:30 UTC (permalink / raw)
  To: davem
  Cc: Filip Sadowski, netdev, nhorman, sassmann, jogreene,
	guru.anbalagane, Jeff Kirsher
In-Reply-To: <1477726258-111563-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Filip Sadowski <filip.sadowski@intel.com>

Incorrect bit mask was used for testing "get link status" response.
Instead of I40E_AQ_LSE_ENABLE (which is actually 0x03) it most probably
should be I40E_AQ_LSE_IS_ENABLED (which is defined as 0x01).

Change-ID: Ia199142906720507f847de3a33a25c61a9781b2f
Signed-off-by: Filip Sadowski <filip.sadowski@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c
index 2154a34..fe8100b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
@@ -1849,7 +1849,7 @@ i40e_status i40e_aq_get_link_info(struct i40e_hw *hw,
 	else
 		hw_link_info->crc_enable = false;
 
-	if (resp->command_flags & cpu_to_le16(I40E_AQ_LSE_ENABLE))
+	if (resp->command_flags & cpu_to_le16(I40E_AQ_LSE_IS_ENABLED))
 		hw_link_info->lse_enable = true;
 	else
 		hw_link_info->lse_enable = false;
-- 
2.7.4

^ permalink raw reply related

* [net-next 14/17] i40e: Removal of workaround for simple MAC address filter deletion
From: Jeff Kirsher @ 2016-10-29  7:30 UTC (permalink / raw)
  To: davem
  Cc: Filip Sadowski, netdev, nhorman, sassmann, jogreene,
	guru.anbalagane, Jeff Kirsher
In-Reply-To: <1477726258-111563-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Filip Sadowski <filip.sadowski@intel.com>

This is code refactoring. This patch removes the workaround which deleted
a default MAC filter added by the firmware when the interface was brought
up. This filter caused frames to pass disregarding the VLAN tagging.
It used to be automatically applied after reset in pre-SRA FW versions.
This workaround is not needed in production NICs and hence can be removed.

Change-ID: I129fe1aae1f17b5a224c9b29a996d916aa1be1ec
Signed-off-by: Filip Sadowski <filip.sadowski@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 41 -----------------------------
 1 file changed, 41 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index a4bae0a..2e787ff 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -1287,39 +1287,6 @@ int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, u8 *macaddr,
 }
 
 /**
- * i40e_rm_default_mac_filter - Remove the default MAC filter set by NVM
- * @vsi: the PF Main VSI - inappropriate for any other VSI
- * @macaddr: the MAC address
- *
- * Remove whatever filter the firmware set up so the driver can manage
- * its own filtering intelligently.
- **/
-static void i40e_rm_default_mac_filter(struct i40e_vsi *vsi, u8 *macaddr)
-{
-	struct i40e_aqc_remove_macvlan_element_data element;
-	struct i40e_pf *pf = vsi->back;
-
-	/* Only appropriate for the PF main VSI */
-	if (vsi->type != I40E_VSI_MAIN)
-		return;
-
-	memset(&element, 0, sizeof(element));
-	ether_addr_copy(element.mac_addr, macaddr);
-	element.vlan_tag = 0;
-	/* Ignore error returns, some firmware does it this way... */
-	element.flags = I40E_AQC_MACVLAN_DEL_PERFECT_MATCH;
-	i40e_aq_remove_macvlan(&pf->hw, vsi->seid, &element, 1, NULL);
-
-	memset(&element, 0, sizeof(element));
-	ether_addr_copy(element.mac_addr, macaddr);
-	element.vlan_tag = 0;
-	/* ...and some firmware does it this way. */
-	element.flags = I40E_AQC_MACVLAN_DEL_PERFECT_MATCH |
-			I40E_AQC_MACVLAN_DEL_IGNORE_VLAN;
-	i40e_aq_remove_macvlan(&pf->hw, vsi->seid, &element, 1, NULL);
-}
-
-/**
  * i40e_add_filter - Add a mac/vlan filter to the VSI
  * @vsi: the VSI to be searched
  * @macaddr: the MAC address
@@ -9218,12 +9185,6 @@ static int i40e_config_netdev(struct i40e_vsi *vsi)
 	if (vsi->type == I40E_VSI_MAIN) {
 		SET_NETDEV_DEV(netdev, &pf->pdev->dev);
 		ether_addr_copy(mac_addr, hw->mac.perm_addr);
-		/* The following steps are necessary to prevent reception
-		 * of tagged packets - some older NVM configurations load a
-		 * default a MAC-VLAN filter that accepts any tagged packet
-		 * which must be replaced by a normal filter.
-		 */
-		i40e_rm_default_mac_filter(vsi, mac_addr);
 		spin_lock_bh(&vsi->mac_filter_list_lock);
 		i40e_add_filter(vsi, mac_addr, I40E_VLAN_ANY, false, true);
 		spin_unlock_bh(&vsi->mac_filter_list_lock);
@@ -9741,8 +9702,6 @@ static struct i40e_vsi *i40e_vsi_reinit_setup(struct i40e_vsi *vsi)
 	pf->vsi[pf->lan_vsi]->tc_config.enabled_tc = 0;
 	pf->vsi[pf->lan_vsi]->seid = pf->main_vsi_seid;
 	i40e_vsi_config_tc(pf->vsi[pf->lan_vsi], enabled_tc);
-	if (vsi->type == I40E_VSI_MAIN)
-		i40e_rm_default_mac_filter(vsi, pf->hw.mac.perm_addr);
 
 	/* assign it some queues */
 	ret = i40e_alloc_rings(vsi);
-- 
2.7.4

^ permalink raw reply related

* [net-next 11/17] i40e: fix confusing dmesg info for ethtool -L option
From: Jeff Kirsher @ 2016-10-29  7:30 UTC (permalink / raw)
  To: davem
  Cc: Lihong Yang, netdev, nhorman, sassmann, jogreene, guru.anbalagane,
	Jeff Kirsher
In-Reply-To: <1477726258-111563-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Lihong Yang <lihong.yang@intel.com>

Ethtool -L option with the combined parameter is for changing the number of
multi-purpose channels of the specified network device. The pre-set maximum
for the combined channels is cpu dependent. Currently, for an i40e device,
when the user sets a value between 64 and the maximum that the cpu can
support for the combined parameter, the i40e driver displays the confusing
info in dmesg to only show 64 as the RSS count regardless of what the
accepted user input is as long as it is larger than 64.

This patch fixes the message in the i40e driver when the user uses
ethtool -L to change the number of the combined channels to consistently
display the user requested value if it is valid and accepted by ethtool.

Change-ID: Ia80a68bc844b779a49e0f76e7d3dcc915032d9af
Signed-off-by: Lihong Yang <lihong.yang@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index c25247f..a4bae0a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -8400,8 +8400,8 @@ int i40e_reconfig_rss_queues(struct i40e_pf *pf, int queue_count)
 
 		i40e_pf_config_rss(pf);
 	}
-	dev_info(&pf->pdev->dev, "RSS count/HW max RSS count:  %d/%d\n",
-		 pf->alloc_rss_size, pf->rss_size_max);
+	dev_info(&pf->pdev->dev, "User requested queue count/HW max RSS count:  %d/%d\n",
+		 vsi->req_queue_pairs, pf->rss_size_max);
 	return pf->alloc_rss_size;
 }
 
-- 
2.7.4

^ permalink raw reply related

* [net-next 08/17] i40e/i40evf: fix interrupt affinity bug
From: Jeff Kirsher @ 2016-10-29  7:30 UTC (permalink / raw)
  To: davem
  Cc: Alan Brady, netdev, nhorman, sassmann, jogreene, guru.anbalagane,
	Jeff Kirsher
In-Reply-To: <1477726258-111563-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Alan Brady <alan.brady@intel.com>

There exists a bug in which a 'perfect storm' can occur and cause
interrupts to fail to be correctly affinitized. This causes unexpected
behavior and has a substantial impact on performance when it happens.

The bug occurs if there is heavy traffic, any number of CPUs that have
an i40e interrupt are pegged at 100%, and the interrupt afffinity for
those CPUs is changed.  Instead of moving to the new CPU, the interrupt
continues to be polled while there is heavy traffic.

The bug is most readily realized as the driver is first brought up and
all interrupts start on CPU0. If there is heavy traffic and the
interrupt starts polling before the interrupt is affinitized, the
interrupt will be stuck on CPU0 until traffic stops. The bug, however,
can also be wrought out more simply by affinitizing all the interrupts
to a single CPU and then attempting to move any of those interrupts off
while there is heavy traffic.

This patch fixes the bug by registering for update notifications from
the kernel when the interrupt affinity changes. When that fires, we
cache the intended affinity mask. Then, while polling, if the cpu is
pegged at 100% and we failed to clean the rings, we check to make sure
we have the correct affinity and stop polling if we're firing on the
wrong CPU.  When the kernel successfully moves the interrupt, it will
start polling on the correct CPU. The performance impact is minimal
since the only time this section gets executed is when performance is
already compromised by the CPU.

Change-ID: I4410a880159b9dba1f8297aa72bef36dca34e830
Signed-off-by: Alan Brady <alan.brady@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e.h          |  2 +
 drivers/net/ethernet/intel/i40e/i40e_main.c     | 64 +++++++++++++++++-----
 drivers/net/ethernet/intel/i40e/i40e_txrx.c     | 36 ++++++++++---
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c   | 31 +++++++++--
 drivers/net/ethernet/intel/i40evf/i40evf.h      |  3 +-
 drivers/net/ethernet/intel/i40evf/i40evf_main.c | 71 +++++++++++++++++--------
 6 files changed, 159 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 34c7a5d..01cce5b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -607,6 +607,8 @@ struct i40e_q_vector {
 	unsigned long hung_detected; /* Set/Reset for hung_detection logic */
 
 	cpumask_t affinity_mask;
+	struct irq_affinity_notify affinity_notify;
+
 	struct rcu_head rcu;	/* to avoid race with update stats on free */
 	char name[I40E_INT_NAME_STR_LEN];
 	bool arm_wb_state;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 83edbe8..9382ba80 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -3317,6 +3317,33 @@ static irqreturn_t i40e_msix_clean_rings(int irq, void *data)
 }
 
 /**
+ * i40e_irq_affinity_notify - Callback for affinity changes
+ * @notify: context as to what irq was changed
+ * @mask: the new affinity mask
+ *
+ * This is a callback function used by the irq_set_affinity_notifier function
+ * so that we may register to receive changes to the irq affinity masks.
+ **/
+static void i40e_irq_affinity_notify(struct irq_affinity_notify *notify,
+				     const cpumask_t *mask)
+{
+	struct i40e_q_vector *q_vector =
+		container_of(notify, struct i40e_q_vector, affinity_notify);
+
+	q_vector->affinity_mask = *mask;
+}
+
+/**
+ * i40e_irq_affinity_release - Callback for affinity notifier release
+ * @ref: internal core kernel usage
+ *
+ * This is a callback function used by the irq_set_affinity_notifier function
+ * to inform the current notification subscriber that they will no longer
+ * receive notifications.
+ **/
+static void i40e_irq_affinity_release(struct kref *ref) {}
+
+/**
  * i40e_vsi_request_irq_msix - Initialize MSI-X interrupts
  * @vsi: the VSI being configured
  * @basename: name for the vector
@@ -3331,10 +3358,13 @@ static int i40e_vsi_request_irq_msix(struct i40e_vsi *vsi, char *basename)
 	int rx_int_idx = 0;
 	int tx_int_idx = 0;
 	int vector, err;
+	int irq_num;
 
 	for (vector = 0; vector < q_vectors; vector++) {
 		struct i40e_q_vector *q_vector = vsi->q_vectors[vector];
 
+		irq_num = pf->msix_entries[base + vector].vector;
+
 		if (q_vector->tx.ring && q_vector->rx.ring) {
 			snprintf(q_vector->name, sizeof(q_vector->name) - 1,
 				 "%s-%s-%d", basename, "TxRx", rx_int_idx++);
@@ -3349,7 +3379,7 @@ static int i40e_vsi_request_irq_msix(struct i40e_vsi *vsi, char *basename)
 			/* skip this unused q_vector */
 			continue;
 		}
-		err = request_irq(pf->msix_entries[base + vector].vector,
+		err = request_irq(irq_num,
 				  vsi->irq_handler,
 				  0,
 				  q_vector->name,
@@ -3359,9 +3389,13 @@ static int i40e_vsi_request_irq_msix(struct i40e_vsi *vsi, char *basename)
 				 "MSIX request_irq failed, error: %d\n", err);
 			goto free_queue_irqs;
 		}
+
+		/* register for affinity change notifications */
+		q_vector->affinity_notify.notify = i40e_irq_affinity_notify;
+		q_vector->affinity_notify.release = i40e_irq_affinity_release;
+		irq_set_affinity_notifier(irq_num, &q_vector->affinity_notify);
 		/* assign the mask for this irq */
-		irq_set_affinity_hint(pf->msix_entries[base + vector].vector,
-				      &q_vector->affinity_mask);
+		irq_set_affinity_hint(irq_num, &q_vector->affinity_mask);
 	}
 
 	vsi->irqs_ready = true;
@@ -3370,10 +3404,10 @@ static int i40e_vsi_request_irq_msix(struct i40e_vsi *vsi, char *basename)
 free_queue_irqs:
 	while (vector) {
 		vector--;
-		irq_set_affinity_hint(pf->msix_entries[base + vector].vector,
-				      NULL);
-		free_irq(pf->msix_entries[base + vector].vector,
-			 &(vsi->q_vectors[vector]));
+		irq_num = pf->msix_entries[base + vector].vector;
+		irq_set_affinity_notifier(irq_num, NULL);
+		irq_set_affinity_hint(irq_num, NULL);
+		free_irq(irq_num, &vsi->q_vectors[vector]);
 	}
 	return err;
 }
@@ -4012,19 +4046,23 @@ static void i40e_vsi_free_irq(struct i40e_vsi *vsi)
 
 		vsi->irqs_ready = false;
 		for (i = 0; i < vsi->num_q_vectors; i++) {
-			u16 vector = i + base;
+			int irq_num;
+			u16 vector;
+
+			vector = i + base;
+			irq_num = pf->msix_entries[vector].vector;
 
 			/* free only the irqs that were actually requested */
 			if (!vsi->q_vectors[i] ||
 			    !vsi->q_vectors[i]->num_ringpairs)
 				continue;
 
+			/* clear the affinity notifier in the IRQ descriptor */
+			irq_set_affinity_notifier(irq_num, NULL);
 			/* clear the affinity_mask in the IRQ descriptor */
-			irq_set_affinity_hint(pf->msix_entries[vector].vector,
-					      NULL);
-			synchronize_irq(pf->msix_entries[vector].vector);
-			free_irq(pf->msix_entries[vector].vector,
-				 vsi->q_vectors[i]);
+			irq_set_affinity_hint(irq_num, NULL);
+			synchronize_irq(irq_num);
+			free_irq(irq_num, vsi->q_vectors[i]);
 
 			/* Tear down the interrupt queue link list
 			 *
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 7d160c9..48e6533 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -1999,12 +1999,25 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)
 
 	/* If work not completed, return budget and polling will return */
 	if (!clean_complete) {
+		const cpumask_t *aff_mask = &q_vector->affinity_mask;
+		int cpu_id = smp_processor_id();
+
+		/* It is possible that the interrupt affinity has changed but,
+		 * if the cpu is pegged at 100%, polling will never exit while
+		 * traffic continues and the interrupt will be stuck on this
+		 * cpu.  We check to make sure affinity is correct before we
+		 * continue to poll, otherwise we must stop polling so the
+		 * interrupt can move to the correct cpu.
+		 */
+		if (likely(cpumask_test_cpu(cpu_id, aff_mask) ||
+			   !(vsi->back->flags & I40E_FLAG_MSIX_ENABLED))) {
 tx_only:
-		if (arm_wb) {
-			q_vector->tx.ring[0].tx_stats.tx_force_wb++;
-			i40e_enable_wb_on_itr(vsi, q_vector);
+			if (arm_wb) {
+				q_vector->tx.ring[0].tx_stats.tx_force_wb++;
+				i40e_enable_wb_on_itr(vsi, q_vector);
+			}
+			return budget;
 		}
-		return budget;
 	}
 
 	if (vsi->back->flags & I40E_TXR_FLAGS_WB_ON_ITR)
@@ -2012,11 +2025,18 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)
 
 	/* Work is done so exit the polling mode and re-enable the interrupt */
 	napi_complete_done(napi, work_done);
-	if (vsi->back->flags & I40E_FLAG_MSIX_ENABLED) {
-		i40e_update_enable_itr(vsi, q_vector);
-	} else { /* Legacy mode */
+
+	/* If we're prematurely stopping polling to fix the interrupt
+	 * affinity we want to make sure polling starts back up so we
+	 * issue a call to i40e_force_wb which triggers a SW interrupt.
+	 */
+	if (!clean_complete)
+		i40e_force_wb(vsi, q_vector);
+	else if (!(vsi->back->flags & I40E_FLAG_MSIX_ENABLED))
 		i40e_irq_dynamic_enable_icr0(vsi->back, false);
-	}
+	else
+		i40e_update_enable_itr(vsi, q_vector);
+
 	return 0;
 }
 
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index 75f2a2c..dd8ad6b 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -1461,12 +1461,24 @@ int i40evf_napi_poll(struct napi_struct *napi, int budget)
 
 	/* If work not completed, return budget and polling will return */
 	if (!clean_complete) {
+		const cpumask_t *aff_mask = &q_vector->affinity_mask;
+		int cpu_id = smp_processor_id();
+
+		/* It is possible that the interrupt affinity has changed but,
+		 * if the cpu is pegged at 100%, polling will never exit while
+		 * traffic continues and the interrupt will be stuck on this
+		 * cpu.  We check to make sure affinity is correct before we
+		 * continue to poll, otherwise we must stop polling so the
+		 * interrupt can move to the correct cpu.
+		 */
+		if (likely(cpumask_test_cpu(cpu_id, aff_mask))) {
 tx_only:
-		if (arm_wb) {
-			q_vector->tx.ring[0].tx_stats.tx_force_wb++;
-			i40e_enable_wb_on_itr(vsi, q_vector);
+			if (arm_wb) {
+				q_vector->tx.ring[0].tx_stats.tx_force_wb++;
+				i40e_enable_wb_on_itr(vsi, q_vector);
+			}
+			return budget;
 		}
-		return budget;
 	}
 
 	if (vsi->back->flags & I40E_TXR_FLAGS_WB_ON_ITR)
@@ -1474,7 +1486,16 @@ int i40evf_napi_poll(struct napi_struct *napi, int budget)
 
 	/* Work is done so exit the polling mode and re-enable the interrupt */
 	napi_complete_done(napi, work_done);
-	i40e_update_enable_itr(vsi, q_vector);
+
+	/* If we're prematurely stopping polling to fix the interrupt
+	 * affinity we want to make sure polling starts back up so we
+	 * issue a call to i40evf_force_wb which triggers a SW interrupt.
+	 */
+	if (!clean_complete)
+		i40evf_force_wb(vsi, q_vector);
+	else
+		i40e_update_enable_itr(vsi, q_vector);
+
 	return 0;
 }
 
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf.h b/drivers/net/ethernet/intel/i40evf/i40evf.h
index c5fd724..fffe4cf 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf.h
+++ b/drivers/net/ethernet/intel/i40evf/i40evf.h
@@ -107,7 +107,8 @@ struct i40e_q_vector {
 	int v_idx;	/* vector index in list */
 	char name[IFNAMSIZ + 9];
 	bool arm_wb_state;
-	cpumask_var_t affinity_mask;
+	cpumask_t affinity_mask;
+	struct irq_affinity_notify affinity_notify;
 };
 
 /* Helper macros to switch between ints/sec and what the register uses.
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 777eb29..e95e873 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -496,6 +496,33 @@ static void i40evf_netpoll(struct net_device *netdev)
 
 #endif
 /**
+ * i40evf_irq_affinity_notify - Callback for affinity changes
+ * @notify: context as to what irq was changed
+ * @mask: the new affinity mask
+ *
+ * This is a callback function used by the irq_set_affinity_notifier function
+ * so that we may register to receive changes to the irq affinity masks.
+ **/
+static void i40evf_irq_affinity_notify(struct irq_affinity_notify *notify,
+				       const cpumask_t *mask)
+{
+	struct i40e_q_vector *q_vector =
+		container_of(notify, struct i40e_q_vector, affinity_notify);
+
+	q_vector->affinity_mask = *mask;
+}
+
+/**
+ * i40evf_irq_affinity_release - Callback for affinity notifier release
+ * @ref: internal core kernel usage
+ *
+ * This is a callback function used by the irq_set_affinity_notifier function
+ * to inform the current notification subscriber that they will no longer
+ * receive notifications.
+ **/
+static void i40evf_irq_affinity_release(struct kref *ref) {}
+
+/**
  * i40evf_request_traffic_irqs - Initialize MSI-X interrupts
  * @adapter: board private structure
  *
@@ -507,6 +534,7 @@ i40evf_request_traffic_irqs(struct i40evf_adapter *adapter, char *basename)
 {
 	int vector, err, q_vectors;
 	int rx_int_idx = 0, tx_int_idx = 0;
+	int irq_num;
 
 	i40evf_irq_disable(adapter);
 	/* Decrement for Other and TCP Timer vectors */
@@ -514,6 +542,7 @@ i40evf_request_traffic_irqs(struct i40evf_adapter *adapter, char *basename)
 
 	for (vector = 0; vector < q_vectors; vector++) {
 		struct i40e_q_vector *q_vector = &adapter->q_vectors[vector];
+		irq_num = adapter->msix_entries[vector + NONQ_VECS].vector;
 
 		if (q_vector->tx.ring && q_vector->rx.ring) {
 			snprintf(q_vector->name, sizeof(q_vector->name) - 1,
@@ -532,21 +561,23 @@ i40evf_request_traffic_irqs(struct i40evf_adapter *adapter, char *basename)
 			/* skip this unused q_vector */
 			continue;
 		}
-		err = request_irq(
-			adapter->msix_entries[vector + NONQ_VECS].vector,
-			i40evf_msix_clean_rings,
-			0,
-			q_vector->name,
-			q_vector);
+		err = request_irq(irq_num,
+				  i40evf_msix_clean_rings,
+				  0,
+				  q_vector->name,
+				  q_vector);
 		if (err) {
 			dev_info(&adapter->pdev->dev,
 				 "Request_irq failed, error: %d\n", err);
 			goto free_queue_irqs;
 		}
+		/* register for affinity change notifications */
+		q_vector->affinity_notify.notify = i40evf_irq_affinity_notify;
+		q_vector->affinity_notify.release =
+						   i40evf_irq_affinity_release;
+		irq_set_affinity_notifier(irq_num, &q_vector->affinity_notify);
 		/* assign the mask for this irq */
-		irq_set_affinity_hint(
-			adapter->msix_entries[vector + NONQ_VECS].vector,
-			q_vector->affinity_mask);
+		irq_set_affinity_hint(irq_num, &q_vector->affinity_mask);
 	}
 
 	return 0;
@@ -554,11 +585,10 @@ i40evf_request_traffic_irqs(struct i40evf_adapter *adapter, char *basename)
 free_queue_irqs:
 	while (vector) {
 		vector--;
-		irq_set_affinity_hint(
-			adapter->msix_entries[vector + NONQ_VECS].vector,
-			NULL);
-		free_irq(adapter->msix_entries[vector + NONQ_VECS].vector,
-			 &adapter->q_vectors[vector]);
+		irq_num = adapter->msix_entries[vector + NONQ_VECS].vector;
+		irq_set_affinity_notifier(irq_num, NULL);
+		irq_set_affinity_hint(irq_num, NULL);
+		free_irq(irq_num, &adapter->q_vectors[vector]);
 	}
 	return err;
 }
@@ -599,16 +629,15 @@ static int i40evf_request_misc_irq(struct i40evf_adapter *adapter)
  **/
 static void i40evf_free_traffic_irqs(struct i40evf_adapter *adapter)
 {
-	int i;
-	int q_vectors;
+	int vector, irq_num, q_vectors;
 
 	q_vectors = adapter->num_msix_vectors - NONQ_VECS;
 
-	for (i = 0; i < q_vectors; i++) {
-		irq_set_affinity_hint(adapter->msix_entries[i+1].vector,
-				      NULL);
-		free_irq(adapter->msix_entries[i+1].vector,
-			 &adapter->q_vectors[i]);
+	for (vector = 0; vector < q_vectors; vector++) {
+		irq_num = adapter->msix_entries[vector + NONQ_VECS].vector;
+		irq_set_affinity_notifier(irq_num, NULL);
+		irq_set_affinity_hint(irq_num, NULL);
+		free_irq(irq_num, &adapter->q_vectors[vector]);
 	}
 }
 
-- 
2.7.4

^ permalink raw reply related

* [net-next 04/17] i40e: Remove unused function i40e_vsi_lookup
From: Jeff Kirsher @ 2016-10-29  7:30 UTC (permalink / raw)
  To: davem
  Cc: Alexander Duyck, netdev, nhorman, sassmann, jogreene,
	guru.anbalagane, Jeff Kirsher
In-Reply-To: <1477726258-111563-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Alexander Duyck <alexander.h.duyck@intel.com>

The function is not used so there is no need to carry it forward.  I have
plans to add a slightly different function that can be inlined to handle
the same kind of functionality.

Change-ID: Ie2dfcb189dc75e5fbc156bac23003e3b4210ae0f
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e.h        |  2 --
 drivers/net/ethernet/intel/i40e/i40e_client.c | 31 ---------------------------
 2 files changed, 33 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 2030d7c..34c7a5d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -728,8 +728,6 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi);
 struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf, u8 type,
 				u16 uplink, u32 param1);
 int i40e_vsi_release(struct i40e_vsi *vsi);
-struct i40e_vsi *i40e_vsi_lookup(struct i40e_pf *pf, enum i40e_vsi_type type,
-				 struct i40e_vsi *start_vsi);
 #ifdef I40E_FCOE
 void i40e_vsi_setup_queue_map(struct i40e_vsi *vsi,
 			      struct i40e_vsi_context *ctxt,
diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c b/drivers/net/ethernet/intel/i40e/i40e_client.c
index 6ffac03..417ac16 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_client.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
@@ -406,37 +406,6 @@ int i40e_vf_client_capable(struct i40e_pf *pf, u32 vf_id,
 }
 
 /**
- * i40e_vsi_lookup - finds a matching VSI from the PF list starting at start_vsi
- * @pf: board private structure
- * @type: vsi type
- * @start_vsi: a VSI pointer from where to start the search
- *
- * Returns non NULL on success or NULL for failure
- **/
-struct i40e_vsi *i40e_vsi_lookup(struct i40e_pf *pf,
-				 enum i40e_vsi_type type,
-				 struct i40e_vsi *start_vsi)
-{
-	struct i40e_vsi *vsi;
-	int i = 0;
-
-	if (start_vsi) {
-		for (i = 0; i < pf->num_alloc_vsi; i++) {
-			vsi = pf->vsi[i];
-			if (vsi == start_vsi)
-				break;
-		}
-	}
-	for (; i < pf->num_alloc_vsi; i++) {
-		vsi = pf->vsi[i];
-		if (vsi && vsi->type == type)
-			return vsi;
-	}
-
-	return NULL;
-}
-
-/**
  * i40e_client_add_instance - add a client instance struct to the instance list
  * @pf: pointer to the board struct
  * @client: pointer to a client struct in the client list.
-- 
2.7.4

^ permalink raw reply related

* [net-next 06/17] i40e: reopen client after reset
From: Jeff Kirsher @ 2016-10-29  7:30 UTC (permalink / raw)
  To: davem
  Cc: Mitch Williams, netdev, nhorman, sassmann, jogreene,
	guru.anbalagane, Jeff Kirsher
In-Reply-To: <1477726258-111563-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Mitch Williams <mitch.a.williams@intel.com>

Allow the client interface to reopen existing clients if they were
closed. This allows clients to recover from reset, which is essential
for supporting VF RDMA. In one instance, the driver was not clearing the
open bit when the client was closed. Add the code to clear this bit so
that the state is accurate and the driver will not attempt to reopen
already-open clients. Remove the ref_cnt variable; it was just getting
in the way and was not being used consistently.

Change-ID: Ic71af4553b096963ac0c56a997f887c9a4ed162d
Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_client.c | 47 ++++++++++-----------------
 drivers/net/ethernet/intel/i40e/i40e_client.h |  2 --
 2 files changed, 17 insertions(+), 32 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c b/drivers/net/ethernet/intel/i40e/i40e_client.c
index 417ac16..7fe72ab 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_client.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
@@ -287,6 +287,7 @@ void i40e_notify_client_of_netdev_close(struct i40e_vsi *vsi, bool reset)
 			}
 			cdev->client->ops->close(&cdev->lan_info, cdev->client,
 						 reset);
+			clear_bit(__I40E_CLIENT_INSTANCE_OPENED, &cdev->state);
 			i40e_client_release_qvlist(&cdev->lan_info);
 		}
 	}
@@ -544,28 +545,27 @@ void i40e_client_subtask(struct i40e_pf *pf)
 			continue;
 
 		if (!existing) {
-			/* Also up the ref_cnt for no. of instances of this
-			 * client.
-			 */
-			atomic_inc(&client->ref_cnt);
 			dev_info(&pf->pdev->dev, "Added instance of Client %s to PF%d bus=0x%02x func=0x%02x\n",
 				 client->name, pf->hw.pf_id,
 				 pf->hw.bus.device, pf->hw.bus.func);
-			mutex_lock(&i40e_client_instance_mutex);
-			atomic_inc(&cdev->ref_cnt);
+		}
+
+		mutex_lock(&i40e_client_instance_mutex);
+		if (!test_bit(__I40E_CLIENT_INSTANCE_OPENED,
+			      &cdev->state)) {
+			/* Send an Open request to the client */
 			if (client->ops && client->ops->open)
 				ret = client->ops->open(&cdev->lan_info,
 							client);
-			atomic_dec(&cdev->ref_cnt);
-			if (ret < 0) {
-				mutex_unlock(&i40e_client_instance_mutex);
+			if (!ret) {
+				set_bit(__I40E_CLIENT_INSTANCE_OPENED,
+					&cdev->state);
+			} else {
+				/* remove client instance */
 				i40e_client_del_instance(pf, client);
-				atomic_dec(&client->ref_cnt);
-				continue;
 			}
-			set_bit(__I40E_CLIENT_INSTANCE_OPENED, &cdev->state);
-			mutex_unlock(&i40e_client_instance_mutex);
 		}
+		mutex_unlock(&i40e_client_instance_mutex);
 	}
 	mutex_unlock(&i40e_client_mutex);
 }
@@ -660,10 +660,6 @@ static int i40e_client_release(struct i40e_client *client)
 			continue;
 		pf = (struct i40e_pf *)cdev->lan_info.pf;
 		if (test_bit(__I40E_CLIENT_INSTANCE_OPENED, &cdev->state)) {
-			if (atomic_read(&cdev->ref_cnt) > 0) {
-				ret = I40E_ERR_NOT_READY;
-				goto out;
-			}
 			if (client->ops && client->ops->close)
 				client->ops->close(&cdev->lan_info, client,
 						   false);
@@ -676,11 +672,9 @@ static int i40e_client_release(struct i40e_client *client)
 		}
 		/* delete the client instance from the list */
 		list_move(&cdev->list, &cdevs_tmp);
-		atomic_dec(&client->ref_cnt);
 		dev_info(&pf->pdev->dev, "Deleted client instance of Client %s\n",
 			 client->name);
 	}
-out:
 	mutex_unlock(&i40e_client_instance_mutex);
 
 	/* free the client device and release its vsi */
@@ -1006,17 +1000,10 @@ int i40e_unregister_client(struct i40e_client *client)
 		ret = -ENODEV;
 		goto out;
 	}
-	if (atomic_read(&client->ref_cnt) == 0) {
-		clear_bit(__I40E_CLIENT_REGISTERED, &client->state);
-		list_del(&client->list);
-		pr_info("i40e: Unregistered client %s with return code %d\n",
-			client->name, ret);
-	} else {
-		ret = I40E_ERR_NOT_READY;
-		pr_err("i40e: Client %s failed unregister - client has open instances\n",
-		       client->name);
-	}
-
+	clear_bit(__I40E_CLIENT_REGISTERED, &client->state);
+	list_del(&client->list);
+	pr_info("i40e: Unregistered client %s with return code %d\n",
+		client->name, ret);
 out:
 	mutex_unlock(&i40e_client_mutex);
 	return ret;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.h b/drivers/net/ethernet/intel/i40e/i40e_client.h
index 38a6c36..528bd79 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_client.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_client.h
@@ -203,8 +203,6 @@ struct i40e_client_instance {
 	struct i40e_info lan_info;
 	struct i40e_client *client;
 	unsigned long  state;
-	/* A count of all the in-progress calls to the client */
-	atomic_t ref_cnt;
 };
 
 struct i40e_client {
-- 
2.7.4

^ permalink raw reply related

* [net-next 00/17][pull request] 40GbE Intel Wired LAN Driver Updates 2016-10-28
From: Jeff Kirsher @ 2016-10-29  7:30 UTC (permalink / raw)
  To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann, jogreene,
	guru.anbalagane

This series contains updates to i40e and i40evf only.

Carolyn provides a couple of fixes, first resolving a problem in the
client interface that was causing random stack traces in the RDMA driver
which was due to a timing related NULL pointer dereference.  Fixed a
problem where it could take a very long time to print the link down
notification, by changing how often we update link info from firmware.

Alex provides a number of changes, first is a re-write of the bust wait
loop in the Flow Director transmit function to reduce code size.  Cleans
up unused code in favor of the same functionality which can be inlined.
Dropped the functionality for SCTP since we cannot currently support it.
Cleans up redundant code in the receive clean-up path.  Finally cleaned
up the convoluted configuration for how the driver handled the debug
flags contained in msg_level.

Filip fixes an incorrect bit mask which was being used for testing the
"get link status".  Cleaned up a workaround that is no longer needed
for production NICs and was causing frames to pass while disregarding
the VLAN tagging.

Mitch brings another fix for the client interface supporting the VF RDMA
driver to allow clients to recover from reset by re-opening existing
clients.

Alan fixes a bug in which a "perfect storm" can occur and cause interrupts
to fail to be correctly affinitized.

Lihong fixes a confusing dmesg reported when users were using ethtool -L
option.

The following are changes since commit b09edbd07f876c9f7046c4aae1831e58919cffea:
  net caif: insert missing spaces in pr_* messages and unbreak multi-line strings
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 40GbE

Alan Brady (1):
  i40e/i40evf: fix interrupt affinity bug

Alexander Duyck (5):
  i40e: Rewrite Flow Director busy wait loop
  i40e: Remove unused function i40e_vsi_lookup
  i40e: Drop code for unsupported flow types
  i40e: Drop redundant Rx descriptor processing code
  i40e: Clean up handling of msglevel flags and debug parameter

Bimmy Pujari (2):
  i40e/i40evf: Changed version from 1.6.16 to 1.6.19
  i40e/i40evf: Changed version from 1.6.19 to 1.6.21

Carolyn Wyborny (2):
  i40e: Fix client interaction
  i40e: Fix for long link down notification time

David Ertman (1):
  i40e: Fix bit logic error in failure case

Filip Sadowski (2):
  i40e: Bit test mask correction
  i40e: Removal of workaround for simple MAC address filter deletion

Joe Perches (1):
  i40e: Make struct i40e_stats const

Lihong Yang (1):
  i40e: fix confusing dmesg info for ethtool -L option

Mitch Williams (1):
  i40e: reopen client after reset

Preethi Banala (1):
  i40e: group base mode VF offload flags

 drivers/net/ethernet/intel/i40e/i40e.h            |   4 +-
 drivers/net/ethernet/intel/i40e/i40e_client.c     |  85 +++-----------
 drivers/net/ethernet/intel/i40e/i40e_client.h     |   2 -
 drivers/net/ethernet/intel/i40e/i40e_common.c     |   7 +-
 drivers/net/ethernet/intel/i40e/i40e_debugfs.c    |  19 ---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c    |   9 +-
 drivers/net/ethernet/intel/i40e/i40e_main.c       | 136 ++++++++++------------
 drivers/net/ethernet/intel/i40e/i40e_txrx.c       |  98 +++++++---------
 drivers/net/ethernet/intel/i40e/i40e_virtchnl.h   |   4 +
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c     |  49 +++++---
 drivers/net/ethernet/intel/i40evf/i40e_virtchnl.h |   4 +
 drivers/net/ethernet/intel/i40evf/i40evf.h        |   3 +-
 drivers/net/ethernet/intel/i40evf/i40evf_main.c   |  73 ++++++++----
 13 files changed, 232 insertions(+), 261 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [net-next 05/17] i40e: Drop code for unsupported flow types
From: Jeff Kirsher @ 2016-10-29  7:30 UTC (permalink / raw)
  To: davem
  Cc: Alexander Duyck, netdev, nhorman, sassmann, jogreene,
	guru.anbalagane, Jeff Kirsher
In-Reply-To: <1477726258-111563-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Alexander Duyck <alexander.h.duyck@intel.com>

We cannot currently support SCTP in the hardware, and IPV4_FLOW is not used
anywhere by the software so we can go through and drop the functionality
related to these two flow types.

In addition we cannot support masking based on the protocol value so if the
user is expecting a value other than TCP or UDP we should simply return an
error rather then trying to allocate a filter for a rule that will only
partially match what the user requested.

Change-ID: I10d52bb97d8104d76255fe244551814ff9531a63
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c | 31 +++++------------------------
 1 file changed, 5 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index af36c44..7d160c9 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -330,22 +330,6 @@ static int i40e_add_del_fdir_tcpv4(struct i40e_vsi *vsi,
 	return err ? -EOPNOTSUPP : 0;
 }
 
-/**
- * i40e_add_del_fdir_sctpv4 - Add/Remove SCTPv4 Flow Director filters for
- * a specific flow spec
- * @vsi: pointer to the targeted VSI
- * @fd_data: the flow director data required for the FDir descriptor
- * @add: true adds a filter, false removes it
- *
- * Returns 0 if the filters were successfully added or removed
- **/
-static int i40e_add_del_fdir_sctpv4(struct i40e_vsi *vsi,
-				    struct i40e_fdir_filter *fd_data,
-				    bool add)
-{
-	return -EOPNOTSUPP;
-}
-
 #define I40E_IP_DUMMY_PACKET_LEN 34
 /**
  * i40e_add_del_fdir_ipv4 - Add/Remove IPv4 Flow Director filters for
@@ -428,12 +412,6 @@ int i40e_add_del_fdir(struct i40e_vsi *vsi,
 	case UDP_V4_FLOW:
 		ret = i40e_add_del_fdir_udpv4(vsi, input, add);
 		break;
-	case SCTP_V4_FLOW:
-		ret = i40e_add_del_fdir_sctpv4(vsi, input, add);
-		break;
-	case IPV4_FLOW:
-		ret = i40e_add_del_fdir_ipv4(vsi, input, add);
-		break;
 	case IP_USER_FLOW:
 		switch (input->ip4_proto) {
 		case IPPROTO_TCP:
@@ -442,15 +420,16 @@ int i40e_add_del_fdir(struct i40e_vsi *vsi,
 		case IPPROTO_UDP:
 			ret = i40e_add_del_fdir_udpv4(vsi, input, add);
 			break;
-		case IPPROTO_SCTP:
-			ret = i40e_add_del_fdir_sctpv4(vsi, input, add);
-			break;
-		default:
+		case IPPROTO_IP:
 			ret = i40e_add_del_fdir_ipv4(vsi, input, add);
 			break;
+		default:
+			/* We cannot support masking based on protocol */
+			goto unsupported_flow;
 		}
 		break;
 	default:
+unsupported_flow:
 		dev_info(&pf->pdev->dev, "Could not specify spec type %d\n",
 			 input->flow_type);
 		ret = -EINVAL;
-- 
2.7.4

^ permalink raw reply related

* [net-next 13/17] i40e: Fix for long link down notification time
From: Jeff Kirsher @ 2016-10-29  7:30 UTC (permalink / raw)
  To: davem
  Cc: Carolyn Wyborny, netdev, nhorman, sassmann, jogreene,
	guru.anbalagane, Jeff Kirsher
In-Reply-To: <1477726258-111563-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Carolyn Wyborny <carolyn.wyborny@intel.com>

This patch fixes a problem where it could take a very
long time (>100 msec) to print the link down notification.
This problem is fixed by changing how often we update link
info from fw, when link is down. Without this patch, it can
take over 100msec to notify user link is down.

Change-ID: Ib876eb30834c7080792becd13ee093b9cbb35d78
Signed-off-by: Carolyn Wyborny <carolyn.wyborny@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_common.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c
index fe8100b..a475946 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
@@ -2494,7 +2494,10 @@ i40e_status i40e_update_link_info(struct i40e_hw *hw)
 	if (status)
 		return status;
 
-	if (hw->phy.link_info.link_info & I40E_AQ_MEDIA_AVAILABLE) {
+	/* extra checking needed to ensure link info to user is timely */
+	if ((hw->phy.link_info.link_info & I40E_AQ_MEDIA_AVAILABLE) &&
+	    ((hw->phy.link_info.link_info & I40E_AQ_LINK_UP) ||
+	     !(hw->phy.link_info_old.link_info & I40E_AQ_LINK_UP))) {
 		status = i40e_aq_get_phy_capabilities(hw, false, false,
 						      &abilities, NULL);
 		if (status)
-- 
2.7.4

^ permalink raw reply related

* [net-next 01/17] i40e: Fix client interaction
From: Jeff Kirsher @ 2016-10-29  7:30 UTC (permalink / raw)
  To: davem
  Cc: Carolyn Wyborny, netdev, nhorman, sassmann, jogreene,
	guru.anbalagane, Jeff Kirsher
In-Reply-To: <1477726258-111563-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Carolyn Wyborny <carolyn.wyborny@intel.com>

This patch fixes a problem in the client interface that
was causing random stack traces in RDMA driver load and
unload tests.  This patch fixes the problem by checking
for an existing client before trying to open it.  Without
this patch, there is a timing related null pointer deref.

Change-ID: Ib73d30671a27f6f9770dd53b3e5292b88d6b62da
Signed-off-by: Carolyn Wyborny <carolyn.wyborny@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_client.c | 29 ++++++++++++---------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c b/drivers/net/ethernet/intel/i40e/i40e_client.c
index 250db0b..6ffac03 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_client.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
@@ -565,7 +565,7 @@ void i40e_client_subtask(struct i40e_pf *pf)
 			if (test_bit(__I40E_DOWN, &pf->vsi[pf->lan_vsi]->state))
 				continue;
 		} else {
-			dev_warn(&pf->pdev->dev, "This client %s is being instanciated at probe\n",
+			dev_warn(&pf->pdev->dev, "This client %s is being instantiated at probe\n",
 				 client->name);
 		}
 
@@ -582,24 +582,21 @@ void i40e_client_subtask(struct i40e_pf *pf)
 			dev_info(&pf->pdev->dev, "Added instance of Client %s to PF%d bus=0x%02x func=0x%02x\n",
 				 client->name, pf->hw.pf_id,
 				 pf->hw.bus.device, pf->hw.bus.func);
-		}
-
-		mutex_lock(&i40e_client_instance_mutex);
-		/* Send an Open request to the client */
-		atomic_inc(&cdev->ref_cnt);
-		if (client->ops && client->ops->open)
-			ret = client->ops->open(&cdev->lan_info, client);
-		atomic_dec(&cdev->ref_cnt);
-		if (!ret) {
+			mutex_lock(&i40e_client_instance_mutex);
+			atomic_inc(&cdev->ref_cnt);
+			if (client->ops && client->ops->open)
+				ret = client->ops->open(&cdev->lan_info,
+							client);
+			atomic_dec(&cdev->ref_cnt);
+			if (ret < 0) {
+				mutex_unlock(&i40e_client_instance_mutex);
+				i40e_client_del_instance(pf, client);
+				atomic_dec(&client->ref_cnt);
+				continue;
+			}
 			set_bit(__I40E_CLIENT_INSTANCE_OPENED, &cdev->state);
-		} else {
-			/* remove client instance */
 			mutex_unlock(&i40e_client_instance_mutex);
-			i40e_client_del_instance(pf, client);
-			atomic_dec(&client->ref_cnt);
-			continue;
 		}
-		mutex_unlock(&i40e_client_instance_mutex);
 	}
 	mutex_unlock(&i40e_client_mutex);
 }
-- 
2.7.4

^ permalink raw reply related

* [net-next 02/17] i40e: Rewrite Flow Director busy wait loop
From: Jeff Kirsher @ 2016-10-29  7:30 UTC (permalink / raw)
  To: davem
  Cc: Alexander Duyck, netdev, nhorman, sassmann, jogreene,
	guru.anbalagane, Jeff Kirsher
In-Reply-To: <1477726258-111563-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Alexander Duyck <alexander.h.duyck@intel.com>

We can reorder the busy wait loop at the start of the Flow Director
transmit function to reduce the overall code size while still retaining the
same functionality.  As such I am taking advantage of the opportunity to do
so.

Change-ID: I34c403ca001953c6ac9816e65d5305e73d869026
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 6287bf6..af36c44 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -122,7 +122,6 @@ static int i40e_program_fdir_filter(struct i40e_fdir_filter *fdir_data,
 	struct device *dev;
 	dma_addr_t dma;
 	u32 td_cmd = 0;
-	u16 delay = 0;
 	u16 i;
 
 	/* find existing FDIR VSI */
@@ -137,15 +136,11 @@ static int i40e_program_fdir_filter(struct i40e_fdir_filter *fdir_data,
 	dev = tx_ring->dev;
 
 	/* we need two descriptors to add/del a filter and we can wait */
-	do {
-		if (I40E_DESC_UNUSED(tx_ring) > 1)
-			break;
+	for (i = I40E_FD_CLEAN_DELAY; I40E_DESC_UNUSED(tx_ring) < 2; i--) {
+		if (!i)
+			return -EAGAIN;
 		msleep_interruptible(1);
-		delay++;
-	} while (delay < I40E_FD_CLEAN_DELAY);
-
-	if (!(I40E_DESC_UNUSED(tx_ring) > 1))
-		return -EAGAIN;
+	}
 
 	dma = dma_map_single(dev, raw_packet,
 			     I40E_FDIR_MAX_RAW_PACKET_SIZE, DMA_TO_DEVICE);
-- 
2.7.4

^ permalink raw reply related

* [PATCH] add one parameter wro_enable for IXGBE
From: Mao Wenan @ 2016-10-29  7:08 UTC (permalink / raw)
  To: jeffrey.t.kirsher, intel-wired-lan, netdev, linux-kernel
In-Reply-To: <1477724932-5440-1-git-send-email-maowenan@huawei.com>

---
 drivers/net/ethernet/intel/ixgbe/ixgbe.h        |  1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c  | 29 ++++++++++++++-----------
 drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 28 +++++++++++++-----------
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c   |  9 ++++++++
 4 files changed, 41 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index b06e32d..9bc0be5 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -1027,4 +1027,5 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
 				  struct ixgbe_ring *tx_ring);
 u32 ixgbe_rss_indir_tbl_entries(struct ixgbe_adapter *adapter);
 void ixgbe_store_reta(struct ixgbe_adapter *adapter);
+bool ixgbe_wro_enable(void);
 #endif /* _IXGBE_H_ */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
index fb51be7..c312aaa 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
@@ -186,20 +186,23 @@ static s32 ixgbe_start_hw_82598(struct ixgbe_hw *hw)
 	ret_val = ixgbe_start_hw_generic(hw);
 
 #ifndef CONFIG_SPARC
-	/* Disable relaxed ordering */
-	for (i = 0; ((i < hw->mac.max_tx_queues) &&
-	     (i < IXGBE_DCA_MAX_QUEUES_82598)); i++) {
-		regval = IXGBE_READ_REG(hw, IXGBE_DCA_TXCTRL(i));
-		regval &= ~IXGBE_DCA_TXCTRL_DESC_WRO_EN;
-		IXGBE_WRITE_REG(hw, IXGBE_DCA_TXCTRL(i), regval);
-	}
+	if(likely(!ixgbe_wro_enable())) {
+
+		/* Disable relaxed ordering */
+		for (i = 0; ((i < hw->mac.max_tx_queues) &&
+		     (i < IXGBE_DCA_MAX_QUEUES_82598)); i++) {
+			regval = IXGBE_READ_REG(hw, IXGBE_DCA_TXCTRL(i));
+			regval &= ~IXGBE_DCA_TXCTRL_DESC_WRO_EN;
+			IXGBE_WRITE_REG(hw, IXGBE_DCA_TXCTRL(i), regval);
+		}
 
-	for (i = 0; ((i < hw->mac.max_rx_queues) &&
-	     (i < IXGBE_DCA_MAX_QUEUES_82598)); i++) {
-		regval = IXGBE_READ_REG(hw, IXGBE_DCA_RXCTRL(i));
-		regval &= ~(IXGBE_DCA_RXCTRL_DATA_WRO_EN |
-			    IXGBE_DCA_RXCTRL_HEAD_WRO_EN);
-		IXGBE_WRITE_REG(hw, IXGBE_DCA_RXCTRL(i), regval);
+		for (i = 0; ((i < hw->mac.max_rx_queues) &&
+		     (i < IXGBE_DCA_MAX_QUEUES_82598)); i++) {
+			regval = IXGBE_READ_REG(hw, IXGBE_DCA_RXCTRL(i));
+			regval &= ~(IXGBE_DCA_RXCTRL_DATA_WRO_EN |
+				    IXGBE_DCA_RXCTRL_HEAD_WRO_EN);
+			IXGBE_WRITE_REG(hw, IXGBE_DCA_RXCTRL(i), regval);
+		}
 	}
 #endif
 	if (ret_val)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
index 77d3039..7115dc0 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
@@ -347,22 +347,24 @@ s32 ixgbe_start_hw_gen2(struct ixgbe_hw *hw)
 	IXGBE_WRITE_FLUSH(hw);
 
 #ifndef CONFIG_SPARC
-	/* Disable relaxed ordering */
-	for (i = 0; i < hw->mac.max_tx_queues; i++) {
-		u32 regval;
-
-		regval = IXGBE_READ_REG(hw, IXGBE_DCA_TXCTRL_82599(i));
-		regval &= ~IXGBE_DCA_TXCTRL_DESC_WRO_EN;
-		IXGBE_WRITE_REG(hw, IXGBE_DCA_TXCTRL_82599(i), regval);
-	}
+	if(likely(!ixgbe_wro_enable())) {
+		/* Disable relaxed ordering */
+		for (i = 0; i < hw->mac.max_tx_queues; i++) {
+			u32 regval;
+
+			regval = IXGBE_READ_REG(hw, IXGBE_DCA_TXCTRL_82599(i));
+			regval &= ~IXGBE_DCA_TXCTRL_DESC_WRO_EN;
+			IXGBE_WRITE_REG(hw, IXGBE_DCA_TXCTRL_82599(i), regval);
+		}
 
-	for (i = 0; i < hw->mac.max_rx_queues; i++) {
-		u32 regval;
+		for (i = 0; i < hw->mac.max_rx_queues; i++) {
+			u32 regval;
 
-		regval = IXGBE_READ_REG(hw, IXGBE_DCA_RXCTRL(i));
-		regval &= ~(IXGBE_DCA_RXCTRL_DATA_WRO_EN |
-			    IXGBE_DCA_RXCTRL_HEAD_WRO_EN);
+			regval = IXGBE_READ_REG(hw, IXGBE_DCA_RXCTRL(i));
+			regval &= ~(IXGBE_DCA_RXCTRL_DATA_WRO_EN |
+				    IXGBE_DCA_RXCTRL_HEAD_WRO_EN);
 		IXGBE_WRITE_REG(hw, IXGBE_DCA_RXCTRL(i), regval);
+		}
 	}
 #endif
 	return 0;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index a244d9a..79ebce3 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -171,11 +171,20 @@ static int debug = -1;
 module_param(debug, int, 0);
 MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
 
+static bool wro_enable = 0;
+module_param(wro_enable, bool, 0);
+MODULE_PARM_DESC(wro_enable, "enable 82599 TX and RX's WRO attribute");
+
 MODULE_AUTHOR("Intel Corporation, <linux.nics@intel.com>");
 MODULE_DESCRIPTION("Intel(R) 10 Gigabit PCI Express Network Driver");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(DRV_VERSION);
 
+bool ixgbe_wro_enable(void)
+{
+	return wro_enable;
+}
+
 static struct workqueue_struct *ixgbe_wq;
 
 static bool ixgbe_check_cfg_remove(struct ixgbe_hw *hw, struct pci_dev *pdev);
-- 
2.5.0

^ permalink raw reply related

* [PATCH] add one parameter wro_enable to enable relaxed ordering for IXGBE
From: Mao Wenan @ 2016-10-29  7:08 UTC (permalink / raw)
  To: jeffrey.t.kirsher, intel-wired-lan, netdev, linux-kernel

This patch provides a way to enable relaxed ordering, where it helps with performance in some architecture.
The default value of wro_enable is 0, if you want to enable relaxed ordering, please set wro_enable=1.

Mao Wenan (1):
  add one parameter wro_enable for IXGBE

 drivers/net/ethernet/intel/ixgbe/ixgbe.h        |  1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c  | 29 ++++++++++++++-----------
 drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 28 +++++++++++++-----------
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c   |  9 ++++++++
 4 files changed, 41 insertions(+), 26 deletions(-)

-- 
2.5.0

^ permalink raw reply

* Re: [PATCH 17/17] batman-adv: Avoid precedence issues in macros
From: Sven Eckelmann @ 2016-10-29  6:52 UTC (permalink / raw)
  To: Joe Perches, netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r, Antonio Quartulli
In-Reply-To: <1477706184.23018.3.camel-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>

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

On Freitag, 28. Oktober 2016 18:56:24 CEST Joe Perches wrote:
[...]
> > But yes, looks to me like it is missing. Do you want to propose a patch or
> > should I do?
> 
> As you are working on this file, perhaps you should.

Ok, I will submit a patch with you as Reported-by to
the batman-adv mailing list

Kind regards,
	Sven

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

^ permalink raw reply

* Re: [PATCH v7 0/6] Add eBPF hooks for cgroups
From: Alexei Starovoitov @ 2016-10-29  6:24 UTC (permalink / raw)
  To: Lorenzo Colitti
  Cc: Daniel Mack, Pablo Neira Ayuso, htejun-b10kYP2dOMg,
	Daniel Borkmann, ast-b10kYP2dOMg, David Miller, kafai-b10kYP2dOMg,
	Florian Westphal, harald-H+wXaHxf7aLQT0dZR+AlfA,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Sargun Dhillon,
	cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAKD1Yr2pMk52h7BdRwTvGwnP5+ONmr4ac6cyUBoZ9P+Kt-B8jw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Sat, Oct 29, 2016 at 01:59:23PM +0900, Lorenzo Colitti wrote:
> On Sat, Oct 29, 2016 at 1:51 PM, Alexei Starovoitov
> <alexei.starovoitov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> What's the use case for egress?
> >>
> >> We (android networking) are currently looking at implementing network
> >> accounting via eBPF in order to replace the out-of-tree xt_qtaguid
> >> code. A per-cgroup eBPF program run on all traffic would be great. But
> >> when we looked at this patchset we realized it would not be useful for
> >> accounting purposes because even if a packet is counted here, it might
> >> still be dropped by netfilter hooks.
> >
> > don't use out-of-tree and instead drop using this mechanism or
> > any other in-kernel method? ;)
> 
> Getting rid of out-of-tree code is the goal, yes. We do have a
> requirement that things continue to work, though. Accounting for a
> packet in ip{,6}_output is not correct if that packet ends up being
> dropped by iptables later on.

understood.
it could be solved by swapping the order of cgroup_bpf_run_filter()
and NF_INET_POST_ROUTING in patch 5. It was proposed some time back, but
the current patch, I think, is more symmetrical.
cgroup+bpf runs after nf hook on rx and runs before it on tx.
imo it's more consistent.
Regardless of this choice... are you going to backport cgroupv2 to
android? Because this set is v2 only.

> > We (facebook infrastructure) have been using iptables and bpf networking
> > together with great success. They nicely co-exist and complement each other.
> > There is no need to reinvent the wheel if existing solution works.
> > iptables are great for their purpose.
> 
> That doesn't really answer my "what is the use case for egress"
> question though, right? Or are you saying "we use this, but we can't
> talk about how we use it"?

if the question is "why patch 4 alone is not enough and patch 5 is needed"?
Then it's symmetrical access. Accounting for RX only is a half done job.

> > there is iptables+cBPF support. It's being used in some cases already.
> 
> Adding eBPF support to the xt_bpf iptables code would be an option for
> what we want to do, yes. I think this requires that the eBPF map to be
> an fd that is available to the process that exec()s iptables, but we
> could do that.

yes. that's certainly doable, but sooner or later such approach will hit
scalability issue when number of cgroups is large. Same issue we saw
with cls_bpf and bpf_skb_under_cgroup(). Hence this patch set was needed
that is centered around cgroups instead of hooks. Note, unlike, tc and nf
there is no way to attach to a hook. The bpf program is attached to a cgroup.
It's an important distinction vs everything that currently exists in the stack.

^ permalink raw reply

* Re: net/dccp: warning in dccp_feat_clone_sp_val/__might_sleep
From: Cong Wang @ 2016-10-29  6:10 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Gerrit Renker, David S. Miller, dccp, netdev, LKML, Dmitry Vyukov,
	Eric Dumazet, Alexander Potapenko, Kostya Serebryany, syzkaller
In-Reply-To: <CAAeHK+z-SAq-YhHarfc94dguyXCJ_tv8n=ZVdLTRsYEy0cK-pQ@mail.gmail.com>

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

On Fri, Oct 28, 2016 at 5:40 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
> Hi,
>
> I've got the following error report while running the syzkaller fuzzer:
>
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 4608 at kernel/sched/core.c:7724
> __might_sleep+0x14c/0x1a0 kernel/sched/core.c:7719
> do not call blocking ops when !TASK_RUNNING; state=1 set at
> [<ffffffff811f5a5c>] prepare_to_wait+0xbc/0x210
> kernel/sched/wait.c:178
> Modules linked in:
> CPU: 0 PID: 4608 Comm: syz-executor Not tainted 4.9.0-rc2+ #320
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>  ffff88006625f7a0 ffffffff81b46914 ffff88006625f818 0000000000000000
>  ffffffff84052960 0000000000000000 ffff88006625f7e8 ffffffff81111237
>  ffff88006aceac00 ffffffff00001e2c ffffed000cc4beff ffffffff84052960
> Call Trace:
>  [<     inline     >] __dump_stack lib/dump_stack.c:15
>  [<ffffffff81b46914>] dump_stack+0xb3/0x10f lib/dump_stack.c:51
>  [<ffffffff81111237>] __warn+0x1a7/0x1f0 kernel/panic.c:550
>  [<ffffffff8111132c>] warn_slowpath_fmt+0xac/0xd0 kernel/panic.c:565
>  [<ffffffff811922fc>] __might_sleep+0x14c/0x1a0 kernel/sched/core.c:7719
>  [<     inline     >] slab_pre_alloc_hook mm/slab.h:393
>  [<     inline     >] slab_alloc_node mm/slub.c:2634
>  [<     inline     >] slab_alloc mm/slub.c:2716
>  [<ffffffff81508da0>] __kmalloc_track_caller+0x150/0x2a0 mm/slub.c:4240
>  [<ffffffff8146be14>] kmemdup+0x24/0x50 mm/util.c:113
>  [<ffffffff8388b2cf>] dccp_feat_clone_sp_val.part.5+0x4f/0xe0
> net/dccp/feat.c:374
>  [<     inline     >] dccp_feat_clone_sp_val net/dccp/feat.c:1141
>  [<     inline     >] dccp_feat_change_recv net/dccp/feat.c:1141
>  [<ffffffff8388d491>] dccp_feat_parse_options+0xaa1/0x13d0 net/dccp/feat.c:1411
>  [<ffffffff83894f01>] dccp_parse_options+0x721/0x1010 net/dccp/options.c:128
>  [<ffffffff83891280>] dccp_rcv_state_process+0x200/0x15b0 net/dccp/input.c:644
>  [<ffffffff838b8a94>] dccp_v4_do_rcv+0xf4/0x1a0 net/dccp/ipv4.c:681
>  [<     inline     >] sk_backlog_rcv ./include/net/sock.h:872
>  [<ffffffff82b7ceb6>] __release_sock+0x126/0x3a0 net/core/sock.c:2044
>  [<ffffffff82b7d189>] release_sock+0x59/0x1c0 net/core/sock.c:2502
>  [<     inline     >] inet_wait_for_connect net/ipv4/af_inet.c:547
>  [<ffffffff8316b2a2>] __inet_stream_connect+0x5d2/0xbb0 net/ipv4/af_inet.c:617
>  [<ffffffff8316b8d5>] inet_stream_connect+0x55/0xa0 net/ipv4/af_inet.c:656
>  [<ffffffff82b705e4>] SYSC_connect+0x244/0x2f0 net/socket.c:1533
>  [<ffffffff82b72dd4>] SyS_connect+0x24/0x30 net/socket.c:1514
>  [<ffffffff83fbf701>] entry_SYSCALL_64_fastpath+0x1f/0xc2
> arch/x86/entry/entry_64.S:209

Should be fixed the attached patch. I will verify it with your
reproducer tomorrow.

Thanks!

[-- Attachment #2: dccp.diff --]
[-- Type: text/plain, Size: 1775 bytes --]

diff --git a/net/dccp/feat.c b/net/dccp/feat.c
index 1704948..c90cb35 100644
--- a/net/dccp/feat.c
+++ b/net/dccp/feat.c
@@ -367,11 +367,11 @@ static inline int dccp_feat_must_be_understood(u8 feat_num)
 }
 
 /* copy constructor, fval must not already contain allocated memory */
-static int dccp_feat_clone_sp_val(dccp_feat_val *fval, u8 const *val, u8 len)
+static int dccp_feat_clone_sp_val(dccp_feat_val *fval, u8 const *val, u8 len, gfp_t flags)
 {
 	fval->sp.len = len;
 	if (fval->sp.len > 0) {
-		fval->sp.vec = kmemdup(val, len, gfp_any());
+		fval->sp.vec = kmemdup(val, len, flags);
 		if (fval->sp.vec == NULL) {
 			fval->sp.len = 0;
 			return -ENOBUFS;
@@ -404,7 +404,8 @@ static void dccp_feat_val_destructor(u8 feat_num, dccp_feat_val *val)
 
 	if (type == FEAT_SP && dccp_feat_clone_sp_val(&new->val,
 						      original->val.sp.vec,
-						      original->val.sp.len)) {
+						      original->val.sp.len,
+						      gfp_any())) {
 		kfree(new);
 		return NULL;
 	}
@@ -735,7 +736,7 @@ static int __feat_register_sp(struct list_head *fn, u8 feat, u8 is_local,
 	if (feat == DCCPF_CCID && !ccid_support_check(sp_val, sp_len))
 		return -EOPNOTSUPP;
 
-	if (dccp_feat_clone_sp_val(&fval, sp_val, sp_len))
+	if (dccp_feat_clone_sp_val(&fval, sp_val, sp_len, gfp_any()))
 		return -ENOMEM;
 
 	return dccp_feat_push_change(fn, feat, is_local, mandatory, &fval);
@@ -1138,7 +1139,7 @@ static u8 dccp_feat_change_recv(struct list_head *fn, u8 is_mandatory, u8 opt,
 		 *   otherwise we accept the preferred value;
 		 * - else if we are the client, we use the first list element.
 		 */
-		if (dccp_feat_clone_sp_val(&fval, val, 1))
+		if (dccp_feat_clone_sp_val(&fval, val, 1, GFP_ATOMIC))
 			return DCCP_RESET_CODE_TOO_BUSY;
 
 		if (len > 1 && server) {

^ permalink raw reply related

* Re: [PATCH v7 0/6] Add eBPF hooks for cgroups
From: Lorenzo Colitti @ 2016-10-29  4:59 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Daniel Mack, Pablo Neira Ayuso, htejun-b10kYP2dOMg,
	Daniel Borkmann, ast-b10kYP2dOMg, David Miller, kafai-b10kYP2dOMg,
	Florian Westphal, harald-H+wXaHxf7aLQT0dZR+AlfA,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Sargun Dhillon,
	cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161029045107.GA61294-+o4/htvd0TDFYCXBM6kdu7fOX0fSgVTm@public.gmane.org>

On Sat, Oct 29, 2016 at 1:51 PM, Alexei Starovoitov
<alexei.starovoitov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> What's the use case for egress?
>>
>> We (android networking) are currently looking at implementing network
>> accounting via eBPF in order to replace the out-of-tree xt_qtaguid
>> code. A per-cgroup eBPF program run on all traffic would be great. But
>> when we looked at this patchset we realized it would not be useful for
>> accounting purposes because even if a packet is counted here, it might
>> still be dropped by netfilter hooks.
>
> don't use out-of-tree and instead drop using this mechanism or
> any other in-kernel method? ;)

Getting rid of out-of-tree code is the goal, yes. We do have a
requirement that things continue to work, though. Accounting for a
packet in ip{,6}_output is not correct if that packet ends up being
dropped by iptables later on.

> We (facebook infrastructure) have been using iptables and bpf networking
> together with great success. They nicely co-exist and complement each other.
> There is no need to reinvent the wheel if existing solution works.
> iptables are great for their purpose.

That doesn't really answer my "what is the use case for egress"
question though, right? Or are you saying "we use this, but we can't
talk about how we use it"?

> there is iptables+cBPF support. It's being used in some cases already.

Adding eBPF support to the xt_bpf iptables code would be an option for
what we want to do, yes. I think this requires that the eBPF map to be
an fd that is available to the process that exec()s iptables, but we
could do that.

^ permalink raw reply

* Re: [PATCH v7 0/6] Add eBPF hooks for cgroups
From: Alexei Starovoitov @ 2016-10-29  4:51 UTC (permalink / raw)
  To: Lorenzo Colitti
  Cc: Daniel Mack, Pablo Neira Ayuso, htejun-b10kYP2dOMg,
	Daniel Borkmann, ast-b10kYP2dOMg, David Miller, kafai-b10kYP2dOMg,
	Florian Westphal, harald-H+wXaHxf7aLQT0dZR+AlfA,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	sargun-GaZTRHToo+CzQB+pC5nmwQ, cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAKD1Yr2aRDNUxX8onReZyURufphxGoSTek=Fjk3Wswq9WOVp4w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Sat, Oct 29, 2016 at 12:51:37PM +0900, Lorenzo Colitti wrote:
> On Thu, Oct 27, 2016 at 5:40 PM, Daniel Mack <daniel-cYrQPVfZoowdnm+yROfE0A@public.gmane.org> wrote:
> > It's not anything new. These hooks live on the very same level as
> > SO_ATTACH_FILTER. The only differences are that the BPF programs are
> > stored in the cgroup, and not in the socket, and that they exist for
> > egress as well.
> 
> What's the use case for egress?
> 
> We (android networking) are currently looking at implementing network
> accounting via eBPF in order to replace the out-of-tree xt_qtaguid
> code. A per-cgroup eBPF program run on all traffic would be great. But
> when we looked at this patchset we realized it would not be useful for
> accounting purposes because even if a packet is counted here, it might
> still be dropped by netfilter hooks.

don't use out-of-tree and instead drop using this mechanism or
any other in-kernel method? ;)
We (facebook infrastructure) have been using iptables and bpf networking
together with great success. They nicely co-exist and complement each other.
There is no need to reinvent the wheel if existing solution works.
iptables are great for their purpose.

> It seems like it would be much more useful to be able to do this in an
> iptables rule.

there is iptables+cBPF support. It's being used in some cases already.

^ permalink raw reply

* Re: [PATCH v7 0/6] Add eBPF hooks for cgroups
From: Lorenzo Colitti @ 2016-10-29  3:51 UTC (permalink / raw)
  To: Daniel Mack
  Cc: Pablo Neira Ayuso, htejun-b10kYP2dOMg, Daniel Borkmann,
	ast-b10kYP2dOMg, David Miller, kafai-b10kYP2dOMg,
	Florian Westphal, harald-H+wXaHxf7aLQT0dZR+AlfA,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	sargun-GaZTRHToo+CzQB+pC5nmwQ, cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <c9683122-d770-355b-e275-7c446e6d1d0f-cYrQPVfZoowdnm+yROfE0A@public.gmane.org>

On Thu, Oct 27, 2016 at 5:40 PM, Daniel Mack <daniel-cYrQPVfZoowdnm+yROfE0A@public.gmane.org> wrote:
> It's not anything new. These hooks live on the very same level as
> SO_ATTACH_FILTER. The only differences are that the BPF programs are
> stored in the cgroup, and not in the socket, and that they exist for
> egress as well.

What's the use case for egress?

We (android networking) are currently looking at implementing network
accounting via eBPF in order to replace the out-of-tree xt_qtaguid
code. A per-cgroup eBPF program run on all traffic would be great. But
when we looked at this patchset we realized it would not be useful for
accounting purposes because even if a packet is counted here, it might
still be dropped by netfilter hooks.

It seems like it would be much more useful to be able to do this in an
iptables rule.

^ permalink raw reply

* RE: [PATCH net-next RFC WIP] Patch for XDP support for virtio_net
From: Shrijeet Mukherjee @ 2016-10-29  3:51 UTC (permalink / raw)
  To: Alexei Starovoitov, Jakub Kicinski
  Cc: John Fastabend, David Miller, alexander.duyck, mst, brouer,
	shrijeet, tom, netdev, Roopa Prabhu, Nikolay Aleksandrov
In-Reply-To: <20161028182223.GA53930@ast-mbp.thefacebook.com>

> -----Original Message-----
> From: Alexei Starovoitov [mailto:alexei.starovoitov@gmail.com]
> Sent: Friday, October 28, 2016 11:22 AM
> To: Jakub Kicinski <kubakici@wp.pl>
> Cc: John Fastabend <john.fastabend@gmail.com>; David Miller
> <davem@davemloft.net>; alexander.duyck@gmail.com; mst@redhat.com;
> brouer@redhat.com; shrijeet@gmail.com; tom@herbertland.com;
> netdev@vger.kernel.org; shm@cumulusnetworks.com;
> roopa@cumulusnetworks.com; nikolay@cumulusnetworks.com
> Subject: Re: [PATCH net-next RFC WIP] Patch for XDP support for
virtio_net
>
> On Fri, Oct 28, 2016 at 05:18:12PM +0100, Jakub Kicinski wrote:
> > On Fri, 28 Oct 2016 08:56:35 -0700, John Fastabend wrote:
> > > On 16-10-27 07:10 PM, David Miller wrote:
> > > > From: Alexander Duyck <alexander.duyck@gmail.com>
> > > > Date: Thu, 27 Oct 2016 18:43:59 -0700
> > > >
> > > >> On Thu, Oct 27, 2016 at 6:35 PM, David Miller
> <davem@davemloft.net> wrote:
> > > >>> From: "Michael S. Tsirkin" <mst@redhat.com>
> > > >>> Date: Fri, 28 Oct 2016 01:25:48 +0300
> > > >>>
> > > >>>> On Thu, Oct 27, 2016 at 05:42:18PM -0400, David Miller wrote:
> > > >>>>> From: "Michael S. Tsirkin" <mst@redhat.com>
> > > >>>>> Date: Fri, 28 Oct 2016 00:30:35 +0300
> > > >>>>>
> > > >>>>>> Something I'd like to understand is how does XDP address the
> > > >>>>>> problem that 100Byte packets are consuming 4K of memory
> now.
> > > >>>>>
> > > >>>>> Via page pools.  We're going to make a generic one, but right
> > > >>>>> now each and every driver implements a quick list of pages to
> > > >>>>> allocate from (and thus avoid the DMA man/unmap overhead,
> > > >>>>> etc.)
> > > >>>>
> > > >>>> So to clarify, ATM virtio doesn't attempt to avoid dma
> > > >>>> map/unmap so there should be no issue with that even when using
> > > >>>> sub/page regions, assuming DMA APIs support sub-page
> map/unmap correctly.
> > > >>>
> > > >>> That's not what I said.
> > > >>>
> > > >>> The page pools are meant to address the performance degradation
> > > >>> from going to having one packet per page for the sake of XDP's
> > > >>> requirements.
> > > >>>
> > > >>> You still need to have one packet per page for correct XDP
> > > >>> operation whether you do page pools or not, and whether you have
> > > >>> DMA mapping (or it's equivalent virutalization operation) or
not.
> > > >>
> > > >> Maybe I am missing something here, but why do you need to limit
> > > >> things to one packet per page for correct XDP operation?  Most of
> > > >> the drivers out there now are usually storing something closer to
> > > >> at least 2 packets per page, and with the DMA API fixes I am
> > > >> working on there should be no issue with changing the contents
> > > >> inside those pages since we won't invalidate or overwrite the
> > > >> data after the DMA buffer has been synchronized for use by the
CPU.
> > > >
> > > > Because with SKB's you can share the page with other packets.
> > > >
> > > > With XDP you simply cannot.
> > > >
> > > > It's software semantics that are the issue.  SKB frag list pages
> > > > are read only, XDP packets are writable.
> > > >
> > > > This has nothing to do with "writability" of the pages wrt. DMA
> > > > mapping or cpu mappings.
> > > >
> > >
> > > Sorry I'm not seeing it either. The current xdp_buff is defined by,
> > >
> > >   struct xdp_buff {
> > > 	void *data;
> > > 	void *data_end;
> > >   };
> > >
> > > The verifier has an xdp_is_valid_access() check to ensure we don't
> > > go past data_end. The page for now at least never leaves the driver.
> > > For the work to get xmit to other devices working I'm still not sure
> > > I see any issue.
> >
> > +1
> >
> > Do we want to make the packet-per-page a requirement because it could
> > be useful in the future from architectural standpoint?  I guess there
> > is a trade-off here between having the comfort of people following
> > this requirement today and making driver support for XDP even more
> complex.
>
> It looks to me that packet per page makes drivers simpler instead of
> complex.
> ixgbe split-page and mlx* order-3/5 tricks are definitely complex.
> The skb truesize concerns come from the host when data is delivered to
> user space and we need to have precise memory accounting for different
> applications and different users. XDP is all root and imo it's far away
from
> the days when multi-user non-root issues start to pop up.
> At the same time XDP doesn't require to use 4k buffer in something like
> Netronome.
> If xdp bpf program can be offloaded into HW with 1800 byte buffers,
great!
> For x86 cpu the 4k byte is a natural allocation chunk. Anything lower
> requires delicate dma tricks paired with even more complex slab
allocator
> and atomic recnts.
> All of that can only drive the performance down.
> Comparing to kernel bypass xdp is using 4k pages whereas dpdk has to use
> huge pages, so xdp is saving a ton of memory already!

Generally agree, but SRIOV nics with multiple queues can end up in a bad
spot if each buffer was 4K right ? I see a specific page pool to be used
by queues which are enabled for XDP as the easiest to swing solution that
way the memory overhead can be restricted to enabled queues and shared
access issues can be restricted to skb's using that pool no ?

Clearly as said later this will not apply to ebpf offload devices ..

^ permalink raw reply

* Re: [bnx2] [Regression 4.8] Driver loading fails without firmware
From: Baoquan He @ 2016-10-29  2:55 UTC (permalink / raw)
  To: Paul Menzel
  Cc: Sony Chacko, Dept-HSGLinuxNICDev, netdev, David S. Miller, dvteam
In-Reply-To: <f055c961-58ca-ea81-46d8-610fa055cce8@molgen.mpg.de>

On 10/27/16 at 03:21pm, Paul Menzel wrote:
> Dear Baoquan,
> > > Baoquan, could you please fix this regression. My suggestion is, that you
> > > add the old code back, but check if the firmware has been loaded. If it
> > > hasn’t, load it again.
> > > 
> > > That way, people can update their Linux kernel, and it continues working
> > > without changing the initramfs, or anything else.
> > 
> > I saw your mail but I am also not familiar with bnx2 driver. As the
> > commit log says I just tried to make bnx2 driver reset itself earlier.
> > 
> > So you did a git bisect and found this commit caused the regression,
> > right? If yes, and network developers have no action, I will look into
> > the code and see if I have idea to fix it.
> 
> Well, I looked through the commits and found that one, which would explain
> the changed behavior.
> 
> To be sure, and to follow your request, I took Linux 4.8.4 and reverted your
> commit (attached). Then I deleted the firmware again from the initramfs, and
> rebooted. The devices showed up just fine as before.
> 
> So to summarize, the commit is indeed the culprit.

Hi Paul,

Sorry for this.

Could you tell the steps to reproduce? I will find a machine with bnx2
NIC and check if there's other ways.

Thanks
Baoquan

> From 61b8dac8796343a797858b4a2eb0a59a0cfcd735 Mon Sep 17 00:00:00 2001
> From: Paul Menzel <pmenzel@molgen.mpg.de>
> Date: Thu, 27 Oct 2016 11:34:52 +0200
> Subject: [PATCH] Revert "bnx2: Reset device during driver initialization"
> 
> This reverts commit 3e1be7ad2d38c6bd6aeef96df9bd0a7822f4e51c.
> ---
>  drivers/net/ethernet/broadcom/bnx2.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnx2.c b/drivers/net/ethernet/broadcom/bnx2.c
> index 27f11a5..ecd357d 100644
> --- a/drivers/net/ethernet/broadcom/bnx2.c
> +++ b/drivers/net/ethernet/broadcom/bnx2.c
> @@ -6356,6 +6356,10 @@ bnx2_open(struct net_device *dev)
>  	struct bnx2 *bp = netdev_priv(dev);
>  	int rc;
>  
> +	rc = bnx2_request_firmware(bp);
> +	if (rc < 0)
> +		goto out;
> +
>  	netif_carrier_off(dev);
>  
>  	bnx2_disable_int(bp);
> @@ -6424,6 +6428,7 @@ bnx2_open(struct net_device *dev)
>  	bnx2_free_irq(bp);
>  	bnx2_free_mem(bp);
>  	bnx2_del_napi(bp);
> +	bnx2_release_firmware(bp);
>  	goto out;
>  }
>  
> @@ -8570,12 +8575,6 @@ bnx2_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  	pci_set_drvdata(pdev, dev);
>  
> -	rc = bnx2_request_firmware(bp);
> -	if (rc < 0)
> -		goto error;
> -
> -
> -	bnx2_reset_chip(bp, BNX2_DRV_MSG_CODE_RESET);
>  	memcpy(dev->dev_addr, bp->mac_addr, ETH_ALEN);
>  
>  	dev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG |
> @@ -8608,7 +8607,6 @@ bnx2_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	return 0;
>  
>  error:
> -	bnx2_release_firmware(bp);
>  	pci_iounmap(pdev, bp->regview);
>  	pci_release_regions(pdev);
>  	pci_disable_device(pdev);
> -- 
> 2.4.1
> 

^ 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