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

[Larysa Zaremba]

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@xxxxxxxxxxx>
> ---
> V8:
>   - Reordered patch
> 
> V7: https://lore.kernel.org/all/20250114125124.2570660-2-srasheed@xxxxxxxxxxx/
>   - Updated octep_get_stats64() to be reentrant
> 
> V6: https://lore.kernel.org/all/20250110122730.2551863-2-srasheed@xxxxxxxxxxx/
>   - No changes
> 
> V5: https://lore.kernel.org/all/20250109103221.2544467-2-srasheed@xxxxxxxxxxx/
>   - 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;