[Intel-wired-lan] [PATCH S39 01/15] ice: Validate config for SW DCB map

[Intel-wired-lan] [PATCH S39 01/15] ice: Validate config for SW DCB map

[Tony Nguyen]

From: Avinash Dayanand <avinash.dayanand@intel.com>

Validate the inputs for SW DCB config received either via lldptool or pcap
file. And don't apply DCB for bad bandwidth inputs or non-contiguous TCs.
Without this patch, any config having bad inputs will cause the loss of
link making PF unusable even after driver reload. Recoverable only via
system reboot.

Signed-off-by: Avinash Dayanand <avinash.dayanand@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 55 ++++++++++++++++++++
 drivers/net/ethernet/intel/ice/ice_dcb_lib.h |  1 +
 drivers/net/ethernet/intel/ice/ice_dcb_nl.c  |  7 +++
 3 files changed, 63 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
index 0f4ca813a7ab..bd361212921c 100644
--- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
@@ -169,6 +169,56 @@ ice_peer_prep_tc_change(struct ice_peer_dev_int *peer_dev_int,
 	return 0;
 }
 
+/**
+ * ice_dcb_bwchk - check if ETS bandwidth input parameters are correct
+ * @dcbcfg: pointer to DCB config structure
+ */
+int ice_dcb_bwchk(struct ice_dcbx_cfg *dcbcfg)
+{
+	struct ice_dcb_ets_cfg *etscfg = &dcbcfg->etscfg;
+	u8 num_tc, total_bw = 0;
+	int i;
+
+	/* returns number of contigous TCs and 1 TC for non-contigous TCs,
+	 * since at least 1 TC has to be configured
+	 */
+	num_tc = ice_dcb_get_num_tc(dcbcfg);
+
+	/* no bandwidth checks required if there's only one TC and assign
+	 * all bandwidth to it i.e. to TC0 and return
+	 */
+	if (num_tc == 1) {
+		etscfg->tcbwtable[0] = ICE_TC_MAX_BW;
+		return 0;
+	}
+	/* There are few rules with which TC bandwidth can be applied for any TC
+	 * with a UP mapped to it.
+	 *	1. All TCs have zero BW - Valid
+	 *	   ex:  tcbw=0,0,0
+	 *	2. First few non-zero and rest zero BW - Valid
+	 *	   ex:  tcbw=100,0,0
+	 *	3. Zero BW in between 2 non-zero BW TCs - Invalid
+	 *	   ex:  tcbw=25,0,75
+	 */
+	for (i = 0; i < num_tc; i++) {
+		/* don't allow zero BW for TCs other than TC0 */
+		if (i && !etscfg->tcbwtable[i])
+			goto err;
+
+		if (etscfg->tsatable[i] == ICE_IEEE_TSA_ETS)
+			total_bw += etscfg->tcbwtable[i];
+	}
+
+	 /* total bandwidth should be equal to 100 */
+	if (total_bw != ICE_TC_MAX_BW)
+		goto err;
+
+	return 0;
+
+err:
+	return -EINVAL;
+}
+
 /**
  * ice_pf_dcb_cfg - Apply new DCB configuration
  * @pf: pointer to the PF struct
@@ -206,6 +256,11 @@ int ice_pf_dcb_cfg(struct ice_pf *pf, struct ice_dcbx_cfg *new_cfg, bool locked)
 	/* Notify capable peers about impending change to TCs */
 	ice_for_each_peer(pf, NULL, ice_peer_prep_tc_change);
 
+	if (ice_dcb_bwchk(new_cfg)) {
+		dev_err(dev, "Invalid config, not applying DCB\n");
+		return -EINVAL;
+	}
+
 	/* Store old config in case FW config fails */
 	old_cfg = kmemdup(curr_cfg, sizeof(*old_cfg), GFP_KERNEL);
 	if (!old_cfg)
diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.h b/drivers/net/ethernet/intel/ice/ice_dcb_lib.h
index bb53edf462ba..2b900da27f57 100644
--- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.h
+++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.h
@@ -20,6 +20,7 @@ u8 ice_dcb_get_num_tc(struct ice_dcbx_cfg *dcbcfg);
 u8 ice_dcb_get_tc(struct ice_vsi *vsi, int queue_index);
 int
 ice_pf_dcb_cfg(struct ice_pf *pf, struct ice_dcbx_cfg *new_cfg, bool locked);
+int ice_dcb_bwchk(struct ice_dcbx_cfg *dcbcfg);
 void ice_pf_dcb_recfg(struct ice_pf *pf);
 void ice_vsi_cfg_dcb_rings(struct ice_vsi *vsi);
 int ice_init_pf_dcb(struct ice_pf *pf, bool locked);
diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_nl.c b/drivers/net/ethernet/intel/ice/ice_dcb_nl.c
index b61aba428adb..a45e8abef8f3 100644
--- a/drivers/net/ethernet/intel/ice/ice_dcb_nl.c
+++ b/drivers/net/ethernet/intel/ice/ice_dcb_nl.c
@@ -95,6 +95,12 @@ static int ice_dcbnl_setets(struct net_device *netdev, struct ieee_ets *ets)
 		new_cfg->etsrec.prio_table[i] = ets->reco_prio_tc[i];
 	}
 
+	if (ice_dcb_bwchk(new_cfg)) {
+		netdev_err(netdev, "Invalid config, not applying DCB\n");
+		err = -EINVAL;
+		goto ets_out;
+	}
+
 	/* max_tc is a 1-8 value count of number of TC's, not a 0-7 value
 	 * for the TC's index number.  Add one to value if not zero, and
 	 * for zero set it to the FW's default value
@@ -119,6 +125,7 @@ static int ice_dcbnl_setets(struct net_device *netdev, struct ieee_ets *ets)
 	if (err == ICE_DCB_NO_HW_CHG)
 		err = ICE_DCB_HW_CHG_RST;
 
+ets_out:
 	mutex_unlock(&pf->tc_mutex);
 	return err;
 }
-- 
2.20.1

[Intel-wired-lan] [PATCH S39 02/15] ice: update malicious driver detection event handling

[Tony Nguyen]

From: Paul Greenwalt <paul.greenwalt@intel.com>

Update the PF VFs MDD event message to rate limit once per second and
report the total number Rx|Tx event count. Add support to print pending
MDD events that occur during the rate limit. The use of net_ratelimit did
not allow for per VF Rx|Tx granularity.

Additional PF MDD log messages are guarded by netif_msg_[rx|tx]_err().

Since VF RX MDD events disable the queue, add ethtool private flag
mdd-auto-reset-vf to configure VF reset to re-enable the queue.

Disable anti-spoof detection interrupt to prevent spurious events
during a function reset.

To avoid race condition do not make PF MDD register reads conditional
on global MDD result.

Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/ice/ice.h          |   4 +
 drivers/net/ethernet/intel/ice/ice_ethtool.c  |   1 +
 .../net/ethernet/intel/ice/ice_hw_autogen.h   |   2 +
 drivers/net/ethernet/intel/ice/ice_main.c     | 126 ++++++++++--------
 .../net/ethernet/intel/ice/ice_virtchnl_pf.c  |  54 +++++++-
 .../net/ethernet/intel/ice/ice_virtchnl_pf.h  |  20 ++-
 6 files changed, 148 insertions(+), 59 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index 8b33fe62198f..92e44a87f905 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -216,6 +216,7 @@ enum ice_state {
 	__ICE_SERVICE_SCHED,
 	__ICE_SERVICE_DIS,
 	__ICE_OICR_INTR_DIS,		/* Global OICR interrupt disabled */
+	__ICE_MDD_VF_PRINT_PENDING,	/* set when MDD event handle */
 	__ICE_STATE_NBITS		/* must be last */
 };
 
@@ -347,6 +348,7 @@ enum ice_pf_flags {
 	ICE_FLAG_FW_LLDP_AGENT,
 	ICE_FLAG_ETHTOOL_CTXT,		/* set when ethtool holds RTNL lock */
 	ICE_FLAG_LEGACY_RX,
+	ICE_FLAG_MDD_AUTO_RESET_VF,
 	ICE_PF_FLAGS_NBITS		/* must be last */
 };
 
