From mboxrd@z Thu Jan 1 00:00:00 1970 From: roopa Subject: Re: [PATCH net-next RFC v2] switchdev: bridge: drop hardware forwarded packets Date: Fri, 20 Mar 2015 15:06:21 -0700 Message-ID: <550C99DD.3090007@cumulusnetworks.com> References: <1426870714-3225-1-git-send-email-roopa@cumulusnetworks.com> <550C54BC.4040509@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: John Fastabend , "David S. Miller" , =?UTF-8?B?SmnFmcOtIFDDrXJrbw==?= , "Arad, Ronen" , Netdev To: Scott Feldman Return-path: Received: from mail-pd0-f172.google.com ([209.85.192.172]:34441 "EHLO mail-pd0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751090AbbCTWGX (ORCPT ); Fri, 20 Mar 2015 18:06:23 -0400 Received: by pdbni2 with SMTP id ni2so120527800pdb.1 for ; Fri, 20 Mar 2015 15:06:22 -0700 (PDT) In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On 3/20/15, 11:13 AM, Scott Feldman wrote: > On Fri, Mar 20, 2015 at 10:11 AM, John Fastabend > wrote: >> On 03/20/2015 09:58 AM, roopa@cumulusnetworks.com wrote: >>> From: Roopa Prabhu >>> >>> On a Linux bridge with bridge forwarding offloaded to switch ASIC, >>> there is a need to not re-forward frames that have already been >>> forwarded in hardware. >>> >>> Typically these are broadcast or multicast frames forwarded by the >>> hardware to multiple destination ports including sending a copy of >>> the packet to the cpu (kernel e.g. an arp broadcast). >>> The bridge driver will try to forward the packet again, resulting in >>> two copies of the same packet. >>> >>> These packets can also come up to the kernel for logging when they hit >>> a LOG acl rule in hardware. In such cases, you do want the packet >>> to go through the bridge netfilter hooks. Hence, this patch adds the >>> required checks just before the packet is being xmited. >>> >>> v2: >>> - Add a new hw_fwded flag in skbuff to indicate that the packet >>> is already hardware forwarded. Switch driver will set this flag. >>> I have been trying to avoid having this flag in the skb >>> and thats why this patch has been in my tree for long. Cant think >>> of other better alternatives. Suggestions are welcome. I have put >>> this under CONFIG_NET_SWITCHDEV to minimize the impact. >>> >>> Signed-off-by: Roopa Prabhu >>> Signed-off-by: Wilson Kok >>> --- >> Interesting. I completely avoid this problem by not instantiating a >> software bridge ;) When these pkts come up the stack I either use a >> raw socket to capture them, put a 'tc' ingress rule to do something, >> or have OVS handle them in some special way. It seems to me that this >> is where the sw/hw model starts to break when you have these magic >> bits to handle the packets differently. >> >> How do you know to set the skb bit? Do you have some indicator in the >> descriptor? I don't have any good way to learn this on my hardware. But >> I can assume if it reached the CPU it was because of some explicit rule. > I was wondering that also, since there was no example. > > This features seems like it belongs in the bridge. yes, it does, the check today is really in the bridge. > We already have > BR_FLOOD to indicate whether unknown unicast traffic is flooded to a > bridge port. Can we add another BR_FLOOD_BCAST (or some name) for > this new feature? You would set/clear this flag on the bridge > (master) port. The default is set. And now: > > - #define BR_AUTO_MASK (BR_FLOOD | BR_LEARNING) > + #define BR_AUTO_MASK (BR_FLOOD | BR_FLOOD_BCAST | BR_LEARNING) > > Does this work for your use-case, Roopa? Note my first RFC patch, sort of did this: https://marc.info/?l=linux-netdev&m=142147999420017&w=2 But there are open things there as listed in the comment and also in the subsequent discussion on the thread. We discussed this flag before and i think it does not allow the case where hw switch ports are bridged with non-hw ports.