Commit 91f2210c authored by Jakub Kicinski's avatar Jakub Kicinski

Merge branch 'bonding-remove-rtnl-from-three-sysfs-files'

Eric Dumazet says:

====================
bonding: remove RTNL from three sysfs files

First patch might fix a potential deadlock.
sysfs handlers should use rtnl_trylock() instead of rtnl_lock().

Following files can be read without acquiring RTNL :

- /sys/class/net/bonding_masters
- /sys/class/net/<name>/bonding/slaves
- /sys/class/net/<name>/bonding/queue_id
====================

Link: https://lore.kernel.org/r/20240408190437.2214473-1-edumazet@google.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents d034d02d 662e451d
...@@ -5245,7 +5245,7 @@ static inline int bond_slave_override(struct bonding *bond, ...@@ -5245,7 +5245,7 @@ static inline int bond_slave_override(struct bonding *bond,
/* Find out if any slaves have the same mapping as this skb. */ /* Find out if any slaves have the same mapping as this skb. */
bond_for_each_slave_rcu(bond, slave, iter) { bond_for_each_slave_rcu(bond, slave, iter) {
if (slave->queue_id == skb_get_queue_mapping(skb)) { if (READ_ONCE(slave->queue_id) == skb_get_queue_mapping(skb)) {
if (bond_slave_is_up(slave) && if (bond_slave_is_up(slave) &&
slave->link == BOND_LINK_UP) { slave->link == BOND_LINK_UP) {
bond_dev_queue_xmit(bond, skb, slave->dev); bond_dev_queue_xmit(bond, skb, slave->dev);
...@@ -5933,7 +5933,7 @@ static void bond_uninit(struct net_device *bond_dev) ...@@ -5933,7 +5933,7 @@ static void bond_uninit(struct net_device *bond_dev)
bond_set_slave_arr(bond, NULL, NULL); bond_set_slave_arr(bond, NULL, NULL);
list_del(&bond->bond_list); list_del_rcu(&bond->bond_list);
bond_debug_unregister(bond); bond_debug_unregister(bond);
} }
...@@ -6347,7 +6347,7 @@ static int bond_init(struct net_device *bond_dev) ...@@ -6347,7 +6347,7 @@ static int bond_init(struct net_device *bond_dev)
spin_lock_init(&bond->stats_lock); spin_lock_init(&bond->stats_lock);
netdev_lockdep_set_classes(bond_dev); netdev_lockdep_set_classes(bond_dev);
list_add_tail(&bond->bond_list, &bn->dev_list); list_add_tail_rcu(&bond->bond_list, &bn->dev_list);
bond_prepare_sysfs_group(bond); bond_prepare_sysfs_group(bond);
......
...@@ -51,7 +51,8 @@ static int bond_fill_slave_info(struct sk_buff *skb, ...@@ -51,7 +51,8 @@ static int bond_fill_slave_info(struct sk_buff *skb,
slave_dev->addr_len, slave->perm_hwaddr)) slave_dev->addr_len, slave->perm_hwaddr))
goto nla_put_failure; goto nla_put_failure;
if (nla_put_u16(skb, IFLA_BOND_SLAVE_QUEUE_ID, slave->queue_id)) if (nla_put_u16(skb, IFLA_BOND_SLAVE_QUEUE_ID,
READ_ONCE(slave->queue_id)))
goto nla_put_failure; goto nla_put_failure;
if (nla_put_s32(skb, IFLA_BOND_SLAVE_PRIO, slave->prio)) if (nla_put_s32(skb, IFLA_BOND_SLAVE_PRIO, slave->prio))
......
...@@ -1589,7 +1589,7 @@ static int bond_option_queue_id_set(struct bonding *bond, ...@@ -1589,7 +1589,7 @@ static int bond_option_queue_id_set(struct bonding *bond,
goto err_no_cmd; goto err_no_cmd;
/* Actually set the qids for the slave */ /* Actually set the qids for the slave */
update_slave->queue_id = qid; WRITE_ONCE(update_slave->queue_id, qid);
out: out:
return ret; return ret;
......
...@@ -209,7 +209,7 @@ static void bond_info_show_slave(struct seq_file *seq, ...@@ -209,7 +209,7 @@ static void bond_info_show_slave(struct seq_file *seq,
seq_printf(seq, "Permanent HW addr: %*phC\n", seq_printf(seq, "Permanent HW addr: %*phC\n",
slave->dev->addr_len, slave->perm_hwaddr); slave->dev->addr_len, slave->perm_hwaddr);
seq_printf(seq, "Slave queue ID: %d\n", slave->queue_id); seq_printf(seq, "Slave queue ID: %d\n", READ_ONCE(slave->queue_id));
if (BOND_MODE(bond) == BOND_MODE_8023AD) { if (BOND_MODE(bond) == BOND_MODE_8023AD) {
const struct port *port = &SLAVE_AD_INFO(slave)->port; const struct port *port = &SLAVE_AD_INFO(slave)->port;
......
...@@ -37,12 +37,12 @@ static ssize_t bonding_show_bonds(const struct class *cls, ...@@ -37,12 +37,12 @@ static ssize_t bonding_show_bonds(const struct class *cls,
{ {
const struct bond_net *bn = const struct bond_net *bn =
container_of_const(attr, struct bond_net, class_attr_bonding_masters); container_of_const(attr, struct bond_net, class_attr_bonding_masters);
int res = 0;
struct bonding *bond; struct bonding *bond;
int res = 0;
rtnl_lock(); rcu_read_lock();
list_for_each_entry(bond, &bn->dev_list, bond_list) { list_for_each_entry_rcu(bond, &bn->dev_list, bond_list) {
if (res > (PAGE_SIZE - IFNAMSIZ)) { if (res > (PAGE_SIZE - IFNAMSIZ)) {
/* not enough space for another interface name */ /* not enough space for another interface name */
if ((PAGE_SIZE - res) > 10) if ((PAGE_SIZE - res) > 10)
...@@ -55,7 +55,7 @@ static ssize_t bonding_show_bonds(const struct class *cls, ...@@ -55,7 +55,7 @@ static ssize_t bonding_show_bonds(const struct class *cls,
if (res) if (res)
buf[res-1] = '\n'; /* eat the leftover space */ buf[res-1] = '\n'; /* eat the leftover space */
rtnl_unlock(); rcu_read_unlock();
return res; return res;
} }
...@@ -170,10 +170,9 @@ static ssize_t bonding_show_slaves(struct device *d, ...@@ -170,10 +170,9 @@ static ssize_t bonding_show_slaves(struct device *d,
struct slave *slave; struct slave *slave;
int res = 0; int res = 0;
if (!rtnl_trylock()) rcu_read_lock();
return restart_syscall();
bond_for_each_slave(bond, slave, iter) { bond_for_each_slave_rcu(bond, slave, iter) {
if (res > (PAGE_SIZE - IFNAMSIZ)) { if (res > (PAGE_SIZE - IFNAMSIZ)) {
/* not enough space for another interface name */ /* not enough space for another interface name */
if ((PAGE_SIZE - res) > 10) if ((PAGE_SIZE - res) > 10)
...@@ -184,7 +183,7 @@ static ssize_t bonding_show_slaves(struct device *d, ...@@ -184,7 +183,7 @@ static ssize_t bonding_show_slaves(struct device *d,
res += sysfs_emit_at(buf, res, "%s ", slave->dev->name); res += sysfs_emit_at(buf, res, "%s ", slave->dev->name);
} }
rtnl_unlock(); rcu_read_unlock();
if (res) if (res)
buf[res-1] = '\n'; /* eat the leftover space */ buf[res-1] = '\n'; /* eat the leftover space */
...@@ -626,10 +625,9 @@ static ssize_t bonding_show_queue_id(struct device *d, ...@@ -626,10 +625,9 @@ static ssize_t bonding_show_queue_id(struct device *d,
struct slave *slave; struct slave *slave;
int res = 0; int res = 0;
if (!rtnl_trylock()) rcu_read_lock();
return restart_syscall();
bond_for_each_slave(bond, slave, iter) { bond_for_each_slave_rcu(bond, slave, iter) {
if (res > (PAGE_SIZE - IFNAMSIZ - 6)) { if (res > (PAGE_SIZE - IFNAMSIZ - 6)) {
/* not enough space for another interface_name:queue_id pair */ /* not enough space for another interface_name:queue_id pair */
if ((PAGE_SIZE - res) > 10) if ((PAGE_SIZE - res) > 10)
...@@ -638,12 +636,13 @@ static ssize_t bonding_show_queue_id(struct device *d, ...@@ -638,12 +636,13 @@ static ssize_t bonding_show_queue_id(struct device *d,
break; break;
} }
res += sysfs_emit_at(buf, res, "%s:%d ", res += sysfs_emit_at(buf, res, "%s:%d ",
slave->dev->name, slave->queue_id); slave->dev->name,
READ_ONCE(slave->queue_id));
} }
if (res) if (res)
buf[res-1] = '\n'; /* eat the leftover space */ buf[res-1] = '\n'; /* eat the leftover space */
rtnl_unlock(); rcu_read_unlock();
return res; return res;
} }
......
...@@ -53,7 +53,7 @@ static SLAVE_ATTR_RO(perm_hwaddr); ...@@ -53,7 +53,7 @@ static SLAVE_ATTR_RO(perm_hwaddr);
static ssize_t queue_id_show(struct slave *slave, char *buf) static ssize_t queue_id_show(struct slave *slave, char *buf)
{ {
return sysfs_emit(buf, "%d\n", slave->queue_id); return sysfs_emit(buf, "%d\n", READ_ONCE(slave->queue_id));
} }
static SLAVE_ATTR_RO(queue_id); static SLAVE_ATTR_RO(queue_id);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment