From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id CA60841B86; Tue, 31 Jan 2023 02:51:55 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5E0DB40EF0; Tue, 31 Jan 2023 02:51:55 +0100 (CET) Received: from mail-vs1-f48.google.com (mail-vs1-f48.google.com [209.85.217.48]) by mails.dpdk.org (Postfix) with ESMTP id BC4B740DFB for ; Tue, 31 Jan 2023 02:51:53 +0100 (CET) Received: by mail-vs1-f48.google.com with SMTP id h19so12822720vsv.13 for ; Mon, 30 Jan 2023 17:51:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=5X1iHwpISoTHbuWRhDkZfbXPdKdPcbkNndgUprCyr2U=; b=LcvXMuG8mo5UDkz3Mk7iZtCDo7w/+0YlVmpZug6G1JnmvQCf91IklxKSOt3y9Ll34j jt7LXBVl4ja768CE83YBdgmds1X4ut0eOhws+xDwSWMDNj4WwGajeFJn28eZu0CBNAwP pjDwE3ebEVlG3COWQTaczwEwdIJf1v2q99SZJAEl2YWuzi3gy776+SD2gYfZMA5IsKCi dVCY9yDK0yzkRnaJnXsUBzCif2Ty95S9npvVCYGyHDv0SawQBbHwsBa9jc7O/8L1CEJv e0g4Cb3dml44MVVgkvP/t0Zq7pxguSwKFqGFkpPhZ5oAcuvNMqk3aGtGIClcvNxZ6Ks7 WI0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=5X1iHwpISoTHbuWRhDkZfbXPdKdPcbkNndgUprCyr2U=; b=p6k1wkpGaH+YDGJR21D2gnqwHrhuR5MZ3qkb4lgdl47OJOznQVjKregOxSwy5bXKee QghXJO86+faBUwpjcWH+qRL/k2382BD4rTGjqU1LIBn1imPLnIOFF+xeZJ5jGV58aAR0 0Vd9Csy6GoEyrzQA2Id/A8k0sDlt8wZIMa/y1UV4dYJ5KWekJphylxdczw0zGWbhig8r hFgd7FDdyi1wI5lTe69OPd24HsXo5+R/XFUT4mO3BFYGKJKeQ/m288lwGogDH4Xx/N+u 12tmYChUlO+lure1f4JWfyeBrBeg6l5vJZIiFlsUMswFf2b610WFdUyarhrf254uGGiU +glw== X-Gm-Message-State: AFqh2kpN2vBYaFVhQQUkTFVms9XnVOTQbkKIJTmDRSqYX0G2RSfLT3Ng oB+R9oT24qDiOccJ0ELCK5+yYhW4OvrJxL70kPvC/g== X-Google-Smtp-Source: AMrXdXttyMob7Mx42WwN4Z13zfOieD6YVWiczEoNDnP5RGe/Sv4IeQkZJV9fi9nxz7S1ymnIZyxxuvM8oOlYladY5t0= X-Received: by 2002:a05:6102:356e:b0:3d2:4b17:1395 with SMTP id bh14-20020a056102356e00b003d24b171395mr7116902vsb.10.1675129912918; Mon, 30 Jan 2023 17:51:52 -0800 (PST) MIME-Version: 1.0 References: <20221124073335.3985214-1-junfeng.guo@intel.com> <20221207083948.22af7cae@hermes.local> <78fcfa68-120b-af64-fb42-aa2031d45592@amd.com> In-Reply-To: <78fcfa68-120b-af64-fb42-aa2031d45592@amd.com> From: Joshua Washington Date: Mon, 30 Jan 2023 17:51:42 -0800 Message-ID: Subject: Re: [PATCH] net/gve: add support for basic stats To: Ferruh Yigit Cc: Rushil Gupta , Stephen Hemminger , Junfeng Guo , qi.z.zhang@intel.com, jingjing.wu@intel.com, beilei.xing@intel.com, dev@dpdk.org, jeroendb@google.com, jrkim@google.com, Xiaoyun Li Content-Type: multipart/alternative; boundary="00000000000069f1fe05f38593ab" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org --00000000000069f1fe05f38593ab Content-Type: text/plain; charset="UTF-8" Hello, I tested it out, and the updates to testpmd seem to work. Before applying the second patch: ---------------------- Forward statistics for port 0 ---------------------- RX-packets: 0 RX-dropped: 0 RX-total: 0 TX-packets: 43769238 TX-dropped: 62634 TX-total: 43831872 ---------------------------------------------------------------------------- ---------------------- Forward statistics for port 1 ---------------------- RX-packets: 0 RX-dropped: 0 RX-total: 0 TX-packets: 43761119 TX-dropped: 70753 TX-total: 43831872 ---------------------------------------------------------------------------- +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++ RX-packets: 0 RX-dropped: 0 RX-total: 0 TX-packets: 87530357 TX-dropped: 157302 TX-total: 87687659 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 62634 + 70753 = 133387 != 157302 After applying the second patch: ---------------------- Forward statistics for port 0 ---------------------- RX-packets: 0 RX-dropped: 0 RX-total: 0 TX-packets: 12590721 TX-dropped: 36638 TX-total: 12627359 ---------------------------------------------------------------------------- ---------------------- Forward statistics for port 1 ---------------------- RX-packets: 0 RX-dropped: 0 RX-total: 0 TX-packets: 12596255 TX-dropped: 31746 TX-total: 12628001 ---------------------------------------------------------------------------- +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++ RX-packets: 0 RX-dropped: 0 RX-total: 0 TX-packets: 25186976 TX-dropped: 68384 TX-total: 25255360 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Thanks, Josh On Wed, Jan 18, 2023 at 8:22 AM Ferruh Yigit wrote: > On 12/19/2022 7:38 PM, Joshua Washington wrote: > > Hello, > > > > As it turns out, this error actually propagates to the "total" stats as > > well, which I assume is just calculated by adding TX-packets and > > TX-dropped. Here are the full stats from the example that Rushil > mentioned: > > > > ---------------------- Forward statistics for port 0 > > ---------------------- > > RX-packets: 2453802 RX-dropped: 0 RX-total: 2453802 > > TX-packets: 34266881 TX-dropped: 447034 TX-total: 34713915 > > > > > ---------------------------------------------------------------------------- > > > > ---------------------- Forward statistics for port 1 > > ---------------------- > > RX-packets: 34713915 RX-dropped: 0 RX-total: 34713915 > > TX-packets: 2453802 TX-dropped: 0 TX-total: 2453802 > > > > > ---------------------------------------------------------------------------- > > > > +++++++++++++++ Accumulated forward statistics for all > > ports+++++++++++++++ > > RX-packets: 37167717 RX-dropped: 0 RX-total: 37167717 > > TX-packets: 36720683 TX-dropped: 807630 TX-total: 37528313 > > > > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > > > It can be seen that the stats for the individual ports are consistent, > > but the TX-total and TX-dropped are not consistent with the stats for > > the individual ports, as I believe that the TX-total and RX-total > > accumulated stats should be equal. > > > > Hi Joshua, Rushil, > > As I checked for it, issue may be related to testpmd stats display, > > While displaying per port TX-dropped value, it only takes > 'ports_stats[pt_id].tx_dropped' into account, > but for accumulated TX-dropped results it takes both > 'ports_stats[pt_id].tx_dropped' & 'stats.oerrors' into account. > > If you can reproduce it easily, can you please test with following change: > > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c > index 134d79a55547..49322d07d7d6 100644 > --- a/app/test-pmd/testpmd.c > +++ b/app/test-pmd/testpmd.c > @@ -2056,6 +2056,8 @@ fwd_stats_display(void) > fwd_cycles += fs->core_cycles; > } > for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) { > + uint64_t tx_dropped = 0; > + > pt_id = fwd_ports_ids[i]; > port = &ports[pt_id]; > > @@ -2077,8 +2079,9 @@ fwd_stats_display(void) > total_recv += stats.ipackets; > total_xmit += stats.opackets; > total_rx_dropped += stats.imissed; > - total_tx_dropped += ports_stats[pt_id].tx_dropped; > - total_tx_dropped += stats.oerrors; > + tx_dropped += ports_stats[pt_id].tx_dropped; > + tx_dropped += stats.oerrors; > + total_tx_dropped += tx_dropped; > total_rx_nombuf += stats.rx_nombuf; > > printf("\n %s Forward statistics for port %-2d %s\n", > @@ -2105,8 +2108,8 @@ fwd_stats_display(void) > > printf(" TX-packets: %-14"PRIu64" TX-dropped: %-14"PRIu64 > "TX-total: %-"PRIu64"\n", > - stats.opackets, ports_stats[pt_id].tx_dropped, > - stats.opackets + ports_stats[pt_id].tx_dropped); > + stats.opackets, tx_dropped, > + stats.opackets + tx_dropped); > > if (record_burst_stats) { > if (ports_stats[pt_id].rx_stream) > > > > > > On Mon, Dec 19, 2022 at 11:17 AM Rushil Gupta > > wrote: > > > > Hi all > > Josh just found out some inconsistencies in the Tx/Rx statistics sum > > for all ports. Not sure if we can screenshot here but it goes like > > this: > > Tx-dropped for port0: 447034 > > Tx-dropped for port1: 0 > > Accumulated forward statistics for all ports: 807630 > > > > Please note that this issue is only with Tx-dropped (not > > Tx-packets/Tx-total). > > > > > > On Wed, Dec 7, 2022 at 8:39 AM Stephen Hemminger > > > > wrote: > > > > > > On Wed, 7 Dec 2022 15:09:08 +0000 > > > Ferruh Yigit > > > wrote: > > > > > > > On 11/24/2022 7:33 AM, Junfeng Guo wrote: > > > > > Add support for dev_ops of stats_get and stats_reset. > > > > > > > > > > Queue stats update will be moved into xstat [1], but the basic > > stats > > > > > items may still be required. So we just keep the remaining > > ones and > > > > > will implement the queue stats via xstats in the coming > release. > > > > > > > > > > [1] > > > > > https://elixir.bootlin.com/dpdk/v22.07/ > > \ > > > > > source/doc/guides/rel_notes/deprecation.rst#L118 > > > > > > > > > > Signed-off-by: Xiaoyun Li > > > > > > > Signed-off-by: Junfeng Guo > > > > > > > > > > <...> > > > > > > > > > +static int > > > > > +gve_dev_stats_get(struct rte_eth_dev *dev, struct > > rte_eth_stats *stats) > > > > > +{ > > > > > + uint16_t i; > > > > > + > > > > > + for (i = 0; i < dev->data->nb_tx_queues; i++) { > > > > > + struct gve_tx_queue *txq = dev->data->tx_queues[i]; > > > > > + if (txq == NULL) > > > > > + continue; > > > > > + > > > > > + stats->opackets += txq->packets; > > > > > + stats->obytes += txq->bytes; > > > > > + stats->oerrors += txq->errors; > > > > > > > > Hi Junfeng, Qi, Jingjing, Beilei, > > > > > > > > Above logic looks wrong to me, did you test it? > > > > > > > > If the 'gve_dev_stats_get()' called multiple times (without > > stats reset > > > > in between), same values will be keep added to stats. > > > > Some hw based implementations does this, because reading from > stats > > > > registers automatically reset those registers but this shouldn't > > be case > > > > for this driver. > > > > > > > > I expect it to be something like: > > > > > > > > local_stats = 0 > > > > foreach queue > > > > local_stats += queue->stats > > > > stats = local_stats > > > > > > The zero of local_stats is unnecessary. > > > The only caller of the PMD stats_get is rte_ethdev_stats_get > > > and it zeros the stats structure before calling the PMD. > > > > > > > > > int > > > rte_eth_stats_get(uint16_t port_id, struct rte_eth_stats *stats) > > > { > > > struct rte_eth_dev *dev; > > > > > > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > > > dev = &rte_eth_devices[port_id]; > > > > > > memset(stats, 0, sizeof(*stats)); > > > ... > > > stats->rx_nombuf = dev->data->rx_mbuf_alloc_failed; > > > return eth_err(port_id, (*dev->dev_ops->stats_get)(dev, > > stats)); > > > > > > If any PMD has extra memset in their stats get that could be > removed. > > > > > > > > -- > > > > Joshua Washington | Software Engineer | joshwash@google.com > > | (414) 366-4423 > > > > -- Joshua Washington | Software Engineer | joshwash@google.com | (414) 366-4423 --00000000000069f1fe05f38593ab Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hello,

I tested it out, and the updates= to testpmd seem to work.

Before applying the second patch:
=C2=A0 ---------------------- Forward statistics= for port 0 =C2=A0----------------------
=C2=A0 RX-packets: 0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0RX-dropped: 0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 RX-total: 0
=C2=A0 TX-packets: 43769238 =C2=A0 =C2= =A0 =C2=A0 TX-dropped: 62634 =C2=A0 =C2=A0 =C2=A0 =C2=A0 TX-total: 43831872=
=C2=A0 ----------------------------------------------------------------= ------------

=C2=A0 ---------------------- Forward statistics for po= rt 1 =C2=A0----------------------
=C2=A0 RX-packets: 0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0RX-dropped: 0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 RX-total: 0
=C2=A0 TX-packets: 43761119 =C2=A0 =C2=A0 =C2= =A0 TX-dropped: 70753 =C2=A0 =C2=A0 =C2=A0 =C2=A0 TX-total: 43831872
=C2= =A0 -----------------------------------------------------------------------= -----

=C2=A0 +++++++++++++++ Accumulated forward statistics for all = ports+++++++++++++++
=C2=A0 RX-packets: 0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0RX-dropped: 0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= RX-total: 0
=C2=A0 TX-packets: 87530357 =C2=A0 =C2=A0 =C2=A0 TX-dropped= : 157302 =C2=A0 =C2=A0 =C2=A0 =C2=A0TX-total: 87687659
=C2=A0 ++++++++++= ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

62634=C2=A0+ 70753 =3D 133387 !=3D 157302

After applying the second patch:
=C2=A0 ---------------------- Forward statistics for po= rt 0 =C2=A0----------------------
=C2=A0 RX-packets: 0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0RX-dropped: 0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 RX-total: 0
=C2=A0 TX-packets: 12590721 =C2=A0 =C2=A0 =C2= =A0 TX-dropped: 36638 =C2=A0 =C2=A0 =C2=A0 =C2=A0 TX-total: 12627359
=C2= =A0 -----------------------------------------------------------------------= -----

=C2=A0 ---------------------- Forward statistics for port 1 = =C2=A0----------------------
=C2=A0 RX-packets: 0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0RX-dropped: 0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 RX-total: 0
=C2=A0 TX-packets: 12596255 =C2=A0 =C2=A0 =C2=A0 TX-= dropped: 31746 =C2=A0 =C2=A0 =C2=A0 =C2=A0 TX-total: 12628001
=C2=A0 ---= -------------------------------------------------------------------------
=C2=A0 +++++++++++++++ Accumulated forward statistics for all ports++= +++++++++++++
=C2=A0 RX-packets: 0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0RX-dropped: 0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 RX-tot= al: 0
=C2=A0 TX-packets: 25186976 =C2=A0 =C2=A0 =C2=A0 TX-dropped: 68384= =C2=A0 =C2=A0 =C2=A0 =C2=A0 TX-total: 25255360
=C2=A0 +++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

Thanks,
Josh

On Wed, Jan 18, 2023 at 8:22 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
On 12/19/2022 7:38 PM, Joshua Wash= ington wrote:
> Hello,
>
> As it turns out, this error actually propagates to the "total&quo= t; stats as
> well,=C2=A0which I assume is just calculated by adding TX-packets and<= br> > TX-dropped. Here are the full stats from the example that Rushil menti= oned:
>
> =C2=A0 ---------------------- Forward statistics for port 0
> =C2=A0----------------------
> =C2=A0 RX-packets: 2453802 =C2=A0 =C2=A0 =C2=A0 =C2=A0RX-dropped: 0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 RX-total: 2453802
> =C2=A0 TX-packets: 34266881 =C2=A0 =C2=A0 =C2=A0 TX-dropped: 447034 = =C2=A0 =C2=A0 =C2=A0 =C2=A0TX-total: 34713915
> =C2=A0
> ----------------------------------------------------------------------= ------
>
> =C2=A0 ---------------------- Forward statistics for port 1
> =C2=A0----------------------
> =C2=A0 RX-packets: 34713915 =C2=A0 =C2=A0 =C2=A0 RX-dropped: 0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 RX-total: 34713915
> =C2=A0 TX-packets: 2453802 =C2=A0 =C2=A0 =C2=A0 =C2=A0TX-dropped: 0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 TX-total: 2453802
> =C2=A0
> ----------------------------------------------------------------------= ------
>
> =C2=A0 +++++++++++++++ Accumulated forward statistics for all
> ports+++++++++++++++
> =C2=A0 RX-packets: 37167717 =C2=A0 =C2=A0 =C2=A0 RX-dropped: 0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 RX-total: 37167717
> =C2=A0 TX-packets: 36720683 =C2=A0 =C2=A0 =C2=A0 TX-dropped: 807630 = =C2=A0 =C2=A0 =C2=A0 =C2=A0TX-total: 37528313
> =C2=A0
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++= ++++++
>
> It can be seen that the stats for the individual ports are consistent,=
> but the TX-total and TX-dropped are not consistent with the stats for<= br> > the individual ports,=C2=A0as I believe that the TX-total and RX-total=
> accumulated stats should be equal.
>

Hi Joshua, Rushil,

As I checked for it, issue may be related to testpmd stats display,

While displaying per port TX-dropped value, it only takes
'ports_stats[pt_id].tx_dropped' into account,
but for accumulated TX-dropped results it takes both
'ports_stats[pt_id].tx_dropped' & 'stats.oerrors' into = account.

If you can reproduce it easily, can you please test with following change:<= br>
=C2=A0diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
=C2=A0index 134d79a55547..49322d07d7d6 100644
=C2=A0--- a/app/test-pmd/testpmd.c
=C2=A0+++ b/app/test-pmd/testpmd.c
=C2=A0@@ -2056,6 +2056,8 @@ fwd_stats_display(void)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0fwd_cycles +=3D fs->core_cycles;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0for (i =3D 0; i < cur_fwd_config.nb_fw= d_ports; i++) {
=C2=A0+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0uint64_t tx_d= ropped =3D 0;
=C2=A0+
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0pt_id =3D fwd= _ports_ids[i];
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0port =3D &= ;ports[pt_id];

=C2=A0@@ -2077,8 +2079,9 @@ fwd_stats_display(void)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0total_recv += =3D stats.ipackets;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0total_xmit += =3D stats.opackets;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0total_rx_drop= ped +=3D stats.imissed;
=C2=A0-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0total_tx_drop= ped +=3D ports_stats[pt_id].tx_dropped;
=C2=A0-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0total_tx_drop= ped +=3D stats.oerrors;
=C2=A0+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0tx_dropped += =3D ports_stats[pt_id].tx_dropped;
=C2=A0+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0tx_dropped += =3D stats.oerrors;
=C2=A0+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0total_tx_drop= ped +=3D tx_dropped;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0total_rx_nomb= uf=C2=A0 +=3D stats.rx_nombuf;

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0printf("= \n=C2=A0 %s Forward statistics for port %-2d %s\n",
=C2=A0@@ -2105,8 +2108,8 @@ fwd_stats_display(void)

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0printf("= =C2=A0 TX-packets: %-14"PRIu64" TX-dropped: %-14"PRIu64
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 "TX-total: %-"PRIu64"\n",
=C2=A0-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 stats.opackets, ports_stats[pt_id].tx_dropped,
=C2=A0-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 stats.opackets + ports_stats[pt_id].tx_dropped);
=C2=A0+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 stats.opackets, tx_dropped,
=C2=A0+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 stats.opackets + tx_dropped);

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (record_bu= rst_stats) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0if (ports_stats[pt_id].rx_stream)


>
> On Mon, Dec 19, 2022 at 11:17 AM Rushil Gupta <rushilg@google.com
> <mailto:rus= hilg@google.com>> wrote:
>
>=C2=A0 =C2=A0 =C2=A0Hi all
>=C2=A0 =C2=A0 =C2=A0Josh just found out some inconsistencies in the Tx/= Rx statistics sum
>=C2=A0 =C2=A0 =C2=A0for all ports. Not sure if we can screenshot here b= ut it goes like
>=C2=A0 =C2=A0 =C2=A0this:
>=C2=A0 =C2=A0 =C2=A0Tx-dropped for port0: 447034
>=C2=A0 =C2=A0 =C2=A0Tx-dropped for port1: 0
>=C2=A0 =C2=A0 =C2=A0Accumulated forward statistics for all ports: 80763= 0
>
>=C2=A0 =C2=A0 =C2=A0Please note that this issue is only with Tx-dropped= (not
>=C2=A0 =C2=A0 =C2=A0Tx-packets/Tx-total).
>
>
>=C2=A0 =C2=A0 =C2=A0On Wed, Dec 7, 2022 at 8:39 AM Stephen Hemminger >=C2=A0 =C2=A0 =C2=A0<stephen@networkplumber.org <mailto:stephen@networkplumber.org<= /a>>> wrote:
>=C2=A0 =C2=A0 =C2=A0>
>=C2=A0 =C2=A0 =C2=A0> On Wed, 7 Dec 2022 15:09:08 +0000
>=C2=A0 =C2=A0 =C2=A0> Ferruh Yigit <ferruh.yigit@amd.com <mailto:ferruh.yigit@amd.com>= ;>
>=C2=A0 =C2=A0 =C2=A0wrote:
>=C2=A0 =C2=A0 =C2=A0>
>=C2=A0 =C2=A0 =C2=A0> > On 11/24/2022 7:33 AM, Junfeng Guo wrote:=
>=C2=A0 =C2=A0 =C2=A0> > > Add support for dev_ops of stats_get= and stats_reset.
>=C2=A0 =C2=A0 =C2=A0> > >
>=C2=A0 =C2=A0 =C2=A0> > > Queue stats update will be moved int= o xstat [1], but the basic
>=C2=A0 =C2=A0 =C2=A0stats
>=C2=A0 =C2=A0 =C2=A0> > > items may still be required. So we j= ust keep the remaining
>=C2=A0 =C2=A0 =C2=A0ones and
>=C2=A0 =C2=A0 =C2=A0> > > will implement the queue stats via x= stats in the coming release.
>=C2=A0 =C2=A0 =C2=A0> > >
>=C2=A0 =C2=A0 =C2=A0> > > [1]
>=C2=A0 =C2=A0 =C2=A0> > > https://elixir.bootlin= .com/dpdk/v22.07/
>=C2=A0 =C2=A0 =C2=A0<https://elixir.bootlin.com/dpdk/v= 22.07/> \
>=C2=A0 =C2=A0 =C2=A0> > >=C2=A0 =C2=A0 =C2=A0source/doc/guides= /rel_notes/deprecation.rst#L118
>=C2=A0 =C2=A0 =C2=A0> > >
>=C2=A0 =C2=A0 =C2=A0> > > Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com
>=C2=A0 =C2=A0 =C2=A0<mailto:xiaoyun.li@intel.com>>
>=C2=A0 =C2=A0 =C2=A0> > > Signed-off-by: Junfeng Guo <junfeng.guo@intel.com=
>=C2=A0 =C2=A0 =C2=A0<mailto:junfeng.guo@intel.com>>
>=C2=A0 =C2=A0 =C2=A0> >
>=C2=A0 =C2=A0 =C2=A0> > <...>
>=C2=A0 =C2=A0 =C2=A0> >
>=C2=A0 =C2=A0 =C2=A0> > > +static int
>=C2=A0 =C2=A0 =C2=A0> > > +gve_dev_stats_get(struct rte_eth_de= v *dev, struct
>=C2=A0 =C2=A0 =C2=A0rte_eth_stats *stats)
>=C2=A0 =C2=A0 =C2=A0> > > +{
>=C2=A0 =C2=A0 =C2=A0> > > +=C2=A0 =C2=A0uint16_t i;
>=C2=A0 =C2=A0 =C2=A0> > > +
>=C2=A0 =C2=A0 =C2=A0> > > +=C2=A0 =C2=A0for (i =3D 0; i < d= ev->data->nb_tx_queues; i++) {
>=C2=A0 =C2=A0 =C2=A0> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0struct gve_tx_queue *txq =3D dev->data->tx_queues[i];
>=C2=A0 =C2=A0 =C2=A0> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0if (txq =3D=3D NULL)
>=C2=A0 =C2=A0 =C2=A0> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0continue;
>=C2=A0 =C2=A0 =C2=A0> > > +
>=C2=A0 =C2=A0 =C2=A0> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0stats->opackets +=3D txq->packets;
>=C2=A0 =C2=A0 =C2=A0> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0stats->obytes +=3D txq->bytes;
>=C2=A0 =C2=A0 =C2=A0> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0stats->oerrors +=3D txq->errors;
>=C2=A0 =C2=A0 =C2=A0> >
>=C2=A0 =C2=A0 =C2=A0> > Hi Junfeng, Qi, Jingjing, Beilei,
>=C2=A0 =C2=A0 =C2=A0> >
>=C2=A0 =C2=A0 =C2=A0> > Above logic looks wrong to me, did you te= st it?
>=C2=A0 =C2=A0 =C2=A0> >
>=C2=A0 =C2=A0 =C2=A0> > If the 'gve_dev_stats_get()' call= ed multiple times (without
>=C2=A0 =C2=A0 =C2=A0stats reset
>=C2=A0 =C2=A0 =C2=A0> > in between), same values will be keep add= ed to stats.
>=C2=A0 =C2=A0 =C2=A0> > Some hw based implementations does this, = because reading from stats
>=C2=A0 =C2=A0 =C2=A0> > registers automatically reset those regis= ters but this shouldn't
>=C2=A0 =C2=A0 =C2=A0be case
>=C2=A0 =C2=A0 =C2=A0> > for this driver.
>=C2=A0 =C2=A0 =C2=A0> >
>=C2=A0 =C2=A0 =C2=A0> > I expect it to be something like:
>=C2=A0 =C2=A0 =C2=A0> >
>=C2=A0 =C2=A0 =C2=A0> > local_stats =3D 0
>=C2=A0 =C2=A0 =C2=A0> > foreach queue
>=C2=A0 =C2=A0 =C2=A0> >=C2=A0 =C2=A0 =C2=A0 =C2=A0local_stats += =3D queue->stats
>=C2=A0 =C2=A0 =C2=A0> > stats =3D local_stats
>=C2=A0 =C2=A0 =C2=A0>
>=C2=A0 =C2=A0 =C2=A0> The zero of local_stats is unnecessary.
>=C2=A0 =C2=A0 =C2=A0> The only caller of the PMD stats_get is rte_et= hdev_stats_get
>=C2=A0 =C2=A0 =C2=A0> and it zeros the stats structure before callin= g the PMD.
>=C2=A0 =C2=A0 =C2=A0>
>=C2=A0 =C2=A0 =C2=A0>
>=C2=A0 =C2=A0 =C2=A0> int
>=C2=A0 =C2=A0 =C2=A0> rte_eth_stats_get(uint16_t port_id, struct rte= _eth_stats *stats)
>=C2=A0 =C2=A0 =C2=A0> {
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0struct rte_et= h_dev *dev;
>=C2=A0 =C2=A0 =C2=A0>
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0RTE_ETH_VALID= _PORTID_OR_ERR_RET(port_id, -ENODEV);
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dev =3D &= rte_eth_devices[port_id];
>=C2=A0 =C2=A0 =C2=A0>
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0memset(stats,= 0, sizeof(*stats));
>=C2=A0 =C2=A0 =C2=A0> ...
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0stats->rx_= nombuf =3D dev->data->rx_mbuf_alloc_failed;
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return eth_er= r(port_id, (*dev->dev_ops->stats_get)(dev,
>=C2=A0 =C2=A0 =C2=A0stats));
>=C2=A0 =C2=A0 =C2=A0>
>=C2=A0 =C2=A0 =C2=A0> If any PMD has extra memset in their stats get= that could be removed.
>
>
>
> --
>
> Joshua Washington=C2=A0|=C2=A0Software Engineer |=C2=A0joshwash@google.com
> <mailto:jo= shwash@google.com>=C2=A0|=C2=A0(414) 366-4423
> =C2=A0



--

Joshua Washington=C2=A0|= =C2=A0Software Engineer= |=C2=A0joshwash@google.com=C2=A0|=C2=A0(414) 366-4423<= /div>= =C2=A0
--00000000000069f1fe05f38593ab--