* [PATCH net v2] bonding: fix missing down path on speed/duplex failure after FAIL->UP
@ 2026-10-28 10:47 Timur Stukan
2026-10-29 1:49 ` Hangbin Liu
2026-10-29 15:51 ` Jay Vosburgh
0 siblings, 2 replies; 3+ messages in thread
From: Timur Stukan @ 2026-10-28 10:47 UTC (permalink / raw)
To: netdev
Cc: Hangbin Liu, Nikolay Aleksandrov, Fernando Fernandez Mancera,
Timur Stukan, Jay Vosburgh, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel
In bond_miimon_commit(), when a slave is proposed as BOND_LINK_UP but
bond_update_speed_duplex() fails in bonding mode that needs speed/duplex
(802.3ad, tlb, alb), the slave is set to BOND_LINK_DOWN directly. This
bypasses bond_set_slave_link_state(), bond_miimon_link_change() and
failover.
This is fine for slave that coming from BOND_LINK_BACK, which was never
brought up and is not in use. But since commit 1899bb325149 ("bonding:
fix state transition issue in link monitoring") the commit phase also
handles BOND_LINK_FAIL -> BOND_LINK_UP which means a slave that is still
in use and may be curr_active_slave.
If the link recovers within downdelay and goes down again before the
commit phase reads speed/duplex, the slave is silently marked down while
remaining the active slave in worst cases. In alb/tlb the bond MAC stays
on the dead port, no failover happens and TLB/RLB state is not cleaned,
so the bond stops passing traffic until that link comes back even though
other slaves are up. A non-active slave is affected too, in alb, RLB
clients assigned to it are not reassigned and become unreachable.
Reproduced in tlb and alb mode with miimon=100 and downdelay=10000 by
flapping a slave within downdelay and dropping it again before commit
(a longer delay was added between inspect and commit to make window
wider and easier to test):
bond0: (slave enp0s3): link status down for interface, disabling it in 10000 ms
bond0: (slave enp0s3): link status up again after 3400 ms
bond0: (slave enp0s3): failed to get link speed/duplex
procfs then shows the active slave with "MII Status: down" and "Link
Failure Count: 0", and all traffic is lost.
Fix this by not committing UP proposals in bond_mii_monitor(): UP is
already committed by bond_miimon_commit() once speed/duplex are valid, so
leaving it there means slave->link still holds the previous state at
that point. When speed/duplex update fails for a slave coming from
BOND_LINK_FAIL, take it through the regular down path, including
failover. Slaves coming from BACK/DOWN keep the current behaviour.
The down path is moved into bond_miimon_slave_down() so both callers share it.
After the fix:
bond0: (slave enp0s3): failed to get link speed/duplex
bond0: (slave enp0s3): link status definitely down, disabling slave
bond0: (slave enp0s4): making interface the new active one
Fixes: 1899bb325149 ("bonding: fix state transition issue in link monitoring")
Signed-off-by: Timur Stukan <timur.stukan.linux@gmail.com>
---
Notes:
Changes in v2:
- Drop link_prev_state. Instead, don't commit UP proposals in
bond_mii_monitor() and let bond_miimon_commit() commit them after the
speed/duplex check, so no extra state is needed (Jay)
- v1: https://lore.kernel.org/netdev/20261022103821.89552-1-timur.stukan.linux@gmail.com/
Sashiko pointed out on v1 that the 802.3ad netdev notifier can set
BOND_LINK_FAIL on a slave that is already down, which then goes through
the full down path again. That is pre-existing and reproducible on an
unpatched net tree:
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
"link status definitely down" is logged twice and enp0s3 ends up with
"Link Failure Count: 2" instead of 1. I will send a separate fix for it.
drivers/net/bonding/bond_main.c | 58 +++++++++++++++++++++++----------
1 file changed, 40 insertions(+), 18 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 7019e5d8fdc8..9bafcddbb453 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2779,6 +2779,27 @@ static void bond_miimon_link_change(struct bonding *bond,
}
}
+static bool bond_miimon_slave_down(struct bonding *bond,
+ struct slave *slave)
+{
+ if (slave->link_failure_count < UINT_MAX)
+ slave->link_failure_count++;
+
+ bond_set_slave_link_state(slave, BOND_LINK_DOWN,
+ BOND_SLAVE_NOTIFY_NOW);
+
+ if (BOND_MODE(bond) == BOND_MODE_ACTIVEBACKUP ||
+ BOND_MODE(bond) == BOND_MODE_8023AD)
+ bond_set_slave_inactive_flags(slave,
+ BOND_SLAVE_NOTIFY_NOW);
+
+ slave_info(bond->dev, slave->dev, "link status definitely down, disabling slave\n");
+
+ bond_miimon_link_change(bond, slave, BOND_LINK_DOWN);
+
+ return slave == rcu_access_pointer(bond->curr_active_slave);
+}
+
static void bond_miimon_commit(struct bonding *bond)
{
struct slave *slave, *primary, *active;
@@ -2807,10 +2828,21 @@ static void bond_miimon_commit(struct bonding *bond)
err = bond_update_speed_duplex(slave);
netdev_unlock_ops(slave->dev);
if (err && bond_needs_speed_duplex(bond)) {
- slave->link = BOND_LINK_DOWN;
if (net_ratelimit())
slave_warn(bond->dev, slave->dev,
"failed to get link speed/duplex\n");
+ /* Coming from FAIL, the slave is still in
+ * use and may be curr_active_slave, so take
+ * it through the full down path. Coming from
+ * DOWN/BACK, it was never brought up, so just
+ * revert it to DOWN.
+ */
+ if (slave->link == BOND_LINK_FAIL) {
+ if (bond_miimon_slave_down(bond, slave))
+ do_failover = true;
+ continue;
+ }
+ slave->link = BOND_LINK_DOWN;
continue;
}
bond_set_slave_link_state(slave, BOND_LINK_UP,
@@ -2839,24 +2871,8 @@ static void bond_miimon_commit(struct bonding *bond)
continue;
case BOND_LINK_DOWN:
- if (slave->link_failure_count < UINT_MAX)
- slave->link_failure_count++;
-
- bond_set_slave_link_state(slave, BOND_LINK_DOWN,
- BOND_SLAVE_NOTIFY_NOW);
-
- if (BOND_MODE(bond) == BOND_MODE_ACTIVEBACKUP ||
- BOND_MODE(bond) == BOND_MODE_8023AD)
- bond_set_slave_inactive_flags(slave,
- BOND_SLAVE_NOTIFY_NOW);
-
- slave_info(bond->dev, slave->dev, "link status definitely down, disabling slave\n");
-
- bond_miimon_link_change(bond, slave, BOND_LINK_DOWN);
-
- if (slave == rcu_access_pointer(bond->curr_active_slave))
+ if (bond_miimon_slave_down(bond, slave))
do_failover = true;
-
continue;
case BOND_LINK_FAIL:
@@ -2919,6 +2935,12 @@ static void bond_mii_monitor(struct work_struct *work)
if (commit) {
bond_for_each_slave(bond, slave, iter) {
+ /* UP state is committed by bond_miimon_commit()
+ * after the speed and duplex check, so that it
+ * can still see the previous link state.
+ */
+ if (slave->link_new_state == BOND_LINK_UP)
+ continue;
bond_commit_link_state(slave,
BOND_SLAVE_NOTIFY_LATER);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net v2] bonding: fix missing down path on speed/duplex failure after FAIL->UP
2026-10-28 10:47 [PATCH net v2] bonding: fix missing down path on speed/duplex failure after FAIL->UP Timur Stukan
@ 2026-10-29 1:49 ` Hangbin Liu
2026-10-29 15:51 ` Jay Vosburgh
1 sibling, 0 replies; 3+ messages in thread
From: Hangbin Liu @ 2026-10-29 1:49 UTC (permalink / raw)
To: Timur Stukan
Cc: netdev, Nikolay Aleksandrov, Fernando Fernandez Mancera,
Jay Vosburgh, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, linux-kernel
Hi Timur,
On Wed, Oct 28, 2026 at 12:47:57PM +0200, Timur Stukan wrote:
> In bond_miimon_commit(), when a slave is proposed as BOND_LINK_UP but
> bond_update_speed_duplex() fails in bonding mode that needs speed/duplex
> (802.3ad, tlb, alb), the slave is set to BOND_LINK_DOWN directly. This
> bypasses bond_set_slave_link_state(), bond_miimon_link_change() and
> failover.
>
> This is fine for slave that coming from BOND_LINK_BACK, which was never
> brought up and is not in use. But since commit 1899bb325149 ("bonding:
> fix state transition issue in link monitoring") the commit phase also
> handles BOND_LINK_FAIL -> BOND_LINK_UP which means a slave that is still
> in use and may be curr_active_slave.
>
> If the link recovers within downdelay and goes down again before the
> commit phase reads speed/duplex, the slave is silently marked down while
> remaining the active slave in worst cases. In alb/tlb the bond MAC stays
> on the dead port, no failover happens and TLB/RLB state is not cleaned,
> so the bond stops passing traffic until that link comes back even though
> other slaves are up. A non-active slave is affected too, in alb, RLB
> clients assigned to it are not reassigned and become unreachable.
>
> Reproduced in tlb and alb mode with miimon=100 and downdelay=10000 by
> flapping a slave within downdelay and dropping it again before commit
> (a longer delay was added between inspect and commit to make window
> wider and easier to test):
> bond0: (slave enp0s3): link status down for interface, disabling it in 10000 ms
> bond0: (slave enp0s3): link status up again after 3400 ms
> bond0: (slave enp0s3): failed to get link speed/duplex
>
> procfs then shows the active slave with "MII Status: down" and "Link
> Failure Count: 0", and all traffic is lost.
>
> Fix this by not committing UP proposals in bond_mii_monitor(): UP is
> already committed by bond_miimon_commit() once speed/duplex are valid, so
> leaving it there means slave->link still holds the previous state at
> that point. When speed/duplex update fails for a slave coming from
> BOND_LINK_FAIL, take it through the regular down path, including
> failover. Slaves coming from BACK/DOWN keep the current behaviour.
> The down path is moved into bond_miimon_slave_down() so both callers share it.
>
> After the fix:
> bond0: (slave enp0s3): failed to get link speed/duplex
> bond0: (slave enp0s3): link status definitely down, disabling slave
> bond0: (slave enp0s4): making interface the new active one
>
> Fixes: 1899bb325149 ("bonding: fix state transition issue in link monitoring")
> Signed-off-by: Timur Stukan <timur.stukan.linux@gmail.com>
> ---
>
> Notes:
> Changes in v2:
> - Drop link_prev_state. Instead, don't commit UP proposals in
> bond_mii_monitor() and let bond_miimon_commit() commit them after the
> speed/duplex check, so no extra state is needed (Jay)
> - v1: https://lore.kernel.org/netdev/20261022103821.89552-1-timur.stukan.linux@gmail.com/
>
> Sashiko pointed out on v1 that the 802.3ad netdev notifier can set
> BOND_LINK_FAIL on a slave that is already down, which then goes through
> the full down path again. That is pre-existing and reproducible on an
> unpatched net tree:
>
> 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
>
> "link status definitely down" is logged twice and enp0s3 ends up with
> "Link Failure Count: 2" instead of 1. I will send a separate fix for it.
>
> drivers/net/bonding/bond_main.c | 58 +++++++++++++++++++++++----------
> 1 file changed, 40 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 7019e5d8fdc8..9bafcddbb453 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2779,6 +2779,27 @@ static void bond_miimon_link_change(struct bonding *bond,
> }
> }
>
> +static bool bond_miimon_slave_down(struct bonding *bond,
> + struct slave *slave)
> +{
> + if (slave->link_failure_count < UINT_MAX)
> + slave->link_failure_count++;
> +
> + bond_set_slave_link_state(slave, BOND_LINK_DOWN,
> + BOND_SLAVE_NOTIFY_NOW);
> +
> + if (BOND_MODE(bond) == BOND_MODE_ACTIVEBACKUP ||
> + BOND_MODE(bond) == BOND_MODE_8023AD)
> + bond_set_slave_inactive_flags(slave,
> + BOND_SLAVE_NOTIFY_NOW);
> +
> + slave_info(bond->dev, slave->dev, "link status definitely down, disabling slave\n");
> +
> + bond_miimon_link_change(bond, slave, BOND_LINK_DOWN);
> +
> + return slave == rcu_access_pointer(bond->curr_active_slave);
> +}
> +
> static void bond_miimon_commit(struct bonding *bond)
> {
> struct slave *slave, *primary, *active;
> @@ -2807,10 +2828,21 @@ static void bond_miimon_commit(struct bonding *bond)
> err = bond_update_speed_duplex(slave);
> netdev_unlock_ops(slave->dev);
> if (err && bond_needs_speed_duplex(bond)) {
> - slave->link = BOND_LINK_DOWN;
> if (net_ratelimit())
> slave_warn(bond->dev, slave->dev,
> "failed to get link speed/duplex\n");
> + /* Coming from FAIL, the slave is still in
> + * use and may be curr_active_slave, so take
> + * it through the full down path. Coming from
> + * DOWN/BACK, it was never brought up, so just
> + * revert it to DOWN.
> + */
> + if (slave->link == BOND_LINK_FAIL) {
> + if (bond_miimon_slave_down(bond, slave))
> + do_failover = true;
> + continue;
> + }
> + slave->link = BOND_LINK_DOWN;
> continue;
> }
> bond_set_slave_link_state(slave, BOND_LINK_UP,
> @@ -2839,24 +2871,8 @@ static void bond_miimon_commit(struct bonding *bond)
> continue;
>
> case BOND_LINK_DOWN:
> - if (slave->link_failure_count < UINT_MAX)
> - slave->link_failure_count++;
> -
> - bond_set_slave_link_state(slave, BOND_LINK_DOWN,
> - BOND_SLAVE_NOTIFY_NOW);
> -
> - if (BOND_MODE(bond) == BOND_MODE_ACTIVEBACKUP ||
> - BOND_MODE(bond) == BOND_MODE_8023AD)
> - bond_set_slave_inactive_flags(slave,
> - BOND_SLAVE_NOTIFY_NOW);
> -
> - slave_info(bond->dev, slave->dev, "link status definitely down, disabling slave\n");
> -
> - bond_miimon_link_change(bond, slave, BOND_LINK_DOWN);
> -
> - if (slave == rcu_access_pointer(bond->curr_active_slave))
> + if (bond_miimon_slave_down(bond, slave))
> do_failover = true;
> -
> continue;
>
> case BOND_LINK_FAIL:
> @@ -2919,6 +2935,12 @@ static void bond_mii_monitor(struct work_struct *work)
>
> if (commit) {
> bond_for_each_slave(bond, slave, iter) {
> + /* UP state is committed by bond_miimon_commit()
> + * after the speed and duplex check, so that it
> + * can still see the previous link state.
> + */
> + if (slave->link_new_state == BOND_LINK_UP)
> + continue;
> bond_commit_link_state(slave,
> BOND_SLAVE_NOTIFY_LATER);
> }
> --
> 2.43.0
>
This version looks good to me. Thanks!
Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net v2] bonding: fix missing down path on speed/duplex failure after FAIL->UP
2026-10-28 10:47 [PATCH net v2] bonding: fix missing down path on speed/duplex failure after FAIL->UP Timur Stukan
2026-10-29 1:49 ` Hangbin Liu
@ 2026-10-29 15:51 ` Jay Vosburgh
1 sibling, 0 replies; 3+ messages in thread
From: Jay Vosburgh @ 2026-10-29 15:51 UTC (permalink / raw)
To: Timur Stukan
Cc: netdev, Hangbin Liu, Nikolay Aleksandrov,
Fernando Fernandez Mancera, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel
Timur Stukan <timur.stukan.linux@gmail.com> wrote:
>In bond_miimon_commit(), when a slave is proposed as BOND_LINK_UP but
>bond_update_speed_duplex() fails in bonding mode that needs speed/duplex
>(802.3ad, tlb, alb), the slave is set to BOND_LINK_DOWN directly. This
>bypasses bond_set_slave_link_state(), bond_miimon_link_change() and
>failover.
>
>This is fine for slave that coming from BOND_LINK_BACK, which was never
>brought up and is not in use. But since commit 1899bb325149 ("bonding:
>fix state transition issue in link monitoring") the commit phase also
>handles BOND_LINK_FAIL -> BOND_LINK_UP which means a slave that is still
>in use and may be curr_active_slave.
>
>If the link recovers within downdelay and goes down again before the
>commit phase reads speed/duplex, the slave is silently marked down while
>remaining the active slave in worst cases. In alb/tlb the bond MAC stays
>on the dead port, no failover happens and TLB/RLB state is not cleaned,
>so the bond stops passing traffic until that link comes back even though
>other slaves are up. A non-active slave is affected too, in alb, RLB
>clients assigned to it are not reassigned and become unreachable.
>
>Reproduced in tlb and alb mode with miimon=100 and downdelay=10000 by
>flapping a slave within downdelay and dropping it again before commit
>(a longer delay was added between inspect and commit to make window
>wider and easier to test):
> bond0: (slave enp0s3): link status down for interface, disabling it in 10000 ms
> bond0: (slave enp0s3): link status up again after 3400 ms
> bond0: (slave enp0s3): failed to get link speed/duplex
>
>procfs then shows the active slave with "MII Status: down" and "Link
>Failure Count: 0", and all traffic is lost.
>
>Fix this by not committing UP proposals in bond_mii_monitor(): UP is
>already committed by bond_miimon_commit() once speed/duplex are valid, so
>leaving it there means slave->link still holds the previous state at
>that point. When speed/duplex update fails for a slave coming from
>BOND_LINK_FAIL, take it through the regular down path, including
>failover. Slaves coming from BACK/DOWN keep the current behaviour.
>The down path is moved into bond_miimon_slave_down() so both callers share it.
>
>After the fix:
> bond0: (slave enp0s3): failed to get link speed/duplex
> bond0: (slave enp0s3): link status definitely down, disabling slave
> bond0: (slave enp0s4): making interface the new active one
>
>Fixes: 1899bb325149 ("bonding: fix state transition issue in link monitoring")
>Signed-off-by: Timur Stukan <timur.stukan.linux@gmail.com>
>---
>
>Notes:
> Changes in v2:
> - Drop link_prev_state. Instead, don't commit UP proposals in
> bond_mii_monitor() and let bond_miimon_commit() commit them after the
> speed/duplex check, so no extra state is needed (Jay)
> - v1: https://lore.kernel.org/netdev/20261022103821.89552-1-timur.stukan.linux@gmail.com/
>
> Sashiko pointed out on v1 that the 802.3ad netdev notifier can set
> BOND_LINK_FAIL on a slave that is already down, which then goes through
> the full down path again. That is pre-existing and reproducible on an
> unpatched net tree:
>
> 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
>
> "link status definitely down" is logged twice and enp0s3 ends up with
> "Link Failure Count: 2" instead of 1. I will send a separate fix for it.
>
> drivers/net/bonding/bond_main.c | 58 +++++++++++++++++++++++----------
> 1 file changed, 40 insertions(+), 18 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 7019e5d8fdc8..9bafcddbb453 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -2779,6 +2779,27 @@ static void bond_miimon_link_change(struct bonding *bond,
> }
> }
>
>+static bool bond_miimon_slave_down(struct bonding *bond,
>+ struct slave *slave)
>+{
>+ if (slave->link_failure_count < UINT_MAX)
>+ slave->link_failure_count++;
>+
>+ bond_set_slave_link_state(slave, BOND_LINK_DOWN,
>+ BOND_SLAVE_NOTIFY_NOW);
>+
>+ if (BOND_MODE(bond) == BOND_MODE_ACTIVEBACKUP ||
>+ BOND_MODE(bond) == BOND_MODE_8023AD)
>+ bond_set_slave_inactive_flags(slave,
>+ BOND_SLAVE_NOTIFY_NOW);
>+
>+ slave_info(bond->dev, slave->dev, "link status definitely down, disabling slave\n");
>+
>+ bond_miimon_link_change(bond, slave, BOND_LINK_DOWN);
>+
>+ return slave == rcu_access_pointer(bond->curr_active_slave);
>+}
>+
> static void bond_miimon_commit(struct bonding *bond)
> {
> struct slave *slave, *primary, *active;
>@@ -2807,10 +2828,21 @@ static void bond_miimon_commit(struct bonding *bond)
> err = bond_update_speed_duplex(slave);
> netdev_unlock_ops(slave->dev);
> if (err && bond_needs_speed_duplex(bond)) {
>- slave->link = BOND_LINK_DOWN;
> if (net_ratelimit())
> slave_warn(bond->dev, slave->dev,
> "failed to get link speed/duplex\n");
>+ /* Coming from FAIL, the slave is still in
>+ * use and may be curr_active_slave, so take
>+ * it through the full down path. Coming from
>+ * DOWN/BACK, it was never brought up, so just
>+ * revert it to DOWN.
>+ */
>+ if (slave->link == BOND_LINK_FAIL) {
>+ if (bond_miimon_slave_down(bond, slave))
>+ do_failover = true;
>+ continue;
>+ }
>+ slave->link = BOND_LINK_DOWN;
> continue;
> }
> bond_set_slave_link_state(slave, BOND_LINK_UP,
>@@ -2839,24 +2871,8 @@ static void bond_miimon_commit(struct bonding *bond)
> continue;
>
> case BOND_LINK_DOWN:
>- if (slave->link_failure_count < UINT_MAX)
>- slave->link_failure_count++;
>-
>- bond_set_slave_link_state(slave, BOND_LINK_DOWN,
>- BOND_SLAVE_NOTIFY_NOW);
>-
>- if (BOND_MODE(bond) == BOND_MODE_ACTIVEBACKUP ||
>- BOND_MODE(bond) == BOND_MODE_8023AD)
>- bond_set_slave_inactive_flags(slave,
>- BOND_SLAVE_NOTIFY_NOW);
>-
>- slave_info(bond->dev, slave->dev, "link status definitely down, disabling slave\n");
>-
>- bond_miimon_link_change(bond, slave, BOND_LINK_DOWN);
>-
>- if (slave == rcu_access_pointer(bond->curr_active_slave))
>+ if (bond_miimon_slave_down(bond, slave))
> do_failover = true;
>-
> continue;
>
> case BOND_LINK_FAIL:
>@@ -2919,6 +2935,12 @@ static void bond_mii_monitor(struct work_struct *work)
>
> if (commit) {
> bond_for_each_slave(bond, slave, iter) {
>+ /* UP state is committed by bond_miimon_commit()
>+ * after the speed and duplex check, so that it
>+ * can still see the previous link state.
>+ */
>+ if (slave->link_new_state == BOND_LINK_UP)
>+ continue;
> bond_commit_link_state(slave,
> BOND_SLAVE_NOTIFY_LATER);
Reading bond_commit_link_state(), for this call site, all it
really does is slave->link = slave->link_new_state, when link_new_state
!= BOND_LINK_NOCHAGE. The changes to ->should_notify_link only matter
for the ARP monitor, so that part is effectively a no-op here.
Therefore, I wonder if it would be cleaner overall to not do the
bond_commit_link_state here at all, but instead keep what you've added
in the BOND_LINK_UP part of bond_miimon_commit, and then add calls to
bond_set_slave_link_state(slave, ..., BOND_SLAVE_NOTIFY_LATER) to the
BOND_LINK_FAIL/BOND_LINK_BACK case to make the change stick.
A quick hack patch that I have not tested to hopefully
illustrate what I mean:
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 756d79281ffc..5fb215fa85f6 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2877,6 +2877,8 @@ static void bond_miimon_commit(struct bonding *bond)
case BOND_LINK_FAIL:
case BOND_LINK_BACK:
+ bond_set_slave_link_state(slave, slave->link_new_state,
+ BOND_SLAVE_NOTIFY_LATER);
slave_dbg(bond->dev, slave->dev, "link_new_state %d on slave\n",
slave->link_new_state);
continue;
@@ -2933,19 +2935,8 @@ static void bond_mii_monitor(struct work_struct *work)
goto re_arm;
}
- if (commit) {
- bond_for_each_slave(bond, slave, iter) {
- /* UP state is committed by bond_miimon_commit()
- * after the speed and duplex check, so that it
- * can still see the previous link state.
- */
- if (slave->link_new_state == BOND_LINK_UP)
- continue;
- bond_commit_link_state(slave,
- BOND_SLAVE_NOTIFY_LATER);
- }
+ if (commit)
bond_miimon_commit(bond);
- }
if (bond->send_peer_notif)
bond_peer_notify_may_events(bond, true);
Thoughts?
-J
---
-Jay Vosburgh, jv@jvosburgh.net
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-29 15:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-28 10:47 [PATCH net v2] bonding: fix missing down path on speed/duplex failure after FAIL->UP Timur Stukan
2026-10-29 1:49 ` Hangbin Liu
2026-10-29 15:51 ` Jay Vosburgh
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.