[dpdk-dev] [PATCH] i40e: fix link management

[dpdk-dev] [PATCH] i40e: fix link management

[Jingjing Wu]

Previously, there was a known issue "On Intel® 40G Ethernet
Controller stopping the port does not really down the port link."
There were two reasons why the port is always kept up.
1. Old version Firmware would cause issue when call "Set PHY config
command" on 40G NIC.
2. Because linux kernel i40e driver didn’t call "Set PHY config
command" when ifconfig up/down, and it assumes the link always up.
While ports are forced down when DPDK quit. So if the port is switched
to controlled by kernel driver, the port will not be up through
"ifconfig <ethx> up".

This patch fixes this issue by reopening "Set PHY config command"
because:
1. New firmware issue is already fixed.
2. After DPDK quit, "ethtool -s <ethx> autoneg on" can be used to
turn on the auto negotiation, and then port can be up through
"ifconfig <ethx> up" in new version kernel i40e driver( >1.4.X ).

Fixes: 2f1e22817420 ("i40e: skip link control as firmware workaround")
Fixes: 16c979f9adf2 ("i40e: disable setting of PHY configuration")
Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 doc/guides/rel_notes/known_issues.rst | 19 ---------
 drivers/net/i40e/i40e_ethdev.c        | 72 +++++++++++++++++++++++++++++------
 2 files changed, 60 insertions(+), 31 deletions(-)

diff --git a/doc/guides/rel_notes/known_issues.rst b/doc/guides/rel_notes/known_issues.rst
index 923a202..489bb92 100644
--- a/doc/guides/rel_notes/known_issues.rst
+++ b/doc/guides/rel_notes/known_issues.rst
@@ -532,25 +532,6 @@ Cannot set link speed on Intel® 40G Ethernet controller
    Poll Mode Driver (PMD).
 
 
-Stopping the port does not down the link on Intel® 40G Ethernet controller
---------------------------------------------------------------------------
-
-**Description**:
-   On Intel® 40G Ethernet Controller stopping the port does not really down the port link.
-
-**Implication**:
-   The port link will be still up after stopping the port.
-
-**Resolution/Workaround**:
-   None
-
-**Affected Environment/Platform**:
-   All.
-
-**Driver/Module**:
-   Poll Mode Driver (PMD).
-
-
 Devices bound to igb_uio with VT-d enabled do not work on Linux kernel 3.15-3.17
 --------------------------------------------------------------------------------
 
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 24777d5..4236d07 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1403,15 +1403,58 @@ i40e_parse_link_speeds(uint16_t link_speeds)
 }
 
 static int
