All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net/ixgbevf: fix stats update after a PF reset
@ 2017-01-11 17:04 Olivier Matz
  2017-02-06 13:58 ` Olivier Matz
  0 siblings, 1 reply; 13+ messages in thread
From: Olivier Matz @ 2017-01-11 17:04 UTC (permalink / raw)
  To: dev, helin.zhang, konstantin.ananyev; +Cc: Guo Fengtian, stable

From: Guo Fengtian <fengtian.guo@6wind.com>

When PF is set down, in VF, the value of stats register is zero.
So only increase stats when it's non zero.

Fixes: af75078fece3 ("first public release")

CC: stable@dpdk.org
Signed-off-by: Guo Fengtian <fengtian.guo@6wind.com>
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 2d8641a..455a0c9 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -389,7 +389,8 @@ static int ixgbe_dev_udp_tunnel_port_del(struct rte_eth_dev *dev,
 #define UPDATE_VF_STAT(reg, last, cur)                          \
 {                                                               \
 	uint32_t latest = IXGBE_READ_REG(hw, reg);              \
-	cur += (latest - last) & UINT_MAX;                      \
+	if (latest != 0)					\
+		cur += (latest - last) & UINT_MAX;              \
 	last = latest;                                          \
 }
 
@@ -398,7 +399,8 @@ static int ixgbe_dev_udp_tunnel_port_del(struct rte_eth_dev *dev,
 	u64 new_lsb = IXGBE_READ_REG(hw, lsb);                   \
 	u64 new_msb = IXGBE_READ_REG(hw, msb);                   \
 	u64 latest = ((new_msb << 32) | new_lsb);                \
-	cur += (0x1000000000LL + latest - last) & 0xFFFFFFFFFLL; \
+	if (latest != 0)					 \
+		cur += (0x1000000000LL + latest - last) & 0xFFFFFFFFFLL; \
 	last = latest;                                           \
 }
 
-- 
2.8.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] net/ixgbevf: fix stats update after a PF reset
  2017-01-11 17:04 [PATCH] net/ixgbevf: fix stats update after a PF reset Olivier Matz
@ 2017-02-06 13:58 ` Olivier Matz
  2017-02-09 14:37   ` Dai, Wei
  2017-02-09 14:50   ` Dai, Wei
  0 siblings, 2 replies; 13+ messages in thread
From: Olivier Matz @ 2017-02-06 13:58 UTC (permalink / raw)
  To: dev, helin.zhang, konstantin.ananyev; +Cc: Guo Fengtian, stable

Hi,

On Wed, 11 Jan 2017 18:04:14 +0100, Olivier Matz
<olivier.matz@6wind.com> wrote:
> From: Guo Fengtian <fengtian.guo@6wind.com>
> 
> When PF is set down, in VF, the value of stats register is zero.
> So only increase stats when it's non zero.
> 
> Fixes: af75078fece3 ("first public release")
> 
> CC: stable@dpdk.org
> Signed-off-by: Guo Fengtian <fengtian.guo@6wind.com>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>

Ping


