[PATCH net v3] bonding: 3ad: select a port when the TX array is empty

[Yun Lu <luyun_611@163.com>]

From: Yun Lu <luyun@kylinos.cn>

This issue was observed on a machine in real use. During 802.3ad
bring-up in production, packets were dropped even though the bond's
carrier was up and LACP had enabled ports for distribution. The
transmit path only consults usable_slaves and returns no slave when
the array is NULL or its count is zero.

LACP port eligibility and the TX array are updated separately. The
state machine enables ports and updates carrier before scheduling
slave_arr_work to rebuild and publish the array. The worker requires
RTNL and retries if rtnl_trylock() fails, so RTNL contention can
prolong the interval in which eligible ports exist but the cached
array is empty.

Add a fallback for 802.3ad when the usable array is empty and carrier
is up. Walk the RCU-protected slave list and use ports whose
aggregator is active and which pass bond_slave_can_tx(), without
taking mode_lock or changing LACP state. In 802.3ad mode the
per-slave active flag is set only by the mux machine for ports of the
active aggregator, so these lockless reads identify distributable
ports with the same criteria bond_update_slave_arr() applies when
building the array.

Fix the aggregator object at the first eligible port so counting and
selection use members of the same object. The object pointer is used
only for comparison within the caller's RCU section. Count eligible
members, then select with the existing hash modulo that count in
slave-list order. This preserves the array path's mapping when the
eligible set is stable. Recheck eligibility in the second pass and
use the first member of the pinned object if the selected index is no
longer reachable. Ports of any other aggregator object are never
used; the fallback drops the packet instead.

Use the fallback for skb transmission and ndo_get_xmit_slave queries
with all_slaves=false. For broadcast_neighbor ARP/ND packets, replicate
to all eligible members of the selected object. Keep the nonempty array
selection unchanged and defer hashing until an eligible set is found.

Persistent empty states with carrier up still incur one lockless list
walk per dropped packet. XDP transmit and ndo_sk_get_lower_dev use
separate frame and socket hashing interfaces. Their fallback handling
needs separate implementation and validation, so this fix leaves their
array-based selection unchanged.

Tested on x86-64 KVM against a two-port LACP peer. With array
publication deferred by a test-only gate, UDP, ARP and ND all left
through ports selected by the fallback while both arrays were NULL,
and the same UDP flows kept their port mapping after the array was
published. All 16 bonding selftests passed without the test-only
modifications.

Fixes: ee6377147409 ("bonding: Simplify the xmit function for modes that use xmit_hash")
Signed-off-by: Yun Lu <luyun@kylinos.cn>
---
Changes in v3:
- Drop the pending latch and all TX-path locking (the mode_lock trylock
  and the netpoll guard); the fallback is now lockless.
- Pin one aggregator object per transmit, by pointer comparison within
  the RCU section, for both hash selection and broadcast replication.
  Never select across aggregator objects.
- Replicate broadcast_neighbor packets to all eligible members of the
  pinned object instead of sending a single copy.
- Defer hashing until the fallback finds a non-empty eligible set.
- Read aggregator is_active with READ_ONCE.

v2: https://lore.kernel.org/all/20260918090137.66515-1-luyun_611@163.com/

Changes in v2:
- Track unpublished array updates with a boolean latch, limiting fallback
  locking and list walks to the asynchronous update window.
- Use spin_trylock_bh() so recursive TX keeps the existing drop or NULL
  behavior instead of deadlocking on mode_lock.
- Use the first eligible port if eligibility changes between the two list
  walks.
- Apply the fallback to ndo_get_xmit_slave and broadcast-neighbor skb
  transmission. Keep control-path readers on the published array and leave
  XDP for a separate follow-up patch.
- Document why hash calculation is deferred until the array count is known.

v1: https://lore.kernel.org/netdev/20260915090207.110987-1-luyun_611@163.com/

Testing: two-port LACP bring-up on x86-64 KVM, injected state changes,
and an unsynchronized peer with no eligible port. A test-only kernel
modification deferred usable_slaves and all_slaves publication from bond
creation; with both arrays NULL after convergence, UDP used one eligible
port and ARP/ND reached both. Releasing the gate and repeating the same
UDP flows preserved port mapping. Test-only hooks injected eligibility
shrink and active aggregator changes between selection passes and during
broadcast; these synthetic cases passed.

All 16 bonding selftests passed without the test-only modifications.

At 32 ports, paired KVM measurements showed about 0.4 us/attempt
additional thread CPU time in the persistent-empty-array case. Timings
include userspace overhead.

 drivers/net/bonding/bond_main.c | 93 +++++++++++++++++++++++++++++++--
 1 file changed, 89 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index de2489c3d9bf..2fed71d7586b 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -5233,11 +5233,11 @@ static struct slave *bond_xmit_3ad_xor_slave_get(struct bonding *bond,
 	unsigned int count;
 	u32 hash;
 
-	hash = bond_xmit_hash(bond, skb);
 	count = slaves ? READ_ONCE(slaves->count) : 0;
 	if (unlikely(!count))
 		return NULL;
 
+	hash = bond_xmit_hash(bond, skb);
 	slave = slaves->arr[hash % count];
 	return slave;
 }
