[dpdk-dev] [PATCH] net/ixgbe: fix link status

[dpdk-dev] [PATCH] net/ixgbe: fix link status

[Cui LunyuanX]

After ports reset, tx laser register will be reset. The link
status for 82599eb got from link status register was not correct.
Set tx laser disable after ports reset.

Fixes: 0408f47ba4d6 ("net/ixgbe: fix busy polling while fiber link update")
Cc: stable@dpdk.org

Signed-off-by: Lunyuan Cui <lunyuanx.cui@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 8c1caac18..5e516599c 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1298,6 +1298,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
 	/* enable support intr */
 	ixgbe_enable_intr(eth_dev);
 
+	ixgbe_dev_set_link_down(eth_dev);
+
 	/* initialize filter info */
 	memset(filter_info, 0,
 	       sizeof(struct ixgbe_filter_info));
@@ -4154,11 +4156,6 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
 		link_up = 0;
 
 	if (link_up == 0) {
-		if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber) {
-			intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
-			rte_eal_alarm_set(10,
-				ixgbe_dev_setup_link_alarm_handler, dev);
-		}
 		return rte_eth_linkstatus_set(dev, &link);
 	}
 
-- 
2.17.1

Re: [dpdk-dev] [PATCH] net/ixgbe: fix link status

[Ye Xiaolong]

On 11/13, Cui LunyuanX wrote:
>After ports reset, tx laser register will be reset. The link
>status for 82599eb got from link status register was not correct.
>Set tx laser disable after ports reset.
>
>Fixes: 0408f47ba4d6 ("net/ixgbe: fix busy polling while fiber link update")
>Cc: stable@dpdk.org
>
>Signed-off-by: Lunyuan Cui <lunyuanx.cui@intel.com>
>---
> drivers/net/ixgbe/ixgbe_ethdev.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
>index 8c1caac18..5e516599c 100644
>--- a/drivers/net/ixgbe/ixgbe_ethdev.c
>+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>@@ -1298,6 +1298,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
> 	/* enable support intr */
> 	ixgbe_enable_intr(eth_dev);
> 
>+	ixgbe_dev_set_link_down(eth_dev);
>+
> 	/* initialize filter info */
> 	memset(filter_info, 0,
> 	       sizeof(struct ixgbe_filter_info));
>@@ -4154,11 +4156,6 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
> 		link_up = 0;
> 
> 	if (link_up == 0) {
>-		if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber) {
>-			intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
>-			rte_eal_alarm_set(10,
>-				ixgbe_dev_setup_link_alarm_handler, dev);
>-		}

why do we need this change?

Thanks,
Xiaolong

> 		return rte_eth_linkstatus_set(dev, &link);
> 	}
> 
>-- 
>2.17.1
>

[dpdk-dev] [PATCH v2] net/ixgbe: fix link status

[Cui LunyuanX]

After ports reset, tx laser register will be reset. The link
status for 82599eb got from link status register was not correct.
Set tx laser disabled after ports reset.

ixgbe_dev_setup_link_alarm_handler() will set tx laser enabled
when show port information. The purpose of the function has already
implemented in ixgbe_dev_start(). There is no need to reuse it
in ixgbe_dev_link_update_share().

Fixes: 0408f47ba4d6 ("net/ixgbe: fix busy polling while fiber link update")
Cc: stable@dpdk.org

Signed-off-by: Cui LunyuanX <lunyuanx.cui@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 8c1caac18..5e516599c 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1298,6 +1298,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
 	/* enable support intr */
 	ixgbe_enable_intr(eth_dev);
 
+	ixgbe_dev_set_link_down(eth_dev);
+
 	/* initialize filter info */
 	memset(filter_info, 0,
 	       sizeof(struct ixgbe_filter_info));
@@ -4154,11 +4156,6 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
 		link_up = 0;
 
 	if (link_up == 0) {
-		if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber) {
-			intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
-			rte_eal_alarm_set(10,
-				ixgbe_dev_setup_link_alarm_handler, dev);
-		}
 		return rte_eth_linkstatus_set(dev, &link);
 	}
 
-- 
2.17.1

Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix link status

[Ilya Maximets]