Thanks,
Olivier

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net/ixgbevf: fix stats update after a PF reset
  2017-02-06 13:58 ` Olivier Matz
@ 2017-02-09 14:37   ` Dai, Wei
  2017-02-09 14:50   ` Dai, Wei
  1 sibling, 0 replies; 13+ messages in thread
From: Dai, Wei @ 2017-02-09 14:37 UTC (permalink / raw)
  To: Olivier Matz, dev, Zhang, Helin, Ananyev, Konstantin; +Cc: Guo Fengtian, stable

The stats register can rewind to zero when the port is running for a long period.
So I am afraid that this check is not always correct.
Why not introduce a variable to directly indicate whether the resulted stats
should be updated or not.

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Monday, February 6, 2017 9:59 PM
> To: 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
> Subject: Re: [dpdk-dev] [PATCH] net/ixgbevf: fix stats update after a PF reset
> 
> Hi,
> 
> On Wed, 11 Jan 2017 18:04:14 +0100, Olivier Matz <olivier.matz@6wind.com>
> wrote:
> > From: Guo Fengtian <fengtian.guo@6wind.com>
> >
> > When PF is set down, in VF, the value of stats register is zero.
> > So only increase stats when it's non zero.
> >
> > Fixes: af75078fece3 ("first public release")
> >
> > CC: stable@dpdk.org
> > Signed-off-by: Guo Fengtian <fengtian.guo@6wind.com>
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> 
> Ping
> 
> 
> Thanks,
> Olivier

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net/ixgbevf: fix stats update after a PF reset
  2017-02-06 13:58 ` Olivier Matz
  2017-02-09 14:37   ` Dai, Wei
@ 2017-02-09 14:50   ` Dai, Wei
  2017-02-16 16:49     ` Olivier Matz
  1 sibling, 1 reply; 13+ messages in thread
From: Dai, Wei @ 2017-02-09 14:50 UTC (permalink / raw)
  To: Olivier Matz, dev, Zhang, Helin, Ananyev, Konstantin; +Cc: Guo Fengtian, stable

Another way is to clear hw_stats->last_vfgprc/last_vfgorc/last_vfgptc/last_vfmprc 
at the same time PF is set down.

> -----Original Message-----
> From: Dai, Wei
> Sent: Thursday, February 9, 2017 10:38 PM
> To: 'Olivier Matz' <olivier.matz@6wind.com>; 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
> Subject: RE: [dpdk-dev] [PATCH] net/ixgbevf: fix stats update after a PF reset
> 
> The stats register can rewind to zero when the port is running for a long period.
> So I am afraid that this check is not always correct.
> Why not introduce a variable to directly indicate whether the resulted stats
> should be updated or not.
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> > Sent: Monday, February 6, 2017 9:59 PM
> > To: 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
> > Subject: Re: [dpdk-dev] [PATCH] net/ixgbevf: fix stats update after a
> > PF reset
> >
> > Hi,
> >
> > On Wed, 11 Jan 2017 18:04:14 +0100, Olivier Matz
> > <olivier.matz@6wind.com>
> > wrote:
> > > From: Guo Fengtian <fengtian.guo@6wind.com>
> > >
> > > When PF is set down, in VF, the value of stats register is zero.
> > > So only increase stats when it's non zero.
> > >
> > > Fixes: af75078fece3 ("first public release")
> > >
> > > CC: stable@dpdk.org
> > > Signed-off-by: Guo Fengtian <fengtian.guo@6wind.com>
> > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> >
> > Ping
> >
> >
> > Thanks,
> > Olivier

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net/ixgbevf: fix stats update after a PF reset
  2017-02-09 14:50   ` Dai, Wei
@ 2017-02-16 16:49     ` Olivier Matz
  2017-03-14  9:46       ` Olivier Matz
  0 siblings, 1 reply; 13+ messages in thread
From: Olivier Matz @ 2017-02-16 16:49 UTC (permalink / raw)
  To: Dai, Wei; +Cc: dev, Zhang, Helin, Ananyev, Konstantin, Guo Fengtian, stable

Hi Wei,

On Thu, 9 Feb 2017 14:50:05 +0000, "Dai, Wei" <wei.dai@intel.com> wrote:
> > -----Original Message-----
> > From: Dai, Wei
> > Sent: Thursday, February 9, 2017 10:38 PM
> > To: 'Olivier Matz' <olivier.matz@6wind.com>; 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
> > Subject: RE: [dpdk-dev] [PATCH] net/ixgbevf: fix stats update after
> > a PF reset
> > 
> > The stats register can rewind to zero when the port is running for
> > a long period. So I am afraid that this check is not always correct.
> > Why not introduce a variable to directly indicate whether the
> > resulted stats should be updated or not.
>
> Another way is to clear
> hw_stats->last_vfgprc/last_vfgorc/last_vfgptc/last_vfmprc at the same
> time PF is set down.
> 

