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

[Anshumali Gaur <agaur@marvell.com>]

On Mon, May 11, 2026 at 12:03:32PM +0200, Dragos Tatulea wrote:
> 
> 
> On 11.05.26 11:42, Anshumali Gaur wrote:
> > Currently netdev driver is not retaining ethtool stats during
> > interface up and down, as per kernel driver standard netdev driver
> > must keep stats constant to avoid race conditions with user space
> > trying to read them. Stats must persist across routine
> > operations like bringing the interface down and up.
> > 
> > Signed-off-by: Anshumali Gaur <agaur@marvell.com>
> > ---
> > v2:
> >   - Fix subject prefix to target net-next
> >  .../marvell/octeontx2/nic/otx2_common.c       | 20 ++++++++++---------
> >  .../marvell/octeontx2/nic/otx2_common.h       |  1 +
> >  .../ethernet/marvell/octeontx2/nic/otx2_pf.c  | 18 +++++++++++++++++
> >  3 files changed, 30 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> > index 971fcab1c248..0ffcc613a4b2 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> > @@ -139,19 +139,21 @@ void otx2_get_stats64(struct net_device *netdev,
> >  		      struct rtnl_link_stats64 *stats)
> >  {
> >  	struct otx2_nic *pfvf = netdev_priv(netdev);
> > -	struct otx2_dev_stats *dev_stats;
> > +	struct otx2_dev_stats *dev_stats, *old_stats;
> >  
> >  	otx2_get_dev_stats(pfvf);
> >  
> >  	dev_stats = &pfvf->hw.dev_stats;
> > -	stats->rx_bytes = dev_stats->rx_bytes;
> > -	stats->rx_packets = dev_stats->rx_frames;
> > -	stats->rx_dropped = dev_stats->rx_drops;
> > -	stats->multicast = dev_stats->rx_mcast_frames;
> > -
> > -	stats->tx_bytes = dev_stats->tx_bytes;
> > -	stats->tx_packets = dev_stats->tx_frames;
> > -	stats->tx_dropped = dev_stats->tx_drops;
> > +	old_stats = &pfvf->hw.old_stats;
> > +
> > +	stats->rx_bytes = old_stats->rx_bytes + dev_stats->rx_bytes;
> > +	stats->rx_packets = old_stats->rx_frames + dev_stats->rx_frames;
> > +	stats->rx_dropped = old_stats->rx_drops + dev_stats->rx_drops;
> > +	stats->multicast = old_stats->rx_mcast_frames + dev_stats->rx_mcast_frames;
> > +
> > +	stats->tx_bytes = old_stats->tx_bytes + dev_stats->tx_bytes;
> > +	stats->tx_packets = old_stats->tx_frames + dev_stats->tx_frames;
> > +	stats->tx_dropped = old_stats->tx_drops + dev_stats->tx_drops;
> 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.

Thanks,
Anshu