[net-next 0/7][pull request] 40GbE Intel Wired LAN Driver Updates 2018-02-26

[net-next 0/7][pull request] 40GbE Intel Wired LAN Driver Updates 2018-02-26

[Jeff Kirsher]

This series contains updates to i40e and i40evf only.

Mariusz adds a new ethtool private flag for forcing true link state with
the requested changes from Jakub Kicinski.

Paweł fixes an issue where we were double locking the same resource
which would generate a kernel panic after bringing an interface up for
i40evf.

Alan modifies both drivers to use software values to determine if there
are packets stalled on the ring with the added benefit of being less CPU
intensive since we do not need to reach into the hardware to get the
values.

Colin Ian King provides a few fixes detected by Coverity, first was to
pass a struct by reference versus by value to be more efficient.  Then
verify the VSI pointer is not NULL before trying to dereference it.
Cleaned up redundant checks that always return true.

Dan Carpenter fixes over indented lines of code.

The following are changes since commit 4869a1476df5ef2d09fa52acc9cfcc21b47194c5:
  sh_eth: TSU_QTAG0/1 registers the same as TSU_QTAGM0/1
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: use SW variables for hang detection

Colin Ian King (3):
  i40evf: pass struct virtchnl_filter by reference rather than by value
  i40e: check that pointer VSI is not null before dereferencing it
  i40evf: remove redundant array comparisons to 0 checks

Dan Carpenter (1):
  i40e: remove some stray indenting

Mariusz Stachura (1):
  i40e: link_down_on_close private flag support

Paweł Jabłoński (1):
  i40evf: Fix double locking the same resource

 drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h  | 37 +++++++++++
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c     |  2 +
 drivers/net/ethernet/intel/i40e/i40e_main.c        | 75 ++++++++++++++++++++++
 drivers/net/ethernet/intel/i40e/i40e_txrx.c        | 16 +++--
 drivers/net/ethernet/intel/i40e/i40e_txrx.h        |  2 +-
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c |  6 +-
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c      |  2 +-
 drivers/net/ethernet/intel/i40evf/i40evf_main.c    | 75 ++++++++++------------
 .../net/ethernet/intel/i40evf/i40evf_virtchnl.c    | 32 +++++----
 9 files changed, 181 insertions(+), 66 deletions(-)

-- 
2.14.3

[net-next 1/7] i40e: link_down_on_close private flag support

[Jeff Kirsher]

From: Mariusz Stachura <mariusz.stachura@intel.com>

This patch introduces new ethtool private flag used for
forcing true link state. Function i40e_force_link_state that implements
this functionality was added, it sets phy_type = 0 in order to
work-around firmware's LESM. False positive error messages were
suppressed.

The ndo_open() should not succeed if there were issues with forcing link
state to be UP.

Added I40E_PHY_TYPES_BITMASK define with all phy types OR-ed together in
one bitmask.  Added after phy type definition, so it will be hard to
forget to include new phy types to the bitmask.

CC: Jakub Kicinski <kubakici@wp.pl>
Signed-off-by: Mariusz Stachura <mariusz.stachura@intel.com>
Signed-off-by: Mitch Williams <mitch.a.williams@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_adminq_cmd.h | 37 +++++++++++
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c    |  2 +
 drivers/net/ethernet/intel/i40e/i40e_main.c       | 75 +++++++++++++++++++++++
 3 files changed, 114 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h b/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
index a852775d3059..0dfc52772c45 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
@@ -1914,6 +1914,43 @@ enum i40e_aq_phy_type {
 	I40E_PHY_TYPE_DEFAULT			= 0xFF,
 };
 