-i40e_phy_conf_link(__rte_unused struct i40e_hw *hw,
-		   __rte_unused uint8_t abilities,
-		   __rte_unused uint8_t force_speed)
-{
-	/* Skip any phy config on both 10G and 40G interfaces, as a workaround
-	 * for the link control limitation of that all link control should be
-	 * handled by firmware. It should follow up if link control will be
-	 * opened to software driver in future firmware versions.
-	 */
+i40e_phy_conf_link(struct i40e_hw *hw,
+		   uint8_t abilities,
+		   uint8_t force_speed)
+{
+	enum i40e_status_code status;
+	struct i40e_aq_get_phy_abilities_resp phy_ab;
+	struct i40e_aq_set_phy_config phy_conf;
+	const uint8_t mask = I40E_AQ_PHY_FLAG_PAUSE_TX |
+			I40E_AQ_PHY_FLAG_PAUSE_RX |
+			I40E_AQ_PHY_FLAG_PAUSE_RX |
+			I40E_AQ_PHY_FLAG_LOW_POWER;
+	const uint8_t advt = I40E_LINK_SPEED_40GB |
+			I40E_LINK_SPEED_10GB |
+			I40E_LINK_SPEED_1GB |
+			I40E_LINK_SPEED_100MB;
+	int ret = -ENOTSUP;
+
+
+	status = i40e_aq_get_phy_capabilities(hw, false, false, &phy_ab,
+					      NULL);
+	if (status)
+		return ret;
+
+	memset(&phy_conf, 0, sizeof(phy_conf));
+
+	/* bits 0-2 use the values from get_phy_abilities_resp */
+	abilities &= ~mask;
+	abilities |= phy_ab.abilities & mask;
+
+	/* update ablities and speed */
+	if (abilities & I40E_AQ_PHY_AN_ENABLED)
+		phy_conf.link_speed = advt;
+	else
+		phy_conf.link_speed = force_speed;
+
+	phy_conf.abilities = abilities;
+
+	/* use get_phy_abilities_resp value for the rest */
+	phy_conf.phy_type = phy_ab.phy_type;
+	phy_conf.eee_capability = phy_ab.eee_capability;
+	phy_conf.eeer = phy_ab.eeer_val;
+	phy_conf.low_power_ctrl = phy_ab.d3_lpan;
+
+	PMD_DRV_LOG(DEBUG, "\tCurrent: abilities %x, link_speed %x",
+		    phy_ab.abilities, phy_ab.link_speed);
+	PMD_DRV_LOG(DEBUG, "\tConfig:  abilities %x, link_speed %x",
+		    phy_conf.abilities, phy_conf.link_speed);
+
+	status = i40e_aq_set_phy_config(hw, &phy_conf, NULL);
+	if (status)
+		return ret;
+
 	return I40E_SUCCESS;
 }
 
@@ -1427,8 +1470,13 @@ i40e_apply_link_speed(struct rte_eth_dev *dev)
 	abilities |= I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
 	if (!(conf->link_speeds & ETH_LINK_SPEED_FIXED))
 		abilities |= I40E_AQ_PHY_AN_ENABLED;
-	else
-		abilities |= I40E_AQ_PHY_LINK_ENABLED;
+	abilities |= I40E_AQ_PHY_LINK_ENABLED;
+
+	/* Skip changing speed on 40G interfaces, FW does not support */
+	if (i40e_is_40G_device(hw->device_id)) {
+		speed =  I40E_LINK_SPEED_UNKNOWN;
+		abilities |= I40E_AQ_PHY_AN_ENABLED;
+	}
 
 	return i40e_phy_conf_link(hw, abilities, speed);
 }
@@ -1738,7 +1786,7 @@ i40e_dev_set_link_up(struct rte_eth_dev *dev)
  * Set device link down.
  */
 static int
