netdev - Re: [PATCH net-next v2 3/4] net: bonding: send peer notify when failure recovery

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b7a91eeb-4bf4-4bb7-ab2b-aa4b03ebcf70@blackwall.org>
Date: Tue, 13 May 2025 13:37:57 +0300
From: Nikolay Aleksandrov <razor@...ckwall.org>
To: Tonghao Zhang <tonghao@...aicloud.com>, netdev@...r.kernel.org
Cc: Jay Vosburgh <jv@...sburgh.net>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
 Jonathan Corbet <corbet@....net>, Andrew Lunn <andrew+netdev@...n.ch>,
 Zengbing Tu <tuzengbing@...iglobal.com>
Subject: Re: [PATCH net-next v2 3/4] net: bonding: send peer notify when
 failure recovery

On 5/13/25 12:47, Tonghao Zhang wrote:
> After LACP protocol recovery, the port can transmit packets.
> However, if the bond port doesn't send gratuitous ARP/ND
> packets to the switch, the switch won't return packets through
> the current interface. This causes traffic imbalance. To resolve
> this issue, when LACP protocol recovers, send ARP/ND packets.
> 
> Cc: Jay Vosburgh <jv@...sburgh.net>
> Cc: "David S. Miller" <davem@...emloft.net>
> Cc: Eric Dumazet <edumazet@...gle.com>
> Cc: Jakub Kicinski <kuba@...nel.org>
> Cc: Paolo Abeni <pabeni@...hat.com>
> Cc: Simon Horman <horms@...nel.org>
> Cc: Jonathan Corbet <corbet@....net>
> Cc: Andrew Lunn <andrew+netdev@...n.ch>
> Signed-off-by: Tonghao Zhang <tonghao@...aicloud.com>
> Signed-off-by: Zengbing Tu <tuzengbing@...iglobal.com>
> ---
>  Documentation/networking/bonding.rst |  5 +++--
>  drivers/net/bonding/bond_3ad.c       | 13 +++++++++++++
>  drivers/net/bonding/bond_main.c      | 21 ++++++++++++++++-----
>  3 files changed, 32 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/networking/bonding.rst b/Documentation/networking/bonding.rst
> index 14f7593d888d..f8f5766703d4 100644
> --- a/Documentation/networking/bonding.rst
> +++ b/Documentation/networking/bonding.rst
> @@ -773,8 +773,9 @@ num_unsol_na
>  	greater than 1.
>  
>  	The valid range is 0 - 255; the default value is 1.  These options
> -	affect only the active-backup mode.  These options were added for
> -	bonding versions 3.3.0 and 3.4.0 respectively.
> +	affect the active-backup or 802.3ad (broadcast_neighbor enabled) mode.
> +	These options were added for bonding versions 3.3.0 and 3.4.0
> +	respectively.
>  
>  	From Linux 3.0 and bonding version 3.7.1, these notifications
>  	are generated by the ipv4 and ipv6 code and the numbers of
> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
> index c6807e473ab7..d1c2d416ac87 100644
> --- a/drivers/net/bonding/bond_3ad.c
> +++ b/drivers/net/bonding/bond_3ad.c
> @@ -982,6 +982,17 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
>  	return 0;
>  }
>  
> +static void ad_cond_set_peer_notif(struct port *port)
> +{
> +	struct bonding *bond = port->slave->bond;
> +
> +	if (bond->params.broadcast_neighbor && rtnl_trylock()) {
> +		bond->send_peer_notif = bond->params.num_peer_notif *
> +			max(1, bond->params.peer_notif_delay);
> +		rtnl_unlock();
> +	}> +}
> +
>  /**
>   * ad_mux_machine - handle a port's mux state machine
>   * @port: the port we're looking at
> @@ -2061,6 +2072,8 @@ static void ad_enable_collecting_distributing(struct port *port,
>  		__enable_port(port);
>  		/* Slave array needs update */
>  		*update_slave_arr = true;
> +		/* Should notify peers if possible */
> +		ad_cond_set_peer_notif(port);
>  	}
>  }
>  
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 8ee26ddddbc8..70bb1e33cee2 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1243,17 +1243,28 @@ static struct slave *bond_find_best_slave(struct bonding *bond)
>  /* must be called in RCU critical section or with RTNL held */
>  static bool bond_should_notify_peers(struct bonding *bond)
>  {
> -	struct slave *slave = rcu_dereference_rtnl(bond->curr_active_slave);
> +	struct bond_up_slave *usable;
> +	struct slave *slave = NULL;
>  
> -	if (!slave || !bond->send_peer_notif ||
> +	if (!bond->send_peer_notif ||
>  	    bond->send_peer_notif %
>  	    max(1, bond->params.peer_notif_delay) != 0 ||
> -	    !netif_carrier_ok(bond->dev) ||
> +	    !netif_carrier_ok(bond->dev))>  		return false;
>  
> +	if (BOND_MODE(bond) == BOND_MODE_8023AD) {
> +		usable = rtnl_dereference(bond->usable_slaves);

If mii monitor is enabled in 802.3ad bond mode then this will be 
dereferenced in bond_mii_monitor() with rcu, so you'd have to use
rcu_dereference_rtnl() instead.

> +		if (!usable || !READ_ONCE(usable->count))
> +			return false;
> +	} else {
> +		slave = rcu_dereference_rtnl(bond->curr_active_slave);
> +		if (!slave || test_bit(__LINK_STATE_LINKWATCH_PENDING,
> +				       &slave->dev->state))
> +			return false;
> +	}
> +
>  	netdev_dbg(bond->dev, "bond_should_notify_peers: slave %s\n",
> -		   slave ? slave->dev->name : "NULL");
> +		   slave ? slave->dev->name : "all");
>  
>  	return true;
>  }