linux-kernel - Re: [PATCH net v8 2/4] octeon_ep: update tx/rx stats locally for persistence

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: <Z4klGpVVsxOPR3RZ@lzaremba-mobl.ger.corp.intel.com>
Date: Thu, 16 Jan 2025 16:26:18 +0100
From: Larysa Zaremba <larysa.zaremba@...el.com>
To: Shinas Rasheed <srasheed@...vell.com>
CC: <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<hgani@...vell.com>, <sedara@...vell.com>, <vimleshk@...vell.com>,
	<thaller@...hat.com>, <wizhao@...hat.com>, <kheib@...hat.com>,
	<konguyen@...hat.com>, <horms@...nel.org>, <einstein.xue@...axg.com>,
	Veerasenareddy Burru <vburru@...vell.com>, Andrew Lunn
	<andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>, "Eric
 Dumazet" <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>
Subject: Re: [PATCH net v8 2/4] octeon_ep: update tx/rx stats locally for
 persistence

On Thu, Jan 16, 2025 at 12:38:23AM -0800, Shinas Rasheed wrote:
> Update tx/rx stats locally, so that ndo_get_stats64()
> can use that and not rely on per queue resources to obtain statistics.
> The latter used to cause race conditions when the device stopped.
> 
> Signed-off-by: Shinas Rasheed <srasheed@...vell.com>
> ---
> V8:
>   - Reordered patch
> 
> V7: https://lore.kernel.org/all/20250114125124.2570660-2-srasheed@marvell.com/
>   - Updated octep_get_stats64() to be reentrant
> 
> V6: https://lore.kernel.org/all/20250110122730.2551863-2-srasheed@marvell.com/
>   - No changes
> 
> V5: https://lore.kernel.org/all/20250109103221.2544467-2-srasheed@marvell.com/
>   - Patch introduced
> 
>  .../marvell/octeon_ep/octep_ethtool.c         | 41 ++++++++-----------
>  .../ethernet/marvell/octeon_ep/octep_main.c   | 19 ++++-----
>  .../ethernet/marvell/octeon_ep/octep_main.h   | 11 +++++
>  .../net/ethernet/marvell/octeon_ep/octep_rx.c | 12 +++---
>  .../net/ethernet/marvell/octeon_ep/octep_rx.h |  4 +-
>  .../net/ethernet/marvell/octeon_ep/octep_tx.c |  7 ++--
>  .../net/ethernet/marvell/octeon_ep/octep_tx.h |  4 +-
>  7 files changed, 51 insertions(+), 47 deletions(-)
>

[...]
 
> diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> index 730aa5632cce..133694a1658d 100644
> --- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c

[...]

> @@ -991,22 +991,19 @@ static netdev_tx_t octep_start_xmit(struct sk_buff *skb,
>  static void octep_get_stats64(struct net_device *netdev,
>  			      struct rtnl_link_stats64 *stats)
>  {
> -	u64 tx_packets, tx_bytes, rx_packets, rx_bytes;
>  	struct octep_device *oct = netdev_priv(netdev);
> +	u64 tx_packets, tx_bytes, rx_packets, rx_bytes;
>  	int q;
>  
>  	tx_packets = 0;
>  	tx_bytes = 0;
>  	rx_packets = 0;
>  	rx_bytes = 0;
> -	for (q = 0; q < oct->num_oqs; q++) {
> -		struct octep_iq *iq = oct->iq[q];
> -		struct octep_oq *oq = oct->oq[q];
> -
> -		tx_packets += iq->stats.instr_completed;
> -		tx_bytes += iq->stats.bytes_sent;
> -		rx_packets += oq->stats.packets;
> -		rx_bytes += oq->stats.bytes;
> +	for (q = 0; q < oct->num_ioq_stats; q++) {
> +		tx_packets += oct->stats_iq[q].instr_completed;
> +		tx_bytes += oct->stats_iq[q].bytes_sent;
> +		rx_packets += oct->stats_oq[q].packets;
> +		rx_bytes += oct->stats_oq[q].bytes;

Correct me if I am wrong, but the interface-wide statistics should not change 
when changing queue number. In such case maybe it would be a good idea to 
always iterate over all OCTEP_MAX_QUEUES queues when calculating the stats.

>  	}
>  	stats->tx_packets = tx_packets;
>  	stats->tx_bytes = tx_bytes;