Message-ID: <20141003075318.GA2814@nanopsycho.orion>
Date:	Fri, 3 Oct 2014 09:53:18 +0200
From:	Jiri Pirko <jiri@...nulli.us>
To:	Scott Feldman <sfeldma@...ulusnetworks.com>
Cc:	Florian Fainelli <f.fainelli@...il.com>, netdev@...r.kernel.org,
	davem@...emloft.net, stephen@...workplumber.org,
	andy@...yhouse.net, tgraf@...g.ch, nbd@...nwrt.org,
	john.r.fastabend@...el.com, edumazet@...gle.com,
	vyasevic@...hat.com, buytenh@...tstofly.org
Subject: Re: HW bridging support using notifiers?

Fri, Oct 03, 2014 at 07:13:35AM CEST, sfeldma@...ulusnetworks.com wrote:
>
>On Oct 2, 2014, at 6:48 PM, Florian Fainelli <f.fainelli@...il.com> wrote:
>
>> Hi all,
>> 
>> I am taking a look at adding HW bridging support to DSA, in way that's
>> usable outside of DSA.
>> 
>> Lennert's approach in 2008 [1] looks conceptually good to me,as he
>> noted, it uses a bunch of new ndo's which is not only limiting to one
>> ndo implementer per struct net_device, but also is mostly consuming the
>> information from the bridge layer, while the ndo is an action
>> 
>> So here's what I am up to now:
>> 
>> - use the NETDEV_JOIN notifier to discover when a bridge port is added
>> - use the NETDEV_LEAVE notifier, still need to verify this does not
>> break netconsole as indicated in net/bridge/br_if.c
>
>I can’t find a NETDEV_LEAVE...is this new?
>
>For this, rocker is using netdev_notifier and watching for NETDEV_CHANGEUPPER.  On CHANGEUPPER, use netdev_master_upper_dev_get(dev) to get master.  If master and master rtnl_link_ops->kind is “bridge”, then dev port is in bridge; otherwise port is not in bridge.  Of course, master could be “openvswitch”, if the driver swings both ways.

I agree that NETDEV_CHANGEUPPER should be used for this.

>
>Would this approach work for DSA?
>
>> - use the NETDEV_CHANGEINFODATA notifier to notify about STP state changes
>> 
>> Now, this raises a bunch of questions:
>> 
>> - we would need a getter to return the stp state of a given network
>> device when called with NETDEV_CHANGEINFODATA, is that acceptable? This
>> would be the first function exported by the bridge layer to expose
>> internal data
>> 
>> NB: this also raises the question of the race condition and locking
>> within br_set_stp_state() and when the network devices notifier callback
>> runs
>> 
>> - or do we need a new network device notifier accepting an opaque
>> pointer which could provide us with the data we what, something like
>> this: call_netdevices_notifier_data(NETDEV_CHANGEINFODATA, dev, info),
>> where info would be a structure/union telling what's this data about
>> 
>
>We need STP state change notification for rocker also, so we can install/remove STP/ARP filters on port state change.  Netdev_notifier would work.  I was also thinking about using Jiri’s ndo_swdev_flow_install/remove to install/remove the STP filters on the port, rather than using netdev_notifier.  In other words, does the HW bridge driver need to know the STP state or can it be dumb (stateless) and told when to accept STP BPDUs, or not, using swdev_flow construct:
>
>  dst_mac 01:80:c2:00:00:00 lasp 0x4242 in_port <port ifindex> actions output <br ifindex>
>
>and later when reaching LEARNING state:
>
>  dst_mac ff:ff:ff:ff:ff:ff eth_type ARP in_port <port ifindex> actions output <br ifindex>
>
>and finally when reaching FORWARDING state, the learned/static bridge fdbs:
>
>  dst_mac <neigh_mac> in_port <br ifindex> actions output <port ifindex>
>
>So driver doesn’t really know what STP is; it’s just installs/removes port filter when told to, using the common ndo_swdev_flow API.  The smarts stay in the bridge driver.
>
>
>> Let me know what you think, thanks!
>> 
>> [1]: http://patchwork.ozlabs.org/patch/16578/
>> --
>> Florian
>
>
>-scott
>
>
>
--
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

Powered by blists - more mailing lists