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

[Nikolay Aleksandrov <razor@blackwall.org>]

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