@@ -370,6 +372,8 @@ struct ice_pf {
 	u16 num_vfs_supported;		/* num VFs supported for this PF */
 	u16 num_vf_qps;			/* num queue pairs per VF */
 	u16 num_vf_msix;		/* num vectors per VF */
+	/* used to ratelimit the MDD event logging */
+	unsigned long last_printed_mdd_jiffies;
 	DECLARE_BITMAP(state, __ICE_STATE_NBITS);
 	DECLARE_BITMAP(flags, ICE_PF_FLAGS_NBITS);
 	unsigned long *avail_txqs;	/* bitmap to track PF Tx queue usage */
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index 7b34c7c04547..db14ec2e0b46 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -157,6 +157,7 @@ struct ice_priv_flag {
 static const struct ice_priv_flag ice_gstrings_priv_flags[] = {
 	ICE_PRIV_FLAG("link-down-on-close", ICE_FLAG_LINK_DOWN_ON_CLOSE_ENA),
 	ICE_PRIV_FLAG("fw-lldp-agent", ICE_FLAG_FW_LLDP_AGENT),
+	ICE_PRIV_FLAG("mdd-auto-reset-vf", ICE_FLAG_MDD_AUTO_RESET_VF),
 	ICE_PRIV_FLAG("legacy-rx", ICE_FLAG_LEGACY_RX),
 };
 
diff --git a/drivers/net/ethernet/intel/ice/ice_hw_autogen.h b/drivers/net/ethernet/intel/ice/ice_hw_autogen.h
index d767c398ac71..57f1c4513c0b 100644
--- a/drivers/net/ethernet/intel/ice/ice_hw_autogen.h
+++ b/drivers/net/ethernet/intel/ice/ice_hw_autogen.h
@@ -218,6 +218,8 @@
 #define VPLAN_TX_QBASE_VFNUMQ_M			ICE_M(0xFF, 16)
 #define VPLAN_TXQ_MAPENA(_VF)			(0x00073800 + ((_VF) * 4))
 #define VPLAN_TXQ_MAPENA_TX_ENA_M		BIT(0)
+#define GL_MDCK_TX_TDPU				0x00049348
+#define GL_MDCK_TX_TDPU_RCU_ANTISPOOF_ITR_DIS_M BIT(1)
 #define GL_MDET_RX				0x00294C00
 #define GL_MDET_RX_QNUM_S			0
 #define GL_MDET_RX_QNUM_M			ICE_M(0x7FFF, 0)
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 3857e3f7a3f2..bbc428c4ec07 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -1190,20 +1190,28 @@ static void ice_service_timer(struct timer_list *t)
  * ice_handle_mdd_event - handle malicious driver detect event
  * @pf: pointer to the PF structure
  *
- * Called from service task. OICR interrupt handler indicates MDD event
+ * Called from service task. OICR interrupt handler indicates MDD event.
+ * VF MDD logging is guarded by net_ratelimit. Additional PF and VF log
+ * messages are wrapped by netif_msg_[rx|tx]_err. Since VF Rx MDD events
+ * disable the queue, the PF can be configured to reset the VF using ethtool
+ * private flag mdd-auto-reset-vf.
  */
 static void ice_handle_mdd_event(struct ice_pf *pf)
 {
 	struct device *dev = ice_pf_to_dev(pf);
 	struct ice_hw *hw = &pf->hw;
-	bool mdd_detected = false;
 	u32 reg;
 	int i;
 
-	if (!test_and_clear_bit(__ICE_MDD_EVENT_PENDING, pf->state))
+	if (!test_and_clear_bit(__ICE_MDD_EVENT_PENDING, pf->state)) {
+		/* Since the VF MDD event logging is rate limited, check if
+		 * there are pending MDD events.
+		 */
+		ice_print_vfs_mdd_events(pf);
 		return;
+	}
 
-	/* find what triggered the MDD event */
+	/* find what triggered an MDD event */
 	reg = rd32(hw, GL_MDET_TX_PQM);
 	if (reg & GL_MDET_TX_PQM_VALID_M) {
 		u8 pf_num = (reg & GL_MDET_TX_PQM_PF_NUM_M) >>
@@ -1219,7 +1227,6 @@ static void ice_handle_mdd_event(struct ice_pf *pf)
 			dev_info(dev, "Malicious Driver Detection event %d on TX queue %d PF# %d VF# %d\n",
 				 event, queue, pf_num, vf_num);
 		wr32(hw, GL_MDET_TX_PQM, 0xffffffff);
-		mdd_detected = true;
 	}
 
 	reg = rd32(hw, GL_MDET_TX_TCLAN);
@@ -1237,7 +1244,6 @@ static void ice_handle_mdd_event(struct ice_pf *pf)
 			dev_info(dev, "Malicious Driver Detection event %d on TX queue %d PF# %d VF# %d\n",
 				 event, queue, pf_num, vf_num);
 		wr32(hw, GL_MDET_TX_TCLAN, 0xffffffff);
-		mdd_detected = true;
 	}
 
 	reg = rd32(hw, GL_MDET_RX);
@@ -1255,85 +1261,85 @@ static void ice_handle_mdd_event(struct ice_pf *pf)
 			dev_info(dev, "Malicious Driver Detection event %d on RX queue %d PF# %d VF# %d\n",
 				 event, queue, pf_num, vf_num);
 		wr32(hw, GL_MDET_RX, 0xffffffff);
-		mdd_detected = true;
 	}
 
-	if (mdd_detected) {
-		bool pf_mdd_detected = false;
-
-		reg = rd32(hw, PF_MDET_TX_PQM);
-		if (reg & PF_MDET_TX_PQM_VALID_M) {
-			wr32(hw, PF_MDET_TX_PQM, 0xFFFF);
-			dev_info(dev, "TX driver issue detected, PF reset issued\n");
-			pf_mdd_detected = true;
-		}
+	/* check to see if this PF caused an MDD event */
+	reg = rd32(hw, PF_MDET_TX_PQM);
+	if (reg & PF_MDET_TX_PQM_VALID_M) {
+		wr32(hw, PF_MDET_TX_PQM, 0xFFFF);
+		if (netif_msg_tx_err(pf))
+			dev_info(dev, "Malicious Driver Detection event TX_PQM detected on PF\n");
+	}
 
-		reg = rd32(hw, PF_MDET_TX_TCLAN);
-		if (reg & PF_MDET_TX_TCLAN_VALID_M) {
-			wr32(hw, PF_MDET_TX_TCLAN, 0xFFFF);
-			dev_info(dev, "TX driver issue detected, PF reset issued\n");
-			pf_mdd_detected = true;
-		}
+	reg = rd32(hw, PF_MDET_TX_TCLAN);
+	if (reg & PF_MDET_TX_TCLAN_VALID_M) {
+		wr32(hw, PF_MDET_TX_TCLAN, 0xFFFF);
+		if (netif_msg_tx_err(pf))
+			dev_info(dev, "Malicious Driver Detection event TX_TCLAN detected on PF\n");
+	}
 
-		reg = rd32(hw, PF_MDET_RX);
-		if (reg & PF_MDET_RX_VALID_M) {
-			wr32(hw, PF_MDET_RX, 0xFFFF);
-			dev_info(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(__ICE_NEEDS_RESTART, pf->state);
-			ice_service_task_schedule(pf);
-		}
+	reg = rd32(hw, PF_MDET_RX);
+	if (reg & PF_MDET_RX_VALID_M) {
+		wr32(hw, PF_MDET_RX, 0xFFFF);
+		if (netif_msg_rx_err(pf))
+			dev_info(dev, "Malicious Driver Detection event RX detected on PF\n");
 	}
 
-	/* check to see if one of the VFs caused the MDD */
+	/* Check to see if one of the VFs caused an MDD event, and then
+	 * increment counters and set print pending
+	 */
 	ice_for_each_vf(pf, i) {
 		struct ice_vf *vf = &pf->vf[i];
 
-		bool vf_mdd_detected = false;
-
 		reg = rd32(hw, VP_MDET_TX_PQM(i));
 		if (reg & VP_MDET_TX_PQM_VALID_M) {
 			wr32(hw, VP_MDET_TX_PQM(i), 0xFFFF);
-			vf_mdd_detected = true;
-			dev_info(dev, "TX driver issue detected on VF %d\n",
-				 i);
+			vf->mdd_tx_events.count++;
+			set_bit(__ICE_MDD_VF_PRINT_PENDING, pf->state);
+			if (netif_msg_tx_err(pf))
+				dev_info(dev, "Malicious Driver Detection event TX_PQM detected on VF %d\n",
+					 i);
 		}
 
 		reg = rd32(hw, VP_MDET_TX_TCLAN(i));
 		if (reg & VP_MDET_TX_TCLAN_VALID_M) {
 			wr32(hw, VP_MDET_TX_TCLAN(i), 0xFFFF);
-			vf_mdd_detected = true;
-			dev_info(dev, "TX driver issue detected on VF %d\n",
-				 i);
+			vf->mdd_tx_events.count++;
+			set_bit(__ICE_MDD_VF_PRINT_PENDING, pf->state);
+			if (netif_msg_tx_err(pf))
+				dev_info(dev, "Malicious Driver Detection event TX_TCLAN detected on VF %d\n",
+					 i);
 		}
 
 		reg = rd32(hw, VP_MDET_TX_TDPU(i));
 		if (reg & VP_MDET_TX_TDPU_VALID_M) {
 			wr32(hw, VP_MDET_TX_TDPU(i), 0xFFFF);
-			vf_mdd_detected = true;
-			dev_info(dev, "TX driver issue detected on VF %d\n",
-				 i);
+			vf->mdd_tx_events.count++;
+			set_bit(__ICE_MDD_VF_PRINT_PENDING, pf->state);
+			if (netif_msg_tx_err(pf))
+				dev_info(dev, "Malicious Driver Detection event TX_TDPU detected on VF %d\n",
+					 i);
 		}
 
 		reg = rd32(hw, VP_MDET_RX(i));
 		if (reg & VP_MDET_RX_VALID_M) {
 			wr32(hw, VP_MDET_RX(i), 0xFFFF);
-			vf_mdd_detected = true;
-			dev_info(dev, "RX driver issue detected on VF %d\n",
-				 i);
-		}
-
-		if (vf_mdd_detected) {
-			vf->num_mdd_events++;
-			if (vf->num_mdd_events &&
-			    vf->num_mdd_events <= ICE_MDD_EVENTS_THRESHOLD)
-				dev_info(dev, "VF %d has had %llu MDD events since last boot, Admin might need to reload AVF driver with this number of events\n",
-					 i, vf->num_mdd_events);
+			vf->mdd_rx_events.count++;
+			set_bit(__ICE_MDD_VF_PRINT_PENDING, pf->state);
+			if (netif_msg_rx_err(pf))
+				dev_info(dev, "Malicious Driver Detection event RX detected on VF %d\n",
+					 i);
+
+			/* Since the queue is disabled on VF Rx MDD events, the
+			 * PF can be configured to reset the VF through ethtool
+			 * private flag mdd-auto-reset-vf.
+			 */
+			if (test_bit(ICE_FLAG_MDD_AUTO_RESET_VF, pf->flags))
+				ice_reset_vf(&pf->vf[i], false);
 		}
 	}
+
+	ice_print_vfs_mdd_events(pf);
 }
 
 /**
@@ -2001,6 +2007,14 @@ static void ice_ena_misc_vector(struct ice_pf *pf)
 	struct ice_hw *hw = &pf->hw;
 	u32 val;
 
+	/* Disable anti-spoof detection interrupt to prevent spurious event
+	 * interrupts during a function reset. Anti-spoof functionally is
+	 * still supported.
+	 */
+	val = rd32(hw, GL_MDCK_TX_TDPU);
+	val |= GL_MDCK_TX_TDPU_RCU_ANTISPOOF_ITR_DIS_M;
+	wr32(hw, GL_MDCK_TX_TDPU, val);
+
 	/* clear things first */
 	wr32(hw, PFINT_OICR_ENA, 0);	/* disable all */
 	rd32(hw, PFINT_OICR);		/* read to clear */
diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c b/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c
index 7d778b3d5d5a..c84ac00a316e 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c
@@ -171,6 +171,11 @@ static void ice_free_vf_res(struct ice_vf *vf)
 	}
 
 	last_vector_idx = vf->first_vector_idx + pf->num_vf_msix - 1;
+
+	/* clear VF MDD event information */
+	memset(&vf->mdd_tx_events, 0, sizeof(vf->mdd_tx_events));
+	memset(&vf->mdd_rx_events, 0, sizeof(vf->mdd_rx_events));
+
 	/* Disable interrupts so that VF starts in a known state */
 	for (i = vf->first_vector_idx; i <= last_vector_idx; i++) {
 		wr32(&pf->hw, GLINT_DYN_CTL(i), GLINT_DYN_CTL_CLEARPBA_M);
@@ -1175,7 +1180,7 @@ static bool ice_is_vf_disabled(struct ice_vf *vf)
  *
  * Returns true if the VF is reset, false otherwise.
  */
-static bool ice_reset_vf(struct ice_vf *vf, bool is_vflr)
+bool ice_reset_vf(struct ice_vf *vf, bool is_vflr)
 {
 	struct ice_pf *pf = vf->pf;
 	struct ice_vsi *vsi;
@@ -3538,3 +3543,50 @@ int ice_get_vf_stats(struct net_device *netdev, int vf_id,
 
 	return 0;
 }
+
+/**
+ * ice_print_vfs_mdd_event - print VFs malicious driver detect event
+ * @pf: pointer to the PF structure
+ *
+ * Called from ice_handle_mdd_event to rate limit and print VFs MDD events.
+ */
+void ice_print_vfs_mdd_events(struct ice_pf *pf)
+{
+	struct device *dev = ice_pf_to_dev(pf);
+	struct ice_hw *hw = &pf->hw;
+	int i;
+
+	/* check that there are pending MDD events to print */
+	if (!test_and_clear_bit(__ICE_MDD_VF_PRINT_PENDING, pf->state))
+		return;
+
+	/* VF MDD event logs are rate limited to one second intervals */
+	if (time_is_after_jiffies(pf->last_printed_mdd_jiffies + HZ * 1))
+		return;
+
+	pf->last_printed_mdd_jiffies = jiffies;
+
+	ice_for_each_vf(pf, i) {
+		struct ice_vf *vf = &pf->vf[i];
+
+		/* only print Rx MDD event message if there are new events */
+		if (vf->mdd_rx_events.count != vf->mdd_rx_events.last_printed) {
+			vf->mdd_rx_events.last_printed =
+							vf->mdd_rx_events.count;
+
+			dev_info(dev, "%d Rx Malicious Driver Detection events detected on PF %d VF %d. mdd-auto-reset-vfs=%s\n",
+				 vf->mdd_rx_events.count, hw->pf_id, i,
+				 test_bit(ICE_FLAG_MDD_AUTO_RESET_VF, pf->flags)
+					  ? "on" : "off");
+		}
+
+		/* only print Tx MDD event message if there are new events */
+		if (vf->mdd_tx_events.count != vf->mdd_tx_events.last_printed) {
+			vf->mdd_tx_events.last_printed =
+							vf->mdd_tx_events.count;
+
+			dev_info(dev, "%d Tx Malicious Driver Detection events detected on PF %d VF %d.\n",
+				 vf->mdd_tx_events.count, hw->pf_id, i);
+		}
+	}
+}
diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.h b/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.h
index 474b2613f09c..656f1909b38f 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.h
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.h
@@ -55,6 +55,13 @@ enum ice_virtchnl_cap {
 	ICE_VIRTCHNL_VF_CAP_PRIVILEGE,
 };
 
+/* VF MDD events print structure */
+struct ice_mdd_vf_events {
+	u16 count;			/* total count of Rx|Tx events */
+	/* count number of the last printed event */
+	u16 last_printed;
+};
+
 /* VF information structure */
 struct ice_vf {
 	struct ice_pf *pf;
@@ -83,13 +90,14 @@ struct ice_vf {
 	unsigned int tx_rate;		/* Tx bandwidth limit in Mbps */
 	DECLARE_BITMAP(vf_states, ICE_VF_STATES_NBITS);	/* VF runtime states */
 
-	u64 num_mdd_events;		/* number of MDD events detected */
 	u64 num_inval_msgs;		/* number of continuous invalid msgs */
 	u64 num_valid_msgs;		/* number of valid msgs detected */
 	unsigned long vf_caps;		/* VF's adv. capabilities */
 	u8 num_req_qs;			/* num of queue pairs requested by VF */
 	u16 num_mac;
 	u16 num_vf_qs;			/* num of queue configured per VF */
+	struct ice_mdd_vf_events mdd_rx_events;
+	struct ice_mdd_vf_events mdd_tx_events;
 };
 
 #ifdef CONFIG_PCI_IOV
@@ -104,6 +112,7 @@ void ice_vc_process_vf_msg(struct ice_pf *pf, struct ice_rq_event_info *event);
 void ice_vc_notify_link_state(struct ice_pf *pf);
 void ice_vc_notify_reset(struct ice_pf *pf);
 bool ice_reset_all_vfs(struct ice_pf *pf, bool is_vflr);
+bool ice_reset_vf(struct ice_vf *vf, bool is_vflr);
 
 int
 ice_set_vf_port_vlan(struct net_device *netdev, int vf_id, u16 vlan_id, u8 qos,
@@ -123,7 +132,7 @@ ice_get_vf_stats(struct net_device *netdev, int vf_id,
 		 struct ifla_vf_stats *vf_stats);
 void
 ice_vf_lan_overflow_event(struct ice_pf *pf, struct ice_rq_event_info *event);
-
+void ice_print_vfs_mdd_events(struct ice_pf *pf);
 #else /* CONFIG_PCI_IOV */
 #define ice_process_vflr_event(pf) do {} while (0)
 #define ice_free_vfs(pf) do {} while (0)
@@ -132,6 +141,7 @@ ice_vf_lan_overflow_event(struct ice_pf *pf, struct ice_rq_event_info *event);
 #define ice_vc_notify_reset(pf) do {} while (0)
 #define ice_set_vf_state_qs_dis(vf) do {} while (0)
 #define ice_vf_lan_overflow_event(pf, event) do {} while (0)
+#define ice_print_vfs_mdd_events(pf) do {} while (0)
 
 static inline bool
 ice_reset_all_vfs(struct ice_pf __always_unused *pf,
@@ -140,6 +150,12 @@ ice_reset_all_vfs(struct ice_pf __always_unused *pf,
 	return true;
 }
 
+static inline bool
+ice_reset_vf(struct ice_vf __always_unused *vf, bool __always_unused is_vflr)
+{
+	return true;
+}
+
 static inline int
 ice_sriov_configure(struct pci_dev __always_unused *pdev,
 		    int __always_unused num_vfs)
-- 
2.20.1

[Intel-wired-lan] [PATCH S39 02/15] ice: update malicious driver detection event handling

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Tony Nguyen
> Sent: Monday, January 27, 2020 12:59 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH S39 02/15] ice: update malicious driver
> detection event handling
> 
> From: Paul Greenwalt <paul.greenwalt@intel.com>
> 
> Update the PF VFs MDD event message to rate limit once per second and
> report the total number Rx|Tx event count. Add support to print pending
> MDD events that occur during the rate limit. The use of net_ratelimit did not
> allow for per VF Rx|Tx granularity.
> 
> Additional PF MDD log messages are guarded by netif_msg_[rx|tx]_err().
> 
> Since VF RX MDD events disable the queue, add ethtool private flag mdd-
> auto-reset-vf to configure VF reset to re-enable the queue.
> 
> Disable anti-spoof detection interrupt to prevent spurious events during a
> function reset.
> 
> To avoid race condition do not make PF MDD register reads conditional on
> global MDD result.
> 
> Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice.h          |   4 +
>  drivers/net/ethernet/intel/ice/ice_ethtool.c  |   1 +
>  .../net/ethernet/intel/ice/ice_hw_autogen.h   |   2 +
>  drivers/net/ethernet/intel/ice/ice_main.c     | 126 ++++++++++--------
>  .../net/ethernet/intel/ice/ice_virtchnl_pf.c  |  54 +++++++-
> .../net/ethernet/intel/ice/ice_virtchnl_pf.h  |  20 ++-
>  6 files changed, 148 insertions(+), 59 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [PATCH S39 03/15] ice: Fix for TCAM entry management

[Tony Nguyen]

From: Dan Nowlin <dan.nowlin@intel.com>

Order intermediate VSIG list correct in order to correctly match existing
VSIG lists.

When overriding pre-existing TCAM entries, properly delete the existing
entry and remove it from the change/update list.

Signed-off-by: Dan Nowlin <dan.nowlin@intel.com>
---
 .../net/ethernet/intel/ice/ice_flex_pipe.c    | 65 +++++++++++++++----
 1 file changed, 51 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_flex_pipe.c b/drivers/net/ethernet/intel/ice/ice_flex_pipe.c
index 99208946224c..42bac3ec5526 100644
--- a/drivers/net/ethernet/intel/ice/ice_flex_pipe.c
+++ b/drivers/net/ethernet/intel/ice/ice_flex_pipe.c
@@ -3470,6 +3470,24 @@ ice_move_vsi(struct ice_hw *hw, enum ice_block blk, u16 vsi, u16 vsig,
 	return 0;
 }
 
+/**
+ * ice_rem_chg_tcam_ent - remove a specific TCAM entry from change list
+ * @hw: pointer to the HW struct
+ * @idx: the index of the TCAM entry to remove
+ * @chg: the list of change structures to search
+ */
+static void
+ice_rem_chg_tcam_ent(struct ice_hw *hw, u16 idx, struct list_head *chg)
+{
+	struct ice_chs_chg *pos, *tmp;
+
+	list_for_each_entry_safe(tmp, pos, chg, list_entry)
+		if (tmp->type == ICE_TCAM_ADD && tmp->tcam_idx == idx) {
+			list_del(&tmp->list_entry);
+			devm_kfree(ice_hw_to_dev(hw), tmp);
+		}
+}
+
 /**
  * ice_prof_tcam_ena_dis - add enable or disable TCAM change
  * @hw: pointer to the HW struct
@@ -3489,14 +3507,19 @@ ice_prof_tcam_ena_dis(struct ice_hw *hw, enum ice_block blk, bool enable,
 	enum ice_status status;
 	struct ice_chs_chg *p;
 
-	/* Default: enable means change the low flag bit to don't care */
-	u8 dc_msk[ICE_TCAM_KEY_VAL_SZ] = { 0x01, 0x00, 0x00, 0x00, 0x00 };
+	u8 vl_msk[ICE_TCAM_KEY_VAL_SZ] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
+	u8 dc_msk[ICE_TCAM_KEY_VAL_SZ] = { 0xFF, 0xFF, 0x00, 0x00, 0x00 };
 	u8 nm_msk[ICE_TCAM_KEY_VAL_SZ] = { 0x00, 0x00, 0x00, 0x00, 0x00 };
-	u8 vl_msk[ICE_TCAM_KEY_VAL_SZ] = { 0x01, 0x00, 0x00, 0x00, 0x00 };
 
 	/* if disabling, free the TCAM */
 	if (!enable) {
-		status = ice_free_tcam_ent(hw, blk, tcam->tcam_idx);
+		status = ice_rel_tcam_idx(hw, blk, tcam->tcam_idx);
+
+		/* if we have already created a change for this TCAM entry, then
+		 * we need to remove that entry, in order to prevent writing to
+		 * a TCAM entry we no longer will have ownership of.
+		 */
+		ice_rem_chg_tcam_ent(hw, tcam->tcam_idx, chg);
 		tcam->tcam_idx = 0;
 		tcam->in_use = 0;
 		return status;
@@ -3612,11 +3635,12 @@ ice_adj_prof_priorities(struct ice_hw *hw, enum ice_block blk, u16 vsig,
  * @blk: hardware block
  * @vsig: the VSIG to which this profile is to be added
  * @hdl: the profile handle indicating the profile to add
+ * @rev: true to add entries to the end of the list
  * @chg: the change list
  */
 static enum ice_status
 ice_add_prof_id_vsig(struct ice_hw *hw, enum ice_block blk, u16 vsig, u64 hdl,
-		     struct list_head *chg)
+		     bool rev, struct list_head *chg)
 {
 	/* Masks that ignore flags */
 	u8 vl_msk[ICE_TCAM_KEY_VAL_SZ] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
@@ -3625,7 +3649,7 @@ ice_add_prof_id_vsig(struct ice_hw *hw, enum ice_block blk, u16 vsig, u64 hdl,
 	struct ice_prof_map *map;
 	struct ice_vsig_prof *t;
 	struct ice_chs_chg *p;
-	u16 i;
+	u16 vsig_idx, i;
 
 	/* Get the details on the profile specified by the handle ID */
 	map = ice_search_prof_id(hw, blk, hdl);
@@ -3687,8 +3711,13 @@ ice_add_prof_id_vsig(struct ice_hw *hw, enum ice_block blk, u16 vsig, u64 hdl,
 	}
 
 	/* add profile to VSIG */
-	list_add(&t->list,
-		 &hw->blk[blk].xlt2.vsig_tbl[(vsig & ICE_VSIG_IDX_M)].prop_lst);
+	vsig_idx = vsig & ICE_VSIG_IDX_M;
+	if (rev)
+		list_add_tail(&t->list,
+			      &hw->blk[blk].xlt2.vsig_tbl[vsig_idx].prop_lst);
+	else
+		list_add(&t->list,
+			 &hw->blk[blk].xlt2.vsig_tbl[vsig_idx].prop_lst);
 
 	return 0;
 
@@ -3728,7 +3757,7 @@ ice_create_prof_id_vsig(struct ice_hw *hw, enum ice_block blk, u16 vsi, u64 hdl,
 	if (status)
 		goto err_ice_create_prof_id_vsig;
 
-	status = ice_add_prof_id_vsig(hw, blk, new_vsig, hdl, chg);
+	status = ice_add_prof_id_vsig(hw, blk, new_vsig, hdl, false, chg);
 	if (status)
 		goto err_ice_create_prof_id_vsig;
 
@@ -3753,11 +3782,13 @@ ice_create_prof_id_vsig(struct ice_hw *hw, enum ice_block blk, u16 vsi, u64 hdl,
  * @blk: hardware block
  * @vsi: the initial VSI that will be in VSIG
  * @lst: the list of profile that will be added to the VSIG
+ * @new_vsig: return of new VSIG
  * @chg: the change list
  */
 static enum ice_status
 ice_create_vsig_from_lst(struct ice_hw *hw, enum ice_block blk, u16 vsi,
-			 struct list_head *lst, struct list_head *chg)
+			 struct list_head *lst, u16 *new_vsig,
+			 struct list_head *chg)
 {
 	struct ice_vsig_prof *t;
 	enum ice_status status;
@@ -3772,12 +3803,15 @@ ice_create_vsig_from_lst(struct ice_hw *hw, enum ice_block blk, u16 vsi,
 		return status;
 
 	list_for_each_entry(t, lst, list) {
+		/* Reverse the order here since we are copying the list */
 		status = ice_add_prof_id_vsig(hw, blk, vsig, t->profile_cookie,
-					      chg);
+					      true, chg);
 		if (status)
 			return status;
 	}
 
+	*new_vsig = vsig;
+
 	return 0;
 }
 
@@ -3899,7 +3933,8 @@ ice_add_prof_id_flow(struct ice_hw *hw, enum ice_block blk, u16 vsi, u64 hdl)
 			 * not sharing entries and we can simply add the new
 			 * profile to the VSIG.
 			 */
-			status = ice_add_prof_id_vsig(hw, blk, vsig, hdl, &chg);
+			status = ice_add_prof_id_vsig(hw, blk, vsig, hdl, false,
+						      &chg);
 			if (status)
 				goto err_ice_add_prof_id_flow;
 
@@ -3910,7 +3945,8 @@ ice_add_prof_id_flow(struct ice_hw *hw, enum ice_block blk, u16 vsi, u64 hdl)
 		} else {
 			/* No match, so we need a new VSIG */
 			status = ice_create_vsig_from_lst(hw, blk, vsi,
-							  &union_lst, &chg);
+							  &union_lst, &vsig,
+							  &chg);
 			if (status)
 				goto err_ice_add_prof_id_flow;
 
@@ -4076,7 +4112,8 @@ ice_rem_prof_id_flow(struct ice_hw *hw, enum ice_block blk, u16 vsi, u64 hdl)
 				 * new VSIG and TCAM entries
 				 */
 				status = ice_create_vsig_from_lst(hw, blk, vsi,
-								  &copy, &chg);
+								  &copy, &vsig,
+								  &chg);
 				if (status)
 					goto err_ice_rem_prof_id_flow;
 
-- 
2.20.1

[Intel-wired-lan] [PATCH S39 03/15] ice: Fix for TCAM entry management

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Tony Nguyen
> Sent: Monday, January 27, 2020 12:59 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH S39 03/15] ice: Fix for TCAM entry
> management
> 
> From: Dan Nowlin <dan.nowlin@intel.com>
> 
> Order intermediate VSIG list correct in order to correctly match existing VSIG
> lists.
> 
> When overriding pre-existing TCAM entries, properly delete the existing
> entry and remove it from the change/update list.
> 
> Signed-off-by: Dan Nowlin <dan.nowlin@intel.com>
> ---
>  .../net/ethernet/intel/ice/ice_flex_pipe.c    | 65 +++++++++++++++----
>  1 file changed, 51 insertions(+), 14 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [PATCH S39 04/15] ice: Always clear the QRXFLXP_CNTXT register for VF Rx queues

[Tony Nguyen]

From: Brett Creeley <brett.creeley@intel.com>

Currently when the PF reduces its number of channels via ethtool and
then VFs are created there may be stale data for some of the Rx queues
belonging to VFs. This happens when a VF reuses an Rx queue that was
previously used by the PF. Specifically, the QRXFLXP_CNTXT register
will have incorrect values. Fix this by always clearing the relevant
values in the QRXFLXP_CNTXT register for VF queues.

Signed-off-by: Brett Creeley <brett.creeley@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_base.c       | 8 ++++++--
 drivers/net/ethernet/intel/ice/ice_hw_autogen.h | 1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_base.c b/drivers/net/ethernet/intel/ice/ice_base.c
index 75cc5a366b26..54aa533f36d4 100644
--- a/drivers/net/ethernet/intel/ice/ice_base.c
+++ b/drivers/net/ethernet/intel/ice/ice_base.c
@@ -386,8 +386,8 @@ int ice_setup_rx_ctx(struct ice_ring *ring)
 	 /* Enable Flexible Descriptors in the queue context which
 	  * allows this driver to select a specific receive descriptor format
 	  */
+	regval = rd32(hw, QRXFLXP_CNTXT(pf_q));
 	if (vsi->type != ICE_VSI_VF) {
-		regval = rd32(hw, QRXFLXP_CNTXT(pf_q));
 		regval |= (rxdid << QRXFLXP_CNTXT_RXDID_IDX_S) &
 			QRXFLXP_CNTXT_RXDID_IDX_M;
 
@@ -398,8 +398,12 @@ int ice_setup_rx_ctx(struct ice_ring *ring)
 		regval |= (0x03 << QRXFLXP_CNTXT_RXDID_PRIO_S) &
 			QRXFLXP_CNTXT_RXDID_PRIO_M;
 
-		wr32(hw, QRXFLXP_CNTXT(pf_q), regval);
+	} else {
+		regval &= ~(QRXFLXP_CNTXT_RXDID_IDX_M |
+			    QRXFLXP_CNTXT_RXDID_PRIO_M |
+			    QRXFLXP_CNTXT_TS_M);
 	}
+	wr32(hw, QRXFLXP_CNTXT(pf_q), regval);
 
 	/* Absolute queue number out of 2K needs to be passed */
 	err = ice_write_rxq_ctx(hw, &rlan_ctx, pf_q);
diff --git a/drivers/net/ethernet/intel/ice/ice_hw_autogen.h b/drivers/net/ethernet/intel/ice/ice_hw_autogen.h
index 57f1c4513c0b..3f40736a8295 100644
--- a/drivers/net/ethernet/intel/ice/ice_hw_autogen.h
+++ b/drivers/net/ethernet/intel/ice/ice_hw_autogen.h
@@ -86,6 +86,7 @@
 #define QRXFLXP_CNTXT_RXDID_IDX_M		ICE_M(0x3F, 0)
 #define QRXFLXP_CNTXT_RXDID_PRIO_S		8
 #define QRXFLXP_CNTXT_RXDID_PRIO_M		ICE_M(0x7, 8)
+#define QRXFLXP_CNTXT_TS_M			BIT(11)
 #define GLGEN_RSTAT				0x000B8188
 #define GLGEN_RSTAT_DEVSTATE_M			ICE_M(0x3, 0)
 #define GLGEN_RSTCTL				0x000B8180
-- 
2.20.1

[Intel-wired-lan] [PATCH S39 04/15] ice: Always clear the QRXFLXP_CNTXT register for VF Rx queues

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Tony Nguyen
> Sent: Monday, January 27, 2020 12:59 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH S39 04/15] ice: Always clear the
> QRXFLXP_CNTXT register for VF Rx queues
> 
> From: Brett Creeley <brett.creeley@intel.com>
> 
> Currently when the PF reduces its number of channels via ethtool and then
> VFs are created there may be stale data for some of the Rx queues belonging
> to VFs. This happens when a VF reuses an Rx queue that was previously used
> by the PF. Specifically, the QRXFLXP_CNTXT register will have incorrect
> values. Fix this by always clearing the relevant values in the QRXFLXP_CNTXT
> register for VF queues.
> 
> Signed-off-by: Brett Creeley <brett.creeley@intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_base.c       | 8 ++++++--
>  drivers/net/ethernet/intel/ice/ice_hw_autogen.h | 1 +
>  2 files changed, 7 insertions(+), 2 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [PATCH S39 05/15] ice: Add DCBNL ops required to configure ETS in CEE for SW DCB

[Tony Nguyen]

From: Avinash JD <avinash.dayanand@intel.com>

Couple of DCBNL ops are required for configuring ETS in SW DCB CEE mode. If
these functions are not added, it'll break the CEE functionality.

Signed-off-by: Avinash JD <avinash.dayanand@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_dcb_nl.c | 43 +++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_nl.c b/drivers/net/ethernet/intel/ice/ice_dcb_nl.c
index a45e8abef8f3..912467add847 100644
--- a/drivers/net/ethernet/intel/ice/ice_dcb_nl.c
+++ b/drivers/net/ethernet/intel/ice/ice_dcb_nl.c
@@ -541,6 +541,30 @@ ice_dcbnl_get_pg_tc_cfg_rx(struct net_device *netdev, int prio,
 	*pgid = pi->local_dcbx_cfg.etscfg.prio_table[prio];
 }
 
+/**
+ * ice_dcbnl_set_pg_tc_cfg_rx
+ * @netdev: relevant netdev struct
+ * @prio: corresponding user priority
+ * @prio_type: the traffic priority type
+ * @pgid: the PG ID
+ * @bw_pct: BW percentage for corresponding BWG
+ * @up_map: prio mapped to corresponding TC
+ *
+ * lldpad requires this function pointer to be non-NULL to complete CEE config.
+ */
+static void
+ice_dcbnl_set_pg_tc_cfg_rx(struct net_device *netdev,
+			   int __always_unused prio,
+			   u8 __always_unused prio_type,
+			   u8 __always_unused pgid,
+			   u8 __always_unused bw_pct,
+			   u8 __always_unused up_map)
+{
+	struct ice_pf *pf = ice_netdev_to_pf(netdev);
+
+	dev_dbg(ice_pf_to_dev(pf), "Rx TC PG Config Not Supported.\n");
+}
+
 /**
  * ice_dcbnl_get_pg_bwg_cfg_rx - Get CEE PG BW Rx config
  * @netdev: pointer to netdev struct
@@ -560,6 +584,23 @@ ice_dcbnl_get_pg_bwg_cfg_rx(struct net_device *netdev, int __always_unused pgid,
 	*bw_pct = 0;
 }
 
+/**
+ * ice_dcbnl_set_pg_bwg_cfg_rx
+ * @netdev: the corresponding netdev
+ * @pgid: corresponding TC
+ * @bw_pct: BW percentage for given TC
+ *
+ * lldpad requires this function pointer to be non-NULL to complete CEE config.
+ */
+static void
+ice_dcbnl_set_pg_bwg_cfg_rx(struct net_device *netdev, int __always_unused pgid,
+			    u8 __always_unused bw_pct)
+{
+	struct ice_pf *pf = ice_netdev_to_pf(netdev);
+
+	dev_dbg(ice_pf_to_dev(pf), "Rx BWG PG Config Not Supported.\n");
+}
+
 /**
  * ice_dcbnl_get_cap - Get DCBX capabilities of adapter
  * @netdev: pointer to netdev struct
@@ -806,6 +847,8 @@ static const struct dcbnl_rtnl_ops dcbnl_ops = {
 	.getpermhwaddr = ice_dcbnl_get_perm_hw_addr,
 	.setpgtccfgtx = ice_dcbnl_set_pg_tc_cfg_tx,
 	.setpgbwgcfgtx = ice_dcbnl_set_pg_bwg_cfg_tx,
+	.setpgtccfgrx = ice_dcbnl_set_pg_tc_cfg_rx,
+	.setpgbwgcfgrx = ice_dcbnl_set_pg_bwg_cfg_rx,
 	.getpgtccfgtx = ice_dcbnl_get_pg_tc_cfg_tx,
 	.getpgbwgcfgtx = ice_dcbnl_get_pg_bwg_cfg_tx,
 	.getpgtccfgrx = ice_dcbnl_get_pg_tc_cfg_rx,
-- 
2.20.1

[Intel-wired-lan] [PATCH S39 05/15] ice: Add DCBNL ops required to configure ETS in CEE for SW DCB

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Tony Nguyen
> Sent: Monday, January 27, 2020 12:59 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH S39 05/15] ice: Add DCBNL ops required to
> configure ETS in CEE for SW DCB
> 
> From: Avinash JD <avinash.dayanand@intel.com>
> 
> Couple of DCBNL ops are required for configuring ETS in SW DCB CEE mode. If
> these functions are not added, it'll break the CEE functionality.
> 
> Signed-off-by: Avinash JD <avinash.dayanand@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_dcb_nl.c | 43 +++++++++++++++++++++
>  1 file changed, 43 insertions(+)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [PATCH S39 06/15] ice: Report correct DCB mode

[Tony Nguyen]

From: Avinash Dayanand <avinash.dayanand@intel.com>

Add code to detect if DCB is in IEEE or CEE mode. Without this the code
will always report as IEEE mode which is incorrect and confuses the
user.

Signed-off-by: Avinash Dayanand <avinash.dayanand@intel.com>
Signed-off-by: Scott Register <scottx.register@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 27 +++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
index bd361212921c..a072d115fe73 100644
--- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
@@ -62,6 +62,26 @@ u8 ice_dcb_get_ena_tc(struct ice_dcbx_cfg *dcbcfg)
 	return ena_tc;
 }
 
+/**
+ * ice_dcb_get_mode - gets the DCB mode
+ * @port_info: pointer to port info structure
+ * @host: if set it's HOST if not it's MANAGED
+ */
+static u8 ice_dcb_get_mode(struct ice_port_info *port_info, bool host)
+{
+	u8 mode;
+
+	if (host)
+		mode = DCB_CAP_DCBX_HOST;
+	else
+		mode = DCB_CAP_DCBX_LLD_MANAGED;
+
+	if (port_info->local_dcbx_cfg.dcbx_mode & ICE_DCBX_MODE_CEE)
+		return (mode | DCB_CAP_DCBX_VER_CEE);
+	else
+		return (mode | DCB_CAP_DCBX_VER_IEEE);
+}
+
 /**
  * ice_dcb_get_num_tc - Get the number of TCs from DCBX config
  * @dcbcfg: config to retrieve number of TCs from
@@ -694,14 +714,14 @@ int ice_init_pf_dcb(struct ice_pf *pf, bool locked)
 
 		ice_cfg_sw_lldp(pf_vsi, false, true);
 
-		pf->dcbx_cap = DCB_CAP_DCBX_HOST | DCB_CAP_DCBX_VER_IEEE;
+		pf->dcbx_cap = ice_dcb_get_mode(port_info, true);
 		return 0;
 	}
 
 	set_bit(ICE_FLAG_FW_LLDP_AGENT, pf->flags);
 
-	/* DCBX in FW and LLDP enabled in FW */
-	pf->dcbx_cap = DCB_CAP_DCBX_LLD_MANAGED | DCB_CAP_DCBX_VER_IEEE;
+	/* DCBX/LLDP enabled in FW, set DCBNL mode advertisement */
+	pf->dcbx_cap = ice_dcb_get_mode(port_info, false);
 
 	err = ice_dcb_init_cfg(pf, locked);
 	if (err)
@@ -893,6 +913,7 @@ ice_dcb_process_lldp_set_mib_change(struct ice_pf *pf,
 	/* No change detected in DCBX configs */
 	if (!memcmp(&tmp_dcbx_cfg, &pi->local_dcbx_cfg, sizeof(tmp_dcbx_cfg))) {
 		dev_dbg(dev, "No change detected in DCBX configuration.\n");
+		pf->dcbx_cap = ice_dcb_get_mode(pi, false);
 		mutex_unlock(&pf->tc_mutex);
 		return;
 	}
-- 
2.20.1

[Intel-wired-lan] [PATCH S39 06/15] ice: Report correct DCB mode

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Tony Nguyen
> Sent: Monday, January 27, 2020 12:59 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH S39 06/15] ice: Report correct DCB mode
> 
> From: Avinash Dayanand <avinash.dayanand@intel.com>
> 
> Add code to detect if DCB is in IEEE or CEE mode. Without this the code will
> always report as IEEE mode which is incorrect and confuses the user.
> 
> Signed-off-by: Avinash Dayanand <avinash.dayanand@intel.com>
> Signed-off-by: Scott Register <scottx.register@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 27 +++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [PATCH S39 07/15] ice: SW DCB, report correct max TC value

[Tony Nguyen]

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

lldpad is using the value reported in the DCB config for
max_tc as the max allowed number of TCs, not the current
max.  ICE driver was reporting it as current maximum TC.

Change DCB_NL function to report maximum TC allowed by
this device.

Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_dcb_nl.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_nl.c b/drivers/net/ethernet/intel/ice/ice_dcb_nl.c
index 912467add847..823c2da62629 100644
--- a/drivers/net/ethernet/intel/ice/ice_dcb_nl.c
+++ b/drivers/net/ethernet/intel/ice/ice_dcb_nl.c
@@ -101,14 +101,7 @@ static int ice_dcbnl_setets(struct net_device *netdev, struct ieee_ets *ets)
 		goto ets_out;
 	}
 
-	/* max_tc is a 1-8 value count of number of TC's, not a 0-7 value
-	 * for the TC's index number.  Add one to value if not zero, and
-	 * for zero set it to the FW's default value
-	 */
-	if (max_tc)
-		max_tc++;
-	else
-		max_tc = IEEE_8021QAZ_MAX_TCS;
+	max_tc = pf->hw.func_caps.common_cap.maxtc;
 
 	new_cfg->etscfg.maxtcs = max_tc;
 
-- 
2.20.1

[Intel-wired-lan] [PATCH S39 07/15] ice: SW DCB, report correct max TC value

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Tony Nguyen
> Sent: Monday, January 27, 2020 12:59 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH S39 07/15] ice: SW DCB, report correct max
> TC value
> 
> From: Dave Ertman <david.m.ertman@intel.com>
> 
> lldpad is using the value reported in the DCB config for max_tc as the max
> allowed number of TCs, not the current max.  ICE driver was reporting it as
> current maximum TC.
> 
> Change DCB_NL function to report maximum TC allowed by this device.
> 
> Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_dcb_nl.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [PATCH S39 08/15] ice: Support XDP UMEM wake up mechanism

[Tony Nguyen]

From: Krzysztof Kazimierczak <krzysztof.kazimierczak@intel.com>

Add support for a new AF_XDP feature that has already been introduced in
upstreamed Intel NIC drivers. If a user space application signals that
it might sleep using the new bind flag XDP_USE_NEED_WAKEUP, the driver
will then set this flag if it has no more buffers on the NIC Rx ring and
yield to the application. For Tx, it will set the flag if it has no
outstanding Tx completion interrupts and return to the application.

Signed-off-by: Krzysztof Kazimierczak <krzysztof.kazimierczak@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_xsk.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
index 55d994f2d71e..3fd31ad73e0e 100644
--- a/drivers/net/ethernet/intel/ice/ice_xsk.c
+++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
@@ -937,6 +937,15 @@ int ice_clean_rx_irq_zc(struct ice_ring *rx_ring, int budget)
 	ice_finalize_xdp_rx(rx_ring, xdp_xmit);
 	ice_update_rx_ring_stats(rx_ring, total_rx_packets, total_rx_bytes);
 
+	if (xsk_umem_uses_need_wakeup(rx_ring->xsk_umem)) {
+		if (failure || rx_ring->next_to_clean == rx_ring->next_to_use)
+			xsk_set_rx_need_wakeup(rx_ring->xsk_umem);
+		else
+			xsk_clear_rx_need_wakeup(rx_ring->xsk_umem);
+
+		return (int)total_rx_packets;
+	}
+
 	return failure ? budget : (int)total_rx_packets;
 }
 
@@ -988,6 +997,8 @@ static bool ice_xmit_zc(struct ice_ring *xdp_ring, int budget)
 	if (tx_desc) {
 		ice_xdp_ring_update_tail(xdp_ring);
 		xsk_umem_consume_tx_done(xdp_ring->xsk_umem);
+		if (xsk_umem_uses_need_wakeup(xdp_ring->xsk_umem))
+			xsk_clear_tx_need_wakeup(xdp_ring->xsk_umem);
 	}
 
 	return budget > 0 && work_done;
@@ -1063,6 +1074,13 @@ bool ice_clean_tx_irq_zc(struct ice_ring *xdp_ring, int budget)
 	if (xsk_frames)
 		xsk_umem_complete_tx(xdp_ring->xsk_umem, xsk_frames);
 
+	if (xsk_umem_uses_need_wakeup(xdp_ring->xsk_umem)) {
+		if (xdp_ring->next_to_clean == xdp_ring->next_to_use)
+			xsk_set_tx_need_wakeup(xdp_ring->xsk_umem);
+		else
+			xsk_clear_tx_need_wakeup(xdp_ring->xsk_umem);
+	}
+
 	ice_update_tx_ring_stats(xdp_ring, total_packets, total_bytes);
 	xmit_done = ice_xmit_zc(xdp_ring, ICE_DFLT_IRQ_WORK);
 
-- 
2.20.1

[Intel-wired-lan] [PATCH S39 08/15] ice: Support XDP UMEM wake up mechanism

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Tony Nguyen
> Sent: Monday, January 27, 2020 12:59 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH S39 08/15] ice: Support XDP UMEM wake
> up mechanism
> 
> From: Krzysztof Kazimierczak <krzysztof.kazimierczak@intel.com>
> 
> Add support for a new AF_XDP feature that has already been introduced in
> upstreamed Intel NIC drivers. If a user space application signals that it might
> sleep using the new bind flag XDP_USE_NEED_WAKEUP, the driver will then
> set this flag if it has no more buffers on the NIC Rx ring and yield to the
> application. For Tx, it will set the flag if it has no outstanding Tx completion
> interrupts and return to the application.
> 
> Signed-off-by: Krzysztof Kazimierczak <krzysztof.kazimierczak@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_xsk.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [PATCH S39 09/15] ice: Don't reject odd values of usecs set by user

[Tony Nguyen]

From: Brett Creeley <brett.creeley@intel.com>

Currently if a user sets an odd [tx|rx]-usecs value through ethtool,
the request is denied because the hardware is set to have an ITR
granularity of 2us. This caused poor customer experience. Fix this by
aligning to a register allowed value, which results in rounding down.
Also, print a once per ring container type message to be clear about
our intentions.

Also, change the ITR_TO_REG define to be the bitwise and of the ITR
setting and the ICE_ITR_MASK. This makes the purpose of ITR_TO_REG more
obvious.

Signed-off-by: Brett Creeley <brett.creeley@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_ethtool.c | 49 +++++++++++++++-----
 drivers/net/ethernet/intel/ice/ice_txrx.h    |  2 +-
 2 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index db14ec2e0b46..ae0b63d5673d 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -3488,21 +3488,13 @@ ice_set_rc_coalesce(enum ice_container_type c_type, struct ethtool_coalesce *ec,
 		return -EINVAL;
 	}
 
-	/* hardware only supports an ITR granularity of 2us */
-	if (coalesce_usecs % 2 != 0) {
-		netdev_info(vsi->netdev, "Invalid value, %s-usecs must be even\n",
-			    c_type_str);
-		return -EINVAL;
-	}
-
 	if (use_adaptive_coalesce) {
 		rc->itr_setting |= ICE_ITR_DYNAMIC;
 	} else {
-		/* store user facing value how it was set */
+		/* save the user set usecs */
 		rc->itr_setting = coalesce_usecs;
-		/* set to static and convert to value HW understands */
-		rc->target_itr =
-			ITR_TO_REG(ITR_REG_ALIGN(rc->itr_setting));
+		/* device ITR granularity is in 2 usec increments */
+		rc->target_itr = ITR_REG_ALIGN(rc->itr_setting);
 	}
 
 	return 0;
@@ -3595,6 +3587,30 @@ ice_is_coalesce_param_invalid(struct net_device *netdev,
 	return 0;
 }
 
+/**
+ * ice_print_if_odd_usecs - print message if user tries to set odd [tx|rx]-usecs
+ * @netdev: netdev used for print
+ * @itr_setting: previous user setting
+ * @use_adaptive_coalesce: if adaptive coalesce is enabled or being enabled
+ * @coalesce_usecs: requested value of [tx|rx]-usecs
+ * @c_type_str: either "rx" or "tx" to match user set field of [tx|rx]-usecs
+ */
+static void
+ice_print_if_odd_usecs(struct net_device *netdev, u16 itr_setting,
+		       u32 use_adaptive_coalesce, u32 coalesce_usecs,
+		       const char *c_type_str)
+{
+	if (use_adaptive_coalesce)
+		return;
+
+	itr_setting = ITR_TO_REG(itr_setting);
+
+	if (itr_setting != coalesce_usecs && (coalesce_usecs % 2))
+		netdev_info(netdev, "User set %s-usecs to %d, device only supports even values. Rounding down and attempting to set %s-usecs to %d\n",
+			    c_type_str, coalesce_usecs, c_type_str,
+			    ITR_REG_ALIGN(coalesce_usecs));
+}
+
 /**
  * __ice_set_coalesce - set ITR/INTRL values for the device
  * @netdev: pointer to the netdev associated with this query
@@ -3615,8 +3631,19 @@ __ice_set_coalesce(struct net_device *netdev, struct ethtool_coalesce *ec,
 		return -EINVAL;
 
 	if (q_num < 0) {
+		struct ice_q_vector *q_vector = vsi->q_vectors[0];
 		int v_idx;
 
+		if (q_vector) {
+			ice_print_if_odd_usecs(netdev, q_vector->rx.itr_setting,
+					       ec->use_adaptive_rx_coalesce,
+					       ec->rx_coalesce_usecs, "rx");
+
+			ice_print_if_odd_usecs(netdev, q_vector->tx.itr_setting,
+					       ec->use_adaptive_tx_coalesce,
+					       ec->tx_coalesce_usecs, "tx");
+		}
+
 		ice_for_each_q_vector(vsi, v_idx) {
 			/* In some cases if DCB is configured the num_[rx|tx]q
 			 * can be less than vsi->num_q_vectors. This check
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.h b/drivers/net/ethernet/intel/ice/ice_txrx.h
index a86270696df1..3e3cc2599824 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.h
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.h
@@ -222,7 +222,7 @@ enum ice_rx_dtype {
 #define ICE_ITR_GRAN_S		1	/* ITR granularity is always 2us */
 #define ICE_ITR_GRAN_US		BIT(ICE_ITR_GRAN_S)
 #define ICE_ITR_MASK		0x1FFE	/* ITR register value alignment mask */
-#define ITR_REG_ALIGN(setting)	__ALIGN_MASK(setting, ~ICE_ITR_MASK)
+#define ITR_REG_ALIGN(setting)	((setting) & ICE_ITR_MASK)
 
 #define ICE_ITR_ADAPTIVE_MIN_INC	0x0002
 #define ICE_ITR_ADAPTIVE_MIN_USECS	0x0002
-- 
2.20.1

[Intel-wired-lan] [PATCH S39 09/15] ice: Don't reject odd values of usecs set by user

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Tony Nguyen
> Sent: Monday, January 27, 2020 12:59 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH S39 09/15] ice: Don't reject odd values of
> usecs set by user
> 
> From: Brett Creeley <brett.creeley@intel.com>
> 
> Currently if a user sets an odd [tx|rx]-usecs value through ethtool, the
> request is denied because the hardware is set to have an ITR granularity of
> 2us. This caused poor customer experience. Fix this by aligning to a register
> allowed value, which results in rounding down.
> Also, print a once per ring container type message to be clear about our
> intentions.
> 
> Also, change the ITR_TO_REG define to be the bitwise and of the ITR setting
> and the ICE_ITR_MASK. This makes the purpose of ITR_TO_REG more
> obvious.
> 
> Signed-off-by: Brett Creeley <brett.creeley@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_ethtool.c | 49 +++++++++++++++-----
>  drivers/net/ethernet/intel/ice/ice_txrx.h    |  2 +-
>  2 files changed, 39 insertions(+), 12 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [PATCH S39 09/15] ice: Don't reject odd values of usecs set by user

