Commit fa52f15c authored by Sean Anderson's avatar Sean Anderson Committed by Jakub Kicinski
Browse files

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.

Fixes: 89e5785f ("[PATCH] Atmel MACB ethernet driver")
Signed-off-by: default avatarSean Anderson <sean.anderson@linux.dev>
Link: https://patch.msgid.link/20250220162950.95941-1-sean.anderson@linux.dev


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 27843ce6

drivers/net/ethernet/cadence/macb.h

+2 −0
Original line number Diff line number Diff line
@@ -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
Original line number Diff line number Diff line
@@ -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);