lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <87k0bt9uq9.fsf@gmail.com> Date: Wed, 13 Apr 2022 11:51:42 +0200 From: Joachim Wiberg <troglobit@...il.com> To: Nikolay Aleksandrov <razor@...ckwall.org>, Roopa Prabhu <roopa@...dia.com> Cc: netdev@...r.kernel.org, bridge@...ts.linux-foundation.org, "David S . Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Tobias Waldekranz <tobias@...dekranz.com>, Vladimir Oltean <vladimir.oltean@....com> Subject: Re: [PATCH RFC net-next 01/13] net: bridge: add control of bum flooding to bridge itself On Tue, Apr 12, 2022 at 21:27, Nikolay Aleksandrov <razor@...ckwall.org> 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.
Powered by blists - more mailing lists