DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/i40e: fix link up failure issue
@ 2018-05-16  6:28 Jeff Guo
  2018-05-16 14:43 ` Zhang, Qi Z
  2018-05-18  2:05 ` Jeff Guo
  0 siblings, 2 replies; 7+ messages in thread
From: Jeff Guo @ 2018-05-16  6:28 UTC (permalink / raw)
  To: beilei.xing, qi.z.zhang; +Cc: dev, jia.guo

If don't enable auto negotiation when set PHY to be link down, will cause
PHY can’t be Link up again in some case, this patch aim to fix this
issue.  When rebind to kernel driver, it can use ethtool to set auto
negotiation and use ifconfig tool to link up the device. And for the
case of PHY config synchronous compatibility with kernel driver issue,
that is known issue and work around it by don't config PHY when stop
device.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 48 ++++++++++++++++++++++++------------------
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 014bfce..accfc05 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2039,16 +2039,11 @@ i40e_phy_conf_link(struct i40e_hw *hw,
 			I40E_LINK_SPEED_100MB;
 	int ret = -ENOTSUP;
 
-
 	status = i40e_aq_get_phy_capabilities(hw, false, false, &phy_ab,
 					      NULL);
 	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 */
@@ -2056,25 +2051,27 @@ i40e_phy_conf_link(struct i40e_hw *hw,
 	abilities |= phy_ab.abilities & mask;
 
 	/* update ablities and speed */
-	if (abilities & I40E_AQ_PHY_AN_ENABLED)
+	if (abilities & I40E_AQ_PHY_AN_ENABLED && is_up) {
+		if (phy_ab.link_speed != I40E_LINK_SPEED_UNKNOWN)
+			return I40E_SUCCESS;
 		phy_conf.link_speed = advt;
-	else
-		phy_conf.link_speed = is_up ? force_speed : phy_ab.link_speed;
-
+	} else {
+		phy_conf.link_speed = force_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 = is_up ? cpu_to_le32(phy_type_mask) : 0;
-	phy_conf.phy_type_ext = is_up ? (I40E_AQ_PHY_TYPE_EXT_25G_KR |
+	phy_conf.phy_type = cpu_to_le32(phy_type_mask);
+	phy_conf.phy_type_ext = 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;
+		I40E_AQ_PHY_TYPE_EXT_25G_LR | I40E_AQ_PHY_TYPE_EXT_25G_AOC |
+		I40E_AQ_PHY_TYPE_EXT_25G_ACC;
+	phy_conf.fec_config = phy_ab.fec_cfg_curr_mod_ext_info |
+		I40E_AQ_PHY_FEC_CONFIG_MASK;
 	phy_conf.eee_capability = phy_ab.eee_capability;
 	phy_conf.eeer = phy_ab.eeer_val;
 	phy_conf.low_power_ctrl = phy_ab.d3_lpan;
@@ -2088,6 +2085,20 @@ i40e_phy_conf_link(struct i40e_hw *hw,
 	if (status)
 		return ret;
 
+	/* Update the link info */
+	status = i40e_update_link_info(hw);
+	if (status) {
+		/**
+		 * Wait a little bit (on 40G cards it sometimes takes a really
+		 * long time for link to come back from the atomic reset)
+		 * and try once more.
+		 */
+		msleep(1000);
+		status = i40e_update_link_info(hw);
+	}
+	if (status)
+		return ret;
+
 	return I40E_SUCCESS;
 }
 
@@ -2103,7 +2114,6 @@ 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;
-	abilities |= I40E_AQ_PHY_LINK_ENABLED;
 
 	return i40e_phy_conf_link(hw, abilities, speed, true);
 }
@@ -2306,9 +2316,6 @@ i40e_dev_stop(struct rte_eth_dev *dev)
 	/* Clear all queues and release memory */
 	i40e_dev_clear_queues(dev);
 
-	/* Set link down */
-	i40e_dev_set_link_down(dev);
-
 	if (!rte_intr_allow_others(intr_handle))
 		/* resume to the default handler */
 		rte_intr_callback_register(intr_handle,
@@ -2516,7 +2523,8 @@ i40e_dev_set_link_down(struct rte_eth_dev *dev)
 	uint8_t abilities = 0;
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-	abilities = I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
+	abilities = I40E_AQ_PHY_ENABLE_AN;
+	abilities |= I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
 	return i40e_phy_conf_link(hw, abilities, speed, false);
 }
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] net/i40e: fix link up failure issue
  2018-05-16  6:28 [dpdk-dev] [PATCH] net/i40e: fix link up failure issue Jeff Guo
@ 2018-05-16 14:43 ` Zhang, Qi Z
  2018-05-17  6:07   ` Guo, Jia
  2018-05-18  2:05 ` Jeff Guo
  1 sibling, 1 reply; 7+ messages in thread
From: Zhang, Qi Z @ 2018-05-16 14:43 UTC (permalink / raw)
  To: Guo, Jia, Xing, Beilei; +Cc: dev

Hi Jeff:

> -----Original Message-----
> From: Guo, Jia
> Sent: Wednesday, May 16, 2018 2:29 PM
> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Guo, Jia <jia.guo@intel.com>
> Subject: [PATCH] net/i40e: fix link up failure issue
> 
> If don't enable auto negotiation when set PHY to be link down, will cause PHY
> can’t be Link up again in some case, this patch aim to fix this issue.  

OK, understand this is an issue, I guess you fixed this in i40e_dev_set_link_down?

>When
> rebind to kernel driver, it can use ethtool to set auto negotiation and use
> ifconfig tool to link up the device. 
>And for the case of PHY config synchronous
> compatibility with kernel driver issue, 

Is this another issue? If it is , could you separate into two patches?

> that is known issue and work around it
> by don't config PHY when stop device.

Seems you did more than just don't "config PHY when stop device"
Could you comment more about changes in i40e_phy_conf_link

And there is no fixed line you may need add.

Regards
Qi

> 
> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 48
> ++++++++++++++++++++++++------------------
>  1 file changed, 28 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 014bfce..accfc05 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2039,16 +2039,11 @@ i40e_phy_conf_link(struct i40e_hw *hw,
>  			I40E_LINK_SPEED_100MB;
>  	int ret = -ENOTSUP;
> 
> -
>  	status = i40e_aq_get_phy_capabilities(hw, false, false, &phy_ab,
>  					      NULL);
>  	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 */ @@ -2056,25
> +2051,27 @@ i40e_phy_conf_link(struct i40e_hw *hw,
>  	abilities |= phy_ab.abilities & mask;
> 
>  	/* update ablities and speed */
> -	if (abilities & I40E_AQ_PHY_AN_ENABLED)
> +	if (abilities & I40E_AQ_PHY_AN_ENABLED && is_up) {
> +		if (phy_ab.link_speed != I40E_LINK_SPEED_UNKNOWN)
> +			return I40E_SUCCESS;
>  		phy_conf.link_speed = advt;
> -	else
> -		phy_conf.link_speed = is_up ? force_speed : phy_ab.link_speed;
> -
> +	} else {
> +		phy_conf.link_speed = force_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 = is_up ? cpu_to_le32(phy_type_mask) : 0;
> -	phy_conf.phy_type_ext = is_up ? (I40E_AQ_PHY_TYPE_EXT_25G_KR |
> +	phy_conf.phy_type = cpu_to_le32(phy_type_mask);
> +	phy_conf.phy_type_ext = 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;
> +		I40E_AQ_PHY_TYPE_EXT_25G_LR |
> I40E_AQ_PHY_TYPE_EXT_25G_AOC |
> +		I40E_AQ_PHY_TYPE_EXT_25G_ACC;
> +	phy_conf.fec_config = phy_ab.fec_cfg_curr_mod_ext_info |
> +		I40E_AQ_PHY_FEC_CONFIG_MASK;
>  	phy_conf.eee_capability = phy_ab.eee_capability;
>  	phy_conf.eeer = phy_ab.eeer_val;
>  	phy_conf.low_power_ctrl = phy_ab.d3_lpan; @@ -2088,6 +2085,20 @@
> i40e_phy_conf_link(struct i40e_hw *hw,
>  	if (status)
>  		return ret;
> 
> +	/* Update the link info */
> +	status = i40e_update_link_info(hw);
> +	if (status) {
> +		/**
> +		 * Wait a little bit (on 40G cards it sometimes takes a really
> +		 * long time for link to come back from the atomic reset)
> +		 * and try once more.
> +		 */
> +		msleep(1000);
> +		status = i40e_update_link_info(hw);
> +	}
> +	if (status)
> +		return ret;
> +
>  	return I40E_SUCCESS;
>  }
> 
> @@ -2103,7 +2114,6 @@ 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;
> -	abilities |= I40E_AQ_PHY_LINK_ENABLED;
> 
>  	return i40e_phy_conf_link(hw, abilities, speed, true);  } @@ -2306,9
> +2316,6 @@ i40e_dev_stop(struct rte_eth_dev *dev)
>  	/* Clear all queues and release memory */
>  	i40e_dev_clear_queues(dev);
> 
> -	/* Set link down */
> -	i40e_dev_set_link_down(dev);
> -
>  	if (!rte_intr_allow_others(intr_handle))
>  		/* resume to the default handler */
>  		rte_intr_callback_register(intr_handle,
> @@ -2516,7 +2523,8 @@ i40e_dev_set_link_down(struct rte_eth_dev *dev)
>  	uint8_t abilities = 0;
>  	struct i40e_hw *hw =
> I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> 
> -	abilities = I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
> +	abilities = I40E_AQ_PHY_ENABLE_AN;
> +	abilities |= I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
>  	return i40e_phy_conf_link(hw, abilities, speed, false);  }
> 
> --
> 2.7.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] net/i40e: fix link up failure issue
  2018-05-16 14:43 ` Zhang, Qi Z
@ 2018-05-17  6:07   ` Guo, Jia
  0 siblings, 0 replies; 7+ messages in thread
From: Guo, Jia @ 2018-05-17  6:07 UTC (permalink / raw)
  To: Zhang, Qi Z, Xing, Beilei; +Cc: dev

make sense , will split the patch more explicit. thanks ,qi.


On 5/16/2018 10:43 PM, Zhang, Qi Z wrote:
> Hi Jeff:
>
>> -----Original Message-----
>> From: Guo, Jia
>> Sent: Wednesday, May 16, 2018 2:29 PM
>> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
>> Cc: dev@dpdk.org; Guo, Jia <jia.guo@intel.com>
>> Subject: [PATCH] net/i40e: fix link up failure issue
>>
>> If don't enable auto negotiation when set PHY to be link down, will cause PHY
>> can’t be Link up again in some case, this patch aim to fix this issue.
> OK, understand this is an issue, I guess you fixed this in i40e_dev_set_link_down?
>
>> When
>> rebind to kernel driver, it can use ethtool to set auto negotiation and use
>> ifconfig tool to link up the device.
>> And for the case of PHY config synchronous
>> compatibility with kernel driver issue,
> Is this another issue? If it is , could you separate into two patches?
>
>> that is known issue and work around it
>> by don't config PHY when stop device.
> Seems you did more than just don't "config PHY when stop device"
> Could you comment more about changes in i40e_phy_conf_link
>
> And there is no fixed line you may need add.
>
> Regards
> Qi
>
>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>> ---
>>   drivers/net/i40e/i40e_ethdev.c | 48
>> ++++++++++++++++++++++++------------------
>>   1 file changed, 28 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
>> index 014bfce..accfc05 100644
>> --- a/drivers/net/i40e/i40e_ethdev.c
>> +++ b/drivers/net/i40e/i40e_ethdev.c
>> @@ -2039,16 +2039,11 @@ i40e_phy_conf_link(struct i40e_hw *hw,
>>   			I40E_LINK_SPEED_100MB;
>>   	int ret = -ENOTSUP;
>>
>> -
>>   	status = i40e_aq_get_phy_capabilities(hw, false, false, &phy_ab,
>>   					      NULL);
>>   	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 */ @@ -2056,25
>> +2051,27 @@ i40e_phy_conf_link(struct i40e_hw *hw,
>>   	abilities |= phy_ab.abilities & mask;
>>
>>   	/* update ablities and speed */
>> -	if (abilities & I40E_AQ_PHY_AN_ENABLED)
>> +	if (abilities & I40E_AQ_PHY_AN_ENABLED && is_up) {
>> +		if (phy_ab.link_speed != I40E_LINK_SPEED_UNKNOWN)
>> +			return I40E_SUCCESS;
>>   		phy_conf.link_speed = advt;
>> -	else
>> -		phy_conf.link_speed = is_up ? force_speed : phy_ab.link_speed;
>> -
>> +	} else {
>> +		phy_conf.link_speed = force_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 = is_up ? cpu_to_le32(phy_type_mask) : 0;
>> -	phy_conf.phy_type_ext = is_up ? (I40E_AQ_PHY_TYPE_EXT_25G_KR |
>> +	phy_conf.phy_type = cpu_to_le32(phy_type_mask);
>> +	phy_conf.phy_type_ext = 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;
>> +		I40E_AQ_PHY_TYPE_EXT_25G_LR |
>> I40E_AQ_PHY_TYPE_EXT_25G_AOC |
>> +		I40E_AQ_PHY_TYPE_EXT_25G_ACC;
>> +	phy_conf.fec_config = phy_ab.fec_cfg_curr_mod_ext_info |
>> +		I40E_AQ_PHY_FEC_CONFIG_MASK;
>>   	phy_conf.eee_capability = phy_ab.eee_capability;
>>   	phy_conf.eeer = phy_ab.eeer_val;
>>   	phy_conf.low_power_ctrl = phy_ab.d3_lpan; @@ -2088,6 +2085,20 @@
>> i40e_phy_conf_link(struct i40e_hw *hw,
>>   	if (status)
>>   		return ret;
>>
>> +	/* Update the link info */
>> +	status = i40e_update_link_info(hw);
>> +	if (status) {
>> +		/**
>> +		 * Wait a little bit (on 40G cards it sometimes takes a really
>> +		 * long time for link to come back from the atomic reset)
>> +		 * and try once more.
>> +		 */
>> +		msleep(1000);
>> +		status = i40e_update_link_info(hw);
>> +	}
>> +	if (status)
>> +		return ret;
>> +
>>   	return I40E_SUCCESS;
>>   }
>>
>> @@ -2103,7 +2114,6 @@ 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;
>> -	abilities |= I40E_AQ_PHY_LINK_ENABLED;
>>
>>   	return i40e_phy_conf_link(hw, abilities, speed, true);  } @@ -2306,9
>> +2316,6 @@ i40e_dev_stop(struct rte_eth_dev *dev)
>>   	/* Clear all queues and release memory */
>>   	i40e_dev_clear_queues(dev);
>>
>> -	/* Set link down */
>> -	i40e_dev_set_link_down(dev);
>> -
>>   	if (!rte_intr_allow_others(intr_handle))
>>   		/* resume to the default handler */
>>   		rte_intr_callback_register(intr_handle,
>> @@ -2516,7 +2523,8 @@ i40e_dev_set_link_down(struct rte_eth_dev *dev)
>>   	uint8_t abilities = 0;
>>   	struct i40e_hw *hw =
>> I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>>
>> -	abilities = I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
>> +	abilities = I40E_AQ_PHY_ENABLE_AN;
>> +	abilities |= I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
>>   	return i40e_phy_conf_link(hw, abilities, speed, false);  }
>>
>> --
>> 2.7.4

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [dpdk-dev] [PATCH] net/i40e: fix link up failure issue
  2018-05-16  6:28 [dpdk-dev] [PATCH] net/i40e: fix link up failure issue Jeff Guo
  2018-05-16 14:43 ` Zhang, Qi Z
@ 2018-05-18  2:05 ` Jeff Guo
  2018-05-18  2:56   ` Zhang, Qi Z
  2018-05-18 13:16   ` Zhang, Qi Z
  1 sibling, 2 replies; 7+ messages in thread
From: Jeff Guo @ 2018-05-18  2:05 UTC (permalink / raw)
  To: qi.z.zhang, beilei.xing; +Cc: dev, jia.guo

In case of the known issue, that DPDK PHY config can't synchronous with
kernel driver and firmware, this will result of the nic can't link up
after rebind to the kernel driver. This patch propose to work around it
by don't config PHY and don't set link down when stop device.

Fixes: 6e145fcc754b ("i40e: support autoneg or force link speed")

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
v2->v1:
delete irrelevent part of code
---
 drivers/net/i40e/i40e_ethdev.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 014bfce..27a5722 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2306,9 +2306,6 @@ i40e_dev_stop(struct rte_eth_dev *dev)
 	/* Clear all queues and release memory */
 	i40e_dev_clear_queues(dev);
 
-	/* Set link down */
-	i40e_dev_set_link_down(dev);
-
 	if (!rte_intr_allow_others(intr_handle))
 		/* resume to the default handler */
 		rte_intr_callback_register(intr_handle,
-- 
2.7.4

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] net/i40e: fix link up failure issue
  2018-05-18  2:05 ` Jeff Guo
@ 2018-05-18  2:56   ` Zhang, Qi Z
  2018-05-18 13:16   ` Zhang, Qi Z
  1 sibling, 0 replies; 7+ messages in thread
From: Zhang, Qi Z @ 2018-05-18  2:56 UTC (permalink / raw)
  To: Guo, Jia, Xing, Beilei; +Cc: dev

> -----Original Message-----
> From: Guo, Jia
> Sent: Friday, May 18, 2018 10:05 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Guo, Jia <jia.guo@intel.com>
> Subject: [PATCH] net/i40e: fix link up failure issue
> 
> In case of the known issue, that DPDK PHY config can't synchronous with
> kernel driver and firmware, this will result of the nic can't link up after rebind
> to the kernel driver. This patch propose to work around it by don't config PHY
> and don't set link down when stop device.
> 
> Fixes: 6e145fcc754b ("i40e: support autoneg or force link speed")
> 
> Signed-off-by: Jeff Guo <jia.guo@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] net/i40e: fix link up failure issue
  2018-05-18  2:05 ` Jeff Guo
  2018-05-18  2:56   ` Zhang, Qi Z
@ 2018-05-18 13:16   ` Zhang, Qi Z
  1 sibling, 0 replies; 7+ messages in thread
From: Zhang, Qi Z @ 2018-05-18 13:16 UTC (permalink / raw)
  To: Guo, Jia, Xing, Beilei; +Cc: dev



> -----Original Message-----
> From: Zhang, Qi Z
> Sent: Friday, May 18, 2018 10:57 AM
> To: Guo, Jia <jia.guo@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH] net/i40e: fix link up failure issue
> 
> > -----Original Message-----
> > From: Guo, Jia
> > Sent: Friday, May 18, 2018 10:05 AM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>
> > Cc: dev@dpdk.org; Guo, Jia <jia.guo@intel.com>
> > Subject: [PATCH] net/i40e: fix link up failure issue
> >
> > In case of the known issue, that DPDK PHY config can't synchronous
> > with kernel driver and firmware, this will result of the nic can't
> > link up after rebind to the kernel driver. This patch propose to work
> > around it by don't config PHY and don't set link down when stop device.
> >
> > Fixes: 6e145fcc754b ("i40e: support autoneg or force link speed")
> >
> > Signed-off-by: Jeff Guo <jia.guo@intel.com>
> 
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Author agree to withdraw the patch, since kernel fix is in plan also this may impact link down experience, so

Nacked-by: Qi Zhang <qi.z.zhang@intel.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [dpdk-dev] [PATCH] net/i40e: fix link up failure issue
@ 2018-05-18  2:02 Jeff Guo
  0 siblings, 0 replies; 7+ messages in thread
From: Jeff Guo @ 2018-05-18  2:02 UTC (permalink / raw)
  To: qi.z.zhang, beilei.xing; +Cc: dev, jia.guo

In case of the known issue, that DPDK PHY config can't synchronous with
kernel driver and firmware, this will result of the nic can't link up
after rebind to the kernel driver. This patch propose to work around it
by don't config PHY and don't set link down when stop device.

Fixes: 6e145fcc754b ("i40e: support autoneg or force link speed")

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
v2->v1:
delete irrelevent part of code
---
 drivers/net/i40e/i40e_ethdev.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 014bfce..27a5722 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2306,9 +2306,6 @@ i40e_dev_stop(struct rte_eth_dev *dev)
 	/* Clear all queues and release memory */
 	i40e_dev_clear_queues(dev);
 
-	/* Set link down */
-	i40e_dev_set_link_down(dev);
-
 	if (!rte_intr_allow_others(intr_handle))
 		/* resume to the default handler */
 		rte_intr_callback_register(intr_handle,
-- 
2.7.4

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-05-18 13:16 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-16  6:28 [dpdk-dev] [PATCH] net/i40e: fix link up failure issue Jeff Guo
2018-05-16 14:43 ` Zhang, Qi Z
2018-05-17  6:07   ` Guo, Jia
2018-05-18  2:05 ` Jeff Guo
2018-05-18  2:56   ` Zhang, Qi Z
2018-05-18 13:16   ` Zhang, Qi Z
2018-05-18  2:02 Jeff Guo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).