bond_get_stats() can be called from rtnetlink (with RTNL held) or from /proc/net/dev seq handler (with RCU held) The logic added in commit 5f0c5f73e5ef ("bonding: make global bonding stats more reliable") kind of assumed only one cpu could run there. If multiple threads are reading /proc/net/dev, stats can be really messed up after a while. A second problem is that some fields are 32bit, so we need to properly handle the wrap around problem. Given that RTNL is not always held, we need to use bond_for_each_slave_rcu().
Unified · Split
| @@ -3301,6 +3301,30 @@ static int bond_close(struct net_device *bond_dev) · 3300 unmodified lines | ||
| 3301 | 3301 | return 0; |
| 3302 | 3302 | } |
| 3303 | 3303 | |
| 3304 | +/* fold stats, assuming all rtnl_link_stats64 fields are u64, but | |
| 3305 | + * that some drivers can provide 32bit values only. | |
| 3306 | + */ | |
| 3307 | +static void bond_fold_stats(struct rtnl_link_stats64 *_res, | |
| 3308 | + const struct rtnl_link_stats64 *_new, | |
| 3309 | + const struct rtnl_link_stats64 *_old) | |
| 3310 | +{ | |
| 3311 | + const u64 *new = (const u64 *)_new; | |
| 3312 | + const u64 *old = (const u64 *)_old; | |
| 3313 | + u64 *res = (u64 *)_res; | |
| 3314 | + int i; | |
| 3315 | + | |
| 3316 | + for (i = 0; i < sizeof(*_res) / sizeof(u64); i++) { | |
| 3317 | + u64 nv = new[i]; | |
| 3318 | + u64 ov = old[i]; | |
| 3319 | + | |
| 3320 | + /* detects if this particular field is 32bit only */ | |
| 3321 | + if (((nv | ov) >> 32) == 0) | |
| 3322 | + res[i] += (u32)nv - (u32)ov; | |
| 3323 | + else | |
| 3324 | + res[i] += nv - ov; | |
| 3325 | + } | |
| 3326 | +} | |
| 3327 | + | |
| 3304 | 3328 | static struct rtnl_link_stats64 *bond_get_stats(struct net_device *bond_dev, |
| 3305 | 3329 | struct rtnl_link_stats64 *stats) |
| 3306 | 3330 | { |
| @@ -3309,44 +3333,23 @@ static struct rtnl_link_stats64 *bond_get_stats(struct net_device *bond_dev, · 2 unmodified lines | ||
| 3309 | 3333 | struct list_head *iter; |
| 3310 | 3334 | struct slave *slave; |
| 3311 | 3335 | |
| 3336 | + spin_lock(&bond->stats_lock); | |
| 3312 | 3337 | memcpy(stats, &bond->bond_stats, sizeof(*stats)); |
| 3313 | 3338 | |
| 3314 | − bond_for_each_slave(bond, slave, iter) { | |
| 3315 | − const struct rtnl_link_stats64 *sstats = | |
| 3339 | + rcu_read_lock(); | |
| 3340 | + bond_for_each_slave_rcu(bond, slave, iter) { | |
| 3341 | + const struct rtnl_link_stats64 *new = | |
| 3316 | 3342 | dev_get_stats(slave->dev, &temp); |
| 3317 | − struct rtnl_link_stats64 *pstats = &slave->slave_stats; | |
| 3318 | − | |
| 3319 | − stats->rx_packets += sstats->rx_packets - pstats->rx_packets; | |
| 3320 | − stats->rx_bytes += sstats->rx_bytes - pstats->rx_bytes; | |
| 3321 | − stats->rx_errors += sstats->rx_errors - pstats->rx_errors; | |
| 3322 | − stats->rx_dropped += sstats->rx_dropped - pstats->rx_dropped; | |
| 3323 | − stats->rx_nohandler += sstats->rx_nohandler - pstats->rx_nohandler; | |
| 3324 | − | |
| 3325 | − stats->tx_packets += sstats->tx_packets - pstats->tx_packets;; | |
| 3326 | − stats->tx_bytes += sstats->tx_bytes - pstats->tx_bytes; | |
| 3327 | − stats->tx_errors += sstats->tx_errors - pstats->tx_errors; | |
| 3328 | − stats->tx_dropped += sstats->tx_dropped - pstats->tx_dropped; | |
| 3329 | − | |
| 3330 | − stats->multicast += sstats->multicast - pstats->multicast; | |
| 3331 | − stats->collisions += sstats->collisions - pstats->collisions; | |
| 3332 | − | |
| 3333 | − stats->rx_length_errors += sstats->rx_length_errors - pstats->rx_length_errors; | |
| 3334 | − stats->rx_over_errors += sstats->rx_over_errors - pstats->rx_over_errors; | |
| 3335 | − stats->rx_crc_errors += sstats->rx_crc_errors - pstats->rx_crc_errors; | |
| 3336 | − stats->rx_frame_errors += sstats->rx_frame_errors - pstats->rx_frame_errors; | |
| 3337 | − stats->rx_fifo_errors += sstats->rx_fifo_errors - pstats->rx_fifo_errors; | |
| 3338 | − stats->rx_missed_errors += sstats->rx_missed_errors - pstats->rx_missed_errors; | |
| 3339 | − | |
| 3340 | − stats->tx_aborted_errors += sstats->tx_aborted_errors - pstats->tx_aborted_errors; | |
| 3341 | − stats->tx_carrier_errors += sstats->tx_carrier_errors - pstats->tx_carrier_errors; | |
| 3342 | − stats->tx_fifo_errors += sstats->tx_fifo_errors - pstats->tx_fifo_errors; | |
| 3343 | − stats->tx_heartbeat_errors += sstats->tx_heartbeat_errors - pstats->tx_heartbeat_errors; | |
| 3344 | − stats->tx_window_errors += sstats->tx_window_errors - pstats->tx_window_errors; | |
| 3343 | + | |
| 3344 | + bond_fold_stats(stats, new, &slave->slave_stats); | |
| 3345 | 3345 | |
| 3346 | 3346 | /* save off the slave stats for the next run */ |
| 3347 | − memcpy(pstats, sstats, sizeof(*sstats)); | |
| 3347 | + memcpy(&slave->slave_stats, new, sizeof(*new)); | |
| 3348 | 3348 | } |
| 3349 | + rcu_read_unlock(); | |
| 3350 | + | |
| 3349 | 3351 | memcpy(&bond->bond_stats, stats, sizeof(*stats)); |
| 3352 | + spin_unlock(&bond->stats_lock); | |
| 3350 | 3353 | |
| 3351 | 3354 | return stats; |
| 3352 | 3355 | } |
| @@ -4160,6 +4163,7 @@ void bond_setup(struct net_device *bond_dev) · 807 unmodified lines | ||
| 4160 | 4163 | struct bonding *bond = netdev_priv(bond_dev); |
| 4161 | 4164 | |
| 4162 | 4165 | spin_lock_init(&bond->mode_lock); |
| 4166 | + spin_lock_init(&bond->stats_lock); | |
| 4163 | 4167 | bond->params = bonding_defaults; |
| 4164 | 4168 | |
| 4165 | 4169 | /* Initialize pointers */ |
| @@ -215,6 +215,7 @@ struct bonding { · 214 unmodified lines | ||
| 215 | 215 | * ALB mode (6) - to sync the use and modifications of its hash table |
| 216 | 216 | */ |
| 217 | 217 | spinlock_t mode_lock; |
| 218 | + spinlock_t stats_lock; | |
| 218 | 219 | u8 send_peer_notif; |
| 219 | 220 | u8 igmp_retrans; |
| 220 | 221 | #ifdef CONFIG_PROC_FS |