DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/ixgbe: fix link status after port reset
@ 2020-04-13  1:38 Shougang Wang
  2020-04-13  2:18 ` Yang, Qiming
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Shougang Wang @ 2020-04-13  1:38 UTC (permalink / raw)
  To: dev; +Cc: Wenzhuo Lu, Qiming Yang, Shougang Wang, stable

It's a normal behavior to change the link status to up after
resetting the port. So it is unnecessary to set link down before
starting port, and changing the link state(link up/down) frequently
will cause link speed unstable.

Fixes: c3f2fbff78cf ("net/ixgbe: fix link status")
Cc: stable@dpdk.org

Signed-off-by: Shougang Wang <shougangx.wang@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 23b3f5b0c..206358b85 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1196,7 +1196,6 @@ 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 */
 
 	/*
@@ -1307,8 +1306,6 @@ 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


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

* Re: [dpdk-dev] [PATCH] net/ixgbe: fix link status after port reset
  2020-04-13  1:38 [dpdk-dev] [PATCH] net/ixgbe: fix link status after port reset Shougang Wang
@ 2020-04-13  2:18 ` Yang, Qiming
       [not found] ` <1027ae0f2b914553a602ff46967c8a98@intel.com>
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Yang, Qiming @ 2020-04-13  2:18 UTC (permalink / raw)
  To: Wang, ShougangX, dev; +Cc: Lu, Wenzhuo, stable



> -----Original Message-----
> From: Wang, ShougangX <shougangx.wang@intel.com>
> Sent: Monday, April 13, 2020 09:39
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Wang, ShougangX <shougangx.wang@intel.com>;
> stable@dpdk.org
> Subject: [PATCH] net/ixgbe: fix link status after port reset
> 
> It's a normal behavior to change the link status to up after resetting the port.
> So it is unnecessary to set link down before starting port, and changing the
> link state(link up/down) frequently will cause link speed unstable.
> 
> Fixes: c3f2fbff78cf ("net/ixgbe: fix link status")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Shougang Wang <shougangx.wang@intel.com>
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 23b3f5b0c..206358b85 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -1196,7 +1196,6 @@ 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 */
> 
>  	/*
> @@ -1307,8 +1306,6 @@ 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

Acked-by: Qiming Yang <qiming.yang@intel.com>

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

* Re: [dpdk-dev] [PATCH] net/ixgbe: fix link status after port reset
       [not found] ` <1027ae0f2b914553a602ff46967c8a98@intel.com>
@ 2020-04-14  6:35   ` Zhang, XuemingX
  0 siblings, 0 replies; 6+ messages in thread
From: Zhang, XuemingX @ 2020-04-14  6:35 UTC (permalink / raw)
  To: Wang, ShougangX, dev
  Cc: Lu, Wenzhuo, Yang, Qiming, Wang, ShougangX, Ma, LihongX

Tested-by: Zhang, XuemingX <xuemingx.zhang@intel.com>


>-----Original Message-----
>From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Shougang Wang
>Sent: Monday, April 13, 2020 9:39 AM
>To: dev@dpdk.org
>Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Yang, Qiming
><qiming.yang@intel.com>; Wang, ShougangX <shougangx.wang@intel.com>;
>stable@dpdk.org
>Subject: [dpdk-dev] [PATCH] net/ixgbe: fix link status after port reset
>
>It's a normal behavior to change the link status to up after resetting the
>port. So it is unnecessary to set link down before starting port, and
>changing the link state(link up/down) frequently will cause link speed
>unstable.
>
>Fixes: c3f2fbff78cf ("net/ixgbe: fix link status")
>Cc: stable@dpdk.org
>
>Signed-off-by: Shougang Wang <shougangx.wang@intel.com>
>---
> drivers/net/ixgbe/ixgbe_ethdev.c | 3 ---
> 1 file changed, 3 deletions(-)
>
>diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>b/drivers/net/ixgbe/ixgbe_ethdev.c
>index 23b3f5b0c..206358b85 100644
>--- a/drivers/net/ixgbe/ixgbe_ethdev.c
>+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>@@ -1196,7 +1196,6 @@ 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 */
>
> 	/*
>@@ -1307,8 +1306,6 @@ 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


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

* Re: [dpdk-dev] [PATCH] net/ixgbe: fix link status after port reset
  2020-04-13  1:38 [dpdk-dev] [PATCH] net/ixgbe: fix link status after port reset Shougang Wang
  2020-04-13  2:18 ` Yang, Qiming
       [not found] ` <1027ae0f2b914553a602ff46967c8a98@intel.com>
@ 2020-04-14  7:53 ` Ye Xiaolong
  2020-04-21  2:49   ` Wang, ShougangX
  2020-04-22  7:18 ` Ye Xiaolong
  3 siblings, 1 reply; 6+ messages in thread
From: Ye Xiaolong @ 2020-04-14  7:53 UTC (permalink / raw)
  To: Shougang Wang; +Cc: dev, Wenzhuo Lu, Qiming Yang, stable

Hi, Shougang

