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

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 <redacted>
---
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;
 }
 EXPORT_SYMBOL(otx2_get_stats64);
 

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
index eecee612b7b2..ad65aa19b80d 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h

@@ -255,6 +255,7 @@ struct otx2_hw {
 
 	/* Stats */
 	struct otx2_dev_stats	dev_stats;
+	struct otx2_dev_stats	old_stats;
 	struct otx2_drv_stats	drv_stats;
 	u64			cgx_rx_stats[CGX_RX_STATS_COUNT];
 	u64			cgx_tx_stats[CGX_TX_STATS_COUNT];

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);
 

-- 
2.25.1

On 11.05.26 11:42, Anshumali Gaur 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

On Mon, 11 May 2026 15:12:05 +0530 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.

I think you're confusing the value of the stats with the list of stats.
Retaining values thru down/up is probably the most common behavior but
that's not what the doc is likely talking about.

Please format your code according to kernel coding style.

 	/* 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);
 

On Mon, May 11, 2026 at 12:03:32PM +0200, Dragos Tatulea wrote:

On 11.05.26 11:42, Anshumali Gaur wrote:

quoted

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

On 14.05.26 08:35, Anshumali Gaur wrote:

On Mon, May 11, 2026 at 12:03:32PM +0200, Dragos Tatulea wrote:

quoted

[...]
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.

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