[dpdk-dev,v2] net/i40e: fix link down and negotiation issue

Message ID 1500606424-83169-1-git-send-email-jia.guo@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Guo, Jia July 21, 2017, 3:07 a.m. UTC
Enable the functions set link down and set link up in i40e by check
phy_type, and fix the issue of auto negotiation failed in XXV710 when
bind kernel driver after unbind from dpdk driver by modify the speed
setting distinguish from set link up and down. With this fix, if unbind
dpdk to bind kernel driver, no need to set auto negotiation and ifconfi
up anymore, remove the part from doc.

Fixes: ca7e599d4506 ("net/i40e: fix link management")
Fixes: 2f1e22817420 ("i40e: skip link control as firmware workaround")
Fixes: 6e145fcc754b ("i40e: support autoneg or force link speed")
Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
v2->v1: fix comment error and redefine variable 
---
 doc/guides/nics/i40e.rst       | 10 ----------
 drivers/net/i40e/i40e_ethdev.c | 34 ++++++++++++++++++++++------------
 2 files changed, 22 insertions(+), 22 deletions(-)
  

Comments

Jingjing Wu July 21, 2017, 6:33 a.m. UTC | #1
> -----Original Message-----
> From: Guo, Jia
> Sent: Friday, July 21, 2017 11:07 AM
> To: Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Guo, Jia <jia.guo@intel.com>; stable@dpdk.org
> Subject: [PATCH v2] net/i40e: fix link down and negotiation issue
> 
> Enable the functions set link down and set link up in i40e by check phy_type,
> and fix the issue of auto negotiation failed in XXV710 when bind kernel driver
> after unbind from dpdk driver by modify the speed setting distinguish from set
> link up and down. With this fix, if unbind dpdk to bind kernel driver, no need to
> set auto negotiation and ifconfi up anymore, remove the part from doc.
> 
> Fixes: ca7e599d4506 ("net/i40e: fix link management")
> Fixes: 2f1e22817420 ("i40e: skip link control as firmware workaround")
> Fixes: 6e145fcc754b ("i40e: support autoneg or force link speed")
> Signed-off-by: Jeff Guo <jia.guo@intel.com>

Acked-by: Jingjing Wu <jingjing.wu@intel.com>


> ---
> v2->v1: fix comment error and redefine variable
> ---
>  doc/guides/nics/i40e.rst       | 10 ----------
>  drivers/net/i40e/i40e_ethdev.c | 34 ++++++++++++++++++++++------------
>  2 files changed, 22 insertions(+), 22 deletions(-)
>
  
Ferruh Yigit July 21, 2017, 9:06 a.m. UTC | #2
On 7/21/2017 4:07 AM, Jeff Guo wrote:
> Enable the functions set link down and set link up in i40e by check
> phy_type, and fix the issue of auto negotiation failed in XXV710 when
> bind kernel driver after unbind from dpdk driver by modify the speed
> setting distinguish from set link up and down. With this fix, if unbind
> dpdk to bind kernel driver, no need to set auto negotiation and ifconfi
> up anymore, remove the part from doc.
> 
> Fixes: ca7e599d4506 ("net/i40e: fix link management")
> Fixes: 2f1e22817420 ("i40e: skip link control as firmware workaround")
> Fixes: 6e145fcc754b ("i40e: support autoneg or force link speed")
Cc: stable@dpdk.org

> Signed-off-by: Jeff Guo <jia.guo@intel.com>

Applied to dpdk-next-net/master, thanks.
  

Patch

diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst
index ba1a821..a0262a9 100644
--- a/doc/guides/nics/i40e.rst
+++ b/doc/guides/nics/i40e.rst
@@ -404,16 +404,6 @@  is used as the VF driver, DPDK cannot choose 16 byte receive descriptor. That
 is to say, user should keep ``CONFIG_RTE_LIBRTE_I40E_16BYTE_RX_DESC=n`` in
 config file.
 
