Re: [PATCH net-next v3 2/4] net: bonding: move bond_should_notify_peers, e.g. into rtnl lock block

[Hangbin Liu <liuhangbin@gmail.com>]

On Sun, Nov 30, 2025 at 03:48:44PM +0800, Tonghao Zhang wrote:
> This patch trys to fix the possible peer notify event loss.
> 
> In bond_mii_monitor()/bond_activebackup_arp_mon(), when we hold the rtnl lock:
> - check send_peer_notif again to avoid unconditionally reducing this value.
> - send_peer_notif may have been reset. Therefore, it is necessary to check
>   whether to send peer notify via bond_should_notify_peers() to avoid the
>   loss of notification events.
> 
> 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>
> Cc: Jason Xing <kerneljasonxing@gmail.com>
> Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
> ---
> v1:
> - splitted from: https://patchwork.kernel.org/project/netdevbpf/patch/20251118090431.35654-1-tonghao@bamaicloud.com/
> - this patch only move the bond_should_notify_peers to rtnl lock.
> - add this patch to series
> ---
>  drivers/net/bonding/bond_main.c | 25 ++++++++++---------------
>  1 file changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 811ced7680c1..1b16c4cd90e0 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2809,11 +2809,10 @@ static void bond_mii_monitor(struct work_struct *work)
>  {
>  	struct bonding *bond = container_of(work, struct bonding,
>  					    mii_work.work);
> -	bool should_notify_peers;
> -	bool commit;
> -	unsigned long delay;
> -	struct slave *slave;
>  	struct list_head *iter;
> +	struct slave *slave;
> +	unsigned long delay;
> +	bool commit;
>  
>  	delay = msecs_to_jiffies(bond->params.miimon);
>  
> @@ -2822,7 +2821,6 @@ static void bond_mii_monitor(struct work_struct *work)
>  
>  	rcu_read_lock();
>  
> -	should_notify_peers = bond_should_notify_peers(bond);
>  	commit = !!bond_miimon_inspect(bond);
>  
>  	rcu_read_unlock();
> @@ -2843,10 +2841,10 @@ static void bond_mii_monitor(struct work_struct *work)
>  		}
>  
>  		if (bond->send_peer_notif) {
> -			bond->send_peer_notif--;
> -			if (should_notify_peers)
> +			if (bond_should_notify_peers(bond))
>  				call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
>  							 bond->dev);
> +			bond->send_peer_notif--;
>  		}
>  
>  		rtnl_unlock();	/* might sleep, hold no other locks */
> @@ -3758,7 +3756,6 @@ static bool bond_ab_arp_probe(struct bonding *bond)
>  
>  static void bond_activebackup_arp_mon(struct bonding *bond)
>  {
> -	bool should_notify_peers = false;
>  	bool should_notify_rtnl = false;
>  	int delta_in_ticks;
>  
> @@ -3769,15 +3766,12 @@ static void bond_activebackup_arp_mon(struct bonding *bond)
>  
>  	rcu_read_lock();
>  
> -	should_notify_peers = bond_should_notify_peers(bond);
> -
>  	if (bond_ab_arp_inspect(bond)) {
>  		rcu_read_unlock();
>  
>  		/* Race avoidance with bond_close flush of workqueue */
>  		if (!rtnl_trylock()) {
>  			delta_in_ticks = 1;
> -			should_notify_peers = false;
>  			goto re_arm;
>  		}
>  
> @@ -3794,14 +3788,15 @@ static void bond_activebackup_arp_mon(struct bonding *bond)
>  	if (bond->params.arp_interval)
>  		queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks);
>  
> -	if (should_notify_peers || should_notify_rtnl) {
> +	if (bond->send_peer_notif || should_notify_rtnl) {
>  		if (!rtnl_trylock())
>  			return;
>  
> -		if (should_notify_peers) {
> +		if (bond->send_peer_notif) {
> +			if (bond_should_notify_peers(bond))
> +				call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
> +							 bond->dev);
>  			bond->send_peer_notif--;
> -			call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
> -						 bond->dev);
>  		}
>  		if (should_notify_rtnl) {
>  			bond_slave_state_notify(bond);
> -- 
> 2.34.1
> 

Reviewed-by: Hangbin Liu <liuhangbin@gmail.com>