| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20191220155023.GA61232@C02YVCJELVCG.dhcp.broadcom.net>
Date: Fri, 20 Dec 2019 10:50:23 -0500
From: Andy Gospodarek <andy@...yhouse.net>
To: Andy Roulin <aroulin@...ulusnetworks.com>
Cc: netdev@...r.kernel.org, dsahern@...il.com,
nikolay@...ulusnetworks.com, roopa@...ulusnetworks.com,
j.vosburgh@...il.com, vfalico@...il.com
Subject: Re: [PATCH net-next] bonding: rename AD_STATE_* to BOND_3AD_STATE_*
On Thu, Dec 19, 2019 at 03:32:59PM -0800, Andy Roulin wrote:
> As the LACP states are now part of the uapi, rename the
> 3ad state defines with BOND_3AD prefix. This way, the naming
> is consistent with the the rest of the bonding uapi.
Thanks for doing this!
>
> Signed-off-by: Andy Roulin <aroulin@...ulusnetworks.com>
> Acked-by: Roopa Prabhu <roopa@...ulusnetworks.com>
> ---
>
> Notes:
> - Most modified lines were already over 80 chars. Some are now over
> 80 though but I don't think it would add any values to break them
> to respect the limit.
>
> - Another choice would be LACP_* instead of BOND_3AD_* but going
> with LACP would mean we should replace everywhere 3AD with
> LACP to be consistent.
I hate to say this, but I think I prefer the string BOND_8023AD_* or
LACP_* to just BOND_3AD_*. As Jay mentioned the movement a decade ago
to 802.1AX also makes me think we should just drop the references to
802.3AD all-together and just go with LACP.
You are right that the downside to moving towards LACP_* is that
everything should move that way as well, but it seems worthwhile to
consider.
>
> drivers/net/bonding/bond_3ad.c | 112 ++++++++++++++++----------------
> include/uapi/linux/if_bonding.h | 16 ++---
> 2 files changed, 64 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
> index 34bfe99641a3..0b4b8c500894 100644
> --- a/drivers/net/bonding/bond_3ad.c
> +++ b/drivers/net/bonding/bond_3ad.c
> @@ -447,8 +447,8 @@ static void __choose_matched(struct lacpdu *lacpdu, struct port *port)
> MAC_ADDRESS_EQUAL(&(lacpdu->partner_system), &(port->actor_system)) &&
> (ntohs(lacpdu->partner_system_priority) == port->actor_system_priority) &&
> (ntohs(lacpdu->partner_key) == port->actor_oper_port_key) &&
> - ((lacpdu->partner_state & AD_STATE_AGGREGATION) == (port->actor_oper_port_state & AD_STATE_AGGREGATION))) ||
> - ((lacpdu->actor_state & AD_STATE_AGGREGATION) == 0)
> + ((lacpdu->partner_state & BOND_3AD_STATE_AGGREGATION) == (port->actor_oper_port_state & BOND_3AD_STATE_AGGREGATION))) ||
> + ((lacpdu->actor_state & BOND_3AD_STATE_AGGREGATION) == 0)
> ) {
> port->sm_vars |= AD_PORT_MATCHED;
> } else {
> @@ -482,18 +482,18 @@ static void __record_pdu(struct lacpdu *lacpdu, struct port *port)
> partner->port_state = lacpdu->actor_state;
>
> /* set actor_oper_port_state.defaulted to FALSE */
> - port->actor_oper_port_state &= ~AD_STATE_DEFAULTED;
> + port->actor_oper_port_state &= ~BOND_3AD_STATE_DEFAULTED;
>
> /* set the partner sync. to on if the partner is sync,
> * and the port is matched
> */
> if ((port->sm_vars & AD_PORT_MATCHED) &&
> - (lacpdu->actor_state & AD_STATE_SYNCHRONIZATION)) {
> - partner->port_state |= AD_STATE_SYNCHRONIZATION;
> + (lacpdu->actor_state & BOND_3AD_STATE_SYNCHRONIZATION)) {
> + partner->port_state |= BOND_3AD_STATE_SYNCHRONIZATION;
> slave_dbg(port->slave->bond->dev, port->slave->dev,
> "partner sync=1\n");
> } else {
> - partner->port_state &= ~AD_STATE_SYNCHRONIZATION;
> + partner->port_state &= ~BOND_3AD_STATE_SYNCHRONIZATION;
> slave_dbg(port->slave->bond->dev, port->slave->dev,
> "partner sync=0\n");
> }
> @@ -516,7 +516,7 @@ static void __record_default(struct port *port)
> sizeof(struct port_params));
>
> /* set actor_oper_port_state.defaulted to true */
> - port->actor_oper_port_state |= AD_STATE_DEFAULTED;
> + port->actor_oper_port_state |= BOND_3AD_STATE_DEFAULTED;
> }
> }
>
> @@ -546,7 +546,7 @@ static void __update_selected(struct lacpdu *lacpdu, struct port *port)
> !MAC_ADDRESS_EQUAL(&lacpdu->actor_system, &partner->system) ||
> ntohs(lacpdu->actor_system_priority) != partner->system_priority ||
> ntohs(lacpdu->actor_key) != partner->key ||
> - (lacpdu->actor_state & AD_STATE_AGGREGATION) != (partner->port_state & AD_STATE_AGGREGATION)) {
> + (lacpdu->actor_state & BOND_3AD_STATE_AGGREGATION) != (partner->port_state & BOND_3AD_STATE_AGGREGATION)) {
> port->sm_vars &= ~AD_PORT_SELECTED;
> }
> }
> @@ -578,8 +578,8 @@ static void __update_default_selected(struct port *port)
> !MAC_ADDRESS_EQUAL(&admin->system, &oper->system) ||
> admin->system_priority != oper->system_priority ||
> admin->key != oper->key ||
> - (admin->port_state & AD_STATE_AGGREGATION)
> - != (oper->port_state & AD_STATE_AGGREGATION)) {
> + (admin->port_state & BOND_3AD_STATE_AGGREGATION)
> + != (oper->port_state & BOND_3AD_STATE_AGGREGATION)) {
> port->sm_vars &= ~AD_PORT_SELECTED;
> }
> }
> @@ -609,10 +609,10 @@ static void __update_ntt(struct lacpdu *lacpdu, struct port *port)
> !MAC_ADDRESS_EQUAL(&(lacpdu->partner_system), &(port->actor_system)) ||
> (ntohs(lacpdu->partner_system_priority) != port->actor_system_priority) ||
> (ntohs(lacpdu->partner_key) != port->actor_oper_port_key) ||
> - ((lacpdu->partner_state & AD_STATE_LACP_ACTIVITY) != (port->actor_oper_port_state & AD_STATE_LACP_ACTIVITY)) ||
> - ((lacpdu->partner_state & AD_STATE_LACP_TIMEOUT) != (port->actor_oper_port_state & AD_STATE_LACP_TIMEOUT)) ||
> - ((lacpdu->partner_state & AD_STATE_SYNCHRONIZATION) != (port->actor_oper_port_state & AD_STATE_SYNCHRONIZATION)) ||
> - ((lacpdu->partner_state & AD_STATE_AGGREGATION) != (port->actor_oper_port_state & AD_STATE_AGGREGATION))
> + ((lacpdu->partner_state & BOND_3AD_STATE_LACP_ACTIVITY) != (port->actor_oper_port_state & BOND_3AD_STATE_LACP_ACTIVITY)) ||
> + ((lacpdu->partner_state & BOND_3AD_STATE_LACP_TIMEOUT) != (port->actor_oper_port_state & BOND_3AD_STATE_LACP_TIMEOUT)) ||
> + ((lacpdu->partner_state & BOND_3AD_STATE_SYNCHRONIZATION) != (port->actor_oper_port_state & BOND_3AD_STATE_SYNCHRONIZATION)) ||
> + ((lacpdu->partner_state & BOND_3AD_STATE_AGGREGATION) != (port->actor_oper_port_state & BOND_3AD_STATE_AGGREGATION))
> ) {
> port->ntt = true;
> }
> @@ -968,7 +968,7 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> * edable port will take place only after this timer)
> */
> if ((port->sm_vars & AD_PORT_SELECTED) &&
> - (port->partner_oper.port_state & AD_STATE_SYNCHRONIZATION) &&
> + (port->partner_oper.port_state & BOND_3AD_STATE_SYNCHRONIZATION) &&
> !__check_agg_selection_timer(port)) {
> if (port->aggregator->is_active)
> port->sm_mux_state =
> @@ -986,14 +986,14 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> port->sm_mux_state = AD_MUX_DETACHED;
> } else if (port->aggregator->is_active) {
> port->actor_oper_port_state |=
> - AD_STATE_SYNCHRONIZATION;
> + BOND_3AD_STATE_SYNCHRONIZATION;
> }
> break;
> case AD_MUX_COLLECTING_DISTRIBUTING:
> if (!(port->sm_vars & AD_PORT_SELECTED) ||
> (port->sm_vars & AD_PORT_STANDBY) ||
> - !(port->partner_oper.port_state & AD_STATE_SYNCHRONIZATION) ||
> - !(port->actor_oper_port_state & AD_STATE_SYNCHRONIZATION)) {
> + !(port->partner_oper.port_state & BOND_3AD_STATE_SYNCHRONIZATION) ||
> + !(port->actor_oper_port_state & BOND_3AD_STATE_SYNCHRONIZATION)) {
> port->sm_mux_state = AD_MUX_ATTACHED;
> } else {
> /* if port state hasn't changed make
> @@ -1022,11 +1022,11 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> port->sm_mux_state);
> switch (port->sm_mux_state) {
> case AD_MUX_DETACHED:
> - port->actor_oper_port_state &= ~AD_STATE_SYNCHRONIZATION;
> + port->actor_oper_port_state &= ~BOND_3AD_STATE_SYNCHRONIZATION;
> ad_disable_collecting_distributing(port,
> update_slave_arr);
> - port->actor_oper_port_state &= ~AD_STATE_COLLECTING;
> - port->actor_oper_port_state &= ~AD_STATE_DISTRIBUTING;
> + port->actor_oper_port_state &= ~BOND_3AD_STATE_COLLECTING;
> + port->actor_oper_port_state &= ~BOND_3AD_STATE_DISTRIBUTING;
> port->ntt = true;
> break;
> case AD_MUX_WAITING:
> @@ -1035,20 +1035,20 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> case AD_MUX_ATTACHED:
> if (port->aggregator->is_active)
> port->actor_oper_port_state |=
> - AD_STATE_SYNCHRONIZATION;
> + BOND_3AD_STATE_SYNCHRONIZATION;
> else
> port->actor_oper_port_state &=
> - ~AD_STATE_SYNCHRONIZATION;
> - port->actor_oper_port_state &= ~AD_STATE_COLLECTING;
> - port->actor_oper_port_state &= ~AD_STATE_DISTRIBUTING;
> + ~BOND_3AD_STATE_SYNCHRONIZATION;
> + port->actor_oper_port_state &= ~BOND_3AD_STATE_COLLECTING;
> + port->actor_oper_port_state &= ~BOND_3AD_STATE_DISTRIBUTING;
> ad_disable_collecting_distributing(port,
> update_slave_arr);
> port->ntt = true;
> break;
> case AD_MUX_COLLECTING_DISTRIBUTING:
> - port->actor_oper_port_state |= AD_STATE_COLLECTING;
> - port->actor_oper_port_state |= AD_STATE_DISTRIBUTING;
> - port->actor_oper_port_state |= AD_STATE_SYNCHRONIZATION;
> + port->actor_oper_port_state |= BOND_3AD_STATE_COLLECTING;
> + port->actor_oper_port_state |= BOND_3AD_STATE_DISTRIBUTING;
> + port->actor_oper_port_state |= BOND_3AD_STATE_SYNCHRONIZATION;
> ad_enable_collecting_distributing(port,
> update_slave_arr);
> port->ntt = true;
> @@ -1146,7 +1146,7 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
> port->sm_vars |= AD_PORT_LACP_ENABLED;
> port->sm_vars &= ~AD_PORT_SELECTED;
> __record_default(port);
> - port->actor_oper_port_state &= ~AD_STATE_EXPIRED;
> + port->actor_oper_port_state &= ~BOND_3AD_STATE_EXPIRED;
> port->sm_rx_state = AD_RX_PORT_DISABLED;
>
> /* Fall Through */
> @@ -1156,9 +1156,9 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
> case AD_RX_LACP_DISABLED:
> port->sm_vars &= ~AD_PORT_SELECTED;
> __record_default(port);
> - port->partner_oper.port_state &= ~AD_STATE_AGGREGATION;
> + port->partner_oper.port_state &= ~BOND_3AD_STATE_AGGREGATION;
> port->sm_vars |= AD_PORT_MATCHED;
> - port->actor_oper_port_state &= ~AD_STATE_EXPIRED;
> + port->actor_oper_port_state &= ~BOND_3AD_STATE_EXPIRED;
> break;
> case AD_RX_EXPIRED:
> /* Reset of the Synchronization flag (Standard 43.4.12)
> @@ -1167,19 +1167,19 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
> * case of EXPIRED even if LINK_DOWN didn't arrive for
> * the port.
> */
> - port->partner_oper.port_state &= ~AD_STATE_SYNCHRONIZATION;
> + port->partner_oper.port_state &= ~BOND_3AD_STATE_SYNCHRONIZATION;
> port->sm_vars &= ~AD_PORT_MATCHED;
> - port->partner_oper.port_state |= AD_STATE_LACP_TIMEOUT;
> - port->partner_oper.port_state |= AD_STATE_LACP_ACTIVITY;
> + port->partner_oper.port_state |= BOND_3AD_STATE_LACP_TIMEOUT;
> + port->partner_oper.port_state |= BOND_3AD_STATE_LACP_ACTIVITY;
> port->sm_rx_timer_counter = __ad_timer_to_ticks(AD_CURRENT_WHILE_TIMER, (u16)(AD_SHORT_TIMEOUT));
> - port->actor_oper_port_state |= AD_STATE_EXPIRED;
> + port->actor_oper_port_state |= BOND_3AD_STATE_EXPIRED;
> port->sm_vars |= AD_PORT_CHURNED;
> break;
> case AD_RX_DEFAULTED:
> __update_default_selected(port);
> __record_default(port);
> port->sm_vars |= AD_PORT_MATCHED;
> - port->actor_oper_port_state &= ~AD_STATE_EXPIRED;
> + port->actor_oper_port_state &= ~BOND_3AD_STATE_EXPIRED;
> break;
> case AD_RX_CURRENT:
> /* detect loopback situation */
> @@ -1192,8 +1192,8 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
> __update_selected(lacpdu, port);
> __update_ntt(lacpdu, port);
> __record_pdu(lacpdu, port);
> - port->sm_rx_timer_counter = __ad_timer_to_ticks(AD_CURRENT_WHILE_TIMER, (u16)(port->actor_oper_port_state & AD_STATE_LACP_TIMEOUT));
> - port->actor_oper_port_state &= ~AD_STATE_EXPIRED;
> + port->sm_rx_timer_counter = __ad_timer_to_ticks(AD_CURRENT_WHILE_TIMER, (u16)(port->actor_oper_port_state & BOND_3AD_STATE_LACP_TIMEOUT));
> + port->actor_oper_port_state &= ~BOND_3AD_STATE_EXPIRED;
> break;
> default:
> break;
> @@ -1221,7 +1221,7 @@ static void ad_churn_machine(struct port *port)
> if (port->sm_churn_actor_timer_counter &&
> !(--port->sm_churn_actor_timer_counter) &&
> port->sm_churn_actor_state == AD_CHURN_MONITOR) {
> - if (port->actor_oper_port_state & AD_STATE_SYNCHRONIZATION) {
> + if (port->actor_oper_port_state & BOND_3AD_STATE_SYNCHRONIZATION) {
> port->sm_churn_actor_state = AD_NO_CHURN;
> } else {
> port->churn_actor_count++;
> @@ -1231,7 +1231,7 @@ static void ad_churn_machine(struct port *port)
> if (port->sm_churn_partner_timer_counter &&
> !(--port->sm_churn_partner_timer_counter) &&
> port->sm_churn_partner_state == AD_CHURN_MONITOR) {
> - if (port->partner_oper.port_state & AD_STATE_SYNCHRONIZATION) {
> + if (port->partner_oper.port_state & BOND_3AD_STATE_SYNCHRONIZATION) {
> port->sm_churn_partner_state = AD_NO_CHURN;
> } else {
> port->churn_partner_count++;
> @@ -1288,7 +1288,7 @@ static void ad_periodic_machine(struct port *port)
>
> /* check if port was reinitialized */
> if (((port->sm_vars & AD_PORT_BEGIN) || !(port->sm_vars & AD_PORT_LACP_ENABLED) || !port->is_enabled) ||
> - (!(port->actor_oper_port_state & AD_STATE_LACP_ACTIVITY) && !(port->partner_oper.port_state & AD_STATE_LACP_ACTIVITY))
> + (!(port->actor_oper_port_state & BOND_3AD_STATE_LACP_ACTIVITY) && !(port->partner_oper.port_state & BOND_3AD_STATE_LACP_ACTIVITY))
> ) {
> port->sm_periodic_state = AD_NO_PERIODIC;
> }
> @@ -1305,11 +1305,11 @@ static void ad_periodic_machine(struct port *port)
> switch (port->sm_periodic_state) {
> case AD_FAST_PERIODIC:
> if (!(port->partner_oper.port_state
> - & AD_STATE_LACP_TIMEOUT))
> + & BOND_3AD_STATE_LACP_TIMEOUT))
> port->sm_periodic_state = AD_SLOW_PERIODIC;
> break;
> case AD_SLOW_PERIODIC:
> - if ((port->partner_oper.port_state & AD_STATE_LACP_TIMEOUT)) {
> + if ((port->partner_oper.port_state & BOND_3AD_STATE_LACP_TIMEOUT)) {
> port->sm_periodic_timer_counter = 0;
> port->sm_periodic_state = AD_PERIODIC_TX;
> }
> @@ -1325,7 +1325,7 @@ static void ad_periodic_machine(struct port *port)
> break;
> case AD_PERIODIC_TX:
> if (!(port->partner_oper.port_state &
> - AD_STATE_LACP_TIMEOUT))
> + BOND_3AD_STATE_LACP_TIMEOUT))
> port->sm_periodic_state = AD_SLOW_PERIODIC;
> else
> port->sm_periodic_state = AD_FAST_PERIODIC;
> @@ -1532,7 +1532,7 @@ static void ad_port_selection_logic(struct port *port, bool *update_slave_arr)
> ad_agg_selection_logic(aggregator, update_slave_arr);
>
> if (!port->aggregator->is_active)
> - port->actor_oper_port_state &= ~AD_STATE_SYNCHRONIZATION;
> + port->actor_oper_port_state &= ~BOND_3AD_STATE_SYNCHRONIZATION;
> }
>
> /* Decide if "agg" is a better choice for the new active aggregator that
> @@ -1838,13 +1838,13 @@ static void ad_initialize_port(struct port *port, int lacp_fast)
> port->actor_port_priority = 0xff;
> port->actor_port_aggregator_identifier = 0;
> port->ntt = false;
> - port->actor_admin_port_state = AD_STATE_AGGREGATION |
> - AD_STATE_LACP_ACTIVITY;
> - port->actor_oper_port_state = AD_STATE_AGGREGATION |
> - AD_STATE_LACP_ACTIVITY;
> + port->actor_admin_port_state = BOND_3AD_STATE_AGGREGATION |
> + BOND_3AD_STATE_LACP_ACTIVITY;
> + port->actor_oper_port_state = BOND_3AD_STATE_AGGREGATION |
> + BOND_3AD_STATE_LACP_ACTIVITY;
>
> if (lacp_fast)
> - port->actor_oper_port_state |= AD_STATE_LACP_TIMEOUT;
> + port->actor_oper_port_state |= BOND_3AD_STATE_LACP_TIMEOUT;
>
> memcpy(&port->partner_admin, &tmpl, sizeof(tmpl));
> memcpy(&port->partner_oper, &tmpl, sizeof(tmpl));
> @@ -2095,10 +2095,10 @@ void bond_3ad_unbind_slave(struct slave *slave)
> aggregator->aggregator_identifier);
>
> /* Tell the partner that this port is not suitable for aggregation */
> - port->actor_oper_port_state &= ~AD_STATE_SYNCHRONIZATION;
> - port->actor_oper_port_state &= ~AD_STATE_COLLECTING;
> - port->actor_oper_port_state &= ~AD_STATE_DISTRIBUTING;
> - port->actor_oper_port_state &= ~AD_STATE_AGGREGATION;
> + port->actor_oper_port_state &= ~BOND_3AD_STATE_SYNCHRONIZATION;
> + port->actor_oper_port_state &= ~BOND_3AD_STATE_COLLECTING;
> + port->actor_oper_port_state &= ~BOND_3AD_STATE_DISTRIBUTING;
> + port->actor_oper_port_state &= ~BOND_3AD_STATE_AGGREGATION;
> __update_lacpdu_from_port(port);
> ad_lacpdu_send(port);
>
> @@ -2685,9 +2685,9 @@ void bond_3ad_update_lacp_rate(struct bonding *bond)
> bond_for_each_slave(bond, slave, iter) {
> port = &(SLAVE_AD_INFO(slave)->port);
> if (lacp_fast)
> - port->actor_oper_port_state |= AD_STATE_LACP_TIMEOUT;
> + port->actor_oper_port_state |= BOND_3AD_STATE_LACP_TIMEOUT;
> else
> - port->actor_oper_port_state &= ~AD_STATE_LACP_TIMEOUT;
> + port->actor_oper_port_state &= ~BOND_3AD_STATE_LACP_TIMEOUT;
> }
> spin_unlock_bh(&bond->mode_lock);
> }
> diff --git a/include/uapi/linux/if_bonding.h b/include/uapi/linux/if_bonding.h
> index 6829213a54c5..0fc5d5ae8f09 100644
> --- a/include/uapi/linux/if_bonding.h
> +++ b/include/uapi/linux/if_bonding.h
> @@ -96,14 +96,14 @@
> #define BOND_XMIT_POLICY_ENCAP34 4 /* encapsulated layer 3+4 */
>
> /* 802.3ad port state definitions (43.4.2.2 in the 802.3ad standard) */
> -#define AD_STATE_LACP_ACTIVITY 0x1
> -#define AD_STATE_LACP_TIMEOUT 0x2
> -#define AD_STATE_AGGREGATION 0x4
> -#define AD_STATE_SYNCHRONIZATION 0x8
> -#define AD_STATE_COLLECTING 0x10
> -#define AD_STATE_DISTRIBUTING 0x20
> -#define AD_STATE_DEFAULTED 0x40
> -#define AD_STATE_EXPIRED 0x80
> +#define BOND_3AD_STATE_LACP_ACTIVITY 0x1
> +#define BOND_3AD_STATE_LACP_TIMEOUT 0x2
> +#define BOND_3AD_STATE_AGGREGATION 0x4
> +#define BOND_3AD_STATE_SYNCHRONIZATION 0x8
> +#define BOND_3AD_STATE_COLLECTING 0x10
> +#define BOND_3AD_STATE_DISTRIBUTING 0x20
> +#define BOND_3AD_STATE_DEFAULTED 0x40
> +#define BOND_3AD_STATE_EXPIRED 0x80
>
> typedef struct ifbond {
> __s32 bond_mode;
> --
> 2.20.1
>
Powered by blists - more mailing lists