diff options
author | Veaceslav Falico <vfalico@redhat.com> | 2013-06-17 13:30:35 -0400 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2013-06-17 19:27:24 -0400 |
commit | cedb743f3e8db12cc76a706888792497953e93bb (patch) | |
tree | d3e356ab352b1cd99f18fd46219472ab20a01d69 | |
parent | 72bb72b0d98847d22c6fae4e170121f3640f0f60 (diff) |
bonding: don't call alb_set_slave_mac_addr() while atomic
alb_set_slave_mac_addr() sets the mac address in alb mode via
dev_set_mac_address(), which might sleep. It's called from
alb_handle_addr_collision_on_attach() in atomic context (under
read_lock(bond->lock)), thus triggering a bug.
Fix this by moving the lock inside alb_handle_addr_collision_on_attach().
v1->v2:
As Nikolay Aleksandrov noticed, we can drop the bond->lock completely.
Also, use bond_slave_has_mac(), when possible.
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r-- | drivers/net/bonding/bond_alb.c | 40 |
1 files changed, 5 insertions, 35 deletions
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c index a236234d2c27..27fe329bdf83 100644 --- a/drivers/net/bonding/bond_alb.c +++ b/drivers/net/bonding/bond_alb.c | |||
@@ -1175,16 +1175,13 @@ static void alb_change_hw_addr_on_detach(struct bonding *bond, struct slave *sla | |||
1175 | * @slave. | 1175 | * @slave. |
1176 | * | 1176 | * |
1177 | * assumption: this function is called before @slave is attached to the | 1177 | * assumption: this function is called before @slave is attached to the |
1178 | * bond slave list. | 1178 | * bond slave list. |
1179 | * | ||
1180 | * caller must hold the bond lock for write since the mac addresses are compared | ||
1181 | * and may be swapped. | ||
1182 | */ | 1179 | */ |
1183 | static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slave *slave) | 1180 | static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slave *slave) |
1184 | { | 1181 | { |
1185 | struct slave *tmp_slave1, *tmp_slave2, *free_mac_slave; | 1182 | struct slave *tmp_slave1, *free_mac_slave = NULL; |
1186 | struct slave *has_bond_addr = bond->curr_active_slave; | 1183 | struct slave *has_bond_addr = bond->curr_active_slave; |
1187 | int i, j, found = 0; | 1184 | int i; |
1188 | 1185 | ||
1189 | if (bond->slave_cnt == 0) { | 1186 | if (bond->slave_cnt == 0) { |
1190 | /* this is the first slave */ | 1187 | /* this is the first slave */ |
@@ -1196,15 +1193,7 @@ static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slav | |||
1196 | * slaves in the bond. | 1193 | * slaves in the bond. |
1197 | */ | 1194 | */ |
1198 | if (!ether_addr_equal_64bits(slave->perm_hwaddr, bond->dev->dev_addr)) { | 1195 | if (!ether_addr_equal_64bits(slave->perm_hwaddr, bond->dev->dev_addr)) { |
1199 | bond_for_each_slave(bond, tmp_slave1, i) { | 1196 | if (!bond_slave_has_mac(bond, slave->dev->dev_addr)) |
1200 | if (ether_addr_equal_64bits(tmp_slave1->dev->dev_addr, | ||
1201 | slave->dev->dev_addr)) { | ||
1202 | found = 1; | ||
1203 | break; | ||
1204 | } | ||
1205 | } | ||
1206 | |||
1207 | if (!found) | ||
1208 | return 0; | 1197 | return 0; |
1209 | 1198 | ||
1210 | /* Try setting slave mac to bond address and fall-through | 1199 | /* Try setting slave mac to bond address and fall-through |
@@ -1215,19 +1204,8 @@ static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slav | |||
1215 | /* The slave's address is equal to the address of the bond. | 1204 | /* The slave's address is equal to the address of the bond. |
1216 | * Search for a spare address in the bond for this slave. | 1205 | * Search for a spare address in the bond for this slave. |
1217 | */ | 1206 | */ |
1218 | free_mac_slave = NULL; | ||
1219 | |||
1220 | bond_for_each_slave(bond, tmp_slave1, i) { | 1207 | bond_for_each_slave(bond, tmp_slave1, i) { |
1221 | found = 0; | 1208 | if (!bond_slave_has_mac(bond, tmp_slave1->perm_hwaddr)) { |
1222 | bond_for_each_slave(bond, tmp_slave2, j) { | ||
1223 | if (ether_addr_equal_64bits(tmp_slave1->perm_hwaddr, | ||
1224 | tmp_slave2->dev->dev_addr)) { | ||
1225 | found = 1; | ||
1226 | break; | ||
1227 | } | ||
1228 | } | ||
1229 | |||
1230 | if (!found) { | ||
1231 | /* no slave has tmp_slave1's perm addr | 1209 | /* no slave has tmp_slave1's perm addr |
1232 | * as its curr addr | 1210 | * as its curr addr |
1233 | */ | 1211 | */ |
@@ -1607,15 +1585,7 @@ int bond_alb_init_slave(struct bonding *bond, struct slave *slave) | |||
1607 | return res; | 1585 | return res; |
1608 | } | 1586 | } |
1609 | 1587 | ||
1610 | /* caller must hold the bond lock for write since the mac addresses | ||
1611 | * are compared and may be swapped. | ||
1612 | */ | ||
1613 | read_lock(&bond->lock); | ||
1614 | |||
1615 | res = alb_handle_addr_collision_on_attach(bond, slave); | 1588 | res = alb_handle_addr_collision_on_attach(bond, slave); |
1616 | |||
1617 | read_unlock(&bond->lock); | ||
1618 | |||
1619 | if (res) { | 1589 | if (res) { |
1620 | return res; | 1590 | return res; |
1621 | } | 1591 | } |