diff mbox series

[DPDK] net/ice: fix Rx statistics

Message ID 1556089570-44487-1-git-send-email-simei.su@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Qi Zhang
Headers
Series [DPDK] net/ice: fix Rx statistics |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK
ci/intel-Performance-Testing success Performance Testing PASS
ci/mellanox-Performance-Testing success Performance Testing PASS

Commit Message

Simei Su April 24, 2019, 7:06 a.m. UTC 
  The RX stats will increase even no packets sent, this patch fix this issue
by modifying ipackets and ibytes statistics based on vsi instead of port
to avoid statistics error.

Fixes: a37bde56314d ("net/ice: support statistics")
Cc: stable@dpdk.org

Signed-off-by: Simei Su <simei.su@intel.com>
---
 drivers/net/ice/ice_ethdev.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comments

Qi Zhang April 25, 2019, 7:06 a.m. UTC | #1 
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of simei
> Sent: Wednesday, April 24, 2019 3:06 PM
> To: qi.zi.zhang@intel.com; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Yang,
> Qiming <qiming.yang@intel.com>
> Cc: dev@dpdk.org; Su, Simei <simei.su@intel.com>; stable@dpdk.org
> Subject: [dpdk-dev] [DPDK] net/ice: fix Rx statistics
> 
> The RX stats will increase even no packets sent, this patch fix this issue by
> modifying ipackets and ibytes statistics based on vsi instead of port to avoid
> statistics error.
> 
> Fixes: a37bde56314d ("net/ice: support statistics")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Simei Su <simei.su@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi
David Marchand April 25, 2019, 7:12 a.m. UTC | #2 
On Wed, Apr 24, 2019 at 9:47 AM simei <simei.su@intel.com> wrote:

> The RX stats will increase even no packets sent, this patch fix this issue
> by modifying ipackets and ibytes statistics based on vsi instead of port
> to avoid statistics error.
>
> Fixes: a37bde56314d ("net/ice: support statistics")
> Cc: stable@dpdk.org
>
> Signed-off-by: Simei Su <simei.su@intel.com>
> ---
>  drivers/net/ice/ice_ethdev.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
> index 0946b19..1c851ac 100644
> --- a/drivers/net/ice/ice_ethdev.c
> +++ b/drivers/net/ice/ice_ethdev.c
> @@ -3305,15 +3305,14 @@ static int ice_rx_queue_intr_disable(struct
> rte_eth_dev *dev,
>         /* call read registers - updates values, now write them to struct
> */
>         ice_read_stats_registers(pf, hw);
>
> -       stats->ipackets = ns->eth.rx_unicast +
> -                         ns->eth.rx_multicast +
> -                         ns->eth.rx_broadcast -
> -                         ns->eth.rx_discards -
> +       stats->ipackets = pf->main_vsi->eth_stats.rx_unicast +
> +                         pf->main_vsi->eth_stats.rx_multicast +
> +                         pf->main_vsi->eth_stats.rx_broadcast -
>                           pf->main_vsi->eth_stats.rx_discards;
>         stats->opackets = ns->eth.tx_unicast +
>                           ns->eth.tx_multicast +
>                           ns->eth.tx_broadcast;
> -       stats->ibytes   = ns->eth.rx_bytes;
> +       stats->ibytes   = pf->main_vsi->eth_stats.rx_bytes;
>         stats->obytes   = ns->eth.tx_bytes;
>         stats->oerrors  = ns->eth.tx_errors +
>                           pf->main_vsi->eth_stats.tx_errors;
> --
> 1.8.3.1
>
>
I don't know this hw nor this code.
However, from the description and this snippet, it looks like
opackets/obytes/oerrors are still referring to port stats instead of the pf
stats.
Can you elaborate ?

Thanks.
Qi Zhang April 25, 2019, 7:30 a.m. UTC | #3 
Hi David:

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of David Marchand
> Sent: Thursday, April 25, 2019 3:13 PM
> To: Su, Simei <simei.su@intel.com>
> Cc: qi.zi.zhang@intel.com; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Yang,
> Qiming <qiming.yang@intel.com>; dev <dev@dpdk.org>; dpdk stable
> <stable@dpdk.org>
> Subject: Re: [dpdk-dev] [DPDK] net/ice: fix Rx statistics
> 
> On Wed, Apr 24, 2019 at 9:47 AM simei <simei.su@intel.com> wrote:
> 
> > The RX stats will increase even no packets sent, this patch fix this
> > issue by modifying ipackets and ibytes statistics based on vsi instead
> > of port to avoid statistics error.
> >
> > Fixes: a37bde56314d ("net/ice: support statistics")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Simei Su <simei.su@intel.com>
> > ---
> >  drivers/net/ice/ice_ethdev.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ice/ice_ethdev.c
> > b/drivers/net/ice/ice_ethdev.c index 0946b19..1c851ac 100644
> > --- a/drivers/net/ice/ice_ethdev.c
> > +++ b/drivers/net/ice/ice_ethdev.c
> > @@ -3305,15 +3305,14 @@ static int ice_rx_queue_intr_disable(struct
> > rte_eth_dev *dev,
> >         /* call read registers - updates values, now write them to
> > struct */
> >         ice_read_stats_registers(pf, hw);
> >
> > -       stats->ipackets = ns->eth.rx_unicast +
> > -                         ns->eth.rx_multicast +
> > -                         ns->eth.rx_broadcast -
> > -                         ns->eth.rx_discards -
> > +       stats->ipackets = pf->main_vsi->eth_stats.rx_unicast +
> > +                         pf->main_vsi->eth_stats.rx_multicast +
> > +                         pf->main_vsi->eth_stats.rx_broadcast -
> >                           pf->main_vsi->eth_stats.rx_discards;
> >         stats->opackets = ns->eth.tx_unicast +
> >                           ns->eth.tx_multicast +
> >                           ns->eth.tx_broadcast;
> > -       stats->ibytes   = ns->eth.rx_bytes;
> > +       stats->ibytes   = pf->main_vsi->eth_stats.rx_bytes;
> >         stats->obytes   = ns->eth.tx_bytes;
> >         stats->oerrors  = ns->eth.tx_errors +
> >                           pf->main_vsi->eth_stats.tx_errors;
> > --
> > 1.8.3.1
> >
> >
> I don't know this hw nor this code.
> However, from the description and this snippet, it looks like
> opackets/obytes/oerrors are still referring to port stats instead of the pf stats.
> Can you elaborate ?

For Rx, packet comes to port then vsi,  so we count vsi->rx for how many valid packet we received.
For Tx, packet comes to vsi then port,  so we count port->tx for how many packet we exactly sent out?

Does this make sense?

Btw, this is the same way we handled statistics on i40e.
  
Regards
Qi

> 
> Thanks.
> 
> 
> --
> David Marchand
David Marchand April 25, 2019, 7:34 a.m. UTC | #4 
On Thu, Apr 25, 2019 at 9:30 AM Zhang, Qi Z <qi.z.zhang@intel.com> wrote:

> Hi David:
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of David Marchand
> > Sent: Thursday, April 25, 2019 3:13 PM
> > To: Su, Simei <simei.su@intel.com>
> > Cc: qi.zi.zhang@intel.com; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Yang,
> > Qiming <qiming.yang@intel.com>; dev <dev@dpdk.org>; dpdk stable
> > <stable@dpdk.org>
> > Subject: Re: [dpdk-dev] [DPDK] net/ice: fix Rx statistics
> >
> > On Wed, Apr 24, 2019 at 9:47 AM simei <simei.su@intel.com> wrote:
> >
> > > The RX stats will increase even no packets sent, this patch fix this
> > > issue by modifying ipackets and ibytes statistics based on vsi instead
> > > of port to avoid statistics error.
> > >
> > > Fixes: a37bde56314d ("net/ice: support statistics")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Simei Su <simei.su@intel.com>
> > > ---
> > >  drivers/net/ice/ice_ethdev.c | 9 ++++-----
> > >  1 file changed, 4 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/net/ice/ice_ethdev.c
> > > b/drivers/net/ice/ice_ethdev.c index 0946b19..1c851ac 100644
> > > --- a/drivers/net/ice/ice_ethdev.c
> > > +++ b/drivers/net/ice/ice_ethdev.c
> > > @@ -3305,15 +3305,14 @@ static int ice_rx_queue_intr_disable(struct
> > > rte_eth_dev *dev,
> > >         /* call read registers - updates values, now write them to
> > > struct */
> > >         ice_read_stats_registers(pf, hw);
> > >
> > > -       stats->ipackets = ns->eth.rx_unicast +
> > > -                         ns->eth.rx_multicast +
> > > -                         ns->eth.rx_broadcast -
> > > -                         ns->eth.rx_discards -
> > > +       stats->ipackets = pf->main_vsi->eth_stats.rx_unicast +
> > > +                         pf->main_vsi->eth_stats.rx_multicast +
> > > +                         pf->main_vsi->eth_stats.rx_broadcast -
> > >                           pf->main_vsi->eth_stats.rx_discards;
> > >         stats->opackets = ns->eth.tx_unicast +
> > >                           ns->eth.tx_multicast +
> > >                           ns->eth.tx_broadcast;
> > > -       stats->ibytes   = ns->eth.rx_bytes;
> > > +       stats->ibytes   = pf->main_vsi->eth_stats.rx_bytes;
> > >         stats->obytes   = ns->eth.tx_bytes;
> > >         stats->oerrors  = ns->eth.tx_errors +
> > >                           pf->main_vsi->eth_stats.tx_errors;
> > > --
> > > 1.8.3.1
> > >
> > >
> > I don't know this hw nor this code.
> > However, from the description and this snippet, it looks like
> > opackets/obytes/oerrors are still referring to port stats instead of the
> pf stats.
> > Can you elaborate ?
>
> For Rx, packet comes to port then vsi,  so we count vsi->rx for how many
> valid packet we received.
> For Tx, packet comes to vsi then port,  so we count port->tx for how many
> packet we exactly sent out?
>
> Does this make sense?
>

Well, sorry, but no :-)
But if you are sure this is the right way, go with it.
diff mbox series

Patch

diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 0946b19..1c851ac 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -3305,15 +3305,14 @@  static int ice_rx_queue_intr_disable(struct rte_eth_dev *dev,
 	/* call read registers - updates values, now write them to struct */
 	ice_read_stats_registers(pf, hw);
 
-	stats->ipackets = ns->eth.rx_unicast +
-			  ns->eth.rx_multicast +
-			  ns->eth.rx_broadcast -
-			  ns->eth.rx_discards -
+	stats->ipackets = pf->main_vsi->eth_stats.rx_unicast +
+			  pf->main_vsi->eth_stats.rx_multicast +
+			  pf->main_vsi->eth_stats.rx_broadcast -
 			  pf->main_vsi->eth_stats.rx_discards;
 	stats->opackets = ns->eth.tx_unicast +
 			  ns->eth.tx_multicast +
 			  ns->eth.tx_broadcast;
-	stats->ibytes   = ns->eth.rx_bytes;
+	stats->ibytes   = pf->main_vsi->eth_stats.rx_bytes;
 	stats->obytes   = ns->eth.tx_bytes;
 	stats->oerrors  = ns->eth.tx_errors +
 			  pf->main_vsi->eth_stats.tx_errors;