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>,
Kuniyuki Iwashima <kuniyu@google.com>,
Andrew Lunn <andrew+netdev@lunn.ch>,
netdev@vger.kernel.org, eric.dumazet@gmail.com,
Eric Dumazet <edumazet@google.com>
Subject: [PATCH net-next 1/2] vlan: annotate data-races in vlan_dev_priv fields
Date: Wed, 16 Sep 2026 13:25:31 +0000 [thread overview]
Message-ID: <20260916132532.1785657-2-edumazet@google.com> (raw)
In-Reply-To: <20260916132532.1785657-1-edumazet@google.com>
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
next prev parent reply other threads:[~2026-09-16 13:25 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-16 13:25 [PATCH net-next 0/2] vlan: lockless vlan_fill_info() Eric Dumazet
2026-09-16 13:25 ` Eric Dumazet [this message]
2026-09-16 13:25 ` [PATCH net-next 2/2] vlan: no longer rely on RTNL in vlan_fill_info() Eric Dumazet
2026-09-19 0:10 ` [PATCH net-next 0/2] vlan: lockless vlan_fill_info() 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=20260916132532.1785657-2-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=kuba@kernel.org \
--cc=kuniyu@google.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox