Re: [PATCH net-next 08/12] net: bridge: vlan: quiesce readers before freeing port VLANs
flat view
From: Nikolay Aleksandrov <razor@blackwall.org>
Date: 2026-10-01 07:28:54
Also in:
bridge
On 30/09/2026 10:14, Nikolay Aleksandrov wrote:
quoted hunk ↗ jump to hunk
Later fdb entries will cache port-VLAN pointers so unpublish a VLAN, wait for a grace period (existing readers) and then purge or rewrite fdb references before releasing it. Cached destinations can continue forwarding until they are cleaned, that is acceptable so add a comment to document it. During port teardown unpublish the complete VLAN group first, clean the port fdbs and then release the VLANs. This lets all VLANs share one grace period. Reviewed-by: Ido Schimmel <idosch@nvidia.com> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org> --- net/bridge/br_fdb.c | 20 ++++++++++++++++---- net/bridge/br_if.c | 7 +++++-- net/bridge/br_private.h | 12 ++++++++++-- net/bridge/br_vlan.c | 21 +++++++++++++++------ 4 files changed, 46 insertions(+), 14 deletions(-)diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c index 7c68b540b358..307f9c12914e 100644 --- a/net/bridge/br_fdb.c +++ b/net/bridge/br_fdb.c@@ -883,7 +883,9 @@ void br_fdb_cleanup_by_dst(struct net_bridge *br, spin_lock_bh(&br->hash_lock); hlist_for_each_entry_safe(f, tmp, &br->fdb_list, fdb_node) { - if (br_fdb_dst_port(f) != p) + struct net_bridge_dst dst = br_fdb_dst_read(f); + + if (br_dst_port(dst) != p) continue; if (vlan && f->key.vlan_id == vlan->vid &&@@ -894,12 +896,22 @@ void br_fdb_cleanup_by_dst(struct net_bridge *br, continue; } - if (!do_all) + if (!do_all) { + if (vid && f->key.vlan_id != vid) + continue; + if (test_bit(BR_FDB_STATIC, &f->flags) || (test_bit(BR_FDB_ADDED_BY_EXT_LEARN, &f->flags) && - !test_bit(BR_FDB_OFFLOADED, &f->flags)) || - (vid && f->key.vlan_id != vid)) + !test_bit(BR_FDB_OFFLOADED, &f->flags))) { + /* The entry outlives the VLAN, so it must fall + * back to the raw port destination + */ + if (vlan && br_dst_vlan(dst) == vlan) + br_fdb_dst_write(f, + br_port_to_dst(p)); continue; + } + } if (test_bit(BR_FDB_LOCAL, &f->flags)) fdb_delete_local(br, p, f);diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c index d94558a5e3e9..2c05ebc1299d 100644 --- a/net/bridge/br_if.c +++ b/net/bridge/br_if.c@@ -333,8 +333,9 @@ static void update_headroom(struct net_bridge *br, int new_hr) */ static void del_nbp(struct net_bridge_port *p) { - struct net_bridge *br = p->br; + struct net_bridge_vlan_group *vg; struct net_device *dev = p->dev; + struct net_bridge *br = p->br; sysfs_remove_link(br->ifobj, p->dev->name);@@ -354,8 +355,10 @@ static void del_nbp(struct net_bridge_port *p) update_headroom(br, get_max_headroom(br)); netdev_reset_rx_headroom(dev); - nbp_vlan_flush(p); + vg = nbp_vlan_group(p); + nbp_vlan_group_unpublish(p); br_fdb_cleanup_by_dst(br, br_port_to_dst(p), 0, 1); + nbp_vlan_flush(p, vg); switchdev_deferred_process(); nbp_backup_clear(p);diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index a790368b69e9..951b6ac5f484 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h@@ -1735,7 +1735,9 @@ int __br_vlan_set_default_pvid(struct net_bridge *br, u16 pvid, int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags, bool *changed, struct netlink_ext_ack *extack); int nbp_vlan_delete(struct net_bridge_port *port, u16 vid); -void nbp_vlan_flush(struct net_bridge_port *port); +void nbp_vlan_group_unpublish(struct net_bridge_port *port); +void nbp_vlan_flush(struct net_bridge_port *port, + struct net_bridge_vlan_group *vg); int nbp_vlan_init(struct net_bridge_port *port, struct netlink_ext_ack *extack); int nbp_get_num_vlan_infos(struct net_bridge_port *p, u32 filter_mask); void br_vlan_get_stats(const struct net_bridge_vlan *v,@@ -1894,7 +1896,13 @@ static inline int nbp_vlan_delete(struct net_bridge_port *port, u16 vid) return -EOPNOTSUPP; } -static inline void nbp_vlan_flush(struct net_bridge_port *port) +static inline void nbp_vlan_group_unpublish(struct net_bridge_port *port) +{ +} + +static inline void +nbp_vlan_flush(struct net_bridge_port *port, + struct net_bridge_vlan_group *vg) { }diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c index 8a914e249767..0675f74cf1b9 100644 --- a/net/bridge/br_vlan.c +++ b/net/bridge/br_vlan.c@@ -1406,23 +1406,32 @@ int nbp_vlan_delete(struct net_bridge_port *port, u16 vid) v = br_vlan_find(vg, vid); if (!v) return -ENOENT; - br_fdb_cleanup_by_dst(port->br, br_vlan_to_dst(v), vid, 0); __vlan_unpublish(vg, v); + synchronize_net();
Sashiko says:
Does this synchronize_net() call introduce an RTNL stall and IPI storm when
processing a large VLAN range?
When a user issues an RTM_DELLINK command specifying a large range of VLANs
(e.g., vid 1-4094), br_process_vlan_info() iterates over the range sequentially:
net/bridge/br_netlink.c:br_process_vlan_info() {
...
for (v = (*vinfo_last)->vid; v <= vinfo_curr->vid; v++) {
bool curr_change = false;
tmp_vinfo.vid = v;
err = br_vlan_info(br, p, cmd, &tmp_vinfo, &curr_change,
extack);
...
}
This loop calls into br_vlan_info(), which then calls nbp_vlan_delete() for
each VLAN. Because the global RTNL lock is held during this process,
synchronize_net() translates to synchronize_rcu_expedited().
Calling this iteratively up to 4094 times blocks the RTNL lock for several
seconds and spams all CPUs with IPIs, causing a severe system-wide network
configuration Denial of Service. This is reachable by any user with
CAP_NET_ADMIN in the network namespace, which can often be obtained by
unprivileged local users via user namespaces.
Could these grace periods be batched or deferred when deleting multiple VLANs?
- Yes, they can be batched and I had them batched initially but when I ran my tests I didn't
notice any problems deleting a full VLAN range. It took 79ms on my host to delete 4k VLANs.
As I've stated in my cover letter - the complexity is not worth it, so I dropped that patch.
quoted hunk ↗ jump to hunk
+ /* Traffic may still use v through cached fdb dsts until they are + * cleaned below. This is acceptable during vlan deletion. Above we + * drain the readers that could republish the dst before cleaning it + */ + br_fdb_cleanup_by_dst(port->br, br_vlan_to_dst(v), vid, 0); __vlan_del(vg, v); return 0; } -void nbp_vlan_flush(struct net_bridge_port *port) +void nbp_vlan_group_unpublish(struct net_bridge_port *port) { - struct net_bridge_vlan_group *vg; - ASSERT_RTNL(); - vg = nbp_vlan_group(port); - __vlan_flush(port->br, port, vg); RCU_INIT_POINTER(port->vlgrp, NULL); synchronize_net(); +} + +void nbp_vlan_flush(struct net_bridge_port *port, + struct net_bridge_vlan_group *vg) +{ + ASSERT_RTNL(); + + __vlan_flush(port->br, port, vg); __vlan_group_free(vg); }