netdev - Re: [patch net-next v3 02/10] switchdev: introduce transaction item queue for attr_set and obj

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAE4R7bA=xwfNT1cEp7_B7zfiuqg=PZ421j+GNCP+hi9DrhccAA@mail.gmail.com>
Date:	Thu, 24 Sep 2015 22:29:43 -0700
From:	Scott Feldman <sfeldma@...il.com>
To:	Vivien Didelot <vivien.didelot@...oirfairelinux.com>
Cc:	Jiri Pirko <jiri@...nulli.us>, Netdev <netdev@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Ido Schimmel <idosch@...lanox.com>, eladr@...lanox.com,
	Florian Fainelli <f.fainelli@...il.com>,
	Guenter Roeck <linux@...ck-us.net>,
	"Rosen, Rami" <rami.rosen@...el.com>,
	Roopa Prabhu <roopa@...ulusnetworks.com>,
	Premkumar Jonnala <pjonnala@...adcom.com>,
	"andrew@...n.ch" <andrew@...n.ch>,
	Andy Gospodarek <gospo@...ulusnetworks.com>,
	Jiri Pirko <jiri@...lanox.com>
Subject: Re: [patch net-next v3 02/10] switchdev: introduce transaction item
 queue for attr_set and obj_add

On Thu, Sep 24, 2015 at 9:36 PM, Vivien Didelot
<vivien.didelot@...oirfairelinux.com> wrote:
> Hi Jiri,
>
> On Sep. Thursday 24 (39) 10:02 AM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@...lanox.com>
>>
>> Now, the memory allocation in prepare/commit state is done separatelly
>> in each driver (rocker). Introduce the similar mechanism in generic
>> switchdev code, in form of queue. That can be used not only for memory
>> allocations, but also for different items. Abort item destruction
>> is handled as well.
>>
>> Signed-off-by: Jiri Pirko <jiri@...lanox.com>
>
> [...]
>
>>  /**
>>   * struct switchdev_ops - switchdev operations
>>   *
>> @@ -94,9 +110,11 @@ struct switchdev_ops {
>>       int     (*switchdev_port_attr_get)(struct net_device *dev,
>>                                          struct switchdev_attr *attr);
>>       int     (*switchdev_port_attr_set)(struct net_device *dev,
>> -                                        struct switchdev_attr *attr);
>> +                                        struct switchdev_attr *attr,
>> +                                        struct switchdev_trans *trans);
>>       int     (*switchdev_port_obj_add)(struct net_device *dev,
>> -                                       struct switchdev_obj *obj);
>> +                                       struct switchdev_obj *obj,
>> +                                       struct switchdev_trans *trans);
>>       int     (*switchdev_port_obj_del)(struct net_device *dev,
>>                                         struct switchdev_obj *obj);
>>       int     (*switchdev_port_obj_dump)(struct net_device *dev,
>
> This version is better than the current state, but it's too bad we
> didn't get feedback yet on the real purpose of this 2-phase model...

Hmmm...I think I missed the question about the 2-phase model.  Let me
see if I can answer it here.

We got here first when working with stacked drivers and we wanted to
propagate an change (attr set or obj add) down the stack in a safe way
as to not leave hardware or software in an inconsistent state when
unexpected things happen.  The simple stacked driver example was two
switch ports bonded.  Setting an attr on the bond would propagate down
to each switch port.  If the setting stuck on the first port, but
didn't on the second port, we didn't want to have to go back and undo
the setting on the first port.  It gets messy real fast trying to
handle the undo operation.  So 2-phase model was suggested by davem to
solve the problem in a generic way.  First phase was to check with
each device in the stack if the operation is OK, and second phase
would commit operation.  The second phase cannot fail; the first phase
already said everything was OK to proceed.  That means no running out
of hardware resources in phase 2.  Phase 1 should have check/reserved
the resources.  And no OOM situations in phase 2.  And so on for other
detectable failures.

Stacked driver case is one case where the 2-phase model strives to
maintain hw/sw consistency.  I could see it working for batch
operations also, although we don't have any examples of batch
processing yet (in switchdev).

I'd rather keep 2-phase not optional, or at least make it some what of
a pain for drivers to opt-out of 2-phase.  Forcing the driver to see
both phases means the driver needs to put some code to skip phase 1
(and hopefully has some persistent comment explaining why its being
skipped).  Something like:

/* I'm skipping phase 1 prepare for this operation.  I have infinite hardware
 * resources and I'm not setting any persistent state in the driver or device
 * and I don't need any dynamic resources from the kernel, so its impossible
 * for me to fail phase 2 commit.  Nothing to prepare, sorry.
 */

-scott
--
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