On 13.11.2019 17:34, Cui LunyuanX wrote:
> After ports reset, tx laser register will be reset. The link
> status for 82599eb got from link status register was not correct.
> Set tx laser disabled after ports reset.
> 
> ixgbe_dev_setup_link_alarm_handler() will set tx laser enabled
> when show port information. The purpose of the function has already
> implemented in ixgbe_dev_start(). There is no need to reuse it
> in ixgbe_dev_link_update_share().

The reason why the alarm handler stays there is the one described
in following commit:

commit c12d22f65b132c56db7b4fdbfd5ddce27d1e9572
Author: Laurent Hardy <laurent.hardy@6wind.com>
Date:   Thu Apr 27 17:03:42 2017 +0200

    net/ixgbe: ensure link status is updated
    
    In case of fiber and link speed set to 1Gb at peer side (with autoneg
    or with defined speed), link status could be not properly updated at
    time cable is plugged-in.
    Indeed if cable was not plugged when device has been configured and
    started then link status will not be updated properly with new speed
    as no link setup will be triggered.
    
    To avoid this issue, IXGBE_FLAG_NEED_LINK_CONFIG is set to try a link
    setup each time link_update() is triggered and current link status is
    down. When cable is plugged-in, link setup will be performed via
    ixgbe_setup_link().
    
    Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
    Acked-by: Wei Dai <wei.dai@intel.com>

Does it fixed?
If not, you should not touch the alarm handler or implement a different
workaround.

Best regards, Ilya Maximets.

Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix link status

[Cui, LunyuanX]

Hi, Ilya Maximets

Before my patch, original treatment is as follows:
	esdp_reg = IXGBE_READ_REG(hw, IXGBE_ESDP);
	if ((esdp_reg & IXGBE_ESDP_SDP3))
		link_up = 0;

	if (link_up == 0) {
		if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber) {
			intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
			rte_eal_alarm_set(10,
				ixgbe_dev_setup_link_alarm_handler, dev);
		}
		return rte_eth_linkstatus_set(dev, &link);
	}

ixgbe_dev_setup_link_alarm_handler() can cause link status change from down to up.
When port stops, link status should always be down. When bug which you fixed occur, link status is down.
But this treatment can't confirm whether the reason is. I thank port status should be used as judgment.
If port stops, we don’t need to ensure link status is updated.

If I change patch like this, will it affect your bug?

Looking forward to your reply.
Thanks
Lunyuan.

> -----Original Message-----
> From: Ilya Maximets [mailto:i.maximets@ovn.org]
> Sent: Wednesday, November 13, 2019 11:07 PM
> To: Cui, LunyuanX <lunyuanx.cui@intel.com>; dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; stable@dpdk.org; Laurent Hardy
> <laurent.hardy@6wind.com>; Wei Dai <wei.dai@intel.com>; Ye, Xiaolong
> <xiaolong.ye@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix link status
> 
> On 13.11.2019 17:34, Cui LunyuanX wrote:
> > After ports reset, tx laser register will be reset. The link status
> > for 82599eb got from link status register was not correct.
> > Set tx laser disabled after ports reset.
> >
> > ixgbe_dev_setup_link_alarm_handler() will set tx laser enabled when
> > show port information. The purpose of the function has already
> > implemented in ixgbe_dev_start(). There is no need to reuse it in
> > ixgbe_dev_link_update_share().
> 
> The reason why the alarm handler stays there is the one described in
> following commit:
> 
> commit c12d22f65b132c56db7b4fdbfd5ddce27d1e9572
> Author: Laurent Hardy <laurent.hardy@6wind.com>
> Date:   Thu Apr 27 17:03:42 2017 +0200
> 
>     net/ixgbe: ensure link status is updated
> 
>     In case of fiber and link speed set to 1Gb at peer side (with autoneg
>     or with defined speed), link status could be not properly updated at
>     time cable is plugged-in.
>     Indeed if cable was not plugged when device has been configured and
>     started then link status will not be updated properly with new speed
>     as no link setup will be triggered.
> 
>     To avoid this issue, IXGBE_FLAG_NEED_LINK_CONFIG is set to try a link
>     setup each time link_update() is triggered and current link status is
>     down. When cable is plugged-in, link setup will be performed via
>     ixgbe_setup_link().
> 
>     Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
>     Acked-by: Wei Dai <wei.dai@intel.com>
> 
> Does it fixed?
> If not, you should not touch the alarm handler or implement a different
> workaround.
> 
> Best regards, Ilya Maximets.

Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix link status

[Ilya Maximets]

