[prev in list] [next in list] [prev in thread] [next in thread] 

List:       dpdk-dev
Subject:    Re: [dpdk-dev] [PATCH] net/ice: fix unexpected link down
From:       "Stillwell Jr, Paul M" <paul.m.stillwell.jr () intel ! com>
Date:       2019-10-31 21:52:38
Message-ID: BN8PR11MB3828FF76688821C88C55063EE0630 () BN8PR11MB3828 ! namprd11 ! prod ! outlook ! com
[Download RAW message or body]

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


[prev in list] [next in list] [prev in thread] [next in thread]