[Allan, Bruce W]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Tony Nguyen
> Sent: Monday, January 27, 2020 12:59 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH S39 09/15] ice: Don't reject odd values of
> usecs set by user
> 
> From: Brett Creeley <brett.creeley@intel.com>
> 
> Currently if a user sets an odd [tx|rx]-usecs value through ethtool,
> the request is denied because the hardware is set to have an ITR
> granularity of 2us. This caused poor customer experience. Fix this by
> aligning to a register allowed value, which results in rounding down.
> Also, print a once per ring container type message to be clear about
> our intentions.
> 
> Also, change the ITR_TO_REG define to be the bitwise and of the ITR
> setting and the ICE_ITR_MASK. This makes the purpose of ITR_TO_REG more
> obvious.
> 
> Signed-off-by: Brett Creeley <brett.creeley@intel.com>

Cc: aaron.f.rowden at intel.com

> ---
>  drivers/net/ethernet/intel/ice/ice_ethtool.c | 49 +++++++++++++++-----
>  drivers/net/ethernet/intel/ice/ice_txrx.h    |  2 +-
>  2 files changed, 39 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> index db14ec2e0b46..ae0b63d5673d 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> @@ -3488,21 +3488,13 @@ ice_set_rc_coalesce(enum ice_container_type
> c_type, struct ethtool_coalesce *ec,
>  		return -EINVAL;
>  	}
> 
> -	/* hardware only supports an ITR granularity of 2us */
> -	if (coalesce_usecs % 2 != 0) {
> -		netdev_info(vsi->netdev, "Invalid value, %s-usecs must be
> even\n",
> -			    c_type_str);
> -		return -EINVAL;
> -	}
> -
>  	if (use_adaptive_coalesce) {
>  		rc->itr_setting |= ICE_ITR_DYNAMIC;
>  	} else {
> -		/* store user facing value how it was set */
> +		/* save the user set usecs */
>  		rc->itr_setting = coalesce_usecs;
> -		/* set to static and convert to value HW understands */
> -		rc->target_itr =
> -			ITR_TO_REG(ITR_REG_ALIGN(rc->itr_setting));
> +		/* device ITR granularity is in 2 usec increments */
> +		rc->target_itr = ITR_REG_ALIGN(rc->itr_setting);
>  	}
> 
>  	return 0;
> @@ -3595,6 +3587,30 @@ ice_is_coalesce_param_invalid(struct net_device
> *netdev,
>  	return 0;
>  }
> 
> +/**
> + * ice_print_if_odd_usecs - print message if user tries to set odd [tx|rx]-usecs
> + * @netdev: netdev used for print
> + * @itr_setting: previous user setting
> + * @use_adaptive_coalesce: if adaptive coalesce is enabled or being enabled
> + * @coalesce_usecs: requested value of [tx|rx]-usecs
> + * @c_type_str: either "rx" or "tx" to match user set field of [tx|rx]-usecs
> + */
> +static void
> +ice_print_if_odd_usecs(struct net_device *netdev, u16 itr_setting,
> +		       u32 use_adaptive_coalesce, u32 coalesce_usecs,
> +		       const char *c_type_str)
> +{
> +	if (use_adaptive_coalesce)
> +		return;
> +
> +	itr_setting = ITR_TO_REG(itr_setting);
> +
> +	if (itr_setting != coalesce_usecs && (coalesce_usecs % 2))
> +		netdev_info(netdev, "User set %s-usecs to %d, device only
> supports even values. Rounding down and attempting to set %s-usecs to
> %d\n",
> +			    c_type_str, coalesce_usecs, c_type_str,
> +			    ITR_REG_ALIGN(coalesce_usecs));
> +}
> +
>  /**
>   * __ice_set_coalesce - set ITR/INTRL values for the device
>   * @netdev: pointer to the netdev associated with this query
> @@ -3615,8 +3631,19 @@ __ice_set_coalesce(struct net_device *netdev,
> struct ethtool_coalesce *ec,
>  		return -EINVAL;
> 
>  	if (q_num < 0) {
> +		struct ice_q_vector *q_vector = vsi->q_vectors[0];
>  		int v_idx;
> 
> +		if (q_vector) {
> +			ice_print_if_odd_usecs(netdev, q_vector-
> >rx.itr_setting,
> +					       ec->use_adaptive_rx_coalesce,
> +					       ec->rx_coalesce_usecs, "rx");
> +
> +			ice_print_if_odd_usecs(netdev, q_vector-
> >tx.itr_setting,
> +					       ec->use_adaptive_tx_coalesce,
> +					       ec->tx_coalesce_usecs, "tx");
> +		}
> +
>  		ice_for_each_q_vector(vsi, v_idx) {
>  			/* In some cases if DCB is configured the num_[rx|tx]q
>  			 * can be less than vsi->num_q_vectors. This check
> diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.h
> b/drivers/net/ethernet/intel/ice/ice_txrx.h
> index a86270696df1..3e3cc2599824 100644
> --- a/drivers/net/ethernet/intel/ice/ice_txrx.h
> +++ b/drivers/net/ethernet/intel/ice/ice_txrx.h
> @@ -222,7 +222,7 @@ enum ice_rx_dtype {
>  #define ICE_ITR_GRAN_S		1	/* ITR granularity is always
> 2us */
>  #define ICE_ITR_GRAN_US		BIT(ICE_ITR_GRAN_S)
>  #define ICE_ITR_MASK		0x1FFE	/* ITR register value alignment mask
> */
> -#define ITR_REG_ALIGN(setting)	__ALIGN_MASK(setting,
> ~ICE_ITR_MASK)
> +#define ITR_REG_ALIGN(setting)	((setting) & ICE_ITR_MASK)
> 
>  #define ICE_ITR_ADAPTIVE_MIN_INC	0x0002
>  #define ICE_ITR_ADAPTIVE_MIN_USECS	0x0002
> --
> 2.20.1
> 
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan at osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH S39 09/15] ice: Don't reject odd values of usecs set by user

