Re: [PATCH net-next v6] bonding: 3ad: update transmit arrays without waiting for RTNL

[Hangbin Liu <hangbin.liu@linux.dev>]

Hi Lu Yun,
On Tue, Nov 10, 2026 at 02:13:57PM +0800, luyun wrote:
> 
> 在 2026/11/10 10:59, Hangbin Liu 写道:
> > Hi Lu Yun,
> > On Thu, Nov 05, 2026 at 03:01:32PM +0800, Yun Lu wrote:
> > > From: Yun Lu <luyun@kylinos.cn>
> > > 
> > > During 802.3ad bring-up, packets can be dropped despite carrier being up
> > > and LACP having enabled ports for distribution. This was observed on a
> > > machine in real use.
> > > 
> > > The LACP worker enables ports before queuing slave_arr_work to rebuild
> > > usable_slaves. The transmit path keeps using that array, which can
> > > remain empty while the array worker retries rtnl_trylock(). Unrelated
> > > RTNL contention can therefore prolong the packet loss.
> 
> Hi Hangbin,
> 
> Thanks for the review.
> 
> > The root issue is the usable_slaves are not available before LACP worker,
> > right?
> No, the real root issue is the delay between LACP enabling ports for
> distribution and publishing those ports in usable_slaves.

Got it, So the LACP entered distribution before the usable_slaves updated.
right?

[...]

> > I still don't understand why we use such a complex approach to fix this
> > small corner-case waiting issue. Can't we just wait for the usable slaves
> > like the following way (not tested)?
> > 
> > diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
> > index d91533b80996..39ecf7f569d3 100644
> > --- a/drivers/net/bonding/bond_3ad.c
> > +++ b/drivers/net/bonding/bond_3ad.c
> > @@ -2555,6 +2555,8 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
> >          struct port *port;
> >          bool should_notify_rtnl = BOND_SLAVE_NOTIFY_LATER;
> >          bool update_slave_arr = false;
> > +       struct bond_up_slave *usable;
> > +       unsigned int count;
> > 
> >          /* Lock to protect data accessed by all (e.g., port->sm_vars) and
> >           * against running with bond_3ad_unbind_slave. ad_rx_machine may run
> > @@ -2564,8 +2566,12 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
> >          rcu_read_lock();
> > 
> >          /* check if there are any slaves */
> > -       if (!bond_has_slaves(bond))
> > +       usable = rcu_dereference(bond->usable_slaves);
> > +       count = usable ? READ_ONCE(usable->count) : 0;
> > +       if (!bond_has_slaves(bond) || !count) {
> > +               update_slave_arr = true;
> >                  goto re_arm;
> > +       }
> > 
> >          if (bond_agg_timer_advance(bond)) {
> >                  slave = bond_first_slave_rcu(bond);
> An empty array before LACP enables any ports is normal. Waiting for it to
> become non-empty before running the state machines would introduce
> a circular dependency: those state machines must first enable the ports
> 
> before the builder can populate the array.

OK, I checked bond_update_slave_arr(), we need to wait for the active
aggregator to build usable slaves. I saw in ad_mux_machine(), it will
check if aggregator->is_active in AD_MUX_ATTACHED. Can we add a usable
slaves checking there before enter to next stage?

Thanks
Hangbin