[PATCH net-next 08/12] net: bridge: vlan: quiesce readers before freeing port VLANs

[Nikolay Aleksandrov <razor@blackwall.org>]

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();
+	/* 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);
 }
 
-- 
2.47.3