Date:	Wed, 12 Nov 2014 14:43:21 +0100
From:	Jiri Pirko <jiri@...nulli.us>
To:	Roopa Prabhu <roopa@...ulusnetworks.com>
Cc:	Andy Gospodarek <gospo@...ulusnetworks.com>,
	Thomas Graf <tgraf@...g.ch>,
	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

Mon, Nov 10, 2014 at 08:03:41PM CET, roopa@...ulusnetworks.com wrote:
>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,

If you look at how drivers implement this, they also only add addr into
uc/mc list. And if the ndo fdb add is not implemented by the driver, core
does it for you - see ndo_dflt_fdb_add.



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


I will think about this more and prepare something like Thomas proposes.
Stay tuned, I will be out undil Monday so I will post v3 Tue/Wed next
week.

In the meantime, feel free to study the fdb_add code and feel free to
give me some more thoughts/patches about this. Thanks!


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