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 <[email protected]> --- 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