On 04/13, Shougang Wang wrote:
>It's a normal behavior to change the link status to up after
>resetting the port. So it is unnecessary to set link down before
>starting port, and changing the link state(link up/down) frequently
>will cause link speed unstable.
>
>Fixes: c3f2fbff78cf ("net/ixgbe: fix link status")
>Cc: stable@dpdk.org
>
>Signed-off-by: Shougang Wang <shougangx.wang@intel.com>
>---
> drivers/net/ixgbe/ixgbe_ethdev.c | 3 ---
> 1 file changed, 3 deletions(-)
>
>diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
>index 23b3f5b0c..206358b85 100644
>--- a/drivers/net/ixgbe/ixgbe_ethdev.c
>+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>@@ -1196,7 +1196,6 @@ 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;

Why do we need this change? Seems it is not mentioned in the commit log.

> #endif /* RTE_LIBRTE_IXGBE_BYPASS */
> 
> 	/*
>@@ -1307,8 +1306,6 @@ 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
>

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

* Re: [dpdk-dev] [PATCH] net/ixgbe: fix link status after port reset
  2020-04-14  7:53 ` Ye Xiaolong
@ 2020-04-21  2:49   ` Wang, ShougangX
  0 siblings, 0 replies; 6+ messages in thread
From: Wang, ShougangX @ 2020-04-21  2:49 UTC (permalink / raw)
  To: Ye, Xiaolong; +Cc: dev, Lu, Wenzhuo, Yang, Qiming, stable

Hi, Xiaolong

> -----Original Message-----
> From: Ye, Xiaolong <xiaolong.ye@intel.com>
> Sent: Tuesday, April 14, 2020 3:54 PM
> To: Wang, ShougangX <shougangx.wang@intel.com>
> Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: fix link status after port reset
> 
> Hi, Shougang
> 
> On 04/13, Shougang Wang wrote:
> >It's a normal behavior to change the link status to up after resetting
> >the port. So it is unnecessary to set link down before starting port,
> >and changing the link state(link up/down) frequently will cause link
> >speed unstable.
> >
> >Fixes: c3f2fbff78cf ("net/ixgbe: fix link status")
> >Cc: stable@dpdk.org
> >
> >Signed-off-by: Shougang Wang <shougangx.wang@intel.com>
> >---
> > drivers/net/ixgbe/ixgbe_ethdev.c | 3 ---
> > 1 file changed, 3 deletions(-)
> >
> >diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> >b/drivers/net/ixgbe/ixgbe_ethdev.c
> >index 23b3f5b0c..206358b85 100644
> >--- a/drivers/net/ixgbe/ixgbe_ethdev.c
> >+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> >@@ -1196,7 +1196,6 @@ 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;
> 
> Why do we need this change? Seems it is not mentioned in the commit log.

In c3f2fbff78cf, port was set to down by following 2 steps.
1. Setting autotry_restart flag to false to prevent NIC from linking up by itself.
2. Calling ixgbe_dev_set_link_down().

Force to set link status to down is unnecessary operation before starting port, so I revert these 2 changes in this patch.

Thanks
Shougang

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

* Re: [dpdk-dev] [PATCH] net/ixgbe: fix link status after port reset
  2020-04-13  1:38 [dpdk-dev] [PATCH] net/ixgbe: fix link status after port reset Shougang Wang
                   ` (2 preceding siblings ...)
  2020-04-14  7:53 ` Ye Xiaolong
@ 2020-04-22  7:18 ` Ye Xiaolong
  3 siblings, 0 replies; 6+ messages in thread
From: Ye Xiaolong @ 2020-04-22  7:18 UTC (permalink / raw)
  To: Shougang Wang; +Cc: dev, Wenzhuo Lu, Qiming Yang, stable

On 04/13, Shougang Wang wrote:
>It's a normal behavior to change the link status to up after
>resetting the port. So it is unnecessary to set link down before
>starting port, and changing the link state(link up/down) frequently
>will cause link speed unstable.
>
>Fixes: c3f2fbff78cf ("net/ixgbe: fix link status")
>Cc: stable@dpdk.org
>
>Signed-off-by: Shougang Wang <shougangx.wang@intel.com>
>---
> drivers/net/ixgbe/ixgbe_ethdev.c | 3 ---
> 1 file changed, 3 deletions(-)
>
>diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
>index 23b3f5b0c..206358b85 100644
>--- a/drivers/net/ixgbe/ixgbe_ethdev.c
>+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>@@ -1196,7 +1196,6 @@ 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 */
> 
> 	/*
>@@ -1307,8 +1306,6 @@ 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
>

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

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

end of thread, other threads:[~2020-04-22  7:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-13  1:38 [dpdk-dev] [PATCH] net/ixgbe: fix link status after port reset Shougang Wang
2020-04-13  2:18 ` Yang, Qiming
     [not found] ` <1027ae0f2b914553a602ff46967c8a98@intel.com>
2020-04-14  6:35   ` Zhang, XuemingX
2020-04-14  7:53 ` Ye Xiaolong
2020-04-21  2:49   ` Wang, ShougangX
2020-04-22  7:18 ` Ye Xiaolong

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).