Skip to content

Commit dfe511b

Browse files
hao022NipaLocal
authored andcommitted
net: bonding: use atomic instead of rtnl_mutex, to make sure peer notify updated
Using atomic to protect the send_peer_notif instead of rtnl_mutex. This approach allows safe updates in both interrupt and process contexts, while avoiding code complexity. In lacp mode, the rtnl might be locked, preventing ad_cond_set_peer_notif() from acquiring the lock and updating send_peer_notif. This patch addresses the issue by using a atomic. Since updating send_peer_notif does not require high real-time performance, such atomic updates are acceptable. After coverting the rtnl lock for send_peer_notif to atomic, in bond_mii_monitor(), we should check the should_notify_peers (rtnllock required) instead of send_peer_notif. By the way, to avoid peer notify event loss, we check again whether to send peer notify, such as active-backup mode failover. Cc: Jay Vosburgh <jv@jvosburgh.net> Cc: "David S. Miller" <davem@davemloft.net> Cc: Eric Dumazet <edumazet@google.com> Cc: Jakub Kicinski <kuba@kernel.org> Cc: Paolo Abeni <pabeni@redhat.com> Cc: Simon Horman <horms@kernel.org> Cc: Jonathan Corbet <corbet@lwn.net> Cc: Andrew Lunn <andrew+netdev@lunn.ch> Cc: Nikolay Aleksandrov <razor@blackwall.org> Cc: Hangbin Liu <liuhangbin@gmail.com> Suggested-by: Jay Vosburgh <jv@jvosburgh.net> Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com> Signed-off-by: NipaLocal <nipa@local>
1 parent 493f97e commit dfe511b

File tree

3 files changed

+32
-30
lines changed

3 files changed

+32
-30
lines changed

drivers/net/bonding/bond_3ad.c

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -999,11 +999,8 @@ static void ad_cond_set_peer_notif(struct port *port)
999999
{
10001000
struct bonding *bond = port->slave->bond;
10011001

1002-
if (bond->params.broadcast_neighbor && rtnl_trylock()) {
1003-
bond->send_peer_notif = bond->params.num_peer_notif *
1004-
max(1, bond->params.peer_notif_delay);
1005-
rtnl_unlock();
1006-
}
1002+
if (bond->params.broadcast_neighbor)
1003+
bond_peer_notify_reset(bond);
10071004
}
10081005

10091006
/**

drivers/net/bonding/bond_main.c

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1167,10 +1167,11 @@ static bool bond_should_notify_peers(struct bonding *bond)
11671167
{
11681168
struct bond_up_slave *usable;
11691169
struct slave *slave = NULL;
1170+
int send_peer_notif;
11701171

1171-
if (!bond->send_peer_notif ||
1172-
bond->send_peer_notif %
1173-
max(1, bond->params.peer_notif_delay) != 0 ||
1172+
send_peer_notif = atomic_read(&bond->send_peer_notif);
1173+
if (!send_peer_notif ||
1174+
send_peer_notif % max(1, bond->params.peer_notif_delay) != 0 ||
11741175
!netif_carrier_ok(bond->dev))
11751176
return false;
11761177

@@ -1270,8 +1271,6 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
12701271
BOND_SLAVE_NOTIFY_NOW);
12711272

12721273
if (new_active) {
1273-
bool should_notify_peers = false;
1274-
12751274
bond_set_slave_active_flags(new_active,
12761275
BOND_SLAVE_NOTIFY_NOW);
12771276

@@ -1280,19 +1279,17 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
12801279
old_active);
12811280

12821281
if (netif_running(bond->dev)) {
1283-
bond->send_peer_notif =
1284-
bond->params.num_peer_notif *
1285-
max(1, bond->params.peer_notif_delay);
1286-
should_notify_peers =
1287-
bond_should_notify_peers(bond);
1282+
bond_peer_notify_reset(bond);
1283+
1284+
if (bond_should_notify_peers(bond)) {
1285+
atomic_dec(&bond->send_peer_notif);
1286+
call_netdevice_notifiers(
1287+
NETDEV_NOTIFY_PEERS,
1288+
bond->dev);
1289+
}
12881290
}
12891291

12901292
call_netdevice_notifiers(NETDEV_BONDING_FAILOVER, bond->dev);
1291-
if (should_notify_peers) {
1292-
bond->send_peer_notif--;
1293-
call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
1294-
bond->dev);
1295-
}
12961293
}
12971294
}
12981295

@@ -2801,7 +2798,7 @@ static void bond_mii_monitor(struct work_struct *work)
28012798

28022799
rcu_read_unlock();
28032800

2804-
if (commit || bond->send_peer_notif) {
2801+
if (commit || should_notify_peers) {
28052802
/* Race avoidance with bond_close cancel of workqueue */
28062803
if (!rtnl_trylock()) {
28072804
delay = 1;
@@ -2816,16 +2813,15 @@ static void bond_mii_monitor(struct work_struct *work)
28162813
bond_miimon_commit(bond);
28172814
}
28182815