+#define I40E_PHY_TYPES_BITMASK (BIT_ULL(I40E_PHY_TYPE_SGMII) | \
+				BIT_ULL(I40E_PHY_TYPE_1000BASE_KX) | \
+				BIT_ULL(I40E_PHY_TYPE_10GBASE_KX4) | \
+				BIT_ULL(I40E_PHY_TYPE_10GBASE_KR) | \
+				BIT_ULL(I40E_PHY_TYPE_40GBASE_KR4) | \
+				BIT_ULL(I40E_PHY_TYPE_XAUI) | \
+				BIT_ULL(I40E_PHY_TYPE_XFI) | \
+				BIT_ULL(I40E_PHY_TYPE_SFI) | \
+				BIT_ULL(I40E_PHY_TYPE_XLAUI) | \
+				BIT_ULL(I40E_PHY_TYPE_XLPPI) | \
+				BIT_ULL(I40E_PHY_TYPE_40GBASE_CR4_CU) | \
+				BIT_ULL(I40E_PHY_TYPE_10GBASE_CR1_CU) | \
+				BIT_ULL(I40E_PHY_TYPE_10GBASE_AOC) | \
+				BIT_ULL(I40E_PHY_TYPE_40GBASE_AOC) | \
+				BIT_ULL(I40E_PHY_TYPE_UNRECOGNIZED) | \
+				BIT_ULL(I40E_PHY_TYPE_UNSUPPORTED) | \
+				BIT_ULL(I40E_PHY_TYPE_100BASE_TX) | \
+				BIT_ULL(I40E_PHY_TYPE_1000BASE_T) | \
+				BIT_ULL(I40E_PHY_TYPE_10GBASE_T) | \
+				BIT_ULL(I40E_PHY_TYPE_10GBASE_SR) | \
+				BIT_ULL(I40E_PHY_TYPE_10GBASE_LR) | \
+				BIT_ULL(I40E_PHY_TYPE_10GBASE_SFPP_CU) | \
+				BIT_ULL(I40E_PHY_TYPE_10GBASE_CR1) | \
+				BIT_ULL(I40E_PHY_TYPE_40GBASE_CR4) | \
+				BIT_ULL(I40E_PHY_TYPE_40GBASE_SR4) | \
+				BIT_ULL(I40E_PHY_TYPE_40GBASE_LR4) | \
+				BIT_ULL(I40E_PHY_TYPE_1000BASE_SX) | \
+				BIT_ULL(I40E_PHY_TYPE_1000BASE_LX) | \
+				BIT_ULL(I40E_PHY_TYPE_1000BASE_T_OPTICAL) | \
+				BIT_ULL(I40E_PHY_TYPE_20GBASE_KR2) | \
+				BIT_ULL(I40E_PHY_TYPE_25GBASE_KR) | \
+				BIT_ULL(I40E_PHY_TYPE_25GBASE_CR) | \
+				BIT_ULL(I40E_PHY_TYPE_25GBASE_SR) | \
+				BIT_ULL(I40E_PHY_TYPE_25GBASE_LR) | \
+				BIT_ULL(I40E_PHY_TYPE_25GBASE_AOC) | \
+				BIT_ULL(I40E_PHY_TYPE_25GBASE_ACC))
+
 #define I40E_LINK_SPEED_100MB_SHIFT	0x1
 #define I40E_LINK_SPEED_1000MB_SHIFT	0x2
 #define I40E_LINK_SPEED_10GB_SHIFT	0x3
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 0dcbbda164c4..89807e32a898 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -230,6 +230,8 @@ static const struct i40e_priv_flags i40e_gstrings_priv_flags[] = {
 	I40E_PRIV_FLAG("flow-director-atr", I40E_FLAG_FD_ATR_ENABLED, 0),
 	I40E_PRIV_FLAG("veb-stats", I40E_FLAG_VEB_STATS_ENABLED, 0),
 	I40E_PRIV_FLAG("hw-atr-eviction", I40E_FLAG_HW_ATR_EVICT_ENABLED, 0),
+	I40E_PRIV_FLAG("link-down-on-close",
+		       I40E_FLAG_LINK_DOWN_ON_CLOSE_ENABLED, 0),
 	I40E_PRIV_FLAG("legacy-rx", I40E_FLAG_LEGACY_RX, 0),
 	I40E_PRIV_FLAG("disable-source-pruning",
 		       I40E_FLAG_SOURCE_PRUNING_DISABLED, 0),
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index f6d37456f3b7..be9a1467a1a1 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6546,6 +6546,75 @@ int i40e_up(struct i40e_vsi *vsi)
 	return err;
 }
 
