It is nore convenient to have prepare and commit phase for attr_set and
obj_add as separete callbacks. If a driver needs to do it differently, it
can easily do in inside its code.

Signed-off-by: Jiri Pirko <j...@resnulli.us>
---
 drivers/net/ethernet/rocker/rocker.c |  88 +++++++++++++++++++-------
 include/net/switchdev.h              |  18 +++---
 net/dsa/slave.c                      | 117 +++++++++++++++++++++++------------
 net/switchdev/switchdev.c            |  74 +++++++++++++++++-----
 4 files changed, 210 insertions(+), 87 deletions(-)

diff --git a/drivers/net/ethernet/rocker/rocker.c 
b/drivers/net/ethernet/rocker/rocker.c
index 0735d90..42aa86c 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -4333,35 +4333,55 @@ static int rocker_port_brport_flags_set(struct 
rocker_port *rocker_port,
        return err;
 }
 
-static int rocker_port_attr_set(struct net_device *dev,
-                               struct switchdev_attr *attr,
-                               struct switchdev_trans *trans)
+static int __rocker_port_attr_set(struct rocker_port *rocker_port,
+                                 struct switchdev_attr *attr,
+                                 struct rocker_trans *rtrans)
 {
-       struct rocker_port *rocker_port = netdev_priv(dev);
-       struct rocker_trans rtrans = {
-               .ph = trans->ph,
-               .trans = trans,
-       };
        int err = 0;
 
        switch (attr->id) {
        case SWITCHDEV_ATTR_PORT_STP_STATE:
-               err = rocker_port_stp_update(rocker_port, &rtrans,
+               err = rocker_port_stp_update(rocker_port, rtrans,
                                             ROCKER_OP_FLAG_NOWAIT,
                                             attr->u.stp_state);
                break;
        case SWITCHDEV_ATTR_PORT_BRIDGE_FLAGS:
-               err = rocker_port_brport_flags_set(rocker_port, &rtrans,
+               err = rocker_port_brport_flags_set(rocker_port, rtrans,
                                                   attr->u.brport_flags);
                break;
        default:
                err = -EOPNOTSUPP;
                break;
        }
-
        return err;
 }
 
+static int rocker_port_attr_pre_set(struct net_device *dev,
+                                   struct switchdev_attr *attr,
+                                   struct switchdev_trans *trans)
+{
+       struct rocker_port *rocker_port = netdev_priv(dev);
+       struct rocker_trans rtrans = {
+               .ph = ROCKER_TRANS_PH_PREPARE,
+               .trans = trans,
+       };
+
+       return __rocker_port_attr_set(rocker_port, attr, &rtrans);
+}
+
+static int rocker_port_attr_set(struct net_device *dev,
+                               struct switchdev_attr *attr,
+                               struct switchdev_trans *trans)
+{
+       struct rocker_port *rocker_port = netdev_priv(dev);
+       struct rocker_trans rtrans = {
+               .ph = ROCKER_TRANS_PH_COMMIT,
+               .trans = trans,
+       };
+
+       return __rocker_port_attr_set(rocker_port, attr, &rtrans);
+}
+
 static int rocker_port_vlan_add(struct rocker_port *rocker_port,
                                struct rocker_trans *rtrans, u16 vid, u16 flags)
 {
@@ -4411,40 +4431,60 @@ static int rocker_port_fdb_add(struct rocker_port 
*rocker_port,
        return rocker_port_fdb(rocker_port, rtrans, fdb->addr, vlan_id, flags);
 }
 
-static int rocker_port_obj_add(struct net_device *dev,
-                              struct switchdev_obj *obj,
-                              struct switchdev_trans *trans)
+static int __rocker_port_obj_add(struct rocker_port *rocker_port,
+                                struct switchdev_obj *obj,
+                                struct rocker_trans *rtrans)
 {
-       struct rocker_port *rocker_port = netdev_priv(dev);
-       struct rocker_trans rtrans = {
-               .ph = trans->ph,
-               .trans = trans,
-       };
        const struct switchdev_obj_ipv4_fib *fib4;
        int err = 0;
 
        switch (obj->id) {
        case SWITCHDEV_OBJ_PORT_VLAN:
-               err = rocker_port_vlans_add(rocker_port, &rtrans,
+               err = rocker_port_vlans_add(rocker_port, rtrans,
                                            &obj->u.vlan);
                break;
        case SWITCHDEV_OBJ_IPV4_FIB:
                fib4 = &obj->u.ipv4_fib;
-               err = rocker_port_fib_ipv4(rocker_port, &rtrans,
+               err = rocker_port_fib_ipv4(rocker_port, rtrans,
                                           htonl(fib4->dst), fib4->dst_len,
                                           fib4->fi, fib4->tb_id, 0);
                break;
        case SWITCHDEV_OBJ_PORT_FDB:
-               err = rocker_port_fdb_add(rocker_port, &rtrans, &obj->u.fdb);
+               err = rocker_port_fdb_add(rocker_port, rtrans, &obj->u.fdb);
                break;
        default:
                err = -EOPNOTSUPP;
                break;
        }
-
        return err;
 }
 
+static int rocker_port_obj_pre_add(struct net_device *dev,
+                                  struct switchdev_obj *obj,
+                                  struct switchdev_trans *trans)
+{
+       struct rocker_port *rocker_port = netdev_priv(dev);
+       struct rocker_trans rtrans = {
+               .ph = ROCKER_TRANS_PH_PREPARE,
+               .trans = trans,
+       };
+
+       return __rocker_port_obj_add(rocker_port, obj, &rtrans);
+}
+
+static int rocker_port_obj_add(struct net_device *dev,
+                              struct switchdev_obj *obj,
+                              struct switchdev_trans *trans)
+{
+       struct rocker_port *rocker_port = netdev_priv(dev);
+       struct rocker_trans rtrans = {
+               .ph = ROCKER_TRANS_PH_COMMIT,
+               .trans = trans,
+       };
+
+       return __rocker_port_obj_add(rocker_port, obj, &rtrans);
+}
+
 static int rocker_port_vlan_del(struct rocker_port *rocker_port,
                                u16 vid, u16 flags)
 {
@@ -4593,7 +4633,9 @@ static int rocker_port_obj_dump(struct net_device *dev,
 
 static const struct switchdev_ops rocker_port_switchdev_ops = {
        .switchdev_port_attr_get        = rocker_port_attr_get,
+       .switchdev_port_attr_pre_set    = rocker_port_attr_pre_set,
        .switchdev_port_attr_set        = rocker_port_attr_set,
+       .switchdev_port_obj_pre_add     = rocker_port_obj_pre_add,
        .switchdev_port_obj_add         = rocker_port_obj_add,
        .switchdev_port_obj_del         = rocker_port_obj_del,
        .switchdev_port_obj_dump        = rocker_port_obj_dump,
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 368a642..30f62f3 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -17,13 +17,6 @@
 
 #define SWITCHDEV_F_NO_RECURSE         BIT(0)
 
-enum switchdev_trans_ph {
-       SWITCHDEV_TRANS_NONE,
-       SWITCHDEV_TRANS_PREPARE,
-       SWITCHDEV_TRANS_ABORT,
-       SWITCHDEV_TRANS_COMMIT,
-};
-
 struct switchdev_trans_item {
        struct list_head list;
        void *data;
@@ -32,7 +25,6 @@ struct switchdev_trans_item {
 
 struct switchdev_trans {
        struct list_head item_list;
-       enum switchdev_trans_ph ph;
 };
 
 enum switchdev_attr_id {
@@ -97,8 +89,12 @@ void *switchdev_trans_item_dequeue(struct switchdev_trans 
*trans);
  *
  * @switchdev_port_attr_get: Get a port attribute (see switchdev_attr).
  *
+ * @switchdev_port_attr_pre_set: Prepare to set a port attribute (see 
switchdev_attr).
+ *
  * @switchdev_port_attr_set: Set a port attribute (see switchdev_attr).
  *
+ * @switchdev_port_obj_pre_add: Prepare to add an object to port (see 
switchdev_obj).
+ *
  * @switchdev_port_obj_add: Add an object to port (see switchdev_obj).
  *
  * @switchdev_port_obj_del: Delete an object from port (see switchdev_obj).
@@ -108,9 +104,15 @@ void *switchdev_trans_item_dequeue(struct switchdev_trans 
*trans);
 struct switchdev_ops {
        int     (*switchdev_port_attr_get)(struct net_device *dev,
                                           struct switchdev_attr *attr);
+       int     (*switchdev_port_attr_pre_set)(struct net_device *dev,
+                                              struct switchdev_attr *attr,
+                                              struct switchdev_trans *trans);
        int     (*switchdev_port_attr_set)(struct net_device *dev,
                                           struct switchdev_attr *attr,
                                           struct switchdev_trans *trans);
+       int     (*switchdev_port_obj_pre_add)(struct net_device *dev,
+                                             struct switchdev_obj *obj,
+                                             struct switchdev_trans *trans);
        int     (*switchdev_port_obj_add)(struct net_device *dev,
                                          struct switchdev_obj *obj,
                                          struct switchdev_trans *trans);
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 748cc63..ed773bc 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -241,6 +241,29 @@ static int dsa_bridge_check_vlan_range(struct dsa_switch 
*ds,
        return err == -ENOENT ? 0 : err;
 }
 
+static int dsa_slave_port_vlan_pre_add(struct net_device *dev,
+                                      struct switchdev_obj *obj,
+                                      struct switchdev_trans *trans)
+{
+       struct switchdev_obj_vlan *vlan = &obj->u.vlan;
+       struct dsa_slave_priv *p = netdev_priv(dev);
+       struct dsa_switch *ds = p->parent;
+       int err;
+
+       if (!ds->drv->port_vlan_add || !ds->drv->port_pvid_set)
+               return -EOPNOTSUPP;
+
+       /* If the requested port doesn't belong to the same bridge as
+        * the VLAN members, fallback to software VLAN (hopefully).
+        */
+       err = dsa_bridge_check_vlan_range(ds, p->bridge_dev,
+                                         vlan->vid_begin,
+                                         vlan->vid_end);
+       if (err)
+               return err;
+       return 0;
+}
+
 static int dsa_slave_port_vlan_add(struct net_device *dev,
                                   struct switchdev_obj *obj,
                                   struct switchdev_trans *trans)
@@ -251,35 +274,15 @@ static int dsa_slave_port_vlan_add(struct net_device *dev,
        u16 vid;
        int err;
 
-       switch (trans->ph) {
-       case SWITCHDEV_TRANS_PREPARE:
-               if (!ds->drv->port_vlan_add || !ds->drv->port_pvid_set)
-                       return -EOPNOTSUPP;
-
-               /* If the requested port doesn't belong to the same bridge as
-                * the VLAN members, fallback to software VLAN (hopefully).
-                */
-               err = dsa_bridge_check_vlan_range(ds, p->bridge_dev,
-                                                 vlan->vid_begin,
-                                                 vlan->vid_end);
+       for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid) {
+               err = ds->drv->port_vlan_add(ds, p->port, vid,
+                                            vlan->flags &
+                                            BRIDGE_VLAN_INFO_UNTAGGED);
+               if (!err && vlan->flags & BRIDGE_VLAN_INFO_PVID)
+                       err = ds->drv->port_pvid_set(ds, p->port, vid);
                if (err)
                        return err;
-               break;
-       case SWITCHDEV_TRANS_COMMIT:
-               for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid) {
-                       err = ds->drv->port_vlan_add(ds, p->port, vid,
-                                                    vlan->flags &
-                                                    BRIDGE_VLAN_INFO_UNTAGGED);
-                       if (!err && vlan->flags & BRIDGE_VLAN_INFO_PVID)
-                               err = ds->drv->port_pvid_set(ds, p->port, vid);
-                       if (err)
-                               return err;
-               }
-               break;
-       default:
-               return -EOPNOTSUPP;
        }
-
        return 0;
 }
 
@@ -347,6 +350,16 @@ static int dsa_slave_port_vlan_dump(struct net_device *dev,
        return err == -ENOENT ? 0 : err;
 }
 
+static int dsa_slave_port_fdb_pre_add(struct net_device *dev,
+                                     struct switchdev_obj *obj,
+                                     struct switchdev_trans *trans)
+{
+       struct dsa_slave_priv *p = netdev_priv(dev);
+       struct dsa_switch *ds = p->parent;
+
+       return ds->drv->port_fdb_add ? 0 : -EOPNOTSUPP;
+}
+
 static int dsa_slave_port_fdb_add(struct net_device *dev,
                                  struct switchdev_obj *obj,
                                  struct switchdev_trans *trans)
@@ -354,14 +367,8 @@ static int dsa_slave_port_fdb_add(struct net_device *dev,
        struct switchdev_obj_fdb *fdb = &obj->u.fdb;
        struct dsa_slave_priv *p = netdev_priv(dev);
        struct dsa_switch *ds = p->parent;
-       int ret = -EOPNOTSUPP;
-
-       if (trans->ph == SWITCHDEV_TRANS_PREPARE)
-               ret = ds->drv->port_fdb_add ? 0 : -EOPNOTSUPP;
-       else if (trans->ph == SWITCHDEV_TRANS_COMMIT)
-               ret = ds->drv->port_fdb_add(ds, p->port, fdb->addr, fdb->vid);
 
-       return ret;
+       return ds->drv->port_fdb_add(ds, p->port, fdb->addr, fdb->vid);
 }
 
 static int dsa_slave_port_fdb_del(struct net_device *dev,
@@ -457,17 +464,24 @@ static int dsa_slave_stp_update(struct net_device *dev, 
u8 state)
        return ret;
 }
 
+static int dsa_slave_port_attr_pre_set(struct net_device *dev,
+                                      struct switchdev_attr *attr,
+                                      struct switchdev_trans *trans)
+{
+       if (attr->id != SWITCHDEV_ATTR_PORT_STP_STATE)
+               return -EOPNOTSUPP;
+       return 0;
+}
+
 static int dsa_slave_port_attr_set(struct net_device *dev,
                                   struct switchdev_attr *attr,
                                   struct switchdev_trans *trans)
 {
-       int ret = 0;
+       int ret;
 
        switch (attr->id) {
        case SWITCHDEV_ATTR_PORT_STP_STATE:
-               if (trans->ph == SWITCHDEV_TRANS_COMMIT)
-                       ret = dsa_slave_stp_update(dev, attr->u.stp_state);
-               break;
+               ret = dsa_slave_stp_update(dev, attr->u.stp_state);
        default:
                ret = -EOPNOTSUPP;
                break;
@@ -476,9 +490,9 @@ static int dsa_slave_port_attr_set(struct net_device *dev,
        return ret;
 }
 
-static int dsa_slave_port_obj_add(struct net_device *dev,
-                                 struct switchdev_obj *obj,
-                                 struct switchdev_trans *trans)
+static int dsa_slave_port_obj_pre_add(struct net_device *dev,
+                                     struct switchdev_obj *obj,
+                                     struct switchdev_trans *trans)
 {
        int err;
 
@@ -489,6 +503,27 @@ static int dsa_slave_port_obj_add(struct net_device *dev,
 
        switch (obj->id) {
        case SWITCHDEV_OBJ_PORT_FDB:
+               err = dsa_slave_port_fdb_pre_add(dev, obj, trans);
+               break;
+       case SWITCHDEV_OBJ_PORT_VLAN:
+               err = dsa_slave_port_vlan_pre_add(dev, obj, trans);
+               break;
+       default:
+               err = -EOPNOTSUPP;
+               break;
+       }
+
+       return err;
+}
+
+static int dsa_slave_port_obj_add(struct net_device *dev,
+                                 struct switchdev_obj *obj,
+                                 struct switchdev_trans *trans)
+{
+       int err;
+
+       switch (obj->id) {
+       case SWITCHDEV_OBJ_PORT_FDB:
                err = dsa_slave_port_fdb_add(dev, obj, trans);
                break;
        case SWITCHDEV_OBJ_PORT_VLAN:
@@ -960,7 +995,9 @@ static const struct net_device_ops dsa_slave_netdev_ops = {
 
 static const struct switchdev_ops dsa_slave_switchdev_ops = {
        .switchdev_port_attr_get        = dsa_slave_port_attr_get,
+       .switchdev_port_attr_pre_set    = dsa_slave_port_attr_pre_set,
        .switchdev_port_attr_set        = dsa_slave_port_attr_set,
+       .switchdev_port_obj_pre_add     = dsa_slave_port_obj_pre_add,
        .switchdev_port_obj_add         = dsa_slave_port_obj_add,
        .switchdev_port_obj_del         = dsa_slave_port_obj_del,
        .switchdev_port_obj_dump        = dsa_slave_port_obj_dump,
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 82f8bcd..9278643 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -110,6 +110,34 @@ int switchdev_port_attr_get(struct net_device *dev, struct 
switchdev_attr *attr)
 }
 EXPORT_SYMBOL_GPL(switchdev_port_attr_get);
 
+static int __switchdev_port_attr_pre_set(struct net_device *dev,
+                                        struct switchdev_attr *attr,
+                                        struct switchdev_trans *trans)
+{
+       const struct switchdev_ops *ops = dev->switchdev_ops;
+       struct net_device *lower_dev;
+       struct list_head *iter;
+       int err = -EOPNOTSUPP;
+
+       if (ops && ops->switchdev_port_attr_pre_set)
+               return ops->switchdev_port_attr_pre_set(dev, attr, trans);
+
+       if (attr->flags & SWITCHDEV_F_NO_RECURSE)
+               return err;
+
+       /* Switch device port(s) may be stacked under
+        * bond/team/vlan dev, so recurse down to prepare to set attr on
+        * each port.
+        */
+
+       netdev_for_each_lower_dev(dev, lower_dev, iter) {
+               err = __switchdev_port_attr_pre_set(lower_dev, attr, trans);
+               if (err)
+                       break;
+       }
+
+       return err;
+}
 static int __switchdev_port_attr_set(struct net_device *dev,
                                     struct switchdev_attr *attr,
                                     struct switchdev_trans *trans)
@@ -216,20 +244,15 @@ int switchdev_port_attr_set(struct net_device *dev, 
struct switchdev_attr *attr)
         * but should not commit the attr.
         */
 
-       trans.ph = SWITCHDEV_TRANS_PREPARE;
-       err = __switchdev_port_attr_set(dev, attr, &trans);
+       err = __switchdev_port_attr_pre_set(dev, attr, &trans);
        if (err) {
                /* Prepare phase failed: abort the transaction.  Any
                 * resources reserved in the prepare phase are
                 * released.
                 */
 
-               if (err != -EOPNOTSUPP) {
-                       trans.ph = SWITCHDEV_TRANS_ABORT;
-                       __switchdev_port_attr_set(dev, attr, &trans);
+               if (err != -EOPNOTSUPP)
                        switchdev_trans_items_destroy(&trans);
-               }
-
                return err;
        }
 
@@ -238,7 +261,6 @@ int switchdev_port_attr_set(struct net_device *dev, struct 
switchdev_attr *attr)
         * because the driver said everythings was OK in phase I.
         */
 
-       trans.ph = SWITCHDEV_TRANS_COMMIT;
        err = __switchdev_port_attr_set(dev, attr, &trans);
        WARN(err, "%s: Commit of attribute (id=%d) failed.\n",
             dev->name, attr->id);
@@ -274,6 +296,32 @@ static int __switchdev_port_obj_add(struct net_device *dev,
        return err;
 }
 
+static int __switchdev_port_obj_pre_add(struct net_device *dev,
+                                       struct switchdev_obj *obj,
+                                       struct switchdev_trans *trans)
+{
+       const struct switchdev_ops *ops = dev->switchdev_ops;
+       struct net_device *lower_dev;
+       struct list_head *iter;
+       int err = -EOPNOTSUPP;
+
+       if (ops && ops->switchdev_port_obj_pre_add)
+               return ops->switchdev_port_obj_pre_add(dev, obj, trans);
+
+       /* Switch device port(s) may be stacked under
+        * bond/team/vlan dev, so recurse down to prepare to add object on
+        * each port.
+        */
+
+       netdev_for_each_lower_dev(dev, lower_dev, iter) {
+               err = __switchdev_port_obj_pre_add(lower_dev, obj, trans);
+               if (err)
+                       break;
+       }
+
+       return err;
+}
+
 /**
  *     switchdev_port_obj_add - Add port object
  *
@@ -302,20 +350,15 @@ int switchdev_port_obj_add(struct net_device *dev, struct 
switchdev_obj *obj)
         * but should not commit the obj.
         */
 
-       trans.ph = SWITCHDEV_TRANS_PREPARE;
-       err = __switchdev_port_obj_add(dev, obj, &trans);
+       err = __switchdev_port_obj_pre_add(dev, obj, &trans);
        if (err) {
                /* Prepare phase failed: abort the transaction.  Any
                 * resources reserved in the prepare phase are
                 * released.
                 */
 
-               if (err != -EOPNOTSUPP) {
-                       trans.ph = SWITCHDEV_TRANS_ABORT;
-                       __switchdev_port_obj_add(dev, obj, &trans);
+               if (err != -EOPNOTSUPP)
                        switchdev_trans_items_destroy(&trans);
-               }
-
                return err;
        }
 
@@ -324,7 +367,6 @@ int switchdev_port_obj_add(struct net_device *dev, struct 
switchdev_obj *obj)
         * because the driver said everythings was OK in phase I.
         */
 
-       trans.ph = SWITCHDEV_TRANS_COMMIT;
        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);
-- 
1.9.3

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to