On 14.11.2019 4:45, Cui, LunyuanX wrote:
> Hi, Ilya Maximets
> 
> Before my patch, original treatment is as follows:
> 	esdp_reg = IXGBE_READ_REG(hw, IXGBE_ESDP);
> 	if ((esdp_reg & IXGBE_ESDP_SDP3))
> 		link_up = 0;
> 
> 	if (link_up == 0) {
> 		if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber) {
> 			intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
> 			rte_eal_alarm_set(10,
> 				ixgbe_dev_setup_link_alarm_handler, dev);
> 		}
> 		return rte_eth_linkstatus_set(dev, &link);
> 	}
> 
> ixgbe_dev_setup_link_alarm_handler() can cause link status change from down to up.
> When port stops, link status should always be down. When bug which you fixed occur, link status is down.
> But this treatment can't confirm whether the reason is. I thank port status should be used as judgment.
> If port stops, we don’t need to ensure link status is updated.
> 
> If I change patch like this, will it affect your bug?

At first, your 'Fixes' tag is not correct because patch 0408f47ba4d6
only changed direct ixgbe_setup_link() call to delayed alarm handler.

You may add some more conditions in order to not schedule the handler
if adapter is stopped, but don't remove the workaround completely.

Best regards, Ilya Maximets.

> 
> Looking forward to your reply.
> Thanks
> Lunyuan.
> 
>> -----Original Message-----
>> From: Ilya Maximets [mailto:i.maximets@ovn.org]
>> Sent: Wednesday, November 13, 2019 11:07 PM
>> To: Cui, LunyuanX <lunyuanx.cui@intel.com>; dev@dpdk.org
>> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Yang, Qiming
>> <qiming.yang@intel.com>; stable@dpdk.org; Laurent Hardy
>> <laurent.hardy@6wind.com>; Wei Dai <wei.dai@intel.com>; Ye, Xiaolong
>> <xiaolong.ye@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix link status
>>
>> On 13.11.2019 17:34, Cui LunyuanX wrote:
>>> After ports reset, tx laser register will be reset. The link status
>>> for 82599eb got from link status register was not correct.
>>> Set tx laser disabled after ports reset.
>>>
>>> ixgbe_dev_setup_link_alarm_handler() will set tx laser enabled when
>>> show port information. The purpose of the function has already
>>> implemented in ixgbe_dev_start(). There is no need to reuse it in
>>> ixgbe_dev_link_update_share().
>>
>> The reason why the alarm handler stays there is the one described in
>> following commit:
>>
>> commit c12d22f65b132c56db7b4fdbfd5ddce27d1e9572
>> Author: Laurent Hardy <laurent.hardy@6wind.com>
>> Date:   Thu Apr 27 17:03:42 2017 +0200
>>
>>     net/ixgbe: ensure link status is updated
>>
>>     In case of fiber and link speed set to 1Gb at peer side (with autoneg
>>     or with defined speed), link status could be not properly updated at
>>     time cable is plugged-in.
>>     Indeed if cable was not plugged when device has been configured and
>>     started then link status will not be updated properly with new speed
>>     as no link setup will be triggered.
>>
>>     To avoid this issue, IXGBE_FLAG_NEED_LINK_CONFIG is set to try a link
>>     setup each time link_update() is triggered and current link status is
>>     down. When cable is plugged-in, link setup will be performed via
>>     ixgbe_setup_link().
>>
>>     Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
>>     Acked-by: Wei Dai <wei.dai@intel.com>
>>
>> Does it fixed?
>> If not, you should not touch the alarm handler or implement a different
>> workaround.
>>
>> Best regards, Ilya Maximets.

[dpdk-dev] [PATCH v3] net/ixgbe: fix link status

[Lunyuan Cui]

After ports reset, tx laser register will be reset. The link
status for 82599eb got from link status register was not correct.
Set tx laser disable when port resets.

ixgbe_flap_tx_laser_multispeed_fiber() can cause link status
change from down to up. This treatment should work after
port starts.

Fixes: 0408f47ba4d6 ("net/ixgbe: fix busy polling while fiber link update")
Cc: stable@dpdk.org

Signed-off-by: Lunyuan Cui <lunyuanx.cui@intel.com>
---
v3:
* Correct countermeasure
	Don't delete ixgbe_dev_setup_link_alarm_handler().

