From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id DEB32C433EF for ; Thu, 17 Mar 2022 12:26:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230134AbiCQM15 (ORCPT ); Thu, 17 Mar 2022 08:27:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50228 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233599AbiCQM1y (ORCPT ); Thu, 17 Mar 2022 08:27:54 -0400 Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com [66.111.4.29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 911611CABF5 for ; Thu, 17 Mar 2022 05:26:38 -0700 (PDT) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.nyi.internal (Postfix) with ESMTP id 29F015C0195; Thu, 17 Mar 2022 08:26:36 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute3.internal (MEProxy); Thu, 17 Mar 2022 08:26:36 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm3; bh=E5F1iZE2erkYDbr1G AMKhk6q/sgDL/5bHJDCFXXFM88=; b=REDM8jq8awSrZCjEVt2nubSa9KvZs+tmq QuCLY29ktIL7p4J6V0wH7UylriphtFQNtNDVtSGVy4g7APKF6MZQO3BFJW41FEja GKEhNkXtIjgWaVl+u+QLrk7llWWMRfFxdFrX+vO4msZUOsU7ZJBxXizKfVt95Tu/ GfRGPoUj/mWpVMt9MjfrrWKQbZcRR7Mq/Z60wuyrzwdERanOH9BcIcl5kI5SmIIB vnswVIViLpoP4O5bxDHFaNOYUmverLctIpuh8SZRu1eEJQ2WUAlTygCc0qfgFbB8 aWkcwXq94lMf1SBJUeHght2km4vC0phRM2XpXwm/zLPfuSnuL4akw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddrudefgedgfeelucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehttdertddttddvnecuhfhrohhmpefkughoucfu tghhihhmmhgvlhcuoehiughoshgthhesihguohhstghhrdhorhhgqeenucggtffrrghtth gvrhhnpedtffekkeefudffveegueejffejhfetgfeuuefgvedtieehudeuueekhfduheel teenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehiug hoshgthhesihguohhstghhrdhorhhg X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 17 Mar 2022 08:26:35 -0400 (EDT) Date: Thu, 17 Mar 2022 14:26:31 +0200 From: Ido Schimmel To: Nikolay Aleksandrov Cc: Mattias Forsblad , Joachim Wiberg , netdev@vger.kernel.org, "David S . Miller" , Jakub Kicinski , Andrew Lunn , Florian Fainelli , Vivien Didelot , Roopa Prabhu , Tobias Waldekranz , Vladimir Oltean Subject: Re: [PATCH 2/5] net: bridge: Implement bridge flood flag Message-ID: References: <20220317065031.3830481-1-mattias.forsblad@gmail.com> <20220317065031.3830481-3-mattias.forsblad@gmail.com> <87r1717xrz.fsf@gmail.com> <50f4e8b0-4eea-d202-383b-bf2c2824322d@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Thu, Mar 17, 2022 at 01:42:55PM +0200, Nikolay Aleksandrov wrote: > On 17/03/2022 13:39, Mattias Forsblad wrote: > > On 2022-03-17 10:07, Joachim Wiberg wrote: > >> On Thu, Mar 17, 2022 at 07:50, Mattias Forsblad wrote: > >>> This patch implements the bridge flood flags. There are three different > >>> flags matching unicast, multicast and broadcast. When the corresponding > >>> flag is cleared packets received on bridge ports will not be flooded > >>> towards the bridge. > >> > >> If I've not completely misunderstood things, I believe the flood and > >> mcast_flood flags operate on unknown unicast and multicast. With that > >> in mind I think the hot path in br_input.c needs a bit more eyes. I'll > >> add my own comments below. > >> > >> Happy incident I saw this patch set, I have a very similar one for these > >> flags to the bridge itself, with the intent to improve handling of all > >> classes of multicast to/from the bridge itself. > >> > >>> [snip] > >>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c > >>> index e0c13fcc50ed..fcb0757bfdcc 100644 > >>> --- a/net/bridge/br_input.c > >>> +++ b/net/bridge/br_input.c > >>> @@ -109,11 +109,12 @@ 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; > >>> + local_rcv = true && br_opt_get(br, BROPT_BCAST_FLOOD); > >> > >> Minor comment, I believe the preferred style is more like this: > >> > >> 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)) > >>> - goto drop; > >>> + if (br_opt_get(br, BROPT_MCAST_FLOOD)) > >>> + if (br_multicast_rcv(&brmctx, &pmctx, vlan, skb, vid)) > >>> + goto drop; > >> > >> Since the BROPT_MCAST_FLOOD flag should only control uknown multicast, > >> we cannot bypass the call to br_multicast_rcv(), which helps with the > >> classifcation. E.g., we want IGMP/MLD reports to be forwarded to all > >> router ports, while the mdb lookup (below) is what an tell us if we > >> have uknown multicast and there we can check the BROPT_MCAST_FLOOD > >> flag for the bridge itself. > > > > The original flag was name was local_receive to separate it from being > > mistaken for the flood unknown flags. However the comment I've got was > > to align it with the existing (port) flags. These flags have nothing to do with > > the port flood unknown flags. Imagine the setup below: > > > > vlan1 > > | > > br0 br1 > > / \ / \ > > swp1 swp2 swp3 swp4 > > > > We want to have swp1/2 as member of a normal vlan filtering bridge br0 /w learning on. > > On br1 we want to just forward packets between swp3/4 and disable learning. > > Additional we don't want this traffic to impact the CPU. > > If we disable learning on swp3/4 all traffic will be unknown and if we also > > have flood unknown on the CPU-port because of requirements for br0 it will > > impact the traffic to br1. Thus we want to restrict traffic between swp3/4<->CPU port > > with the help of the PVT. > > > > /Mattias > > The feedback was correct and we all assumed unknown traffic control. > If you don't want any local receive then use filtering rules. Don't add unnecessary flags. Yep. Very easy with tc: # tc qdisc add dev br1 clsact # tc filter add dev br1 ingress pref 1 proto all matchall action drop This can be fully implemented inside the relevant device driver, no changes needed in the bridge driver.