Re: [PATCH net] bonding: 3ad: don't set BOND_LINK_FAIL on a slave that is not up

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

On Sun, Nov 01, 2026 at 04:56:53PM +0200, Timur Stukan wrote:
> In 802.3ad mode, when bond_update_speed_duplex() fails on NETDEV_UP or
> NETDEV_CHANGE, bond_slave_netdev_event() sets the slave link to FAIL if
> the slave has ever been up (last_link_up != 0), and to DOWN otherwise,
> so that miimon can set it right once speed/duplex are valid.
> 
> last_link_up is never cleared, so a slave that is currently DOWN but
> was up before is set to FAIL as well. miimon then treats it as a slave
> that is still in use and takes it through the full down path again:
> the link failure count is incremented, "link status definitely down"
> is logged and the 802.3ad port is disabled a second time, although the
> slave has not been up since it went down.
> 
> Reproduced in 802.3ad mode with miimon=100 by setting a slave down and
> up again. On NETDEV_UP the speed is still unknown, so the notifier sets
> FAIL, and the next miimon pass sees no carrier and proposes DOWN:
> 
> 	ip link add bond0 type bond mode 802.3ad miimon 100 lacp_rate 1
> 	ip link set enp0s3 down
> 	ip link set enp0s4 down
> 	ip link set enp0s3 master bond0
> 	ip link set enp0s4 master bond0
> 	ip link set enp0s3 up
> 	ip link set enp0s4 up
> 	ip link set bond0 up
> 	# wait for LACP to settle
> 	ip link set enp0s3 down
> 	ip link set enp0s3 up
> 
> procfs then shows "Link Failure Count: 2" in enp0s3 slave part. Also in
> dmesg wrote twice: "link status definitely down, disabling slave".
> 
> Fix this by setting FAIL only for a slave that is currently UP and
> leaving other states alone. An up slave that loses speed/duplex still
> goes through FAIL as before, and DOWN/BACK slaves are left to miimon,
> as in the other modes.
> 
> Fixes: ea53abfab960 ("bonding/802.3ad: fix link_failure_count tracking")
> Signed-off-by: Timur Stukan <timur.stukan.linux@gmail.com>
> ---
>  drivers/net/bonding/bond_main.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 7019e5d8fdc8..8212d406f3bc 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -3973,18 +3973,13 @@ static int bond_slave_netdev_event(unsigned long event,
>  	case NETDEV_CHANGE:
>  		/* For 802.3ad mode only:
>  		 * Getting invalid Speed/Duplex values here will put slave
> -		 * in weird state. Mark it as link-fail if the link was
> -		 * previously up or link-down if it hasn't yet come up, and
> -		 * let link-monitoring (miimon) set it right when correct
> -		 * speeds/duplex are available.
> +		 * in weird state. Mark an up slave as link-fail and let
> +		 * miimon set it right when correct speed/duplex are available.
>  		 */
>  		if (bond_update_speed_duplex(slave) &&
> -		    BOND_MODE(bond) == BOND_MODE_8023AD) {
> -			if (slave->last_link_up)
> -				slave->link = BOND_LINK_FAIL;
> -			else
> -				slave->link = BOND_LINK_DOWN;
> -		}
> +		    BOND_MODE(bond) == BOND_MODE_8023AD &&
> +		    slave->link == BOND_LINK_UP)
> +			slave->link = BOND_LINK_FAIL;
>  
>  		if (BOND_MODE(bond) == BOND_MODE_8023AD)
>  			bond_3ad_adapter_speed_duplex_changed(slave);
> -- 
> 2.43.0
> 

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