-i40e_dev_set_link_down(__rte_unused struct rte_eth_dev *dev)
+i40e_dev_set_link_down(struct rte_eth_dev *dev)
 {
 	uint8_t speed = I40E_LINK_SPEED_UNKNOWN;
 	uint8_t abilities = I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
-- 
2.4.0

Re: [dpdk-dev] [PATCH] i40e: fix link management

[Bruce Richardson]

On Thu, May 12, 2016 at 03:21:04PM +0800, Jingjing Wu wrote:
> Previously, there was a known issue "On Intel® 40G Ethernet
> Controller stopping the port does not really down the port link."
> There were two reasons why the port is always kept up.
> 1. Old version Firmware would cause issue when call "Set PHY config
> command" on 40G NIC.
> 2. Because linux kernel i40e driver didn’t call "Set PHY config
> command" when ifconfig up/down, and it assumes the link always up.
> While ports are forced down when DPDK quit. So if the port is switched
> to controlled by kernel driver, the port will not be up through
> "ifconfig <ethx> up".
> 
> This patch fixes this issue by reopening "Set PHY config command"
> because:
> 1. New firmware issue is already fixed.
> 2. After DPDK quit, "ethtool -s <ethx> autoneg on" can be used to
> turn on the auto negotiation, and then port can be up through
> "ifconfig <ethx> up" in new version kernel i40e driver( >1.4.X ).
> 
> Fixes: 2f1e22817420 ("i40e: skip link control as firmware workaround")
> Fixes: 16c979f9adf2 ("i40e: disable setting of PHY configuration")
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>

Patch applied to dpdk-next-net/rel_16_07, with some commit message cleanups
applied.

/Bruce

Re: [dpdk-dev] [PATCH] i40e: fix link management

[Peng, Yuan]

Tested-by: Peng Yuan <yuan.peng@intel.com>

- Test Commit: 4cc20da049e0614eee99aeb097e648dbfa9fc655
- OS/Kernel: Fedora 23/4.2.3
- GCC: gcc version 5.3.1 20151207 (Red Hat 5.3.1-2) (GCC)
- CPU: Intel(R) Xeon(R) CPU E5-2699 v3 @ 2.30GHz
- Total 2 cases, 2 passed, 0 failed.

Case 1 
After "set link-down port 0", the expection is" Link status: down" 

DUT: 
./dpdk_nic_bind.py -b igb_uio 8a:00.1 
./testpmd -c 0x6 -n 4  -- -i --portmask=0x1  --port-topology=loop --txqflags=0 
testpmd>set fwd mac 
testpmd>start 
testpmd>set link-down port 0 
testpmd>show port info 0 
it shows"link status: down" 

TESTER: 
ethtool enp138s0f0 
it shows"Link detected: no"

case 2
after "port stop all", it will display " Link status: down"

DUT: 
./dpdk_nic_bind.py -b igb_uio 8a:00.1 
./testpmd -c 0x6 -n 4  -- -i --portmask=0x1  --port-topology=loop --txqflags=0 
testpmd>set fwd mac 
testpmd>start 
testpmd>stop 
testpmd>port stop all 
testpmd>show port info 0 
it shows"link status: down" 

TESTER: 
ethtool enp138s0f0 
it shows"Link detected: no"

The link management runs normally.

Thanks
Yuan.




-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jingjing Wu
Sent: Thursday, May 12, 2016 3:21 PM
To: Zhang, Helin <helin.zhang@intel.com>
Cc: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; Pei, Yulong <yulong.pei@intel.com>; Chilikin, Andrey <andrey.chilikin@intel.com>
Subject: [dpdk-dev] [PATCH] i40e: fix link management

Previously, there was a known issue "On Intel® 40G Ethernet Controller stopping the port does not really down the port link."
There were two reasons why the port is always kept up.
1. Old version Firmware would cause issue when call "Set PHY config command" on 40G NIC.
2. Because linux kernel i40e driver didn’t call "Set PHY config command" when ifconfig up/down, and it assumes the link always up.
While ports are forced down when DPDK quit. So if the port is switched to controlled by kernel driver, the port will not be up through "ifconfig <ethx> up".

This patch fixes this issue by reopening "Set PHY config command"
because:
1. New firmware issue is already fixed.
2. After DPDK quit, "ethtool -s <ethx> autoneg on" can be used to turn on the auto negotiation, and then port can be up through "ifconfig <ethx> up" in new version kernel i40e driver( >1.4.X ).

Fixes: 2f1e22817420 ("i40e: skip link control as firmware workaround")
Fixes: 16c979f9adf2 ("i40e: disable setting of PHY configuration")
Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 doc/guides/rel_notes/known_issues.rst | 19 ---------
 drivers/net/i40e/i40e_ethdev.c        | 72 +++++++++++++++++++++++++++++------
 2 files changed, 60 insertions(+), 31 deletions(-)

diff --git a/doc/guides/rel_notes/known_issues.rst b/doc/guides/rel_notes/known_issues.rst
index 923a202..489bb92 100644
--- a/doc/guides/rel_notes/known_issues.rst
+++ b/doc/guides/rel_notes/known_issues.rst
@@ -532,25 +532,6 @@ Cannot set link speed on Intel® 40G Ethernet controller
    Poll Mode Driver (PMD).
 
 
-Stopping the port does not down the link on Intel® 40G Ethernet controller
---------------------------------------------------------------------------
-
-**Description**:
-   On Intel® 40G Ethernet Controller stopping the port does not really down the port link.
-
-**Implication**:
-   The port link will be still up after stopping the port.
-
-**Resolution/Workaround**:
-   None
-
-**Affected Environment/Platform**:
-   All.
-
-**Driver/Module**:
-   Poll Mode Driver (PMD).
-
-
 Devices bound to igb_uio with VT-d enabled do not work on Linux kernel 3.15-3.17
 --------------------------------------------------------------------------------
 
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c index 24777d5..4236d07 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1403,15 +1403,58 @@ i40e_parse_link_speeds(uint16_t link_speeds)  }
 
 static int