@@ -5289,9 +5289,60 @@ static bool bond_should_broadcast_neighbor(struct sk_buff *skb,
 	return false;
 }
 
-/* Use this Xmit function for 3AD as well as XOR modes. The current
- * usable slave array is formed in the control path. The xmit function
- * just calculates hash and sends the packet out.
+/* Called with RCU held and only in 802.3ad mode. */
+static const struct aggregator *bond_3ad_tx_eligible_agg(struct slave *slave)
+{
+	const struct aggregator *agg;
+
+	agg = rcu_dereference(SLAVE_AD_INFO(slave)->port.aggregator);
+	if (!agg || !READ_ONCE(agg->is_active) || !bond_slave_can_tx(slave))
+		return NULL;
+
+	return agg;
+}
+
+/* Called with RCU held when the usable slave array is empty. */
+static struct slave *bond_3ad_xmit_fallback(struct bonding *bond,
+					    struct sk_buff *skb)
+{
+	const struct aggregator *agg = NULL, *slave_agg;
+	struct slave *slave, *first = NULL;
+	struct list_head *iter;
+	unsigned int count = 0, target;
+
+	if (!netif_carrier_ok(bond->dev))
+		return NULL;
+
+	bond_for_each_slave_rcu(bond, slave, iter) {
+		slave_agg = bond_3ad_tx_eligible_agg(slave);
+		if (!slave_agg)
+			continue;
+		if (!agg)
+			agg = slave_agg;
+		if (slave_agg == agg)
+			count++;
+	}
+
+	if (!count)
+		return NULL;
+
+	target = bond_xmit_hash(bond, skb) % count;
+	bond_for_each_slave_rcu(bond, slave, iter) {
+		if (bond_3ad_tx_eligible_agg(slave) != agg)
+			continue;
+		if (!first)
+			first = slave;
+		if (!target)
+			return slave;
+		target--;
+	}
+
+	return first;
+}
+
+/* Use this Xmit function for 3AD as well as XOR modes. The usable
+ * slave array is formed in the control path; 3AD can use current state
+ * while the array is empty.
  */
 static netdev_tx_t bond_3ad_xor_xmit(struct sk_buff *skb,
 				     struct net_device *dev)
@@ -5302,6 +5353,8 @@ static netdev_tx_t bond_3ad_xor_xmit(struct sk_buff *skb,
 
 	slaves = rcu_dereference(bond->usable_slaves);
 	slave = bond_xmit_3ad_xor_slave_get(bond, skb, slaves);
+	if (unlikely(!slave) && BOND_MODE(bond) == BOND_MODE_8023AD)
+		slave = bond_3ad_xmit_fallback(bond, skb);
 	if (likely(slave))
 		return bond_dev_queue_xmit(bond, skb, slave->dev);
 
@@ -5327,6 +5380,35 @@ static netdev_tx_t bond_xmit_broadcast(struct sk_buff *skb,
 		slaves = rcu_dereference(bond->usable_slaves);
 
 	slaves_count = slaves ? READ_ONCE(slaves->count) : 0;
+	if (!slaves_count && !all_slaves &&
+	    BOND_MODE(bond) == BOND_MODE_8023AD &&
+	    netif_carrier_ok(bond_dev)) {
+		const struct aggregator *agg = NULL, *slave_agg;
+		struct list_head *iter;
+		struct slave *slave;
+
+		bond_for_each_slave_rcu(bond, slave, iter) {
+			struct sk_buff *skb2;
+
+			slave_agg = bond_3ad_tx_eligible_agg(slave);
+			if (!slave_agg)
+				continue;
+			if (!agg)
+				agg = slave_agg;
+			if (slave_agg != agg)
+				continue;
+
+			skb2 = skb_clone(skb, GFP_ATOMIC);
+			if (!skb2) {
+				net_err_ratelimited("%s: Error: %s: skb_clone() failed\n",
+						    bond_dev->name, __func__);
+				continue;
+			}
+
+			if (bond_dev_queue_xmit(bond, skb2, slave->dev) == NETDEV_TX_OK)
+				xmit_suc = true;
+		}
+	}
 	for (i = 0; i < slaves_count; i++) {
 		struct slave *slave = slaves->arr[i];
 		struct sk_buff *skb2;
@@ -5432,6 +5514,9 @@ static struct net_device *bond_xmit_get_slave(struct net_device *master_dev,
 		else
 			slaves = rcu_dereference(bond->usable_slaves);
 		slave = bond_xmit_3ad_xor_slave_get(bond, skb, slaves);
+		if (unlikely(!slave) && !all_slaves &&
+		    BOND_MODE(bond) == BOND_MODE_8023AD)
+			slave = bond_3ad_xmit_fallback(bond, skb);
 		break;
 	case BOND_MODE_BROADCAST:
 		break;

base-commit: 6e0022b5ae3dc5b833af0fcc1578dc20a1d6bc71
-- 
2.43.0