Re: [Patch v2] net/netvsc: report correct stats values

[Ferruh Yigit <ferruh.yigit@xilinx.com>]

On 3/24/2022 5:45 PM, longli@linuxonhyperv.com wrote:
> From: Long Li <longli@microsoft.com>
> 
> The netvsc should add to the values from the VF and report the sum.
> 

Per port stats already accumulated, like:
'stats->opackets += txq->stats.packets;'

> Fixes: 4e9c73e96e ("net/netvsc: add Hyper-V network device")
> Cc: stable@dpdk.org
> Signed-off-by: Long Li <longli@microsoft.com>
> ---
>   drivers/net/netvsc/hn_ethdev.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
> index 0a357d3645..a6202d898b 100644
> --- a/drivers/net/netvsc/hn_ethdev.c
> +++ b/drivers/net/netvsc/hn_ethdev.c
> @@ -804,8 +804,8 @@ static int hn_dev_stats_get(struct rte_eth_dev *dev,
>   		stats->oerrors += txq->stats.errors;
>   
>   		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.

>   		}
>   	}
>   
> @@ -821,12 +821,12 @@ static int hn_dev_stats_get(struct rte_eth_dev *dev,
>   		stats->imissed += rxq->stats.ring_full;
>   
>   		if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
> -			stats->q_ipackets[i] = rxq->stats.packets;
> -			stats->q_ibytes[i] = rxq->stats.bytes;
> +			stats->q_ipackets[i] += rxq->stats.packets;
> +			stats->q_ibytes[i] += rxq->stats.bytes;
>   		}
>   	}
>   
> -	stats->rx_nombuf = dev->data->rx_mbuf_alloc_failed;
> +	stats->rx_nombuf += dev->data->rx_mbuf_alloc_failed;

Why '+='? Is 'dev->data->rx_mbuf_alloc_failed' reset somewhere between 
two consecutive stats get call?

Anyway, above line has no affect, since the 'stats->rx_nombuf' is 
overwritten by 'rte_eth_stats_get()'. So above line can be removed.

>   	return 0;
>   }
>