[patch net-next RFC 6/6] switchdev: split commit and prepare phase into two callbacks
flat view
STALE4051d
From: Jiri Pirko <jiri@resnulli.us>
Date: 2015-09-19 12:29:26
Subsystem:
networking drivers, networking [dsa], networking [general], rocker driver, switchdev, the rest · Maintainers:
Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn, Vladimir Oltean, Jiri Pirko, Ivan Vecera, Linus Torvalds
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 <jiri@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