[PATCH net v4 0/2] bonding: fix the initial last_rx seed for the ARP monitor

[PATCH net v4 0/2] bonding: fix the initial last_rx seed for the ARP monitor

[Ramses de Norre via B4 Relay]

bond_enslave() seeds a new slave's last_rx before it has received
anything. The ARP monitor checks that seed with an up test for a slave
that starts DOWN, and with a down test for a slave that starts UP. The
seed is wrong for both.

Patch 1 fixes a slave that starts DOWN. Since the up test gained
arp_interval/2 of slack, the seed falls inside the up window, so the
slave is reported UP for half an interval without having received
anything.

Patch 2 fixes a slave that starts UP. The seed brings its down deadline
forward by one interval compared to what arp_missed_max implies. In
load-balance mode, which has no grace period after enslavement, a
healthy slave can flap DOWN with arp_missed_max=1.

Jay, on your v3 question: the up test always uses a single interval, so
the fix for a slave that starts DOWN holds for any arp_missed_max. But
v3 also changed the seed of a slave that starts UP, which made the
load-balance flap happen with the default arp_missed_max=2 too. v4 only
uses the older seed for a slave that starts DOWN, and patch 2 gives a
slave that starts UP a full arp_missed_max window.

Signed-off-by: Ramses de Norre <ramses@well-founded.dev>
---
Changes in v4:
- Patch 1: only use the older seed for a slave that starts DOWN. v3
  also applied it to a slave that starts UP, which brought its down
  deadline forward by one interval (Jay).
- New patch 2: seed a slave that starts UP with last_link_up, giving it
  a full arp_missed_max window. Fixes an existing flap in load-balance
  mode with arp_missed_max=1.
- Drop Hangbin's Reviewed-by, patch 1 changed.
- Link to v3: https://patch.msgid.link/20261027-bonding-last-rx-v3-1-54907f1fb042@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

---
Ramses de Norre (2):
      bonding: fix initial last_rx vs ARP-monitor slack window
      bonding: give a slave that starts UP a full arp_missed_max window

 drivers/net/bonding/bond_main.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)
---
base-commit: f75062536e2d45ee182828f0e47e0124be2fdc8d
change-id: 20260817-bonding-last-rx-0303fa048f9d

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

[PATCH net v4 1/2] 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 so that the ARP monitor does
not report it UP before a real ARP reply arrives, see commit
f31c7937c254 ("bonding: start slaves with link down for ARP monitor").

The seed is one arp_interval (plus a tick) in the past. That was old
enough when the up 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 that test without moving the seed, so
the seed now falls inside the up window. For the first half interval
after enslavement, bond_ab_arp_inspect() proposes a slave that starts
DOWN as UP although it has received nothing. The up test always uses a
single interval, so this does not depend on primary_reselect or
arp_missed_max.

