[PATCH net 0/2] bond: fix 2 link state issues

[PATCH net 0/2] bond: fix 2 link state issues

[Hangbin Liu]

This patch set fixes two bonding link state issues:

1. Broadcast mode incorrectly sets usable_slaves, causing updelay to be ignored
2. BOND_LINK_FAIL and BOND_LINK_BACK are treated as invalid states, generating
   confusing error messages

Here is the reproducer:

```
ip netns add ns
ip -n ns link add bond0 type bond mode 3 miimon 100 updelay 200 downdelay 200
ip -n ns link add type veth
ip -n ns link add type veth
ip -n ns link set veth1 up
ip -n ns link set veth3 up
ip -n ns link set veth0 master bond0
ip -n ns link set veth2 master bond0
ip -n ns link set bond0 up
sleep 1
ip -n ns link set veth3 down
sleep 1
ip -n ns link set veth3 up
sleep 1
dmesg | tail
```

---
Hangbin Liu (2):
      bonding: do not set usable_slaves for broadcast mode
      bonding: handle BOND_LINK_FAIL, BOND_LINK_BACK as valid link states

 drivers/net/bonding/bond_main.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)
---
base-commit: 46d0d6f50dab706637f4c18a470aac20a21900d3
change-id: 20260304-b4-bond_updelay-266e8ce1048e

Best regards,
-- 
Hangbin Liu <liuhangbin@gmail.com>

[PATCH net 1/2] bonding: do not set usable_slaves for broadcast mode

[Hangbin Liu]

After commit e0caeb24f538 ("net: bonding: update the slave array for broadcast mode"),
broadcast mode will also set all_slaves and usable_slaves during
bond_enslave(). But if we also set updelay, during enslave, the
slave init state will be BOND_LINK_BACK. And later
bond_update_slave_arr() will alloc usable_slaves but add nothing.
This will cause bond_miimon_inspect() to have ignore_updelay
always true. So the updelay will be always ignored. e.g.

[    6.498368] bond0: (slave veth2): link status definitely down, disabling slave
[    7.536371] bond0: (slave veth2): link status up, enabling it in 0 ms
[    7.536402] bond0: (slave veth2): link status definitely up, 10000 Mbps full duplex

To fix it, we can either always call bond_update_slave_arr() on every
place when link changes. Or, let's just not set usable_slaves for
broadcast mode.

Fixes: e0caeb24f538 ("net: bonding: update the slave array for broadcast mode")
Reported-by: Liang Li <liali@redhat.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 drivers/net/bonding/bond_main.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 14ed91391fcc..93a32a368d31 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -5069,13 +5069,18 @@ static void bond_set_slave_arr(struct bonding *bond,
 {
 	struct bond_up_slave *usable, *all;
 
-	usable = rtnl_dereference(bond->usable_slaves);
-	rcu_assign_pointer(bond->usable_slaves, usable_slaves);
-	kfree_rcu(usable, rcu);
-
 	all = rtnl_dereference(bond->all_slaves);
 	rcu_assign_pointer(bond->all_slaves, all_slaves);
 	kfree_rcu(all, rcu);
+
+	if (BOND_MODE(bond) == BOND_MODE_BROADCAST) {
+		kfree_rcu(usable_slaves, rcu);
+		return;
+	}
+
+	usable = rtnl_dereference(bond->usable_slaves);
+	rcu_assign_pointer(bond->usable_slaves, usable_slaves);
+	kfree_rcu(usable, rcu);
 }
 
 static void bond_reset_slave_arr(struct bonding *bond)

-- 
Git-155)

[PATCH net 2/2] bonding: handle BOND_LINK_FAIL, BOND_LINK_BACK as valid link states

[Hangbin Liu]

Before the fixed commit, we check slave->new_link during commit
state, which values are only BOND_LINK_{NOCHANGE, UP, DOWN}. After
the commit, we start using slave->link_new_state, which state also could
be BOND_LINK_{FAIL, BACK}.

For example, when we set updelay/downdelay, after a failover,
the slave->link_new_state could be set to BOND_LINK_{FAIL, BACK} in
bond_miimon_inspect(). And later in bond_miimon_commit(), it will treat
it as invalid and print an error, which would cause confusion for users.

