Add support for the actor_port_prio option for bond slaves. This per-port priority can be used by the bonding driver in ad_select to choose the higher-priority aggregator during failover.
Signed-off-by: Hangbin Liu liuhangbin@gmail.com --- v4: no update v3: rename ad_actor_port_prio to actor_port_prio v2: no update --- ip/iplink_bond.c | 1 + ip/iplink_bond_slave.c | 18 ++++++++++++++++-- man/man8/ip-link.8.in | 6 ++++++ 3 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c index d6960f6d9b03..1a2c1b3042a0 100644 --- a/ip/iplink_bond.c +++ b/ip/iplink_bond.c @@ -91,6 +91,7 @@ static const char *ad_select_tbl[] = { "stable", "bandwidth", "count", + "prio", NULL, };
diff --git a/ip/iplink_bond_slave.c b/ip/iplink_bond_slave.c index ad6875006950..c88100e248dd 100644 --- a/ip/iplink_bond_slave.c +++ b/ip/iplink_bond_slave.c @@ -15,7 +15,9 @@
static void print_explain(FILE *f) { - fprintf(f, "Usage: ... bond_slave [ queue_id ID ] [ prio PRIORITY ]\n"); + fprintf(f, "Usage: ... bond_slave [ queue_id ID ] [ prio PRIORITY ]\n" + " [ actor_port_prio PRIORITY ]\n" + ); }
static void explain(void) @@ -145,12 +147,18 @@ static void bond_slave_print_opt(struct link_util *lu, FILE *f, struct rtattr *t state); print_slave_oper_state(f, "ad_partner_oper_port_state_str", state); } + + if (tb[IFLA_BOND_SLAVE_ACTOR_PORT_PRIO]) + print_int(PRINT_ANY, + "actor_port_prio", + "actor_port_prio %d ", + rta_getattr_u16(tb[IFLA_BOND_SLAVE_ACTOR_PORT_PRIO])); }
static int bond_slave_parse_opt(struct link_util *lu, int argc, char **argv, struct nlmsghdr *n) { - __u16 queue_id; + __u16 queue_id, actor_port_prio; int prio;
while (argc > 0) { @@ -164,6 +172,12 @@ static int bond_slave_parse_opt(struct link_util *lu, int argc, char **argv, if (get_s32(&prio, *argv, 0)) invarg("prio is invalid", *argv); addattr32(n, 1024, IFLA_BOND_SLAVE_PRIO, prio); + } else if (strcmp(*argv, "actor_port_prio") == 0) { + NEXT_ARG(); + if (get_u16(&actor_port_prio, *argv, 0)) + invarg("actor prio is invalid", *argv); + addattr16(n, 1024, IFLA_BOND_SLAVE_ACTOR_PORT_PRIO, + actor_port_prio); } else { if (matches(*argv, "help") != 0) fprintf(stderr, diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in index e3297c577152..7995943ab11f 100644 --- a/man/man8/ip-link.8.in +++ b/man/man8/ip-link.8.in @@ -2846,6 +2846,12 @@ the following additional arguments are supported: (a 32bit signed value). This option only valid for active-backup(1), balance-tlb (5) and balance-alb (6) mode.
+.sp +.BI actor_port_prio " PRIORITY" +- set the slave's ad actor port priority for 802.3ad aggregation selection +logic during failover (a 16bit unsigned value). This option only valid for +802.3ad (4) mode. + .in -8
.TP
Hangbin Liu liuhangbin@gmail.com wrote:
Add support for the actor_port_prio option for bond slaves. This per-port priority can be used by the bonding driver in ad_select to choose the higher-priority aggregator during failover.
Signed-off-by: Hangbin Liu liuhangbin@gmail.com
v4: no update v3: rename ad_actor_port_prio to actor_port_prio v2: no update
ip/iplink_bond.c | 1 + ip/iplink_bond_slave.c | 18 ++++++++++++++++-- man/man8/ip-link.8.in | 6 ++++++ 3 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c index d6960f6d9b03..1a2c1b3042a0 100644 --- a/ip/iplink_bond.c +++ b/ip/iplink_bond.c @@ -91,6 +91,7 @@ static const char *ad_select_tbl[] = { "stable", "bandwidth", "count",
- "prio",
Should this be actor_port_prio?
-J
NULL, };
diff --git a/ip/iplink_bond_slave.c b/ip/iplink_bond_slave.c index ad6875006950..c88100e248dd 100644 --- a/ip/iplink_bond_slave.c +++ b/ip/iplink_bond_slave.c @@ -15,7 +15,9 @@
static void print_explain(FILE *f) {
- fprintf(f, "Usage: ... bond_slave [ queue_id ID ] [ prio PRIORITY ]\n");
- fprintf(f, "Usage: ... bond_slave [ queue_id ID ] [ prio PRIORITY ]\n"
" [ actor_port_prio PRIORITY ]\n"- );
}
static void explain(void) @@ -145,12 +147,18 @@ static void bond_slave_print_opt(struct link_util *lu, FILE *f, struct rtattr *t state); print_slave_oper_state(f, "ad_partner_oper_port_state_str", state); }
- if (tb[IFLA_BOND_SLAVE_ACTOR_PORT_PRIO])
print_int(PRINT_ANY,"actor_port_prio","actor_port_prio %d ",rta_getattr_u16(tb[IFLA_BOND_SLAVE_ACTOR_PORT_PRIO]));}
static int bond_slave_parse_opt(struct link_util *lu, int argc, char **argv, struct nlmsghdr *n) {
- __u16 queue_id;
__u16 queue_id, actor_port_prio; int prio;
while (argc > 0) {
@@ -164,6 +172,12 @@ static int bond_slave_parse_opt(struct link_util *lu, int argc, char **argv, if (get_s32(&prio, *argv, 0)) invarg("prio is invalid", *argv); addattr32(n, 1024, IFLA_BOND_SLAVE_PRIO, prio);
} else if (strcmp(*argv, "actor_port_prio") == 0) {NEXT_ARG();if (get_u16(&actor_port_prio, *argv, 0))invarg("actor prio is invalid", *argv);addattr16(n, 1024, IFLA_BOND_SLAVE_ACTOR_PORT_PRIO,actor_port_prio);diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in index e3297c577152..7995943ab11f 100644 --- a/man/man8/ip-link.8.in +++ b/man/man8/ip-link.8.in @@ -2846,6 +2846,12 @@ the following additional arguments are supported: (a 32bit signed value). This option only valid for active-backup(1), balance-tlb (5) and balance-alb (6) mode.
+.sp +.BI actor_port_prio " PRIORITY" +- set the slave's ad actor port priority for 802.3ad aggregation selection +logic during failover (a 16bit unsigned value). This option only valid for +802.3ad (4) mode.
.in -8
.TP
2.50.1
--- -Jay Vosburgh, jv@jvosburgh.net
On Wed, Aug 27, 2025 at 11:43:19AM -0700, Jay Vosburgh wrote:
Hangbin Liu liuhangbin@gmail.com wrote:
Add support for the actor_port_prio option for bond slaves. This per-port priority can be used by the bonding driver in ad_select to choose the higher-priority aggregator during failover.
Signed-off-by: Hangbin Liu liuhangbin@gmail.com
v4: no update v3: rename ad_actor_port_prio to actor_port_prio v2: no update
ip/iplink_bond.c | 1 + ip/iplink_bond_slave.c | 18 ++++++++++++++++-- man/man8/ip-link.8.in | 6 ++++++ 3 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c index d6960f6d9b03..1a2c1b3042a0 100644 --- a/ip/iplink_bond.c +++ b/ip/iplink_bond.c @@ -91,6 +91,7 @@ static const char *ad_select_tbl[] = { "stable", "bandwidth", "count",
- "prio",
Should this be actor_port_prio?
hmm, actor_port_prio correspond to the ip link option name, which is also acceptable.
While in kernel, we defined the select policy as
{ "stable", BOND_AD_STABLE, BOND_VALFLAG_DEFAULT}, { "bandwidth", BOND_AD_BANDWIDTH, 0}, { "count", BOND_AD_COUNT, 0}, + { "prio", BOND_AD_PRIO, 0},
So I think the prio here should also be OK.
You can decide which one to use.
Thanks Hangbin
Hangbin Liu liuhangbin@gmail.com wrote:
On Wed, Aug 27, 2025 at 11:43:19AM -0700, Jay Vosburgh wrote:
Hangbin Liu liuhangbin@gmail.com wrote:
Add support for the actor_port_prio option for bond slaves. This per-port priority can be used by the bonding driver in ad_select to choose the higher-priority aggregator during failover.
Signed-off-by: Hangbin Liu liuhangbin@gmail.com
v4: no update v3: rename ad_actor_port_prio to actor_port_prio v2: no update
ip/iplink_bond.c | 1 + ip/iplink_bond_slave.c | 18 ++++++++++++++++-- man/man8/ip-link.8.in | 6 ++++++ 3 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c index d6960f6d9b03..1a2c1b3042a0 100644 --- a/ip/iplink_bond.c +++ b/ip/iplink_bond.c @@ -91,6 +91,7 @@ static const char *ad_select_tbl[] = { "stable", "bandwidth", "count",
- "prio",
Should this be actor_port_prio?
hmm, actor_port_prio correspond to the ip link option name, which is also acceptable.
Isn't this the text of the ip link option name right here (in the sense of what goes on the "ip link" command line)?
While in kernel, we defined the select policy as
{ "stable", BOND_AD_STABLE, BOND_VALFLAG_DEFAULT}, { "bandwidth", BOND_AD_BANDWIDTH, 0}, { "count", BOND_AD_COUNT, 0},
{ "prio", BOND_AD_PRIO, 0},
Maybe my memory is starting to go, but I thought in a prior discussion we'd agreed to change this as well for consistency.
So I think the prio here should also be OK.
You can decide which one to use.
I would prefer that the two options have discrete names, or, really, that we not repeat "prio" as it's already used elsewhere. Plus, who knows, maybe in the future we'll have another priority option.
-J
--- -Jay Vosburgh, jv@jvosburgh.net
On Thu, Aug 28, 2025 at 09:59:20AM -0700, Jay Vosburgh wrote:
Hangbin Liu liuhangbin@gmail.com wrote:
On Wed, Aug 27, 2025 at 11:43:19AM -0700, Jay Vosburgh wrote:
Hangbin Liu liuhangbin@gmail.com wrote:
Add support for the actor_port_prio option for bond slaves. This per-port priority can be used by the bonding driver in ad_select to choose the higher-priority aggregator during failover.
Signed-off-by: Hangbin Liu liuhangbin@gmail.com
v4: no update v3: rename ad_actor_port_prio to actor_port_prio v2: no update
ip/iplink_bond.c | 1 + ip/iplink_bond_slave.c | 18 ++++++++++++++++-- man/man8/ip-link.8.in | 6 ++++++ 3 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c index d6960f6d9b03..1a2c1b3042a0 100644 --- a/ip/iplink_bond.c +++ b/ip/iplink_bond.c @@ -91,6 +91,7 @@ static const char *ad_select_tbl[] = { "stable", "bandwidth", "count",
- "prio",
Should this be actor_port_prio?
hmm, actor_port_prio correspond to the ip link option name, which is also acceptable.
Isn't this the text of the ip link option name right here (in the sense of what goes on the "ip link" command line)?
"stable", "bandwidth", "count" are not ip link parameters, and same with kernel names, so I also used the kernel name "prio" here.
While in kernel, we defined the select policy as
{ "stable", BOND_AD_STABLE, BOND_VALFLAG_DEFAULT}, { "bandwidth", BOND_AD_BANDWIDTH, 0}, { "count", BOND_AD_COUNT, 0},
{ "prio", BOND_AD_PRIO, 0},Maybe my memory is starting to go, but I thought in a prior discussion we'd agreed to change this as well for consistency.
Maybe I didn't get your comment[1] correctly. I only changed `ad_actor_port_prio` to `actor_port_prio` last time.
So I think the prio here should also be OK.
You can decide which one to use.
I would prefer that the two options have discrete names, or, really, that we not repeat "prio" as it's already used elsewhere. Plus, who knows, maybe in the future we'll have another priority option.
OK, do not use same name for different usage. I will also change the "prio" to "actor_port_prio" in next patch.
[1] https://lore.kernel.org/netdev/1109153.1755380673@famine/
Thanks Hangbin
Hangbin Liu liuhangbin@gmail.com wrote:
On Thu, Aug 28, 2025 at 09:59:20AM -0700, Jay Vosburgh wrote:
Hangbin Liu liuhangbin@gmail.com wrote:
On Wed, Aug 27, 2025 at 11:43:19AM -0700, Jay Vosburgh wrote:
Hangbin Liu liuhangbin@gmail.com wrote:
Add support for the actor_port_prio option for bond slaves. This per-port priority can be used by the bonding driver in ad_select to choose the higher-priority aggregator during failover.
Signed-off-by: Hangbin Liu liuhangbin@gmail.com
v4: no update v3: rename ad_actor_port_prio to actor_port_prio v2: no update
ip/iplink_bond.c | 1 + ip/iplink_bond_slave.c | 18 ++++++++++++++++-- man/man8/ip-link.8.in | 6 ++++++ 3 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c index d6960f6d9b03..1a2c1b3042a0 100644 --- a/ip/iplink_bond.c +++ b/ip/iplink_bond.c @@ -91,6 +91,7 @@ static const char *ad_select_tbl[] = { "stable", "bandwidth", "count",
- "prio",
Should this be actor_port_prio?
hmm, actor_port_prio correspond to the ip link option name, which is also acceptable.
Isn't this the text of the ip link option name right here (in the sense of what goes on the "ip link" command line)?
"stable", "bandwidth", "count" are not ip link parameters, and same with kernel names, so I also used the kernel name "prio" here.
Perhaps I wasn't explaining sufficiently; I mean that the text in ad_select_tbl[] is where the name of the option settings comes from for ip link whatever commands. E.g., If I do something like
ip link add dev bond0 type bond [...] ad_select rutabaga
The "rutabaga" text is being compared to the elements of ad_select_tbl[], correct?
Anyway, what I meant is that I think setting this new priority option should look something like:
ip link add dev bond0 type bond [...] ad_select actor_port_prio
So that it's called the same thing everwhere, and it matches the nomenclature used in the standard.
-J
While in kernel, we defined the select policy as
{ "stable", BOND_AD_STABLE, BOND_VALFLAG_DEFAULT}, { "bandwidth", BOND_AD_BANDWIDTH, 0}, { "count", BOND_AD_COUNT, 0},
{ "prio", BOND_AD_PRIO, 0},Maybe my memory is starting to go, but I thought in a prior discussion we'd agreed to change this as well for consistency.
Maybe I didn't get your comment[1] correctly. I only changed `ad_actor_port_prio` to `actor_port_prio` last time.
So I think the prio here should also be OK.
You can decide which one to use.
I would prefer that the two options have discrete names, or, really, that we not repeat "prio" as it's already used elsewhere. Plus, who knows, maybe in the future we'll have another priority option.
OK, do not use same name for different usage. I will also change the "prio" to "actor_port_prio" in next patch.
[1] https://lore.kernel.org/netdev/1109153.1755380673@famine/
Thanks Hangbin
--- -Jay Vosburgh, jv@jvosburgh.net
linux-kselftest-mirror@lists.linaro.org
-
Hangbin Liu -
Jay Vosburgh