Message-Id: <EB500915-0C30-4491-8709-80894B771EC4@bamaicloud.com>
Date: Tue, 17 Jun 2025 18:47:09 +0800
From: Tonghao Zhang <tonghao@...aicloud.com>
To: Jay Vosburgh <jv@...sburgh.net>
Cc: netdev@...r.kernel.org,
 "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>,
 Steven Rostedt <rostedt@...dmis.org>,
 Masami Hiramatsu <mhiramat@...nel.org>,
 Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 Nikolay Aleksandrov <razor@...ckwall.org>,
 Zengbing Tu <tuzengbing@...iglobal.com>
Subject: Re: [net-next v6 3/4] net: bonding: send peer notify when failure
 recovery



> 2025年6月17日 08:36，Jay Vosburgh <jv@...sburgh.net> 写道：
> 
> Tonghao Zhang <tonghao@...aicloud.com> 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.
> 
> I think the description above needs to mention that the
> gratuitous ARP/ND only happens if broadcast_neighbor is enabled.
Ok, thanks
> 
> I'll note that the documentation update does include this
> caveat.
> 
>> 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>
>> Cc: Steven Rostedt <rostedt@...dmis.org>
>> Cc: Masami Hiramatsu <mhiramat@...nel.org>
>> Cc: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
>> Cc: Nikolay Aleksandrov <razor@...ckwall.org>
>> Signed-off-by: Tonghao Zhang <tonghao@...aicloud.com>
>> Signed-off-by: Zengbing Tu <tuzengbing@...iglobal.com>
>> Reviewed-by: Nikolay Aleksandrov <razor@...ckwall.org>
>> ---
>> 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 12046ef51569..0acece55d9cb 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -1237,17 +1237,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) ||
>> -     test_bit(__LINK_STATE_LINKWATCH_PENDING, &slave->dev->state))
>> +     !netif_carrier_ok(bond->dev))
>> return false;
>> 
>> + if (BOND_MODE(bond) == BOND_MODE_8023AD) {
>> + usable = rcu_dereference_rtnl(bond->usable_slaves);
>> + 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");
> 
> Is it actually correct that if slave == NULL, the notify peers
> logic will send to all ports?  I'm not sure why this changed.
when bond is lacp mode, and send_peer_notif > 0, usable_slaves > 0, then slave == NULL, the debug log will print info "bond_should_notify_peers: slave all".
In active-backup mode, slave is not NULL, that means "bond_should_notify_peers: slave xx".
> 
> -J
> 
>> 
>> return true;
>> }
>> -- 
>> 2.34.1
> 
> ---
> -Jay Vosburgh, jv@...sburgh.net

Powered by blists - more mailing lists