Re: [dpdk-stable] [dpdk-dev] [PATCH] net/ixgbevf: reset hardware whenstoppingport

[Olivier Matz <olivier.matz@6wind.com>]

Hi Wenzhuo,


On Fri, 24 Feb 2017 11:42:56 +0100, Olivier Matz
<olivier.matz@6wind.com> wrote:
> Hi Fengtian,
> 
> On Fri, 24 Feb 2017 18:15:27 +0800, "fengtian Guo"
> <fengtian.guo@6wind.com> wrote:
> > Hi, Wenzhuo
> > 
> > The attachment is test report without product.  
> 
> The test report uses is a use-case with 6WINDGate, so it cannot be
> reproduced by Intel guys.
> 
> I'll see how to reproduce the same case with testpmd and send an
> update.

I failed to reproduce the issue with testpmd. After a deeper
analysis, it turn outs that the issue we've seen is not related to the
ixgbe dpdk driver.

Sorry for the noise. I changed the patch status as rejected in
patchwork.


Thanks,
Olivier



> 
> Thanks,
> Olivier
> 
> 
> 
> 
> 
> > 
> >  
> > ------------------ Original ------------------
> > From:  "Lu, Wenzhuo"<wenzhuo.lu@intel.com>;
> > Date:  Fri, Feb 24, 2017 04:54 PM
> > To:  "fengtian Guo"<fengtian.guo@6wind.com>; "Olivier
> > Matz"<olivier.matz@6wind.com>; "dev@dpdk.org"<dev@dpdk.org>; "Zhang,
> > Helin"<helin.zhang@intel.com>; "Ananyev,
> > Konstantin"<konstantin.ananyev@intel.com>; Cc:
> > "stable@dpdk.org"<stable@dpdk.org>; "David
> > Marchand"<david.marchand@6wind.com>; Subject:  RE: Re:RE: [dpdk-dev]
> > [PATCH] net/ixgbevf: reset hardware whenstoppingport
> > 
> >  
> >   
> > Hi Fengtian,
> >  
> > Would you like to let us know how you validate this patch? As I’ve
> > tried just stop -> start VF  can make it work again after PF down ->
> > up. Thanks. 
> >  
> >  
> >     
> > From: fengtian.guo@6wind.com.cn [mailto:fengtian.guo@6wind.com.cn]
> > On Behalf Of fengtian Guo Sent: Friday, February 24, 2017 4:47 PM
> >  To: Lu, Wenzhuo; Olivier Matz; dev@dpdk.org; Zhang, Helin; Ananyev,
> > Konstantin Cc: stable@dpdk.org; David Marchand
> >  Subject: Re:RE: [dpdk-dev] [PATCH] net/ixgbevf: reset hardware when
> > stoppingport 
> >  
> >  
> >  
> >  
> > Hi, Wenzhuo
> >  
> >  Sorry, Now I just notice the email is related with me.
> >    
> >  > Seems the commit log doesn't match the code change.
> >  > This change cannot resolve the described problem. The problem is
> >  > VF doesn't handle the PF reset event. Moving the hw_reset from
> >  > close to stop doesn't help and not necessary. Because PF has
> >  > already reset VF stuff during resetting itself, that's why VF not
> >  > working anymore.    
> >  
> >  Explain the problem of this issue.
> >  1) it seems VF have to call ixgbe_reset_hw() when it receive PF
> > reset event. it seems VF is client and PF is server and VF has to
> > reset the register on his side, after PF has reset the register on
> > his side. 2) with this patch, when VF receive the event after PF is
> > set link down, it will call ixgbe_reset_hw() in ixgbevf_dev_close().
> > and we have validate this patch 
> >  
> >  Thanks
> >  Fengtian
> >    
> >  
> >  
> >   
> >  
> >  
> >    
> > ------------------ Original ------------------
> >  
> >    
> > From:  "Lu, Wenzhuo"<wenzhuo.lu@intel.com>;
> >  
> >   
> > Date:  Fri, Feb 24, 2017 01:21 PM
> >  
> >   
> > To:  "Olivier Matz"<olivier.matz@6wind.com>;
> > "dev@dpdk.org"<dev@dpdk.org>; "Zhang, Helin"<helin.zhang@intel.com>;
> > "Ananyev, Konstantin"<konstantin.ananyev@intel.com>; 
> > Cc:  "Guo Fengtian"<fengtian.guo@6wind.com>;
> > "stable@dpdk.org"<stable@dpdk.org>; "David
> > Marchand"<david.marchand@6wind.com>; 
> > Subject:  RE: [dpdk-dev] [PATCH] net/ixgbevf: reset hardware when
> > stoppingport 
> >  
> >   
> >  
> >  
> >  
> > Hi Olivier, Fengtian,
> >  
> >    
> >  > -----Original Message-----
> >  > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> >  > Sent: Thursday, January 12, 2017 12:52 AM
> >  > To: dev@dpdk.org; Zhang, Helin; Ananyev, Konstantin
> >  > Cc: Guo Fengtian; stable@dpdk.org; David Marchand
> >  > Subject: [dpdk-dev] [PATCH] net/ixgbevf: reset hardware when
> >  > stopping port
> >  > 
> >  > From: Guo Fengtian <fengtian.guo@6wind.com>
> >  > 
> >  > When PF triggers a reset, VF port must acknowledge it by calling
> >  > ixgbe_reset_hw().
> >  > 
> >  > Before this patch, the port link status, speed and duplex are
> >  > invalid (all set to 0).
> >  > 
> >  > The patch move the call to ixgbe_reset_hw() from
> >  > ixgbevf_dev_close() to ixgbevf_dev_stop(), so that after a port
> >  > restart on the VM, the link status is properly marked down, with
> >  > correct speed and duplex.
> >  > 
> >  > Fixes: f0160874c041 ("ixgbe: various updates")
> >  > 
> >  > CC: stable@dpdk.org
> >  > Signed-off-by: Guo Fengtian <fengtian.guo@6wind.com>
> >  > Signed-off-by: David Marchand <david.marchand@6wind.com>
> >  > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> >  > ---
> >  > 
> >  > Hi,
> >  > 
> >  > The use case for this problem was described some time ago, see:
> >  > http://dpdk.org/ml/archives/dev/2015-December/030067.html
> >  > 
> >  > It is also part of 2.2 release note:
> >  >  http://dpdk.org/browse/dpdk/tree/doc/guides/rel_notes/release_2_2.rst#n4
> >  > 71
> >  > 
> >  > Regards,
> >  > Olivier    
> >  Seems the commit log doesn't match the code change.
> >  This change cannot resolve the described problem. The problem is VF
> > doesn't handle the PF reset event. Moving the hw_reset from close to
> > stop doesn't help and not necessary. Because PF has already reset VF
> > stuff during  resetting itself, that's why VF not working
> > anymore.   
> >  > 
> >  > 
> >  >  drivers/net/ixgbe/ixgbe_ethdev.c | 5 +----
> >  >  1 file changed, 1 insertion(+), 4 deletions(-)
> >  > 
> >  > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> >  > b/drivers/net/ixgbe/ixgbe_ethdev.c
> >  > index b7ddd4f..2d8641a 100644
> >  > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> >  > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> >  > @@ -4268,6 +4268,7 @@ ixgbevf_dev_stop(struct rte_eth_dev *dev)
> >  >  PMD_INIT_FUNC_TRACE();
> >  > 
> >  >  ixgbevf_intr_disable(hw);
> >  > + ixgbe_reset_hw(hw);
> >  > 
> >  >  hw->adapter_stopped = 1;
> >  >  ixgbe_stop_adapter(hw);
> >  > @@ -4294,12 +4295,8 @@ ixgbevf_dev_stop(struct rte_eth_dev *dev)
> >  > static void  ixgbevf_dev_close(struct rte_eth_dev *dev)  {
> >  > - struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-    
> >  > >dev_private);    
> >  > -
> >  >  PMD_INIT_FUNC_TRACE();
> >  > 
> >  > - ixgbe_reset_hw(hw);
> >  > -
> >  >  ixgbevf_dev_stop(dev);
> >  > 
> >  >  ixgbe_dev_free_queues(dev);
> >  > --
> >  > 2.8.    
>