On 5/4/2022 7:38 PM, Long Li wrote:
Subject: Re: [Patch v2] net/netvsc: report correct stats values

On 5/3/2022 9:48 PM, Long Li wrote:
Subject: Re: [Patch v2] net/netvsc: report correct stats values

On 5/3/2022 8:14 PM, Long Li wrote:
Subject: Re: [Patch v2] net/netvsc: report correct stats values

On 5/3/2022 7:18 PM, Long Li wrote:
Subject: Re: [Patch v2] net/netvsc: report correct stats values

On Tue, 26 Apr 2022 22:56:14 +0100 Ferruh Yigit
<ferruh.yi...@xilinx.com> wrote:

                if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
-                       stats->q_opackets[i] = txq->stats.packets;
-                       stats->q_obytes[i] = txq->stats.bytes;
+                       stats->q_opackets[i] += txq->stats.packets;
+                       stats->q_obytes[i] += txq->stats.bytes;

This is per queue stats, 'stats->q_opackets[i]', in next
iteration of the loop, 'i' will be increased and 'txq' will be
updated, so as far as I can see the above change has no affect.

Agree, that is why it was just assignment originally.

The condition here is a little different. NETVSC is a master
device with
another PMD running as a slave. When reporting stats values, it
needs to add the values from the slave PMD. The original code just
overwrites the values from its slave PMD.

Where the initial values are coming from, 'hn_vf_stats_get()'?

If 'hn_vf_stats_get()' fills the stats, what are the values kept in
'txq-
stats.*'
in above updated loop?

Yes, hn_vf_stats_get() fills in the stats from the slave PMD.
txq->stats
values are from the master PMD. Those values are different and
accounted separated from the values from the slave PMD.

I see, since this is a little different than what most of the PMDs
do, can you please put a little more info to the commit log? Or
perhaps can add some comments to the code.

Ok, will do.


And still 'stats->rx_nombuf' change is not required right? If so can
you remove it in the next version?

It is still needed. NETVSC unconditionally calls the slave PMD to receive
packets, even if it can't allocate a mbuf to receive a synthetic packet itself. 
The
accounting of rx_nombuf is valid because the synthetic packets (to NETVSC) and
VF packets (to slave PMD) are routed separately from Hyper-V.

I am not referring to the "+=" update, my comment was because 'stats-
rx_nombuf' is overwritten in 'rte_eth_stats_get()' [1].
Is it still required?

Yes, it is still needed. NETVSC calls the rte_eth_stats_get() on its slave PMD first, and 
stats->rx_nombuf is updated (overwritten) for its slave PMD. Afte that, it needs to add 
to its own dev->data->rx_mbuf_alloc_failed back to stats->rx_nombuf.


But its own stat also will be overwritten (not in PMD function, but in ethdev layer). 'stats->rx_nombuf' assignment in the PMD seems has no effect and can be removed.

I can't see how it is needed, can you please put a call stack to describe?


[1]
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.dpdk
.org%2Fdpdk%2Ftree%2Flib%2Fethdev%2Frte_ethdev.c%3Fh%3Dv22.03%23n25
18&amp;data=05%7C01%7Clongli%40microsoft.com%7Cea473df2344c460d575
d08da2dca3e53%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63787
2643902917430%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQ
IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sd
ata=FZO%2B%2BnWtLGstHHIZ2aXsDUKNI%2Fi9tbj6jONhp174qKw%3D&amp;res
erved=0

Reply via email to