[RFC PATCH net-next 05/13] net: bonding: hold the netdev lists lock when retrieving device statistics

Vladimir Oltean
2020-12-07T00:01:20Z

In the effort of making .ndo_get_stats64 be able to sleep, we need to
ensure the callers of dev_get_stats do not use atomic context.

The bonding driver uses an RCU read-side critical section to ensure the
integrity of the list of network interfaces, because the driver iterates
through all net devices in the netns to find the ones which are its
configured slaves. We still need some protection against an interface
registering or deregistering, and the writer-side lock, the netns mutex,
is fine for that, because it offers sleepable context.

This mutex now serves double duty. It offers code serialization,
something which the stats_lock already did. So now that serves no
purpose, let's remove it.

Cc: Jay Vosburgh <j.vosburgh@gmail.com>
Cc: Veaceslav Falico <vfalico@gmail.com>
Cc: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/bonding/bond_main.c | 16 +++++-----------
include/net/bonding.h | 1 -
2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e0880a3840d7..f788f9fa1858 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3738,21 +3738,16 @@ static void bond_get_stats(struct net_device *bond_dev,
struct rtnl_link_stats64 *stats)
{
struct bonding *bond = netdev_priv(bond_dev);
+ struct net *net = dev_net(bond_dev);
struct rtnl_link_stats64 temp;
struct list_head *iter;
struct slave *slave;
- int nest_level = 0;

+ mutex_lock(&net->netdev_lists_lock);

- rcu_read_lock();
-#ifdef CONFIG_LOCKDEP
- nest_level = bond_get_lowest_level_rcu(bond_dev);
-#endif
-
- spin_lock_nested(&bond->stats_lock, nest_level);
memcpy(stats, &bond->bond_stats, sizeof(*stats));

- bond_for_each_slave_rcu(bond, slave, iter) {
+ bond_for_each_slave(bond, slave, iter) {
const struct rtnl_link_stats64 *new =
dev_get_stats(slave->dev, &temp);

@@ -3763,8 +3758,8 @@ static void bond_get_stats(struct net_device *bond_dev,
}

memcpy(&bond->bond_stats, stats, sizeof(*stats));
- spin_unlock(&bond->stats_lock);
- rcu_read_unlock();
+
+ mutex_unlock(&net->netdev_lists_lock);
}

static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd)
@@ -5192,7 +5187,6 @@ static int bond_init(struct net_device *bond_dev)
if (!bond->wq)
return -ENOMEM;

- spin_lock_init(&bond->stats_lock);
netdev_lockdep_set_classes(bond_dev);

list_add_tail(&bond->bond_list, &bn->dev_list);
diff --git a/include/net/bonding.h b/include/net/bonding.h
index d9d0ff3b0ad3..6fbde9713424 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -224,7 +224,6 @@ struct bonding {
* ALB mode (6) - to sync the use and modifications of its hash table
*/
spinlock_t mode_lock;
- spinlock_t stats_lock;
u8 send_peer_notif;
u8 igmp_retrans;
#ifdef CONFIG_PROC_FS
--
2.25.1