[PATCH net-next 0/9] net: bridge: vlan: broadcast fwding path optimizations

[PATCH net-next 0/9] net: bridge: vlan: broadcast fwding path optimizations

[Nikolay Aleksandrov]

Hi,
This patch-set is the first step to removing unnecessary VLAN hash lookups
from the bridge fast-path and also provides additional broadcast path
optimization by considering only the port-VLANs that actually participate
in the VLAN instead of all bridge ports. Combined these changes bring
considerable improvements[1] and open the path to do the same for the most
common fdb forwarding path. As expected the pathological cases have the
largest gain, e.g. if we have 1 client / VM facing port and 1 upstream port
only participating in a VLAN with many other bridge ports, we consider only
those 2 when broadcasting in that VLAN.

For up to 8 ports we only use the port-VLAN list as the array doesn't make
much of a difference and is unnecessary. Note that the array is rebuilt on
every member add or delete when >8 members, I tested that and could do
2000 VLAN add/delete with 64 port-VLAN members / sec on my test VM.
If it ever becomes a problem we can optimize it further, for the time
being I prefer it is simpler.

Patches 01-02: factor out flooding code into helpers that will be used
               later (no functional changes intended).
Patch 03: Cache the pvid VLAN entry directly so VLAN ingress can return
          it without a lookup while using the entry as the single source
          of VLAN state.
Patch 04: Introduces a VLAN's port-VLAN rcu list, used to build rcu array
          and as a fallback when flooding without array.
Patch 05: Consider only port-VLAN members when broadcasting, the first
          major optimization, especially in bridges with more ports.
Patch 06: Build RCU array of current port-VLAN members to be used for
          flooding when ports are > BR_VLAN_PORT_ARRAY_THRESHOLD (8).
Patch 07: Introduce a bridge forwarding dst structure used to pass around
          port and VLAN (later could be used for more state) enabling the
          next patch to remove VLAN lookups.
Patch 08: Use the port-VLAN carried in the forwarding destination to remove
          two redundant VLAN hash lookups in the fast-path when
          broadcasting. Same optimization will be made for the common fdb
          forwarding path in a following patch-set.
Patch 09: Avoid unnecessary VLAN hash lookups in the flood neigh
          suppression path.

Thanks,
 Nik

[1] Test measurements, also present in patch 08's commit message

  The series were tested in a two CPU VM with packet generation and bridge
  forwarding on separate pinned CPUs. The results are medians of seven runs
  with 300000 64b tagged broadcast packets.

                   Mpps                   br_flood TSC cycles/input
  VIDs Ports  before    after    gain   before  after  reduction
     1  8/2   1.209298  1.636580   35.3%     1444    845      41.5%
     1  8/4   0.843175  0.986541   17.0%     3036   2527      16.8%
     1  8/8   0.508857  0.559844   10.0%     5999   5465       8.9%
     1 32/2   0.893896  1.586532   77.5%     3013    841      72.1%
     1 32/16  0.250567  0.282339   12.7%    13670  11137      18.5%
     1 32/32  0.135351  0.148461    9.7%    25429  22320      12.2%
     1 64/2   0.555010  1.603776  189.0%     5001    837      83.3%
     1 64/32  0.123831  0.149639   20.8%    27674  22602      18.3%
     1 64/64  0.069122  0.076047   10.0%    51508  45138      12.4%
    64  8/2   1.044185  1.500468   43.7%     1528    851      44.3%
    64  8/4   0.802680  0.977700   21.8%     3178   2546      19.9%
    64  8/8   0.479748  0.555419   15.8%     6283   5635      10.3%
    64 32/2   0.861074  1.561266   81.3%     3063    849      72.3%
    64 32/16  0.234011  0.280027   19.7%    14332  11165      22.1%
    64 32/32  0.126145  0.148085   17.4%    27225  22613      16.9%
    64 64/2   0.530587  1.496179  182.0%     4377    826      81.1%
    64 64/32  0.121105  0.147943   22.2%    29286  22541      23.0%
    64 64/64  0.063626  0.077051   21.1%    56147  45520      18.9%
  1024  8/2   1.008693  1.358408   34.7%     1538    879      42.8%
  1024  8/4   0.785636  0.943545   20.1%     3280   2704      17.6%
  1024  8/8   0.448194  0.521765   16.4%     6678   6011      10.0%
  1024 32/2   0.840864  1.417311   68.6%     2756    859      68.8%
  1024 32/16  0.195786  0.247129   26.2%    16269  12639      22.3%
  1024 32/32  0.097978  0.122742   25.3%    35249  29273      17.0%
  1024 64/2   0.536947  1.400985  160.9%     4421    849      80.8%
  1024 64/32  0.093962  0.123383   31.3%    37425  29197      22.0%
  1024 64/64  0.045147  0.057867   28.2%    72626  59198      18.5%


Nikolay Aleksandrov (9):
  net: bridge: factor out common flood completion handling
  net: bridge: factor out port flooding
  net: bridge: vlan: cache the pvid vlan entry directly
  net: bridge: vlan: introduce a list of port-VLANs in the master VLAN
  net: bridge: consider only port-VLAN members when flooding
  net: bridge: vlan: use an RCU array for large flood sets
  net: bridge: introduce a forwarding destination structure
  net: bridge: avoid egress VLAN lookups when flooding
  net: bridge: avoid VLAN lookups for flood neighbour suppression

 net/bridge/br_arp_nd_proxy.c |  51 ++++---
 net/bridge/br_device.c       |   8 +-
 net/bridge/br_forward.c      | 272 ++++++++++++++++++++++-------------
 net/bridge/br_input.c        |   4 +-
 net/bridge/br_mst.c          |  13 +-
 net/bridge/br_private.h      |  59 +++++---
 net/bridge/br_vlan.c         | 155 ++++++++++++--------
 net/bridge/br_vlan_options.c |  12 +-
 8 files changed, 347 insertions(+), 227 deletions(-)

-- 
2.47.3

[PATCH net-next 1/9] net: bridge: factor out common flood completion handling

[Nikolay Aleksandrov]

Factor out the common flood completion handling shared by br_flood and
br_multicast_flood into a new helper - br_flood_finish. No functional
changes intended.

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 net/bridge/br_forward.c | 65 ++++++++++++++++++-----------------------
 1 file changed, 29 insertions(+), 36 deletions(-)

diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 46c762ca5177..31d54125b441 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -197,12 +197,33 @@ static struct net_bridge_port *maybe_deliver(
 	return p;
 }
 
+static void br_flood_finish(struct net_bridge_port *prev, struct sk_buff *skb,
+			    bool local_rcv, bool local_orig)
+{
+	enum skb_drop_reason reason = SKB_DROP_REASON_NO_TX_TARGET;
+
+	if (IS_ERR_OR_NULL(prev)) {
+		if (IS_ERR(prev)) {
+			reason = PTR_ERR(prev) == -ENOMEM ? SKB_DROP_REASON_NOMEM :
+				 SKB_DROP_REASON_NOT_SPECIFIED;
+		}
+
+		if (!local_rcv)
+			kfree_skb_reason(skb, reason);
+		return;
+	}
+
+	if (local_rcv)
+		deliver_clone(prev, skb, local_orig);
+	else
+		__br_forward(prev, skb, local_orig);
+}
+
 /* called under rcu_read_lock */
 void br_flood(struct net_bridge *br, struct sk_buff *skb,
 	      enum br_pkt_type pkt_type, bool local_rcv, bool local_orig,
 	      u16 vid)
 {
-	enum skb_drop_reason reason = SKB_DROP_REASON_NO_TX_TARGET;
 	struct net_bridge_port *prev = NULL;
 	struct net_bridge_port *p;
 
@@ -243,25 +264,11 @@ void br_flood(struct net_bridge *br, struct sk_buff *skb,
 		}
 
 		prev = maybe_deliver(prev, p, skb, local_orig);
-		if (IS_ERR(prev)) {
-			reason = PTR_ERR(prev) == -ENOMEM ? SKB_DROP_REASON_NOMEM :
-				 SKB_DROP_REASON_NOT_SPECIFIED;
-			goto out;
-		}
+		if (IS_ERR(prev))
+			break;
 	}
 
