Message-Id: <A97C2083-CFD9-4238-9C43-97EF43200124@cumulusnetworks.com>
Date:	Thu, 2 Oct 2014 22:13:35 -0700
From:	Scott Feldman <sfeldma@...ulusnetworks.com>
To:	Florian Fainelli <f.fainelli@...il.com>
Cc:	netdev@...r.kernel.org, davem@...emloft.net, jiri@...nulli.us,
	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?


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.

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