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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Thu, 18 Dec 2014 10:14:01 -0800
From:	Roopa Prabhu <roopa@...ulusnetworks.com>
To:	"Varlese, Marco" <marco.varlese@...el.com>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"Fastabend, John R" <john.r.fastabend@...el.com>,
	Thomas Graf <tgraf@...g.ch>, Jiri Pirko <jiri@...nulli.us>,
	"sfeldma@...il.com" <sfeldma@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration

On 12/18/14, 10:02 AM, Varlese, Marco wrote:
> Removed unnecessary content for ease of reading...
>
>>>>>>> +/* Switch Port Attributes section */
>>>>>>> +
>>>>>>> +enum {
>>>>>>> +	IFLA_ATTR_UNSPEC,
>>>>>>> +	IFLA_ATTR_LEARNING,
>>>>>> Any reason you want learning here ?. This is covered as part  of
>>>>>> the bridge setlink attributes.
>>>>>>
>>>>> Yes, because the user may _not_ want to go through a bridge
>>>>> interface
>>>> necessarily.
>>>> But, the bridge setlink/getlink interface was changed to accommodate
>> 'self'
>>>> for exactly such cases.
>>>> I kind of understand your case for the other attributes (these are
>>>> per port settings that switch asics provide).
>>>>
>>>> However, i don't understand the reason to pull in bridge attributes here.
>>>>
>>> Maybe, I am missing something so you might help. The learning attribute -
>> in my case - it is like all other attributes: a port attribute (as you said, port
>> settings that the switch provides per port).
>>> So, what I was saying is "why the user shall go through a bridge to configure
>> the learning attribute"? From my perspective, it is as any other attribute and
>> as such configurable on the port.
>>
>> Thinking about this some more, i don't see why any of these attributes
>> (except loopback. I dont understand the loopback attribute) cant be part of
>> the birdge port attributes.
>>
>> With this we will end up adding l2 attributes in two places: the general link
>> attributes and bridge attributes.
>>
>> And since we have gone down the path of using ndo_bridge_setlink/getlink
>> with 'self'....we should stick to that for all l2 attributes.
>>
>> The idea of overloading ndo_bridge_set/getlink, was to have the same set of
>> attributes but support both cases where the user wants to go through the
>> bridge driver or directly to the switch port driver. So, you are not really going
>> through the bridge driver if you use 'self' and ndo_bridge_setlink/getlink.
>>
> Roopa, one of the comments I got from Thomas Graf on my v1 patch was that your patch and mine were supplementary ("I think Roopa's patches are supplementary. Not all switchdev users will be backed with a Linux Bridge. I therefore welcome your patches very much")... I also understood by others that the patch made sense for the same reason. I simply do not understand why these attributes (and maybe others in the future) could not be configured directly on a standard port but have to go through a bridge.
>
ok, i am very confused in that case. The whole moving of bridge 
attributes from the bridge driver to rtnetlink.c was to make the bridge 
attributes accessible to any driver who wants to set l2/bridge 
attributes on their switch ports. So, its unclear to me why we are doing 
this parallel thing again.
This move to rtnetlink.c was done during the recent rocker support. so, 
maybe scott/jiri can elaborate more.
--
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