[PATCH] net/gve: add support for basic stats

Ferruh Yigit ferruh.yigit at amd.com
Wed Dec 7 16:09:08 CET 2022


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 <xiaoyun.li at intel.com>
> Signed-off-by: Junfeng Guo <junfeng.guo at intel.com>

<...>

> +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




More information about the dev mailing list