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

[Hangbin Liu <liuhangbin@gmail.com>]

On Fri, Jul 25, 2025 at 12:02:02PM +0300, Nikolay Aleksandrov wrote:
> > 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

Thanks, I will fix this.

Hangbin
> 
> > +
> > +	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) */
>