[Paul Menzel]

Dear Tony,


On 2020-01-27 09:59, Tony Nguyen wrote:
> From: Brett Creeley <brett.creeley@intel.com>
> 
> Currently if a user sets an odd [tx|rx]-usecs value through ethtool,
> the request is denied because the hardware is set to have an ITR
> granularity of 2us. This caused poor customer experience. Fix this by
> aligning to a register allowed value, which results in rounding down.
> Also, print a once per ring container type message to be clear about
> our intentions.
> 
> Also, change the ITR_TO_REG define to be the bitwise and of the ITR
> setting and the ICE_ITR_MASK. This makes the purpose of ITR_TO_REG more
> obvious.
> 
> Signed-off-by: Brett Creeley <brett.creeley@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_ethtool.c | 49 +++++++++++++++-----
>  drivers/net/ethernet/intel/ice/ice_txrx.h    |  2 +-
>  2 files changed, 39 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> index db14ec2e0b46..ae0b63d5673d 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> @@ -3488,21 +3488,13 @@ ice_set_rc_coalesce(enum ice_container_type c_type, struct ethtool_coalesce *ec,
>  		return -EINVAL;
>  	}
>  
> -	/* hardware only supports an ITR granularity of 2us */
> -	if (coalesce_usecs % 2 != 0) {
> -		netdev_info(vsi->netdev, "Invalid value, %s-usecs must be even\n",
> -			    c_type_str);
> -		return -EINVAL;
> -	}
> -
>  	if (use_adaptive_coalesce) {
>  		rc->itr_setting |= ICE_ITR_DYNAMIC;
>  	} else {
> -		/* store user facing value how it was set */
> +		/* save the user set usecs */
>  		rc->itr_setting = coalesce_usecs;
> -		/* set to static and convert to value HW understands */
> -		rc->target_itr =
> -			ITR_TO_REG(ITR_REG_ALIGN(rc->itr_setting));
> +		/* device ITR granularity is in 2 usec increments */
> +		rc->target_itr = ITR_REG_ALIGN(rc->itr_setting);
>  	}
>  
>  	return 0;
> @@ -3595,6 +3587,30 @@ ice_is_coalesce_param_invalid(struct net_device *netdev,
>  	return 0;
>  }
>  
> +/**
> + * ice_print_if_odd_usecs - print message if user tries to set odd [tx|rx]-usecs
> + * @netdev: netdev used for print
> + * @itr_setting: previous user setting
> + * @use_adaptive_coalesce: if adaptive coalesce is enabled or being enabled
> + * @coalesce_usecs: requested value of [tx|rx]-usecs
> + * @c_type_str: either "rx" or "tx" to match user set field of [tx|rx]-usecs

Why `c_type_str`? What does it mean?

> + */
> +static void
> +ice_print_if_odd_usecs(struct net_device *netdev, u16 itr_setting,
> +		       u32 use_adaptive_coalesce, u32 coalesce_usecs,
> +		       const char *c_type_str)
> +{
> +	if (use_adaptive_coalesce)
> +		return;

Why not check that before calling the function and not passing the it to this
one.

> +
> +	itr_setting = ITR_TO_REG(itr_setting);
> +
> +	if (itr_setting != coalesce_usecs && (coalesce_usecs % 2))
> +		netdev_info(netdev, "User set %s-usecs to %d, device only supports even values. Rounding down and attempting to set %s-usecs to %d\n",
> +			    c_type_str, coalesce_usecs, c_type_str,
> +			    ITR_REG_ALIGN(coalesce_usecs));
> +}
> +
>  /**
>   * __ice_set_coalesce - set ITR/INTRL values for the device
>   * @netdev: pointer to the netdev associated with this query
> @@ -3615,8 +3631,19 @@ __ice_set_coalesce(struct net_device *netdev, struct ethtool_coalesce *ec,
>  		return -EINVAL;
>  
>  	if (q_num < 0) {
> +		struct ice_q_vector *q_vector = vsi->q_vectors[0];
>  		int v_idx;
>  
> +		if (q_vector) {

Why not check for `(coalesce_usecs % 2)` here already?

> +			ice_print_if_odd_usecs(netdev, q_vector->rx.itr_setting,
> +					       ec->use_adaptive_rx_coalesce,
> +					       ec->rx_coalesce_usecs, "rx");
> +
> +			ice_print_if_odd_usecs(netdev, q_vector->tx.itr_setting,
> +					       ec->use_adaptive_tx_coalesce,
> +					       ec->tx_coalesce_usecs, "tx");
> +		}
> +

I do not know of such a construct in the rest of the Linux kernel. Is there
a better way to achieve your goal?

>  		ice_for_each_q_vector(vsi, v_idx) {
>  			/* In some cases if DCB is configured the num_[rx|tx]q
>  			 * can be less than vsi->num_q_vectors. This check
> diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.h b/drivers/net/ethernet/intel/ice/ice_txrx.h
> index a86270696df1..3e3cc2599824 100644
> --- a/drivers/net/ethernet/intel/ice/ice_txrx.h
> +++ b/drivers/net/ethernet/intel/ice/ice_txrx.h
> @@ -222,7 +222,7 @@ enum ice_rx_dtype {
>  #define ICE_ITR_GRAN_S		1	/* ITR granularity is always 2us */
>  #define ICE_ITR_GRAN_US		BIT(ICE_ITR_GRAN_S)
>  #define ICE_ITR_MASK		0x1FFE	/* ITR register value alignment mask */
> -#define ITR_REG_ALIGN(setting)	__ALIGN_MASK(setting, ~ICE_ITR_MASK)
> +#define ITR_REG_ALIGN(setting)	((setting) & ICE_ITR_MASK)
>  
>  #define ICE_ITR_ADAPTIVE_MIN_INC	0x0002
>  #define ICE_ITR_ADAPTIVE_MIN_USECS	0x0002
> 