v2:
* Change commit log
	Add a log why I delete ixgbe_dev_setup_link_alarm_handler().
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 8c1caac18..260484fbf 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1188,6 +1188,7 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
 	diag = ixgbe_bypass_init_hw(hw);
 #else
 	diag = ixgbe_init_hw(hw);
+	hw->mac.autotry_restart = false;
 #endif /* RTE_LIBRTE_IXGBE_BYPASS */
 
 	/*
@@ -1298,6 +1299,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
 	/* enable support intr */
 	ixgbe_enable_intr(eth_dev);
 
+	ixgbe_dev_set_link_down(eth_dev);
+
 	/* initialize filter info */
 	memset(filter_info, 0,
 	       sizeof(struct ixgbe_filter_info));
-- 
2.17.1

Re: [dpdk-dev] [PATCH v3] net/ixgbe: fix link status

[Ye Xiaolong]

On 11/18, Lunyuan Cui wrote:
>After ports reset, tx laser register will be reset. The link
>status for 82599eb got from link status register was not correct.
>Set tx laser disable when port resets.

Above message is unclear to me, what's the relation between tx laser register
and link status register? Better to describe what problem you've met, and how
your change fixes the issue?

>
>ixgbe_flap_tx_laser_multispeed_fiber() can cause link status
>change from down to up. This treatment should work after
>port starts.

Now the v3 doesn't touch ixgbe_flap_tx_laser_multispeed_fiber, you should
add explanation about the autotry_restart change you introduced.

Thanks,
Xiaolong

>
>Fixes: 0408f47ba4d6 ("net/ixgbe: fix busy polling while fiber link update")
>Cc: stable@dpdk.org
>
>Signed-off-by: Lunyuan Cui <lunyuanx.cui@intel.com>
>---
>v3:
>* Correct countermeasure
>	Don't delete ixgbe_dev_setup_link_alarm_handler().
>
>v2:
>* Change commit log
>	Add a log why I delete ixgbe_dev_setup_link_alarm_handler().
>---
> drivers/net/ixgbe/ixgbe_ethdev.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
>index 8c1caac18..260484fbf 100644
>--- a/drivers/net/ixgbe/ixgbe_ethdev.c
>+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>@@ -1188,6 +1188,7 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
> 	diag = ixgbe_bypass_init_hw(hw);
> #else
> 	diag = ixgbe_init_hw(hw);
>+	hw->mac.autotry_restart = false;
> #endif /* RTE_LIBRTE_IXGBE_BYPASS */
> 
> 	/*
>@@ -1298,6 +1299,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
> 	/* enable support intr */
> 	ixgbe_enable_intr(eth_dev);
> 
>+	ixgbe_dev_set_link_down(eth_dev);
>+
> 	/* initialize filter info */
> 	memset(filter_info, 0,
> 	       sizeof(struct ixgbe_filter_info));
>-- 
>2.17.1
>

[dpdk-dev] [PATCH v4] net/ixgbe: fix link status

[Lunyuan Cui]

The link status for 82599eb got from link status register was not
correct. Check the enable/disable flag of tx laser, set the link
status down if tx laser disabled. Then, we can get correct status.
But after port reset, tx laser register will be reset enable.
Link status will always be up. So set tx laser disable when port resets.

When hw->mac.autotry_restart is true, whether tx laser is disable or
enable, it will be set enable in ixgbe_flap_tx_laser_multispeed_fiber().
hw->mac.autotry_restart can be set true in both port init and port start.
Because we don't need this treatment before port starts, set
hw->mac.autotry_restart false when port init.

Fixes: 0408f47ba4d6 ("net/ixgbe: fix busy polling while fiber link update")
Cc: stable@dpdk.org

Signed-off-by: Lunyuan Cui <lunyuanx.cui@intel.com>
---
v4:
* modifier commit log
	Describe the problem in more detail.

v3:
* Correct countermeasure
	Don't delete ixgbe_dev_setup_link_alarm_handler().

v2:
* modifier commit log
	Add a log why I delete ixgbe_dev_setup_link_alarm_handler().
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 8c1caac18..260484fbf 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1188,6 +1188,7 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
 	diag = ixgbe_bypass_init_hw(hw);
 #else
 	diag = ixgbe_init_hw(hw);
+	hw->mac.autotry_restart = false;
 #endif /* RTE_LIBRTE_IXGBE_BYPASS */
 
 	/*
@@ -1298,6 +1299,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
 	/* enable support intr */
 	ixgbe_enable_intr(eth_dev);
 