-Link down with i40e kernel driver after DPDK application exit
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-After DPDK application quit, and the device is bound back to Linux i40e
-kernel driver, the link cannot be up after ``ifconfig <dev> up``.
-To work around this issue, ``ethtool -s <dev> autoneg on`` should be
-set first and then the link can be brought up through ``ifconfig <dev> up``.
-
-NOTE: requires Linux kernel i40e driver version >= 1.4.X
-
 Receive packets with Ethertype 0x88A8
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 9fcccda..b71506a 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1814,11 +1814,15 @@  i40e_parse_link_speeds(uint16_t link_speeds)
 static int
 i40e_phy_conf_link(struct i40e_hw *hw,
 		   uint8_t abilities,
-		   uint8_t force_speed)
+		   uint8_t force_speed,
+		   bool is_up)
 {
 	enum i40e_status_code status;
 	struct i40e_aq_get_phy_abilities_resp phy_ab;
 	struct i40e_aq_set_phy_config phy_conf;
+	enum i40e_aq_phy_type cnt;
+	uint32_t phy_type_mask = 0;
+
 	const uint8_t mask = I40E_AQ_PHY_FLAG_PAUSE_TX |
 			I40E_AQ_PHY_FLAG_PAUSE_RX |
 			I40E_AQ_PHY_FLAG_PAUSE_RX |
@@ -1836,6 +1840,10 @@  i40e_phy_conf_link(struct i40e_hw *hw,
 	if (status)
 		return ret;
 
+	/* If link already up, no need to set up again */
+	if (is_up && phy_ab.phy_type != 0)
+		return I40E_SUCCESS;
+
 	memset(&phy_conf, 0, sizeof(phy_conf));
 
 	/* bits 0-2 use the values from get_phy_abilities_resp */
@@ -1846,13 +1854,21 @@  i40e_phy_conf_link(struct i40e_hw *hw,
 	if (abilities & I40E_AQ_PHY_AN_ENABLED)
 		phy_conf.link_speed = advt;
 	else
-		phy_conf.link_speed = force_speed;
+		phy_conf.link_speed = is_up ? force_speed : phy_ab.link_speed;
 
 	phy_conf.abilities = abilities;
 
+
+
+	/* To enable link, phy_type mask needs to include each type */
+	for (cnt = I40E_PHY_TYPE_SGMII; cnt < I40E_PHY_TYPE_MAX; cnt++)
+		phy_type_mask |= 1 << cnt;
+
 	/* use get_phy_abilities_resp value for the rest */
-	phy_conf.phy_type = phy_ab.phy_type;
-	phy_conf.phy_type_ext = phy_ab.phy_type_ext;
+	phy_conf.phy_type = is_up ? cpu_to_le32(phy_type_mask) : 0;
+	phy_conf.phy_type_ext = is_up ? (I40E_AQ_PHY_TYPE_EXT_25G_KR |
+		I40E_AQ_PHY_TYPE_EXT_25G_CR | I40E_AQ_PHY_TYPE_EXT_25G_SR |
+		I40E_AQ_PHY_TYPE_EXT_25G_LR) : 0;
 	phy_conf.fec_config = phy_ab.fec_cfg_curr_mod_ext_info;
 	phy_conf.eee_capability = phy_ab.eee_capability;
 	phy_conf.eeer = phy_ab.eeer_val;
@@ -1884,13 +1900,7 @@  i40e_apply_link_speed(struct rte_eth_dev *dev)
 		abilities |= I40E_AQ_PHY_AN_ENABLED;
 	abilities |= I40E_AQ_PHY_LINK_ENABLED;
 
-	/* Skip changing speed on 40G interfaces, FW does not support */
-	if (I40E_PHY_TYPE_SUPPORT_40G(hw->phy.phy_types)) {
-		speed =  I40E_LINK_SPEED_UNKNOWN;
-		abilities |= I40E_AQ_PHY_AN_ENABLED;
-	}
-
-	return i40e_phy_conf_link(hw, abilities, speed);
+	return i40e_phy_conf_link(hw, abilities, speed, true);
 }
 
 static int
@@ -2245,7 +2255,7 @@  i40e_dev_set_link_down(struct rte_eth_dev *dev)
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
 	abilities = I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
-	return i40e_phy_conf_link(hw, abilities, speed);
+	return i40e_phy_conf_link(hw, abilities, speed, false);
 }
 
 int