Kind regards,

Paul

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 5174 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20200211/a4501f0f/attachment-0001.p7s>

[Intel-wired-lan] [PATCH S39 09/15] ice: Don't reject odd values of usecs set by user

[Nguyen, Anthony L]

On Tue, 2020-02-11 at 18:21 +0100, Paul Menzel wrote:
> Dear Tony,
> 
> 
> On 2020-01-27 09:59, Tony Nguyen wrote:
> > From: Brett Creeley <brett.creeley@intel.com>
> > 
> > Currently if a user sets an odd [tx|rx]-usecs value through
> > ethtool,
> > the request is denied because the hardware is set to have an ITR
> > granularity of 2us. This caused poor customer experience. Fix this
> > by
> > aligning to a register allowed value, which results in rounding
> > down.
> > Also, print a once per ring container type message to be clear
> > about
> > our intentions.
> > 
> > Also, change the ITR_TO_REG define to be the bitwise and of the ITR
> > setting and the ICE_ITR_MASK. This makes the purpose of ITR_TO_REG
> > more
> > obvious.
> > 
> > Signed-off-by: Brett Creeley <brett.creeley@intel.com>
> > ---
> >  drivers/net/ethernet/intel/ice/ice_ethtool.c | 49 +++++++++++++++-
> > ----
> >  drivers/net/ethernet/intel/ice/ice_txrx.h    |  2 +-
> >  2 files changed, 39 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > index db14ec2e0b46..ae0b63d5673d 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> > @@ -3488,21 +3488,13 @@ ice_set_rc_coalesce(enum ice_container_type
> > c_type, struct ethtool_coalesce *ec,
> >  		return -EINVAL;
> >  	}
> >  
> > -	/* hardware only supports an ITR granularity of 2us */
> > -	if (coalesce_usecs % 2 != 0) {
> > -		netdev_info(vsi->netdev, "Invalid value, %s-usecs must
> > be even\n",
> > -			    c_type_str);
> > -		return -EINVAL;
> > -	}
> > -
> >  	if (use_adaptive_coalesce) {
> >  		rc->itr_setting |= ICE_ITR_DYNAMIC;
> >  	} else {
> > -		/* store user facing value how it was set */
> > +		/* save the user set usecs */
> >  		rc->itr_setting = coalesce_usecs;
> > -		/* set to static and convert to value HW understands */
> > -		rc->target_itr =
> > -			ITR_TO_REG(ITR_REG_ALIGN(rc->itr_setting));
> > +		/* device ITR granularity is in 2 usec increments */
> > +		rc->target_itr = ITR_REG_ALIGN(rc->itr_setting);
> >  	}
> >  
> >  	return 0;
> > @@ -3595,6 +3587,30 @@ ice_is_coalesce_param_invalid(struct
> > net_device *netdev,
> >  	return 0;
> >  }
> >  
> > +/**
> > + * ice_print_if_odd_usecs - print message if user tries to set odd
> > [tx|rx]-usecs
> > + * @netdev: netdev used for print
> > + * @itr_setting: previous user setting
> > + * @use_adaptive_coalesce: if adaptive coalesce is enabled or
> > being enabled
> > + * @coalesce_usecs: requested value of [tx|rx]-usecs
> > + * @c_type_str: either "rx" or "tx" to match user set field of
> > [tx|rx]-usecs
> 
> Why `c_type_str`? What does it mean?

It stands for container type string; so whether the container type is
"rx" or "tx".  It is has the same usage in ice_set_rx_coalesce() so we
continued with the convention here.

> > + */
> > +static void
> > +ice_print_if_odd_usecs(struct net_device *netdev, u16 itr_setting,
> > +		       u32 use_adaptive_coalesce, u32 coalesce_usecs,
> > +		       const char *c_type_str)
> > +{
> > +	if (use_adaptive_coalesce)
> > +		return;
> 
> Why not check that before calling the function and not passing the it
> to this
> one.

This is put in the function instead of before calling it to reduce code
duplication.  Adaptive coalesce can be set for Tx and Rx; usecs can
also be set for Tx and RX so putting it in here allows us to use the
same code to do the checks.  Otherwise, we will need to check each
condition for both Tx and Rx before calling this function.

> > +
> > +	itr_setting = ITR_TO_REG(itr_setting);
> > +
> > +	if (itr_setting != coalesce_usecs && (coalesce_usecs % 2))
> > +		netdev_info(netdev, "User set %s-usecs to %d, device
> > only supports even values. Rounding down and attempting to set %s-
> > usecs to %d\n",
> > +			    c_type_str, coalesce_usecs, c_type_str,
> > +			    ITR_REG_ALIGN(coalesce_usecs));
> > +}
> > +
> >  /**
> >   * __ice_set_coalesce - set ITR/INTRL values for the device
> >   * @netdev: pointer to the netdev associated with this query
> > @@ -3615,8 +3631,19 @@ __ice_set_coalesce(struct net_device
> > *netdev, struct ethtool_coalesce *ec,
> >  		return -EINVAL;
> >  
> >  	if (q_num < 0) {
> > +		struct ice_q_vector *q_vector = vsi->q_vectors[0];
> >  		int v_idx;
> >  
> > +		if (q_vector) {
> 
> Why not check for `(coalesce_usecs % 2)` here already?

Explained in previous response.

> > +			ice_print_if_odd_usecs(netdev, q_vector-
> > >rx.itr_setting,
> > +					       ec-
> > >use_adaptive_rx_coalesce,
> > +					       ec->rx_coalesce_usecs,
> > "rx");
> > +
> > +			ice_print_if_odd_usecs(netdev, q_vector-
> > >tx.itr_setting,
> > +					       ec-
> > >use_adaptive_tx_coalesce,
> > +					       ec->tx_coalesce_usecs,
> > "tx");
> > +		}
> > +
> 
> I do not know of such a construct in the rest of the Linux kernel. Is
> there
> a better way to achieve your goal?

I'm not aware of anything else to use, but I'm open to other thoughts
or suggestions if you have any.

Thanks,
Tony
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3277 bytes
Desc: not available
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20200211/9efe9e02/attachment-0001.bin>

[Intel-wired-lan] [PATCH S39 10/15] ice: Don't tell the OS that link is going down

[Tony Nguyen]

From: Michal Swiatkowski <michal.swiatkowski@intel.com>

Remove code that tell the OS that link is going down when user
change flow control via ethtool. When link is up it isn't certain
that link goes down after 0x0605 aq command. If link doesn't go
down, OS thinks that link is down, but physical link is up. To
reset this state user have to take interface down and up.

If link goes down after 0x0605 command, FW send information
about that and after that driver tells the OS that the link goes
down. So this code in ethtool is unnecessary.

Signed-off-by: Michal Swiatkowski <michal.swiatkowski@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_ethtool.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index ae0b63d5673d..c302470b095b 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -2935,13 +2935,6 @@ ice_set_pauseparam(struct net_device *netdev, struct ethtool_pauseparam *pause)
 	else
 		return -EINVAL;
 
-	/* Tell the OS link is going down, the link will go back up when fw
-	 * says it is ready asynchronously
-	 */
-	ice_print_link_msg(vsi, false);
-	netif_carrier_off(netdev);
-	netif_tx_stop_all_queues(netdev);
-
 	/* Set the FC mode and only restart AN if link is up */
 	status = ice_set_fc(pi, &aq_failures, link_up);
 
