* Re: [dpdk-stable] [PATCH] net/ixgbe: fix link status after port reset
2020-04-13 1:38 [dpdk-stable] [PATCH] net/ixgbe: fix link status after port reset Shougang Wang
@ 2020-04-13 2:18 ` Yang, Qiming
2020-04-14 7:53 ` [dpdk-stable] [dpdk-dev] " Ye Xiaolong
2020-04-22 7:18 ` Ye Xiaolong
2 siblings, 0 replies; 5+ 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] 5+ messages in thread* Re: [dpdk-stable] [dpdk-dev] [PATCH] net/ixgbe: fix link status after port reset
2020-04-13 1:38 [dpdk-stable] [PATCH] net/ixgbe: fix link status after port reset Shougang Wang
2020-04-13 2:18 ` Yang, Qiming
@ 2020-04-14 7:53 ` Ye Xiaolong
2020-04-21 2:49 ` Wang, ShougangX
2020-04-22 7:18 ` Ye Xiaolong
2 siblings, 1 reply; 5+ 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] 5+ messages in thread* Re: [dpdk-stable] [dpdk-dev] [PATCH] net/ixgbe: fix link status after port reset
2020-04-14 7:53 ` [dpdk-stable] [dpdk-dev] " Ye Xiaolong
@ 2020-04-21 2:49 ` Wang, ShougangX
0 siblings, 0 replies; 5+ 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] 5+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH] net/ixgbe: fix link status after port reset
2020-04-13 1:38 [dpdk-stable] [PATCH] net/ixgbe: fix link status after port reset Shougang Wang
2020-04-13 2:18 ` Yang, Qiming
2020-04-14 7:53 ` [dpdk-stable] [dpdk-dev] " Ye Xiaolong
@ 2020-04-22 7:18 ` Ye Xiaolong
2 siblings, 0 replies; 5+ 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] 5+ messages in thread