+/**
+ * i40e_force_link_state - Force the link status
+ * @pf: board private structure
+ * @is_up: whether the link state should be forced up or down
+ **/
+static i40e_status i40e_force_link_state(struct i40e_pf *pf, bool is_up)
+{
+	struct i40e_aq_get_phy_abilities_resp abilities;
+	struct i40e_aq_set_phy_config config = {0};
+	struct i40e_hw *hw = &pf->hw;
+	i40e_status err;
+	u64 mask;
+
+	/* Get the current phy config */
+	err = i40e_aq_get_phy_capabilities(hw, false, false, &abilities,
+					   NULL);
+	if (err) {
+		dev_err(&pf->pdev->dev,
+			"failed to get phy cap., ret =  %s last_status =  %s\n",
+			i40e_stat_str(hw, err),
+			i40e_aq_str(hw, hw->aq.asq_last_status));
+		return err;
+	}
+
+	/* If link needs to go up, but was not forced to go down,
+	 * no need for a flap
+	 */
+	if (is_up && abilities.phy_type != 0)
+		return I40E_SUCCESS;
+
+	/* To force link we need to set bits for all supported PHY types,
+	 * but there are now more than 32, so we need to split the bitmap
+	 * across two fields.
+	 */
+	mask = I40E_PHY_TYPES_BITMASK;
+	config.phy_type = is_up ? cpu_to_le32((u32)(mask & 0xffffffff)) : 0;
+	config.phy_type_ext = is_up ? (u8)((mask >> 32) & 0xff) : 0;
+	/* Copy the old settings, except of phy_type */
+	config.abilities = abilities.abilities;
+	config.link_speed = abilities.link_speed;
+	config.eee_capability = abilities.eee_capability;
+	config.eeer = abilities.eeer_val;
+	config.low_power_ctrl = abilities.d3_lpan;
+	err = i40e_aq_set_phy_config(hw, &config, NULL);
+
+	if (err) {
+		dev_err(&pf->pdev->dev,
+			"set phy config ret =  %s last_status =  %s\n",
+			i40e_stat_str(&pf->hw, err),
+			i40e_aq_str(&pf->hw, pf->hw.aq.asq_last_status));
+		return err;
+	}
+
+	/* Update the link info */
+	err = i40e_update_link_info(hw);
+	if (err) {
+		/* Wait a little bit (on 40G cards it sometimes takes a really
+		 * long time for link to come back from the atomic reset)
+		 * and try once more
+		 */
+		msleep(1000);
+		i40e_update_link_info(hw);
+	}
+
+	i40e_aq_set_link_restart_an(hw, true, NULL);
+
+	return I40E_SUCCESS;
+}
+
 /**
  * i40e_down - Shutdown the connection processing
  * @vsi: the VSI being stopped
@@ -6563,6 +6632,9 @@ void i40e_down(struct i40e_vsi *vsi)
 	}
 	i40e_vsi_disable_irq(vsi);
 	i40e_vsi_stop_rings(vsi);
+	if (vsi->type == I40E_VSI_MAIN &&
+	    vsi->back->flags & I40E_FLAG_LINK_DOWN_ON_CLOSE_ENABLED)
+		i40e_force_link_state(vsi->back, false);
 	i40e_napi_disable_all(vsi);
 
 	for (i = 0; i < vsi->num_queue_pairs; i++) {
@@ -7524,6 +7596,9 @@ int i40e_open(struct net_device *netdev)
 
 	netif_carrier_off(netdev);
 
+	if (i40e_force_link_state(pf, true))
+		return -EAGAIN;
+
 	err = i40e_vsi_open(vsi);
 	if (err)
 		return err;
-- 
2.14.3

[net-next 2/7] i40evf: Fix double locking the same resource

[Jeff Kirsher]

From: Paweł Jabłoński <pawel.jablonski@intel.com>

Removes the locking of adapter->mac_vlan_list_lock resource in
i40evf_add_filter(). The locking part is moved above i40evf_add_filter().
i40evf_add_filter(), called by i40evf_addr_sync(), was trying to lock the
resource again and double locking generated a kernel panic after bringing
an interface up.

Fixes: 8946b56354b7 ("i40evf: use __dev_[um]c_sync routines in
       .set_rx_mode")
Signed-off-by: Paweł Jabłoński <pawel.jablonski@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40evf/i40evf_main.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 4955ce3ab6a2..4f5744d488aa 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -815,13 +815,11 @@ i40evf_mac_filter *i40evf_add_filter(struct i40evf_adapter *adapter,
 	if (!macaddr)
 		return NULL;
 
-	spin_lock_bh(&adapter->mac_vlan_list_lock);
-
 	f = i40evf_find_filter(adapter, macaddr);
 	if (!f) {
 		f = kzalloc(sizeof(*f), GFP_ATOMIC);
 		if (!f)
-			goto clearout;
+			return f;
 
 		ether_addr_copy(f->macaddr, macaddr);
 
@@ -832,8 +830,6 @@ i40evf_mac_filter *i40evf_add_filter(struct i40evf_adapter *adapter,
 		f->remove = false;
 	}
 
-clearout:
-	spin_unlock_bh(&adapter->mac_vlan_list_lock);
 	return f;
 }
 
@@ -868,9 +864,10 @@ static int i40evf_set_mac(struct net_device *netdev, void *p)
 		adapter->aq_required |= I40EVF_FLAG_AQ_DEL_MAC_FILTER;
 	}
 
+	f = i40evf_add_filter(adapter, addr->sa_data);
+
 	spin_unlock_bh(&adapter->mac_vlan_list_lock);
 
-	f = i40evf_add_filter(adapter, addr->sa_data);
 	if (f) {
 		ether_addr_copy(hw->mac.addr, addr->sa_data);
 		ether_addr_copy(netdev->dev_addr, adapter->hw.mac.addr);
@@ -3040,7 +3037,12 @@ static int i40evf_open(struct net_device *netdev)
 	if (err)
 		goto err_req_irq;
 
+	spin_lock_bh(&adapter->mac_vlan_list_lock);
+
 	i40evf_add_filter(adapter, adapter->hw.mac.addr);
+
+	spin_unlock_bh(&adapter->mac_vlan_list_lock);
+
 	i40evf_configure(adapter);
 
 	i40evf_up_complete(adapter);
-- 
2.14.3

[net-next 3/7] i40e/i40evf: use SW variables for hang detection

[Jeff Kirsher]

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

The i40e_detect_recover_hung function uses the i40e_get_tx_pending
function to determine if there are packets stalled on the ring.
i40e_get_tx_pending calculates the pending packets using the head
writeback value and HW tail.  If the queue is stopped and we lose the
interrupt to update our next_to_clean then we a) won't get another
interrupt to clean because queue is stopped b) we won't catch the
problem with i40e_detect_recover_hung because the HW values look like
there's no packets waiting to be transmitted.  Using the SW values we
can catch the issue because next_to_clean will be out of sync with head
writeback.

This has the added benefit being less CPU intensive because we don't
need to reach into the hardware to get the values.

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_txrx.c   | 16 +++++++++++-----
 drivers/net/ethernet/intel/i40e/i40e_txrx.h   |  2 +-
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c |  2 +-
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 1ec9b1d8023d..97cfe944b568 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -708,16 +708,22 @@ void i40e_free_tx_resources(struct i40e_ring *tx_ring)
 /**
  * i40e_get_tx_pending - how many tx descriptors not processed
  * @tx_ring: the ring of descriptors
+ * @in_sw: use SW variables
  *
  * Since there is no access to the ring head register
  * in XL710, we need to use our local copies
  **/