-- 
2.20.1

[Intel-wired-lan] [PATCH S39 10/15] ice: Don't tell the OS that link is going down

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Tony Nguyen
> Sent: Monday, January 27, 2020 12:59 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH S39 10/15] ice: Don't tell the OS that link is
> going down
> 
> From: Michal Swiatkowski <michal.swiatkowski@intel.com>
> 
> Remove code that tell the OS that link is going down when user change flow
> control via ethtool. When link is up it isn't certain that link goes down after
> 0x0605 aq command. If link doesn't go down, OS thinks that link is down, but
> physical link is up. To reset this state user have to take interface down and
> up.
> 
> If link goes down after 0x0605 command, FW send information about that
> and after that driver tells the OS that the link goes down. So this code in
> ethtool is unnecessary.
> 
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_ethtool.c | 7 -------
>  1 file changed, 7 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [PATCH S39 11/15] ice: increase PF reset wait timeout to 300 milliseconds

[Tony Nguyen]

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

Increase the maximum time that the driver will wait for a PF reset from
200 milliseconds to 300 milliseconds, to account for possibility of
a slightly longer than expected PF reset.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index f70e601610fc..6d7d7dbf29fe 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -6,7 +6,7 @@
 #include "ice_adminq_cmd.h"
 #include "ice_flow.h"
 
-#define ICE_PF_RESET_WAIT_COUNT	200
+#define ICE_PF_RESET_WAIT_COUNT	300
 
 /**
  * ice_set_mac_type - Sets MAC type
-- 
2.20.1

[Intel-wired-lan] [PATCH S39 11/15] ice: increase PF reset wait timeout to 300 milliseconds

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Tony Nguyen
> Sent: Monday, January 27, 2020 12:59 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH S39 11/15] ice: increase PF reset wait
> timeout to 300 milliseconds
> 
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> Increase the maximum time that the driver will wait for a PF reset from
> 200 milliseconds to 300 milliseconds, to account for possibility of a slightly
> longer than expected PF reset.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [PATCH S39 12/15] ice: add backslash-n to strings

[Tony Nguyen]

From: Jesse Brandeburg <jesse.brandeburg@intel.com>

There were several strings found without line feeds, fix
them by adding a line feed, as is typical.  Without this
lotsofmessagescanbejumbledtogether.

This patch has known checkpatch warnings from long lines
for the NL_* messages, because checkpatch doesn't know
how to ignore them.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_base.c        |  3 +--
 drivers/net/ethernet/intel/ice/ice_ethtool.c     |  8 ++++----
 drivers/net/ethernet/intel/ice/ice_main.c        | 12 ++++--------
 drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c |  2 +-
 drivers/net/ethernet/intel/ice/ice_xsk.c         |  6 +++---
 5 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_base.c b/drivers/net/ethernet/intel/ice/ice_base.c
index 54aa533f36d4..a19cd6f5436b 100644
--- a/drivers/net/ethernet/intel/ice/ice_base.c
+++ b/drivers/net/ethernet/intel/ice/ice_base.c
@@ -203,8 +203,7 @@ static void ice_cfg_itr_gran(struct ice_hw *hw)
  */
 static u16 ice_calc_q_handle(struct ice_vsi *vsi, struct ice_ring *ring, u8 tc)
 {
-	WARN_ONCE(ice_ring_is_xdp(ring) && tc,
-		  "XDP ring can't belong to TC other than 0");
+	WARN_ONCE(ice_ring_is_xdp(ring) && tc, "XDP ring can't belong to TC other than 0\n");
 
 	/* Idea here for calculation is that we subtract the number of queue
 	 * count from TC that ring belongs to from it's absolute queue index
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index c302470b095b..6d7089534d42 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -673,7 +673,7 @@ static u64 ice_loopback_test(struct net_device *netdev)
 
 	test_vsi = ice_lb_vsi_setup(pf, pf->hw.port_info);
 	if (!test_vsi) {
-		netdev_err(netdev, "Failed to create a VSI for the loopback test");
+		netdev_err(netdev, "Failed to create a VSI for the loopback test\n");
 		return 1;
 	}
 
@@ -732,7 +732,7 @@ static u64 ice_loopback_test(struct net_device *netdev)
 	devm_kfree(dev, tx_frame);
 remove_mac_filters:
 	if (ice_remove_mac(&pf->hw, &tmp_list))
-		netdev_err(netdev, "Could not remove MAC filter for the test VSI");
+		netdev_err(netdev, "Could not remove MAC filter for the test VSI\n");
 free_mac_list:
 	ice_free_fltr_list(dev, &tmp_list);
 lbtest_mac_dis:
@@ -745,7 +745,7 @@ static u64 ice_loopback_test(struct net_device *netdev)
 lbtest_vsi_close:
 	test_vsi->netdev = NULL;
 	if (ice_vsi_release(test_vsi))
-		netdev_err(netdev, "Failed to remove the test VSI");
+		netdev_err(netdev, "Failed to remove the test VSI\n");
 
 	return ret;
 }
@@ -835,7 +835,7 @@ ice_self_test(struct net_device *netdev, struct ethtool_test *eth_test,
 			int status = ice_open(netdev);
 
 			if (status) {
-				dev_err(dev, "Could not open device %s, err %d",
+				dev_err(dev, "Could not open device %s, err %d\n",
 					pf->int_name, status);
 			}
 		}
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index bbc428c4ec07..b695a65b2b3c 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -1930,8 +1930,7 @@ ice_xdp_setup_prog(struct ice_vsi *vsi, struct bpf_prog *prog,
 	if (if_running && !test_and_set_bit(__ICE_DOWN, vsi->state)) {
 		ret = ice_down(vsi);
 		if (ret) {
-			NL_SET_ERR_MSG_MOD(extack,
-					   "Preparing device for XDP attach failed");
+			NL_SET_ERR_MSG_MOD(extack, "Preparing device for XDP attach failed");
 			return ret;
 		}
 	}
@@ -1940,13 +1939,11 @@ ice_xdp_setup_prog(struct ice_vsi *vsi, struct bpf_prog *prog,
 		vsi->num_xdp_txq = vsi->alloc_txq;
 		xdp_ring_err = ice_prepare_xdp_rings(vsi, prog);
 		if (xdp_ring_err)
-			NL_SET_ERR_MSG_MOD(extack,
-					   "Setting up XDP Tx resources failed");
+			NL_SET_ERR_MSG_MOD(extack, "Setting up XDP Tx resources failed");
 	} else if (ice_is_xdp_ena_vsi(vsi) && !prog) {
 		xdp_ring_err = ice_destroy_xdp_rings(vsi);
 		if (xdp_ring_err)
-			NL_SET_ERR_MSG_MOD(extack,
-					   "Freeing XDP Tx resources failed");
+			NL_SET_ERR_MSG_MOD(extack, "Freeing XDP Tx resources failed");
 	} else {
 		ice_vsi_assign_bpf_prog(vsi, prog);
 	}
@@ -1979,8 +1976,7 @@ static int ice_xdp(struct net_device *dev, struct netdev_bpf *xdp)
 	struct ice_vsi *vsi = np->vsi;
 
 	if (vsi->type != ICE_VSI_PF) {
-		NL_SET_ERR_MSG_MOD(xdp->extack,
-				   "XDP can be loaded only on PF VSI");
+		NL_SET_ERR_MSG_MOD(xdp->extack, "XDP can be loaded only on PF VSI");
 		return -EINVAL;
 	}
 
diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c b/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c
index c84ac00a316e..5f9ef7d1d3a8 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c
@@ -1993,7 +1993,7 @@ int ice_set_vf_spoofchk(struct net_device *netdev, int vf_id, bool ena)
 
 	status = ice_update_vsi(&pf->hw, vf_vsi->idx, ctx, NULL);
 	if (status) {
-		dev_err(dev, "Failed to %sable spoofchk on VF %d VSI %d\n error %d",
+		dev_err(dev, "Failed to %sable spoofchk on VF %d VSI %d\n error %d\n",
 			ena ? "en" : "dis", vf->vf_id, vf_vsi->vsi_num, status);
 		ret = -EIO;
 		goto out;
diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
index 3fd31ad73e0e..8279db15e870 100644
--- a/drivers/net/ethernet/intel/ice/ice_xsk.c
+++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
@@ -457,7 +457,7 @@ int ice_xsk_umem_setup(struct ice_vsi *vsi, struct xdp_umem *umem, u16 qid)
 	if (if_running) {
 		ret = ice_qp_dis(vsi, qid);
 		if (ret) {
-			netdev_err(vsi->netdev, "ice_qp_dis error = %d", ret);
+			netdev_err(vsi->netdev, "ice_qp_dis error = %d\n", ret);
 			goto xsk_umem_if_up;
 		}
 	}
@@ -471,11 +471,11 @@ int ice_xsk_umem_setup(struct ice_vsi *vsi, struct xdp_umem *umem, u16 qid)
 		if (!ret && umem_present)
 			napi_schedule(&vsi->xdp_rings[qid]->q_vector->napi);
 		else if (ret)
-			netdev_err(vsi->netdev, "ice_qp_ena error = %d", ret);
+			netdev_err(vsi->netdev, "ice_qp_ena error = %d\n", ret);
 	}
 
 	if (umem_failure) {
-		netdev_err(vsi->netdev, "Could not %sable UMEM, error = %d",
+		netdev_err(vsi->netdev, "Could not %sable UMEM, error = %d\n",
 			   umem_present ? "en" : "dis", umem_failure);
 		return umem_failure;
 	}
-- 
2.20.1

[Intel-wired-lan] [PATCH S39 12/15] ice: add backslash-n to strings

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Tony Nguyen
> Sent: Monday, January 27, 2020 12:59 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH S39 12/15] ice: add backslash-n to strings
> 
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> 
> There were several strings found without line feeds, fix them by adding a line
> feed, as is typical.  Without this lotsofmessagescanbejumbledtogether.
> 
> This patch has known checkpatch warnings from long lines for the NL_*
> messages, because checkpatch doesn't know how to ignore them.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_base.c        |  3 +--
>  drivers/net/ethernet/intel/ice/ice_ethtool.c     |  8 ++++----
>  drivers/net/ethernet/intel/ice/ice_main.c        | 12 ++++--------
>  drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c |  2 +-
>  drivers/net/ethernet/intel/ice/ice_xsk.c         |  6 +++---
>  5 files changed, 13 insertions(+), 18 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [PATCH S39 13/15] ice: add additional E810 device id

[Tony Nguyen]

From: Bruce Allan <bruce.w.allan@intel.com>

Add support for device id 0x159b.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_devids.h | 2 ++
 drivers/net/ethernet/intel/ice/ice_main.c   | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_devids.h b/drivers/net/ethernet/intel/ice/ice_devids.h
index ce63017c56c7..56952d89ada8 100644
--- a/drivers/net/ethernet/intel/ice/ice_devids.h
+++ b/drivers/net/ethernet/intel/ice/ice_devids.h
@@ -11,6 +11,8 @@
 #define ICE_DEV_ID_E810C_QSFP		0x1592
 /* Intel(R) Ethernet Controller E810-C for SFP */
 #define ICE_DEV_ID_E810C_SFP		0x1593
+/* Intel(R) Ethernet Controller E810-XXV for SFP */
+#define ICE_DEV_ID_E810_XXV_SFP		0x159B
 /* Intel(R) Ethernet Connection E822-C for backplane */
 #define ICE_DEV_ID_E822C_BACKPLANE	0x1890
 /* Intel(R) Ethernet Connection E822-C for QSFP */
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index b695a65b2b3c..017d68b158fc 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3615,6 +3615,7 @@ static const struct pci_device_id ice_pci_tbl[] = {
 	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E810C_BACKPLANE), 0 },
 	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E810C_QSFP), 0 },
 	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E810C_SFP), 0 },
