[net-next 00/12][pull request] 40GbE Intel Wired LAN Driver Updates 2019-04-29

[net-next 00/12][pull request] 40GbE Intel Wired LAN Driver Updates 2019-04-29

[Jeff Kirsher]

This series contains updates to the i40e driver only.

Björn improves retpoline enabled builds by replacing the expensive switch
statements with 'if' statements that acts on the XDP program result.

Carolyn changes the driver behavior to now disable the VF after one MDD
event instead of allowing a couple of MDD events before doing the reset.

Aleksandr changes the driver to only report an error when a VF tries to
remove VLAN when a port VLAN is configured, unless it is VLAN 0.  Also
extends the LLDP support to be able to keep the current LLDP state
persistent across a power cycle.

Maciej fixes the checksum calculation due to firmware changes, which
requires the driver to perform a double shadow RAM dump in some cases.

Adam adds advertising support for 40GBase_LR4, 40GBase_CR4 and fibre in
the driver.

Jake cleans up a check that is not needed and was producing a warning in
GCC 8.

Harshitha fixes a misleading message by ensuring that a success message
is only printed on the host side when the promiscuous mode change has
been successful.

Stefan Assmann adds the vendor id and device id to the dmesg log entry
during probe to help with bug reports when lspci output may not be
available.

Alice and Piotr add recovery mode support in the i40e driver, which is
needed for migrating from a structured to a flat firmware image.

The following are changes since commit 88d6272acaaa4bfb03da0a87a8754ec431471680:
  net: phy: avoid unneeded MDIO reads in genphy_read_status
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 40GbE

Adam Ludkiewicz (1):
  i40e: Report advertised link modes on 40GBase_LR4, CR4 and fibre

Aleksandr Loktionov (2):
  i40e: remove error msg when vf with port vlan tries to remove vlan 0
  i40e: Further implementation of LLDP

Alice Michael (2):
  i40e: update version number
  i40e: Introduce recovery mode support

Björn Töpel (1):
  i40e: replace switch-statement to speed-up retpoline-enabled builds

Carolyn Wyborny (2):
  i40e: Fix for allowing too many MDD events on VF
  i40e: change behavior on PF in response to MDD event

Harshitha Ramamurthy (1):
  i40e: fix misleading message about promisc setting on un-trusted VF

Jacob Keller (1):
  i40e: remove out-of-range comparisons in i40e_validate_cloud_filter

Maciej Paczkowski (1):
  i40e: ShadowRAM checksum calculation change

Stefan Assmann (1):
  i40e: print PCI vendor and device ID during probe

 drivers/net/ethernet/intel/i40e/i40e.h        |   1 +
 drivers/net/ethernet/intel/i40e/i40e_adminq.c |   5 +
 .../net/ethernet/intel/i40e/i40e_adminq_cmd.h |  20 +-
 drivers/net/ethernet/intel/i40e/i40e_common.c |  62 ++-
 .../net/ethernet/intel/i40e/i40e_debugfs.c    |   4 +-
 .../net/ethernet/intel/i40e/i40e_ethtool.c    |  28 +-
 drivers/net/ethernet/intel/i40e/i40e_main.c   | 352 +++++++++++++++---
 drivers/net/ethernet/intel/i40e/i40e_nvm.c    |  29 +-
 .../net/ethernet/intel/i40e/i40e_prototype.h  |   8 +-
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |  32 +-
 .../ethernet/intel/i40e/i40e_txrx_common.h    |  27 ++
 drivers/net/ethernet/intel/i40e/i40e_type.h   |   1 +
 .../ethernet/intel/i40e/i40e_virtchnl_pf.c    |  35 +-
 drivers/net/ethernet/intel/i40e/i40e_xsk.c    |  24 +-
 14 files changed, 498 insertions(+), 130 deletions(-)

-- 
2.20.1

[net-next 01/12] i40e: replace switch-statement to speed-up retpoline-enabled builds

[Jeff Kirsher]

From: Björn Töpel <bjorn.topel@intel.com>

GCC will generate jump tables for switch-statements with more than 5
case statements. An entry into the jump table is an indirect call,
which means that for CONFIG_RETPOLINE builds, this is rather
expensive.

This commit replaces the switch-statement that acts on the XDP program
result with an if-clause.

The if-clause was also refactored into a common function that can be
used by AF_XDP zero-copy and non-zero-copy code.

Performance prior this patch:
$ sudo ./xdp_rxq_info --dev enp134s0f0 --action XDP_DROP
Running XDP on dev:enp134s0f0 (ifindex:7) action:XDP_DROP options:no_touch
XDP stats       CPU     pps         issue-pps
XDP-RX CPU      20      18983018    0
XDP-RX CPU      total   18983018

RXQ stats       RXQ:CPU pps         issue-pps
rx_queue_index   20:20  18983012    0
rx_queue_index   20:sum 18983012

$ sudo ./xdpsock -i enp134s0f0 -q 20 -n 2 -z -r
 sock0@enp134s0f0:20 rxdrop
                pps         pkts        2.00
rx              14,641,496  144,751,092
tx              0           0

And after:
$ sudo ./xdp_rxq_info --dev enp134s0f0 --action XDP_DROP
Running XDP on dev:enp134s0f0 (ifindex:7) action:XDP_DROP options:no_touch
XDP stats       CPU     pps         issue-pps
XDP-RX CPU      20      24000986    0
XDP-RX CPU      total   24000986

RXQ stats       RXQ:CPU pps         issue-pps
rx_queue_index   20:20  24000985    0
rx_queue_index   20:sum 24000985

  +26%

$ sudo ./xdpsock -i enp134s0f0 -q 20 -n 2 -z -r
 sock0@enp134s0f0:20 rxdrop
                pps         pkts        2.00
rx              17,623,578  163,503,263
tx              0           0

  +20%

Signed-off-by: Björn Töpel <bjorn.topel@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   | 32 ++++---------------
 .../ethernet/intel/i40e/i40e_txrx_common.h    | 27 ++++++++++++++++
 drivers/net/ethernet/intel/i40e/i40e_xsk.c    | 24 ++------------
 3 files changed, 36 insertions(+), 47 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index e1931701cd7e..d21d9377e9a7 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2,7 +2,7 @@
 /* Copyright(c) 2013 - 2018 Intel Corporation. */
 
 #include <linux/prefetch.h>
-#include <linux/bpf_trace.h>
+#include <linux/compiler.h>
 #include <net/xdp.h>
 #include "i40e.h"
 #include "i40e_trace.h"
