Message-ID: <547346EB.7060302@cumulusnetworks.com>
Date:	Mon, 24 Nov 2014 06:55:39 -0800
From:	Roopa Prabhu <roopa@...ulusnetworks.com>
To:	Scott Feldman <sfeldma@...il.com>
CC:	Jiří Pírko <jiri@...nulli.us>,
	Jamal Hadi Salim <jhs@...atatu.com>,
	Benjamin LaHaise <bcrl@...ck.org>, Thomas Graf <tgraf@...g.ch>,
	john.fastabend@...il.com, stephen@...workplumber.org,
	John Linville <linville@...driver.com>, nhorman@...driver.com,
	Nicolas Dichtel <nicolas.dichtel@...nd.com>,
	vyasevic@...hat.com, Florian Fainelli <f.fainelli@...il.com>,
	buytenh@...tstofly.org, Aviad Raveh <aviadr@...lanox.com>,
	Netdev <netdev@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>,
	shrijeet@...ulusnetworks.com,
	Andy Gospodarek <gospo@...ulusnetworks.com>
Subject: Re: [RFC PATCH 0/4] switch device: offload policy attributes

On 11/24/14, 2:18 AM, Scott Feldman wrote:
> Hi Roopa,
>
> I have a patch pending against Jiri's v2 that's uses existing
> ndo_bridge_setlink/getlink to push policy settings down to port driver
> for controlling HW offload.  I had to make a few tweaks, but for the
> most part setlink/getlink already has the master/self semantics so
> users can set policy flags on bridge's SW version of the port (master)
> or on the offloaded version of the port (self).
>    I added the new
> hwmode option "swdev" to the existing "vepa"|"veb" choices.  When you
> specify hwmode, SELF is set and the port driver's setlink get's
> called.  Did you look at setlink/getlink?  It looks like the kernel
> and iproute2 where going down this route of using setlink/getlink for
> SELF policy, so I'm wondering if we need more?
If i understand you correctly, this will mean the port driver implements 
the
setlink/getlink with the  bridge port flags and attributes. And, it also 
means
the port driver will parse the netlink msg instead of the bridge driver
parsing all attributes and then calling offloads.

But, in cases where we want bridge port flags and attributes set both in 
hw and sw,
the user (iproute2) will have to set both MASTER and SELF flags, and the
netlink parsing will now happen in two places, the bridge driver and the 
bridge port
driver ?

For bridge stp state updates, the bridge driver will call the 
ports->setlink ndo op ?

(We should probably rename the ndo_bridge_setlink to ndo_setlink)

>
> On FDB entries, using master/self semantics that exist, it's clear
> which are owned by offloaded device and which are owned by bridge.
> The one missing annotation was a flag indicating FDB entry in bridge
> was synced from device.  And a policy flag to turn on/off syncing from
> the device.  The policy flag is just another IFLA_BRPORT flags passed
> with setlink/getlink.
>
> The setlink/getlink patch will go out in v3 once I finish testing it
> and push it to Jiri.  Hopefully tomorrow.

In my patches, I used newlink..., but in most cases all attributes set 
via newlink can be
used with setlink and hence getlink. So, i think we are close here.

But, Oh wait, i am talking about rtnl_link_ops 
->newlink/changelink/getlink/dellink. I did not
realize there was a parallel ndo op to this. But thats probably because, 
rtnl_link_ops can be
used only for logical devices. But, i see bridge driver implementing 
both the
rtnl_link_ops changelink and ndo op setlink. hmm..seems like a lot of 
duplication.
Will look closely some more.


Coming back to my series, i was trying to get a common set of flags for 
all netdevs
(bridge, bond, vxlans so far), and hence the common flag in 'struct 
ifiinfomsg'.

But, I am all for using existing infrastructure if it fits well.

For the fdb offloads, the NTF_SELF and NTF_MASTER is in 'struct 
ndmsg->ndm_flags', which is also
what i was proposing. So, ack there.

For the bridge netdev, using the flag in IFLA_BRIDGE_FLAGS, is also ok.


I will look at your patches when they are out.

Thanks,
Roopa




On Fri, Nov 21, 2014 at 12:49 PM, <roopa@...ulusnetworks.com> wrote:
>> From: Roopa Prabhu <roopa@...ulusnetworks.com>
>>
>>
>> This series aims at introducing new policy attibutes/flags to enable
>> selective offloading of kernel network objects.
>> This is in the context of supporting switch devices in the linux kernel.
>>
>> Assumption:
>>      - All kernel network objects (routes, neighs, bridges, bonds, vxlans)
>>        can be offloaded (This is true today with a few exceptions maybe)
>>
>> policy points:
>>      - By default all objects exist in software (kernel)
>>      - Per object flag to add/del/show in kernel, hardware or both
>>      - System global option to turn on/off offloads for all network objects.
>>        This is for systems who want to turn offloading on for all network objects
>>        by default. us :). Apps dont need to know about offloading in this
>>        model. (TBD)
>>
>> Patches are based on jiri/sfeldma's rocker work.
>>
>> Apologize for the incomplete and untested code. This is a sample patch
>>   to get some initial feedback.
>>
>> Roopa Prabhu (4):
>>    rtnetlink: new flag NLM_F_HW_OFFLOAD to indicate kernel object
>>      offload to hardware
>>    netdev: new feature flag NETIF_F_HW_OFFLOAD to indicate netdev object
>>      offload to hardware
>>    swdevice: new generic op to set bridge port attr
>>    bridge: make hw offload conditional on bridge and bridge port offload
>>      flags
>>
>>   include/linux/netdev_features.h |    1 +
>>   include/net/switchdev.h         |    8 ++++++-
>>   include/uapi/linux/netlink.h    |    2 ++
>>   net/bridge/br_netlink.c         |   50 +++++++++++++++++++++++++++++++--------
>>   net/bridge/br_private.h         |    2 ++
>>   net/bridge/br_stp.c             |    9 ++++---
>>   net/bridge/br_stp_if.c          |    8 +++++--
>>   net/core/rtnetlink.c            |    7 ++++++
>>   net/switchdev/switchdev.c       |   17 +++++++++++++
>>   9 files changed, 88 insertions(+), 16 deletions(-)
>>
>> --
>> 1.7.10.4
>>

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