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

[Nikolay Aleksandrov <razor@blackwall.org>]

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(-)

diff --git a/net/bridge/br_mst.c b/net/bridge/br_mst.c
index 63fc1dd0e9bc..252d5c927f71 100644
--- a/net/bridge/br_mst.c
+++ b/net/bridge/br_mst.c
@@ -79,16 +79,11 @@ int br_mst_get_state(const struct net_device *dev, u16 msti, u8 *state)
 }
 EXPORT_SYMBOL_GPL(br_mst_get_state);
 
-static void br_mst_vlan_set_state(struct net_bridge_vlan_group *vg,
-				  struct net_bridge_vlan *v,
-				  u8 state)
+static void br_mst_vlan_set_state(struct net_bridge_vlan *v, u8 state)
 {
 	if (br_vlan_get_state(v) == state)
 		return;
 
-	if (v->vid == br_get_pvid(vg))
-		br_vlan_set_pvid_state(vg, state);
-
 	br_vlan_set_state(v, state);
 }
 
@@ -129,7 +124,7 @@ int br_mst_set_state(struct net_bridge_port *p, u16 msti, u8 state,
 		if (READ_ONCE(v->brvlan->msti) != msti)
 			continue;
 
-		br_mst_vlan_set_state(vg, v, state);
+		br_mst_vlan_set_state(v, state);
 	}
 
 out_rcu_unlock:
@@ -149,13 +144,13 @@ static void br_mst_vlan_sync_state(struct net_bridge_vlan *pv, u16 msti)
 		 * it.
 		 */
 		if (v != pv && v->brvlan->msti == msti) {
-			br_mst_vlan_set_state(vg, pv, br_vlan_get_state(v));
+			br_mst_vlan_set_state(pv, br_vlan_get_state(v));
 			return;
 		}
 	}
 
 	/* Otherwise, start out in a new MSTI with all ports disabled. */
-	return br_mst_vlan_set_state(vg, pv, BR_STATE_DISABLED);
+	return br_mst_vlan_set_state(pv, BR_STATE_DISABLED);
 }
 
 int br_mst_vlan_set_msti(struct net_bridge_vlan *mv, u16 msti)
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 09c397e30330..bda25f851342 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -256,8 +256,7 @@ struct net_bridge_vlan {
  * @tunnel_hash: Hash table to map from tunnel key ID (e.g. VXLAN VNI) to VLAN
  * @vlan_list: sorted VLAN entry list
  * @num_vlans: number of total VLAN entries
- * @pvid: PVID VLAN id
- * @pvid_state: PVID's STP state (e.g. forwarding, learning, blocking)
+ * @pvid: RCU-protected PVID VLAN entry
  *
  * IMPORTANT: Be careful when checking if there're VLAN entries using list
  *            primitives because the bridge can have entries in its list which
@@ -269,9 +268,8 @@ struct net_bridge_vlan_group {
 	struct rhashtable		vlan_hash;
 	struct rhashtable		tunnel_hash;
 	struct list_head		vlan_list;
+	struct net_bridge_vlan		__rcu *pvid;
 	u16				num_vlans;
-	u16				pvid;
-	u8				pvid_state;
 };
 
 /* bridge fdb flags */
@@ -1687,10 +1685,16 @@ static inline int br_vlan_get_tag(const struct sk_buff *skb, u16 *vid)
 
 static inline u16 br_get_pvid(const struct net_bridge_vlan_group *vg)
 {
+	struct net_bridge_vlan *pvid;
+
 	if (!vg)
 		return 0;
 
-	return READ_ONCE(vg->pvid);
+	pvid = rcu_dereference_rtnl(vg->pvid);
+	if (!pvid)
+		return 0;
+
+	return pvid->vid;
 }
 
 static inline u16 br_vlan_flags(const struct net_bridge_vlan *v, u16 pvid)
@@ -1922,17 +1926,6 @@ static inline void br_vlan_set_state(struct net_bridge_vlan *v, u8 state)
 	br_multicast_update_vlan_mcast_ctx(v, state);
 }
 
-static inline u8 br_vlan_get_pvid_state(const struct net_bridge_vlan_group *vg)
-{
-	return READ_ONCE(vg->pvid_state);
-}
-
-static inline void br_vlan_set_pvid_state(struct net_bridge_vlan_group *vg,
-					  u8 state)
-{
-	WRITE_ONCE(vg->pvid_state, state);
-}
-
 /* learn_allow is true at ingress and false at egress */
 static inline bool br_vlan_state_allowed(u8 state, bool learn_allow)
 {
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 34ede0f3e452..bfedd13e3fd4 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -35,21 +35,21 @@ static struct net_bridge_vlan *br_vlan_lookup(struct rhashtable *tbl, u16 vid)
 }
 
 static void __vlan_add_pvid(struct net_bridge_vlan_group *vg,
-			    const struct net_bridge_vlan *v)
+			    struct net_bridge_vlan *v)
 {
-	if (vg->pvid == v->vid)
+	if (rcu_access_pointer(vg->pvid) == v)
 		return;
 
-	br_vlan_set_pvid_state(vg, br_vlan_get_state(v));
-	WRITE_ONCE(vg->pvid, v->vid);
+	RCU_INIT_POINTER(vg->pvid, v);
 }
 