If that slave is the configured primary, force_primary makes it the
active slave. ARP validation fails it shortly after (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 is 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.

Move the seed below the initial link state and seed a slave that starts
DOWN two intervals in the past, outside the up window. A slave that
starts UP keeps its current seed: the up test does not apply to it, and
an older seed would only bring its down deadline forward by one
interval. The initial link state is unchanged.

Fixes: da210f559019 ("bonding: add some slack to arp monitoring time limits")
Signed-off-by: Ramses de Norre <ramses@well-founded.dev>
---
 drivers/net/bonding/bond_main.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 7019e5d8fdc8e..bfa4cb97e647c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2122,13 +2122,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 	if (res && bond_needs_speed_duplex(bond))
 		new_slave->link = BOND_LINK_DOWN;
 
-	new_slave->last_rx = jiffies -
-		(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;
-
-	new_slave->last_tx = new_slave->last_rx;
-
 	/* check for initial state */
 	new_slave->link = BOND_LINK_NOCHANGE;
 	if (bond->params.miimon) {
@@ -2159,6 +2152,22 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 
 	if (new_slave->link != BOND_LINK_DOWN)
 		new_slave->last_link_up = jiffies;
+
+	/* The ARP monitor's up test includes arp_interval/2 of slack, see
+	 * bond_time_in_interval(). Seed a slave that starts DOWN outside
+	 * that window, so it stays DOWN until a real reply arrives.
+	 */
+	if (new_slave->link == BOND_LINK_DOWN)
+		new_slave->last_rx = jiffies -
+			(2 * msecs_to_jiffies(bond->params.arp_interval) + 1);
+	else
+		new_slave->last_rx = jiffies -
+			(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;
+
+	new_slave->last_tx = new_slave->last_rx;
+
 	slave_dbg(bond_dev, slave_dev, "Initial state of slave is BOND_LINK_%s\n",
 		  new_slave->link == BOND_LINK_DOWN ? "DOWN" :
 		  (new_slave->link == BOND_LINK_UP ? "UP" : "BACK"));

-- 
2.56.0

Re: [PATCH net v4 1/2] bonding: fix initial last_rx vs ARP-monitor slack window

[Hangbin Liu]

On Wed, Oct 28, 2026 at 12:48:07PM +0100, Ramses de Norre via B4 Relay wrote:
> From: Ramses de Norre <ramses@well-founded.dev>
> 
> bond_enslave() seeds a new slave's last_rx so that the ARP monitor does
> not report it UP before a real ARP reply arrives, see commit
> f31c7937c254 ("bonding: start slaves with link down for ARP monitor").
> 
> The seed is one arp_interval (plus a tick) in the past. That was old
> enough when the up 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 that test without moving the seed, so
> the seed now falls inside the up window. For the first half interval
> after enslavement, bond_ab_arp_inspect() proposes a slave that starts
> DOWN as UP although it has received nothing. The up test always uses a
> single interval, so this does not depend on primary_reselect or
> arp_missed_max.
> 
> If that slave is the configured primary, force_primary makes it the
> active slave. ARP validation fails it shortly after (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 is 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.
> 
> Move the seed below the initial link state and seed a slave that starts
> DOWN two intervals in the past, outside the up window. A slave that
> starts UP keeps its current seed: the up test does not apply to it, and
> an older seed would only bring its down deadline forward by one
> interval. The initial link state is unchanged.
> 
> Fixes: da210f559019 ("bonding: add some slack to arp monitoring time limits")
> Signed-off-by: Ramses de Norre <ramses@well-founded.dev>
> ---
>  drivers/net/bonding/bond_main.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 7019e5d8fdc8e..bfa4cb97e647c 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2122,13 +2122,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
>  	if (res && bond_needs_speed_duplex(bond))
>  		new_slave->link = BOND_LINK_DOWN;
>  
> -	new_slave->last_rx = jiffies -
> -		(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;
> -
> -	new_slave->last_tx = new_slave->last_rx;
> -
>  	/* check for initial state */
>  	new_slave->link = BOND_LINK_NOCHANGE;
>  	if (bond->params.miimon) {
> @@ -2159,6 +2152,22 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
>  
>  	if (new_slave->link != BOND_LINK_DOWN)
>  		new_slave->last_link_up = jiffies;

Nit: This part could move down and merge with the "else" part.

> +
> +	/* The ARP monitor's up test includes arp_interval/2 of slack, see
> +	 * bond_time_in_interval(). Seed a slave that starts DOWN outside
> +	 * that window, so it stays DOWN until a real reply arrives.
> +	 */
> +	if (new_slave->link == BOND_LINK_DOWN)
> +		new_slave->last_rx = jiffies -
> +			(2 * msecs_to_jiffies(bond->params.arp_interval) + 1);
> +	else
> +		new_slave->last_rx = jiffies -
> +			(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;
> +
> +	new_slave->last_tx = new_slave->last_rx;
> +
>  	slave_dbg(bond_dev, slave_dev, "Initial state of slave is BOND_LINK_%s\n",
>  		  new_slave->link == BOND_LINK_DOWN ? "DOWN" :
>  		  (new_slave->link == BOND_LINK_UP ? "UP" : "BACK"));
> 
> -- 
> 2.56.0
> 
> 

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

[PATCH net v4 2/2] bonding: give a slave that starts UP a full arp_missed_max window

[Ramses de Norre via B4 Relay]

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

The ARP monitor marks an UP slave DOWN once jiffies passes
last_rx + arp_missed_max * arp_interval + arp_interval/2. bond_enslave()
seeds last_rx one arp_interval (plus a tick) in the past, which brings
the down deadline of a slave that starts UP forward by one interval
compared to what arp_missed_max promises.

Active-backup mode hides this with a grace period after enslavement in
bond_ab_arp_inspect(). Load-balance mode has no grace period after
enslavement. With arp_missed_max=1 the deadline is half an interval
after enslavement, and the first monitor run checks the slave before it
sends the first probe. If that run lands in the second half of the
interval, a healthy slave is reported "interface is now down" and comes
back UP on the next run.

Seed a slave that starts UP with last_link_up instead, so it gets a full
arp_missed_max window for its first reply. bond_arp_rcv() and
bond_na_rcv() only trust ARPs seen on backup slaves once the active
slave's last_rx is after its last_link_up. A seed equal to last_link_up
keeps that check false until a real reply arrives, as it is today.

Fixes: 5944b5abd864 ("Bonding: add arp_missed_max option")
Signed-off-by: Ramses de Norre <ramses@well-founded.dev>
---
 drivers/net/bonding/bond_main.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index bfa4cb97e647c..de5b196f4a1ee 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2160,9 +2160,12 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 	if (new_slave->link == BOND_LINK_DOWN)
 		new_slave->last_rx = jiffies -
 			(2 * msecs_to_jiffies(bond->params.arp_interval) + 1);
+	/* Give a slave that starts UP a full arp_missed_max window. Not
+	 * seeding it later than last_link_up keeps bond_arp_rcv() from
+	 * trusting ARPs on backup slaves before the active one got a reply.
+	 */
 	else
-		new_slave->last_rx = jiffies -
-			(msecs_to_jiffies(bond->params.arp_interval) + 1);
+		new_slave->last_rx = new_slave->last_link_up;
 	for (i = 0; i < BOND_MAX_ARP_TARGETS; i++)
 		new_slave->target_last_arp_rx[i] = new_slave->last_rx;
 

-- 
2.56.0