[PATCH net-next v4 00/24] switchdev: spring cleanup

[PATCH net-next v4 00/24] switchdev: spring cleanup

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

v4:

Well, it was a lot of work, but now prepare-commit transaction model is how
davem advises: if prepare fails, abort the transaction.  The driver must do
resource reservations up front in prepare phase and return those resources if
aborting.  Commit phase would use reserved resources.  The good news is the
driver code (for rocker) now handles resource allocation failures better by not
leaving partially device or driver states.  This is a side-effect of the
prepare phase where state isn't modified; only validation of inputs and
resource reservations happen in the prepare phase.  Since we're supporting
setting attrs and add objs across lower devs in the stacked case, we need to
hold rtnl_lock (or ensure rtnl_lock is held) so lower devs don't move on us
during the prepare-commit transaction.  DSA driver code skips the prepare phase
and goes straight for the commit phase since no up-front allocations are done
and no device failures (that could be detected in the prepare phase) can
happen.

Remove NETIF_F_HW_SWITCH_OFFLOAD from rocker and the swdev_attr_set/get
wrappers.  DSA doesn't set NETIF_F_HW_SWITCH_OFFLOAD, so it can't be in
swdev_attr_set/get.  rocker doesn't need it; or rather can't support
NETIF_F_HW_SWITCH_OFFLOAD being set/cleared at run-time after the device
port is already up and offloading L2/L3.  NETIF_F_HW_SWITCH_OFFLOAD is still
left as a feature flag for drivers that can use it.

Drop the renaming patch for netdev_switch_notifier.  Other renames are a
result of moving to the attr get/set or obj add/del model.  Everything
but the netdev_switch_notifier is still prefixed with "swdev_".

v3:

Move to two-phase prepare-commit transaction model for attr set and obj add.
Driver gets a change in prepare phase to NACK transaction if lack of resources
or support in device.

v2:

Address review comments:

 - [Jiri] squash a few related patches
 - [Roopa] don't remove NETIF_F_HW_SWITCH_OFFLOAD
 - [Roopa] address VLAN setlink/dellink
 - [Ronen] print warning is attr set revert fails

Not address:

 - Using something other than "swdev_" prefix
 - Vendor extentions

The patch set grew a bit to not only support port attr get/set but also add
support for port obj add/del.  Example of port objs are VLAN, FDB entries, and
FIB entries.  The VLAN support now allows the swdev driver to get VLAN ranges
and flags like PVID and "untagged".  Sridhar will be adding FDB obj support
in follow-on patch.


v1:

The main theme of this patch set is to cleanup swdev in preparation for
new features or fixes to be added soon.  We have a pretty good idea now how
to handle stacked drivers in swdev, but there where some loose ends.  For
example, if a set failed in the middle of walking the lower devs, we would
leave the system in an undefined state...there was no way to recover back to
the previous state.  Speaking of sets, also recognize a pattern that most
swdev API accesses are gets or sets of port attributes, so go ahead and make
port attr get/set the central swdev API, and convert everything that is
set-ish/get-ish to this new API.

Features/fixes that should follow from this cleanup:

 - solve the duplicate pkt forwarding issue
 - get/set bridge attrs, like ageing_time, from/to device
 - get/set more bridge port attrs from/to device

There are some rename cleanups tagging along at the end, to give swdev
consistent naming.

And finally, some much needed updates to the switchdev.txt documentation to
hopefully capture the state-of-the-art of swdev.  Hopefully, we can do a better
job keeping this document up-to-date.

Tested with rocker, of course, to make sure nothing functional broke.  There
are a couple minor tweaks to DSA code for getting switch ID and setting STP
updates to use new API, but not expecting amy breakage there.



Scott Feldman (24):
  switchdev: introduce get/set attrs ops
  switchdev: convert parent_id_get to swdev attr get
  switchdev: convert STP update to swdev attr set
  switchdev: add bridge port flags attr
  rocker: use swdev get/set attr for bridge port flags
  switchdev: introduce swdev add/del obj ops
  switchdev: add port vlan obj
  rocker: use swdev add/del obj for bridge port vlans
  switchdev: add new swdev bridge setlink
  rocker: cut over to new swdev_port_bridge_setlink
  bonding: cut over to new swdev_port_bridge_setlink
  team: cut over to new swdev_port_bridge_setlink
  switchdev: remove old netdev_switch_port_bridge_setlink
  switchdev: add new swdev_port_bridge_dellink
  rocker: cut over to new swdev_port_bridge_dellink
  bonding: cut over to new swdev_port_bridge_dellink
  team: cut over to new swdev_port_bridge_dellink
  switchdev: remove unused netdev_switch_port_bridge_dellink
  switchdev: add new swdev_port_bridge_getlink
  rocker: cut over to new swdev_port_bridge_getlink
  bonding: cut over to new swdev_port_bridge_getlink
  team: cut over to new swdev_port_bridge_getlink
  switchdev: convert fib_ipv4_add/del over to swdev_port_obj_add/del
  switchdev: bring documentation up-to-date

 Documentation/networking/switchdev.txt |  414 ++++++++++++++++++---
 drivers/net/bonding/bond_main.c        |    5 +-
 drivers/net/ethernet/rocker/rocker.c   |  497 ++++++++++++++++++-------
 drivers/net/team/team.c                |    5 +-
 include/net/switchdev.h                |  177 +++++----
 net/bridge/br_netlink.c                |   24 +-
 net/bridge/br_stp.c                    |    6 +-
 net/core/net-sysfs.c                   |   10 +-
 net/core/rtnetlink.c                   |    9 +-
 net/dsa/slave.c                        |   36 +-
 net/ipv4/fib_trie.c                    |   38 +-
 net/switchdev/switchdev.c              |  623 +++++++++++++++++++++++++-------
 12 files changed, 1386 insertions(+), 458 deletions(-)

-- 
1.7.10.4

[PATCH net-next v4 01/24] switchdev: introduce get/set attrs ops

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

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.  For sets, set the same attr value for all lower devs.  We'll
use a two-phase prepare-commit transaction model for sets.  In the first phase,
the driver(s) are asked if attr set is OK.  If all OK, the commit attr set in
second phase.  A driver would NACK the prepare phase if it can't set the attr
due to lack of resources or support, within it's control.  RTNL lock must be
held across both phases because we'll recurse all lower devs first in prepare
phase, and then recurse all lower devs again in commit phase.  If any lower dev
fails the prepare phase, we need to abort the transaction for all lower devs.

If lower dev recusion isn't desired, allow a flag SWDEV_F_NO_RECURSE to
indicate get/set only work on port (lowest) device.

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 include/net/switchdev.h   |   41 +++++++++++
 net/switchdev/switchdev.c |  176 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 217 insertions(+)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index d2e69ee..1d45621 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -14,6 +14,25 @@
 #include <linux/netdevice.h>
 #include <linux/notifier.h>
 
