netdev - Re: [PATCH 2/3] bridge: offload bridge port attributes to switch asic if feature flag set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 05 Dec 2014 07:41:05 -0500
From:	Jamal Hadi Salim <jhs@...atatu.com>
To:	Roopa Prabhu <roopa@...ulusnetworks.com>,
	John Fastabend <john.fastabend@...il.com>
CC:	Scott Feldman <sfeldma@...il.com>,
	Jiří Pírko <jiri@...nulli.us>,
	Benjamin LaHaise <bcrl@...ck.org>, Thomas Graf <tgraf@...g.ch>,
	"stephen@...workplumber.org" <stephen@...workplumber.org>,
	John Linville <linville@...driver.com>,
	"nhorman@...driver.com" <nhorman@...driver.com>,
	Nicolas Dichtel <nicolas.dichtel@...nd.com>,
	"vyasevic@...hat.com" <vyasevic@...hat.com>,
	Florian Fainelli <f.fainelli@...il.com>,
	"buytenh@...tstofly.org" <buytenh@...tstofly.org>,
	Aviad Raveh <aviadr@...lanox.com>,
	Netdev <netdev@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>, shm@...ulusnetworks.com,
	Andy Gospodarek <gospo@...ulusnetworks.com>
Subject: Re: [PATCH 2/3] bridge: offload bridge port attributes to switch
 asic if feature flag set

On 12/05/14 02:10, Roopa Prabhu wrote:
> On 12/4/14, 10:55 PM, John Fastabend wrote:
>> On 12/04/2014 10:41 PM, Scott Feldman wrote:
>>> On Thu, Dec 4, 2014 at 6:26 PM, <roopa@...ulusnetworks.com> wrote:
>>>> From: Roopa Prabhu <roopa@...ulusnetworks.com>
>>>>
>>>> This allows offloading to switch asic without having the user to set
>>>> any flag. And this is done in the bridge driver to rollback kernel
>>>> settings
>>>> on hw offload failure if required in the future.
>>>>
>>>> With this, it also makes sure a notification goes out only after the
>>>> attributes are set both in the kernel and hw.
>>>
>>> I like this approach as it streamlines the steps for the user in
>>> setting port flags.  There is one case for FLOODING where you'll have
>>> to turn off flooding for both, and then turn on flooding in hw. You
>>> don't want flooding turned on on kernel and hw.
>>>
>>>> ---
>>>>   net/bridge/br_netlink.c |   27 ++++++++++++++++++++++++++-
>>>>   1 file changed, 26 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>>>> index 9f5eb55..ce173f0 100644
>>>> --- a/net/bridge/br_netlink.c
>>>> +++ b/net/bridge/br_netlink.c
>>>> @@ -407,9 +407,21 @@ int br_setlink(struct net_device *dev, struct
>>>> nlmsghdr *nlh)
>>>>                                  afspec, RTM_SETLINK);
>>>>          }
>>>>
>>>> +       if ((dev->features & NETIF_F_HW_SWITCH_OFFLOAD) &&
>>>> + dev->netdev_ops->ndo_bridge_setlink) {
>>>> +               int ret = dev->netdev_ops->ndo_bridge_setlink(dev,
>>>> nlh);
>>>
>>> I think you want to up-level this to net/core/rtnetlink.c because
>>> you're only enabling the feature for one instance of a driver that
>>> implements ndo_bridge_setlink: the bridge driver.  If another driver
>>> was MASTER and implemented ndo_bridge_setlink, you'd want same check
>>> to push setting down to SELF port driver.
>>
>> Also if the user set SELF && MASTER flags && had HW_SWITCH_OFFLOAD bit
>> set we would call ndo_bridge_setlink twice on the dev. Maybe you can
>> catch this case in rtnetlink.c and only call it once.
>
> yes, thought about this and when i looked at iproute2 code, it is either
> master
> or self today and i don't think anybody else uses both flags with the
> current
> kernel implementation. But yes, that does not stop anybody from setting
> both flags.
> I will handle it better in v2.

folks, can we have probably 2-3 sets of patches?
#1 introduces the flags and doesnt change anything.
Others to introduce other features.

cheers,
jamal


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