On 18/09/2026 18:29, Nikolay Aleksandrov wrote:
VLAN groups cache the pvid and its state separately requiring state
updates to keep both copies synchronized. Lockless readers can also
observe the pvid and state from different updates. Cache an rcu protected
pointer to the pvid vlan entry instead. This makes the vlan entry the
single source of truth and lets the ingress path reuse it without another
lookup. It also makes the vlan entry always available at the ingress path
for subsequent forwarding-path optimizations.
Signed-off-by: Nikolay Aleksandrov <[email protected]>
---
net/bridge/br_mst.c | 13 ++----
net/bridge/br_private.h | 25 +++++-------
net/bridge/br_vlan.c | 76 ++++++++++++++----------------------
net/bridge/br_vlan_options.c | 12 ++----
4 files changed, 46 insertions(+), 80 deletions(-)
for this patch Sashiko says:
Does this code introduce a memory corruption regression due to a missing
release barrier?
RCU_INIT_POINTER() publishes the pointer to the datapath without an
smp_store_release() barrier, unlike rcu_assign_pointer().
If a lockless reader observes the new vg->pvid pointer before the VLAN
fields are fully initialized (due to CPU out-of-order execution), it might
read an uninitialized v->stats pointer in __allowed_ingress() when an
untagged packet arrives.
Calling this_cpu_ptr() on an uninitialized v->stats pointer could resolve
to the base of the per-cpu memory region or an invalid address, causing
subsequent u64_stats_add() increments to silently corrupt per-cpu variables
or cause a kernel oops.
Could rcu_assign_pointer() be used instead to ensure prior initialization
is visible to readers?
Nik says: No, if that could happen we would be in trouble even today. These
fields are initialized before the VLAN is published, i.e. before
inserting it in the VLAN rhashtable and linking it to the VLAN
list, only after that the pvid is applied by __vlan_flags_commit()
Cheers,
Nik
