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

[Jay Vosburgh <jv@jvosburgh.net>]

tonghao@bamaicloud.com wrote:

>From: Tonghao Zhang <tonghao@bamaicloud.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@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>
>Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.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@jvosburgh.net