Date:	Mon, 10 Nov 2014 12:30:58 -0500
From:	Andy Gospodarek <gospo@...ulusnetworks.com>
To:	Thomas Graf <tgraf@...g.ch>
Cc:	Jiri Pirko <jiri@...nulli.us>, Jamal Hadi Salim <jhs@...atatu.com>,
	netdev@...r.kernel.org, davem@...emloft.net, nhorman@...driver.com,
	andy@...yhouse.net, dborkman@...hat.com, ogerlitz@...lanox.com,
	jesse@...ira.com, pshelar@...ira.com, azhou@...ira.com,
	ben@...adent.org.uk, stephen@...workplumber.org,
	jeffrey.t.kirsher@...el.com, vyasevic@...hat.com,
	xiyou.wangcong@...il.com, john.r.fastabend@...el.com,
	edumazet@...gle.com, sfeldma@...il.com, f.fainelli@...il.com,
	roopa@...ulusnetworks.com, linville@...driver.com,
	jasowang@...hat.com, ebiederm@...ssion.com,
	nicolas.dichtel@...nd.com, ryazanov.s.a@...il.com,
	buytenh@...tstofly.org, aviadr@...lanox.com, nbd@...nwrt.org,
	alexei.starovoitov@...il.com, Neil.Jerram@...aswitch.com,
	ronye@...lanox.com, simon.horman@...ronome.com,
	alexander.h.duyck@...hat.com, john.ronciak@...el.com,
	mleitner@...hat.com, shrijeet@...il.com, bcrl@...ck.org
Subject: Re: [patch net-next v2 06/10] bridge: introduce fdb offloading via
 switchdev

On Mon, Nov 10, 2014 at 01:51:00PM +0000, Thomas Graf wrote:
> On 11/10/14 at 09:15am, Jiri Pirko wrote:
> > There are few problems in re-using this. It is netlink based so for calling
> > it from bridge code, we would have to construct netlink message. But
> > that could be probably changed.
> > As you can see from the list of parameters, this is no longer about fdb (addr,
> > vlanid) but this has been extended to something else. See vxlan code for
> > what this is used for. I believe that fdb_add/del should be renamed to
> > something else, perhaps l2neigh_add/del or something like that.
> > The other problem is that fdb_add/del is currently used by various
> > drivers for different purpose (adding macs to unicast list).
> 
> Can you elaborate a bit on the intended semantic differences between
> the existing ndo_fdb_add() and ndo_sw_port_fdb_add()? I'm not sure we
> need the sw_ prefix for this specific ndo.
> 
> I completely agree that relying on Netlink is wrong because we'll have
> in-kernel users of the API but I believe that existing ndo_fdb_add()
> implementations in i40e, ixgbe, qlcnic and macvlan could use the new
> API you propose.
I also think the same API could be used quite easily on the current
drivers that use it.

I was looking at this earlier today and there are only 5 drivers
(outside the bridge code) that support ndo_fdb_add.  The 3 hardware
drivers and vxlan driver seem like they could use this new API.  The
macvlan code appears to simply set the uc and mc lists, which seems like
it could be done other ways -- confirmation from John Fastabend, Roopa,
and mst would be good.

> How about we rename the existing ndo_fdb_add() to ndo_neigh_add() as
> you propose and convert vxlan over to it and have all others which don't
> even depend on the Netlink attributes being passed in (i40e, ixgbe,
> qlcnic, macvlan) use ndo_fdb_add() which would have the behaviour of your
> proposed ndo_sw_port_fdb_add()?
I would much rather see something like Thomas proposes here.  I know you
would like to see these patches get included (I'm anxious to see better
in-kernel offload support too!), but separate, possibly unnecessary
APIs like this can get painful for driver maintainers (upstream and
distro maintainers).

--
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