[PATCH 6.12.y] bonding: re-check pending peer notification under RTNL in ARP monitor

[PATCH 6.12.y] bonding: re-check pending peer notification under RTNL in ARP monitor

[Struan Bartlett]

commit 70fc6649b36cf22167bbd7e2a6cc0960b746316e upstream.

This is not a backport of that commit but a minimal equivalent of it
for 6.12.y, as explained below.

In active-backup mode with the ARP monitor, if bond_ab_arp_commit()
fails over while a peer notification is pending,
bond_activebackup_arp_mon() decrements send_peer_notif twice and wraps
it:

1. A failover sets send_peer_notif to num_peer_notif (1 by default).
   If the notification cannot be sent at once, for example because the
   new active slave still has __LINK_STATE_LINKWATCH_PENDING, it stays
   pending.
2. The next run of bond_activebackup_arp_mon() sets
   should_notify_peers from bond_should_notify_peers() before calling
   bond_ab_arp_inspect(). If bond_ab_arp_commit() then fails over
   again, for example to the primary as its link comes up,
   bond_change_active_slave() resets send_peer_notif to 1, sends the
   notification and decrements send_peer_notif to 0.
3. The monitor then acts on its earlier should_notify_peers under
   RTNL: send_peer_notif-- wraps from 0 to UINT_MAX, and it sends a
   second notification.

bond_should_notify_peers() is then true on every run, so the bond
sends NETDEV_NOTIFY_PEERS every arp_interval until the counter runs
down, which takes years. At boot this happens whenever a backup
slave's link comes up shortly before the primary's. Every listener
sees the notifications: FRR's zebra counts each one as the bond's link
coming up, and ntpd postpones its interface rescan for as long as they
continue (which can lead to the clock being unsynchronised).

Fix it by checking bond_should_notify_peers() again once RTNL is held,
before decrementing the counter and sending the notification.
bond_change_active_slave() runs under RTNL, so the re-check sees any
failover made since should_notify_peers was set.