2819-
if (bond->send_peer_notif) {
2820-
bond->send_peer_notif--;
2821-
if (should_notify_peers)
2822-
call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
2823-
bond->dev);
2824-
}
2816+
/* check again to avoid send_peer_notif has been changed. */
2817+
if (bond_should_notify_peers(bond))
2818+
call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, bond->dev);
28252819

28262820
rtnl_unlock(); /* might sleep, hold no other locks */
28272821
}
28282822

2823+
atomic_dec_if_positive(&bond->send_peer_notif);
2824+
28292825
re_arm:
28302826
if (bond->params.miimon)
28312827
queue_delayed_work(bond->wq, &bond->mii_work, delay);
@@ -3773,7 +3769,7 @@ static void bond_activebackup_arp_mon(struct bonding *bond)
37733769
return;
37743770

37753771
if (should_notify_peers) {
3776-
bond->send_peer_notif--;
3772+
atomic_dec(&bond->send_peer_notif);
37773773
call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
37783774
bond->dev);
37793775
}
@@ -4267,6 +4263,8 @@ static int bond_open(struct net_device *bond_dev)
42674263
queue_delayed_work(bond->wq, &bond->alb_work, 0);
42684264
}
42694265

4266+
atomic_set(&bond->send_peer_notif, 0);
4267+
42704268
if (bond->params.miimon) /* link check interval, in milliseconds. */
42714269
queue_delayed_work(bond->wq, &bond->mii_work, 0);
42724270

@@ -4300,7 +4298,7 @@ static int bond_close(struct net_device *bond_dev)
43004298
struct slave *slave;
43014299

43024300
bond_work_cancel_all(bond);
4303-
bond->send_peer_notif = 0;
4301+
atomic_set(&bond->send_peer_notif, 0);
43044302
if (bond_is_lb(bond))
43054303
bond_alb_deinitialize(bond);
43064304
bond->recv_probe = NULL;

include/net/bonding.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ struct bonding {
236236
*/
237237
spinlock_t mode_lock;
238238
spinlock_t stats_lock;
239-
u32 send_peer_notif;
239+
atomic_t send_peer_notif;
240240
u8 igmp_retrans;
241241
#ifdef CONFIG_PROC_FS
242242
struct proc_dir_entry *proc_entry;
@@ -814,4 +814,11 @@ static inline netdev_tx_t bond_tx_drop(struct net_device *dev, struct sk_buff *s
814814
return NET_XMIT_DROP;
815815
}
816816

817+
static inline void bond_peer_notify_reset(struct bonding *bond)
818+
{
819+
atomic_set(&bond->send_peer_notif,
820+
bond->params.num_peer_notif *
821+
max(1, bond->params.peer_notif_delay));
822+
}
823+
817824
#endif /* _NET_BONDING_H */

0 commit comments

Comments
 (0)