Skip to content

Commit 5d6dd5b

Browse files
davem330davem330
committed
Merge branch 'bonding_rtnl'
Ding Tianhong says: ==================== Fix RTNL: assertion failed at net/core/rtnetlink.c The commit 1d3ee88 (bonding: add netlink attributes to slave link dev) make the bond_set_active_slave() and bond_set_backup_slave() use rtmsg_ifinfo to send slave's states and this functions should be called in RTNL. But the 902.3ad and ARP monitor did not hold the RTNL when calling thses two functions, so fix them. v1->v2: Add new micro to indicate that the notification should be send later, not never. And add a new patch to fix the same problem for ARP mode. v2->v3: modify the bond_should_notify to should_notify_rtnl, it is more reasonable, and use bool for should_notify_rtnl. ==================== Signed-off-by: David S. Miller <davem@davemloft.net>
2 parents bc90d29 + b092991 commit 5d6dd5b

File tree

3 files changed

+120
-58
lines changed

3 files changed

+120
-58
lines changed

‎drivers/net/bonding/bond_3ad.c

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ static inline int __agg_has_partner(struct aggregator *agg)
181181
*/
182182
static inline void __disable_port(struct port *port)
183183
{
184-
bond_set_slave_inactive_flags(port->slave);
184+
bond_set_slave_inactive_flags(port->slave, BOND_SLAVE_NOTIFY_LATER);
185185
}
186186

187187
/**
@@ -193,7 +193,7 @@ static inline void __enable_port(struct port *port)
193193
struct slave *slave = port->slave;
194194

195195
if ((slave->link == BOND_LINK_UP) && IS_UP(slave->dev))
196-
bond_set_slave_active_flags(slave);
196+
bond_set_slave_active_flags(slave, BOND_SLAVE_NOTIFY_LATER);
197197
}
198198

199199
/**
@@ -2062,6 +2062,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
20622062
struct list_head *iter;
20632063
struct slave *slave;
20642064
struct port *port;
2065+
bool should_notify_rtnl = BOND_SLAVE_NOTIFY_LATER;
20652066

20662067
read_lock(&bond->lock);
20672068
rcu_read_lock();
@@ -2119,8 +2120,19 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
21192120
}
21202121

21212122
re_arm:
2123+
bond_for_each_slave_rcu(bond, slave, iter) {
2124+
if (slave->should_notify) {
2125+
should_notify_rtnl = BOND_SLAVE_NOTIFY_NOW;
2126+
break;
2127+
}
2128+
}
21222129
rcu_read_unlock();
21232130
read_unlock(&bond->lock);
2131+
2132+
if (should_notify_rtnl && rtnl_trylock()) {
2133+
bond_slave_state_notify(bond);
2134+
rtnl_unlock();
2135+
}
21242136
queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks);
21252137
}
21262138

‎drivers/net/bonding/bond_main.c

Lines changed: 64 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -829,21 +829,25 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
829829
if (bond_is_lb(bond)) {
830830
bond_alb_handle_active_change(bond, new_active);
831831
if (old_active)
832-
bond_set_slave_inactive_flags(old_active);
832+
bond_set_slave_inactive_flags(old_active,
833+
BOND_SLAVE_NOTIFY_NOW);
833834
if (new_active)
834-
bond_set_slave_active_flags(new_active);
835+
bond_set_slave_active_flags(new_active,
836+
BOND_SLAVE_NOTIFY_NOW);
835837
} else {
836838
rcu_assign_pointer(bond->curr_active_slave, new_active);
837839
}
838840

839841
if (bond->params.mode == BOND_MODE_ACTIVEBACKUP) {
840842
if (old_active)
841-
bond_set_slave_inactive_flags(old_active);
843+
bond_set_slave_inactive_flags(old_active,
844+
BOND_SLAVE_NOTIFY_NOW);
842845

843846
if (new_active) {
844847
bool should_notify_peers = false;
845848

846-
bond_set_slave_active_flags(new_active);
849+
bond_set_slave_active_flags(new_active,
850+
BOND_SLAVE_NOTIFY_NOW);
847851

848852
if (bond->params.fail_over_mac)
849853
bond_do_fail_over_mac(bond, new_active,
@@ -1463,14 +1467,15 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
14631467

14641468
switch (bond->params.mode) {
14651469
case BOND_MODE_ACTIVEBACKUP:
1466-
bond_set_slave_inactive_flags(new_slave);
1470+
bond_set_slave_inactive_flags(new_slave,
1471+
BOND_SLAVE_NOTIFY_NOW);
14671472
break;
14681473
case BOND_MODE_8023AD:
14691474
/* in 802.3ad mode, the internal mechanism
14701475
* will activate the slaves in the selected
14711476
* aggregator
14721477
*/
1473-
bond_set_slave_inactive_flags(new_slave);
1478+
bond_set_slave_inactive_flags(new_slave, BOND_SLAVE_NOTIFY_NOW);
14741479
/* if this is the first slave */
14751480
if (!prev_slave) {
14761481
SLAVE_AD_INFO(new_slave).id = 1;
@@ -1488,7 +1493,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
14881493
case BOND_MODE_TLB:
14891494
case BOND_MODE_ALB:
14901495
bond_set_active_slave(new_slave);
1491-
bond_set_slave_inactive_flags(new_slave);
1496+
bond_set_slave_inactive_flags(new_slave, BOND_SLAVE_NOTIFY_NOW);
14921497
break;
14931498
default:
14941499
pr_debug("This slave is always active in trunk mode\n");
@@ -2015,7 +2020,8 @@ static void bond_miimon_commit(struct bonding *bond)
20152020

20162021
if (bond->params.mode == BOND_MODE_ACTIVEBACKUP ||
20172022
bond->params.mode == BOND_MODE_8023AD)
2018-
bond_set_slave_inactive_flags(slave);
2023+
bond_set_slave_inactive_flags(slave,
2024+
BOND_SLAVE_NOTIFY_NOW);
20192025

20202026
pr_info("%s: link status definitely down for interface %s, disabling it\n",
20212027
bond->dev->name, slave->dev->name);
@@ -2562,7 +2568,8 @@ static void bond_ab_arp_commit(struct bonding *bond)
25622568
slave->link = BOND_LINK_UP;
25632569
if (bond->current_arp_slave) {
25642570
bond_set_slave_inactive_flags(
2565-
bond->current_arp_slave);
2571+
bond->current_arp_slave,
2572+
BOND_SLAVE_NOTIFY_NOW);
25662573
bond->current_arp_slave = NULL;
25672574
}
25682575

