Subject: Re: [PATCH net-next v2 01/26] switchdev: introduce get/set attrs ops

From: Jamal Hadi Salim <[email-masked]>
Date: Wed, 01 Apr 2015 08:19:00 -0400

On 04/01/15 06:07, [email-masked] wrote:
> From: Scott Feldman <[email-masked]>
>
> Add two new swdev ops for get/set switch port attributes.  Most swdev
> interactions on a port are gets or sets on port attributes, so rather than
> adding ops for each attribute, let's define clean get/set ops for all
> attributes, and then we can have clear, consistent rules on how attributes
> propagate on stacked devs.
>
> Add the basic algorithms for get/set attr ops.  Use the same recusive algo to
> walk lower devs we've used for STP updates, for example.  For get, compare attr
> value for each lower dev and only return success if attr values match across
> all lower devs.

So this GET is very specific? All underlying children have to have the
exact same value?
My impression of GET is, depending on the target object, i retrieve
one thing or a lot of things that probably user space is asking for.
I wasnt getting that impression looking at this code.

  For sets, set the same attr value for all lower devs.  If
> something goes wrong on one of the lower devs, revert all back to previous attr
> value.

And from staring at the code - the reverting could fail as well..

>
> If lower dev recusion isn't desired, allow a flag SWDEV_F_NO_RECURSE to
> indicate get/set only work on port (lowest) device.
>
> On set, allow a flag SWDEV_F_NO_RECOVER to turn off automatic err recovery.
>

uh-oh did i read that last one correctly?;-> I dont see it in the code
but are you now allowing for policy to dictate whether we want something
atomic or not? That should be a generic much higher level flag imo.
Note, I am for supporting the different scenarios but i thought
the initial approach was all transactions are atomic (all-or-nothing).

Also why invent new namespace for these switch attributes? They all seem
to have netlink IDs already. I note in patches afterwards are using the
netlink IDs.

cheers,
jamal