Can we know easily in VF if the PF was set down?

Thanks,
Olivier

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net/ixgbevf: fix stats update after a PF reset
  2017-02-16 16:49     ` Olivier Matz
@ 2017-03-14  9:46       ` Olivier Matz
  2017-03-24 14:11         ` Olivier Matz
  0 siblings, 1 reply; 13+ messages in thread
From: Olivier Matz @ 2017-03-14  9:46 UTC (permalink / raw)
  To: Dai, Wei; +Cc: dev, Zhang, Helin, Ananyev, Konstantin, Guo Fengtian, stable

Hi Wei,

On Thu, 16 Feb 2017 17:49:22 +0100, Olivier Matz <olivier.matz@6wind.com> wrote:
> Hi Wei,
> 
> On Thu, 9 Feb 2017 14:50:05 +0000, "Dai, Wei" <wei.dai@intel.com> wrote:
> > > -----Original Message-----
> > > From: Dai, Wei
> > > Sent: Thursday, February 9, 2017 10:38 PM
> > > To: 'Olivier Matz' <olivier.matz@6wind.com>; 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
> > > Subject: RE: [dpdk-dev] [PATCH] net/ixgbevf: fix stats update after
> > > a PF reset
> > > 
> > > The stats register can rewind to zero when the port is running for
> > > a long period. So I am afraid that this check is not always correct.
> > > Why not introduce a variable to directly indicate whether the
> > > resulted stats should be updated or not.  
> >
> > Another way is to clear
> > hw_stats->last_vfgprc/last_vfgorc/last_vfgptc/last_vfmprc at the same
> > time PF is set down.
> >   
> 
> Can we know easily in VF if the PF was set down?

Any guideline for this? Or is it something we cannot fix?

Thanks,
Olivier

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net/ixgbevf: fix stats update after a PF reset
  2017-03-14  9:46       ` Olivier Matz
@ 2017-03-24 14:11         ` Olivier Matz
  2017-03-28 15:22           ` Thomas Monjalon
  0 siblings, 1 reply; 13+ messages in thread
From: Olivier Matz @ 2017-03-24 14:11 UTC (permalink / raw)
  To: Dai, Wei; +Cc: dev, Zhang, Helin, Ananyev, Konstantin, Guo Fengtian

Hi,

(remove stable@dpdk.org)

On Tue, 14 Mar 2017 10:46:40 +0100, Olivier Matz <olivier.matz@6wind.com> wrote:
> Hi Wei,
> 
> On Thu, 16 Feb 2017 17:49:22 +0100, Olivier Matz <olivier.matz@6wind.com> wrote:
> > Hi Wei,
> > 
> > On Thu, 9 Feb 2017 14:50:05 +0000, "Dai, Wei" <wei.dai@intel.com> wrote:  
> > > > -----Original Message-----
> > > > From: Dai, Wei
> > > > Sent: Thursday, February 9, 2017 10:38 PM
> > > > To: 'Olivier Matz' <olivier.matz@6wind.com>; 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
> > > > Subject: RE: [dpdk-dev] [PATCH] net/ixgbevf: fix stats update after
> > > > a PF reset
> > > > 
> > > > The stats register can rewind to zero when the port is running for
> > > > a long period. So I am afraid that this check is not always correct.
> > > > Why not introduce a variable to directly indicate whether the
> > > > resulted stats should be updated or not.    
> > >
> > > Another way is to clear
> > > hw_stats->last_vfgprc/last_vfgorc/last_vfgptc/last_vfmprc at the same
> > > time PF is set down.
> > >     
> > 
> > Can we know easily in VF if the PF was set down?  
> 
> Any guideline for this? Or is it something we cannot fix?
> 

Is any maintainer available to help to fix that?

