netdev - Re: [PATCH net-next 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: <1133780.1746882081@vermin>
Date: Sat, 10 May 2025 15:01:21 +0200
From: Jay Vosburgh <jv@...sburgh.net>
To: tonghao@...aicloud.com
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>
Subject: Re: [PATCH net-next 3/4] net: bonding: send peer notify when failure recovery

tonghao@...aicloud.com wrote:

>From: Tonghao Zhang <tonghao@...aicloud.com>
>
>While hardware failures in NICs, optical transceivers, or switches
>are unavoidable, rapid system recovery can be achieved post-restoration.
>For example, triggering immediate ARP/ND packet transmission upon
>LACP failure recovery enables the system to swiftly resume normal
>operations, thereby minimizing service downtime.

	I think this comment needs to be prefaced with something that
explains that this logic is for the "no stack" architecture.  It don't
need the entire blurb about what that is, though.

>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>
>---
> drivers/net/bonding/bond_3ad.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index c6807e473ab7..6577ce54d115 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -982,6 +982,19 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
> 	return 0;
> }
> 
>+static void ad_peer_notif_send(struct port *port)
>+{
>+	if (!port->aggregator->is_active)
>+		return;
>+
>+	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();
>+	}
>+}
>+

	I'm not a fan of the function name, as this doesn't actually
send any notifications.  Perhaps "ad_cond_set_peer_notif"?  I.e.,
conditionally set peer notifications on?

> /**
>  * ad_mux_machine - handle a port's mux state machine
>  * @port: the port we're looking at
>@@ -1164,6 +1177,7 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> 			port->actor_oper_port_state |= LACP_STATE_COLLECTING;
> 			port->actor_oper_port_state |= LACP_STATE_DISTRIBUTING;
> 			port->actor_oper_port_state |= LACP_STATE_SYNCHRONIZATION;
>+			ad_peer_notif_send(port);
> 			ad_enable_collecting_distributing(port,
> 							  update_slave_arr);
> 			port->ntt = true;

	This is in the AD_MUX_COLLECTING_DISTRIBUTING case, I think you
need another one of these in the AD_MUX_DISTRIBUTING case a few lines
further down to handle the situation when coupled_control is disabled.

	-J

>-- 
>2.34.1
>

---
	-Jay Vosburgh, jv@...sburgh.net