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

[Jiri Pirko <jiri@resnulli.us>]

Fri, Dec 19, 2014 at 01:35:10AM CET, tgraf@suug.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.