Unfortunately I don't have enough knowledge on this driver to do the fix
by myself. It would be helpful to have some guidelines by a maintainer so
I can understand what is the proper way to fix the issue.

Thanks,
Olivier

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net/ixgbevf: fix stats update after a PF reset
  2017-03-24 14:11         ` Olivier Matz
@ 2017-03-28 15:22           ` Thomas Monjalon
  2017-03-29  9:22             ` Dai, Wei
  2017-03-29  9:37             ` Dai, Wei
  0 siblings, 2 replies; 13+ messages in thread
From: Thomas Monjalon @ 2017-03-28 15:22 UTC (permalink / raw)
  To: Zhang, Helin, Ananyev, Konstantin
  Cc: dev, Olivier Matz, Dai, Wei, Guo Fengtian, ferruh.yigit

Please ixgbe maintainers, what can be done for this fix?

2017-03-24 15:11, Olivier Matz:
> Hi,
> 
> (remove stable@dpdk.org)
> 
> On Tue, 14 Mar 2017 10:46:40 +0100, Olivier Matz <olivier.matz@6wind.com> wrote:
> > Hi Wei,
> > 
> > On Thu, 16 Feb 2017 17:49:22 +0100, Olivier Matz <olivier.matz@6wind.com> wrote:
> > > Hi Wei,
> > > 
> > > On Thu, 9 Feb 2017 14:50:05 +0000, "Dai, Wei" <wei.dai@intel.com> wrote:  
> > > > > -----Original Message-----
> > > > > From: Dai, Wei
> > > > > Sent: Thursday, February 9, 2017 10:38 PM
> > > > > To: 'Olivier Matz' <olivier.matz@6wind.com>; 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
> > > > > Subject: RE: [dpdk-dev] [PATCH] net/ixgbevf: fix stats update after
> > > > > a PF reset
> > > > > 
> > > > > The stats register can rewind to zero when the port is running for
> > > > > a long period. So I am afraid that this check is not always correct.
> > > > > Why not introduce a variable to directly indicate whether the
> > > > > resulted stats should be updated or not.    
> > > >
> > > > Another way is to clear
> > > > hw_stats->last_vfgprc/last_vfgorc/last_vfgptc/last_vfmprc at the same
> > > > time PF is set down.
> > > >     
> > > 
> > > Can we know easily in VF if the PF was set down?  
> > 
> > Any guideline for this? Or is it something we cannot fix?
> > 
> 
> Is any maintainer available to help to fix that?
> 
> Unfortunately I don't have enough knowledge on this driver to do the fix
> by myself. It would be helpful to have some guidelines by a maintainer so
> I can understand what is the proper way to fix the issue.
> 
> Thanks,
> Olivier

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net/ixgbevf: fix stats update after a PF reset
  2017-03-28 15:22           ` Thomas Monjalon
@ 2017-03-29  9:22             ` Dai, Wei
  2017-03-29 10:08               ` Olivier Matz
  2017-03-29  9:37             ` Dai, Wei
  1 sibling, 1 reply; 13+ messages in thread
From: Dai, Wei @ 2017-03-29  9:22 UTC (permalink / raw)
  To: Thomas Monjalon, Zhang, Helin, Ananyev, Konstantin
  Cc: dev, Olivier Matz, Guo Fengtian, Yigit, Ferruh

First of all, I don't agree the method in this patch because the register can also rewind to 0 in normal mode except
reset/PF down.

In the function void ixgbe_down(struct ixgbe_adapter *adapter) of ixgbe_main.c in the ixgbe kernel PF driver (version 5.0.4),
the PF in kernel driver will ping all the active vfs to let them know PF is going down by mailbox messages.

On other side, ixgbe VF in DPDK PMD will handle the ping mailbox message in ixgbevf_dev_interrupt_handler( ) which is
registered interrupt routine. 
And ixgbevf_dev_interrupt_handler( ) will call ixgbevf_mbx_process( ) to process mailbox message from PF.

