* [net-next v8 0/3] add broadcast_neighbor for no-stacking networking arch
@ 2025-06-27 13:49 Tonghao Zhang
2025-06-27 13:49 ` [net-next v8 1/3] net: bonding: add broadcast_neighbor option for 802.3ad Tonghao Zhang
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Tonghao Zhang @ 2025-06-27 13:49 UTC (permalink / raw)
To: netdev
Cc: Tonghao Zhang, Jay Vosburgh, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Andrew Lunn, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Nikolay Aleksandrov, Zengbing Tu
For no-stacking networking arch, and enable the bond mode 4(lacp) in
datacenter, the switch require arp/nd packets as session synchronization.
More details please see patch.
v8 change log:
- please see change log in every patch
v7 change log:
- cleanup broadcast_neighbor when changing mode
- drop 4/4 patch, post separately
- add more commit info in 3/3 patch
v6 change log:
- del unnecessary rcu_read_lock in bond_xmit_broadcast
v5 change log:
- format commit message of all patches
- use skb_header_pointer instead of pskb_may_pull
- send only packets to active slaves instead of all ports, and add more commit log
v4 change log:
- fix dec option in bond_close
v3 change log:
- inc/dec broadcast_neighbor option in bond_open/close and UP state.
- remove explicit inline of bond_should_broadcast_neighbor
- remove sysfs option
- remove EXPORT_SYMBOL_GPL
- reorder option bond_opt_value
- use rcu_xxx in bond_should_notify_peers.
v2 change log:
- add static branch for performance
- add more info about no-stacking arch in commit message
- add broadcast_neighbor info and format doc
- invoke bond_should_broadcast_neighbor only in BOND_MODE_8023AD mode for performance
- explain why we need sending peer notify when failure recovery
- change the doc about num_unsol_na
- refine function name to ad_cond_set_peer_notif
- ad_cond_set_peer_notif invoked in ad_enable_collecting_distributing
- refine bond_should_notify_peers for lacp mode.
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>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Nikolay Aleksandrov <razor@blackwall.org>
Cc: Zengbing Tu <tuzengbing@didiglobal.com>
Tonghao Zhang (3):
net: bonding: add broadcast_neighbor option for 802.3ad
net: bonding: add broadcast_neighbor netlink option
net: bonding: send peer notify when failure recovery
Documentation/networking/bonding.rst | 11 +++-
drivers/net/bonding/bond_3ad.c | 13 ++++
drivers/net/bonding/bond_main.c | 91 ++++++++++++++++++++++++----
drivers/net/bonding/bond_netlink.c | 16 +++++
drivers/net/bonding/bond_options.c | 42 +++++++++++++
include/net/bond_options.h | 1 +
include/net/bonding.h | 3 +
include/uapi/linux/if_link.h | 1 +
8 files changed, 165 insertions(+), 13 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [net-next v8 1/3] net: bonding: add broadcast_neighbor option for 802.3ad
2025-06-27 13:49 [net-next v8 0/3] add broadcast_neighbor for no-stacking networking arch Tonghao Zhang
@ 2025-06-27 13:49 ` Tonghao Zhang
2025-10-14 8:41 ` Jiri Slaby
2025-10-14 9:12 ` Jiri Slaby
2025-06-27 13:49 ` [net-next v8 2/3] net: bonding: add broadcast_neighbor netlink option Tonghao Zhang
` (3 subsequent siblings)
4 siblings, 2 replies; 17+ messages in thread
From: Tonghao Zhang @ 2025-06-27 13:49 UTC (permalink / raw)
To: netdev
Cc: Tonghao Zhang, Jay Vosburgh, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Andrew Lunn, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Nikolay Aleksandrov, Zengbing Tu
Stacking technology is a type of technology used to expand ports on
Ethernet switches. It is widely used as a common access method in
large-scale Internet data center architectures. Years of practice
have proved that stacking technology has advantages and disadvantages
in high-reliability network architecture scenarios. For instance,
in stacking networking arch, conventional switch system upgrades
require multiple stacked devices to restart at the same time.
Therefore, it is inevitable that the business will be interrupted
for a while. It is for this reason that "no-stacking" in data centers
has become a trend. Additionally, when the stacking link connecting
the switches fails or is abnormal, the stack will split. Although it is
not common, it still happens in actual operation. The problem is that
after the split, it is equivalent to two switches with the same
configuration appearing in the network, causing network configuration
conflicts and ultimately interrupting the services carried by the
stacking system.
To improve network stability, "non-stacking" solutions have been
increasingly adopted, particularly by public cloud providers and
tech companies like Alibaba, Tencent, and Didi. "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.
Note that, with multiple aggregators, the current broadcast mode
logic will send only packets to the selected aggregator(s).
+-----------+ +-----------+
| switch1 | | switch2 |
+-----------+ +-----------+
^ ^
| |
+-----------------+
| bond4 lacp |
+-----------------+
| |
| NIC1 | NIC2
+-----------------+
| server |
+-----------------+
- 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>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Nikolay Aleksandrov <razor@blackwall.org>
Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
Signed-off-by: Zengbing Tu <tuzengbing@didiglobal.com>
---
v8: add comments info in bond_option_mode_set, explain why we only
clear broadcast_neighbor to 0.
Note that selftest will be post after I post the iproute2 patch about
this option.
---
Documentation/networking/bonding.rst | 6 +++
drivers/net/bonding/bond_main.c | 66 +++++++++++++++++++++++++---
drivers/net/bonding/bond_options.c | 42 ++++++++++++++++++
include/net/bond_options.h | 1 +
include/net/bonding.h | 3 ++
5 files changed, 112 insertions(+), 6 deletions(-)
diff --git a/Documentation/networking/bonding.rst b/Documentation/networking/bonding.rst
index a4c1291d2561..14f7593d888d 100644
--- a/Documentation/networking/bonding.rst
+++ b/Documentation/networking/bonding.rst
@@ -562,6 +562,12 @@ lacp_rate
The default is slow.
+broadcast_neighbor
+
+ Option specifying whether to broadcast ARP/ND packets to all
+ active slaves. This option has no effect in modes other than
+ 802.3ad mode. 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 c4d53e8e7c15..12046ef51569 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -212,6 +212,8 @@ atomic_t netpoll_block_tx = ATOMIC_INIT(0);
unsigned int bond_net_id __read_mostly;
+DEFINE_STATIC_KEY_FALSE(bond_bcast_neigh_enabled);
+
static const struct flow_dissector_key flow_keys_bonding_keys[] = {
{
.key_id = FLOW_DISSECTOR_KEY_CONTROL,
@@ -4456,6 +4458,9 @@ static int bond_open(struct net_device *bond_dev)
bond_for_each_slave(bond, slave, iter)
dev_mc_add(slave->dev, lacpdu_mcast_addr);
+
+ if (bond->params.broadcast_neighbor)
+ static_branch_inc(&bond_bcast_neigh_enabled);
}
if (bond_mode_can_use_xmit_hash(bond))
@@ -4475,6 +4480,10 @@ static int bond_close(struct net_device *bond_dev)
bond_alb_deinitialize(bond);
bond->recv_probe = NULL;
+ if (BOND_MODE(bond) == BOND_MODE_8023AD &&
+ bond->params.broadcast_neighbor)
+ static_branch_dec(&bond_bcast_neigh_enabled);
+
if (bond_uses_primary(bond)) {
rcu_read_lock();
slave = rcu_dereference(bond->curr_active_slave);
@@ -5310,6 +5319,37 @@ static struct slave *bond_xdp_xmit_3ad_xor_slave_get(struct bonding *bond,
return slaves->arr[hash % count];
}
+static bool bond_should_broadcast_neighbor(struct sk_buff *skb,
+ struct net_device *dev)
+{
+ struct bonding *bond = netdev_priv(dev);
+ struct {
+ struct ipv6hdr ip6;
+ struct icmp6hdr icmp6;
+ } *combined, _combined;
+
+ if (!static_branch_unlikely(&bond_bcast_neigh_enabled))
+ return false;
+
+ if (!bond->params.broadcast_neighbor)
+ return false;
+
+ if (skb->protocol == htons(ETH_P_ARP))
+ return true;
+
+ if (skb->protocol == htons(ETH_P_IPV6)) {
+ combined = skb_header_pointer(skb, skb_mac_header_len(skb),
+ sizeof(_combined),
+ &_combined);
+ if (combined && combined->ip6.nexthdr == NEXTHDR_ICMP &&
+ (combined->icmp6.icmp6_type == NDISC_NEIGHBOUR_SOLICITATION ||
+ combined->icmp6.icmp6_type == NDISC_NEIGHBOUR_ADVERTISEMENT))
+ return true;
+ }
+
+ 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.
@@ -5329,17 +5369,27 @@ static netdev_tx_t bond_3ad_xor_xmit(struct sk_buff *skb,
return bond_tx_drop(dev, skb);
}
-/* in broadcast mode, we send everything to all usable interfaces. */
+/* in broadcast mode, we send everything to all or usable slave interfaces.
+ * under rcu_read_lock when this function is called.
+ */
static netdev_tx_t bond_xmit_broadcast(struct sk_buff *skb,
- struct net_device *bond_dev)
+ struct net_device *bond_dev,
+ bool all_slaves)
{
struct bonding *bond = netdev_priv(bond_dev);
- struct slave *slave = NULL;
- struct list_head *iter;
+ struct bond_up_slave *slaves;
bool xmit_suc = false;
bool skb_used = false;
+ int slaves_count, i;
- bond_for_each_slave_rcu(bond, slave, iter) {
+ if (all_slaves)
+ slaves = rcu_dereference(bond->all_slaves);
+ else
+ slaves = rcu_dereference(bond->usable_slaves);
+
+ slaves_count = slaves ? READ_ONCE(slaves->count) : 0;
+ for (i = 0; i < slaves_count; i++) {
+ struct slave *slave = slaves->arr[i];
struct sk_buff *skb2;
if (!(bond_slave_is_up(slave) && slave->link == BOND_LINK_UP))
@@ -5577,10 +5627,13 @@ 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, false);
+ fallthrough;
case BOND_MODE_XOR:
return bond_3ad_xor_xmit(skb, dev);
case BOND_MODE_BROADCAST:
- return bond_xmit_broadcast(skb, dev);
+ return bond_xmit_broadcast(skb, dev, true);
case BOND_MODE_ALB:
return bond_alb_xmit(skb, dev);
case BOND_MODE_TLB:
@@ -6456,6 +6509,7 @@ static int __init bond_check_params(struct bond_params *params)
eth_zero_addr(params->ad_actor_system);
params->ad_user_port_key = ad_user_port_key;
params->coupled_control = 1;
+ params->broadcast_neighbor = 0;
if (packets_per_slave > 0) {
params->reciprocal_packets_per_slave =
reciprocal_value(packets_per_slave);
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 91893c29b899..1d639a3be6ba 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[] = {
+ { "off", 0, BOND_VALFLAG_DEFAULT},
+ { "on", 1, 0},
+ { 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 active slaves",
+ .unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
+ .values = bond_broadcast_neigh_tbl,
+ .set = bond_option_broadcast_neigh_set,
}
};
@@ -894,6 +910,13 @@ static int bond_option_mode_set(struct bonding *bond,
bond->params.arp_validate = BOND_ARP_VALIDATE_NONE;
bond->params.mode = newval->value;
+ /* When changing mode, the bond device is down, we may reduce
+ * the bond_bcast_neigh_enabled in bond_close() if broadcast_neighbor
+ * enabled in 8023ad mode. Therefore, only clear broadcast_neighbor
+ * to 0.
+ */
+ bond->params.broadcast_neighbor = 0;
+
if (bond->dev->reg_state == NETREG_REGISTERED) {
bool update = false;
@@ -1840,3 +1863,22 @@ 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)
+{
+ if (bond->params.broadcast_neighbor == newval->value)
+ return 0;
+
+ bond->params.broadcast_neighbor = newval->value;
+ if (bond->dev->flags & IFF_UP) {
+ if (bond->params.broadcast_neighbor)
+ static_branch_inc(&bond_bcast_neigh_enabled);
+ else
+ static_branch_dec(&bond_bcast_neigh_enabled);
+ }
+
+ netdev_dbg(bond->dev, "Setting broadcast_neighbor to %s (%llu)\n",
+ newval->string, newval->value);
+ return 0;
+}
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..e06f0d63b2c1 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -115,6 +115,8 @@ static inline int is_netpoll_tx_blocked(struct net_device *dev)
#define is_netpoll_tx_blocked(dev) (0)
#endif
+DECLARE_STATIC_KEY_FALSE(bond_bcast_neigh_enabled);
+
struct bond_params {
int mode;
int xmit_policy;
@@ -149,6 +151,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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [net-next v8 2/3] net: bonding: add broadcast_neighbor netlink option
2025-06-27 13:49 [net-next v8 0/3] add broadcast_neighbor for no-stacking networking arch Tonghao Zhang
2025-06-27 13:49 ` [net-next v8 1/3] net: bonding: add broadcast_neighbor option for 802.3ad Tonghao Zhang
@ 2025-06-27 13:49 ` Tonghao Zhang
2025-09-23 2:07 ` Hangbin Liu
2025-06-27 13:49 ` [net-next v8 3/3] net: bonding: send peer notify when failure recovery Tonghao Zhang
` (2 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Tonghao Zhang @ 2025-06-27 13:49 UTC (permalink / raw)
To: netdev
Cc: Tonghao Zhang, Jay Vosburgh, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Andrew Lunn, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Nikolay Aleksandrov, Zengbing Tu
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>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Nikolay Aleksandrov <razor@blackwall.org>
Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
Signed-off-by: Zengbing Tu <tuzengbing@didiglobal.com>
Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
---
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 873c285996fe..784ace3a519c 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -1535,6 +1535,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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [net-next v8 3/3] net: bonding: send peer notify when failure recovery
2025-06-27 13:49 [net-next v8 0/3] add broadcast_neighbor for no-stacking networking arch Tonghao Zhang
2025-06-27 13:49 ` [net-next v8 1/3] net: bonding: add broadcast_neighbor option for 802.3ad Tonghao Zhang
2025-06-27 13:49 ` [net-next v8 2/3] net: bonding: add broadcast_neighbor netlink option Tonghao Zhang
@ 2025-06-27 13:49 ` Tonghao Zhang
2025-10-15 3:36 ` Hangbin Liu
2025-07-08 9:10 ` [net-next v8 0/3] add broadcast_neighbor for no-stacking networking arch patchwork-bot+netdevbpf
2025-09-24 2:23 ` Hangbin Liu
4 siblings, 1 reply; 17+ messages in thread
From: Tonghao Zhang @ 2025-06-27 13:49 UTC (permalink / raw)
To: netdev
Cc: Tonghao Zhang, Jay Vosburgh, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Andrew Lunn, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Nikolay Aleksandrov, Zengbing Tu
In LACP mode with broadcast_neighbor enabled, after LACP protocol
recovery, the port can transmit packets. However, if the bond port
doesn't send gratuitous ARP/ND packets to the switch, the switch
won't return packets through the current interface. This causes
traffic imbalance. To resolve this issue, when LACP protocol recovers,
send ARP/ND packets if broadcast_neighbor is enabled.
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>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Nikolay Aleksandrov <razor@blackwall.org>
Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
Signed-off-by: Zengbing Tu <tuzengbing@didiglobal.com>
Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
---
v8: add comments info in bond_should_notify_peers, explain why we can
check only the mode even through mode is changed.
---
Documentation/networking/bonding.rst | 5 +++--
drivers/net/bonding/bond_3ad.c | 13 +++++++++++++
drivers/net/bonding/bond_main.c | 25 ++++++++++++++++++++-----
3 files changed, 36 insertions(+), 7 deletions(-)
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 12046ef51569..17c7542be6a5 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1237,17 +1237,32 @@ 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 *usable;
+ struct slave *slave = NULL;
- 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;
+ /* The send_peer_notif is set by active-backup or 8023ad
+ * mode, and cleared in bond_close() when changing mode.
+ * It is safe to only check bond mode here.
+ */
+ if (BOND_MODE(bond) == BOND_MODE_8023AD) {
+ usable = rcu_dereference_rtnl(bond->usable_slaves);
+ if (!usable || !READ_ONCE(usable->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;
+ }
+
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [net-next v8 0/3] add broadcast_neighbor for no-stacking networking arch
2025-06-27 13:49 [net-next v8 0/3] add broadcast_neighbor for no-stacking networking arch Tonghao Zhang
` (2 preceding siblings ...)
2025-06-27 13:49 ` [net-next v8 3/3] net: bonding: send peer notify when failure recovery Tonghao Zhang
@ 2025-07-08 9:10 ` patchwork-bot+netdevbpf
2025-09-24 2:23 ` Hangbin Liu
4 siblings, 0 replies; 17+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-07-08 9:10 UTC (permalink / raw)
To: Tonghao Zhang
Cc: netdev, jv, davem, edumazet, kuba, pabeni, horms, corbet,
andrew+netdev, rostedt, mhiramat, mathieu.desnoyers, razor,
tuzengbing
Hello:
This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Fri, 27 Jun 2025 21:49:27 +0800 you wrote:
> For no-stacking networking arch, and enable the bond mode 4(lacp) in
> datacenter, the switch require arp/nd packets as session synchronization.
> More details please see patch.
>
> v8 change log:
> - please see change log in every patch
>
> [...]
Here is the summary with links:
- [net-next,v8,1/3] net: bonding: add broadcast_neighbor option for 802.3ad
https://git.kernel.org/netdev/net-next/c/ce7a381697cb
- [net-next,v8,2/3] net: bonding: add broadcast_neighbor netlink option
https://git.kernel.org/netdev/net-next/c/3d98ee52659c
- [net-next,v8,3/3] net: bonding: send peer notify when failure recovery
https://git.kernel.org/netdev/net-next/c/2f9afffc399d
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [net-next v8 2/3] net: bonding: add broadcast_neighbor netlink option
2025-06-27 13:49 ` [net-next v8 2/3] net: bonding: add broadcast_neighbor netlink option Tonghao Zhang
@ 2025-09-23 2:07 ` Hangbin Liu
2025-09-23 8:32 ` Tonghao Zhang
0 siblings, 1 reply; 17+ messages in thread
From: Hangbin Liu @ 2025-09-23 2:07 UTC (permalink / raw)
To: Tonghao Zhang
Cc: netdev, Jay Vosburgh, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Andrew Lunn, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Nikolay Aleksandrov, Zengbing Tu
On Fri, Jun 27, 2025 at 09:49:29PM +0800, Tonghao Zhang wrote:
> User can config or display the bonding broadcast_neighbor option via
> iproute2/netlink.
Hi Tonghao,
Have you post the iproute2 patch? I can't find it.
Thanks
Hangbin
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [net-next v8 2/3] net: bonding: add broadcast_neighbor netlink option
2025-09-23 2:07 ` Hangbin Liu
@ 2025-09-23 8:32 ` Tonghao Zhang
0 siblings, 0 replies; 17+ messages in thread
From: Tonghao Zhang @ 2025-09-23 8:32 UTC (permalink / raw)
To: Hangbin Liu
Cc: netdev, Jay Vosburgh, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Andrew Lunn, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Nikolay Aleksandrov, Zengbing Tu
> 2025年9月23日 10:07,Hangbin Liu <liuhangbin@gmail.com> 写道:
>
> On Fri, Jun 27, 2025 at 09:49:29PM +0800, Tonghao Zhang wrote:
>> User can config or display the bonding broadcast_neighbor option via
>> iproute2/netlink.
>
> Hi Tonghao,
>
> Have you post the iproute2 patch? I can't find it.
Yes, today, David suggested that I repost the patch for iproute2. I will cc you.
>
> Thanks
> Hangbin
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [net-next v8 0/3] add broadcast_neighbor for no-stacking networking arch
2025-06-27 13:49 [net-next v8 0/3] add broadcast_neighbor for no-stacking networking arch Tonghao Zhang
` (3 preceding siblings ...)
2025-07-08 9:10 ` [net-next v8 0/3] add broadcast_neighbor for no-stacking networking arch patchwork-bot+netdevbpf
@ 2025-09-24 2:23 ` Hangbin Liu
2025-09-28 4:11 ` Tonghao Zhang
4 siblings, 1 reply; 17+ messages in thread
From: Hangbin Liu @ 2025-09-24 2:23 UTC (permalink / raw)
To: Tonghao Zhang
Cc: netdev, Jay Vosburgh, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Andrew Lunn, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Nikolay Aleksandrov, Zengbing Tu, Liang Li
On Fri, Jun 27, 2025 at 09:49:27PM +0800, Tonghao Zhang wrote:
> For no-stacking networking arch, and enable the bond mode 4(lacp) in
> datacenter, the switch require arp/nd packets as session synchronization.
> More details please see patch.
Hi Tonghao,
Our engineer has a question about this feature. Since the switch requires
ARP/ND packets for session synchronization, do we also need to send IGMP
join/leave messages to the switch for synchronization?
Thanks
Hangbin
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [net-next v8 0/3] add broadcast_neighbor for no-stacking networking arch
2025-09-24 2:23 ` Hangbin Liu
@ 2025-09-28 4:11 ` Tonghao Zhang
2025-09-29 8:35 ` Hangbin Liu
0 siblings, 1 reply; 17+ messages in thread
From: Tonghao Zhang @ 2025-09-28 4:11 UTC (permalink / raw)
To: Hangbin Liu
Cc: netdev, Jay Vosburgh, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Andrew Lunn, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Nikolay Aleksandrov, Zengbing Tu, Liang Li
> On Sep 24, 2025, at 10:23, Hangbin Liu <liuhangbin@gmail.com> wrote:
>
> On Fri, Jun 27, 2025 at 09:49:27PM +0800, Tonghao Zhang wrote:
>> For no-stacking networking arch, and enable the bond mode 4(lacp) in
>> datacenter, the switch require arp/nd packets as session synchronization.
>> More details please see patch.
>
> Hi Tonghao,
>
> Our engineer has a question about this feature. Since the switch requires
> ARP/ND packets for session synchronization, do we also need to send IGMP
> join/leave messages to the switch for synchronization?
Hello, I'm very sorry for replying to your question so late. In fact, the non-stacking network architecture disables the multicast function to prevent the server from learning other server real mac addresses. This architecture uses the arp proxy. To better answer your question, I post a blog:
https://huatuo.tech/blog/2025-09-26-some-thoughts-on-non-stacking-network-architecture/
>
> Thanks
> Hangbin
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [net-next v8 0/3] add broadcast_neighbor for no-stacking networking arch
2025-09-28 4:11 ` Tonghao Zhang
@ 2025-09-29 8:35 ` Hangbin Liu
0 siblings, 0 replies; 17+ messages in thread
From: Hangbin Liu @ 2025-09-29 8:35 UTC (permalink / raw)
To: Tonghao Zhang
Cc: netdev, Jay Vosburgh, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Andrew Lunn, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Nikolay Aleksandrov, Zengbing Tu, Liang Li
On Sun, Sep 28, 2025 at 12:11:49PM +0800, Tonghao Zhang wrote:
>
>
> > On Sep 24, 2025, at 10:23, Hangbin Liu <liuhangbin@gmail.com> wrote:
> >
> > On Fri, Jun 27, 2025 at 09:49:27PM +0800, Tonghao Zhang wrote:
> >> For no-stacking networking arch, and enable the bond mode 4(lacp) in
> >> datacenter, the switch require arp/nd packets as session synchronization.
> >> More details please see patch.
> >
> > Hi Tonghao,
> >
> > Our engineer has a question about this feature. Since the switch requires
> > ARP/ND packets for session synchronization, do we also need to send IGMP
> > join/leave messages to the switch for synchronization?
> Hello, I'm very sorry for replying to your question so late. In fact, the non-stacking network architecture disables the multicast function to prevent the server from learning other server real mac addresses. This architecture uses the arp proxy. To better answer your question, I post a blog:
> https://huatuo.tech/blog/2025-09-26-some-thoughts-on-non-stacking-network-architecture/
Thanks for your explanation.
Regards
Hangbin
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [net-next v8 1/3] net: bonding: add broadcast_neighbor option for 802.3ad
2025-06-27 13:49 ` [net-next v8 1/3] net: bonding: add broadcast_neighbor option for 802.3ad Tonghao Zhang
@ 2025-10-14 8:41 ` Jiri Slaby
2025-10-14 9:12 ` Jiri Slaby
1 sibling, 0 replies; 17+ messages in thread
From: Jiri Slaby @ 2025-10-14 8:41 UTC (permalink / raw)
To: Tonghao Zhang, netdev
Cc: Jay Vosburgh, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Jonathan Corbet, Andrew Lunn,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Nikolay Aleksandrov, Zengbing Tu
On 27. 06. 25, 15:49, Tonghao Zhang wrote:
> Stacking technology is a type of technology used to expand ports on
> Ethernet switches. It is widely used as a common access method in
> large-scale Internet data center architectures. Years of practice
> have proved that stacking technology has advantages and disadvantages
> in high-reliability network architecture scenarios. For instance,
> in stacking networking arch, conventional switch system upgrades
> require multiple stacked devices to restart at the same time.
> Therefore, it is inevitable that the business will be interrupted
> for a while. It is for this reason that "no-stacking" in data centers
> has become a trend. Additionally, when the stacking link connecting
> the switches fails or is abnormal, the stack will split. Although it is
> not common, it still happens in actual operation. The problem is that
> after the split, it is equivalent to two switches with the same
> configuration appearing in the network, causing network configuration
> conflicts and ultimately interrupting the services carried by the
> stacking system.
>
> To improve network stability, "non-stacking" solutions have been
> increasingly adopted, particularly by public cloud providers and
> tech companies like Alibaba, Tencent, and Didi. "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.
>
> Note that, with multiple aggregators, the current broadcast mode
> logic will send only packets to the selected aggregator(s).
>
> +-----------+ +-----------+
> | switch1 | | switch2 |
> +-----------+ +-----------+
> ^ ^
> | |
> +-----------------+
> | bond4 lacp |
> +-----------------+
> | |
> | NIC1 | NIC2
> +-----------------+
> | server |
> +-----------------+
Hi,
this breaks broadcast bonding in 6.17. Reverting these three (the two
depend on this one) makes 6.17 work again:
2f9afffc399d net: bonding: send peer notify when failure recovery
3d98ee52659c net: bonding: add broadcast_neighbor netlink option
ce7a381697cb net: bonding: add broadcast_neighbor option for 802.3ad
This was reported downstream as an error in our openQA:
https://bugzilla.suse.com/show_bug.cgi?id=1250894
I bisected using this in qemu:
systemctl stop network
ip link del bond0 || true
ip link set dev eth0 down
ip addr flush eth0
ip link add bond0 type bond mode broadcast
ip link set dev eth0 master bond0
ip addr add 10.0.2.15/24 dev bond0
ip link set bond0 up
sleep 1
exec nmap -sS 10.0.2.2/32
Any ideas?
> - 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>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Nikolay Aleksandrov <razor@blackwall.org>
> Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
> Signed-off-by: Zengbing Tu <tuzengbing@didiglobal.com>
> ---
--
js
suse labs
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [net-next v8 1/3] net: bonding: add broadcast_neighbor option for 802.3ad
2025-06-27 13:49 ` [net-next v8 1/3] net: bonding: add broadcast_neighbor option for 802.3ad Tonghao Zhang
2025-10-14 8:41 ` Jiri Slaby
@ 2025-10-14 9:12 ` Jiri Slaby
2025-10-14 9:22 ` Jiri Slaby
2025-10-14 12:52 ` Tonghao Zhang
1 sibling, 2 replies; 17+ messages in thread
From: Jiri Slaby @ 2025-10-14 9:12 UTC (permalink / raw)
To: Tonghao Zhang, netdev
Cc: Jay Vosburgh, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Jonathan Corbet, Andrew Lunn,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Nikolay Aleksandrov, Zengbing Tu
On 27. 06. 25, 15:49, Tonghao Zhang wrote:
> Stacking technology is a type of technology used to expand ports on
> Ethernet switches. It is widely used as a common access method in
> large-scale Internet data center architectures. Years of practice
> have proved that stacking technology has advantages and disadvantages
> in high-reliability network architecture scenarios. For instance,
> in stacking networking arch, conventional switch system upgrades
> require multiple stacked devices to restart at the same time.
> Therefore, it is inevitable that the business will be interrupted
> for a while. It is for this reason that "no-stacking" in data centers
> has become a trend. Additionally, when the stacking link connecting
> the switches fails or is abnormal, the stack will split. Although it is
> not common, it still happens in actual operation. The problem is that
> after the split, it is equivalent to two switches with the same
> configuration appearing in the network, causing network configuration
> conflicts and ultimately interrupting the services carried by the
> stacking system.
>
> To improve network stability, "non-stacking" solutions have been
> increasingly adopted, particularly by public cloud providers and
> tech companies like Alibaba, Tencent, and Didi. "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.
>
> Note that, with multiple aggregators, the current broadcast mode
> logic will send only packets to the selected aggregator(s).
>
> +-----------+ +-----------+
> | switch1 | | switch2 |
> +-----------+ +-----------+
> ^ ^
> | |
> +-----------------+
> | bond4 lacp |
> +-----------------+
> | |
> | NIC1 | NIC2
> +-----------------+
> | server |
> +-----------------+
Hi,
this breaks broadcast bonding in 6.17. Reverting these three (the two
depend on this one) makes 6.17 work again:
2f9afffc399d net: bonding: send peer notify when failure recovery
3d98ee52659c net: bonding: add broadcast_neighbor netlink option
ce7a381697cb net: bonding: add broadcast_neighbor option for 802.3ad
This was reported downstream as an error in our openQA:
https://bugzilla.suse.com/show_bug.cgi?id=1250894
I bisected using this in qemu:
systemctl stop network
ip link del bond0 || true
ip link set dev eth0 down
ip addr flush eth0
ip link add bond0 type bond mode broadcast
ip link set dev eth0 master bond0
ip addr add 10.0.2.15/24 dev bond0
ip link set bond0 up
sleep 1
exec nmap -sS 10.0.2.2/32
Any ideas?
> - 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>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Nikolay Aleksandrov <razor@blackwall.org>
> Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
> Signed-off-by: Zengbing Tu <tuzengbing@didiglobal.com>
> ---
> v8: add comments info in bond_option_mode_set, explain why we only
> clear broadcast_neighbor to 0.
> Note that selftest will be post after I post the iproute2 patch about
> this option.
> ---
> Documentation/networking/bonding.rst | 6 +++
> drivers/net/bonding/bond_main.c | 66 +++++++++++++++++++++++++---
> drivers/net/bonding/bond_options.c | 42 ++++++++++++++++++
> include/net/bond_options.h | 1 +
> include/net/bonding.h | 3 ++
> 5 files changed, 112 insertions(+), 6 deletions(-)
>
...
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
...
> @@ -5329,17 +5369,27 @@ static netdev_tx_t bond_3ad_xor_xmit(struct sk_buff *skb,
> return bond_tx_drop(dev, skb);
> }
>
> -/* in broadcast mode, we send everything to all usable interfaces. */
> +/* in broadcast mode, we send everything to all or usable slave interfaces.
> + * under rcu_read_lock when this function is called.
> + */
> static netdev_tx_t bond_xmit_broadcast(struct sk_buff *skb,
> - struct net_device *bond_dev)
> + struct net_device *bond_dev,
> + bool all_slaves)
> {
> struct bonding *bond = netdev_priv(bond_dev);
> - struct slave *slave = NULL;
> - struct list_head *iter;
> + struct bond_up_slave *slaves;
> bool xmit_suc = false;
> bool skb_used = false;
> + int slaves_count, i;
>
> - bond_for_each_slave_rcu(bond, slave, iter) {
> + if (all_slaves)
> + slaves = rcu_dereference(bond->all_slaves);
> + else
> + slaves = rcu_dereference(bond->usable_slaves);
> +
> + slaves_count = slaves ? READ_ONCE(slaves->count) : 0;
OK, slaves_count is now 0 (slaves and bond->all_slaves are NULL), but
bond_for_each_slave_rcu() used to yield 1 iface.
Well, bond_update_slave_arr() is not called for broadcast AFAICS.
> + for (i = 0; i < slaves_count; i++) {
> + struct slave *slave = slaves->arr[i];
> struct sk_buff *skb2;
>
> if (!(bond_slave_is_up(slave) && slave->link == BOND_LINK_UP))
> @@ -5577,10 +5627,13 @@ 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, false);
> + fallthrough;
> case BOND_MODE_XOR:
> return bond_3ad_xor_xmit(skb, dev);
> case BOND_MODE_BROADCAST:
> - return bond_xmit_broadcast(skb, dev);
> + return bond_xmit_broadcast(skb, dev, true);
> case BOND_MODE_ALB:
> return bond_alb_xmit(skb, dev);
> case BOND_MODE_TLB:
> @@ -6456,6 +6509,7 @@ static int __init bond_check_params(struct bond_params *params)
> eth_zero_addr(params->ad_actor_system);
> params->ad_user_port_key = ad_user_port_key;
> params->coupled_control = 1;
> + params->broadcast_neighbor = 0;
> if (packets_per_slave > 0) {
> params->reciprocal_packets_per_slave =
> reciprocal_value(packets_per_slave);
--
js
suse labs
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [net-next v8 1/3] net: bonding: add broadcast_neighbor option for 802.3ad
2025-10-14 9:12 ` Jiri Slaby
@ 2025-10-14 9:22 ` Jiri Slaby
2025-10-14 12:52 ` Tonghao Zhang
1 sibling, 0 replies; 17+ messages in thread
From: Jiri Slaby @ 2025-10-14 9:22 UTC (permalink / raw)
To: Tonghao Zhang, netdev
Cc: Jay Vosburgh, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Jonathan Corbet, Andrew Lunn,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Nikolay Aleksandrov, Zengbing Tu
[-- Attachment #1: Type: text/plain, Size: 6337 bytes --]
On 14. 10. 25, 11:12, Jiri Slaby wrote:
> On 27. 06. 25, 15:49, Tonghao Zhang wrote:
>> Stacking technology is a type of technology used to expand ports on
>> Ethernet switches. It is widely used as a common access method in
>> large-scale Internet data center architectures. Years of practice
>> have proved that stacking technology has advantages and disadvantages
>> in high-reliability network architecture scenarios. For instance,
>> in stacking networking arch, conventional switch system upgrades
>> require multiple stacked devices to restart at the same time.
>> Therefore, it is inevitable that the business will be interrupted
>> for a while. It is for this reason that "no-stacking" in data centers
>> has become a trend. Additionally, when the stacking link connecting
>> the switches fails or is abnormal, the stack will split. Although it is
>> not common, it still happens in actual operation. The problem is that
>> after the split, it is equivalent to two switches with the same
>> configuration appearing in the network, causing network configuration
>> conflicts and ultimately interrupting the services carried by the
>> stacking system.
>>
>> To improve network stability, "non-stacking" solutions have been
>> increasingly adopted, particularly by public cloud providers and
>> tech companies like Alibaba, Tencent, and Didi. "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.
>>
>> Note that, with multiple aggregators, the current broadcast mode
>> logic will send only packets to the selected aggregator(s).
>>
>> +-----------+ +-----------+
>> | switch1 | | switch2 |
>> +-----------+ +-----------+
>> ^ ^
>> | |
>> +-----------------+
>> | bond4 lacp |
>> +-----------------+
>> | |
>> | NIC1 | NIC2
>> +-----------------+
>> | server |
>> +-----------------+
>
> Hi,
>
> this breaks broadcast bonding in 6.17. Reverting these three (the two
> depend on this one) makes 6.17 work again:
> 2f9afffc399d net: bonding: send peer notify when failure recovery
> 3d98ee52659c net: bonding: add broadcast_neighbor netlink option
> ce7a381697cb net: bonding: add broadcast_neighbor option for 802.3ad
>
> This was reported downstream as an error in our openQA:
> https://bugzilla.suse.com/show_bug.cgi?id=1250894
>
> I bisected using this in qemu:
> systemctl stop network
> ip link del bond0 || true
> ip link set dev eth0 down
> ip addr flush eth0
> ip link add bond0 type bond mode broadcast
> ip link set dev eth0 master bond0
> ip addr add 10.0.2.15/24 dev bond0
> ip link set bond0 up
> sleep 1
> exec nmap -sS 10.0.2.2/32
>
> Any ideas?
>
>> - 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>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Masami Hiramatsu <mhiramat@kernel.org>
>> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> Cc: Nikolay Aleksandrov <razor@blackwall.org>
>> Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
>> Signed-off-by: Zengbing Tu <tuzengbing@didiglobal.com>
>> ---
>> v8: add comments info in bond_option_mode_set, explain why we only
>> clear broadcast_neighbor to 0.
>> Note that selftest will be post after I post the iproute2 patch about
>> this option.
>> ---
>> Documentation/networking/bonding.rst | 6 +++
>> drivers/net/bonding/bond_main.c | 66 +++++++++++++++++++++++++---
>> drivers/net/bonding/bond_options.c | 42 ++++++++++++++++++
>> include/net/bond_options.h | 1 +
>> include/net/bonding.h | 3 ++
>> 5 files changed, 112 insertions(+), 6 deletions(-)
>>
> ...
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
> ...
>> @@ -5329,17 +5369,27 @@ static netdev_tx_t bond_3ad_xor_xmit(struct
>> sk_buff *skb,
>> return bond_tx_drop(dev, skb);
>> }
>> -/* in broadcast mode, we send everything to all usable interfaces. */
>> +/* in broadcast mode, we send everything to all or usable slave
>> interfaces.
>> + * under rcu_read_lock when this function is called.
>> + */
>> static netdev_tx_t bond_xmit_broadcast(struct sk_buff *skb,
>> - struct net_device *bond_dev)
>> + struct net_device *bond_dev,
>> + bool all_slaves)
>> {
>> struct bonding *bond = netdev_priv(bond_dev);
>> - struct slave *slave = NULL;
>> - struct list_head *iter;
>> + struct bond_up_slave *slaves;
>> bool xmit_suc = false;
>> bool skb_used = false;
>> + int slaves_count, i;
>> - bond_for_each_slave_rcu(bond, slave, iter) {
>> + if (all_slaves)
>> + slaves = rcu_dereference(bond->all_slaves);
>> + else
>> + slaves = rcu_dereference(bond->usable_slaves);
>> +
>> + slaves_count = slaves ? READ_ONCE(slaves->count) : 0;
>
> OK, slaves_count is now 0 (slaves and bond->all_slaves are NULL), but
> bond_for_each_slave_rcu() used to yield 1 iface.
>
> Well, bond_update_slave_arr() is not called for broadcast AFAICS.
The attached patch fixes it for me. But I have no idea if it is correct...
thanks,
--
js
suse labs
[-- Attachment #2: 0001-fix.patch --]
[-- Type: text/x-patch, Size: 2749 bytes --]
From b1759462922450aad50e4ccdbb0450f4c3674204 Mon Sep 17 00:00:00 2001
From: Jiri Slaby <jslaby@suse.cz>
Date: Tue, 14 Oct 2025 11:21:21 +0200
Subject: [PATCH] fix
---
drivers/net/bonding/bond_main.c | 62 +++++++++++++++++++--------------
1 file changed, 35 insertions(+), 27 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 57be04f6cb11..e90ab5d0b718 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -5384,6 +5384,31 @@ static netdev_tx_t bond_3ad_xor_xmit(struct sk_buff *skb,
return bond_tx_drop(dev, skb);
}
+static void loop(struct net_device *bond_dev, struct slave *slave, struct sk_buff *skb,
+ bool *skb_used, bool *xmit_suc)
+{
+ struct bonding *bond = netdev_priv(bond_dev);
+ struct sk_buff *skb2;
+
+ if (!(bond_slave_is_up(slave) && slave->link == BOND_LINK_UP))
+ return;
+
+ if (bond_is_last_slave(bond, slave)) {
+ skb2 = skb;
+ *skb_used = true;
+ } else {
+ skb2 = skb_clone(skb, GFP_ATOMIC);
+ if (!skb2) {
+ net_err_ratelimited("%s: Error: %s: skb_clone() failed\n",
+ bond_dev->name, __func__);
+ return;
+ }
+ }
+
+ if (bond_dev_queue_xmit(bond, skb2, slave->dev) == NETDEV_TX_OK)
+ *xmit_suc = true;
+}
+
/* in broadcast mode, we send everything to all or usable slave interfaces.
* under rcu_read_lock when this function is called.
*/
@@ -5395,35 +5420,18 @@ static netdev_tx_t bond_xmit_broadcast(struct sk_buff *skb,
struct bond_up_slave *slaves;
bool xmit_suc = false;
bool skb_used = false;
- int slaves_count, i;
-
- if (all_slaves)
- slaves = rcu_dereference(bond->all_slaves);
- else
- slaves = rcu_dereference(bond->usable_slaves);
- slaves_count = slaves ? READ_ONCE(slaves->count) : 0;
- for (i = 0; i < slaves_count; i++) {
- struct slave *slave = slaves->arr[i];
- struct sk_buff *skb2;
-
- if (!(bond_slave_is_up(slave) && slave->link == BOND_LINK_UP))
- continue;
-
- if (bond_is_last_slave(bond, slave)) {
- skb2 = skb;
- skb_used = true;
- } else {
- skb2 = skb_clone(skb, GFP_ATOMIC);
- if (!skb2) {
- net_err_ratelimited("%s: Error: %s: skb_clone() failed\n",
- bond_dev->name, __func__);
- continue;
- }
+ if (all_slaves) {
+ struct slave *slave = NULL;
+ struct list_head *iter;
+ bond_for_each_slave_rcu(bond, slave, iter) {
+ loop(bond_dev, slave, skb, &skb_used, &xmit_suc);
}
-
- if (bond_dev_queue_xmit(bond, skb2, slave->dev) == NETDEV_TX_OK)
- xmit_suc = true;
+ } else {
+ slaves = rcu_dereference(bond->usable_slaves);
+ int slaves_count = slaves ? READ_ONCE(slaves->count) : 0;
+ for (int i = 0; i < slaves_count; i++)
+ loop(bond_dev, slaves->arr[i], skb, &skb_used, &xmit_suc);
}
if (!skb_used)
--
2.51.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [net-next v8 1/3] net: bonding: add broadcast_neighbor option for 802.3ad
2025-10-14 9:12 ` Jiri Slaby
2025-10-14 9:22 ` Jiri Slaby
@ 2025-10-14 12:52 ` Tonghao Zhang
2025-10-15 5:31 ` Jiri Slaby
1 sibling, 1 reply; 17+ messages in thread
From: Tonghao Zhang @ 2025-10-14 12:52 UTC (permalink / raw)
To: Jiri Slaby
Cc: netdev, Jay Vosburgh, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Andrew Lunn, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Nikolay Aleksandrov, Zengbing Tu
> On Oct 14, 2025, at 17:12, Jiri Slaby <jirislaby@kernel.org> wrote:
>
> On 27. 06. 25, 15:49, Tonghao Zhang wrote:
>> Stacking technology is a type of technology used to expand ports on
>> Ethernet switches. It is widely used as a common access method in
>> large-scale Internet data center architectures. Years of practice
>> have proved that stacking technology has advantages and disadvantages
>> in high-reliability network architecture scenarios. For instance,
>> in stacking networking arch, conventional switch system upgrades
>> require multiple stacked devices to restart at the same time.
>> Therefore, it is inevitable that the business will be interrupted
>> for a while. It is for this reason that "no-stacking" in data centers
>> has become a trend. Additionally, when the stacking link connecting
>> the switches fails or is abnormal, the stack will split. Although it is
>> not common, it still happens in actual operation. The problem is that
>> after the split, it is equivalent to two switches with the same
>> configuration appearing in the network, causing network configuration
>> conflicts and ultimately interrupting the services carried by the
>> stacking system.
>> To improve network stability, "non-stacking" solutions have been
>> increasingly adopted, particularly by public cloud providers and
>> tech companies like Alibaba, Tencent, and Didi. "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.
>> Note that, with multiple aggregators, the current broadcast mode
>> logic will send only packets to the selected aggregator(s).
>> +-----------+ +-----------+
>> | switch1 | | switch2 |
>> +-----------+ +-----------+
>> ^ ^
>> | |
>> +-----------------+
>> | bond4 lacp |
>> +-----------------+
>> | |
>> | NIC1 | NIC2
>> +-----------------+
>> | server |
>> +-----------------+
>
> Hi,
>
> this breaks broadcast bonding in 6.17. Reverting these three (the two depend on this one) makes 6.17 work again:
> 2f9afffc399d net: bonding: send peer notify when failure recovery
> 3d98ee52659c net: bonding: add broadcast_neighbor netlink option
> ce7a381697cb net: bonding: add broadcast_neighbor option for 802.3ad
>
> This was reported downstream as an error in our openQA:
> https://bugzilla.suse.com/show_bug.cgi?id=1250894
>
> I bisected using this in qemu:
> systemctl stop network
> ip link del bond0 || true
> ip link set dev eth0 down
> ip addr flush eth0
> ip link add bond0 type bond mode broadcast
> ip link set dev eth0 master bond0
> ip addr add 10.0.2.15/24 dev bond0
> ip link set bond0 up
> sleep 1
> exec nmap -sS 10.0.2.2/32
>
> Any ideas?
>
>> - 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>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Masami Hiramatsu <mhiramat@kernel.org>
>> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> Cc: Nikolay Aleksandrov <razor@blackwall.org>
>> Signed-off-by: Tonghao Zhang <tonghao@bamaicloud.com>
>> Signed-off-by: Zengbing Tu <tuzengbing@didiglobal.com>
>> ---
>> v8: add comments info in bond_option_mode_set, explain why we only
>> clear broadcast_neighbor to 0.
>> Note that selftest will be post after I post the iproute2 patch about
>> this option.
>> ---
>> Documentation/networking/bonding.rst | 6 +++
>> drivers/net/bonding/bond_main.c | 66 +++++++++++++++++++++++++---
>> drivers/net/bonding/bond_options.c | 42 ++++++++++++++++++
>> include/net/bond_options.h | 1 +
>> include/net/bonding.h | 3 ++
>> 5 files changed, 112 insertions(+), 6 deletions(-)
> ...
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
> ...
>> @@ -5329,17 +5369,27 @@ static netdev_tx_t bond_3ad_xor_xmit(struct sk_buff *skb,
>> return bond_tx_drop(dev, skb);
>> }
>> -/* in broadcast mode, we send everything to all usable interfaces. */
>> +/* in broadcast mode, we send everything to all or usable slave interfaces.
>> + * under rcu_read_lock when this function is called.
>> + */
>> static netdev_tx_t bond_xmit_broadcast(struct sk_buff *skb,
>> - struct net_device *bond_dev)
>> + struct net_device *bond_dev,
>> + bool all_slaves)
>> {
>> struct bonding *bond = netdev_priv(bond_dev);
>> - struct slave *slave = NULL;
>> - struct list_head *iter;
>> + struct bond_up_slave *slaves;
>> bool xmit_suc = false;
>> bool skb_used = false;
>> + int slaves_count, i;
>> - bond_for_each_slave_rcu(bond, slave, iter) {
>> + if (all_slaves)
>> + slaves = rcu_dereference(bond->all_slaves);
>> + else
>> + slaves = rcu_dereference(bond->usable_slaves);
>> +
>> + slaves_count = slaves ? READ_ONCE(slaves->count) : 0;
>
> OK, slaves_count is now 0 (slaves and bond->all_slaves are NULL), but bond_for_each_slave_rcu() used to yield 1 iface.
>
> Well, bond_update_slave_arr() is not called for broadcast AFAICS.
Thank you for pointing out this issue. We don't need to revert the patch. can you test if the following patch is useful to you. I will add test cases later.
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index a8034a561011..c950e1e7f284 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2384,7 +2384,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
unblock_netpoll_tx();
}
- if (bond_mode_can_use_xmit_hash(bond))
+ if (bond_mode_can_use_xmit_hash(bond) ||
+ BOND_MODE(bond) == BOND_MODE_BROADCAST)
bond_update_slave_arr(bond, NULL);
if (!slave_dev->netdev_ops->ndo_bpf ||
@@ -2560,7 +2561,8 @@ static int __bond_release_one(struct net_device *bond_dev,
bond_upper_dev_unlink(bond, slave);
- if (bond_mode_can_use_xmit_hash(bond))
+ if (bond_mode_can_use_xmit_hash(bond) ||
+ BOND_MODE(bond) == BOND_MODE_BROADCAST)
bond_update_slave_arr(bond, slave);
slave_info(bond_dev, slave_dev, "Releasing %s interface\n",
>
>> + for (i = 0; i < slaves_count; i++) {
>> + struct slave *slave = slaves->arr[i];
>> struct sk_buff *skb2;
>> if (!(bond_slave_is_up(slave) && slave->link == BOND_LINK_UP))
>> @@ -5577,10 +5627,13 @@ 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, false);
>> + fallthrough;
>> case BOND_MODE_XOR:
>> return bond_3ad_xor_xmit(skb, dev);
>> case BOND_MODE_BROADCAST:
>> - return bond_xmit_broadcast(skb, dev);
>> + return bond_xmit_broadcast(skb, dev, true);
>> case BOND_MODE_ALB:
>> return bond_alb_xmit(skb, dev);
>> case BOND_MODE_TLB:
>> @@ -6456,6 +6509,7 @@ static int __init bond_check_params(struct bond_params *params)
>> eth_zero_addr(params->ad_actor_system);
>> params->ad_user_port_key = ad_user_port_key;
>> params->coupled_control = 1;
>> + params->broadcast_neighbor = 0;
>> if (packets_per_slave > 0) {
>> params->reciprocal_packets_per_slave =
>> reciprocal_value(packets_per_slave);
>
> --
> js
> suse labs
>
>
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [net-next v8 3/3] net: bonding: send peer notify when failure recovery
2025-06-27 13:49 ` [net-next v8 3/3] net: bonding: send peer notify when failure recovery Tonghao Zhang
@ 2025-10-15 3:36 ` Hangbin Liu
2025-10-15 13:29 ` Tonghao Zhang
0 siblings, 1 reply; 17+ messages in thread
From: Hangbin Liu @ 2025-10-15 3:36 UTC (permalink / raw)
To: Tonghao Zhang
Cc: netdev, Jay Vosburgh, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Andrew Lunn, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Nikolay Aleksandrov, Zengbing Tu, Antoine Tenart
On Fri, Jun 27, 2025 at 09:49:30PM +0800, Tonghao Zhang wrote:
> +static void ad_cond_set_peer_notif(struct port *port)
> +{
> + struct bonding *bond = port->slave->bond;
> +
> + if (bond->params.broadcast_neighbor && rtnl_trylock()) {
Hi Tonghao,
When do our internal review, Antoine pointed that this rtnl_trylock() may
fail and cause the notify not send. The other places of bonding using
workqueues to reschedule when the rtnl_trylock() failed. Do you think
if we should also do some similar thing to avoid notify failed?
Thanks
Hangbin
> + bond->send_peer_notif = bond->params.num_peer_notif *
> + max(1, bond->params.peer_notif_delay);
> + rtnl_unlock();
> + }
> +}
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [net-next v8 1/3] net: bonding: add broadcast_neighbor option for 802.3ad
2025-10-14 12:52 ` Tonghao Zhang
@ 2025-10-15 5:31 ` Jiri Slaby
0 siblings, 0 replies; 17+ messages in thread
From: Jiri Slaby @ 2025-10-15 5:31 UTC (permalink / raw)
To: Tonghao Zhang
Cc: netdev, Jay Vosburgh, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Andrew Lunn, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Nikolay Aleksandrov, Zengbing Tu
On 14. 10. 25, 14:52, Tonghao Zhang wrote:
>> On Oct 14, 2025, at 17:12, Jiri Slaby <jirislaby@kernel.org> wrote:
>> On 27. 06. 25, 15:49, Tonghao Zhang wrote:
>> this breaks broadcast bonding in 6.17. Reverting these three (the two depend on this one) makes 6.17 work again:
>> 2f9afffc399d net: bonding: send peer notify when failure recovery
>> 3d98ee52659c net: bonding: add broadcast_neighbor netlink option
>> ce7a381697cb net: bonding: add broadcast_neighbor option for 802.3ad
>>
>> This was reported downstream as an error in our openQA:
>> https://bugzilla.suse.com/show_bug.cgi?id=1250894
>>
>> I bisected using this in qemu:
>> systemctl stop network
>> ip link del bond0 || true
>> ip link set dev eth0 down
>> ip addr flush eth0
>> ip link add bond0 type bond mode broadcast
>> ip link set dev eth0 master bond0
>> ip addr add 10.0.2.15/24 dev bond0
>> ip link set bond0 up
>> sleep 1
>> exec nmap -sS 10.0.2.2/32
>>
>> Any ideas?
>> ...
>>> --- a/drivers/net/bonding/bond_main.c
>>> +++ b/drivers/net/bonding/bond_main.c
>> ...
>>> @@ -5329,17 +5369,27 @@ static netdev_tx_t bond_3ad_xor_xmit(struct sk_buff *skb,
>>> return bond_tx_drop(dev, skb);
>>> }
>>> -/* in broadcast mode, we send everything to all usable interfaces. */
>>> +/* in broadcast mode, we send everything to all or usable slave interfaces.
>>> + * under rcu_read_lock when this function is called.
>>> + */
>>> static netdev_tx_t bond_xmit_broadcast(struct sk_buff *skb,
>>> - struct net_device *bond_dev)
>>> + struct net_device *bond_dev,
>>> + bool all_slaves)
>>> {
>>> struct bonding *bond = netdev_priv(bond_dev);
>>> - struct slave *slave = NULL;
>>> - struct list_head *iter;
>>> + struct bond_up_slave *slaves;
>>> bool xmit_suc = false;
>>> bool skb_used = false;
>>> + int slaves_count, i;
>>> - bond_for_each_slave_rcu(bond, slave, iter) {
>>> + if (all_slaves)
>>> + slaves = rcu_dereference(bond->all_slaves);
>>> + else
>>> + slaves = rcu_dereference(bond->usable_slaves);
>>> +
>>> + slaves_count = slaves ? READ_ONCE(slaves->count) : 0;
>>
>> OK, slaves_count is now 0 (slaves and bond->all_slaves are NULL), but bond_for_each_slave_rcu() used to yield 1 iface.
>>
>> Well, bond_update_slave_arr() is not called for broadcast AFAICS.
> Thank you for pointing out this issue. We don't need to revert the patch. can you test if the following patch is useful to you. I will add test cases later.
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index a8034a561011..c950e1e7f284 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2384,7 +2384,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
> unblock_netpoll_tx();
> }
>
> - if (bond_mode_can_use_xmit_hash(bond))
> + if (bond_mode_can_use_xmit_hash(bond) ||
> + BOND_MODE(bond) == BOND_MODE_BROADCAST)
> bond_update_slave_arr(bond, NULL);
>
> if (!slave_dev->netdev_ops->ndo_bpf ||
> @@ -2560,7 +2561,8 @@ static int __bond_release_one(struct net_device *bond_dev,
>
> bond_upper_dev_unlink(bond, slave);
>
> - if (bond_mode_can_use_xmit_hash(bond))
> + if (bond_mode_can_use_xmit_hash(bond) ||
> + BOND_MODE(bond) == BOND_MODE_BROADCAST)
> bond_update_slave_arr(bond, slave);
This indeed works. What about the other two call locations of
bond_update_slave_arr()?
thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [net-next v8 3/3] net: bonding: send peer notify when failure recovery
2025-10-15 3:36 ` Hangbin Liu
@ 2025-10-15 13:29 ` Tonghao Zhang
0 siblings, 0 replies; 17+ messages in thread
From: Tonghao Zhang @ 2025-10-15 13:29 UTC (permalink / raw)
To: Hangbin Liu
Cc: netdev, Jay Vosburgh, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Andrew Lunn, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Nikolay Aleksandrov, Zengbing Tu, Antoine Tenart
> On Oct 15, 2025, at 11:36, Hangbin Liu <liuhangbin@gmail.com> wrote:
>
> On Fri, Jun 27, 2025 at 09:49:30PM +0800, Tonghao Zhang wrote:
>> +static void ad_cond_set_peer_notif(struct port *port)
>> +{
>> + struct bonding *bond = port->slave->bond;
>> +
>> + if (bond->params.broadcast_neighbor && rtnl_trylock()) {
>
> Hi Tonghao,
>
> When do our internal review, Antoine pointed that this rtnl_trylock() may
> fail and cause the notify not send. The other places of bonding using
> workqueues to reschedule when the rtnl_trylock() failed. Do you think
> if we should also do some similar thing to avoid notify failed?
This is indeed a good optimization. I will submit the patch later.
>
> Thanks
> Hangbin
>> + bond->send_peer_notif = bond->params.num_peer_notif *
>> + max(1, bond->params.peer_notif_delay);
>> + rtnl_unlock();
>> + }
>> +}
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2025-10-15 13:30 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-27 13:49 [net-next v8 0/3] add broadcast_neighbor for no-stacking networking arch Tonghao Zhang
2025-06-27 13:49 ` [net-next v8 1/3] net: bonding: add broadcast_neighbor option for 802.3ad Tonghao Zhang
2025-10-14 8:41 ` Jiri Slaby
2025-10-14 9:12 ` Jiri Slaby
2025-10-14 9:22 ` Jiri Slaby
2025-10-14 12:52 ` Tonghao Zhang
2025-10-15 5:31 ` Jiri Slaby
2025-06-27 13:49 ` [net-next v8 2/3] net: bonding: add broadcast_neighbor netlink option Tonghao Zhang
2025-09-23 2:07 ` Hangbin Liu
2025-09-23 8:32 ` Tonghao Zhang
2025-06-27 13:49 ` [net-next v8 3/3] net: bonding: send peer notify when failure recovery Tonghao Zhang
2025-10-15 3:36 ` Hangbin Liu
2025-10-15 13:29 ` Tonghao Zhang
2025-07-08 9:10 ` [net-next v8 0/3] add broadcast_neighbor for no-stacking networking arch patchwork-bot+netdevbpf
2025-09-24 2:23 ` Hangbin Liu
2025-09-28 4:11 ` Tonghao Zhang
2025-09-29 8:35 ` Hangbin Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).