-i40e_phy_conf_link(__rte_unused struct i40e_hw *hw,
-		   __rte_unused uint8_t abilities,
-		   __rte_unused uint8_t force_speed)
-{
-	/* Skip any phy config on both 10G and 40G interfaces, as a workaround
-	 * for the link control limitation of that all link control should be
-	 * handled by firmware. It should follow up if link control will be
-	 * opened to software driver in future firmware versions.
-	 */
+i40e_phy_conf_link(struct i40e_hw *hw,
+		   uint8_t abilities,
+		   uint8_t force_speed)
+{
+	enum i40e_status_code status;
+	struct i40e_aq_get_phy_abilities_resp phy_ab;
+	struct i40e_aq_set_phy_config phy_conf;
+	const uint8_t mask = I40E_AQ_PHY_FLAG_PAUSE_TX |
+			I40E_AQ_PHY_FLAG_PAUSE_RX |
+			I40E_AQ_PHY_FLAG_PAUSE_RX |
+			I40E_AQ_PHY_FLAG_LOW_POWER;
+	const uint8_t advt = I40E_LINK_SPEED_40GB |
+			I40E_LINK_SPEED_10GB |
+			I40E_LINK_SPEED_1GB |
+			I40E_LINK_SPEED_100MB;
+	int ret = -ENOTSUP;
+
+
+	status = i40e_aq_get_phy_capabilities(hw, false, false, &phy_ab,
+					      NULL);
+	if (status)
+		return ret;
+
+	memset(&phy_conf, 0, sizeof(phy_conf));
+
+	/* bits 0-2 use the values from get_phy_abilities_resp */
+	abilities &= ~mask;
+	abilities |= phy_ab.abilities & mask;
+
+	/* update ablities and speed */
+	if (abilities & I40E_AQ_PHY_AN_ENABLED)
+		phy_conf.link_speed = advt;
+	else
+		phy_conf.link_speed = force_speed;
+
+	phy_conf.abilities = abilities;
+
+	/* use get_phy_abilities_resp value for the rest */
+	phy_conf.phy_type = phy_ab.phy_type;
+	phy_conf.eee_capability = phy_ab.eee_capability;
+	phy_conf.eeer = phy_ab.eeer_val;
+	phy_conf.low_power_ctrl = phy_ab.d3_lpan;
+
+	PMD_DRV_LOG(DEBUG, "\tCurrent: abilities %x, link_speed %x",
+		    phy_ab.abilities, phy_ab.link_speed);
+	PMD_DRV_LOG(DEBUG, "\tConfig:  abilities %x, link_speed %x",
+		    phy_conf.abilities, phy_conf.link_speed);
+
+	status = i40e_aq_set_phy_config(hw, &phy_conf, NULL);
+	if (status)
+		return ret;
+
 	return I40E_SUCCESS;
 }
 
@@ -1427,8 +1470,13 @@ i40e_apply_link_speed(struct rte_eth_dev *dev)
 	abilities |= I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
 	if (!(conf->link_speeds & ETH_LINK_SPEED_FIXED))
 		abilities |= I40E_AQ_PHY_AN_ENABLED;
-	else
-		abilities |= I40E_AQ_PHY_LINK_ENABLED;
+	abilities |= I40E_AQ_PHY_LINK_ENABLED;
+
+	/* Skip changing speed on 40G interfaces, FW does not support */
+	if (i40e_is_40G_device(hw->device_id)) {
+		speed =  I40E_LINK_SPEED_UNKNOWN;
+		abilities |= I40E_AQ_PHY_AN_ENABLED;
+	}
 
 	return i40e_phy_conf_link(hw, abilities, speed);  } @@ -1738,7 +1786,7 @@ i40e_dev_set_link_up(struct rte_eth_dev *dev)
  * Set device link down.
  */
 static int
-i40e_dev_set_link_down(__rte_unused struct rte_eth_dev *dev)
+i40e_dev_set_link_down(struct rte_eth_dev *dev)
 {
 	uint8_t speed = I40E_LINK_SPEED_UNKNOWN;
 	uint8_t abilities = I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
--
2.4.0