Message-ID: <aFsq8QSZRNAE8PYs@do-x1carbon>
Date: Tue, 24 Jun 2025 17:47:13 -0500
From: Seth Forshee <sforshee@...nel.org>
To: Jay Vosburgh <jv@...sburgh.net>
Cc: Tonghao Zhang <tonghao@...aicloud.com>, carlos.bilbao@...nel.org,
	andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, horms@...nel.org,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org, bilbao@...edu
Subject: Re: [PATCH] bonding: Improve the accuracy of LACPDU transmissions

On Tue, Jun 24, 2025 at 02:15:32PM -0700, Jay Vosburgh wrote:
> Tonghao Zhang <tonghao@...aicloud.com> wrote:
> 
> >
> >
> >
> >> 2025年6月19日 03:53，carlos.bilbao@...nel.org 写道：
> >> 
> >> From: Carlos Bilbao <carlos.bilbao@...nel.org>
> >> 
> >> Improve the timing accuracy of LACPDU transmissions in the bonding 802.3ad
> >> (LACP) driver. The current approach relies on a decrementing counter to
> >> limit the transmission rate. In our experience, this method is susceptible
> >> to delays (such as those caused by CPU contention or soft lockups) which
> >> can lead to accumulated drift in the LACPDU send interval. Over time, this
> >> drift can cause synchronization issues with the top-of-rack (ToR) switch
> >> managing the LAG, manifesting as lag map flapping. This in turn can trigger
> >> temporary interface removal and potential packet loss.
> 
> 	So, you're saying that contention or soft lockups are causing
> the queue_delayed_work() of bond_3ad_state_machine_handler() to be
> scheduled late, and, then, because the LACPDU TX limiter is based on the
> number of state machine executions (which should be every 100ms), it is
> then late sending LACPDUs?
> 
> 	If the core problem is that the state machine as a whole isn't
> running regularly, how is doing a clock-based time check reliable?  Or
> should I take the word "improve" from the Subject literally, and assume
> it's making things "better" but not "totally perfect"?
> 
> 	Is the sync issue with the TOR due to missing / delayed LACPDUs,
> or is there more to it?  At the fast LACP rate, the periodic timeout is
> 3 seconds for a nominal 1 second LACPDU interval, which is fairly
> generous.

To take a step back: we originally started looking at this code because
we've seen cases where TORs drop a link from the bond due to not getting
a LACPDU within the short timeout period. We don't currently know the
root cause of that problem, so this patch isn't trying to fix that issue.

But when looking at the LACPDUs that the TOR is receiving, we saw that
the packets were pretty consistently late by hundreds of milliseconds.
This patch from Carlos is trying to improve that inconsistency.

Yes, the math is incorrect, and that's partly my fault from giving
Carlos bad advice after looking at the code too quickly. But the fact
that this mistake improved the timing points to what I think may be at
the root of the inconsistency we see.

ad_tx_machine() resets port->sm_tx_timer_counter every time it reaches
zero, regardless of when the last LACPDU was sent. This means that
ad_tx_machine() is delaying LACPDU tx to the next expiration of this
periodic timer. So when Carlos made the counter very small, he made the
tx delay much shorter.

As I understand it the intention is to ensure that LACPDU tx is spaced
out by at least ~300ms, not to align them to an arbitrary ~300ms
boundary. If so, a simple improvement would be to reset the counter only
when an LACPDU is sent, then allow sending a LACPDU any time after it
reaches zero. Though I still think it makes sense to make the state
machines time-based rather than counter-based to ensure they aren't
sensitive to delays in running the delayed work.

Thanks,
Seth

Powered by blists - more mailing lists