-	if (!prev)
-		goto out;
-
-	if (local_rcv)
-		deliver_clone(prev, skb, local_orig);
-	else
-		__br_forward(prev, skb, local_orig);
-	return;
-
-out:
-	if (!local_rcv)
-		kfree_skb_reason(skb, reason);
+	br_flood_finish(prev, skb, local_rcv, local_orig);
 }
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
@@ -301,7 +308,6 @@ void br_multicast_flood(struct net_bridge_mdb_entry *mdst,
 			struct net_bridge_mcast *brmctx,
 			bool local_rcv, bool local_orig)
 {
-	enum skb_drop_reason reason = SKB_DROP_REASON_NO_TX_TARGET;
 	struct net_bridge_port *prev = NULL;
 	struct net_bridge_port_group *p;
 	bool allow_mode_include = true;
@@ -343,11 +349,9 @@ void br_multicast_flood(struct net_bridge_mdb_entry *mdst,
 		}
 
 		prev = maybe_deliver(prev, port, skb, local_orig);
-		if (IS_ERR(prev)) {
-			reason = PTR_ERR(prev) == -ENOMEM ? SKB_DROP_REASON_NOMEM :
-				 SKB_DROP_REASON_NOT_SPECIFIED;
-			goto out;
-		}
+		if (IS_ERR(prev))
+			break;
+
 delivered:
 		if ((unsigned long)lport >= (unsigned long)port)
 			p = rcu_dereference(p->next);
@@ -355,17 +359,6 @@ void br_multicast_flood(struct net_bridge_mdb_entry *mdst,
 			rp = rcu_dereference(hlist_next_rcu(rp));
 	}
 
-	if (!prev)
-		goto out;
-
-	if (local_rcv)
-		deliver_clone(prev, skb, local_orig);
-	else
-		__br_forward(prev, skb, local_orig);
-	return;
-
-out:
-	if (!local_rcv)
-		kfree_skb_reason(skb, reason);
+	br_flood_finish(prev, skb, local_rcv, local_orig);
 }
 #endif
-- 
2.47.3

[PATCH net-next 2/9] net: bridge: factor out port flooding

[Nikolay Aleksandrov]

Factor out port flooding code into its own helper, it will be used by later
changes to flood to specific port VLANs. No functional changes intended.

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 net/bridge/br_forward.c | 77 +++++++++++++++++++++++------------------
 1 file changed, 43 insertions(+), 34 deletions(-)

diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 31d54125b441..a696c6c128e3 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -219,6 +219,48 @@ static void br_flood_finish(struct net_bridge_port *prev, struct sk_buff *skb,
 		__br_forward(prev, skb, local_orig);
 }
 
