Add support for the independent control state machine per IEEE
802.1AX-2008 5.4.15 in addition to the existing implementation of the
coupled control state machine.
Introduces two new states, AD_MUX_COLLECTING and AD_MUX_DISTRIBUTING in
the LACP MUX state machine for separated handling of an initial
Collecting state before the Collecting and Distributing state. This
enables a port to be in a state where it can receive incoming packets
while not still distributing. This is useful for reducing packet loss when
a port begins distributing before its partner is able to collect.
Added new functions such as bond_set_slave_tx_disabled_flags and
bond_set_slave_rx_enabled_flags to precisely manage the port's collecting
and distributing states. Previously, there was no dedicated method to
disable TX while keeping RX enabled, which this patch addresses.
Note that the regular flow process in the kernel's bonding driver remains
unaffected by this patch. The extension requires explicit opt-in by the
user (in order to ensure no disruptions for existing setups) via netlink
support using the new bonding parameter coupled_control. The default value
for coupled_control is set to 1 so as to preserve existing behaviour.
Signed-off-by: Aahil Awatramani <[email protected]>
v5:
Merge documentation patch with changes patch
Add version history in comment description
v4:
Remove inline references from c source files
v3:
Edited commit description
Edited documentation description
Changed function names
Only allow coupled_control change when the bond is down
v2:
Removed sysfs changes
Added documentation for new paramater
Renamed parameter to coupled_control
Update bond_set_slave_inactive_flags() with a 8023ad check
---
Documentation/networking/bonding.rst | 12 ++
drivers/net/bonding/bond_3ad.c | 157 +++++++++++++++++++++++++--
drivers/net/bonding/bond_main.c | 1 +
drivers/net/bonding/bond_netlink.c | 16 +++
drivers/net/bonding/bond_options.c | 28 ++++-
include/net/bond_3ad.h | 2 +
include/net/bond_options.h | 1 +
include/net/bonding.h | 23 ++++
include/uapi/linux/if_link.h | 1 +
tools/include/uapi/linux/if_link.h | 1 +
10 files changed, 234 insertions(+), 8 deletions(-)
diff --git a/Documentation/networking/bonding.rst b/Documentation/networking/bonding.rst
index f7a73421eb76..e774b48de9f5 100644
--- a/Documentation/networking/bonding.rst
+++ b/Documentation/networking/bonding.rst
@@ -444,6 +444,18 @@ arp_missed_max
The default value is 2, and the allowable range is 1 - 255.
+coupled_control
+
+ Specifies whether the LACP state machine's MUX in the 802.3ad mode
+ should have separate Collecting and Distributing states.
+
+ This is by implementing the independent control state machine per
+ IEEE 802.1AX-2008 5.4.15 in addition to the existing coupled control
+ state machine.
+
+ The default value is 1. This setting does not separate the Collecting
+ and Distributing states, maintaining the bond in coupled control.
+
downdelay
Specifies the time, in milliseconds, to wait before disabling
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index c99ffe6c683a..f2942e8c6c91 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -106,6 +106,9 @@ static void ad_agg_selection_logic(struct aggregator *aggregator,
static void ad_clear_agg(struct aggregator *aggregator);
static void ad_initialize_agg(struct aggregator *aggregator);
static void ad_initialize_port(struct port *port, int lacp_fast);
+static void ad_enable_collecting(struct port *port);
+static void ad_disable_distributing(struct port *port,
+ bool *update_slave_arr);
static void ad_enable_collecting_distributing(struct port *port,
bool *update_slave_arr);
static void ad_disable_collecting_distributing(struct port *port,
@@ -171,9 +174,38 @@ static inline int __agg_has_partner(struct aggregator *agg)
return !is_zero_ether_addr(agg->partner_system.mac_addr_value);
}
+/**
+ * __disable_distributing_port - disable the port's slave for distributing.
+ * Port will still be able to collect.
+ * @port: the port we're looking at
+ *
+ * This will disable only distributing on the port's slave.
+ */
+static void __disable_distributing_port(struct port *port)
+{
+ bond_set_slave_tx_disabled_flags(port->slave, BOND_SLAVE_NOTIFY_LATER);
+}
+
+/**
+ * __enable_collecting_port - enable the port's slave for collecting,
+ * if it's up
+ * @port: the port we're looking at
+ *
+ * This will enable only collecting on the port's slave.
+ */
+static void __enable_collecting_port(struct port *port)
+{
+ struct slave *slave = port->slave;
+
+ if (slave->link == BOND_LINK_UP && bond_slave_is_up(slave))
+ bond_set_slave_rx_enabled_flags(slave, BOND_SLAVE_NOTIFY_LATER);
+}
+
/**
* __disable_port - disable the port's slave
* @port: the port we're looking at
+ *
+ * This will disable both collecting and distributing on the port's slave.
*/
static inline void __disable_port(struct port *port)
{
@@ -183,6 +215,8 @@ static inline void __disable_port(struct port *port)
/**
* __enable_port - enable the port's slave, if it's up
* @port: the port we're looking at
+ *
+ * This will enable both collecting and distributing on the port's slave.
*/
static inline void __enable_port(struct port *port)
{
@@ -193,10 +227,27 @@ static inline void __enable_port(struct port *port)
}
/**
- * __port_is_enabled - check if the port's slave is in active state
+ * __port_move_to_attached_state - check if port should transition back to attached
+ * state.
+ * @port: the port we're looking at
+ */
+static bool __port_move_to_attached_state(struct port *port)
+{
+ if (!(port->sm_vars & AD_PORT_SELECTED) ||
+ (port->sm_vars & AD_PORT_STANDBY) ||
+ !(port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) ||
+ !(port->actor_oper_port_state & LACP_STATE_SYNCHRONIZATION))
+ port->sm_mux_state = AD_MUX_ATTACHED;
+
+ return port->sm_mux_state == AD_MUX_ATTACHED;
+}
+
+/**
+ * __port_is_collecting_distributing - check if the port's slave is in the
+ * combined collecting/distributing state
* @port: the port we're looking at
*/
-static inline int __port_is_enabled(struct port *port)
+static int __port_is_collecting_distributing(struct port *port)
{
return bond_is_active_slave(port->slave);
}
@@ -942,6 +993,7 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
*/
static void ad_mux_machine(struct port *port, bool *update_slave_arr)
{
+ struct bonding *bond = __get_bond_by_port(port);
mux_states_t last_state;
/* keep current State Machine state to compare later if it was
@@ -999,9 +1051,13 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
if ((port->sm_vars & AD_PORT_SELECTED) &&
(port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) &&
!__check_agg_selection_timer(port)) {
- if (port->aggregator->is_active)
- port->sm_mux_state =
- AD_MUX_COLLECTING_DISTRIBUTING;
+ if (port->aggregator->is_active) {
+ int state = AD_MUX_COLLECTING_DISTRIBUTING;
+
+ if (!bond->params.coupled_control)
+ state = AD_MUX_COLLECTING;
+ port->sm_mux_state = state;
+ }
} else if (!(port->sm_vars & AD_PORT_SELECTED) ||
(port->sm_vars & AD_PORT_STANDBY)) {
/* if UNSELECTED or STANDBY */
@@ -1019,11 +1075,45 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
}
break;
case AD_MUX_COLLECTING_DISTRIBUTING:
+ if (!__port_move_to_attached_state(port)) {
+ /* if port state hasn't changed make
+ * sure that a collecting distributing
+ * port in an active aggregator is enabled
+ */
+ if (port->aggregator->is_active &&
+ !__port_is_collecting_distributing(port)) {
+ __enable_port(port);
+ *update_slave_arr = true;
+ }
+ }
+ break;
+ case AD_MUX_COLLECTING:
+ if (!__port_move_to_attached_state(port)) {
+ if ((port->sm_vars & AD_PORT_SELECTED) &&
+ (port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) &&
+ (port->partner_oper.port_state & LACP_STATE_COLLECTING)) {
+ port->sm_mux_state = AD_MUX_DISTRIBUTING;
+ } else {
+ /* If port state hasn't changed, make sure that a collecting
+ * port is enabled for an active aggregator.
+ */
+ struct slave *slave = port->slave;
+
+ if (port->aggregator->is_active &&
+ bond_is_slave_rx_disabled(slave)) {
+ ad_enable_collecting(port);
+ *update_slave_arr = true;
+ }
+ }
+ }
+ break;
+ case AD_MUX_DISTRIBUTING:
if (!(port->sm_vars & AD_PORT_SELECTED) ||
(port->sm_vars & AD_PORT_STANDBY) ||
+ !(port->partner_oper.port_state & LACP_STATE_COLLECTING) ||
!(port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) ||
!(port->actor_oper_port_state & LACP_STATE_SYNCHRONIZATION)) {
- port->sm_mux_state = AD_MUX_ATTACHED;
+ port->sm_mux_state = AD_MUX_COLLECTING;
} else {
/* if port state hasn't changed make
* sure that a collecting distributing
@@ -1031,7 +1121,7 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
*/
if (port->aggregator &&
port->aggregator->is_active &&
- !__port_is_enabled(port)) {
+ !__port_is_collecting_distributing(port)) {
__enable_port(port);
*update_slave_arr = true;
}
@@ -1082,6 +1172,20 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
update_slave_arr);
port->ntt = true;
break;
+ case AD_MUX_COLLECTING:
+ port->actor_oper_port_state |= LACP_STATE_COLLECTING;
+ port->actor_oper_port_state &= ~LACP_STATE_DISTRIBUTING;
+ port->actor_oper_port_state |= LACP_STATE_SYNCHRONIZATION;
+ ad_enable_collecting(port);
+ ad_disable_distributing(port, update_slave_arr);
+ port->ntt = true;
+ break;
+ case AD_MUX_DISTRIBUTING:
+ port->actor_oper_port_state |= LACP_STATE_DISTRIBUTING;
+ port->actor_oper_port_state |= LACP_STATE_SYNCHRONIZATION;
+ ad_enable_collecting_distributing(port,
+ update_slave_arr);
+ break;
default:
break;
}
@@ -1906,6 +2010,45 @@ static void ad_initialize_port(struct port *port, int lacp_fast)
}
}
+/**
+ * ad_enable_collecting - enable a port's receive
+ * @port: the port we're looking at
+ *
+ * Enable @port if it's in an active aggregator
+ */
+static void ad_enable_collecting(struct port *port)
+{
+ if (port->aggregator->is_active) {
+ struct slave *slave = port->slave;
+
+ slave_dbg(slave->bond->dev, slave->dev,
+ "Enabling collecting on port %d (LAG %d)\n",
+ port->actor_port_number,
+ port->aggregator->aggregator_identifier);
+ __enable_collecting_port(port);
+ }
+}
+
+/**
+ * ad_disable_distributing - disable a port's transmit
+ * @port: the port we're looking at
+ * @update_slave_arr: Does slave array need update?
+ */
+static void ad_disable_distributing(struct port *port, bool *update_slave_arr)
+{
+ if (port->aggregator &&
+ !MAC_ADDRESS_EQUAL(&port->aggregator->partner_system,
+ &(null_mac_addr))) {
+ slave_dbg(port->slave->bond->dev, port->slave->dev,
+ "Disabling distributing on port %d (LAG %d)\n",
+ port->actor_port_number,
+ port->aggregator->aggregator_identifier);
+ __disable_distributing_port(port);
+ /* Slave array needs an update */
+ *update_slave_arr = true;
+ }
+}
+
/**
* ad_enable_collecting_distributing - enable a port's transmit/receive
* @port: the port we're looking at
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 8e6cc0e133b7..30f4b0ff01c0 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -6331,6 +6331,7 @@ static int __init bond_check_params(struct bond_params *params)
params->ad_actor_sys_prio = ad_actor_sys_prio;
eth_zero_addr(params->ad_actor_system);
params->ad_user_port_key = ad_user_port_key;
+ params->coupled_control = 1;
if (packets_per_slave > 0) {
params->reciprocal_packets_per_slave =
reciprocal_value(packets_per_slave);
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index cfa74cf8bb1a..29b4c3d1b9b6 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -122,6 +122,7 @@ static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = {
[IFLA_BOND_PEER_NOTIF_DELAY] = NLA_POLICY_FULL_RANGE(NLA_U32, &delay_range),
[IFLA_BOND_MISSED_MAX] = { .type = NLA_U8 },
[IFLA_BOND_NS_IP6_TARGET] = { .type = NLA_NESTED },
+ [IFLA_BOND_COUPLED_CONTROL] = { .type = NLA_U8 },
};
static const struct nla_policy bond_slave_policy[IFLA_BOND_SLAVE_MAX + 1] = {
@@ -549,6 +550,16 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
return err;
}
+ if (data[IFLA_BOND_COUPLED_CONTROL]) {
+ int coupled_control = nla_get_u8(data[IFLA_BOND_COUPLED_CONTROL]);
+
+ bond_opt_initval(&newval, coupled_control);
+ err = __bond_opt_set(bond, BOND_OPT_COUPLED_CONTROL, &newval,
+ data[IFLA_BOND_COUPLED_CONTROL], extack);
+ if (err)
+ return err;
+ }
+
return 0;
}
@@ -615,6 +626,7 @@ static size_t bond_get_size(const struct net_device *bond_dev)
/* IFLA_BOND_NS_IP6_TARGET */
nla_total_size(sizeof(struct nlattr)) +
nla_total_size(sizeof(struct in6_addr)) * BOND_MAX_NS_TARGETS +
+ nla_total_size(sizeof(u8)) + /* IFLA_BOND_COUPLED_CONTROL */
0;
}
@@ -774,6 +786,10 @@ static int bond_fill_info(struct sk_buff *skb,
bond->params.missed_max))
goto nla_put_failure;
+ if (nla_put_u8(skb, IFLA_BOND_COUPLED_CONTROL,
+ bond->params.coupled_control))
+ goto nla_put_failure;
+
if (BOND_MODE(bond) == BOND_MODE_8023AD) {
struct ad_info info;
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index f3f27f0bd2a6..4cdbc7e084f4 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -84,7 +84,8 @@ static int bond_option_ad_user_port_key_set(struct bonding *bond,
const struct bond_opt_value *newval);
static int bond_option_missed_max_set(struct bonding *bond,
const struct bond_opt_value *newval);
-
+static int bond_option_coupled_control_set(struct bonding *bond,
+ const struct bond_opt_value *newval);
static const struct bond_opt_value bond_mode_tbl[] = {
{ "balance-rr", BOND_MODE_ROUNDROBIN, BOND_VALFLAG_DEFAULT},
@@ -232,6 +233,12 @@ static const struct bond_opt_value bond_missed_max_tbl[] = {
{ NULL, -1, 0},
};
+static const struct bond_opt_value bond_coupled_control_tbl[] = {
+ { "on", 1, BOND_VALFLAG_DEFAULT},
+ { "off", 0, 0},
+ { NULL, -1, 0},
+};
+
static const struct bond_option bond_opts[BOND_OPT_LAST] = {
[BOND_OPT_MODE] = {
.id = BOND_OPT_MODE,
@@ -496,6 +503,15 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
.desc = "Delay between each peer notification on failover event, in milliseconds",
.values = bond_peer_notif_delay_tbl,
.set = bond_option_peer_notif_delay_set
+ },
+ [BOND_OPT_COUPLED_CONTROL] = {
+ .id = BOND_OPT_COUPLED_CONTROL,
+ .name = "coupled_control",
+ .desc = "Opt into using coupled control MUX for LACP states",
+ .unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
+ .flags = BOND_OPTFLAG_IFDOWN,
+ .values = bond_coupled_control_tbl,
+ .set = bond_option_coupled_control_set,
}
};
@@ -1692,3 +1708,13 @@ static int bond_option_ad_user_port_key_set(struct bonding *bond,
bond->params.ad_user_port_key = newval->value;
return 0;
}
+
+static int bond_option_coupled_control_set(struct bonding *bond,
+ const struct bond_opt_value *newval)
+{
+ netdev_info(bond->dev, "Setting coupled_control to %s (%llu)\n",
+ newval->string, newval->value);
+
+ bond->params.coupled_control = newval->value;
+ return 0;
+}
diff --git a/include/net/bond_3ad.h b/include/net/bond_3ad.h
index c5e57c6bd873..9ce5ac2bfbad 100644
--- a/include/net/bond_3ad.h
+++ b/include/net/bond_3ad.h
@@ -54,6 +54,8 @@ typedef enum {
AD_MUX_DETACHED, /* mux machine */
AD_MUX_WAITING, /* mux machine */
AD_MUX_ATTACHED, /* mux machine */
+ AD_MUX_COLLECTING, /* mux machine */
+ AD_MUX_DISTRIBUTING, /* mux machine */
AD_MUX_COLLECTING_DISTRIBUTING /* mux machine */
} mux_states_t;
diff --git a/include/net/bond_options.h b/include/net/bond_options.h
index 69292ecc0325..473a0147769e 100644
--- a/include/net/bond_options.h
+++ b/include/net/bond_options.h
@@ -76,6 +76,7 @@ enum {
BOND_OPT_MISSED_MAX,
BOND_OPT_NS_TARGETS,
BOND_OPT_PRIO,
+ BOND_OPT_COUPLED_CONTROL,
BOND_OPT_LAST
};
diff --git a/include/net/bonding.h b/include/net/bonding.h
index 5b8b1b644a2d..b61fb1aa3a56 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -148,6 +148,7 @@ struct bond_params {
#if IS_ENABLED(CONFIG_IPV6)
struct in6_addr ns_targets[BOND_MAX_NS_TARGETS];
#endif
+ int coupled_control;
/* 2 bytes of padding : see ether_addr_equal_64bits() */
u8 ad_actor_system[ETH_ALEN + 2];
@@ -167,6 +168,7 @@ struct slave {
u8 backup:1, /* indicates backup slave. Value corresponds with
BOND_STATE_ACTIVE and BOND_STATE_BACKUP */
inactive:1, /* indicates inactive slave */
+ rx_disabled:1, /* indicates whether slave's Rx is disabled */
should_notify:1, /* indicates whether the state changed */
should_notify_link:1; /* indicates whether the link changed */
u8 duplex;
@@ -568,6 +570,14 @@ static inline void bond_set_slave_inactive_flags(struct slave *slave,
bond_set_slave_state(slave, BOND_STATE_BACKUP, notify);
if (!slave->bond->params.all_slaves_active)
slave->inactive = 1;
+ if (BOND_MODE(slave->bond) == BOND_MODE_8023AD)
+ slave->rx_disabled = 1;
+}
+
+static inline void bond_set_slave_tx_disabled_flags(struct slave *slave,
+ bool notify)
+{
+ bond_set_slave_state(slave, BOND_STATE_BACKUP, notify);
}
static inline void bond_set_slave_active_flags(struct slave *slave,
@@ -575,6 +585,14 @@ static inline void bond_set_slave_active_flags(struct slave *slave,
{
bond_set_slave_state(slave, BOND_STATE_ACTIVE, notify);
slave->inactive = 0;
+ if (BOND_MODE(slave->bond) == BOND_MODE_8023AD)
+ slave->rx_disabled = 0;
+}
+
+static inline void bond_set_slave_rx_enabled_flags(struct slave *slave,
+ bool notify)
+{
+ slave->rx_disabled = 0;
}
static inline bool bond_is_slave_inactive(struct slave *slave)
@@ -582,6 +600,11 @@ static inline bool bond_is_slave_inactive(struct slave *slave)
return slave->inactive;
}
+static inline bool bond_is_slave_rx_disabled(struct slave *slave)
+{
+ return slave->rx_disabled;
+}
+
static inline void bond_propose_link_state(struct slave *slave, int state)
{
slave->link_new_state = state;
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 29ff80da2775..7a54fcff2eec 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -976,6 +976,7 @@ enum {
IFLA_BOND_AD_LACP_ACTIVE,
IFLA_BOND_MISSED_MAX,
IFLA_BOND_NS_IP6_TARGET,
+ IFLA_BOND_COUPLED_CONTROL,
__IFLA_BOND_MAX,
};
diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h
index a0aa05a28cf2..f0d71b2a3f1e 100644
--- a/tools/include/uapi/linux/if_link.h
+++ b/tools/include/uapi/linux/if_link.h
@@ -974,6 +974,7 @@ enum {
IFLA_BOND_AD_LACP_ACTIVE,
IFLA_BOND_MISSED_MAX,
IFLA_BOND_NS_IP6_TARGET,
+ IFLA_BOND_COUPLED_CONTROL,
__IFLA_BOND_MAX,
};
--
2.43.0.429.g432eaa2c6b-goog
Aahil Awatramani <[email protected]> wrote:
>Add support for the independent control state machine per IEEE
>802.1AX-2008 5.4.15 in addition to the existing implementation of the
>coupled control state machine.
>
>Introduces two new states, AD_MUX_COLLECTING and AD_MUX_DISTRIBUTING in
>the LACP MUX state machine for separated handling of an initial
>Collecting state before the Collecting and Distributing state. This
>enables a port to be in a state where it can receive incoming packets
>while not still distributing. This is useful for reducing packet loss when
>a port begins distributing before its partner is able to collect.
>
>Added new functions such as bond_set_slave_tx_disabled_flags and
>bond_set_slave_rx_enabled_flags to precisely manage the port's collecting
>and distributing states. Previously, there was no dedicated method to
>disable TX while keeping RX enabled, which this patch addresses.
>
>Note that the regular flow process in the kernel's bonding driver remains
>unaffected by this patch. The extension requires explicit opt-in by the
>user (in order to ensure no disruptions for existing setups) via netlink
>support using the new bonding parameter coupled_control. The default value
>for coupled_control is set to 1 so as to preserve existing behaviour.
Do you have a corresponding iproute2 patch to enable changing
the coupled_control option?
-J
>Signed-off-by: Aahil Awatramani <[email protected]>
>
>v5:
> Merge documentation patch with changes patch
> Add version history in comment description
>v4:
> Remove inline references from c source files
>v3:
> Edited commit description
> Edited documentation description
> Changed function names
> Only allow coupled_control change when the bond is down
>v2:
> Removed sysfs changes
> Added documentation for new paramater
> Renamed parameter to coupled_control
> Update bond_set_slave_inactive_flags() with a 8023ad check
>
>---
> Documentation/networking/bonding.rst | 12 ++
> drivers/net/bonding/bond_3ad.c | 157 +++++++++++++++++++++++++--
> drivers/net/bonding/bond_main.c | 1 +
> drivers/net/bonding/bond_netlink.c | 16 +++
> drivers/net/bonding/bond_options.c | 28 ++++-
> include/net/bond_3ad.h | 2 +
> include/net/bond_options.h | 1 +
> include/net/bonding.h | 23 ++++
> include/uapi/linux/if_link.h | 1 +
> tools/include/uapi/linux/if_link.h | 1 +
> 10 files changed, 234 insertions(+), 8 deletions(-)
>
>diff --git a/Documentation/networking/bonding.rst b/Documentation/networking/bonding.rst
>index f7a73421eb76..e774b48de9f5 100644
>--- a/Documentation/networking/bonding.rst
>+++ b/Documentation/networking/bonding.rst
>@@ -444,6 +444,18 @@ arp_missed_max
>
> The default value is 2, and the allowable range is 1 - 255.
>
>+coupled_control
>+
>+ Specifies whether the LACP state machine's MUX in the 802.3ad mode
>+ should have separate Collecting and Distributing states.
>+
>+ This is by implementing the independent control state machine per
>+ IEEE 802.1AX-2008 5.4.15 in addition to the existing coupled control
>+ state machine.
>+
>+ The default value is 1. This setting does not separate the Collecting
>+ and Distributing states, maintaining the bond in coupled control.
>+
> downdelay
>
> Specifies the time, in milliseconds, to wait before disabling
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index c99ffe6c683a..f2942e8c6c91 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -106,6 +106,9 @@ static void ad_agg_selection_logic(struct aggregator *aggregator,
> static void ad_clear_agg(struct aggregator *aggregator);
> static void ad_initialize_agg(struct aggregator *aggregator);
> static void ad_initialize_port(struct port *port, int lacp_fast);
>+static void ad_enable_collecting(struct port *port);
>+static void ad_disable_distributing(struct port *port,
>+ bool *update_slave_arr);
> static void ad_enable_collecting_distributing(struct port *port,
> bool *update_slave_arr);
> static void ad_disable_collecting_distributing(struct port *port,
>@@ -171,9 +174,38 @@ static inline int __agg_has_partner(struct aggregator *agg)
> return !is_zero_ether_addr(agg->partner_system.mac_addr_value);
> }
>
>+/**
>+ * __disable_distributing_port - disable the port's slave for distributing.
>+ * Port will still be able to collect.
>+ * @port: the port we're looking at
>+ *
>+ * This will disable only distributing on the port's slave.
>+ */
>+static void __disable_distributing_port(struct port *port)
>+{
>+ bond_set_slave_tx_disabled_flags(port->slave, BOND_SLAVE_NOTIFY_LATER);
>+}
>+
>+/**
>+ * __enable_collecting_port - enable the port's slave for collecting,
>+ * if it's up
>+ * @port: the port we're looking at
>+ *
>+ * This will enable only collecting on the port's slave.
>+ */
>+static void __enable_collecting_port(struct port *port)
>+{
>+ struct slave *slave = port->slave;
>+
>+ if (slave->link == BOND_LINK_UP && bond_slave_is_up(slave))
>+ bond_set_slave_rx_enabled_flags(slave, BOND_SLAVE_NOTIFY_LATER);
>+}
>+
> /**
> * __disable_port - disable the port's slave
> * @port: the port we're looking at
>+ *
>+ * This will disable both collecting and distributing on the port's slave.
> */
> static inline void __disable_port(struct port *port)
> {
>@@ -183,6 +215,8 @@ static inline void __disable_port(struct port *port)
> /**
> * __enable_port - enable the port's slave, if it's up
> * @port: the port we're looking at
>+ *
>+ * This will enable both collecting and distributing on the port's slave.
> */
> static inline void __enable_port(struct port *port)
> {
>@@ -193,10 +227,27 @@ static inline void __enable_port(struct port *port)
> }
>
> /**
>- * __port_is_enabled - check if the port's slave is in active state
>+ * __port_move_to_attached_state - check if port should transition back to attached
>+ * state.
>+ * @port: the port we're looking at
>+ */
>+static bool __port_move_to_attached_state(struct port *port)
>+{
>+ if (!(port->sm_vars & AD_PORT_SELECTED) ||
>+ (port->sm_vars & AD_PORT_STANDBY) ||
>+ !(port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) ||
>+ !(port->actor_oper_port_state & LACP_STATE_SYNCHRONIZATION))
>+ port->sm_mux_state = AD_MUX_ATTACHED;
>+
>+ return port->sm_mux_state == AD_MUX_ATTACHED;
>+}
>+
>+/**
>+ * __port_is_collecting_distributing - check if the port's slave is in the
>+ * combined collecting/distributing state
> * @port: the port we're looking at
> */
>-static inline int __port_is_enabled(struct port *port)
>+static int __port_is_collecting_distributing(struct port *port)
> {
> return bond_is_active_slave(port->slave);
> }
>@@ -942,6 +993,7 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
> */
> static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> {
>+ struct bonding *bond = __get_bond_by_port(port);
> mux_states_t last_state;
>
> /* keep current State Machine state to compare later if it was
>@@ -999,9 +1051,13 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> if ((port->sm_vars & AD_PORT_SELECTED) &&
> (port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) &&
> !__check_agg_selection_timer(port)) {
>- if (port->aggregator->is_active)
>- port->sm_mux_state =
>- AD_MUX_COLLECTING_DISTRIBUTING;
>+ if (port->aggregator->is_active) {
>+ int state = AD_MUX_COLLECTING_DISTRIBUTING;
>+
>+ if (!bond->params.coupled_control)
>+ state = AD_MUX_COLLECTING;
>+ port->sm_mux_state = state;
>+ }
> } else if (!(port->sm_vars & AD_PORT_SELECTED) ||
> (port->sm_vars & AD_PORT_STANDBY)) {
> /* if UNSELECTED or STANDBY */
>@@ -1019,11 +1075,45 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> }
> break;
> case AD_MUX_COLLECTING_DISTRIBUTING:
>+ if (!__port_move_to_attached_state(port)) {
>+ /* if port state hasn't changed make
>+ * sure that a collecting distributing
>+ * port in an active aggregator is enabled
>+ */
>+ if (port->aggregator->is_active &&
>+ !__port_is_collecting_distributing(port)) {
>+ __enable_port(port);
>+ *update_slave_arr = true;
>+ }
>+ }
>+ break;
>+ case AD_MUX_COLLECTING:
>+ if (!__port_move_to_attached_state(port)) {
>+ if ((port->sm_vars & AD_PORT_SELECTED) &&
>+ (port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) &&
>+ (port->partner_oper.port_state & LACP_STATE_COLLECTING)) {
>+ port->sm_mux_state = AD_MUX_DISTRIBUTING;
>+ } else {
>+ /* If port state hasn't changed, make sure that a collecting
>+ * port is enabled for an active aggregator.
>+ */
>+ struct slave *slave = port->slave;
>+
>+ if (port->aggregator->is_active &&
>+ bond_is_slave_rx_disabled(slave)) {
>+ ad_enable_collecting(port);
>+ *update_slave_arr = true;
>+ }
>+ }
>+ }
>+ break;
>+ case AD_MUX_DISTRIBUTING:
> if (!(port->sm_vars & AD_PORT_SELECTED) ||
> (port->sm_vars & AD_PORT_STANDBY) ||
>+ !(port->partner_oper.port_state & LACP_STATE_COLLECTING) ||
> !(port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) ||
> !(port->actor_oper_port_state & LACP_STATE_SYNCHRONIZATION)) {
>- port->sm_mux_state = AD_MUX_ATTACHED;
>+ port->sm_mux_state = AD_MUX_COLLECTING;
> } else {
> /* if port state hasn't changed make
> * sure that a collecting distributing
>@@ -1031,7 +1121,7 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> */
> if (port->aggregator &&
> port->aggregator->is_active &&
>- !__port_is_enabled(port)) {
>+ !__port_is_collecting_distributing(port)) {
> __enable_port(port);
> *update_slave_arr = true;
> }
>@@ -1082,6 +1172,20 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> update_slave_arr);
> port->ntt = true;
> break;
>+ case AD_MUX_COLLECTING:
>+ port->actor_oper_port_state |= LACP_STATE_COLLECTING;
>+ port->actor_oper_port_state &= ~LACP_STATE_DISTRIBUTING;
>+ port->actor_oper_port_state |= LACP_STATE_SYNCHRONIZATION;
>+ ad_enable_collecting(port);
>+ ad_disable_distributing(port, update_slave_arr);
>+ port->ntt = true;
>+ break;
>+ case AD_MUX_DISTRIBUTING:
>+ port->actor_oper_port_state |= LACP_STATE_DISTRIBUTING;
>+ port->actor_oper_port_state |= LACP_STATE_SYNCHRONIZATION;
>+ ad_enable_collecting_distributing(port,
>+ update_slave_arr);
>+ break;
> default:
> break;
> }
>@@ -1906,6 +2010,45 @@ static void ad_initialize_port(struct port *port, int lacp_fast)
> }
> }
>
>+/**
>+ * ad_enable_collecting - enable a port's receive
>+ * @port: the port we're looking at
>+ *
>+ * Enable @port if it's in an active aggregator
>+ */
>+static void ad_enable_collecting(struct port *port)
>+{
>+ if (port->aggregator->is_active) {
>+ struct slave *slave = port->slave;
>+
>+ slave_dbg(slave->bond->dev, slave->dev,
>+ "Enabling collecting on port %d (LAG %d)\n",
>+ port->actor_port_number,
>+ port->aggregator->aggregator_identifier);
>+ __enable_collecting_port(port);
>+ }
>+}
>+
>+/**
>+ * ad_disable_distributing - disable a port's transmit
>+ * @port: the port we're looking at
>+ * @update_slave_arr: Does slave array need update?
>+ */
>+static void ad_disable_distributing(struct port *port, bool *update_slave_arr)
>+{
>+ if (port->aggregator &&
>+ !MAC_ADDRESS_EQUAL(&port->aggregator->partner_system,
>+ &(null_mac_addr))) {
>+ slave_dbg(port->slave->bond->dev, port->slave->dev,
>+ "Disabling distributing on port %d (LAG %d)\n",
>+ port->actor_port_number,
>+ port->aggregator->aggregator_identifier);
>+ __disable_distributing_port(port);
>+ /* Slave array needs an update */
>+ *update_slave_arr = true;
>+ }
>+}
>+
> /**
> * ad_enable_collecting_distributing - enable a port's transmit/receive
> * @port: the port we're looking at
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 8e6cc0e133b7..30f4b0ff01c0 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -6331,6 +6331,7 @@ static int __init bond_check_params(struct bond_params *params)
> params->ad_actor_sys_prio = ad_actor_sys_prio;
> eth_zero_addr(params->ad_actor_system);
> params->ad_user_port_key = ad_user_port_key;
>+ params->coupled_control = 1;
> if (packets_per_slave > 0) {
> params->reciprocal_packets_per_slave =
> reciprocal_value(packets_per_slave);
>diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
>index cfa74cf8bb1a..29b4c3d1b9b6 100644
>--- a/drivers/net/bonding/bond_netlink.c
>+++ b/drivers/net/bonding/bond_netlink.c
>@@ -122,6 +122,7 @@ static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = {
> [IFLA_BOND_PEER_NOTIF_DELAY] = NLA_POLICY_FULL_RANGE(NLA_U32, &delay_range),
> [IFLA_BOND_MISSED_MAX] = { .type = NLA_U8 },
> [IFLA_BOND_NS_IP6_TARGET] = { .type = NLA_NESTED },
>+ [IFLA_BOND_COUPLED_CONTROL] = { .type = NLA_U8 },
> };
>
> static const struct nla_policy bond_slave_policy[IFLA_BOND_SLAVE_MAX + 1] = {
>@@ -549,6 +550,16 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> return err;
> }
>
>+ if (data[IFLA_BOND_COUPLED_CONTROL]) {
>+ int coupled_control = nla_get_u8(data[IFLA_BOND_COUPLED_CONTROL]);
>+
>+ bond_opt_initval(&newval, coupled_control);
>+ err = __bond_opt_set(bond, BOND_OPT_COUPLED_CONTROL, &newval,
>+ data[IFLA_BOND_COUPLED_CONTROL], extack);
>+ if (err)
>+ return err;
>+ }
>+
> return 0;
> }
>
>@@ -615,6 +626,7 @@ static size_t bond_get_size(const struct net_device *bond_dev)
> /* IFLA_BOND_NS_IP6_TARGET */
> nla_total_size(sizeof(struct nlattr)) +
> nla_total_size(sizeof(struct in6_addr)) * BOND_MAX_NS_TARGETS +
>+ nla_total_size(sizeof(u8)) + /* IFLA_BOND_COUPLED_CONTROL */
> 0;
> }
>
>@@ -774,6 +786,10 @@ static int bond_fill_info(struct sk_buff *skb,
> bond->params.missed_max))
> goto nla_put_failure;
>
>+ if (nla_put_u8(skb, IFLA_BOND_COUPLED_CONTROL,
>+ bond->params.coupled_control))
>+ goto nla_put_failure;
>+
> if (BOND_MODE(bond) == BOND_MODE_8023AD) {
> struct ad_info info;
>
>diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>index f3f27f0bd2a6..4cdbc7e084f4 100644
>--- a/drivers/net/bonding/bond_options.c
>+++ b/drivers/net/bonding/bond_options.c
>@@ -84,7 +84,8 @@ static int bond_option_ad_user_port_key_set(struct bonding *bond,
> const struct bond_opt_value *newval);
> static int bond_option_missed_max_set(struct bonding *bond,
> const struct bond_opt_value *newval);
>-
>+static int bond_option_coupled_control_set(struct bonding *bond,
>+ const struct bond_opt_value *newval);
>
> static const struct bond_opt_value bond_mode_tbl[] = {
> { "balance-rr", BOND_MODE_ROUNDROBIN, BOND_VALFLAG_DEFAULT},
>@@ -232,6 +233,12 @@ static const struct bond_opt_value bond_missed_max_tbl[] = {
> { NULL, -1, 0},
> };
>
>+static const struct bond_opt_value bond_coupled_control_tbl[] = {
>+ { "on", 1, BOND_VALFLAG_DEFAULT},
>+ { "off", 0, 0},
>+ { NULL, -1, 0},
>+};
>+
> static const struct bond_option bond_opts[BOND_OPT_LAST] = {
> [BOND_OPT_MODE] = {
> .id = BOND_OPT_MODE,
>@@ -496,6 +503,15 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
> .desc = "Delay between each peer notification on failover event, in milliseconds",
> .values = bond_peer_notif_delay_tbl,
> .set = bond_option_peer_notif_delay_set
>+ },
>+ [BOND_OPT_COUPLED_CONTROL] = {
>+ .id = BOND_OPT_COUPLED_CONTROL,
>+ .name = "coupled_control",
>+ .desc = "Opt into using coupled control MUX for LACP states",
>+ .unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
>+ .flags = BOND_OPTFLAG_IFDOWN,
>+ .values = bond_coupled_control_tbl,
>+ .set = bond_option_coupled_control_set,
> }
> };
>
>@@ -1692,3 +1708,13 @@ static int bond_option_ad_user_port_key_set(struct bonding *bond,
> bond->params.ad_user_port_key = newval->value;
> return 0;
> }
>+
>+static int bond_option_coupled_control_set(struct bonding *bond,
>+ const struct bond_opt_value *newval)
>+{
>+ netdev_info(bond->dev, "Setting coupled_control to %s (%llu)\n",
>+ newval->string, newval->value);
>+
>+ bond->params.coupled_control = newval->value;
>+ return 0;
>+}
>diff --git a/include/net/bond_3ad.h b/include/net/bond_3ad.h
>index c5e57c6bd873..9ce5ac2bfbad 100644
>--- a/include/net/bond_3ad.h
>+++ b/include/net/bond_3ad.h
>@@ -54,6 +54,8 @@ typedef enum {
> AD_MUX_DETACHED, /* mux machine */
> AD_MUX_WAITING, /* mux machine */
> AD_MUX_ATTACHED, /* mux machine */
>+ AD_MUX_COLLECTING, /* mux machine */
>+ AD_MUX_DISTRIBUTING, /* mux machine */
> AD_MUX_COLLECTING_DISTRIBUTING /* mux machine */
> } mux_states_t;
>
>diff --git a/include/net/bond_options.h b/include/net/bond_options.h
>index 69292ecc0325..473a0147769e 100644
>--- a/include/net/bond_options.h
>+++ b/include/net/bond_options.h
>@@ -76,6 +76,7 @@ enum {
> BOND_OPT_MISSED_MAX,
> BOND_OPT_NS_TARGETS,
> BOND_OPT_PRIO,
>+ BOND_OPT_COUPLED_CONTROL,
> BOND_OPT_LAST
> };
>
>diff --git a/include/net/bonding.h b/include/net/bonding.h
>index 5b8b1b644a2d..b61fb1aa3a56 100644
>--- a/include/net/bonding.h
>+++ b/include/net/bonding.h
>@@ -148,6 +148,7 @@ struct bond_params {
> #if IS_ENABLED(CONFIG_IPV6)
> struct in6_addr ns_targets[BOND_MAX_NS_TARGETS];
> #endif
>+ int coupled_control;
>
> /* 2 bytes of padding : see ether_addr_equal_64bits() */
> u8 ad_actor_system[ETH_ALEN + 2];
>@@ -167,6 +168,7 @@ struct slave {
> u8 backup:1, /* indicates backup slave. Value corresponds with
> BOND_STATE_ACTIVE and BOND_STATE_BACKUP */
> inactive:1, /* indicates inactive slave */
>+ rx_disabled:1, /* indicates whether slave's Rx is disabled */
> should_notify:1, /* indicates whether the state changed */
> should_notify_link:1; /* indicates whether the link changed */
> u8 duplex;
>@@ -568,6 +570,14 @@ static inline void bond_set_slave_inactive_flags(struct slave *slave,
> bond_set_slave_state(slave, BOND_STATE_BACKUP, notify);
> if (!slave->bond->params.all_slaves_active)
> slave->inactive = 1;
>+ if (BOND_MODE(slave->bond) == BOND_MODE_8023AD)
>+ slave->rx_disabled = 1;
>+}
>+
>+static inline void bond_set_slave_tx_disabled_flags(struct slave *slave,
>+ bool notify)
>+{
>+ bond_set_slave_state(slave, BOND_STATE_BACKUP, notify);
> }
>
> static inline void bond_set_slave_active_flags(struct slave *slave,
>@@ -575,6 +585,14 @@ static inline void bond_set_slave_active_flags(struct slave *slave,
> {
> bond_set_slave_state(slave, BOND_STATE_ACTIVE, notify);
> slave->inactive = 0;
>+ if (BOND_MODE(slave->bond) == BOND_MODE_8023AD)
>+ slave->rx_disabled = 0;
>+}
>+
>+static inline void bond_set_slave_rx_enabled_flags(struct slave *slave,
>+ bool notify)
>+{
>+ slave->rx_disabled = 0;
> }
>
> static inline bool bond_is_slave_inactive(struct slave *slave)
>@@ -582,6 +600,11 @@ static inline bool bond_is_slave_inactive(struct slave *slave)
> return slave->inactive;
> }
>
>+static inline bool bond_is_slave_rx_disabled(struct slave *slave)
>+{
>+ return slave->rx_disabled;
>+}
>+
> static inline void bond_propose_link_state(struct slave *slave, int state)
> {
> slave->link_new_state = state;
>diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>index 29ff80da2775..7a54fcff2eec 100644
>--- a/include/uapi/linux/if_link.h
>+++ b/include/uapi/linux/if_link.h
>@@ -976,6 +976,7 @@ enum {
> IFLA_BOND_AD_LACP_ACTIVE,
> IFLA_BOND_MISSED_MAX,
> IFLA_BOND_NS_IP6_TARGET,
>+ IFLA_BOND_COUPLED_CONTROL,
> __IFLA_BOND_MAX,
> };
>
>diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h
>index a0aa05a28cf2..f0d71b2a3f1e 100644
>--- a/tools/include/uapi/linux/if_link.h
>+++ b/tools/include/uapi/linux/if_link.h
>@@ -974,6 +974,7 @@ enum {
> IFLA_BOND_AD_LACP_ACTIVE,
> IFLA_BOND_MISSED_MAX,
> IFLA_BOND_NS_IP6_TARGET,
>+ IFLA_BOND_COUPLED_CONTROL,
> __IFLA_BOND_MAX,
> };
>
>--
>2.43.0.429.g432eaa2c6b-goog
>
>
> Do you have a corresponding iproute2 patch to enable changing
> the coupled_control option?
I do have the corresponding patch for iproute2 to enable changing
coupled_control, I planned to push that patch for review directly
after this one.
On Tue, Jan 23, 2024 at 10:16 PM Jay Vosburgh
<[email protected]> wrote:
>
> Aahil Awatramani <[email protected]> wrote:
>
> >Add support for the independent control state machine per IEEE
> >802.1AX-2008 5.4.15 in addition to the existing implementation of the
> >coupled control state machine.
> >
> >Introduces two new states, AD_MUX_COLLECTING and AD_MUX_DISTRIBUTING in
> >the LACP MUX state machine for separated handling of an initial
> >Collecting state before the Collecting and Distributing state. This
> >enables a port to be in a state where it can receive incoming packets
> >while not still distributing. This is useful for reducing packet loss when
> >a port begins distributing before its partner is able to collect.
> >
> >Added new functions such as bond_set_slave_tx_disabled_flags and
> >bond_set_slave_rx_enabled_flags to precisely manage the port's collecting
> >and distributing states. Previously, there was no dedicated method to
> >disable TX while keeping RX enabled, which this patch addresses.
> >
> >Note that the regular flow process in the kernel's bonding driver remains
> >unaffected by this patch. The extension requires explicit opt-in by the
> >user (in order to ensure no disruptions for existing setups) via netlink
> >support using the new bonding parameter coupled_control. The default value
> >for coupled_control is set to 1 so as to preserve existing behaviour.
>
> Do you have a corresponding iproute2 patch to enable changing
> the coupled_control option?
>
> -J
>
> >Signed-off-by: Aahil Awatramani <[email protected]>
> >
> >v5:
> > Merge documentation patch with changes patch
> > Add version history in comment description
> >v4:
> > Remove inline references from c source files
> >v3:
> > Edited commit description
> > Edited documentation description
> > Changed function names
> > Only allow coupled_control change when the bond is down
> >v2:
> > Removed sysfs changes
> > Added documentation for new paramater
> > Renamed parameter to coupled_control
> > Update bond_set_slave_inactive_flags() with a 8023ad check
> >
> >---
> > Documentation/networking/bonding.rst | 12 ++
> > drivers/net/bonding/bond_3ad.c | 157 +++++++++++++++++++++++++--
> > drivers/net/bonding/bond_main.c | 1 +
> > drivers/net/bonding/bond_netlink.c | 16 +++
> > drivers/net/bonding/bond_options.c | 28 ++++-
> > include/net/bond_3ad.h | 2 +
> > include/net/bond_options.h | 1 +
> > include/net/bonding.h | 23 ++++
> > include/uapi/linux/if_link.h | 1 +
> > tools/include/uapi/linux/if_link.h | 1 +
> > 10 files changed, 234 insertions(+), 8 deletions(-)
> >
> >diff --git a/Documentation/networking/bonding.rst b/Documentation/networking/bonding.rst
> >index f7a73421eb76..e774b48de9f5 100644
> >--- a/Documentation/networking/bonding.rst
> >+++ b/Documentation/networking/bonding.rst
> >@@ -444,6 +444,18 @@ arp_missed_max
> >
> > The default value is 2, and the allowable range is 1 - 255.
> >
> >+coupled_control
> >+
> >+ Specifies whether the LACP state machine's MUX in the 802.3ad mode
> >+ should have separate Collecting and Distributing states.
> >+
> >+ This is by implementing the independent control state machine per
> >+ IEEE 802.1AX-2008 5.4.15 in addition to the existing coupled control
> >+ state machine.
> >+
> >+ The default value is 1. This setting does not separate the Collecting
> >+ and Distributing states, maintaining the bond in coupled control.
> >+
> > downdelay
> >
> > Specifies the time, in milliseconds, to wait before disabling
> >diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
> >index c99ffe6c683a..f2942e8c6c91 100644
> >--- a/drivers/net/bonding/bond_3ad.c
> >+++ b/drivers/net/bonding/bond_3ad.c
> >@@ -106,6 +106,9 @@ static void ad_agg_selection_logic(struct aggregator *aggregator,
> > static void ad_clear_agg(struct aggregator *aggregator);
> > static void ad_initialize_agg(struct aggregator *aggregator);
> > static void ad_initialize_port(struct port *port, int lacp_fast);
> >+static void ad_enable_collecting(struct port *port);
> >+static void ad_disable_distributing(struct port *port,
> >+ bool *update_slave_arr);
> > static void ad_enable_collecting_distributing(struct port *port,
> > bool *update_slave_arr);
> > static void ad_disable_collecting_distributing(struct port *port,
> >@@ -171,9 +174,38 @@ static inline int __agg_has_partner(struct aggregator *agg)
> > return !is_zero_ether_addr(agg->partner_system.mac_addr_value);
> > }
> >
> >+/**
> >+ * __disable_distributing_port - disable the port's slave for distributing.
> >+ * Port will still be able to collect.
> >+ * @port: the port we're looking at
> >+ *
> >+ * This will disable only distributing on the port's slave.
> >+ */
> >+static void __disable_distributing_port(struct port *port)
> >+{
> >+ bond_set_slave_tx_disabled_flags(port->slave, BOND_SLAVE_NOTIFY_LATER);
> >+}
> >+
> >+/**
> >+ * __enable_collecting_port - enable the port's slave for collecting,
> >+ * if it's up
> >+ * @port: the port we're looking at
> >+ *
> >+ * This will enable only collecting on the port's slave.
> >+ */
> >+static void __enable_collecting_port(struct port *port)
> >+{
> >+ struct slave *slave = port->slave;
> >+
> >+ if (slave->link == BOND_LINK_UP && bond_slave_is_up(slave))
> >+ bond_set_slave_rx_enabled_flags(slave, BOND_SLAVE_NOTIFY_LATER);
> >+}
> >+
> > /**
> > * __disable_port - disable the port's slave
> > * @port: the port we're looking at
> >+ *
> >+ * This will disable both collecting and distributing on the port's slave.
> > */
> > static inline void __disable_port(struct port *port)
> > {
> >@@ -183,6 +215,8 @@ static inline void __disable_port(struct port *port)
> > /**
> > * __enable_port - enable the port's slave, if it's up
> > * @port: the port we're looking at
> >+ *
> >+ * This will enable both collecting and distributing on the port's slave.
> > */
> > static inline void __enable_port(struct port *port)
> > {
> >@@ -193,10 +227,27 @@ static inline void __enable_port(struct port *port)
> > }
> >
> > /**
> >- * __port_is_enabled - check if the port's slave is in active state
> >+ * __port_move_to_attached_state - check if port should transition back to attached
> >+ * state.
> >+ * @port: the port we're looking at
> >+ */
> >+static bool __port_move_to_attached_state(struct port *port)
> >+{
> >+ if (!(port->sm_vars & AD_PORT_SELECTED) ||
> >+ (port->sm_vars & AD_PORT_STANDBY) ||
> >+ !(port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) ||
> >+ !(port->actor_oper_port_state & LACP_STATE_SYNCHRONIZATION))
> >+ port->sm_mux_state = AD_MUX_ATTACHED;
> >+
> >+ return port->sm_mux_state == AD_MUX_ATTACHED;
> >+}
> >+
> >+/**
> >+ * __port_is_collecting_distributing - check if the port's slave is in the
> >+ * combined collecting/distributing state
> > * @port: the port we're looking at
> > */
> >-static inline int __port_is_enabled(struct port *port)
> >+static int __port_is_collecting_distributing(struct port *port)
> > {
> > return bond_is_active_slave(port->slave);
> > }
> >@@ -942,6 +993,7 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
> > */
> > static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> > {
> >+ struct bonding *bond = __get_bond_by_port(port);
> > mux_states_t last_state;
> >
> > /* keep current State Machine state to compare later if it was
> >@@ -999,9 +1051,13 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> > if ((port->sm_vars & AD_PORT_SELECTED) &&
> > (port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) &&
> > !__check_agg_selection_timer(port)) {
> >- if (port->aggregator->is_active)
> >- port->sm_mux_state =
> >- AD_MUX_COLLECTING_DISTRIBUTING;
> >+ if (port->aggregator->is_active) {
> >+ int state = AD_MUX_COLLECTING_DISTRIBUTING;
> >+
> >+ if (!bond->params.coupled_control)
> >+ state = AD_MUX_COLLECTING;
> >+ port->sm_mux_state = state;
> >+ }
> > } else if (!(port->sm_vars & AD_PORT_SELECTED) ||
> > (port->sm_vars & AD_PORT_STANDBY)) {
> > /* if UNSELECTED or STANDBY */
> >@@ -1019,11 +1075,45 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> > }
> > break;
> > case AD_MUX_COLLECTING_DISTRIBUTING:
> >+ if (!__port_move_to_attached_state(port)) {
> >+ /* if port state hasn't changed make
> >+ * sure that a collecting distributing
> >+ * port in an active aggregator is enabled
> >+ */
> >+ if (port->aggregator->is_active &&
> >+ !__port_is_collecting_distributing(port)) {
> >+ __enable_port(port);
> >+ *update_slave_arr = true;
> >+ }
> >+ }
> >+ break;
> >+ case AD_MUX_COLLECTING:
> >+ if (!__port_move_to_attached_state(port)) {
> >+ if ((port->sm_vars & AD_PORT_SELECTED) &&
> >+ (port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) &&
> >+ (port->partner_oper.port_state & LACP_STATE_COLLECTING)) {
> >+ port->sm_mux_state = AD_MUX_DISTRIBUTING;
> >+ } else {
> >+ /* If port state hasn't changed, make sure that a collecting
> >+ * port is enabled for an active aggregator.
> >+ */
> >+ struct slave *slave = port->slave;
> >+
> >+ if (port->aggregator->is_active &&
> >+ bond_is_slave_rx_disabled(slave)) {
> >+ ad_enable_collecting(port);
> >+ *update_slave_arr = true;
> >+ }
> >+ }
> >+ }
> >+ break;
> >+ case AD_MUX_DISTRIBUTING:
> > if (!(port->sm_vars & AD_PORT_SELECTED) ||
> > (port->sm_vars & AD_PORT_STANDBY) ||
> >+ !(port->partner_oper.port_state & LACP_STATE_COLLECTING) ||
> > !(port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) ||
> > !(port->actor_oper_port_state & LACP_STATE_SYNCHRONIZATION)) {
> >- port->sm_mux_state = AD_MUX_ATTACHED;
> >+ port->sm_mux_state = AD_MUX_COLLECTING;
> > } else {
> > /* if port state hasn't changed make
> > * sure that a collecting distributing
> >@@ -1031,7 +1121,7 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> > */
> > if (port->aggregator &&
> > port->aggregator->is_active &&
> >- !__port_is_enabled(port)) {
> >+ !__port_is_collecting_distributing(port)) {
> > __enable_port(port);
> > *update_slave_arr = true;
> > }
> >@@ -1082,6 +1172,20 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> > update_slave_arr);
> > port->ntt = true;
> > break;
> >+ case AD_MUX_COLLECTING:
> >+ port->actor_oper_port_state |= LACP_STATE_COLLECTING;
> >+ port->actor_oper_port_state &= ~LACP_STATE_DISTRIBUTING;
> >+ port->actor_oper_port_state |= LACP_STATE_SYNCHRONIZATION;
> >+ ad_enable_collecting(port);
> >+ ad_disable_distributing(port, update_slave_arr);
> >+ port->ntt = true;
> >+ break;
> >+ case AD_MUX_DISTRIBUTING:
> >+ port->actor_oper_port_state |= LACP_STATE_DISTRIBUTING;
> >+ port->actor_oper_port_state |= LACP_STATE_SYNCHRONIZATION;
> >+ ad_enable_collecting_distributing(port,
> >+ update_slave_arr);
> >+ break;
> > default:
> > break;
> > }
> >@@ -1906,6 +2010,45 @@ static void ad_initialize_port(struct port *port, int lacp_fast)
> > }
> > }
> >
> >+/**
> >+ * ad_enable_collecting - enable a port's receive
> >+ * @port: the port we're looking at
> >+ *
> >+ * Enable @port if it's in an active aggregator
> >+ */
> >+static void ad_enable_collecting(struct port *port)
> >+{
> >+ if (port->aggregator->is_active) {
> >+ struct slave *slave = port->slave;
> >+
> >+ slave_dbg(slave->bond->dev, slave->dev,
> >+ "Enabling collecting on port %d (LAG %d)\n",
> >+ port->actor_port_number,
> >+ port->aggregator->aggregator_identifier);
> >+ __enable_collecting_port(port);
> >+ }
> >+}
> >+
> >+/**
> >+ * ad_disable_distributing - disable a port's transmit
> >+ * @port: the port we're looking at
> >+ * @update_slave_arr: Does slave array need update?
> >+ */
> >+static void ad_disable_distributing(struct port *port, bool *update_slave_arr)
> >+{
> >+ if (port->aggregator &&
> >+ !MAC_ADDRESS_EQUAL(&port->aggregator->partner_system,
> >+ &(null_mac_addr))) {
> >+ slave_dbg(port->slave->bond->dev, port->slave->dev,
> >+ "Disabling distributing on port %d (LAG %d)\n",
> >+ port->actor_port_number,
> >+ port->aggregator->aggregator_identifier);
> >+ __disable_distributing_port(port);
> >+ /* Slave array needs an update */
> >+ *update_slave_arr = true;
> >+ }
> >+}
> >+
> > /**
> > * ad_enable_collecting_distributing - enable a port's transmit/receive
> > * @port: the port we're looking at
> >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> >index 8e6cc0e133b7..30f4b0ff01c0 100644
> >--- a/drivers/net/bonding/bond_main.c
> >+++ b/drivers/net/bonding/bond_main.c
> >@@ -6331,6 +6331,7 @@ static int __init bond_check_params(struct bond_params *params)
> > params->ad_actor_sys_prio = ad_actor_sys_prio;
> > eth_zero_addr(params->ad_actor_system);
> > params->ad_user_port_key = ad_user_port_key;
> >+ params->coupled_control = 1;
> > if (packets_per_slave > 0) {
> > params->reciprocal_packets_per_slave =
> > reciprocal_value(packets_per_slave);
> >diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
> >index cfa74cf8bb1a..29b4c3d1b9b6 100644
> >--- a/drivers/net/bonding/bond_netlink.c
> >+++ b/drivers/net/bonding/bond_netlink.c
> >@@ -122,6 +122,7 @@ static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = {
> > [IFLA_BOND_PEER_NOTIF_DELAY] = NLA_POLICY_FULL_RANGE(NLA_U32, &delay_range),
> > [IFLA_BOND_MISSED_MAX] = { .type = NLA_U8 },
> > [IFLA_BOND_NS_IP6_TARGET] = { .type = NLA_NESTED },
> >+ [IFLA_BOND_COUPLED_CONTROL] = { .type = NLA_U8 },
> > };
> >
> > static const struct nla_policy bond_slave_policy[IFLA_BOND_SLAVE_MAX + 1] = {
> >@@ -549,6 +550,16 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> > return err;
> > }
> >
> >+ if (data[IFLA_BOND_COUPLED_CONTROL]) {
> >+ int coupled_control = nla_get_u8(data[IFLA_BOND_COUPLED_CONTROL]);
> >+
> >+ bond_opt_initval(&newval, coupled_control);
> >+ err = __bond_opt_set(bond, BOND_OPT_COUPLED_CONTROL, &newval,
> >+ data[IFLA_BOND_COUPLED_CONTROL], extack);
> >+ if (err)
> >+ return err;
> >+ }
> >+
> > return 0;
> > }
> >
> >@@ -615,6 +626,7 @@ static size_t bond_get_size(const struct net_device *bond_dev)
> > /* IFLA_BOND_NS_IP6_TARGET */
> > nla_total_size(sizeof(struct nlattr)) +
> > nla_total_size(sizeof(struct in6_addr)) * BOND_MAX_NS_TARGETS +
> >+ nla_total_size(sizeof(u8)) + /* IFLA_BOND_COUPLED_CONTROL */
> > 0;
> > }
> >
> >@@ -774,6 +786,10 @@ static int bond_fill_info(struct sk_buff *skb,
> > bond->params.missed_max))
> > goto nla_put_failure;
> >
> >+ if (nla_put_u8(skb, IFLA_BOND_COUPLED_CONTROL,
> >+ bond->params.coupled_control))
> >+ goto nla_put_failure;
> >+
> > if (BOND_MODE(bond) == BOND_MODE_8023AD) {
> > struct ad_info info;
> >
> >diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
> >index f3f27f0bd2a6..4cdbc7e084f4 100644
> >--- a/drivers/net/bonding/bond_options.c
> >+++ b/drivers/net/bonding/bond_options.c
> >@@ -84,7 +84,8 @@ static int bond_option_ad_user_port_key_set(struct bonding *bond,
> > const struct bond_opt_value *newval);
> > static int bond_option_missed_max_set(struct bonding *bond,
> > const struct bond_opt_value *newval);
> >-
> >+static int bond_option_coupled_control_set(struct bonding *bond,
> >+ const struct bond_opt_value *newval);
> >
> > static const struct bond_opt_value bond_mode_tbl[] = {
> > { "balance-rr", BOND_MODE_ROUNDROBIN, BOND_VALFLAG_DEFAULT},
> >@@ -232,6 +233,12 @@ static const struct bond_opt_value bond_missed_max_tbl[] = {
> > { NULL, -1, 0},
> > };
> >
> >+static const struct bond_opt_value bond_coupled_control_tbl[] = {
> >+ { "on", 1, BOND_VALFLAG_DEFAULT},
> >+ { "off", 0, 0},
> >+ { NULL, -1, 0},
> >+};
> >+
> > static const struct bond_option bond_opts[BOND_OPT_LAST] = {
> > [BOND_OPT_MODE] = {
> > .id = BOND_OPT_MODE,
> >@@ -496,6 +503,15 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
> > .desc = "Delay between each peer notification on failover event, in milliseconds",
> > .values = bond_peer_notif_delay_tbl,
> > .set = bond_option_peer_notif_delay_set
> >+ },
> >+ [BOND_OPT_COUPLED_CONTROL] = {
> >+ .id = BOND_OPT_COUPLED_CONTROL,
> >+ .name = "coupled_control",
> >+ .desc = "Opt into using coupled control MUX for LACP states",
> >+ .unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
> >+ .flags = BOND_OPTFLAG_IFDOWN,
> >+ .values = bond_coupled_control_tbl,
> >+ .set = bond_option_coupled_control_set,
> > }
> > };
> >
> >@@ -1692,3 +1708,13 @@ static int bond_option_ad_user_port_key_set(struct bonding *bond,
> > bond->params.ad_user_port_key = newval->value;
> > return 0;
> > }
> >+
> >+static int bond_option_coupled_control_set(struct bonding *bond,
> >+ const struct bond_opt_value *newval)
> >+{
> >+ netdev_info(bond->dev, "Setting coupled_control to %s (%llu)\n",
> >+ newval->string, newval->value);
> >+
> >+ bond->params.coupled_control = newval->value;
> >+ return 0;
> >+}
> >diff --git a/include/net/bond_3ad.h b/include/net/bond_3ad.h
> >index c5e57c6bd873..9ce5ac2bfbad 100644
> >--- a/include/net/bond_3ad.h
> >+++ b/include/net/bond_3ad.h
> >@@ -54,6 +54,8 @@ typedef enum {
> > AD_MUX_DETACHED, /* mux machine */
> > AD_MUX_WAITING, /* mux machine */
> > AD_MUX_ATTACHED, /* mux machine */
> >+ AD_MUX_COLLECTING, /* mux machine */
> >+ AD_MUX_DISTRIBUTING, /* mux machine */
> > AD_MUX_COLLECTING_DISTRIBUTING /* mux machine */
> > } mux_states_t;
> >
> >diff --git a/include/net/bond_options.h b/include/net/bond_options.h
> >index 69292ecc0325..473a0147769e 100644
> >--- a/include/net/bond_options.h
> >+++ b/include/net/bond_options.h
> >@@ -76,6 +76,7 @@ enum {
> > BOND_OPT_MISSED_MAX,
> > BOND_OPT_NS_TARGETS,
> > BOND_OPT_PRIO,
> >+ BOND_OPT_COUPLED_CONTROL,
> > BOND_OPT_LAST
> > };
> >
> >diff --git a/include/net/bonding.h b/include/net/bonding.h
> >index 5b8b1b644a2d..b61fb1aa3a56 100644
> >--- a/include/net/bonding.h
> >+++ b/include/net/bonding.h
> >@@ -148,6 +148,7 @@ struct bond_params {
> > #if IS_ENABLED(CONFIG_IPV6)
> > struct in6_addr ns_targets[BOND_MAX_NS_TARGETS];
> > #endif
> >+ int coupled_control;
> >
> > /* 2 bytes of padding : see ether_addr_equal_64bits() */
> > u8 ad_actor_system[ETH_ALEN + 2];
> >@@ -167,6 +168,7 @@ struct slave {
> > u8 backup:1, /* indicates backup slave. Value corresponds with
> > BOND_STATE_ACTIVE and BOND_STATE_BACKUP */
> > inactive:1, /* indicates inactive slave */
> >+ rx_disabled:1, /* indicates whether slave's Rx is disabled */
> > should_notify:1, /* indicates whether the state changed */
> > should_notify_link:1; /* indicates whether the link changed */
> > u8 duplex;
> >@@ -568,6 +570,14 @@ static inline void bond_set_slave_inactive_flags(struct slave *slave,
> > bond_set_slave_state(slave, BOND_STATE_BACKUP, notify);
> > if (!slave->bond->params.all_slaves_active)
> > slave->inactive = 1;
> >+ if (BOND_MODE(slave->bond) == BOND_MODE_8023AD)
> >+ slave->rx_disabled = 1;
> >+}
> >+
> >+static inline void bond_set_slave_tx_disabled_flags(struct slave *slave,
> >+ bool notify)
> >+{
> >+ bond_set_slave_state(slave, BOND_STATE_BACKUP, notify);
> > }
> >
> > static inline void bond_set_slave_active_flags(struct slave *slave,
> >@@ -575,6 +585,14 @@ static inline void bond_set_slave_active_flags(struct slave *slave,
> > {
> > bond_set_slave_state(slave, BOND_STATE_ACTIVE, notify);
> > slave->inactive = 0;
> >+ if (BOND_MODE(slave->bond) == BOND_MODE_8023AD)
> >+ slave->rx_disabled = 0;
> >+}
> >+
> >+static inline void bond_set_slave_rx_enabled_flags(struct slave *slave,
> >+ bool notify)
> >+{
> >+ slave->rx_disabled = 0;
> > }
> >
> > static inline bool bond_is_slave_inactive(struct slave *slave)
> >@@ -582,6 +600,11 @@ static inline bool bond_is_slave_inactive(struct slave *slave)
> > return slave->inactive;
> > }
> >
> >+static inline bool bond_is_slave_rx_disabled(struct slave *slave)
> >+{
> >+ return slave->rx_disabled;
> >+}
> >+
> > static inline void bond_propose_link_state(struct slave *slave, int state)
> > {
> > slave->link_new_state = state;
> >diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> >index 29ff80da2775..7a54fcff2eec 100644
> >--- a/include/uapi/linux/if_link.h
> >+++ b/include/uapi/linux/if_link.h
> >@@ -976,6 +976,7 @@ enum {
> > IFLA_BOND_AD_LACP_ACTIVE,
> > IFLA_BOND_MISSED_MAX,
> > IFLA_BOND_NS_IP6_TARGET,
> >+ IFLA_BOND_COUPLED_CONTROL,
> > __IFLA_BOND_MAX,
> > };
> >
> >diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h
> >index a0aa05a28cf2..f0d71b2a3f1e 100644
> >--- a/tools/include/uapi/linux/if_link.h
> >+++ b/tools/include/uapi/linux/if_link.h
> >@@ -974,6 +974,7 @@ enum {
> > IFLA_BOND_AD_LACP_ACTIVE,
> > IFLA_BOND_MISSED_MAX,
> > IFLA_BOND_NS_IP6_TARGET,
> >+ IFLA_BOND_COUPLED_CONTROL,
> > __IFLA_BOND_MAX,
> > };
> >
> >--
> >2.43.0.429.g432eaa2c6b-goog
> >
> >
Aahil Awatramani <[email protected]> wrote:
>> Do you have a corresponding iproute2 patch to enable changing
>> the coupled_control option?
>
>I do have the corresponding patch for iproute2 to enable changing
>coupled_control, I planned to push that patch for review directly
>after this one.
Could you post it now? They're logically parts of a single
feature, so should go together. As a practical matter, though, it's
easier to test this patch with the iproute patch available.
-J
>
>On Tue, Jan 23, 2024 at 10:16 PM Jay Vosburgh
><[email protected]> wrote:
>>
>> Aahil Awatramani <[email protected]> wrote:
>>
>> >Add support for the independent control state machine per IEEE
>> >802.1AX-2008 5.4.15 in addition to the existing implementation of the
>> >coupled control state machine.
>> >
>> >Introduces two new states, AD_MUX_COLLECTING and AD_MUX_DISTRIBUTING in
>> >the LACP MUX state machine for separated handling of an initial
>> >Collecting state before the Collecting and Distributing state. This
>> >enables a port to be in a state where it can receive incoming packets
>> >while not still distributing. This is useful for reducing packet loss when
>> >a port begins distributing before its partner is able to collect.
>> >
>> >Added new functions such as bond_set_slave_tx_disabled_flags and
>> >bond_set_slave_rx_enabled_flags to precisely manage the port's collecting
>> >and distributing states. Previously, there was no dedicated method to
>> >disable TX while keeping RX enabled, which this patch addresses.
>> >
>> >Note that the regular flow process in the kernel's bonding driver remains
>> >unaffected by this patch. The extension requires explicit opt-in by the
>> >user (in order to ensure no disruptions for existing setups) via netlink
>> >support using the new bonding parameter coupled_control. The default value
>> >for coupled_control is set to 1 so as to preserve existing behaviour.
>>
>> Do you have a corresponding iproute2 patch to enable changing
>> the coupled_control option?
>>
>> -J
>>
>> >Signed-off-by: Aahil Awatramani <[email protected]>
>> >
>> >v5:
>> > Merge documentation patch with changes patch
>> > Add version history in comment description
>> >v4:
>> > Remove inline references from c source files
>> >v3:
>> > Edited commit description
>> > Edited documentation description
>> > Changed function names
>> > Only allow coupled_control change when the bond is down
>> >v2:
>> > Removed sysfs changes
>> > Added documentation for new paramater
>> > Renamed parameter to coupled_control
>> > Update bond_set_slave_inactive_flags() with a 8023ad check
>> >
>> >---
>> > Documentation/networking/bonding.rst | 12 ++
>> > drivers/net/bonding/bond_3ad.c | 157 +++++++++++++++++++++++++--
>> > drivers/net/bonding/bond_main.c | 1 +
>> > drivers/net/bonding/bond_netlink.c | 16 +++
>> > drivers/net/bonding/bond_options.c | 28 ++++-
>> > include/net/bond_3ad.h | 2 +
>> > include/net/bond_options.h | 1 +
>> > include/net/bonding.h | 23 ++++
>> > include/uapi/linux/if_link.h | 1 +
>> > tools/include/uapi/linux/if_link.h | 1 +
>> > 10 files changed, 234 insertions(+), 8 deletions(-)
>> >
>> >diff --git a/Documentation/networking/bonding.rst b/Documentation/networking/bonding.rst
>> >index f7a73421eb76..e774b48de9f5 100644
>> >--- a/Documentation/networking/bonding.rst
>> >+++ b/Documentation/networking/bonding.rst
>> >@@ -444,6 +444,18 @@ arp_missed_max
>> >
>> > The default value is 2, and the allowable range is 1 - 255.
>> >
>> >+coupled_control
>> >+
>> >+ Specifies whether the LACP state machine's MUX in the 802.3ad mode
>> >+ should have separate Collecting and Distributing states.
>> >+
>> >+ This is by implementing the independent control state machine per
>> >+ IEEE 802.1AX-2008 5.4.15 in addition to the existing coupled control
>> >+ state machine.
>> >+
>> >+ The default value is 1. This setting does not separate the Collecting
>> >+ and Distributing states, maintaining the bond in coupled control.
>> >+
>> > downdelay
>> >
>> > Specifies the time, in milliseconds, to wait before disabling
>> >diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>> >index c99ffe6c683a..f2942e8c6c91 100644
>> >--- a/drivers/net/bonding/bond_3ad.c
>> >+++ b/drivers/net/bonding/bond_3ad.c
>> >@@ -106,6 +106,9 @@ static void ad_agg_selection_logic(struct aggregator *aggregator,
>> > static void ad_clear_agg(struct aggregator *aggregator);
>> > static void ad_initialize_agg(struct aggregator *aggregator);
>> > static void ad_initialize_port(struct port *port, int lacp_fast);
>> >+static void ad_enable_collecting(struct port *port);
>> >+static void ad_disable_distributing(struct port *port,
>> >+ bool *update_slave_arr);
>> > static void ad_enable_collecting_distributing(struct port *port,
>> > bool *update_slave_arr);
>> > static void ad_disable_collecting_distributing(struct port *port,
>> >@@ -171,9 +174,38 @@ static inline int __agg_has_partner(struct aggregator *agg)
>> > return !is_zero_ether_addr(agg->partner_system.mac_addr_value);
>> > }
>> >
>> >+/**
>> >+ * __disable_distributing_port - disable the port's slave for distributing.
>> >+ * Port will still be able to collect.
>> >+ * @port: the port we're looking at
>> >+ *
>> >+ * This will disable only distributing on the port's slave.
>> >+ */
>> >+static void __disable_distributing_port(struct port *port)
>> >+{
>> >+ bond_set_slave_tx_disabled_flags(port->slave, BOND_SLAVE_NOTIFY_LATER);
>> >+}
>> >+
>> >+/**
>> >+ * __enable_collecting_port - enable the port's slave for collecting,
>> >+ * if it's up
>> >+ * @port: the port we're looking at
>> >+ *
>> >+ * This will enable only collecting on the port's slave.
>> >+ */
>> >+static void __enable_collecting_port(struct port *port)
>> >+{
>> >+ struct slave *slave = port->slave;
>> >+
>> >+ if (slave->link == BOND_LINK_UP && bond_slave_is_up(slave))
>> >+ bond_set_slave_rx_enabled_flags(slave, BOND_SLAVE_NOTIFY_LATER);
>> >+}
>> >+
>> > /**
>> > * __disable_port - disable the port's slave
>> > * @port: the port we're looking at
>> >+ *
>> >+ * This will disable both collecting and distributing on the port's slave.
>> > */
>> > static inline void __disable_port(struct port *port)
>> > {
>> >@@ -183,6 +215,8 @@ static inline void __disable_port(struct port *port)
>> > /**
>> > * __enable_port - enable the port's slave, if it's up
>> > * @port: the port we're looking at
>> >+ *
>> >+ * This will enable both collecting and distributing on the port's slave.
>> > */
>> > static inline void __enable_port(struct port *port)
>> > {
>> >@@ -193,10 +227,27 @@ static inline void __enable_port(struct port *port)
>> > }
>> >
>> > /**
>> >- * __port_is_enabled - check if the port's slave is in active state
>> >+ * __port_move_to_attached_state - check if port should transition back to attached
>> >+ * state.
>> >+ * @port: the port we're looking at
>> >+ */
>> >+static bool __port_move_to_attached_state(struct port *port)
>> >+{
>> >+ if (!(port->sm_vars & AD_PORT_SELECTED) ||
>> >+ (port->sm_vars & AD_PORT_STANDBY) ||
>> >+ !(port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) ||
>> >+ !(port->actor_oper_port_state & LACP_STATE_SYNCHRONIZATION))
>> >+ port->sm_mux_state = AD_MUX_ATTACHED;
>> >+
>> >+ return port->sm_mux_state == AD_MUX_ATTACHED;
>> >+}
>> >+
>> >+/**
>> >+ * __port_is_collecting_distributing - check if the port's slave is in the
>> >+ * combined collecting/distributing state
>> > * @port: the port we're looking at
>> > */
>> >-static inline int __port_is_enabled(struct port *port)
>> >+static int __port_is_collecting_distributing(struct port *port)
>> > {
>> > return bond_is_active_slave(port->slave);
>> > }
>> >@@ -942,6 +993,7 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
>> > */
>> > static void ad_mux_machine(struct port *port, bool *update_slave_arr)
>> > {
>> >+ struct bonding *bond = __get_bond_by_port(port);
>> > mux_states_t last_state;
>> >
>> > /* keep current State Machine state to compare later if it was
>> >@@ -999,9 +1051,13 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
>> > if ((port->sm_vars & AD_PORT_SELECTED) &&
>> > (port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) &&
>> > !__check_agg_selection_timer(port)) {
>> >- if (port->aggregator->is_active)
>> >- port->sm_mux_state =
>> >- AD_MUX_COLLECTING_DISTRIBUTING;
>> >+ if (port->aggregator->is_active) {
>> >+ int state = AD_MUX_COLLECTING_DISTRIBUTING;
>> >+
>> >+ if (!bond->params.coupled_control)
>> >+ state = AD_MUX_COLLECTING;
>> >+ port->sm_mux_state = state;
>> >+ }
>> > } else if (!(port->sm_vars & AD_PORT_SELECTED) ||
>> > (port->sm_vars & AD_PORT_STANDBY)) {
>> > /* if UNSELECTED or STANDBY */
>> >@@ -1019,11 +1075,45 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
>> > }
>> > break;
>> > case AD_MUX_COLLECTING_DISTRIBUTING:
>> >+ if (!__port_move_to_attached_state(port)) {
>> >+ /* if port state hasn't changed make
>> >+ * sure that a collecting distributing
>> >+ * port in an active aggregator is enabled
>> >+ */
>> >+ if (port->aggregator->is_active &&
>> >+ !__port_is_collecting_distributing(port)) {
>> >+ __enable_port(port);
>> >+ *update_slave_arr = true;
>> >+ }
>> >+ }
>> >+ break;
>> >+ case AD_MUX_COLLECTING:
>> >+ if (!__port_move_to_attached_state(port)) {
>> >+ if ((port->sm_vars & AD_PORT_SELECTED) &&
>> >+ (port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) &&
>> >+ (port->partner_oper.port_state & LACP_STATE_COLLECTING)) {
>> >+ port->sm_mux_state = AD_MUX_DISTRIBUTING;
>> >+ } else {
>> >+ /* If port state hasn't changed, make sure that a collecting
>> >+ * port is enabled for an active aggregator.
>> >+ */
>> >+ struct slave *slave = port->slave;
>> >+
>> >+ if (port->aggregator->is_active &&
>> >+ bond_is_slave_rx_disabled(slave)) {
>> >+ ad_enable_collecting(port);
>> >+ *update_slave_arr = true;
>> >+ }
>> >+ }
>> >+ }
>> >+ break;
>> >+ case AD_MUX_DISTRIBUTING:
>> > if (!(port->sm_vars & AD_PORT_SELECTED) ||
>> > (port->sm_vars & AD_PORT_STANDBY) ||
>> >+ !(port->partner_oper.port_state & LACP_STATE_COLLECTING) ||
>> > !(port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) ||
>> > !(port->actor_oper_port_state & LACP_STATE_SYNCHRONIZATION)) {
>> >- port->sm_mux_state = AD_MUX_ATTACHED;
>> >+ port->sm_mux_state = AD_MUX_COLLECTING;
>> > } else {
>> > /* if port state hasn't changed make
>> > * sure that a collecting distributing
>> >@@ -1031,7 +1121,7 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
>> > */
>> > if (port->aggregator &&
>> > port->aggregator->is_active &&
>> >- !__port_is_enabled(port)) {
>> >+ !__port_is_collecting_distributing(port)) {
>> > __enable_port(port);
>> > *update_slave_arr = true;
>> > }
>> >@@ -1082,6 +1172,20 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
>> > update_slave_arr);
>> > port->ntt = true;
>> > break;
>> >+ case AD_MUX_COLLECTING:
>> >+ port->actor_oper_port_state |= LACP_STATE_COLLECTING;
>> >+ port->actor_oper_port_state &= ~LACP_STATE_DISTRIBUTING;
>> >+ port->actor_oper_port_state |= LACP_STATE_SYNCHRONIZATION;
>> >+ ad_enable_collecting(port);
>> >+ ad_disable_distributing(port, update_slave_arr);
>> >+ port->ntt = true;
>> >+ break;
>> >+ case AD_MUX_DISTRIBUTING:
>> >+ port->actor_oper_port_state |= LACP_STATE_DISTRIBUTING;
>> >+ port->actor_oper_port_state |= LACP_STATE_SYNCHRONIZATION;
>> >+ ad_enable_collecting_distributing(port,
>> >+ update_slave_arr);
>> >+ break;
>> > default:
>> > break;
>> > }
>> >@@ -1906,6 +2010,45 @@ static void ad_initialize_port(struct port *port, int lacp_fast)
>> > }
>> > }
>> >
>> >+/**
>> >+ * ad_enable_collecting - enable a port's receive
>> >+ * @port: the port we're looking at
>> >+ *
>> >+ * Enable @port if it's in an active aggregator
>> >+ */
>> >+static void ad_enable_collecting(struct port *port)
>> >+{
>> >+ if (port->aggregator->is_active) {
>> >+ struct slave *slave = port->slave;
>> >+
>> >+ slave_dbg(slave->bond->dev, slave->dev,
>> >+ "Enabling collecting on port %d (LAG %d)\n",
>> >+ port->actor_port_number,
>> >+ port->aggregator->aggregator_identifier);
>> >+ __enable_collecting_port(port);
>> >+ }
>> >+}
>> >+
>> >+/**
>> >+ * ad_disable_distributing - disable a port's transmit
>> >+ * @port: the port we're looking at
>> >+ * @update_slave_arr: Does slave array need update?
>> >+ */
>> >+static void ad_disable_distributing(struct port *port, bool *update_slave_arr)
>> >+{
>> >+ if (port->aggregator &&
>> >+ !MAC_ADDRESS_EQUAL(&port->aggregator->partner_system,
>> >+ &(null_mac_addr))) {
>> >+ slave_dbg(port->slave->bond->dev, port->slave->dev,
>> >+ "Disabling distributing on port %d (LAG %d)\n",
>> >+ port->actor_port_number,
>> >+ port->aggregator->aggregator_identifier);
>> >+ __disable_distributing_port(port);
>> >+ /* Slave array needs an update */
>> >+ *update_slave_arr = true;
>> >+ }
>> >+}
>> >+
>> > /**
>> > * ad_enable_collecting_distributing - enable a port's transmit/receive
>> > * @port: the port we're looking at
>> >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> >index 8e6cc0e133b7..30f4b0ff01c0 100644
>> >--- a/drivers/net/bonding/bond_main.c
>> >+++ b/drivers/net/bonding/bond_main.c
>> >@@ -6331,6 +6331,7 @@ static int __init bond_check_params(struct bond_params *params)
>> > params->ad_actor_sys_prio = ad_actor_sys_prio;
>> > eth_zero_addr(params->ad_actor_system);
>> > params->ad_user_port_key = ad_user_port_key;
>> >+ params->coupled_control = 1;
>> > if (packets_per_slave > 0) {
>> > params->reciprocal_packets_per_slave =
>> > reciprocal_value(packets_per_slave);
>> >diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
>> >index cfa74cf8bb1a..29b4c3d1b9b6 100644
>> >--- a/drivers/net/bonding/bond_netlink.c
>> >+++ b/drivers/net/bonding/bond_netlink.c
>> >@@ -122,6 +122,7 @@ static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = {
>> > [IFLA_BOND_PEER_NOTIF_DELAY] = NLA_POLICY_FULL_RANGE(NLA_U32, &delay_range),
>> > [IFLA_BOND_MISSED_MAX] = { .type = NLA_U8 },
>> > [IFLA_BOND_NS_IP6_TARGET] = { .type = NLA_NESTED },
>> >+ [IFLA_BOND_COUPLED_CONTROL] = { .type = NLA_U8 },
>> > };
>> >
>> > static const struct nla_policy bond_slave_policy[IFLA_BOND_SLAVE_MAX + 1] = {
>> >@@ -549,6 +550,16 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
>> > return err;
>> > }
>> >
>> >+ if (data[IFLA_BOND_COUPLED_CONTROL]) {
>> >+ int coupled_control = nla_get_u8(data[IFLA_BOND_COUPLED_CONTROL]);
>> >+
>> >+ bond_opt_initval(&newval, coupled_control);
>> >+ err = __bond_opt_set(bond, BOND_OPT_COUPLED_CONTROL, &newval,
>> >+ data[IFLA_BOND_COUPLED_CONTROL], extack);
>> >+ if (err)
>> >+ return err;
>> >+ }
>> >+
>> > return 0;
>> > }
>> >
>> >@@ -615,6 +626,7 @@ static size_t bond_get_size(const struct net_device *bond_dev)
>> > /* IFLA_BOND_NS_IP6_TARGET */
>> > nla_total_size(sizeof(struct nlattr)) +
>> > nla_total_size(sizeof(struct in6_addr)) * BOND_MAX_NS_TARGETS +
>> >+ nla_total_size(sizeof(u8)) + /* IFLA_BOND_COUPLED_CONTROL */
>> > 0;
>> > }
>> >
>> >@@ -774,6 +786,10 @@ static int bond_fill_info(struct sk_buff *skb,
>> > bond->params.missed_max))
>> > goto nla_put_failure;
>> >
>> >+ if (nla_put_u8(skb, IFLA_BOND_COUPLED_CONTROL,
>> >+ bond->params.coupled_control))
>> >+ goto nla_put_failure;
>> >+
>> > if (BOND_MODE(bond) == BOND_MODE_8023AD) {
>> > struct ad_info info;
>> >
>> >diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>> >index f3f27f0bd2a6..4cdbc7e084f4 100644
>> >--- a/drivers/net/bonding/bond_options.c
>> >+++ b/drivers/net/bonding/bond_options.c
>> >@@ -84,7 +84,8 @@ static int bond_option_ad_user_port_key_set(struct bonding *bond,
>> > const struct bond_opt_value *newval);
>> > static int bond_option_missed_max_set(struct bonding *bond,
>> > const struct bond_opt_value *newval);
>> >-
>> >+static int bond_option_coupled_control_set(struct bonding *bond,
>> >+ const struct bond_opt_value *newval);
>> >
>> > static const struct bond_opt_value bond_mode_tbl[] = {
>> > { "balance-rr", BOND_MODE_ROUNDROBIN, BOND_VALFLAG_DEFAULT},
>> >@@ -232,6 +233,12 @@ static const struct bond_opt_value bond_missed_max_tbl[] = {
>> > { NULL, -1, 0},
>> > };
>> >
>> >+static const struct bond_opt_value bond_coupled_control_tbl[] = {
>> >+ { "on", 1, BOND_VALFLAG_DEFAULT},
>> >+ { "off", 0, 0},
>> >+ { NULL, -1, 0},
>> >+};
>> >+
>> > static const struct bond_option bond_opts[BOND_OPT_LAST] = {
>> > [BOND_OPT_MODE] = {
>> > .id = BOND_OPT_MODE,
>> >@@ -496,6 +503,15 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
>> > .desc = "Delay between each peer notification on failover event, in milliseconds",
>> > .values = bond_peer_notif_delay_tbl,
>> > .set = bond_option_peer_notif_delay_set
>> >+ },
>> >+ [BOND_OPT_COUPLED_CONTROL] = {
>> >+ .id = BOND_OPT_COUPLED_CONTROL,
>> >+ .name = "coupled_control",
>> >+ .desc = "Opt into using coupled control MUX for LACP states",
>> >+ .unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
>> >+ .flags = BOND_OPTFLAG_IFDOWN,
>> >+ .values = bond_coupled_control_tbl,
>> >+ .set = bond_option_coupled_control_set,
>> > }
>> > };
>> >
>> >@@ -1692,3 +1708,13 @@ static int bond_option_ad_user_port_key_set(struct bonding *bond,
>> > bond->params.ad_user_port_key = newval->value;
>> > return 0;
>> > }
>> >+
>> >+static int bond_option_coupled_control_set(struct bonding *bond,
>> >+ const struct bond_opt_value *newval)
>> >+{
>> >+ netdev_info(bond->dev, "Setting coupled_control to %s (%llu)\n",
>> >+ newval->string, newval->value);
>> >+
>> >+ bond->params.coupled_control = newval->value;
>> >+ return 0;
>> >+}
>> >diff --git a/include/net/bond_3ad.h b/include/net/bond_3ad.h
>> >index c5e57c6bd873..9ce5ac2bfbad 100644
>> >--- a/include/net/bond_3ad.h
>> >+++ b/include/net/bond_3ad.h
>> >@@ -54,6 +54,8 @@ typedef enum {
>> > AD_MUX_DETACHED, /* mux machine */
>> > AD_MUX_WAITING, /* mux machine */
>> > AD_MUX_ATTACHED, /* mux machine */
>> >+ AD_MUX_COLLECTING, /* mux machine */
>> >+ AD_MUX_DISTRIBUTING, /* mux machine */
>> > AD_MUX_COLLECTING_DISTRIBUTING /* mux machine */
>> > } mux_states_t;
>> >
>> >diff --git a/include/net/bond_options.h b/include/net/bond_options.h
>> >index 69292ecc0325..473a0147769e 100644
>> >--- a/include/net/bond_options.h
>> >+++ b/include/net/bond_options.h
>> >@@ -76,6 +76,7 @@ enum {
>> > BOND_OPT_MISSED_MAX,
>> > BOND_OPT_NS_TARGETS,
>> > BOND_OPT_PRIO,
>> >+ BOND_OPT_COUPLED_CONTROL,
>> > BOND_OPT_LAST
>> > };
>> >
>> >diff --git a/include/net/bonding.h b/include/net/bonding.h
>> >index 5b8b1b644a2d..b61fb1aa3a56 100644
>> >--- a/include/net/bonding.h
>> >+++ b/include/net/bonding.h
>> >@@ -148,6 +148,7 @@ struct bond_params {
>> > #if IS_ENABLED(CONFIG_IPV6)
>> > struct in6_addr ns_targets[BOND_MAX_NS_TARGETS];
>> > #endif
>> >+ int coupled_control;
>> >
>> > /* 2 bytes of padding : see ether_addr_equal_64bits() */
>> > u8 ad_actor_system[ETH_ALEN + 2];
>> >@@ -167,6 +168,7 @@ struct slave {
>> > u8 backup:1, /* indicates backup slave. Value corresponds with
>> > BOND_STATE_ACTIVE and BOND_STATE_BACKUP */
>> > inactive:1, /* indicates inactive slave */
>> >+ rx_disabled:1, /* indicates whether slave's Rx is disabled */
>> > should_notify:1, /* indicates whether the state changed */
>> > should_notify_link:1; /* indicates whether the link changed */
>> > u8 duplex;
>> >@@ -568,6 +570,14 @@ static inline void bond_set_slave_inactive_flags(struct slave *slave,
>> > bond_set_slave_state(slave, BOND_STATE_BACKUP, notify);
>> > if (!slave->bond->params.all_slaves_active)
>> > slave->inactive = 1;
>> >+ if (BOND_MODE(slave->bond) == BOND_MODE_8023AD)
>> >+ slave->rx_disabled = 1;
>> >+}
>> >+
>> >+static inline void bond_set_slave_tx_disabled_flags(struct slave *slave,
>> >+ bool notify)
>> >+{
>> >+ bond_set_slave_state(slave, BOND_STATE_BACKUP, notify);
>> > }
>> >
>> > static inline void bond_set_slave_active_flags(struct slave *slave,
>> >@@ -575,6 +585,14 @@ static inline void bond_set_slave_active_flags(struct slave *slave,
>> > {
>> > bond_set_slave_state(slave, BOND_STATE_ACTIVE, notify);
>> > slave->inactive = 0;
>> >+ if (BOND_MODE(slave->bond) == BOND_MODE_8023AD)
>> >+ slave->rx_disabled = 0;
>> >+}
>> >+
>> >+static inline void bond_set_slave_rx_enabled_flags(struct slave *slave,
>> >+ bool notify)
>> >+{
>> >+ slave->rx_disabled = 0;
>> > }
>> >
>> > static inline bool bond_is_slave_inactive(struct slave *slave)
>> >@@ -582,6 +600,11 @@ static inline bool bond_is_slave_inactive(struct slave *slave)
>> > return slave->inactive;
>> > }
>> >
>> >+static inline bool bond_is_slave_rx_disabled(struct slave *slave)
>> >+{
>> >+ return slave->rx_disabled;
>> >+}
>> >+
>> > static inline void bond_propose_link_state(struct slave *slave, int state)
>> > {
>> > slave->link_new_state = state;
>> >diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>> >index 29ff80da2775..7a54fcff2eec 100644
>> >--- a/include/uapi/linux/if_link.h
>> >+++ b/include/uapi/linux/if_link.h
>> >@@ -976,6 +976,7 @@ enum {
>> > IFLA_BOND_AD_LACP_ACTIVE,
>> > IFLA_BOND_MISSED_MAX,
>> > IFLA_BOND_NS_IP6_TARGET,
>> >+ IFLA_BOND_COUPLED_CONTROL,
>> > __IFLA_BOND_MAX,
>> > };
>> >
>> >diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h
>> >index a0aa05a28cf2..f0d71b2a3f1e 100644
>> >--- a/tools/include/uapi/linux/if_link.h
>> >+++ b/tools/include/uapi/linux/if_link.h
>> >@@ -974,6 +974,7 @@ enum {
>> > IFLA_BOND_AD_LACP_ACTIVE,
>> > IFLA_BOND_MISSED_MAX,
>> > IFLA_BOND_NS_IP6_TARGET,
>> >+ IFLA_BOND_COUPLED_CONTROL,
>> > __IFLA_BOND_MAX,
>> > };
>> >
>> >--
>> >2.43.0.429.g432eaa2c6b-goog
>> >
>> >
---
-Jay Vosburgh, [email protected]
> Could you post it now? They're logically parts of a single
> feature, so should go together. As a practical matter, though, it's
> easier to test this patch with the iproute patch available.
Sure, I have uploaded the patch for review, and also cc'd you on the email.
I also realised that I haven't appended coupled_control to procfs.
Will add that change in v6.
On Wed, Jan 24, 2024 at 10:16 AM Jay Vosburgh
<[email protected]> wrote:
>
> Aahil Awatramani <[email protected]> wrote:
>
> >> Do you have a corresponding iproute2 patch to enable changing
> >> the coupled_control option?
> >
> >I do have the corresponding patch for iproute2 to enable changing
> >coupled_control, I planned to push that patch for review directly
> >after this one.
>
> Could you post it now? They're logically parts of a single
> feature, so should go together. As a practical matter, though, it's
> easier to test this patch with the iproute patch available.
>
> -J
>
> >
> >On Tue, Jan 23, 2024 at 10:16 PM Jay Vosburgh
> ><[email protected]> wrote:
> >>
> >> Aahil Awatramani <[email protected]> wrote:
> >>
> >> >Add support for the independent control state machine per IEEE
> >> >802.1AX-2008 5.4.15 in addition to the existing implementation of the
> >> >coupled control state machine.
> >> >
> >> >Introduces two new states, AD_MUX_COLLECTING and AD_MUX_DISTRIBUTING in
> >> >the LACP MUX state machine for separated handling of an initial
> >> >Collecting state before the Collecting and Distributing state. This
> >> >enables a port to be in a state where it can receive incoming packets
> >> >while not still distributing. This is useful for reducing packet loss when
> >> >a port begins distributing before its partner is able to collect.
> >> >
> >> >Added new functions such as bond_set_slave_tx_disabled_flags and
> >> >bond_set_slave_rx_enabled_flags to precisely manage the port's collecting
> >> >and distributing states. Previously, there was no dedicated method to
> >> >disable TX while keeping RX enabled, which this patch addresses.
> >> >
> >> >Note that the regular flow process in the kernel's bonding driver remains
> >> >unaffected by this patch. The extension requires explicit opt-in by the
> >> >user (in order to ensure no disruptions for existing setups) via netlink
> >> >support using the new bonding parameter coupled_control. The default value
> >> >for coupled_control is set to 1 so as to preserve existing behaviour.
> >>
> >> Do you have a corresponding iproute2 patch to enable changing
> >> the coupled_control option?
> >>
> >> -J
> >>
> >> >Signed-off-by: Aahil Awatramani <[email protected]>
> >> >
> >> >v5:
> >> > Merge documentation patch with changes patch
> >> > Add version history in comment description
> >> >v4:
> >> > Remove inline references from c source files
> >> >v3:
> >> > Edited commit description
> >> > Edited documentation description
> >> > Changed function names
> >> > Only allow coupled_control change when the bond is down
> >> >v2:
> >> > Removed sysfs changes
> >> > Added documentation for new paramater
> >> > Renamed parameter to coupled_control
> >> > Update bond_set_slave_inactive_flags() with a 8023ad check
> >> >
> >> >---
> >> > Documentation/networking/bonding.rst | 12 ++
> >> > drivers/net/bonding/bond_3ad.c | 157 +++++++++++++++++++++++++--
> >> > drivers/net/bonding/bond_main.c | 1 +
> >> > drivers/net/bonding/bond_netlink.c | 16 +++
> >> > drivers/net/bonding/bond_options.c | 28 ++++-
> >> > include/net/bond_3ad.h | 2 +
> >> > include/net/bond_options.h | 1 +
> >> > include/net/bonding.h | 23 ++++
> >> > include/uapi/linux/if_link.h | 1 +
> >> > tools/include/uapi/linux/if_link.h | 1 +
> >> > 10 files changed, 234 insertions(+), 8 deletions(-)
> >> >
> >> >diff --git a/Documentation/networking/bonding.rst b/Documentation/networking/bonding.rst
> >> >index f7a73421eb76..e774b48de9f5 100644
> >> >--- a/Documentation/networking/bonding.rst
> >> >+++ b/Documentation/networking/bonding.rst
> >> >@@ -444,6 +444,18 @@ arp_missed_max
> >> >
> >> > The default value is 2, and the allowable range is 1 - 255.
> >> >
> >> >+coupled_control
> >> >+
> >> >+ Specifies whether the LACP state machine's MUX in the 802.3ad mode
> >> >+ should have separate Collecting and Distributing states.
> >> >+
> >> >+ This is by implementing the independent control state machine per
> >> >+ IEEE 802.1AX-2008 5.4.15 in addition to the existing coupled control
> >> >+ state machine.
> >> >+
> >> >+ The default value is 1. This setting does not separate the Collecting
> >> >+ and Distributing states, maintaining the bond in coupled control.
> >> >+
> >> > downdelay
> >> >
> >> > Specifies the time, in milliseconds, to wait before disabling
> >> >diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
> >> >index c99ffe6c683a..f2942e8c6c91 100644
> >> >--- a/drivers/net/bonding/bond_3ad.c
> >> >+++ b/drivers/net/bonding/bond_3ad.c
> >> >@@ -106,6 +106,9 @@ static void ad_agg_selection_logic(struct aggregator *aggregator,
> >> > static void ad_clear_agg(struct aggregator *aggregator);
> >> > static void ad_initialize_agg(struct aggregator *aggregator);
> >> > static void ad_initialize_port(struct port *port, int lacp_fast);
> >> >+static void ad_enable_collecting(struct port *port);
> >> >+static void ad_disable_distributing(struct port *port,
> >> >+ bool *update_slave_arr);
> >> > static void ad_enable_collecting_distributing(struct port *port,
> >> > bool *update_slave_arr);
> >> > static void ad_disable_collecting_distributing(struct port *port,
> >> >@@ -171,9 +174,38 @@ static inline int __agg_has_partner(struct aggregator *agg)
> >> > return !is_zero_ether_addr(agg->partner_system.mac_addr_value);
> >> > }
> >> >
> >> >+/**
> >> >+ * __disable_distributing_port - disable the port's slave for distributing.
> >> >+ * Port will still be able to collect.
> >> >+ * @port: the port we're looking at
> >> >+ *
> >> >+ * This will disable only distributing on the port's slave.
> >> >+ */
> >> >+static void __disable_distributing_port(struct port *port)
> >> >+{
> >> >+ bond_set_slave_tx_disabled_flags(port->slave, BOND_SLAVE_NOTIFY_LATER);
> >> >+}
> >> >+
> >> >+/**
> >> >+ * __enable_collecting_port - enable the port's slave for collecting,
> >> >+ * if it's up
> >> >+ * @port: the port we're looking at
> >> >+ *
> >> >+ * This will enable only collecting on the port's slave.
> >> >+ */
> >> >+static void __enable_collecting_port(struct port *port)
> >> >+{
> >> >+ struct slave *slave = port->slave;
> >> >+
> >> >+ if (slave->link == BOND_LINK_UP && bond_slave_is_up(slave))
> >> >+ bond_set_slave_rx_enabled_flags(slave, BOND_SLAVE_NOTIFY_LATER);
> >> >+}
> >> >+
> >> > /**
> >> > * __disable_port - disable the port's slave
> >> > * @port: the port we're looking at
> >> >+ *
> >> >+ * This will disable both collecting and distributing on the port's slave.
> >> > */
> >> > static inline void __disable_port(struct port *port)
> >> > {
> >> >@@ -183,6 +215,8 @@ static inline void __disable_port(struct port *port)
> >> > /**
> >> > * __enable_port - enable the port's slave, if it's up
> >> > * @port: the port we're looking at
> >> >+ *
> >> >+ * This will enable both collecting and distributing on the port's slave.
> >> > */
> >> > static inline void __enable_port(struct port *port)
> >> > {
> >> >@@ -193,10 +227,27 @@ static inline void __enable_port(struct port *port)
> >> > }
> >> >
> >> > /**
> >> >- * __port_is_enabled - check if the port's slave is in active state
> >> >+ * __port_move_to_attached_state - check if port should transition back to attached
> >> >+ * state.
> >> >+ * @port: the port we're looking at
> >> >+ */
> >> >+static bool __port_move_to_attached_state(struct port *port)
> >> >+{
> >> >+ if (!(port->sm_vars & AD_PORT_SELECTED) ||
> >> >+ (port->sm_vars & AD_PORT_STANDBY) ||
> >> >+ !(port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) ||
> >> >+ !(port->actor_oper_port_state & LACP_STATE_SYNCHRONIZATION))
> >> >+ port->sm_mux_state = AD_MUX_ATTACHED;
> >> >+
> >> >+ return port->sm_mux_state == AD_MUX_ATTACHED;
> >> >+}
> >> >+
> >> >+/**
> >> >+ * __port_is_collecting_distributing - check if the port's slave is in the
> >> >+ * combined collecting/distributing state
> >> > * @port: the port we're looking at
> >> > */
> >> >-static inline int __port_is_enabled(struct port *port)
> >> >+static int __port_is_collecting_distributing(struct port *port)
> >> > {
> >> > return bond_is_active_slave(port->slave);
> >> > }
> >> >@@ -942,6 +993,7 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
> >> > */
> >> > static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> >> > {
> >> >+ struct bonding *bond = __get_bond_by_port(port);
> >> > mux_states_t last_state;
> >> >
> >> > /* keep current State Machine state to compare later if it was
> >> >@@ -999,9 +1051,13 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> >> > if ((port->sm_vars & AD_PORT_SELECTED) &&
> >> > (port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) &&
> >> > !__check_agg_selection_timer(port)) {
> >> >- if (port->aggregator->is_active)
> >> >- port->sm_mux_state =
> >> >- AD_MUX_COLLECTING_DISTRIBUTING;
> >> >+ if (port->aggregator->is_active) {
> >> >+ int state = AD_MUX_COLLECTING_DISTRIBUTING;
> >> >+
> >> >+ if (!bond->params.coupled_control)
> >> >+ state = AD_MUX_COLLECTING;
> >> >+ port->sm_mux_state = state;
> >> >+ }
> >> > } else if (!(port->sm_vars & AD_PORT_SELECTED) ||
> >> > (port->sm_vars & AD_PORT_STANDBY)) {
> >> > /* if UNSELECTED or STANDBY */
> >> >@@ -1019,11 +1075,45 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> >> > }
> >> > break;
> >> > case AD_MUX_COLLECTING_DISTRIBUTING:
> >> >+ if (!__port_move_to_attached_state(port)) {
> >> >+ /* if port state hasn't changed make
> >> >+ * sure that a collecting distributing
> >> >+ * port in an active aggregator is enabled
> >> >+ */
> >> >+ if (port->aggregator->is_active &&
> >> >+ !__port_is_collecting_distributing(port)) {
> >> >+ __enable_port(port);
> >> >+ *update_slave_arr = true;
> >> >+ }
> >> >+ }
> >> >+ break;
> >> >+ case AD_MUX_COLLECTING:
> >> >+ if (!__port_move_to_attached_state(port)) {
> >> >+ if ((port->sm_vars & AD_PORT_SELECTED) &&
> >> >+ (port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) &&
> >> >+ (port->partner_oper.port_state & LACP_STATE_COLLECTING)) {
> >> >+ port->sm_mux_state = AD_MUX_DISTRIBUTING;
> >> >+ } else {
> >> >+ /* If port state hasn't changed, make sure that a collecting
> >> >+ * port is enabled for an active aggregator.
> >> >+ */
> >> >+ struct slave *slave = port->slave;
> >> >+
> >> >+ if (port->aggregator->is_active &&
> >> >+ bond_is_slave_rx_disabled(slave)) {
> >> >+ ad_enable_collecting(port);
> >> >+ *update_slave_arr = true;
> >> >+ }
> >> >+ }
> >> >+ }
> >> >+ break;
> >> >+ case AD_MUX_DISTRIBUTING:
> >> > if (!(port->sm_vars & AD_PORT_SELECTED) ||
> >> > (port->sm_vars & AD_PORT_STANDBY) ||
> >> >+ !(port->partner_oper.port_state & LACP_STATE_COLLECTING) ||
> >> > !(port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) ||
> >> > !(port->actor_oper_port_state & LACP_STATE_SYNCHRONIZATION)) {
> >> >- port->sm_mux_state = AD_MUX_ATTACHED;
> >> >+ port->sm_mux_state = AD_MUX_COLLECTING;
> >> > } else {
> >> > /* if port state hasn't changed make
> >> > * sure that a collecting distributing
> >> >@@ -1031,7 +1121,7 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> >> > */
> >> > if (port->aggregator &&
> >> > port->aggregator->is_active &&
> >> >- !__port_is_enabled(port)) {
> >> >+ !__port_is_collecting_distributing(port)) {
> >> > __enable_port(port);
> >> > *update_slave_arr = true;
> >> > }
> >> >@@ -1082,6 +1172,20 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> >> > update_slave_arr);
> >> > port->ntt = true;
> >> > break;
> >> >+ case AD_MUX_COLLECTING:
> >> >+ port->actor_oper_port_state |= LACP_STATE_COLLECTING;
> >> >+ port->actor_oper_port_state &= ~LACP_STATE_DISTRIBUTING;
> >> >+ port->actor_oper_port_state |= LACP_STATE_SYNCHRONIZATION;
> >> >+ ad_enable_collecting(port);
> >> >+ ad_disable_distributing(port, update_slave_arr);
> >> >+ port->ntt = true;
> >> >+ break;
> >> >+ case AD_MUX_DISTRIBUTING:
> >> >+ port->actor_oper_port_state |= LACP_STATE_DISTRIBUTING;
> >> >+ port->actor_oper_port_state |= LACP_STATE_SYNCHRONIZATION;
> >> >+ ad_enable_collecting_distributing(port,
> >> >+ update_slave_arr);
> >> >+ break;
> >> > default:
> >> > break;
> >> > }
> >> >@@ -1906,6 +2010,45 @@ static void ad_initialize_port(struct port *port, int lacp_fast)
> >> > }
> >> > }
> >> >
> >> >+/**
> >> >+ * ad_enable_collecting - enable a port's receive
> >> >+ * @port: the port we're looking at
> >> >+ *
> >> >+ * Enable @port if it's in an active aggregator
> >> >+ */
> >> >+static void ad_enable_collecting(struct port *port)
> >> >+{
> >> >+ if (port->aggregator->is_active) {
> >> >+ struct slave *slave = port->slave;
> >> >+
> >> >+ slave_dbg(slave->bond->dev, slave->dev,
> >> >+ "Enabling collecting on port %d (LAG %d)\n",
> >> >+ port->actor_port_number,
> >> >+ port->aggregator->aggregator_identifier);
> >> >+ __enable_collecting_port(port);
> >> >+ }
> >> >+}
> >> >+
> >> >+/**
> >> >+ * ad_disable_distributing - disable a port's transmit
> >> >+ * @port: the port we're looking at
> >> >+ * @update_slave_arr: Does slave array need update?
> >> >+ */
> >> >+static void ad_disable_distributing(struct port *port, bool *update_slave_arr)
> >> >+{
> >> >+ if (port->aggregator &&
> >> >+ !MAC_ADDRESS_EQUAL(&port->aggregator->partner_system,
> >> >+ &(null_mac_addr))) {
> >> >+ slave_dbg(port->slave->bond->dev, port->slave->dev,
> >> >+ "Disabling distributing on port %d (LAG %d)\n",
> >> >+ port->actor_port_number,
> >> >+ port->aggregator->aggregator_identifier);
> >> >+ __disable_distributing_port(port);
> >> >+ /* Slave array needs an update */
> >> >+ *update_slave_arr = true;
> >> >+ }
> >> >+}
> >> >+
> >> > /**
> >> > * ad_enable_collecting_distributing - enable a port's transmit/receive
> >> > * @port: the port we're looking at
> >> >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> >> >index 8e6cc0e133b7..30f4b0ff01c0 100644
> >> >--- a/drivers/net/bonding/bond_main.c
> >> >+++ b/drivers/net/bonding/bond_main.c
> >> >@@ -6331,6 +6331,7 @@ static int __init bond_check_params(struct bond_params *params)
> >> > params->ad_actor_sys_prio = ad_actor_sys_prio;
> >> > eth_zero_addr(params->ad_actor_system);
> >> > params->ad_user_port_key = ad_user_port_key;
> >> >+ params->coupled_control = 1;
> >> > if (packets_per_slave > 0) {
> >> > params->reciprocal_packets_per_slave =
> >> > reciprocal_value(packets_per_slave);
> >> >diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
> >> >index cfa74cf8bb1a..29b4c3d1b9b6 100644
> >> >--- a/drivers/net/bonding/bond_netlink.c
> >> >+++ b/drivers/net/bonding/bond_netlink.c
> >> >@@ -122,6 +122,7 @@ static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = {
> >> > [IFLA_BOND_PEER_NOTIF_DELAY] = NLA_POLICY_FULL_RANGE(NLA_U32, &delay_range),
> >> > [IFLA_BOND_MISSED_MAX] = { .type = NLA_U8 },
> >> > [IFLA_BOND_NS_IP6_TARGET] = { .type = NLA_NESTED },
> >> >+ [IFLA_BOND_COUPLED_CONTROL] = { .type = NLA_U8 },
> >> > };
> >> >
> >> > static const struct nla_policy bond_slave_policy[IFLA_BOND_SLAVE_MAX + 1] = {
> >> >@@ -549,6 +550,16 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> >> > return err;
> >> > }
> >> >
> >> >+ if (data[IFLA_BOND_COUPLED_CONTROL]) {
> >> >+ int coupled_control = nla_get_u8(data[IFLA_BOND_COUPLED_CONTROL]);
> >> >+
> >> >+ bond_opt_initval(&newval, coupled_control);
> >> >+ err = __bond_opt_set(bond, BOND_OPT_COUPLED_CONTROL, &newval,
> >> >+ data[IFLA_BOND_COUPLED_CONTROL], extack);
> >> >+ if (err)
> >> >+ return err;
> >> >+ }
> >> >+
> >> > return 0;
> >> > }
> >> >
> >> >@@ -615,6 +626,7 @@ static size_t bond_get_size(const struct net_device *bond_dev)
> >> > /* IFLA_BOND_NS_IP6_TARGET */
> >> > nla_total_size(sizeof(struct nlattr)) +
> >> > nla_total_size(sizeof(struct in6_addr)) * BOND_MAX_NS_TARGETS +
> >> >+ nla_total_size(sizeof(u8)) + /* IFLA_BOND_COUPLED_CONTROL */
> >> > 0;
> >> > }
> >> >
> >> >@@ -774,6 +786,10 @@ static int bond_fill_info(struct sk_buff *skb,
> >> > bond->params.missed_max))
> >> > goto nla_put_failure;
> >> >
> >> >+ if (nla_put_u8(skb, IFLA_BOND_COUPLED_CONTROL,
> >> >+ bond->params.coupled_control))
> >> >+ goto nla_put_failure;
> >> >+
> >> > if (BOND_MODE(bond) == BOND_MODE_8023AD) {
> >> > struct ad_info info;
> >> >
> >> >diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
> >> >index f3f27f0bd2a6..4cdbc7e084f4 100644
> >> >--- a/drivers/net/bonding/bond_options.c
> >> >+++ b/drivers/net/bonding/bond_options.c
> >> >@@ -84,7 +84,8 @@ static int bond_option_ad_user_port_key_set(struct bonding *bond,
> >> > const struct bond_opt_value *newval);
> >> > static int bond_option_missed_max_set(struct bonding *bond,
> >> > const struct bond_opt_value *newval);
> >> >-
> >> >+static int bond_option_coupled_control_set(struct bonding *bond,
> >> >+ const struct bond_opt_value *newval);
> >> >
> >> > static const struct bond_opt_value bond_mode_tbl[] = {
> >> > { "balance-rr", BOND_MODE_ROUNDROBIN, BOND_VALFLAG_DEFAULT},
> >> >@@ -232,6 +233,12 @@ static const struct bond_opt_value bond_missed_max_tbl[] = {
> >> > { NULL, -1, 0},
> >> > };
> >> >
> >> >+static const struct bond_opt_value bond_coupled_control_tbl[] = {
> >> >+ { "on", 1, BOND_VALFLAG_DEFAULT},
> >> >+ { "off", 0, 0},
> >> >+ { NULL, -1, 0},
> >> >+};
> >> >+
> >> > static const struct bond_option bond_opts[BOND_OPT_LAST] = {
> >> > [BOND_OPT_MODE] = {
> >> > .id = BOND_OPT_MODE,
> >> >@@ -496,6 +503,15 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
> >> > .desc = "Delay between each peer notification on failover event, in milliseconds",
> >> > .values = bond_peer_notif_delay_tbl,
> >> > .set = bond_option_peer_notif_delay_set
> >> >+ },
> >> >+ [BOND_OPT_COUPLED_CONTROL] = {
> >> >+ .id = BOND_OPT_COUPLED_CONTROL,
> >> >+ .name = "coupled_control",
> >> >+ .desc = "Opt into using coupled control MUX for LACP states",
> >> >+ .unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
> >> >+ .flags = BOND_OPTFLAG_IFDOWN,
> >> >+ .values = bond_coupled_control_tbl,
> >> >+ .set = bond_option_coupled_control_set,
> >> > }
> >> > };
> >> >
> >> >@@ -1692,3 +1708,13 @@ static int bond_option_ad_user_port_key_set(struct bonding *bond,
> >> > bond->params.ad_user_port_key = newval->value;
> >> > return 0;
> >> > }
> >> >+
> >> >+static int bond_option_coupled_control_set(struct bonding *bond,
> >> >+ const struct bond_opt_value *newval)
> >> >+{
> >> >+ netdev_info(bond->dev, "Setting coupled_control to %s (%llu)\n",
> >> >+ newval->string, newval->value);
> >> >+
> >> >+ bond->params.coupled_control = newval->value;
> >> >+ return 0;
> >> >+}
> >> >diff --git a/include/net/bond_3ad.h b/include/net/bond_3ad.h
> >> >index c5e57c6bd873..9ce5ac2bfbad 100644
> >> >--- a/include/net/bond_3ad.h
> >> >+++ b/include/net/bond_3ad.h
> >> >@@ -54,6 +54,8 @@ typedef enum {
> >> > AD_MUX_DETACHED, /* mux machine */
> >> > AD_MUX_WAITING, /* mux machine */
> >> > AD_MUX_ATTACHED, /* mux machine */
> >> >+ AD_MUX_COLLECTING, /* mux machine */
> >> >+ AD_MUX_DISTRIBUTING, /* mux machine */
> >> > AD_MUX_COLLECTING_DISTRIBUTING /* mux machine */
> >> > } mux_states_t;
> >> >
> >> >diff --git a/include/net/bond_options.h b/include/net/bond_options.h
> >> >index 69292ecc0325..473a0147769e 100644
> >> >--- a/include/net/bond_options.h
> >> >+++ b/include/net/bond_options.h
> >> >@@ -76,6 +76,7 @@ enum {
> >> > BOND_OPT_MISSED_MAX,
> >> > BOND_OPT_NS_TARGETS,
> >> > BOND_OPT_PRIO,
> >> >+ BOND_OPT_COUPLED_CONTROL,
> >> > BOND_OPT_LAST
> >> > };
> >> >
> >> >diff --git a/include/net/bonding.h b/include/net/bonding.h
> >> >index 5b8b1b644a2d..b61fb1aa3a56 100644
> >> >--- a/include/net/bonding.h
> >> >+++ b/include/net/bonding.h
> >> >@@ -148,6 +148,7 @@ struct bond_params {
> >> > #if IS_ENABLED(CONFIG_IPV6)
> >> > struct in6_addr ns_targets[BOND_MAX_NS_TARGETS];
> >> > #endif
> >> >+ int coupled_control;
> >> >
> >> > /* 2 bytes of padding : see ether_addr_equal_64bits() */
> >> > u8 ad_actor_system[ETH_ALEN + 2];
> >> >@@ -167,6 +168,7 @@ struct slave {
> >> > u8 backup:1, /* indicates backup slave. Value corresponds with
> >> > BOND_STATE_ACTIVE and BOND_STATE_BACKUP */
> >> > inactive:1, /* indicates inactive slave */
> >> >+ rx_disabled:1, /* indicates whether slave's Rx is disabled */
> >> > should_notify:1, /* indicates whether the state changed */
> >> > should_notify_link:1; /* indicates whether the link changed */
> >> > u8 duplex;
> >> >@@ -568,6 +570,14 @@ static inline void bond_set_slave_inactive_flags(struct slave *slave,
> >> > bond_set_slave_state(slave, BOND_STATE_BACKUP, notify);
> >> > if (!slave->bond->params.all_slaves_active)
> >> > slave->inactive = 1;
> >> >+ if (BOND_MODE(slave->bond) == BOND_MODE_8023AD)
> >> >+ slave->rx_disabled = 1;
> >> >+}
> >> >+
> >> >+static inline void bond_set_slave_tx_disabled_flags(struct slave *slave,
> >> >+ bool notify)
> >> >+{
> >> >+ bond_set_slave_state(slave, BOND_STATE_BACKUP, notify);
> >> > }
> >> >
> >> > static inline void bond_set_slave_active_flags(struct slave *slave,
> >> >@@ -575,6 +585,14 @@ static inline void bond_set_slave_active_flags(struct slave *slave,
> >> > {
> >> > bond_set_slave_state(slave, BOND_STATE_ACTIVE, notify);
> >> > slave->inactive = 0;
> >> >+ if (BOND_MODE(slave->bond) == BOND_MODE_8023AD)
> >> >+ slave->rx_disabled = 0;
> >> >+}
> >> >+
> >> >+static inline void bond_set_slave_rx_enabled_flags(struct slave *slave,
> >> >+ bool notify)
> >> >+{
> >> >+ slave->rx_disabled = 0;
> >> > }
> >> >
> >> > static inline bool bond_is_slave_inactive(struct slave *slave)
> >> >@@ -582,6 +600,11 @@ static inline bool bond_is_slave_inactive(struct slave *slave)
> >> > return slave->inactive;
> >> > }
> >> >
> >> >+static inline bool bond_is_slave_rx_disabled(struct slave *slave)
> >> >+{
> >> >+ return slave->rx_disabled;
> >> >+}
> >> >+
> >> > static inline void bond_propose_link_state(struct slave *slave, int state)
> >> > {
> >> > slave->link_new_state = state;
> >> >diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> >> >index 29ff80da2775..7a54fcff2eec 100644
> >> >--- a/include/uapi/linux/if_link.h
> >> >+++ b/include/uapi/linux/if_link.h
> >> >@@ -976,6 +976,7 @@ enum {
> >> > IFLA_BOND_AD_LACP_ACTIVE,
> >> > IFLA_BOND_MISSED_MAX,
> >> > IFLA_BOND_NS_IP6_TARGET,
> >> >+ IFLA_BOND_COUPLED_CONTROL,
> >> > __IFLA_BOND_MAX,
> >> > };
> >> >
> >> >diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h
> >> >index a0aa05a28cf2..f0d71b2a3f1e 100644
> >> >--- a/tools/include/uapi/linux/if_link.h
> >> >+++ b/tools/include/uapi/linux/if_link.h
> >> >@@ -974,6 +974,7 @@ enum {
> >> > IFLA_BOND_AD_LACP_ACTIVE,
> >> > IFLA_BOND_MISSED_MAX,
> >> > IFLA_BOND_NS_IP6_TARGET,
> >> >+ IFLA_BOND_COUPLED_CONTROL,
> >> > __IFLA_BOND_MAX,
> >> > };
> >> >
> >> >--
> >> >2.43.0.429.g432eaa2c6b-goog
> >> >
> >> >
>
> ---
> -Jay Vosburgh, [email protected]
> I also realised that I haven't appended coupled_control to procfs.
To clarify, I will hold off on adding coupled_control to procfs until requested.
Thanks,
Aahil
On Wed, Jan 24, 2024 at 4:38 PM Aahil Awatramani <[email protected]> wrote:
>
> > Could you post it now? They're logically parts of a single
> > feature, so should go together. As a practical matter, though, it's
> > easier to test this patch with the iproute patch available.
>
> Sure, I have uploaded the patch for review, and also cc'd you on the email.
> I also realised that I haven't appended coupled_control to procfs.
> Will add that change in v6.
>
> On Wed, Jan 24, 2024 at 10:16 AM Jay Vosburgh
> <[email protected]> wrote:
> >
> > Aahil Awatramani <[email protected]> wrote:
> >
> > >> Do you have a corresponding iproute2 patch to enable changing
> > >> the coupled_control option?
> > >
> > >I do have the corresponding patch for iproute2 to enable changing
> > >coupled_control, I planned to push that patch for review directly
> > >after this one.
> >
> > Could you post it now? They're logically parts of a single
> > feature, so should go together. As a practical matter, though, it's
> > easier to test this patch with the iproute patch available.
> >
> > -J
> >
> > >
> > >On Tue, Jan 23, 2024 at 10:16 PM Jay Vosburgh
> > ><[email protected]> wrote:
> > >>
> > >> Aahil Awatramani <[email protected]> wrote:
> > >>
> > >> >Add support for the independent control state machine per IEEE
> > >> >802.1AX-2008 5.4.15 in addition to the existing implementation of the
> > >> >coupled control state machine.
> > >> >
> > >> >Introduces two new states, AD_MUX_COLLECTING and AD_MUX_DISTRIBUTING in
> > >> >the LACP MUX state machine for separated handling of an initial
> > >> >Collecting state before the Collecting and Distributing state. This
> > >> >enables a port to be in a state where it can receive incoming packets
> > >> >while not still distributing. This is useful for reducing packet loss when
> > >> >a port begins distributing before its partner is able to collect.
> > >> >
> > >> >Added new functions such as bond_set_slave_tx_disabled_flags and
> > >> >bond_set_slave_rx_enabled_flags to precisely manage the port's collecting
> > >> >and distributing states. Previously, there was no dedicated method to
> > >> >disable TX while keeping RX enabled, which this patch addresses.
> > >> >
> > >> >Note that the regular flow process in the kernel's bonding driver remains
> > >> >unaffected by this patch. The extension requires explicit opt-in by the
> > >> >user (in order to ensure no disruptions for existing setups) via netlink
> > >> >support using the new bonding parameter coupled_control. The default value
> > >> >for coupled_control is set to 1 so as to preserve existing behaviour.
> > >>
> > >> Do you have a corresponding iproute2 patch to enable changing
> > >> the coupled_control option?
> > >>
> > >> -J
> > >>
> > >> >Signed-off-by: Aahil Awatramani <[email protected]>
> > >> >
> > >> >v5:
> > >> > Merge documentation patch with changes patch
> > >> > Add version history in comment description
> > >> >v4:
> > >> > Remove inline references from c source files
> > >> >v3:
> > >> > Edited commit description
> > >> > Edited documentation description
> > >> > Changed function names
> > >> > Only allow coupled_control change when the bond is down
> > >> >v2:
> > >> > Removed sysfs changes
> > >> > Added documentation for new paramater
> > >> > Renamed parameter to coupled_control
> > >> > Update bond_set_slave_inactive_flags() with a 8023ad check
> > >> >
> > >> >---
> > >> > Documentation/networking/bonding.rst | 12 ++
> > >> > drivers/net/bonding/bond_3ad.c | 157 +++++++++++++++++++++++++--
> > >> > drivers/net/bonding/bond_main.c | 1 +
> > >> > drivers/net/bonding/bond_netlink.c | 16 +++
> > >> > drivers/net/bonding/bond_options.c | 28 ++++-
> > >> > include/net/bond_3ad.h | 2 +
> > >> > include/net/bond_options.h | 1 +
> > >> > include/net/bonding.h | 23 ++++
> > >> > include/uapi/linux/if_link.h | 1 +
> > >> > tools/include/uapi/linux/if_link.h | 1 +
> > >> > 10 files changed, 234 insertions(+), 8 deletions(-)
> > >> >
> > >> >diff --git a/Documentation/networking/bonding.rst b/Documentation/networking/bonding.rst
> > >> >index f7a73421eb76..e774b48de9f5 100644
> > >> >--- a/Documentation/networking/bonding.rst
> > >> >+++ b/Documentation/networking/bonding.rst
> > >> >@@ -444,6 +444,18 @@ arp_missed_max
> > >> >
> > >> > The default value is 2, and the allowable range is 1 - 255.
> > >> >
> > >> >+coupled_control
> > >> >+
> > >> >+ Specifies whether the LACP state machine's MUX in the 802.3ad mode
> > >> >+ should have separate Collecting and Distributing states.
> > >> >+
> > >> >+ This is by implementing the independent control state machine per
> > >> >+ IEEE 802.1AX-2008 5.4.15 in addition to the existing coupled control
> > >> >+ state machine.
> > >> >+
> > >> >+ The default value is 1. This setting does not separate the Collecting
> > >> >+ and Distributing states, maintaining the bond in coupled control.
> > >> >+
> > >> > downdelay
> > >> >
> > >> > Specifies the time, in milliseconds, to wait before disabling
> > >> >diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
> > >> >index c99ffe6c683a..f2942e8c6c91 100644
> > >> >--- a/drivers/net/bonding/bond_3ad.c
> > >> >+++ b/drivers/net/bonding/bond_3ad.c
> > >> >@@ -106,6 +106,9 @@ static void ad_agg_selection_logic(struct aggregator *aggregator,
> > >> > static void ad_clear_agg(struct aggregator *aggregator);
> > >> > static void ad_initialize_agg(struct aggregator *aggregator);
> > >> > static void ad_initialize_port(struct port *port, int lacp_fast);
> > >> >+static void ad_enable_collecting(struct port *port);
> > >> >+static void ad_disable_distributing(struct port *port,
> > >> >+ bool *update_slave_arr);
> > >> > static void ad_enable_collecting_distributing(struct port *port,
> > >> > bool *update_slave_arr);
> > >> > static void ad_disable_collecting_distributing(struct port *port,
> > >> >@@ -171,9 +174,38 @@ static inline int __agg_has_partner(struct aggregator *agg)
> > >> > return !is_zero_ether_addr(agg->partner_system.mac_addr_value);
> > >> > }
> > >> >
> > >> >+/**
> > >> >+ * __disable_distributing_port - disable the port's slave for distributing.
> > >> >+ * Port will still be able to collect.
> > >> >+ * @port: the port we're looking at
> > >> >+ *
> > >> >+ * This will disable only distributing on the port's slave.
> > >> >+ */
> > >> >+static void __disable_distributing_port(struct port *port)
> > >> >+{
> > >> >+ bond_set_slave_tx_disabled_flags(port->slave, BOND_SLAVE_NOTIFY_LATER);
> > >> >+}
> > >> >+
> > >> >+/**
> > >> >+ * __enable_collecting_port - enable the port's slave for collecting,
> > >> >+ * if it's up
> > >> >+ * @port: the port we're looking at
> > >> >+ *
> > >> >+ * This will enable only collecting on the port's slave.
> > >> >+ */
> > >> >+static void __enable_collecting_port(struct port *port)
> > >> >+{
> > >> >+ struct slave *slave = port->slave;
> > >> >+
> > >> >+ if (slave->link == BOND_LINK_UP && bond_slave_is_up(slave))
> > >> >+ bond_set_slave_rx_enabled_flags(slave, BOND_SLAVE_NOTIFY_LATER);
> > >> >+}
> > >> >+
> > >> > /**
> > >> > * __disable_port - disable the port's slave
> > >> > * @port: the port we're looking at
> > >> >+ *
> > >> >+ * This will disable both collecting and distributing on the port's slave.
> > >> > */
> > >> > static inline void __disable_port(struct port *port)
> > >> > {
> > >> >@@ -183,6 +215,8 @@ static inline void __disable_port(struct port *port)
> > >> > /**
> > >> > * __enable_port - enable the port's slave, if it's up
> > >> > * @port: the port we're looking at
> > >> >+ *
> > >> >+ * This will enable both collecting and distributing on the port's slave.
> > >> > */
> > >> > static inline void __enable_port(struct port *port)
> > >> > {
> > >> >@@ -193,10 +227,27 @@ static inline void __enable_port(struct port *port)
> > >> > }
> > >> >
> > >> > /**
> > >> >- * __port_is_enabled - check if the port's slave is in active state
> > >> >+ * __port_move_to_attached_state - check if port should transition back to attached
> > >> >+ * state.
> > >> >+ * @port: the port we're looking at
> > >> >+ */
> > >> >+static bool __port_move_to_attached_state(struct port *port)
> > >> >+{
> > >> >+ if (!(port->sm_vars & AD_PORT_SELECTED) ||
> > >> >+ (port->sm_vars & AD_PORT_STANDBY) ||
> > >> >+ !(port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) ||
> > >> >+ !(port->actor_oper_port_state & LACP_STATE_SYNCHRONIZATION))
> > >> >+ port->sm_mux_state = AD_MUX_ATTACHED;
> > >> >+
> > >> >+ return port->sm_mux_state == AD_MUX_ATTACHED;
> > >> >+}
> > >> >+
> > >> >+/**
> > >> >+ * __port_is_collecting_distributing - check if the port's slave is in the
> > >> >+ * combined collecting/distributing state
> > >> > * @port: the port we're looking at
> > >> > */
> > >> >-static inline int __port_is_enabled(struct port *port)
> > >> >+static int __port_is_collecting_distributing(struct port *port)
> > >> > {
> > >> > return bond_is_active_slave(port->slave);
> > >> > }
> > >> >@@ -942,6 +993,7 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
> > >> > */
> > >> > static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> > >> > {
> > >> >+ struct bonding *bond = __get_bond_by_port(port);
> > >> > mux_states_t last_state;
> > >> >
> > >> > /* keep current State Machine state to compare later if it was
> > >> >@@ -999,9 +1051,13 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> > >> > if ((port->sm_vars & AD_PORT_SELECTED) &&
> > >> > (port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) &&
> > >> > !__check_agg_selection_timer(port)) {
> > >> >- if (port->aggregator->is_active)
> > >> >- port->sm_mux_state =
> > >> >- AD_MUX_COLLECTING_DISTRIBUTING;
> > >> >+ if (port->aggregator->is_active) {
> > >> >+ int state = AD_MUX_COLLECTING_DISTRIBUTING;
> > >> >+
> > >> >+ if (!bond->params.coupled_control)
> > >> >+ state = AD_MUX_COLLECTING;
> > >> >+ port->sm_mux_state = state;
> > >> >+ }
> > >> > } else if (!(port->sm_vars & AD_PORT_SELECTED) ||
> > >> > (port->sm_vars & AD_PORT_STANDBY)) {
> > >> > /* if UNSELECTED or STANDBY */
> > >> >@@ -1019,11 +1075,45 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> > >> > }
> > >> > break;
> > >> > case AD_MUX_COLLECTING_DISTRIBUTING:
> > >> >+ if (!__port_move_to_attached_state(port)) {
> > >> >+ /* if port state hasn't changed make
> > >> >+ * sure that a collecting distributing
> > >> >+ * port in an active aggregator is enabled
> > >> >+ */
> > >> >+ if (port->aggregator->is_active &&
> > >> >+ !__port_is_collecting_distributing(port)) {
> > >> >+ __enable_port(port);
> > >> >+ *update_slave_arr = true;
> > >> >+ }
> > >> >+ }
> > >> >+ break;
> > >> >+ case AD_MUX_COLLECTING:
> > >> >+ if (!__port_move_to_attached_state(port)) {
> > >> >+ if ((port->sm_vars & AD_PORT_SELECTED) &&
> > >> >+ (port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) &&
> > >> >+ (port->partner_oper.port_state & LACP_STATE_COLLECTING)) {
> > >> >+ port->sm_mux_state = AD_MUX_DISTRIBUTING;
> > >> >+ } else {
> > >> >+ /* If port state hasn't changed, make sure that a collecting
> > >> >+ * port is enabled for an active aggregator.
> > >> >+ */
> > >> >+ struct slave *slave = port->slave;
> > >> >+
> > >> >+ if (port->aggregator->is_active &&
> > >> >+ bond_is_slave_rx_disabled(slave)) {
> > >> >+ ad_enable_collecting(port);
> > >> >+ *update_slave_arr = true;
> > >> >+ }
> > >> >+ }
> > >> >+ }
> > >> >+ break;
> > >> >+ case AD_MUX_DISTRIBUTING:
> > >> > if (!(port->sm_vars & AD_PORT_SELECTED) ||
> > >> > (port->sm_vars & AD_PORT_STANDBY) ||
> > >> >+ !(port->partner_oper.port_state & LACP_STATE_COLLECTING) ||
> > >> > !(port->partner_oper.port_state & LACP_STATE_SYNCHRONIZATION) ||
> > >> > !(port->actor_oper_port_state & LACP_STATE_SYNCHRONIZATION)) {
> > >> >- port->sm_mux_state = AD_MUX_ATTACHED;
> > >> >+ port->sm_mux_state = AD_MUX_COLLECTING;
> > >> > } else {
> > >> > /* if port state hasn't changed make
> > >> > * sure that a collecting distributing
> > >> >@@ -1031,7 +1121,7 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> > >> > */
> > >> > if (port->aggregator &&
> > >> > port->aggregator->is_active &&
> > >> >- !__port_is_enabled(port)) {
> > >> >+ !__port_is_collecting_distributing(port)) {
> > >> > __enable_port(port);
> > >> > *update_slave_arr = true;
> > >> > }
> > >> >@@ -1082,6 +1172,20 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> > >> > update_slave_arr);
> > >> > port->ntt = true;
> > >> > break;
> > >> >+ case AD_MUX_COLLECTING:
> > >> >+ port->actor_oper_port_state |= LACP_STATE_COLLECTING;
> > >> >+ port->actor_oper_port_state &= ~LACP_STATE_DISTRIBUTING;
> > >> >+ port->actor_oper_port_state |= LACP_STATE_SYNCHRONIZATION;
> > >> >+ ad_enable_collecting(port);
> > >> >+ ad_disable_distributing(port, update_slave_arr);
> > >> >+ port->ntt = true;
> > >> >+ break;
> > >> >+ case AD_MUX_DISTRIBUTING:
> > >> >+ port->actor_oper_port_state |= LACP_STATE_DISTRIBUTING;
> > >> >+ port->actor_oper_port_state |= LACP_STATE_SYNCHRONIZATION;
> > >> >+ ad_enable_collecting_distributing(port,
> > >> >+ update_slave_arr);
> > >> >+ break;
> > >> > default:
> > >> > break;
> > >> > }
> > >> >@@ -1906,6 +2010,45 @@ static void ad_initialize_port(struct port *port, int lacp_fast)
> > >> > }
> > >> > }
> > >> >
> > >> >+/**
> > >> >+ * ad_enable_collecting - enable a port's receive
> > >> >+ * @port: the port we're looking at
> > >> >+ *
> > >> >+ * Enable @port if it's in an active aggregator
> > >> >+ */
> > >> >+static void ad_enable_collecting(struct port *port)
> > >> >+{
> > >> >+ if (port->aggregator->is_active) {
> > >> >+ struct slave *slave = port->slave;
> > >> >+
> > >> >+ slave_dbg(slave->bond->dev, slave->dev,
> > >> >+ "Enabling collecting on port %d (LAG %d)\n",
> > >> >+ port->actor_port_number,
> > >> >+ port->aggregator->aggregator_identifier);
> > >> >+ __enable_collecting_port(port);
> > >> >+ }
> > >> >+}
> > >> >+
> > >> >+/**
> > >> >+ * ad_disable_distributing - disable a port's transmit
> > >> >+ * @port: the port we're looking at
> > >> >+ * @update_slave_arr: Does slave array need update?
> > >> >+ */
> > >> >+static void ad_disable_distributing(struct port *port, bool *update_slave_arr)
> > >> >+{
> > >> >+ if (port->aggregator &&
> > >> >+ !MAC_ADDRESS_EQUAL(&port->aggregator->partner_system,
> > >> >+ &(null_mac_addr))) {
> > >> >+ slave_dbg(port->slave->bond->dev, port->slave->dev,
> > >> >+ "Disabling distributing on port %d (LAG %d)\n",
> > >> >+ port->actor_port_number,
> > >> >+ port->aggregator->aggregator_identifier);
> > >> >+ __disable_distributing_port(port);
> > >> >+ /* Slave array needs an update */
> > >> >+ *update_slave_arr = true;
> > >> >+ }
> > >> >+}
> > >> >+
> > >> > /**
> > >> > * ad_enable_collecting_distributing - enable a port's transmit/receive
> > >> > * @port: the port we're looking at
> > >> >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> > >> >index 8e6cc0e133b7..30f4b0ff01c0 100644
> > >> >--- a/drivers/net/bonding/bond_main.c
> > >> >+++ b/drivers/net/bonding/bond_main.c
> > >> >@@ -6331,6 +6331,7 @@ static int __init bond_check_params(struct bond_params *params)
> > >> > params->ad_actor_sys_prio = ad_actor_sys_prio;
> > >> > eth_zero_addr(params->ad_actor_system);
> > >> > params->ad_user_port_key = ad_user_port_key;
> > >> >+ params->coupled_control = 1;
> > >> > if (packets_per_slave > 0) {
> > >> > params->reciprocal_packets_per_slave =
> > >> > reciprocal_value(packets_per_slave);
> > >> >diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
> > >> >index cfa74cf8bb1a..29b4c3d1b9b6 100644
> > >> >--- a/drivers/net/bonding/bond_netlink.c
> > >> >+++ b/drivers/net/bonding/bond_netlink.c
> > >> >@@ -122,6 +122,7 @@ static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = {
> > >> > [IFLA_BOND_PEER_NOTIF_DELAY] = NLA_POLICY_FULL_RANGE(NLA_U32, &delay_range),
> > >> > [IFLA_BOND_MISSED_MAX] = { .type = NLA_U8 },
> > >> > [IFLA_BOND_NS_IP6_TARGET] = { .type = NLA_NESTED },
> > >> >+ [IFLA_BOND_COUPLED_CONTROL] = { .type = NLA_U8 },
> > >> > };
> > >> >
> > >> > static const struct nla_policy bond_slave_policy[IFLA_BOND_SLAVE_MAX + 1] = {
> > >> >@@ -549,6 +550,16 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> > >> > return err;
> > >> > }
> > >> >
> > >> >+ if (data[IFLA_BOND_COUPLED_CONTROL]) {
> > >> >+ int coupled_control = nla_get_u8(data[IFLA_BOND_COUPLED_CONTROL]);
> > >> >+
> > >> >+ bond_opt_initval(&newval, coupled_control);
> > >> >+ err = __bond_opt_set(bond, BOND_OPT_COUPLED_CONTROL, &newval,
> > >> >+ data[IFLA_BOND_COUPLED_CONTROL], extack);
> > >> >+ if (err)
> > >> >+ return err;
> > >> >+ }
> > >> >+
> > >> > return 0;
> > >> > }
> > >> >
> > >> >@@ -615,6 +626,7 @@ static size_t bond_get_size(const struct net_device *bond_dev)
> > >> > /* IFLA_BOND_NS_IP6_TARGET */
> > >> > nla_total_size(sizeof(struct nlattr)) +
> > >> > nla_total_size(sizeof(struct in6_addr)) * BOND_MAX_NS_TARGETS +
> > >> >+ nla_total_size(sizeof(u8)) + /* IFLA_BOND_COUPLED_CONTROL */
> > >> > 0;
> > >> > }
> > >> >
> > >> >@@ -774,6 +786,10 @@ static int bond_fill_info(struct sk_buff *skb,
> > >> > bond->params.missed_max))
> > >> > goto nla_put_failure;
> > >> >
> > >> >+ if (nla_put_u8(skb, IFLA_BOND_COUPLED_CONTROL,
> > >> >+ bond->params.coupled_control))
> > >> >+ goto nla_put_failure;
> > >> >+
> > >> > if (BOND_MODE(bond) == BOND_MODE_8023AD) {
> > >> > struct ad_info info;
> > >> >
> > >> >diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
> > >> >index f3f27f0bd2a6..4cdbc7e084f4 100644
> > >> >--- a/drivers/net/bonding/bond_options.c
> > >> >+++ b/drivers/net/bonding/bond_options.c
> > >> >@@ -84,7 +84,8 @@ static int bond_option_ad_user_port_key_set(struct bonding *bond,
> > >> > const struct bond_opt_value *newval);
> > >> > static int bond_option_missed_max_set(struct bonding *bond,
> > >> > const struct bond_opt_value *newval);
> > >> >-
> > >> >+static int bond_option_coupled_control_set(struct bonding *bond,
> > >> >+ const struct bond_opt_value *newval);
> > >> >
> > >> > static const struct bond_opt_value bond_mode_tbl[] = {
> > >> > { "balance-rr", BOND_MODE_ROUNDROBIN, BOND_VALFLAG_DEFAULT},
> > >> >@@ -232,6 +233,12 @@ static const struct bond_opt_value bond_missed_max_tbl[] = {
> > >> > { NULL, -1, 0},
> > >> > };
> > >> >
> > >> >+static const struct bond_opt_value bond_coupled_control_tbl[] = {
> > >> >+ { "on", 1, BOND_VALFLAG_DEFAULT},
> > >> >+ { "off", 0, 0},
> > >> >+ { NULL, -1, 0},
> > >> >+};
> > >> >+
> > >> > static const struct bond_option bond_opts[BOND_OPT_LAST] = {
> > >> > [BOND_OPT_MODE] = {
> > >> > .id = BOND_OPT_MODE,
> > >> >@@ -496,6 +503,15 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
> > >> > .desc = "Delay between each peer notification on failover event, in milliseconds",
> > >> > .values = bond_peer_notif_delay_tbl,
> > >> > .set = bond_option_peer_notif_delay_set
> > >> >+ },
> > >> >+ [BOND_OPT_COUPLED_CONTROL] = {
> > >> >+ .id = BOND_OPT_COUPLED_CONTROL,
> > >> >+ .name = "coupled_control",
> > >> >+ .desc = "Opt into using coupled control MUX for LACP states",
> > >> >+ .unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
> > >> >+ .flags = BOND_OPTFLAG_IFDOWN,
> > >> >+ .values = bond_coupled_control_tbl,
> > >> >+ .set = bond_option_coupled_control_set,
> > >> > }
> > >> > };
> > >> >
> > >> >@@ -1692,3 +1708,13 @@ static int bond_option_ad_user_port_key_set(struct bonding *bond,
> > >> > bond->params.ad_user_port_key = newval->value;
> > >> > return 0;
> > >> > }
> > >> >+
> > >> >+static int bond_option_coupled_control_set(struct bonding *bond,
> > >> >+ const struct bond_opt_value *newval)
> > >> >+{
> > >> >+ netdev_info(bond->dev, "Setting coupled_control to %s (%llu)\n",
> > >> >+ newval->string, newval->value);
> > >> >+
> > >> >+ bond->params.coupled_control = newval->value;
> > >> >+ return 0;
> > >> >+}
> > >> >diff --git a/include/net/bond_3ad.h b/include/net/bond_3ad.h
> > >> >index c5e57c6bd873..9ce5ac2bfbad 100644
> > >> >--- a/include/net/bond_3ad.h
> > >> >+++ b/include/net/bond_3ad.h
> > >> >@@ -54,6 +54,8 @@ typedef enum {
> > >> > AD_MUX_DETACHED, /* mux machine */
> > >> > AD_MUX_WAITING, /* mux machine */
> > >> > AD_MUX_ATTACHED, /* mux machine */
> > >> >+ AD_MUX_COLLECTING, /* mux machine */
> > >> >+ AD_MUX_DISTRIBUTING, /* mux machine */
> > >> > AD_MUX_COLLECTING_DISTRIBUTING /* mux machine */
> > >> > } mux_states_t;
> > >> >
> > >> >diff --git a/include/net/bond_options.h b/include/net/bond_options.h
> > >> >index 69292ecc0325..473a0147769e 100644
> > >> >--- a/include/net/bond_options.h
> > >> >+++ b/include/net/bond_options.h
> > >> >@@ -76,6 +76,7 @@ enum {
> > >> > BOND_OPT_MISSED_MAX,
> > >> > BOND_OPT_NS_TARGETS,
> > >> > BOND_OPT_PRIO,
> > >> >+ BOND_OPT_COUPLED_CONTROL,
> > >> > BOND_OPT_LAST
> > >> > };
> > >> >
> > >> >diff --git a/include/net/bonding.h b/include/net/bonding.h
> > >> >index 5b8b1b644a2d..b61fb1aa3a56 100644
> > >> >--- a/include/net/bonding.h
> > >> >+++ b/include/net/bonding.h
> > >> >@@ -148,6 +148,7 @@ struct bond_params {
> > >> > #if IS_ENABLED(CONFIG_IPV6)
> > >> > struct in6_addr ns_targets[BOND_MAX_NS_TARGETS];
> > >> > #endif
> > >> >+ int coupled_control;
> > >> >
> > >> > /* 2 bytes of padding : see ether_addr_equal_64bits() */
> > >> > u8 ad_actor_system[ETH_ALEN + 2];
> > >> >@@ -167,6 +168,7 @@ struct slave {
> > >> > u8 backup:1, /* indicates backup slave. Value corresponds with
> > >> > BOND_STATE_ACTIVE and BOND_STATE_BACKUP */
> > >> > inactive:1, /* indicates inactive slave */
> > >> >+ rx_disabled:1, /* indicates whether slave's Rx is disabled */
> > >> > should_notify:1, /* indicates whether the state changed */
> > >> > should_notify_link:1; /* indicates whether the link changed */
> > >> > u8 duplex;
> > >> >@@ -568,6 +570,14 @@ static inline void bond_set_slave_inactive_flags(struct slave *slave,
> > >> > bond_set_slave_state(slave, BOND_STATE_BACKUP, notify);
> > >> > if (!slave->bond->params.all_slaves_active)
> > >> > slave->inactive = 1;
> > >> >+ if (BOND_MODE(slave->bond) == BOND_MODE_8023AD)
> > >> >+ slave->rx_disabled = 1;
> > >> >+}
> > >> >+
> > >> >+static inline void bond_set_slave_tx_disabled_flags(struct slave *slave,
> > >> >+ bool notify)
> > >> >+{
> > >> >+ bond_set_slave_state(slave, BOND_STATE_BACKUP, notify);
> > >> > }
> > >> >
> > >> > static inline void bond_set_slave_active_flags(struct slave *slave,
> > >> >@@ -575,6 +585,14 @@ static inline void bond_set_slave_active_flags(struct slave *slave,
> > >> > {
> > >> > bond_set_slave_state(slave, BOND_STATE_ACTIVE, notify);
> > >> > slave->inactive = 0;
> > >> >+ if (BOND_MODE(slave->bond) == BOND_MODE_8023AD)
> > >> >+ slave->rx_disabled = 0;
> > >> >+}
> > >> >+
> > >> >+static inline void bond_set_slave_rx_enabled_flags(struct slave *slave,
> > >> >+ bool notify)
> > >> >+{
> > >> >+ slave->rx_disabled = 0;
> > >> > }
> > >> >
> > >> > static inline bool bond_is_slave_inactive(struct slave *slave)
> > >> >@@ -582,6 +600,11 @@ static inline bool bond_is_slave_inactive(struct slave *slave)
> > >> > return slave->inactive;
> > >> > }
> > >> >
> > >> >+static inline bool bond_is_slave_rx_disabled(struct slave *slave)
> > >> >+{
> > >> >+ return slave->rx_disabled;
> > >> >+}
> > >> >+
> > >> > static inline void bond_propose_link_state(struct slave *slave, int state)
> > >> > {
> > >> > slave->link_new_state = state;
> > >> >diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> > >> >index 29ff80da2775..7a54fcff2eec 100644
> > >> >--- a/include/uapi/linux/if_link.h
> > >> >+++ b/include/uapi/linux/if_link.h
> > >> >@@ -976,6 +976,7 @@ enum {
> > >> > IFLA_BOND_AD_LACP_ACTIVE,
> > >> > IFLA_BOND_MISSED_MAX,
> > >> > IFLA_BOND_NS_IP6_TARGET,
> > >> >+ IFLA_BOND_COUPLED_CONTROL,
> > >> > __IFLA_BOND_MAX,
> > >> > };
> > >> >
> > >> >diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h
> > >> >index a0aa05a28cf2..f0d71b2a3f1e 100644
> > >> >--- a/tools/include/uapi/linux/if_link.h
> > >> >+++ b/tools/include/uapi/linux/if_link.h
> > >> >@@ -974,6 +974,7 @@ enum {
> > >> > IFLA_BOND_AD_LACP_ACTIVE,
> > >> > IFLA_BOND_MISSED_MAX,
> > >> > IFLA_BOND_NS_IP6_TARGET,
> > >> >+ IFLA_BOND_COUPLED_CONTROL,
> > >> > __IFLA_BOND_MAX,
> > >> > };
> > >> >
> > >> >--
> > >> >2.43.0.429.g432eaa2c6b-goog
> > >> >
> > >> >
> >
> > ---
> > -Jay Vosburgh, [email protected]