-u32 i40e_get_tx_pending(struct i40e_ring *ring)
+u32 i40e_get_tx_pending(struct i40e_ring *ring, bool in_sw)
 {
 	u32 head, tail;
 
-	head = i40e_get_head(ring);
-	tail = readl(ring->tail);
+	if (!in_sw) {
+		head = i40e_get_head(ring);
+		tail = readl(ring->tail);
+	} else {
+		head = ring->next_to_clean;
+		tail = ring->next_to_use;
+	}
 
 	if (head != tail)
 		return (head < tail) ?
@@ -774,7 +780,7 @@ void i40e_detect_recover_hung(struct i40e_vsi *vsi)
 			 */
 			smp_rmb();
 			tx_ring->tx_stats.prev_pkt_ctr =
-			    i40e_get_tx_pending(tx_ring) ? packets : -1;
+			    i40e_get_tx_pending(tx_ring, true) ? packets : -1;
 		}
 	}
 }
@@ -898,7 +904,7 @@ static bool i40e_clean_tx_irq(struct i40e_vsi *vsi,
 		 * them to be written back in case we stay in NAPI.
 		 * In this mode on X722 we do not enable Interrupt.
 		 */
-		unsigned int j = i40e_get_tx_pending(tx_ring);
+		unsigned int j = i40e_get_tx_pending(tx_ring, false);
 
 		if (budget &&
 		    ((j / WB_STRIDE) == 0) && (j > 0) &&
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
index f75a8fe68fcf..3c80ea784389 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
@@ -505,7 +505,7 @@ void i40e_free_tx_resources(struct i40e_ring *tx_ring);
 void i40e_free_rx_resources(struct i40e_ring *rx_ring);
 int i40e_napi_poll(struct napi_struct *napi, int budget);
 void i40e_force_wb(struct i40e_vsi *vsi, struct i40e_q_vector *q_vector);
-u32 i40e_get_tx_pending(struct i40e_ring *ring);
+u32 i40e_get_tx_pending(struct i40e_ring *ring, bool in_sw);
 void i40e_detect_recover_hung(struct i40e_vsi *vsi);
 int __i40e_maybe_stop_tx(struct i40e_ring *tx_ring, int size);
 bool __i40e_chk_linearize(struct sk_buff *skb);
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index eb8f3e327f6b..e088d23eb083 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -196,7 +196,7 @@ void i40evf_detect_recover_hung(struct i40e_vsi *vsi)
 			 */
 			smp_rmb();
 			tx_ring->tx_stats.prev_pkt_ctr =
-			  i40evf_get_tx_pending(tx_ring, false) ? packets : -1;
+			  i40evf_get_tx_pending(tx_ring, true) ? packets : -1;
 		}
 	}
 }
-- 
2.14.3

[net-next 4/7] i40evf: pass struct virtchnl_filter by reference rather than by value

[Jeff Kirsher]

