[PATCH net-next 0/4] add broadcast_neighbor for no-stacking networking arch

[PATCH net-next 0/4] add broadcast_neighbor for no-stacking networking arch

[tonghao]

From: Tonghao Zhang <tonghao@bamaicloud.com>

For no-stacking networking arch, and enable the bond mode 4(lacp) in
datacenter, the switch group require arp/nd packets as session
synchronization. More details please see patch.

Cc: Jay Vosburgh <jv@jvosburgh.net>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Andrew Lunn <andrew+netdev@lunn.ch>

Tonghao Zhang (4):
  net: bonding: add broadcast_neighbor option for 802.3ad
  net: bonding: add broadcast_neighbor netlink option
  net: bonding: send peer notify when failure recovery
  net: bonding: add tracepoint for 802.3ad

 Documentation/networking/bonding.rst |  5 +++
 drivers/net/bonding/bond_3ad.c       | 20 ++++++++++
 drivers/net/bonding/bond_main.c      | 58 +++++++++++++++++++++-------
 drivers/net/bonding/bond_netlink.c   | 16 ++++++++
 drivers/net/bonding/bond_options.c   | 25 ++++++++++++
 drivers/net/bonding/bond_sysfs.c     | 18 +++++++++
 include/net/bond_options.h           |  1 +
 include/net/bonding.h                |  1 +
 include/trace/events/bonding.h       | 37 ++++++++++++++++++
 include/uapi/linux/if_link.h         |  1 +
 10 files changed, 168 insertions(+), 14 deletions(-)
 create mode 100644 include/trace/events/bonding.h

-- 
2.34.1

[PATCH net-next 1/4] net: bonding: add broadcast_neighbor option for 802.3ad

[tonghao]

From: Tonghao Zhang <tonghao@bamaicloud.com>

Stacking technology provides technical benefits but has inherent drawbacks.
For instance, switch software or system upgrades require simultaneous reboots
of all stacked switches. Additionally, stacking link failures may cause
stack splitting.

To improve network stability, non-stacking solutions have been increasingly
adopted, particularly by public cloud providers and technology companies
like Alibaba, Tencent, and Didi. The server still uses dual network cards and
dual uplinks to two switches, and the network card mode is set to
bond mode 4 (IEEE 802.3ad). As aggregation ports transmit ARP/ND data
exclusively through one physical port, both switches in non-stacking
deployments must receive server ARP/ND requests. This requires bonding driver
modifications to broadcast ARP/ND packets through all active slave links.

- https://www.ruijie.com/fr-fr/support/tech-gallery/de-stack-data-center-network-architecture/

Cc: Jay Vosburgh <jv@jvosburgh.net>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Andrew Lunn <andrew+netdev@lunn.ch>
Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
---
 Documentation/networking/bonding.rst |  5 +++
 drivers/net/bonding/bond_main.c      | 58 +++++++++++++++++++++-------
 drivers/net/bonding/bond_options.c   | 25 ++++++++++++
 drivers/net/bonding/bond_sysfs.c     | 18 +++++++++
 include/net/bond_options.h           |  1 +
 include/net/bonding.h                |  1 +
 6 files changed, 94 insertions(+), 14 deletions(-)

diff --git a/Documentation/networking/bonding.rst b/Documentation/networking/bonding.rst
index a4c1291d2561..0aca6e7599db 100644
--- a/Documentation/networking/bonding.rst
+++ b/Documentation/networking/bonding.rst
@@ -562,6 +562,11 @@ lacp_rate
 
 	The default is slow.
 
+broadcast_neighbor
+
+    Option specifying whether to broadcast ARP/ND packets to all
+    active slaves. The default is off (0).
+
 max_bonds
 
 	Specifies the number of bonding devices to create for this
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index d05226484c64..c54bfba10688 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -5316,23 +5316,31 @@ static struct slave *bond_xdp_xmit_3ad_xor_slave_get(struct bonding *bond,
 	return slaves->arr[hash % count];
 }
 
-/* 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.
- */
-static netdev_tx_t bond_3ad_xor_xmit(struct sk_buff *skb,
-				     struct net_device *dev)
+static bool bond_should_broadcast_neighbor(struct bonding *bond,
+					   struct sk_buff *skb)
 {
-	struct bonding *bond = netdev_priv(dev);
-	struct bond_up_slave *slaves;
-	struct slave *slave;
+	if (BOND_MODE(bond) != BOND_MODE_8023AD)
+		return false;
 
-	slaves = rcu_dereference(bond->usable_slaves);
-	slave = bond_xmit_3ad_xor_slave_get(bond, skb, slaves);
-	if (likely(slave))
-		return bond_dev_queue_xmit(bond, skb, slave->dev);
+	if (!bond->params.broadcast_neighbor)
+		return false;
 
-	return bond_tx_drop(dev, skb);
+	if (skb->protocol == htons(ETH_P_ARP))
+		return true;
+
+        if (skb->protocol == htons(ETH_P_IPV6) &&
+            pskb_may_pull(skb,
+                          sizeof(struct ipv6hdr) + sizeof(struct icmp6hdr))) {
+                if (ipv6_hdr(skb)->nexthdr == IPPROTO_ICMPV6) {
+                        struct icmp6hdr *icmph = icmp6_hdr(skb);
+
+                        if ((icmph->icmp6_type == NDISC_NEIGHBOUR_SOLICITATION) ||
+                            (icmph->icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT))
+                                return true;
+                }
+        }
+
+        return false;
 }
 
 /* in broadcast mode, we send everything to all usable interfaces. */
@@ -5377,6 +5385,28 @@ static netdev_tx_t bond_xmit_broadcast(struct sk_buff *skb,
 	return NET_XMIT_DROP;
 }
 
+/* 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.
+ */
+static netdev_tx_t bond_3ad_xor_xmit(struct sk_buff *skb,
+				     struct net_device *dev)
+{
+	struct bonding *bond = netdev_priv(dev);
+	struct bond_up_slave *slaves;
+	struct slave *slave;
+
+	if (bond_should_broadcast_neighbor(bond, skb))
+		return bond_xmit_broadcast(skb, dev);
+
+	slaves = rcu_dereference(bond->usable_slaves);
+	slave = bond_xmit_3ad_xor_slave_get(bond, skb, slaves);
+	if (likely(slave))
+		return bond_dev_queue_xmit(bond, skb, slave->dev);
+
+	return bond_tx_drop(dev, skb);
+}
+
 /*------------------------- Device initialization ---------------------------*/
 
 /* Lookup the slave that corresponds to a qid */
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 91893c29b899..38e8f03d1707 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -87,6 +87,8 @@ static int bond_option_missed_max_set(struct bonding *bond,
 				      const struct bond_opt_value *newval);
 static int bond_option_coupled_control_set(struct bonding *bond,
 					   const struct bond_opt_value *newval);
