On 13/04/2022 12:51, Joachim Wiberg wrote: > On Tue, Apr 12, 2022 at 21:27, Nikolay Aleksandrov <[email protected]> > wrote: >> On 11/04/2022 16:38, Joachim Wiberg wrote: >>> @@ -526,6 +526,10 @@ void br_dev_setup(struct net_device *dev) >>> br->bridge_ageing_time = br->ageing_time = BR_DEFAULT_AGEING_TIME; >>> dev->max_mtu = ETH_MAX_MTU; >>> + br_opt_toggle(br, BROPT_UNICAST_FLOOD, 1); >> This one must be false by default. It changes current default behaviour. >> Unknown unicast is not currently passed up to the bridge if the port is >> not in promisc mode, this will change it. You'll have to make it consistent >> with promisc (e.g. one way would be for promisc always to enable unicast >> flood >> and it won't be possible to be disabled while promisc). > > Ouch, my bad! Will look into how to let this have as little impact as > possible. I like your semantics there, promisc should always win. > >>> + br_opt_toggle(br, BROPT_MCAST_FLOOD, 1); >>> + br_opt_toggle(br, BROPT_BCAST_FLOOD, 1); >> >> s/1/true/ for consistency > > Of course, thanks! > >>> @@ -118,7 +118,8 @@ int br_handle_frame_finish(struct net *net, struct sock >>> *sk, struct sk_buff *skb >>> /* by definition the broadcast is also a multicast address */ >>> if (is_broadcast_ether_addr(eth_hdr(skb)->h_dest)) { >>> pkt_type = BR_PKT_BROADCAST; >>> - local_rcv = true; >>> + if (br_opt_get(br, BROPT_BCAST_FLOOD)) >>> + local_rcv = true; >>> } else { >>> pkt_type = BR_PKT_MULTICAST; >>> if (br_multicast_rcv(&brmctx, &pmctx, vlan, skb, vid)) >>> @@ -161,12 +162,16 @@ int br_handle_frame_finish(struct net *net, struct >>> sock *sk, struct sk_buff *skb >>> } >>> mcast_hit = true; >>> } else { >>> - local_rcv = true; >>> - br->dev->stats.multicast++; >>> + if (br_opt_get(br, BROPT_MCAST_FLOOD)) { >>> + local_rcv = true; >>> + br->dev->stats.multicast++; >>> + } >>> } >>> break; >>> case BR_PKT_UNICAST: >>> dst = br_fdb_find_rcu(br, eth_hdr(skb)->h_dest, vid); >>> + if (!dst && br_opt_get(br, BROPT_UNICAST_FLOOD)) >>> + local_rcv = true; >>> break; >> >> This adds new tests for all fast paths for host traffic, especially >> the port - port communication which is the most critical one. Please >> at least move the unicast test to the "else" block of "if (dst)" >> later. > > OK, will fix! > >> The other tests can be moved to host only code too, but would require >> bigger changes. Please try to keep the impact on the fast-path at >> minimum. > > Interesting, you mean by speculatively setting local_rcv = true and > postpone the decsion to br_pass_frame_up(), right? Yeah that would > indeed be a bit more work.
Yes, I was thinking maybe local_rcv can become an enum with an exact reason for the local_rcv, so if it's > 0 do the local_rcv and br_pass_frame_up() then can make a proper decision without passing all of the vars. I haven't tried it, so not sure if it's feasible. :)
