From: Eric Dumazet <edumazet@google.com>
To: "David S . Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>,
netdev@vger.kernel.org, eric.dumazet@gmail.com,
Eric Dumazet <edumazet@google.com>,
Jay Vosburgh <jv@jvosburgh.net>,
Andrew Lunn <andrew+netdev@lunn.ch>
Subject: [PATCH v4 net-next] bonding: no longer rely on RTNL in bond_fill_info()
Date: Mon, 29 Jun 2026 17:32:00 +0000 [thread overview]
Message-ID: <20260629173200.469953-1-edumazet@google.com> (raw)
Add READ_ONCE()/WRITE_ONCE() annotations on port->is_enabled.
While this field is written under bond->mode_lock protection,
is is read without this lock being held.
Change bond_fill_info() to acquire RCU and use READ_ONCE()
to read bond->params fields that can be updated concurrently
from sysfs/procfs/rtnetlink.
Add const qualifiers to bond_uses_primary(), __agg_active_ports(),
bond_option_active_slave_get_rcu(), bond_3ad_get_active_agg_info(),
__bond_3ad_get_active_agg_info() helpers.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Jay Vosburgh <jv@jvosburgh.net>
Cc: Andrew Lunn <andrew+netdev@lunn.ch>
---
v4: addressed Sashiko/Jakub feedback
drivers/net/bonding/bond_3ad.c | 24 ++++---
drivers/net/bonding/bond_netlink.c | 109 ++++++++++++++++-------------
drivers/net/bonding/bond_options.c | 8 +--
include/net/bond_3ad.h | 4 +-
include/net/bonding.h | 8 +--
5 files changed, 85 insertions(+), 68 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index acbba08dbdfada00c118bd41b89a60e60f8521e7..b8e4b4d68dd65f1381ba7c0aad84609fcfbabbef 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -760,14 +760,14 @@ static int __agg_usable_ports(struct aggregator *agg)
return valid;
}
-static int __agg_active_ports(struct aggregator *agg)
+static int __agg_active_ports(const struct aggregator *agg)
{
- struct port *port;
+ const struct port *port;
int active = 0;
for (port = agg->lag_ports; port;
port = port->next_port_in_aggregator) {
- if (port->is_enabled)
+ if (READ_ONCE(port->is_enabled))
active++;
}
@@ -2801,11 +2801,11 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
* some of he adaptors(ce1000.lan) report.
*/
if (link == BOND_LINK_UP) {
- port->is_enabled = true;
+ WRITE_ONCE(port->is_enabled, true);
ad_update_actor_keys(port, false);
} else {
/* link has failed */
- port->is_enabled = false;
+ WRITE_ONCE(port->is_enabled, false);
ad_update_actor_keys(port, true);
}
agg = __get_first_agg(port);
@@ -2878,16 +2878,20 @@ int bond_3ad_set_carrier(struct bonding *bond)
* Returns: 0 on success
* < 0 on error
*/
-int __bond_3ad_get_active_agg_info(struct bonding *bond,
+int __bond_3ad_get_active_agg_info(const struct bonding *bond,
struct ad_info *ad_info)
{
- struct aggregator *aggregator = NULL, *tmp;
+ const struct aggregator *aggregator = NULL, *tmp;
+ struct ad_slave_info *ad_slave_info;
+ const struct port *port;
struct list_head *iter;
struct slave *slave;
- struct port *port;
bond_for_each_slave_rcu(bond, slave, iter) {
- port = &(SLAVE_AD_INFO(slave)->port);
+ ad_slave_info = SLAVE_AD_INFO(slave);
+ if (!ad_slave_info)
+ continue;
+ port = &ad_slave_info->port;
tmp = rcu_dereference(port->aggregator);
if (tmp && tmp->is_active) {
aggregator = tmp;
@@ -2907,7 +2911,7 @@ int __bond_3ad_get_active_agg_info(struct bonding *bond,
return 0;
}
-int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info)
+int bond_3ad_get_active_agg_info(const struct bonding *bond, struct ad_info *ad_info)
{
int ret;
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index 4a11572f663d3127fb2901468939556d34df16ed..55d2f8a539d46ad825768a0fb91ffe5648028d28 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -686,53 +686,58 @@ static size_t bond_get_size(const struct net_device *bond_dev)
0;
}
-static int bond_option_active_slave_get_ifindex(struct bonding *bond)
+static int bond_option_active_slave_get_ifindex_rcu(const struct bonding *bond)
{
- const struct net_device *slave;
- int ifindex;
+ const struct net_device *dev = NULL;
+ const struct slave *slave;
- rcu_read_lock();
- slave = bond_option_active_slave_get_rcu(bond);
- ifindex = slave ? slave->ifindex : 0;
- rcu_read_unlock();
- return ifindex;
+ slave = rcu_dereference(bond->curr_active_slave);
+ if (slave)
+ dev = slave->dev;
+ return dev ? dev->ifindex : 0;
}
static int bond_fill_info(struct sk_buff *skb,
const struct net_device *bond_dev)
{
- struct bonding *bond = netdev_priv(bond_dev);
- unsigned int packets_per_slave;
- int ifindex, i, targets_added;
+ const struct bonding *bond = netdev_priv(bond_dev);
+ int i, targets_added, miimon, mode;
+ const struct slave *primary;
struct nlattr *targets;
- struct slave *primary;
- if (nla_put_u8(skb, IFLA_BOND_MODE, BOND_MODE(bond)))
+ rcu_read_lock();
+ mode = READ_ONCE(bond->params.mode);
+ if (nla_put_u8(skb, IFLA_BOND_MODE, mode))
goto nla_put_failure;
- ifindex = bond_option_active_slave_get_ifindex(bond);
- if (ifindex && nla_put_u32(skb, IFLA_BOND_ACTIVE_SLAVE, ifindex))
- goto nla_put_failure;
+ if (bond_mode_uses_primary(mode)) {
+ int ifindex = bond_option_active_slave_get_ifindex_rcu(bond);
+
+ if (ifindex && nla_put_u32(skb, IFLA_BOND_ACTIVE_SLAVE, ifindex))
+ goto nla_put_failure;
+ }
- if (nla_put_u32(skb, IFLA_BOND_MIIMON, bond->params.miimon))
+ miimon = READ_ONCE(bond->params.miimon);
+ if (nla_put_u32(skb, IFLA_BOND_MIIMON, miimon))
goto nla_put_failure;
if (nla_put_u32(skb, IFLA_BOND_UPDELAY,
- bond->params.updelay * bond->params.miimon))
+ READ_ONCE(bond->params.updelay) * miimon))
goto nla_put_failure;
if (nla_put_u32(skb, IFLA_BOND_DOWNDELAY,
- bond->params.downdelay * bond->params.miimon))
+ READ_ONCE(bond->params.downdelay) * miimon))
goto nla_put_failure;
if (nla_put_u32(skb, IFLA_BOND_PEER_NOTIF_DELAY,
- bond->params.peer_notif_delay * bond->params.miimon))
+ READ_ONCE(bond->params.peer_notif_delay) * miimon))
goto nla_put_failure;
if (nla_put_u8(skb, IFLA_BOND_USE_CARRIER, 1))
goto nla_put_failure;
- if (nla_put_u32(skb, IFLA_BOND_ARP_INTERVAL, bond->params.arp_interval))
+ if (nla_put_u32(skb, IFLA_BOND_ARP_INTERVAL,
+ READ_ONCE(bond->params.arp_interval)))
goto nla_put_failure;
targets = nla_nest_start_noflag(skb, IFLA_BOND_ARP_IP_TARGET);
@@ -741,8 +746,10 @@ static int bond_fill_info(struct sk_buff *skb,
targets_added = 0;
for (i = 0; i < BOND_MAX_ARP_TARGETS; i++) {
- if (bond->params.arp_targets[i]) {
- if (nla_put_be32(skb, i, bond->params.arp_targets[i]))
+ __be32 t = READ_ONCE(bond->params.arp_targets[i]);
+
+ if (t) {
+ if (nla_put_be32(skb, i, t))
goto nla_put_failure;
targets_added = 1;
}
@@ -753,11 +760,12 @@ static int bond_fill_info(struct sk_buff *skb,
else
nla_nest_cancel(skb, targets);
- if (nla_put_u32(skb, IFLA_BOND_ARP_VALIDATE, bond->params.arp_validate))
+ if (nla_put_u32(skb, IFLA_BOND_ARP_VALIDATE,
+ READ_ONCE(bond->params.arp_validate)))
goto nla_put_failure;
if (nla_put_u32(skb, IFLA_BOND_ARP_ALL_TARGETS,
- bond->params.arp_all_targets))
+ READ_ONCE(bond->params.arp_all_targets)))
goto nla_put_failure;
#if IS_ENABLED(CONFIG_IPV6)
@@ -767,6 +775,9 @@ static int bond_fill_info(struct sk_buff *skb,
targets_added = 0;
for (i = 0; i < BOND_MAX_NS_TARGETS; i++) {
+ /* Note: IPv6 addresses can not be read in an atomic READ_ONCE() yet.
+ * We accept this minor race for the moment.
+ */
if (!ipv6_addr_any(&bond->params.ns_targets[i])) {
if (nla_put_in6_addr(skb, i, &bond->params.ns_targets[i]))
goto nla_put_failure;
@@ -780,97 +791,97 @@ static int bond_fill_info(struct sk_buff *skb,
nla_nest_cancel(skb, targets);
#endif
- primary = rtnl_dereference(bond->primary_slave);
+ primary = rcu_dereference(bond->primary_slave);
if (primary &&
nla_put_u32(skb, IFLA_BOND_PRIMARY, primary->dev->ifindex))
goto nla_put_failure;
if (nla_put_u8(skb, IFLA_BOND_PRIMARY_RESELECT,
- bond->params.primary_reselect))
+ READ_ONCE(bond->params.primary_reselect)))
goto nla_put_failure;
if (nla_put_u8(skb, IFLA_BOND_FAIL_OVER_MAC,
- bond->params.fail_over_mac))
+ READ_ONCE(bond->params.fail_over_mac)))
goto nla_put_failure;
if (nla_put_u8(skb, IFLA_BOND_XMIT_HASH_POLICY,
- bond->params.xmit_policy))
+ READ_ONCE(bond->params.xmit_policy)))
goto nla_put_failure;
if (nla_put_u32(skb, IFLA_BOND_RESEND_IGMP,
- bond->params.resend_igmp))
+ READ_ONCE(bond->params.resend_igmp)))
goto nla_put_failure;
if (nla_put_u8(skb, IFLA_BOND_NUM_PEER_NOTIF,
- bond->params.num_peer_notif))
+ READ_ONCE(bond->params.num_peer_notif)))
goto nla_put_failure;
if (nla_put_u8(skb, IFLA_BOND_ALL_SLAVES_ACTIVE,
- bond->params.all_slaves_active))
+ READ_ONCE(bond->params.all_slaves_active)))
goto nla_put_failure;
if (nla_put_u32(skb, IFLA_BOND_MIN_LINKS,
- bond->params.min_links))
+ READ_ONCE(bond->params.min_links)))
goto nla_put_failure;
if (nla_put_u32(skb, IFLA_BOND_LP_INTERVAL,
- bond->params.lp_interval))
+ READ_ONCE(bond->params.lp_interval)))
goto nla_put_failure;
- packets_per_slave = bond->params.packets_per_slave;
if (nla_put_u32(skb, IFLA_BOND_PACKETS_PER_SLAVE,
- packets_per_slave))
+ READ_ONCE(bond->params.packets_per_slave)))
goto nla_put_failure;
if (nla_put_u8(skb, IFLA_BOND_AD_LACP_ACTIVE,
- bond->params.lacp_active))
+ READ_ONCE(bond->params.lacp_active)))
goto nla_put_failure;
if (nla_put_u8(skb, IFLA_BOND_AD_LACP_RATE,
- bond->params.lacp_fast))
+ READ_ONCE(bond->params.lacp_fast)))
goto nla_put_failure;
if (nla_put_u8(skb, IFLA_BOND_AD_SELECT,
- bond->params.ad_select))
+ READ_ONCE(bond->params.ad_select)))
goto nla_put_failure;
if (nla_put_u8(skb, IFLA_BOND_TLB_DYNAMIC_LB,
- bond->params.tlb_dynamic_lb))
+ READ_ONCE(bond->params.tlb_dynamic_lb)))
goto nla_put_failure;
if (nla_put_u8(skb, IFLA_BOND_MISSED_MAX,
- bond->params.missed_max))
+ READ_ONCE(bond->params.missed_max)))
goto nla_put_failure;
if (nla_put_u8(skb, IFLA_BOND_COUPLED_CONTROL,
- bond->params.coupled_control))
+ READ_ONCE(bond->params.coupled_control)))
goto nla_put_failure;
if (nla_put_u8(skb, IFLA_BOND_BROADCAST_NEIGH,
- bond->params.broadcast_neighbor))
+ READ_ONCE(bond->params.broadcast_neighbor)))
goto nla_put_failure;
if (nla_put_u8(skb, IFLA_BOND_LACP_STRICT,
- bond->params.lacp_strict))
+ READ_ONCE(bond->params.lacp_strict)))
goto nla_put_failure;
- if (BOND_MODE(bond) == BOND_MODE_8023AD) {
+ if (mode == BOND_MODE_8023AD) {
struct ad_info info;
if (capable(CAP_NET_ADMIN)) {
if (nla_put_u16(skb, IFLA_BOND_AD_ACTOR_SYS_PRIO,
- bond->params.ad_actor_sys_prio))
+ READ_ONCE(bond->params.ad_actor_sys_prio)))
goto nla_put_failure;
if (nla_put_u16(skb, IFLA_BOND_AD_USER_PORT_KEY,
- bond->params.ad_user_port_key))
+ READ_ONCE(bond->params.ad_user_port_key)))
goto nla_put_failure;
+ /* Small race here, this is a minor trade off. */
if (nla_put(skb, IFLA_BOND_AD_ACTOR_SYSTEM,
ETH_ALEN, &bond->params.ad_actor_system))
goto nla_put_failure;
}
- if (!bond_3ad_get_active_agg_info(bond, &info)) {
+ if (!__bond_3ad_get_active_agg_info(bond, &info)) {
struct nlattr *nest;
nest = nla_nest_start_noflag(skb, IFLA_BOND_AD_INFO);
@@ -898,9 +909,11 @@ static int bond_fill_info(struct sk_buff *skb,
}
}
+ rcu_read_unlock();
return 0;
nla_put_failure:
+ rcu_read_unlock();
return -EMSGSIZE;
}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index e590c8dee86e1e7168e400b1321fc22c8f75b87c..36b8d89387ee5d67a51087fa2c6edd0de579ab3f 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -934,14 +934,14 @@ static int bond_option_mode_set(struct bonding *bond,
/* don't cache arp_validate between modes */
WRITE_ONCE(bond->params.arp_validate, BOND_ARP_VALIDATE_NONE);
- bond->params.mode = newval->value;
+ WRITE_ONCE(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;
+ WRITE_ONCE(bond->params.broadcast_neighbor, 0);
if (bond->dev->reg_state == NETREG_REGISTERED) {
bool update = false;
@@ -1706,7 +1706,7 @@ static int bond_option_lacp_strict_set(struct bonding *bond,
{
netdev_dbg(bond->dev, "Setting LACP fallback to %s (%llu)\n",
newval->string, newval->value);
- bond->params.lacp_strict = newval->value;
+ WRITE_ONCE(bond->params.lacp_strict, newval->value);
bond_3ad_set_carrier(bond);
return 0;
@@ -1927,7 +1927,7 @@ static int bond_option_broadcast_neigh_set(struct bonding *bond,
if (bond->params.broadcast_neighbor == newval->value)
return 0;
- bond->params.broadcast_neighbor = newval->value;
+ WRITE_ONCE(bond->params.broadcast_neighbor, newval->value);
if (bond->dev->flags & IFF_UP) {
if (bond->params.broadcast_neighbor)
static_branch_inc(&bond_bcast_neigh_enabled);
diff --git a/include/net/bond_3ad.h b/include/net/bond_3ad.h
index 05572c19e14b7ae97d497cc9c5d97d4314eab295..ef667dff297294dc78d98701aae10c31ef09df3f 100644
--- a/include/net/bond_3ad.h
+++ b/include/net/bond_3ad.h
@@ -302,8 +302,8 @@ void bond_3ad_state_machine_handler(struct work_struct *);
void bond_3ad_initiate_agg_selection(struct bonding *bond, int timeout);
void bond_3ad_adapter_speed_duplex_changed(struct slave *slave);
void bond_3ad_handle_link_change(struct slave *slave, char link);
-int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info);
-int __bond_3ad_get_active_agg_info(struct bonding *bond,
+int bond_3ad_get_active_agg_info(const struct bonding *bond, struct ad_info *ad_info);
+int __bond_3ad_get_active_agg_info(const struct bonding *bond,
struct ad_info *ad_info);
int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
struct slave *slave);
diff --git a/include/net/bonding.h b/include/net/bonding.h
index 2c54a36a8477b98dc7a4a1f45d27f7972efecba0..598d56b1bc97063365c88dd002ec1423850e56eb 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -345,14 +345,14 @@ static inline bool bond_mode_uses_primary(int mode)
mode == BOND_MODE_ALB;
}
-static inline bool bond_uses_primary(struct bonding *bond)
+static inline bool bond_uses_primary(const struct bonding *bond)
{
return bond_mode_uses_primary(BOND_MODE(bond));
}
-static inline struct net_device *bond_option_active_slave_get_rcu(struct bonding *bond)
+static inline struct net_device *bond_option_active_slave_get_rcu(const struct bonding *bond)
{
- struct slave *slave = rcu_dereference_rtnl(bond->curr_active_slave);
+ const struct slave *slave = rcu_dereference_rtnl(bond->curr_active_slave);
return bond_uses_primary(bond) && slave ? slave->dev : NULL;
}
@@ -703,7 +703,7 @@ void bond_setup(struct net_device *bond_dev);
unsigned int bond_get_num_tx_queues(void);
int bond_netlink_init(void);
void bond_netlink_fini(void);
-struct net_device *bond_option_active_slave_get_rcu(struct bonding *bond);
+struct net_device *bond_option_active_slave_get_rcu(const struct bonding *bond);
const char *bond_slave_link_status(s8 link);
struct bond_vlan_tag *bond_verify_device_path(struct net_device *start_dev,
struct net_device *end_dev,
--
2.55.0.rc0.799.gd6f94ed593-goog
next reply other threads:[~2026-06-29 17:32 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-29 17:32 Eric Dumazet [this message]
2026-06-30 11:14 ` [PATCH v4 net-next] bonding: no longer rely on RTNL in bond_fill_info() Nikolay Aleksandrov
2026-07-01 0:20 ` patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260629173200.469953-1-edumazet@google.com \
--to=edumazet@google.com \
--cc=andrew+netdev@lunn.ch \
--cc=davem@davemloft.net \
--cc=eric.dumazet@gmail.com \
--cc=horms@kernel.org \
--cc=jv@jvosburgh.net \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.