Re: [PATCH net-next 2/3] bonding: support aggregator selection based on port priority

[Nikolay Aleksandrov <razor@blackwall.org>]

On 7/24/25 11:16, Hangbin Liu wrote:
> Now that per-port actor priority is supported via ad_actor_port_prio, enable
> a new ad_select policy to choose the aggregator based on port priority.
> 
> This allows users to influence aggregator selection by assigning higher
> or lower priorities to individual ports.
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>  Documentation/networking/bonding.rst |  9 ++++++++-
>  drivers/net/bonding/bond_3ad.c       | 29 ++++++++++++++++++++++++++++
>  drivers/net/bonding/bond_options.c   |  1 +
>  include/net/bond_3ad.h               |  1 +
>  4 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/networking/bonding.rst b/Documentation/networking/bonding.rst
> index 5e105e7ac8e6..a234ec12ece7 100644
> --- a/Documentation/networking/bonding.rst
> +++ b/Documentation/networking/bonding.rst
> @@ -250,7 +250,14 @@ ad_select
>  		ports (slaves).  Reselection occurs as described under the
>  		"bandwidth" setting, above.
>  
> -	The bandwidth and count selection policies permit failover of
> +	prio or 3
> +
> +		The active aggregator is chosen by the highest total sum of
> +		actor port priorities across its active ports. Note this
> +		priority is ad_actor_port_prio, not per port prio, which is
> +		used for primary reselect.
> +
> +	The bandwidth, count and prio selection policies permit failover of
>  	802.3ad aggregations when partial failure of the active aggregator
>  	occurs.  This keeps the aggregator with the highest availability
>  	(either in bandwidth or in number of ports) active at all times.
> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
> index 4a1b2f01fe37..6f8a406ed34a 100644
> --- a/drivers/net/bonding/bond_3ad.c
> +++ b/drivers/net/bonding/bond_3ad.c
> @@ -747,6 +747,20 @@ static int __agg_active_ports(struct aggregator *agg)
>  	return active;
>  }
>  
> +static unsigned int __agg_ports_priority(struct aggregator *agg)

const agg?

> +{
> +	struct port *port;
> +	unsigned int prio = 0;

reverse xmas tree or alternatively you can save a line below with
port = agg->lag_ports above

> +
> +	for (port = agg->lag_ports; port;
> +	     port = port->next_port_in_aggregator) {
> +		if (port->is_enabled)
> +			prio += port->actor_port_priority;
> +	}

minor nit: {} are unnecessary

> +
> +	return prio;
> +}
> +
>  /**
>   * __get_agg_bandwidth - get the total bandwidth of an aggregator
>   * @aggregator: the aggregator we're looking at
> @@ -1695,6 +1709,9 @@ static struct aggregator *ad_agg_selection_test(struct aggregator *best,
>  	 * BOND_AD_COUNT: Select by count of ports.  If count is equal,
>  	 *     select by bandwidth.
>  	 *
> +	 * BOND_AD_PRIO: Select by total priority of ports. If priority
> +	 *     is equal, select by count.
> +	 *
>  	 * BOND_AD_STABLE, BOND_AD_BANDWIDTH: Select by bandwidth.
>  	 */
>  	if (!best)
> @@ -1713,6 +1730,14 @@ static struct aggregator *ad_agg_selection_test(struct aggregator *best,
>  		return best;
>  
>  	switch (__get_agg_selection_mode(curr->lag_ports)) {
> +	case BOND_AD_PRIO:
> +		if (__agg_ports_priority(curr) > __agg_ports_priority(best))
> +			return curr;
> +
> +		if (__agg_ports_priority(curr) < __agg_ports_priority(best))
> +			return best;
> +
> +		fallthrough;
>  	case BOND_AD_COUNT:
>  		if (__agg_active_ports(curr) > __agg_active_ports(best))
>  			return curr;
> @@ -1778,6 +1803,10 @@ static int agg_device_up(const struct aggregator *agg)
>   * (slaves), and reselect whenever a link state change takes place or the
>   * set of slaves in the bond changes.
>   *
> + * BOND_AD_PRIO: select the aggregator with highest total priority of ports
> + * (slaves), and reselect whenever a link state change takes place or the
> + * set of slaves in the bond changes.
> + *
>   * FIXME: this function MUST be called with the first agg in the bond, or
>   * __get_active_agg() won't work correctly. This function should be better
>   * called with the bond itself, and retrieve the first agg from it.
> diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
> index 2b8606b4e4f5..708ca1f18a00 100644
> --- a/drivers/net/bonding/bond_options.c
> +++ b/drivers/net/bonding/bond_options.c
> @@ -163,6 +163,7 @@ static const struct bond_opt_value bond_ad_select_tbl[] = {
>  	{ "stable",    BOND_AD_STABLE,    BOND_VALFLAG_DEFAULT},
>  	{ "bandwidth", BOND_AD_BANDWIDTH, 0},
>  	{ "count",     BOND_AD_COUNT,     0},
> +	{ "prio",      BOND_AD_PRIO,      0},
>  	{ NULL,        -1,                0},
>  };
>  
> diff --git a/include/net/bond_3ad.h b/include/net/bond_3ad.h
> index bf551ca70359..34495df965f0 100644
> --- a/include/net/bond_3ad.h
> +++ b/include/net/bond_3ad.h
> @@ -26,6 +26,7 @@ enum {
>  	BOND_AD_STABLE = 0,
>  	BOND_AD_BANDWIDTH = 1,
>  	BOND_AD_COUNT = 2,
> +	BOND_AD_PRIO = 3,
>  };
>  
>  /* rx machine states(43.4.11 in the 802.3ad standard) */