Skip to content

Commit

Permalink
bonding: fix race that causes invalid statistics
Browse files Browse the repository at this point in the history
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 <andy@greyhouse.net>
Signed-off-by: Chris Snook <csnook@redhat.com>
Acked-by: Jay Vosburgh <fubar@us.ibm.com>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Andy Gospodarek authored and David S. Miller committed Feb 3, 2008
1 parent 4fe4763 commit 2439f9e
Showing 1 changed file with 30 additions and 27 deletions.
57 changes: 30 additions & 27 deletions drivers/net/bonding/bond_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down

0 comments on commit 2439f9e

Please sign in to comment.