@@ -2196,41 +2196,23 @@ int i40e_xmit_xdp_tx_ring(struct xdp_buff *xdp, struct i40e_ring *xdp_ring)
 static struct sk_buff *i40e_run_xdp(struct i40e_ring *rx_ring,
 				    struct xdp_buff *xdp)
 {
-	int err, result = I40E_XDP_PASS;
-	struct i40e_ring *xdp_ring;
 	struct bpf_prog *xdp_prog;
+	int result;
 	u32 act;
 
 	rcu_read_lock();
 	xdp_prog = READ_ONCE(rx_ring->xdp_prog);
 
-	if (!xdp_prog)
+	if (!xdp_prog) {
+		result = I40E_XDP_PASS;
 		goto xdp_out;
+	}
 
 	prefetchw(xdp->data_hard_start); /* xdp_frame write */
 
 	act = bpf_prog_run_xdp(xdp_prog, xdp);
-	switch (act) {
-	case XDP_PASS:
-		break;
-	case XDP_TX:
-		xdp_ring = rx_ring->vsi->xdp_rings[rx_ring->queue_index];
-		result = i40e_xmit_xdp_tx_ring(xdp, xdp_ring);
-		break;
-	case XDP_REDIRECT:
-		err = xdp_do_redirect(rx_ring->netdev, xdp, xdp_prog);
-		result = !err ? I40E_XDP_REDIR : I40E_XDP_CONSUMED;
-		break;
-	default:
-		bpf_warn_invalid_xdp_action(act);
-		/* fall through */
-	case XDP_ABORTED:
-		trace_xdp_exception(rx_ring->netdev, xdp_prog, act);
-		/* fall through -- handle aborts by dropping packet */
-	case XDP_DROP:
-		result = I40E_XDP_CONSUMED;
-		break;
-	}
+	i40e_xdp_do_action(act, &result, rx_ring, xdp, xdp_prog);
+
 xdp_out:
 	rcu_read_unlock();
 	return ERR_PTR(-result);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx_common.h b/drivers/net/ethernet/intel/i40e/i40e_txrx_common.h
index 8af0e99c6c0d..8cc4d8365f9e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx_common.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx_common.h
@@ -4,6 +4,8 @@
 #ifndef I40E_TXRX_COMMON_
 #define I40E_TXRX_COMMON_
 
+#include <linux/bpf_trace.h>
+
 void i40e_fd_handle_status(struct i40e_ring *rx_ring,
 			   union i40e_rx_desc *rx_desc, u8 prog_id);
 int i40e_xmit_xdp_tx_ring(struct xdp_buff *xdp, struct i40e_ring *xdp_ring);
@@ -88,4 +90,29 @@ void i40e_xsk_clean_rx_ring(struct i40e_ring *rx_ring);
 void i40e_xsk_clean_tx_ring(struct i40e_ring *tx_ring);
 bool i40e_xsk_any_rx_ring_enabled(struct i40e_vsi *vsi);
 
+static inline void i40e_xdp_do_action(u32 act, int *result,
+				      struct i40e_ring *rx_ring,
+				      struct xdp_buff *xdp,
+				      struct bpf_prog *xdp_prog)
+{
+	struct i40e_ring *xdp_ring;
+	int err;
+
+	if (act == XDP_TX) {
+		xdp_ring = rx_ring->vsi->xdp_rings[rx_ring->queue_index];
+		*result = i40e_xmit_xdp_tx_ring(xdp, xdp_ring);
+	} else if (act == XDP_REDIRECT) {
+		err = xdp_do_redirect(rx_ring->netdev, xdp, xdp_prog);
+		*result = !err ? I40E_XDP_REDIR : I40E_XDP_CONSUMED;
+	} else if (act == XDP_PASS) {
+		*result = I40E_XDP_PASS;
+	} else if (act == XDP_DROP) {
+		*result = I40E_XDP_CONSUMED;
+	} else {
+		if (act != XDP_ABORTED)
+			bpf_warn_invalid_xdp_action(act);
+		trace_xdp_exception(rx_ring->netdev, xdp_prog, act);
+		*result = I40E_XDP_CONSUMED;
+	}
+}
 #endif /* I40E_TXRX_COMMON_ */
diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 1b17486543ac..a16d9b78ade9 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -190,9 +190,8 @@ int i40e_xsk_umem_setup(struct i40e_vsi *vsi, struct xdp_umem *umem,
  **/
 static int i40e_run_xdp_zc(struct i40e_ring *rx_ring, struct xdp_buff *xdp)
 {
-	int err, result = I40E_XDP_PASS;
-	struct i40e_ring *xdp_ring;
 	struct bpf_prog *xdp_prog;
+	int result;
 	u32 act;
 
 	rcu_read_lock();
@@ -202,26 +201,7 @@ static int i40e_run_xdp_zc(struct i40e_ring *rx_ring, struct xdp_buff *xdp)
 	xdp_prog = READ_ONCE(rx_ring->xdp_prog);
 	act = bpf_prog_run_xdp(xdp_prog, xdp);
 	xdp->handle += xdp->data - xdp->data_hard_start;
-	switch (act) {
-	case XDP_PASS:
-		break;
-	case XDP_TX:
-		xdp_ring = rx_ring->vsi->xdp_rings[rx_ring->queue_index];
-		result = i40e_xmit_xdp_tx_ring(xdp, xdp_ring);
-		break;
-	case XDP_REDIRECT:
-		err = xdp_do_redirect(rx_ring->netdev, xdp, xdp_prog);
-		result = !err ? I40E_XDP_REDIR : I40E_XDP_CONSUMED;
-		break;
-	default:
-		bpf_warn_invalid_xdp_action(act);
-	case XDP_ABORTED:
-		trace_xdp_exception(rx_ring->netdev, xdp_prog, act);
-		/* fallthrough -- handle aborts by dropping packet */
-	case XDP_DROP:
-		result = I40E_XDP_CONSUMED;
-		break;
-	}
+	i40e_xdp_do_action(act, &result, rx_ring, xdp, xdp_prog);
 	rcu_read_unlock();
 	return result;
 }
-- 
2.20.1

[net-next 02/12] i40e: Fix for allowing too many MDD events on VF

[Jeff Kirsher]

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

This patch changes the driver behavior when detecting a VF MDD event.
It now disables the VF after one event, which indicates a hw detected
problem in the VF.  Before this change, the PF would allow a couple of
events before doing the reset.

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_main.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 65c2b9d2652b..b52a9d5644b8 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -9767,6 +9767,9 @@ static void i40e_handle_mdd_event(struct i40e_pf *pf)
 			vf->num_mdd_events++;
 			dev_info(&pf->pdev->dev, "TX driver issue detected on VF %d\n",
 				 i);
+			dev_info(&pf->pdev->dev,
+				 "Use PF Control I/F to re-enable the VF\n");
+			set_bit(I40E_VF_STATE_DISABLED, &vf->vf_states);
 		}
 
 		reg = rd32(hw, I40E_VP_MDET_RX(i));
@@ -9775,11 +9778,6 @@ static void i40e_handle_mdd_event(struct i40e_pf *pf)
 			vf->num_mdd_events++;
 			dev_info(&pf->pdev->dev, "RX driver issue detected on VF %d\n",
 				 i);
-		}
-
-		if (vf->num_mdd_events > I40E_DEFAULT_NUM_MDD_EVENTS_ALLOWED) {
-			dev_info(&pf->pdev->dev,
-				 "Too many MDD events on VF %d, disabled\n", i);
 			dev_info(&pf->pdev->dev,
 				 "Use PF Control I/F to re-enable the VF\n");
 			set_bit(I40E_VF_STATE_DISABLED, &vf->vf_states);
-- 
2.20.1

[net-next 03/12] i40e: change behavior on PF in response to MDD event

[Jeff Kirsher]

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

TX MDD events reported on the PF are the result of the
PF misconfiguring a descriptor and not because of "bad actions"
by anything else.  No need to reset now because if it
results in a Tx hang, the Tx hang check will take care of it.

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_main.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index b52a9d5644b8..3e15df1d5f52 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -9696,7 +9696,6 @@ static void i40e_handle_mdd_event(struct i40e_pf *pf)
 {
 	struct i40e_hw *hw = &pf->hw;
 	bool mdd_detected = false;
-	bool pf_mdd_detected = false;
 	struct i40e_vf *vf;
 	u32 reg;
 	int i;
@@ -9742,19 +9741,12 @@ static void i40e_handle_mdd_event(struct i40e_pf *pf)
 		reg = rd32(hw, I40E_PF_MDET_TX);
 		if (reg & I40E_PF_MDET_TX_VALID_MASK) {
 			wr32(hw, I40E_PF_MDET_TX, 0xFFFF);
-			dev_info(&pf->pdev->dev, "TX driver issue detected, PF reset issued\n");
-			pf_mdd_detected = true;
+			dev_dbg(&pf->pdev->dev, "TX driver issue detected on PF\n");
 		}
 		reg = rd32(hw, I40E_PF_MDET_RX);
 		if (reg & I40E_PF_MDET_RX_VALID_MASK) {
 			wr32(hw, I40E_PF_MDET_RX, 0xFFFF);
-			dev_info(&pf->pdev->dev, "RX driver issue detected, PF reset issued\n");
-			pf_mdd_detected = true;
-		}
-		/* Queue belongs to the PF, initiate a reset */
-		if (pf_mdd_detected) {
-			set_bit(__I40E_PF_RESET_REQUESTED, pf->state);
-			i40e_service_event_schedule(pf);
+			dev_dbg(&pf->pdev->dev, "RX driver issue detected on PF\n");
 		}
 	}
 
-- 
2.20.1

[net-next 04/12] i40e: remove error msg when vf with port vlan tries to remove vlan 0

[Jeff Kirsher]

From: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

VF's attempt to delete vlan 0 when a port vlan is configured is harmless
in this case pf driver just does nothing.  If vf will try to remove
other vlans when a port vlan is configured it will still produce error
as before.

Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@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 | 3 ++-
 1 file changed, 2 insertions(+), 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 71cd159e7902..24628de8e624 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -2766,7 +2766,8 @@ static int i40e_vc_remove_vlan_msg(struct i40e_vf *vf, u8 *msg)
 
 	vsi = pf->vsi[vf->lan_vsi_idx];
 	if (vsi->info.pvid) {
-		aq_ret = I40E_ERR_PARAM;
+		if (vfl->num_elements > 1 || vfl->vlan_id[0])
+			aq_ret = I40E_ERR_PARAM;
 		goto error_param;
 	}
 
-- 
2.20.1

[net-next 05/12] i40e: ShadowRAM checksum calculation change

[Jeff Kirsher]

From: Maciej Paczkowski <maciej.paczkowski@intel.com>

Due to changes in FW the SW is required to perform double SR dump in
some cases.

Implementation adds two new steps to update nvm checksum function:
* recalculate checksum and check if checksum in NVM is correct
* if checksum in NVM is not correct then update it again

Signed-off-by: Maciej Paczkowski <maciej.paczkowski@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_nvm.c | 29 +++++++++++++++++++---
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_nvm.c b/drivers/net/ethernet/intel/i40e/i40e_nvm.c
index 0299e5bbb902..ee89779a9a6f 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_nvm.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_nvm.c
@@ -574,13 +574,34 @@ static i40e_status i40e_calc_nvm_checksum(struct i40e_hw *hw,
 i40e_status i40e_update_nvm_checksum(struct i40e_hw *hw)
 {
 	i40e_status ret_code;
-	u16 checksum;
+	u16 checksum, checksum_sr;
 	__le16 le_sum;
 
 	ret_code = i40e_calc_nvm_checksum(hw, &checksum);
-	if (!ret_code) {
-		le_sum = cpu_to_le16(checksum);
-		ret_code = i40e_write_nvm_aq(hw, 0x00, I40E_SR_SW_CHECKSUM_WORD,
+	if (ret_code)
+		return ret_code;
+
+	le_sum = cpu_to_le16(checksum);
+	ret_code = i40e_write_nvm_aq(hw, 0x00, I40E_SR_SW_CHECKSUM_WORD,
+				     1, &le_sum, true);
+	if (ret_code)
+		return ret_code;
+
+	/* Due to changes in FW the SW is required to perform double SR-dump
+	 * in some cases. SR-dump is the process when internal shadow RAM is
+	 * dumped into flash bank. It is triggered by setting "last_command"
+	 * argument in i40e_write_nvm_aq function call.
+	 * Since FW 1.8 we need to calculate SR checksum again and update it
+	 * in flash if it is not equal to previously computed checksum.
+	 * This situation would occur only in FW >= 1.8
+	 */
+	ret_code = i40e_calc_nvm_checksum(hw, &checksum_sr);
+	if (ret_code)
+		return ret_code;
+	if (checksum_sr != checksum) {
+		le_sum = cpu_to_le16(checksum_sr);
+		ret_code = i40e_write_nvm_aq(hw, 0x00,
+					     I40E_SR_SW_CHECKSUM_WORD,
 					     1, &le_sum, true);
 	}
 
-- 
2.20.1

[net-next 06/12] i40e: Report advertised link modes on 40GBase_LR4, CR4 and fibre

[Jeff Kirsher]

From: Adam Ludkiewicz <adam.ludkiewicz@intel.com>

Add assignments for advertising 40GBase_LR4, 40GBase_CR4 and fibre

Signed-off-by: Adam Ludkiewicz <adam.ludkiewicz@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_ethtool.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 9eaea1bee4a1..0d923c13c9a1 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -541,9 +541,12 @@ static void i40e_phy_type_to_ethtool(struct i40e_pf *pf,
 		ethtool_link_ksettings_add_link_mode(ks, advertising,
 						     40000baseSR4_Full);
 	}
-	if (phy_types & I40E_CAP_PHY_TYPE_40GBASE_LR4)
+	if (phy_types & I40E_CAP_PHY_TYPE_40GBASE_LR4) {
 		ethtool_link_ksettings_add_link_mode(ks, supported,
 						     40000baseLR4_Full);
+		ethtool_link_ksettings_add_link_mode(ks, advertising,
+						     40000baseLR4_Full);
+	}
 	if (phy_types & I40E_CAP_PHY_TYPE_40GBASE_KR4) {
 		ethtool_link_ksettings_add_link_mode(ks, supported,
 						     40000baseLR4_Full);
@@ -723,6 +726,8 @@ static void i40e_get_settings_link_up(struct i40e_hw *hw,
 	case I40E_PHY_TYPE_40GBASE_AOC:
 		ethtool_link_ksettings_add_link_mode(ks, supported,
 						     40000baseCR4_Full);
+		ethtool_link_ksettings_add_link_mode(ks, advertising,
+						     40000baseCR4_Full);
 		break;
 	case I40E_PHY_TYPE_40GBASE_SR4:
 		ethtool_link_ksettings_add_link_mode(ks, supported,
@@ -733,6 +738,8 @@ static void i40e_get_settings_link_up(struct i40e_hw *hw,
 	case I40E_PHY_TYPE_40GBASE_LR4:
 		ethtool_link_ksettings_add_link_mode(ks, supported,
 						     40000baseLR4_Full);
+		ethtool_link_ksettings_add_link_mode(ks, advertising,
+						     40000baseLR4_Full);
 		break;
 	case I40E_PHY_TYPE_25GBASE_SR:
 	case I40E_PHY_TYPE_25GBASE_LR:
@@ -1038,6 +1045,7 @@ static int i40e_get_link_ksettings(struct net_device *netdev,
 		break;
 	case I40E_MEDIA_TYPE_FIBER:
 		ethtool_link_ksettings_add_link_mode(ks, supported, FIBRE);
+		ethtool_link_ksettings_add_link_mode(ks, advertising, FIBRE);
 		ks->base.port = PORT_FIBRE;
 		break;
 	case I40E_MEDIA_TYPE_UNKNOWN:
-- 
2.20.1

[net-next 07/12] i40e: Further implementation of LLDP

[Jeff Kirsher]

From: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

This code implements driver code changes necessary for LLDP
Agent support. Modified i40e_aq_start_lldp() and
i40e_aq_stop_lldp() adding false parameter whether LLDP state
should be persistent across power cycles.

Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@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.c |  5 ++
 .../net/ethernet/intel/i40e/i40e_adminq_cmd.h | 20 ++++--
 drivers/net/ethernet/intel/i40e/i40e_common.c | 62 ++++++++++++++++++-
 .../net/ethernet/intel/i40e/i40e_debugfs.c    |  4 +-
 .../net/ethernet/intel/i40e/i40e_ethtool.c    |  4 +-
 drivers/net/ethernet/intel/i40e/i40e_main.c   |  2 +-
 .../net/ethernet/intel/i40e/i40e_prototype.h  |  8 ++-
 drivers/net/ethernet/intel/i40e/i40e_type.h   |  1 +
 8 files changed, 93 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
index 45f6adc8ff2f..243dcd4bec19 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
@@ -608,6 +608,11 @@ i40e_status i40e_init_adminq(struct i40e_hw *hw)
 	     hw->aq.api_min_ver >= 7))
 		hw->flags |= I40E_HW_FLAG_802_1AD_CAPABLE;
 
+	if (hw->aq.api_maj_ver > 1 ||
+	    (hw->aq.api_maj_ver == 1 &&
+	     hw->aq.api_min_ver >= 8))
+		hw->flags |= I40E_HW_FLAG_FW_LLDP_PERSISTENT;
+
 	if (hw->aq.api_maj_ver > I40E_FW_API_VERSION_MAJOR) {
 		ret_code = I40E_ERR_FIRMWARE_API_VERSION;
 		goto init_adminq_free_arq;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h b/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
index 522058a7d4be..abcf79eb3261 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
@@ -261,6 +261,7 @@ enum i40e_admin_queue_opc {
 	i40e_aqc_opc_get_cee_dcb_cfg	= 0x0A07,
 	i40e_aqc_opc_lldp_set_local_mib	= 0x0A08,
 	i40e_aqc_opc_lldp_stop_start_spec_agent	= 0x0A09,
+	i40e_aqc_opc_lldp_restore		= 0x0A0A,
 
 	/* Tunnel commands */
 	i40e_aqc_opc_add_udp_tunnel	= 0x0B00,
@@ -2498,18 +2499,19 @@ I40E_CHECK_CMD_LENGTH(i40e_aqc_lldp_update_tlv);
 /* Stop LLDP (direct 0x0A05) */
 struct i40e_aqc_lldp_stop {
 	u8	command;
-#define I40E_AQ_LLDP_AGENT_STOP		0x0
-#define I40E_AQ_LLDP_AGENT_SHUTDOWN	0x1
+#define I40E_AQ_LLDP_AGENT_STOP			0x0
+#define I40E_AQ_LLDP_AGENT_SHUTDOWN		0x1
+#define I40E_AQ_LLDP_AGENT_STOP_PERSIST		0x2
 	u8	reserved[15];
 };
 
 I40E_CHECK_CMD_LENGTH(i40e_aqc_lldp_stop);
 
 /* Start LLDP (direct 0x0A06) */
-
 struct i40e_aqc_lldp_start {
 	u8	command;
-#define I40E_AQ_LLDP_AGENT_START	0x1
+#define I40E_AQ_LLDP_AGENT_START		0x1
+#define I40E_AQ_LLDP_AGENT_START_PERSIST	0x2
 	u8	reserved[15];
 };
 
@@ -2633,6 +2635,16 @@ struct i40e_aqc_lldp_stop_start_specific_agent {
 
 I40E_CHECK_CMD_LENGTH(i40e_aqc_lldp_stop_start_specific_agent);
 
+/* Restore LLDP Agent factory settings (direct 0x0A0A) */
+struct i40e_aqc_lldp_restore {
+	u8	command;
+#define I40E_AQ_LLDP_AGENT_RESTORE_NOT		0x0
+#define I40E_AQ_LLDP_AGENT_RESTORE		0x1
+	u8	reserved[15];
+};
+
+I40E_CHECK_CMD_LENGTH(i40e_aqc_lldp_restore);
+
 /* Add Udp Tunnel command and completion (direct 0x0B00) */
 struct i40e_aqc_add_udp_tunnel {
 	__le16	udp_port;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c
index dd6b3b3ac5c6..e7d500f92a90 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
@@ -3623,15 +3623,55 @@ i40e_status i40e_aq_cfg_lldp_mib_change_event(struct i40e_hw *hw,
 	return status;
 }
 
+/**
+ * i40e_aq_restore_lldp
+ * @hw: pointer to the hw struct
+ * @setting: pointer to factory setting variable or NULL
+ * @restore: True if factory settings should be restored
+ * @cmd_details: pointer to command details structure or NULL
+ *
+ * Restore LLDP Agent factory settings if @restore set to True. In other case
+ * only returns factory setting in AQ response.
+ **/
+enum i40e_status_code
+i40e_aq_restore_lldp(struct i40e_hw *hw, u8 *setting, bool restore,
+		     struct i40e_asq_cmd_details *cmd_details)
+{
+	struct i40e_aq_desc desc;
+	struct i40e_aqc_lldp_restore *cmd =
+		(struct i40e_aqc_lldp_restore *)&desc.params.raw;
+	i40e_status status;
+
+	if (!(hw->flags & I40E_HW_FLAG_FW_LLDP_PERSISTENT)) {
+		i40e_debug(hw, I40E_DEBUG_ALL,
+			   "Restore LLDP not supported by current FW version.\n");
+		return I40E_ERR_DEVICE_NOT_SUPPORTED;
+	}
+
+	i40e_fill_default_direct_cmd_desc(&desc, i40e_aqc_opc_lldp_restore);
+
+	if (restore)
+		cmd->command |= I40E_AQ_LLDP_AGENT_RESTORE;
+
+	status = i40e_asq_send_command(hw, &desc, NULL, 0, cmd_details);
+
+	if (setting)
+		*setting = cmd->command & 1;
+
+	return status;
+}
+
 /**
  * i40e_aq_stop_lldp
  * @hw: pointer to the hw struct
  * @shutdown_agent: True if LLDP Agent needs to be Shutdown
+ * @persist: True if stop of LLDP should be persistent across power cycles
  * @cmd_details: pointer to command details structure or NULL
  *
  * Stop or Shutdown the embedded LLDP Agent
  **/
 i40e_status i40e_aq_stop_lldp(struct i40e_hw *hw, bool shutdown_agent,
+				bool persist,
 				struct i40e_asq_cmd_details *cmd_details)
 {
 	struct i40e_aq_desc desc;
@@ -3644,6 +3684,14 @@ i40e_status i40e_aq_stop_lldp(struct i40e_hw *hw, bool shutdown_agent,
 	if (shutdown_agent)
 		cmd->command |= I40E_AQ_LLDP_AGENT_SHUTDOWN;
 
+	if (persist) {
+		if (hw->flags & I40E_HW_FLAG_FW_LLDP_PERSISTENT)
+			cmd->command |= I40E_AQ_LLDP_AGENT_STOP_PERSIST;
+		else
+			i40e_debug(hw, I40E_DEBUG_ALL,
+				   "Persistent Stop LLDP not supported by current FW version.\n");
+	}
+
 	status = i40e_asq_send_command(hw, &desc, NULL, 0, cmd_details);
 
 	return status;
@@ -3653,13 +3701,14 @@ i40e_status i40e_aq_stop_lldp(struct i40e_hw *hw, bool shutdown_agent,
  * i40e_aq_start_lldp
  * @hw: pointer to the hw struct
  * @buff: buffer for result
+ * @persist: True if start of LLDP should be persistent across power cycles
  * @buff_size: buffer size
  * @cmd_details: pointer to command details structure or NULL
  *
  * Start the embedded LLDP Agent on all ports.
  **/
-i40e_status i40e_aq_start_lldp(struct i40e_hw *hw,
-				struct i40e_asq_cmd_details *cmd_details)
+i40e_status i40e_aq_start_lldp(struct i40e_hw *hw, bool persist,
+			       struct i40e_asq_cmd_details *cmd_details)
 {
 	struct i40e_aq_desc desc;
 	struct i40e_aqc_lldp_start *cmd =
@@ -3669,6 +3718,15 @@ i40e_status i40e_aq_start_lldp(struct i40e_hw *hw,
 	i40e_fill_default_direct_cmd_desc(&desc, i40e_aqc_opc_lldp_start);
 
 	cmd->command = I40E_AQ_LLDP_AGENT_START;
+
+	if (persist) {
+		if (hw->flags & I40E_HW_FLAG_FW_LLDP_PERSISTENT)
+			cmd->command |= I40E_AQ_LLDP_AGENT_START_PERSIST;
+		else
+			i40e_debug(hw, I40E_DEBUG_ALL,
+				   "Persistent Start LLDP not supported by current FW version.\n");
+	}
+
 	status = i40e_asq_send_command(hw, &desc, NULL, 0, cmd_details);
 
 	return status;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
index c67d485d6f99..7ea4f09229e4 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
@@ -1321,7 +1321,7 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
 		if (strncmp(&cmd_buf[5], "stop", 4) == 0) {
 			int ret;
 
-			ret = i40e_aq_stop_lldp(&pf->hw, false, NULL);
+			ret = i40e_aq_stop_lldp(&pf->hw, false, false, NULL);
 			if (ret) {
 				dev_info(&pf->pdev->dev,
 					 "Stop LLDP AQ command failed =0x%x\n",
@@ -1358,7 +1358,7 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
 				/* Continue and start FW LLDP anyways */
 			}
 
-			ret = i40e_aq_start_lldp(&pf->hw, NULL);
+			ret = i40e_aq_start_lldp(&pf->hw, false, NULL);
 			if (ret) {
 				dev_info(&pf->pdev->dev,
 					 "Start LLDP AQ command failed =0x%x\n",
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 0d923c13c9a1..32e137499063 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -4958,7 +4958,7 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags)
 		if (pf->flags & I40E_FLAG_DISABLE_FW_LLDP) {
 			struct i40e_dcbx_config *dcbcfg;
 
-			i40e_aq_stop_lldp(&pf->hw, true, NULL);
+			i40e_aq_stop_lldp(&pf->hw, true, false, NULL);
 			i40e_aq_set_dcb_parameters(&pf->hw, true, NULL);
 			/* reset local_dcbx_config to default */
 			dcbcfg = &pf->hw.local_dcbx_config;
@@ -4973,7 +4973,7 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags)
 			dcbcfg->pfc.willing = 1;
 			dcbcfg->pfc.pfccap = I40E_MAX_TRAFFIC_CLASS;
 		} else {
-			i40e_aq_start_lldp(&pf->hw, NULL);
+			i40e_aq_start_lldp(&pf->hw, false, NULL);
 		}
 	}
 
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 3e15df1d5f52..54c172c50479 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -14132,7 +14132,7 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	 */
 	if (pf->hw_features & I40E_HW_STOP_FW_LLDP) {
 		dev_info(&pdev->dev, "Stopping firmware LLDP agent.\n");
-		i40e_aq_stop_lldp(hw, true, NULL);
+		i40e_aq_stop_lldp(hw, true, false, NULL);
 	}
 
 	/* allow a platform config to override the HW addr */
diff --git a/drivers/net/ethernet/intel/i40e/i40e_prototype.h b/drivers/net/ethernet/intel/i40e/i40e_prototype.h
index 663c8bf4d3d8..882627073dce 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_prototype.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_prototype.h
@@ -203,14 +203,18 @@ i40e_status i40e_aq_get_lldp_mib(struct i40e_hw *hw, u8 bridge_type,
 i40e_status i40e_aq_cfg_lldp_mib_change_event(struct i40e_hw *hw,
 				bool enable_update,
 				struct i40e_asq_cmd_details *cmd_details);
+enum i40e_status_code
+i40e_aq_restore_lldp(struct i40e_hw *hw, u8 *setting, bool restore,
+		     struct i40e_asq_cmd_details *cmd_details);
 i40e_status i40e_aq_stop_lldp(struct i40e_hw *hw, bool shutdown_agent,
+			      bool persist,
 				struct i40e_asq_cmd_details *cmd_details);
 i40e_status i40e_aq_set_dcb_parameters(struct i40e_hw *hw,
 				       bool dcb_enable,
 				       struct i40e_asq_cmd_details
 				       *cmd_details);
-i40e_status i40e_aq_start_lldp(struct i40e_hw *hw,
-				struct i40e_asq_cmd_details *cmd_details);
+i40e_status i40e_aq_start_lldp(struct i40e_hw *hw, bool persist,
+			       struct i40e_asq_cmd_details *cmd_details);
 i40e_status i40e_aq_get_cee_dcb_config(struct i40e_hw *hw,
 				       void *buff, u16 buff_size,
 				       struct i40e_asq_cmd_details *cmd_details);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h
index 79420bcc7414..820af0043cc8 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_type.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_type.h
@@ -616,6 +616,7 @@ struct i40e_hw {
 #define I40E_HW_FLAG_AQ_PHY_ACCESS_CAPABLE  BIT_ULL(2)
 #define I40E_HW_FLAG_NVM_READ_REQUIRES_LOCK BIT_ULL(3)
 #define I40E_HW_FLAG_FW_LLDP_STOPPABLE      BIT_ULL(4)
+#define I40E_HW_FLAG_FW_LLDP_PERSISTENT     BIT_ULL(5)
 	u64 flags;
 
 	/* Used in set switch config AQ command */
-- 
2.20.1

[net-next 08/12] i40e: remove out-of-range comparisons in i40e_validate_cloud_filter

[Jeff Kirsher]

From: Jacob Keller <jacob.e.keller@intel.com>

The function i40e_validate_cloud_filter checks that the destination and
source port numbers are valid by attempting to ensure that the number is
non-zero and no larger than 0xFFFF. However, the types for the dst_port
and src_port variable are __be16 which by definition cannot be larger
than 0xFFFF

Since these values cannot be larger than 2 bytes, the check to see if
they exceed 0xFFFF is meaningless.

One might consider these checks as some sort of defensive coding, in
case the type was later changed. However, these checks also byte-swap
the value before comparison using be16_to_cpu, which will truncate the
values to 16bits anyways. Additionally, changing the type would require
updating the opcodes to support new data layout of these virtchnl
commands.

Remove the check to silence the -Wtype-limits warning that was added to
GCC 8.

Signed-off-by: Jacob Keller <jacob.e.keller@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 | 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 24628de8e624..925ca880bea3 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -3129,7 +3129,7 @@ static int i40e_validate_cloud_filter(struct i40e_vf *vf,
 	}
 
 	if (mask.dst_port & data.dst_port) {
-		if (!data.dst_port || be16_to_cpu(data.dst_port) > 0xFFFF) {
+		if (!data.dst_port) {
 			dev_info(&pf->pdev->dev, "VF %d: Invalid Dest port\n",
 				 vf->vf_id);
 			goto err;
@@ -3137,7 +3137,7 @@ static int i40e_validate_cloud_filter(struct i40e_vf *vf,
 	}
 
 	if (mask.src_port & data.src_port) {
-		if (!data.src_port || be16_to_cpu(data.src_port) > 0xFFFF) {
+		if (!data.src_port) {
 			dev_info(&pf->pdev->dev, "VF %d: Invalid Source port\n",
 				 vf->vf_id);
 			goto err;
-- 
2.20.1

[net-next 09/12] i40e: update version number

[Jeff Kirsher]

From: Alice Michael <alice.michael@intel.com>

Just bumping the version number appropriately.

Signed-off-by: Alice Michael <alice.michael@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 | 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 54c172c50479..9ea0556c8962 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -27,7 +27,7 @@ static const char i40e_driver_string[] =
 
 #define DRV_VERSION_MAJOR 2
 #define DRV_VERSION_MINOR 8
-#define DRV_VERSION_BUILD 10
+#define DRV_VERSION_BUILD 20
 #define DRV_VERSION __stringify(DRV_VERSION_MAJOR) "." \
 	     __stringify(DRV_VERSION_MINOR) "." \
 	     __stringify(DRV_VERSION_BUILD)    DRV_KERN
-- 
2.20.1

[net-next 10/12] i40e: fix misleading message about promisc setting on un-trusted VF

[Jeff Kirsher]

From: Harshitha Ramamurthy <harshitha.ramamurthy@intel.com>

A refactor of the i40e_vc_config_promiscuous_mode_msg function moved
the check for un-trusted VF into another function. We have to lie to
an un-trusted VF that its request to set promiscuous mode is
successful even when it is not because we don't want the VF to find
out its trust status this way. With the refactor, we were running into
a case where even though we were not setting promiscuous mode for an
un-trusted VF, we still printed a misleading message that it was
successful.

This patch fixes that by ensuring that a success message is printed
on the host side only when the promiscuous mode change has been
successful.

Signed-off-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>
---
 .../ethernet/intel/i40e/i40e_virtchnl_pf.c    | 28 +++++++++++--------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 925ca880bea3..8a6fb9c03955 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -1112,15 +1112,6 @@ static i40e_status i40e_config_vf_promiscuous_mode(struct i40e_vf *vf,
 	if (!i40e_vc_isvalid_vsi_id(vf, vsi_id) || !vsi)
 		return I40E_ERR_PARAM;
 
-	if (!test_bit(I40E_VIRTCHNL_VF_CAP_PRIVILEGE, &vf->vf_caps) &&
-	    (allmulti || alluni)) {
-		dev_err(&pf->pdev->dev,
-			"Unprivileged VF %d is attempting to configure promiscuous mode\n",
-			vf->vf_id);
-		/* Lie to the VF on purpose. */
-		return 0;
-	}
-
 	if (vf->port_vlan_id) {
 		aq_ret = i40e_aq_set_vsi_mc_promisc_on_vlan(hw, vsi->seid,
 							    allmulti,
@@ -1997,8 +1988,21 @@ static int i40e_vc_config_promiscuous_mode_msg(struct i40e_vf *vf, u8 *msg)
 	bool allmulti = false;
 	bool alluni = false;
 
-	if (!test_bit(I40E_VF_STATE_ACTIVE, &vf->vf_states))
-		return I40E_ERR_PARAM;
+	if (!test_bit(I40E_VF_STATE_ACTIVE, &vf->vf_states)) {
+		aq_ret = I40E_ERR_PARAM;
+		goto err_out;
+	}
+	if (!test_bit(I40E_VIRTCHNL_VF_CAP_PRIVILEGE, &vf->vf_caps)) {
+		dev_err(&pf->pdev->dev,
+			"Unprivileged VF %d is attempting to configure promiscuous mode\n",
+			vf->vf_id);
+
+		/* Lie to the VF on purpose, because this is an error we can
+		 * ignore. Unprivileged VF is not a virtual channel error.
+		 */
+		aq_ret = 0;
+		goto err_out;
+	}
 
 	/* Multicast promiscuous handling*/
 	if (info->flags & FLAG_VF_MULTICAST_PROMISC)
@@ -2032,7 +2036,7 @@ static int i40e_vc_config_promiscuous_mode_msg(struct i40e_vf *vf, u8 *msg)
 			clear_bit(I40E_VF_STATE_UC_PROMISC, &vf->vf_states);
 		}
 	}
-
+err_out:
 	/* send the response to the VF */
 	return i40e_vc_send_resp_to_vf(vf,
 				       VIRTCHNL_OP_CONFIG_PROMISCUOUS_MODE,
-- 
2.20.1

[net-next 11/12] i40e: print PCI vendor and device ID during probe

[Jeff Kirsher]

From: Stefan Assmann <sassmann@kpanic.de>

Printing each devices PCI vendor and device ID has the advantage of
easily revealing what hardware we're dealing with exactly. It's no
longer necessary to match the PCI bus information to the lspci output.

Helps with bug reports where no lspci output is available.

Output before
i40e 0000:08:00.0: fw 6.1.49420 api 1.7 nvm 6.80 0x80003c64 1.2007.0
and after
i40e 0000:08:00.0: fw 6.1.49420 api 1.7 nvm 6.80 0x80003c64 1.2007.0 [8086:1572] [8086:0004]

Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
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 | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 9ea0556c8962..c2673d2cef8e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -14073,11 +14073,12 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	}
 	i40e_get_oem_version(hw);
 
-	/* provide nvm, fw, api versions */
-	dev_info(&pdev->dev, "fw %d.%d.%05d api %d.%d nvm %s\n",
+	/* provide nvm, fw, api versions, vendor:device id, subsys vendor:device id */
+	dev_info(&pdev->dev, "fw %d.%d.%05d api %d.%d nvm %s [%04x:%04x] [%04x:%04x]\n",
 		 hw->aq.fw_maj_ver, hw->aq.fw_min_ver, hw->aq.fw_build,
 		 hw->aq.api_maj_ver, hw->aq.api_min_ver,
-		 i40e_nvm_version_str(hw));
+		 i40e_nvm_version_str(hw), hw->vendor_id, hw->device_id,
+		 hw->subsystem_vendor_id, hw->subsystem_device_id);
 
 	if (hw->aq.api_maj_ver == I40E_FW_API_VERSION_MAJOR &&
 	    hw->aq.api_min_ver > I40E_FW_MINOR_VERSION(hw))
-- 
2.20.1

[net-next 12/12] i40e: Introduce recovery mode support

[Jeff Kirsher]

From: Alice Michael <alice.michael@intel.com>

This patch introduces "recovery mode" to the i40e driver. It is
part of a new Any2Any idea of upgrading the firmware. In this
approach, it is required for the driver to have support for
"transition firmware", that is used for migrating from structured
to flat firmware image. In this new, very basic mode, i40e driver
must be able to handle particular IOCTL calls from the NVM Update
Tool and run a small set of AQ commands.

Signed-off-by: Alice Michael <alice.michael@intel.com>
Signed-off-by: Piotr Marczak <piotr.marczak@intel.com>
Tested-by: Don Buchholz <donald.buchholz@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e.h        |   1 +
 .../net/ethernet/intel/i40e/i40e_ethtool.c    |  14 +-
 drivers/net/ethernet/intel/i40e/i40e_main.c   | 321 ++++++++++++++++--
 3 files changed, 305 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index c4afb852cb57..7ce42040b851 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -149,6 +149,7 @@ enum i40e_state_t {
 	__I40E_CLIENT_L2_CHANGE,
 	__I40E_CLIENT_RESET,
 	__I40E_VIRTCHNL_OP_PENDING,
+	__I40E_RECOVERY_MODE,
 	/* This must be last as it determines the size of the BITMAP */
 	__I40E_STATE_SIZE__,
 };
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 32e137499063..2c81afbd7c58 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -5141,6 +5141,12 @@ static int i40e_get_module_eeprom(struct net_device *netdev,
 	return 0;
 }
 
+static const struct ethtool_ops i40e_ethtool_recovery_mode_ops = {
+	.set_eeprom		= i40e_set_eeprom,
+	.get_eeprom_len		= i40e_get_eeprom_len,
+	.get_eeprom		= i40e_get_eeprom,
+};
+
 static const struct ethtool_ops i40e_ethtool_ops = {
 	.get_drvinfo		= i40e_get_drvinfo,
 	.get_regs_len		= i40e_get_regs_len,
@@ -5189,5 +5195,11 @@ static const struct ethtool_ops i40e_ethtool_ops = {
 
 void i40e_set_ethtool_ops(struct net_device *netdev)
 {
-	netdev->ethtool_ops = &i40e_ethtool_ops;
+	struct i40e_netdev_priv *np = netdev_priv(netdev);
+	struct i40e_pf		*pf = np->vsi->back;
+
+	if (!test_bit(__I40E_RECOVERY_MODE, pf->state))
+		netdev->ethtool_ops = &i40e_ethtool_ops;
+	else
+		netdev->ethtool_ops = &i40e_ethtool_recovery_mode_ops;
 }
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index c2673d2cef8e..6c48a96604df 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -46,6 +46,10 @@ static int i40e_setup_pf_filter_control(struct i40e_pf *pf);
 static void i40e_prep_for_reset(struct i40e_pf *pf, bool lock_acquired);
 static int i40e_reset(struct i40e_pf *pf);
 static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired);
+static int i40e_setup_misc_vector_for_recovery_mode(struct i40e_pf *pf);
+static int i40e_restore_interrupt_scheme(struct i40e_pf *pf);
+static bool i40e_check_recovery_mode(struct i40e_pf *pf);
+static int i40e_init_recovery_mode(struct i40e_pf *pf, struct i40e_hw *hw);
 static void i40e_fdir_sb_setup(struct i40e_pf *pf);
 static int i40e_veb_get_bw_info(struct i40e_veb *veb);
 static int i40e_get_capabilities(struct i40e_pf *pf,
@@ -278,8 +282,9 @@ struct i40e_vsi *i40e_find_vsi_from_id(struct i40e_pf *pf, u16 id)
  **/
 void i40e_service_event_schedule(struct i40e_pf *pf)
 {
-	if (!test_bit(__I40E_DOWN, pf->state) &&
-	    !test_bit(__I40E_RESET_RECOVERY_PENDING, pf->state))
+	if ((!test_bit(__I40E_DOWN, pf->state) &&
+	     !test_bit(__I40E_RESET_RECOVERY_PENDING, pf->state)) ||
+	      test_bit(__I40E_RECOVERY_MODE, pf->state))
 		queue_work(i40e_wq, &pf->service_task);
 }
 
@@ -4019,7 +4024,8 @@ static irqreturn_t i40e_intr(int irq, void *data)
 enable_intr:
 	/* re-enable interrupt causes */
 	wr32(hw, I40E_PFINT_ICR0_ENA, ena_mask);
-	if (!test_bit(__I40E_DOWN, pf->state)) {
+	if (!test_bit(__I40E_DOWN, pf->state) ||
+	    test_bit(__I40E_RECOVERY_MODE, pf->state)) {
 		i40e_service_event_schedule(pf);
 		i40e_irq_dynamic_enable_icr0(pf);
 	}
@@ -9409,6 +9415,7 @@ static int i40e_reset(struct i40e_pf *pf)
  **/
 static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired)
 {
+	int old_recovery_mode_bit = test_bit(__I40E_RECOVERY_MODE, pf->state);
 	struct i40e_vsi *vsi = pf->vsi[pf->lan_vsi];
 	struct i40e_hw *hw = &pf->hw;
 	u8 set_fc_aq_fail = 0;
@@ -9416,7 +9423,16 @@ static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired)
 	u32 val;
 	int v;
 
-	if (test_bit(__I40E_DOWN, pf->state))
+	if (test_bit(__I40E_EMP_RESET_INTR_RECEIVED, pf->state) &&
+	    i40e_check_recovery_mode(pf)) {
+#ifdef SIOCETHTOOL
+		i40e_set_ethtool_ops(pf->vsi[pf->lan_vsi]->netdev);
+#endif
+	}
+
+	if (test_bit(__I40E_DOWN, pf->state) &&
+	    !test_bit(__I40E_RECOVERY_MODE, pf->state) &&
+	    !old_recovery_mode_bit)
 		goto clear_recovery;
 	dev_dbg(&pf->pdev->dev, "Rebuilding internal switch\n");
 
@@ -9445,6 +9461,44 @@ static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired)
 	if (test_and_clear_bit(__I40E_EMP_RESET_INTR_RECEIVED, pf->state))
 		i40e_verify_eeprom(pf);
 
+	/* if we are going out of or into recovery mode we have to act
+	 * accordingly with regard to resources initialization
+	 * and deinitialization
+	 */
+	if (test_bit(__I40E_RECOVERY_MODE, pf->state) ||
+	    old_recovery_mode_bit) {
+		if (i40e_get_capabilities(pf,
+					  i40e_aqc_opc_list_func_capabilities))
+			goto end_unlock;
+
+		if (test_bit(__I40E_RECOVERY_MODE, pf->state)) {
+			/* we're staying in recovery mode so we'll reinitialize
+			 * misc vector here
+			 */
+			if (i40e_setup_misc_vector_for_recovery_mode(pf))
+				goto end_unlock;
+		} else {
+			if (!lock_acquired)
+				rtnl_lock();
+			/* we're going out of recovery mode so we'll free
+			 * the IRQ allocated specifically for recovery mode
+			 * and restore the interrupt scheme
+			 */
+			free_irq(pf->pdev->irq, pf);
+			i40e_clear_interrupt_scheme(pf);
+			if (i40e_restore_interrupt_scheme(pf))
+				goto end_unlock;
+		}
+
+		/* tell the firmware that we're starting */
+		i40e_send_version(pf);
+
+		/* bail out in case recovery mode was detected, as there is
+		 * no need for further configuration.
+		 */
+		goto end_unlock;
+	}
+
 	i40e_clear_pxe_mode(hw);
 	ret = i40e_get_capabilities(pf, i40e_aqc_opc_list_func_capabilities);
 	if (ret)
@@ -9896,31 +9950,38 @@ static void i40e_service_task(struct work_struct *work)
 	unsigned long start_time = jiffies;
 
 	/* don't bother with service tasks if a reset is in progress */
-	if (test_bit(__I40E_RESET_RECOVERY_PENDING, pf->state))
+	if (test_bit(__I40E_RESET_RECOVERY_PENDING, pf->state) ||
+	    test_bit(__I40E_SUSPENDED, pf->state))
 		return;
 
 	if (test_and_set_bit(__I40E_SERVICE_SCHED, pf->state))
 		return;
 
-	i40e_detect_recover_hung(pf->vsi[pf->lan_vsi]);
-	i40e_sync_filters_subtask(pf);
-	i40e_reset_subtask(pf);
-	i40e_handle_mdd_event(pf);
-	i40e_vc_process_vflr_event(pf);
-	i40e_watchdog_subtask(pf);
-	i40e_fdir_reinit_subtask(pf);
-	if (test_and_clear_bit(__I40E_CLIENT_RESET, pf->state)) {
-		/* Client subtask will reopen next time through. */
-		i40e_notify_client_of_netdev_close(pf->vsi[pf->lan_vsi], true);
+	if (!test_bit(__I40E_RECOVERY_MODE, pf->state)) {
+		i40e_detect_recover_hung(pf->vsi[pf->lan_vsi]);
+		i40e_sync_filters_subtask(pf);
+		i40e_reset_subtask(pf);
+		i40e_handle_mdd_event(pf);
+		i40e_vc_process_vflr_event(pf);
+		i40e_watchdog_subtask(pf);
+		i40e_fdir_reinit_subtask(pf);
+		if (test_and_clear_bit(__I40E_CLIENT_RESET, pf->state)) {
+			/* Client subtask will reopen next time through. */
+			i40e_notify_client_of_netdev_close(pf->vsi[pf->lan_vsi],
+							   true);
+		} else {
+			i40e_client_subtask(pf);
+			if (test_and_clear_bit(__I40E_CLIENT_L2_CHANGE,
+					       pf->state))
+				i40e_notify_client_of_l2_param_changes(
+								pf->vsi[pf->lan_vsi]);
+		}
+		i40e_sync_filters_subtask(pf);
+		i40e_sync_udp_filters_subtask(pf);
 	} else {
-		i40e_client_subtask(pf);
-		if (test_and_clear_bit(__I40E_CLIENT_L2_CHANGE,
-				       pf->state))
-			i40e_notify_client_of_l2_param_changes(
-							pf->vsi[pf->lan_vsi]);
-	}
-	i40e_sync_filters_subtask(pf);
-	i40e_sync_udp_filters_subtask(pf);
+		i40e_reset_subtask(pf);
+	}
+
 	i40e_clean_adminq_subtask(pf);
 
 	/* flush memory to make sure state is correct before next watchdog */
@@ -10742,6 +10803,48 @@ static int i40e_restore_interrupt_scheme(struct i40e_pf *pf)
 	return err;
 }
 
+/**
+ * i40e_setup_misc_vector_for_recovery_mode - Setup the misc vector to handle
+ * non queue events in recovery mode
+ * @pf: board private structure
+ *
+ * This sets up the handler for MSIX 0 or MSI/legacy, which is used to manage
+ * the non-queue interrupts, e.g. AdminQ and errors in recovery mode.
+ * This is handled differently than in recovery mode since no Tx/Rx resources
+ * are being allocated.
+ **/
+static int i40e_setup_misc_vector_for_recovery_mode(struct i40e_pf *pf)
+{
+	int err;
+
+	if (pf->flags & I40E_FLAG_MSIX_ENABLED) {
+		err = i40e_setup_misc_vector(pf);
+
+		if (err) {
+			dev_info(&pf->pdev->dev,
+				 "MSI-X misc vector request failed, error %d\n",
+				 err);
+			return err;
+		}
+	} else {
+		u32 flags = pf->flags & I40E_FLAG_MSI_ENABLED ? 0 : IRQF_SHARED;
+
+		err = request_irq(pf->pdev->irq, i40e_intr, flags,
+				  pf->int_name, pf);
+
+		if (err) {
+			dev_info(&pf->pdev->dev,
+				 "MSI/legacy misc vector request failed, error %d\n",
+				 err);
+			return err;
+		}
+		i40e_enable_misc_int_causes(pf);
+		i40e_irq_dynamic_enable_icr0(pf);
+	}
+
+	return 0;
+}
+
 /**
  * i40e_setup_misc_vector - Setup the misc vector to handle non queue events
  * @pf: board private structure
@@ -13904,6 +14007,134 @@ void i40e_set_fec_in_flags(u8 fec_cfg, u32 *flags)
 		*flags &= ~(I40E_FLAG_RS_FEC | I40E_FLAG_BASE_R_FEC);
 }
 
+/**
+ * i40e_check_recovery_mode - check if we are running transition firmware
+ * @pf: board private structure
+ *
+ * Check registers indicating the firmware runs in recovery mode. Sets the
+ * appropriate driver state.
+ *
+ * Returns true if the recovery mode was detected, false otherwise
+ **/
+static bool i40e_check_recovery_mode(struct i40e_pf *pf)
+{
+	u32 val = rd32(&pf->hw, I40E_GL_FWSTS);
+
+	if (val & I40E_GL_FWSTS_FWS1B_MASK) {
+		dev_notice(&pf->pdev->dev, "Firmware recovery mode detected. Limiting functionality.\n");
+		dev_notice(&pf->pdev->dev, "Refer to the Intel(R) Ethernet Adapters and Devices User Guide for details on firmware recovery mode.\n");
+		set_bit(__I40E_RECOVERY_MODE, pf->state);
+
+		return true;
+	}
+	if (test_and_clear_bit(__I40E_RECOVERY_MODE, pf->state))
+		dev_info(&pf->pdev->dev, "Reinitializing in normal mode with full functionality.\n");
+
+	return false;
+}
+
+/**
+ * i40e_init_recovery_mode - initialize subsystems needed in recovery mode
+ * @pf: board private structure
+ * @hw: ptr to the hardware info
+ *
+ * This function does a minimal setup of all subsystems needed for running
+ * recovery mode.
+ *
+ * Returns 0 on success, negative on failure
+ **/
+static int i40e_init_recovery_mode(struct i40e_pf *pf, struct i40e_hw *hw)
+{
+	struct i40e_vsi *vsi;
+	int err;
+	int v_idx;
+
+#ifdef HAVE_PCI_ERS
+	pci_save_state(pf->pdev);
+#endif
+
+	/* set up periodic task facility */
+	timer_setup(&pf->service_timer, i40e_service_timer, 0);
+	pf->service_timer_period = HZ;
+
+	INIT_WORK(&pf->service_task, i40e_service_task);
+	clear_bit(__I40E_SERVICE_SCHED, pf->state);
+
+	err = i40e_init_interrupt_scheme(pf);
+	if (err)
+		goto err_switch_setup;
+
+	/* The number of VSIs reported by the FW is the minimum guaranteed
+	 * to us; HW supports far more and we share the remaining pool with
+	 * the other PFs. We allocate space for more than the guarantee with
+	 * the understanding that we might not get them all later.
+	 */
+	if (pf->hw.func_caps.num_vsis < I40E_MIN_VSI_ALLOC)
+		pf->num_alloc_vsi = I40E_MIN_VSI_ALLOC;
+	else
+		pf->num_alloc_vsi = pf->hw.func_caps.num_vsis;
+
+	/* Set up the vsi struct and our local tracking of the MAIN PF vsi. */
+	pf->vsi = kcalloc(pf->num_alloc_vsi, sizeof(struct i40e_vsi *),
+			  GFP_KERNEL);
+	if (!pf->vsi) {
+		err = -ENOMEM;
+		goto err_switch_setup;
+	}
+
+	/* We allocate one VSI which is needed as absolute minimum
+	 * in order to register the netdev
+	 */
+	v_idx = i40e_vsi_mem_alloc(pf, I40E_VSI_MAIN);
+	if (v_idx < 0)
+		goto err_switch_setup;
+	pf->lan_vsi = v_idx;
+	vsi = pf->vsi[v_idx];
+	if (!vsi)
+		goto err_switch_setup;
+	vsi->alloc_queue_pairs = 1;
+	err = i40e_config_netdev(vsi);
+	if (err)
+		goto err_switch_setup;
+	err = register_netdev(vsi->netdev);
+	if (err)
+		goto err_switch_setup;
+	vsi->netdev_registered = true;
+	i40e_dbg_pf_init(pf);
+
+	err = i40e_setup_misc_vector_for_recovery_mode(pf);
+	if (err)
+		goto err_switch_setup;
+
+	/* tell the firmware that we're starting */
+	i40e_send_version(pf);
+
+	/* since everything's happy, start the service_task timer */
+	mod_timer(&pf->service_timer,
+		  round_jiffies(jiffies + pf->service_timer_period));
+
+	return 0;
+
+err_switch_setup:
+	i40e_reset_interrupt_capability(pf);
+	del_timer_sync(&pf->service_timer);
+#ifdef NOT_FOR_UPSTREAM
+	dev_warn(&pf->pdev->dev, "previous errors forcing module to load in debug mode\n");
+	i40e_dbg_pf_init(pf);
+	set_bit(__I40E_DEBUG_MODE, pf->state);
+	return 0;
+#else
+	i40e_shutdown_adminq(hw);
+	iounmap(hw->hw_addr);
+	pci_disable_pcie_error_reporting(pf->pdev);
+	pci_release_mem_regions(pf->pdev);
+	pci_disable_device(pf->pdev);
+	kfree(pf);
+
+	return err;
+#endif
+}
+
 /**
  * i40e_probe - Device initialization routine
  * @pdev: PCI device information struct
@@ -14029,13 +14260,14 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	/* Reset here to make sure all is clean and to define PF 'n' */
 	i40e_clear_hw(hw);
-	err = i40e_pf_reset(hw);
-	if (err) {
-		dev_info(&pdev->dev, "Initial pf_reset failed: %d\n", err);
-		goto err_pf_reset;
+	if (!i40e_check_recovery_mode(pf)) {
+		err = i40e_pf_reset(hw);
+		if (err) {
+			dev_info(&pdev->dev, "Initial pf_reset failed: %d\n", err);
+			goto err_pf_reset;
+		}
+		pf->pfr_count++;
 	}
-	pf->pfr_count++;
-
 	hw->aq.num_arq_entries = I40E_AQ_LEN;
 	hw->aq.num_asq_entries = I40E_AQ_LEN;
 	hw->aq.arq_buf_size = I40E_MAX_AQ_BUF_SIZE;
@@ -14103,6 +14335,7 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		dev_warn(&pdev->dev, "This device is a pre-production adapter/LOM. Please be aware there may be issues with your hardware. If you are experiencing problems please contact your Intel or hardware representative who provided you with this hardware.\n");
 
 	i40e_clear_pxe_mode(hw);
+
 	err = i40e_get_capabilities(pf, i40e_aqc_opc_list_func_capabilities);
 	if (err)
 		goto err_adminq_setup;
@@ -14113,6 +14346,9 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto err_sw_init;
 	}
 
+	if (test_bit(__I40E_RECOVERY_MODE, pf->state))
+		return i40e_init_recovery_mode(pf, hw);
+
 	err = i40e_init_lan_hmc(hw, hw->func_caps.num_tx_qp,
 				hw->func_caps.num_rx_qp, 0, 0);
 	if (err) {
@@ -14498,6 +14734,19 @@ static void i40e_remove(struct pci_dev *pdev)
 	if (pf->service_task.func)
 		cancel_work_sync(&pf->service_task);
 
+	if (test_bit(__I40E_RECOVERY_MODE, pf->state)) {
+		struct i40e_vsi *vsi = pf->vsi[0];
+
+		/* We know that we have allocated only one vsi for this PF,
+		 * it was just for registering netdevice, so the interface
+		 * could be visible in the 'ifconfig' output
+		 */
+		unregister_netdev(vsi->netdev);
+		free_netdev(vsi->netdev);
+
+		goto unmap;
+	}
+
 	/* Client close must be called explicitly here because the timer
 	 * has been stopped.
 	 */
@@ -14547,6 +14796,12 @@ static void i40e_remove(struct pci_dev *pdev)
 				 ret_code);
 	}
 
+unmap:
+	/* Free MSI/legacy interrupt 0 when in recovery mode. */
+	if (test_bit(__I40E_RECOVERY_MODE, pf->state) &&
+	    !(pf->flags & I40E_FLAG_MSIX_ENABLED))
+		free_irq(pf->pdev->irq, pf);
+
 	/* shutdown the adminq */
 	i40e_shutdown_adminq(hw);
 
@@ -14559,7 +14814,8 @@ static void i40e_remove(struct pci_dev *pdev)
 	i40e_clear_interrupt_scheme(pf);
 	for (i = 0; i < pf->num_alloc_vsi; i++) {
 		if (pf->vsi[i]) {
-			i40e_vsi_clear_rings(pf->vsi[i]);
+			if (!test_bit(__I40E_RECOVERY_MODE, pf->state))
+				i40e_vsi_clear_rings(pf->vsi[i]);
 			i40e_vsi_clear(pf->vsi[i]);
 			pf->vsi[i] = NULL;
 		}
@@ -14767,6 +15023,11 @@ static void i40e_shutdown(struct pci_dev *pdev)
 	wr32(hw, I40E_PFPM_WUFC,
 	     (pf->wol_en ? I40E_PFPM_WUFC_MAG_MASK : 0));
 
+	/* Free MSI/legacy interrupt 0 when in recovery mode. */
+	if (test_bit(__I40E_RECOVERY_MODE, pf->state) &&
+	    !(pf->flags & I40E_FLAG_MSIX_ENABLED))
+		free_irq(pf->pdev->irq, pf);
+
 	/* Since we're going to destroy queues during the
 	 * i40e_clear_interrupt_scheme() we should hold the RTNL lock for this
 	 * whole section
-- 
2.20.1

Re: [net-next 01/12] i40e: replace switch-statement to speed-up retpoline-enabled builds

[Josh Elsasser]

On Apr 29, 2019, at 12:16 PM, Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:

> From: Björn Töpel <bjorn.topel@intel.com>
> 
> GCC will generate jump tables for switch-statements with more than 5
> case statements. An entry into the jump table is an indirect call,
> which means that for CONFIG_RETPOLINE builds, this is rather
> expensive.
> 
> This commit replaces the switch-statement that acts on the XDP program
> result with an if-clause.

Apologies for the noise, but is this patch still required after the
recent threshold bump[0] and later removal[1] of switch-case jump
table generation when building with CONFIG_RETPOLINE?

[0]: https://lore.kernel.org/patchwork/patch/1044863/
[1]: https://lore.kernel.org/patchwork/patch/1054472/

If nothing else the commit message no longer seems accurate.

Regards,
-- Josh

Re: [net-next 12/12] i40e: Introduce recovery mode support

[Jakub Kicinski]

On Mon, 29 Apr 2019 12:16:28 -0700, Jeff Kirsher wrote:
> From: Alice Michael <alice.michael@intel.com>
> 
> This patch introduces "recovery mode" to the i40e driver. It is
> part of a new Any2Any idea of upgrading the firmware. In this
> approach, it is required for the driver to have support for
> "transition firmware", that is used for migrating from structured
> to flat firmware image. In this new, very basic mode, i40e driver
> must be able to handle particular IOCTL calls from the NVM Update
> Tool and run a small set of AQ commands.

Could you show us commands that get executed?  I think that'd be much
quicker for people to parse.

> Signed-off-by: Alice Michael <alice.michael@intel.com>
> Signed-off-by: Piotr Marczak <piotr.marczak@intel.com>
> Tested-by: Don Buchholz <donald.buchholz@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

From a cursory look it seems you create a "basic" netdev.  Can this
netdev pass traffic?

I'd suggest you implement devlink "limp mode".  Devlink can flash the
device now.  You can register a devlink instance without registering
any "minimal" netdevs, and flash with devlink.

> @@ -13904,6 +14007,134 @@ void i40e_set_fec_in_flags(u8 fec_cfg, u32 *flags)
>  		*flags &= ~(I40E_FLAG_RS_FEC | I40E_FLAG_BASE_R_FEC);
>  }
>  
> +/**
> + * i40e_check_recovery_mode - check if we are running transition firmware
> + * @pf: board private structure
> + *
> + * Check registers indicating the firmware runs in recovery mode. Sets the
> + * appropriate driver state.
> + *
> + * Returns true if the recovery mode was detected, false otherwise
> + **/
> +static bool i40e_check_recovery_mode(struct i40e_pf *pf)
> +{
> +	u32 val = rd32(&pf->hw, I40E_GL_FWSTS);
> +
> +	if (val & I40E_GL_FWSTS_FWS1B_MASK) {
> +		dev_notice(&pf->pdev->dev, "Firmware recovery mode detected. Limiting functionality.\n");
> +		dev_notice(&pf->pdev->dev, "Refer to the Intel(R) Ethernet Adapters and Devices User Guide for details on firmware recovery mode.\n");
> +		set_bit(__I40E_RECOVERY_MODE, pf->state);
> +
> +		return true;
> +	}
> +	if (test_and_clear_bit(__I40E_RECOVERY_MODE, pf->state))
> +		dev_info(&pf->pdev->dev, "Reinitializing in normal mode with full functionality.\n");
> +
> +	return false;
> +}
> +
> +/**
> + * i40e_init_recovery_mode - initialize subsystems needed in recovery mode
> + * @pf: board private structure
> + * @hw: ptr to the hardware info
> + *
> + * This function does a minimal setup of all subsystems needed for running
> + * recovery mode.
> + *
> + * Returns 0 on success, negative on failure
> + **/
> +static int i40e_init_recovery_mode(struct i40e_pf *pf, struct i40e_hw *hw)
> +{
> +	struct i40e_vsi *vsi;
> +	int err;
> +	int v_idx;
> +
> +#ifdef HAVE_PCI_ERS
> +	pci_save_state(pf->pdev);
> +#endif
> +
> +	/* set up periodic task facility */
> +	timer_setup(&pf->service_timer, i40e_service_timer, 0);
> +	pf->service_timer_period = HZ;
> +
> +	INIT_WORK(&pf->service_task, i40e_service_task);
> +	clear_bit(__I40E_SERVICE_SCHED, pf->state);
> +
> +	err = i40e_init_interrupt_scheme(pf);
> +	if (err)
> +		goto err_switch_setup;
> +
> +	/* The number of VSIs reported by the FW is the minimum guaranteed
> +	 * to us; HW supports far more and we share the remaining pool with
> +	 * the other PFs. We allocate space for more than the guarantee with
> +	 * the understanding that we might not get them all later.
> +	 */
> +	if (pf->hw.func_caps.num_vsis < I40E_MIN_VSI_ALLOC)
> +		pf->num_alloc_vsi = I40E_MIN_VSI_ALLOC;
> +	else
> +		pf->num_alloc_vsi = pf->hw.func_caps.num_vsis;
> +
> +	/* Set up the vsi struct and our local tracking of the MAIN PF vsi. */
> +	pf->vsi = kcalloc(pf->num_alloc_vsi, sizeof(struct i40e_vsi *),
> +			  GFP_KERNEL);
> +	if (!pf->vsi) {
> +		err = -ENOMEM;
> +		goto err_switch_setup;
> +	}
> +
> +	/* We allocate one VSI which is needed as absolute minimum
> +	 * in order to register the netdev
> +	 */
> +	v_idx = i40e_vsi_mem_alloc(pf, I40E_VSI_MAIN);
> +	if (v_idx < 0)
> +		goto err_switch_setup;
> +	pf->lan_vsi = v_idx;
> +	vsi = pf->vsi[v_idx];
> +	if (!vsi)
> +		goto err_switch_setup;
> +	vsi->alloc_queue_pairs = 1;
> +	err = i40e_config_netdev(vsi);
> +	if (err)
> +		goto err_switch_setup;
> +	err = register_netdev(vsi->netdev);
> +	if (err)
> +		goto err_switch_setup;
> +	vsi->netdev_registered = true;
> +	i40e_dbg_pf_init(pf);
> +
> +	err = i40e_setup_misc_vector_for_recovery_mode(pf);
> +	if (err)
> +		goto err_switch_setup;
> +
> +	/* tell the firmware that we're starting */
> +	i40e_send_version(pf);
> +
> +	/* since everything's happy, start the service_task timer */
> +	mod_timer(&pf->service_timer,
> +		  round_jiffies(jiffies + pf->service_timer_period));
> +
> +	return 0;
> +
> +err_switch_setup:
> +	i40e_reset_interrupt_capability(pf);
> +	del_timer_sync(&pf->service_timer);
> +#ifdef NOT_FOR_UPSTREAM

Delightful :)

> +	dev_warn(&pf->pdev->dev, "previous errors forcing module to load in debug mode\n");
> +	i40e_dbg_pf_init(pf);
> +	set_bit(__I40E_DEBUG_MODE, pf->state);
> +	return 0;
> +#else
> +	i40e_shutdown_adminq(hw);
> +	iounmap(hw->hw_addr);
> +	pci_disable_pcie_error_reporting(pf->pdev);
> +	pci_release_mem_regions(pf->pdev);
> +	pci_disable_device(pf->pdev);
> +	kfree(pf);
> +
> +	return err;
> +#endif
> +}
> +
>  /**
>   * i40e_probe - Device initialization routine
>   * @pdev: PCI device information struct

RE: [net-next 01/12] i40e: replace switch-statement to speed-up retpoline-enabled builds

[David Laight]

From: Josh Elsasser
> Sent: 29 April 2019 21:02
> On Apr 29, 2019, at 12:16 PM, Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> 
> > From: Björn Töpel <bjorn.topel@intel.com>
> >
> > GCC will generate jump tables for switch-statements with more than 5
> > case statements. An entry into the jump table is an indirect call,
> > which means that for CONFIG_RETPOLINE builds, this is rather
> > expensive.
> >
> > This commit replaces the switch-statement that acts on the XDP program
> > result with an if-clause.
> 
> Apologies for the noise, but is this patch still required after the
> recent threshold bump[0] and later removal[1] of switch-case jump
> table generation when building with CONFIG_RETPOLINE?
> 
> [0]: https://lore.kernel.org/patchwork/patch/1044863/
> [1]: https://lore.kernel.org/patchwork/patch/1054472/
> 
> If nothing else the commit message no longer seems accurate.

Looking at those two patches, the second one seems wrong:

   # Additionally, avoid generating expensive indirect jumps which
   # are subject to retpolines for small number of switch cases.
   # clang turns off jump table generation by default when under
-  # retpoline builds, however, gcc does not for x86.
-  KBUILD_CFLAGS += $(call cc-option,--param=case-values-threshold=20)
+  # retpoline builds, however, gcc does not for x86. This has
+  # only been fixed starting from gcc stable version 8.4.0 and
+  # onwards, but not for older ones. See gcc bug #86952.
+  ifndef CONFIG_CC_IS_CLANG
+    KBUILD_CFLAGS += $(call cc-option,-fno-jump-tables)
+  endif

If -fno-jump-tables isn't supported then --param=case-values-threshold=20
needs to be set (if supported).

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [net-next 12/12] i40e: Introduce recovery mode support

[Jeff Kirsher]

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

On Mon, 2019-04-29 at 21:07 -0400, Jakub Kicinski wrote:
> On Mon, 29 Apr 2019 12:16:28 -0700, Jeff Kirsher wrote:
> > From: Alice Michael <alice.michael@intel.com>
> > 
> > This patch introduces "recovery mode" to the i40e driver. It is
> > part of a new Any2Any idea of upgrading the firmware. In this
> > approach, it is required for the driver to have support for
> > "transition firmware", that is used for migrating from structured
> > to flat firmware image. In this new, very basic mode, i40e driver
> > must be able to handle particular IOCTL calls from the NVM Update
> > Tool and run a small set of AQ commands.
> 
> Could you show us commands that get executed?  I think that'd be much
> quicker for people to parse.
> 
> > Signed-off-by: Alice Michael <alice.michael@intel.com>
> > Signed-off-by: Piotr Marczak <piotr.marczak@intel.com>
> > Tested-by: Don Buchholz <donald.buchholz@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> 
> From a cursory look it seems you create a "basic" netdev.  Can this
> netdev pass traffic?
> 
> I'd suggest you implement devlink "limp mode".  Devlink can flash the
> device now.  You can register a devlink instance without registering
> any "minimal" netdevs, and flash with devlink.

Good to know, currently this driver and our other LAN drivers do not have
devlink support, but we are currently working to rectify that.  I am hoping
that we can get devlink support in i40e and other drivers in the 5.3 or 5.4
kernel.  

Alice has updated this patch to add the requested additional information to
the patch description and cleaned up the code not intended for the Linux
kernel driver.  So I will resubmit this series with the updates, once it
goes through our validation checks.

> 
> > @@ -13904,6 +14007,134 @@ void i40e_set_fec_in_flags(u8 fec_cfg, u32
> > *flags)
> >  		*flags &= ~(I40E_FLAG_RS_FEC | I40E_FLAG_BASE_R_FEC);
> >  }
> >  
> > +/**
> > + * i40e_check_recovery_mode - check if we are running transition
> > firmware
> > + * @pf: board private structure
> > + *
> > + * Check registers indicating the firmware runs in recovery mode. Sets
> > the
> > + * appropriate driver state.
> > + *
> > + * Returns true if the recovery mode was detected, false otherwise
> > + **/
> > +static bool i40e_check_recovery_mode(struct i40e_pf *pf)
> > +{
> > +	u32 val = rd32(&pf->hw, I40E_GL_FWSTS);
> > +
> > +	if (val & I40E_GL_FWSTS_FWS1B_MASK) {
> > +		dev_notice(&pf->pdev->dev, "Firmware recovery mode
> > detected. Limiting functionality.\n");
> > +		dev_notice(&pf->pdev->dev, "Refer to the Intel(R) Ethernet
> > Adapters and Devices User Guide for details on firmware recovery
> > mode.\n");
> > +		set_bit(__I40E_RECOVERY_MODE, pf->state);
> > +
> > +		return true;
> > +	}
> > +	if (test_and_clear_bit(__I40E_RECOVERY_MODE, pf->state))
> > +		dev_info(&pf->pdev->dev, "Reinitializing in normal mode
> > with full functionality.\n");
> > +
> > +	return false;
> > +}
> > +
> > +/**
> > + * i40e_init_recovery_mode - initialize subsystems needed in recovery
> > mode
> > + * @pf: board private structure
> > + * @hw: ptr to the hardware info
> > + *
> > + * This function does a minimal setup of all subsystems needed for
> > running
> > + * recovery mode.
> > + *
> > + * Returns 0 on success, negative on failure
> > + **/
> > +static int i40e_init_recovery_mode(struct i40e_pf *pf, struct i40e_hw
> > *hw)
> > +{
> > +	struct i40e_vsi *vsi;
> > +	int err;
> > +	int v_idx;
> > +
> > +#ifdef HAVE_PCI_ERS
> > +	pci_save_state(pf->pdev);
> > +#endif
> > +
> > +	/* set up periodic task facility */
> > +	timer_setup(&pf->service_timer, i40e_service_timer, 0);
> > +	pf->service_timer_period = HZ;
> > +
> > +	INIT_WORK(&pf->service_task, i40e_service_task);
> > +	clear_bit(__I40E_SERVICE_SCHED, pf->state);
> > +
> > +	err = i40e_init_interrupt_scheme(pf);
> > +	if (err)
> > +		goto err_switch_setup;
> > +
> > +	/* The number of VSIs reported by the FW is the minimum guaranteed
> > +	 * to us; HW supports far more and we share the remaining pool with
> > +	 * the other PFs. We allocate space for more than the guarantee
> > with
> > +	 * the understanding that we might not get them all later.
> > +	 */
> > +	if (pf->hw.func_caps.num_vsis < I40E_MIN_VSI_ALLOC)
> > +		pf->num_alloc_vsi = I40E_MIN_VSI_ALLOC;
> > +	else
> > +		pf->num_alloc_vsi = pf->hw.func_caps.num_vsis;
> > +
> > +	/* Set up the vsi struct and our local tracking of the MAIN PF vsi.
> > */
> > +	pf->vsi = kcalloc(pf->num_alloc_vsi, sizeof(struct i40e_vsi *),
> > +			  GFP_KERNEL);
> > +	if (!pf->vsi) {
> > +		err = -ENOMEM;
> > +		goto err_switch_setup;
> > +	}
> > +
> > +	/* We allocate one VSI which is needed as absolute minimum
> > +	 * in order to register the netdev
> > +	 */
> > +	v_idx = i40e_vsi_mem_alloc(pf, I40E_VSI_MAIN);
> > +	if (v_idx < 0)
> > +		goto err_switch_setup;
> > +	pf->lan_vsi = v_idx;
> > +	vsi = pf->vsi[v_idx];
> > +	if (!vsi)
> > +		goto err_switch_setup;
> > +	vsi->alloc_queue_pairs = 1;
> > +	err = i40e_config_netdev(vsi);
> > +	if (err)
> > +		goto err_switch_setup;
> > +	err = register_netdev(vsi->netdev);
> > +	if (err)
> > +		goto err_switch_setup;
> > +	vsi->netdev_registered = true;
> > +	i40e_dbg_pf_init(pf);
> > +
> > +	err = i40e_setup_misc_vector_for_recovery_mode(pf);
> > +	if (err)
> > +		goto err_switch_setup;
> > +
> > +	/* tell the firmware that we're starting */
> > +	i40e_send_version(pf);
> > +
> > +	/* since everything's happy, start the service_task timer */
> > +	mod_timer(&pf->service_timer,
> > +		  round_jiffies(jiffies + pf->service_timer_period));
> > +
> > +	return 0;
> > +
> > +err_switch_setup:
> > +	i40e_reset_interrupt_capability(pf);
> > +	del_timer_sync(&pf->service_timer);
> > +#ifdef NOT_FOR_UPSTREAM
> 
> Delightful :)
> 
> > +	dev_warn(&pf->pdev->dev, "previous errors forcing module to load in
> > debug mode\n");
> > +	i40e_dbg_pf_init(pf);
> > +	set_bit(__I40E_DEBUG_MODE, pf->state);
> > +	return 0;
> > +#else
> > +	i40e_shutdown_adminq(hw);
> > +	iounmap(hw->hw_addr);
> > +	pci_disable_pcie_error_reporting(pf->pdev);
> > +	pci_release_mem_regions(pf->pdev);
> > +	pci_disable_device(pf->pdev);
> > +	kfree(pf);
> > +
> > +	return err;
> > +#endif
> > +}
> > +
> >  /**
> >   * i40e_probe - Device initialization routine
> >   * @pdev: PCI device information struct


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

Re: [net-next 01/12] i40e: replace switch-statement to speed-up retpoline-enabled builds

[Daniel Borkmann]

On 04/29/2019 09:16 PM, Jeff Kirsher wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
> 
> GCC will generate jump tables for switch-statements with more than 5
> case statements. An entry into the jump table is an indirect call,
> which means that for CONFIG_RETPOLINE builds, this is rather
> expensive.
> 
> This commit replaces the switch-statement that acts on the XDP program
> result with an if-clause.
> 
> The if-clause was also refactored into a common function that can be
> used by AF_XDP zero-copy and non-zero-copy code.

Isn't it fixed upstream by now already (also in gcc)?

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ce02ef06fcf7a399a6276adb83f37373d10cbbe1
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a9d57ef15cbe327fe54416dd194ee0ea66ae53a4

Re: [net-next 01/12] i40e: replace switch-statement to speed-up retpoline-enabled builds

[Björn Töpel]

On 2019-05-02 16:47, Daniel Borkmann wrote:
> On 04/29/2019 09:16 PM, Jeff Kirsher wrote:
>> From: Björn Töpel <bjorn.topel@intel.com>
>>
>> GCC will generate jump tables for switch-statements with more than 5
>> case statements. An entry into the jump table is an indirect call,
>> which means that for CONFIG_RETPOLINE builds, this is rather
>> expensive.
>>
>> This commit replaces the switch-statement that acts on the XDP program
>> result with an if-clause.
>>
>> The if-clause was also refactored into a common function that can be
>> used by AF_XDP zero-copy and non-zero-copy code.
> 
> Isn't it fixed upstream by now already (also in gcc)?
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ce02ef06fcf7a399a6276adb83f37373d10cbbe1
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a9d57ef15cbe327fe54416dd194ee0ea66ae53a4
> 

Hmm, given that Daniel's work is upstream, this patch doesn't really
make sense any more. OTOH it can stay in the series, and be cleaned up
later.

I'll leave it for you to decide, Jeff!


Cheers,
Björn

Re: [net-next 01/12] i40e: replace switch-statement to speed-up retpoline-enabled builds

[Jeff Kirsher]

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

On Thu, 2019-05-02 at 22:29 +0200, Björn Töpel wrote:
> On 2019-05-02 16:47, Daniel Borkmann wrote:
> > On 04/29/2019 09:16 PM, Jeff Kirsher wrote:
> > > From: Björn Töpel <bjorn.topel@intel.com>
> > > 
> > > GCC will generate jump tables for switch-statements with more than 5
> > > case statements. An entry into the jump table is an indirect call,
> > > which means that for CONFIG_RETPOLINE builds, this is rather
> > > expensive.
> > > 
> > > This commit replaces the switch-statement that acts on the XDP
> > > program
> > > result with an if-clause.
> > > 
> > > The if-clause was also refactored into a common function that can be
> > > used by AF_XDP zero-copy and non-zero-copy code.
> > 
> > Isn't it fixed upstream by now already (also in gcc)?
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ce02ef06fcf7a399a6276adb83f37373d10cbbe1
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a9d57ef15cbe327fe54416dd194ee0ea66ae53a4
> > 
> 
> Hmm, given that Daniel's work is upstream, this patch doesn't really
> make sense any more. OTOH it can stay in the series, and be cleaned up
> later.
> 
> I'll leave it for you to decide, Jeff!

I am already making revisions to the series due to another patch, so if
these changes are no longer needed to improve performance in RETPOLINE
builds, then lets drop it.

Björn, can you confirm that with or without these changes, XDP performance
stays the same for RETPOLINE builds?

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

Re: [net-next 01/12] i40e: replace switch-statement to speed-up retpoline-enabled builds

[Björn Töpel]

On 2019-05-02 22:40, Jeff Kirsher wrote:
> On Thu, 2019-05-02 at 22:29 +0200, Björn Töpel wrote:
>> On 2019-05-02 16:47, Daniel Borkmann wrote:
>>> On 04/29/2019 09:16 PM, Jeff Kirsher wrote:
>>>> From: Björn Töpel <bjorn.topel@intel.com>
>>>>
>>>> GCC will generate jump tables for switch-statements with more than 5
>>>> case statements. An entry into the jump table is an indirect call,
>>>> which means that for CONFIG_RETPOLINE builds, this is rather
>>>> expensive.
>>>>
>>>> This commit replaces the switch-statement that acts on the XDP
>>>> program
>>>> result with an if-clause.
>>>>
>>>> The if-clause was also refactored into a common function that can be
>>>> used by AF_XDP zero-copy and non-zero-copy code.
>>>
>>> Isn't it fixed upstream by now already (also in gcc)?
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ce02ef06fcf7a399a6276adb83f37373d10cbbe1
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a9d57ef15cbe327fe54416dd194ee0ea66ae53a4
>>>
>>
>> Hmm, given that Daniel's work is upstream, this patch doesn't really
>> make sense any more. OTOH it can stay in the series, and be cleaned up
>> later.
>>
>> I'll leave it for you to decide, Jeff!
> 
> I am already making revisions to the series due to another patch, so if
> these changes are no longer needed to improve performance in RETPOLINE
> builds, then lets drop it.
> 
> Björn, can you confirm that with or without these changes, XDP performance
> stays the same for RETPOLINE builds?
> 

Confirmed (on i40e using xdp1 and xdpsock samples); Same performance
with/without this patch.

IOW, please drop this from your next spin.


Thanks,
Björn

Re: [net-next 01/12] i40e: replace switch-statement to speed-up retpoline-enabled builds

[Jeff Kirsher]

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

On Thu, 2019-05-02 at 22:56 +0200, Björn Töpel wrote:
> On 2019-05-02 22:40, Jeff Kirsher wrote:
> > On Thu, 2019-05-02 at 22:29 +0200, Björn Töpel wrote:
> > > On 2019-05-02 16:47, Daniel Borkmann wrote:
> > > > On 04/29/2019 09:16 PM, Jeff Kirsher wrote:
> > > > > From: Björn Töpel <bjorn.topel@intel.com>
> > > > > 
> > > > > GCC will generate jump tables for switch-statements with more
> > > > > than 5
> > > > > case statements. An entry into the jump table is an indirect
> > > > > call,
> > > > > which means that for CONFIG_RETPOLINE builds, this is rather
> > > > > expensive.
> > > > > 
> > > > > This commit replaces the switch-statement that acts on the XDP
> > > > > program
> > > > > result with an if-clause.
> > > > > 
> > > > > The if-clause was also refactored into a common function that can
> > > > > be
> > > > > used by AF_XDP zero-copy and non-zero-copy code.
> > > > 
> > > > Isn't it fixed upstream by now already (also in gcc)?
> > > > 
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ce02ef06fcf7a399a6276adb83f37373d10cbbe1
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a9d57ef15cbe327fe54416dd194ee0ea66ae53a4
> > > > 
> > > 
> > > Hmm, given that Daniel's work is upstream, this patch doesn't really
> > > make sense any more. OTOH it can stay in the series, and be cleaned
> > > up
> > > later.
> > > 
> > > I'll leave it for you to decide, Jeff!
> > 
> > I am already making revisions to the series due to another patch, so if
> > these changes are no longer needed to improve performance in RETPOLINE
> > builds, then lets drop it.
> > 
> > Björn, can you confirm that with or without these changes, XDP
> > performance
> > stays the same for RETPOLINE builds?
> > 
> 
> Confirmed (on i40e using xdp1 and xdpsock samples); Same performance
> with/without this patch.
> 
> IOW, please drop this from your next spin.

Ok, dropped.

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

Re: [net-next 01/12] i40e: replace switch-statement to speed-up retpoline-enabled builds

[Daniel Borkmann]

On 04/30/2019 12:42 PM, David Laight wrote:
> From: Josh Elsasser
>> Sent: 29 April 2019 21:02
>> On Apr 29, 2019, at 12:16 PM, Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
>>
>>> From: Björn Töpel <bjorn.topel@intel.com>
>>>
>>> GCC will generate jump tables for switch-statements with more than 5
>>> case statements. An entry into the jump table is an indirect call,
>>> which means that for CONFIG_RETPOLINE builds, this is rather
>>> expensive.
>>>
>>> This commit replaces the switch-statement that acts on the XDP program
>>> result with an if-clause.
>>
>> Apologies for the noise, but is this patch still required after the
>> recent threshold bump[0] and later removal[1] of switch-case jump
>> table generation when building with CONFIG_RETPOLINE?
>>
>> [0]: https://lore.kernel.org/patchwork/patch/1044863/
>> [1]: https://lore.kernel.org/patchwork/patch/1054472/
>>
>> If nothing else the commit message no longer seems accurate.
> 
> Looking at those two patches, the second one seems wrong:
> 
>    # Additionally, avoid generating expensive indirect jumps which
>    # are subject to retpolines for small number of switch cases.
>    # clang turns off jump table generation by default when under
> -  # retpoline builds, however, gcc does not for x86.
> -  KBUILD_CFLAGS += $(call cc-option,--param=case-values-threshold=20)
> +  # retpoline builds, however, gcc does not for x86. This has
> +  # only been fixed starting from gcc stable version 8.4.0 and
> +  # onwards, but not for older ones. See gcc bug #86952.
> +  ifndef CONFIG_CC_IS_CLANG
> +    KBUILD_CFLAGS += $(call cc-option,-fno-jump-tables)
> +  endif
> 
> If -fno-jump-tables isn't supported then --param=case-values-threshold=20
> needs to be set (if supported).

Nope, not really, -fno-jump-tables support predates the latter, and
both are supported for gcc versions the kernel cares about.