+	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E810_XXV_SFP), 0 },
 	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E822C_BACKPLANE), 0 },
 	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E822C_QSFP), 0 },
 	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E822C_SFP), 0 },
-- 
2.20.1

[Intel-wired-lan] [PATCH S39 13/15] ice: add additional E810 device id

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Tony Nguyen
> Sent: Monday, January 27, 2020 12:59 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH S39 13/15] ice: add additional E810 device id
> 
> From: Bruce Allan <bruce.w.allan@intel.com>
> 
> Add support for device id 0x159b.
> 
> Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_devids.h | 2 ++
>  drivers/net/ethernet/intel/ice/ice_main.c   | 1 +
>  2 files changed, 3 insertions(+)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [PATCH S39 14/15] ice: add support for E823 devices

[Tony Nguyen]

From: Bruce Allan <bruce.w.allan@intel.com>

Add E823 device ids and convert conditional expressions to a more
appropriate switch statement.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_devids.h | 20 +++++++++++++
 drivers/net/ethernet/intel/ice/ice_main.c   | 10 +++++++
 drivers/net/ethernet/intel/ice/ice_nvm.c    | 32 +++++++++++++++------
 3 files changed, 53 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_devids.h b/drivers/net/ethernet/intel/ice/ice_devids.h
index 56952d89ada8..6fccb66ff43c 100644
--- a/drivers/net/ethernet/intel/ice/ice_devids.h
+++ b/drivers/net/ethernet/intel/ice/ice_devids.h
@@ -5,6 +5,16 @@
 #define _ICE_DEVIDS_H_
 
 /* Device IDs */
+/* Intel(R) Ethernet Connection E823-L for backplane */
+#define ICE_DEV_ID_E823L_BACKPLANE	0x124C
+/* Intel(R) Ethernet Connection E823-L for SFP */
+#define ICE_DEV_ID_E823L_SFP		0x124D
+/* Intel(R) Ethernet Connection E823-L/X557-AT 10GBASE-T */
+#define ICE_DEV_ID_E823L_10G_BASE_T	0x124E
+/* Intel(R) Ethernet Connection E823-L 1GbE */
+#define ICE_DEV_ID_E823L_1GBE		0x124F
+/* Intel(R) Ethernet Connection E823-L for QSFP */
+#define ICE_DEV_ID_E823L_QSFP		0x151D
 /* Intel(R) Ethernet Controller E810-C for backplane */
 #define ICE_DEV_ID_E810C_BACKPLANE	0x1591
 /* Intel(R) Ethernet Controller E810-C for QSFP */
@@ -13,6 +23,16 @@
 #define ICE_DEV_ID_E810C_SFP		0x1593
 /* Intel(R) Ethernet Controller E810-XXV for SFP */
 #define ICE_DEV_ID_E810_XXV_SFP		0x159B
+/* Intel(R) Ethernet Connection E823-C for backplane */
+#define ICE_DEV_ID_E823C_BACKPLANE	0x188A
+/* Intel(R) Ethernet Connection E823-C for QSFP */
+#define ICE_DEV_ID_E823C_QSFP		0x188B
+/* Intel(R) Ethernet Connection E823-C for SFP */
+#define ICE_DEV_ID_E823C_SFP		0x188C
+/* Intel(R) Ethernet Connection E823-C/X557-AT 10GBASE-T */
+#define ICE_DEV_ID_E823C_10G_BASE_T	0x188D
+/* Intel(R) Ethernet Connection E823-C 1GbE */
+#define ICE_DEV_ID_E823C_SGMII		0x188E
 /* Intel(R) Ethernet Connection E822-C for backplane */
 #define ICE_DEV_ID_E822C_BACKPLANE	0x1890
 /* Intel(R) Ethernet Connection E822-C for QSFP */
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 017d68b158fc..5f23806669e2 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3616,6 +3616,11 @@ static const struct pci_device_id ice_pci_tbl[] = {
 	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E810C_QSFP), 0 },
 	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E810C_SFP), 0 },
 	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E810_XXV_SFP), 0 },
+	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E823C_BACKPLANE), 0 },
+	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E823C_QSFP), 0 },
+	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E823C_SFP), 0 },
+	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E823C_10G_BASE_T), 0 },
+	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E823C_SGMII), 0 },
 	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E822C_BACKPLANE), 0 },
 	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E822C_QSFP), 0 },
 	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E822C_SFP), 0 },
@@ -3625,6 +3630,11 @@ static const struct pci_device_id ice_pci_tbl[] = {
 	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E822L_SFP), 0 },
 	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E822L_10G_BASE_T), 0 },
 	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E822L_SGMII), 0 },
+	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E823L_BACKPLANE), 0 },
+	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E823L_SFP), 0 },
+	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E823L_10G_BASE_T), 0 },
+	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E823L_1GBE), 0 },
+	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E823L_QSFP), 0 },
 	/* required last entry */
 	{ 0, }
 };
diff --git a/drivers/net/ethernet/intel/ice/ice_nvm.c b/drivers/net/ethernet/intel/ice/ice_nvm.c
index 7525ac50742e..75cd4cbb473b 100644
--- a/drivers/net/ethernet/intel/ice/ice_nvm.c
+++ b/drivers/net/ethernet/intel/ice/ice_nvm.c
@@ -289,17 +289,31 @@ enum ice_status ice_init_nvm(struct ice_hw *hw)
 
 	nvm->eetrack = (eetrack_hi << 16) | eetrack_lo;
 
+	switch (hw->device_id) {
 	/* the following devices do not have boot_cfg_tlv yet */
-	if (hw->device_id == ICE_DEV_ID_E822C_BACKPLANE ||
-	    hw->device_id == ICE_DEV_ID_E822C_QSFP ||
-	    hw->device_id == ICE_DEV_ID_E822C_10G_BASE_T ||
-	    hw->device_id == ICE_DEV_ID_E822C_SGMII ||
-	    hw->device_id == ICE_DEV_ID_E822C_SFP ||
-	    hw->device_id == ICE_DEV_ID_E822X_BACKPLANE ||
-	    hw->device_id == ICE_DEV_ID_E822L_SFP ||
-	    hw->device_id == ICE_DEV_ID_E822L_10G_BASE_T ||
-	    hw->device_id == ICE_DEV_ID_E822L_SGMII)
+	case ICE_DEV_ID_E823C_BACKPLANE:
+	case ICE_DEV_ID_E823C_QSFP:
+	case ICE_DEV_ID_E823C_SFP:
+	case ICE_DEV_ID_E823C_10G_BASE_T:
+	case ICE_DEV_ID_E823C_SGMII:
+	case ICE_DEV_ID_E822C_BACKPLANE:
+	case ICE_DEV_ID_E822C_QSFP:
+	case ICE_DEV_ID_E822C_10G_BASE_T:
+	case ICE_DEV_ID_E822C_SGMII:
+	case ICE_DEV_ID_E822C_SFP:
+	case ICE_DEV_ID_E822X_BACKPLANE:
+	case ICE_DEV_ID_E822L_SFP:
+	case ICE_DEV_ID_E822L_10G_BASE_T:
+	case ICE_DEV_ID_E822L_SGMII:
+	case ICE_DEV_ID_E823L_BACKPLANE:
+	case ICE_DEV_ID_E823L_SFP:
+	case ICE_DEV_ID_E823L_10G_BASE_T:
+	case ICE_DEV_ID_E823L_1GBE:
+	case ICE_DEV_ID_E823L_QSFP:
 		return status;
+	default:
+		break;
+	}
 
 	status = ice_get_pfa_module_tlv(hw, &boot_cfg_tlv, &boot_cfg_tlv_len,
 					ICE_SR_BOOT_CFG_PTR);
-- 
2.20.1

[Intel-wired-lan] [PATCH S39 14/15] ice: add support for E823 devices

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Tony Nguyen
> Sent: Monday, January 27, 2020 12:59 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH S39 14/15] ice: add support for E823
> devices
> 
> From: Bruce Allan <bruce.w.allan@intel.com>
> 
> Add E823 device ids and convert conditional expressions to a more
> appropriate switch statement.
> 
> Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_devids.h | 20 +++++++++++++
>  drivers/net/ethernet/intel/ice/ice_main.c   | 10 +++++++
>  drivers/net/ethernet/intel/ice/ice_nvm.c    | 32 +++++++++++++++------
>  3 files changed, 53 insertions(+), 9 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [PATCH S39 15/15] ice: fix define for E822 backplane device

[Tony Nguyen]

From: Bruce Allan <bruce.w.allan@intel.com>

This product's name has changed; update the macro identifier accordingly.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_devids.h | 4 ++--
 drivers/net/ethernet/intel/ice/ice_main.c   | 2 +-
 drivers/net/ethernet/intel/ice/ice_nvm.c    | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_devids.h b/drivers/net/ethernet/intel/ice/ice_devids.h
index 6fccb66ff43c..9d8194671f6a 100644
--- a/drivers/net/ethernet/intel/ice/ice_devids.h
+++ b/drivers/net/ethernet/intel/ice/ice_devids.h
@@ -43,8 +43,8 @@
 #define ICE_DEV_ID_E822C_10G_BASE_T	0x1893
 /* Intel(R) Ethernet Connection E822-C 1GbE */
 #define ICE_DEV_ID_E822C_SGMII		0x1894
-/* Intel(R) Ethernet Connection E822-X for backplane */
-#define ICE_DEV_ID_E822X_BACKPLANE	0x1897
+/* Intel(R) Ethernet Connection E822-L for backplane */
+#define ICE_DEV_ID_E822L_BACKPLANE	0x1897
 /* Intel(R) Ethernet Connection E822-L for SFP */
 #define ICE_DEV_ID_E822L_SFP		0x1898
 /* Intel(R) Ethernet Connection E822-L/X557-AT 10GBASE-T */
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 5f23806669e2..b25ac179c8b8 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3626,7 +3626,7 @@ static const struct pci_device_id ice_pci_tbl[] = {
 	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E822C_SFP), 0 },
 	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E822C_10G_BASE_T), 0 },
 	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E822C_SGMII), 0 },
-	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E822X_BACKPLANE), 0 },
+	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E822L_BACKPLANE), 0 },
 	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E822L_SFP), 0 },
 	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E822L_10G_BASE_T), 0 },
 	{ PCI_VDEVICE(INTEL, ICE_DEV_ID_E822L_SGMII), 0 },
diff --git a/drivers/net/ethernet/intel/ice/ice_nvm.c b/drivers/net/ethernet/intel/ice/ice_nvm.c
index 75cd4cbb473b..f6e25db22c23 100644
--- a/drivers/net/ethernet/intel/ice/ice_nvm.c
+++ b/drivers/net/ethernet/intel/ice/ice_nvm.c
@@ -301,7 +301,7 @@ enum ice_status ice_init_nvm(struct ice_hw *hw)
 	case ICE_DEV_ID_E822C_10G_BASE_T:
 	case ICE_DEV_ID_E822C_SGMII:
 	case ICE_DEV_ID_E822C_SFP:
-	case ICE_DEV_ID_E822X_BACKPLANE:
+	case ICE_DEV_ID_E822L_BACKPLANE:
 	case ICE_DEV_ID_E822L_SFP:
 	case ICE_DEV_ID_E822L_10G_BASE_T:
 	case ICE_DEV_ID_E822L_SGMII:
