[PATCH net-next 0/2] vlan: lockless vlan_fill_info()

[PATCH net-next 0/2] vlan: lockless vlan_fill_info()

[Eric Dumazet]

vlan_fill_info() currently relies on RTNL being held, because it
uses rcu_dereference_rtnl() to walk vlan->egress_priority_map[].

While auditing the fields exposed there, it appears that
vlan->flags, vlan->ingress_priority_map[] and vlan->nr_*_mappings
are written under RTNL but already read from lockless contexts
(RX, TX, /proc/net/vlan/<dev> and vlan_get_size()).

- Patch 1 adds the missing READ_ONCE()/WRITE_ONCE() annotations.

- Patch 2 converts vlan_fill_info() to RCU.

vlan->egress_priority_map[] needed no work: it is already updated
with rcu_assign_pointer() and freed with kfree_rcu().

This is part of a larger effort to remove the RTNL dependency from
rtnl_link_ops->fill_info().

Eric Dumazet (2):
  vlan: annotate data-races in vlan_dev_priv fields
  vlan: no longer rely on RTNL in vlan_fill_info()

 net/8021q/vlan.h         |  2 +-
 net/8021q/vlan_core.c    |  2 +-
 net/8021q/vlan_dev.c     | 37 ++++++++++++++++------------
 net/8021q/vlan_netlink.c | 52 ++++++++++++++++++++++++----------------
 net/8021q/vlanproc.c     | 18 +++++++-------
 5 files changed, 65 insertions(+), 46 deletions(-)

-- 
2.55.0.1032.g73a4cd73de-goog

[PATCH net-next 1/2] vlan: annotate data-races in vlan_dev_priv fields

[Eric Dumazet]

vlan->flags, vlan->ingress_priority_map[] and vlan->nr_*_mappings
are written under RTNL, but read from lockless contexts:

- the RX path reads vlan->flags (VLAN_FLAG_REORDER_HDR) and
  the ingress priority map,
- the TX path reads vlan->flags,
- /proc/net/vlan/<dev> reads vlan->flags and the ingress priority
  map,
- vlan_get_size() reads vlan->nr_*_mappings,
- vlan_fill_info() will soon run without RTNL.

Add the missing READ_ONCE()/WRITE_ONCE() annotations.

vlan_dev_change_flags() is also made a bit more readable by using
a local new_flags variable, avoiding the repeated re-reads of
vlan->flags after it has been published.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/8021q/vlan.h         |  2 +-
 net/8021q/vlan_core.c    |  2 +-
 net/8021q/vlan_dev.c     | 37 ++++++++++++++++++++++---------------
 net/8021q/vlan_netlink.c |  4 ++--
 net/8021q/vlanproc.c     | 18 +++++++++---------
 5 files changed, 35 insertions(+), 28 deletions(-)

diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index d874ab323d320f7a21c658d31656350754df0c20..4930e6e8610913000f1e789c71cab0f94978e4f4 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -158,7 +158,7 @@ static inline u32 vlan_get_ingress_priority(struct net_device *dev,
 {
 	struct vlan_dev_priv *vip = vlan_dev_priv(dev);
 
-	return vip->ingress_priority_map[(vlan_tci >> VLAN_PRIO_SHIFT) & 0x7];
+	return READ_ONCE(vip->ingress_priority_map[(vlan_tci >> VLAN_PRIO_SHIFT) & 0x7]);
 }
 
 #ifdef CONFIG_VLAN_8021Q_GVRP
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index d23965e76c167b3124afb74ae447ed211a701cc7..1fc6ebd3313218255a404255619d330527a73f38 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -38,7 +38,7 @@ bool vlan_do_receive(struct sk_buff **skbp)
 			skb->pkt_type = PACKET_HOST;
 	}
 
-	if (!(vlan_dev_priv(vlan_dev)->flags & VLAN_FLAG_REORDER_HDR) &&
+	if (!(READ_ONCE(vlan_dev_priv(vlan_dev)->flags) & VLAN_FLAG_REORDER_HDR) &&
 	    !netif_is_macvlan_port(vlan_dev) &&
 	    !netif_is_bridge_port(vlan_dev)) {
 		unsigned int offset = skb->data - skb_mac_header(skb);
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 2859cbac3f266b7c4e3f44f41280d33ab69c5270..16e917e9c23036c1a62de2c28ac27c29f728bc8e 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -54,7 +54,7 @@ static int vlan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
 	u16 vlan_tci = 0;
 	int rc;
 
-	if (!(vlan->flags & VLAN_FLAG_REORDER_HDR)) {
+	if (!(READ_ONCE(vlan->flags) & VLAN_FLAG_REORDER_HDR)) {
 		vhdr = skb_push(skb, VLAN_HLEN);
 
 		vlan_tci = vlan->vlan_id;
@@ -110,7 +110,7 @@ static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
 	 * NOTE: THIS ASSUMES DIX ETHERNET, SPECIFICALLY NOT SUPPORTING
 	 * OTHER THINGS LIKE FDDI/TokenRing/802.3 SNAPs...
 	 */
-	if (vlan->flags & VLAN_FLAG_REORDER_HDR ||
+	if (READ_ONCE(vlan->flags) & VLAN_FLAG_REORDER_HDR ||
 	    veth->h_vlan_proto != vlan->vlan_proto) {
 		u16 vlan_tci;
 		vlan_tci = vlan->vlan_id;
@@ -159,13 +159,16 @@ void vlan_dev_set_ingress_priority(const struct net_device *dev,
 				   u32 skb_prio, u16 vlan_prio)
 {
 	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
+	u32 *map = &vlan->ingress_priority_map[vlan_prio & 0x7];
 
-	if (vlan->ingress_priority_map[vlan_prio & 0x7] && !skb_prio)
-		vlan->nr_ingress_mappings--;
-	else if (!vlan->ingress_priority_map[vlan_prio & 0x7] && skb_prio)
-		vlan->nr_ingress_mappings++;
+	if (*map && !skb_prio)
+		WRITE_ONCE(vlan->nr_ingress_mappings,
+			   vlan->nr_ingress_mappings - 1);
+	else if (!*map && skb_prio)
+		WRITE_ONCE(vlan->nr_ingress_mappings,
+			   vlan->nr_ingress_mappings + 1);
 
-	vlan->ingress_priority_map[vlan_prio & 0x7] = skb_prio;
+	WRITE_ONCE(*map, skb_prio);
 }
 
 int vlan_dev_set_egress_priority(const struct net_device *dev,
@@ -185,7 +188,8 @@ int vlan_dev_set_egress_priority(const struct net_device *dev,
 		if (mp->priority == skb_prio) {
 			if (!vlan_qos) {
 				rcu_assign_pointer(*mpp, rtnl_dereference(mp->next));
-				vlan->nr_egress_mappings--;
+				WRITE_ONCE(vlan->nr_egress_mappings,
+					   vlan->nr_egress_mappings - 1);
 				kfree_rcu(mp, rcu);
 			} else {
 				WRITE_ONCE(mp->vlan_qos, vlan_qos);
@@ -209,7 +213,8 @@ int vlan_dev_set_egress_priority(const struct net_device *dev,
 	RCU_INIT_POINTER(np->next, rtnl_dereference(vlan->egress_priority_map[bucket]));
 	rcu_assign_pointer(vlan->egress_priority_map[bucket], np);
 	if (vlan_qos)
-		vlan->nr_egress_mappings++;
+		WRITE_ONCE(vlan->nr_egress_mappings,
+			   vlan->nr_egress_mappings + 1);
 	return 0;
 }
 
@@ -220,23 +225,25 @@ int vlan_dev_change_flags(const struct net_device *dev, u32 flags, u32 mask)
 {
 	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
 	u32 old_flags = vlan->flags;
+	u32 new_flags;
 
 	if (mask & ~(VLAN_FLAG_REORDER_HDR | VLAN_FLAG_GVRP |
 		     VLAN_FLAG_LOOSE_BINDING | VLAN_FLAG_MVRP |
 		     VLAN_FLAG_BRIDGE_BINDING))
 		return -EINVAL;
 
-	vlan->flags = (old_flags & ~mask) | (flags & mask);
+	new_flags = (old_flags & ~mask) | (flags & mask);
+	WRITE_ONCE(vlan->flags, new_flags);
 
-	if (netif_running(dev) && (vlan->flags ^ old_flags) & VLAN_FLAG_GVRP) {
-		if (vlan->flags & VLAN_FLAG_GVRP)
+	if (netif_running(dev) && (new_flags ^ old_flags) & VLAN_FLAG_GVRP) {
+		if (new_flags & VLAN_FLAG_GVRP)
 			vlan_gvrp_request_join(dev);
 		else
 			vlan_gvrp_request_leave(dev);
 	}
 
-	if (netif_running(dev) && (vlan->flags ^ old_flags) & VLAN_FLAG_MVRP) {
-		if (vlan->flags & VLAN_FLAG_MVRP)
+	if (netif_running(dev) && (new_flags ^ old_flags) & VLAN_FLAG_MVRP) {
+		if (new_flags & VLAN_FLAG_MVRP)
 			vlan_mvrp_request_join(dev);
 		else
 			vlan_mvrp_request_leave(dev);
@@ -599,7 +606,7 @@ void vlan_dev_free_egress_priority(const struct net_device *dev)
 			pm = next;
 		}
 	}
-	vlan->nr_egress_mappings = 0;
+	WRITE_ONCE(vlan->nr_egress_mappings, 0);
 }
 
 static void vlan_dev_uninit(struct net_device *dev)
diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
index 368d53ca7d870998d4ce12bb9a48f946fedcb8ee..71cb98950511fe30f9b117834ce8b9107889f0cb 100644
--- a/net/8021q/vlan_netlink.c
+++ b/net/8021q/vlan_netlink.c
@@ -214,8 +214,8 @@ static size_t vlan_get_size(const struct net_device *dev)
 	return nla_total_size(2) +	/* IFLA_VLAN_PROTOCOL */
 	       nla_total_size(2) +	/* IFLA_VLAN_ID */
 	       nla_total_size(sizeof(struct ifla_vlan_flags)) + /* IFLA_VLAN_FLAGS */
-	       vlan_qos_map_size(vlan->nr_ingress_mappings) +
-	       vlan_qos_map_size(vlan->nr_egress_mappings);
+	       vlan_qos_map_size(READ_ONCE(vlan->nr_ingress_mappings)) +
+	       vlan_qos_map_size(READ_ONCE(vlan->nr_egress_mappings));
 }
 
 static int vlan_fill_info(struct sk_buff *skb, const struct net_device *dev)
diff --git a/net/8021q/vlanproc.c b/net/8021q/vlanproc.c
index 0e424e0895b7e860138c89743c2646bcb83750cd..5dd27438db9b39982fe978317587dfd341c17d92 100644
--- a/net/8021q/vlanproc.c
+++ b/net/8021q/vlanproc.c
@@ -240,7 +240,7 @@ static int vlandev_seq_show(struct seq_file *seq, void *offset)
 	seq_printf(seq,
 		   "%s  VID: %d	 REORDER_HDR: %i  dev->priv_flags: %x\n",
 		   vlandev->name, vlan->vlan_id,
-		   (int)(vlan->flags & 1), (u32)vlandev->priv_flags);
+		   (int)(READ_ONCE(vlan->flags) & 1), (u32)vlandev->priv_flags);
 
 	seq_printf(seq, fmt64, "total frames received", stats->rx_packets);
 	seq_printf(seq, fmt64, "total bytes received", stats->rx_bytes);
@@ -252,14 +252,14 @@ static int vlandev_seq_show(struct seq_file *seq, void *offset)
 	/* now show all PRIORITY mappings relating to this VLAN */
 	seq_printf(seq, "\nINGRESS priority mappings: "
 			"0:%u  1:%u  2:%u  3:%u  4:%u  5:%u  6:%u 7:%u\n",
-		   vlan->ingress_priority_map[0],
-		   vlan->ingress_priority_map[1],
-		   vlan->ingress_priority_map[2],
-		   vlan->ingress_priority_map[3],
-		   vlan->ingress_priority_map[4],
-		   vlan->ingress_priority_map[5],
-		   vlan->ingress_priority_map[6],
-		   vlan->ingress_priority_map[7]);
+		   READ_ONCE(vlan->ingress_priority_map[0]),
+		   READ_ONCE(vlan->ingress_priority_map[1]),
+		   READ_ONCE(vlan->ingress_priority_map[2]),
+		   READ_ONCE(vlan->ingress_priority_map[3]),
+		   READ_ONCE(vlan->ingress_priority_map[4]),
+		   READ_ONCE(vlan->ingress_priority_map[5]),
+		   READ_ONCE(vlan->ingress_priority_map[6]),
+		   READ_ONCE(vlan->ingress_priority_map[7]));
 
 	seq_printf(seq, " EGRESS priority mappings: ");
 	rcu_read_lock();
-- 
2.55.0.1032.g73a4cd73de-goog

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

[Eric Dumazet]

vlan_fill_info() used rcu_dereference_rtnl() to walk the egress
priority map, forcing "ip link show" dumps to hold RTNL.

The egress priority map is already fully RCU compliant:
entries are published with rcu_assign_pointer() and freed with
kfree_rcu() from vlan_dev_set_egress_priority() and
vlan_dev_free_egress_priority().

Wrap the QoS mapping serialization in rcu_read_lock() /
rcu_read_unlock() and use rcu_dereference().

Now that all the involved fields are annotated, use READ_ONCE()
for vlan->flags, vlan->nr_ingress_mappings, vlan->nr_egress_mappings
and vlan->ingress_priority_map[].

Also add missing const qualifiers in vlan_get_size() and
vlan_fill_info().

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/8021q/vlan_netlink.c | 48 +++++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
index 71cb98950511fe30f9b117834ce8b9107889f0cb..8c0c33cff4539f22d7fdca2bec68dc484c55209f 100644
--- a/net/8021q/vlan_netlink.c
+++ b/net/8021q/vlan_netlink.c
@@ -209,7 +209,7 @@ static inline size_t vlan_qos_map_size(unsigned int n)
 
 static size_t vlan_get_size(const struct net_device *dev)
 {
-	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
+	const struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
 
 	return nla_total_size(2) +	/* IFLA_VLAN_PROTOCOL */
 	       nla_total_size(2) +	/* IFLA_VLAN_ID */
@@ -220,60 +220,72 @@ static size_t vlan_get_size(const struct net_device *dev)
 
 static int vlan_fill_info(struct sk_buff *skb, const struct net_device *dev)
 {
-	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
-	struct vlan_priority_tci_mapping *pm;
-	struct ifla_vlan_flags f;
+	const struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
+	const struct vlan_priority_tci_mapping *pm;
 	struct ifla_vlan_qos_mapping m;
+	struct ifla_vlan_flags f;
 	struct nlattr *nest;
 	unsigned int i;
+	u32 flags;
 
 	if (nla_put_be16(skb, IFLA_VLAN_PROTOCOL, vlan->vlan_proto) ||
 	    nla_put_u16(skb, IFLA_VLAN_ID, vlan->vlan_id))
 		goto nla_put_failure;
-	if (vlan->flags) {
-		f.flags = vlan->flags;
+
+	flags = READ_ONCE(vlan->flags);
+	if (flags) {
+		f.flags = flags;
 		f.mask  = ~0;
 		if (nla_put(skb, IFLA_VLAN_FLAGS, sizeof(f), &f))
 			goto nla_put_failure;
 	}
-	if (vlan->nr_ingress_mappings) {
+
+	rcu_read_lock();
+
+	if (READ_ONCE(vlan->nr_ingress_mappings)) {
 		nest = nla_nest_start_noflag(skb, IFLA_VLAN_INGRESS_QOS);
-		if (nest == NULL)
-			goto nla_put_failure;
+		if (!nest)
+			goto nla_put_failure_unlock;
 
 		for (i = 0; i < ARRAY_SIZE(vlan->ingress_priority_map); i++) {
-			if (!vlan->ingress_priority_map[i])
+			u32 skb_prio = READ_ONCE(vlan->ingress_priority_map[i]);
+
+			if (!skb_prio)
 				continue;
 
 			m.from = i;
-			m.to   = vlan->ingress_priority_map[i];
+			m.to   = skb_prio;
 			if (nla_put(skb, IFLA_VLAN_QOS_MAPPING,
 				    sizeof(m), &m))
-				goto nla_put_failure;
+				goto nla_put_failure_unlock;
 		}
 		nla_nest_end(skb, nest);
 	}
 
-	if (vlan->nr_egress_mappings) {
+	if (READ_ONCE(vlan->nr_egress_mappings)) {
 		nest = nla_nest_start_noflag(skb, IFLA_VLAN_EGRESS_QOS);
-		if (nest == NULL)
-			goto nla_put_failure;
+		if (!nest)
+			goto nla_put_failure_unlock;
 
 		for (i = 0; i < ARRAY_SIZE(vlan->egress_priority_map); i++) {
-			for (pm = rcu_dereference_rtnl(vlan->egress_priority_map[i]); pm;
-			     pm = rcu_dereference_rtnl(pm->next)) {
+			for (pm = rcu_dereference(vlan->egress_priority_map[i]); pm;
+			     pm = rcu_dereference(pm->next)) {
 				u16 vlan_qos = READ_ONCE(pm->vlan_qos);
+
 				m.from = pm->priority;
 				m.to   = (vlan_qos >> 13) & 0x7;
 				if (nla_put(skb, IFLA_VLAN_QOS_MAPPING,
 					    sizeof(m), &m))
-					goto nla_put_failure;
+					goto nla_put_failure_unlock;
 			}
 		}
 		nla_nest_end(skb, nest);
 	}
+	rcu_read_unlock();
 	return 0;
 
+nla_put_failure_unlock:
+	rcu_read_unlock();
 nla_put_failure:
 	return -EMSGSIZE;
 }
-- 
2.55.0.1032.g73a4cd73de-goog

Re: [PATCH net-next 0/2] vlan: lockless vlan_fill_info()

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 16 Sep 2026 13:25:30 +0000 you wrote:
> vlan_fill_info() currently relies on RTNL being held, because it
> uses rcu_dereference_rtnl() to walk vlan->egress_priority_map[].
> 
> While auditing the fields exposed there, it appears that
> vlan->flags, vlan->ingress_priority_map[] and vlan->nr_*_mappings
> are written under RTNL but already read from lockless contexts
> (RX, TX, /proc/net/vlan/<dev> and vlan_get_size()).
> 
> [...]

Here is the summary with links:
  - [net-next,1/2] vlan: annotate data-races in vlan_dev_priv fields
    https://git.kernel.org/netdev/net-next/c/ca6ff8dd70eb
  - [net-next,2/2] vlan: no longer rely on RTNL in vlan_fill_info()
    https://git.kernel.org/netdev/net-next/c/ea5bd52c8bdd

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html