[dpdk-dev] [PATCH] net/ice: fix unexpected link down

[dpdk-dev] [PATCH] net/ice: fix unexpected link down

[Qi Zhang]

Not to turn down link during dev_stop, it will cause the device
can't be bind by kernel driver after DPDK driver quit.

Fixes: e6161345d8a9 ("net/ice: support link status change")
Cc: stable@dpdk.org

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
 drivers/net/ice/ice_ethdev.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index d74675842..ae6eba63e 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -2277,8 +2277,6 @@ ice_dev_stop(struct rte_eth_dev *dev)
 	/* Clear all queues and release mbufs */
 	ice_clear_queues(dev);
 
-	ice_dev_set_link_down(dev);
-
 	/* Clean datapath event and queue/vec mapping */
 	rte_intr_efd_disable(intr_handle);
 	if (intr_handle->intr_vec) {
-- 
2.13.6

Re: [dpdk-dev] [PATCH] net/ice: fix unexpected link down

[Ye Xiaolong]

Hi, Qi

On 10/29, Qi Zhang wrote:
>Not to turn down link during dev_stop, it will cause the device
>can't be bind by kernel driver after DPDK driver quit.
>
>Fixes: e6161345d8a9 ("net/ice: support link status change")
>Cc: stable@dpdk.org
>
>Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
>---
> drivers/net/ice/ice_ethdev.c | 2 --
> 1 file changed, 2 deletions(-)
>
>diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
>index d74675842..ae6eba63e 100644
>--- a/drivers/net/ice/ice_ethdev.c
>+++ b/drivers/net/ice/ice_ethdev.c
>@@ -2277,8 +2277,6 @@ ice_dev_stop(struct rte_eth_dev *dev)
> 	/* Clear all queues and release mbufs */
> 	ice_clear_queues(dev);
> 
>-	ice_dev_set_link_down(dev);
>-

Is this ice specific issue? Calling set_link_down seems correct thing to do in stop
ops, as we can see in other PMDs like i40e, mvneta, ... 

Thanks,
Xiaolong


> 	/* Clean datapath event and queue/vec mapping */
> 	rte_intr_efd_disable(intr_handle);
> 	if (intr_handle->intr_vec) {
>-- 
>2.13.6
>

Re: [dpdk-dev] [PATCH] net/ice: fix unexpected link down

[Stillwell Jr, Paul M]

Hi Xiaolong,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Ye Xiaolong
> Sent: Monday, October 28, 2019 10:35 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Yang, Qiming <qiming.yang@intel.com>; dev@dpdk.org;
> stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/ice: fix unexpected link down
> 
> Hi, Qi
> 
> On 10/29, Qi Zhang wrote:
> >Not to turn down link during dev_stop, it will cause the device can't
> >be bind by kernel driver after DPDK driver quit.
> >
> >Fixes: e6161345d8a9 ("net/ice: support link status change")
> >Cc: stable@dpdk.org
> >
> >Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> >---
> > drivers/net/ice/ice_ethdev.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> >diff --git a/drivers/net/ice/ice_ethdev.c
> >b/drivers/net/ice/ice_ethdev.c index d74675842..ae6eba63e 100644
> >--- a/drivers/net/ice/ice_ethdev.c
> >+++ b/drivers/net/ice/ice_ethdev.c
> >@@ -2277,8 +2277,6 @@ ice_dev_stop(struct rte_eth_dev *dev)
> > 	/* Clear all queues and release mbufs */
> > 	ice_clear_queues(dev);
> >
> >-	ice_dev_set_link_down(dev);
> >-
> 
> Is this ice specific issue? Calling set_link_down seems correct thing to do in
> stop ops, as we can see in other PMDs like i40e, mvneta, ...
> 

Technically, this issue also existed in i40e. The i40e kernel driver added a workaround to fix this when the DPDK driver would force the link down when it closed. The FW handles the link status and the driver shouldn't mess with it unless the user indicates they want it in some other state. I think the best idea is to return the link state into whatever the link state was when DPDK started (probably up) instead of setting it to down when the PMD exits.

> Thanks,
> Xiaolong
> 
> 
> > 	/* Clean datapath event and queue/vec mapping */
> > 	rte_intr_efd_disable(intr_handle);
> > 	if (intr_handle->intr_vec) {
> >--
> >2.13.6
> >