[dpdk-dev] net/ixgbevf: reset hardware when stopping port

Message ID 1484153524-19337-1-git-send-email-olivier.matz@6wind.com (mailing list archive)
State Rejected, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel compilation success Compilation OK

Commit Message

Olivier Matz Jan. 11, 2017, 4:52 p.m. UTC
  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#n471

Regards,
Olivier


 drivers/net/ixgbe/ixgbe_ethdev.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
  

Comments

Olivier Matz Feb. 6, 2017, 1:58 p.m. UTC | #1
Hi,

On Wed, 11 Jan 2017 17:52:04 +0100, Olivier Matz
<olivier.matz@6wind.com> wrote:
> 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#n471
> 
> Regards,
> Olivier

Ping


Thanks,
Olivier
  
Wenzhuo Lu Feb. 24, 2017, 5:21 a.m. UTC | #2
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.1
  
=?utf-8?B?ZmVuZ3RpYW4gR3Vv?= Feb. 24, 2017, 8:47 a.m. UTC | #3
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.1
  
Wenzhuo Lu Feb. 24, 2017, 8:54 a.m. UTC | #4
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]<mailto:[mailto:dev-bounces@dpdk.org]> On Behalf Of Olivier Matz

> Sent: Thursday, January 12, 2017 12:52 AM

> To: dev@dpdk.org<mailto:dev@dpdk.org>; Zhang, Helin; Ananyev, Konstantin

> Cc: Guo Fengtian; stable@dpdk.org<mailto:stable@dpdk.org>; David Marchand

> Subject: [dpdk-dev] [PATCH] net/ixgbevf: reset hardware when stopping port

>

> From: Guo Fengtian <fengtian.guo@6wind.com<mailto: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<mailto:stable@dpdk.org>

> Signed-off-by: Guo Fengtian <fengtian.guo@6wind.com<mailto:fengtian.guo@6wind.com>>

> Signed-off-by: David Marchand <david.marchand@6wind.com<mailto:david.marchand@6wind.com>>

> Signed-off-by: Olivier Matz <olivier.matz@6wind.com<mailto: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.1
  
=?utf-8?B?ZmVuZ3RpYW4gR3Vv?= Feb. 24, 2017, 10:15 a.m. UTC | #5
Hi, Wenzhuo

The attachment is test report without product.

 
------------------ 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.1
  
Olivier Matz Feb. 24, 2017, 10:42 a.m. UTC | #6
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.

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.
  
Olivier Matz Feb. 27, 2017, 2:21 p.m. UTC | #7
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.    
>
  

Patch

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