[<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 linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists