Re: [PATCH net] bonding: fix missing down path on speed/duplex failure after FAIL->UP

From: Timur Stukan

Date: Sun Oct 25 2026 - 08:08:13 EST


On Fri, 23 Oct 2026 at 16:24, Jay Vosburgh <jv@xxxxxxxxxxxxx> wrote:
> Reading the description and patch, I'm concerned that this patch
> is introducing the same sort of issue that 1899bb325149 was solving: two
> state variables that end up becoming unsynchronized.
>
> For example, here you set link_prev_state in one place, inspect
> it in another, but never clear it. Thus, once set, it will remain set
> forever, raising the possibility of false matches in the future. Not
> necessarily from this patch, which appears to always set then check in
> that order, but from subsequent changes down the road that inadvertently
> break this guarantee.
>
> Yes, that can be resolved by clearing it after use, but I'd
> first ask whether there is an effective solution that does not require
> keeping additional state?
>
> -J


Hi Jay,

Thanks for the review.

Yes, I think it can be solved without additional state. The previous
link state is only needed when link_new_state is BOND_LINK_UP, so
bond_mii_monitor() could skip committing just that state and leave it
to bond_miimon_commit(), which already sets the link UP after the
speed/duplex check. Then slave->link still holds FAIL, BACK or DOWN at
that point, so no new field is needed.

This also seems more correct to me: a slave becomes BOND_LINK_UP only
after the speed/duplex check, so other slaves processed earlier in the
same pass never see it as up before that.

If this approach is acceptable to you, I'll send a v2.

Thanks,
Timur