public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Simon Horman <simon.horman@netronome•com>
To: Jiri Pirko <jiri@resnulli•us>
Cc: Scott Feldman <sfeldma@gmail•com>,
	roopa <roopa@cumulusnetworks•com>,
	Florian Fainelli <f.fainelli@gmail•com>,
	Guenter Roeck <linux@roeck-us•net>,
	John Fastabend <john.fastabend@gmail•com>,
	Andrew Lunn <andrew@lunn•ch>, David Miller <davem@davemloft•net>,
	"Arad, Ronen" <ronen.arad@intel•com>,
	Netdev <netdev@vger•kernel.org>
Subject: Re: [PATCH net-next RFC v2] switchdev: bridge: drop hardware forwarded packets
Date: Fri, 27 Mar 2015 10:08:23 +0900	[thread overview]
Message-ID: <20150327010821.GA10514@vergenet.net> (raw)
In-Reply-To: <20150326144922.GC2010@nanopsycho.orion>

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 <jiri@resnulli•us> wrote:
> >> Thu, Mar 26, 2015 at 08:44:27AM CET, sfeldma@gmail•com wrote:
> >>>On Wed, Mar 25, 2015 at 10:01 AM, roopa <roopa@cumulusnetworks•com> 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?

  reply	other threads:[~2015-03-27  1:08 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-20 16:58 [PATCH net-next RFC v2] switchdev: bridge: drop hardware forwarded packets roopa
2015-03-20 17:11 ` John Fastabend
2015-03-20 18:13   ` Scott Feldman
2015-03-20 18:30     ` John Fastabend
2015-03-20 22:06     ` roopa
2015-03-20 22:37       ` Scott Feldman
2015-03-20 23:30         ` roopa
2015-03-21  0:26           ` Scott Feldman
2015-03-21  5:53             ` roopa
2015-03-20 21:03   ` roopa
2015-03-20 21:23     ` John Fastabend
2015-03-20 22:04       ` Andrew Lunn
2015-03-20 23:12       ` roopa
2015-03-20 18:03 ` Scott Feldman
2015-03-20 21:20   ` roopa
2015-03-20 20:36 ` David Miller
2015-03-20 21:36   ` roopa
2015-03-20 22:09     ` Andrew Lunn
2015-03-20 23:43       ` Florian Fainelli
2015-03-23  0:22       ` Guenter Roeck
2015-03-23  1:33         ` John Fastabend
2015-03-23  2:57           ` Guenter Roeck
2015-03-23  3:18             ` John Fastabend
2015-03-23  3:33               ` Guenter Roeck
2015-03-23 17:12                 ` roopa
2015-03-24  5:59                   ` Scott Feldman
2015-03-24 13:13                     ` Guenter Roeck
2015-03-24 18:08                       ` Scott Feldman
2015-03-24 14:29                     ` Jiri Pirko
2015-03-24 16:01                       ` Guenter Roeck
2015-03-24 17:45                         ` roopa
2015-03-24 17:58                           ` Guenter Roeck
2015-03-24 18:14                             ` Scott Feldman
2015-03-25  3:10                               ` Guenter Roeck
2015-03-25  3:46                               ` Florian Fainelli
2015-03-25  5:06                                 ` Scott Feldman
2015-03-25 17:01                                   ` roopa
2015-03-26  7:44                                     ` Scott Feldman
2015-03-26  8:20                                       ` Jiri Pirko
2015-03-26 14:28                                         ` Scott Feldman
2015-03-26 14:49                                           ` Jiri Pirko
2015-03-27  1:08                                             ` Simon Horman [this message]
2015-03-27  6:02                                               ` Jiri Pirko
2015-03-27  6:43                                             ` Scott Feldman
2015-03-27  7:01                                               ` Jiri Pirko
2015-03-27 23:19                                                 ` Scott Feldman
2015-03-30 14:06                                       ` roopa
2015-03-24 18:48                             ` David Christensen
2015-03-24 17:58                         ` Scott Feldman
2015-03-23 17:10           ` roopa
2015-03-23 14:00         ` roopa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150327010821.GA10514@vergenet.net \
    --to=simon.horman@netronome$(echo .)com \
    --cc=andrew@lunn$(echo .)ch \
    --cc=davem@davemloft$(echo .)net \
    --cc=f.fainelli@gmail$(echo .)com \
    --cc=jiri@resnulli$(echo .)us \
    --cc=john.fastabend@gmail$(echo .)com \
    --cc=linux@roeck-us$(echo .)net \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=ronen.arad@intel$(echo .)com \
    --cc=roopa@cumulusnetworks$(echo .)com \
    --cc=sfeldma@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox