Re: [PATCH] bonding: Switch periodic LACPDU state machine from counter to jiffies

[Carlos Bilbao <bilbao@vt.edu>]

FYI, I was able to test this locally but couldn’t find any kselftests to
stress the bonding state machine. If anyone knows of additional ways to
test it, I’d be happy to run them.

Thanks!

Carlos

On 7/15/25 15:57, carlos.bilbao@kernel.org wrote:
> From: Carlos Bilbao <carlos.bilbao@kernel.org>
>
> Replace the bonding periodic state machine for LACPDU transmission of
> function ad_periodic_machine() with a jiffies-based mechanism, which is
> more accurate and can help reduce drift under contention.
>
> Signed-off-by: Carlos Bilbao (DigitalOcean) <carlos.bilbao@kernel.org>
> ---
>   drivers/net/bonding/bond_3ad.c | 79 +++++++++++++---------------------
>   include/net/bond_3ad.h         |  2 +-
>   2 files changed, 32 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
> index c6807e473ab7..8654a51266a3 100644
> --- a/drivers/net/bonding/bond_3ad.c
> +++ b/drivers/net/bonding/bond_3ad.c
> @@ -1421,44 +1421,24 @@ static void ad_periodic_machine(struct port *port, struct bond_params *bond_para
>   	    (!(port->actor_oper_port_state & LACP_STATE_LACP_ACTIVITY) && !(port->partner_oper.port_state & LACP_STATE_LACP_ACTIVITY)) ||
>   	    !bond_params->lacp_active) {
>   		port->sm_periodic_state = AD_NO_PERIODIC;
> -	}
> -	/* check if state machine should change state */
> -	else if (port->sm_periodic_timer_counter) {
> -		/* check if periodic state machine expired */
> -		if (!(--port->sm_periodic_timer_counter)) {
> -			/* if expired then do tx */
> -			port->sm_periodic_state = AD_PERIODIC_TX;
> -		} else {
> -			/* If not expired, check if there is some new timeout
> -			 * parameter from the partner state
> -			 */
> -			switch (port->sm_periodic_state) {
> -			case AD_FAST_PERIODIC:
> -				if (!(port->partner_oper.port_state
> -				      & LACP_STATE_LACP_TIMEOUT))
> -					port->sm_periodic_state = AD_SLOW_PERIODIC;
> -				break;
> -			case AD_SLOW_PERIODIC:
> -				if ((port->partner_oper.port_state & LACP_STATE_LACP_TIMEOUT)) {
> -					port->sm_periodic_timer_counter = 0;
> -					port->sm_periodic_state = AD_PERIODIC_TX;
> -				}
> -				break;
> -			default:
> -				break;
> -			}
> -		}
> +	} else if (port->sm_periodic_state == AD_NO_PERIODIC)
> +		port->sm_periodic_state = AD_FAST_PERIODIC;
> +	/* check if periodic state machine expired */
> +	else if (time_after_eq(jiffies, port->sm_periodic_next_jiffies)) {
> +		/* if expired then do tx */
> +		port->sm_periodic_state = AD_PERIODIC_TX;
>   	} else {
> +		/* If not expired, check if there is some new timeout
> +		 * parameter from the partner state
> +		 */
>   		switch (port->sm_periodic_state) {
> -		case AD_NO_PERIODIC:
> -			port->sm_periodic_state = AD_FAST_PERIODIC;
> -			break;
> -		case AD_PERIODIC_TX:
> -			if (!(port->partner_oper.port_state &
> -			    LACP_STATE_LACP_TIMEOUT))
> +		case AD_FAST_PERIODIC:
> +			if (!(port->partner_oper.port_state & LACP_STATE_LACP_TIMEOUT))
>   				port->sm_periodic_state = AD_SLOW_PERIODIC;
> -			else
> -				port->sm_periodic_state = AD_FAST_PERIODIC;
> +			break;
> +		case AD_SLOW_PERIODIC:
> +			if ((port->partner_oper.port_state & LACP_STATE_LACP_TIMEOUT))
> +				port->sm_periodic_state = AD_PERIODIC_TX;
>   			break;
>   		default:
>   			break;
> @@ -1471,21 +1451,24 @@ static void ad_periodic_machine(struct port *port, struct bond_params *bond_para
>   			  "Periodic Machine: Port=%d, Last State=%d, Curr State=%d\n",
>   			  port->actor_port_number, last_state,
>   			  port->sm_periodic_state);
> +
>   		switch (port->sm_periodic_state) {
> -		case AD_NO_PERIODIC:
> -			port->sm_periodic_timer_counter = 0;
> -			break;
> -		case AD_FAST_PERIODIC:
> -			/* decrement 1 tick we lost in the PERIODIC_TX cycle */
> -			port->sm_periodic_timer_counter = __ad_timer_to_ticks(AD_PERIODIC_TIMER, (u16)(AD_FAST_PERIODIC_TIME))-1;
> -			break;
> -		case AD_SLOW_PERIODIC:
> -			/* decrement 1 tick we lost in the PERIODIC_TX cycle */
> -			port->sm_periodic_timer_counter = __ad_timer_to_ticks(AD_PERIODIC_TIMER, (u16)(AD_SLOW_PERIODIC_TIME))-1;
> -			break;
>   		case AD_PERIODIC_TX:
>   			port->ntt = true;
> -			break;
> +			if (!(port->partner_oper.port_state &
> +						LACP_STATE_LACP_TIMEOUT))
> +				port->sm_periodic_state = AD_SLOW_PERIODIC;
> +			else
> +				port->sm_periodic_state = AD_FAST_PERIODIC;
> +		fallthrough;
> +		case AD_SLOW_PERIODIC:
> +		case AD_FAST_PERIODIC:
> +			if (port->sm_periodic_state == AD_SLOW_PERIODIC)
> +				port->sm_periodic_next_jiffies = jiffies
> +					+ HZ * AD_SLOW_PERIODIC_TIME;
> +			else /* AD_FAST_PERIODIC */
> +				port->sm_periodic_next_jiffies = jiffies
> +					+ HZ * AD_FAST_PERIODIC_TIME;
>   		default:
>   			break;
>   		}
> @@ -1987,7 +1970,7 @@ static void ad_initialize_port(struct port *port, int lacp_fast)
>   		port->sm_rx_state = 0;
>   		port->sm_rx_timer_counter = 0;
>   		port->sm_periodic_state = 0;
> -		port->sm_periodic_timer_counter = 0;
> +		port->sm_periodic_next_jiffies = 0;
>   		port->sm_mux_state = 0;
>   		port->sm_mux_timer_counter = 0;
>   		port->sm_tx_state = 0;
> diff --git a/include/net/bond_3ad.h b/include/net/bond_3ad.h
> index 2053cd8e788a..aabb8c97caf4 100644
> --- a/include/net/bond_3ad.h
> +++ b/include/net/bond_3ad.h
> @@ -227,7 +227,7 @@ typedef struct port {
>   	rx_states_t sm_rx_state;	/* state machine rx state */
>   	u16 sm_rx_timer_counter;	/* state machine rx timer counter */
>   	periodic_states_t sm_periodic_state;	/* state machine periodic state */
> -	u16 sm_periodic_timer_counter;	/* state machine periodic timer counter */
> +	unsigned long sm_periodic_next_jiffies;	/* state machine periodic next expected sent */
>   	mux_states_t sm_mux_state;	/* state machine mux state */
>   	u16 sm_mux_timer_counter;	/* state machine mux timer counter */
>   	tx_states_t sm_tx_state;	/* state machine tx state */