Upstream, commit 70fc6649b36c ("net: bonding: move
bond_should_notify_peers, e.g. into rtnl lock block") also checks
send_peer_notif again under RTNL before decrementing it, in both
monitors, using bond_peer_notify_may_events() from
commit 11ea9b8a88dd ("net: bonding: use workqueue to make sure peer
notify updated in lacp mode"). Neither commit applies to 6.12.y, and
together they also change when the counter is decremented. This patch
adds only the re-check, to the ARP monitor, and otherwise keeps
6.12.y's behaviour: the ARP monitor still sends and counts a
notification only when bond_should_notify_peers() allows it. The MII
monitor in 6.12.y already checks send_peer_notif under RTNL before
decrementing it, so it cannot wrap, and is left unchanged.

Fixes: 7a9214f3d88c ("bonding: ARP monitor spams NETDEV_NOTIFY_PEERS notifiers")
Assisted-by: claude-opus-5-5
Signed-off-by: Struan Bartlett <struan.bartlett@newsnow.co.uk>
---

Notes:
    Please apply to 6.12.y. Newer stable trees: 7.x has 70fc6649b36c, and
    for 6.18.y, where both upstream commits apply cleanly, I have asked
    separately for them to be applied.
    
    The same change also applies to 6.6.y, 6.1.y, 5.15.y and 5.10.y, which
    have the same ARP monitor code; I have not tested it on them.
    
    Testing: each kernel below is Debian's, booted in a VM. The bonding
    module was built from the matching stable sources against Debian's
    headers with the compiler Debian used, and loaded in place of Debian's
    own. A fuller version of the reproducer below counted the bond's
    NETDEV_NOTIFY_PEERS events for 3 s just after the failback to the
    primary and, if there were any, for 3 s again 30 s later, after the
    255 set before the failover would have run out. With arp_interval
    100, a bond that notifies on every ARP monitor run sends about 30 in
    3 s:
    
      kernel             bonding module                       after  30 s later
      6.12.95-1~bpo12+1  Debian's                               29     28
      6.12.95-1~bpo12+1  v6.12.95, unchanged                    29     29
      6.12.95-1~bpo12+1  v6.12.95 + this patch                   0      -
      6.18.9-1~bpo13+1   Debian's                               29     28
      6.18.9-1~bpo13+1   v6.18.9, unchanged                     29     29
      6.18.9-1~bpo13+1   v6.18.9 + this patch                    0      -
      6.18.9-1~bpo13+1   v6.18.9 + 11ea9b8a88dd + 70fc6649b36c   0      -
    
    The reproducer below, exactly as given, printed 29 with Debian's
    6.12.95 module and 0 with this patch. The patch applies to v6.12.115
    with an offset of one line; the code it touches is the same as in
    6.12.95.
    
    We found this on a Dell R630 server (igb) running 6.12.95 with:
    mode active-backup, miimon 0, arp_interval 250, arp_validate all,
    six arp_ip_targets, primary set, num_grat_arp 1. At boot a backup
    port's link came up 0.3 s before the primary's, and from then on
    "ip -d monitor link" showed "bond0: ... event NOTIFY PEERS" every
    256 ms. A kprobe on call_netdevice_notifiers for NETDEV_NOTIFY_PEERS
    showed each one coming from bond_arp_monitor. Moving the active slave
    away and back through sysfs (active_slave) stops it, as that failover
    happens outside the ARP monitor.
    
    Reproducer (as root; it uses two new network namespaces, so it does
    not touch the host's own interfaces). It prints the number of
    NETDEV_NOTIFY_PEERS events the bond sends in 3 s, 30 s after the last
    failover: about 30 (one per 100 ms ARP monitor run) on an affected
    kernel, 0 on a fixed one.
    
    	ip netns add h; ip netns add s
    	ip -n s link add br0 type bridge
    	ip -n s addr add 10.255.0.1/24 dev br0; ip -n s link set br0 up
    	ip -n h link add bond0 type bond mode active-backup arp_interval 100 \
    		arp_ip_target 10.255.0.1 arp_validate all num_grat_arp 1
    	for i in 1 2; do
    		ip -n h link add p$i type veth peer name s$i netns s
    		ip -n s link set s$i master br0 up
    		ip -n h link set p$i master bond0
    	done
    	ip -n h link set bond0 type bond primary p1
    	ip -n h addr add 10.255.0.2/24 dev bond0; ip -n h link set bond0 up
    	sleep 5		# p1 active, both slaves up
    	# Fail over to p2 with 255 notifications to send: one is pending at
    	# every ARP monitor run for the next ~25 s.
    	ip -n h link set bond0 type bond num_grat_arp 255
    	ip -n s link set s1 down; sleep 2	# p2 active
    	# The ARP monitor fails back to the primary while a notification is
    	# pending.
    	ip -n h link set bond0 type bond num_grat_arp 1
    	ip -n s link set s1 up; sleep 30	# p1 active; the 255 have run out
    	timeout 3 ip -n h -d monitor link | grep -c 'bond0: .*NOTIFY PEERS'
    	ip netns del h; ip netns del s
    
    An AI coding assistant (Claude, model claude-opus-5-5) helped trace
    the fault, wrote this patch, its changelog and the reproducer, and
    built and ran the tests above under my direction. I have reviewed the
    patch and changelog.

 drivers/net/bonding/bond_main.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index a6f3ed0..e8bb50f 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3937,7 +3937,12 @@ re_arm:
 		if (!rtnl_trylock())
 			return;
 
-		if (should_notify_peers) {
+		/* A failover in bond_ab_arp_commit() may have reset and used
+		 * send_peer_notif since should_notify_peers was set, so check it
+		 * again now that RTNL is held; decrementing it unconditionally
+		 * could wrap it and send a notification on every probe.
+		 */
+		if (should_notify_peers && bond_should_notify_peers(bond)) {
 			bond->send_peer_notif--;
 			call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
 						 bond->dev);
-- 
2.39.5

Re: [PATCH 6.12.y] bonding: re-check pending peer notification under RTNL in ARP monitor

[Greg KH]

On Fri, Oct 30, 2026 at 10:51:19PM +0000, Struan Bartlett wrote:
> commit 70fc6649b36cf22167bbd7e2a6cc0960b746316e upstream.
> 
> This is not a backport of that commit but a minimal equivalent of it
> for 6.12.y, as explained below.

Yeah, it's not close to that at all, so please don't claim it is :)

We can take this if you get approval from the maintainer of the
subsystem for it.  But really, why not move to 6.18.y instead?

thanks,

greg k-h