-static void __vlan_delete_pvid(struct net_bridge_vlan_group *vg, u16 vid)
+static void __vlan_delete_pvid(struct net_bridge_vlan_group *vg,
+			       struct net_bridge_vlan *v)
 {
-	if (vg->pvid != vid)
+	if (rcu_access_pointer(vg->pvid) != v)
 		return;
 
-	WRITE_ONCE(vg->pvid, 0);
+	RCU_INIT_POINTER(vg->pvid, NULL);
 }
 
 /* Update the BRIDGE_VLAN_INFO_PVID and BRIDGE_VLAN_INFO_UNTAGGED flags of @v.
@@ -60,6 +60,7 @@ static bool __vlan_flags_update(struct net_bridge_vlan *v, u16 flags,
 				bool commit)
 {
 	struct net_bridge_vlan_group *vg;
+	struct net_bridge_vlan *pvid;
 	u16 vlan_flags;
 	bool change;
 
@@ -70,7 +71,8 @@ static bool __vlan_flags_update(struct net_bridge_vlan *v, u16 flags,
 
 	/* check if anything would be changed on commit */
 	vlan_flags = v->flags;
-	change = !!(flags & BRIDGE_VLAN_INFO_PVID) == !!(vg->pvid != v->vid) ||
+	pvid = rtnl_dereference(vg->pvid);
+	change = !!(flags & BRIDGE_VLAN_INFO_PVID) == !!(pvid != v) ||
 		 ((flags ^ vlan_flags) & BRIDGE_VLAN_INFO_UNTAGGED);
 
 	if (!commit)
@@ -79,7 +81,7 @@ static bool __vlan_flags_update(struct net_bridge_vlan *v, u16 flags,
 	if (flags & BRIDGE_VLAN_INFO_PVID)
 		__vlan_add_pvid(vg, v);
 	else
-		__vlan_delete_pvid(vg, v->vid);
+		__vlan_delete_pvid(vg, v);
 
 	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
 		vlan_flags |= BRIDGE_VLAN_INFO_UNTAGGED;
@@ -403,7 +405,7 @@ static void __vlan_del(struct net_bridge_vlan *v)
 		masterv = v->brvlan;
 	}
 