From: Colin Ian King <colin.king@canonical.com>

Passing struct virtchnl_filter f by value requires a 272 byte copy
on x86_64, so instead pass it by reference is much more efficient. Also
adjust some lines that are over 80 chars.

Detected by CoverityScan, CID#1465285 ("Big parameter passed by value")

Signed-off-by: Colin Ian King <colin.king@canonical.com>
Acked-by: Harshitha Ramamurthy <harshitha.ramamurthy@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 .../net/ethernet/intel/i40evf/i40evf_virtchnl.c    | 32 ++++++++++++----------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c b/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
index 6134b61e0938..3c76c817ca1a 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
@@ -1048,24 +1048,28 @@ void i40evf_disable_channels(struct i40evf_adapter *adapter)
  * Print the cloud filter
  **/
 static void i40evf_print_cloud_filter(struct i40evf_adapter *adapter,
-				      struct virtchnl_filter f)
+				      struct virtchnl_filter *f)
 {
-	switch (f.flow_type) {
+	switch (f->flow_type) {
 	case VIRTCHNL_TCP_V4_FLOW:
 		dev_info(&adapter->pdev->dev, "dst_mac: %pM src_mac: %pM vlan_id: %hu dst_ip: %pI4 src_ip %pI4 dst_port %hu src_port %hu\n",
-			 &f.data.tcp_spec.dst_mac, &f.data.tcp_spec.src_mac,
-			 ntohs(f.data.tcp_spec.vlan_id),
-			 &f.data.tcp_spec.dst_ip[0], &f.data.tcp_spec.src_ip[0],
-			 ntohs(f.data.tcp_spec.dst_port),
-			 ntohs(f.data.tcp_spec.src_port));
+			 &f->data.tcp_spec.dst_mac,
+			 &f->data.tcp_spec.src_mac,
+			 ntohs(f->data.tcp_spec.vlan_id),
+			 &f->data.tcp_spec.dst_ip[0],
+			 &f->data.tcp_spec.src_ip[0],
+			 ntohs(f->data.tcp_spec.dst_port),
+			 ntohs(f->data.tcp_spec.src_port));
 		break;
 	case VIRTCHNL_TCP_V6_FLOW:
 		dev_info(&adapter->pdev->dev, "dst_mac: %pM src_mac: %pM vlan_id: %hu dst_ip: %pI6 src_ip %pI6 dst_port %hu src_port %hu\n",
-			 &f.data.tcp_spec.dst_mac, &f.data.tcp_spec.src_mac,
-			 ntohs(f.data.tcp_spec.vlan_id),
-			 &f.data.tcp_spec.dst_ip, &f.data.tcp_spec.src_ip,
-			 ntohs(f.data.tcp_spec.dst_port),
-			 ntohs(f.data.tcp_spec.src_port));
+			 &f->data.tcp_spec.dst_mac,
+			 &f->data.tcp_spec.src_mac,
+			 ntohs(f->data.tcp_spec.vlan_id),
+			 &f->data.tcp_spec.dst_ip,
+			 &f->data.tcp_spec.src_ip,
+			 ntohs(f->data.tcp_spec.dst_port),
+			 ntohs(f->data.tcp_spec.src_port));
 		break;
 	}
 }
@@ -1303,7 +1307,7 @@ void i40evf_virtchnl_completion(struct i40evf_adapter *adapter,
 						 i40evf_stat_str(&adapter->hw,
 								 v_retval));
 					i40evf_print_cloud_filter(adapter,
-								  cf->f);
+								  &cf->f);
 					list_del(&cf->list);
 					kfree(cf);
 					adapter->num_cloud_filters--;
@@ -1322,7 +1326,7 @@ void i40evf_virtchnl_completion(struct i40evf_adapter *adapter,
 						 i40evf_stat_str(&adapter->hw,
 								 v_retval));
 					i40evf_print_cloud_filter(adapter,
-								  cf->f);
+								  &cf->f);
 				}
 			}
 			}
-- 
2.14.3

[net-next 5/7] i40e: check that pointer VSI is not null before dereferencing it

[Jeff Kirsher]

From: Colin Ian King <colin.king@canonical.com>

Function i40e_find_vsi_from_id can potentially return null, hence
VSI may be null, so defensively check it is non-null before
dereferencing it to check the seid.

Fixes: e284fc280473 ("i40e: Add and delete cloud filter")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Acked-by: Avinash Dayanand <avinash.dayanand@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_pf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 5cca083da93c..14bbd5c1db78 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -3062,7 +3062,7 @@ static struct i40e_vsi *i40e_find_vsi_from_seid(struct i40e_vf *vf, u16 seid)
 
 	for (i = 0; i < vf->num_tc ; i++) {
 		vsi = i40e_find_vsi_from_id(pf, vf->ch[i].vsi_id);
-		if (vsi->seid == seid)
+		if (vsi && vsi->seid == seid)
 			return vsi;
 	}
 	return NULL;