+static void br_flood_port(struct net_bridge_port **prev,
+			  struct net_bridge_port *p, struct sk_buff *skb,
+			  enum br_pkt_type pkt_type, bool local_orig, u16 vid)
+{
+	/* Do not flood unicast traffic to ports that turn it off, nor
+	 * other traffic if flood off, except for traffic we originate
+	 */
+	switch (pkt_type) {
+	case BR_PKT_UNICAST:
+		if (!test_bit(BR_FLOOD_BIT, &p->flags))
+			return;
+		break;
+	case BR_PKT_MULTICAST:
+		if (!test_bit(BR_MCAST_FLOOD_BIT, &p->flags) &&
+		    skb->dev != p->br->dev)
+			return;
+		break;
+	case BR_PKT_BROADCAST:
+		if (!test_bit(BR_BCAST_FLOOD_BIT, &p->flags) &&
+		    skb->dev != p->br->dev)
+			return;
+		break;
+	}
+
+	/* Do not flood to ports that enable proxy ARP */
+	if (test_bit(BR_PROXYARP_BIT, &p->flags))
+		return;
+	if (BR_INPUT_SKB_CB(skb)->proxyarp_replied) {
+		if (test_bit(BR_PROXYARP_WIFI_BIT, &p->flags))
+			return;
+		/* For gratuitous ARPs/NAs, check neigh_forward_grat.
+		 * For regular ARPs/NDs, check only neigh_suppress.
+		 */
+		if (br_is_neigh_suppress_enabled(p, vid) &&
+		    (!BR_INPUT_SKB_CB(skb)->grat_arp ||
+		     !br_is_neigh_forward_grat_enabled(p, vid)))
+			return;
+	}
+
+	*prev = maybe_deliver(*prev, p, skb, local_orig);
+}
+
 /* called under rcu_read_lock */
 void br_flood(struct net_bridge *br, struct sk_buff *skb,
 	      enum br_pkt_type pkt_type, bool local_rcv, bool local_orig,
@@ -230,40 +272,7 @@ void br_flood(struct net_bridge *br, struct sk_buff *skb,
 	br_tc_skb_miss_set(skb, pkt_type != BR_PKT_BROADCAST);
 
 	list_for_each_entry_rcu(p, &br->port_list, list) {
-		/* Do not flood unicast traffic to ports that turn it off, nor
-		 * other traffic if flood off, except for traffic we originate
-		 */
-		switch (pkt_type) {
-		case BR_PKT_UNICAST:
-			if (!test_bit(BR_FLOOD_BIT, &p->flags))
-				continue;
-			break;
-		case BR_PKT_MULTICAST:
-			if (!test_bit(BR_MCAST_FLOOD_BIT, &p->flags) && skb->dev != br->dev)
-				continue;
-			break;
-		case BR_PKT_BROADCAST:
-			if (!test_bit(BR_BCAST_FLOOD_BIT, &p->flags) && skb->dev != br->dev)
-				continue;
-			break;
-		}
-
-		/* Do not flood to ports that enable proxy ARP */
-		if (test_bit(BR_PROXYARP_BIT, &p->flags))
-			continue;
-		if (BR_INPUT_SKB_CB(skb)->proxyarp_replied) {
-			if (test_bit(BR_PROXYARP_WIFI_BIT, &p->flags))
-				continue;
-			/* For gratuitous ARPs/NAs, check neigh_forward_grat.
-			 * For regular ARPs/NDs, check only neigh_suppress.
-			 */
-			if (br_is_neigh_suppress_enabled(p, vid) &&
-			    (!BR_INPUT_SKB_CB(skb)->grat_arp ||
-			     !br_is_neigh_forward_grat_enabled(p, vid)))
-				continue;
-		}
-
-		prev = maybe_deliver(prev, p, skb, local_orig);
+		br_flood_port(&prev, p, skb, pkt_type, local_orig, vid);
 		if (IS_ERR(prev))
 			break;
 	}
-- 
2.47.3

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

[Nikolay Aleksandrov]

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

[PATCH net-next 4/9] net: bridge: vlan: introduce a list of port-VLANs in the master VLAN

[Nikolay Aleksandrov]

Keep a list of all port-VLANs that have been created within their
master VLAN. The list is rcu-safe and will be used for optimizing
the flood path.

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 net/bridge/br_private.h |  2 ++
 net/bridge/br_vlan.c    | 11 +++++++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index bda25f851342..b2a12b6298cb 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -210,6 +210,7 @@ enum {
  * @port_mcast_ctx: if MASTER flag unset, this is the per-port/vlan multicast
  *                  context
  * @msti: if MASTER flag set, this holds the VLANs MST instance
+ * @port_vlist: if MASTER flag set, this is the port-VLAN list
  * @vlist: sorted list of VLAN entries
  * @rcu: used for entry destruction
  *
@@ -244,6 +245,7 @@ struct net_bridge_vlan {
 
 	u16				msti;
 
+	struct list_head		port_vlist;
 	struct list_head		vlist;
 
 	struct rcu_head			rcu;
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index bfedd13e3fd4..34d1df59d190 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -130,10 +130,12 @@ static void __vlan_add_list(struct net_bridge_vlan *v)
 	struct list_head *headp, *hpos;
 	struct net_bridge_vlan *vent;
 
-	if (br_vlan_is_master(v))
+	if (br_vlan_is_master(v)) {
 		vg = br_vlan_group(v->br);
-	else
+	} else {
 		vg = nbp_vlan_group(v->port);
+		list_add_rcu(&v->port_vlist, &v->brvlan->port_vlist);
+	}
 
 	headp = &vg->vlan_list;
 	list_for_each_prev(hpos, headp) {
@@ -147,6 +149,10 @@ static void __vlan_add_list(struct net_bridge_vlan *v)
 static void __vlan_del_list(struct net_bridge_vlan *v)
 {
 	list_del_rcu(&v->vlist);
+	if (br_vlan_is_master(v))
+		WARN_ON(!list_empty(&v->port_vlist));
+	else
+		list_del_rcu(&v->port_vlist);
 }
 
 static int __vlan_vid_del(struct net_device *dev, struct net_bridge *br,
@@ -335,6 +341,7 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags,
 		}
 		br_multicast_ctx_init(br, v, &v->br_mcast_ctx);
 		v->priv_flags |= BR_VLFLAG_GLOBAL_MCAST_ENABLED;
+		INIT_LIST_HEAD(&v->port_vlist);
 	}
 
 	/* Add the dev mac and count the vlan only if it's usable */
-- 
2.47.3

[PATCH net-next 5/9] net: bridge: consider only port-VLAN members when flooding

[Nikolay Aleksandrov]

Use the port-VLAN list that is in every master VLAN and consider only
participating port-VLANs when flooding packets. This is the first
optimization that greatly improves performance for sparse port VLANs
e.g. 2 out of 32 ports participating in a single VLAN: before the
bridge would consider all 32 ports and forward the packet only through
the 2, now it will only consider the participating 2 ports.

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 From local sashiko run:
 [Severity: Medium]
 Can this loop skip a newly added port VLAN while __vlan_add() is between
 its two RCU publication steps?

 In net/bridge/br_vlan.c:__vlan_add(), the port VLAN is first published in
 the ingress lookup hash:

    err = rhashtable_lookup_insert_fast(&vg->vlan_hash, &v->vnode,
                                        br_vlan_rht_params);
    if (err)
        goto out_fdb_insert;

    __vlan_add_list(v);

 Only the subsequent net/bridge/br_vlan.c:__vlan_add_list() call publishes
 it in the list now used by net/bridge/br_forward.c:br_flood():

    list_add_rcu(&v->port_vlist, &v->brvlan->port_vlist);

 A concurrent tagged frame can follow br_handle_frame_finish() through
 net/bridge/br_vlan.c:__allowed_ingress() and find the new VLAN here:

    v = br_vlan_find(vg, *vid);

 If the destination requires flooding, can it then reach br_flood() before
 the port VLAN appears in port_vlist?  With hairpin mode enabled, that would
 omit the required copy through the ingress port.  The same window can omit
 the newly enabled port for broadcast, multicast, or unknown-unicast traffic
 originated by net/bridge/br_device.c:br_dev_xmit().

 RTNL serializes configuration writers, but the RCU packet path does not take
 RTNL.  Is there another mechanism that makes the hash insertion and
 port_vlist insertion appear atomic to these readers?

 Before this change, br_flood() traversed every bridge port, and its egress
 VLAN lookup could find the already hash-published VLAN.  Could the flood
 membership publication be made consistent with the hash publication before
 the secondary membership set becomes authoritative?

 The later series state appears to select masterv->port_array when present
 and masterv->port_vlist otherwise, so it still floods exclusively through a
 secondary membership set.  Would the same hash-before-secondary-publication
 window remain in that state as well?

 Nik: Yes, both loops (list and array) can skip it, but that is ok.

 net/bridge/br_device.c  |  8 ++++----
 net/bridge/br_forward.c | 29 +++++++++++++++++++++--------
 net/bridge/br_input.c   |  2 +-
 net/bridge/br_private.h |  6 +++---
 4 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index e01c44a90d84..ce9ea9ac3d0a 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -89,10 +89,10 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	dest = eth_hdr(skb)->h_dest;
 	if (is_broadcast_ether_addr(dest)) {
-		br_flood(br, skb, BR_PKT_BROADCAST, false, true, vid);
+		br_flood(br, vlan, skb, BR_PKT_BROADCAST, false, true);
 	} else if (is_multicast_ether_addr(dest)) {
 		if (unlikely(netpoll_tx_running(dev))) {
-			br_flood(br, skb, BR_PKT_MULTICAST, false, true, vid);
+			br_flood(br, vlan, skb, BR_PKT_MULTICAST, false, true);
 			goto out;
 		}
 		if (br_multicast_rcv(&brmctx, &pmctx_null, vlan, skb, vid)) {
@@ -105,11 +105,11 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
 		    br_multicast_querier_exists(brmctx, eth_hdr(skb), mdst))
 			br_multicast_flood(mdst, skb, brmctx, false, true);
 		else
-			br_flood(br, skb, BR_PKT_MULTICAST, false, true, vid);
+			br_flood(br, vlan, skb, BR_PKT_MULTICAST, false, true);
 	} else if ((dst = br_fdb_find_rcu(br, dest, vid)) != NULL) {
 		br_forward(READ_ONCE(dst->dst), skb, false, true);
 	} else {
-		br_flood(br, skb, BR_PKT_UNICAST, false, true, vid);
+		br_flood(br, vlan, skb, BR_PKT_UNICAST, false, true);
 	}
 out:
 	rcu_read_unlock();
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index a696c6c128e3..251d61e7c312 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -262,19 +262,32 @@ static void br_flood_port(struct net_bridge_port **prev,
 }
 
 /* called under rcu_read_lock */
-void br_flood(struct net_bridge *br, struct sk_buff *skb,
-	      enum br_pkt_type pkt_type, bool local_rcv, bool local_orig,
-	      u16 vid)
+void br_flood(struct net_bridge *br, struct net_bridge_vlan *v,
+	      struct sk_buff *skb, enum br_pkt_type pkt_type,
+	      bool local_rcv, bool local_orig)
 {
 	struct net_bridge_port *prev = NULL;
-	struct net_bridge_port *p;
 
 	br_tc_skb_miss_set(skb, pkt_type != BR_PKT_BROADCAST);
 
-	list_for_each_entry_rcu(p, &br->port_list, list) {
-		br_flood_port(&prev, p, skb, pkt_type, local_orig, vid);
-		if (IS_ERR(prev))
-			break;
+	if (v) {
+		struct net_bridge_vlan *masterv, *pv;
+
+		masterv = br_vlan_is_master(v) ? v : v->brvlan;
+		list_for_each_entry_rcu(pv, &masterv->port_vlist, port_vlist) {
+			br_flood_port(&prev, pv->port, skb, pkt_type,
+				      local_orig, v->vid);
+			if (IS_ERR(prev))
+				break;
+		}
+	} else {
+		struct net_bridge_port *p;
+
+		list_for_each_entry_rcu(p, &br->port_list, list) {
+			br_flood_port(&prev, p, skb, pkt_type, local_orig, 0);
+			if (IS_ERR(prev))
+				break;
+		}
 	}
 
 	br_flood_finish(prev, skb, local_rcv, local_orig);
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 8bed72baf161..b20c7c182a80 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -226,7 +226,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
 		br_forward(READ_ONCE(dst->dst), skb, local_rcv, false);
 	} else {
 		if (!mcast_hit)
-			br_flood(br, skb, pkt_type, local_rcv, false, vid);
+			br_flood(br, vlan, skb, pkt_type, local_rcv, false);
 		else
 			br_multicast_flood(mdst, skb, brmctx, local_rcv, false);
 	}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index b2a12b6298cb..239cf58d2268 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -912,9 +912,9 @@ int br_dev_queue_push_xmit(struct net *net, struct sock *sk, struct sk_buff *skb
 void br_forward(const struct net_bridge_port *to, struct sk_buff *skb,
 		bool local_rcv, bool local_orig);
 int br_forward_finish(struct net *net, struct sock *sk, struct sk_buff *skb);
-void br_flood(struct net_bridge *br, struct sk_buff *skb,
-	      enum br_pkt_type pkt_type, bool local_rcv, bool local_orig,
-	      u16 vid);
+void br_flood(struct net_bridge *br, struct net_bridge_vlan *v,
+	      struct sk_buff *skb, enum br_pkt_type pkt_type,
+	      bool local_rcv, bool local_orig);
 
 /* return true if both source port and dest port are isolated */
 static inline bool br_skb_isolated(const struct net_bridge_port *to,
-- 
2.47.3

[PATCH net-next 6/9] net: bridge: vlan: use an RCU array for large flood sets

[Nikolay Aleksandrov]

Walking the master VLAN's port-VLAN list avoids considering ports outside
the VLAN but its pointer chasing becomes more expensive than array when
many ports participate. Add an rcu array of port-VLAN pointers for larger
flood sets. Rebuild and publish the array under RTNL when VLAN membership
changes. Continue using the list for small flood sets and as a fallback
if the array allocation fails.

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 From local sashiko run:
 [Severity: Medium]
 Could these rebuilds be amortized or batched for large VLAN memberships?

 In net/bridge/br_vlan.c, br_vlan_rebuild_port_array() allocates and copies the
 complete masterv->port_vlist whenever count exceeds
 BR_VLAN_PORT_ARRAY_THRESHOLD. It then defers freeing the previous complete
 array through kvfree_rcu(), so sustained updates can retain several full-array
 generations until their RCU grace periods finish.

 Every successful individual port-VLAN addition calls this helper from
 __vlan_add(), while every individual deletion calls it from __vlan_del().
 Growing a flood set from nine entries to N therefore copies
 9 + 10 + ... + N pointers, and shrinking it performs the same quadratic work
 while RTNL is held.

 Can this cause a control-plane CPU and transient-memory regression during
 large incremental bridge VLAN updates? The later patches in the series retain
 this rebuild-on-add/delete path in the final series state.

 Nik: Yes, that is well understood but it is control path and I have tested
      sustained 2k / sec VLAN add/delete with 64 VLAN ports in each VLAN.
      If it ever becomes a problem we can optimize it, I think for the
      initial implementation would be best to keep it simple.

 net/bridge/br_forward.c | 39 ++++++++++++++++++++++++++++++---------
 net/bridge/br_private.h | 13 +++++++++++++
 net/bridge/br_vlan.c    | 37 ++++++++++++++++++++++++++++++++++++-
 3 files changed, 79 insertions(+), 10 deletions(-)

diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 251d61e7c312..e8f30f2df1ed 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -261,6 +261,35 @@ static void br_flood_port(struct net_bridge_port **prev,
 	*prev = maybe_deliver(*prev, p, skb, local_orig);
 }
 
+static void br_flood_vlan(struct net_bridge_port **prev,
+			  struct net_bridge_vlan *v, struct sk_buff *skb,
+			  enum br_pkt_type pkt_type, bool local_orig)
+{
+	struct net_bridge_vlan_port_array *array;
+	struct net_bridge_vlan *masterv, *pv;
+
+	masterv = br_vlan_is_master(v) ? v : v->brvlan;
+	array = rcu_dereference(masterv->port_array);
+	if (array) {
+		unsigned int i;
+
+		for (i = 0; i < array->count; i++) {
+			pv = array->vlans[i];
+			br_flood_port(prev, pv->port, skb, pkt_type,
+				      local_orig, v->vid);
+			if (IS_ERR(*prev))
+				break;
+		}
+	} else {
+		list_for_each_entry_rcu(pv, &masterv->port_vlist, port_vlist) {
+			br_flood_port(prev, pv->port, skb, pkt_type,
+				      local_orig, v->vid);
+			if (IS_ERR(*prev))
+				break;
+		}
+	}
+}
+
 /* called under rcu_read_lock */
 void br_flood(struct net_bridge *br, struct net_bridge_vlan *v,
 	      struct sk_buff *skb, enum br_pkt_type pkt_type,
@@ -271,15 +300,7 @@ void br_flood(struct net_bridge *br, struct net_bridge_vlan *v,
 	br_tc_skb_miss_set(skb, pkt_type != BR_PKT_BROADCAST);
 
 	if (v) {
-		struct net_bridge_vlan *masterv, *pv;
-
-		masterv = br_vlan_is_master(v) ? v : v->brvlan;
-		list_for_each_entry_rcu(pv, &masterv->port_vlist, port_vlist) {
-			br_flood_port(&prev, pv->port, skb, pkt_type,
-				      local_orig, v->vid);
-			if (IS_ERR(prev))
-				break;
-		}
+		br_flood_vlan(&prev, v, skb, pkt_type, local_orig);
 	} else {
 		struct net_bridge_port *p;
 
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 239cf58d2268..a33da6e9765f 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -190,6 +190,17 @@ enum {
 	BR_VLFLAG_NEIGH_FORWARD_GRAT_ENABLED = BIT(6),
 };
 
+/* start publishing arrays when there're > BR_VLAN_PORT_ARRAY_THRESHOLD
+ * port-VLANs
+ */
+#define BR_VLAN_PORT_ARRAY_THRESHOLD 8
+
+struct net_bridge_vlan_port_array {
+	struct rcu_head		rcu;
+	unsigned int		count;
+	struct net_bridge_vlan	*vlans[];
+};
+
 /**
  * struct net_bridge_vlan - per-vlan entry
  *
@@ -210,6 +221,7 @@ enum {
  * @port_mcast_ctx: if MASTER flag unset, this is the per-port/vlan multicast
  *                  context
  * @msti: if MASTER flag set, this holds the VLANs MST instance
+ * @port_array: if MASTER flag set, this is the port-VLAN array
  * @port_vlist: if MASTER flag set, this is the port-VLAN list
  * @vlist: sorted list of VLAN entries
  * @rcu: used for entry destruction
@@ -245,6 +257,7 @@ struct net_bridge_vlan {
 
 	u16				msti;
 
+	struct net_bridge_vlan_port_array __rcu *port_array;
 	struct list_head		port_vlist;
 	struct list_head		vlist;
 
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 34d1df59d190..d750581df64d 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -258,6 +258,36 @@ static void br_vlan_init_state(struct net_bridge_vlan *v)
 	v->msti = 0;
 }
 
+static unsigned int br_vlan_num_ports(const struct net_bridge_vlan *masterv)
+{
+	return refcount_read(&masterv->refcnt) - br_vlan_is_brentry(masterv);
+}
+
+static void br_vlan_rebuild_port_array(struct net_bridge_vlan *masterv,
+				       unsigned int count)
+{
+	struct net_bridge_vlan_port_array *array = NULL, *old;
+	unsigned int i = 0;
+
+	WARN_ON(!br_vlan_is_master(masterv));
+
+	if (count > BR_VLAN_PORT_ARRAY_THRESHOLD)
+		array = kvmalloc(struct_size(array, vlans, count), GFP_KERNEL);
+
+	if (array) {
+		struct net_bridge_vlan *pv;
+
+		array->count = count;
+		list_for_each_entry(pv, &masterv->port_vlist, port_vlist)
+			array->vlans[i++] = pv;
+	}
+
+	old = rtnl_dereference(masterv->port_array);
+	rcu_assign_pointer(masterv->port_array, array);
+	if (old)
+		kvfree_rcu(old, rcu);
+}
+
 /* This is the shared VLAN add function which works for both ports and bridge
  * devices. There are four possible calls to this function in terms of the
  * vlan entry type:
@@ -368,8 +398,10 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags,
 	__vlan_flags_commit(v, flags);
 	br_multicast_toggle_one_vlan(v, true);
 
-	if (p)
+	if (p) {
+		br_vlan_rebuild_port_array(masterv, br_vlan_num_ports(masterv));
 		nbp_vlan_set_vlan_dev_state(p, v->vid);
+	}
 out:
 	return err;
 
@@ -438,6 +470,9 @@ static void __vlan_del(struct net_bridge_vlan *v)
 		rhashtable_remove_fast(&vg->vlan_hash, &v->vnode,
 				       br_vlan_rht_params);
 		__vlan_del_list(v);
+		/* -1 because br_vlan_put_master() is called later */
+		br_vlan_rebuild_port_array(masterv,
+					   br_vlan_num_ports(masterv) - 1);
 		nbp_vlan_set_vlan_dev_state(p, v->vid);
 		br_multicast_toggle_one_vlan(v, false);
 		br_multicast_port_ctx_deinit(&v->port_mcast_ctx);
-- 
2.47.3

[PATCH net-next 7/9] net: bridge: introduce a forwarding destination structure

[Nikolay Aleksandrov]

Introduce struct br_fwd_dst to keep an egress port together with its
corresponding VLAN while passing it through the forwarding helpers. This
allows paths which already resolved the port VLAN to preserve that
information for later egress processing. maybe_deliver() now returns the
clone error directly and we pass it separately to br_flood_finish()
avoiding the error-pointer encoding and decoding which overloaded the
previous port pointer.
No functional changes intended.

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 net/bridge/br_forward.c | 146 +++++++++++++++++++++++++---------------
 1 file changed, 90 insertions(+), 56 deletions(-)

diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index e8f30f2df1ed..845193baf992 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -7,7 +7,6 @@
  *	Lennert Buytenhek		<buytenh@gnu.org>
  */
 
-#include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/kernel.h>
 #include <linux/netdevice.h>
@@ -17,10 +16,16 @@
 #include <linux/netfilter_bridge.h>
 #include "br_private.h"
 
+struct br_fwd_dst {
+	const struct net_bridge_port *port;
+	struct net_bridge_vlan *vlan;
+};
+
 /* Don't forward packets to originating port or forwarding disabled */
-static inline int should_deliver(const struct net_bridge_port *p,
+static inline int should_deliver(const struct br_fwd_dst *fwd,
 				 const struct sk_buff *skb)
 {
+	const struct net_bridge_port *p = fwd->port;
 	struct net_bridge_vlan_group *vg;
 
 	vg = nbp_vlan_group_rcu(p);
@@ -70,9 +75,10 @@ int br_forward_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
 }
 EXPORT_SYMBOL_GPL(br_forward_finish);
 
-static void __br_forward(const struct net_bridge_port *to,
+static void __br_forward(const struct br_fwd_dst *fwd,
 			 struct sk_buff *skb, bool local_orig)
 {
+	const struct net_bridge_port *to = fwd->port;
 	struct net_bridge_vlan_group *vg;
 	struct net_device *indev;
 	struct net *net;
@@ -117,7 +123,7 @@ static void __br_forward(const struct net_bridge_port *to,
 		br_forward_finish);
 }
 
-static int deliver_clone(const struct net_bridge_port *prev,
+static int deliver_clone(const struct br_fwd_dst *fwd,
 			 struct sk_buff *skb, bool local_orig)
 {
 	struct net_device *dev = BR_INPUT_SKB_CB(skb)->brdev;
@@ -128,7 +134,7 @@ static int deliver_clone(const struct net_bridge_port *prev,
 		return -ENOMEM;
 	}
 
-	__br_forward(prev, skb, local_orig);
+	__br_forward(fwd, skb, local_orig);
 	return 0;
 }
 
@@ -144,6 +150,8 @@ static int deliver_clone(const struct net_bridge_port *prev,
 void br_forward(const struct net_bridge_port *to,
 		struct sk_buff *skb, bool local_rcv, bool local_orig)
 {
+	struct br_fwd_dst fwd;
+
 	if (unlikely(!to))
 		goto out;
 
@@ -159,11 +167,13 @@ void br_forward(const struct net_bridge_port *to,
 		to = backup_port;
 	}
 
-	if (should_deliver(to, skb)) {
+	fwd.port = to;
+	fwd.vlan = NULL;
+	if (should_deliver(&fwd, skb)) {
 		if (local_rcv)
-			deliver_clone(to, skb, local_orig);
+			deliver_clone(&fwd, skb, local_orig);
 		else
-			__br_forward(to, skb, local_orig);
+			__br_forward(&fwd, skb, local_orig);
 		return;
 	}
 
@@ -173,40 +183,41 @@ void br_forward(const struct net_bridge_port *to,
 }
 EXPORT_SYMBOL_GPL(br_forward);
 
-static struct net_bridge_port *maybe_deliver(
-	struct net_bridge_port *prev, struct net_bridge_port *p,
-	struct sk_buff *skb, bool local_orig)
+static int maybe_deliver(struct br_fwd_dst *prev, const struct br_fwd_dst *fwd,
+			 struct sk_buff *skb, bool local_orig)
 {
+	const struct net_bridge_port *p = fwd->port;
 	u8 igmp_type = br_multicast_igmp_type(skb);
 	int err;
 
-	if (!should_deliver(p, skb))
-		return prev;
+	if (!should_deliver(fwd, skb))
+		return 0;
 
 	nbp_switchdev_frame_mark_tx_fwd_to_hwdom(p, skb);
 
-	if (!prev)
+	if (!prev->port)
 		goto out;
 
 	err = deliver_clone(prev, skb, local_orig);
 	if (err)
-		return ERR_PTR(err);
+		return err;
 out:
 	br_multicast_count(p->br, p, skb, igmp_type, BR_MCAST_DIR_TX);
+	*prev = *fwd;
 
-	return p;
+	return 0;
 }
 
-static void br_flood_finish(struct net_bridge_port *prev, struct sk_buff *skb,
+static void br_flood_finish(const struct br_fwd_dst *fwd, int err,
+			    struct sk_buff *skb,
 			    bool local_rcv, bool local_orig)
 {
 	enum skb_drop_reason reason = SKB_DROP_REASON_NO_TX_TARGET;
 
-	if (IS_ERR_OR_NULL(prev)) {
-		if (IS_ERR(prev)) {
-			reason = PTR_ERR(prev) == -ENOMEM ? SKB_DROP_REASON_NOMEM :
-				 SKB_DROP_REASON_NOT_SPECIFIED;
-		}
+	if (err || !fwd->port) {
+		if (err)
+			reason = err == -ENOMEM ? SKB_DROP_REASON_NOMEM :
+						  SKB_DROP_REASON_NOT_SPECIFIED;
 
 		if (!local_rcv)
 			kfree_skb_reason(skb, reason);
@@ -214,59 +225,64 @@ static void br_flood_finish(struct net_bridge_port *prev, struct sk_buff *skb,
 	}
 
 	if (local_rcv)
-		deliver_clone(prev, skb, local_orig);
+		deliver_clone(fwd, skb, local_orig);
 	else
-		__br_forward(prev, skb, local_orig);
+		__br_forward(fwd, skb, local_orig);
 }
 
-static void br_flood_port(struct net_bridge_port **prev,
-			  struct net_bridge_port *p, struct sk_buff *skb,
-			  enum br_pkt_type pkt_type, bool local_orig, u16 vid)
+static int br_flood_port(struct br_fwd_dst *prev,
+			 const struct br_fwd_dst *fwd, struct sk_buff *skb,
+			 enum br_pkt_type pkt_type, bool local_orig)
 {
+	const struct net_bridge_port *p = fwd->port;
+	u16 vid = fwd->vlan ? fwd->vlan->vid : 0;
+
 	/* Do not flood unicast traffic to ports that turn it off, nor
 	 * other traffic if flood off, except for traffic we originate
 	 */
 	switch (pkt_type) {
 	case BR_PKT_UNICAST:
 		if (!test_bit(BR_FLOOD_BIT, &p->flags))
-			return;
+			return 0;
 		break;
 	case BR_PKT_MULTICAST:
 		if (!test_bit(BR_MCAST_FLOOD_BIT, &p->flags) &&
 		    skb->dev != p->br->dev)
-			return;
+			return 0;
 		break;
 	case BR_PKT_BROADCAST:
 		if (!test_bit(BR_BCAST_FLOOD_BIT, &p->flags) &&
 		    skb->dev != p->br->dev)
-			return;
+			return 0;
 		break;
 	}
 
 	/* Do not flood to ports that enable proxy ARP */
 	if (test_bit(BR_PROXYARP_BIT, &p->flags))
-		return;
+		return 0;
 	if (BR_INPUT_SKB_CB(skb)->proxyarp_replied) {
 		if (test_bit(BR_PROXYARP_WIFI_BIT, &p->flags))
-			return;
+			return 0;
 		/* For gratuitous ARPs/NAs, check neigh_forward_grat.
 		 * For regular ARPs/NDs, check only neigh_suppress.
 		 */
 		if (br_is_neigh_suppress_enabled(p, vid) &&
 		    (!BR_INPUT_SKB_CB(skb)->grat_arp ||
 		     !br_is_neigh_forward_grat_enabled(p, vid)))
-			return;
+			return 0;
 	}
 
-	*prev = maybe_deliver(*prev, p, skb, local_orig);
+	return maybe_deliver(prev, fwd, skb, local_orig);
 }
 
-static void br_flood_vlan(struct net_bridge_port **prev,
-			  struct net_bridge_vlan *v, struct sk_buff *skb,
-			  enum br_pkt_type pkt_type, bool local_orig)
+static int br_flood_vlan(struct br_fwd_dst *prev,
+			 struct net_bridge_vlan *v, struct sk_buff *skb,
+			 enum br_pkt_type pkt_type, bool local_orig)
 {
 	struct net_bridge_vlan_port_array *array;
 	struct net_bridge_vlan *masterv, *pv;
+	struct br_fwd_dst dst;
+	int err;
 
 	masterv = br_vlan_is_master(v) ? v : v->brvlan;
 	array = rcu_dereference(masterv->port_array);
@@ -275,19 +291,25 @@ static void br_flood_vlan(struct net_bridge_port **prev,
 
 		for (i = 0; i < array->count; i++) {
 			pv = array->vlans[i];
-			br_flood_port(prev, pv->port, skb, pkt_type,
-				      local_orig, v->vid);
-			if (IS_ERR(*prev))
-				break;
+			dst.port = pv->port;
+			dst.vlan = pv;
+			err = br_flood_port(prev, &dst, skb, pkt_type,
+					    local_orig);
+			if (err)
+				return err;
 		}
 	} else {
 		list_for_each_entry_rcu(pv, &masterv->port_vlist, port_vlist) {
-			br_flood_port(prev, pv->port, skb, pkt_type,
-				      local_orig, v->vid);
-			if (IS_ERR(*prev))
-				break;
+			dst.port = pv->port;
+			dst.vlan = pv;
+			err = br_flood_port(prev, &dst, skb, pkt_type,
+					    local_orig);
+			if (err)
+				return err;
 		}
 	}
+
+	return 0;
 }
 
 /* called under rcu_read_lock */
@@ -295,23 +317,29 @@ void br_flood(struct net_bridge *br, struct net_bridge_vlan *v,
 	      struct sk_buff *skb, enum br_pkt_type pkt_type,
 	      bool local_rcv, bool local_orig)
 {
-	struct net_bridge_port *prev = NULL;
+	struct br_fwd_dst prev = {};
+	int err = 0;
 
 	br_tc_skb_miss_set(skb, pkt_type != BR_PKT_BROADCAST);
 
 	if (v) {
-		br_flood_vlan(&prev, v, skb, pkt_type, local_orig);
+		err = br_flood_vlan(&prev, v, skb, pkt_type, local_orig);
 	} else {
 		struct net_bridge_port *p;
 
 		list_for_each_entry_rcu(p, &br->port_list, list) {
-			br_flood_port(&prev, p, skb, pkt_type, local_orig, 0);
-			if (IS_ERR(prev))
+			struct br_fwd_dst fwd = {
+				.port = p,
+			};
+
+			err = br_flood_port(&prev, &fwd, skb, pkt_type,
+					    local_orig);
+			if (err)
 				break;
 		}
 	}
 
-	br_flood_finish(prev, skb, local_rcv, local_orig);
+	br_flood_finish(&prev, err, skb, local_rcv, local_orig);
 }
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
@@ -320,9 +348,12 @@ static void maybe_deliver_addr(struct net_bridge_port *p, struct sk_buff *skb,
 {
 	struct net_device *dev = BR_INPUT_SKB_CB(skb)->brdev;
 	const unsigned char *src = eth_hdr(skb)->h_source;
+	struct br_fwd_dst fwd = {
+		.port = p,
+	};
 	struct sk_buff *nskb;
 
-	if (!should_deliver(p, skb))
+	if (!should_deliver(&fwd, skb))
 		return;
 
 	/* Even with hairpin, no soliloquies - prevent breaking IPv6 DAD */
@@ -342,7 +373,7 @@ static void maybe_deliver_addr(struct net_bridge_port *p, struct sk_buff *skb,
 	if (!is_broadcast_ether_addr(addr))
 		memcpy(eth_hdr(skb)->h_dest, addr, ETH_ALEN);
 
-	__br_forward(p, skb, local_orig);
+	__br_forward(&fwd, skb, local_orig);
 }
 
 /* called with rcu_read_lock */
@@ -351,10 +382,11 @@ void br_multicast_flood(struct net_bridge_mdb_entry *mdst,
 			struct net_bridge_mcast *brmctx,
 			bool local_rcv, bool local_orig)
 {
-	struct net_bridge_port *prev = NULL;
 	struct net_bridge_port_group *p;
 	bool allow_mode_include = true;
+	struct br_fwd_dst prev = {};
 	struct hlist_node *rp;
+	int err = 0;
 
 	rp = br_multicast_get_first_rport_node(brmctx, skb);
 
@@ -370,6 +402,7 @@ void br_multicast_flood(struct net_bridge_mdb_entry *mdst,
 
 	while (p || rp) {
 		struct net_bridge_port *port, *lport, *rport;
+		struct br_fwd_dst fwd = {};
 
 		lport = p ? p->key.port : NULL;
 		rport = br_multicast_rport_from_node_skb(rp, skb);
@@ -391,8 +424,9 @@ void br_multicast_flood(struct net_bridge_mdb_entry *mdst,
 			port = rport;
 		}
 
-		prev = maybe_deliver(prev, port, skb, local_orig);
-		if (IS_ERR(prev))
+		fwd.port = port;
+		err = maybe_deliver(&prev, &fwd, skb, local_orig);
+		if (err)
 			break;
 
 delivered:
@@ -402,6 +436,6 @@ void br_multicast_flood(struct net_bridge_mdb_entry *mdst,
 			rp = rcu_dereference(hlist_next_rcu(rp));
 	}
 
-	br_flood_finish(prev, skb, local_rcv, local_orig);
+	br_flood_finish(&prev, err, skb, local_rcv, local_orig);
 }
 #endif
-- 
2.47.3

[PATCH net-next 8/9] net: bridge: avoid egress VLAN lookups when flooding

[Nikolay Aleksandrov]

VLAN flooding already provides the exact port-VLAN entry associated with
each destination port. Use that entry directly for the egress state check
and VLAN handling instead of resolving it again from the packet VLAN ID.
Keep the existing lookup path for forwarding destinations which do not
provide a port-VLAN entry. This removes two VLAN hash lookups for every
flooded egress port. Use __always_inline for should_deliver as it provides
a measurable 3% additional gain, should_deliver wasn't inlined even though
it had the inline specifier, the bridge module size increases by 173 bytes.

The series were tested in a two CPU VM with packet generation and bridge
forwarding on separate pinned CPUs. The results are medians of seven runs
with 300000 64b tagged broadcast packets.

                   Mpps                   br_flood TSC cycles/input
  VIDs Ports  before    after    gain   before  after  reduction
     1  8/2   1.209298  1.636580   35.3%     1444    845      41.5%
     1  8/4   0.843175  0.986541   17.0%     3036   2527      16.8%
     1  8/8   0.508857  0.559844   10.0%     5999   5465       8.9%
     1 32/2   0.893896  1.586532   77.5%     3013    841      72.1%
     1 32/16  0.250567  0.282339   12.7%    13670  11137      18.5%
     1 32/32  0.135351  0.148461    9.7%    25429  22320      12.2%
     1 64/2   0.555010  1.603776  189.0%     5001    837      83.3%
     1 64/32  0.123831  0.149639   20.8%    27674  22602      18.3%
     1 64/64  0.069122  0.076047   10.0%    51508  45138      12.4%
    64  8/2   1.044185  1.500468   43.7%     1528    851      44.3%
    64  8/4   0.802680  0.977700   21.8%     3178   2546      19.9%
    64  8/8   0.479748  0.555419   15.8%     6283   5635      10.3%
    64 32/2   0.861074  1.561266   81.3%     3063    849      72.3%
    64 32/16  0.234011  0.280027   19.7%    14332  11165      22.1%
    64 32/32  0.126145  0.148085   17.4%    27225  22613      16.9%
    64 64/2   0.530587  1.496179  182.0%     4377    826      81.1%
    64 64/32  0.121105  0.147943   22.2%    29286  22541      23.0%
    64 64/64  0.063626  0.077051   21.1%    56147  45520      18.9%
  1024  8/2   1.008693  1.358408   34.7%     1538    879      42.8%
  1024  8/4   0.785636  0.943545   20.1%     3280   2704      17.6%
  1024  8/8   0.448194  0.521765   16.4%     6678   6011      10.0%
  1024 32/2   0.840864  1.417311   68.6%     2756    859      68.8%
  1024 32/16  0.195786  0.247129   26.2%    16269  12639      22.3%
  1024 32/32  0.097978  0.122742   25.3%    35249  29273      17.0%
  1024 64/2   0.536947  1.400985  160.9%     4421    849      80.8%
  1024 64/32  0.093962  0.123383   31.3%    37425  29197      22.0%
  1024 64/64  0.045147  0.057867   28.2%    72626  59198      18.5%

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 net/bridge/br_forward.c | 23 ++++++++++++++++-------
 net/bridge/br_input.c   |  2 +-
 net/bridge/br_private.h |  7 +++++++
 net/bridge/br_vlan.c    | 31 +++++++++++++++++--------------
 4 files changed, 41 insertions(+), 22 deletions(-)

diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 845193baf992..d89f1b7d6def 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -21,17 +21,26 @@ struct br_fwd_dst {
 	struct net_bridge_vlan *vlan;
 };
 
+static bool should_deliver_vlan(const struct br_fwd_dst *fwd,
+				const struct sk_buff *skb)
+{
+	if (fwd->vlan)
+		return br_vlan_state_allowed(br_vlan_get_state(fwd->vlan),
+					     false);
+
+	return br_allowed_egress(nbp_vlan_group_rcu(fwd->port), skb);
+}
+
 /* Don't forward packets to originating port or forwarding disabled */
-static inline int should_deliver(const struct br_fwd_dst *fwd,
-				 const struct sk_buff *skb)
+static __always_inline bool should_deliver(const struct br_fwd_dst *fwd,
+					   const struct sk_buff *skb)
 {
 	const struct net_bridge_port *p = fwd->port;
-	struct net_bridge_vlan_group *vg;
 
-	vg = nbp_vlan_group_rcu(p);
 	return (test_bit(BR_HAIRPIN_MODE_BIT, &p->flags) || skb->dev != p->dev) &&
 		(br_mst_is_enabled(p) || p->state == BR_STATE_FORWARDING) &&
-		br_allowed_egress(vg, skb) && nbp_switchdev_allowed_egress(p, skb) &&
+		should_deliver_vlan(fwd, skb) &&
+		nbp_switchdev_allowed_egress(p, skb) &&
 		!br_skb_isolated(p, skb);
 }
 
@@ -89,8 +98,8 @@ static void __br_forward(const struct br_fwd_dst *fwd,
 	 */
 	nbp_switchdev_frame_mark_tx_fwd_offload(to, skb);
 
-	vg = nbp_vlan_group_rcu(to);
-	skb = br_handle_vlan(to->br, to, vg, skb);
+	vg = fwd->vlan ? NULL : nbp_vlan_group_rcu(to);
+	skb = br_handle_vlan(to->br, to, vg, fwd->vlan, skb);
 	if (!skb)
 		return;
 
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index b20c7c182a80..4357d78524a6 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -58,7 +58,7 @@ static int br_pass_frame_up(struct sk_buff *skb, bool promisc)
 
 	indev = skb->dev;
 	skb->dev = brdev;
-	skb = br_handle_vlan(br, NULL, vg, skb);
+	skb = br_handle_vlan(br, NULL, vg, NULL, skb);
 	if (!skb)
 		return NET_RX_DROP;
 	/* update the multicast stats if the packet is IGMP/MLD */
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index a33da6e9765f..7c0b1d3e7931 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1606,6 +1606,7 @@ bool br_should_learn(struct net_bridge_port *p, struct sk_buff *skb, u16 *vid);
 struct sk_buff *br_handle_vlan(struct net_bridge *br,
 			       const struct net_bridge_port *port,
 			       struct net_bridge_vlan_group *vg,
+			       struct net_bridge_vlan *vlan,
 			       struct sk_buff *skb);
 int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags,
 		bool *changed, struct netlink_ext_ack *extack);
@@ -1743,6 +1744,7 @@ static inline bool br_should_learn(struct net_bridge_port *p,
 static inline struct sk_buff *br_handle_vlan(struct net_bridge *br,
 					     const struct net_bridge_port *port,
 					     struct net_bridge_vlan_group *vg,
+					     struct net_bridge_vlan *vlan,
 					     struct sk_buff *skb)
 {
 	return skb;
@@ -1953,6 +1955,11 @@ static inline bool br_vlan_state_allowed(u8 state, bool learn_allow)
 		return false;
 	}
 }
+#else
+static inline bool br_vlan_state_allowed(u8 state, bool learn_allow)
+{
+	return false;
+}
 #endif
 
 /* br_mst.c */
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index d750581df64d..ce5aa15c4540 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -520,10 +520,10 @@ static void __vlan_flush(const struct net_bridge *br,
 struct sk_buff *br_handle_vlan(struct net_bridge *br,
 			       const struct net_bridge_port *p,
 			       struct net_bridge_vlan_group *vg,
+			       struct net_bridge_vlan *v,
 			       struct sk_buff *skb)
 {
 	struct pcpu_sw_netstats *stats;
-	struct net_bridge_vlan *v;
 	u16 vid;
 
 	/* If this packet was not filtered at input, let it pass */
@@ -534,19 +534,22 @@ struct sk_buff *br_handle_vlan(struct net_bridge *br,
 	 * a valid vlan id.  If the vlan id has untagged flag set,
 	 * send untagged; otherwise, send tagged.
 	 */
-	br_vlan_get_tag(skb, &vid);
-	v = br_vlan_find(vg, vid);
-	/* Vlan entry must be configured at this point.  The
-	 * only exception is the bridge is set in promisc mode and the
-	 * packet is destined for the bridge device.  In this case
-	 * pass the packet as is.
-	 */
-	if (!v || !br_vlan_should_use(v)) {
-		if ((br->dev->flags & IFF_PROMISC) && skb->dev == br->dev) {
-			goto out;
-		} else {
-			kfree_skb(skb);
-			return NULL;
+	if (!v) {
+		br_vlan_get_tag(skb, &vid);
+		v = br_vlan_find(vg, vid);
+		/* Vlan entry must be configured at this point.  The
+		 * only exception is the bridge is set in promisc mode and the
+		 * packet is destined for the bridge device.  In this case
+		 * pass the packet as is.
+		 */
+		if (!v || !br_vlan_should_use(v)) {
+			if ((br->dev->flags & IFF_PROMISC) &&
+			    skb->dev == br->dev) {
+				goto out;
+			} else {
+				kfree_skb(skb);
+				return NULL;
+			}
 		}
 	}
 	if (br_opt_get(br, BROPT_VLAN_STATS_ENABLED)) {
-- 
2.47.3

[PATCH net-next 9/9] net: bridge: avoid VLAN lookups for flood neighbour suppression

[Nikolay Aleksandrov]

We can remove more unnecessary VLAN lookups if we pass the port-VLAN
entry to the neighbour suppression helpers so they can use it directly
instead of resolving it again. Use a vid wrapper for callers which don't
have the port-VLAN entry. This removes up to two additional VLAN hash
lookups per flooded egress port for packets marked as proxy replied.

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 net/bridge/br_arp_nd_proxy.c | 51 ++++++++++++++++++++----------------
 net/bridge/br_forward.c      |  5 ++--
 net/bridge/br_private.h      |  6 +++--
 3 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/net/bridge/br_arp_nd_proxy.c b/net/bridge/br_arp_nd_proxy.c
index 9ce8f440e38f..da15f4d7c1ae 100644
--- a/net/bridge/br_arp_nd_proxy.c
+++ b/net/bridge/br_arp_nd_proxy.c
@@ -40,6 +40,23 @@ void br_recalculate_neigh_suppress_enabled(struct net_bridge *br)
 }
 
 #if IS_ENABLED(CONFIG_INET)
+static bool
+br_is_neigh_suppress_enabled_vid(const struct net_bridge_port *p, u16 vid)
+{
+	const struct net_bridge_vlan *v = NULL;
+
+	if (p && vid && test_bit(BR_NEIGH_VLAN_SUPPRESS_BIT, &p->flags)) {
+		struct net_bridge_vlan_group *vg;
+
+		vg = nbp_vlan_group_rcu(p);
+		v = br_vlan_find(vg, vid);
+		if (!v)
+			return false;
+	}
+
+	return br_is_neigh_suppress_enabled(p, v);
+}
+
 static void br_arp_send(struct net_bridge *br, struct net_bridge_port *p,
 			struct net_device *dev, __be32 dest_ip, __be32 src_ip,
 			const unsigned char *dest_hw,
@@ -159,7 +176,7 @@ void br_do_proxy_suppress_arp(struct sk_buff *skb, struct net_bridge *br,
 		return;
 
 	if (br_opt_get(br, BROPT_NEIGH_SUPPRESS_ENABLED)) {
-		if (br_is_neigh_suppress_enabled(p, vid))
+		if (br_is_neigh_suppress_enabled_vid(p, vid))
 			return;
 		if (is_unicast_ether_addr(eth_hdr(skb)->h_dest) &&
 		    parp->ar_op == htons(ARPOP_REQUEST))
@@ -211,7 +228,7 @@ void br_do_proxy_suppress_arp(struct sk_buff *skb, struct net_bridge *br,
 
 			if ((p && test_bit(BR_PROXYARP_BIT, &p->flags)) ||
 			    (dst && test_bit(BR_PROXYARP_WIFI_BIT, &dst->flags)) ||
-			    br_is_neigh_suppress_enabled(dst, vid)) {
+			    br_is_neigh_suppress_enabled_vid(dst, vid)) {
 				if (!vid)
 					br_arp_send(br, p, skb->dev, sip, tip,
 						    sha, ha, sha, 0, 0);
@@ -424,7 +441,7 @@ void br_do_suppress_nd(struct sk_buff *skb, struct net_bridge *br,
 	BR_INPUT_SKB_CB(skb)->proxyarp_replied = 0;
 	BR_INPUT_SKB_CB(skb)->grat_arp = 0;
 
-	if (br_is_neigh_suppress_enabled(p, vid))
+	if (br_is_neigh_suppress_enabled_vid(p, vid))
 		return;
 
 	if (is_unicast_ether_addr(eth_hdr(skb)->h_dest) &&
@@ -486,7 +503,7 @@ void br_do_suppress_nd(struct sk_buff *skb, struct net_bridge *br,
 			const struct net_bridge_port *dst = READ_ONCE(f->dst);
 			bool replied = false;
 
-			if (br_is_neigh_suppress_enabled(dst, vid)) {
+			if (br_is_neigh_suppress_enabled_vid(dst, vid)) {
 				if (vid != 0)
 					br_nd_send(br, p, skb, n, ha,
 						   skb->vlan_proto,
@@ -509,35 +526,25 @@ void br_do_suppress_nd(struct sk_buff *skb, struct net_bridge *br,
 }
 #endif
 
-bool br_is_neigh_suppress_enabled(const struct net_bridge_port *p, u16 vid)
+bool br_is_neigh_suppress_enabled(const struct net_bridge_port *p,
+				  const struct net_bridge_vlan *v)
 {
 	if (!p)
 		return false;
 
-	if (vid && test_bit(BR_NEIGH_VLAN_SUPPRESS_BIT, &p->flags)) {
-		struct net_bridge_vlan_group *vg = nbp_vlan_group_rcu(p);
-		struct net_bridge_vlan *v;
-
-		v = br_vlan_find(vg, vid);
-		if (!v)
-			return false;
+	if (v && test_bit(BR_NEIGH_VLAN_SUPPRESS_BIT, &p->flags))
 		return !!(READ_ONCE(v->priv_flags) &
 			  BR_VLFLAG_NEIGH_SUPPRESS_ENABLED);
-	}
+
 	return test_bit(BR_NEIGH_SUPPRESS_BIT, &p->flags);
 }
 
-bool br_is_neigh_forward_grat_enabled(const struct net_bridge_port *p, u16 vid)
+bool br_is_neigh_forward_grat_enabled(const struct net_bridge_port *p,
+				      const struct net_bridge_vlan *v)
 {
-	if (vid && test_bit(BR_NEIGH_VLAN_SUPPRESS_BIT, &p->flags)) {
-		struct net_bridge_vlan_group *vg = nbp_vlan_group_rcu(p);
-		struct net_bridge_vlan *v;
-
-		v = br_vlan_find(vg, vid);
-		if (!v)
-			return false;
+	if (v && test_bit(BR_NEIGH_VLAN_SUPPRESS_BIT, &p->flags))
 		return !!(READ_ONCE(v->priv_flags) &
 			  BR_VLFLAG_NEIGH_FORWARD_GRAT_ENABLED);
-	}
+
 	return test_bit(BR_NEIGH_FORWARD_GRAT_BIT, &p->flags);
 }
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index d89f1b7d6def..b5eece1ff9e8 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -244,7 +244,6 @@ static int br_flood_port(struct br_fwd_dst *prev,
 			 enum br_pkt_type pkt_type, bool local_orig)
 {
 	const struct net_bridge_port *p = fwd->port;
-	u16 vid = fwd->vlan ? fwd->vlan->vid : 0;
 
 	/* Do not flood unicast traffic to ports that turn it off, nor
 	 * other traffic if flood off, except for traffic we originate
@@ -275,9 +274,9 @@ static int br_flood_port(struct br_fwd_dst *prev,
 		/* For gratuitous ARPs/NAs, check neigh_forward_grat.
 		 * For regular ARPs/NDs, check only neigh_suppress.
 		 */
-		if (br_is_neigh_suppress_enabled(p, vid) &&
+		if (br_is_neigh_suppress_enabled(p, fwd->vlan) &&
 		    (!BR_INPUT_SKB_CB(skb)->grat_arp ||
-		     !br_is_neigh_forward_grat_enabled(p, vid)))
+		     !br_is_neigh_forward_grat_enabled(p, fwd->vlan)))
 			return 0;
 	}
 
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 7c0b1d3e7931..67117fb3dc88 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -2390,6 +2390,8 @@ void br_do_proxy_suppress_arp(struct sk_buff *skb, struct net_bridge *br,
 void br_do_suppress_nd(struct sk_buff *skb, struct net_bridge *br,
 		       u16 vid, struct net_bridge_port *p, struct nd_msg *msg);
 struct nd_msg *br_is_nd_neigh_msg(struct sk_buff *skb);
-bool br_is_neigh_suppress_enabled(const struct net_bridge_port *p, u16 vid);
-bool br_is_neigh_forward_grat_enabled(const struct net_bridge_port *p, u16 vid);
+bool br_is_neigh_suppress_enabled(const struct net_bridge_port *p,
+				  const struct net_bridge_vlan *v);
+bool br_is_neigh_forward_grat_enabled(const struct net_bridge_port *p,
+				      const struct net_bridge_vlan *v);
 #endif
-- 
2.47.3

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

[Nikolay Aleksandrov]

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

Re: [PATCH net-next 0/9] net: bridge: vlan: broadcast fwding path optimizations

[Ido Schimmel]

On Fri, Sep 18, 2026 at 06:29:41PM +0300, Nikolay Aleksandrov wrote:
> Hi,
> This patch-set is the first step to removing unnecessary VLAN hash lookups
> from the bridge fast-path and also provides additional broadcast path
> optimization by considering only the port-VLANs that actually participate
> in the VLAN instead of all bridge ports. Combined these changes bring
> considerable improvements[1] and open the path to do the same for the most
> common fdb forwarding path. As expected the pathological cases have the
> largest gain, e.g. if we have 1 client / VM facing port and 1 upstream port
> only participating in a VLAN with many other bridge ports, we consider only
> those 2 when broadcasting in that VLAN.
> 
> For up to 8 ports we only use the port-VLAN list as the array doesn't make
> much of a difference and is unnecessary. Note that the array is rebuilt on
> every member add or delete when >8 members, I tested that and could do
> 2000 VLAN add/delete with 64 port-VLAN members / sec on my test VM.
> If it ever becomes a problem we can optimize it further, for the time
> being I prefer it is simpler.

Very nice!

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

A couple of nits that you can handle later (or not):

1. Patch #3:

__vlan_delete_pvid(vg, rtnl_dereference(vg->pvid));

Can be simplified to:

RCU_INIT_POINTER(vg->pvid, NULL);

2. Patch #7:

Initially I thought that 'fwd.vlan' is never set, but then I realized
that in the only place where this field is actually set it's called
'dst.vlan', which is a bit confusing given that in the context of the
bridge driver 'dst' is usually an FDB entry. I would probably rename it
to 'fwd' to be consistent with the rest of the code.

Re: [PATCH net-next 0/9] net: bridge: vlan: broadcast fwding path optimizations

[Nikolay Aleksandrov]

On 21/09/2026 16:44, Ido Schimmel wrote:
> On Fri, Sep 18, 2026 at 06:29:41PM +0300, Nikolay Aleksandrov wrote:
>> Hi,
>> This patch-set is the first step to removing unnecessary VLAN hash lookups
>> from the bridge fast-path and also provides additional broadcast path
>> optimization by considering only the port-VLANs that actually participate
>> in the VLAN instead of all bridge ports. Combined these changes bring
>> considerable improvements[1] and open the path to do the same for the most
>> common fdb forwarding path. As expected the pathological cases have the
>> largest gain, e.g. if we have 1 client / VM facing port and 1 upstream port
>> only participating in a VLAN with many other bridge ports, we consider only
>> those 2 when broadcasting in that VLAN.
>>
>> For up to 8 ports we only use the port-VLAN list as the array doesn't make
>> much of a difference and is unnecessary. Note that the array is rebuilt on
>> every member add or delete when >8 members, I tested that and could do
>> 2000 VLAN add/delete with 64 port-VLAN members / sec on my test VM.
>> If it ever becomes a problem we can optimize it further, for the time
>> being I prefer it is simpler.
> 
> Very nice!
> 
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> 

Thank you!

> A couple of nits that you can handle later (or not):
> 
> 1. Patch #3:
> 
> __vlan_delete_pvid(vg, rtnl_dereference(vg->pvid));
> 
> Can be simplified to:
> 
> RCU_INIT_POINTER(vg->pvid, NULL);
> 
> 2. Patch #7:
> 
> Initially I thought that 'fwd.vlan' is never set, but then I realized
> that in the only place where this field is actually set it's called
> 'dst.vlan', which is a bit confusing given that in the context of the
> bridge driver 'dst' is usually an FDB entry. I would probably rename it
> to 'fwd' to be consistent with the rest of the code.

Ack, I will deal with these in a follow-up, I have another set that removes
the two egress vlan hash lookups for standard unicast fdb path and will add
these as cleanups to it.

Cheers,
  Nik

Re: [PATCH net-next 0/9] net: bridge: vlan: broadcast fwding path optimizations

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 18 Sep 2026 18:29:41 +0300 you wrote:
> Hi,
> This patch-set is the first step to removing unnecessary VLAN hash lookups
> from the bridge fast-path and also provides additional broadcast path
> optimization by considering only the port-VLANs that actually participate
> in the VLAN instead of all bridge ports. Combined these changes bring
> considerable improvements[1] and open the path to do the same for the most
> common fdb forwarding path. As expected the pathological cases have the
> largest gain, e.g. if we have 1 client / VM facing port and 1 upstream port
> only participating in a VLAN with many other bridge ports, we consider only
> those 2 when broadcasting in that VLAN.
> 
> [...]

Here is the summary with links:
  - [net-next,1/9] net: bridge: factor out common flood completion handling
    https://git.kernel.org/netdev/net-next/c/d7fddeed325d
  - [net-next,2/9] net: bridge: factor out port flooding
    https://git.kernel.org/netdev/net-next/c/25622bef8885
  - [net-next,3/9] net: bridge: vlan: cache the pvid vlan entry directly
    https://git.kernel.org/netdev/net-next/c/b82e41db0085
  - [net-next,4/9] net: bridge: vlan: introduce a list of port-VLANs in the master VLAN
    https://git.kernel.org/netdev/net-next/c/f69acdd819b0
  - [net-next,5/9] net: bridge: consider only port-VLAN members when flooding
    https://git.kernel.org/netdev/net-next/c/d5219589e4cf
  - [net-next,6/9] net: bridge: vlan: use an RCU array for large flood sets
    https://git.kernel.org/netdev/net-next/c/9c30ccaa7327
  - [net-next,7/9] net: bridge: introduce a forwarding destination structure
    https://git.kernel.org/netdev/net-next/c/911afcbaa7cd
  - [net-next,8/9] net: bridge: avoid egress VLAN lookups when flooding
    https://git.kernel.org/netdev/net-next/c/4e0c8f54f605
  - [net-next,9/9] net: bridge: avoid VLAN lookups for flood neighbour suppression
    https://git.kernel.org/netdev/net-next/c/fee4e7663e5c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html