So it is in ixgbevf_mbx_process( ) where the SW stats registers of VF can be fixed.
There is more than one line to send ping message to VF in kernel PF drvier codes.
I am studying it to make it clear how to identify the ping message due to PF down.

So can we defer this patch first.
And another patch in above way should be submitted.

Thanks

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Tuesday, March 28, 2017 11:23 PM
> To: Zhang, Helin <helin.zhang@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org; Olivier Matz <olivier.matz@6wind.com>; Dai, Wei
> <wei.dai@intel.com>; Guo Fengtian <fengtian.guo@6wind.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] net/ixgbevf: fix stats update after a PF reset
> 
> Please ixgbe maintainers, what can be done for this fix?
> 
> 2017-03-24 15:11, Olivier Matz:
> > Hi,
> >
> > (remove stable@dpdk.org)
> >
> > On Tue, 14 Mar 2017 10:46:40 +0100, Olivier Matz <olivier.matz@6wind.com>
> wrote:
> > > Hi Wei,
> > >
> > > On Thu, 16 Feb 2017 17:49:22 +0100, Olivier Matz
> <olivier.matz@6wind.com> wrote:
> > > > Hi Wei,
> > > >
> > > > On Thu, 9 Feb 2017 14:50:05 +0000, "Dai, Wei" <wei.dai@intel.com>
> wrote:
> > > > > > -----Original Message-----
> > > > > > From: Dai, Wei
> > > > > > Sent: Thursday, February 9, 2017 10:38 PM
> > > > > > To: 'Olivier Matz' <olivier.matz@6wind.com>; 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
> > > > > > Subject: RE: [dpdk-dev] [PATCH] net/ixgbevf: fix stats update
> > > > > > after a PF reset
> > > > > >
> > > > > > The stats register can rewind to zero when the port is running
> > > > > > for a long period. So I am afraid that this check is not always correct.
> > > > > > Why not introduce a variable to directly indicate whether the
> > > > > > resulted stats should be updated or not.
> > > > >
> > > > > Another way is to clear
> > > > > hw_stats->last_vfgprc/last_vfgorc/last_vfgptc/last_vfmprc at the
> > > > > same time PF is set down.
> > > > >
> > > >
> > > > Can we know easily in VF if the PF was set down?
> > >
> > > Any guideline for this? Or is it something we cannot fix?
> > >
> >
> > Is any maintainer available to help to fix that?
> >
> > Unfortunately I don't have enough knowledge on this driver to do the
> > fix by myself. It would be helpful to have some guidelines by a
> > maintainer so I can understand what is the proper way to fix the issue.
> >
> > Thanks,
> > Olivier
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net/ixgbevf: fix stats update after a PF reset
  2017-03-28 15:22           ` Thomas Monjalon
  2017-03-29  9:22             ` Dai, Wei