-	__vlan_delete_pvid(vg, v->vid);
+	__vlan_delete_pvid(vg, v);
 	if (p) {
 		err = __vlan_vid_del(p->dev, p->br, v);
 		if (err)
@@ -453,7 +455,7 @@ static void __vlan_flush(const struct net_bridge *br,
 	struct net_bridge_vlan *vlan, *tmp;
 	u16 v_start = 0, v_end = 0;
 
-	__vlan_delete_pvid(vg, vg->pvid);
+	__vlan_delete_pvid(vg, rtnl_dereference(vg->pvid));
 	list_for_each_entry_safe(vlan, tmp, &vg->vlan_list, vlist) {
 		/* take care of disjoint ranges */
 		if (!v_start) {
@@ -579,42 +581,32 @@ static bool __allowed_ingress(const struct net_bridge *br,
 	}
 
 	if (!*vid) {
-		u16 pvid = br_get_pvid(vg);
-
+		v = vg ? rcu_dereference(vg->pvid) : NULL;
 		/* Frame had a tag with VID 0 or did not have a tag.
 		 * See if pvid is set on this port.  That tells us which
 		 * vlan untagged or priority-tagged traffic belongs to.
 		 */
-		if (!pvid)
+		if (!v)
 			goto drop;
 
 		/* PVID is set on this port.  Any untagged or priority-tagged
 		 * ingress frame is considered to belong to this vlan.
 		 */
-		*vid = pvid;
+		*vid = v->vid;
 		if (likely(!tagged))
 			/* Untagged Frame. */
-			__vlan_hwaccel_put_tag(skb, br->vlan_proto, pvid);
+			__vlan_hwaccel_put_tag(skb, br->vlan_proto, v->vid);
 		else
 			/* Priority-tagged Frame.
 			 * At this point, we know that skb->vlan_tci VID
 			 * field was 0.
 			 * We update only VID field and preserve PCP field.
 			 */
-			skb->vlan_tci |= pvid;
-
-		/* if snooping and stats are disabled we can avoid the lookup */
-		if (!br_opt_get(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED) &&
-		    !br_opt_get(br, BROPT_VLAN_STATS_ENABLED)) {
-			if (*state == BR_STATE_FORWARDING) {
-				*state = br_vlan_get_pvid_state(vg);
-				if (!br_vlan_state_allowed(*state, true))
-					goto drop;
-			}
-			return true;
-		}
+			skb->vlan_tci |= v->vid;
+	} else {
+		v = br_vlan_find(vg, *vid);
 	}
-	v = br_vlan_find(vg, *vid);
+
 	if (!v || !br_vlan_should_use(v))
 		goto drop;
 
@@ -697,11 +689,10 @@ bool br_should_learn(struct net_bridge_port *p, struct sk_buff *skb, u16 *vid)
 		*vid = 0;
 
 	if (!*vid) {
-		*vid = br_get_pvid(vg);
-		if (!*vid ||
-		    !br_vlan_state_allowed(br_vlan_get_pvid_state(vg), true))
+		v = rcu_dereference(vg->pvid);
+		if (!v || !br_vlan_state_allowed(br_vlan_get_state(v), true))
 			return false;
-
+		*vid = v->vid;
 		return true;
 	}
 
@@ -1061,17 +1052,10 @@ int br_vlan_set_stats_per_port(struct net_bridge *br, unsigned long val)
 
 static bool vlan_default_pvid(struct net_bridge_vlan_group *vg, u16 vid)
 {
-	struct net_bridge_vlan *v;
-
-	if (vid != vg->pvid)
-		return false;
+	struct net_bridge_vlan *pvid = rtnl_dereference(vg->pvid);
 
-	v = br_vlan_lookup(&vg->vlan_hash, vid);
-	if (v && br_vlan_should_use(v) &&
-	    (v->flags & BRIDGE_VLAN_INFO_UNTAGGED))
-		return true;
-
-	return false;
+	return pvid && pvid->vid == vid && br_vlan_should_use(pvid) &&
+	       (pvid->flags & BRIDGE_VLAN_INFO_UNTAGGED);
 }
 
 static void br_vlan_disable_default_pvid(struct net_bridge *br)
@@ -1524,7 +1508,7 @@ int br_vlan_get_info(const struct net_device *dev, u16 vid,
 
 	p_vinfo->vid = vid;
 	p_vinfo->flags = v->flags;
-	if (vid == br_get_pvid(vg))
+	if (v == rcu_access_pointer(vg->pvid))
 		p_vinfo->flags |= BRIDGE_VLAN_INFO_PVID;
 	return 0;
 }
@@ -1551,7 +1535,7 @@ int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid,
 
 	p_vinfo->vid = vid;
 	p_vinfo->flags = READ_ONCE(v->flags);
-	if (vid == br_get_pvid(vg))
+	if (v == rcu_access_pointer(vg->pvid))
 		p_vinfo->flags |= BRIDGE_VLAN_INFO_PVID;
 	return 0;
 }
@@ -1956,7 +1940,7 @@ void br_vlan_notify(const struct net_bridge *br,
 			goto out_kfree;
 
 		flags = v->flags;
-		if (br_get_pvid(vg) == v->vid)
+		if (v == rcu_access_pointer(vg->pvid))
 			flags |= BRIDGE_VLAN_INFO_PVID;
 		break;
 	case RTM_DELVLAN:
diff --git a/net/bridge/br_vlan_options.c b/net/bridge/br_vlan_options.c
index 506668fc31d9..e4b7a147e42b 100644
--- a/net/bridge/br_vlan_options.c
+++ b/net/bridge/br_vlan_options.c
@@ -110,8 +110,7 @@ size_t br_vlan_opts_nl_size(void)
 	       + 0;
 }
 
-static int br_vlan_modify_state(struct net_bridge_vlan_group *vg,
-				struct net_bridge_vlan *v,
+static int br_vlan_modify_state(struct net_bridge_vlan *v,
 				u8 state,
 				bool *changed,
 				struct netlink_ext_ack *extack)
@@ -143,9 +142,6 @@ static int br_vlan_modify_state(struct net_bridge_vlan_group *vg,
 	if (v->state == state)
 		return 0;
 
-	if (v->vid == br_get_pvid(vg))
-		br_vlan_set_pvid_state(vg, state);
-
 	br_vlan_set_state(v, state);
 	*changed = true;
 
@@ -216,7 +212,6 @@ static int br_vlan_modify_tunnel(const struct net_bridge_port *p,
 
 static int br_vlan_process_one_opts(const struct net_bridge *br,
 				    const struct net_bridge_port *p,
-				    struct net_bridge_vlan_group *vg,
 				    struct net_bridge_vlan *v,
 				    struct nlattr **tb,
 				    bool *changed,
@@ -228,7 +223,7 @@ static int br_vlan_process_one_opts(const struct net_bridge *br,
 	if (tb[BRIDGE_VLANDB_ENTRY_STATE]) {
 		u8 state = nla_get_u8(tb[BRIDGE_VLANDB_ENTRY_STATE]);
 
-		err = br_vlan_modify_state(vg, v, state, changed, extack);
+		err = br_vlan_modify_state(v, state, changed, extack);
 		if (err)
 			return err;
 	}
@@ -339,8 +334,7 @@ int br_vlan_process_options(const struct net_bridge *br,
 			break;
 		}
 
-		err = br_vlan_process_one_opts(br, p, vg, v, tb, &changed,
-					       extack);
+		err = br_vlan_process_one_opts(br, p, v, tb, &changed, extack);
 		if (err)
 			break;
 
-- 
2.47.3