+#define SWDEV_F_NO_RECURSE		BIT(1)
+
+enum swdev_trans {
+	SWDEV_TRANS_NONE,
+	SWDEV_TRANS_PREPARE,
+	SWDEV_TRANS_ABORT,
+	SWDEV_TRANS_COMMIT,
+};
+
+enum swdev_attr_id {
+	SWDEV_ATTR_UNDEFINED,
+};
+
+struct swdev_attr {
+	enum swdev_attr_id id;
+	enum swdev_trans trans;
+	u32 flags;
+};
+
 struct fib_info;
 
 /**
@@ -23,6 +42,10 @@ struct fib_info;
  *   is part of.  If driver implements this, it indicates that it
  *   represents a port of a switch chip.
  *
+ * @swdev_port_attr_get: Get a port attribute (see swdev_attr).
+ *
+ * @swdev_port_attr_set: Set a port attribute (see swdev_attr).
+ *
  * @swdev_port_stp_update: Called to notify switch device port of bridge
  *   port STP state change.
  *
@@ -33,6 +56,10 @@ struct fib_info;
 struct swdev_ops {
 	int	(*swdev_parent_id_get)(struct net_device *dev,
 				       struct netdev_phys_item_id *psid);
+	int	(*swdev_port_attr_get)(struct net_device *dev,
+				       struct swdev_attr *attr);
+	int	(*swdev_port_attr_set)(struct net_device *dev,
+				       struct swdev_attr *attr);
 	int	(*swdev_port_stp_update)(struct net_device *dev, u8 state);
 	int	(*swdev_fib_ipv4_add)(struct net_device *dev, __be32 dst,
 				      int dst_len, struct fib_info *fi,
@@ -68,6 +95,8 @@ netdev_switch_notifier_info_to_dev(const struct netdev_switch_notifier_info *inf
 
 int netdev_switch_parent_id_get(struct net_device *dev,
 				struct netdev_phys_item_id *psid);
+int swdev_port_attr_get(struct net_device *dev, struct swdev_attr *attr);
+int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr);
 int netdev_switch_port_stp_update(struct net_device *dev, u8 state);
 int register_netdev_switch_notifier(struct notifier_block *nb);
 int unregister_netdev_switch_notifier(struct notifier_block *nb);
@@ -95,6 +124,18 @@ static inline int netdev_switch_parent_id_get(struct net_device *dev,
 	return -EOPNOTSUPP;
 }
 
+static inline int swdev_port_attr_get(struct net_device *dev,
+				      enum swdev_attr *attr)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int swdev_port_attr_set(struct net_device *dev,
+				      enum swdev_attr *attr)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline int netdev_switch_port_stp_update(struct net_device *dev,
 						u8 state)
 {
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 46568b8..db0cc7b 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -37,6 +37,182 @@ int netdev_switch_parent_id_get(struct net_device *dev,
 EXPORT_SYMBOL_GPL(netdev_switch_parent_id_get);
 
 /**
+ *	swdev_port_attr_get - Get port attribute
+ *
+ *	@dev: port device
+ *	@attr: attribute to get
+ */
+int swdev_port_attr_get(struct net_device *dev, struct swdev_attr *attr)
+{
+	const struct swdev_ops *ops = dev->swdev_ops;
+	struct net_device *lower_dev;
+	struct list_head *iter;
+	struct swdev_attr first = {
+		.id = SWDEV_ATTR_UNDEFINED
+	};
+	int err = -EOPNOTSUPP;
+
+	if (ops && ops->swdev_port_attr_get)
+		return ops->swdev_port_attr_get(dev, attr);
+
+	if (attr->flags & SWDEV_F_NO_RECURSE)
+		return err;
+
+	/* Switch device port(s) may be stacked under
+	 * bond/team/vlan dev, so recurse down to get attr on
+	 * each port.  Return -ENODATA if attr values don't
+	 * compare across ports.
+	 */
+
+	netdev_for_each_lower_dev(dev, lower_dev, iter) {
+		err = swdev_port_attr_get(lower_dev, attr);
+		if (err)
+			break;
+		if (first.id == SWDEV_ATTR_UNDEFINED)
+			first = *attr;
+		else if (memcmp(&first, attr, sizeof(*attr)))
+			return -ENODATA;
+	}
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(swdev_port_attr_get);
+
+static int __swdev_port_attr_set(struct net_device *dev,
+				 struct swdev_attr *attr)
+{
+	const struct swdev_ops *ops = dev->swdev_ops;
+	struct net_device *lower_dev;
+	struct list_head *iter;
+	int err = -EOPNOTSUPP;
+
+	if (ops && ops->swdev_port_attr_set)
+		return ops->swdev_port_attr_set(dev, attr);
+
+	if (attr->flags & SWDEV_F_NO_RECURSE)
+		return err;
+
+	/* Switch device port(s) may be stacked under
+	 * bond/team/vlan dev, so recurse down to set attr on
+	 * each port.
+	 */
+
+	netdev_for_each_lower_dev(dev, lower_dev, iter) {
+		err = __swdev_port_attr_set(lower_dev, attr);
+		if (err)
+			break;
+	}
+
+	return err;
+}
+
+struct swdev_attr_set_defer {
+	struct work_struct work;
+	struct net *net;
+	int ifindex;
+	struct swdev_attr attr;
+};
+
+static void swdev_port_attr_set_defer_work(struct work_struct *work)
+{
+	struct swdev_attr_set_defer *aw =
+		container_of(work, struct swdev_attr_set_defer, work);
+	struct net_device *dev;
+	int err;
+
+	rtnl_lock();
+	dev = __dev_get_by_index(aw->net, aw->ifindex);
+	if (dev) {
+		err = swdev_port_attr_set(dev, &aw->attr);
+		WARN(err, "%s: Deferred set of attr (id=%d) failed.\n",
+		     dev->name, aw->attr.id);
+	}
+	rtnl_unlock();
+
+	kfree(work);
+}
+
+static int swdev_port_attr_set_defer(struct net_device *dev,
+				     struct swdev_attr *attr)
+{
+	struct swdev_attr_set_defer *aw;
+
+	aw = kmalloc(sizeof(*aw), GFP_ATOMIC);
+	if (!aw)
+		return -ENOMEM;
+
+	INIT_WORK(&aw->work, swdev_port_attr_set_defer_work);
+
+	aw->net = dev_net(dev);
+	aw->ifindex = dev->ifindex;
+	memcpy(&aw->attr, attr, sizeof(aw->attr));
+
+	schedule_work(&aw->work);
+
+	return 0;
+}
+
+/**
+ *	swdev_port_attr_set - Set port attribute
+ *
+ *	@dev: port device
+ *	@attr: attribute to set
+ *
+ *	Use a 2-phase prepare-commit transaction model to ensure
+ *	system is not left in a partially updated state due to
+ *	failure from driver/device.
+ *
+ *	rtnl_lock must be held.
+ */
+int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
+{
+	int err;
+
+	if (!rtnl_is_locked()) {
+		/* Running prepare-commit transaction across stacked
+		 * devices requires nothing moves, so if rtnl_lock is
+		 * not held, schedule a worker thread to hold rtnl_lock
+		 * while setting attr.
+		 */
+
+		return swdev_port_attr_set_defer(dev, attr);
+	}
+
+	/* Phase I: prepare for attr set. 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
+	 * should reserve resources needed for the commit phase here,
+	 * but should not commit the attr.
+	 */
+
+	attr->trans = SWDEV_TRANS_PREPARE;
+	err = __swdev_port_attr_set(dev, attr);
+	if (err) {
+		/* Prepare phase failed: abort the transaction.  Any
+		 * resources reserved in the prepare phase are
+		 * released.
+		 */
+
+		attr->trans = SWDEV_TRANS_ABORT;
+		__swdev_port_attr_set(dev, attr);
+
+		return err;
+	}
+
+	/* Phase II: commit attr set.  This cannot fail as a fault
+	 * of driver/device.  If it does, it's a bug in the driver/device
+	 * because the driver said everythings was OK in phase I.
+	 */
+
+	attr->trans = SWDEV_TRANS_COMMIT;
+	err = __swdev_port_attr_set(dev, attr);
+	WARN(err, "%s: Commit of attr (id=%d) failed.\n", dev->name, attr->id);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(swdev_port_attr_set);
+
+/**
  *	netdev_switch_port_stp_update - Notify switch device port of STP
  *					state change
  *	@dev: port device
-- 
1.7.10.4

[PATCH net-next v4 02/24] switchdev: convert parent_id_get to swdev attr get

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

Switch ID is just a gettable port attribute.  Convert swdev op
swdev_parent_id_get to a swdev attr.

Note: for sysfs and netlink interfaces, SWDEV_ATTR_PORT_PARENT_ID is called
with SWDEV_F_NO_RECUSE to limit switch ID user-visiblity to only port
netdevs.  So port is stacked under bond/bridge, the user can only see switch
ID for port.

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 drivers/net/ethernet/rocker/rocker.c |   16 +++++++++-----
 include/net/switchdev.h              |   18 ++++------------
 net/core/net-sysfs.c                 |   10 ++++++---
 net/core/rtnetlink.c                 |    9 +++++---
 net/dsa/slave.c                      |   16 +++++++++-----
 net/switchdev/switchdev.c            |   38 ++++++++++------------------------
 6 files changed, 50 insertions(+), 57 deletions(-)

diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index a87b177..1c468df 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -4220,14 +4220,20 @@ static const struct net_device_ops rocker_port_netdev_ops = {
  * swdev interface
  ********************/
 
-static int rocker_port_swdev_parent_id_get(struct net_device *dev,
-					   struct netdev_phys_item_id *psid)
+static int rocker_port_attr_get(struct net_device *dev, struct swdev_attr *attr)
 {
 	struct rocker_port *rocker_port = netdev_priv(dev);
 	struct rocker *rocker = rocker_port->rocker;
 
-	psid->id_len = sizeof(rocker->hw.id);
-	memcpy(&psid->id, &rocker->hw.id, psid->id_len);
+	switch (attr->id) {
+	case SWDEV_ATTR_PORT_PARENT_ID:
+		attr->ppid.id_len = sizeof(rocker->hw.id);
+		memcpy(&attr->ppid.id, &rocker->hw.id, attr->ppid.id_len);
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
 	return 0;
 }
 
@@ -4264,7 +4270,7 @@ static int rocker_port_swdev_fib_ipv4_del(struct net_device *dev,
 }
 
 static const struct swdev_ops rocker_port_swdev_ops = {
-	.swdev_parent_id_get		= rocker_port_swdev_parent_id_get,
+	.swdev_port_attr_get		= rocker_port_attr_get,
 	.swdev_port_stp_update		= rocker_port_swdev_port_stp_update,
 	.swdev_fib_ipv4_add		= rocker_port_swdev_fib_ipv4_add,
 	.swdev_fib_ipv4_del		= rocker_port_swdev_fib_ipv4_del,
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 1d45621..dcacd54 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -25,12 +25,16 @@ enum swdev_trans {
 
 enum swdev_attr_id {
 	SWDEV_ATTR_UNDEFINED,
+	SWDEV_ATTR_PORT_PARENT_ID,
 };
 
 struct swdev_attr {
 	enum swdev_attr_id id;
 	enum swdev_trans trans;
 	u32 flags;
+	union {
+		struct netdev_phys_item_id ppid;	/* PORT_PARENT_ID */
+	};
 };
 
 struct fib_info;
@@ -38,10 +42,6 @@ struct fib_info;
 /**
  * struct switchdev_ops - switchdev operations
  *
- * @swdev_parent_id_get: Called to get an ID of the switch chip this port
- *   is part of.  If driver implements this, it indicates that it
- *   represents a port of a switch chip.
- *
  * @swdev_port_attr_get: Get a port attribute (see swdev_attr).
  *
  * @swdev_port_attr_set: Set a port attribute (see swdev_attr).
@@ -54,8 +54,6 @@ struct fib_info;
  * @swdev_fib_ipv4_del: Called to delete IPv4 route from switch device.
  */
 struct swdev_ops {
-	int	(*swdev_parent_id_get)(struct net_device *dev,
-				       struct netdev_phys_item_id *psid);
 	int	(*swdev_port_attr_get)(struct net_device *dev,
 				       struct swdev_attr *attr);
 	int	(*swdev_port_attr_set)(struct net_device *dev,
@@ -93,8 +91,6 @@ netdev_switch_notifier_info_to_dev(const struct netdev_switch_notifier_info *inf
 
 #ifdef CONFIG_NET_SWITCHDEV
 
-int netdev_switch_parent_id_get(struct net_device *dev,
-				struct netdev_phys_item_id *psid);
 int swdev_port_attr_get(struct net_device *dev, struct swdev_attr *attr);
 int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr);
 int netdev_switch_port_stp_update(struct net_device *dev, u8 state);
@@ -118,12 +114,6 @@ void netdev_switch_fib_ipv4_abort(struct fib_info *fi);
 
 #else
 
-static inline int netdev_switch_parent_id_get(struct net_device *dev,
-					      struct netdev_phys_item_id *psid)
-{
-	return -EOPNOTSUPP;
-}
-
 static inline int swdev_port_attr_get(struct net_device *dev,
 				      enum swdev_attr *attr)
 {
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 4238d6d..5e83d50 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -458,11 +458,15 @@ static ssize_t phys_switch_id_show(struct device *dev,
 		return restart_syscall();
 
 	if (dev_isalive(netdev)) {
-		struct netdev_phys_item_id ppid;
+		struct swdev_attr attr = {
+			.id = SWDEV_ATTR_PORT_PARENT_ID,
+			.flags = SWDEV_F_NO_RECURSE,
+		};
 
-		ret = netdev_switch_parent_id_get(netdev, &ppid);
+		ret = swdev_port_attr_get(netdev, &attr);
 		if (!ret)
-			ret = sprintf(buf, "%*phN\n", ppid.id_len, ppid.id);
+			ret = sprintf(buf, "%*phN\n", attr.ppid.id_len,
+				      attr.ppid.id);
 	}
 	rtnl_unlock();
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 358d52a..756f1cf 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1004,16 +1004,19 @@ static int rtnl_phys_port_name_fill(struct sk_buff *skb, struct net_device *dev)
 static int rtnl_phys_switch_id_fill(struct sk_buff *skb, struct net_device *dev)
 {
 	int err;
-	struct netdev_phys_item_id psid;
+	struct swdev_attr attr = {
+		.id = SWDEV_ATTR_PORT_PARENT_ID,
+		.flags = SWDEV_F_NO_RECURSE,
+	};
 
-	err = netdev_switch_parent_id_get(dev, &psid);
+	err = swdev_port_attr_get(dev, &attr);
 	if (err) {
 		if (err == -EOPNOTSUPP)
 			return 0;
 		return err;
 	}
 
-	if (nla_put(skb, IFLA_PHYS_SWITCH_ID, psid.id_len, psid.id))
+	if (nla_put(skb, IFLA_PHYS_SWITCH_ID, attr.ppid.id_len, attr.ppid.id))
 		return -EMSGSIZE;
 
 	return 0;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 827cda56..6ff2261 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -382,14 +382,20 @@ static int dsa_slave_bridge_port_leave(struct net_device *dev)
 	return ret;
 }
 
-static int dsa_slave_parent_id_get(struct net_device *dev,
-				   struct netdev_phys_item_id *psid)
+static int dsa_slave_port_attr_get(struct net_device *dev,
+				   struct swdev_attr *attr)
 {
 	struct dsa_slave_priv *p = netdev_priv(dev);
 	struct dsa_switch *ds = p->parent;
 
-	psid->id_len = sizeof(ds->index);
-	memcpy(&psid->id, &ds->index, psid->id_len);
+	switch (attr->id) {
+	case SWDEV_ATTR_PORT_PARENT_ID:
+		attr->ppid.id_len = sizeof(ds->index);
+		memcpy(&attr->ppid.id, &ds->index, attr->ppid.id_len);
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
 
 	return 0;
 }
@@ -676,7 +682,7 @@ static const struct net_device_ops dsa_slave_netdev_ops = {
 };
 
 static const struct swdev_ops dsa_slave_swdev_ops = {
-	.swdev_parent_id_get = dsa_slave_parent_id_get,
+	.swdev_port_attr_get = dsa_slave_port_attr_get,
 	.swdev_port_stp_update = dsa_slave_stp_update,
 };
 
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index db0cc7b..7d94724 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -19,24 +19,6 @@
 #include <net/switchdev.h>
 
 /**
- *	netdev_switch_parent_id_get - Get ID of a switch
- *	@dev: port device
- *	@psid: switch ID
- *
- *	Get ID of a switch this port is part of.
- */
-int netdev_switch_parent_id_get(struct net_device *dev,
-				struct netdev_phys_item_id *psid)
-{
-	const struct swdev_ops *ops = dev->swdev_ops;
-
-	if (!ops || !ops->swdev_parent_id_get)
-		return -EOPNOTSUPP;
-	return ops->swdev_parent_id_get(dev, psid);
-}
-EXPORT_SYMBOL_GPL(netdev_switch_parent_id_get);
-
-/**
  *	swdev_port_attr_get - Get port attribute
  *
  *	@dev: port device
@@ -421,11 +403,10 @@ static struct net_device *netdev_switch_get_lowest_dev(struct net_device *dev)
 	struct list_head *iter;
 
 	/* Recusively search down until we find a sw port dev.
-	 * (A sw port dev supports swdev_parent_id_get).
+	 * (A sw port dev supports swdev_port_attr_get).
 	 */
 
-	if (dev->features & NETIF_F_HW_SWITCH_OFFLOAD &&
-	    ops && ops->swdev_parent_id_get)
+	if (ops && ops->swdev_port_attr_get)
 		return dev;
 
 	netdev_for_each_lower_dev(dev, lower_dev, iter) {
@@ -439,8 +420,10 @@ static struct net_device *netdev_switch_get_lowest_dev(struct net_device *dev)
 
 static struct net_device *netdev_switch_get_dev_by_nhs(struct fib_info *fi)
 {
-	struct netdev_phys_item_id psid;
-	struct netdev_phys_item_id prev_psid;
+	struct swdev_attr attr = {
+		.id = SWDEV_ATTR_PORT_PARENT_ID,
+	};
+	struct swdev_attr prev_attr;
 	struct net_device *dev = NULL;
 	int nhsel;
 
@@ -456,17 +439,18 @@ static struct net_device *netdev_switch_get_dev_by_nhs(struct fib_info *fi)
 		if (!dev)
 			return NULL;
 
-		if (netdev_switch_parent_id_get(dev, &psid))
+		if (swdev_port_attr_get(dev, &attr))
 			return NULL;
 
 		if (nhsel > 0) {
-			if (prev_psid.id_len != psid.id_len)
+			if (prev_attr.ppid.id_len != attr.ppid.id_len)
 				return NULL;
-			if (memcmp(prev_psid.id, psid.id, psid.id_len))
+			if (memcmp(prev_attr.ppid.id, attr.ppid.id,
+				   attr.ppid.id_len))
 				return NULL;
 		}
 
-		prev_psid = psid;
+		prev_attr = attr;
 	}
 
 	return dev;
-- 
1.7.10.4

[PATCH net-next v4 03/24] switchdev: convert STP update to swdev attr set

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

STP update is just a writable port attribute, so convert swdev_port_stp_update
to an attr set.

For rocker, support prepare-commit transaction model for setting STP state.
This requires rocker to preallocate memory needed for the commit up front in
the prepare phase.  Since rtnl_lock is held between prepare-commit, store the
allocated memory on a queue hanging off of the rocker_port.  Also, in prepare
phase, do everything right up to calling into HW.  The same code paths are
tranversed in the driver for both prepare and commit phases.  In some cases,
any state modified in the prepare phase must be reverted before returning
so the commit phase makes the same decisions.

For DSA, the prepare phase is skipped and STP updates are only done in the
commit phase.  This is because currently the DSA drivers don't need to allocate
any memory for STP updates and the STP update will not fail to HW (unless
something horrible goes wrong on the MDIO bus, in which case the prepare phase
wouldn't have been able to predict anyway).

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 drivers/net/ethernet/rocker/rocker.c |  244 ++++++++++++++++++++++++++--------
 include/net/switchdev.h              |   13 +-
 net/bridge/br_stp.c                  |    6 +-
 net/dsa/slave.c                      |   20 ++-
 net/switchdev/switchdev.c            |   28 ----
 5 files changed, 216 insertions(+), 95 deletions(-)

diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index 1c468df..80c7f6f 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -225,6 +225,8 @@ struct rocker_port {
 	struct napi_struct napi_rx;
 	struct rocker_dma_ring_info tx_ring;
 	struct rocker_dma_ring_info rx_ring;
+	enum swdev_trans trans;
+	struct list_head trans_mem;
 };
 
 struct rocker {
@@ -325,6 +327,72 @@ static bool rocker_port_is_bridged(struct rocker_port *rocker_port)
 	return !!rocker_port->bridge_dev;
 }
 
+static void *__rocker_port_alloc(struct rocker_port *rocker_port,
+				 size_t size, gfp_t flags,
+				 void *(*alloc)(size_t size, gfp_t flags))
+{
+	struct list_head *elem = NULL;
+
+	/* If in transaction prepare phase, allocate the memory
+	 * and enqueue it on a per-port list.  If in transaction
+	 * commit phase, dequeue the memory from the per-port list
+	 * rather than re-allocating the memory.  The idea is the
+	 * driver code paths for prepare and commit are identical
+	 * so the memory allocated in the prepare phase is the
+	 * memory used in the commit phase.
+	 */
+
+	switch (rocker_port->trans) {
+	case SWDEV_TRANS_PREPARE:
+		elem = alloc(size + sizeof(*elem), flags);
+		if (!elem)
+			return NULL;
+		list_add_tail(elem, &rocker_port->trans_mem);
+		break;
+	case SWDEV_TRANS_COMMIT:
+		BUG_ON(list_empty(&rocker_port->trans_mem));
+		elem = rocker_port->trans_mem.next;
+		list_del_init(elem);
+		break;
+	case SWDEV_TRANS_NONE:
+		elem = alloc(size + sizeof(*elem), flags);
+		if (elem)
+			INIT_LIST_HEAD(elem);
+		break;
+	}
+
+	return elem ? elem + 1 : NULL;
+}
+
+static void *rocker_port_kzalloc(struct rocker_port *rocker_port,
+				 size_t size, gfp_t flags)
+{
+	return __rocker_port_alloc(rocker_port, size, flags, kzalloc);
+}
+
+static void *rocker_port_kcalloc(struct rocker_port *rocker_port,
+				 size_t n, size_t size, gfp_t flags)
+{
+	return __rocker_port_alloc(rocker_port, n * size, flags, kzalloc);
+}
+
+static void rocker_port_kfree(struct rocker_port *rocker_port, const void *mem)
+{
+	struct list_head *elem;
+
+	/* Frees are ignored if in transaction prepare phase.  The
+	 * memory remains on the per-port list until freed in the
+	 * commit phase.
+	 */
+
+	if (rocker_port->trans == SWDEV_TRANS_PREPARE)
+		return;
+
+	elem = (struct list_head *)mem - 1;
+	BUG_ON(!list_empty(elem));
+	kfree(elem);
+}
+
 struct rocker_wait {
 	wait_queue_head_t wait;
 	bool done;
@@ -354,9 +422,9 @@ static struct rocker_wait *rocker_wait_create(gfp_t gfp)
 	return wait;
 }
 
-static void rocker_wait_destroy(struct rocker_wait *work)
+static void rocker_wait_destroy(struct rocker_wait *wait)
 {
-	kfree(work);
+	kfree(wait);
 }
 
 static bool rocker_wait_event_timeout(struct rocker_wait *wait,
@@ -2325,7 +2393,7 @@ static int rocker_flow_tbl_add(struct rocker_port *rocker_port,
 	if (found) {
 		match->cookie = found->cookie;
 		hash_del(&found->entry);
-		kfree(found);
+		rocker_port_kfree(rocker_port, found);
 		found = match;
 		found->cmd = ROCKER_TLV_CMD_TYPE_OF_DPA_FLOW_MOD;
 	} else {
@@ -2366,13 +2434,13 @@ static int rocker_flow_tbl_del(struct rocker_port *rocker_port,
 
 	spin_unlock_irqrestore(&rocker->flow_tbl_lock, flags);
 
-	kfree(match);
+	rocker_port_kfree(rocker_port, match);
 
 	if (found) {
 		err = rocker_cmd_exec(rocker, rocker_port,
 				      rocker_cmd_flow_tbl_del,
 				      found, NULL, NULL, nowait);
-		kfree(found);
+		rocker_port_kfree(rocker_port, found);
 	}
 
 	return err;
@@ -2388,6 +2456,9 @@ static int rocker_flow_tbl_do(struct rocker_port *rocker_port,
 {
 	bool nowait = flags & ROCKER_OP_FLAG_NOWAIT;
 
+	if (rocker_port->trans == SWDEV_TRANS_PREPARE)
+		return 0;
+
 	if (flags & ROCKER_OP_FLAG_REMOVE)
 		return rocker_flow_tbl_del(rocker_port, entry, nowait);
 	else
@@ -2400,7 +2471,8 @@ static int rocker_flow_tbl_ig_port(struct rocker_port *rocker_port,
 {
 	struct rocker_flow_tbl_entry *entry;
 
-	entry = kzalloc(sizeof(*entry), rocker_op_flags_gfp(flags));
+	entry = rocker_port_kzalloc(rocker_port, sizeof(*entry),
+				    rocker_op_flags_gfp(flags));
 	if (!entry)
 		return -ENOMEM;
 
@@ -2421,7 +2493,8 @@ static int rocker_flow_tbl_vlan(struct rocker_port *rocker_port,
 {
 	struct rocker_flow_tbl_entry *entry;
 
-	entry = kzalloc(sizeof(*entry), rocker_op_flags_gfp(flags));
+	entry = rocker_port_kzalloc(rocker_port, sizeof(*entry),
+				    rocker_op_flags_gfp(flags));
 	if (!entry)
 		return -ENOMEM;
 
@@ -2447,7 +2520,8 @@ static int rocker_flow_tbl_term_mac(struct rocker_port *rocker_port,
 {
 	struct rocker_flow_tbl_entry *entry;
 
-	entry = kzalloc(sizeof(*entry), rocker_op_flags_gfp(flags));
+	entry = rocker_port_kzalloc(rocker_port, sizeof(*entry),
+				    rocker_op_flags_gfp(flags));
 	if (!entry)
 		return -ENOMEM;
 
@@ -2487,7 +2561,8 @@ static int rocker_flow_tbl_bridge(struct rocker_port *rocker_port,
 	bool dflt = !eth_dst || (eth_dst && eth_dst_mask);
 	bool wild = false;
 
-	entry = kzalloc(sizeof(*entry), rocker_op_flags_gfp(flags));
+	entry = rocker_port_kzalloc(rocker_port, sizeof(*entry),
+				    rocker_op_flags_gfp(flags));
 	if (!entry)
 		return -ENOMEM;
 
@@ -2536,7 +2611,8 @@ static int rocker_flow_tbl_ucast4_routing(struct rocker_port *rocker_port,
 {
 	struct rocker_flow_tbl_entry *entry;
 
-	entry = kzalloc(sizeof(*entry), rocker_op_flags_gfp(flags));
+	entry = rocker_port_kzalloc(rocker_port, sizeof(*entry),
+				    rocker_op_flags_gfp(flags));
 	if (!entry)
 		return -ENOMEM;
 
@@ -2567,7 +2643,8 @@ static int rocker_flow_tbl_acl(struct rocker_port *rocker_port,
 	u32 priority;
 	struct rocker_flow_tbl_entry *entry;
 
-	entry = kzalloc(sizeof(*entry), rocker_op_flags_gfp(flags));
+	entry = rocker_port_kzalloc(rocker_port, sizeof(*entry),
+				    rocker_op_flags_gfp(flags));
 	if (!entry)
 		return -ENOMEM;
 
@@ -2620,17 +2697,18 @@ rocker_group_tbl_find(struct rocker *rocker,
 	return NULL;
 }
 
-static void rocker_group_tbl_entry_free(struct rocker_group_tbl_entry *entry)
+static void rocker_group_tbl_entry_free(struct rocker_port *rocker_port,
+					struct rocker_group_tbl_entry *entry)
 {
 	switch (ROCKER_GROUP_TYPE_GET(entry->group_id)) {
 	case ROCKER_OF_DPA_GROUP_TYPE_L2_FLOOD:
 	case ROCKER_OF_DPA_GROUP_TYPE_L2_MCAST:
-		kfree(entry->group_ids);
+		rocker_port_kfree(rocker_port, entry->group_ids);
 		break;
 	default:
 		break;
 	}
-	kfree(entry);
+	rocker_port_kfree(rocker_port, entry);
 }
 
 static int rocker_group_tbl_add(struct rocker_port *rocker_port,
@@ -2647,7 +2725,7 @@ static int rocker_group_tbl_add(struct rocker_port *rocker_port,
 
 	if (found) {
 		hash_del(&found->entry);
-		rocker_group_tbl_entry_free(found);
+		rocker_group_tbl_entry_free(rocker_port, found);
 		found = match;
 		found->cmd = ROCKER_TLV_CMD_TYPE_OF_DPA_GROUP_MOD;
 	} else {
@@ -2684,13 +2762,13 @@ static int rocker_group_tbl_del(struct rocker_port *rocker_port,
 
 	spin_unlock_irqrestore(&rocker->group_tbl_lock, flags);
 
-	rocker_group_tbl_entry_free(match);
+	rocker_group_tbl_entry_free(rocker_port, match);
 
 	if (found) {
 		err = rocker_cmd_exec(rocker, rocker_port,
 				      rocker_cmd_group_tbl_del,
 				      found, NULL, NULL, nowait);
-		rocker_group_tbl_entry_free(found);
+		rocker_group_tbl_entry_free(rocker_port, found);
 	}
 
 	return err;
@@ -2701,6 +2779,9 @@ static int rocker_group_tbl_do(struct rocker_port *rocker_port,
 {
 	bool nowait = flags & ROCKER_OP_FLAG_NOWAIT;
 
+	if (rocker_port->trans == SWDEV_TRANS_PREPARE)
+		return 0;
+
 	if (flags & ROCKER_OP_FLAG_REMOVE)
 		return rocker_group_tbl_del(rocker_port, entry, nowait);
 	else
@@ -2713,7 +2794,8 @@ static int rocker_group_l2_interface(struct rocker_port *rocker_port,
 {
 	struct rocker_group_tbl_entry *entry;
 
-	entry = kzalloc(sizeof(*entry), rocker_op_flags_gfp(flags));
+	entry = rocker_port_kzalloc(rocker_port, sizeof(*entry),
+				    rocker_op_flags_gfp(flags));
 	if (!entry)
 		return -ENOMEM;
 
@@ -2729,17 +2811,19 @@ static int rocker_group_l2_fan_out(struct rocker_port *rocker_port,
 {
 	struct rocker_group_tbl_entry *entry;
 
-	entry = kzalloc(sizeof(*entry), rocker_op_flags_gfp(flags));
+	entry = rocker_port_kzalloc(rocker_port, sizeof(*entry),
+				    rocker_op_flags_gfp(flags));
 	if (!entry)
 		return -ENOMEM;
 
 	entry->group_id = group_id;
 	entry->group_count = group_count;
 
-	entry->group_ids = kcalloc(group_count, sizeof(u32),
-				   rocker_op_flags_gfp(flags));
+	entry->group_ids = rocker_port_kcalloc(rocker_port, group_count,
+					       sizeof(u32),
+					       rocker_op_flags_gfp(flags));
 	if (!entry->group_ids) {
-		kfree(entry);
+		rocker_port_kfree(rocker_port, entry);
 		return -ENOMEM;
 	}
 	memcpy(entry->group_ids, group_ids, group_count * sizeof(u32));
@@ -2764,7 +2848,8 @@ static int rocker_group_l3_unicast(struct rocker_port *rocker_port,
 {
 	struct rocker_group_tbl_entry *entry;
 
-	entry = kzalloc(sizeof(*entry), rocker_op_flags_gfp(flags));
+	entry = rocker_port_kzalloc(rocker_port, sizeof(*entry),
+				    rocker_op_flags_gfp(flags));
 	if (!entry)
 		return -ENOMEM;
 
@@ -2802,17 +2887,16 @@ static void _rocker_neigh_add(struct rocker *rocker,
 		 be32_to_cpu(entry->ip_addr));
 }
 
-static void _rocker_neigh_del(struct rocker *rocker,
+static void _rocker_neigh_del(struct rocker_port *rocker_port,
 			      struct rocker_neigh_tbl_entry *entry)
 {
 	if (--entry->ref_count == 0) {
 		hash_del(&entry->entry);
-		kfree(entry);
+		rocker_port_kfree(rocker_port, entry);
 	}
 }
 
-static void _rocker_neigh_update(struct rocker *rocker,
-				 struct rocker_neigh_tbl_entry *entry,
+static void _rocker_neigh_update(struct rocker_neigh_tbl_entry *entry,
 				 u8 *eth_dst, bool ttl_check)
 {
 	if (eth_dst) {
@@ -2840,7 +2924,8 @@ static int rocker_port_ipv4_neigh(struct rocker_port *rocker_port,
 	bool removing;
 	int err = 0;
 
-	entry = kzalloc(sizeof(*entry), rocker_op_flags_gfp(flags));
+	entry = rocker_port_kzalloc(rocker_port, sizeof(*entry),
+				    rocker_op_flags_gfp(flags));
 	if (!entry)
 		return -ENOMEM;
 
@@ -2860,9 +2945,9 @@ static int rocker_port_ipv4_neigh(struct rocker_port *rocker_port,
 		_rocker_neigh_add(rocker, entry);
 	} else if (removing) {
 		memcpy(entry, found, sizeof(*entry));
-		_rocker_neigh_del(rocker, found);
+		_rocker_neigh_del(rocker_port, found);
 	} else if (updating) {
-		_rocker_neigh_update(rocker, found, eth_dst, true);
+		_rocker_neigh_update(found, eth_dst, true);
 		memcpy(entry, found, sizeof(*entry));
 	} else {
 		err = -ENOENT;
@@ -2909,7 +2994,7 @@ static int rocker_port_ipv4_neigh(struct rocker_port *rocker_port,
 
 err_out:
 	if (!adding)
-		kfree(entry);
+		rocker_port_kfree(rocker_port, entry);
 
 	return err;
 }
@@ -2952,7 +3037,8 @@ static int rocker_port_ipv4_nh(struct rocker_port *rocker_port, int flags,
 	bool resolved = true;
 	int err = 0;
 
-	entry = kzalloc(sizeof(*entry), rocker_op_flags_gfp(flags));
+	entry = rocker_port_kzalloc(rocker_port, sizeof(*entry),
+				    rocker_op_flags_gfp(flags));
 	if (!entry)
 		return -ENOMEM;
 
@@ -2973,9 +3059,9 @@ static int rocker_port_ipv4_nh(struct rocker_port *rocker_port, int flags,
 		*index = entry->index;
 		resolved = false;
 	} else if (removing) {
-		_rocker_neigh_del(rocker, found);
+		_rocker_neigh_del(rocker_port, found);
 	} else if (updating) {
-		_rocker_neigh_update(rocker, found, NULL, false);
+		_rocker_neigh_update(found, NULL, false);
 		resolved = !is_zero_ether_addr(found->eth_dst);
 	} else {
 		err = -ENOENT;
@@ -2984,7 +3070,7 @@ static int rocker_port_ipv4_nh(struct rocker_port *rocker_port, int flags,
 	spin_unlock_irqrestore(&rocker->neigh_tbl_lock, lock_flags);
 
 	if (!adding)
-		kfree(entry);
+		rocker_port_kfree(rocker_port, entry);
 
 	if (err)
 		return err;
@@ -3008,8 +3094,9 @@ static int rocker_port_vlan_flood_group(struct rocker_port *rocker_port,
 	int err = 0;
 	int i;
 
-	group_ids = kcalloc(rocker->port_count, sizeof(u32),
-			    rocker_op_flags_gfp(flags));
+	group_ids = rocker_port_kcalloc(rocker_port, rocker->port_count,
+					sizeof(u32),
+					rocker_op_flags_gfp(flags));
 	if (!group_ids)
 		return -ENOMEM;
 
@@ -3040,7 +3127,7 @@ static int rocker_port_vlan_flood_group(struct rocker_port *rocker_port,
 			   "Error (%d) port VLAN l2 flood group\n", err);
 
 no_ports_in_vlan:
-	kfree(group_ids);
+	rocker_port_kfree(rocker_port, group_ids);
 	return err;
 }
 
@@ -3460,7 +3547,8 @@ static int rocker_port_fdb(struct rocker_port *rocker_port,
 	bool removing = (flags & ROCKER_OP_FLAG_REMOVE);
 	unsigned long lock_flags;
 
-	fdb = kzalloc(sizeof(*fdb), rocker_op_flags_gfp(flags));
+	fdb = rocker_port_kzalloc(rocker_port, sizeof(*fdb),
+				  rocker_op_flags_gfp(flags));
 	if (!fdb)
 		return -ENOMEM;
 
@@ -3475,7 +3563,7 @@ static int rocker_port_fdb(struct rocker_port *rocker_port,
 	found = rocker_fdb_tbl_find(rocker, fdb);
 
 	if (removing && found) {
-		kfree(fdb);
+		rocker_port_kfree(rocker_port, fdb);
 		hash_del(&found->entry);
 	} else if (!removing && !found) {
 		hash_add(rocker->fdb_tbl, &fdb->entry, fdb->key_crc32);
@@ -3485,7 +3573,7 @@ static int rocker_port_fdb(struct rocker_port *rocker_port,
 
 	/* Check if adding and already exists, or removing and can't find */
 	if (!found != !removing) {
-		kfree(fdb);
+		rocker_port_kfree(rocker_port, fdb);
 		if (!found && removing)
 			return 0;
 		/* Refreshing existing to update aging timers */
@@ -3500,7 +3588,7 @@ static int rocker_port_fdb_flush(struct rocker_port *rocker_port)
 	struct rocker *rocker = rocker_port->rocker;
 	struct rocker_fdb_tbl_entry *found;
 	unsigned long lock_flags;
-	int flags = ROCKER_OP_FLAG_NOWAIT | ROCKER_OP_FLAG_REMOVE;
+	int flags = ROCKER_OP_FLAG_REMOVE;
 	struct hlist_node *tmp;
 	int bkt;
 	int err = 0;
@@ -3568,7 +3656,7 @@ static int rocker_port_fwding(struct rocker_port *rocker_port)
 	u32 out_pport;
 	__be16 vlan_id;
 	u16 vid;
-	int flags = ROCKER_OP_FLAG_NOWAIT;
+	int flags = 0;
 	int err;
 
 	/* Port will be forwarding-enabled if its STP state is LEARNING
@@ -3605,10 +3693,17 @@ static int rocker_port_fwding(struct rocker_port *rocker_port)
 static int rocker_port_stp_update(struct rocker_port *rocker_port, u8 state)
 {
 	bool want[ROCKER_CTRL_MAX] = { 0, };
+	bool prev_ctrls[ROCKER_CTRL_MAX];
+	u8 prev_state;
 	int flags;
 	int err;
 	int i;
 
+	if (rocker_port->trans == SWDEV_TRANS_PREPARE) {
+		memcpy(prev_ctrls, rocker_port->ctrls, sizeof(prev_ctrls));
+		prev_state = rocker_port->stp_state;
+	}
+
 	if (rocker_port->stp_state == state)
 		return 0;
 
@@ -3636,21 +3731,28 @@ static int rocker_port_stp_update(struct rocker_port *rocker_port, u8 state)
 
 	for (i = 0; i < ROCKER_CTRL_MAX; i++) {
 		if (want[i] != rocker_port->ctrls[i]) {
-			flags = ROCKER_OP_FLAG_NOWAIT |
-				(want[i] ? 0 : ROCKER_OP_FLAG_REMOVE);
+			flags = (want[i] ? 0 : ROCKER_OP_FLAG_REMOVE);
 			err = rocker_port_ctrl(rocker_port, flags,
 					       &rocker_ctrls[i]);
 			if (err)
-				return err;
+				goto err_out;
 			rocker_port->ctrls[i] = want[i];
 		}
 	}
 
 	err = rocker_port_fdb_flush(rocker_port);
 	if (err)
-		return err;
+		goto err_out;
 
-	return rocker_port_fwding(rocker_port);
+	err = rocker_port_fwding(rocker_port);
+
+err_out:
+	if (rocker_port->trans == SWDEV_TRANS_PREPARE) {
+		memcpy(rocker_port->ctrls, prev_ctrls, sizeof(prev_ctrls));
+		rocker_port->stp_state = prev_state;
+	}
+
+	return err;
 }
 
 static int rocker_port_fwd_enable(struct rocker_port *rocker_port)
@@ -3696,7 +3798,7 @@ static __be16 rocker_port_internal_vlan_id_get(struct rocker_port *rocker_port,
 	unsigned long lock_flags;
 	int i;
 
-	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	entry = rocker_port_kzalloc(rocker_port, sizeof(*entry), GFP_KERNEL);
 	if (!entry)
 		return 0;
 
@@ -3706,7 +3808,7 @@ static __be16 rocker_port_internal_vlan_id_get(struct rocker_port *rocker_port,
 
 	found = rocker_internal_vlan_tbl_find(rocker, ifindex);
 	if (found) {
-		kfree(entry);
+		rocker_port_kfree(rocker_port, entry);
 		goto found;
 	}
 
@@ -3751,7 +3853,7 @@ static void rocker_port_internal_vlan_id_put(struct rocker_port *rocker_port,
 		bit = ntohs(found->vlan_id) - ROCKER_INTERNAL_VLAN_ID_BASE;
 		clear_bit(bit, rocker->internal_vlan_bitmap);
 		hash_del(&found->entry);
-		kfree(found);
+		rocker_port_kfree(rocker_port, found);
 	}
 
 not_found:
@@ -4237,11 +4339,44 @@ static int rocker_port_attr_get(struct net_device *dev, struct swdev_attr *attr)
 	return 0;
 }
 
-static int rocker_port_swdev_port_stp_update(struct net_device *dev, u8 state)
+static void rocker_port_trans_abort(struct rocker_port *rocker_port)
+{
+	struct list_head *mem, *tmp;
+
+	list_for_each_safe(mem, tmp, &rocker_port->trans_mem) {
+		list_del(mem);
+		kfree(mem);
+	}
+}
+
+static int rocker_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
 {
 	struct rocker_port *rocker_port = netdev_priv(dev);
+	int err = 0;
+
+	rocker_port->trans = attr->trans;
+
+	switch (rocker_port->trans) {
+	case SWDEV_TRANS_PREPARE:
+		BUG_ON(!list_empty(&rocker_port->trans_mem));
+		break;
+	case SWDEV_TRANS_ABORT:
+		rocker_port_trans_abort(rocker_port);
+		return 0;
+	}
 
-	return rocker_port_stp_update(rocker_port, state);
+	switch (attr->id) {
+	case SWDEV_ATTR_PORT_STP_STATE:
+		err = rocker_port_stp_update(rocker_port, attr->stp_state);
+		break;
+	default:
+		err = -EOPNOTSUPP;
+		break;
+	}
+
+	rocker_port->trans = SWDEV_TRANS_NONE;
+
+	return err;
 }
 
 static int rocker_port_swdev_fib_ipv4_add(struct net_device *dev,
@@ -4271,7 +4406,7 @@ static int rocker_port_swdev_fib_ipv4_del(struct net_device *dev,
 
 static const struct swdev_ops rocker_port_swdev_ops = {
 	.swdev_port_attr_get		= rocker_port_attr_get,
-	.swdev_port_stp_update		= rocker_port_swdev_port_stp_update,
+	.swdev_port_attr_set		= rocker_port_attr_set,
 	.swdev_fib_ipv4_add		= rocker_port_swdev_fib_ipv4_add,
 	.swdev_fib_ipv4_del		= rocker_port_swdev_fib_ipv4_del,
 };
@@ -4624,6 +4759,7 @@ static int rocker_probe_port(struct rocker *rocker, unsigned int port_number)
 	rocker_port->port_number = port_number;
 	rocker_port->pport = port_number + 1;
 	rocker_port->brport_flags = BR_LEARNING | BR_LEARNING_SYNC;
+	INIT_LIST_HEAD(&rocker_port->trans_mem);
 
 	rocker_port_dev_addr_init(rocker, rocker_port);
 	dev->netdev_ops = &rocker_port_netdev_ops;
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index dcacd54..fa553ee 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -26,6 +26,7 @@ enum swdev_trans {
 enum swdev_attr_id {
 	SWDEV_ATTR_UNDEFINED,
 	SWDEV_ATTR_PORT_PARENT_ID,
+	SWDEV_ATTR_PORT_STP_STATE,
 };
 
 struct swdev_attr {
@@ -34,6 +35,7 @@ struct swdev_attr {
 	u32 flags;
 	union {
 		struct netdev_phys_item_id ppid;	/* PORT_PARENT_ID */
+		u8 stp_state;				/* PORT_STP_STATE */
 	};
 };
 
@@ -46,9 +48,6 @@ struct fib_info;
  *
  * @swdev_port_attr_set: Set a port attribute (see swdev_attr).
  *
- * @swdev_port_stp_update: Called to notify switch device port of bridge
- *   port STP state change.
- *
  * @swdev_fib_ipv4_add: Called to add/modify IPv4 route to switch device.
  *
  * @swdev_fib_ipv4_del: Called to delete IPv4 route from switch device.
@@ -58,7 +57,6 @@ struct swdev_ops {
 				       struct swdev_attr *attr);
 	int	(*swdev_port_attr_set)(struct net_device *dev,
 				       struct swdev_attr *attr);
-	int	(*swdev_port_stp_update)(struct net_device *dev, u8 state);
 	int	(*swdev_fib_ipv4_add)(struct net_device *dev, __be32 dst,
 				      int dst_len, struct fib_info *fi,
 				      u8 tos, u8 type, u32 nlflags,
@@ -93,7 +91,6 @@ netdev_switch_notifier_info_to_dev(const struct netdev_switch_notifier_info *inf
 
 int swdev_port_attr_get(struct net_device *dev, struct swdev_attr *attr);
 int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr);
-int netdev_switch_port_stp_update(struct net_device *dev, u8 state);
 int register_netdev_switch_notifier(struct notifier_block *nb);
 int unregister_netdev_switch_notifier(struct notifier_block *nb);
 int call_netdev_switch_notifiers(unsigned long val, struct net_device *dev,
@@ -126,12 +123,6 @@ static inline int swdev_port_attr_set(struct net_device *dev,
 	return -EOPNOTSUPP;
 }
 
-static inline int netdev_switch_port_stp_update(struct net_device *dev,
-						u8 state)
-{
-	return -EOPNOTSUPP;
-}
-
 static inline int register_netdev_switch_notifier(struct notifier_block *nb)
 {
 	return 0;
diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
index fb3ebe6..f888dbc 100644
--- a/net/bridge/br_stp.c
+++ b/net/bridge/br_stp.c
@@ -39,10 +39,14 @@ void br_log_state(const struct net_bridge_port *p)
 
 void br_set_state(struct net_bridge_port *p, unsigned int state)
 {
+	struct swdev_attr attr = {
+		.id = SWDEV_ATTR_PORT_STP_STATE,
+		.stp_state = state,
+	};
 	int err;
 
 	p->state = state;
-	err = netdev_switch_port_stp_update(p->dev, state);
+	err = swdev_port_attr_set(p->dev, &attr);
 	if (err && err != -EOPNOTSUPP)
 		br_warn(p->br, "error setting offload STP state on port %u(%s)\n",
 				(unsigned int) p->port_no, p->dev->name);
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 6ff2261..04baa8a 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -345,6 +345,24 @@ static int dsa_slave_stp_update(struct net_device *dev, u8 state)
 	return ret;
 }
 
+static int dsa_slave_port_attr_set(struct net_device *dev,
+				   struct swdev_attr *attr)
+{
+	int ret = 0;
+
+	switch (attr->id) {
+	case SWDEV_ATTR_PORT_STP_STATE:
+		if (attr->trans == SWDEV_TRANS_COMMIT)
+			ret = dsa_slave_stp_update(dev, attr->stp_state);
+		break;
+	default:
+		ret = -EOPNOTSUPP;
+		break;
+	}
+
+	return ret;
+}
+
 static int dsa_slave_bridge_port_join(struct net_device *dev,
 				      struct net_device *br)
 {
@@ -683,7 +701,7 @@ static const struct net_device_ops dsa_slave_netdev_ops = {
 
 static const struct swdev_ops dsa_slave_swdev_ops = {
 	.swdev_port_attr_get = dsa_slave_port_attr_get,
-	.swdev_port_stp_update = dsa_slave_stp_update,
+	.swdev_port_attr_set = dsa_slave_port_attr_set,
 };
 
 static void dsa_slave_adjust_link(struct net_device *dev)
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 7d94724..6f81cbf 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -194,34 +194,6 @@ int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
 }
 EXPORT_SYMBOL_GPL(swdev_port_attr_set);
 
-/**
- *	netdev_switch_port_stp_update - Notify switch device port of STP
- *					state change
- *	@dev: port device
- *	@state: port STP state
- *
- *	Notify switch device port of bridge port STP state change.
- */
-int netdev_switch_port_stp_update(struct net_device *dev, u8 state)
-{
-	const struct swdev_ops *ops = dev->swdev_ops;
-	struct net_device *lower_dev;
-	struct list_head *iter;
-	int err = -EOPNOTSUPP;
-
-	if (ops && ops->swdev_port_stp_update)
-		return ops->swdev_port_stp_update(dev, state);
-
-	netdev_for_each_lower_dev(dev, lower_dev, iter) {
-		err = netdev_switch_port_stp_update(lower_dev, state);
-		if (err && err != -EOPNOTSUPP)
-			return err;
-	}
-
-	return err;
-}
-EXPORT_SYMBOL_GPL(netdev_switch_port_stp_update);
-
 static DEFINE_MUTEX(netdev_switch_mutex);
 static RAW_NOTIFIER_HEAD(netdev_switch_notif_chain);
 
-- 
1.7.10.4

[PATCH net-next v4 04/24] switchdev: add bridge port flags attr

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 include/net/switchdev.h |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index fa553ee..512945d 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -27,6 +27,7 @@ enum swdev_attr_id {
 	SWDEV_ATTR_UNDEFINED,
 	SWDEV_ATTR_PORT_PARENT_ID,
 	SWDEV_ATTR_PORT_STP_STATE,
+	SWDEV_ATTR_PORT_BRIDGE_FLAGS,
 };
 
 struct swdev_attr {
@@ -36,6 +37,7 @@ struct swdev_attr {
 	union {
 		struct netdev_phys_item_id ppid;	/* PORT_PARENT_ID */
 		u8 stp_state;				/* PORT_STP_STATE */
+		unsigned long brport_flags;		/* PORT_BRIDGE_FLAGS */
 	};
 };
 
-- 
1.7.10.4

[PATCH net-next v4 05/24] rocker: use swdev get/set attr for bridge port flags

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 drivers/net/ethernet/rocker/rocker.c |   31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index 80c7f6f..9bd4fc6 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -359,6 +359,8 @@ static void *__rocker_port_alloc(struct rocker_port *rocker_port,
 		if (elem)
 			INIT_LIST_HEAD(elem);
 		break;
+	default:
+		break;
 	}
 
 	return elem ? elem + 1 : NULL;
@@ -4332,6 +4334,9 @@ static int rocker_port_attr_get(struct net_device *dev, struct swdev_attr *attr)
 		attr->ppid.id_len = sizeof(rocker->hw.id);
 		memcpy(&attr->ppid.id, &rocker->hw.id, attr->ppid.id_len);
 		break;
+	case SWDEV_ATTR_PORT_BRIDGE_FLAGS:
+		attr->brport_flags = rocker_port->brport_flags;
+		break;
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -4349,6 +4354,23 @@ static void rocker_port_trans_abort(struct rocker_port *rocker_port)
 	}
 }
 
+static int rocker_port_brport_flags_set(struct rocker_port *rocker_port,
+					unsigned long brport_flags)
+{
+	unsigned long orig_flags;
+	int err = 0;
+
+	orig_flags = rocker_port->brport_flags;
+	rocker_port->brport_flags = brport_flags;
+	if ((orig_flags ^ rocker_port->brport_flags) & BR_LEARNING)
+		err = rocker_port_set_learning(rocker_port);
+
+	if (rocker_port->trans == SWDEV_TRANS_PREPARE)
+		rocker_port->brport_flags = orig_flags;
+
+	return err;
+}
+
 static int rocker_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
 {
 	struct rocker_port *rocker_port = netdev_priv(dev);
@@ -4363,12 +4385,18 @@ static int rocker_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
 	case SWDEV_TRANS_ABORT:
 		rocker_port_trans_abort(rocker_port);
 		return 0;
+	default:
+		break;
 	}
 
 	switch (attr->id) {
 	case SWDEV_ATTR_PORT_STP_STATE:
 		err = rocker_port_stp_update(rocker_port, attr->stp_state);
 		break;
+	case SWDEV_ATTR_PORT_BRIDGE_FLAGS:
+		err = rocker_port_brport_flags_set(rocker_port,
+						   attr->brport_flags);
+		break;
 	default:
 		err = -EOPNOTSUPP;
 		break;
@@ -4772,8 +4800,7 @@ static int rocker_probe_port(struct rocker *rocker, unsigned int port_number)
 	rocker_carrier_init(rocker_port);
 
 	dev->features |= NETIF_F_NETNS_LOCAL |
-			 NETIF_F_HW_VLAN_CTAG_FILTER |
-			 NETIF_F_HW_SWITCH_OFFLOAD;
+			 NETIF_F_HW_VLAN_CTAG_FILTER;
 
 	err = register_netdev(dev);
 	if (err) {
-- 
1.7.10.4

[PATCH net-next v4 06/24] switchdev: introduce swdev add/del obj ops

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

Like swdev attr get/set, add new swdev obj add/del.  swdev objs will be
things like VLANs or FIB entries, so add/del fits better for objects than
get/set used for attributes.

Use same two-phase prepare-commit transaction model as in attr set.

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 include/net/switchdev.h   |   32 +++++++++++++
 net/switchdev/switchdev.c |  113 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 145 insertions(+)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 512945d..f5f7733 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -43,6 +43,16 @@ struct swdev_attr {
 
 struct fib_info;
 
+enum swdev_obj_id {
+	SWDEV_OBJ_UNDEFINED,
+};
+
+struct swdev_obj {
+	enum swdev_obj_id id;
+	enum swdev_trans trans;
+	u32 flags;
+};
+
 /**
  * struct switchdev_ops - switchdev operations
  *
@@ -50,6 +60,10 @@ struct fib_info;
  *
  * @swdev_port_attr_set: Set a port attribute (see swdev_attr).
  *
+ * @swdev_port_obj_add: Add an object to port (see swdev_obj).
+ *
+ * @swdev_port_obj_del: Delete an object from port (see swdev_obj).
+ *
  * @swdev_fib_ipv4_add: Called to add/modify IPv4 route to switch device.
  *
  * @swdev_fib_ipv4_del: Called to delete IPv4 route from switch device.
@@ -59,6 +73,10 @@ struct swdev_ops {
 				       struct swdev_attr *attr);
 	int	(*swdev_port_attr_set)(struct net_device *dev,
 				       struct swdev_attr *attr);
+	int	(*swdev_port_obj_add)(struct net_device *dev,
+				      struct swdev_obj *obj);
+	int	(*swdev_port_obj_del)(struct net_device *dev,
+				      struct swdev_obj *obj);
 	int	(*swdev_fib_ipv4_add)(struct net_device *dev, __be32 dst,
 				      int dst_len, struct fib_info *fi,
 				      u8 tos, u8 type, u32 nlflags,
@@ -93,6 +111,8 @@ netdev_switch_notifier_info_to_dev(const struct netdev_switch_notifier_info *inf
 
 int swdev_port_attr_get(struct net_device *dev, struct swdev_attr *attr);
 int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr);
+int swdev_port_obj_add(struct net_device *dev, struct swdev_obj *obj);
+int swdev_port_obj_del(struct net_device *dev, struct swdev_obj *obj);
 int register_netdev_switch_notifier(struct notifier_block *nb);
 int unregister_netdev_switch_notifier(struct notifier_block *nb);
 int call_netdev_switch_notifiers(unsigned long val, struct net_device *dev,
@@ -125,6 +145,18 @@ static inline int swdev_port_attr_set(struct net_device *dev,
 	return -EOPNOTSUPP;
 }
 
+static inline int swdev_port_obj_add(struct net_device *dev,
+				     enum swdev_obj *obj)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int swdev_port_obj_del(struct net_device *dev,
+				     enum swdev_obj *obj)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline int register_netdev_switch_notifier(struct notifier_block *nb)
 {
 	return 0;
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 6f81cbf..05fded9 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -194,6 +194,119 @@ int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
 }
 EXPORT_SYMBOL_GPL(swdev_port_attr_set);
 
+int __swdev_port_obj_add(struct net_device *dev, struct swdev_obj *obj)
+{
+	const struct swdev_ops *ops = dev->swdev_ops;
+	struct net_device *lower_dev;
+	struct list_head *iter;
+	int err = -EOPNOTSUPP;
+
+	if (ops && ops->swdev_port_obj_add)
+		return ops->swdev_port_obj_add(dev, obj);
+
+	if (obj->flags & SWDEV_F_NO_RECURSE)
+		return err;
+
+	/* Switch device port(s) may be stacked under
+	 * bond/team/vlan dev, so recurse down to add object on
+	 * each port.
+	 */
+
+	netdev_for_each_lower_dev(dev, lower_dev, iter) {
+		err = __swdev_port_obj_add(lower_dev, obj);
+		if (err)
+			break;
+	}
+
+	return err;
+}
+
+/**
+ *	swdev_port_obj_add - Add port object
+ *
+ *	@dev: port device
+ *	@obj: object to add
+ *
+ *	Use a 2-phase prepare-commit transaction model to ensure
+ *	system is not left in a partially updated state due to
+ *	failure from driver/device.
+ *
+ *	rtnl_lock must be held.
+ */
+int swdev_port_obj_add(struct net_device *dev, struct swdev_obj *obj)
+{
+	int err;
+
+	ASSERT_RTNL();
+
+	/* 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
+	 * should reserve resources needed for the commit phase here,
+	 * but should not commit the obj.
+	 */
+
+	obj->trans = SWDEV_TRANS_PREPARE;
+	err = __swdev_port_obj_add(dev, obj);
+	if (err) {
+		/* Prepare phase failed: abort the transaction.  Any
+		 * resources reserved in the prepare phase are
+		 * released.
+		 */
+
+		obj->trans = SWDEV_TRANS_ABORT;
+		__swdev_port_obj_add(dev, obj);
+
+		return err;
+	}
+
+	/* Phase II: commit obj add.  This cannot fail as a fault
+	 * of driver/device.  If it does, it's a bug in the driver/device
+	 * because the driver said everythings was OK in phase I.
+	 */
+
+	obj->trans = SWDEV_TRANS_COMMIT;
+	err = __swdev_port_obj_add(dev, obj);
+	WARN(err, "%s: Commit of object (id=%d) failed.\n", dev->name, obj->id);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(swdev_port_obj_add);
+
+/**
+ *	swdev_port_obj_del - Delete port object
+ *
+ *	@dev: port device
+ *	@obj: object to delete
+ */
+int swdev_port_obj_del(struct net_device *dev, struct swdev_obj *obj)
+{
+	const struct swdev_ops *ops = dev->swdev_ops;
+	struct net_device *lower_dev;
+	struct list_head *iter;
+	int err = -EOPNOTSUPP;
+
+	if (ops && ops->swdev_port_obj_del)
+		return ops->swdev_port_obj_del(dev, obj);
+
+	if (obj->flags & SWDEV_F_NO_RECURSE)
+		return err;
+
+	/* Switch device port(s) may be stacked under
+	 * bond/team/vlan dev, so recurse down to delete object on
+	 * each port.
+	 */
+
+	netdev_for_each_lower_dev(dev, lower_dev, iter) {
+		err = swdev_port_obj_del(lower_dev, obj);
+		if (err)
+			break;
+	}
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(swdev_port_obj_del);
+
 static DEFINE_MUTEX(netdev_switch_mutex);
 static RAW_NOTIFIER_HEAD(netdev_switch_notif_chain);
 
-- 
1.7.10.4

[PATCH net-next v4 07/24] switchdev: add port vlan obj

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

VLAN obj has flags (PVID and untagged) as well as start and end vid ranges.
The swdev driver can optimize programing the device using the ranges.

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 include/net/switchdev.h |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index f5f7733..d3cc8eb 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -45,12 +45,20 @@ struct fib_info;
 
 enum swdev_obj_id {
 	SWDEV_OBJ_UNDEFINED,
+	SWDEV_OBJ_PORT_VLAN,
 };
 
 struct swdev_obj {
 	enum swdev_obj_id id;
 	enum swdev_trans trans;
 	u32 flags;
+	union {
+		struct swdev_obj_vlan {			/* PORT_VLAN */
+			u16 flags;
+			u16 vid_start;
+			u16 vid_end;
+		} vlan;
+	};
 };
 
 /**
-- 
1.7.10.4

[PATCH net-next v4 08/24] rocker: use swdev add/del obj for bridge port vlans

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 drivers/net/ethernet/rocker/rocker.c |  128 ++++++++++++++++++++++++++++++++--
 1 file changed, 121 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index 9bd4fc6..f81e362 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -3384,20 +3384,22 @@ static int rocker_port_vlan(struct rocker_port *rocker_port, int flags,
 
 	internal_vlan_id = rocker_port_vid_to_vlan(rocker_port, vid, &untagged);
 
-	if (adding && test_and_set_bit(ntohs(internal_vlan_id),
-				       rocker_port->vlan_bitmap))
+	if (adding && test_bit(ntohs(internal_vlan_id),
+			       rocker_port->vlan_bitmap))
 			return 0; /* already added */
-	else if (!adding && !test_and_clear_bit(ntohs(internal_vlan_id),
-						rocker_port->vlan_bitmap))
+	else if (!adding && !test_bit(ntohs(internal_vlan_id),
+				      rocker_port->vlan_bitmap))
 			return 0; /* already removed */
 
+	change_bit(ntohs(internal_vlan_id), rocker_port->vlan_bitmap);
+
 	if (adding) {
 		err = rocker_port_ctrl_vlan_add(rocker_port, flags,
 						internal_vlan_id);
 		if (err) {
 			netdev_err(rocker_port->dev,
 				   "Error (%d) port ctrl vlan add\n", err);
-			return err;
+			goto err_out;
 		}
 	}
 
@@ -3406,7 +3408,7 @@ static int rocker_port_vlan(struct rocker_port *rocker_port, int flags,
 	if (err) {
 		netdev_err(rocker_port->dev,
 			   "Error (%d) port VLAN l2 groups\n", err);
-		return err;
+		goto err_out;
 	}
 
 	err = rocker_port_vlan_flood_group(rocker_port, flags,
@@ -3414,7 +3416,7 @@ static int rocker_port_vlan(struct rocker_port *rocker_port, int flags,
 	if (err) {
 		netdev_err(rocker_port->dev,
 			   "Error (%d) port VLAN l2 flood group\n", err);
-		return err;
+		goto err_out;
 	}
 
 	err = rocker_flow_tbl_vlan(rocker_port, flags,
@@ -3424,6 +3426,10 @@ static int rocker_port_vlan(struct rocker_port *rocker_port, int flags,
 		netdev_err(rocker_port->dev,
 			   "Error (%d) port VLAN table\n", err);
 
+err_out:
+	if (rocker_port->trans == SWDEV_TRANS_PREPARE)
+		change_bit(ntohs(internal_vlan_id), rocker_port->vlan_bitmap);
+
 	return err;
 }
 
@@ -4407,6 +4413,112 @@ static int rocker_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
 	return err;
 }
 
+static int rocker_port_vlan_add(struct rocker_port *rocker_port,
+				u16 vid, u16 flags)
+{
+	int err;
+
+	/* XXX deal with flags for PVID and untagged */
+
+	err = rocker_port_vlan(rocker_port, 0, vid);
+	if (err)
+		return err;
+
+	return rocker_port_router_mac(rocker_port, 0, htons(vid));
+}
+
+static int rocker_port_vlans_add(struct rocker_port *rocker_port,
+				 struct swdev_obj_vlan *vlan)
+{
+	u16 vid;
+	int err;
+
+	for (vid = vlan->vid_start; vid <= vlan->vid_end; vid++) {
+		err = rocker_port_vlan_add(rocker_port, vid, vlan->flags);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static int rocker_port_obj_add(struct net_device *dev, struct swdev_obj *obj)
+{
+	struct rocker_port *rocker_port = netdev_priv(dev);
+	int err = 0;
+
+	rocker_port->trans = obj->trans;
+
+	switch (rocker_port->trans) {
+	case SWDEV_TRANS_PREPARE:
+		BUG_ON(!list_empty(&rocker_port->trans_mem));
+		break;
+	case SWDEV_TRANS_ABORT:
+		rocker_port_trans_abort(rocker_port);
+		return 0;
+	default:
+		break;
+	}
+
+	switch (obj->id) {
+	case SWDEV_OBJ_PORT_VLAN:
+		err = rocker_port_vlans_add(rocker_port, &obj->vlan);
+		break;
+	default:
+		err = -EOPNOTSUPP;
+		break;
+	}
+
+	rocker_port->trans = SWDEV_TRANS_NONE;
+
+	return err;
+}
+
+static int rocker_port_vlan_del(struct rocker_port *rocker_port,
+				u16 vid, u16 flags)
+{
+	int err;
+
+	err = rocker_port_router_mac(rocker_port, ROCKER_OP_FLAG_REMOVE,
+				     htons(vid));
+	if (err)
+		return err;
+
+	return rocker_port_vlan(rocker_port, ROCKER_OP_FLAG_REMOVE, vid);
+}
+
+static int rocker_port_vlans_del(struct rocker_port *rocker_port,
+				 struct swdev_obj_vlan *vlan)
+{
+	u16 vid;
+	int err;
+
+	for (vid = vlan->vid_start; vid <= vlan->vid_end; vid++) {
+		err = rocker_port_vlan_del(rocker_port, vid, vlan->flags);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static int rocker_port_obj_del(struct net_device *dev, struct swdev_obj *obj)
+{
+	struct rocker_port *rocker_port = netdev_priv(dev);
+	int err = 0;
+
+	switch (obj->id) {
+	case SWDEV_OBJ_PORT_VLAN:
+		err = rocker_port_vlans_del(rocker_port, &obj->vlan);
+		break;
+	default:
+		err = -EOPNOTSUPP;
+		break;
+	}
+
+	return err;
+}
+
 static int rocker_port_swdev_fib_ipv4_add(struct net_device *dev,
 					  __be32 dst, int dst_len,
 					  struct fib_info *fi,
@@ -4435,6 +4547,8 @@ static int rocker_port_swdev_fib_ipv4_del(struct net_device *dev,
 static const struct swdev_ops rocker_port_swdev_ops = {
 	.swdev_port_attr_get		= rocker_port_attr_get,
 	.swdev_port_attr_set		= rocker_port_attr_set,
+	.swdev_port_obj_add		= rocker_port_obj_add,
+	.swdev_port_obj_del		= rocker_port_obj_del,
 	.swdev_fib_ipv4_add		= rocker_port_swdev_fib_ipv4_add,
 	.swdev_fib_ipv4_del		= rocker_port_swdev_fib_ipv4_del,
 };
-- 
1.7.10.4

[PATCH net-next v4 09/24] switchdev: add new swdev bridge setlink

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

Add new swdev_port_bridge_setlink that can be used by drivers implementing
.ndo_bridge_setlink to set swdev bridge attributes.  Basically turn the raw
rtnl_bridge_setlink netlink into swdev attr sets.  Proper netlink attr policy
checking is done on the protinfo part of the netlink msg.

Currently, for protinfo, only bridge port attrs BR_LEARNING and
BR_LEARNING_SYNC are parsed and passed to port driver.

For afspec, VLAN objs are passed so swdev driver can set VLANs assigned to
SELF.  To illustrate with iproute2 cmd, we have:

	bridge vlan add vid 10 dev sw1p1 self master

To add VLAN 10 to port sw1p1 for both the bridge (master) and the device
(self).

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 include/net/switchdev.h   |    8 +++
 net/switchdev/switchdev.c |  152 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 160 insertions(+)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index d3cc8eb..cff53ae 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -121,6 +121,8 @@ int swdev_port_attr_get(struct net_device *dev, struct swdev_attr *attr);
 int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr);
 int swdev_port_obj_add(struct net_device *dev, struct swdev_obj *obj);
 int swdev_port_obj_del(struct net_device *dev, struct swdev_obj *obj);
+int swdev_port_bridge_setlink(struct net_device *dev, struct nlmsghdr *nlh,
+			      u16 flags);
 int register_netdev_switch_notifier(struct notifier_block *nb);
 int unregister_netdev_switch_notifier(struct notifier_block *nb);
 int call_netdev_switch_notifiers(unsigned long val, struct net_device *dev,
@@ -165,6 +167,12 @@ static inline int swdev_port_obj_del(struct net_device *dev,
 	return -EOPNOTSUPP;
 }
 
+static inline int swdev_port_bridge_setlink(struct net_device *dev,
+					    struct nlmsghdr *nlh, u16 flags)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline int register_netdev_switch_notifier(struct notifier_block *nb)
 {
 	return 0;
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 05fded9..31d55e7 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -15,6 +15,7 @@
 #include <linux/mutex.h>
 #include <linux/notifier.h>
 #include <linux/netdevice.h>
+#include <linux/if_bridge.h>
 #include <net/ip_fib.h>
 #include <net/switchdev.h>
 
@@ -395,6 +396,157 @@ int netdev_switch_port_bridge_setlink(struct net_device *dev,
 }
 EXPORT_SYMBOL_GPL(netdev_switch_port_bridge_setlink);
 
+static int swdev_port_br_setflag(struct net_device *dev, struct nlattr *nlattr,
+				 unsigned long brport_flag)
+{
+	struct swdev_attr attr = {
+		.id = SWDEV_ATTR_PORT_BRIDGE_FLAGS,
+	};
+	u8 flag = nla_get_u8(nlattr);
+	int err;
+
+	err = swdev_port_attr_get(dev, &attr);
+	if (err)
+		return err;
+
+	if (flag)
+		attr.brport_flags |= brport_flag;
+	else
+		attr.brport_flags &= ~brport_flag;
+
+	return swdev_port_attr_set(dev, &attr);
+}
+
+static const struct nla_policy swdev_port_bridge_policy[IFLA_BRPORT_MAX + 1] = {
+	[IFLA_BRPORT_STATE]		= { .type = NLA_U8 },
+	[IFLA_BRPORT_COST]		= { .type = NLA_U32 },
+	[IFLA_BRPORT_PRIORITY]		= { .type = NLA_U16 },
+	[IFLA_BRPORT_MODE]		= { .type = NLA_U8 },
+	[IFLA_BRPORT_GUARD]		= { .type = NLA_U8 },
+	[IFLA_BRPORT_PROTECT]		= { .type = NLA_U8 },
+	[IFLA_BRPORT_FAST_LEAVE]	= { .type = NLA_U8 },
+	[IFLA_BRPORT_LEARNING]		= { .type = NLA_U8 },
+	[IFLA_BRPORT_LEARNING_SYNC]	= { .type = NLA_U8 },
+	[IFLA_BRPORT_UNICAST_FLOOD]	= { .type = NLA_U8 },
+};
+
+static int swdev_port_br_setlink_protinfo(struct net_device *dev,
+					  struct nlattr *protinfo)
+{
+	struct nlattr *attr;
+	int rem;
+	int err;
+
+	err = nla_validate_nested(protinfo, IFLA_BRPORT_MAX,
+				  swdev_port_bridge_policy);
+	if (err)
+		return err;
+
+	nla_for_each_nested(attr, protinfo, rem) {
+		switch (nla_type(attr)) {
+		case IFLA_BRPORT_LEARNING:
+			err = swdev_port_br_setflag(dev, attr,
+						    BR_LEARNING);
+			break;
+		case IFLA_BRPORT_LEARNING_SYNC:
+			err = swdev_port_br_setflag(dev, attr,
+						    BR_LEARNING_SYNC);
+			break;
+		default:
+			err = -EOPNOTSUPP;
+			break;
+		}
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static int swdev_port_br_afspec(struct net_device *dev,
+				struct nlattr *afspec,
+				int (*f)(struct net_device *dev,
+					 struct swdev_obj *obj))
+{
+	struct nlattr *attr;
+	struct bridge_vlan_info *vinfo;
+	struct swdev_obj obj = {
+		.id = SWDEV_OBJ_PORT_VLAN,
+	};
+	int rem;
+	int err;
+
+	nla_for_each_nested(attr, afspec, rem) {
+		if (nla_type(attr) != IFLA_BRIDGE_VLAN_INFO)
+			continue;
+		if (nla_len(attr) != sizeof(struct bridge_vlan_info))
+			return -EINVAL;
+		vinfo = nla_data(attr);
+		obj.vlan.flags = vinfo->flags;
+		if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_BEGIN) {
+			if (obj.vlan.vid_start)
+				return -EINVAL;
+			obj.vlan.vid_start = vinfo->vid;
+		} else if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_END) {
+			if (!obj.vlan.vid_start)
+				return -EINVAL;
+			obj.vlan.vid_end = vinfo->vid;
+			if (obj.vlan.vid_end <= obj.vlan.vid_start)
+				return -EINVAL;
+			err = f(dev, &obj);
+			if (err)
+				return err;
+			memset(&obj.vlan, 0, sizeof(obj.vlan));
+		} else {
+			if (obj.vlan.vid_start)
+				return -EINVAL;
+			obj.vlan.vid_start = vinfo->vid;
+			obj.vlan.vid_end = vinfo->vid;
+			err = f(dev, &obj);
+			if (err)
+				return err;
+			memset(&obj.vlan, 0, sizeof(obj.vlan));
+		}
+	}
+
+	return 0;
+}
+
+/**
+ *	swdev_port_bridge_setlink - Set bridge port attributes
+ *
+ *	@dev: port device
+ *	@nlh: netlink header
+ *	@flags: netlink flags
+ *
+ *	Called for SELF on rtnl_bridge_setlink to set bridge port
+ *	attributes.
+ */
+int swdev_port_bridge_setlink(struct net_device *dev,
+			      struct nlmsghdr *nlh, u16 flags)
+{
+	struct nlattr *protinfo;
+	struct nlattr *afspec;
+	int err = 0;
+
+	protinfo = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg),
+				   IFLA_PROTINFO);
+	if (protinfo) {
+		err = swdev_port_br_setlink_protinfo(dev, protinfo);
+		if (err)
+			return err;
+	}
+
+	afspec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg),
+				 IFLA_AF_SPEC);
+	if (afspec)
+		err = swdev_port_br_afspec(dev, afspec,
+					   swdev_port_obj_add);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(swdev_port_bridge_setlink);
+
 /**
  *	netdev_switch_port_bridge_dellink - Notify switch device port of bridge
  *	port attribute delete
-- 
1.7.10.4

[PATCH net-next v4 10/24] rocker: cut over to new swdev_port_bridge_setlink

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

Rocker can now use the swdev bridge setlink to parse raw netlink; no need
to duplicate this code in each driver.

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 drivers/net/ethernet/rocker/rocker.c |   41 +---------------------------------
 1 file changed, 1 insertion(+), 40 deletions(-)

diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index f81e362..b2d0477 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -4245,45 +4245,6 @@ skip:
 	return idx;
 }
 
-static int rocker_port_bridge_setlink(struct net_device *dev,
-				      struct nlmsghdr *nlh, u16 flags)
-{
-	struct rocker_port *rocker_port = netdev_priv(dev);
-	struct nlattr *protinfo;
-	struct nlattr *attr;
-	int err;
-
-	protinfo = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg),
-				   IFLA_PROTINFO);
-	if (protinfo) {
-		attr = nla_find_nested(protinfo, IFLA_BRPORT_LEARNING);
-		if (attr) {
-			if (nla_len(attr) < sizeof(u8))
-				return -EINVAL;
-
-			if (nla_get_u8(attr))
-				rocker_port->brport_flags |= BR_LEARNING;
-			else
-				rocker_port->brport_flags &= ~BR_LEARNING;
-			err = rocker_port_set_learning(rocker_port);
-			if (err)
-				return err;
-		}
-		attr = nla_find_nested(protinfo, IFLA_BRPORT_LEARNING_SYNC);
-		if (attr) {
-			if (nla_len(attr) < sizeof(u8))
-				return -EINVAL;
-
-			if (nla_get_u8(attr))
-				rocker_port->brport_flags |= BR_LEARNING_SYNC;
-			else
-				rocker_port->brport_flags &= ~BR_LEARNING_SYNC;
-		}
-	}
-
-	return 0;
-}
-
 static int rocker_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
 				      struct net_device *dev,
 				      u32 filter_mask)
@@ -4321,7 +4282,7 @@ static const struct net_device_ops rocker_port_netdev_ops = {
 	.ndo_fdb_add			= rocker_port_fdb_add,
 	.ndo_fdb_del			= rocker_port_fdb_del,
 	.ndo_fdb_dump			= rocker_port_fdb_dump,
-	.ndo_bridge_setlink		= rocker_port_bridge_setlink,
+	.ndo_bridge_setlink		= swdev_port_bridge_setlink,
 	.ndo_bridge_getlink		= rocker_port_bridge_getlink,
 	.ndo_get_phys_port_name		= rocker_port_get_phys_port_name,
 };
-- 
1.7.10.4

[PATCH net-next v4 11/24] bonding: cut over to new swdev_port_bridge_setlink

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

swdev_port_bridge_setlink knows how to recurse stacked devices, so make it
the default bridge_setlink op for bonds.

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 drivers/net/bonding/bond_main.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 78dde56..ef5207c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4037,7 +4037,7 @@ static const struct net_device_ops bond_netdev_ops = {
 	.ndo_add_slave		= bond_enslave,
 	.ndo_del_slave		= bond_release,
 	.ndo_fix_features	= bond_fix_features,
-	.ndo_bridge_setlink	= ndo_dflt_netdev_switch_port_bridge_setlink,
+	.ndo_bridge_setlink	= swdev_port_bridge_setlink,
 	.ndo_bridge_dellink	= ndo_dflt_netdev_switch_port_bridge_dellink,
 	.ndo_features_check	= passthru_features_check,
 };
-- 
1.7.10.4

[PATCH net-next v4 12/24] team: cut over to new swdev_port_bridge_setlink

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

swdev_port_bridge_setlink knows how to recurse stacked devices, so make it
the default bridge_setlink op for teams.

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 drivers/net/team/team.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 6928448..6a3debc 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1977,7 +1977,7 @@ static const struct net_device_ops team_netdev_ops = {
 	.ndo_del_slave		= team_del_slave,
 	.ndo_fix_features	= team_fix_features,
 	.ndo_change_carrier     = team_change_carrier,
-	.ndo_bridge_setlink     = ndo_dflt_netdev_switch_port_bridge_setlink,
+	.ndo_bridge_setlink	= swdev_port_bridge_setlink,
 	.ndo_bridge_dellink     = ndo_dflt_netdev_switch_port_bridge_dellink,
 	.ndo_features_check	= passthru_features_check,
 };
-- 
1.7.10.4

[PATCH net-next v4 13/24] switchdev: remove old netdev_switch_port_bridge_setlink

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

New attr-based bridge_setlink can recurse lower devs and recover on err, so
remove old wrapper.  Also, restore br_setlink back to original and don't call
into SELF port driver.  rtnetlink.c:bridge_setlink already does a call into
port driver for SELF.

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 include/net/switchdev.h   |   18 ---------------
 net/bridge/br_netlink.c   |   12 +---------
 net/switchdev/switchdev.c |   55 ---------------------------------------------
 3 files changed, 1 insertion(+), 84 deletions(-)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index cff53ae..70874fc 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -127,14 +127,10 @@ int register_netdev_switch_notifier(struct notifier_block *nb);
 int unregister_netdev_switch_notifier(struct notifier_block *nb);
 int call_netdev_switch_notifiers(unsigned long val, struct net_device *dev,
 				 struct netdev_switch_notifier_info *info);
-int netdev_switch_port_bridge_setlink(struct net_device *dev,
-				struct nlmsghdr *nlh, u16 flags);
 int netdev_switch_port_bridge_dellink(struct net_device *dev,
 				struct nlmsghdr *nlh, u16 flags);
 int ndo_dflt_netdev_switch_port_bridge_dellink(struct net_device *dev,
 					       struct nlmsghdr *nlh, u16 flags);
-int ndo_dflt_netdev_switch_port_bridge_setlink(struct net_device *dev,
-					       struct nlmsghdr *nlh, u16 flags);
 int netdev_switch_fib_ipv4_add(u32 dst, int dst_len, struct fib_info *fi,
 			       u8 tos, u8 type, u32 nlflags, u32 tb_id);
 int netdev_switch_fib_ipv4_del(u32 dst, int dst_len, struct fib_info *fi,
@@ -189,13 +185,6 @@ static inline int call_netdev_switch_notifiers(unsigned long val, struct net_dev
 	return NOTIFY_DONE;
 }
 
-static inline int netdev_switch_port_bridge_setlink(struct net_device *dev,
-						    struct nlmsghdr *nlh,
-						    u16 flags)
-{
-	return -EOPNOTSUPP;
-}
-
 static inline int netdev_switch_port_bridge_dellink(struct net_device *dev,
 						    struct nlmsghdr *nlh,
 						    u16 flags)
@@ -210,13 +199,6 @@ static inline int ndo_dflt_netdev_switch_port_bridge_dellink(struct net_device *
 	return 0;
 }
 
-static inline int ndo_dflt_netdev_switch_port_bridge_setlink(struct net_device *dev,
-							struct nlmsghdr *nlh,
-							u16 flags)
-{
-	return 0;
-}
-
 static inline int netdev_switch_fib_ipv4_add(u32 dst, int dst_len,
 					     struct fib_info *fi,
 					     u8 tos, u8 type,
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 0e4ddb8..45ed1ce 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -586,7 +586,7 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh, u16 flags)
 	struct nlattr *afspec;
 	struct net_bridge_port *p;
 	struct nlattr *tb[IFLA_BRPORT_MAX + 1];
-	int err = 0, ret_offload = 0;
+	int err = 0;
 
 	protinfo = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_PROTINFO);
 	afspec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_AF_SPEC);
@@ -628,16 +628,6 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh, u16 flags)
 				afspec, RTM_SETLINK);
 	}
 
-	if (p && !(flags & BRIDGE_FLAGS_SELF)) {
-		/* set bridge attributes in hardware if supported
-		 */
-		ret_offload = netdev_switch_port_bridge_setlink(dev, nlh,
-								flags);
-		if (ret_offload && ret_offload != -EOPNOTSUPP)
-			br_warn(p->br, "error setting attrs on port %u(%s)\n",
-				(unsigned int)p->port_no, p->dev->name);
-	}
-
 	if (err == 0)
 		br_ifinfo_notify(RTM_NEWLINK, p);
 out:
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 31d55e7..6691537 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -371,31 +371,6 @@ int call_netdev_switch_notifiers(unsigned long val, struct net_device *dev,
 }
 EXPORT_SYMBOL_GPL(call_netdev_switch_notifiers);
 
-/**
- *	netdev_switch_port_bridge_setlink - Notify switch device port of bridge
- *	port attributes
- *
- *	@dev: port device
- *	@nlh: netlink msg with bridge port attributes
- *	@flags: bridge setlink flags
- *
- *	Notify switch device port of bridge port attributes
- */
-int netdev_switch_port_bridge_setlink(struct net_device *dev,
-				      struct nlmsghdr *nlh, u16 flags)
-{
-	const struct net_device_ops *ops = dev->netdev_ops;
-
-	if (!(dev->features & NETIF_F_HW_SWITCH_OFFLOAD))
-		return 0;
-
-	if (!ops->ndo_bridge_setlink)
-		return -EOPNOTSUPP;
-
-	return ops->ndo_bridge_setlink(dev, nlh, flags);
-}
-EXPORT_SYMBOL_GPL(netdev_switch_port_bridge_setlink);
-
 static int swdev_port_br_setflag(struct net_device *dev, struct nlattr *nlattr,
 				 unsigned long brport_flag)
 {
@@ -573,36 +548,6 @@ int netdev_switch_port_bridge_dellink(struct net_device *dev,
 EXPORT_SYMBOL_GPL(netdev_switch_port_bridge_dellink);
 
 /**
- *	ndo_dflt_netdev_switch_port_bridge_setlink - default ndo bridge setlink
- *						     op for master devices
- *
- *	@dev: port device
- *	@nlh: netlink msg with bridge port attributes
- *	@flags: bridge setlink flags
- *
- *	Notify master device slaves of bridge port attributes
- */
-int ndo_dflt_netdev_switch_port_bridge_setlink(struct net_device *dev,
-					       struct nlmsghdr *nlh, u16 flags)
-{
-	struct net_device *lower_dev;
-	struct list_head *iter;
-	int ret = 0, err = 0;
-
-	if (!(dev->features & NETIF_F_HW_SWITCH_OFFLOAD))
-		return ret;
-
-	netdev_for_each_lower_dev(dev, lower_dev, iter) {
-		err = netdev_switch_port_bridge_setlink(lower_dev, nlh, flags);
-		if (err && err != -EOPNOTSUPP)
-			ret = err;
-	}
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(ndo_dflt_netdev_switch_port_bridge_setlink);
-
-/**
  *	ndo_dflt_netdev_switch_port_bridge_dellink - default ndo bridge dellink
  *						     op for master devices
  *
-- 
1.7.10.4

[PATCH net-next v4 14/24] switchdev: add new swdev_port_bridge_dellink

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

Same change as setlink.  Provide the wrapper op for SELF ndo_bridge_dellink
and call into the swdev driver to delete afspec VLANs.

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 include/net/switchdev.h   |    8 ++++++++
 net/switchdev/switchdev.c |   24 ++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 70874fc..5443c9b 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -123,6 +123,8 @@ int swdev_port_obj_add(struct net_device *dev, struct swdev_obj *obj);
 int swdev_port_obj_del(struct net_device *dev, struct swdev_obj *obj);
 int swdev_port_bridge_setlink(struct net_device *dev, struct nlmsghdr *nlh,
 			      u16 flags);
+int swdev_port_bridge_dellink(struct net_device *dev, struct nlmsghdr *nlh,
+			      u16 flags);
 int register_netdev_switch_notifier(struct notifier_block *nb);
 int unregister_netdev_switch_notifier(struct notifier_block *nb);
 int call_netdev_switch_notifiers(unsigned long val, struct net_device *dev,
@@ -169,6 +171,12 @@ static inline int swdev_port_bridge_setlink(struct net_device *dev,
 	return -EOPNOTSUPP;
 }
 
+static inline int swdev_port_bridge_dellink(struct net_device *dev,
+					    struct nlmsghdr *nlh, u16 flags)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline int register_netdev_switch_notifier(struct notifier_block *nb)
 {
 	return 0;
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 6691537..febd67e 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -523,6 +523,30 @@ int swdev_port_bridge_setlink(struct net_device *dev,
 EXPORT_SYMBOL_GPL(swdev_port_bridge_setlink);
 
 /**
+ *	swdev_port_bridge_dellink - Set bridge port attributes
+ *
+ *	@dev: port device
+ *	@nlh: netlink header
+ *	@flags: netlink flags
+ *
+ *	Called for SELF on rtnl_bridge_dellink to set bridge port
+ *	attributes.
+ */
+int swdev_port_bridge_dellink(struct net_device *dev,
+			      struct nlmsghdr *nlh, u16 flags)
+{
+	struct nlattr *afspec;
+
+	afspec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg),
+				 IFLA_AF_SPEC);
+	if (afspec)
+		return swdev_port_br_afspec(dev, afspec, swdev_port_obj_del);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(swdev_port_bridge_dellink);
+
+/**
  *	netdev_switch_port_bridge_dellink - Notify switch device port of bridge
  *	port attribute delete
  *
-- 
1.7.10.4

[PATCH net-next v4 15/24] rocker: cut over to new swdev_port_bridge_dellink

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 drivers/net/ethernet/rocker/rocker.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index b2d0477..b04fb5f 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -4283,7 +4283,7 @@ static const struct net_device_ops rocker_port_netdev_ops = {
 	.ndo_fdb_del			= rocker_port_fdb_del,
 	.ndo_fdb_dump			= rocker_port_fdb_dump,
 	.ndo_bridge_setlink		= swdev_port_bridge_setlink,
-	.ndo_bridge_getlink		= rocker_port_bridge_getlink,
+	.ndo_bridge_dellink		= swdev_port_bridge_dellink,
 	.ndo_get_phys_port_name		= rocker_port_get_phys_port_name,
 };
 
-- 
1.7.10.4

[PATCH net-next v4 16/24] bonding: cut over to new swdev_port_bridge_dellink

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 drivers/net/bonding/bond_main.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index ef5207c..83c02de 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4038,7 +4038,7 @@ static const struct net_device_ops bond_netdev_ops = {
 	.ndo_del_slave		= bond_release,
 	.ndo_fix_features	= bond_fix_features,
 	.ndo_bridge_setlink	= swdev_port_bridge_setlink,
-	.ndo_bridge_dellink	= ndo_dflt_netdev_switch_port_bridge_dellink,
+	.ndo_bridge_dellink	= swdev_port_bridge_dellink,
 	.ndo_features_check	= passthru_features_check,
 };
 
-- 
1.7.10.4

[PATCH net-next v4 17/24] team: cut over to new swdev_port_bridge_dellink

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 drivers/net/team/team.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 6a3debc..261e9c5 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1978,7 +1978,7 @@ static const struct net_device_ops team_netdev_ops = {
 	.ndo_fix_features	= team_fix_features,
 	.ndo_change_carrier     = team_change_carrier,
 	.ndo_bridge_setlink	= swdev_port_bridge_setlink,
-	.ndo_bridge_dellink     = ndo_dflt_netdev_switch_port_bridge_dellink,
+	.ndo_bridge_dellink	= swdev_port_bridge_dellink,
 	.ndo_features_check	= passthru_features_check,
 };
 
-- 
1.7.10.4

[PATCH net-next v4 18/24] switchdev: remove unused netdev_switch_port_bridge_dellink

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

Now we can remove old wrappers for dellink.

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 include/net/switchdev.h   |   18 ---------------
 net/bridge/br_netlink.c   |   12 +---------
 net/switchdev/switchdev.c |   55 ---------------------------------------------
 3 files changed, 1 insertion(+), 84 deletions(-)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 5443c9b..8c9814f 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -129,10 +129,6 @@ int register_netdev_switch_notifier(struct notifier_block *nb);
 int unregister_netdev_switch_notifier(struct notifier_block *nb);
 int call_netdev_switch_notifiers(unsigned long val, struct net_device *dev,
 				 struct netdev_switch_notifier_info *info);
-int netdev_switch_port_bridge_dellink(struct net_device *dev,
-				struct nlmsghdr *nlh, u16 flags);
-int ndo_dflt_netdev_switch_port_bridge_dellink(struct net_device *dev,
-					       struct nlmsghdr *nlh, u16 flags);
 int netdev_switch_fib_ipv4_add(u32 dst, int dst_len, struct fib_info *fi,
 			       u8 tos, u8 type, u32 nlflags, u32 tb_id);
 int netdev_switch_fib_ipv4_del(u32 dst, int dst_len, struct fib_info *fi,
@@ -193,20 +189,6 @@ static inline int call_netdev_switch_notifiers(unsigned long val, struct net_dev
 	return NOTIFY_DONE;
 }
 
-static inline int netdev_switch_port_bridge_dellink(struct net_device *dev,
-						    struct nlmsghdr *nlh,
-						    u16 flags)
-{
-	return -EOPNOTSUPP;
-}
-
-static inline int ndo_dflt_netdev_switch_port_bridge_dellink(struct net_device *dev,
-							struct nlmsghdr *nlh,
-							u16 flags)
-{
-	return 0;
-}
-
 static inline int netdev_switch_fib_ipv4_add(u32 dst, int dst_len,
 					     struct fib_info *fi,
 					     u8 tos, u8 type,
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 45ed1ce..7d2274a 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -639,7 +639,7 @@ int br_dellink(struct net_device *dev, struct nlmsghdr *nlh, u16 flags)
 {
 	struct nlattr *afspec;
 	struct net_bridge_port *p;
-	int err = 0, ret_offload = 0;
+	int err = 0;
 
 	afspec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_AF_SPEC);
 	if (!afspec)
@@ -658,16 +658,6 @@ int br_dellink(struct net_device *dev, struct nlmsghdr *nlh, u16 flags)
 		 */
 		br_ifinfo_notify(RTM_NEWLINK, p);
 
-	if (p && !(flags & BRIDGE_FLAGS_SELF)) {
-		/* del bridge attributes in hardware
-		 */
-		ret_offload = netdev_switch_port_bridge_dellink(dev, nlh,
-								flags);
-		if (ret_offload && ret_offload != -EOPNOTSUPP)
-			br_warn(p->br, "error deleting attrs on port %u (%s)\n",
-				(unsigned int)p->port_no, p->dev->name);
-	}
-
 	return err;
 }
 static int br_validate(struct nlattr *tb[], struct nlattr *data[])
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index febd67e..ae0ed06 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -546,61 +546,6 @@ int swdev_port_bridge_dellink(struct net_device *dev,
 }
 EXPORT_SYMBOL_GPL(swdev_port_bridge_dellink);
 
-/**
- *	netdev_switch_port_bridge_dellink - Notify switch device port of bridge
- *	port attribute delete
- *
- *	@dev: port device
- *	@nlh: netlink msg with bridge port attributes
- *	@flags: bridge setlink flags
- *
- *	Notify switch device port of bridge port attribute delete
- */
-int netdev_switch_port_bridge_dellink(struct net_device *dev,
-				      struct nlmsghdr *nlh, u16 flags)
-{
-	const struct net_device_ops *ops = dev->netdev_ops;
-
-	if (!(dev->features & NETIF_F_HW_SWITCH_OFFLOAD))
-		return 0;
-
-	if (!ops->ndo_bridge_dellink)
-		return -EOPNOTSUPP;
-
-	return ops->ndo_bridge_dellink(dev, nlh, flags);
-}
-EXPORT_SYMBOL_GPL(netdev_switch_port_bridge_dellink);
-
-/**
- *	ndo_dflt_netdev_switch_port_bridge_dellink - default ndo bridge dellink
- *						     op for master devices
- *
- *	@dev: port device
- *	@nlh: netlink msg with bridge port attributes
- *	@flags: bridge dellink flags
- *
- *	Notify master device slaves of bridge port attribute deletes
- */
-int ndo_dflt_netdev_switch_port_bridge_dellink(struct net_device *dev,
-					       struct nlmsghdr *nlh, u16 flags)
-{
-	struct net_device *lower_dev;
-	struct list_head *iter;
-	int ret = 0, err = 0;
-
-	if (!(dev->features & NETIF_F_HW_SWITCH_OFFLOAD))
-		return ret;
-
-	netdev_for_each_lower_dev(dev, lower_dev, iter) {
-		err = netdev_switch_port_bridge_dellink(lower_dev, nlh, flags);
-		if (err && err != -EOPNOTSUPP)
-			ret = err;
-	}
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(ndo_dflt_netdev_switch_port_bridge_dellink);
-
 static struct net_device *netdev_switch_get_lowest_dev(struct net_device *dev)
 {
 	const struct swdev_ops *ops = dev->swdev_ops;
-- 
1.7.10.4

[PATCH net-next v4 19/24] switchdev: add new swdev_port_bridge_getlink

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

Like bridge_setlink, add swdev wrapper to handle bridge_getlink and call into
port driver to get port attrs.  For now, only BR_LEARNING and BR_LEARNING_SYNC
are returned.  To add more, we'll probably want to break away from
ndo_dflt_bridge_getlink() and build the netlink skb directly in the swdev code.

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 include/net/switchdev.h   |    9 +++++++++
 net/switchdev/switchdev.c |   27 +++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 8c9814f..d04322e 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -121,6 +121,8 @@ int swdev_port_attr_get(struct net_device *dev, struct swdev_attr *attr);
 int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr);
 int swdev_port_obj_add(struct net_device *dev, struct swdev_obj *obj);
 int swdev_port_obj_del(struct net_device *dev, struct swdev_obj *obj);
+int swdev_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
+			      struct net_device *dev, u32 filter_mask);
 int swdev_port_bridge_setlink(struct net_device *dev, struct nlmsghdr *nlh,
 			      u16 flags);
 int swdev_port_bridge_dellink(struct net_device *dev, struct nlmsghdr *nlh,
@@ -161,6 +163,13 @@ static inline int swdev_port_obj_del(struct net_device *dev,
 	return -EOPNOTSUPP;
 }
 
+static inline int swdev_port_bridge_getlink(struct sk_buff *skb, u32 pid,
+					    u32 seq, struct net_device *dev,
+					    u32 filter_mask)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline int swdev_port_bridge_setlink(struct net_device *dev,
 					    struct nlmsghdr *nlh, u16 flags)
 {
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index ae0ed06..6866e0c 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -371,6 +371,33 @@ int call_netdev_switch_notifiers(unsigned long val, struct net_device *dev,
 }
 EXPORT_SYMBOL_GPL(call_netdev_switch_notifiers);
 
+/**
+ *	swdev_port_bridge_getlink - Get bridge port attributes
+ *
+ *	@dev: port device
+ *
+ *	Called for SELF on rtnl_bridge_getlink to get bridge port
+ *	attributes.
+ */
+int swdev_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
+			      struct net_device *dev, u32 filter_mask)
+{
+	struct swdev_attr attr = {
+		.id = SWDEV_ATTR_PORT_BRIDGE_FLAGS,
+	};
+	u16 mode = BRIDGE_MODE_UNDEF;
+	u32 mask = BR_LEARNING | BR_LEARNING_SYNC;
+	int err;
+
+	err = swdev_port_attr_get(dev, &attr);
+	if (err)
+		return err;
+
+	return ndo_dflt_bridge_getlink(skb, pid, seq, dev, mode,
+				       attr.brport_flags, mask);
+}
+EXPORT_SYMBOL_GPL(swdev_port_bridge_getlink);
+
 static int swdev_port_br_setflag(struct net_device *dev, struct nlattr *nlattr,
 				 unsigned long brport_flag)
 {
-- 
1.7.10.4

[PATCH net-next v4 20/24] rocker: cut over to new swdev_port_bridge_getlink

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 drivers/net/ethernet/rocker/rocker.c |   13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index b04fb5f..e927e22 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -4245,18 +4245,6 @@ skip:
 	return idx;
 }
 
-static int rocker_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
-				      struct net_device *dev,
-				      u32 filter_mask)
-{
-	struct rocker_port *rocker_port = netdev_priv(dev);
-	u16 mode = BRIDGE_MODE_UNDEF;
-	u32 mask = BR_LEARNING | BR_LEARNING_SYNC;
-
-	return ndo_dflt_bridge_getlink(skb, pid, seq, dev, mode,
-				       rocker_port->brport_flags, mask);
-}
-
 static int rocker_port_get_phys_port_name(struct net_device *dev,
 					  char *buf, size_t len)
 {
@@ -4283,6 +4271,7 @@ static const struct net_device_ops rocker_port_netdev_ops = {
 	.ndo_fdb_del			= rocker_port_fdb_del,
 	.ndo_fdb_dump			= rocker_port_fdb_dump,
 	.ndo_bridge_setlink		= swdev_port_bridge_setlink,
+	.ndo_bridge_getlink		= swdev_port_bridge_getlink,
 	.ndo_bridge_dellink		= swdev_port_bridge_dellink,
 	.ndo_get_phys_port_name		= rocker_port_get_phys_port_name,
 };
-- 
1.7.10.4

[PATCH net-next v4 21/24] bonding: cut over to new swdev_port_bridge_getlink

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 drivers/net/bonding/bond_main.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 83c02de..19903a4 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4038,6 +4038,7 @@ static const struct net_device_ops bond_netdev_ops = {
 	.ndo_del_slave		= bond_release,
 	.ndo_fix_features	= bond_fix_features,
 	.ndo_bridge_setlink	= swdev_port_bridge_setlink,
+	.ndo_bridge_getlink	= swdev_port_bridge_getlink,
 	.ndo_bridge_dellink	= swdev_port_bridge_dellink,
 	.ndo_features_check	= passthru_features_check,
 };
-- 
1.7.10.4

[PATCH net-next v4 22/24] team: cut over to new swdev_port_bridge_getlink

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 drivers/net/team/team.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 261e9c5..6500fd7 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1978,6 +1978,7 @@ static const struct net_device_ops team_netdev_ops = {
 	.ndo_fix_features	= team_fix_features,
 	.ndo_change_carrier     = team_change_carrier,
 	.ndo_bridge_setlink	= swdev_port_bridge_setlink,
+	.ndo_bridge_getlink	= swdev_port_bridge_getlink,
 	.ndo_bridge_dellink	= swdev_port_bridge_dellink,
 	.ndo_features_check	= passthru_features_check,
 };
-- 
1.7.10.4

[PATCH net-next v4 23/24] switchdev: convert fib_ipv4_add/del over to swdev_port_obj_add/del

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

The IPv4 FIB ops convert nicely to the swdev objs and we're left with only
four swdev ops: port get/set and port add/del.  Other objs will follow, such
as FDB.  So go ahead and convert IPv4 FIB over to swdev obj for consistency,
anticipating more objs to come.

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 drivers/net/ethernet/rocker/rocker.c |   42 ++++++-----------
 include/net/switchdev.h              |   44 ++++++++----------
 net/ipv4/fib_trie.c                  |   38 +++++++---------
 net/switchdev/switchdev.c            |   83 ++++++++++++++++++++--------------
 4 files changed, 99 insertions(+), 108 deletions(-)

diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index e927e22..261cb7f 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -4395,6 +4395,7 @@ static int rocker_port_vlans_add(struct rocker_port *rocker_port,
 static int rocker_port_obj_add(struct net_device *dev, struct swdev_obj *obj)
 {
 	struct rocker_port *rocker_port = netdev_priv(dev);
+	struct swdev_obj_ipv4_fib *fib4;
 	int err = 0;
 
 	rocker_port->trans = obj->trans;
@@ -4414,6 +4415,12 @@ static int rocker_port_obj_add(struct net_device *dev, struct swdev_obj *obj)
 	case SWDEV_OBJ_PORT_VLAN:
 		err = rocker_port_vlans_add(rocker_port, &obj->vlan);
 		break;
+	case SWDEV_OBJ_IPV4_FIB:
+		fib4 = &obj->ipv4_fib;
+		err = rocker_port_fib_ipv4(rocker_port, fib4->dst,
+					   fib4->dst_len, fib4->fi,
+					   fib4->tb_id, 0);
+		break;
 	default:
 		err = -EOPNOTSUPP;
 		break;
@@ -4455,12 +4462,20 @@ static int rocker_port_vlans_del(struct rocker_port *rocker_port,
 static int rocker_port_obj_del(struct net_device *dev, struct swdev_obj *obj)
 {
 	struct rocker_port *rocker_port = netdev_priv(dev);
+	struct swdev_obj_ipv4_fib *fib4;
 	int err = 0;
 
 	switch (obj->id) {
 	case SWDEV_OBJ_PORT_VLAN:
 		err = rocker_port_vlans_del(rocker_port, &obj->vlan);
 		break;
+	case SWDEV_OBJ_IPV4_FIB:
+		fib4 = &obj->ipv4_fib;
+		err = rocker_port_fib_ipv4(rocker_port, fib4->dst,
+					   fib4->dst_len, fib4->fi,
+					   fib4->tb_id,
+					   ROCKER_OP_FLAG_REMOVE);
+		break;
 	default:
 		err = -EOPNOTSUPP;
 		break;
@@ -4469,38 +4484,11 @@ static int rocker_port_obj_del(struct net_device *dev, struct swdev_obj *obj)
 	return err;
 }
 
-static int rocker_port_swdev_fib_ipv4_add(struct net_device *dev,
-					  __be32 dst, int dst_len,
-					  struct fib_info *fi,
-					  u8 tos, u8 type,
-					  u32 nlflags, u32 tb_id)
-{
-	struct rocker_port *rocker_port = netdev_priv(dev);
-	int flags = 0;
-
-	return rocker_port_fib_ipv4(rocker_port, dst, dst_len,
-				    fi, tb_id, flags);
-}
-
-static int rocker_port_swdev_fib_ipv4_del(struct net_device *dev,
-					  __be32 dst, int dst_len,
-					  struct fib_info *fi,
-					  u8 tos, u8 type, u32 tb_id)
-{
-	struct rocker_port *rocker_port = netdev_priv(dev);
-	int flags = ROCKER_OP_FLAG_REMOVE;
-
-	return rocker_port_fib_ipv4(rocker_port, dst, dst_len,
-				    fi, tb_id, flags);
-}
-
 static const struct swdev_ops rocker_port_swdev_ops = {
 	.swdev_port_attr_get		= rocker_port_attr_get,
 	.swdev_port_attr_set		= rocker_port_attr_set,
 	.swdev_port_obj_add		= rocker_port_obj_add,
 	.swdev_port_obj_del		= rocker_port_obj_del,
-	.swdev_fib_ipv4_add		= rocker_port_swdev_fib_ipv4_add,
-	.swdev_fib_ipv4_del		= rocker_port_swdev_fib_ipv4_del,
 };
 
 /********************
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index d04322e..85f0c0d 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -46,6 +46,7 @@ struct fib_info;
 enum swdev_obj_id {
 	SWDEV_OBJ_UNDEFINED,
 	SWDEV_OBJ_PORT_VLAN,
+	SWDEV_OBJ_IPV4_FIB,
 };
 
 struct swdev_obj {
@@ -58,6 +59,15 @@ struct swdev_obj {
 			u16 vid_start;
 			u16 vid_end;
 		} vlan;
+		struct swdev_obj_ipv4_fib {		/* IPV4_FIB */
+			u32 dst;
+			int dst_len;
+			struct fib_info *fi;
+			u8 tos;
+			u8 type;
+			u32 nlflags;
+			u32 tb_id;
+		} ipv4_fib;
 	};
 };
 
@@ -71,10 +81,6 @@ struct swdev_obj {
  * @swdev_port_obj_add: Add an object to port (see swdev_obj).
  *
  * @swdev_port_obj_del: Delete an object from port (see swdev_obj).
- *
- * @swdev_fib_ipv4_add: Called to add/modify IPv4 route to switch device.
- *
- * @swdev_fib_ipv4_del: Called to delete IPv4 route from switch device.
  */
 struct swdev_ops {
 	int	(*swdev_port_attr_get)(struct net_device *dev,
@@ -85,13 +91,6 @@ struct swdev_ops {
 				      struct swdev_obj *obj);
 	int	(*swdev_port_obj_del)(struct net_device *dev,
 				      struct swdev_obj *obj);
-	int	(*swdev_fib_ipv4_add)(struct net_device *dev, __be32 dst,
-				      int dst_len, struct fib_info *fi,
-				      u8 tos, u8 type, u32 nlflags,
-				      u32 tb_id);
-	int	(*swdev_fib_ipv4_del)(struct net_device *dev, __be32 dst,
-				      int dst_len, struct fib_info *fi,
-				      u8 tos, u8 type, u32 tb_id);
 };
 
 enum netdev_switch_notifier_type {
@@ -131,11 +130,11 @@ int register_netdev_switch_notifier(struct notifier_block *nb);
 int unregister_netdev_switch_notifier(struct notifier_block *nb);
 int call_netdev_switch_notifiers(unsigned long val, struct net_device *dev,
 				 struct netdev_switch_notifier_info *info);
-int netdev_switch_fib_ipv4_add(u32 dst, int dst_len, struct fib_info *fi,
-			       u8 tos, u8 type, u32 nlflags, u32 tb_id);
-int netdev_switch_fib_ipv4_del(u32 dst, int dst_len, struct fib_info *fi,
-			       u8 tos, u8 type, u32 tb_id);
-void netdev_switch_fib_ipv4_abort(struct fib_info *fi);
+int swdev_fib_ipv4_add(u32 dst, int dst_len, struct fib_info *fi,
+		       u8 tos, u8 type, u32 nlflags, u32 tb_id);
+int swdev_fib_ipv4_del(u32 dst, int dst_len, struct fib_info *fi,
+		       u8 tos, u8 type, u32 tb_id);
+void swdev_fib_ipv4_abort(struct fib_info *fi);
 
 #else
 
@@ -198,22 +197,19 @@ static inline int call_netdev_switch_notifiers(unsigned long val, struct net_dev
 	return NOTIFY_DONE;
 }
 
-static inline int netdev_switch_fib_ipv4_add(u32 dst, int dst_len,
-					     struct fib_info *fi,
-					     u8 tos, u8 type,
-					     u32 nlflags, u32 tb_id)
+static inline int swdev_fib_ipv4_add(u32 dst, int dst_len, struct fib_info *fi,
+				     u8 tos, u8 type, u32 nlflags, u32 tb_id)
 {
 	return 0;
 }
 
-static inline int netdev_switch_fib_ipv4_del(u32 dst, int dst_len,
-					     struct fib_info *fi,
-					     u8 tos, u8 type, u32 tb_id)
+static inline int swdev_fib_ipv4_del(u32 dst, int dst_len, struct fib_info *fi,
+				     u8 tos, u8 type, u32 tb_id)
 {
 	return 0;
 }
 
-static inline void netdev_switch_fib_ipv4_abort(struct fib_info *fi)
+static inline void swdev_fib_ipv4_abort(struct fib_info *fi)
 {
 }
 
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index e13fcc6..f108714 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -1165,13 +1165,11 @@ int fib_table_insert(struct fib_table *tb, struct fib_config *cfg)
 			new_fa->fa_state = state & ~FA_S_ACCESSED;
 			new_fa->fa_slen = fa->fa_slen;
 
-			err = netdev_switch_fib_ipv4_add(key, plen, fi,
-							 new_fa->fa_tos,
-							 cfg->fc_type,
-							 cfg->fc_nlflags,
-							 tb->tb_id);
+			err = swdev_fib_ipv4_add(key, plen, fi, new_fa->fa_tos,
+						 cfg->fc_type, cfg->fc_nlflags,
+						 tb->tb_id);
 			if (err) {
-				netdev_switch_fib_ipv4_abort(fi);
+				swdev_fib_ipv4_abort(fi);
 				kmem_cache_free(fn_alias_kmem, new_fa);
 				goto out;
 			}
@@ -1215,12 +1213,10 @@ int fib_table_insert(struct fib_table *tb, struct fib_config *cfg)
 	new_fa->tb_id = tb->tb_id;
 
 	/* (Optionally) offload fib entry to switch hardware. */
-	err = netdev_switch_fib_ipv4_add(key, plen, fi, tos,
-					 cfg->fc_type,
-					 cfg->fc_nlflags,
-					 tb->tb_id);
+	err = swdev_fib_ipv4_add(key, plen, fi, tos, cfg->fc_type,
+				 cfg->fc_nlflags, tb->tb_id);
 	if (err) {
-		netdev_switch_fib_ipv4_abort(fi);
+		swdev_fib_ipv4_abort(fi);
 		goto out_free_new_fa;
 	}
 
@@ -1239,7 +1235,7 @@ succeeded:
 	return 0;
 
 out_sw_fib_del:
-	netdev_switch_fib_ipv4_del(key, plen, fi, tos, cfg->fc_type, tb->tb_id);
+	swdev_fib_ipv4_del(key, plen, fi, tos, cfg->fc_type, tb->tb_id);
 out_free_new_fa:
 	kmem_cache_free(fn_alias_kmem, new_fa);
 out:
@@ -1517,8 +1513,8 @@ int fib_table_delete(struct fib_table *tb, struct fib_config *cfg)
 	if (!fa_to_delete)
 		return -ESRCH;
 
-	netdev_switch_fib_ipv4_del(key, plen, fa_to_delete->fa_info, tos,
-				   cfg->fc_type, tb->tb_id);
+	swdev_fib_ipv4_del(key, plen, fa_to_delete->fa_info, tos,
+			   cfg->fc_type, tb->tb_id);
 
 	rtmsg_fib(RTM_DELROUTE, htonl(key), fa_to_delete, plen, tb->tb_id,
 		  &cfg->fc_nlinfo, 0);
@@ -1767,10 +1763,9 @@ void fib_table_flush_external(struct fib_table *tb)
 			if (!fi || !(fi->fib_flags & RTNH_F_EXTERNAL))
 				continue;
 
-			netdev_switch_fib_ipv4_del(n->key,
-						   KEYLENGTH - fa->fa_slen,
-						   fi, fa->fa_tos,
-						   fa->fa_type, tb->tb_id);
+			swdev_fib_ipv4_del(n->key, KEYLENGTH - fa->fa_slen,
+					   fi, fa->fa_tos, fa->fa_type,
+					   tb->tb_id);
 		}
 
 		/* update leaf slen */
@@ -1835,10 +1830,9 @@ int fib_table_flush(struct fib_table *tb)
 				continue;
 			}
 
-			netdev_switch_fib_ipv4_del(n->key,
-						   KEYLENGTH - fa->fa_slen,
-						   fi, fa->fa_tos,
-						   fa->fa_type, tb->tb_id);
+			swdev_fib_ipv4_del(n->key, KEYLENGTH - fa->fa_slen,
+					   fi, fa->fa_tos, fa->fa_type,
+					   tb->tb_id);
 			hlist_del_rcu(&fa->fa_list);
 			fib_release_info(fa->fa_info);
 			alias_free_mem_rcu(fa);
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 6866e0c..bb3481d 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -573,7 +573,7 @@ int swdev_port_bridge_dellink(struct net_device *dev,
 }
 EXPORT_SYMBOL_GPL(swdev_port_bridge_dellink);
 
-static struct net_device *netdev_switch_get_lowest_dev(struct net_device *dev)
+static struct net_device *swdev_get_lowest_dev(struct net_device *dev)
 {
 	const struct swdev_ops *ops = dev->swdev_ops;
 	struct net_device *lower_dev;
@@ -588,7 +588,7 @@ static struct net_device *netdev_switch_get_lowest_dev(struct net_device *dev)
 		return dev;
 
 	netdev_for_each_lower_dev(dev, lower_dev, iter) {
-		port_dev = netdev_switch_get_lowest_dev(lower_dev);
+		port_dev = swdev_get_lowest_dev(lower_dev);
 		if (port_dev)
 			return port_dev;
 	}
@@ -596,7 +596,7 @@ static struct net_device *netdev_switch_get_lowest_dev(struct net_device *dev)
 	return NULL;
 }
 
-static struct net_device *netdev_switch_get_dev_by_nhs(struct fib_info *fi)
+static struct net_device *swdev_get_dev_by_nhs(struct fib_info *fi)
 {
 	struct swdev_attr attr = {
 		.id = SWDEV_ATTR_PORT_PARENT_ID,
@@ -613,7 +613,7 @@ static struct net_device *netdev_switch_get_dev_by_nhs(struct fib_info *fi)
 		if (!nh->nh_dev)
 			return NULL;
 
-		dev = netdev_switch_get_lowest_dev(nh->nh_dev);
+		dev = swdev_get_lowest_dev(nh->nh_dev);
 		if (!dev)
 			return NULL;
 
@@ -635,7 +635,7 @@ static struct net_device *netdev_switch_get_dev_by_nhs(struct fib_info *fi)
 }
 
 /**
- *	netdev_switch_fib_ipv4_add - Add IPv4 route entry to switch
+ *	swdev_fib_ipv4_add - Add IPv4 route entry to switch
  *
  *	@dst: route's IPv4 destination address
  *	@dst_len: destination address length (prefix length)
@@ -647,11 +647,22 @@ static struct net_device *netdev_switch_get_dev_by_nhs(struct fib_info *fi)
  *
  *	Add IPv4 route entry to switch device.
  */
-int netdev_switch_fib_ipv4_add(u32 dst, int dst_len, struct fib_info *fi,
-			       u8 tos, u8 type, u32 nlflags, u32 tb_id)
+int swdev_fib_ipv4_add(u32 dst, int dst_len, struct fib_info *fi,
+		       u8 tos, u8 type, u32 nlflags, u32 tb_id)
 {
+	struct swdev_obj fib_obj = {
+		.id = SWDEV_OBJ_IPV4_FIB,
+		.ipv4_fib = {
+			.dst = htonl(dst),
+			.dst_len = dst_len,
+			.fi = fi,
+			.tos = tos,
+			.type = type,
+			.nlflags = nlflags,
+			.tb_id = tb_id,
+		},
+	};
 	struct net_device *dev;
-	const struct swdev_ops *ops;
 	int err = 0;
 
 	/* Don't offload route if using custom ip rules or if
@@ -666,25 +677,20 @@ int netdev_switch_fib_ipv4_add(u32 dst, int dst_len, struct fib_info *fi,
 	if (fi->fib_net->ipv4.fib_offload_disabled)
 		return 0;
 
-	dev = netdev_switch_get_dev_by_nhs(fi);
+	dev = swdev_get_dev_by_nhs(fi);
 	if (!dev)
 		return 0;
-	ops = dev->swdev_ops;
-
-	if (ops->swdev_fib_ipv4_add) {
-		err = ops->swdev_fib_ipv4_add(dev, htonl(dst), dst_len,
-					      fi, tos, type, nlflags,
-					      tb_id);
-		if (!err)
-			fi->fib_flags |= RTNH_F_EXTERNAL;
-	}
+
+	err = swdev_port_obj_add(dev, &fib_obj);
+	if (!err)
+		fi->fib_flags |= RTNH_F_EXTERNAL;
 
 	return err;
 }
-EXPORT_SYMBOL_GPL(netdev_switch_fib_ipv4_add);
+EXPORT_SYMBOL_GPL(swdev_fib_ipv4_add);
 
 /**
- *	netdev_switch_fib_ipv4_del - Delete IPv4 route entry from switch
+ *	swdev_fib_ipv4_del - Delete IPv4 route entry from switch
  *
  *	@dst: route's IPv4 destination address
  *	@dst_len: destination address length (prefix length)
@@ -695,38 +701,45 @@ EXPORT_SYMBOL_GPL(netdev_switch_fib_ipv4_add);
  *
  *	Delete IPv4 route entry from switch device.
  */
-int netdev_switch_fib_ipv4_del(u32 dst, int dst_len, struct fib_info *fi,
-			       u8 tos, u8 type, u32 tb_id)
+int swdev_fib_ipv4_del(u32 dst, int dst_len, struct fib_info *fi,
+		       u8 tos, u8 type, u32 tb_id)
 {
+	struct swdev_obj fib_obj = {
+		.id = SWDEV_OBJ_IPV4_FIB,
+		.ipv4_fib = {
+			.dst = htonl(dst),
+			.dst_len = dst_len,
+			.fi = fi,
+			.tos = tos,
+			.type = type,
+			.nlflags = 0,
+			.tb_id = tb_id,
+		},
+	};
 	struct net_device *dev;
-	const struct swdev_ops *ops;
 	int err = 0;
 
 	if (!(fi->fib_flags & RTNH_F_EXTERNAL))
 		return 0;
 
-	dev = netdev_switch_get_dev_by_nhs(fi);
+	dev = swdev_get_dev_by_nhs(fi);
 	if (!dev)
 		return 0;
-	ops = dev->swdev_ops;
 
-	if (ops->swdev_fib_ipv4_del) {
-		err = ops->swdev_fib_ipv4_del(dev, htonl(dst), dst_len,
-					      fi, tos, type, tb_id);
-		if (!err)
-			fi->fib_flags &= ~RTNH_F_EXTERNAL;
-	}
+	err = swdev_port_obj_del(dev, &fib_obj);
+	if (!err)
+		fi->fib_flags &= ~RTNH_F_EXTERNAL;
 
 	return err;
 }
-EXPORT_SYMBOL_GPL(netdev_switch_fib_ipv4_del);
+EXPORT_SYMBOL_GPL(swdev_fib_ipv4_del);
 
 /**
- *	netdev_switch_fib_ipv4_abort - Abort an IPv4 FIB operation
+ *	swdev_fib_ipv4_abort - Abort an IPv4 FIB operation
  *
  *	@fi: route FIB info structure
  */
-void netdev_switch_fib_ipv4_abort(struct fib_info *fi)
+void swdev_fib_ipv4_abort(struct fib_info *fi)
 {
 	/* There was a problem installing this route to the offload
 	 * device.  For now, until we come up with more refined
@@ -739,4 +752,4 @@ void netdev_switch_fib_ipv4_abort(struct fib_info *fi)
 	fib_flush_external(fi->fib_net);
 	fi->fib_net->ipv4.fib_offload_disabled = true;
 }
-EXPORT_SYMBOL_GPL(netdev_switch_fib_ipv4_abort);
+EXPORT_SYMBOL_GPL(swdev_fib_ipv4_abort);
-- 
1.7.10.4

[PATCH net-next v4 24/24] switchdev: bring documentation up-to-date

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

Much need updated of swdev documentation to cover what's been implmented
to-date.  There are some XXX comments in the text for unimplemented or broken
items.  I'd like to keep these in there (poor-man's TODO list) and update the
document once each issue is resolved.

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 Documentation/networking/switchdev.txt |  414 +++++++++++++++++++++++++++-----
 1 file changed, 355 insertions(+), 59 deletions(-)

diff --git a/Documentation/networking/switchdev.txt b/Documentation/networking/switchdev.txt
index f981a92..f03d0f3 100644
--- a/Documentation/networking/switchdev.txt
+++ b/Documentation/networking/switchdev.txt
@@ -1,59 +1,355 @@
-Switch (and switch-ish) device drivers HOWTO
-===========================
-
-Please note that the word "switch" is here used in very generic meaning.
-This include devices supporting L2/L3 but also various flow offloading chips,
-including switches embedded into SR-IOV NICs.
-
-Lets describe a topology a bit. Imagine the following example:
-
-       +----------------------------+    +---------------+
-       |     SOME switch chip       |    |      CPU      |
-       +----------------------------+    +---------------+
-       port1 port2 port3 port4 MNGMNT    |     PCI-E     |
-         |     |     |     |     |       +---------------+
-        PHY   PHY    |     |     |         |  NIC0 NIC1
-                     |     |     |         |   |    |
-                     |     |     +- PCI-E -+   |    |
-                     |     +------- MII -------+    |
-                     +------------- MII ------------+
-
-In this example, there are two independent lines between the switch silicon
-and CPU. NIC0 and NIC1 drivers are not aware of a switch presence. They are
-separate from the switch driver. SOME switch chip is by managed by a driver
-via PCI-E device MNGMNT. Note that MNGMNT device, NIC0 and NIC1 may be
-connected to some other type of bus.
-
-Now, for the previous example show the representation in kernel:
-
-       +----------------------------+    +---------------+
-       |     SOME switch chip       |    |      CPU      |
-       +----------------------------+    +---------------+
-       sw0p0 sw0p1 sw0p2 sw0p3 MNGMNT    |     PCI-E     |
-         |     |     |     |     |       +---------------+
-        PHY   PHY    |     |     |         |  eth0 eth1
-                     |     |     |         |   |    |
-                     |     |     +- PCI-E -+   |    |
-                     |     +------- MII -------+    |
-                     +------------- MII ------------+
-
-Lets call the example switch driver for SOME switch chip "SOMEswitch". This
-driver takes care of PCI-E device MNGMNT. There is a netdevice instance sw0pX
-created for each port of a switch. These netdevices are instances
-of "SOMEswitch" driver. sw0pX netdevices serve as a "representation"
-of the switch chip. eth0 and eth1 are instances of some other existing driver.
-
-The only difference of the switch-port netdevice from the ordinary netdevice
-is that is implements couple more NDOs:
-
-  ndo_switch_parent_id_get - This returns the same ID for two port netdevices
-			     of the same physical switch chip. This is
-			     mandatory to be implemented by all switch drivers
-			     and serves the caller for recognition of a port
-			     netdevice.
-  ndo_switch_parent_* - Functions that serve for a manipulation of the switch
-			chip itself (it can be though of as a "parent" of the
-			port, therefore the name). They are not port-specific.
-			Caller might use arbitrary port netdevice of the same
-			switch and it will make no difference.
-  ndo_switch_port_* - Functions that serve for a port-specific manipulation.
+Ethernet switch device driver model (switchdev)
+===============================================
+Copyright (c) 2014 Jiri Pirko <jiri@resnulli.us>
+Copyright (c) 2014-2015 Scott Feldman <sfeldma@gmail.com>
+
+
+The Ethernet switch device driver model (switchdev) is an in-kernel driver
+model for switch devices which offload the forwarding (data) plane from the
+kernel.
+
+Figure 1 is a block diagram showing the components of the switchdev model for
+an example setup using a data-center-class switch ASIC chip.  Other setups
+with SR-IOV or soft switches, such as OVS, are possible.
+
+
+                             User-space tools                                 
+                                                                              
+       user space                   |                                         
+      +-------------------------------------------------------------------+   
+       kernel                       | Netlink                                 
+                                    |                                         
+                     +--------------+-------------------------------+         
+                     |         Network stack                        |         
+                     |           (Linux)                            |         
+                     |                                              |         
+                     +----------------------------------------------+         
+                                                                              
+                           sw1p2     sw1p4     sw1p6
+                      sw1p1  +  sw1p3  +  sw1p5  +          eth1             
+                        +    |    +    |    +    |            +               
+                        |    |    |    |    |    |            |               
+                     +--+----+----+----+-+--+----+---+  +-----+-----+         
+                     |         Switch driver         |  |    mgmt   |         
+                     |        (this document)        |  |   driver  |         
+                     |                               |  |           |         
+                     +--------------+----------------+  +-----------+         
+                                    |                                         
+       kernel                       | HW bus (eg PCI)                         
+      +-------------------------------------------------------------------+   
+       hardware                     |                                         
+                     +--------------+---+------------+                        
+                     |         Switch device (sw1)   |                        
+                     |  +----+                       +--------+               
+                     |  |    v offloaded data path   | mgmt port              
+                     |  |    |                       |                        
+                     +--|----|----+----+----+----+---+                        
+                        |    |    |    |    |    |                            
+                        +    +    +    +    +    +                            
+                       p1   p2   p3   p4   p5   p6
+                                       
+                             front-panel ports                                
+                                                                              
+
+                                    Fig 1.
+
+
+Include Files
+-------------
+
+#include <linux/netdevice.h>
+#include <net/switchdev.h>
+
+
+Configuration
+-------------
+
+Use "depends NET_SWITCHDEV" in driver's Kconfig to ensure switchdev model
+support is built for driver.
+
+
+Switch Ports
+------------
+
+On switchdev driver initialization, the driver will allocate and register a
+struct net_device (using register_netdev()) for each enumerated physical switch
+port, called the port netdev.  A port netdev is the software representation of
+the physical port and provides a conduit for control traffic to/from the
+controller (the kernel) and the network, as well as an anchor point for higher
+level constructs such as bridges, bonds, VLANs, tunnels, and L3 routers.  Using
+standard netdev tools (iproute2, ethtool, etc), the port netdev can also
+provide to the user access to the physical properties of the switch port such
+as PHY link state and I/O statistics.
+
+There is (currently) no higher-level kernel object for the switch beyond the
+port netdevs.  All of the switchdev driver ops are netdev ops or swdev ops.
+
+A switch management port is outside the scope of the switchdev driver model.
+Typically, the management port is not participating in offloaded data plane and
+is loaded with a different driver, such as a NIC driver, on the management port
+device.
+
+Port Netdev Naming
+^^^^^^^^^^^^^^^^^^
+
+Udev rules should be used for port netdev naming, using some unique attribute
+of the port as a key, for example the port MAC address or the port PHYS name.
+Hard-coding of kernel netdev names within the driver is discouraged; let the
+kernel pick the default netdev name, and let udev set the final name based on a
+port attribute.
+
+Using port PHYS name (ndo_get_phys_port_name) for the key is particularly
+useful for dynically-named ports where the device names it's ports based on
+external configuration.  For example, if a physical 40G port is split logically
+into 4 10G ports, resulting in 4 port netdevs, the device can give a unique
+name for each port using port PHYS name.  The udev rule would be:
+
+SUBSYSTEM=="net", ACTION=="add", DRIVER="<driver>", ATTR{phys_port_name}!="", \
+	NAME="$attr{phys_port_name}"
+
+Suggested naming convention is "swXpYsZ", where X is the switch name or ID, Y
+is the port name or ID, and Z is the sub-port name or ID.  For example, sw1p1s0
+would be sub-port 0 on port 1 on switch 1.
+
+Switch ID
+^^^^^^^^^
+
+The switchdev driver must implement the swdev op swdev_port_attr_get for
+SWDEV_ATTR_PORT_PARENT_ID for each port netdev, returning the same physical ID
+for each port of a switch.  The ID must be unique between switches on the same
+system.  The ID does not need to be unique between switches on different
+systems.
+
+The switch ID is used to locate ports on a switch and to know if aggregated
+ports belong to the same switch.
+
+Port Features
+^^^^^^^^^^^^^
+
+NETIF_F_NETNS_LOCAL
+
+If the switchdev driver (and device) only supports offloading of the default
+network namespace (netns), the driver should set this feature flag to prevent
+the port netdev from being moved out of the default netns.  A netns-aware
+driver/device would not set this flag and be resposible for partitioning
+hardware to preserve netns containment.  This means hardware cannot forward
+traffic from a port in one namespace to another port in another namespace.
+
+Port Topology
+^^^^^^^^^^^^^
+
+The port netdevs representing the physical switch ports can be organized into
+higher-level switching constructs.  The default construct is a standalone
+router port, used to offload L3 forwarding.  Two or more ports can be bonded
+together to form a LAG.  Two or more ports (or LAGs) can be bridged to bridge
+to L2 networks.  VLANs can be applied to sub-divide L2 networks.  L2-over-L3
+tunnels can be built on ports.  These constructs are built using standard Linux
+tools such as the bridge driver, the bonding/team drivers, and netlink-based
+tools such as iproute2.
+
+The switchdev driver can know a particular port's position in the topology by
+monitoring NETDEV_CHANGEUPPER notifications.  For example, a port moved into a
+bond will see it's upper master change.  If that bond is moved into a bridge,
+the bond's upper master will change.  And so on.  The driver will track such
+movements to know what position a port is in in the overall topology by
+registering for netdevice events and acting on NETDEV_CHANGEUPPER.
+
+L2 Forwarding Offload
+---------------------
+
+The idea is to offload the L2 data forwarding (switching) path from the kernel
+to the switchdev device by mirroring bridge FDB entries down to the device.  An
+FDB entry is the {port, MAC, VLAN} tuple forwarding destination.
+
+To offloading L2 bridging, the switchdev driver/device should support:
+
+	- Static FDB entries installed on a bridge port
+	- Notification of learned/forgotten src mac/vlans from device
+	- STP state changes on the port
+	- VLAN flooding of multicast/broadcast and unknown unicast packets
+
+Static FDB Entries
+^^^^^^^^^^^^^^^^^^
+
+The switchdev driver should implement ndo_fdb_add, ndo_fdb_del and ndo_fdb_dump
+to support static FDB entries installed to the device.  Static bridge FDB
+entries are installed, for example, using iproute2 bridge cmd:
+
+	bridge fdb add ADDR dev DEV [vlan VID] [self]
+
+Note: by default, the bridge does not filter on VLAN and only bridges untagged
+traffic.  To enable VLAN support, turn on VLAN filtering:
+
+	echo 1 >/sys/class/net/<bridge>/bridge/vlan_filtering
+
+Notification of Learned/Forgotten Source MAC/VLANs
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The switch device will learn/forget source MAC address/VLAN on ingress packets
+and notify the switch driver of the mac/vlan/port tuples.  The switch driver,
+in turn, will notify the bridge driver using the swdev notifier call:
+
+	err = call_swdev_notifiers(val, dev, info);
+
+Where val is SWDEV_FDB_ADD when learning and SWDEV_FDB_DEL when forgetting, and
+info points to a struct swdev_notifier_fdb_info.  On SWDEV_FDB_ADD, the bridge
+driver will install the FDB entry into the bridge's FDB and mark the entry as
+NTF_EXT_LEARNED.  The iproute2 bridge command will label these entries
+"offload":
+
+	$ bridge fdb
+	52:54:00:12:35:01 dev sw1p1 master br0 permanent
+	00:02:00:00:02:00 dev sw1p1 master br0 offload
+	00:02:00:00:02:00 dev sw1p1 self
+	52:54:00:12:35:02 dev sw1p2 master br0 permanent
+	00:02:00:00:03:00 dev sw1p2 master br0 offload
+	00:02:00:00:03:00 dev sw1p2 self
+	33:33:00:00:00:01 dev eth0 self permanent
+	01:00:5e:00:00:01 dev eth0 self permanent
+	33:33:ff:00:00:00 dev eth0 self permanent
+	01:80:c2:00:00:0e dev eth0 self permanent
+	33:33:00:00:00:01 dev br0 self permanent
+	01:00:5e:00:00:01 dev br0 self permanent
+	33:33:ff:12:35:01 dev br0 self permanent
+
+Learning on the port should be disabled on the bridge using the bridge command:
+
+	bridge link set dev DEV learning off
+
+Learning on the device port should be enabled, as well as learning_sync:
+
+	bridge link set dev DEV learning on self
+	bridge link set dev DEV learning_sync on self
+
+Learning_sync attribute enables syncing of the learned/forgotton FDB entry to
+the bridge's FDB.  It's possible, but not optimal, to enable learning on the
+device port and on the bridge port, and disable learning_sync.
+
+To support learning and learning_sync port attributes, the driver implements
+swdev op swdev_port_attr_get/set for SWDEV_ATTR_PORT_BRIDGE_FLAGS.  The driver
+should initialize the attributes to the hardware defaults.
+
+FDB Ageing
+^^^^^^^^^^
+
+There are two FDB ageing models supported: 1) ageing by the device, and 2)
+ageing by the kernel.  Ageing by the device is preferred if many FDB entries
+are supported.  The driver calls call_swdev_notifiers(SWDEV_FDB_DEL, ...) to
+age out the FDB entry.  In this model, ageing by the kernel should be turned
+off.  XXX: how to turn off ageing in kernel on a per-port basis or otherwise
+prevent the kernel from ageing out the FDB entry?
+
+In the kernel ageing model, the standard bridge ageing mechanism is used to age
+out stale FDB entries.  To keep an FDB entry "alive", the driver should refresh
+the FDB entry by calling call_swdev_notifiers(SWDEV_FDB_ADD, ...).  The
+notification will reset the FDB entry's last-used time to now.  The driver
+should rate limit refresh notifications, for example, no more than once a
+second.  If the FDB entry expires, ndo_fdb_del is called to remove entry from
+the device.  XXX: this last part isn't currently correct: ndo_fdb_del isn't
+called, so the stale entry remains in device...this need to get fixed.
+
+FDB Flush
+^^^^^^^^^
+
+XXX: Unimplemented.  Need to support FDB flush by bridge driver for port and
+remove both static and learned FDB entries.
+
+STP State Change on Port
+^^^^^^^^^^^^^^^^^^^^^^^^
+
+Internally or with a third-party STP protocol implementation (e.g. mstpd), the
+bridge driver maintains the STP state for ports, and will notify the switch
+driver of STP state change on a port using the swdev op swdev_attr_port_set for
+SWDEV_ATTR_PORT_STP_UPDATE.
+
+State is one of BR_STATE_*.  The switch driver can use STP state updates to
+update ingress packet filter list for the port.  For example, if port is
+DISABLED, no packets should pass, but if port moves to BLOCKED, then STP BPDUs
+and other IEEE 01:80:c2:xx:xx:xx link-local multicast packets can pass.
+
+Note that STP BDPUs are untagged and STP state applies to all VLANs on the port
+so packet filters should be applied consistently across untagged and tagged
+VLANs on the port.
+
+Flooding L2 domain
+^^^^^^^^^^^^^^^^^^
+
+For a given L2 VLAN domain, the switch device should flood multicast/broadcast
+and unknown unicast packets to all ports in domain, if allowed by port's
+current STP state.  The switch driver, knowing which ports are within which
+vlan L2 domain, can program the switch device for flooding.  The packet should
+also be sent to the port netdev for processing by the bridge driver.  The
+bridge should not reflood the packet to the same ports the device flooded.
+XXX: the mechanism to avoid duplicate flood packets is being discuseed.
+
+It is possible for the switch device to not handle flooding and push the
+packets up to the bridge driver for flooding.  This is not ideal as the number
+of ports scale in the L2 domain as the device is much more efficient at
+flooding packets that software.
+
+IGMP Snooping
+^^^^^^^^^^^^^
+
+XXX: complete this section
+
+
+L3 routing
+----------
+
+Offloading L3 routing requires that device be programmed with FIB entries from
+the kernel, with the device doing the FIB lookup and forwarding.  The device
+does a longest prefix match (LPM) on FIB entries matching route prefix and
+forwards the packet to the matching FIB entry's nexthop(s) egress ports.  To
+program the device, the switchdev driver is called with add/delete ops for IPv4
+and IPv6 FIB entries.  For IPv4, the driver implements swdev ops:
+
+	int (*swdev_fib_ipv4_add)(struct net_device *dev,
+				  __be32 dst, int dst_len,
+				  struct fib_info *fi,
+				  u8 tos, u8 type,
+				  u32 nlflags, u32 tb_id);
+
+	int (*swdev_fib_ipv4_del)(struct net_device *dev,
+				  __be32 dst, int dst_len,
+				  struct fib_info *fi,
+				  u8 tos, u8 type,
+				  u32 tb_id);
+
+to add/delete IPv4 dst/dest_len prefix on table tb_id.  The *fi structure holds
+details on the route and route's nexthops.  *dev is one of the port netdevs
+mentioned in the routes next hop list.  If the output port netdevs referenced
+in the route's nexthop list don't all have the same switch ID, the driver is
+not called to add/delete the FIB entry.
+
+Routes offloaded to the device are labeled with "offload" in the ip route
+listing:
+
+	$ ip route show
+	default via 192.168.0.2 dev eth0
+	11.0.0.0/30 dev sw1p1  proto kernel  scope link  src 11.0.0.2 offload
+	11.0.0.4/30 via 11.0.0.1 dev sw1p1  proto zebra  metric 20 offload
+	11.0.0.8/30 dev sw1p2  proto kernel  scope link  src 11.0.0.10 offload
+	11.0.0.12/30 via 11.0.0.9 dev sw1p2  proto zebra  metric 20 offload
+	12.0.0.2  proto zebra  metric 30 offload
+		nexthop via 11.0.0.1  dev sw1p1 weight 1
+		nexthop via 11.0.0.9  dev sw1p2 weight 1
+	12.0.0.3 via 11.0.0.1 dev sw1p1  proto zebra  metric 20 offload
+	12.0.0.4 via 11.0.0.9 dev sw1p2  proto zebra  metric 20 offload
+	192.168.0.0/24 dev eth0  proto kernel  scope link  src 192.168.0.15
+
+XXX: add/del IPv6 FIB API
+
+Nexthop Resolution
+^^^^^^^^^^^^^^^^^^
+
+The FIB entry's nexthop list contains the nexthop tuple (gateway, dev), but for
+the switch device to forward the packet with the correct dst mac address, the
+nexthop gateways must be resolved to the neighbor's mac address.  Neighbor mac
+address discovery comes via the ARP (or ND) process and is available via the
+arp_tbl neighbor table.  To resolve the routes nexthop gateways, the driver
+should trigger the kernel's neighbor resolution process.  See the rocker
+driver's rocker_port_ipv4_resolve() for an example.
+
+The driver can monitor for updates to arp_tbl using the netevent notifier
+NETEVENT_NEIGH_UPDATE.  The device can be programmed with resolved nexthops
+for the routes as arp_tbl updates.
-- 
1.7.10.4

Re: [PATCH net-next v4 00/24] switchdev: spring cleanup

[Jiri Pirko]

Mon, Apr 13, 2015 at 08:16:54AM CEST, sfeldma@gmail.com wrote:
>From: Scott Feldman <sfeldma@gmail.com>
>
>v4:
>
>Well, it was a lot of work, but now prepare-commit transaction model is how
>davem advises: if prepare fails, abort the transaction.  The driver must do
>resource reservations up front in prepare phase and return those resources if
>aborting.  Commit phase would use reserved resources.  The good news is the
>driver code (for rocker) now handles resource allocation failures better by not
>leaving partially device or driver states.  This is a side-effect of the
>prepare phase where state isn't modified; only validation of inputs and
>resource reservations happen in the prepare phase.  Since we're supporting
>setting attrs and add objs across lower devs in the stacked case, we need to
>hold rtnl_lock (or ensure rtnl_lock is held) so lower devs don't move on us
>during the prepare-commit transaction.  DSA driver code skips the prepare phase
>and goes straight for the commit phase since no up-front allocations are done
>and no device failures (that could be detected in the prepare phase) can
>happen.
>
>Remove NETIF_F_HW_SWITCH_OFFLOAD from rocker and the swdev_attr_set/get
>wrappers.  DSA doesn't set NETIF_F_HW_SWITCH_OFFLOAD, so it can't be in
>swdev_attr_set/get.  rocker doesn't need it; or rather can't support
>NETIF_F_HW_SWITCH_OFFLOAD being set/cleared at run-time after the device
>port is already up and offloading L2/L3.  NETIF_F_HW_SWITCH_OFFLOAD is still
>left as a feature flag for drivers that can use it.
>
>Drop the renaming patch for netdev_switch_notifier.  Other renames are a
>result of moving to the attr get/set or obj add/del model.  Everything
>but the netdev_switch_notifier is still prefixed with "swdev_".


I plan to review this patchset deeper today. But as I mentioned in the
v2 thread, "swdev_" is frowned upon by people. We should figure this out
now. How about to use full "switchdev"? Thanks.


>
>v3:
>
>Move to two-phase prepare-commit transaction model for attr set and obj add.
>Driver gets a change in prepare phase to NACK transaction if lack of resources
>or support in device.
>
>v2:
>
>Address review comments:
>
> - [Jiri] squash a few related patches
> - [Roopa] don't remove NETIF_F_HW_SWITCH_OFFLOAD
> - [Roopa] address VLAN setlink/dellink
> - [Ronen] print warning is attr set revert fails
>
>Not address:
>
> - Using something other than "swdev_" prefix
> - Vendor extentions
>
>The patch set grew a bit to not only support port attr get/set but also add
>support for port obj add/del.  Example of port objs are VLAN, FDB entries, and
>FIB entries.  The VLAN support now allows the swdev driver to get VLAN ranges
>and flags like PVID and "untagged".  Sridhar will be adding FDB obj support
>in follow-on patch.
>
>
>v1:
>
>The main theme of this patch set is to cleanup swdev in preparation for
>new features or fixes to be added soon.  We have a pretty good idea now how
>to handle stacked drivers in swdev, but there where some loose ends.  For
>example, if a set failed in the middle of walking the lower devs, we would
>leave the system in an undefined state...there was no way to recover back to
>the previous state.  Speaking of sets, also recognize a pattern that most
>swdev API accesses are gets or sets of port attributes, so go ahead and make
>port attr get/set the central swdev API, and convert everything that is
>set-ish/get-ish to this new API.
>
>Features/fixes that should follow from this cleanup:
>
> - solve the duplicate pkt forwarding issue
> - get/set bridge attrs, like ageing_time, from/to device
> - get/set more bridge port attrs from/to device
>
>There are some rename cleanups tagging along at the end, to give swdev
>consistent naming.
>
>And finally, some much needed updates to the switchdev.txt documentation to
>hopefully capture the state-of-the-art of swdev.  Hopefully, we can do a better
>job keeping this document up-to-date.
>
>Tested with rocker, of course, to make sure nothing functional broke.  There
>are a couple minor tweaks to DSA code for getting switch ID and setting STP
>updates to use new API, but not expecting amy breakage there.
>
>
>
>Scott Feldman (24):
>  switchdev: introduce get/set attrs ops
>  switchdev: convert parent_id_get to swdev attr get
>  switchdev: convert STP update to swdev attr set
>  switchdev: add bridge port flags attr
>  rocker: use swdev get/set attr for bridge port flags
>  switchdev: introduce swdev add/del obj ops
>  switchdev: add port vlan obj
>  rocker: use swdev add/del obj for bridge port vlans
>  switchdev: add new swdev bridge setlink
>  rocker: cut over to new swdev_port_bridge_setlink
>  bonding: cut over to new swdev_port_bridge_setlink
>  team: cut over to new swdev_port_bridge_setlink
>  switchdev: remove old netdev_switch_port_bridge_setlink
>  switchdev: add new swdev_port_bridge_dellink
>  rocker: cut over to new swdev_port_bridge_dellink
>  bonding: cut over to new swdev_port_bridge_dellink
>  team: cut over to new swdev_port_bridge_dellink
>  switchdev: remove unused netdev_switch_port_bridge_dellink
>  switchdev: add new swdev_port_bridge_getlink
>  rocker: cut over to new swdev_port_bridge_getlink
>  bonding: cut over to new swdev_port_bridge_getlink
>  team: cut over to new swdev_port_bridge_getlink
>  switchdev: convert fib_ipv4_add/del over to swdev_port_obj_add/del
>  switchdev: bring documentation up-to-date
>
> Documentation/networking/switchdev.txt |  414 ++++++++++++++++++---
> drivers/net/bonding/bond_main.c        |    5 +-
> drivers/net/ethernet/rocker/rocker.c   |  497 ++++++++++++++++++-------
> drivers/net/team/team.c                |    5 +-
> include/net/switchdev.h                |  177 +++++----
> net/bridge/br_netlink.c                |   24 +-
> net/bridge/br_stp.c                    |    6 +-
> net/core/net-sysfs.c                   |   10 +-
> net/core/rtnetlink.c                   |    9 +-
> net/dsa/slave.c                        |   36 +-
> net/ipv4/fib_trie.c                    |   38 +-
> net/switchdev/switchdev.c              |  623 +++++++++++++++++++++++++-------
> 12 files changed, 1386 insertions(+), 458 deletions(-)
>
>-- 
>1.7.10.4
>

Re: [PATCH net-next v4 01/24] switchdev: introduce get/set attrs ops

[Jiri Pirko]

Mon, Apr 13, 2015 at 08:16:55AM CEST, sfeldma@gmail.com wrote:
>From: Scott Feldman <sfeldma@gmail.com>
>
>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.  For sets, set the same attr value for all lower devs.  We'll
>use a two-phase prepare-commit transaction model for sets.  In the first phase,
>the driver(s) are asked if attr set is OK.  If all OK, the commit attr set in
>second phase.  A driver would NACK the prepare phase if it can't set the attr
>due to lack of resources or support, within it's control.  RTNL lock must be
>held across both phases because we'll recurse all lower devs first in prepare
>phase, and then recurse all lower devs again in commit phase.  If any lower dev
>fails the prepare phase, we need to abort the transaction for all lower devs.
>
>If lower dev recusion isn't desired, allow a flag SWDEV_F_NO_RECURSE to
>indicate get/set only work on port (lowest) device.
>
>Signed-off-by: Scott Feldman <sfeldma@gmail.com>

...

>+
>+struct swdev_attr_set_defer {

how about
PREFIX_port_attr_set_work ?

>+	struct work_struct work;
>+	struct net *net;
>+	int ifindex;
>+	struct swdev_attr attr;
>+};
>+
>+static void swdev_port_attr_set_defer_work(struct work_struct *work)

PREFIX_port_attr_set_work ?

>+{
>+	struct swdev_attr_set_defer *aw =
>+		container_of(work, struct swdev_attr_set_defer, work);
>+	struct net_device *dev;
>+	int err;
>+
>+	rtnl_lock();
>+	dev = __dev_get_by_index(aw->net, aw->ifindex);
>+	if (dev) {
>+		err = swdev_port_attr_set(dev, &aw->attr);
>+		WARN(err, "%s: Deferred set of attr (id=%d) failed.\n",
>+		     dev->name, aw->attr.id);
>+	}
>+	rtnl_unlock();
>+
>+	kfree(work);
>+}
>+
>+static int swdev_port_attr_set_defer(struct net_device *dev,
>+				     struct swdev_attr *attr)
>+{
>+	struct swdev_attr_set_defer *aw;

swdev_attr_set_defer and "aw"? How about rather:
PREFIX_port_attr_set_work and "asw"?


>+
>+	aw = kmalloc(sizeof(*aw), GFP_ATOMIC);
>+	if (!aw)
>+		return -ENOMEM;
>+
>+	INIT_WORK(&aw->work, swdev_port_attr_set_defer_work);
>+
>+	aw->net = dev_net(dev);
>+	aw->ifindex = dev->ifindex;
>+	memcpy(&aw->attr, attr, sizeof(aw->attr));
>+
>+	schedule_work(&aw->work);
>+
>+	return 0;
>+}
>+
>+/**
>+ *	swdev_port_attr_set - Set port attribute
>+ *
>+ *	@dev: port device
>+ *	@attr: attribute to set
>+ *
>+ *	Use a 2-phase prepare-commit transaction model to ensure
>+ *	system is not left in a partially updated state due to
>+ *	failure from driver/device.
>+ *
>+ *	rtnl_lock must be held.

Not true.

>+ */
>+int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
>+{
>+	int err;
>+
>+	if (!rtnl_is_locked()) {
>+		/* Running prepare-commit transaction across stacked
>+		 * devices requires nothing moves, so if rtnl_lock is
>+		 * not held, schedule a worker thread to hold rtnl_lock
>+		 * while setting attr.
>+		 */
>+
>+		return swdev_port_attr_set_defer(dev, attr);
>+	}
>+
>+	/* Phase I: prepare for attr set. 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
>+	 * should reserve resources needed for the commit phase here,
>+	 * but should not commit the attr.
>+	 */
>+
>+	attr->trans = SWDEV_TRANS_PREPARE;
>+	err = __swdev_port_attr_set(dev, attr);


Hmm. This looks odd having the type of action as a field in struct. I
understand that it remover the code duplication. But how about to make
it a param of __swdev_port_attr_set and there, use different ndo for
different action? The "switch-case" has to be somewhere, so why don't
have it in common code instead of every driver.



>+	if (err) {
>+		/* Prepare phase failed: abort the transaction.  Any
>+		 * resources reserved in the prepare phase are
>+		 * released.
>+		 */
>+
>+		attr->trans = SWDEV_TRANS_ABORT;
>+		__swdev_port_attr_set(dev, attr);
>+
>+		return err;
>+	}
>+
>+	/* Phase II: commit attr set.  This cannot fail as a fault
>+	 * of driver/device.  If it does, it's a bug in the driver/device
>+	 * because the driver said everythings was OK in phase I.
>+	 */
>+
>+	attr->trans = SWDEV_TRANS_COMMIT;
>+	err = __swdev_port_attr_set(dev, attr);
>+	WARN(err, "%s: Commit of attr (id=%d) failed.\n", dev->name, attr->id);

I would not be afraid to BUG it here...


>+
>+	return err;
>+}
>+EXPORT_SYMBOL_GPL(swdev_port_attr_set);
>+
>+/**
>  *	netdev_switch_port_stp_update - Notify switch device port of STP
>  *					state change
>  *	@dev: port device
>-- 
>1.7.10.4
>

Re: [PATCH net-next v4 05/24] rocker: use swdev get/set attr for bridge port flags

[Jiri Pirko]

Mon, Apr 13, 2015 at 08:16:59AM CEST, sfeldma@gmail.com wrote:
>From: Scott Feldman <sfeldma@gmail.com>
>
>Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>---
> drivers/net/ethernet/rocker/rocker.c |   31 +++++++++++++++++++++++++++++--
> 1 file changed, 29 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
>index 80c7f6f..9bd4fc6 100644
>--- a/drivers/net/ethernet/rocker/rocker.c
>+++ b/drivers/net/ethernet/rocker/rocker.c
>@@ -359,6 +359,8 @@ static void *__rocker_port_alloc(struct rocker_port *rocker_port,
> 		if (elem)
> 			INIT_LIST_HEAD(elem);
> 		break;
>+	default:
>+		break;
> 	}
> 
> 	return elem ? elem + 1 : NULL;
>@@ -4332,6 +4334,9 @@ static int rocker_port_attr_get(struct net_device *dev, struct swdev_attr *attr)
> 		attr->ppid.id_len = sizeof(rocker->hw.id);
> 		memcpy(&attr->ppid.id, &rocker->hw.id, attr->ppid.id_len);
> 		break;
>+	case SWDEV_ATTR_PORT_BRIDGE_FLAGS:


This patch should be squashed with the previous one (04)

Re: [PATCH net-next v4 07/24] switchdev: add port vlan obj

[Jiri Pirko]

Mon, Apr 13, 2015 at 08:17:01AM CEST, sfeldma@gmail.com wrote:
>From: Scott Feldman <sfeldma@gmail.com>
>
>VLAN obj has flags (PVID and untagged) as well as start and end vid ranges.
>The swdev driver can optimize programing the device using the ranges.
>
>Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>---
> include/net/switchdev.h |    8 ++++++++
> 1 file changed, 8 insertions(+)
>
>diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>index f5f7733..d3cc8eb 100644
>--- a/include/net/switchdev.h
>+++ b/include/net/switchdev.h
>@@ -45,12 +45,20 @@ struct fib_info;
> 
> enum swdev_obj_id {
> 	SWDEV_OBJ_UNDEFINED,
>+	SWDEV_OBJ_PORT_VLAN,
> };
> 
> struct swdev_obj {
> 	enum swdev_obj_id id;
> 	enum swdev_trans trans;
> 	u32 flags;
>+	union {
>+		struct swdev_obj_vlan {			/* PORT_VLAN */
>+			u16 flags;
>+			u16 vid_start;
>+			u16 vid_end;
>+		} vlan;
>+	};

Same here. Please squash this with the next patch.


> };
> 
> /**
>-- 
>1.7.10.4
>

Re: [PATCH net-next v4 07/24] switchdev: add port vlan obj

[Jiri Pirko]

Mon, Apr 13, 2015 at 03:16:25PM CEST, jiri@resnulli.us wrote:
>Mon, Apr 13, 2015 at 08:17:01AM CEST, sfeldma@gmail.com wrote:
>>From: Scott Feldman <sfeldma@gmail.com>
>>
>>VLAN obj has flags (PVID and untagged) as well as start and end vid ranges.
>>The swdev driver can optimize programing the device using the ranges.
>>
>>Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>>---
>> include/net/switchdev.h |    8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>>diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>>index f5f7733..d3cc8eb 100644
>>--- a/include/net/switchdev.h
>>+++ b/include/net/switchdev.h
>>@@ -45,12 +45,20 @@ struct fib_info;
>> 
>> enum swdev_obj_id {
>> 	SWDEV_OBJ_UNDEFINED,
>>+	SWDEV_OBJ_PORT_VLAN,
>> };
>> 
>> struct swdev_obj {
>> 	enum swdev_obj_id id;
>> 	enum swdev_trans trans;
>> 	u32 flags;
>>+	union {
>>+		struct swdev_obj_vlan {			/* PORT_VLAN */
>>+			u16 flags;
>>+			u16 vid_start;
>>+			u16 vid_end;
>>+		} vlan;
>>+	};
>
>Same here. Please squash this with the next patch.

I mean 09. The infrastructure should be first, usage (by driver) after
that.

>
>
>> };
>> 
>> /**
>>-- 
>>1.7.10.4
>>

Re: [PATCH net-next v4 07/24] switchdev: add port vlan obj

[Florian Fainelli]

On 12/04/15 23:17, sfeldma@gmail.com wrote:
> From: Scott Feldman <sfeldma@gmail.com>
> 
> VLAN obj has flags (PVID and untagged) as well as start and end vid ranges.
> The swdev driver can optimize programing the device using the ranges.
> 
> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
> ---
>  include/net/switchdev.h |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
> index f5f7733..d3cc8eb 100644
> --- a/include/net/switchdev.h
> +++ b/include/net/switchdev.h
> @@ -45,12 +45,20 @@ struct fib_info;
>  
>  enum swdev_obj_id {
>  	SWDEV_OBJ_UNDEFINED,
> +	SWDEV_OBJ_PORT_VLAN,
>  };
>  
>  struct swdev_obj {
>  	enum swdev_obj_id id;
>  	enum swdev_trans trans;
>  	u32 flags;
> +	union {
> +		struct swdev_obj_vlan {			/* PORT_VLAN */
> +			u16 flags;

Should not we enforce some sort of definition of how we plan on using
the "flags" member here?, if not, we might just as well remove it from
this patch set until we actually use it?
-- 
Florian

Re: [PATCH net-next v4 03/24] switchdev: convert STP update to swdev attr set

[Florian Fainelli]

On 12/04/15 23:16, sfeldma@gmail.com wrote:
> From: Scott Feldman <sfeldma@gmail.com>
> 
> STP update is just a writable port attribute, so convert swdev_port_stp_update
> to an attr set.
> 
> For rocker, support prepare-commit transaction model for setting STP state.
> This requires rocker to preallocate memory needed for the commit up front in
> the prepare phase.  Since rtnl_lock is held between prepare-commit, store the
> allocated memory on a queue hanging off of the rocker_port.  Also, in prepare
> phase, do everything right up to calling into HW.  The same code paths are
> tranversed in the driver for both prepare and commit phases.  In some cases,
> any state modified in the prepare phase must be reverted before returning
> so the commit phase makes the same decisions.

I now better understand the reason for introducing both the object and
transactional model, but that is really a dramatic change in the API
complexity now, and unfortunately this is reflected in the drivers using
it...

Can we try to classify switches by their underlying control/transport
bus, essentially sleeping vs. non-sleeping I/O operations and provide a
good enough abstraction from there which avoids drivers like rocker to
have to maintain a list of transactions?
-- 
Florian

Re: [PATCH net-next v4 13/24] switchdev: remove old netdev_switch_port_bridge_setlink

[roopa]

On 4/12/15, 11:17 PM, sfeldma@gmail.com wrote:
> From: Scott Feldman <sfeldma@gmail.com>
>
> New attr-based bridge_setlink can recurse lower devs and recover on err, so
> remove old wrapper.  Also, restore br_setlink back to original and don't call
> into SELF port driver.  rtnetlink.c:bridge_setlink already does a call into
> port driver for SELF.
>
> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
> ---
>   include/net/switchdev.h   |   18 ---------------
>   net/bridge/br_netlink.c   |   12 +---------
>   net/switchdev/switchdev.c |   55 ---------------------------------------------
>   3 files changed, 1 insertion(+), 84 deletions(-)
>
> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
> index cff53ae..70874fc 100644
> --- a/include/net/switchdev.h
> +++ b/include/net/switchdev.h
> @@ -127,14 +127,10 @@ int register_netdev_switch_notifier(struct notifier_block *nb);
>   int unregister_netdev_switch_notifier(struct notifier_block *nb);
>   int call_netdev_switch_notifiers(unsigned long val, struct net_device *dev,
>   				 struct netdev_switch_notifier_info *info);
> -int netdev_switch_port_bridge_setlink(struct net_device *dev,
> -				struct nlmsghdr *nlh, u16 flags);
>   int netdev_switch_port_bridge_dellink(struct net_device *dev,
>   				struct nlmsghdr *nlh, u16 flags);
>   int ndo_dflt_netdev_switch_port_bridge_dellink(struct net_device *dev,
>   					       struct nlmsghdr *nlh, u16 flags);
> -int ndo_dflt_netdev_switch_port_bridge_setlink(struct net_device *dev,
> -					       struct nlmsghdr *nlh, u16 flags);
>   int netdev_switch_fib_ipv4_add(u32 dst, int dst_len, struct fib_info *fi,
>   			       u8 tos, u8 type, u32 nlflags, u32 tb_id);
>   int netdev_switch_fib_ipv4_del(u32 dst, int dst_len, struct fib_info *fi,
> @@ -189,13 +185,6 @@ static inline int call_netdev_switch_notifiers(unsigned long val, struct net_dev
>   	return NOTIFY_DONE;
>   }
>   
> -static inline int netdev_switch_port_bridge_setlink(struct net_device *dev,
> -						    struct nlmsghdr *nlh,
> -						    u16 flags)
> -{
> -	return -EOPNOTSUPP;
> -}
> -
>   static inline int netdev_switch_port_bridge_dellink(struct net_device *dev,
>   						    struct nlmsghdr *nlh,
>   						    u16 flags)
> @@ -210,13 +199,6 @@ static inline int ndo_dflt_netdev_switch_port_bridge_dellink(struct net_device *
>   	return 0;
>   }
>   
> -static inline int ndo_dflt_netdev_switch_port_bridge_setlink(struct net_device *dev,
> -							struct nlmsghdr *nlh,
> -							u16 flags)
> -{
> -	return 0;
> -}
> -
>   static inline int netdev_switch_fib_ipv4_add(u32 dst, int dst_len,
>   					     struct fib_info *fi,
>   					     u8 tos, u8 type,
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 0e4ddb8..45ed1ce 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -586,7 +586,7 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh, u16 flags)
>   	struct nlattr *afspec;
>   	struct net_bridge_port *p;
>   	struct nlattr *tb[IFLA_BRPORT_MAX + 1];
> -	int err = 0, ret_offload = 0;
> +	int err = 0;
>   
>   	protinfo = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_PROTINFO);
>   	afspec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_AF_SPEC);
> @@ -628,16 +628,6 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh, u16 flags)
>   				afspec, RTM_SETLINK);
>   	}
>   
> -	if (p && !(flags & BRIDGE_FLAGS_SELF)) {
> -		/* set bridge attributes in hardware if supported
> -		 */
> -		ret_offload = netdev_switch_port_bridge_setlink(dev, nlh,
> -								flags);
> -		if (ret_offload && ret_offload != -EOPNOTSUPP)
> -			br_warn(p->br, "error setting attrs on port %u(%s)\n",
> -				(unsigned int)p->port_no, p->dev->name);
> -	}
> -

This was there to keep the offload semantics consistent with offload of 
other kernel objects (eg. fib).
your patch-set mandates 'master' and 'self' to program hardware and this 
also generates two notifications
to users-pace.

Re: [PATCH net-next v4 09/24] switchdev: add new swdev bridge setlink

[roopa]

On 4/12/15, 11:17 PM, sfeldma@gmail.com wrote:
> From: Scott Feldman <sfeldma@gmail.com>
>
> Add new swdev_port_bridge_setlink that can be used by drivers implementing
> .ndo_bridge_setlink to set swdev bridge attributes.  Basically turn the raw
> rtnl_bridge_setlink netlink into swdev attr sets.  Proper netlink attr policy
> checking is done on the protinfo part of the netlink msg.
>
> Currently, for protinfo, only bridge port attrs BR_LEARNING and
> BR_LEARNING_SYNC are parsed and passed to port driver.
>
> For afspec, VLAN objs are passed so swdev driver can set VLANs assigned to
> SELF.  To illustrate with iproute2 cmd, we have:
>
> 	bridge vlan add vid 10 dev sw1p1 self master
>
> To add VLAN 10 to port sw1p1 for both the bridge (master) and the device
> (self).
>
> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
> ---
>   include/net/switchdev.h   |    8 +++
>   net/switchdev/switchdev.c |  152 +++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 160 insertions(+)
>
> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
> index d3cc8eb..cff53ae 100644
> --- a/include/net/switchdev.h
> +++ b/include/net/switchdev.h
> @@ -121,6 +121,8 @@ int swdev_port_attr_get(struct net_device *dev, struct swdev_attr *attr);
>   int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr);
>   int swdev_port_obj_add(struct net_device *dev, struct swdev_obj *obj);
>   int swdev_port_obj_del(struct net_device *dev, struct swdev_obj *obj);
> +int swdev_port_bridge_setlink(struct net_device *dev, struct nlmsghdr *nlh,
> +			      u16 flags);
>   int register_netdev_switch_notifier(struct notifier_block *nb);
>   int unregister_netdev_switch_notifier(struct notifier_block *nb);
>   int call_netdev_switch_notifiers(unsigned long val, struct net_device *dev,
> @@ -165,6 +167,12 @@ static inline int swdev_port_obj_del(struct net_device *dev,
>   	return -EOPNOTSUPP;
>   }
>   
> +static inline int swdev_port_bridge_setlink(struct net_device *dev,
> +					    struct nlmsghdr *nlh, u16 flags)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>   static inline int register_netdev_switch_notifier(struct notifier_block *nb)
>   {
>   	return 0;
> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
> index 05fded9..31d55e7 100644
> --- a/net/switchdev/switchdev.c
> +++ b/net/switchdev/switchdev.c
> @@ -15,6 +15,7 @@
>   #include <linux/mutex.h>
>   #include <linux/notifier.h>
>   #include <linux/netdevice.h>
> +#include <linux/if_bridge.h>
>   #include <net/ip_fib.h>
>   #include <net/switchdev.h>
>   
> @@ -395,6 +396,157 @@ int netdev_switch_port_bridge_setlink(struct net_device *dev,
>   }
>   EXPORT_SYMBOL_GPL(netdev_switch_port_bridge_setlink);
>   
> +static int swdev_port_br_setflag(struct net_device *dev, struct nlattr *nlattr,
> +				 unsigned long brport_flag)
> +{
> +	struct swdev_attr attr = {
> +		.id = SWDEV_ATTR_PORT_BRIDGE_FLAGS,
> +	};
> +	u8 flag = nla_get_u8(nlattr);
> +	int err;
> +
> +	err = swdev_port_attr_get(dev, &attr);
> +	if (err)
> +		return err;
> +
> +	if (flag)
> +		attr.brport_flags |= brport_flag;
> +	else
> +		attr.brport_flags &= ~brport_flag;
> +
> +	return swdev_port_attr_set(dev, &attr);
> +}
> +
> +static const struct nla_policy swdev_port_bridge_policy[IFLA_BRPORT_MAX + 1] = {
> +	[IFLA_BRPORT_STATE]		= { .type = NLA_U8 },
> +	[IFLA_BRPORT_COST]		= { .type = NLA_U32 },
> +	[IFLA_BRPORT_PRIORITY]		= { .type = NLA_U16 },
> +	[IFLA_BRPORT_MODE]		= { .type = NLA_U8 },
> +	[IFLA_BRPORT_GUARD]		= { .type = NLA_U8 },
> +	[IFLA_BRPORT_PROTECT]		= { .type = NLA_U8 },
> +	[IFLA_BRPORT_FAST_LEAVE]	= { .type = NLA_U8 },
> +	[IFLA_BRPORT_LEARNING]		= { .type = NLA_U8 },
> +	[IFLA_BRPORT_LEARNING_SYNC]	= { .type = NLA_U8 },
> +	[IFLA_BRPORT_UNICAST_FLOOD]	= { .type = NLA_U8 },
> +};
> +
> +static int swdev_port_br_setlink_protinfo(struct net_device *dev,
> +					  struct nlattr *protinfo)
> +{
> +	struct nlattr *attr;
> +	int rem;
> +	int err;
> +
> +	err = nla_validate_nested(protinfo, IFLA_BRPORT_MAX,
> +				  swdev_port_bridge_policy);
> +	if (err)
> +		return err;
> +
> +	nla_for_each_nested(attr, protinfo, rem) {
> +		switch (nla_type(attr)) {
> +		case IFLA_BRPORT_LEARNING:
> +			err = swdev_port_br_setflag(dev, attr,
> +						    BR_LEARNING);
> +			break;
> +		case IFLA_BRPORT_LEARNING_SYNC:
> +			err = swdev_port_br_setflag(dev, attr,
> +						    BR_LEARNING_SYNC);
> +			break;
> +		default:
> +			err = -EOPNOTSUPP;
> +			break;
> +		}
> +		if (err)
> +			return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static int swdev_port_br_afspec(struct net_device *dev,
> +				struct nlattr *afspec,
> +				int (*f)(struct net_device *dev,
> +					 struct swdev_obj *obj))
> +{
> +	struct nlattr *attr;
> +	struct bridge_vlan_info *vinfo;
> +	struct swdev_obj obj = {
> +		.id = SWDEV_OBJ_PORT_VLAN,
> +	};
> +	int rem;
> +	int err;
> +
> +	nla_for_each_nested(attr, afspec, rem) {
> +		if (nla_type(attr) != IFLA_BRIDGE_VLAN_INFO)
> +			continue;
> +		if (nla_len(attr) != sizeof(struct bridge_vlan_info))
> +			return -EINVAL;
> +		vinfo = nla_data(attr);
> +		obj.vlan.flags = vinfo->flags;
> +		if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_BEGIN) {
> +			if (obj.vlan.vid_start)
> +				return -EINVAL;
> +			obj.vlan.vid_start = vinfo->vid;
> +		} else if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_END) {
> +			if (!obj.vlan.vid_start)
> +				return -EINVAL;
> +			obj.vlan.vid_end = vinfo->vid;
> +			if (obj.vlan.vid_end <= obj.vlan.vid_start)
> +				return -EINVAL;
> +			err = f(dev, &obj);
> +			if (err)
> +				return err;
> +			memset(&obj.vlan, 0, sizeof(obj.vlan));
> +		} else {
> +			if (obj.vlan.vid_start)
> +				return -EINVAL;
> +			obj.vlan.vid_start = vinfo->vid;
> +			obj.vlan.vid_end = vinfo->vid;
> +			err = f(dev, &obj);
> +			if (err)
> +				return err;
> +			memset(&obj.vlan, 0, sizeof(obj.vlan));
> +		}
> +	}
> +
> +	return 0;
> +}
> +

This seems to be duplicating code that is already there in the bridge 
driver.

Re: [PATCH net-next v4 09/24] switchdev: add new swdev bridge setlink

[Jiri Pirko]

Tue, Apr 14, 2015 at 06:09:28AM CEST, roopa@cumulusnetworks.com wrote:
>On 4/12/15, 11:17 PM, sfeldma@gmail.com wrote:
>>From: Scott Feldman <sfeldma@gmail.com>
>>
>>Add new swdev_port_bridge_setlink that can be used by drivers implementing
>>.ndo_bridge_setlink to set swdev bridge attributes.  Basically turn the raw
>>rtnl_bridge_setlink netlink into swdev attr sets.  Proper netlink attr policy
>>checking is done on the protinfo part of the netlink msg.
>>
>>Currently, for protinfo, only bridge port attrs BR_LEARNING and
>>BR_LEARNING_SYNC are parsed and passed to port driver.
>>
>>For afspec, VLAN objs are passed so swdev driver can set VLANs assigned to
>>SELF.  To illustrate with iproute2 cmd, we have:
>>
>>	bridge vlan add vid 10 dev sw1p1 self master
>>
>>To add VLAN 10 to port sw1p1 for both the bridge (master) and the device
>>(self).
>>
>>Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>>---
>>  include/net/switchdev.h   |    8 +++
>>  net/switchdev/switchdev.c |  152 +++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 160 insertions(+)
>>

...

>>+static int swdev_port_br_afspec(struct net_device *dev,
>>+				struct nlattr *afspec,
>>+				int (*f)(struct net_device *dev,
>>+					 struct swdev_obj *obj))
>>+{
>>+	struct nlattr *attr;
>>+	struct bridge_vlan_info *vinfo;
>>+	struct swdev_obj obj = {
>>+		.id = SWDEV_OBJ_PORT_VLAN,
>>+	};
>>+	int rem;
>>+	int err;
>>+
>>+	nla_for_each_nested(attr, afspec, rem) {
>>+		if (nla_type(attr) != IFLA_BRIDGE_VLAN_INFO)
>>+			continue;
>>+		if (nla_len(attr) != sizeof(struct bridge_vlan_info))
>>+			return -EINVAL;
>>+		vinfo = nla_data(attr);
>>+		obj.vlan.flags = vinfo->flags;
>>+		if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_BEGIN) {
>>+			if (obj.vlan.vid_start)
>>+				return -EINVAL;
>>+			obj.vlan.vid_start = vinfo->vid;
>>+		} else if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_END) {
>>+			if (!obj.vlan.vid_start)
>>+				return -EINVAL;
>>+			obj.vlan.vid_end = vinfo->vid;
>>+			if (obj.vlan.vid_end <= obj.vlan.vid_start)
>>+				return -EINVAL;
>>+			err = f(dev, &obj);
>>+			if (err)
>>+				return err;
>>+			memset(&obj.vlan, 0, sizeof(obj.vlan));
>>+		} else {
>>+			if (obj.vlan.vid_start)
>>+				return -EINVAL;
>>+			obj.vlan.vid_start = vinfo->vid;
>>+			obj.vlan.vid_end = vinfo->vid;
>>+			err = f(dev, &obj);
>>+			if (err)
>>+				return err;
>>+			memset(&obj.vlan, 0, sizeof(obj.vlan));
>>+		}
>>+	}
>>+
>>+	return 0;
>>+}
>>+
>
>This seems to be duplicating code that is already there in the bridge driver.
>
 
Sure, this is used by swdev_port_bridge_setlink, default ndo.

Re: [PATCH net-next v4 09/24] switchdev: add new swdev bridge setlink

[roopa]

On 4/12/15, 11:17 PM, sfeldma@gmail.com wrote:
> From: Scott Feldman <sfeldma@gmail.com>
>
> Add new swdev_port_bridge_setlink that can be used by drivers implementing
> .ndo_bridge_setlink to set swdev bridge attributes.  Basically turn the raw
> rtnl_bridge_setlink netlink into swdev attr sets.  Proper netlink attr policy
> checking is done on the protinfo part of the netlink msg.
>
> Currently, for protinfo, only bridge port attrs BR_LEARNING and
> BR_LEARNING_SYNC are parsed and passed to port driver.
>
> For afspec, VLAN objs are passed so swdev driver can set VLANs assigned to
> SELF.  To illustrate with iproute2 cmd, we have:
>
> 	bridge vlan add vid 10 dev sw1p1 self master
>
> To add VLAN 10 to port sw1p1 for both the bridge (master) and the device
> (self).

This results in vlan sets/dels/notification code duplicated in both 
bridge and switchdev layer.
>
> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
> ---
>   include/net/switchdev.h   |    8 +++
>   net/switchdev/switchdev.c |  152 +++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 160 insertions(+)
>
> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
> index d3cc8eb..cff53ae 100644
> --- a/include/net/switchdev.h
> +++ b/include/net/switchdev.h
> @@ -121,6 +121,8 @@ int swdev_port_attr_get(struct net_device *dev, struct swdev_attr *attr);
>   int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr);
>   int swdev_port_obj_add(struct net_device *dev, struct swdev_obj *obj);
>   int swdev_port_obj_del(struct net_device *dev, struct swdev_obj *obj);
> +int swdev_port_bridge_setlink(struct net_device *dev, struct nlmsghdr *nlh,
> +			      u16 flags);
>   int register_netdev_switch_notifier(struct notifier_block *nb);
>   int unregister_netdev_switch_notifier(struct notifier_block *nb);
>   int call_netdev_switch_notifiers(unsigned long val, struct net_device *dev,
> @@ -165,6 +167,12 @@ static inline int swdev_port_obj_del(struct net_device *dev,
>   	return -EOPNOTSUPP;
>   }
>   
> +static inline int swdev_port_bridge_setlink(struct net_device *dev,
> +					    struct nlmsghdr *nlh, u16 flags)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>   static inline int register_netdev_switch_notifier(struct notifier_block *nb)
>   {
>   	return 0;
> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
> index 05fded9..31d55e7 100644
> --- a/net/switchdev/switchdev.c
> +++ b/net/switchdev/switchdev.c
> @@ -15,6 +15,7 @@
>   #include <linux/mutex.h>
>   #include <linux/notifier.h>
>   #include <linux/netdevice.h>
> +#include <linux/if_bridge.h>
>   #include <net/ip_fib.h>
>   #include <net/switchdev.h>
>   
> @@ -395,6 +396,157 @@ int netdev_switch_port_bridge_setlink(struct net_device *dev,
>   }
>   EXPORT_SYMBOL_GPL(netdev_switch_port_bridge_setlink);
>   
> +static int swdev_port_br_setflag(struct net_device *dev, struct nlattr *nlattr,
> +				 unsigned long brport_flag)
> +{
> +	struct swdev_attr attr = {
> +		.id = SWDEV_ATTR_PORT_BRIDGE_FLAGS,
> +	};
> +	u8 flag = nla_get_u8(nlattr);
> +	int err;
> +
> +	err = swdev_port_attr_get(dev, &attr);
> +	if (err)
> +		return err;
> +
> +	if (flag)
> +		attr.brport_flags |= brport_flag;
> +	else
> +		attr.brport_flags &= ~brport_flag;
> +
> +	return swdev_port_attr_set(dev, &attr);
> +}
> +
> +static const struct nla_policy swdev_port_bridge_policy[IFLA_BRPORT_MAX + 1] = {
> +	[IFLA_BRPORT_STATE]		= { .type = NLA_U8 },
> +	[IFLA_BRPORT_COST]		= { .type = NLA_U32 },
> +	[IFLA_BRPORT_PRIORITY]		= { .type = NLA_U16 },
> +	[IFLA_BRPORT_MODE]		= { .type = NLA_U8 },
> +	[IFLA_BRPORT_GUARD]		= { .type = NLA_U8 },
> +	[IFLA_BRPORT_PROTECT]		= { .type = NLA_U8 },
> +	[IFLA_BRPORT_FAST_LEAVE]	= { .type = NLA_U8 },
> +	[IFLA_BRPORT_LEARNING]		= { .type = NLA_U8 },
> +	[IFLA_BRPORT_LEARNING_SYNC]	= { .type = NLA_U8 },
> +	[IFLA_BRPORT_UNICAST_FLOOD]	= { .type = NLA_U8 },
> +};
> +
> +static int swdev_port_br_setlink_protinfo(struct net_device *dev,
> +					  struct nlattr *protinfo)
> +{
> +	struct nlattr *attr;
> +	int rem;
> +	int err;
> +
> +	err = nla_validate_nested(protinfo, IFLA_BRPORT_MAX,
> +				  swdev_port_bridge_policy);
> +	if (err)
> +		return err;
> +
> +	nla_for_each_nested(attr, protinfo, rem) {
> +		switch (nla_type(attr)) {
> +		case IFLA_BRPORT_LEARNING:
> +			err = swdev_port_br_setflag(dev, attr,
> +						    BR_LEARNING);
> +			break;
> +		case IFLA_BRPORT_LEARNING_SYNC:
> +			err = swdev_port_br_setflag(dev, attr,
> +						    BR_LEARNING_SYNC);
> +			break;
> +		default:
> +			err = -EOPNOTSUPP;
> +			break;
> +		}
> +		if (err)
> +			return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static int swdev_port_br_afspec(struct net_device *dev,
> +				struct nlattr *afspec,
> +				int (*f)(struct net_device *dev,
> +					 struct swdev_obj *obj))
> +{
> +	struct nlattr *attr;
> +	struct bridge_vlan_info *vinfo;
> +	struct swdev_obj obj = {
> +		.id = SWDEV_OBJ_PORT_VLAN,
> +	};
> +	int rem;
> +	int err;
> +
> +	nla_for_each_nested(attr, afspec, rem) {
> +		if (nla_type(attr) != IFLA_BRIDGE_VLAN_INFO)
> +			continue;
> +		if (nla_len(attr) != sizeof(struct bridge_vlan_info))
> +			return -EINVAL;
> +		vinfo = nla_data(attr);
> +		obj.vlan.flags = vinfo->flags;
> +		if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_BEGIN) {
> +			if (obj.vlan.vid_start)
> +				return -EINVAL;
> +			obj.vlan.vid_start = vinfo->vid;
> +		} else if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_END) {
> +			if (!obj.vlan.vid_start)
> +				return -EINVAL;
> +			obj.vlan.vid_end = vinfo->vid;
> +			if (obj.vlan.vid_end <= obj.vlan.vid_start)
> +				return -EINVAL;
> +			err = f(dev, &obj);
> +			if (err)
> +				return err;
> +			memset(&obj.vlan, 0, sizeof(obj.vlan));
> +		} else {
> +			if (obj.vlan.vid_start)
> +				return -EINVAL;
> +			obj.vlan.vid_start = vinfo->vid;
> +			obj.vlan.vid_end = vinfo->vid;
> +			err = f(dev, &obj);
> +			if (err)
> +				return err;
> +			memset(&obj.vlan, 0, sizeof(obj.vlan));
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + *	swdev_port_bridge_setlink - Set bridge port attributes
> + *
> + *	@dev: port device
> + *	@nlh: netlink header
> + *	@flags: netlink flags
> + *
> + *	Called for SELF on rtnl_bridge_setlink to set bridge port
> + *	attributes.
> + */
> +int swdev_port_bridge_setlink(struct net_device *dev,
> +			      struct nlmsghdr *nlh, u16 flags)
> +{
> +	struct nlattr *protinfo;
> +	struct nlattr *afspec;
> +	int err = 0;
> +
> +	protinfo = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg),
> +				   IFLA_PROTINFO);
> +	if (protinfo) {
> +		err = swdev_port_br_setlink_protinfo(dev, protinfo);
> +		if (err)
> +			return err;
> +	}
> +
> +	afspec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg),
> +				 IFLA_AF_SPEC);
> +	if (afspec)
> +		err = swdev_port_br_afspec(dev, afspec,
> +					   swdev_port_obj_add);
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(swdev_port_bridge_setlink);
> +
>   /**
>    *	netdev_switch_port_bridge_dellink - Notify switch device port of bridge
>    *	port attribute delete

Re: [PATCH net-next v4 19/24] switchdev: add new swdev_port_bridge_getlink

[roopa]

On 4/12/15, 11:17 PM, sfeldma@gmail.com wrote:
> From: Scott Feldman <sfeldma@gmail.com>
>
> Like bridge_setlink, add swdev wrapper to handle bridge_getlink and call into
> port driver to get port attrs.  For now, only BR_LEARNING and BR_LEARNING_SYNC
> are returned.  To add more, we'll probably want to break away from
> ndo_dflt_bridge_getlink() and build the netlink skb directly in the swdev code.
>
> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
> ---
>   include/net/switchdev.h   |    9 +++++++++
>   net/switchdev/switchdev.c |   27 +++++++++++++++++++++++++++
>   2 files changed, 36 insertions(+)
>
> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
> index 8c9814f..d04322e 100644
> --- a/include/net/switchdev.h
> +++ b/include/net/switchdev.h
> @@ -121,6 +121,8 @@ int swdev_port_attr_get(struct net_device *dev, struct swdev_attr *attr);
>   int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr);
>   int swdev_port_obj_add(struct net_device *dev, struct swdev_obj *obj);
>   int swdev_port_obj_del(struct net_device *dev, struct swdev_obj *obj);
> +int swdev_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
> +			      struct net_device *dev, u32 filter_mask);
>   int swdev_port_bridge_setlink(struct net_device *dev, struct nlmsghdr *nlh,
>   			      u16 flags);
>   int swdev_port_bridge_dellink(struct net_device *dev, struct nlmsghdr *nlh,
> @@ -161,6 +163,13 @@ static inline int swdev_port_obj_del(struct net_device *dev,
>   	return -EOPNOTSUPP;
>   }
>   
> +static inline int swdev_port_bridge_getlink(struct sk_buff *skb, u32 pid,
> +					    u32 seq, struct net_device *dev,
> +					    u32 filter_mask)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>   static inline int swdev_port_bridge_setlink(struct net_device *dev,
>   					    struct nlmsghdr *nlh, u16 flags)
>   {
> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
> index ae0ed06..6866e0c 100644
> --- a/net/switchdev/switchdev.c
> +++ b/net/switchdev/switchdev.c
> @@ -371,6 +371,33 @@ int call_netdev_switch_notifiers(unsigned long val, struct net_device *dev,
>   }
>   EXPORT_SYMBOL_GPL(call_netdev_switch_notifiers);
>   
> +/**
> + *	swdev_port_bridge_getlink - Get bridge port attributes
> + *
> + *	@dev: port device
> + *
> + *	Called for SELF on rtnl_bridge_getlink to get bridge port
> + *	attributes.
> + */
> +int swdev_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
> +			      struct net_device *dev, u32 filter_mask)
> +{
> +	struct swdev_attr attr = {
> +		.id = SWDEV_ATTR_PORT_BRIDGE_FLAGS,
> +	};
> +	u16 mode = BRIDGE_MODE_UNDEF;
> +	u32 mask = BR_LEARNING | BR_LEARNING_SYNC;
> +	int err;
> +
> +	err = swdev_port_attr_get(dev, &attr);
> +	if (err)
> +		return err;
> +
> +	return ndo_dflt_bridge_getlink(skb, pid, seq, dev, mode,
> +				       attr.brport_flags, mask);
> +}
> +EXPORT_SYMBOL_GPL(swdev_port_bridge_getlink);

This should ideally get vlans as well because setlink/dellink supports 
add/del vlans.
This can however be an incremental patch. I think this was the point 
arad was making as well some time back.

Re: [PATCH net-next v4 00/24] switchdev: spring cleanup

[roopa]

On 4/12/15, 11:16 PM, sfeldma@gmail.com wrote:
> From: Scott Feldman <sfeldma@gmail.com>
>
> v4:
>
> Well, it was a lot of work, but now prepare-commit transaction model is how
> davem advises: if prepare fails, abort the transaction.  The driver must do
> resource reservations up front in prepare phase and return those resources if
> aborting.  Commit phase would use reserved resources.  The good news is the
> driver code (for rocker) now handles resource allocation failures better by not
> leaving partially device or driver states.  This is a side-effect of the
> prepare phase where state isn't modified; only validation of inputs and
> resource reservations happen in the prepare phase.  Since we're supporting
> setting attrs and add objs across lower devs in the stacked case, we need to
> hold rtnl_lock (or ensure rtnl_lock is held) so lower devs don't move on us
> during the prepare-commit transaction.  DSA driver code skips the prepare phase
> and goes straight for the commit phase since no up-front allocations are done
> and no device failures (that could be detected in the prepare phase) can
> happen.

thanks for the series. It definitely does look cleaner and less 
confusing now!.
I do love the abstraction but i was one of the people voting against 
duplicating the
kernel objects into swdev objs which this patches does (which i am still 
not convinced
we should have).

>
> Remove NETIF_F_HW_SWITCH_OFFLOAD from rocker and the swdev_attr_set/get
> wrappers.  DSA doesn't set NETIF_F_HW_SWITCH_OFFLOAD, so it can't be in
> swdev_attr_set/get.  rocker doesn't need it; or rather can't support
> NETIF_F_HW_SWITCH_OFFLOAD being set/cleared at run-time after the device
> port is already up and offloading L2/L3.  NETIF_F_HW_SWITCH_OFFLOAD is still
> left as a feature flag for drivers that can use it.

I see that this series removes all uses of it in the switchdev api 
later. I had summarized
the need for the flag in reply to one of your questions a few weeks 
back. Since you have moved all
ndo ops to swdev ops (including ndo_bridge_setlink/dellink), I don't 
want to hold on to the
  feature flag if no one is using it. yes, my userspace driver uses it 
today.
I will come back with stronger justification to keep it or
will submit a patch to remove it and add it back at a later point if needed.


Thanks,
Roopa

Re: [PATCH net-next v4 01/24] switchdev: introduce get/set attrs ops

[Scott Feldman]

On Mon, Apr 13, 2015 at 3:43 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Mon, Apr 13, 2015 at 08:16:55AM CEST, sfeldma@gmail.com wrote:
>>From: Scott Feldman <sfeldma@gmail.com>
>>
>>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.  For sets, set the same attr value for all lower devs.  We'll
>>use a two-phase prepare-commit transaction model for sets.  In the first phase,
>>the driver(s) are asked if attr set is OK.  If all OK, the commit attr set in
>>second phase.  A driver would NACK the prepare phase if it can't set the attr
>>due to lack of resources or support, within it's control.  RTNL lock must be
>>held across both phases because we'll recurse all lower devs first in prepare
>>phase, and then recurse all lower devs again in commit phase.  If any lower dev
>>fails the prepare phase, we need to abort the transaction for all lower devs.
>>
>>If lower dev recusion isn't desired, allow a flag SWDEV_F_NO_RECURSE to
>>indicate get/set only work on port (lowest) device.
>>
>>Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>
> ...
>
>>+
>>+struct swdev_attr_set_defer {
>
> how about
> PREFIX_port_attr_set_work ?
>
>>+      struct work_struct work;
>>+      struct net *net;
>>+      int ifindex;
>>+      struct swdev_attr attr;
>>+};
>>+
>>+static void swdev_port_attr_set_defer_work(struct work_struct *work)
>
> PREFIX_port_attr_set_work ?

done in v5

>>+{
>>+      struct swdev_attr_set_defer *aw =
>>+              container_of(work, struct swdev_attr_set_defer, work);
>>+      struct net_device *dev;
>>+      int err;
>>+
>>+      rtnl_lock();
>>+      dev = __dev_get_by_index(aw->net, aw->ifindex);
>>+      if (dev) {
>>+              err = swdev_port_attr_set(dev, &aw->attr);
>>+              WARN(err, "%s: Deferred set of attr (id=%d) failed.\n",
>>+                   dev->name, aw->attr.id);
>>+      }
>>+      rtnl_unlock();
>>+
>>+      kfree(work);
>>+}
>>+
>>+static int swdev_port_attr_set_defer(struct net_device *dev,
>>+                                   struct swdev_attr *attr)
>>+{
>>+      struct swdev_attr_set_defer *aw;
>
> swdev_attr_set_defer and "aw"? How about rather:
> PREFIX_port_attr_set_work and "asw"?

done in v5

>>+
>>+      aw = kmalloc(sizeof(*aw), GFP_ATOMIC);
>>+      if (!aw)
>>+              return -ENOMEM;
>>+
>>+      INIT_WORK(&aw->work, swdev_port_attr_set_defer_work);
>>+
>>+      aw->net = dev_net(dev);
>>+      aw->ifindex = dev->ifindex;
>>+      memcpy(&aw->attr, attr, sizeof(aw->attr));
>>+
>>+      schedule_work(&aw->work);
>>+
>>+      return 0;
>>+}
>>+
>>+/**
>>+ *    swdev_port_attr_set - Set port attribute
>>+ *
>>+ *    @dev: port device
>>+ *    @attr: attribute to set
>>+ *
>>+ *    Use a 2-phase prepare-commit transaction model to ensure
>>+ *    system is not left in a partially updated state due to
>>+ *    failure from driver/device.
>>+ *
>>+ *    rtnl_lock must be held.
>
> Not true.

updated comment in v5.


>>+ */
>>+int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
>>+{
>>+      int err;
>>+
>>+      if (!rtnl_is_locked()) {
>>+              /* Running prepare-commit transaction across stacked
>>+               * devices requires nothing moves, so if rtnl_lock is
>>+               * not held, schedule a worker thread to hold rtnl_lock
>>+               * while setting attr.
>>+               */
>>+
>>+              return swdev_port_attr_set_defer(dev, attr);
>>+      }
>>+
>>+      /* Phase I: prepare for attr set. 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
>>+       * should reserve resources needed for the commit phase here,
>>+       * but should not commit the attr.
>>+       */
>>+
>>+      attr->trans = SWDEV_TRANS_PREPARE;
>>+      err = __swdev_port_attr_set(dev, attr);
>
>
> Hmm. This looks odd having the type of action as a field in struct. I
> understand that it remover the code duplication. But how about to make
> it a param of __swdev_port_attr_set and there, use different ndo for
> different action? The "switch-case" has to be somewhere, so why don't
> have it in common code instead of every driver.

You'll end up with duplicate code in driver if using a ndo op for each
action because prepare and commit actions typically want the same code
path in the driver.

>
>
>
>>+      if (err) {
>>+              /* Prepare phase failed: abort the transaction.  Any
>>+               * resources reserved in the prepare phase are
>>+               * released.
>>+               */
>>+
>>+              attr->trans = SWDEV_TRANS_ABORT;
>>+              __swdev_port_attr_set(dev, attr);
>>+
>>+              return err;
>>+      }
>>+
>>+      /* Phase II: commit attr set.  This cannot fail as a fault
>>+       * of driver/device.  If it does, it's a bug in the driver/device
>>+       * because the driver said everythings was OK in phase I.
>>+       */
>>+
>>+      attr->trans = SWDEV_TRANS_COMMIT;
>>+      err = __swdev_port_attr_set(dev, attr);
>>+      WARN(err, "%s: Commit of attr (id=%d) failed.\n", dev->name, attr->id);
>
> I would not be afraid to BUG it here...

Do we take down the system when we detect a driver/device bug?  I'll
change it to BUG_ON(err)

Re: [PATCH net-next v4 07/24] switchdev: add port vlan obj

[Scott Feldman]

On Mon, Apr 13, 2015 at 10:49 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 12/04/15 23:17, sfeldma@gmail.com wrote:
>> From: Scott Feldman <sfeldma@gmail.com>
>>
>> VLAN obj has flags (PVID and untagged) as well as start and end vid ranges.
>> The swdev driver can optimize programing the device using the ranges.
>>
>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>> ---
>>  include/net/switchdev.h |    8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>> index f5f7733..d3cc8eb 100644
>> --- a/include/net/switchdev.h
>> +++ b/include/net/switchdev.h
>> @@ -45,12 +45,20 @@ struct fib_info;
>>
>>  enum swdev_obj_id {
>>       SWDEV_OBJ_UNDEFINED,
>> +     SWDEV_OBJ_PORT_VLAN,
>>  };
>>
>>  struct swdev_obj {
>>       enum swdev_obj_id id;
>>       enum swdev_trans trans;
>>       u32 flags;
>> +     union {
>> +             struct swdev_obj_vlan {                 /* PORT_VLAN */
>> +                     u16 flags;
>
> Should not we enforce some sort of definition of how we plan on using
> the "flags" member here?, if not, we might just as well remove it from
> this patch set until we actually use it?

removed in v5

Re: [PATCH net-next v4 03/24] switchdev: convert STP update to swdev attr set

[Scott Feldman]

On Mon, Apr 13, 2015 at 12:22 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 12/04/15 23:16, sfeldma@gmail.com wrote:
>> From: Scott Feldman <sfeldma@gmail.com>
>>
>> STP update is just a writable port attribute, so convert swdev_port_stp_update
>> to an attr set.
>>
>> For rocker, support prepare-commit transaction model for setting STP state.
>> This requires rocker to preallocate memory needed for the commit up front in
>> the prepare phase.  Since rtnl_lock is held between prepare-commit, store the
>> allocated memory on a queue hanging off of the rocker_port.  Also, in prepare
>> phase, do everything right up to calling into HW.  The same code paths are
>> tranversed in the driver for both prepare and commit phases.  In some cases,
>> any state modified in the prepare phase must be reverted before returning
>> so the commit phase makes the same decisions.
>
> I now better understand the reason for introducing both the object and
> transactional model, but that is really a dramatic change in the API
> complexity now, and unfortunately this is reflected in the drivers using
> it...

Yes, the drivers must now individually handle each transaction phase
at the level that makes sense for the driver/device.  For DSA, the
prepare phase is skipped and only the commit phase is run, as would
have happened before all this transactional model work.  Rocker gets
more interesting because the driver allocates memory as it goes and
updates driver and device state in multiple steps, any which can fail,
so the prepare phase needs to do a lot of work before giving the all
clear for commit.  I'm not sure how to up-level the prepare phase
since it's very device/driver specific.

> Can we try to classify switches by their underlying control/transport
> bus, essentially sleeping vs. non-sleeping I/O operations and provide a
> good enough abstraction from there which avoids drivers like rocker to
> have to maintain a list of transactions?

I'm not sure I follow the sleeping vs. non-sleeping classification.

Re: [PATCH net-next v4 19/24] switchdev: add new swdev_port_bridge_getlink

[Scott Feldman]

On Mon, Apr 13, 2015 at 10:47 PM, roopa <roopa@cumulusnetworks.com> wrote:
> On 4/12/15, 11:17 PM, sfeldma@gmail.com wrote:
>>
>> From: Scott Feldman <sfeldma@gmail.com>
>>
>> Like bridge_setlink, add swdev wrapper to handle bridge_getlink and call
>> into
>> port driver to get port attrs.  For now, only BR_LEARNING and
>> BR_LEARNING_SYNC
>> are returned.  To add more, we'll probably want to break away from
>> ndo_dflt_bridge_getlink() and build the netlink skb directly in the swdev
>> code.
>>
>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>> ---
>>   include/net/switchdev.h   |    9 +++++++++
>>   net/switchdev/switchdev.c |   27 +++++++++++++++++++++++++++
>>   2 files changed, 36 insertions(+)
>>
>> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>> index 8c9814f..d04322e 100644
>> --- a/include/net/switchdev.h
>> +++ b/include/net/switchdev.h
>> @@ -121,6 +121,8 @@ int swdev_port_attr_get(struct net_device *dev, struct
>> swdev_attr *attr);
>>   int swdev_port_attr_set(struct net_device *dev, struct swdev_attr
>> *attr);
>>   int swdev_port_obj_add(struct net_device *dev, struct swdev_obj *obj);
>>   int swdev_port_obj_del(struct net_device *dev, struct swdev_obj *obj);
>> +int swdev_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
>> +                             struct net_device *dev, u32 filter_mask);
>>   int swdev_port_bridge_setlink(struct net_device *dev, struct nlmsghdr
>> *nlh,
>>                               u16 flags);
>>   int swdev_port_bridge_dellink(struct net_device *dev, struct nlmsghdr
>> *nlh,
>> @@ -161,6 +163,13 @@ static inline int swdev_port_obj_del(struct
>> net_device *dev,
>>         return -EOPNOTSUPP;
>>   }
>>   +static inline int swdev_port_bridge_getlink(struct sk_buff *skb, u32
>> pid,
>> +                                           u32 seq, struct net_device
>> *dev,
>> +                                           u32 filter_mask)
>> +{
>> +       return -EOPNOTSUPP;
>> +}
>> +
>>   static inline int swdev_port_bridge_setlink(struct net_device *dev,
>>                                             struct nlmsghdr *nlh, u16
>> flags)
>>   {
>> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
>> index ae0ed06..6866e0c 100644
>> --- a/net/switchdev/switchdev.c
>> +++ b/net/switchdev/switchdev.c
>> @@ -371,6 +371,33 @@ int call_netdev_switch_notifiers(unsigned long val,
>> struct net_device *dev,
>>   }
>>   EXPORT_SYMBOL_GPL(call_netdev_switch_notifiers);
>>   +/**
>> + *     swdev_port_bridge_getlink - Get bridge port attributes
>> + *
>> + *     @dev: port device
>> + *
>> + *     Called for SELF on rtnl_bridge_getlink to get bridge port
>> + *     attributes.
>> + */
>> +int swdev_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
>> +                             struct net_device *dev, u32 filter_mask)
>> +{
>> +       struct swdev_attr attr = {
>> +               .id = SWDEV_ATTR_PORT_BRIDGE_FLAGS,
>> +       };
>> +       u16 mode = BRIDGE_MODE_UNDEF;
>> +       u32 mask = BR_LEARNING | BR_LEARNING_SYNC;
>> +       int err;
>> +
>> +       err = swdev_port_attr_get(dev, &attr);
>> +       if (err)
>> +               return err;
>> +
>> +       return ndo_dflt_bridge_getlink(skb, pid, seq, dev, mode,
>> +                                      attr.brport_flags, mask);
>> +}
>> +EXPORT_SYMBOL_GPL(swdev_port_bridge_getlink);
>
>
> This should ideally get vlans as well because setlink/dellink supports
> add/del vlans.
> This can however be an incremental patch. I think this was the point arad
> was making as well some time back.

Ronan (Arad) has this patch ready but he's waiting on my patchset to
go in before he submits it.

Re: [PATCH net-next v4 05/24] rocker: use swdev get/set attr for bridge port flags

[Simon Horman]

Hi Scott,

On Sun, Apr 12, 2015 at 11:16:59PM -0700, sfeldma@gmail.com wrote:
> From: Scott Feldman <sfeldma@gmail.com>
> 
> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
> ---
>  drivers/net/ethernet/rocker/rocker.c |   31 +++++++++++++++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
> index 80c7f6f..9bd4fc6 100644
> --- a/drivers/net/ethernet/rocker/rocker.c
> +++ b/drivers/net/ethernet/rocker/rocker.c

[snip]

> @@ -4349,6 +4354,23 @@ static void rocker_port_trans_abort(struct rocker_port *rocker_port)
>  	}
>  }
>  
> +static int rocker_port_brport_flags_set(struct rocker_port *rocker_port,
> +					unsigned long brport_flags)
> +{
> +	unsigned long orig_flags;
> +	int err = 0;
> +
> +	orig_flags = rocker_port->brport_flags;
> +	rocker_port->brport_flags = brport_flags;
> +	if ((orig_flags ^ rocker_port->brport_flags) & BR_LEARNING)
> +		err = rocker_port_set_learning(rocker_port);

My understanding is that the two-phase set scheme that this
patch-set proposes allows failure, e.g. due resource constraints,
during the prepare phase but that failure in the commit phase
indicates a hardware or driver bug.

It seems to me that this patch doesn't follow the above scheme because
I see the following call-chain:

	rocker_port_set_learning()
	-> rocker_cmd_exec()
	   -> rocker_wait_create()
	      -> kmalloc()

I am probably missing something but it does seem to me that the above is
called in both the prepare and commit phases and that kalloc() may fail in
ether case, the latter case being the problem I see.

> +
> +	if (rocker_port->trans == SWDEV_TRANS_PREPARE)
> +		rocker_port->brport_flags = orig_flags;
> +
> +	return err;
> +}
> +
>  static int rocker_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
>  {
>  	struct rocker_port *rocker_port = netdev_priv(dev);
> @@ -4363,12 +4385,18 @@ static int rocker_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
>  	case SWDEV_TRANS_ABORT:
>  		rocker_port_trans_abort(rocker_port);
>  		return 0;
> +	default:
> +		break;
>  	}
>  
>  	switch (attr->id) {
>  	case SWDEV_ATTR_PORT_STP_STATE:
>  		err = rocker_port_stp_update(rocker_port, attr->stp_state);
>  		break;
> +	case SWDEV_ATTR_PORT_BRIDGE_FLAGS:
> +		err = rocker_port_brport_flags_set(rocker_port,
> +						   attr->brport_flags);
> +		break;
>  	default:
>  		err = -EOPNOTSUPP;
>  		break;
> @@ -4772,8 +4800,7 @@ static int rocker_probe_port(struct rocker *rocker, unsigned int port_number)

[snip]

Re: [PATCH net-next v4 05/24] rocker: use swdev get/set attr for bridge port flags

[Scott Feldman]

On Tue, Apr 14, 2015 at 10:25 PM, Simon Horman
<simon.horman@netronome.com> wrote:
> Hi Scott,
>
> On Sun, Apr 12, 2015 at 11:16:59PM -0700, sfeldma@gmail.com wrote:
>> From: Scott Feldman <sfeldma@gmail.com>
>>
>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>> ---
>>  drivers/net/ethernet/rocker/rocker.c |   31 +++++++++++++++++++++++++++++--
>>  1 file changed, 29 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
>> index 80c7f6f..9bd4fc6 100644
>> --- a/drivers/net/ethernet/rocker/rocker.c
>> +++ b/drivers/net/ethernet/rocker/rocker.c
>
> [snip]
>
>> @@ -4349,6 +4354,23 @@ static void rocker_port_trans_abort(struct rocker_port *rocker_port)
>>       }
>>  }
>>
>> +static int rocker_port_brport_flags_set(struct rocker_port *rocker_port,
>> +                                     unsigned long brport_flags)
>> +{
>> +     unsigned long orig_flags;
>> +     int err = 0;
>> +
>> +     orig_flags = rocker_port->brport_flags;
>> +     rocker_port->brport_flags = brport_flags;
>> +     if ((orig_flags ^ rocker_port->brport_flags) & BR_LEARNING)
>> +             err = rocker_port_set_learning(rocker_port);
>
> My understanding is that the two-phase set scheme that this
> patch-set proposes allows failure, e.g. due resource constraints,
> during the prepare phase but that failure in the commit phase
> indicates a hardware or driver bug.
>
> It seems to me that this patch doesn't follow the above scheme because
> I see the following call-chain:
>
>         rocker_port_set_learning()
>         -> rocker_cmd_exec()
>            -> rocker_wait_create()
>               -> kmalloc()
>
> I am probably missing something but it does seem to me that the above is
> called in both the prepare and commit phases and that kalloc() may fail in
> ether case, the latter case being the problem I see.

Nope, you're not missing something, you found a bug.   A little later
you'll see we use a desc on the cmd ring to issue the cmd to HW.  If
we're in prepare phase, we want to put that desc back on the ring so
commit phase has a desc.  So this needs to be fixed also.  More work
for v5.  But I'm totally glad you're reviewing at this level and
finding issues like this.  Thank you very much.

It's been a little tricky retrofitting rocker for this prepare-commit
scheme.  Prepare phase really forces you to examine all failure paths
and make sure everything is just how you found it and resources are
reserved so commit has a happy life.  My hope is this will be easier
with new drivers knowing the prepare-commit rules up-front.

-scott

Re: [PATCH net-next v4 05/24] rocker: use swdev get/set attr for bridge port flags

[Simon Horman]

Hi Scott,

On Tue, Apr 14, 2015 at 11:03:21PM -0700, Scott Feldman wrote:
> On Tue, Apr 14, 2015 at 10:25 PM, Simon Horman
> <simon.horman@netronome.com> wrote:
> > Hi Scott,
> >
> > On Sun, Apr 12, 2015 at 11:16:59PM -0700, sfeldma@gmail.com wrote:
> >> From: Scott Feldman <sfeldma@gmail.com>
> >>
> >> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
> >> ---
> >>  drivers/net/ethernet/rocker/rocker.c |   31 +++++++++++++++++++++++++++++--
> >>  1 file changed, 29 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
> >> index 80c7f6f..9bd4fc6 100644
> >> --- a/drivers/net/ethernet/rocker/rocker.c
> >> +++ b/drivers/net/ethernet/rocker/rocker.c
> >
> > [snip]
> >
> >> @@ -4349,6 +4354,23 @@ static void rocker_port_trans_abort(struct rocker_port *rocker_port)
> >>       }
> >>  }
> >>
> >> +static int rocker_port_brport_flags_set(struct rocker_port *rocker_port,
> >> +                                     unsigned long brport_flags)
> >> +{
> >> +     unsigned long orig_flags;
> >> +     int err = 0;
> >> +
> >> +     orig_flags = rocker_port->brport_flags;
> >> +     rocker_port->brport_flags = brport_flags;
> >> +     if ((orig_flags ^ rocker_port->brport_flags) & BR_LEARNING)
> >> +             err = rocker_port_set_learning(rocker_port);
> >
> > My understanding is that the two-phase set scheme that this
> > patch-set proposes allows failure, e.g. due resource constraints,
> > during the prepare phase but that failure in the commit phase
> > indicates a hardware or driver bug.
> >
> > It seems to me that this patch doesn't follow the above scheme because
> > I see the following call-chain:
> >
> >         rocker_port_set_learning()
> >         -> rocker_cmd_exec()
> >            -> rocker_wait_create()
> >               -> kmalloc()
> >
> > I am probably missing something but it does seem to me that the above is
> > called in both the prepare and commit phases and that kalloc() may fail in
> > ether case, the latter case being the problem I see.
> 
> Nope, you're not missing something, you found a bug.   A little later
> you'll see we use a desc on the cmd ring to issue the cmd to HW.  If
> we're in prepare phase, we want to put that desc back on the ring so
> commit phase has a desc.  So this needs to be fixed also.  More work
> for v5.  But I'm totally glad you're reviewing at this level and
> finding issues like this.  Thank you very much.

I'm glad its useful :)
FWIW, I also like the direction of this patchset.

> It's been a little tricky retrofitting rocker for this prepare-commit
> scheme.  Prepare phase really forces you to examine all failure paths
> and make sure everything is just how you found it and resources are
> reserved so commit has a happy life.  My hope is this will be easier
> with new drivers knowing the prepare-commit rules up-front.
> 
> -scott

Re: [PATCH net-next v4 00/24] switchdev: spring cleanup

[roopa]

On 4/13/15, 10:47 PM, roopa wrote:
> On 4/12/15, 11:16 PM, sfeldma@gmail.com wrote:
>> From: Scott Feldman <sfeldma@gmail.com>
>>
>> v4:
>>
>> Well, it was a lot of work, but now prepare-commit transaction model 
>> is how
>> davem advises: if prepare fails, abort the transaction.  The driver 
>> must do
>> resource reservations up front in prepare phase and return those 
>> resources if
>> aborting.  Commit phase would use reserved resources.  The good news 
>> is the
>> driver code (for rocker) now handles resource allocation failures 
>> better by not
>> leaving partially device or driver states.  This is a side-effect of the
>> prepare phase where state isn't modified; only validation of inputs and
>> resource reservations happen in the prepare phase.  Since we're 
>> supporting
>> setting attrs and add objs across lower devs in the stacked case, we 
>> need to
>> hold rtnl_lock (or ensure rtnl_lock is held) so lower devs don't move 
>> on us
>> during the prepare-commit transaction.  DSA driver code skips the 
>> prepare phase
>> and goes straight for the commit phase since no up-front allocations 
>> are done
>> and no device failures (that could be detected in the prepare phase) can
>> happen.
>
> thanks for the series. It definitely does look cleaner and less 
> confusing now!.
> I do love the abstraction but i was one of the people voting against 
> duplicating the
> kernel objects into swdev objs which this patches does (which i am 
> still not convinced
> we should have).
>
>>
>> Remove NETIF_F_HW_SWITCH_OFFLOAD from rocker and the swdev_attr_set/get
>> wrappers.  DSA doesn't set NETIF_F_HW_SWITCH_OFFLOAD, so it can't be in
>> swdev_attr_set/get.  rocker doesn't need it; or rather can't support
>> NETIF_F_HW_SWITCH_OFFLOAD being set/cleared at run-time after the device
>> port is already up and offloading L2/L3. NETIF_F_HW_SWITCH_OFFLOAD is 
>> still
>> left as a feature flag for drivers that can use it.
>
> I see that this series removes all uses of it in the switchdev api 
> later. I had summarized
> the need for the flag in reply to one of your questions a few weeks 
> back. Since you have moved all
> ndo ops to swdev ops (including ndo_bridge_setlink/dellink), I don't 
> want to hold on to the
>  feature flag if no one is using it. yes, my userspace driver uses it 
> today.
> I will come back with stronger justification to keep it or
> will submit a patch to remove it and add it back at a later point if 
> needed.
>
scott,  I see that you will be spinning v5 of the series. In which case 
feel free to remove the feature flag during your
code restructuring effort. If i need it again, i will resubmit. thanks.