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

Hangbin Liu liuhangbin@gmail.com wrote:

Add a new ad_select policy 'port_priority' that uses the per-port actor priority values (set via ad_actor_port_prio) to determine aggregator selection.

This allows administrators to influence which ports are preferred for aggregation by assigning different priority values, providing more flexible load balancing control in LACP configurations.

Signed-off-by: Hangbin Liu liuhangbin@gmail.com

Documentation/networking/bonding.rst | 9 ++++++++- drivers/net/bonding/bond_3ad.c | 27 +++++++++++++++++++++++++++ drivers/net/bonding/bond_options.c | 1 + include/net/bond_3ad.h | 1 + 4 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/bonding.rst b/Documentation/networking/bonding.rst index 874d8a4681ec..151c964562db 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

Needing to have a caveat here makes me think we should instead change the nomenclature. Perhaps "lacp_port_prio"? The standard hasn't had "ad" in its name for 20-ish years, so I don't think we should use "ad" in user facing options, and common usage these days is to just call it "lacp."

Simiarly, I don't think we need "ad" in the option name, either; the standard just calls it "actor_port_priority", is there a good reason not to use that?

-J

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 19b389b81600..dcc1a1750df5 100644 --- a/drivers/net/bonding/bond_3ad.c +++ b/drivers/net/bonding/bond_3ad.c @@ -747,6 +747,18 @@ static int __agg_active_ports(struct aggregator *agg) return active; }

+static unsigned int __agg_ports_priority(const struct aggregator *agg) +{

• struct port *port = agg->lag_ports;
• unsigned int prio = 0;
• for (; port; port = port->next_port_in_aggregator)

• if (port->is_enabled)
  

• 	prio += port->actor_port_priority;
  

• return prio;

+}

/**

• __get_agg_bandwidth - get the total bandwidth of an aggregator
• @aggregator: the aggregator we're looking at

@@ -1707,6 +1719,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)

@@ -1725,6 +1740,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;

@@ -1790,6 +1813,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 5b58326dd24c..4bfff38b9ad0 100644 --- a/drivers/net/bonding/bond_options.c +++ b/drivers/net/bonding/bond_options.c @@ -165,6 +165,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) */

2.50.1

--- -Jay Vosburgh, jv@jvosburgh.net