Date:	Thu, 4 Dec 2014 12:49:26 -0800
From:	Scott Feldman <sfeldma@...il.com>
To:	Roopa Prabhu <roopa@...ulusnetworks.com>
Cc:	Jiri Pirko <jiri@...nulli.us>, Netdev <netdev@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>,
	"nhorman@...driver.com" <nhorman@...driver.com>,
	Andy Gospodarek <andy@...yhouse.net>,
	Thomas Graf <tgraf@...g.ch>,
	"dborkman@...hat.com" <dborkman@...hat.com>,
	"ogerlitz@...lanox.com" <ogerlitz@...lanox.com>,
	"jesse@...ira.com" <jesse@...ira.com>,
	"pshelar@...ira.com" <pshelar@...ira.com>,
	"azhou@...ira.com" <azhou@...ira.com>,
	"ben@...adent.org.uk" <ben@...adent.org.uk>,
	"stephen@...workplumber.org" <stephen@...workplumber.org>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"vyasevic@...hat.com" <vyasevic@...hat.com>,
	Cong Wang <xiyou.wangcong@...il.com>,
	"Fastabend, John R" <john.r.fastabend@...el.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Jamal Hadi Salim <jhs@...atatu.com>,
	Florian Fainelli <f.fainelli@...il.com>,
	John Linville <linville@...driver.com>,
	"jasowang@...hat.com" <jasowang@...hat.com>,
	"ebiederm@...ssion.com" <ebiederm@...ssion.com>,
	Nicolas Dichtel <nicolas.dichtel@...nd.com>,
	"ryazanov.s.a@...il.com" <ryazanov.s.a@...il.com>,
	"buytenh@...tstofly.org" <buytenh@...tstofly.org>,
	Aviad Raveh <aviadr@...lanox.com>,
	"nbd@...nwrt.org" <nbd@...nwrt.org>,
	Alexei Starovoitov <alexei.starovoitov@...il.com>,
	Neil Jerram <Neil.Jerram@...aswitch.com>,
	"ronye@...lanox.com" <ronye@...lanox.com>,
	"simon.horman@...ronome.com" <simon.horman@...ronome.com>,
	"alexander.h.duyck@...hat.com" <alexander.h.duyck@...hat.com>,
	"Ronciak, John" <john.ronciak@...el.com>,
	"mleitner@...hat.com" <mleitner@...hat.com>,
	Shrijeet Mukherjee <shrijeet@...il.com>,
	Andy Gospodarek <gospo@...ulusnetworks.com>,
	Benjamin LaHaise <bcrl@...ck.org>,
	Hemal Shah <hemal@...adcom.com>
Subject: Re: [patch iproute2 0/6] iproute2: add changes for switchdev

On Thu, Dec 4, 2014 at 8:55 AM, Roopa Prabhu <roopa@...ulusnetworks.com> wrote:
> On 12/4/14, 8:04 AM, Jiri Pirko wrote:
>>
>> Thu, Dec 04, 2014 at 03:45:44PM CET, roopa@...ulusnetworks.com wrote:
>>>
>>> On 12/4/14, 6:34 AM, Jiri Pirko wrote:
>>>>
>>>> Thu, Dec 04, 2014 at 03:26:50PM CET, roopa@...ulusnetworks.com wrote:
>>>>>
>>>>> On 12/4/14, 12:57 AM, Jiri Pirko wrote:
>>>>>>
>>>>>> Jiri Pirko (1):
>>>>>>    iproute2: ipa: show switch id
>>>>>>
>>>>>> Scott Feldman (5):
>>>>>>    bridge/fdb: fix statistics output spacing
>>>>>>    bridge/fdb: add flag/indication for FDB entry synced from offload
>>>>>>      device
>>>>>>    bridge/link: add new offload hwmode swdev
>>>>>
>>>>> Ack to most patches but nack on this one. The todo list still has a
>>>>> note to
>>>>> revist the flag to indicate switchdev offloads.
>>>>> Exposing this to userspace does not help that.
>>>>
>>>> Hmm, note that this is already exposed to userspace, this patchset is
>>>> for iproute2 (userspace tool).
>>>
>>> hmmm, all feedback on the switchdev patches seemed to indicate we can
>>> change
>>> this later.
>>> I don't see swdev mode being used in the kernel anywhere today.
>>
>> Well, it is, in rocker:
>> $ git grep BRIDGE_MODE_SWDEV
>> drivers/net/ethernet/rocker/rocker.c:                   if (mode !=
>> BRIDGE_MODE_SWDEV)
>> drivers/net/ethernet/rocker/rocker.c:   u16 mode = BRIDGE_MODE_SWDEV;
>> include/uapi/linux/if_bridge.h:#define BRIDGE_MODE_SWDEV        2       /*
>> Full switch device offload */
>
>
> The problem is rocker is not the only one who is going to be using this. And
> so, we need something that fits everybody.
> And i am not going to make my user set a mode for him to enable offload to
> hw.
>
>>
>>> I will send a patch to remove it. Its still in net-next and so can be
>>> changed
>>> ?.
>>> I was going to resend my patch to introduce a common offload flag for all
>>> link objects.
>>> It would be nice if all of them had a consistent flag to indicate hw
>>> offload
>>> and iproute2 could display the same flag for all.
>>> Including bonds and vxlan's.
>>
>> I do not understand the connection with BRIDGE_MODE_SWDEV. We discussed
>> this already. BRIDGE_MODE_SWDEV is a bridge mode, similar to for example
>> BRIDGE_MODE_VEPA and makes perfect sense to have it.
>
> I dont think everybody acked it. But it went in with a note saying that it
> can be changed.

I thought that was the plan: this new mode goes in now for net-next
and iproute2, and you would supply follow up patch for each to move to
your switch port flag.  That will give us time to review your work
without have net-next and iproute2 out-of-sync.

>>
>>
>> How vxlan and bonds come into the mixture, that is a puzzler for me.
>> Maybe I have to see patches.
>
>
> I had posted a version of the patch previously:
> http://www.spinics.net/lists/netdev/msg305472.html
>
> I have a v2 patch in my stack which does not touch the netlink header.
> But in the past hour, i have been thinking about it some more. Do we really
> need this set by the user ?. In my use case i don't need it.

Look at how iproute2 figures out if SELF should be set or not.  It's
only set if hwmode is set, otherwise it defaults to MASTER.  So with
SWDEV a new hwmode, we can push settings (learning, leraning_sync) to
port with SELF set.  It's probably not an ideal arrangement having to
set hwmode each time, but this was the low-touch change to iproute2 to
push port settings.

I'm hoping your new patch will kind of straighten this all out.  But
you've got extra work to make sure backward compat with older iproute2
still works, including this weirdness around hwmode.

>
> We do need a feature flag (or net_device_flags), but it does not need to be
> set by the user explicitly.
> This flag can be set by the switch port driver on the switch ports. And the
> logical device: bridge/bond/vxlan
> can inherit it from the port. There was a need of a flag in some usecases,
> to control offloading of specific bridge port flags
> to hw/sw (example learning in hw or sw). example patch:
> https://patchwork.ozlabs.org/patch/413211/
>
> I will post something today.

Can you include matching iproute2 changes?  (Assuming you'll building
on top of what's already in net-next and this iproute2 set Jiri sent
out).  It's helpful to see the iproute2 changes to see what the new
cmd structure is and how legacy is handled.

-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