Re: [PATCH net-next 3/9] net: bridge: vlan: cache the pvid vlan entry directly

[Nikolay Aleksandrov <razor@blackwall.org>]

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 <razor@blackwall.org>
> ---
>   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