netdev - Re: [PATCH net-next RFC v2] switchdev: bridge: drop hardware forwarded packets

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Mon, 30 Mar 2015 07:06:06 -0700
From:	roopa <roopa@...ulusnetworks.com>
To:	Scott Feldman <sfeldma@...il.com>
CC:	Florian Fainelli <f.fainelli@...il.com>,
	Guenter Roeck <linux@...ck-us.net>,
	Jiri Pirko <jiri@...nulli.us>,
	John Fastabend <john.fastabend@...il.com>,
	Andrew Lunn <andrew@...n.ch>,
	David Miller <davem@...emloft.net>,
	"Arad, Ronen" <ronen.arad@...el.com>,
	Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next RFC v2] switchdev: bridge: drop hardware forwarded
 packets

On 3/26/15, 12:44 AM, Scott Feldman wrote:
> On Wed, Mar 25, 2015 at 10:01 AM, roopa <roopa@...ulusnetworks.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.
>
> 2) port driver knows the switch_id for the port, so any pkts it sends
> up to the CPU which has already been flooded/fwded by the device are
> marked with the switch_id.  So the skb is marked, somehow.  Some
> options:
>
>    a) add a new skb switch_id field that's wrapped with
> CONFIG_NET_SWITCHDEV; seems bad, to add a new field.
>    b) put switch_id into skb->cb, but not sure how this doesn't get
> stomped on by upper drivers, or how
>        bridge knows if something valid is in there or not.  Too bad we
> don't have a TLV format for skb->cb, so
>        layers could pile things on.  But 48 bytes isn't much to play with.
>    c) squash switch_id into u32 skb->mark.  We loose information here
> and could collide between switch_ids.
>
> 3) bridge driver, in br_flood(), does check if skb switch_id mark
> matches dst port switch_id.  If so, skips fwding pkt to that port.
> The switch_id compare check compares switch_id len and contents.  If
> skb has no switch_id mark, then compare can be skipped.
>
>
> The only tough part is figuring out 2).

c) might be out of the question if userspace is using any markings and 
it may get overwritten.

> Just need someway to stuff
> switch_id into skb.  With bridge driver doing match on switch_id on a
> per-packet basis, we can support Florian's case where sometimes we
> want the bridge driver to fwd pkts (in those cases, the driver just
> leaves skb switch_id mark empty).
I have this case too and that's why i had the flag in the skb.
Agree, having switchid there will help with the overhead associated with 
looking up the switchid again.

>    Mixed offloaded and non-offloaded
> ports works because switch_id comparison fails for non-offload ports.
> Same for mixed switches bridged together.  The per-pkt overhead
> concerns are minimized.
>
Thanks for keeping this discussion going.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html