+static int bond_option_broadcast_neigh_set(struct bonding *bond,
+					   const struct bond_opt_value *newval);
 
 static const struct bond_opt_value bond_mode_tbl[] = {
 	{ "balance-rr",    BOND_MODE_ROUNDROBIN,   BOND_VALFLAG_DEFAULT},
@@ -240,6 +242,12 @@ static const struct bond_opt_value bond_coupled_control_tbl[] = {
 	{ NULL,  -1, 0},
 };
 
+static const struct bond_opt_value bond_broadcast_neigh_tbl[] = {
+	{ "on",	 1, 0},
+	{ "off", 0, BOND_VALFLAG_DEFAULT},
+	{ NULL,  -1, 0}
+};
+
 static const struct bond_option bond_opts[BOND_OPT_LAST] = {
 	[BOND_OPT_MODE] = {
 		.id = BOND_OPT_MODE,
@@ -513,6 +521,14 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
 		.flags = BOND_OPTFLAG_IFDOWN,
 		.values = bond_coupled_control_tbl,
 		.set = bond_option_coupled_control_set,
+	},
+	[BOND_OPT_BROADCAST_NEIGH] = {
+		.id = BOND_OPT_BROADCAST_NEIGH,
+		.name = "broadcast_neighbor",
+		.desc = "Broadcast neighbor packets to all slaves",
+		.unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
+		.values = bond_broadcast_neigh_tbl,
+		.set = bond_option_broadcast_neigh_set,
 	}
 };
 
@@ -1840,3 +1856,12 @@ static int bond_option_coupled_control_set(struct bonding *bond,
 	bond->params.coupled_control = newval->value;
 	return 0;
 }
+
+static int bond_option_broadcast_neigh_set(struct bonding *bond,
+					   const struct bond_opt_value *newval)
+{
+	netdev_dbg(bond->dev, "Setting broadcast_neighbor to %llu\n",
+		   newval->value);
+	bond->params.broadcast_neighbor = newval->value;
+	return 0;
+}
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 1e13bb170515..76f2a1bf57c2 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -752,6 +752,23 @@ static ssize_t bonding_show_ad_user_port_key(struct device *d,
 static DEVICE_ATTR(ad_user_port_key, 0644,
 		   bonding_show_ad_user_port_key, bonding_sysfs_store_option);
 
+static ssize_t bonding_show_broadcast_neighbor(struct device *d,
+					       struct device_attribute *attr,
+					       char *buf)
+{
+	struct bonding *bond = to_bond(d);
+	const struct bond_opt_value *val;
+
+	val = bond_opt_get_val(BOND_OPT_BROADCAST_NEIGH,
+			bond->params.broadcast_neighbor);
+
+	return sysfs_emit(buf, "%s %d\n", val->string,
+			bond->params.broadcast_neighbor);
+}
+
+static DEVICE_ATTR(broadcast_neighbor, 0644,
+		   bonding_show_broadcast_neighbor, bonding_sysfs_store_option);
+
 static struct attribute *per_bond_attrs[] = {
 	&dev_attr_slaves.attr,
 	&dev_attr_mode.attr,
@@ -791,6 +808,7 @@ static struct attribute *per_bond_attrs[] = {
 	&dev_attr_ad_actor_system.attr,
 	&dev_attr_ad_user_port_key.attr,
 	&dev_attr_arp_missed_max.attr,
+	&dev_attr_broadcast_neighbor.attr,
 	NULL,
 };
 
diff --git a/include/net/bond_options.h b/include/net/bond_options.h
index 18687ccf0638..022b122a9fb6 100644
--- a/include/net/bond_options.h
+++ b/include/net/bond_options.h
@@ -77,6 +77,7 @@ enum {
 	BOND_OPT_NS_TARGETS,
 	BOND_OPT_PRIO,
 	BOND_OPT_COUPLED_CONTROL,
+	BOND_OPT_BROADCAST_NEIGH,
 	BOND_OPT_LAST
 };
 
diff --git a/include/net/bonding.h b/include/net/bonding.h
index 95f67b308c19..1eafd15eaad9 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -149,6 +149,7 @@ struct bond_params {
 	struct in6_addr ns_targets[BOND_MAX_NS_TARGETS];
 #endif
 	int coupled_control;
+	int broadcast_neighbor;
 
 	/* 2 bytes of padding : see ether_addr_equal_64bits() */
 	u8 ad_actor_system[ETH_ALEN + 2];
-- 
2.34.1

Re: [PATCH net-next 1/4] net: bonding: add broadcast_neighbor option for 802.3ad

[Jay Vosburgh]

tonghao@bamaicloud.com wrote:

>From: Tonghao Zhang <tonghao@bamaicloud.com>
>
>Stacking technology provides technical benefits but has inherent drawbacks.
>For instance, switch software or system upgrades require simultaneous reboots
>of all stacked switches. Additionally, stacking link failures may cause
>stack splitting.
>
>To improve network stability, non-stacking solutions have been increasingly
>adopted, particularly by public cloud providers and technology companies
>like Alibaba, Tencent, and Didi. The server still uses dual network cards and
>dual uplinks to two switches, and the network card mode is set to
>bond mode 4 (IEEE 802.3ad). As aggregation ports transmit ARP/ND data
>exclusively through one physical port, both switches in non-stacking
>deployments must receive server ARP/ND requests. This requires bonding driver
>modifications to broadcast ARP/ND packets through all active slave links.
>
>- https://www.ruijie.com/fr-fr/support/tech-gallery/de-stack-data-center-network-architecture/

	I didn't really follow the explanation here without reading the
linked article.  I think it would be better to explain the basics of
what this "non-stacking" architecture is, then describe the change to
bonding necessary to support it.  This description need not go into
great detail; assuming I understand correctly, perhaps something along
the lines of:

	"non-stacking" is a method of mimicing switch stacking that
convinces a LACP peer, bonding in this case, connected to a set of
"non-stacked" switches that all of its ports are connected to a single
switch (i.e., LACP aggregator), as if those switches were stacked.  This
enables the LACP peer's ports to aggregate together, and requires (a)
special switch configuration, described in the linked article, and (b)
modifications to the bonding 802.3ad (LACP) mode to send all ARP / ND
packets across all ports of the active aggregator.

	Is that a fair summary?

	Regardless, the commit message should stand on its own, even if
the linked article is gone at some point in the future.

>Cc: Jay Vosburgh <jv@jvosburgh.net>
>Cc: "David S. Miller" <davem@davemloft.net>
>Cc: Eric Dumazet <edumazet@google.com>
>Cc: Jakub Kicinski <kuba@kernel.org>
>Cc: Paolo Abeni <pabeni@redhat.com>
>Cc: Simon Horman <horms@kernel.org>
>Cc: Jonathan Corbet <corbet@lwn.net>
>Cc: Andrew Lunn <andrew+netdev@lunn.ch>
>Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
>---
> Documentation/networking/bonding.rst |  5 +++
> drivers/net/bonding/bond_main.c      | 58 +++++++++++++++++++++-------
> drivers/net/bonding/bond_options.c   | 25 ++++++++++++
> drivers/net/bonding/bond_sysfs.c     | 18 +++++++++
> include/net/bond_options.h           |  1 +
> include/net/bonding.h                |  1 +
> 6 files changed, 94 insertions(+), 14 deletions(-)
>
>diff --git a/Documentation/networking/bonding.rst b/Documentation/networking/bonding.rst
>index a4c1291d2561..0aca6e7599db 100644
>--- a/Documentation/networking/bonding.rst
>+++ b/Documentation/networking/bonding.rst
>@@ -562,6 +562,11 @@ lacp_rate
> 
> 	The default is slow.
> 
>+broadcast_neighbor
>+
>+    Option specifying whether to broadcast ARP/ND packets to all
>+    active slaves. The default is off (0).
>+

	I'm happy to see documentation updates; however, please add the
caveat that this option has no effect in modes other than 802.3ad mode.
Also, the text is not formatted consistently with the options around it
(block indented one tab).

> max_bonds
> 
> 	Specifies the number of bonding devices to create for this
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index d05226484c64..c54bfba10688 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -5316,23 +5316,31 @@ static struct slave *bond_xdp_xmit_3ad_xor_slave_get(struct bonding *bond,
> 	return slaves->arr[hash % count];
> }
> 
>-/* 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.
>- */
>-static netdev_tx_t bond_3ad_xor_xmit(struct sk_buff *skb,
>-				     struct net_device *dev)
>+static bool bond_should_broadcast_neighbor(struct bonding *bond,
>+					   struct sk_buff *skb)
> {
>-	struct bonding *bond = netdev_priv(dev);
>-	struct bond_up_slave *slaves;
>-	struct slave *slave;
>+	if (BOND_MODE(bond) != BOND_MODE_8023AD)
>+		return false;
> 
>-	slaves = rcu_dereference(bond->usable_slaves);
>-	slave = bond_xmit_3ad_xor_slave_get(bond, skb, slaves);
>-	if (likely(slave))
>-		return bond_dev_queue_xmit(bond, skb, slave->dev);
>+	if (!bond->params.broadcast_neighbor)
>+		return false;
> 
>-	return bond_tx_drop(dev, skb);
>+	if (skb->protocol == htons(ETH_P_ARP))
>+		return true;
>+
>+        if (skb->protocol == htons(ETH_P_IPV6) &&
>+            pskb_may_pull(skb,
>+                          sizeof(struct ipv6hdr) + sizeof(struct icmp6hdr))) {
>+                if (ipv6_hdr(skb)->nexthdr == IPPROTO_ICMPV6) {
>+                        struct icmp6hdr *icmph = icmp6_hdr(skb);
>+
>+                        if ((icmph->icmp6_type == NDISC_NEIGHBOUR_SOLICITATION) ||
>+                            (icmph->icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT))
>+                                return true;
>+                }
>+        }
>+
>+        return false;
> }
> 
> /* in broadcast mode, we send everything to all usable interfaces. */
>@@ -5377,6 +5385,28 @@ static netdev_tx_t bond_xmit_broadcast(struct sk_buff *skb,
> 	return NET_XMIT_DROP;
> }
> 
>+/* 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.
>+ */
>+static netdev_tx_t bond_3ad_xor_xmit(struct sk_buff *skb,
>+				     struct net_device *dev)
>+{
>+	struct bonding *bond = netdev_priv(dev);
>+	struct bond_up_slave *slaves;
>+	struct slave *slave;
>+
>+	if (bond_should_broadcast_neighbor(bond, skb))
>+		return bond_xmit_broadcast(skb, dev);

	I feel like there has to be a way to implement this that won't
add two or three branches to the transmit fast path for every packet
when this option is not enabled (which will be the vast majority of
cases).  I'm not sure what that would look like, needs some thought.

	Is the call to bond_should_broadcast_neighbor inlined at compile
time?

	-J

>+
>+	slaves = rcu_dereference(bond->usable_slaves);
>+	slave = bond_xmit_3ad_xor_slave_get(bond, skb, slaves);
>+	if (likely(slave))
>+		return bond_dev_queue_xmit(bond, skb, slave->dev);
>+
>+	return bond_tx_drop(dev, skb);
>+}
>+
> /*------------------------- Device initialization ---------------------------*/
> 
> /* Lookup the slave that corresponds to a qid */
>diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>index 91893c29b899..38e8f03d1707 100644
>--- a/drivers/net/bonding/bond_options.c
>+++ b/drivers/net/bonding/bond_options.c
>@@ -87,6 +87,8 @@ static int bond_option_missed_max_set(struct bonding *bond,
> 				      const struct bond_opt_value *newval);
> static int bond_option_coupled_control_set(struct bonding *bond,
> 					   const struct bond_opt_value *newval);
>+static int bond_option_broadcast_neigh_set(struct bonding *bond,
>+					   const struct bond_opt_value *newval);
> 
> static const struct bond_opt_value bond_mode_tbl[] = {
> 	{ "balance-rr",    BOND_MODE_ROUNDROBIN,   BOND_VALFLAG_DEFAULT},
>@@ -240,6 +242,12 @@ static const struct bond_opt_value bond_coupled_control_tbl[] = {
> 	{ NULL,  -1, 0},
> };
> 
>+static const struct bond_opt_value bond_broadcast_neigh_tbl[] = {
>+	{ "on",	 1, 0},
>+	{ "off", 0, BOND_VALFLAG_DEFAULT},
>+	{ NULL,  -1, 0}
>+};
>+
> static const struct bond_option bond_opts[BOND_OPT_LAST] = {
> 	[BOND_OPT_MODE] = {
> 		.id = BOND_OPT_MODE,
>@@ -513,6 +521,14 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
> 		.flags = BOND_OPTFLAG_IFDOWN,
> 		.values = bond_coupled_control_tbl,
> 		.set = bond_option_coupled_control_set,
>+	},
>+	[BOND_OPT_BROADCAST_NEIGH] = {
>+		.id = BOND_OPT_BROADCAST_NEIGH,
>+		.name = "broadcast_neighbor",
>+		.desc = "Broadcast neighbor packets to all slaves",
>+		.unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
>+		.values = bond_broadcast_neigh_tbl,
>+		.set = bond_option_broadcast_neigh_set,
> 	}
> };
> 
>@@ -1840,3 +1856,12 @@ static int bond_option_coupled_control_set(struct bonding *bond,
> 	bond->params.coupled_control = newval->value;
> 	return 0;
> }
>+
>+static int bond_option_broadcast_neigh_set(struct bonding *bond,
>+					   const struct bond_opt_value *newval)
>+{
>+	netdev_dbg(bond->dev, "Setting broadcast_neighbor to %llu\n",
>+		   newval->value);
>+	bond->params.broadcast_neighbor = newval->value;
>+	return 0;
>+}
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index 1e13bb170515..76f2a1bf57c2 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -752,6 +752,23 @@ static ssize_t bonding_show_ad_user_port_key(struct device *d,
> static DEVICE_ATTR(ad_user_port_key, 0644,
> 		   bonding_show_ad_user_port_key, bonding_sysfs_store_option);
> 
>+static ssize_t bonding_show_broadcast_neighbor(struct device *d,
>+					       struct device_attribute *attr,
>+					       char *buf)
>+{
>+	struct bonding *bond = to_bond(d);
>+	const struct bond_opt_value *val;
>+
>+	val = bond_opt_get_val(BOND_OPT_BROADCAST_NEIGH,
>+			bond->params.broadcast_neighbor);
>+
>+	return sysfs_emit(buf, "%s %d\n", val->string,
>+			bond->params.broadcast_neighbor);
>+}
>+
>+static DEVICE_ATTR(broadcast_neighbor, 0644,
>+		   bonding_show_broadcast_neighbor, bonding_sysfs_store_option);
>+
> static struct attribute *per_bond_attrs[] = {
> 	&dev_attr_slaves.attr,
> 	&dev_attr_mode.attr,
>@@ -791,6 +808,7 @@ static struct attribute *per_bond_attrs[] = {
> 	&dev_attr_ad_actor_system.attr,
> 	&dev_attr_ad_user_port_key.attr,
> 	&dev_attr_arp_missed_max.attr,
>+	&dev_attr_broadcast_neighbor.attr,
> 	NULL,
> };
> 
>diff --git a/include/net/bond_options.h b/include/net/bond_options.h
>index 18687ccf0638..022b122a9fb6 100644
>--- a/include/net/bond_options.h
>+++ b/include/net/bond_options.h
>@@ -77,6 +77,7 @@ enum {
> 	BOND_OPT_NS_TARGETS,
> 	BOND_OPT_PRIO,
> 	BOND_OPT_COUPLED_CONTROL,
>+	BOND_OPT_BROADCAST_NEIGH,
> 	BOND_OPT_LAST
> };
> 
>diff --git a/include/net/bonding.h b/include/net/bonding.h
>index 95f67b308c19..1eafd15eaad9 100644
>--- a/include/net/bonding.h
>+++ b/include/net/bonding.h
>@@ -149,6 +149,7 @@ struct bond_params {
> 	struct in6_addr ns_targets[BOND_MAX_NS_TARGETS];
> #endif
> 	int coupled_control;
>+	int broadcast_neighbor;
> 
> 	/* 2 bytes of padding : see ether_addr_equal_64bits() */
> 	u8 ad_actor_system[ETH_ALEN + 2];
>-- 
>2.34.1
>

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

Re: [PATCH net-next 1/4] net: bonding: add broadcast_neighbor option for 802.3ad

[Andrew Lunn]

> >+static netdev_tx_t bond_3ad_xor_xmit(struct sk_buff *skb,
> >+				     struct net_device *dev)
> >+{
> >+	struct bonding *bond = netdev_priv(dev);
> >+	struct bond_up_slave *slaves;
> >+	struct slave *slave;
> >+
> >+	if (bond_should_broadcast_neighbor(bond, skb))
> >+		return bond_xmit_broadcast(skb, dev);
> 
> 	I feel like there has to be a way to implement this that won't
> add two or three branches to the transmit fast path for every packet
> when this option is not enabled (which will be the vast majority of
> cases).  I'm not sure what that would look like, needs some thought.

Maybe include/linux/jump_label.h. It is what netfilter uses to hook
into the IP stack at various points.

	Andrew

Re: [PATCH net-next 1/4] net: bonding: add broadcast_neighbor option for 802.3ad

[Tonghao Zhang]

> 2025年5月10日 下午8:44，Jay Vosburgh <jv@jvosburgh.net> 写道：
> 
> tonghao@bamaicloud.com wrote:
> 
>> From: Tonghao Zhang <tonghao@bamaicloud.com>
>> 
>> Stacking technology provides technical benefits but has inherent drawbacks.
>> For instance, switch software or system upgrades require simultaneous reboots
>> of all stacked switches. Additionally, stacking link failures may cause
>> stack splitting.
>> 
>> To improve network stability, non-stacking solutions have been increasingly
>> adopted, particularly by public cloud providers and technology companies
>> like Alibaba, Tencent, and Didi. The server still uses dual network cards and
>> dual uplinks to two switches, and the network card mode is set to
>> bond mode 4 (IEEE 802.3ad). As aggregation ports transmit ARP/ND data
>> exclusively through one physical port, both switches in non-stacking
>> deployments must receive server ARP/ND requests. This requires bonding driver
>> modifications to broadcast ARP/ND packets through all active slave links.
>> 
>> - https://www.ruijie.com/fr-fr/support/tech-gallery/de-stack-data-center-network-architecture/
> 
> 	I didn't really follow the explanation here without reading the
> linked article.  I think it would be better to explain the basics of
> what this "non-stacking" architecture is, then describe the change to
> bonding necessary to support it.  This description need not go into
> great detail; assuming I understand correctly, perhaps something along
> the lines of:
> 
> 	"non-stacking" is a method of mimicing switch stacking that
> convinces a LACP peer, bonding in this case, connected to a set of
> "non-stacked" switches that all of its ports are connected to a single
> switch (i.e., LACP aggregator), as if those switches were stacked.  This
> enables the LACP peer's ports to aggregate together, and requires (a)
> special switch configuration, described in the linked article, and (b)
> modifications to the bonding 802.3ad (LACP) mode to send all ARP / ND
> packets across all ports of the active aggregator.
> 
> 	Is that a fair summary?
Yes, pretty good. I will add more info of “no-stacking” arch and your summary.
> 	Regardless, the commit message should stand on its own, even if
> the linked article is gone at some point in the future.
Yes
> 
>> Cc: Jay Vosburgh <jv@jvosburgh.net>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Cc: Eric Dumazet <edumazet@google.com>
>> Cc: Jakub Kicinski <kuba@kernel.org>
>> Cc: Paolo Abeni <pabeni@redhat.com>
>> Cc: Simon Horman <horms@kernel.org>
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: Andrew Lunn <andrew+netdev@lunn.ch>
>> Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
>> ---
>> Documentation/networking/bonding.rst |  5 +++
>> drivers/net/bonding/bond_main.c      | 58 +++++++++++++++++++++-------
>> drivers/net/bonding/bond_options.c   | 25 ++++++++++++
>> drivers/net/bonding/bond_sysfs.c     | 18 +++++++++
>> include/net/bond_options.h           |  1 +
>> include/net/bonding.h                |  1 +
>> 6 files changed, 94 insertions(+), 14 deletions(-)
>> 
>> diff --git a/Documentation/networking/bonding.rst b/Documentation/networking/bonding.rst
>> index a4c1291d2561..0aca6e7599db 100644
>> --- a/Documentation/networking/bonding.rst
>> +++ b/Documentation/networking/bonding.rst
>> @@ -562,6 +562,11 @@ lacp_rate
>> 
>> 	The default is slow.
>> 
>> +broadcast_neighbor
>> +
>> +    Option specifying whether to broadcast ARP/ND packets to all
>> +    active slaves. The default is off (0).
>> +
> 
> 	I'm happy to see documentation updates; however, please add the
> caveat that this option has no effect in modes other than 802.3ad mode.
> Also, the text is not formatted consistently with the options around it
> (block indented one tab).
Ok, update it in next patch version.
>> max_bonds
>> 
>> 	Specifies the number of bonding devices to create for this
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index d05226484c64..c54bfba10688 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -5316,23 +5316,31 @@ static struct slave *bond_xdp_xmit_3ad_xor_slave_get(struct bonding *bond,
>> 	return slaves->arr[hash % count];
>> }
>> 
>> -/* 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.
>> - */
>> -static netdev_tx_t bond_3ad_xor_xmit(struct sk_buff *skb,
>> -				     struct net_device *dev)
>> +static bool bond_should_broadcast_neighbor(struct bonding *bond,
>> +					   struct sk_buff *skb)
>> {
>> -	struct bonding *bond = netdev_priv(dev);
>> -	struct bond_up_slave *slaves;
>> -	struct slave *slave;
>> +	if (BOND_MODE(bond) != BOND_MODE_8023AD)
>> +		return false;
>> 
>> -	slaves = rcu_dereference(bond->usable_slaves);
>> -	slave = bond_xmit_3ad_xor_slave_get(bond, skb, slaves);
>> -	if (likely(slave))
>> -		return bond_dev_queue_xmit(bond, skb, slave->dev);
>> +	if (!bond->params.broadcast_neighbor)
>> +		return false;
>> 
>> -	return bond_tx_drop(dev, skb);
>> +	if (skb->protocol == htons(ETH_P_ARP))
>> +		return true;
>> +
>> +        if (skb->protocol == htons(ETH_P_IPV6) &&
>> +            pskb_may_pull(skb,
>> +                          sizeof(struct ipv6hdr) + sizeof(struct icmp6hdr))) {
>> +                if (ipv6_hdr(skb)->nexthdr == IPPROTO_ICMPV6) {
>> +                        struct icmp6hdr *icmph = icmp6_hdr(skb);
>> +
>> +                        if ((icmph->icmp6_type == NDISC_NEIGHBOUR_SOLICITATION) ||
>> +                            (icmph->icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT))
>> +                                return true;
>> +                }
>> +        }
>> +
>> +        return false;
>> }
>> 
>> /* in broadcast mode, we send everything to all usable interfaces. */
>> @@ -5377,6 +5385,28 @@ static netdev_tx_t bond_xmit_broadcast(struct sk_buff *skb,
>> 	return NET_XMIT_DROP;
>> }
>> 
>> +/* 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.
>> + */
>> +static netdev_tx_t bond_3ad_xor_xmit(struct sk_buff *skb,
>> +				     struct net_device *dev)
>> +{
>> +	struct bonding *bond = netdev_priv(dev);
>> +	struct bond_up_slave *slaves;
>> +	struct slave *slave;
>> +
>> +	if (bond_should_broadcast_neighbor(bond, skb))
>> +		return bond_xmit_broadcast(skb, dev);
> 
> 	I feel like there has to be a way to implement this that won't
> add two or three branches to the transmit fast path for every packet
> when this option is not enabled (which will be the vast majority of
> cases).  I'm not sure what that would look like, needs some thought.
The patch should check the .broadcast_neighbor and bond mode firstly, and ipv4 packets quickly, and then ipv6. Parsing network packets is unavoidable, especially for IPv6 packets.

static inline bool bond_should_broadcast_neighbor(struct bonding *bond,
                                                  struct sk_buff *skb)
{
        if (!bond->params.broadcast_neighbor ||
            BOND_MODE(bond) != BOND_MODE_8023AD)
                return false;

        if (skb->protocol == htons(ETH_P_ARP))
                return true;

        if (skb->protocol == htons(ETH_P_IPV6) &&
            pskb_may_pull(skb,
                          sizeof(struct ipv6hdr) + sizeof(struct icmp6hdr))) {
                if (ipv6_hdr(skb)->nexthdr == IPPROTO_ICMPV6) {
                        struct icmp6hdr *icmph = icmp6_hdr(skb);

                        if ((icmph->icmp6_type == NDISC_NEIGHBOUR_SOLICITATION) ||
                            (icmph->icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT))
                                return true;
                }
        }

        return false;
}

> 
>> max_bonds
>> 
>> 	Specifies the number of bonding devices to create for this
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index d05226484c64..c54bfba10688 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -5316,23 +5316,31 @@ static struct slave *bond_xdp_xmit_3ad_xor_slave_get(struct bonding *bond,
>> 	return slaves->arr[hash % count];
>> }
>> 
>> -/* 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.
>> - */
>> -static netdev_tx_t bond_3ad_xor_xmit(struct sk_buff *skb,
>> -				     struct net_device *dev)
>> +static bool bond_should_broadcast_neighbor(struct bonding *bond,
>> +					   struct sk_buff *skb)
>> {
>> -	struct bonding *bond = netdev_priv(dev);
>> -	struct bond_up_slave *slaves;
>> -	struct slave *slave;
>> +	if (BOND_MODE(bond) != BOND_MODE_8023AD)
>> +		return false;
>> 
>> -	slaves = rcu_dereference(bond->usable_slaves);
>> -	slave = bond_xmit_3ad_xor_slave_get(bond, skb, slaves);
>> -	if (likely(slave))
>> -		return bond_dev_queue_xmit(bond, skb, slave->dev);
>> +	if (!bond->params.broadcast_neighbor)
>> +		return false;
>> 
>> -	return bond_tx_drop(dev, skb);
>> +	if (skb->protocol == htons(ETH_P_ARP))
>> +		return true;
>> +
>> +        if (skb->protocol == htons(ETH_P_IPV6) &&
>> +            pskb_may_pull(skb,
>> +                          sizeof(struct ipv6hdr) + sizeof(struct icmp6hdr))) {
>> +                if (ipv6_hdr(skb)->nexthdr == IPPROTO_ICMPV6) {
>> +                        struct icmp6hdr *icmph = icmp6_hdr(skb);
>> +
>> +                        if ((icmph->icmp6_type == NDISC_NEIGHBOUR_SOLICITATION) ||
>> +                            (icmph->icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT))
>> +                                return true;
>> +                }
>> +        }
>> +
>> +        return false;
>> }
>> 
>> /* in broadcast mode, we send everything to all usable interfaces. */
>> @@ -5377,6 +5385,28 @@ static netdev_tx_t bond_xmit_broadcast(struct sk_buff *skb,
>> 	return NET_XMIT_DROP;
>> }
>> 
>> +/* 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.
>> + */
>> +static netdev_tx_t bond_3ad_xor_xmit(struct sk_buff *skb,
>> +				     struct net_device *dev)
>> +{
>> +	struct bonding *bond = netdev_priv(dev);
>> +	struct bond_up_slave *slaves;
>> +	struct slave *slave;
>> +
>> +	if (bond_should_broadcast_neighbor(bond, skb))
>> +		return bond_xmit_broadcast(skb, dev);
> 
> 	I feel like there has to be a way to implement this that won't
> add two or three branches to the transmit fast path for every packet
> when this option is not enabled (which will be the vast majority of
> cases).  I'm not sure what that would look like, needs some thought.
> 
> 	Is the call to bond_should_broadcast_neighbor inlined at compile
> time?
> 
> 	-J
> 
>> +
>> +	slaves = rcu_dereference(bond->usable_slaves);
>> +	slave = bond_xmit_3ad_xor_slave_get(bond, skb, slaves);
>> +	if (likely(slave))
>> +		return bond_dev_queue_xmit(bond, skb, slave->dev);
>> +
>> +	return bond_tx_drop(dev, skb);
>> +}
>> +
>> /*------------------------- Device initialization ---------------------------*/
>> 
>> /* Lookup the slave that corresponds to a qid */
>> diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>> index 91893c29b899..38e8f03d1707 100644
>> --- a/drivers/net/bonding/bond_options.c
>> +++ b/drivers/net/bonding/bond_options.c
>> @@ -87,6 +87,8 @@ static int bond_option_missed_max_set(struct bonding *bond,
>> 				      const struct bond_opt_value *newval);
>> static int bond_option_coupled_control_set(struct bonding *bond,
>> 					   const struct bond_opt_value *newval);
>> +static int bond_option_broadcast_neigh_set(struct bonding *bond,
>> +					   const struct bond_opt_value *newval);
>> 
>> static const struct bond_opt_value bond_mode_tbl[] = {
>> 	{ "balance-rr",    BOND_MODE_ROUNDROBIN,   BOND_VALFLAG_DEFAULT},
>> @@ -240,6 +242,12 @@ static const struct bond_opt_value bond_coupled_control_tbl[] = {
>> 	{ NULL,  -1, 0},
>> };
>> 
>> +static const struct bond_opt_value bond_broadcast_neigh_tbl[] = {
>> +	{ "on",	 1, 0},
>> +	{ "off", 0, BOND_VALFLAG_DEFAULT},
>> +	{ NULL,  -1, 0}
>> +};
>> +
>> static const struct bond_option bond_opts[BOND_OPT_LAST] = {
>> 	[BOND_OPT_MODE] = {
>> 		.id = BOND_OPT_MODE,
>> @@ -513,6 +521,14 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
>> 		.flags = BOND_OPTFLAG_IFDOWN,
>> 		.values = bond_coupled_control_tbl,
>> 		.set = bond_option_coupled_control_set,
>> +	},
>> +	[BOND_OPT_BROADCAST_NEIGH] = {
>> +		.id = BOND_OPT_BROADCAST_NEIGH,
>> +		.name = "broadcast_neighbor",
>> +		.desc = "Broadcast neighbor packets to all slaves",
>> +		.unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
>> +		.values = bond_broadcast_neigh_tbl,
>> +		.set = bond_option_broadcast_neigh_set,
>> 	}
>> };
>> 
>> @@ -1840,3 +1856,12 @@ static int bond_option_coupled_control_set(struct bonding *bond,
>> 	bond->params.coupled_control = newval->value;
>> 	return 0;
>> }
>> +
>> +static int bond_option_broadcast_neigh_set(struct bonding *bond,
>> +					   const struct bond_opt_value *newval)
>> +{
>> +	netdev_dbg(bond->dev, "Setting broadcast_neighbor to %llu\n",
>> +		   newval->value);
>> +	bond->params.broadcast_neighbor = newval->value;
>> +	return 0;
>> +}
>> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>> index 1e13bb170515..76f2a1bf57c2 100644
>> --- a/drivers/net/bonding/bond_sysfs.c
>> +++ b/drivers/net/bonding/bond_sysfs.c
>> @@ -752,6 +752,23 @@ static ssize_t bonding_show_ad_user_port_key(struct device *d,
>> static DEVICE_ATTR(ad_user_port_key, 0644,
>> 		   bonding_show_ad_user_port_key, bonding_sysfs_store_option);
>> 
>> +static ssize_t bonding_show_broadcast_neighbor(struct device *d,
>> +					       struct device_attribute *attr,
>> +					       char *buf)
>> +{
>> +	struct bonding *bond = to_bond(d);
>> +	const struct bond_opt_value *val;
>> +
>> +	val = bond_opt_get_val(BOND_OPT_BROADCAST_NEIGH,
>> +			bond->params.broadcast_neighbor);
>> +
>> +	return sysfs_emit(buf, "%s %d\n", val->string,
>> +			bond->params.broadcast_neighbor);
>> +}
>> +
>> +static DEVICE_ATTR(broadcast_neighbor, 0644,
>> +		   bonding_show_broadcast_neighbor, bonding_sysfs_store_option);
>> +
>> static struct attribute *per_bond_attrs[] = {
>> 	&dev_attr_slaves.attr,
>> 	&dev_attr_mode.attr,
>> @@ -791,6 +808,7 @@ static struct attribute *per_bond_attrs[] = {
>> 	&dev_attr_ad_actor_system.attr,
>> 	&dev_attr_ad_user_port_key.attr,
>> 	&dev_attr_arp_missed_max.attr,
>> +	&dev_attr_broadcast_neighbor.attr,
>> 	NULL,
>> };
>> 
>> diff --git a/include/net/bond_options.h b/include/net/bond_options.h
>> index 18687ccf0638..022b122a9fb6 100644
>> --- a/include/net/bond_options.h
>> +++ b/include/net/bond_options.h
>> @@ -77,6 +77,7 @@ enum {
>> 	BOND_OPT_NS_TARGETS,
>> 	BOND_OPT_PRIO,
>> 	BOND_OPT_COUPLED_CONTROL,
>> +	BOND_OPT_BROADCAST_NEIGH,
>> 	BOND_OPT_LAST
>> };
>> 
>> diff --git a/include/net/bonding.h b/include/net/bonding.h
>> index 95f67b308c19..1eafd15eaad9 100644
>> --- a/include/net/bonding.h
>> +++ b/include/net/bonding.h
>> @@ -149,6 +149,7 @@ struct bond_params {
>> 	struct in6_addr ns_targets[BOND_MAX_NS_TARGETS];
>> #endif
>> 	int coupled_control;
>> +	int broadcast_neighbor;
>> 
>> 	/* 2 bytes of padding : see ether_addr_equal_64bits() */
>> 	u8 ad_actor_system[ETH_ALEN + 2];
>> -- 
>> 2.34.1
>> 
> 
> ---
> 	-Jay Vosburgh, jv@jvosburgh.net

Re: [PATCH net-next 1/4] net: bonding: add broadcast_neighbor option for 802.3ad

[Andrew Lunn]

> static inline bool bond_should_broadcast_neighbor(struct bonding *bond,
>                                                   struct sk_buff *skb)
> {
>         if (!bond->params.broadcast_neighbor ||
>             BOND_MODE(bond) != BOND_MODE_8023AD)
>                 return false;

I think you missed the point. You have added these two tests to every
packet on the fast path. And it is very likely to return false. Is
bond.params.broadcast_neighbor likely to be in the cache? A cache miss
is expensive. Is bond.params.mode also likely to be in cache? You
placed broadcast_neighbor at the end of params, so it is unlikely to
be in the same cache line as bond.params.mode. So two cache misses.

What Jay would like is that the cost on the fast path is ~0 for when
this feature is not in use. Jump labels can achieve this. It inserts
either a NOP or a jump instruction, which costs nearly nothing, and
then uses self modifying code to swap between a NOP or a jump. You can
keep a global view of is any bond is using this new mode? If no, this
test is eliminated. If yes, you do the test.

	Andrew

Re: [PATCH net-next 1/4] net: bonding: add broadcast_neighbor option for 802.3ad

[Tonghao Zhang]

> 2025年5月11日 下午11:53，Andrew Lunn <andrew@lunn.ch> 写道：
> 
>> static inline bool bond_should_broadcast_neighbor(struct bonding *bond,
>>                                                  struct sk_buff *skb)
>> {
>>        if (!bond->params.broadcast_neighbor ||
>>            BOND_MODE(bond) != BOND_MODE_8023AD)
>>                return false;
> 
> I think you missed the point. You have added these two tests to every
> packet on the fast path. And it is very likely to return false. Is
> bond.params.broadcast_neighbor likely to be in the cache? A cache miss
> is expensive. Is bond.params.mode also likely to be in cache? You
> placed broadcast_neighbor at the end of params, so it is unlikely to
> be in the same cache line as bond.params.mode. So two cache misses.
> 
> What Jay would like is that the cost on the fast path is ~0 for when
> this feature is not in use. Jump labels can achieve this. It inserts
> either a NOP or a jump instruction, which costs nearly nothing, and
> then uses self modifying code to swap between a NOP or a jump. You can
> keep a global view of is any bond is using this new mode? If no, this
No, no mode uses jump labels instead of bond.params checking.
> test is eliminated. If yes, you do the test.
I test the lacp mode with broadcast_neighbor enabled, there is no performance drop. This patch has been running in our production environment for a long time. We only use this option in lacp mode, for performance, the code can be modified as follows:
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index ce31445e85b6..8743bf007b7e 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -5330,11 +5330,12 @@ static struct slave *bond_xdp_xmit_3ad_xor_slave_get(struct bonding *bond,
        return slaves->arr[hash % count];
 }

-static inline bool bond_should_broadcast_neighbor(struct bonding *bond,
-                                                 struct sk_buff *skb)
+static inline bool bond_should_broadcast_neighbor(struct sk_buff *skb,
+                                                 struct net_device *dev)
 {
-       if (!bond->params.broadcast_neighbor ||
-           BOND_MODE(bond) != BOND_MODE_8023AD)
+       struct bonding *bond = netdev_priv(dev);
+
+       if (!bond->params.broadcast_neighbor)
                return false;

        if (skb->protocol == htons(ETH_P_ARP))
@@ -5408,9 +5409,6 @@ static netdev_tx_t bond_3ad_xor_xmit(struct sk_buff *skb,
        struct bond_up_slave *slaves;
        struct slave *slave;

-       if (bond_should_broadcast_neighbor(bond, skb))
-               return bond_xmit_broadcast(skb, dev);
-
        slaves = rcu_dereference(bond->usable_slaves);
        slave = bond_xmit_3ad_xor_slave_get(bond, skb, slaves);
        if (likely(slave))
@@ -5625,6 +5623,9 @@ static netdev_tx_t __bond_start_xmit(struct sk_buff *skb, struct net_device *dev
        case BOND_MODE_ACTIVEBACKUP:
                return bond_xmit_activebackup(skb, dev);
        case BOND_MODE_8023AD:
+               if (bond_should_broadcast_neighbor(skb, dev))
+                       return bond_xmit_broadcast(skb, dev);
+               fallthrough;
        case BOND_MODE_XOR:
                return bond_3ad_xor_xmit(skb, dev);
        case BOND_MODE_BROADCAST:
> 
> 	Andrew
> 
> 

Re: [PATCH net-next 1/4] net: bonding: add broadcast_neighbor option for 802.3ad

[Jay Vosburgh]

Tonghao Zhang <tonghao@bamaicloud.com> wrote:

>> 2025年5月11日 下午11:53，Andrew Lunn <andrew@lunn.ch> 写道：
>> 
>>> static inline bool bond_should_broadcast_neighbor(struct bonding *bond,
>>>                                                  struct sk_buff *skb)
>>> {
>>>        if (!bond->params.broadcast_neighbor ||
>>>            BOND_MODE(bond) != BOND_MODE_8023AD)
>>>                return false;
>> 
>> I think you missed the point. You have added these two tests to every
>> packet on the fast path. And it is very likely to return false. Is
>> bond.params.broadcast_neighbor likely to be in the cache? A cache miss
>> is expensive. Is bond.params.mode also likely to be in cache? You
>> placed broadcast_neighbor at the end of params, so it is unlikely to
>> be in the same cache line as bond.params.mode. So two cache misses.
>> 
>> What Jay would like is that the cost on the fast path is ~0 for when
>> this feature is not in use. Jump labels can achieve this. It inserts
>> either a NOP or a jump instruction, which costs nearly nothing, and
>> then uses self modifying code to swap between a NOP or a jump. You can
>> keep a global view of is any bond is using this new mode? If no, this
>No, no mode uses jump labels instead of bond.params checking.

	The suggestion here is to use a jump label (static branch) to
essentially eliminate the overhead of the options test for the common case
for most users, which is with broadcast_neighbor disabled.

	As described below, the static branch would be tracked
separately from the per-bond option.

>> test is eliminated. If yes, you do the test.
>I test the lacp mode with broadcast_neighbor enabled, there is no performance drop. This patch has been running in our production environment for a long time. We only use this option in lacp mode, for performance, the code can be modified as follows:

	How did you test this?  The performance under discussion here is
that branches in the packet transmit path can affect overall packet
transmission rates at very high rates (think in terms of small packet
rates at 40 Gb/sec and higher).  Bonding already has a significant
number of TX path branches, and we should be working to reduce that
number, not increase it.

>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index ce31445e85b6..8743bf007b7e 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -5330,11 +5330,12 @@ static struct slave *bond_xdp_xmit_3ad_xor_slave_get(struct bonding *bond,
>        return slaves->arr[hash % count];
> }
>
>-static inline bool bond_should_broadcast_neighbor(struct bonding *bond,
>-                                                 struct sk_buff *skb)
>+static inline bool bond_should_broadcast_neighbor(struct sk_buff *skb,
>+                                                 struct net_device *dev)
> {
>-       if (!bond->params.broadcast_neighbor ||
>-           BOND_MODE(bond) != BOND_MODE_8023AD)
>+       struct bonding *bond = netdev_priv(dev);
>+
>+       if (!bond->params.broadcast_neighbor)
>                return false;

	Using a static branch, the above would be preceded by something
like:

	if (!static_branch_unlikely(&bond_bcast_neigh_enabled))
		return false;

	With additional logic in the options code that enables and
disables broadcast_neighbor that will increment or decrement (via
static_branch_inc / _dec) bond_bcast_neigh_enabled as the
broadcast_neighbor option is enabled or disabled.  The static branch
becomes a fast way to ask "is any bond in the system using
broadcast_neighbor" at very low cost.

	As Andrew helpfully pointed out, netfilter makes extensive use
of these; I'd suggest looking at the usage of something like
nft_trace_enabled as an example of what we're referring to.

	-J

>        if (skb->protocol == htons(ETH_P_ARP))
>@@ -5408,9 +5409,6 @@ static netdev_tx_t bond_3ad_xor_xmit(struct sk_buff *skb,
>        struct bond_up_slave *slaves;
>        struct slave *slave;
>
>-       if (bond_should_broadcast_neighbor(bond, skb))
>-               return bond_xmit_broadcast(skb, dev);
>-
>        slaves = rcu_dereference(bond->usable_slaves);
>        slave = bond_xmit_3ad_xor_slave_get(bond, skb, slaves);
>        if (likely(slave))
>@@ -5625,6 +5623,9 @@ static netdev_tx_t __bond_start_xmit(struct sk_buff *skb, struct net_device *dev
>        case BOND_MODE_ACTIVEBACKUP:
>                return bond_xmit_activebackup(skb, dev);
>        case BOND_MODE_8023AD:
>+               if (bond_should_broadcast_neighbor(skb, dev))
>+                       return bond_xmit_broadcast(skb, dev);
>+               fallthrough;
>        case BOND_MODE_XOR:
>                return bond_3ad_xor_xmit(skb, dev);
>        case BOND_MODE_BROADCAST:
>> 
>> 	Andrew

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

Re: [PATCH net-next 1/4] net: bonding: add broadcast_neighbor option for 802.3ad

[Tonghao Zhang]

> 2025年5月12日 17:19，Jay Vosburgh <jv@jvosburgh.net> 写道：
> 
> Tonghao Zhang <tonghao@bamaicloud.com> wrote:
> 
>>> 2025年5月11日 下午11:53，Andrew Lunn <andrew@lunn.ch> 写道：
>>> 
>>>> static inline bool bond_should_broadcast_neighbor(struct bonding *bond,
>>>>                                                 struct sk_buff *skb)
>>>> {
>>>>       if (!bond->params.broadcast_neighbor ||
>>>>           BOND_MODE(bond) != BOND_MODE_8023AD)
>>>>               return false;
>>> 
>>> I think you missed the point. You have added these two tests to every
>>> packet on the fast path. And it is very likely to return false. Is
>>> bond.params.broadcast_neighbor likely to be in the cache? A cache miss
>>> is expensive. Is bond.params.mode also likely to be in cache? You
>>> placed broadcast_neighbor at the end of params, so it is unlikely to
>>> be in the same cache line as bond.params.mode. So two cache misses.
>>> 
>>> What Jay would like is that the cost on the fast path is ~0 for when
>>> this feature is not in use. Jump labels can achieve this. It inserts
>>> either a NOP or a jump instruction, which costs nearly nothing, and
>>> then uses self modifying code to swap between a NOP or a jump. You can
>>> keep a global view of is any bond is using this new mode? If no, this
>> No, no mode uses jump labels instead of bond.params checking.
> 
> The suggestion here is to use a jump label (static branch) to
> essentially eliminate the overhead of the options test for the common case
> for most users, which is with broadcast_neighbor disabled.
> 
> As described below, the static branch would be tracked
> separately from the per-bond option.
> 
>>> test is eliminated. If yes, you do the test.
>> I test the lacp mode with broadcast_neighbor enabled, there is no performance drop. This patch has been running in our production environment for a long time. We only use this option in lacp mode, for performance, the code can be modified as follows:
> 
> How did you test this?  The performance under discussion here is
> that branches in the packet transmit path can affect overall packet
> transmission rates at very high rates (think in terms of small packet
> rates at 40 Gb/sec and higher).  Bonding already has a significant
> number of TX path branches, and we should be working to reduce that
> number, not increase it.
> 
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index ce31445e85b6..8743bf007b7e 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -5330,11 +5330,12 @@ static struct slave *bond_xdp_xmit_3ad_xor_slave_get(struct bonding *bond,
>>       return slaves->arr[hash % count];
>> }
>> 
>> -static inline bool bond_should_broadcast_neighbor(struct bonding *bond,
>> -                                                 struct sk_buff *skb)
>> +static inline bool bond_should_broadcast_neighbor(struct sk_buff *skb,
>> +                                                 struct net_device *dev)
>> {
>> -       if (!bond->params.broadcast_neighbor ||
>> -           BOND_MODE(bond) != BOND_MODE_8023AD)
>> +       struct bonding *bond = netdev_priv(dev);
>> +
>> +       if (!bond->params.broadcast_neighbor)
>>               return false;
> 
> Using a static branch, the above would be preceded by something
> like:
> 
> if (!static_branch_unlikely(&bond_bcast_neigh_enabled))
> return false;
> 
> With additional logic in the options code that enables and
> disables broadcast_neighbor that will increment or decrement (via
> static_branch_inc / _dec) bond_bcast_neigh_enabled as the
> broadcast_neighbor option is enabled or disabled.  The static branch
> becomes a fast way to ask "is any bond in the system using
> broadcast_neighbor" at very low cost.
> 
> As Andrew helpfully pointed out, netfilter makes extensive use
> of these; I'd suggest looking at the usage of something like
> nft_trace_enabled as an example of what we're referring to.
I got it, thanks Jay, and Andrew.
> 
> -J
> 
>>       if (skb->protocol == htons(ETH_P_ARP))
>> @@ -5408,9 +5409,6 @@ static netdev_tx_t bond_3ad_xor_xmit(struct sk_buff *skb,
>>       struct bond_up_slave *slaves;
>>       struct slave *slave;
>> 
>> -       if (bond_should_broadcast_neighbor(bond, skb))
>> -               return bond_xmit_broadcast(skb, dev);
>> -
>>       slaves = rcu_dereference(bond->usable_slaves);
>>       slave = bond_xmit_3ad_xor_slave_get(bond, skb, slaves);
>>       if (likely(slave))
>> @@ -5625,6 +5623,9 @@ static netdev_tx_t __bond_start_xmit(struct sk_buff *skb, struct net_device *dev
>>       case BOND_MODE_ACTIVEBACKUP:
>>               return bond_xmit_activebackup(skb, dev);
>>       case BOND_MODE_8023AD:
>> +               if (bond_should_broadcast_neighbor(skb, dev))
>> +                       return bond_xmit_broadcast(skb, dev);
>> +               fallthrough;
>>       case BOND_MODE_XOR:
>>               return bond_3ad_xor_xmit(skb, dev);
>>       case BOND_MODE_BROADCAST:
>>> 
>>> Andrew
> 
> ---
> -Jay Vosburgh, jv@jvosburgh.net

Re: [PATCH net-next 1/4] net: bonding: add broadcast_neighbor option for 802.3ad

[Hangbin Liu]

On Sat, May 10, 2025 at 12:45:01PM +0800, tonghao@bamaicloud.com wrote:
> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
> index 1e13bb170515..76f2a1bf57c2 100644
> --- a/drivers/net/bonding/bond_sysfs.c
> +++ b/drivers/net/bonding/bond_sysfs.c
> @@ -752,6 +752,23 @@ static ssize_t bonding_show_ad_user_port_key(struct device *d,
>  static DEVICE_ATTR(ad_user_port_key, 0644,
>  		   bonding_show_ad_user_port_key, bonding_sysfs_store_option);
>  
> +static ssize_t bonding_show_broadcast_neighbor(struct device *d,
> +					       struct device_attribute *attr,
> +					       char *buf)
> +{
> +	struct bonding *bond = to_bond(d);
> +	const struct bond_opt_value *val;
> +
> +	val = bond_opt_get_val(BOND_OPT_BROADCAST_NEIGH,
> +			bond->params.broadcast_neighbor);

nit: please take care of the code alignment here

> +
> +	return sysfs_emit(buf, "%s %d\n", val->string,
> +			bond->params.broadcast_neighbor);

                       here
> +}
> +
> +static DEVICE_ATTR(broadcast_neighbor, 0644,
> +		   bonding_show_broadcast_neighbor, bonding_sysfs_store_option);

and here.

Thanks
Hangbin

Re: [PATCH net-next 1/4] net: bonding: add broadcast_neighbor option for 802.3ad

[Tonghao Zhang]

> 2025年5月12日 下午4:22，Hangbin Liu <liuhangbin@gmail.com> 写道：
> 
> On Sat, May 10, 2025 at 12:45:01PM +0800, tonghao@bamaicloud.com wrote:
>> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>> index 1e13bb170515..76f2a1bf57c2 100644
>> --- a/drivers/net/bonding/bond_sysfs.c
>> +++ b/drivers/net/bonding/bond_sysfs.c
>> @@ -752,6 +752,23 @@ static ssize_t bonding_show_ad_user_port_key(struct device *d,
>> static DEVICE_ATTR(ad_user_port_key, 0644,
>> 		   bonding_show_ad_user_port_key, bonding_sysfs_store_option);
>> 
>> +static ssize_t bonding_show_broadcast_neighbor(struct device *d,
>> +					       struct device_attribute *attr,
>> +					       char *buf)
>> +{
>> +	struct bonding *bond = to_bond(d);
>> +	const struct bond_opt_value *val;
>> +
>> +	val = bond_opt_get_val(BOND_OPT_BROADCAST_NEIGH,
>> +			bond->params.broadcast_neighbor);
> 
> nit: please take care of the code alignment here
OK
>> +
>> +	return sysfs_emit(buf, "%s %d\n", val->string,
>> +			bond->params.broadcast_neighbor);
> 
>                       here
Ok, thanks Hangbin.
>> +}
>> +
>> +static DEVICE_ATTR(broadcast_neighbor, 0644,
>> +		   bonding_show_broadcast_neighbor, bonding_sysfs_store_option);
> 
> and here.
OK
> 
> Thanks
> Hangbin
> 

[PATCH net-next 2/4] net: bonding: add broadcast_neighbor netlink option

[tonghao]

From: Tonghao Zhang <tonghao@bamaicloud.com>

User can config or display the bonding broadcast_neighbor option via
iproute2/netlink.

Cc: Jay Vosburgh <jv@jvosburgh.net>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Andrew Lunn <andrew+netdev@lunn.ch>
Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
---
 drivers/net/bonding/bond_netlink.c | 16 ++++++++++++++++
 include/uapi/linux/if_link.h       |  1 +
 2 files changed, 17 insertions(+)

diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index ac5e402c34bc..57fff2421f1b 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -124,6 +124,7 @@ static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = {
 	[IFLA_BOND_MISSED_MAX]		= { .type = NLA_U8 },
 	[IFLA_BOND_NS_IP6_TARGET]	= { .type = NLA_NESTED },
 	[IFLA_BOND_COUPLED_CONTROL]	= { .type = NLA_U8 },
+	[IFLA_BOND_BROADCAST_NEIGH]	= { .type = NLA_U8 },
 };
 
 static const struct nla_policy bond_slave_policy[IFLA_BOND_SLAVE_MAX + 1] = {
@@ -561,6 +562,16 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
 			return err;
 	}
 
+	if (data[IFLA_BOND_BROADCAST_NEIGH]) {
+		int broadcast_neigh = nla_get_u8(data[IFLA_BOND_BROADCAST_NEIGH]);
+
+		bond_opt_initval(&newval, broadcast_neigh);
+		err = __bond_opt_set(bond, BOND_OPT_BROADCAST_NEIGH, &newval,
+				     data[IFLA_BOND_BROADCAST_NEIGH], extack);
+		if (err)
+			return err;
+	}
+
 	return 0;
 }
 
@@ -630,6 +641,7 @@ static size_t bond_get_size(const struct net_device *bond_dev)
 		nla_total_size(sizeof(struct nlattr)) +
 		nla_total_size(sizeof(struct in6_addr)) * BOND_MAX_NS_TARGETS +
 		nla_total_size(sizeof(u8)) +	/* IFLA_BOND_COUPLED_CONTROL */
+		nla_total_size(sizeof(u8)) +	/* IFLA_BOND_BROADCAST_NEIGH */
 		0;
 }
 
@@ -793,6 +805,10 @@ static int bond_fill_info(struct sk_buff *skb,
 		       bond->params.coupled_control))
 		goto nla_put_failure;
 
+	if (nla_put_u8(skb, IFLA_BOND_BROADCAST_NEIGH,
+		       bond->params.broadcast_neighbor))
+		goto nla_put_failure;
+
 	if (BOND_MODE(bond) == BOND_MODE_8023AD) {
 		struct ad_info info;
 
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 3ad2d5d98034..53b2f6ebda8b 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -1534,6 +1534,7 @@ enum {
 	IFLA_BOND_MISSED_MAX,
 	IFLA_BOND_NS_IP6_TARGET,
 	IFLA_BOND_COUPLED_CONTROL,
+	IFLA_BOND_BROADCAST_NEIGH,
 	__IFLA_BOND_MAX,
 };
 
-- 
2.34.1

[PATCH net-next 3/4] net: bonding: send peer notify when failure recovery

[tonghao]

From: Tonghao Zhang <tonghao@bamaicloud.com>

While hardware failures in NICs, optical transceivers, or switches
are unavoidable, rapid system recovery can be achieved post-restoration.
For example, triggering immediate ARP/ND packet transmission upon
LACP failure recovery enables the system to swiftly resume normal
operations, thereby minimizing service downtime.

Cc: Jay Vosburgh <jv@jvosburgh.net>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Andrew Lunn <andrew+netdev@lunn.ch>
Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
---
 drivers/net/bonding/bond_3ad.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index c6807e473ab7..6577ce54d115 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -982,6 +982,19 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
 	return 0;
 }
 
+static void ad_peer_notif_send(struct port *port)
+{
+	if (!port->aggregator->is_active)
+		return;
+
+	struct bonding *bond = port->slave->bond;
+	if (bond->params.broadcast_neighbor && rtnl_trylock()) {
+		bond->send_peer_notif = bond->params.num_peer_notif *
+			max(1, bond->params.peer_notif_delay);
+		rtnl_unlock();
+	}
+}
+
 /**
  * ad_mux_machine - handle a port's mux state machine
  * @port: the port we're looking at
@@ -1164,6 +1177,7 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
 			port->actor_oper_port_state |= LACP_STATE_COLLECTING;
 			port->actor_oper_port_state |= LACP_STATE_DISTRIBUTING;
 			port->actor_oper_port_state |= LACP_STATE_SYNCHRONIZATION;
+			ad_peer_notif_send(port);
 			ad_enable_collecting_distributing(port,
 							  update_slave_arr);
 			port->ntt = true;
-- 
2.34.1

Re: [PATCH net-next 3/4] net: bonding: send peer notify when failure recovery

[Jay Vosburgh]

tonghao@bamaicloud.com wrote:

>From: Tonghao Zhang <tonghao@bamaicloud.com>
>
>While hardware failures in NICs, optical transceivers, or switches
>are unavoidable, rapid system recovery can be achieved post-restoration.
>For example, triggering immediate ARP/ND packet transmission upon
>LACP failure recovery enables the system to swiftly resume normal
>operations, thereby minimizing service downtime.

	I think this comment needs to be prefaced with something that
explains that this logic is for the "no stack" architecture.  It don't
need the entire blurb about what that is, though.

>Cc: Jay Vosburgh <jv@jvosburgh.net>
>Cc: "David S. Miller" <davem@davemloft.net>
>Cc: Eric Dumazet <edumazet@google.com>
>Cc: Jakub Kicinski <kuba@kernel.org>
>Cc: Paolo Abeni <pabeni@redhat.com>
>Cc: Simon Horman <horms@kernel.org>
>Cc: Jonathan Corbet <corbet@lwn.net>
>Cc: Andrew Lunn <andrew+netdev@lunn.ch>
>Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
>---
> drivers/net/bonding/bond_3ad.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index c6807e473ab7..6577ce54d115 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -982,6 +982,19 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
> 	return 0;
> }
> 
>+static void ad_peer_notif_send(struct port *port)
>+{
>+	if (!port->aggregator->is_active)
>+		return;
>+
>+	struct bonding *bond = port->slave->bond;
>+	if (bond->params.broadcast_neighbor && rtnl_trylock()) {
>+		bond->send_peer_notif = bond->params.num_peer_notif *
>+			max(1, bond->params.peer_notif_delay);
>+		rtnl_unlock();
>+	}
>+}
>+

	I'm not a fan of the function name, as this doesn't actually
send any notifications.  Perhaps "ad_cond_set_peer_notif"?  I.e.,
conditionally set peer notifications on?

> /**
>  * ad_mux_machine - handle a port's mux state machine
>  * @port: the port we're looking at
>@@ -1164,6 +1177,7 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> 			port->actor_oper_port_state |= LACP_STATE_COLLECTING;
> 			port->actor_oper_port_state |= LACP_STATE_DISTRIBUTING;
> 			port->actor_oper_port_state |= LACP_STATE_SYNCHRONIZATION;
>+			ad_peer_notif_send(port);
> 			ad_enable_collecting_distributing(port,
> 							  update_slave_arr);
> 			port->ntt = true;

	This is in the AD_MUX_COLLECTING_DISTRIBUTING case, I think you
need another one of these in the AD_MUX_DISTRIBUTING case a few lines
further down to handle the situation when coupled_control is disabled.

	-J

>-- 
>2.34.1
>

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

Re: [PATCH net-next 3/4] net: bonding: send peer notify when failure recovery

[Tonghao Zhang]

> 2025年5月10日 下午9:01，Jay Vosburgh <jv@jvosburgh.net> 写道：
> 
> tonghao@bamaicloud.com wrote:
> 
>> From: Tonghao Zhang <tonghao@bamaicloud.com>
>> 
>> While hardware failures in NICs, optical transceivers, or switches
>> are unavoidable, rapid system recovery can be achieved post-restoration.
>> For example, triggering immediate ARP/ND packet transmission upon
>> LACP failure recovery enables the system to swiftly resume normal
>> operations, thereby minimizing service downtime.
> 
> 	I think this comment needs to be prefaced with something that
> explains that this logic is for the "no stack" architecture.  It don't
> need the entire blurb about what that is, though.
Ok, Update the commit message in v2.
> 
>> Cc: Jay Vosburgh <jv@jvosburgh.net>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Cc: Eric Dumazet <edumazet@google.com>
>> Cc: Jakub Kicinski <kuba@kernel.org>
>> Cc: Paolo Abeni <pabeni@redhat.com>
>> Cc: Simon Horman <horms@kernel.org>
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: Andrew Lunn <andrew+netdev@lunn.ch>
>> Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
>> ---
>> drivers/net/bonding/bond_3ad.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>> 
>> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>> index c6807e473ab7..6577ce54d115 100644
>> --- a/drivers/net/bonding/bond_3ad.c
>> +++ b/drivers/net/bonding/bond_3ad.c
>> @@ -982,6 +982,19 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
>> 	return 0;
>> }
>> 
>> +static void ad_peer_notif_send(struct port *port)
>> +{
>> +	if (!port->aggregator->is_active)
>> +		return;
>> +
>> +	struct bonding *bond = port->slave->bond;
>> +	if (bond->params.broadcast_neighbor && rtnl_trylock()) {
>> +		bond->send_peer_notif = bond->params.num_peer_notif *
>> +			max(1, bond->params.peer_notif_delay);
>> +		rtnl_unlock();
>> +	}
>> +}
>> +
> 
> 	I'm not a fan of the function name, as this doesn't actually
> send any notifications.  Perhaps "ad_cond_set_peer_notif"?  I.e.,
> conditionally set peer notifications on?
> 
>> /**
>> * ad_mux_machine - handle a port's mux state machine
>> * @port: the port we're looking at
>> @@ -1164,6 +1177,7 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
>> 			port->actor_oper_port_state |= LACP_STATE_COLLECTING;
>> 			port->actor_oper_port_state |= LACP_STATE_DISTRIBUTING;
>> 			port->actor_oper_port_state |= LACP_STATE_SYNCHRONIZATION;
>> +			ad_peer_notif_send(port);
>> 			ad_enable_collecting_distributing(port,
>> 							  update_slave_arr);
>> 			port->ntt = true;
> 
> 	This is in the AD_MUX_COLLECTING_DISTRIBUTING case, I think you
> need another one of these in the AD_MUX_DISTRIBUTING case a few lines
> further down to handle the situation when coupled_control is disabled.
pretty good, moving the ad_cond_set_peer_notif to ad_enable_collecting_distributing makes more sense.

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index c6807e473ab7..d1c2d416ac87 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -982,6 +982,17 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
        return 0;
 }

+static void ad_cond_set_peer_notif(struct port *port)
+{
+       struct bonding *bond = port->slave->bond;
+
+       if (bond->params.broadcast_neighbor && rtnl_trylock()) {
+               bond->send_peer_notif = bond->params.num_peer_notif *
+                       max(1, bond->params.peer_notif_delay);
+               rtnl_unlock();
+       }
+}
+
 /**
  * ad_mux_machine - handle a port's mux state machine
  * @port: the port we're looking at
@@ -2061,6 +2072,8 @@ static void ad_enable_collecting_distributing(struct port *port,
                __enable_port(port);
                /* Slave array needs update */
                *update_slave_arr = true;
+               /* Should notify peers if possible */
+               ad_cond_set_peer_notif(port);
        }
 }


By the way, bond_should_notify_peers should check params.broadcast_neighbor, because curr_active_slave may be NULL. In my test environment, ip li set slave-ethx down and when up again, bond_change_active_slave will set bond->curr_active_slave in lacp mode.

static bool bond_should_notify_peers(struct bonding *bond)
{
        struct bond_up_slave *slaves;
        struct slave *slave = NULL;

        if (BOND_MODE(bond) == BOND_MODE_8023AD) {
                if (!bond->params.broadcast_neighbor)
                        return false;

                slaves = rtnl_dereference(bond->usable_slaves);
                if (!slaves || !READ_ONCE(slaves->count))
                        return false;
        } else {
                slave = rcu_dereference_rtnl(bond->curr_active_slave);
                if (!save || test_bit(__LINK_STATE_LINKWATCH_PENDING,
                                      &slave->dev->state))
                        return false;
        }

        if (!bond->send_peer_notif ||
            bond->send_peer_notif %
            max(1, bond->params.peer_notif_delay) != 0 ||
            !netif_carrier_ok(bond->dev))
                return false;

        netdev_dbg(bond->dev, "bond_should_notify_peers: slave %s\n",
                   slave ? slave->dev->name : "all");

        return true;
}

> 
> 	-J
> 
>> -- 
>> 2.34.1
>> 
> 
> ---
> 	-Jay Vosburgh, jv@jvosburgh.net

Re: [PATCH net-next 3/4] net: bonding: send peer notify when failure recovery

[Hangbin Liu]

Hi Tonghao,
On Sat, May 10, 2025 at 12:45:03PM +0800, tonghao@bamaicloud.com wrote:
> From: Tonghao Zhang <tonghao@bamaicloud.com>
> 
> While hardware failures in NICs, optical transceivers, or switches
> are unavoidable, rapid system recovery can be achieved post-restoration.
> For example, triggering immediate ARP/ND packet transmission upon
> LACP failure recovery enables the system to swiftly resume normal
> operations, thereby minimizing service downtime.
> 
> Cc: Jay Vosburgh <jv@jvosburgh.net>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Simon Horman <horms@kernel.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Andrew Lunn <andrew+netdev@lunn.ch>
> Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
> ---
>  drivers/net/bonding/bond_3ad.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
> index c6807e473ab7..6577ce54d115 100644
> --- a/drivers/net/bonding/bond_3ad.c
> +++ b/drivers/net/bonding/bond_3ad.c
> @@ -982,6 +982,19 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
>  	return 0;
>  }
>  
> +static void ad_peer_notif_send(struct port *port)
> +{
> +	if (!port->aggregator->is_active)
> +		return;
> +
> +	struct bonding *bond = port->slave->bond;
> +	if (bond->params.broadcast_neighbor && rtnl_trylock()) {
> +		bond->send_peer_notif = bond->params.num_peer_notif *
> +			max(1, bond->params.peer_notif_delay);
> +		rtnl_unlock();
> +	}
> +}
> +
>  /**
>   * ad_mux_machine - handle a port's mux state machine
>   * @port: the port we're looking at
> @@ -1164,6 +1177,7 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
>  			port->actor_oper_port_state |= LACP_STATE_COLLECTING;
>  			port->actor_oper_port_state |= LACP_STATE_DISTRIBUTING;
>  			port->actor_oper_port_state |= LACP_STATE_SYNCHRONIZATION;
> +			ad_peer_notif_send(port);
>  			ad_enable_collecting_distributing(port,
>  							  update_slave_arr);
>  			port->ntt = true;

Maybe enable notify after collecting/distributing?

And also please rebase to latest net-next. There is another switch case
AD_MUX_DISTRIBUTING that enables collecting/distributing, which should
also send notify.

Thanks
Hangbin

Re: [PATCH net-next 3/4] net: bonding: send peer notify when failure recovery

[Tonghao Zhang]

> 2025年5月12日 下午4:05，Hangbin Liu <liuhangbin@gmail.com> 写道：
> 
> Hi Tonghao,
> On Sat, May 10, 2025 at 12:45:03PM +0800, tonghao@bamaicloud.com wrote:
>> From: Tonghao Zhang <tonghao@bamaicloud.com>
>> 
>> While hardware failures in NICs, optical transceivers, or switches
>> are unavoidable, rapid system recovery can be achieved post-restoration.
>> For example, triggering immediate ARP/ND packet transmission upon
>> LACP failure recovery enables the system to swiftly resume normal
>> operations, thereby minimizing service downtime.
>> 
>> Cc: Jay Vosburgh <jv@jvosburgh.net>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Cc: Eric Dumazet <edumazet@google.com>
>> Cc: Jakub Kicinski <kuba@kernel.org>
>> Cc: Paolo Abeni <pabeni@redhat.com>
>> Cc: Simon Horman <horms@kernel.org>
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: Andrew Lunn <andrew+netdev@lunn.ch>
>> Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
>> ---
>> drivers/net/bonding/bond_3ad.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>> 
>> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>> index c6807e473ab7..6577ce54d115 100644
>> --- a/drivers/net/bonding/bond_3ad.c
>> +++ b/drivers/net/bonding/bond_3ad.c
>> @@ -982,6 +982,19 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
>> 	return 0;
>> }
>> 
>> +static void ad_peer_notif_send(struct port *port)
>> +{
>> +	if (!port->aggregator->is_active)
>> +		return;
>> +
>> +	struct bonding *bond = port->slave->bond;
>> +	if (bond->params.broadcast_neighbor && rtnl_trylock()) {
>> +		bond->send_peer_notif = bond->params.num_peer_notif *
>> +			max(1, bond->params.peer_notif_delay);
>> +		rtnl_unlock();
>> +	}
>> +}
>> +
>> /**
>>  * ad_mux_machine - handle a port's mux state machine
>>  * @port: the port we're looking at
>> @@ -1164,6 +1177,7 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
>> 			port->actor_oper_port_state |= LACP_STATE_COLLECTING;
>> 			port->actor_oper_port_state |= LACP_STATE_DISTRIBUTING;
>> 			port->actor_oper_port_state |= LACP_STATE_SYNCHRONIZATION;
>> +			ad_peer_notif_send(port);
>> 			ad_enable_collecting_distributing(port,
>> 							  update_slave_arr);
>> 			port->ntt = true;
> 
> Maybe enable notify after collecting/distributing?
Yes, The same suggestion was provided by Jay. V2:

diff --git a/Documentation/networking/bonding.rst b/Documentation/networking/bonding.rst
index 14f7593d888d..f8f5766703d4 100644
--- a/Documentation/networking/bonding.rst
+++ b/Documentation/networking/bonding.rst
@@ -773,8 +773,9 @@ num_unsol_na
 	greater than 1.

 	The valid range is 0 - 255; the default value is 1.  These options
-	affect only the active-backup mode.  These options were added for
-	bonding versions 3.3.0 and 3.4.0 respectively.
+	affect the active-backup or 802.3ad (broadcast_neighbor enabled) mode.
+	These options were added for bonding versions 3.3.0 and 3.4.0
+	respectively.

 	From Linux 3.0 and bonding version 3.7.1, these notifications
 	are generated by the ipv4 and ipv6 code and the numbers of
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index c6807e473ab7..d1c2d416ac87 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -982,6 +982,17 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
 	return 0;
 }

+static void ad_cond_set_peer_notif(struct port *port)
+{
+	struct bonding *bond = port->slave->bond;
+
+	if (bond->params.broadcast_neighbor && rtnl_trylock()) {
+		bond->send_peer_notif = bond->params.num_peer_notif *
+			max(1, bond->params.peer_notif_delay);
+		rtnl_unlock();
+	}
+}
+
 /**
  * ad_mux_machine - handle a port's mux state machine
  * @port: the port we're looking at
@@ -2061,6 +2072,8 @@ static void ad_enable_collecting_distributing(struct port *port,
 		__enable_port(port);
 		/* Slave array needs update */
 		*update_slave_arr = true;
+		/* Should notify peers if possible */
+		ad_cond_set_peer_notif(port);
 	}
 }

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 342f2dc64116..ce31445e85b6 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1240,17 +1240,31 @@ static struct slave *bond_find_best_slave(struct bonding *bond)
 /* must be called in RCU critical section or with RTNL held */
 static bool bond_should_notify_peers(struct bonding *bond)
 {
-	struct slave *slave = rcu_dereference_rtnl(bond->curr_active_slave);
+	struct bond_up_slave *slaves;
+	struct slave *slave = NULL;
+
+	if (BOND_MODE(bond) == BOND_MODE_8023AD) {
+		if (!bond->params.broadcast_neighbor)
+			return false;
+
+		slaves = rtnl_dereference(bond->usable_slaves);
+		if (!slaves || !READ_ONCE(slaves->count))
+			return false;
+	} else {
+		slave = rcu_dereference_rtnl(bond->curr_active_slave);
+		if (!slave || test_bit(__LINK_STATE_LINKWATCH_PENDING,
+				       &slave->dev->state))
+			return false;
+	}

-	if (!slave || !bond->send_peer_notif ||
+	if (!bond->send_peer_notif ||
 	    bond->send_peer_notif %
 	    max(1, bond->params.peer_notif_delay) != 0 ||
-	    !netif_carrier_ok(bond->dev) ||
-	    test_bit(__LINK_STATE_LINKWATCH_PENDING, &slave->dev->state))
+	    !netif_carrier_ok(bond->dev))
 		return false;

 	netdev_dbg(bond->dev, "bond_should_notify_peers: slave %s\n",
-		   slave ? slave->dev->name : "NULL");
+		   slave ? slave->dev->name : "all");

 	return true;
 }
--
2.34.1

> 
> And also please rebase to latest net-next. There is another switch case
> AD_MUX_DISTRIBUTING that enables collecting/distributing, which should
> also send notify.
> 
> Thanks
> Hangbin
> 
> 

[PATCH net-next 4/4] net: bonding: add tracepoint for 802.3ad

[tonghao]

From: Tonghao Zhang <tonghao@bamaicloud.com>

Users can monitor NIC link status changes through netlink. However, LACP
protocol failures may occur despite operational physical links. There is
no way to detect LACP state changes. This patch add tracepoint at LACP state
transition.

Cc: Jay Vosburgh <jv@jvosburgh.net>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Andrew Lunn <andrew+netdev@lunn.ch>
Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
---
 drivers/net/bonding/bond_3ad.c |  6 ++++++
 include/trace/events/bonding.h | 37 ++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)
 create mode 100644 include/trace/events/bonding.h

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 6577ce54d115..962be118f3a8 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -16,6 +16,9 @@
 #include <net/bond_3ad.h>
 #include <net/netlink.h>
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/bonding.h>
+
 /* General definitions */
 #define AD_SHORT_TIMEOUT           1
 #define AD_LONG_TIMEOUT            0
@@ -1148,6 +1151,9 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
 			  port->actor_port_number,
 			  last_state,
 			  port->sm_mux_state);
+
+		trace_3ad_mux_state(port->slave->dev, last_state, port->sm_mux_state);
+
 		switch (port->sm_mux_state) {
 		case AD_MUX_DETACHED:
 			port->actor_oper_port_state &= ~LACP_STATE_SYNCHRONIZATION;
diff --git a/include/trace/events/bonding.h b/include/trace/events/bonding.h
new file mode 100644
index 000000000000..1ee4b07d912a
--- /dev/null
+++ b/include/trace/events/bonding.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#if !defined(_TRACE_BONDING_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_BONDING_H
+
+#include <linux/netdevice.h>
+#include <linux/tracepoint.h>
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM bonding
+
+TRACE_EVENT(3ad_mux_state,
+	TP_PROTO(struct net_device *dev, u32 last_state, u32 curr_state),
+	TP_ARGS(dev, last_state, curr_state),
+
+	TP_STRUCT__entry(
+		__field(int, ifindex)
+		__string(dev_name, dev->name)
+		__field(u32, last_state)
+		__field(u32, curr_state)
+	),
+
+	TP_fast_assign(
+		__entry->ifindex = dev->ifindex;
+		__assign_str(dev_name);
+		__entry->last_state = last_state;
+		__entry->curr_state = curr_state;
+	),
+
+	TP_printk("ifindex %d dev %s last_state 0x%x curr_state 0x%x",
+		  __entry->ifindex, __get_str(dev_name),
+		  __entry->last_state, __entry->curr_state)
+);
+
+#endif /* _TRACE_BONDING_H */
+
+#include <trace/define_trace.h>
-- 
2.34.1