net: cadence: macb: Synchronize stats calculations
Subsystems (3)
PATCH v2 1/1 net: cadence: macb: Synchronize stats calculations
Stats calculations involve a RMW to add the stat update to the existing value. This is currently not protected by any synchronization mechanism, so data races are possible. Add a spinlock to protect the update. The reader side could be protected using u64_stats, but we would still need a spinlock for the update side anyway. And we always do an update immediately before reading the stats anyway.
---
Changes in v2:
- Use spin_lock_irq when not in IRQ context since we take this lock in macb_interrupt
drivers/net/ethernet/cadence/macb.h | 2 ++ drivers/net/ethernet/cadence/macb_main.c | 12 ++++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-)
drivers/net/ethernet/cadence/macb.h +2 -0
@@ -1279,6 +1279,8 @@ struct macb {struct clk *rx_clk;struct clk *tsu_clk;struct net_device *dev; +/* Protects hw_stats and ethtool_stats */ +spinlock_t stats_lock;union {struct macb_stats macb;struct gem_stats gem;
drivers/net/ethernet/cadence/macb_main.c +10 -2
@@ -1978,10 +1978,12 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)if (status &MACB_BIT (ISR_ROVR )) {/* We missed at least one packet */ +spin_lock (&bp->stats_lock);if (macb_is_gem (bp)) bp->hw_stats.gem.rx_overruns++;else bp->hw_stats.macb.rx_overruns++; +spin_unlock (&bp->stats_lock);if (bp->caps &MACB_CAPS_ISR_CLEAR_ON_WRITE )queue_writel (queue,ISR ,MACB_BIT (ISR_ROVR ));
@@ -3102,6 +3104,7 @@ static struct net_device_stats *gem_get_stats(struct macb *bp)if (!netif_running (bp->dev))return nstat; +spin_lock_irq (&bp->stats_lock);gem_update_stats (bp); nstat->rx_errors = (hwstat->rx_frame_check_sequence_errors +
@@ -3131,6 +3134,7 @@ static struct net_device_stats *gem_get_stats(struct macb *bp) nstat->tx_aborted_errors = hwstat->tx_excessive_collisions; nstat->tx_carrier_errors = hwstat->tx_carrier_sense_errors; nstat->tx_fifo_errors = hwstat->tx_underrun; +spin_unlock_irq (&bp->stats_lock);return nstat; }
@@ -3138,12 +3142,13 @@ static struct net_device_stats *gem_get_stats(struct macb *bp)static void gem_get_ethtool_stats (struct net_device *dev,struct ethtool_stats *stats,u64 *data) { -struct macb *bp; +struct macb *bp =netdev_priv (dev); - bp =netdev_priv (dev); +spin_lock_irq (&bp->stats_lock);gem_update_stats (bp);memcpy (data, &bp->ethtool_stats,sizeof (u64) * (GEM_STATS_LEN +QUEUE_STATS_LEN *MACB_MAX_QUEUES )); +spin_unlock_irq (&bp->stats_lock); }static int gem_get_sset_count (struct net_device *dev,int sset)
@@ -3193,6 +3198,7 @@ static struct net_device_stats *macb_get_stats(struct net_device *dev)return gem_get_stats (bp);/* read stats from hardware */ +spin_lock_irq (&bp->stats_lock);macb_update_stats (bp);/* Convert HW stats into netdevice stats */
@@ -3226,6 +3232,7 @@ static struct net_device_stats *macb_get_stats(struct net_device *dev) nstat->tx_carrier_errors = hwstat->tx_carrier_errors; nstat->tx_fifo_errors = hwstat->tx_underruns;/* Don't know about heartbeat or window errors... */ +spin_unlock_irq (&bp->stats_lock);return nstat; }
@@ -5097,6 +5104,7 @@ static int macb_probe(struct platform_device *pdev) } }spin_lock_init (&bp->lock); +spin_lock_init (&bp->stats_lock);/* setup capabilities */ macb_configure_caps (bp, macb_config);
Signature · 1 line
2.35.1.1320.gc452695387.dirty
This patch was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:
Quoted 8 lines · On Thu, 20 Feb 2025 11:29:50 -0500 you wrote:
Stats calculations involve a RMW to add the stat update to the existing value. This is currently not protected by any synchronization mechanism, so data races are possible. Add a spinlock to protect the update. The reader side could be protected using
u64_stats, but we would still need a spinlock for the update side anyway. And we always do an update immediately before reading the stats anyway.[...]
Here is the summary with links:
- [net,v2] net: cadence: macb: Synchronize stats calculations
https://git.kernel.org/netdev/net/c/fa52f15c745c
You are awesome, thank you!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html