From: Andy Gospodarek Date: Wed, 30 Jan 2008 02:07:46 +0000 (-0800) Subject: bonding: fix race that causes invalid statistics X-Git-Tag: v2.6.25-rc1~1065^2~32 X-Git-Url: http://pilppa.com/gitweb/?a=commitdiff_plain;h=2439f9ebd45349246b0fec7c47e6d0e05b1357c7;p=linux-2.6-omap-h63xx.git bonding: fix race that causes invalid statistics I've seen reports of invalid stats in /proc/net/dev for bonding interfaces, and found it's a pretty easy problem to reproduce. Since the current code zeros the bonding stats when a read is requested and a pointer to that data is returned to the caller we cannot guarantee that the caller has completely accessed the data before a successive call to request the stats zeroes the stats again. This patch creates a new stack variable to keep track of the updated stats and copies the data from that variable into the bonding stats structure. This ensures that the value for any of the bonding stats should not incorrectly return zero for any of the bonding statistics. This does use more stack space and require an extra memcpy, but it seems like a fair trade-off for consistently correct bonding statistics. Signed-off-by: Andy Gospodarek Signed-off-by: Chris Snook Acked-by: Jay Vosburgh Signed-off-by: Jeff Garzik Signed-off-by: David S. Miller --- diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 81b45740ed7..8a8d5c3de9e 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -3775,41 +3775,44 @@ static struct net_device_stats *bond_get_stats(struct net_device *bond_dev) { struct bonding *bond = bond_dev->priv; struct net_device_stats *stats = &(bond->stats), *sstats; + struct net_device_stats local_stats; struct slave *slave; int i; - memset(stats, 0, sizeof(struct net_device_stats)); + memset(&local_stats, 0, sizeof(struct net_device_stats)); read_lock_bh(&bond->lock); bond_for_each_slave(bond, slave, i) { sstats = slave->dev->get_stats(slave->dev); - stats->rx_packets += sstats->rx_packets; - stats->rx_bytes += sstats->rx_bytes; - stats->rx_errors += sstats->rx_errors; - stats->rx_dropped += sstats->rx_dropped; - - stats->tx_packets += sstats->tx_packets; - stats->tx_bytes += sstats->tx_bytes; - stats->tx_errors += sstats->tx_errors; - stats->tx_dropped += sstats->tx_dropped; - - stats->multicast += sstats->multicast; - stats->collisions += sstats->collisions; - - stats->rx_length_errors += sstats->rx_length_errors; - stats->rx_over_errors += sstats->rx_over_errors; - stats->rx_crc_errors += sstats->rx_crc_errors; - stats->rx_frame_errors += sstats->rx_frame_errors; - stats->rx_fifo_errors += sstats->rx_fifo_errors; - stats->rx_missed_errors += sstats->rx_missed_errors; - - stats->tx_aborted_errors += sstats->tx_aborted_errors; - stats->tx_carrier_errors += sstats->tx_carrier_errors; - stats->tx_fifo_errors += sstats->tx_fifo_errors; - stats->tx_heartbeat_errors += sstats->tx_heartbeat_errors; - stats->tx_window_errors += sstats->tx_window_errors; - } + local_stats.rx_packets += sstats->rx_packets; + local_stats.rx_bytes += sstats->rx_bytes; + local_stats.rx_errors += sstats->rx_errors; + local_stats.rx_dropped += sstats->rx_dropped; + + local_stats.tx_packets += sstats->tx_packets; + local_stats.tx_bytes += sstats->tx_bytes; + local_stats.tx_errors += sstats->tx_errors; + local_stats.tx_dropped += sstats->tx_dropped; + + local_stats.multicast += sstats->multicast; + local_stats.collisions += sstats->collisions; + + local_stats.rx_length_errors += sstats->rx_length_errors; + local_stats.rx_over_errors += sstats->rx_over_errors; + local_stats.rx_crc_errors += sstats->rx_crc_errors; + local_stats.rx_frame_errors += sstats->rx_frame_errors; + local_stats.rx_fifo_errors += sstats->rx_fifo_errors; + local_stats.rx_missed_errors += sstats->rx_missed_errors; + + local_stats.tx_aborted_errors += sstats->tx_aborted_errors; + local_stats.tx_carrier_errors += sstats->tx_carrier_errors; + local_stats.tx_fifo_errors += sstats->tx_fifo_errors; + local_stats.tx_heartbeat_errors += sstats->tx_heartbeat_errors; + local_stats.tx_window_errors += sstats->tx_window_errors; + } + + memcpy(stats, &local_stats, sizeof(struct net_device_stats)); read_unlock_bh(&bond->lock);