<div dir="ltr">Hello,<div><br></div><div>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:</div><div><br></div><div><font face="monospace"> ---------------------- Forward statistics for port 0 ----------------------<br> RX-packets: 2453802 RX-dropped: 0 RX-total: 2453802<br> TX-packets: 34266881 TX-dropped: 447034 TX-total: 34713915<br> ----------------------------------------------------------------------------<br><br> ---------------------- Forward statistics for port 1 ----------------------<br> RX-packets: 34713915 RX-dropped: 0 RX-total: 34713915<br> TX-packets: 2453802 TX-dropped: 0 TX-total: 2453802<br> ----------------------------------------------------------------------------<br><br> +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++<br> RX-packets: 37167717 RX-dropped: 0 RX-total: 37167717<br> TX-packets: 36720683 TX-dropped: 807630 TX-total: 37528313<br> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++</font><br></div><div><font face="monospace"><br></font></div><div><font face="arial, sans-serif">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.</font></div><div><font face="monospace"><br></font></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Dec 19, 2022 at 11:17 AM Rushil Gupta <<a href="mailto:rushilg@google.com">rushilg@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi all<br>
Josh just found out some inconsistencies in the Tx/Rx statistics sum<br>
for all ports. Not sure if we can screenshot here but it goes like<br>
this:<br>
Tx-dropped for port0: 447034<br>
Tx-dropped for port1: 0<br>
Accumulated forward statistics for all ports: 807630<br>
<br>
Please note that this issue is only with Tx-dropped (not Tx-packets/Tx-total).<br>
<br>
<br>
On Wed, Dec 7, 2022 at 8:39 AM Stephen Hemminger<br>
<<a href="mailto:stephen@networkplumber.org" target="_blank">stephen@networkplumber.org</a>> wrote:<br>
><br>
> On Wed, 7 Dec 2022 15:09:08 +0000<br>
> Ferruh Yigit <<a href="mailto:ferruh.yigit@amd.com" target="_blank">ferruh.yigit@amd.com</a>> wrote:<br>
><br>
> > On 11/24/2022 7:33 AM, Junfeng Guo wrote:<br>
> > > Add support for dev_ops of stats_get and stats_reset.<br>
> > ><br>
> > > Queue stats update will be moved into xstat [1], but the basic stats<br>
> > > items may still be required. So we just keep the remaining ones and<br>
> > > will implement the queue stats via xstats in the coming release.<br>
> > ><br>
> > > [1]<br>
> > > <a href="https://elixir.bootlin.com/dpdk/v22.07/" rel="noreferrer" target="_blank">https://elixir.bootlin.com/dpdk/v22.07/</a> \<br>
> > > source/doc/guides/rel_notes/deprecation.rst#L118<br>
> > ><br>
> > > Signed-off-by: Xiaoyun Li <<a href="mailto:xiaoyun.li@intel.com" target="_blank">xiaoyun.li@intel.com</a>><br>
> > > Signed-off-by: Junfeng Guo <<a href="mailto:junfeng.guo@intel.com" target="_blank">junfeng.guo@intel.com</a>><br>
> ><br>
> > <...><br>
> ><br>
> > > +static int<br>
> > > +gve_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)<br>
> > > +{<br>
> > > + uint16_t i;<br>
> > > +<br>
> > > + for (i = 0; i < dev->data->nb_tx_queues; i++) {<br>
> > > + struct gve_tx_queue *txq = dev->data->tx_queues[i];<br>
> > > + if (txq == NULL)<br>
> > > + continue;<br>
> > > +<br>
> > > + stats->opackets += txq->packets;<br>
> > > + stats->obytes += txq->bytes;<br>
> > > + stats->oerrors += txq->errors;<br>
> ><br>
> > Hi Junfeng, Qi, Jingjing, Beilei,<br>
> ><br>
> > Above logic looks wrong to me, did you test it?<br>
> ><br>
> > If the 'gve_dev_stats_get()' called multiple times (without stats reset<br>
> > in between), same values will be keep added to stats.<br>
> > Some hw based implementations does this, because reading from stats<br>
> > registers automatically reset those registers but this shouldn't be case<br>
> > for this driver.<br>
> ><br>
> > I expect it to be something like:<br>
> ><br>
> > local_stats = 0<br>
> > foreach queue<br>
> > local_stats += queue->stats<br>
> > stats = local_stats<br>
><br>
> The zero of local_stats is unnecessary.<br>
> The only caller of the PMD stats_get is rte_ethdev_stats_get<br>
> and it zeros the stats structure before calling the PMD.<br>
><br>
><br>
> int<br>
> rte_eth_stats_get(uint16_t port_id, struct rte_eth_stats *stats)<br>
> {<br>
> struct rte_eth_dev *dev;<br>
><br>
> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);<br>
> dev = &rte_eth_devices[port_id];<br>
><br>
> memset(stats, 0, sizeof(*stats));<br>
> ...<br>
> stats->rx_nombuf = dev->data->rx_mbuf_alloc_failed;<br>
> return eth_err(port_id, (*dev->dev_ops->stats_get)(dev, stats));<br>
><br>
> If any PMD has extra memset in their stats get that could be removed.<br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><br><div style="line-height:1.5em;padding-top:10px;margin-top:10px;color:rgb(85,85,85);font-family:sans-serif"><span style="border-width:2px 0px 0px;border-style:solid;border-color:rgb(213,15,37);padding-top:2px;margin-top:2px">Joshua Washington |</span><span style="border-width:2px 0px 0px;border-style:solid;border-color:rgb(51,105,232);padding-top:2px;margin-top:2px"> Software Engineer |</span><span style="border-width:2px 0px 0px;border-style:solid;border-color:rgb(0,153,57);padding-top:2px;margin-top:2px"> <a href="mailto:joshwash@google.com" target="_blank">joshwash@google.com</a> |</span><span style="border-width:2px 0px 0px;border-style:solid;border-color:rgb(238,178,17);padding-top:2px;margin-top:2px"> (414) 366-4423</span></div><span style="color:rgb(0,0,0);font-family:Tinos;font-size:medium"> </span><br></div></div>