+	ixgbe_dev_set_link_down(eth_dev);
+
 	/* initialize filter info */
 	memset(filter_info, 0,
 	       sizeof(struct ixgbe_filter_info));
-- 
2.17.1

Re: [dpdk-dev] [PATCH v4] net/ixgbe: fix link status

[Ye Xiaolong]

On 11/18, Lunyuan Cui wrote:
>The link status for 82599eb got from link status register was not
>correct. Check the enable/disable flag of tx laser, set the link
>status down if tx laser disabled. Then, we can get correct status.
>But after port reset, tx laser register will be reset enable.
>Link status will always be up. So set tx laser disable when port resets.

So you call ixgbe_dev_set_link_down to disable tx laser, but ixgbe_dev_set_link_down
is more than just disable tx laser, will it has some side effects?

>
>When hw->mac.autotry_restart is true, whether tx laser is disable or
>enable, it will be set enable in ixgbe_flap_tx_laser_multispeed_fiber().
>hw->mac.autotry_restart can be set true in both port init and port start.
>Because we don't need this treatment before port starts, set
>hw->mac.autotry_restart false when port init.
>
>Fixes: 0408f47ba4d6 ("net/ixgbe: fix busy polling while fiber link update")
>Cc: stable@dpdk.org
>
>Signed-off-by: Lunyuan Cui <lunyuanx.cui@intel.com>
>---
>v4:
>* modifier commit log
>	Describe the problem in more detail.
>
>v3:
>* Correct countermeasure
>	Don't delete ixgbe_dev_setup_link_alarm_handler().
>
>v2:
>* modifier commit log
>	Add a log why I delete ixgbe_dev_setup_link_alarm_handler().
>---
> drivers/net/ixgbe/ixgbe_ethdev.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
>index 8c1caac18..260484fbf 100644
>--- a/drivers/net/ixgbe/ixgbe_ethdev.c
>+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>@@ -1188,6 +1188,7 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
> 	diag = ixgbe_bypass_init_hw(hw);
> #else
> 	diag = ixgbe_init_hw(hw);
>+	hw->mac.autotry_restart = false;
> #endif /* RTE_LIBRTE_IXGBE_BYPASS */
> 
> 	/*
>@@ -1298,6 +1299,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
> 	/* enable support intr */
> 	ixgbe_enable_intr(eth_dev);
> 
>+	ixgbe_dev_set_link_down(eth_dev);
>+
> 	/* initialize filter info */
> 	memset(filter_info, 0,
> 	       sizeof(struct ixgbe_filter_info));
>-- 
>2.17.1
>

Re: [dpdk-dev] [PATCH v4] net/ixgbe: fix link status

[Cui, LunyuanX]

Hi, xiaolong

> -----Original Message-----
> From: Ye, Xiaolong
> Sent: Tuesday, November 19, 2019 2:28 PM
> To: Cui, LunyuanX <lunyuanx.cui@intel.com>
> Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4] net/ixgbe: fix link status
> 
> On 11/18, Lunyuan Cui wrote:
> >The link status for 82599eb got from link status register was not
> >correct. Check the enable/disable flag of tx laser, set the link status
> >down if tx laser disabled. Then, we can get correct status.
> >But after port reset, tx laser register will be reset enable.
> >Link status will always be up. So set tx laser disable when port resets.
> 
> So you call ixgbe_dev_set_link_down to disable tx laser, but
> ixgbe_dev_set_link_down is more than just disable tx laser, will it has some
> side effects?

There are not side effects.
I call ixgbe_dev_set_link_down when port init.
When port starts, It will deal with follows:
	if (hw->mac.ops.get_media_type(hw) == ixgbe_media_type_copper) {
		/* Turn on the copper */
		ixgbe_set_phy_power(hw, true);
	} else {
		/* Turn on the laser */
		ixgbe_enable_tx_laser(hw);
	}

So I think there are not side effects.