@@ -2582,7 +2589,8 @@ static void bond_ab_arp_commit(struct bonding *bond)
25822589
slave->link_failure_count++;
25832590

25842591
slave->link = BOND_LINK_DOWN;
2585-
bond_set_slave_inactive_flags(slave);
2592+
bond_set_slave_inactive_flags(slave,
2593+
BOND_SLAVE_NOTIFY_NOW);
25862594

25872595
pr_info("%s: link status definitely down for interface %s, disabling it\n",
25882596
bond->dev->name, slave->dev->name);
@@ -2615,17 +2623,17 @@ static void bond_ab_arp_commit(struct bonding *bond)
26152623

26162624
/*
26172625
* Send ARP probes for active-backup mode ARP monitor.
2626+
*
2627+
* Called with rcu_read_lock hold.
26182628
*/
26192629
static bool bond_ab_arp_probe(struct bonding *bond)
26202630
{
26212631
struct slave *slave, *before = NULL, *new_slave = NULL,
2622-
*curr_arp_slave, *curr_active_slave;
2632+
*curr_arp_slave = rcu_dereference(bond->current_arp_slave),
2633+
*curr_active_slave = rcu_dereference(bond->curr_active_slave);
26232634
struct list_head *iter;
26242635
bool found = false;
2625-
2626-
rcu_read_lock();
2627-
curr_arp_slave = rcu_dereference(bond->current_arp_slave);
2628-
curr_active_slave = rcu_dereference(bond->curr_active_slave);
2636+
bool should_notify_rtnl = BOND_SLAVE_NOTIFY_LATER;
26292637

26302638
if (curr_arp_slave && curr_active_slave)
26312639
pr_info("PROBE: c_arp %s && cas %s BAD\n",
@@ -2634,32 +2642,23 @@ static bool bond_ab_arp_probe(struct bonding *bond)
26342642

26352643
if (curr_active_slave) {
26362644
bond_arp_send_all(bond, curr_active_slave);
2637-
rcu_read_unlock();
2638-
return true;
2645+
return should_notify_rtnl;
26392646
}
2640-
rcu_read_unlock();
26412647

26422648
/* if we don't have a curr_active_slave, search for the next available
26432649
* backup slave from the current_arp_slave and make it the candidate
26442650
* for becoming the curr_active_slave
26452651
*/
26462652

2647-
if (!rtnl_trylock())
2648-
return false;
2649-
/* curr_arp_slave might have gone away */
2650-
curr_arp_slave = ACCESS_ONCE(bond->current_arp_slave);
2651-
26522653
if (!curr_arp_slave) {
2653-
curr_arp_slave = bond_first_slave(bond);
2654-
if (!curr_arp_slave) {
2655-
rtnl_unlock();
2656-
return true;
2657-
}
2654+
curr_arp_slave = bond_first_slave_rcu(bond);
2655+
if (!curr_arp_slave)
2656+
return should_notify_rtnl;
26582657
}
26592658

2660-
bond_set_slave_inactive_flags(curr_arp_slave);
2659+
bond_set_slave_inactive_flags(curr_arp_slave, BOND_SLAVE_NOTIFY_LATER);
26612660

2662-
bond_for_each_slave(bond, slave, iter) {
2661+
bond_for_each_slave_rcu(bond, slave, iter) {
26632662
if (!found && !before && IS_UP(slave->dev))
26642663
before = slave;
26652664

@@ -2677,7 +2676,8 @@ static bool bond_ab_arp_probe(struct bonding *bond)
26772676
if (slave->link_failure_count < UINT_MAX)
26782677
slave->link_failure_count++;
26792678

2680-
bond_set_slave_inactive_flags(slave);
2679+
bond_set_slave_inactive_flags(slave,
2680+
BOND_SLAVE_NOTIFY_LATER);
26812681

26822682
pr_info("%s: backup interface %s is now down.\n",
26832683
bond->dev->name, slave->dev->name);
@@ -2689,26 +2689,31 @@ static bool bond_ab_arp_probe(struct bonding *bond)
26892689
if (!new_slave && before)
26902690
new_slave = before;
26912691

2692-
if (!new_slave) {
2693-
rtnl_unlock();
2694-
return true;
2695-
}
2692+
if (!new_slave)
2693+
goto check_state;
26962694

26972695
new_slave->link = BOND_LINK_BACK;
2698-
bond_set_slave_active_flags(new_slave);
2696+
bond_set_slave_active_flags(new_slave, BOND_SLAVE_NOTIFY_LATER);
26992697
bond_arp_send_all(bond, new_slave);
27002698
new_slave->jiffies = jiffies;
27012699
rcu_assign_pointer(bond->current_arp_slave, new_slave);
2702-
rtnl_unlock();
27032700

2704-
return true;
2701+
check_state:
2702+
bond_for_each_slave_rcu(bond, slave, iter) {
2703+
if (slave->should_notify) {
2704+
should_notify_rtnl = BOND_SLAVE_NOTIFY_NOW;
2705+
break;
2706+
}
2707+
}
2708+
return should_notify_rtnl;
27052709
}
27062710

27072711
static void bond_activebackup_arp_mon(struct work_struct *work)
27082712
{
27092713
struct bonding *bond = container_of(work, struct bonding,
27102714
arp_work.work);
2711-
bool should_notify_peers = false, should_commit = false;
2715+
bool should_notify_peers = false;
2716+
bool should_notify_rtnl = false;
27122717
int delta_in_ticks;
27132718

27142719
delta_in_ticks = msecs_to_jiffies(bond->params.arp_interval);
@@ -2717,11 +2722,12 @@ static void bond_activebackup_arp_mon(struct work_struct *work)
27172722
goto re_arm;
27182723

27192724
rcu_read_lock();
2725+
27202726
should_notify_peers = bond_should_notify_peers(bond);
2721-
should_commit = bond_ab_arp_inspect(bond);
2722-
rcu_read_unlock();
27232727

2724-
if (should_commit) {
2728+
if (bond_ab_arp_inspect(bond)) {
2729+
rcu_read_unlock();
2730+
27252731
/* Race avoidance with bond_close flush of workqueue */
27262732
if (!rtnl_trylock()) {
27272733
delta_in_ticks = 1;
@@ -2730,23 +2736,28 @@ static void bond_activebackup_arp_mon(struct work_struct *work)
27302736
}
27312737

27322738
bond_ab_arp_commit(bond);
2739+
27332740
rtnl_unlock();
2741+
rcu_read_lock();
27342742
}
27352743

2736-
if (!bond_ab_arp_probe(bond)) {
2737-
/* rtnl locking failed, re-arm */
2738-
delta_in_ticks = 1;
2739-
should_notify_peers = false;
2740-
}
2744+
should_notify_rtnl = bond_ab_arp_probe(bond);
2745+
rcu_read_unlock();
27412746

27422747
re_arm:
27432748
if (bond->params.arp_interval)
27442749
queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks);
27452750

2746-
if (should_notify_peers) {
2751+
if (should_notify_peers || should_notify_rtnl) {
27472752
if (!rtnl_trylock())
27482753
return;
2749-
call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, bond->dev);
2754+
2755+
if (should_notify_peers)
2756+
call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
2757+
bond->dev);
2758+
if (should_notify_rtnl)
2759+
bond_slave_state_notify(bond);
2760+
27502761
rtnl_unlock();
27512762
}
27522763
}
@@ -3046,9 +3057,11 @@ static int bond_open(struct net_device *bond_dev)
30463057
bond_for_each_slave(bond, slave, iter) {
30473058
if ((bond->params.mode == BOND_MODE_ACTIVEBACKUP)
30483059
&& (slave != bond->curr_active_slave)) {
3049-
bond_set_slave_inactive_flags(slave);
3060+
bond_set_slave_inactive_flags(slave,
3061+
BOND_SLAVE_NOTIFY_NOW);
30503062
} else {
3051-
bond_set_slave_active_flags(slave);
3063+
bond_set_slave_active_flags(slave,
3064+
BOND_SLAVE_NOTIFY_NOW);
30523065
}
30533066
}
30543067
read_unlock(&bond->curr_slave_lock);

‎drivers/net/bonding/bonding.h

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,8 @@ struct slave {
195195
s8 new_link;
196196
u8 backup:1, /* indicates backup slave. Value corresponds with
197197
BOND_STATE_ACTIVE and BOND_STATE_BACKUP */
198-
inactive:1; /* indicates inactive slave */
198+
inactive:1, /* indicates inactive slave */
199+
should_notify:1; /* indicateds whether the state changed */
199200
u8 duplex;
200201
u32 original_mtu;
201202
u32 link_failure_count;
@@ -303,6 +304,24 @@ static inline void bond_set_backup_slave(struct slave *slave)
303304
}
304305
}
305306

307+
static inline void bond_set_slave_state(struct slave *slave,
308+
int slave_state, bool notify)
309+
{
310+
if (slave->backup == slave_state)
311+
return;
312+
313+
slave->backup = slave_state;
314+
if (notify) {
315+
rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_KERNEL);
316+
slave->should_notify = 0;
317+
} else {
318+
if (slave->should_notify)
319+
slave->should_notify = 0;
320+
else
321+
slave->should_notify = 1;
322+
}
323+
}
324+
306325
static inline void bond_slave_state_change(struct bonding *bond)
307326
{
308327
struct list_head *iter;
@@ -316,6 +335,19 @@ static inline void bond_slave_state_change(struct bonding *bond)
316335
}
317336
}
318337

