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

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: <aIN9OtPcnPtkqrks@fedora>
Date: Fri, 25 Jul 2025 12:48:58 +0000
From: Hangbin Liu <liuhangbin@...il.com>
To: Nikolay Aleksandrov <razor@...ckwall.org>
Cc: netdev@...r.kernel.org, Jay Vosburgh <jv@...sburgh.net>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"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>, Shuah Khan <shuah@...nel.org>,
	Jonathan Corbet <corbet@....net>, Petr Machata <petrm@...dia.com>,
	Amit Cohen <amcohen@...dia.com>,
	Vladimir Oltean <vladimir.oltean@....com>,
	Alessandro Zanni <alessandro.zanni87@...il.com>,
	linux-doc@...r.kernel.org, linux-kselftest@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 2/3] bonding: support aggregator selection based
 on port priority

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