Hangbin Liu <liuhangbin@xxxxxxxxx> 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@xxxxxxxxx>
>---
> 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@xxxxxxxxxxxxx
