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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Yk2X6KPyeN3z7OUW@lunn.ch>
Date:   Wed, 6 Apr 2022 15:38:48 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Lorenzo Bianconi <lorenzo@...nel.org>
Cc:     netdev@...r.kernel.org, lorenzo.bianconi@...hat.com,
        davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
        thomas.petazzoni@...tlin.com, linux@...linux.org.uk,
        jbrouer@...hat.com, ilias.apalodimas@...aro.org, jdamato@...tly.com
Subject: Re: [PATCH net-next] net: mvneta: add support for page_pool_get_stats

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