netdev - Re: [patch net-next v2 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]

Date:	Wed, 23 Sep 2015 08:46:44 -0700
From:	Scott Feldman <sfeldma@...il.com>
To:	Jiri Pirko <jiri@...nulli.us>
Cc:	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>,
	Vivien Didelot <vivien.didelot@...oirfairelinux.com>,
	"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 v2 02/10] switchdev: introduce transaction item
 queue for attr_set and obj_add

On Wed, Sep 23, 2015 at 12:57 AM, Jiri Pirko <jiri@...nulli.us> 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. Commit/abort item destruction
> is handled as well.
>
> Signed-off-by: Jiri Pirko <jiri@...lanox.com>
> ---
> v1-v2:
> - added comment blocks to exported functions as suggested by Scott
> - made switchdev_trans_init and switchdev_trans_items_destroy static
> - added documentation for this to switchdev.txt as suggested by Scott

[cut]

> @@ -232,10 +304,13 @@ static int __switchdev_port_obj_add(struct net_device *dev,
>   */
>  int switchdev_port_obj_add(struct net_device *dev, struct switchdev_obj *obj)
>  {
> +       struct switchdev_trans trans;
>         int err;
>
>         ASSERT_RTNL();
>
> +       switchdev_trans_init(&trans);
> +
>         /* Phase I: prepare for obj add. Driver/device should fail
>          * here if there are going to be issues in the commit phase,
>          * such as lack of resources or support.  The driver/device
> @@ -244,7 +319,7 @@ int switchdev_port_obj_add(struct net_device *dev, struct switchdev_obj *obj)
>          */
>
>         obj->trans_ph = SWITCHDEV_TRANS_PREPARE;
> -       err = __switchdev_port_obj_add(dev, obj);
> +       err = __switchdev_port_obj_add(dev, obj, &trans);
>         if (err) {
>                 /* Prepare phase failed: abort the transaction.  Any
>                  * resources reserved in the prepare phase are
> @@ -253,7 +328,8 @@ int switchdev_port_obj_add(struct net_device *dev, struct switchdev_obj *obj)
>
>                 if (err != -EOPNOTSUPP) {
>                         obj->trans_ph = SWITCHDEV_TRANS_ABORT;
> -                       __switchdev_port_obj_add(dev, obj);
> +                       __switchdev_port_obj_add(dev, obj, &trans);
> +                       switchdev_trans_items_destroy(&trans);
>                 }
>
>                 return err;
> @@ -265,8 +341,9 @@ int switchdev_port_obj_add(struct net_device *dev, struct switchdev_obj *obj)
>          */
>
>         obj->trans_ph = SWITCHDEV_TRANS_COMMIT;
> -       err = __switchdev_port_obj_add(dev, obj);
> +       err = __switchdev_port_obj_add(dev, obj, &trans);
>         WARN(err, "%s: Commit of object (id=%d) failed.\n", dev->name, obj->id);
> +       switchdev_trans_items_destroy(&trans);

Before this patchset, rocker would BUG_ON that the list was empty
after commit.  This was really helpful in finding bugs in the driver
as it meant the commit and prepare phases didn't follow the same code
path.  (Everything the driver queued in prepare was dequeued in
commit, so the list should be empty after commit).

Should this last switchdev_trans_items_destroy(&trans) be replaced with:

    BUG_ON(!list_empty(&trans.item_list));
--
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