From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Horman Subject: Re: [PATCH net-next RFC v2] switchdev: bridge: drop hardware forwarded packets Date: Fri, 27 Mar 2015 10:08:23 +0900 Message-ID: <20150327010821.GA10514@vergenet.net> References: <5511A29F.5010006@cumulusnetworks.com> <20150324175825.GA1465@roeck-us.net> <5512E9EC.5020504@cumulusnetworks.com> <20150326082011.GA2010@nanopsycho.orion> <20150326144922.GC2010@nanopsycho.orion> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Scott Feldman , roopa , Florian Fainelli , Guenter Roeck , John Fastabend , Andrew Lunn , David Miller , "Arad, Ronen" , Netdev To: Jiri Pirko Return-path: Received: from mail-pa0-f52.google.com ([209.85.220.52]:35023 "EHLO mail-pa0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752530AbbC0BIg (ORCPT ); Thu, 26 Mar 2015 21:08:36 -0400 Received: by pacwz10 with SMTP id wz10so28408316pac.2 for ; Thu, 26 Mar 2015 18:08:35 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20150326144922.GC2010@nanopsycho.orion> Sender: netdev-owner@vger.kernel.org List-ID: On Thu, Mar 26, 2015 at 03:49:22PM +0100, Jiri Pirko wrote: > Thu, Mar 26, 2015 at 03:28:28PM CET, sfeldma@gmail.com wrote: > >On Thu, Mar 26, 2015 at 1:20 AM, Jiri Pirko wrote: > >> Thu, Mar 26, 2015 at 08:44:27AM CET, sfeldma@gmail.com wrote: > >>>On Wed, Mar 25, 2015 at 10:01 AM, roopa wrote: > >>> > >>>[cut] > >>> > >>>So just to keep the discussion alive (because we really need to solve > >>>this problem), my current thinking is back to Roopa's RFC patch to > >>>mark the skb to avoid fwding in bridge driver. One idea (sorry if > >>>this was already suggested, thread is long) is to use > >>>swdev_parent_id_get op in the following way: > >>> > >>>1) when port interface is added to bridge, bridge calls > >>>swdev_parent_id_get() on port to get switch id. > >>>swdev_parent_id_get() needs to be modified to work on stacked drivers. > >>>For example, if a bond is the new bridge port, swdev_parent_id_get() > >>>on the bond interface should get switch_id for bond member. We stash > >>>the switch_id in the bridge port private structure for later > >>>comparison. > >> > >> Nope, that cannot work. You can bond 2 ports each belonging to a > >> different switch. > > > >Are you thinking about two switch ASICs in the same box, and bonding > >ports from each? Or are you thinking about bonding ports from > >different boxes, ala MLAG? > > One machine, 2 switches. > > > > >In the first case the bond would report NULL switch_id if the member > >ports don't all have the same switch_id. If bond switch_id is NULL, > >the bridge driver would fwd pkts to bond and now bond would make same > >check as bridge: if dst port switch_id is same as skb switch_id, then > >drop pkt. In bridge, if bond switch_id is non-NULL and matches skb > >switch_id, then drop pkt. So it works as desired for this case. It > >requires the bonding/teaming driver to modify the default behavior for > >swdev_parent_id_get() to only return switch_id if all ports agree on > >switch_id. > > > >For second case using MLAG, I suspect bond member port switch_ids > >would likely be different, and so with same logic in bonding/bridge > >drivers as above in first case, the pkt would be fwded down. > > > >Is there another case to consider? I think converting > >swdev_parent_id_get() to use same algo we have for stp, allowing for > >any layer to override like in my bonding example, will have benefits > >down the road. > > > >What is the argument for not allowing stacked version of swdev_parent_id_get()? > > That was suppose wo identify a switch port. "ip link" will show you that > and you see right away what is going on. If bond implements that, that > brigs a mess. I don't like that. I'm not sure that I follow how this messes things up from a bridging point of view. Would it help if bonds consistently returned a NULL parent id even if all its slaves have the same parent id?