> 
> >
> >When hw->mac.autotry_restart is true, whether tx laser is disable or
> >enable, it will be set enable in ixgbe_flap_tx_laser_multispeed_fiber().
> >hw->mac.autotry_restart can be set true in both port init and port start.
> >Because we don't need this treatment before port starts, set
> >hw->mac.autotry_restart false when port init.
> >
> >Fixes: 0408f47ba4d6 ("net/ixgbe: fix busy polling while fiber link
> >update")
> >Cc: stable@dpdk.org
> >
> >Signed-off-by: Lunyuan Cui <lunyuanx.cui@intel.com>
> >---
> >v4:
> >* modifier commit log
> >	Describe the problem in more detail.
> >
> >v3:
> >* Correct countermeasure
> >	Don't delete ixgbe_dev_setup_link_alarm_handler().
> >
> >v2:
> >* modifier commit log
> >	Add a log why I delete ixgbe_dev_setup_link_alarm_handler().
> >---
> > drivers/net/ixgbe/ixgbe_ethdev.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> >diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> >b/drivers/net/ixgbe/ixgbe_ethdev.c
> >index 8c1caac18..260484fbf 100644
> >--- a/drivers/net/ixgbe/ixgbe_ethdev.c
> >+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> >@@ -1188,6 +1188,7 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev,
> void *init_params __rte_unused)
> > 	diag = ixgbe_bypass_init_hw(hw);
> > #else
> > 	diag = ixgbe_init_hw(hw);
> >+	hw->mac.autotry_restart = false;
> > #endif /* RTE_LIBRTE_IXGBE_BYPASS */
> >
> > 	/*
> >@@ -1298,6 +1299,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev,
> void *init_params __rte_unused)
> > 	/* enable support intr */
> > 	ixgbe_enable_intr(eth_dev);
> >
> >+	ixgbe_dev_set_link_down(eth_dev);
> >+
> > 	/* initialize filter info */
> > 	memset(filter_info, 0,
> > 	       sizeof(struct ixgbe_filter_info));
> >--
> >2.17.1
> >

Re: [dpdk-dev] [PATCH v4] net/ixgbe: fix link status

[Lu, Wenzhuo]

Hi,


> -----Original Message-----
> From: Cui, LunyuanX
> Sent: Monday, November 18, 2019 11:38 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Cui, LunyuanX
> <lunyuanx.cui@intel.com>; stable@dpdk.org
> Subject: [PATCH v4] net/ixgbe: fix link status
> 
> The link status for 82599eb got from link status register was not correct.
> Check the enable/disable flag of tx laser, set the link status down if tx laser
> disabled. Then, we can get correct status.
> But after port reset, tx laser register will be reset enable.
> Link status will always be up. So set tx laser disable when port resets.
> 
> When hw->mac.autotry_restart is true, whether tx laser is disable or enable,
> it will be set enable in ixgbe_flap_tx_laser_multispeed_fiber().
> hw->mac.autotry_restart can be set true in both port init and port start.
> Because we don't need this treatment before port starts, set
> hw->mac.autotry_restart false when port init.
> 
> Fixes: 0408f47ba4d6 ("net/ixgbe: fix busy polling while fiber link update")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Lunyuan Cui <lunyuanx.cui@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

Re: [dpdk-dev] [PATCH v4] net/ixgbe: fix link status

[Ye Xiaolong]

On 11/26, Lu, Wenzhuo wrote:
>Hi,
>
>
>> -----Original Message-----
>> From: Cui, LunyuanX
>> Sent: Monday, November 18, 2019 11:38 PM
>> To: dev@dpdk.org
>> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Cui, LunyuanX
>> <lunyuanx.cui@intel.com>; stable@dpdk.org
>> Subject: [PATCH v4] net/ixgbe: fix link status
>> 
>> The link status for 82599eb got from link status register was not correct.
>> Check the enable/disable flag of tx laser, set the link status down if tx laser
>> disabled. Then, we can get correct status.
>> But after port reset, tx laser register will be reset enable.
>> Link status will always be up. So set tx laser disable when port resets.
>> 
>> When hw->mac.autotry_restart is true, whether tx laser is disable or enable,
>> it will be set enable in ixgbe_flap_tx_laser_multispeed_fiber().
>> hw->mac.autotry_restart can be set true in both port init and port start.
>> Because we don't need this treatment before port starts, set
>> hw->mac.autotry_restart false when port init.
>> 
>> Fixes: 0408f47ba4d6 ("net/ixgbe: fix busy polling while fiber link update")
>> Cc: stable@dpdk.org
>> 
>> Signed-off-by: Lunyuan Cui <lunyuanx.cui@intel.com>
>Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

Applied to dpdk-next-net-intel, Thanks.