Re: [PATCH net-next v2] octeontx2-pf: persist netdev stats across routine operations

[Dragos Tatulea <dtatulea@nvidia.com>]

On 14.05.26 08:35, Anshumali Gaur wrote:
> On Mon, May 11, 2026 at 12:03:32PM +0200, Dragos Tatulea wrote:
>>
>>
>> [...]
>> I don't quite understand why the additional old_stats is required.
>> Why not accumulate dev_stats in place? Either directly in
>> otx2_get_dev_stats() or pass a local struct otx2_dev_stats var
>> and then accumulate that into dev_stats?
>>
>> Thanks,
>> Dragos
>>
> Hi Dragos,
> Thanks for the review.
> otx2_get_dev_stats() reads absolute HW counter values from
> NIX LF stats registers and does direct assignment to dev_stats 
> (e.g., dev_stats->rx_bytes = OTX2_GET_RX_STATS(RX_OCTS)).
> These HW counters reset to zero when the interface goes down.
> Since they report absolute values, not deltas,
> accumulating in-place with += would double-count on every stats query.
>
I see. That makes sense.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> index ee623476e5ff..9606c8cb8c82 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> @@ -2158,10 +2158,28 @@ int otx2_stop(struct net_device *netdev)
>  	struct otx2_qset *qset = &pf->qset;
>  	int qidx, vec, wrk;
>  
> +	struct otx2_dev_stats *dev = &pf->hw.dev_stats;
> +	struct otx2_dev_stats *old_stats = &pf->hw.old_stats;
> +
>  	/* If the DOWN flag is set resources are already freed */
>  	if (pf->flags & OTX2_FLAG_INTF_DOWN)
>  		return 0;
>  
> +	/* Accumulate old stats */
> +	old_stats->rx_bytes     += dev->rx_bytes;
> +	old_stats->rx_drops     += dev->rx_drops;
> +	old_stats->rx_bcast_frames      += dev->rx_bcast_frames;
> +	old_stats->rx_mcast_frames      += dev->rx_mcast_frames;
> +	old_stats->rx_ucast_frames      += dev->rx_ucast_frames;
> +	old_stats->rx_frames            += dev->rx_frames;
> +
> +	old_stats->tx_bytes             += dev->tx_bytes;
> +	old_stats->tx_drops             += dev->tx_drops;
> +	old_stats->tx_bcast_frames      += dev->tx_bcast_frames;
> +	old_stats->tx_mcast_frames      += dev->tx_mcast_frames;
> +	old_stats->tx_ucast_frames      += dev->tx_ucast_frames;
> +	old_stats->tx_frames            += dev->tx_frames;
> +
>  	netif_carrier_off(netdev);
>  	netif_tx_stop_all_queues(netdev);
>  
It would be more accurate to read the counters from HW after disabling
tx/rx instead of saving the old values.

Sashiko agrees:
https://sashiko.dev/#/patchset/20260511094205.1422268-1-agaur%40marvell.com

Thanks,
Dragos