[PATCH net-next 0/2] macvlan: RTNL-less macvlan_fill_info()

[PATCH net-next 0/2] macvlan: RTNL-less macvlan_fill_info()

[Eric Dumazet]

This series removes the RTNL lock dependency from macvlan_fill_info(),
allowing it to run under RCU read lock.

The first patch annotates data races on 'mode' and 'flags' fields which
are accessed locklessly in the RX/TX paths.

The second patch transitions macvlan_fill_info() to RCU, adding necessary
annotations for other fields and handling concurrent updates to the MAC
address list by computing the count dynamically.

Eric Dumazet (2):
  macvlan: annotate data-races around vlan->mode and vlan->flags
  macvlan: no longer rely on RTNL in macvlan_fill_info()

 drivers/net/macvlan.c | 109 ++++++++++++++++++++++++++----------------
 1 file changed, 69 insertions(+), 40 deletions(-)

-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH net-next 1/2] macvlan: annotate data-races around vlan->mode and vlan->flags

[Eric Dumazet]

Both fields can be changed in macvlan_changelink() while being read
locklessly.

Add READ_ONCE()/WRITE_ONCE() annotations.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/macvlan.c | 38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index c40fa331836bb2395267914807542ae5094e1a3c..8b69cc9b70f98d7991110f5eda76d58d5d96fa81 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -277,7 +277,7 @@ static void macvlan_broadcast(struct sk_buff *skb,
 		return;
 
 	hash_for_each_rcu(port->vlan_hash, i, vlan, hlist) {
-		if (vlan->dev == src || !(vlan->mode & mode))
+		if (vlan->dev == src || !(READ_ONCE(vlan->mode) & mode))
 			continue;
 
 		hash = mc_hash(vlan, eth->h_dest);
@@ -306,7 +306,7 @@ static void macvlan_multicast_rx(const struct macvlan_port *port,
 				  MACVLAN_MODE_VEPA    |
 				  MACVLAN_MODE_PASSTHRU|
 				  MACVLAN_MODE_BRIDGE);
-	else if (src->mode == MACVLAN_MODE_VEPA)
+	else if (READ_ONCE(src->mode) == MACVLAN_MODE_VEPA)
 		/* flood to everyone except source */
 		macvlan_broadcast(skb, port, src->dev,
 				  MACVLAN_MODE_VEPA |
@@ -447,7 +447,7 @@ static bool macvlan_forward_source(struct sk_buff *skb,
 			if (!vlan)
 				continue;
 
-			if (vlan->flags & MACVLAN_FLAG_NODST)
+			if (READ_ONCE(vlan->flags) & MACVLAN_FLAG_NODST)
 				consume = true;
 			macvlan_forward_source_one(skb, vlan);
 		}
@@ -487,14 +487,18 @@ static rx_handler_result_t macvlan_handle_frame(struct sk_buff **pskb)
 			return RX_HANDLER_CONSUMED;
 		}
 		src = macvlan_hash_lookup(port, eth->h_source);
-		if (src && src->mode != MACVLAN_MODE_VEPA &&
-		    src->mode != MACVLAN_MODE_BRIDGE) {
-			/* forward to original port. */
-			vlan = src;
-			ret = macvlan_broadcast_one(skb, vlan, eth, 0) ?:
-			      __netif_rx(skb);
-			handle_res = RX_HANDLER_CONSUMED;
-			goto out;
+		if (src) {
+			enum macvlan_mode mode = READ_ONCE(src->mode);
+
+			if (mode != MACVLAN_MODE_VEPA &&
+			    mode != MACVLAN_MODE_BRIDGE) {
+				/* forward to original port. */
+				vlan = src;
+				ret = macvlan_broadcast_one(skb, vlan, eth, 0) ?:
+				      __netif_rx(skb);
+				handle_res = RX_HANDLER_CONSUMED;
+				goto out;
+			}
 		}
 
 		hash = mc_hash(NULL, eth->h_dest);
@@ -515,7 +519,7 @@ static rx_handler_result_t macvlan_handle_frame(struct sk_buff **pskb)
 					      struct macvlan_dev, list);
 	else
 		vlan = macvlan_hash_lookup(port, eth->h_dest);
-	if (!vlan || vlan->mode == MACVLAN_MODE_SOURCE)
+	if (!vlan || READ_ONCE(vlan->mode) == MACVLAN_MODE_SOURCE)
 		return RX_HANDLER_PASS;
 
 	dev = vlan->dev;
@@ -548,7 +552,7 @@ static int macvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev)
 	const struct macvlan_port *port = vlan->port;
 	const struct macvlan_dev *dest;
 
-	if (vlan->mode == MACVLAN_MODE_BRIDGE) {
+	if (READ_ONCE(vlan->mode) == MACVLAN_MODE_BRIDGE) {
 		const struct ethhdr *eth = skb_eth_hdr(skb);
 
 		/* send to other bridge ports directly */
@@ -559,7 +563,7 @@ static int macvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev)
 		}
 
 		dest = macvlan_hash_lookup(port, eth->h_dest);
-		if (dest && dest->mode == MACVLAN_MODE_BRIDGE) {
+		if (dest && READ_ONCE(dest->mode) == MACVLAN_MODE_BRIDGE) {
 			/* send to lowerdev first for its network taps */
 			dev_forward_skb(vlan->lowerdev, skb);
 
@@ -777,7 +781,7 @@ static int macvlan_set_mac_address(struct net_device *dev, void *p)
 	if (ether_addr_equal(dev->dev_addr, addr->__data))
 		return 0;
 
-	if (vlan->mode == MACVLAN_MODE_PASSTHRU) {
+	if (READ_ONCE(vlan->mode) == MACVLAN_MODE_PASSTHRU) {
 		macvlan_set_addr_change(vlan->port);
 		return dev_set_mac_address(vlan->lowerdev, addr, NULL);
 	}
@@ -1645,7 +1649,7 @@ static int macvlan_changelink(struct net_device *dev,
 			if (err < 0)
 				return err;
 		}
-		vlan->flags = flags;
+		WRITE_ONCE(vlan->flags, flags);
 	}
 
 	if (data && data[IFLA_MACVLAN_BC_QUEUE_LEN]) {
@@ -1658,7 +1662,7 @@ static int macvlan_changelink(struct net_device *dev,
 			vlan, nla_get_s32(data[IFLA_MACVLAN_BC_CUTOFF]));
 
 	if (set_mode)
-		vlan->mode = mode;
+		WRITE_ONCE(vlan->mode, mode);
 	if (data && data[IFLA_MACVLAN_MACADDR_MODE]) {
 		if (vlan->mode != MACVLAN_MODE_SOURCE)
 			return -EINVAL;
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH net-next 2/2] macvlan: no longer rely on RTNL in macvlan_fill_info()

[Eric Dumazet]

Add READ_ONCE()/WRITE_ONCE() annotations on vlan->mode, vlan->flags,
vlan->bc_queue_len_req and port->bc_cutoff.

Fill IFLA_MACVLAN_MACADDR_DATA nested attribute and compute
on the fly the precise number of elements we put in it,
to fill an accurate IFLA_MACVLAN_MACADDR_COUNT attribute
as some user space applications could depend on its value
and the attributes order.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/macvlan.c | 71 +++++++++++++++++++++++++++++--------------
 1 file changed, 48 insertions(+), 23 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 8b69cc9b70f98d7991110f5eda76d58d5d96fa81..9a4bc99dbf53b5f2cd6345f0af899f56fdb3a46b 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -171,7 +171,7 @@ static int macvlan_hash_add_source(struct macvlan_dev *vlan,
 	RCU_INIT_POINTER(entry->vlan, vlan);
 	h = &port->vlan_source_hash[macvlan_eth_hash(addr)];
 	hlist_add_head_rcu(&entry->hlist, h);
-	vlan->macaddr_count++;
+	WRITE_ONCE(vlan->macaddr_count, vlan->macaddr_count + 1);
 
 	return 0;
 }
@@ -402,7 +402,7 @@ static void macvlan_flush_sources(struct macvlan_port *port,
 		if (rcu_access_pointer(entry->vlan) == vlan)
 			macvlan_hash_del_source(entry);
 
-	vlan->macaddr_count = 0;
+	WRITE_ONCE(vlan->macaddr_count, 0);
 }
 
 static void macvlan_forward_source_one(struct sk_buff *skb,
@@ -874,7 +874,7 @@ static void update_port_bc_cutoff(struct macvlan_dev *vlan, int cutoff)
 	if (vlan->port->bc_cutoff == cutoff)
 		return;
 
-	vlan->port->bc_cutoff = cutoff;
+	WRITE_ONCE(vlan->port->bc_cutoff, cutoff);
 	macvlan_recompute_bc_filter(vlan);
 }
 
@@ -1427,7 +1427,7 @@ static int macvlan_changelink_sources(struct macvlan_dev *vlan, u32 mode,
 		entry = macvlan_hash_lookup_source(vlan, addr);
 		if (entry) {
 			macvlan_hash_del_source(entry);
-			vlan->macaddr_count--;
+			WRITE_ONCE(vlan->macaddr_count, vlan->macaddr_count - 1);
 		}
 	} else if (mode == MACVLAN_MACADDR_FLUSH) {
 		macvlan_flush_sources(vlan->port, vlan);
@@ -1653,7 +1653,8 @@ static int macvlan_changelink(struct net_device *dev,
 	}
 
 	if (data && data[IFLA_MACVLAN_BC_QUEUE_LEN]) {
-		vlan->bc_queue_len_req = nla_get_u32(data[IFLA_MACVLAN_BC_QUEUE_LEN]);
+		WRITE_ONCE(vlan->bc_queue_len_req,
+			   nla_get_u32(data[IFLA_MACVLAN_BC_QUEUE_LEN]));
 		update_port_bc_queue_len(vlan->port);
 	}
 
@@ -1676,10 +1677,12 @@ static int macvlan_changelink(struct net_device *dev,
 
 static size_t macvlan_get_size_mac(const struct macvlan_dev *vlan)
 {
-	if (vlan->macaddr_count == 0)
+	unsigned int macaddr_count = READ_ONCE(vlan->macaddr_count);
+
+	if (!macaddr_count)
 		return 0;
 	return nla_total_size(0) /* IFLA_MACVLAN_MACADDR_DATA */
-		+ vlan->macaddr_count * nla_total_size(sizeof(u8) * ETH_ALEN);
+		+ macaddr_count * nla_total_size(sizeof(u8) * ETH_ALEN);
 }
 
 static size_t macvlan_get_size(const struct net_device *dev)
@@ -1702,53 +1705,75 @@ static int macvlan_fill_info_macaddr(struct sk_buff *skb,
 				     const int i)
 {
 	struct hlist_head *h = &vlan->port->vlan_source_hash[i];
-	struct macvlan_source_entry *entry;
+	const struct macvlan_source_entry *entry;
+	int cnt = 0;
 
-	hlist_for_each_entry_rcu(entry, h, hlist, lockdep_rtnl_is_held()) {
+	hlist_for_each_entry_rcu(entry, h, hlist) {
 		if (rcu_access_pointer(entry->vlan) != vlan)
 			continue;
 		if (nla_put(skb, IFLA_MACVLAN_MACADDR, ETH_ALEN, entry->addr))
-			return 1;
+			return -EMSGSIZE;
+		cnt++;
 	}
-	return 0;
+	return cnt;
 }
 
 static int macvlan_fill_info(struct sk_buff *skb,
 				const struct net_device *dev)
 {
-	struct macvlan_dev *vlan = netdev_priv(dev);
+	const struct macvlan_dev *vlan = netdev_priv(dev);
 	struct macvlan_port *port = vlan->port;
-	int i;
-	struct nlattr *nest;
+	unsigned int macaddr_count = 0;
+	struct nlattr *nest, *attr;
+	int bc_cutoff, cnt, i;
 
-	if (nla_put_u32(skb, IFLA_MACVLAN_MODE, vlan->mode))
+	rcu_read_lock();
+	if (nla_put_u32(skb, IFLA_MACVLAN_MODE, READ_ONCE(vlan->mode)))
 		goto nla_put_failure;
-	if (nla_put_u16(skb, IFLA_MACVLAN_FLAGS, vlan->flags))
+
+	if (nla_put_u16(skb, IFLA_MACVLAN_FLAGS, READ_ONCE(vlan->flags)))
 		goto nla_put_failure;
-	if (nla_put_u32(skb, IFLA_MACVLAN_MACADDR_COUNT, vlan->macaddr_count))
+
+	attr = nla_reserve(skb, IFLA_MACVLAN_MACADDR_COUNT, sizeof(u32));
+	if (!attr)
 		goto nla_put_failure;
-	if (vlan->macaddr_count > 0) {
+
+	if (READ_ONCE(vlan->macaddr_count) > 0) {
 		nest = nla_nest_start_noflag(skb, IFLA_MACVLAN_MACADDR_DATA);
 		if (nest == NULL)
 			goto nla_put_failure;
 
 		for (i = 0; i < MACVLAN_HASH_SIZE; i++) {
-			if (macvlan_fill_info_macaddr(skb, vlan, i))
+			cnt = macvlan_fill_info_macaddr(skb, vlan, i);
+			if (cnt < 0)
 				goto nla_put_failure;
+			macaddr_count += cnt;
 		}
-		nla_nest_end(skb, nest);
+		if (!macaddr_count)
+			nla_nest_cancel(skb, nest);
+		else if (nla_nest_end_safe(skb, nest) < 0)
+			goto nla_put_failure;
 	}
-	if (nla_put_u32(skb, IFLA_MACVLAN_BC_QUEUE_LEN, vlan->bc_queue_len_req))
+	*(u32 *)nla_data(attr) = macaddr_count;
+
+	if (nla_put_u32(skb, IFLA_MACVLAN_BC_QUEUE_LEN,
+			READ_ONCE(vlan->bc_queue_len_req)))
 		goto nla_put_failure;
+
 	if (nla_put_u32(skb, IFLA_MACVLAN_BC_QUEUE_LEN_USED,
 			READ_ONCE(port->bc_queue_len_used)))
 		goto nla_put_failure;
-	if (port->bc_cutoff != 1 &&
-	    nla_put_s32(skb, IFLA_MACVLAN_BC_CUTOFF, port->bc_cutoff))
+
+	bc_cutoff = READ_ONCE(port->bc_cutoff);
+	if (bc_cutoff != 1 &&
+	    nla_put_s32(skb, IFLA_MACVLAN_BC_CUTOFF, bc_cutoff))
 		goto nla_put_failure;
+
+	rcu_read_unlock();
 	return 0;
 
 nla_put_failure:
+	rcu_read_unlock();
 	return -EMSGSIZE;
 }
 
-- 
2.55.0.rc0.799.gd6f94ed593-goog

Re: [PATCH net-next 1/2] macvlan: annotate data-races around vlan->mode and vlan->flags

[Nikolay Aleksandrov]

On 01/07/2026 11:22, Eric Dumazet wrote:
> Both fields can be changed in macvlan_changelink() while being read
> locklessly.
> 
> Add READ_ONCE()/WRITE_ONCE() annotations.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>   drivers/net/macvlan.c | 38 +++++++++++++++++++++-----------------
>   1 file changed, 21 insertions(+), 17 deletions(-)
> 

Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>

Re: [PATCH net-next 2/2] macvlan: no longer rely on RTNL in macvlan_fill_info()

[Nikolay Aleksandrov]

On 01/07/2026 11:22, Eric Dumazet wrote:
> Add READ_ONCE()/WRITE_ONCE() annotations on vlan->mode, vlan->flags,
> vlan->bc_queue_len_req and port->bc_cutoff.
> 
> Fill IFLA_MACVLAN_MACADDR_DATA nested attribute and compute
> on the fly the precise number of elements we put in it,
> to fill an accurate IFLA_MACVLAN_MACADDR_COUNT attribute
> as some user space applications could depend on its value
> and the attributes order.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>   drivers/net/macvlan.c | 71 +++++++++++++++++++++++++++++--------------
>   1 file changed, 48 insertions(+), 23 deletions(-)
> 

The snippet that sets macaddr_count gave me pause. :)
Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>

[snip]
> +
> +	if (READ_ONCE(vlan->macaddr_count) > 0) {
>   		nest = nla_nest_start_noflag(skb, IFLA_MACVLAN_MACADDR_DATA);
>   		if (nest == NULL)
>   			goto nla_put_failure;
>   
>   		for (i = 0; i < MACVLAN_HASH_SIZE; i++) {
> -			if (macvlan_fill_info_macaddr(skb, vlan, i))
> +			cnt = macvlan_fill_info_macaddr(skb, vlan, i);
> +			if (cnt < 0)
>   				goto nla_put_failure;
> +			macaddr_count += cnt;
>   		}
> -		nla_nest_end(skb, nest);
> +		if (!macaddr_count)
> +			nla_nest_cancel(skb, nest);
> +		else if (nla_nest_end_safe(skb, nest) < 0)
> +			goto nla_put_failure;
>   	}
> -	if (nla_put_u32(skb, IFLA_MACVLAN_BC_QUEUE_LEN, vlan->bc_queue_len_req))
> +	*(u32 *)nla_data(attr) = macaddr_count;
> +