@ 2017-03-29  9:37             ` Dai, Wei
  1 sibling, 0 replies; 13+ messages in thread
From: Dai, Wei @ 2017-03-29  9:37 UTC (permalink / raw)
  To: Thomas Monjalon, Zhang, Helin, Ananyev, Konstantin, dev
  Cc: dev, Olivier Matz, Guo Fengtian, Yigit, Ferruh

First of all, I don't agree the method in this patch because the register can also rewind to 0 in normal mode except reset/PF down.

In the function void ixgbe_down(struct ixgbe_adapter *adapter) of ixgbe_main.c in the ixgbe kernel PF driver (version 5.0.4), the PF in kernel driver will ping all the active vfs to let them know PF is going down by mailbox messages.

On other side, ixgbe VF in DPDK PMD will handle the ping mailbox message in ixgbevf_dev_interrupt_handler( ) which is registered interrupt routine. 
And ixgbevf_dev_interrupt_handler( ) will call ixgbevf_mbx_process( ) to process mailbox message from PF.

So it is in ixgbevf_mbx_process( ) where the SW stats registers of VF can be fixed.
There is more than one line to send ping message to VF in kernel PF drvier codes.
I am studying it to make it clear how to identify the ping message due to PF down.

So can we defer this patch first.
And another patch in above way should be submitted.

Thanks

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Tuesday, March 28, 2017 11:23 PM
> To: Zhang, Helin <helin.zhang@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org; Olivier Matz <olivier.matz@6wind.com>; Dai, Wei
> <wei.dai@intel.com>; Guo Fengtian <fengtian.guo@6wind.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] net/ixgbevf: fix stats update after a PF reset
> 
> Please ixgbe maintainers, what can be done for this fix?
> 
> 2017-03-24 15:11, Olivier Matz:
> > Hi,
> >
> > (remove stable@dpdk.org)
> >
> > On Tue, 14 Mar 2017 10:46:40 +0100, Olivier Matz <olivier.matz@6wind.com>
> wrote:
> > > Hi Wei,
> > >
> > > On Thu, 16 Feb 2017 17:49:22 +0100, Olivier Matz
> <olivier.matz@6wind.com> wrote:
> > > > Hi Wei,
> > > >
> > > > On Thu, 9 Feb 2017 14:50:05 +0000, "Dai, Wei" <wei.dai@intel.com>
> wrote:
> > > > > > -----Original Message-----
> > > > > > From: Dai, Wei
> > > > > > Sent: Thursday, February 9, 2017 10:38 PM
> > > > > > To: 'Olivier Matz' <olivier.matz@6wind.com>; 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
> > > > > > Subject: RE: [dpdk-dev] [PATCH] net/ixgbevf: fix stats update
> > > > > > after a PF reset
> > > > > >
> > > > > > The stats register can rewind to zero when the port is running
> > > > > > for a long period. So I am afraid that this check is not always correct.
> > > > > > Why not introduce a variable to directly indicate whether the
> > > > > > resulted stats should be updated or not.
> > > > >
> > > > > Another way is to clear
> > > > > hw_stats->last_vfgprc/last_vfgorc/last_vfgptc/last_vfmprc at the
> > > > > same time PF is set down.
> > > > >
> > > >
> > > > Can we know easily in VF if the PF was set down?
> > >
> > > Any guideline for this? Or is it something we cannot fix?
> > >
> >
> > Is any maintainer available to help to fix that?
> >
> > Unfortunately I don't have enough knowledge on this driver to do the
> > fix by myself. It would be helpful to have some guidelines by a
> > maintainer so I can understand what is the proper way to fix the issue.
> >
> > Thanks,
> > Olivier
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net/ixgbevf: fix stats update after a PF reset
  2017-03-29  9:22             ` Dai, Wei
@ 2017-03-29 10:08               ` Olivier Matz
  2017-03-30 10:46                 ` Dai, Wei
  0 siblings, 1 reply; 13+ messages in thread
From: Olivier Matz @ 2017-03-29 10:08 UTC (permalink / raw)
  To: Dai, Wei
  Cc: Thomas Monjalon, Zhang, Helin, Ananyev, Konstantin, dev,
	Guo Fengtian, Yigit, Ferruh

Hi Wei,

On Wed, 29 Mar 2017 09:22:55 +0000, "Dai, Wei" <wei.dai@intel.com> wrote:
> First of all, I don't agree the method in this patch because the register can also rewind to 0 in normal mode except
> reset/PF down.
> 
> In the function void ixgbe_down(struct ixgbe_adapter *adapter) of ixgbe_main.c in the ixgbe kernel PF driver (version 5.0.4),
> the PF in kernel driver will ping all the active vfs to let them know PF is going down by mailbox messages.
> 
> On other side, ixgbe VF in DPDK PMD will handle the ping mailbox message in ixgbevf_dev_interrupt_handler( ) which is
> registered interrupt routine. 
> And ixgbevf_dev_interrupt_handler( ) will call ixgbevf_mbx_process( ) to process mailbox message from PF.
> 
> So it is in ixgbevf_mbx_process( ) where the SW stats registers of VF can be fixed.
> There is more than one line to send ping message to VF in kernel PF drvier codes.

