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

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

[Ramses de Norre via B4 Relay]

From: Ramses de Norre <ramses@well-founded.dev>

bond_enslave() seeds a new slave's last_rx with a timestamp old enough
that the ARP monitor does not consider the slave up until a real ARP
reply arrives. That is the purpose of the seed, see commit f31c7937c254
("bonding: start slaves with link down for ARP monitor").

The seed is one arp_interval (plus a tick) in the past, which was just
old enough when the test was "jiffies <= last_rx + arp_interval".
Commit da210f559019 ("bonding: add some slack to arp monitoring time
limits") added arp_interval/2 of slack to every such comparison without
adjusting the seed, so the seed now sits inside the window instead of
outside it. For the first half ARP interval after enslavement,
bond_ab_arp_inspect() therefore sees "received recently" for a slave
that has not received anything and proposes it UP. This happens for any
freshly enslaved slave, independent of primary_reselect.

When the slave is the configured primary, the still-armed force_primary
then makes it the active slave. ARP validation fails it shortly after,
since the link has not received anything (on SFP+ ports the PHY is
often still negotiating), and the bond falls back to the backup. With
primary_reselect=always the bond recovers once the port really comes
up, but drops traffic until then. With primary_reselect=failure,
force_primary has been spent and the bond stays on the backup.

Reproducer:

  ip link add bond0 type bond mode active-backup arp_interval 1000 \
      arp_validate all arp_ip_target 192.0.2.1 \
      primary eth0 primary_reselect failure
  # eth0: SFP+ (slow link-up), eth1: RJ45 (fast link-up)
  ip link set eth0 master bond0
  ip link set eth1 master bond0
  ip link set bond0 up
  # bond0 lands on eth0 via force_primary, ARP-fails it before the
  # SFP+ has carrier, falls to eth1, and stays there.

Seed last_rx (and with it the per-target array and last_tx) two
intervals in the past, so it is outside the slack window from the
start. The initial link state is unchanged, it still comes from
netif_carrier_ok().

Fixes: da210f559019 ("bonding: add some slack to arp monitoring time limits")
Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>
Signed-off-by: Ramses de Norre <ramses@well-founded.dev>
---
Changes in v3:
- No code changes. Rebase on current net.
- Reword the commit message along the lines of the v2 discussion: lead
  with what the last_rx seed is for, state that the false UP happens
  independent of primary_reselect, and that the initial link state is
  unchanged.
- Pick up Reviewed-by from Hangbin.
- Cc Nikolay. Hangbin asked for a second review, Jay or Nikolay, would
  you have a look?
- Link to v2: https://patch.msgid.link/20260827-bonding-last-rx-v2-1-7c3833822c23@well-founded.dev

Changes in v2:
- No code changes. Resend with my personal name in the From and
  Signed-off-by lines instead of my handle, as requested for the DCO.
- Link to v1: https://patch.msgid.link/20260817-bonding-last-rx-v1-1-9da6f7fdf812@well-founded.dev
---
 drivers/net/bonding/bond_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 7019e5d8fdc8e..269384fadfa6f 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2123,7 +2123,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 		new_slave->link = BOND_LINK_DOWN;
 
 	new_slave->last_rx = jiffies -
-		(msecs_to_jiffies(bond->params.arp_interval) + 1);
+		(2 * msecs_to_jiffies(bond->params.arp_interval) + 1);
 	for (i = 0; i < BOND_MAX_ARP_TARGETS; i++)
 		new_slave->target_last_arp_rx[i] = new_slave->last_rx;
 

---
base-commit: f75062536e2d45ee182828f0e47e0124be2fdc8d
change-id: 20260817-bonding-last-rx-0303fa048f9d

Best regards,
--  
Ramses de Norre <ramses@well-founded.dev>

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

[Jay Vosburgh]

Ramses de Norre via B4 Relay <devnull+ramses.well-founded.dev@kernel.org> wrote:

>From: Ramses de Norre <ramses@well-founded.dev>
>
>bond_enslave() seeds a new slave's last_rx with a timestamp old enough
>that the ARP monitor does not consider the slave up until a real ARP
>reply arrives. That is the purpose of the seed, see commit f31c7937c254
>("bonding: start slaves with link down for ARP monitor").
>
>The seed is one arp_interval (plus a tick) in the past, which was just
>old enough when the test was "jiffies <= last_rx + arp_interval".
>Commit da210f559019 ("bonding: add some slack to arp monitoring time
>limits") added arp_interval/2 of slack to every such comparison without
>adjusting the seed, so the seed now sits inside the window instead of
>outside it. For the first half ARP interval after enslavement,
>bond_ab_arp_inspect() therefore sees "received recently" for a slave
>that has not received anything and proposes it UP. This happens for any
>freshly enslaved slave, independent of primary_reselect.
>
>When the slave is the configured primary, the still-armed force_primary
>then makes it the active slave. ARP validation fails it shortly after,
>since the link has not received anything (on SFP+ ports the PHY is
>often still negotiating), and the bond falls back to the backup. With
>primary_reselect=always the bond recovers once the port really comes
>up, but drops traffic until then. With primary_reselect=failure,
>force_primary has been spent and the bond stays on the backup.
>
>Reproducer:
>
>  ip link add bond0 type bond mode active-backup arp_interval 1000 \
>      arp_validate all arp_ip_target 192.0.2.1 \
>      primary eth0 primary_reselect failure
>  # eth0: SFP+ (slow link-up), eth1: RJ45 (fast link-up)
>  ip link set eth0 master bond0
>  ip link set eth1 master bond0
>  ip link set bond0 up
>  # bond0 lands on eth0 via force_primary, ARP-fails it before the
>  # SFP+ has carrier, falls to eth1, and stays there.
>
>Seed last_rx (and with it the per-target array and last_tx) two
>intervals in the past, so it is outside the slack window from the
>start. The initial link state is unchanged, it still comes from
>netif_carrier_ok().

	If the missed_max bonding parameter is set larger or smaller
than the default of 2, is your seed of two intervals still sufficient to
resolve the problem?

	-J

>Fixes: da210f559019 ("bonding: add some slack to arp monitoring time limits")
>Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>
>Signed-off-by: Ramses de Norre <ramses@well-founded.dev>
>---
>Changes in v3:
>- No code changes. Rebase on current net.
>- Reword the commit message along the lines of the v2 discussion: lead
>  with what the last_rx seed is for, state that the false UP happens
>  independent of primary_reselect, and that the initial link state is
>  unchanged.
>- Pick up Reviewed-by from Hangbin.
>- Cc Nikolay. Hangbin asked for a second review, Jay or Nikolay, would
>  you have a look?
>- Link to v2: https://patch.msgid.link/20260827-bonding-last-rx-v2-1-7c3833822c23@well-founded.dev
>
>Changes in v2:
>- No code changes. Resend with my personal name in the From and
>  Signed-off-by lines instead of my handle, as requested for the DCO.
>- Link to v1: https://patch.msgid.link/20260817-bonding-last-rx-v1-1-9da6f7fdf812@well-founded.dev
>---
> drivers/net/bonding/bond_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 7019e5d8fdc8e..269384fadfa6f 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -2123,7 +2123,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
> 		new_slave->link = BOND_LINK_DOWN;
> 
> 	new_slave->last_rx = jiffies -
>-		(msecs_to_jiffies(bond->params.arp_interval) + 1);
>+		(2 * msecs_to_jiffies(bond->params.arp_interval) + 1);
> 	for (i = 0; i < BOND_MAX_ARP_TARGETS; i++)
> 		new_slave->target_last_arp_rx[i] = new_slave->last_rx;
> 
>
>---
>base-commit: f75062536e2d45ee182828f0e47e0124be2fdc8d
>change-id: 20260817-bonding-last-rx-0303fa048f9d
>
>Best regards,
>--  
>Ramses de Norre <ramses@well-founded.dev>
>
>

---
	-Jay Vosburgh, jv@jvosburgh.net

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The patch ages last_tx along with last_rx (new_slave->last_tx
  = new_slave->last_rx), so last_tx is now two intervals old instead of…

--

bonding: fix initial last_rx vs ARP-monitor slack window

This commit moves the initial last_rx seed in bond_enslave() from one
arp_interval in the past to two. The goal is to keep the seed outside the
arp_interval/2 slack window that commit da210f559019 added. The
per-target array and last_tx get the same seed, because they are copied
from last_rx.

> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 7019e5d8fdc8e..269384fadfa6f 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2123,7 +2123,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
>  		new_slave->link = BOND_LINK_DOWN;
>  
>  	new_slave->last_rx = jiffies -
> -		(msecs_to_jiffies(bond->params.arp_interval) + 1);
> +		(2 * msecs_to_jiffies(bond->params.arp_interval) + 1);
>  	for (i = 0; i < BOND_MAX_ARP_TARGETS; i++)
>  		new_slave->target_last_arp_rx[i] = new_slave->last_rx;
>  

[Severity: Medium]
Just after this hunk, bond_enslave() copies the same value into last_tx:

	new_slave->last_tx = new_slave->last_rx;

So last_tx now also starts two intervals old. Could that cause a spurious
UP->DOWN transition in bond_loadbalance_arp_mon(), used by balance-rr,
balance-xor and broadcast with arp_interval set?

A slave enslaved with carrier OK starts as BOND_LINK_UP. On the first
arp_work pass, the UP branch checks last_tx before this slave's first
bond_send_validate() call in the same loop:

	if (!bond_time_in_interval(bond, last_tx,
				   bond->params.missed_max) ||
	    !bond_time_in_interval(bond, READ_ONCE(slave->last_rx),
				   bond->params.missed_max)) {

		bond_propose_link_state(slave, BOND_LINK_DOWN);
	...
	if (bond_slave_is_up(slave))
		bond_send_validate(bond, slave);

Unlike bond_ab_arp_inspect(), this path has no last_link_up grace. The
comment there says each slave is given "as long as it needs to get the
tx/rx within the delta".

bond_time_in_interval() accepts jiffies up to:

	last_act + mod * delta_in_ticks + delta_in_ticks/2

With d = arp_interval in jiffies, J0 = enslave time and the new seed
J0 - 2d - 1, the upper bound is:

  missed_max = 2 (default): J0 + d/2 - 1
  missed_max = 1:           J0 - d/2 - 1

With the old seed and missed_max = 2 the bound was J0 + 1.5d - 1, which a
first tick at or before J0 + d never crossed.

The arp_work tick phase does not depend on when the slave is enslaved.
With default settings, does that mean roughly half of enslavements into a
running load-balance bond see the slave marked down on the first tick,
before any probe has gone out? With missed_max = 1, would every carrier-up
slave be marked down on its first tick?

When that happens, link_failure_count is incremented, "interface is now
down" is logged, and do_failover is set if this is curr_active_slave. The
link only comes back on the next tick, after the first ARP has been sent.

Documentation/networking/bonding.rst describes arp_missed_max as:

	Specifies the number of arp_interval monitor checks that must
	fail in order for an interface to be marked down by the ARP monitor.

The commit message says:

  Seed last_rx (and with it the per-target array and last_tx) two
  intervals in the past, so it is outside the slack window from the
  start.

It does not mention the effect on the UP->DOWN checks in
bond_loadbalance_arp_mon(). Would it make sense to keep last_tx at its
previous seed, or to describe this behaviour change in the commit message?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261027-bonding-last-rx-v3-1-54907f1fb042%40well-founded.dev

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

[Ramses]

Oct 28, 2026, 13:25 by netdev-bot+sashiko@kernel.org:

>
> [Severity: Medium]
> Just after this hunk, bond_enslave() copies the same value into last_tx:
>
>  new_slave->last_tx = new_slave->last_rx;
>
> So last_tx now also starts two intervals old. Could that cause a spurious
> UP->DOWN transition in bond_loadbalance_arp_mon(), used by balance-rr,
> balance-xor and broadcast with arp_interval set?
>
>
This has already been fixed in v4 of this patch series which I sent this morning.

v4 only uses the older seed for a slave that starts DOWN, and patch 2 of v4 seeds a slave that starts UP with last_link_up, so last_tx gets a full arp_missed_max window too.

See: https://lore.kernel.org/netdev/20261028-bonding-last-rx-v4-0-4d8e4cd47467@well-founded.dev/

Ramses