i40e: Properly check allowed advertisement capabilities

PATCH New Alice Michael 2 participants 11 messages Mar 12, 2018 – Mar 13, 2018

Subsystems (2)

Network drivers Intel Ethernet

lore

Share & tools

lore

Alice Michael

author

2018-03-12 14:00 UTC #0 lore raw

PATCH 1/8 i40e: Properly check allowed advertisement capabilities

From: Jan Sokolowski <[email protected]>

The i40e_set_link_ksettings and i40e_get_link_ksettings use different codepaths to check available and supported advertisement modes. This creates scenarios where it's possible to set a mode that's not allowed, resulting in a link down.

Fix setting advertisement in i40e_set_link_ksettings by calling i40e_get_link_ksettings to check what modes are allowed.

Signed-off-by Jan Sokolowski <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

drivers/net/ethernet/intel/i40e/i40e_ethtool.c +7 -9

index f9493f1..9bb85da 100644
@@ -871,23 +871,21 @@ static int i40e_set_link_ksettings(struct net_device *netdev,
 	/* save autoneg out of ksettings */
 	autoneg = copy_ks.base.autoneg;
 
-	memset(&safe_ks, 0, sizeof(safe_ks));
+	/* get our own copy of the bits to check against */
+	memset(&safe_ks, 0, sizeof(struct ethtool_link_ksettings));
+	safe_ks.base.cmd = copy_ks.base.cmd;
+	safe_ks.base.link_mode_masks_nwords =
+		copy_ks.base.link_mode_masks_nwords;
+	i40e_get_link_ksettings(netdev, &safe_ks);
+
 	/* Get link modes supported by hardware and check against modes
 	 * requested by the user.  Return an error if unsupported mode was set.
 	 */
-	i40e_phy_type_to_ethtool(pf, &safe_ks);
 	if (!bitmap_subset(copy_ks.link_modes.advertising,
 			   safe_ks.link_modes.supported,
 			   __ETHTOOL_LINK_MODE_MASK_NBITS))
 		return -EINVAL;
 
-	/* get our own copy of the bits to check against */
-	memset(&safe_ks, 0, sizeof(struct ethtool_link_ksettings));
-	safe_ks.base.cmd = copy_ks.base.cmd;
-	safe_ks.base.link_mode_masks_nwords =
-		copy_ks.base.link_mode_masks_nwords;
-	i40e_get_link_ksettings(netdev, &safe_ks);
-
 	/* set autoneg back to what it currently is */
 	copy_ks.base.autoneg = safe_ks.base.autoneg;

Signature · 1 line

2.9.5

Alice Michael

author

2018-03-12 14:00 UTC #1 lore raw

PATCH 2/8 i40e: Prevent setting link speed on KX_X722

From: Patryk Ma?ek <[email protected]>

Setting link settings on backplane devices shouldn't be allowed. This patch adds one more device id to the list which we check that against.

Signed-off-by Patryk Ma?ek <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/net/ethernet/intel/i40e/i40e_ethtool.c +2 -1

index 9bb85da..d86d090 100644
@@ -860,7 +860,8 @@ static int i40e_set_link_ksettings(struct net_device *netdev,
 	if (hw->device_id == I40E_DEV_ID_KX_B ||
 	    hw->device_id == I40E_DEV_ID_KX_C ||
 	    hw->device_id == I40E_DEV_ID_20G_KR2 ||
-	    hw->device_id == I40E_DEV_ID_20G_KR2_A) {
+	    hw->device_id == I40E_DEV_ID_20G_KR2_A ||
+	    hw->device_id == I40E_DEV_ID_KX_X722) {
 		netdev_info(netdev, "Changing settings is not supported on backplane.\n");
 		return -EOPNOTSUPP;
 	}

Signature · 1 line

2.9.5

Malek

2018-03-13 08:20 UTC #2 lore raw

PATCH 2/8 i40e: Prevent setting link speed on KX_X722

ACK, thanks

-----Original Message-----
From: Michael, Alice
Sent: Monday, March 12, 2018 15:00
To: alice.michael; intel-wired-lan at lists.osuosl.org
Cc: Malek, Patryk <[email protected]>
Subject: [next PATCH S88 2/8] i40e: Prevent setting link speed on KX_X722

From: Patryk Ma?ek <[email protected]>

Setting link settings on backplane devices shouldn't be allowed. This patch adds one more device id to the list which we check that against.

Signed-off-by Patryk Ma?ek <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/net/ethernet/intel/i40e/i40e_ethtool.c +2 -1

index 9bb85da..d86d090 100644
@@ -860,7 +860,8 @@ static int i40e_set_link_ksettings(struct net_device *netdev,
 	if (hw->device_id == I40E_DEV_ID_KX_B ||
 	    hw->device_id == I40E_DEV_ID_KX_C ||
 	    hw->device_id == I40E_DEV_ID_20G_KR2 ||
-	    hw->device_id == I40E_DEV_ID_20G_KR2_A) {
+	    hw->device_id == I40E_DEV_ID_20G_KR2_A ||
+	    hw->device_id == I40E_DEV_ID_KX_X722) {
 		netdev_info(netdev, "Changing settings is not supported on backplane.\n");
 		return -EOPNOTSUPP;
 	}

--
2.9.5

--------------------------------------------------------------------

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

Alice Michael

author

2018-03-12 14:00 UTC #3 lore raw

PATCH 3/8 i40e: Close client on suspend and restore client MSIx on resume

In reply to Alice Michael

From: Shiraz Saleem <[email protected]>

During suspend client MSIx vectors are freed while they are still in use causing a crash on entering S3.

Fix this calling client close before freeing up its MSIx vectors. Also update the client MSIx vectors on resume before client open is called.

Fixes commit b980c0634fe5 ("i40e: shutdown all IRQs and disable MSI-X when suspended")

Reported-by Stefan Assmann <[email protected]>

Signed-off-by Shiraz Saleem <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e.h        |  1 +
 drivers/net/ethernet/intel/i40e/i40e_client.c | 16 +++++++++++++---
 drivers/net/ethernet/intel/i40e/i40e_main.c   |  8 ++++++++
 3 files changed, 22 insertions(+), 3 deletions(-)

drivers/net/ethernet/intel/i40e/i40e.h +1 -0

index 956299c..1e1cd52 100644
@@ -1042,6 +1042,7 @@ void i40e_notify_client_of_l2_param_changes(struct i40e_vsi *vsi);
 void i40e_notify_client_of_netdev_close(struct i40e_vsi *vsi, bool reset);
 void i40e_notify_client_of_vf_enable(struct i40e_pf *pf, u32 num_vfs);
 void i40e_notify_client_of_vf_reset(struct i40e_pf *pf, u32 vf_id);
+void i40e_client_update_msix_info(struct i40e_pf *pf);
 int i40e_vf_client_capable(struct i40e_pf *pf, u32 vf_id);
 /**
  * i40e_irq_dynamic_enable - Enable default interrupt generation settings

drivers/net/ethernet/intel/i40e/i40e_client.c +13 -3

index 0680a62..a04bbfc 100644
@@ -288,6 +288,17 @@ int i40e_vf_client_capable(struct i40e_pf *pf, u32 vf_id)
 	return capable;
 }
 
+void i40e_client_update_msix_info(struct i40e_pf *pf)
+{
+	struct i40e_client_instance *cdev = pf->cinst;
+
+	if (!cdev || !cdev->client)
+		return;
+
+	cdev->lan_info.msix_count = pf->num_iwarp_msix;
+	cdev->lan_info.msix_entries = &pf->msix_entries[pf->iwarp_base_vector];
+}
+
 /**
  * i40e_client_add_instance - add a client instance struct to the instance list
  * @pf: pointer to the board struct

@@ -329,9 +340,6 @@ static void i40e_client_add_instance(struct i40e_pf *pf)
 		return;
 	}
 
-	cdev->lan_info.msix_count = pf->num_iwarp_msix;
-	cdev->lan_info.msix_entries = &pf->msix_entries[pf->iwarp_base_vector];
-
 	mac = list_first_entry(&cdev->lan_info.netdev->dev_addrs.list,
 			       struct netdev_hw_addr, list);
 	if (mac)

@@ -341,6 +349,8 @@ static void i40e_client_add_instance(struct i40e_pf *pf)
 
 	cdev->client = registered_client;
 	pf->cinst = cdev;
+
+	i40e_client_update_msix_info(pf);
 }
 
 /**

drivers/net/ethernet/intel/i40e/i40e_main.c +8 -0

index 0df21e4..5a77dbf 100644
@@ -10595,6 +10595,9 @@ static int i40e_restore_interrupt_scheme(struct i40e_pf *pf)
 	if (err)
 		goto err_unwind;
 
+	if (pf->flags & I40E_FLAG_IWARP_ENABLED)
+		i40e_client_update_msix_info(pf);
+
 	return 0;
 
 err_unwind:

@@ -14345,6 +14348,11 @@ static int __maybe_unused i40e_suspend(struct device *dev)
 	del_timer_sync(&pf->service_timer);
 	cancel_work_sync(&pf->service_task);
 
+	/* Client close must be called explicitly here because the timer
+	 * has been stopped.
+	 */
+	i40e_notify_client_of_netdev_close(pf->vsi[pf->lan_vsi], false);
+
 	if (pf->wol_en && (pf->hw_features & I40E_HW_WOL_MC_MAGIC_PKT_WAKE))
 		i40e_enable_mc_magic_wake(pf);

Signature · 1 line

2.9.5

Saleem

2018-03-12 22:55 UTC #4 lore raw

PATCH 3/8 i40e: Close client on suspend and restore client MSIx on resume

Quoted 11 lines

Subject: [next PATCH S88 3/8] i40e: Close client on suspend and restore client MSIx on resume
From: Shiraz Saleem <[email protected]>
During suspend client MSIx vectors are freed while they are still in use causing a crash on entering S3.
Fix this calling client close before freeing up its MSIx vectors. Also update the client MSIx vectors on resume before client open is called.

Quoted 6 lines

Fixes commit b980c0634fe5 ("i40e: shutdown all IRQs and disable MSI-X when suspended")
Reported-by Stefan Assmann <[email protected]>
Signed-off-by Shiraz Saleem <[email protected]>
---

ACK. Thank you Alice!

Alice Michael

author

2018-03-12 14:00 UTC #5 lore raw

PATCH 4/8 i40e: add doxygen comment for new mode parameter

In reply to Alice Michael

From: Jacob Keller <[email protected]>

A recent patch updated the signature for i40e_aq_set_switch_config() to add a new parameter 'mode'. It forgot to document the parameter in the doxygen function header comment. Add the parameter to the function description now.

Signed-off-by Jacob Keller <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_common.c | 1 +
 1 file changed, 1 insertion(+)

drivers/net/ethernet/intel/i40e/i40e_common.c +1 -0

index 792d420..2d00e95 100644
@@ -2416,6 +2416,7 @@ i40e_status i40e_aq_get_switch_config(struct i40e_hw *hw,
  * i40e_aq_set_switch_config
  * @hw: pointer to the hardware structure
  * @flags: bit flag values to set
+ * @mode: cloud filter mode
  * @valid_flags: which bit flags to set
  * @mode: cloud filter mode
  * @cmd_details: pointer to command details structure or NULL

Signature · 1 line

2.9.5

Alice Michael

author

2018-03-12 14:00 UTC #6 lore raw

PATCH 5/8 i40e: Fix incorrect return types

In reply to Alice Michael

From: Doug Dziggel <[email protected]>

Fix return types from i40e_status to enum i40e_status_code.

Signed-off-by Doug Dziggel <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_common.c    | 4 ++--
 drivers/net/ethernet/intel/i40e/i40e_prototype.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

drivers/net/ethernet/intel/i40e/i40e_common.c +2 -2

index 2d00e95..27d4d20 100644
@@ -5554,7 +5554,7 @@ i40e_aq_add_cloud_filters(struct i40e_hw *hw, u16 seid,
  * function.
  *
  **/
-i40e_status
+enum i40e_status_code
 i40e_aq_add_cloud_filters_bb(struct i40e_hw *hw, u16 seid,
 			     struct i40e_aqc_cloud_filters_element_bb *filters,
 			     u8 filter_count)

@@ -5648,7 +5648,7 @@ i40e_aq_rem_cloud_filters(struct i40e_hw *hw, u16 seid,
  * function.
  *
  **/
-i40e_status
+enum i40e_status_code
 i40e_aq_rem_cloud_filters_bb(struct i40e_hw *hw, u16 seid,
 			     struct i40e_aqc_cloud_filters_element_bb *filters,
 			     u8 filter_count)

drivers/net/ethernet/intel/i40e/i40e_prototype.h +2 -2

index 57f6720..fc810a9 100644
@@ -288,7 +288,7 @@ i40e_status i40e_aq_query_switch_comp_bw_config(struct i40e_hw *hw,
 		struct i40e_asq_cmd_details *cmd_details);
 i40e_status i40e_aq_resume_port_tx(struct i40e_hw *hw,
 				   struct i40e_asq_cmd_details *cmd_details);
-i40e_status
+enum i40e_status_code
 i40e_aq_add_cloud_filters_bb(struct i40e_hw *hw, u16 seid,
 			     struct i40e_aqc_cloud_filters_element_bb *filters,
 			     u8 filter_count);

@@ -300,7 +300,7 @@ enum i40e_status_code
 i40e_aq_rem_cloud_filters(struct i40e_hw *hw, u16 vsi,
 			  struct i40e_aqc_cloud_filters_element_data *filters,
 			  u8 filter_count);
-i40e_status
+enum i40e_status_code
 i40e_aq_rem_cloud_filters_bb(struct i40e_hw *hw, u16 seid,
 			     struct i40e_aqc_cloud_filters_element_bb *filters,
 			     u8 filter_count);

Signature · 1 line

2.9.5

Alice Michael

author

2018-03-12 14:00 UTC #7 lore raw

PATCH 6/8 i40e: Prevent setting link speed on I40E_DEV_ID_25G_B

In reply to Alice Michael

From: Patryk Ma?ek <[email protected]>

Setting link settings on backplane devices shouldn't be allowed. This patch adds one more device id to the list which we check that against.

Signed-off-by Patryk Ma?ek <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 1 +
 1 file changed, 1 insertion(+)

drivers/net/ethernet/intel/i40e/i40e_ethtool.c +1 -0

index d86d090..722eb6b 100644
@@ -861,6 +861,7 @@ static int i40e_set_link_ksettings(struct net_device *netdev,
 	    hw->device_id == I40E_DEV_ID_KX_C ||
 	    hw->device_id == I40E_DEV_ID_20G_KR2 ||
 	    hw->device_id == I40E_DEV_ID_20G_KR2_A ||
+	    hw->device_id == I40E_DEV_ID_25G_B ||
 	    hw->device_id == I40E_DEV_ID_KX_X722) {
 		netdev_info(netdev, "Changing settings is not supported on backplane.\n");
 		return -EOPNOTSUPP;

Signature · 1 line

2.9.5

Malek

2018-03-13 08:20 UTC #8 lore raw

PATCH 6/8 i40e: Prevent setting link speed on I40E_DEV_ID_25G_B

ACK, thanks

From: Patryk Ma?ek <[email protected]>

Setting link settings on backplane devices shouldn't be allowed. This patch adds one more device id to the list which we check that against.

Signed-off-by Patryk Ma?ek <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 1 +
 1 file changed, 1 insertion(+)

drivers/net/ethernet/intel/i40e/i40e_ethtool.c +1 -0

index d86d090..722eb6b 100644
@@ -861,6 +861,7 @@ static int i40e_set_link_ksettings(struct net_device *netdev,
 	    hw->device_id == I40E_DEV_ID_KX_C ||
 	    hw->device_id == I40E_DEV_ID_20G_KR2 ||
 	    hw->device_id == I40E_DEV_ID_20G_KR2_A ||
+	    hw->device_id == I40E_DEV_ID_25G_B ||
 	    hw->device_id == I40E_DEV_ID_KX_X722) {
 		netdev_info(netdev, "Changing settings is not supported on backplane.\n");
 		return -EOPNOTSUPP;

--
2.9.5

--------------------------------------------------------------------

Alice Michael

author

2018-03-12 14:00 UTC #9 lore raw

PATCH 7/8 i40evf: remove flags that are never used

In reply to Alice Michael

From: Jacob Keller <[email protected]>

These flags were defined, but there is no use within the driver code, so we don't need to keep them.

Signed-off-by Jacob Keller <[email protected]>

---
 drivers/net/ethernet/intel/i40evf/i40evf.h | 3 ---
 1 file changed, 3 deletions(-)

drivers/net/ethernet/intel/i40evf/i40evf.h +0 -3

index 8550f78..d8fe200 100644
@@ -281,13 +281,10 @@ struct i40evf_adapter {
 
 	u32 flags;
 #define I40EVF_FLAG_RX_CSUM_ENABLED		BIT(0)
-#define I40EVF_FLAG_IMIR_ENABLED		BIT(1)
-#define I40EVF_FLAG_MQ_CAPABLE			BIT(2)
 #define I40EVF_FLAG_PF_COMMS_FAILED		BIT(3)
 #define I40EVF_FLAG_RESET_PENDING		BIT(4)
 #define I40EVF_FLAG_RESET_NEEDED		BIT(5)
 #define I40EVF_FLAG_WB_ON_ITR_CAPABLE		BIT(6)
-#define I40EVF_FLAG_OUTER_UDP_CSUM_CAPABLE	BIT(7)
 #define I40EVF_FLAG_ADDR_SET_BY_PF		BIT(8)
 #define I40EVF_FLAG_SERVICE_CLIENT_REQUESTED	BIT(9)
 #define I40EVF_FLAG_CLIENT_NEEDS_OPEN		BIT(10)

Signature · 1 line

2.9.5

Alice Michael

author

2018-03-12 14:00 UTC #10 lore raw

PATCH 8/8 i40e: Fix the polling mechanism of GLGEN_RSTAT.DEVSTATE

In reply to Alice Michael

From: Pawe? Jab?o?ski <[email protected]>

This fixes the polling mechanism of GLGEN_RSTAT.DEVSTATE in the PF Reset path when Global Reset is in progress. While the driver is polling for the end of the PF Reset and the Global Reset is triggered, abandon the PF Reset path and prepare for the upcoming Global Reset.

Signed-off-by Pawe? Jab?o?ski <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_common.c | 35 ++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 6 deletions(-)

drivers/net/ethernet/intel/i40e/i40e_common.c +29 -6

index 27d4d20..b70a6d5 100644
@@ -1209,6 +1209,29 @@ static enum i40e_media_type i40e_get_media_type(struct i40e_hw *hw)
 	return media;
 }
 
+/**
+ * i40e_poll_globr - Poll for Global Reset completion
+ * @hw: pointer to the hardware structure
+ * @retry_limit: how many times to retry before failure
+ **/
+static i40e_status i40e_poll_globr(struct i40e_hw *hw,
+				   u32 retry_limit)
+{
+	u32 cnt, reg = 0;
+
+	for (cnt = 0; cnt < retry_limit; cnt++) {
+		reg = rd32(hw, I40E_GLGEN_RSTAT);
+		if (!(reg & I40E_GLGEN_RSTAT_DEVSTATE_MASK))
+			return 0;
+		msleep(100);
+	}
+
+	hw_dbg(hw, "Global reset failed.\n");
+	hw_dbg(hw, "I40E_GLGEN_RSTAT = 0x%x\n", reg);
+
+	return I40E_ERR_RESET_FAILED;
+}
+
 #define I40E_PF_RESET_WAIT_COUNT_A0	200
 #define I40E_PF_RESET_WAIT_COUNT	200
 /**

@@ -1285,14 +1308,14 @@ i40e_status i40e_pf_reset(struct i40e_hw *hw)
 			if (!(reg & I40E_PFGEN_CTRL_PFSWR_MASK))
 				break;
 			reg2 = rd32(hw, I40E_GLGEN_RSTAT);
-			if (reg2 & I40E_GLGEN_RSTAT_DEVSTATE_MASK) {
-				hw_dbg(hw, "Core reset upcoming. Skipping PF reset request.\n");
-				hw_dbg(hw, "I40E_GLGEN_RSTAT = 0x%x\n", reg2);
-				return I40E_ERR_NOT_READY;
-			}
+			if (reg2 & I40E_GLGEN_RSTAT_DEVSTATE_MASK)
+				break;
 			usleep_range(1000, 2000);
 		}
-		if (reg & I40E_PFGEN_CTRL_PFSWR_MASK) {
+		if (reg2 & I40E_GLGEN_RSTAT_DEVSTATE_MASK) {
+			if (i40e_poll_globr(hw, grst_del))
+				return I40E_ERR_RESET_FAILED;
+		} else if (reg & I40E_PFGEN_CTRL_PFSWR_MASK) {
 			hw_dbg(hw, "PF reset polling failed to complete.\n");
 			return I40E_ERR_RESET_FAILED;
 		}

Signature · 1 line

2.9.5