Hangbin Liu <liuhangbin@xxxxxxxxx> wrote:
>Introduce a new netlink attribute 'ad_actor_port_prio' to allow setting
Nit here "actor_port_prio"?
One more comment below ...
>the LACP actor port priority on a per-slave basis. This extends the
>existing bonding infrastructure to support more granular control over
>LACP negotiations.
>
>The priority value is embedded in LACPDU packets and will be used by
>subsequent patches to influence aggregator selection policies.
>
>Signed-off-by: Hangbin Liu <liuhangbin@xxxxxxxxx>
>---
> Documentation/networking/bonding.rst | 9 +++++++
> drivers/net/bonding/bond_3ad.c | 4 ++++
> drivers/net/bonding/bond_netlink.c | 16 +++++++++++++
> drivers/net/bonding/bond_options.c | 36 ++++++++++++++++++++++++++++
> include/net/bond_3ad.h | 1 +
> include/net/bond_options.h | 1 +
> include/uapi/linux/if_link.h | 1 +
> 7 files changed, 68 insertions(+)
>
>diff --git a/Documentation/networking/bonding.rst b/Documentation/networking/bonding.rst
>index f8f5766703d4..1ca7830c24ea 100644
>--- a/Documentation/networking/bonding.rst
>+++ b/Documentation/networking/bonding.rst
>@@ -193,6 +193,15 @@ ad_actor_sys_prio
> This parameter has effect only in 802.3ad mode and is available through
> SysFs interface.
>
>+actor_port_prio
>+
>+ In an AD system, this specifies the port priority. The allowed range
>+ is 1 - 65535. If the value is not specified, it takes 255 as the
>+ default value.
>+
>+ This parameter has effect only in 802.3ad mode and is available through
>+ netlink interface.
>+
> ad_actor_system
>
> In an AD system, this specifies the mac-address for the actor in
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index 2fca8e84ab10..eb0fb7374391 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -436,6 +436,7 @@ static void __ad_actor_update_port(struct port *port)
>
> port->actor_system = BOND_AD_INFO(bond).system.sys_mac_addr;
> port->actor_system_priority = BOND_AD_INFO(bond).system.sys_priority;
>+ port->actor_port_priority = SLAVE_AD_INFO(port->slave)->port_priority;
> }
>
> /* Conversions */
>@@ -2203,6 +2204,9 @@ void bond_3ad_bind_slave(struct slave *slave)
>
> ad_initialize_port(port, bond->params.lacp_fast);
>
>+ /* Port priority is initialized. Update it to slave's ad info */
>+ SLAVE_AD_INFO(slave)->port_priority = port->actor_port_priority;
>+
> port->slave = slave;
> port->actor_port_number = SLAVE_AD_INFO(slave)->id;
> /* key is determined according to the link speed, duplex and
>diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
>index 57fff2421f1b..16ef13ddcf22 100644
>--- a/drivers/net/bonding/bond_netlink.c
>+++ b/drivers/net/bonding/bond_netlink.c
>@@ -28,6 +28,7 @@ static size_t bond_get_slave_size(const struct net_device *bond_dev,
> nla_total_size(sizeof(u8)) + /* IFLA_BOND_SLAVE_AD_ACTOR_OPER_PORT_STATE */
> nla_total_size(sizeof(u16)) + /* IFLA_BOND_SLAVE_AD_PARTNER_OPER_PORT_STATE */
> nla_total_size(sizeof(s32)) + /* IFLA_BOND_SLAVE_PRIO */
>+ nla_total_size(sizeof(u16)) + /* IFLA_BOND_SLAVE_ACTOR_PORT_PRIO */
> 0;
> }
>
>@@ -77,6 +78,10 @@ static int bond_fill_slave_info(struct sk_buff *skb,
> ad_port->partner_oper.port_state))
> goto nla_put_failure;
> }
>+
>+ if (nla_put_u16(skb, IFLA_BOND_SLAVE_ACTOR_PORT_PRIO,
>+ SLAVE_AD_INFO(slave)->port_priority))
>+ goto nla_put_failure;
> }
>
> return 0;
>@@ -130,6 +135,7 @@ static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = {
> static const struct nla_policy bond_slave_policy[IFLA_BOND_SLAVE_MAX + 1] = {
> [IFLA_BOND_SLAVE_QUEUE_ID] = { .type = NLA_U16 },
> [IFLA_BOND_SLAVE_PRIO] = { .type = NLA_S32 },
>+ [IFLA_BOND_SLAVE_ACTOR_PORT_PRIO] = { .type = NLA_U16 },
> };
>
> static int bond_validate(struct nlattr *tb[], struct nlattr *data[],
>@@ -180,6 +186,16 @@ static int bond_slave_changelink(struct net_device *bond_dev,
> return err;
> }
>
>+ if (data[IFLA_BOND_SLAVE_ACTOR_PORT_PRIO]) {
>+ u16 ad_prio = nla_get_u16(data[IFLA_BOND_SLAVE_ACTOR_PORT_PRIO]);
>+
>+ bond_opt_slave_initval(&newval, &slave_dev, ad_prio);
>+ err = __bond_opt_set(bond, BOND_OPT_ACTOR_PORT_PRIO, &newval,
>+ data[IFLA_BOND_SLAVE_ACTOR_PORT_PRIO], extack);
>+ if (err)
>+ return err;
>+ }
>+
> return 0;
> }
>
>diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>index 1d639a3be6ba..8f78f83fb29c 100644
>--- a/drivers/net/bonding/bond_options.c
>+++ b/drivers/net/bonding/bond_options.c
>@@ -79,6 +79,8 @@ static int bond_option_tlb_dynamic_lb_set(struct bonding *bond,
> const struct bond_opt_value *newval);
> static int bond_option_ad_actor_sys_prio_set(struct bonding *bond,
> const struct bond_opt_value *newval);
>+static int bond_option_actor_port_prio_set(struct bonding *bond,
>+ const struct bond_opt_value *newval);
> static int bond_option_ad_actor_system_set(struct bonding *bond,
> const struct bond_opt_value *newval);
> static int bond_option_ad_user_port_key_set(struct bonding *bond,
>@@ -223,6 +225,13 @@ static const struct bond_opt_value bond_ad_actor_sys_prio_tbl[] = {
> { NULL, -1, 0},
> };
>
>+static const struct bond_opt_value bond_actor_port_prio_tbl[] = {
>+ { "minval", 1, BOND_VALFLAG_MIN},
>+ { "maxval", 65535, BOND_VALFLAG_MAX},
>+ { "default", 255, BOND_VALFLAG_DEFAULT},
>+ { NULL, -1, 0},
Does the standard forbid actor_port_prio being set to zero? The
description I'm finding says only that it's an unsigned integer and two
octets in size (802.1AX-2014 6.4.2.3 LACPDU structure).
-J
>+};
>+
> static const struct bond_opt_value bond_ad_user_port_key_tbl[] = {
> { "minval", 0, BOND_VALFLAG_MIN | BOND_VALFLAG_DEFAULT},
> { "maxval", 1023, BOND_VALFLAG_MAX},
>@@ -484,6 +493,13 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
> .values = bond_ad_actor_sys_prio_tbl,
> .set = bond_option_ad_actor_sys_prio_set,
> },
>+ [BOND_OPT_ACTOR_PORT_PRIO] = {
>+ .id = BOND_OPT_ACTOR_PORT_PRIO,
>+ .name = "actor_port_prio",
>+ .unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
>+ .values = bond_actor_port_prio_tbl,
>+ .set = bond_option_actor_port_prio_set,
>+ },
> [BOND_OPT_AD_ACTOR_SYSTEM] = {
> .id = BOND_OPT_AD_ACTOR_SYSTEM,
> .name = "ad_actor_system",
>@@ -1816,6 +1832,26 @@ static int bond_option_ad_actor_sys_prio_set(struct bonding *bond,
> return 0;
> }
>
>+static int bond_option_actor_port_prio_set(struct bonding *bond,
>+ const struct bond_opt_value *newval)
>+{
>+ struct slave *slave;
>+
>+ slave = bond_slave_get_rtnl(newval->slave_dev);
>+ if (!slave) {
>+ netdev_dbg(bond->dev, "%s called on NULL slave\n", __func__);
>+ return -ENODEV;
>+ }
>+
>+ netdev_dbg(newval->slave_dev, "Setting actor_port_prio to %llu\n",
>+ newval->value);
>+
>+ SLAVE_AD_INFO(slave)->port_priority = newval->value;
>+ bond_3ad_update_ad_actor_settings(bond);
>+
>+ return 0;
>+}
>+
> static int bond_option_ad_actor_system_set(struct bonding *bond,
> const struct bond_opt_value *newval)
> {
>diff --git a/include/net/bond_3ad.h b/include/net/bond_3ad.h
>index 2053cd8e788a..bf551ca70359 100644
>--- a/include/net/bond_3ad.h
>+++ b/include/net/bond_3ad.h
>@@ -274,6 +274,7 @@ struct ad_slave_info {
> struct port port; /* 802.3ad port structure */
> struct bond_3ad_stats stats;
> u16 id;
>+ u16 port_priority;
> };
>
> static inline const char *bond_3ad_churn_desc(churn_state_t state)
>diff --git a/include/net/bond_options.h b/include/net/bond_options.h
>index 022b122a9fb6..e6eedf23aea1 100644
>--- a/include/net/bond_options.h
>+++ b/include/net/bond_options.h
>@@ -78,6 +78,7 @@ enum {
> BOND_OPT_PRIO,
> BOND_OPT_COUPLED_CONTROL,
> BOND_OPT_BROADCAST_NEIGH,
>+ BOND_OPT_ACTOR_PORT_PRIO,
> BOND_OPT_LAST
> };
>
>diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>index 784ace3a519c..45f56c9f95d9 100644
>--- a/include/uapi/linux/if_link.h
>+++ b/include/uapi/linux/if_link.h
>@@ -1564,6 +1564,7 @@ enum {
> IFLA_BOND_SLAVE_AD_ACTOR_OPER_PORT_STATE,
> IFLA_BOND_SLAVE_AD_PARTNER_OPER_PORT_STATE,
> IFLA_BOND_SLAVE_PRIO,
>+ IFLA_BOND_SLAVE_ACTOR_PORT_PRIO,
> __IFLA_BOND_SLAVE_MAX,
> };
>
>--
>2.50.1
---
-Jay Vosburgh, jv@xxxxxxxxxxxxx