-- 
2.14.3

[net-next 6/7] i40evf: remove redundant array comparisons to 0 checks

[Jeff Kirsher]

From: Colin Ian King <colin.king@canonical.com>

The checks to see if key->dst.s6_addr and key->src.s6_addr are null
pointers are redundant because these are constant size arrays and
so the checks always return true.  Fix this by removing the redundant
checks.   Also replace filter->f with vf, allowing wide lines to be
condensed and to rejoin some split wide lines.

Detected by CoverityScan, CID#1465279 ("Array compared to 0")

Signed-off-by: Colin Ian King <colin.king@canonical.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40evf/i40evf_main.c | 61 ++++++++++---------------
 1 file changed, 25 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 4f5744d488aa..dae121877935 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -2490,6 +2490,7 @@ static int i40evf_parse_cls_flower(struct i40evf_adapter *adapter,
 	u16 addr_type = 0;
 	u16 n_proto = 0;
 	int i = 0;
+	struct virtchnl_filter *vf = &filter->f;
 
 	if (f->dissector->used_keys &
 	    ~(BIT(FLOW_DISSECTOR_KEY_CONTROL) |
@@ -2537,7 +2538,7 @@ static int i40evf_parse_cls_flower(struct i40evf_adapter *adapter,
 			return -EINVAL;
 		if (n_proto == ETH_P_IPV6) {
 			/* specify flow type as TCP IPv6 */
-			filter->f.flow_type = VIRTCHNL_TCP_V6_FLOW;
+			vf->flow_type = VIRTCHNL_TCP_V6_FLOW;
 		}
 
 		if (key->ip_proto != IPPROTO_TCP) {
@@ -2582,9 +2583,8 @@ static int i40evf_parse_cls_flower(struct i40evf_adapter *adapter,
 			    is_multicast_ether_addr(key->dst)) {
 				/* set the mask if a valid dst_mac address */
 				for (i = 0; i < ETH_ALEN; i++)
-					filter->f.mask.tcp_spec.dst_mac[i] |=
-									0xff;
-				ether_addr_copy(filter->f.data.tcp_spec.dst_mac,
+					vf->mask.tcp_spec.dst_mac[i] |= 0xff;
+				ether_addr_copy(vf->data.tcp_spec.dst_mac,
 						key->dst);
 			}
 
@@ -2593,9 +2593,8 @@ static int i40evf_parse_cls_flower(struct i40evf_adapter *adapter,
 			    is_multicast_ether_addr(key->src)) {
 				/* set the mask if a valid dst_mac address */
 				for (i = 0; i < ETH_ALEN; i++)
-					filter->f.mask.tcp_spec.src_mac[i] |=
-									0xff;
-				ether_addr_copy(filter->f.data.tcp_spec.src_mac,
+					vf->mask.tcp_spec.src_mac[i] |= 0xff;
+				ether_addr_copy(vf->data.tcp_spec.src_mac,
 						key->src);
 		}
 	}
@@ -2619,8 +2618,8 @@ static int i40evf_parse_cls_flower(struct i40evf_adapter *adapter,
 				return I40E_ERR_CONFIG;
 			}
 		}
-		filter->f.mask.tcp_spec.vlan_id |= cpu_to_be16(0xffff);
-		filter->f.data.tcp_spec.vlan_id = cpu_to_be16(key->vlan_id);
+		vf->mask.tcp_spec.vlan_id |= cpu_to_be16(0xffff);
+		vf->data.tcp_spec.vlan_id = cpu_to_be16(key->vlan_id);
 	}
 
 	if (dissector_uses_key(f->dissector, FLOW_DISSECTOR_KEY_CONTROL)) {
@@ -2667,14 +2666,12 @@ static int i40evf_parse_cls_flower(struct i40evf_adapter *adapter,
 			return I40E_ERR_CONFIG;
 		}
 		if (key->dst) {
-			filter->f.mask.tcp_spec.dst_ip[0] |=
-							cpu_to_be32(0xffffffff);
-			filter->f.data.tcp_spec.dst_ip[0] = key->dst;
+			vf->mask.tcp_spec.dst_ip[0] |= cpu_to_be32(0xffffffff);
+			vf->data.tcp_spec.dst_ip[0] = key->dst;
 		}
 		if (key->src) {
-			filter->f.mask.tcp_spec.src_ip[0] |=
-							cpu_to_be32(0xffffffff);
-			filter->f.data.tcp_spec.src_ip[0] = key->src;
+			vf->mask.tcp_spec.src_ip[0] |= cpu_to_be32(0xffffffff);
+			vf->data.tcp_spec.src_ip[0] = key->src;
 		}
 	}
 
@@ -2707,22 +2704,14 @@ static int i40evf_parse_cls_flower(struct i40evf_adapter *adapter,
 		if (!ipv6_addr_any(&mask->dst) || !ipv6_addr_any(&mask->src))
 			field_flags |= I40EVF_CLOUD_FIELD_IIP;
 
-		if (key->dst.s6_addr) {
-			for (i = 0; i < 4; i++)
-				filter->f.mask.tcp_spec.dst_ip[i] |=
-							cpu_to_be32(0xffffffff);
-			memcpy(&filter->f.data.tcp_spec.dst_ip,
-			       &key->dst.s6_addr32,
-			       sizeof(filter->f.data.tcp_spec.dst_ip));
-		}
-		if (key->src.s6_addr) {
-			for (i = 0; i < 4; i++)
-				filter->f.mask.tcp_spec.src_ip[i] |=
-							cpu_to_be32(0xffffffff);
-			memcpy(&filter->f.data.tcp_spec.src_ip,
-			       &key->src.s6_addr32,
-			       sizeof(filter->f.data.tcp_spec.src_ip));
-		}
+		for (i = 0; i < 4; i++)
+			vf->mask.tcp_spec.dst_ip[i] |= cpu_to_be32(0xffffffff);
+		memcpy(&vf->data.tcp_spec.dst_ip, &key->dst.s6_addr32,
+		       sizeof(vf->data.tcp_spec.dst_ip));
+		for (i = 0; i < 4; i++)
+			vf->mask.tcp_spec.src_ip[i] |= cpu_to_be32(0xffffffff);
+		memcpy(&vf->data.tcp_spec.src_ip, &key->src.s6_addr32,
+		       sizeof(vf->data.tcp_spec.src_ip));
 	}
 	if (dissector_uses_key(f->dissector, FLOW_DISSECTOR_KEY_PORTS)) {
 		struct flow_dissector_key_ports *key =
@@ -2754,16 +2743,16 @@ static int i40evf_parse_cls_flower(struct i40evf_adapter *adapter,
 			}
 		}
 		if (key->dst) {
-			filter->f.mask.tcp_spec.dst_port |= cpu_to_be16(0xffff);
-			filter->f.data.tcp_spec.dst_port = key->dst;
+			vf->mask.tcp_spec.dst_port |= cpu_to_be16(0xffff);
+			vf->data.tcp_spec.dst_port = key->dst;
 		}
 
 		if (key->src) {
-			filter->f.mask.tcp_spec.src_port |= cpu_to_be16(0xffff);
-			filter->f.data.tcp_spec.src_port = key->dst;
+			vf->mask.tcp_spec.src_port |= cpu_to_be16(0xffff);
+			vf->data.tcp_spec.src_port = key->dst;
 		}
 	}
-	filter->f.field_flags = field_flags;
+	vf->field_flags = field_flags;
 
 	return 0;
 }
-- 
2.14.3

[net-next 7/7] i40e: remove some stray indenting

[Jeff Kirsher]

From: Dan Carpenter <dan.carpenter@oracle.com>

These two lines are indented too far.

Signed-off-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_virtchnl_pf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 14bbd5c1db78..e23975c67417 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -3146,8 +3146,8 @@ static int i40e_vc_del_cloud_filter(struct i40e_vf *vf, u8 *msg)
 		dev_info(&pf->pdev->dev,
 			 "VF %d: Invalid input, can't apply cloud filter\n",
 			 vf->vf_id);
-			aq_ret = I40E_ERR_PARAM;
-			goto err;
+		aq_ret = I40E_ERR_PARAM;
+		goto err;
 	}
 
 	memset(&cfilter, 0, sizeof(cfilter));
-- 
2.14.3

Re: [net-next 1/7] i40e: link_down_on_close private flag support

[Jakub Kicinski]

On Mon, 26 Feb 2018 13:39:25 -0800, Jeff Kirsher wrote:
> From: Mariusz Stachura <mariusz.stachura@intel.com>
> 
> This patch introduces new ethtool private flag used for
> forcing true link state. Function i40e_force_link_state that implements
> this functionality was added, it sets phy_type = 0 in order to
> work-around firmware's LESM. False positive error messages were
> suppressed.
> 
> The ndo_open() should not succeed if there were issues with forcing link
> state to be UP.
> 
> Added I40E_PHY_TYPES_BITMASK define with all phy types OR-ed together in
> one bitmask.  Added after phy type definition, so it will be hard to
> forget to include new phy types to the bitmask.
> 
> CC: Jakub Kicinski <kubakici@wp.pl>
> Signed-off-by: Mariusz Stachura <mariusz.stachura@intel.com>
> Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Thanks, from functional perspective this looks reasonable AFAICT.

We may want to have a conversation about priv flags like this at later
time, since I guess more NICs won't bring the link down when netdev is
closed today..  Although it maybe a larger endeavour, perhaps it would 
be cool to communicate to the user the reason why MAC/PHY is on in some
standard way..  NC-SI and multi-host comes to mind.  

> @@ -7524,6 +7596,9 @@ int i40e_open(struct net_device *netdev)
>  
>  	netif_carrier_off(netdev);
>  
> +	if (i40e_force_link_state(pf, true))
> +		return -EAGAIN;

There are some minor style issues with the rest of the patch, but here
you may want to (a) propagate the actual error value;

>  	err = i40e_vsi_open(vsi);
>  	if (err)
>  		return err;

(b) undo the link state if vsi_open() fails.

Maybe that's a bit of a nitpick...

RE: [net-next 1/7] i40e: link_down_on_close private flag support

[Stachura, Mariusz]

>> From: Mariusz Stachura <mariusz.stachura@intel.com>
>> 
>> This patch introduces new ethtool private flag used for forcing true 
>> link state. Function i40e_force_link_state that implements this 
>> functionality was added, it sets phy_type = 0 in order to work-around 
>> firmware's LESM. False positive error messages were suppressed.
>> 
>> The ndo_open() should not succeed if there were issues with forcing 
>> link state to be UP.
>> 
>> Added I40E_PHY_TYPES_BITMASK define with all phy types OR-ed together 
>> in one bitmask.  Added after phy type definition, so it will be hard 
>> to forget to include new phy types to the bitmask.
>> 
>> CC: Jakub Kicinski <kubakici@wp.pl>
>> Signed-off-by: Mariusz Stachura <mariusz.stachura@intel.com>
>> Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
>> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> 
> Thanks, from functional perspective this looks reasonable AFAICT.
> 
> We may want to have a conversation about priv flags like this at later time, since I guess more NICs won't bring the link down when netdev is closed today..  Although it maybe a larger endeavour, perhaps it would be cool to communicate to the user the reason why MAC/PHY is on in some standard way..  NC-SI and multi-host comes to mind.  
> 

Yep, the main reason for not bringing the link down is NPAR and SRIOV functionality. Link is up by default, so I think it was assumed that there is no reason to inform the user about it. The operator can use private flag to force it down, assuming she/he knows it will disrupt any undergoing traffic. I get your idea, nonetheless it was designed to use slightly different approach.

>> @@ -7524,6 +7596,9 @@ int i40e_open(struct net_device *netdev)
>>  
>>  	netif_carrier_off(netdev);
>>  
>> +	if (i40e_force_link_state(pf, true))
>> +		return -EAGAIN;
> 
> There are some minor style issues with the rest of the patch, but here you may want to (a) propagate the actual error value;
> 

The actual error value is printed inside the i40e_force_link_state() function [with KERN_ERR severity], here I wanted to let the user know the generic information that the resource is temporarily unavailable.

>>  	err = i40e_vsi_open(vsi);
>>  	if (err)
>>  		return err;
> 
> (b) undo the link state if vsi_open() fails.
> 
> Maybe that's a bit of a nitpick...
> 

I think it is, in my humble opinion there is no need to force link back down if something went wrong during further setup. Of course I'm always open for discussion :)
--------------------------------------------------------------------

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.

Re: [net-next 0/7][pull request] 40GbE Intel Wired LAN Driver Updates 2018-02-26

[David Miller]

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 26 Feb 2018 13:39:24 -0800

> This series contains updates to i40e and i40evf only.

Pulled, but...

Please stop splitting Fixes: tags up into multiple lines.  Make it
always one line no matter how long the string is.

Also, please make sure that Jakub Kicinski's discussion on patch #1
continues to be followed up properly.

Thank you.

Re: [net-next 0/7][pull request] 40GbE Intel Wired LAN Driver Updates 2018-02-26

[Jeff Kirsher]

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

On Tue, 2018-02-27 at 12:57 -0500, David Miller wrote:
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Mon, 26 Feb 2018 13:39:24 -0800
> 
> > This series contains updates to i40e and i40evf only.
> 
> Pulled, but...
> 
> Please stop splitting Fixes: tags up into multiple lines.  Make it
> always one line no matter how long the string is.

Heard and understood, I will add it to my patch review and checks.

> Also, please make sure that Jakub Kicinski's discussion on patch #1
> continues to be followed up properly.

Agreed, Mariusz has been actively responding and following up on Jakub's
comments.

> Thank you.

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