338+
static inline void bond_slave_state_notify(struct bonding *bond)
339+
{
340+
struct list_head *iter;
341+
struct slave *tmp;
342+
343+
bond_for_each_slave(bond, tmp, iter) {
344+
if (tmp->should_notify) {
345+
rtmsg_ifinfo(RTM_NEWLINK, tmp->dev, 0, GFP_KERNEL);
346+
tmp->should_notify = 0;
347+
}
348+
}
349+
}
350+
319351
static inline int bond_slave_state(struct slave *slave)
320352
{
321353
return slave->backup;
@@ -343,6 +375,9 @@ static inline bool bond_is_active_slave(struct slave *slave)
343375
#define BOND_ARP_VALIDATE_ALL (BOND_ARP_VALIDATE_ACTIVE | \
344376
BOND_ARP_VALIDATE_BACKUP)
345377

378+
#define BOND_SLAVE_NOTIFY_NOW true
379+
#define BOND_SLAVE_NOTIFY_LATER false
380+
346381
static inline int slave_do_arp_validate(struct bonding *bond,
347382
struct slave *slave)
348383
{
@@ -394,17 +429,19 @@ static inline void bond_netpoll_send_skb(const struct slave *slave,
394429
}
395430
#endif
396431

397-
static inline void bond_set_slave_inactive_flags(struct slave *slave)
432+
static inline void bond_set_slave_inactive_flags(struct slave *slave,
433+
bool notify)
398434
{
399435
if (!bond_is_lb(slave->bond))
400-
bond_set_backup_slave(slave);
436+
bond_set_slave_state(slave, BOND_STATE_BACKUP, notify);
401437
if (!slave->bond->params.all_slaves_active)
402438
slave->inactive = 1;
403439
}
404440

405-
static inline void bond_set_slave_active_flags(struct slave *slave)
441+
static inline void bond_set_slave_active_flags(struct slave *slave,
442+
bool notify)
406443
{
407-
bond_set_active_slave(slave);
444+
bond_set_slave_state(slave, BOND_STATE_ACTIVE, notify);
408445
slave->inactive = 0;
409446
}
410447

0 commit comments

Comments
 (0)