Message-ID: <54610C0D.1050500@cumulusnetworks.com>
Date:	Mon, 10 Nov 2014 11:03:41 -0800
From:	Roopa Prabhu <roopa@...ulusnetworks.com>
To:	Andy Gospodarek <gospo@...ulusnetworks.com>
CC:	Thomas Graf <tgraf@...g.ch>, 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, 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 11/10/14, 9:30 AM, Andy Gospodarek wrote:
> 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.
yes, that is correct. The macvlan code, when not set for passthru mode, 
seems to just program the uc-mc lists,
  which again get synched to the lowerdev (and possibly from lowerdev to 
hw in some cases).

I agree that it would be really nice if the existing api's can be made 
to work.
>> 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).
>
Ack!.
--
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