netdev - Re: [RFC PATCH net-next v3 1/1] net: Support for switch port configuration

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20141219081216.GB1848@nanopsycho.orion>
Date:	Fri, 19 Dec 2014 09:12:16 +0100
From:	Jiri Pirko <jiri@...nulli.us>
To:	Thomas Graf <tgraf@...g.ch>
Cc:	John Fastabend <john.fastabend@...il.com>,
	"Varlese, Marco" <marco.varlese@...el.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"Fastabend, John R" <john.r.fastabend@...el.com>,
	"roopa@...ulusnetworks.com" <roopa@...ulusnetworks.com>,
	"sfeldma@...il.com" <sfeldma@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH net-next v3 1/1] net: Support for switch port
 configuration

Fri, Dec 19, 2014 at 01:35:10AM CET, tgraf@...g.ch wrote:
>On 12/18/14 at 08:03am, John Fastabend wrote:
>> On 12/18/2014 07:30 AM, Varlese, Marco wrote:
>> Could you also document the attributes. I think they are mostly
>> clear but what is IFLA_SW_LOOPBACK. It will help later when we
>> try to read the code in 6months and implement drivers.
>> 
>> I am thinking something like
>> 
>> /* Switch Port Attributes section
>>  * IFLA_SW_LEARNING - turns learning on in the bridge
>>  * IFLA_SW_LOOPBACK - does something interesting
>> 
>>  [...]
>>  */
>
>+1. I would even ask for more than that. While clear in the bridge
>context, "learning" for this API targetting multi layer switches
>is ambigious. The expectation towards the driver must be crystical
>clear.
>
>> >+
>> >+enum {
>> >+	IFLA_SW_UNSPEC,
>> >+	IFLA_SW_LEARNING,
>> 
>> Can you address Roopa's feedback. I'm also a bit confused by the
>> duplication.
>
>Agreed. Can we decide on the ndo first and then build the APIs on
>top of that? While I agree that we should have a non-bridge based
>Netlink API, the underlying ndo should be the same.

Maybe we can use this kind of ndos (proposed by this patch) and call it
for switchdevs instead of bridge_get/setlink. Would make sense to me.
single set of ndos, many possible users (userspace/in-kernel).
bridge_get/setlink would be just a wrapper for this.

>
>> >+static const struct nla_policy ifla_sw_attr_policy[IFLA_SW_ATTR_MAX+1] = {
>> >+	[IFLA_SW_LEARNING]	= { .type = NLA_U64 },
>> >+	[IFLA_SW_LOOPBACK]	= { .type = NLA_U64 },
>> >+	[IFLA_SW_BCAST_FLOODING] = { .type = NLA_U64 },
>> >+	[IFLA_SW_UCAST_FLOODING] = { .type = NLA_U64 },
>> >+	[IFLA_SW_MCAST_FLOODING] = { .type = NLA_U64 },
>> >+};
>> 
>> Why U64 values? What would we pass in these? Are these just boolean
>> bits? Maybe the annotation above will help me understand this.
>
>I think the intent is to keep the ndo API as simple as possible
>but I agree that this is wasteful. I gave this feedback on v2 already.
--
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