-- 
2.20.1

[Intel-wired-lan] [PATCH S39 15/15] ice: fix define for E822 backplane device

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Tony Nguyen
> Sent: Monday, January 27, 2020 12:59 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH S39 15/15] ice: fix define for E822
> backplane device
> 
> From: Bruce Allan <bruce.w.allan@intel.com>
> 
> This product's name has changed; update the macro identifier accordingly.
> 
> Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_devids.h | 4 ++--
>  drivers/net/ethernet/intel/ice/ice_main.c   | 2 +-
>  drivers/net/ethernet/intel/ice/ice_nvm.c    | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [PATCH S39 01/15] ice: Validate config for SW DCB map

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Tony Nguyen
> Sent: Monday, January 27, 2020 12:59 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH S39 01/15] ice: Validate config for SW DCB
> map
> 
> From: Avinash Dayanand <avinash.dayanand@intel.com>
> 
> Validate the inputs for SW DCB config received either via lldptool or pcap file.
> And don't apply DCB for bad bandwidth inputs or non-contiguous TCs.
> Without this patch, any config having bad inputs will cause the loss of link
> making PF unusable even after driver reload. Recoverable only via system
> reboot.
> 
> Signed-off-by: Avinash Dayanand <avinash.dayanand@intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 55 ++++++++++++++++++++
> drivers/net/ethernet/intel/ice/ice_dcb_lib.h |  1 +
> drivers/net/ethernet/intel/ice/ice_dcb_nl.c  |  7 +++
>  3 files changed, 63 insertions(+)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [PATCH S39 01/15] ice: Validate config for SW DCB map

[Paul Menzel]

Dear Tony,


On 2020-01-27 09:59, Tony Nguyen wrote:
> From: Avinash Dayanand <avinash.dayanand@intel.com>
> 
> Validate the inputs for SW DCB config received either via lldptool or pcap
> file. And don't apply DCB for bad bandwidth inputs or non-contiguous TCs.
> Without this patch, any config having bad inputs will cause the loss of
> link making PF unusable even after driver reload. Recoverable only via
> system reboot.
> 
> Signed-off-by: Avinash Dayanand <avinash.dayanand@intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 55 ++++++++++++++++++++
>  drivers/net/ethernet/intel/ice/ice_dcb_lib.h |  1 +
>  drivers/net/ethernet/intel/ice/ice_dcb_nl.c  |  7 +++
>  3 files changed, 63 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
> index 0f4ca813a7ab..bd361212921c 100644
> --- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
> @@ -169,6 +169,56 @@ ice_peer_prep_tc_change(struct ice_peer_dev_int *peer_dev_int,
>  	return 0;
>  }
>  
> +/**
> + * ice_dcb_bwchk - check if ETS bandwidth input parameters are correct
> + * @dcbcfg: pointer to DCB config structure
> + */
> +int ice_dcb_bwchk(struct ice_dcbx_cfg *dcbcfg)
> +{
> +	struct ice_dcb_ets_cfg *etscfg = &dcbcfg->etscfg;
> +	u8 num_tc, total_bw = 0;
> +	int i;
> +
> +	/* returns number of contigous TCs and 1 TC for non-contigous TCs,
> +	 * since at least 1 TC has to be configured
> +	 */
> +	num_tc = ice_dcb_get_num_tc(dcbcfg);
> +
> +	/* no bandwidth checks required if there's only one TC and assign
> +	 * all bandwidth to it i.e. to TC0 and return

?, so assign all bandwidth to TC0 and return

> +	 */
> +	if (num_tc == 1) {
> +		etscfg->tcbwtable[0] = ICE_TC_MAX_BW;
> +		return 0;
> +	}
> +	/* There are few rules with which TC bandwidth can be applied for any TC
> +	 * with a UP mapped to it.
> +	 *	1. All TCs have zero BW - Valid
> +	 *	   ex:  tcbw=0,0,0
> +	 *	2. First few non-zero and rest zero BW - Valid
> +	 *	   ex:  tcbw=100,0,0
> +	 *	3. Zero BW in between 2 non-zero BW TCs - Invalid
> +	 *	   ex:  tcbw=25,0,75
> +	 */
> +	for (i = 0; i < num_tc; i++) {
> +		/* don't allow zero BW for TCs other than TC0 */
> +		if (i && !etscfg->tcbwtable[i])
> +			goto err;

As the error handling is just `return -EINVAL`, please do that directly
here.

> +
> +		if (etscfg->tsatable[i] == ICE_IEEE_TSA_ETS)
> +			total_bw += etscfg->tcbwtable[i];
> +	}
> +
> +	 /* total bandwidth should be equal to 100 */
> +	if (total_bw != ICE_TC_MAX_BW)
> +		goto err;

Ditto.

Also, why not print an error for this case?

> +
> +	return 0;
> +
> +err:
> +	return -EINVAL;
> +}
> +
>  /**
>   * ice_pf_dcb_cfg - Apply new DCB configuration
>   * @pf: pointer to the PF struct
> @@ -206,6 +256,11 @@ int ice_pf_dcb_cfg(struct ice_pf *pf, struct ice_dcbx_cfg *new_cfg, bool locked)
>  	/* Notify capable peers about impending change to TCs */
>  	ice_for_each_peer(pf, NULL, ice_peer_prep_tc_change);
>  
> +	if (ice_dcb_bwchk(new_cfg)) {
> +		dev_err(dev, "Invalid config, not applying DCB\n");

It?d be useful to know what is incorrect. So, maybe move the error message
into the function.

> +		return -EINVAL;
> +	}
> +
>  	/* Store old config in case FW config fails */
>  	old_cfg = kmemdup(curr_cfg, sizeof(*old_cfg), GFP_KERNEL);
>  	if (!old_cfg)
> diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.h b/drivers/net/ethernet/intel/ice/ice_dcb_lib.h
> index bb53edf462ba..2b900da27f57 100644
> --- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.h
> +++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.h
> @@ -20,6 +20,7 @@ u8 ice_dcb_get_num_tc(struct ice_dcbx_cfg *dcbcfg);
>  u8 ice_dcb_get_tc(struct ice_vsi *vsi, int queue_index);
>  int
>  ice_pf_dcb_cfg(struct ice_pf *pf, struct ice_dcbx_cfg *new_cfg, bool locked);
> +int ice_dcb_bwchk(struct ice_dcbx_cfg *dcbcfg);
>  void ice_pf_dcb_recfg(struct ice_pf *pf);
>  void ice_vsi_cfg_dcb_rings(struct ice_vsi *vsi);
>  int ice_init_pf_dcb(struct ice_pf *pf, bool locked);
> diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_nl.c b/drivers/net/ethernet/intel/ice/ice_dcb_nl.c
> index b61aba428adb..a45e8abef8f3 100644
> --- a/drivers/net/ethernet/intel/ice/ice_dcb_nl.c
> +++ b/drivers/net/ethernet/intel/ice/ice_dcb_nl.c
> @@ -95,6 +95,12 @@ static int ice_dcbnl_setets(struct net_device *netdev, struct ieee_ets *ets)
>  		new_cfg->etsrec.prio_table[i] = ets->reco_prio_tc[i];
>  	}
>  
> +	if (ice_dcb_bwchk(new_cfg)) {
> +		netdev_err(netdev, "Invalid config, not applying DCB\n");
> +		err = -EINVAL;
> +		goto ets_out;

Is that good style to use goto in this case? Why can?t it be put after
`ice_pf_dcb_cfg()`?

> +	}
> +
>  	/* max_tc is a 1-8 value count of number of TC's, not a 0-7 value
>  	 * for the TC's index number.  Add one to value if not zero, and
>  	 * for zero set it to the FW's default value
> @@ -119,6 +125,7 @@ static int ice_dcbnl_setets(struct net_device *netdev, struct ieee_ets *ets)
>  	if (err == ICE_DCB_NO_HW_CHG)
>  		err = ICE_DCB_HW_CHG_RST;
>  
> +ets_out:
>  	mutex_unlock(&pf->tc_mutex);
>  	return err;
>  }


Kind regards,

Paul

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 5174 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20200211/34f2edfc/attachment.p7s>

[Intel-wired-lan] [PATCH S39 01/15] ice: Validate config for SW DCB map

[Nguyen, Anthony L]

On Tue, 2020-02-11 at 18:07 +0100, Paul Menzel wrote:
> Dear Tony,
> 
> 
> On 2020-01-27 09:59, Tony Nguyen wrote:
> > From: Avinash Dayanand <avinash.dayanand@intel.com>
> > 
> > Validate the inputs for SW DCB config received either via lldptool
> > or pcap
> > file. And don't apply DCB for bad bandwidth inputs or non-
> > contiguous TCs.
> > Without this patch, any config having bad inputs will cause the
> > loss of
> > link making PF unusable even after driver reload. Recoverable only
> > via
> > system reboot.
> > 
> > Signed-off-by: Avinash Dayanand <avinash.dayanand@intel.com>
> > Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> > ---
> >  drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 55
> > ++++++++++++++++++++
> >  drivers/net/ethernet/intel/ice/ice_dcb_lib.h |  1 +
> >  drivers/net/ethernet/intel/ice/ice_dcb_nl.c  |  7 +++
> >  3 files changed, 63 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
> > b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
> > index 0f4ca813a7ab..bd361212921c 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
> > @@ -169,6 +169,56 @@ ice_peer_prep_tc_change(struct
> > ice_peer_dev_int *peer_dev_int,
> >  	return 0;
> >  }
> >  
> > +/**
> > + * ice_dcb_bwchk - check if ETS bandwidth input parameters are
> > correct
> > + * @dcbcfg: pointer to DCB config structure
> > + */
> > +int ice_dcb_bwchk(struct ice_dcbx_cfg *dcbcfg)
> > +{
> > +	struct ice_dcb_ets_cfg *etscfg = &dcbcfg->etscfg;
> > +	u8 num_tc, total_bw = 0;
> > +	int i;
> > +
> > +	/* returns number of contigous TCs and 1 TC for non-contigous
> > TCs,
> > +	 * since at least 1 TC has to be configured
> > +	 */
> > +	num_tc = ice_dcb_get_num_tc(dcbcfg);
> > +
> > +	/* no bandwidth checks required if there's only one TC and
> > assign
> > +	 * all bandwidth to it i.e. to TC0 and return
> 
> ?, so assign all bandwidth to TC0 and return
> 

Will fix the comment to include this

> > +	 */
> > +	if (num_tc == 1) {
> > +		etscfg->tcbwtable[0] = ICE_TC_MAX_BW;
> > +		return 0;
> > +	}
> > +	/* There are few rules with which TC bandwidth can be applied
> > for any TC
> > +	 * with a UP mapped to it.
> > +	 *	1. All TCs have zero BW - Valid
> > +	 *	   ex:  tcbw=0,0,0
> > +	 *	2. First few non-zero and rest zero BW - Valid
> > +	 *	   ex:  tcbw=100,0,0
> > +	 *	3. Zero BW in between 2 non-zero BW TCs - Invalid
> > +	 *	   ex:  tcbw=25,0,75
> > +	 */
> > +	for (i = 0; i < num_tc; i++) {
> > +		/* don't allow zero BW for TCs other than TC0 */
> > +		if (i && !etscfg->tcbwtable[i])
> > +			goto err;
> 
> As the error handling is just `return -EINVAL`, please do that
> directly
> here.

After talking to others this check is not needed so I'll take it out
altogether.

> > +
> > +		if (etscfg->tsatable[i] == ICE_IEEE_TSA_ETS)
> > +			total_bw += etscfg->tcbwtable[i];
> > +	}
> > +
> > +	 /* total bandwidth should be equal to 100 */
> > +	if (total_bw != ICE_TC_MAX_BW)
> > +		goto err;
> 
> Ditto.
> 
> Also, why not print an error for this case?

Will do both.

> > +
> > +	return 0;
> > +
> > +err:
> > +	return -EINVAL;
> > +}
> > +
> >  /**
> >   * ice_pf_dcb_cfg - Apply new DCB configuration
> >   * @pf: pointer to the PF struct
> > @@ -206,6 +256,11 @@ int ice_pf_dcb_cfg(struct ice_pf *pf, struct
> > ice_dcbx_cfg *new_cfg, bool locked)
> >  	/* Notify capable peers about impending change to TCs */
> >  	ice_for_each_peer(pf, NULL, ice_peer_prep_tc_change);
> >  
> > +	if (ice_dcb_bwchk(new_cfg)) {
> > +		dev_err(dev, "Invalid config, not applying DCB\n");
> 
> It?d be useful to know what is incorrect. So, maybe move the error
> message
> into the function.

Will include messaging in the function on what is incorrect.

> 
> > +		return -EINVAL;
> > +	}
> > +
> >  	/* Store old config in case FW config fails */
> >  	old_cfg = kmemdup(curr_cfg, sizeof(*old_cfg), GFP_KERNEL);
> >  	if (!old_cfg)
> > diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.h
> > b/drivers/net/ethernet/intel/ice/ice_dcb_lib.h
> > index bb53edf462ba..2b900da27f57 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.h
> > +++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.h
> > @@ -20,6 +20,7 @@ u8 ice_dcb_get_num_tc(struct ice_dcbx_cfg
> > *dcbcfg);
> >  u8 ice_dcb_get_tc(struct ice_vsi *vsi, int queue_index);
> >  int
> >  ice_pf_dcb_cfg(struct ice_pf *pf, struct ice_dcbx_cfg *new_cfg,
> > bool locked);
> > +int ice_dcb_bwchk(struct ice_dcbx_cfg *dcbcfg);
> >  void ice_pf_dcb_recfg(struct ice_pf *pf);
> >  void ice_vsi_cfg_dcb_rings(struct ice_vsi *vsi);
> >  int ice_init_pf_dcb(struct ice_pf *pf, bool locked);
> > diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_nl.c
> > b/drivers/net/ethernet/intel/ice/ice_dcb_nl.c
> > index b61aba428adb..a45e8abef8f3 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_dcb_nl.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_dcb_nl.c
> > @@ -95,6 +95,12 @@ static int ice_dcbnl_setets(struct net_device
> > *netdev, struct ieee_ets *ets)
> >  		new_cfg->etsrec.prio_table[i] = ets->reco_prio_tc[i];
> >  	}
> >  
> > +	if (ice_dcb_bwchk(new_cfg)) {
> > +		netdev_err(netdev, "Invalid config, not applying
> > DCB\n");
> > +		err = -EINVAL;
> > +		goto ets_out;
> 
> Is that good style to use goto in this case? Why can?t it be put
> after
> `ice_pf_dcb_cfg()`?

We need to validate the config before applying it, which is what
ice_pf_dcb_cfg() does. We can't apply the config without knowing that
it's a good configuration.

For the goto, we're trying to have a single point of unlock/exit.

Thanks,
Tony
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3277 bytes
Desc: not available
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20200211/1255c25f/attachment.bin>