Re: [PATCH net-next] net: mvneta: add support for page_pool_get_stats

[Andrew Lunn <andrew@lunn.ch>]

> +static void mvneta_ethtool_update_pp_stats(struct mvneta_port *pp,
> +					   struct page_pool_stats *stats)
> +{
> +	int i;
> +
> +	memset(stats, 0, sizeof(*stats));
> +	for (i = 0; i < rxq_number; i++) {
> +		struct page_pool *page_pool = pp->rxqs[i].page_pool;
> +		struct page_pool_stats pp_stats = {};
> +
> +		if (!page_pool_get_stats(page_pool, &pp_stats))
> +			continue;
> +
> +		stats->alloc_stats.fast += pp_stats.alloc_stats.fast;
> +		stats->alloc_stats.slow += pp_stats.alloc_stats.slow;
> +		stats->alloc_stats.slow_high_order +=
> +			pp_stats.alloc_stats.slow_high_order;
> +		stats->alloc_stats.empty += pp_stats.alloc_stats.empty;
> +		stats->alloc_stats.refill += pp_stats.alloc_stats.refill;
> +		stats->alloc_stats.waive += pp_stats.alloc_stats.waive;
> +		stats->recycle_stats.cached += pp_stats.recycle_stats.cached;
> +		stats->recycle_stats.cache_full +=
> +			pp_stats.recycle_stats.cache_full;
> +		stats->recycle_stats.ring += pp_stats.recycle_stats.ring;
> +		stats->recycle_stats.ring_full +=
> +			pp_stats.recycle_stats.ring_full;
> +		stats->recycle_stats.released_refcnt +=
> +			pp_stats.recycle_stats.released_refcnt;

Am i right in saying, these are all software stats? They are also
generic for any receive queue using the page pool?

It seems odd the driver is doing the addition here. Why not pass stats
into page_pool_get_stats()? That will make it easier when you add
additional statistics?

I'm also wondering if ethtool -S is even the correct API. It should be
for hardware dependent statistics, those which change between
implementations. Where as these statistics should be generic. Maybe
they should be in /sys/class/net/ethX/statistics/ and the driver
itself is not even involved, the page pool code implements it?

       Andrew