Re: [PATCH net v2] bonding: fix initial last_rx vs ARP-monitor slack window

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

On Sun, Sep 06, 2026 at 11:02:37AM +0200, Ramses wrote:
> Aug 28, 2026, 04:50 by hangbin.liu@linux.dev:
> 
> > On Thu, Aug 27, 2026 at 01:44:42PM +0200, Ramses de Norre via B4 Relay wrote:
> >
> >> From: Ramses de Norre <ramses@well-founded.dev>
> >>
> >> Commit f31c7937c254 ("bonding: start slaves with link down for ARP
> >> monitor") initialises a freshly enslaved port's last_rx to
> >> jiffies - (arp_interval + 1) so that it does not "immediately cause
> >> fake detection of 'up' state". At the time, the comparison was a plain
> >> <= arp_interval and the value was just stale enough.
> >>
> >> Commit da210f559019 ("bonding: add some slack to arp monitoring time
> >> limits"), four months later, added a +arp_interval/2 slack term to
> >> every comparison (now bond_time_in_interval()) but did not widen the
> >> init to match. Since then, bond_time_in_interval(bond, last_rx, 1) is
> >> true for the first ~arp_interval/2 after enslavement even though no
> >> packet has been received: the upper bound is last_rx + 1.5*delta and
> >> last_rx was set to jiffies - delta - 1.
> >>
> >> If the ARP monitor tick lands in that window, bond_ab_arp_inspect()
> >> proposes the slave UP. If the slave is the configured primary,
> >> bond_ab_arp_commit() sets do_failover and the still-armed
> >> force_primary in bond_choose_primary_or_current() makes it the active
> >> slave regardless of primary_reselect. ARP validation as the active
> >> slave then fails (the link has not actually received anything; on
> >> SFP+ ports the PHY is often still negotiating) and the bond falls back
> >> to the backup. With primary_reselect=failure, force_primary has now
> >> been spent and the bond stays on the backup until something else
> >> triggers a reselect.
> >>
> >
> > Can we set primary_reselect to always or better to avoid this? If you prefer
> > to using the primary slave.
> >
> That hides the symptom, but the wrong link state happens nonetheless, in bond_ab_arp_inspect(), independent of any selection policy:
> 
> if (slave->link != BOND_LINK_UP) {
>   if (bond_time_in_interval(bond, last_rx, 1)) {
>     bond_propose_link_state(slave, BOND_LINK_UP);
> 
> bond_enslave() needs to seed last_rx with a timestamp old enough that this test fails until a real ARP reply arrives. That is what the seed is for, see f31c7937c254 ("bonding: start slaves with link down for ARP monitor").
> 
> The seed is one arp_interval old, which was just old enough back when the test was "jiffies <= last_rx + delta". da210f559019 then added delta/2 of slack to every such comparison without adjusting the seed accordingly, so the seed now sits inside the window instead of outside it. So for the first half ARP interval after enslavement the test indicates "received recently" for a slave that hasn't actually received anything, and that is true for any freshly enslaved slave, primary or not, whatever primary_reselect is set to.
> 
> With the always policy the bond does recover once the port really comes up, so it is not permanent, but until then traffic goes to a port whose PHY has not finished negotiating and is dropped.
> So in my case, I use the failure policy precisely to avoid the bond flapping back to the primary while it is not actually up, which would cause dropped packets, but it would be much nicer if the link state was just detected correctly and this workaround wasn't needed.

Ah, I got what you mean. The primary slave is set "UP" but no arp reply yet,
which cause a false UP and then goes down. Your fix is similar with
f31c7937c254 ("bonding: start slaves with link down for ARP monitor"), the
difference is your version is based on the extra arp_interval/2 delta.

The code looks good to me.

Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>

But it would be good if someone else also could help review in case I missed
anything.

Thanks
Hangbin