Thank you for the explanation.

Just to be sure I correctly understand:
1- the ping message is properly sent by ixgbe pf linux driver
2- the ping message is not yet sent by ixgbe pf dpdk driver
3- the ping message is properly received by ixgbe vf dpdk driver, but
   it is not used yet to stop wrong stats accumulation

> I am studying it to make it clear how to identify the ping message due to PF down.

Are you saying that you are currently working on a fix for 2 and 3 (that
would be great!) or are you expecting that I work on it? In any case, I
can help, at least to test the patches.


> So can we defer this patch first.
> And another patch in above way should be submitted.


I'm of course ok to defer the initial patch, I agree it's more a workaround
than a proper solution. I was stuck because I did not know how the VF is notified
by the PF when it is reset.


Thanks
Olivier

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net/ixgbevf: fix stats update after a PF reset
  2017-03-29 10:08               ` Olivier Matz
@ 2017-03-30 10:46                 ` Dai, Wei
  2017-03-30 11:58                   ` Olivier Matz
  0 siblings, 1 reply; 13+ messages in thread
From: Dai, Wei @ 2017-03-30 10:46 UTC (permalink / raw)
  To: Olivier Matz
  Cc: Thomas Monjalon, Zhang, Helin, Ananyev, Konstantin, dev,
	Guo Fengtian, Yigit, Ferruh

HI, Olivier

> 
> Hi Wei,
> 
> On Wed, 29 Mar 2017 09:22:55 +0000, "Dai, Wei" <wei.dai@intel.com> wrote:
> > First of all, I don't agree the method in this patch because the
> > register can also rewind to 0 in normal mode except reset/PF down.
> >
> > In the function void ixgbe_down(struct ixgbe_adapter *adapter) of
> > ixgbe_main.c in the ixgbe kernel PF driver (version 5.0.4), the PF in kernel
> driver will ping all the active vfs to let them know PF is going down by mailbox
> messages.
> >
> > On other side, ixgbe VF in DPDK PMD will handle the ping mailbox
> > message in ixgbevf_dev_interrupt_handler( ) which is registered interrupt
> routine.
> > And ixgbevf_dev_interrupt_handler( ) will call ixgbevf_mbx_process( ) to
> process mailbox message from PF.
> >
> > So it is in ixgbevf_mbx_process( ) where the SW stats registers of VF can be
> fixed.
> > There is more than one line to send ping message to VF in kernel PF drvier
> codes.
> 
> Thank you for the explanation.
> 
> Just to be sure I correctly understand:
> 1- the ping message is properly sent by ixgbe pf linux driver
Yes, I think so.

> 2- the ping message is not yet sent by ixgbe pf dpdk driver
I have not find any codes in DPDK PF to send them.

> 3- the ping message is properly received by ixgbe vf dpdk driver, but
>    it is not used yet to stop wrong stats accumulation
Yes, need register some callback function on to handle it.

> 
> > I am studying it to make it clear how to identify the ping message due to PF
> down.
> 
> Are you saying that you are currently working on a fix for 2 and 3 (that would be
> great!) or are you expecting that I work on it? In any case, I can help, at least
> to test the patches.

I am working on it now. Need check both ixgbe kernel driver and DPDK driver and also
need look up some pages in datasheet including 82599, X540 and X550.
So it is not an easy task.
By the way, not only statistics function but also other configurations should be 
restored.

Anyway, I always appreciate your help. Of course, you can help me test my upcoming patches.

Thanks 
-Wei

> 
> 
> > So can we defer this patch first.
> > And another patch in above way should be submitted.
> 
> 
> I'm of course ok to defer the initial patch, I agree it's more a workaround than a
> proper solution. I was stuck because I did not know how the VF is notified by
> the PF when it is reset.
> 
> 
> Thanks
> Olivier

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net/ixgbevf: fix stats update after a PF reset
  2017-03-30 10:46                 ` Dai, Wei
