On Tue, Apr 12, 2022 at 21:27, Nikolay Aleksandrov <razor@xxxxxxxxxxxxx> 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.
