netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next] bonding: rename AD_STATE_* to BOND_3AD_STATE_*
@ 2019-12-19 23:32 Andy Roulin
  2019-12-20 15:50 ` Andy Gospodarek
  2019-12-24 23:42 ` David Miller
  0 siblings, 2 replies; 6+ messages in thread
From: Andy Roulin @ 2019-12-19 23:32 UTC (permalink / raw)
  To: netdev; +Cc: dsahern, nikolay, roopa, j.vosburgh, vfalico, andy, aroulin

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.

Signed-off-by: Andy Roulin <aroulin@cumulusnetworks.com>
Acked-by: Roopa Prabhu <roopa@cumulusnetworks.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.

 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] bonding: rename AD_STATE_* to BOND_3AD_STATE_*
  2019-12-19 23:32 [PATCH net-next] bonding: rename AD_STATE_* to BOND_3AD_STATE_* Andy Roulin
@ 2019-12-20 15:50 ` Andy Gospodarek
  2019-12-20 16:35   ` Jay Vosburgh
  2019-12-24 23:42 ` David Miller
  1 sibling, 1 reply; 6+ messages in thread
From: Andy Gospodarek @ 2019-12-20 15:50 UTC (permalink / raw)
  To: Andy Roulin; +Cc: netdev, dsahern, nikolay, roopa, j.vosburgh, vfalico

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@cumulusnetworks.com>
> Acked-by: Roopa Prabhu <roopa@cumulusnetworks.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
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] bonding: rename AD_STATE_* to BOND_3AD_STATE_*
  2019-12-20 15:50 ` Andy Gospodarek
@ 2019-12-20 16:35   ` Jay Vosburgh
  2019-12-20 17:18     ` Andy Gospodarek
  0 siblings, 1 reply; 6+ messages in thread
From: Jay Vosburgh @ 2019-12-20 16:35 UTC (permalink / raw)
  To: Andy Gospodarek; +Cc: Andy Roulin, netdev, dsahern, nikolay, roopa, vfalico

Andy Gospodarek <andy@greyhouse.net> wrote:

>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@cumulusnetworks.com>
>> Acked-by: Roopa Prabhu <roopa@cumulusnetworks.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.

	I agree; pretty much everything refers to this protocol as LACP
and not by either the old or current IEEE standard names.

>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.

	But the immediate concern is to get the UAPI correct, as once we
make public API changes there's no going back later.

	-J

>> 
>>  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
>> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] bonding: rename AD_STATE_* to BOND_3AD_STATE_*
  2019-12-20 16:35   ` Jay Vosburgh
@ 2019-12-20 17:18     ` Andy Gospodarek
  2019-12-20 17:39       ` Andy Roulin
  0 siblings, 1 reply; 6+ messages in thread
From: Andy Gospodarek @ 2019-12-20 17:18 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: Andy Roulin, netdev, dsahern, nikolay, roopa, vfalico

On Fri, Dec 20, 2019 at 08:35:33AM -0800, Jay Vosburgh wrote:
> Andy Gospodarek <andy@greyhouse.net> wrote:
> 
> >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@cumulusnetworks.com>
> >> Acked-by: Roopa Prabhu <roopa@cumulusnetworks.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.
> 
> 	I agree; pretty much everything refers to this protocol as LACP
> and not by either the old or current IEEE standard names.
> 
> >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.
> 
> 	But the immediate concern is to get the UAPI correct, as once we
> make public API changes there's no going back later.

Right.

So I'm OK with just a change that adds 'LACP_*' in the place of '*AD_*'
in include/uapi/linux/if_bonding.h and the needed changes across the
rest of the kernel to reflect those changes for now and we can pickup
the rest of the non-uapi changes later.

> 
> 	-J
> 
> >> 
> >>  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
> >> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] bonding: rename AD_STATE_* to BOND_3AD_STATE_*
  2019-12-20 17:18     ` Andy Gospodarek
@ 2019-12-20 17:39       ` Andy Roulin
  0 siblings, 0 replies; 6+ messages in thread
From: Andy Roulin @ 2019-12-20 17:39 UTC (permalink / raw)
  To: Andy Gospodarek
  Cc: Jay Vosburgh, netdev, David Ahern, Nikolay Aleksandrov,
	Roopa Prabhu, vfalico

> So I'm OK with just a change that adds 'LACP_*' in the place of '*AD_*'
> in include/uapi/linux/if_bonding.h and the needed changes across the
> rest of the kernel to reflect those changes for now and we can pickup
> the rest of the non-uapi changes later.

Sounds good, I will resend the patch.

Andy

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] bonding: rename AD_STATE_* to BOND_3AD_STATE_*
  2019-12-19 23:32 [PATCH net-next] bonding: rename AD_STATE_* to BOND_3AD_STATE_* Andy Roulin
  2019-12-20 15:50 ` Andy Gospodarek
@ 2019-12-24 23:42 ` David Miller
  1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2019-12-24 23:42 UTC (permalink / raw)
  To: aroulin; +Cc: netdev, dsahern, nikolay, roopa, j.vosburgh, vfalico, andy

From: Andy Roulin <aroulin@cumulusnetworks.com>
Date: Thu, 19 Dec 2019 15:32:59 -0800

> 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.
> 
> Signed-off-by: Andy Roulin <aroulin@cumulusnetworks.com>
> Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>

When the next version of this is posted please provide an appropriate
Fixes: tag.

My understanding of this situaion is that we only started putting this in
the UAPI header in the current merge window, which is why it is valid to
change UAPI like this.

If you provided an appropriate Fixes: tag, it would have been very clear
to me that this is in fact the case.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-12-24 23:47 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-19 23:32 [PATCH net-next] bonding: rename AD_STATE_* to BOND_3AD_STATE_* Andy Roulin
2019-12-20 15:50 ` Andy Gospodarek
2019-12-20 16:35   ` Jay Vosburgh
2019-12-20 17:18     ` Andy Gospodarek
2019-12-20 17:39       ` Andy Roulin
2019-12-24 23:42 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).