@ 2017-03-30 11:58                   ` Olivier Matz
  0 siblings, 0 replies; 13+ messages in thread
From: Olivier Matz @ 2017-03-30 11:58 UTC (permalink / raw)
  To: Dai, Wei
  Cc: Thomas Monjalon, Zhang, Helin, Ananyev, Konstantin, dev,
	Guo Fengtian, Yigit, Ferruh

Hi Wei,

On Thu, 30 Mar 2017 10:46:45 +0000, "Dai, Wei" <wei.dai@intel.com> wrote:
> HI, Olivier
> 
> > 
> > Hi Wei,
> > 
> > On Wed, 29 Mar 2017 09:22:55 +0000, "Dai, Wei" <wei.dai@intel.com> wrote:  
> > > First of all, I don't agree the method in this patch because the
> > > register can also rewind to 0 in normal mode except reset/PF down.
> > >
> > > In the function void ixgbe_down(struct ixgbe_adapter *adapter) of
> > > ixgbe_main.c in the ixgbe kernel PF driver (version 5.0.4), the PF in kernel  
> > driver will ping all the active vfs to let them know PF is going down by mailbox
> > messages.  
> > >
> > > On other side, ixgbe VF in DPDK PMD will handle the ping mailbox
> > > message in ixgbevf_dev_interrupt_handler( ) which is registered interrupt  
> > routine.  
> > > And ixgbevf_dev_interrupt_handler( ) will call ixgbevf_mbx_process( ) to  
> > process mailbox message from PF.  
> > >
> > > So it is in ixgbevf_mbx_process( ) where the SW stats registers of VF can be  
> > fixed.  
> > > There is more than one line to send ping message to VF in kernel PF drvier  
> > codes.
> > 
> > Thank you for the explanation.
> > 
> > Just to be sure I correctly understand:
> > 1- the ping message is properly sent by ixgbe pf linux driver  
> Yes, I think so.
> 
> > 2- the ping message is not yet sent by ixgbe pf dpdk driver  
> I have not find any codes in DPDK PF to send them.
> 
> > 3- the ping message is properly received by ixgbe vf dpdk driver, but
> >    it is not used yet to stop wrong stats accumulation  
> Yes, need register some callback function on to handle it.
> 
> >   
> > > I am studying it to make it clear how to identify the ping message due to PF  
> > down.
> > 
> > Are you saying that you are currently working on a fix for 2 and 3 (that would be
> > great!) or are you expecting that I work on it? In any case, I can help, at least
> > to test the patches.  
> 
> I am working on it now. Need check both ixgbe kernel driver and DPDK driver and also
> need look up some pages in datasheet including 82599, X540 and X550.
> So it is not an easy task.
> By the way, not only statistics function but also other configurations should be 
> restored.
> 
> Anyway, I always appreciate your help. Of course, you can help me test my upcoming patches.

Great, thank you.
Please CC me when you send the patches, I'll test them.

Regards,
Olivier

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-03-30 11:58 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-11 17:04 [PATCH] net/ixgbevf: fix stats update after a PF reset Olivier Matz
2017-02-06 13:58 ` Olivier Matz
2017-02-09 14:37   ` Dai, Wei
2017-02-09 14:50   ` Dai, Wei
2017-02-16 16:49     ` Olivier Matz
2017-03-14  9:46       ` Olivier Matz
2017-03-24 14:11         ` Olivier Matz
2017-03-28 15:22           ` Thomas Monjalon
2017-03-29  9:22             ` Dai, Wei
2017-03-29 10:08               ` Olivier Matz
2017-03-30 10:46                 ` Dai, Wei
2017-03-30 11:58                   ` Olivier Matz
2017-03-29  9:37             ` Dai, Wei

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.