Re: [PATCHv4 iproute2-next] iplink: bond_slave: add support for actor_port_prio

[Hangbin Liu <liuhangbin@gmail.com>]

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