Received-SPF: pass (google.com: domain of linux-kernel+bounces-8865-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Thu, 21 Dec 2023 18:35:31 +0100
From: Simon Horman <horms@kernel.org>
To: Aahil Awatramani <aahila@google.com>
Cc: Mahesh Bandewar <maheshb@google.com>,
	Jay Vosburgh <j.vosburgh@gmail.com>,
	Andy Gospodarek <andy@greyhouse.net>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Martin KaFai Lau <martin.lau@kernel.org>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	Daniel Borkmann <daniel@iogearbox.net>, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH next] bonding: Extending LACP MUX State Machine to
 include a Collecting State.
Message-ID: <20231221173531.GA1202958@kernel.org>
References: <20231221023650.3208767-1-aahila@google.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231221023650.3208767-1-aahila@google.com>

On Thu, Dec 21, 2023 at 02:36:50AM +0000, Aahil Awatramani wrote:
> Introduces two new states, AD_MUX_COLLECTING and AD_MUX_DISTRIBUTING in
> the LACP MUX state machine for separated handling of an initial
> Collecting state before the Collecting and Distributing state. This
> enables a port to be in a state where it can receive incoming packets
> while not still distributing. This is useful for reducing packet loss when
> a port begins distributing before its partner is able to collect.
> Additionally this also brings the 802.3ad bonding driver's implementation
> closer to the LACP specification which already predefined this behaviour.
> 
> With this change, 802.3ad mode will use new
> bond_set_slave_txrx_{enabled|disabled}_flags() set of functions only
> instead of the earlier one (bond_set_slave_{active|inactive}_flags).
> Additionally, it adds new functions such as
> bond_set_slave_tx_disabled_flags and bond_set_slave_rx_enabled_flags to
> precisely manage the port's collecting and distributing states.
> Previously, there was no dedicated method to disable TX while keeping RX
> enabled, which this patch addresses.
> 
> Note that the regular flow process in the kernel's bonding driver remains
> unaffected by this patch. The extension requires explicit opt-in by the
> user (in order to ensure no disruptions for existing setups) via netlink
> or sysfs support using the new bonding parameter lacp_extended_mux. The
> default value for lacp_extended_mux is set to 0 so as to preserve existing
> behaviour.
> 
> Signed-off-by: Aahil Awatramani <aahila@google.com>

...

> @@ -1906,6 +2005,46 @@ static void ad_initialize_port(struct port *port, int lacp_fast)
>  	}
>  }
>  
> +/**
> + * ad_enable_collecting - enable a port's receive
> + * @port: the port we're looking at
> + * @update_slave_arr: Does slave array need update?

The line above documenting @update_slave_arr
should be removed as the parameter is not in
the function definition below.

> + *
> + * Enable @port if it's in an active aggregator
> + */
> +static void ad_enable_collecting(struct port *port)
> +{
> +	if (port->aggregator->is_active) {
> +		struct slave *slave = port->slave;
> +
> +		slave_dbg(slave->bond->dev, slave->dev,
> +			  "Enabling collecting on port %d (LAG %d)\n",
> +			  port->actor_port_number,
> +			  port->aggregator->aggregator_identifier);
> +		__enable_collecting_port(port);
> +	}
> +}

The above is a pretty minor problem, but the bots are likely
to complain about it, so please fix it in v2 after waiting
for feedback from others on this patch.

When posting v2 please target it at the net-next tree.

	Subject: [PATCH net-next v2] ...

-- 
pw-bot: changes-requested