[  106.440254] bond0: (slave veth2): link status down for interface, disabling it in 200 ms
[  106.440265] bond0: (slave veth2): invalid new link 1 on slave
[  106.648276] bond0: (slave veth2): link status definitely down, disabling slave
[  107.480271] bond0: (slave veth2): link status up, enabling it in 200 ms
[  107.480288] bond0: (slave veth2): invalid new link 3 on slave
[  107.688302] bond0: (slave veth2): link status definitely up, 10000 Mbps full duplex

Let's handle BOND_LINK_{FAIL, BACK} as valid link states.

Fixes: 1899bb325149 ("bonding: fix state transition issue in link monitoring")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 drivers/net/bonding/bond_main.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 93a32a368d31..444519078da3 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2801,8 +2801,14 @@ static void bond_miimon_commit(struct bonding *bond)
 
 			continue;
 
+		case BOND_LINK_FAIL:
+		case BOND_LINK_BACK:
+			slave_dbg(bond->dev, slave->dev, "link_new_state %d on slave\n",
+				  slave->link_new_state);
+			continue;
+
 		default:
-			slave_err(bond->dev, slave->dev, "invalid new link %d on slave\n",
+			slave_err(bond->dev, slave->dev, "invalid link_new_state %d on slave\n",
 				  slave->link_new_state);
 			bond_propose_link_state(slave, BOND_LINK_NOCHANGE);
 

-- 
Git-155)

Re: [PATCH net 0/2] bond: fix 2 link state issues

[Jakub Kicinski]

On Wed, 04 Mar 2026 15:13:52 +0800 Hangbin Liu wrote:
> Here is the reproducer:
> 
> ```
> ip netns add ns
> ip -n ns link add bond0 type bond mode 3 miimon 100 updelay 200 downdelay 200
> ip -n ns link add type veth
> ip -n ns link add type veth
> ip -n ns link set veth1 up
> ip -n ns link set veth3 up
> ip -n ns link set veth0 master bond0
> ip -n ns link set veth2 master bond0
> ip -n ns link set bond0 up
> sleep 1
> ip -n ns link set veth3 down
> sleep 1
> ip -n ns link set veth3 up
> sleep 1
> dmesg | tail

Are you planning to add a regression test? Or you think this is too
much of a one-off regression? Just curious.. 

Re: [PATCH net 0/2] bond: fix 2 link state issues

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 04 Mar 2026 15:13:52 +0800 you wrote:
> This patch set fixes two bonding link state issues:
> 
> 1. Broadcast mode incorrectly sets usable_slaves, causing updelay to be ignored
> 2. BOND_LINK_FAIL and BOND_LINK_BACK are treated as invalid states, generating
>    confusing error messages
> 
> Here is the reproducer:
> 
> [...]

Here is the summary with links:
  - [net,1/2] bonding: do not set usable_slaves for broadcast mode
    https://git.kernel.org/netdev/net/c/45fc134bcfad
  - [net,2/2] bonding: handle BOND_LINK_FAIL, BOND_LINK_BACK as valid link states
    https://git.kernel.org/netdev/net/c/3348be7978f4

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net 0/2] bond: fix 2 link state issues

[Hangbin Liu]

On Fri, Mar 06, 2026 at 04:26:13PM -0800, Jakub Kicinski wrote:
> On Wed, 04 Mar 2026 15:13:52 +0800 Hangbin Liu wrote:
> > Here is the reproducer:
> > 
> > ```
> > ip netns add ns
> > ip -n ns link add bond0 type bond mode 3 miimon 100 updelay 200 downdelay 200
> > ip -n ns link add type veth
> > ip -n ns link add type veth
> > ip -n ns link set veth1 up
> > ip -n ns link set veth3 up
> > ip -n ns link set veth0 master bond0
> > ip -n ns link set veth2 master bond0
> > ip -n ns link set bond0 up
> > sleep 1
> > ip -n ns link set veth3 down
> > sleep 1
> > ip -n ns link set veth3 up
> > sleep 1
> > dmesg | tail
> 
> Are you planning to add a regression test? Or you think this is too
> much of a one-off regression? Just curious.. 

Hmm, I didn't plan to write a test case for this corner issue. But with your
reminder, may be I can add a common updealy/downdelay parameter test case.
I will add this on my todo list.

Thanks
Hangbin