[PATCH net-next v2 00/26] switchdev: spring cleanup

[PATCH net-next v2 00/26] switchdev: spring cleanup

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

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 (26):
  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: rename netdev_switch_fib_* to swdev_fib_*
  switchdev: rename netdev_switch_notifier_* to swdev_notifier_*
  switchdev: convert swdev_fib_ipv4_add/del over to
    swdev_port_obj_add/del
  switchdev: bring documentation up-to-date

 Documentation/networking/switchdev.txt |  420 +++++++++++++++++++++----
 drivers/net/bonding/bond_main.c        |    5 +-
 drivers/net/ethernet/rocker/rocker.c   |  221 ++++++++-----
 drivers/net/team/team.c                |    5 +-
 include/net/switchdev.h                |  192 +++++++-----
 net/bridge/br.c                        |   22 +-
 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                        |   35 ++-
 net/ipv4/fib_trie.c                    |   38 +--
 net/switchdev/switchdev.c              |  530 +++++++++++++++++++++++---------
 13 files changed, 1081 insertions(+), 436 deletions(-)

-- 
1.7.10.4

[PATCH net-next v2 01/26] 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.  If
something goes wrong on one of the lower devs, revert all back to previous attr
value.

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

On set, allow a flag SWDEV_F_NO_RECOVER to turn off automatic err recovery.

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

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index d2e69ee..b5a7d06 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -14,6 +14,18 @@
 #include <linux/netdevice.h>
 #include <linux/notifier.h>
 
+#define SWDEV_F_NO_RECURSE		BIT(0)
+#define SWDEV_F_NO_RECOVER		BIT(1)
+
+enum swdev_attr_id {
+	SWDEV_ATTR_UNDEFINED,
+};
+
+struct swdev_attr {
+	enum swdev_attr_id id;
+	u32 flags;
+};
+
 struct fib_info;
 
 /**
@@ -23,6 +35,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 +49,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 +88,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 +117,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..e41cf72 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -37,6 +37,108 @@ 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 (!(dev->features & NETIF_F_HW_SWITCH_OFFLOAD))
+		return err;
+
+	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 (!(dev->features & NETIF_F_HW_SWITCH_OFFLOAD))
+		return err;
+
+	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;
+}
+
+/**
+ *	swdev_port_attr_set - Set port attribute
+ *
+ *	@dev: port device
+ *	@attr: attribute to set
+ */
+int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
+{
+	struct swdev_attr prev = *attr;
+	int err, get_err, revert_err;
+
+	get_err = swdev_port_attr_get(dev, &prev);
+
+	err = __swdev_port_attr_set(dev, attr);
+	if (err && !get_err && !(attr->flags & SWDEV_F_NO_RECOVER)) {
+		/* Some err on set: revert to previous value */
+		revert_err = __swdev_port_attr_set(dev, &prev);
+		if (revert_err)
+			netdev_err(dev, "Reverting swdev port attr %d failed\n",
+				   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 v2 02/26] 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 c9558e6..00f55ab 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 b5a7d06..041fa42 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -19,11 +19,15 @@
 
 enum swdev_attr_id {
 	SWDEV_ATTR_UNDEFINED,
+	SWDEV_ATTR_PORT_PARENT_ID,
 };
 
 struct swdev_attr {
 	enum swdev_attr_id id;
 	u32 flags;
+	union {
+		struct netdev_phys_item_id ppid;	/* PORT_PARENT_ID */
+	};
 };
 
 struct fib_info;
@@ -31,10 +35,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).
@@ -47,8 +47,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,
@@ -86,8 +84,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);
@@ -111,12 +107,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 cc5cf68..1239c99 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -450,11 +450,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 b96ac21..3434724 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1003,16 +1003,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 3597724..e994cba 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -384,14 +384,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;
 }
@@ -678,7 +684,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 e41cf72..753f117 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
@@ -347,11 +329,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) {
@@ -365,8 +346,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;
 
@@ -382,17 +365,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 v2 03/26] 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.

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

diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index 00f55ab..1b29b0a 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -4237,11 +4237,21 @@ 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 int rocker_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
 {
 	struct rocker_port *rocker_port = netdev_priv(dev);
+	int err = 0;
+
+	switch (attr->id) {
+	case SWDEV_ATTR_PORT_STP_STATE:
+		err = rocker_port_stp_update(rocker_port, attr->stp_state);
+		break;
+	default:
+		err = -EOPNOTSUPP;
+		break;
+	}
 
-	return rocker_port_stp_update(rocker_port, state);
+	return err;
 }
 
 static int rocker_port_swdev_fib_ipv4_add(struct net_device *dev,
@@ -4271,7 +4281,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,
 };
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 041fa42..86b614c 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -20,6 +20,7 @@
 enum swdev_attr_id {
 	SWDEV_ATTR_UNDEFINED,
 	SWDEV_ATTR_PORT_PARENT_ID,
+	SWDEV_ATTR_PORT_STP_STATE,
 };
 
 struct swdev_attr {
@@ -27,6 +28,7 @@ struct swdev_attr {
 	u32 flags;
 	union {
 		struct netdev_phys_item_id ppid;	/* PORT_PARENT_ID */
+		u8 stp_state;				/* PORT_STP_STATE */
 	};
 };
 
@@ -39,9 +41,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.
@@ -51,7 +50,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,
@@ -86,7 +84,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,
@@ -119,12 +116,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 e994cba..e14ec38 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -347,6 +347,23 @@ 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:
+		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)
 {
@@ -685,7 +702,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 753f117..ad6d851 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -120,34 +120,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 v2 04/26] 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 86b614c..73fa2a0 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -21,6 +21,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 {
@@ -29,6 +30,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 v2 05/26] 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 |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index 1b29b0a..7602973 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -4230,6 +4230,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;
 	}
@@ -4240,12 +4243,19 @@ static int rocker_port_attr_get(struct net_device *dev, struct swdev_attr *attr)
 static int rocker_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
 {
 	struct rocker_port *rocker_port = netdev_priv(dev);
+	unsigned long orig_flags;
 	int err = 0;
 
 	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:
+		orig_flags = rocker_port->brport_flags;
+		rocker_port->brport_flags = attr->brport_flags;
+		if ((orig_flags ^ rocker_port->brport_flags) & BR_LEARNING)
+			err = rocker_port_set_learning(rocker_port);
+		break;
 	default:
 		err = -EOPNOTSUPP;
 		break;
-- 
1.7.10.4

[PATCH net-next v2 06/26] 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.

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

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 73fa2a0..39665b4 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -36,6 +36,15 @@ struct swdev_attr {
 
 struct fib_info;
 
+enum swdev_obj_id {
+	SWDEV_OBJ_UNDEFINED,
+};
+
+struct swdev_obj {
+	enum swdev_obj_id id;
+	u32 flags;
+};
+
 /**
  * struct switchdev_ops - switchdev operations
  *
@@ -43,6 +52,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.
@@ -52,6 +65,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,
@@ -86,6 +103,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,
@@ -118,6 +137,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 ad6d851..6a527f5 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -120,6 +120,80 @@ int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
 }
 EXPORT_SYMBOL_GPL(swdev_port_attr_set);
 
+/**
+ *	swdev_port_obj_add - Add port object
+ *
+ *	@dev: port device
+ *	@obj: object to add
+ */
+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 (!(dev->features & NETIF_F_HW_SWITCH_OFFLOAD))
+		return err;
+
+	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;
+}
+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 (!(dev->features & NETIF_F_HW_SWITCH_OFFLOAD))
+		return err;
+
+	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 v2 07/26] 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 39665b4..f1fa5fc 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -38,11 +38,19 @@ struct fib_info;
 
 enum swdev_obj_id {
 	SWDEV_OBJ_UNDEFINED,
+	SWDEV_OBJ_PORT_VLAN,
 };
 
 struct swdev_obj {
 	enum swdev_obj_id id;
 	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 v2 08/26] 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 |   93 ++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index 7602973..900d4fb 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -4264,6 +4264,97 @@ 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;
+
+	switch (obj->id) {
+	case SWDEV_OBJ_PORT_VLAN:
+		err = rocker_port_vlans_add(rocker_port, &obj->vlan);
+		break;
+	default:
+		err = -EOPNOTSUPP;
+		break;
+	}
+
+	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,
@@ -4292,6 +4383,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 v2 09/26] 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 |  150 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 158 insertions(+)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index f1fa5fc..96489d7 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -113,6 +113,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,
@@ -157,6 +159,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 6a527f5..5426a36 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>
 
@@ -282,6 +283,155 @@ 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_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_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 v2 10/26] 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 900d4fb..29973db 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -4135,45 +4135,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)
@@ -4211,7 +4172,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 v2 11/26] 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 7b4684c..99ab282 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4036,7 +4036,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 v2 12/26] 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 v2 13/26] 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 96489d7..8e62528 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -119,14 +119,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,
@@ -181,13 +177,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)
@@ -202,13 +191,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 e1115a2..5deb063 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 5426a36..8786f91 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -258,31 +258,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)
 {
@@ -458,36 +433,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 v2 14/26] 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 8e62528..60b83b0 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -115,6 +115,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,
@@ -161,6 +163,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 8786f91..a720a3c 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -408,6 +408,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 v2 15/26] 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 29973db..018b04a 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -4173,7 +4173,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 v2 16/26] 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 99ab282..0cd3cd7 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_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 v2 17/26] 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 v2 18/26] 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 60b83b0..9147a35 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -121,10 +121,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,
@@ -185,20 +181,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 5deb063..cfee027 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 a720a3c..96cdee0 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -431,61 +431,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 v2 19/26] 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 9147a35..d46809b 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -113,6 +113,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,
@@ -153,6 +155,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 96cdee0..bcf7ccb 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -258,6 +258,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 v2 20/26] 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 018b04a..2b62f02 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -4135,18 +4135,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)
 {
@@ -4173,6 +4161,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 v2 21/26] 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 0cd3cd7..3368ebc 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4037,6 +4037,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 v2 22/26] 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 v2 23/26] switchdev: rename netdev_switch_fib_* to swdev_fib_*

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

To be consistent with other swdev code, rename the L3 FIB ops with swdev_
prefix.

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 include/net/switchdev.h   |   23 ++++++++++-------------
 net/ipv4/fib_trie.c       |   38 ++++++++++++++++----------------------
 net/switchdev/switchdev.c |   34 +++++++++++++++++-----------------
 3 files changed, 43 insertions(+), 52 deletions(-)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index d46809b..55f9347 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -123,11 +123,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
 
@@ -190,22 +190,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 2c7c299..a452f38 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 bcf7ccb..3142c57 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -458,7 +458,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;
@@ -473,7 +473,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;
 	}
@@ -481,7 +481,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,
@@ -498,7 +498,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;
 
@@ -520,7 +520,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)
@@ -532,8 +532,8 @@ 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 net_device *dev;
 	const struct swdev_ops *ops;
@@ -551,7 +551,7 @@ 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;
@@ -566,10 +566,10 @@ int netdev_switch_fib_ipv4_add(u32 dst, int dst_len, struct fib_info *fi,
 
 	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)
@@ -580,8 +580,8 @@ 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 net_device *dev;
 	const struct swdev_ops *ops;
@@ -590,7 +590,7 @@ int netdev_switch_fib_ipv4_del(u32 dst, int dst_len, struct fib_info *fi,
 	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;
@@ -604,14 +604,14 @@ int netdev_switch_fib_ipv4_del(u32 dst, int dst_len, struct fib_info *fi,
 
 	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
@@ -624,4 +624,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 v2 24/26] switchdev: rename netdev_switch_notifier_* to swdev_notifier_*

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

To be consistent with other swdev code, rename swdev notifier with swdev_
prefix.

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 drivers/net/ethernet/rocker/rocker.c |    8 +++-----
 include/net/switchdev.h              |   31 ++++++++++++++++---------------
 net/bridge/br.c                      |   22 +++++++++++-----------
 net/switchdev/switchdev.c            |   20 ++++++++++----------
 4 files changed, 40 insertions(+), 41 deletions(-)

diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index 2b62f02..b7e54c3 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -3377,17 +3377,15 @@ static void rocker_port_fdb_learn_work(struct work_struct *work)
 		container_of(work, struct rocker_fdb_learn_work, work);
 	bool removing = (lw->flags & ROCKER_OP_FLAG_REMOVE);
 	bool learned = (lw->flags & ROCKER_OP_FLAG_LEARNED);
-	struct netdev_switch_notifier_fdb_info info;
+	struct swdev_notifier_fdb_info info;
 
 	info.addr = lw->addr;
 	info.vid = lw->vid;
 
 	if (learned && removing)
-		call_netdev_switch_notifiers(NETDEV_SWITCH_FDB_DEL,
-					     lw->dev, &info.info);
+		call_swdev_notifiers(SWDEV_FDB_DEL, lw->dev, &info.info);
 	else if (learned && !removing)
-		call_netdev_switch_notifiers(NETDEV_SWITCH_FDB_ADD,
-					     lw->dev, &info.info);
+		call_swdev_notifiers(SWDEV_FDB_ADD, lw->dev, &info.info);
 
 	kfree(work);
 }
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 55f9347..37a2607 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -86,23 +86,23 @@ struct swdev_ops {
 				      u8 tos, u8 type, u32 tb_id);
 };
 
-enum netdev_switch_notifier_type {
-	NETDEV_SWITCH_FDB_ADD = 1,
-	NETDEV_SWITCH_FDB_DEL,
+enum swdev_notifier_type {
+	SWDEV_FDB_ADD = 1,
+	SWDEV_FDB_DEL,
 };
 
-struct netdev_switch_notifier_info {
+struct swdev_notifier_info {
 	struct net_device *dev;
 };
 
-struct netdev_switch_notifier_fdb_info {
-	struct netdev_switch_notifier_info info; /* must be first */
+struct swdev_notifier_fdb_info {
+	struct swdev_notifier_info info; /* must be first */
 	const unsigned char *addr;
 	u16 vid;
 };
 
 static inline struct net_device *
-netdev_switch_notifier_info_to_dev(const struct netdev_switch_notifier_info *info)
+swdev_notifier_info_to_dev(const struct swdev_notifier_info *info)
 {
 	return info->dev;
 }
@@ -119,10 +119,10 @@ 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,
-				 struct netdev_switch_notifier_info *info);
+int register_swdev_notifier(struct notifier_block *nb);
+int unregister_swdev_notifier(struct notifier_block *nb);
+int call_swdev_notifiers(unsigned long val, struct net_device *dev,
+			 struct swdev_notifier_info *info);
 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,
@@ -174,18 +174,19 @@ static inline int swdev_port_bridge_dellink(struct net_device *dev,
 	return -EOPNOTSUPP;
 }
 
-static inline int register_netdev_switch_notifier(struct notifier_block *nb)
+static inline int register_swdev_notifier(struct notifier_block *nb)
 {
 	return 0;
 }
 
-static inline int unregister_netdev_switch_notifier(struct notifier_block *nb)
+static inline int unregister_swdev_notifier(struct notifier_block *nb)
 {
 	return 0;
 }
 
-static inline int call_netdev_switch_notifiers(unsigned long val, struct net_device *dev,
-					       struct netdev_switch_notifier_info *info)
+static inline int call_swdev_notifiers(unsigned long val,
+				       struct net_device *dev,
+				       struct swdev_notifier_info *info)
 {
 	return NOTIFY_DONE;
 }
diff --git a/net/bridge/br.c b/net/bridge/br.c
index 02c24cf..9d54965 100644
--- a/net/bridge/br.c
+++ b/net/bridge/br.c
@@ -121,13 +121,13 @@ static struct notifier_block br_device_notifier = {
 	.notifier_call = br_device_event
 };
 
-static int br_netdev_switch_event(struct notifier_block *unused,
-				  unsigned long event, void *ptr)
+static int br_swdev_event(struct notifier_block *unused,
+			  unsigned long event, void *ptr)
 {
-	struct net_device *dev = netdev_switch_notifier_info_to_dev(ptr);
+	struct net_device *dev = swdev_notifier_info_to_dev(ptr);
 	struct net_bridge_port *p;
 	struct net_bridge *br;
-	struct netdev_switch_notifier_fdb_info *fdb_info;
+	struct swdev_notifier_fdb_info *fdb_info;
 	int err = NOTIFY_DONE;
 
 	rtnl_lock();
@@ -138,14 +138,14 @@ static int br_netdev_switch_event(struct notifier_block *unused,
 	br = p->br;
 
 	switch (event) {
-	case NETDEV_SWITCH_FDB_ADD:
+	case SWDEV_FDB_ADD:
 		fdb_info = ptr;
 		err = br_fdb_external_learn_add(br, p, fdb_info->addr,
 						fdb_info->vid);
 		if (err)
 			err = notifier_from_errno(err);
 		break;
-	case NETDEV_SWITCH_FDB_DEL:
+	case SWDEV_FDB_DEL:
 		fdb_info = ptr;
 		err = br_fdb_external_learn_del(br, p, fdb_info->addr,
 						fdb_info->vid);
@@ -159,8 +159,8 @@ out:
 	return err;
 }
 
-static struct notifier_block br_netdev_switch_notifier = {
-	.notifier_call = br_netdev_switch_event,
+static struct notifier_block br_swdev_notifier = {
+	.notifier_call = br_swdev_event,
 };
 
 static void __net_exit br_net_exit(struct net *net)
@@ -214,7 +214,7 @@ static int __init br_init(void)
 	if (err)
 		goto err_out3;
 
-	err = register_netdev_switch_notifier(&br_netdev_switch_notifier);
+	err = register_swdev_notifier(&br_swdev_notifier);
 	if (err)
 		goto err_out4;
 
@@ -235,7 +235,7 @@ static int __init br_init(void)
 	return 0;
 
 err_out5:
-	unregister_netdev_switch_notifier(&br_netdev_switch_notifier);
+	unregister_swdev_notifier(&br_swdev_notifier);
 err_out4:
 	unregister_netdevice_notifier(&br_device_notifier);
 err_out3:
@@ -253,7 +253,7 @@ static void __exit br_deinit(void)
 {
 	stp_proto_unregister(&br_stp_proto);
 	br_netlink_fini();
-	unregister_netdev_switch_notifier(&br_netdev_switch_notifier);
+	unregister_swdev_notifier(&br_swdev_notifier);
 	unregister_netdevice_notifier(&br_device_notifier);
 	brioctl_set(NULL);
 	unregister_pernet_subsys(&br_net_ops);
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 3142c57..98c631c 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -199,14 +199,14 @@ static DEFINE_MUTEX(netdev_switch_mutex);
 static RAW_NOTIFIER_HEAD(netdev_switch_notif_chain);
 
 /**
- *	register_netdev_switch_notifier - Register notifier
+ *	register_swdev_notifier - Register notifier
  *	@nb: notifier_block
  *
  *	Register switch device notifier. This should be used by code
  *	which needs to monitor events happening in particular device.
  *	Return values are same as for atomic_notifier_chain_register().
  */
-int register_netdev_switch_notifier(struct notifier_block *nb)
+int register_swdev_notifier(struct notifier_block *nb)
 {
 	int err;
 
@@ -215,16 +215,16 @@ int register_netdev_switch_notifier(struct notifier_block *nb)
 	mutex_unlock(&netdev_switch_mutex);
 	return err;
 }
-EXPORT_SYMBOL_GPL(register_netdev_switch_notifier);
+EXPORT_SYMBOL_GPL(register_swdev_notifier);
 
 /**
- *	unregister_netdev_switch_notifier - Unregister notifier
+ *	unregister_swdev_notifier - Unregister notifier
  *	@nb: notifier_block
  *
  *	Unregister switch device notifier.
  *	Return values are same as for atomic_notifier_chain_unregister().
  */
-int unregister_netdev_switch_notifier(struct notifier_block *nb)
+int unregister_swdev_notifier(struct notifier_block *nb)
 {
 	int err;
 
@@ -233,10 +233,10 @@ int unregister_netdev_switch_notifier(struct notifier_block *nb)
 	mutex_unlock(&netdev_switch_mutex);
 	return err;
 }
-EXPORT_SYMBOL_GPL(unregister_netdev_switch_notifier);
+EXPORT_SYMBOL_GPL(unregister_swdev_notifier);
 
 /**
- *	call_netdev_switch_notifiers - Call notifiers
+ *	call_swdev_notifiers - Call notifiers
  *	@val: value passed unmodified to notifier function
  *	@dev: port device
  *	@info: notifier information data
@@ -245,8 +245,8 @@ EXPORT_SYMBOL_GPL(unregister_netdev_switch_notifier);
  *	when it needs to propagate hardware event.
  *	Return values are same as for atomic_notifier_call_chain().
  */
-int call_netdev_switch_notifiers(unsigned long val, struct net_device *dev,
-				 struct netdev_switch_notifier_info *info)
+int call_swdev_notifiers(unsigned long val, struct net_device *dev,
+			 struct swdev_notifier_info *info)
 {
 	int err;
 
@@ -256,7 +256,7 @@ int call_netdev_switch_notifiers(unsigned long val, struct net_device *dev,
 	mutex_unlock(&netdev_switch_mutex);
 	return err;
 }
-EXPORT_SYMBOL_GPL(call_netdev_switch_notifiers);
+EXPORT_SYMBOL_GPL(call_swdev_notifiers);
 
 /**
  *	swdev_port_bridge_getlink - Get bridge port attributes
-- 
1.7.10.4

[PATCH net-next v2 25/26] switchdev: convert swdev_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              |   21 +++++++--------
 net/switchdev/switchdev.c            |   49 +++++++++++++++++++++-------------
 3 files changed, 56 insertions(+), 56 deletions(-)

diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index b7e54c3..b34efd8 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -4244,12 +4244,19 @@ 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;
 
 	switch (obj->id) {
 	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;
@@ -4289,12 +4296,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;
@@ -4303,38 +4318,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 37a2607..efaac55 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -39,6 +39,7 @@ struct fib_info;
 enum swdev_obj_id {
 	SWDEV_OBJ_UNDEFINED,
 	SWDEV_OBJ_PORT_VLAN,
+	SWDEV_OBJ_IPV4_FIB,
 };
 
 struct swdev_obj {
@@ -50,6 +51,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;
 	};
 };
 
@@ -63,10 +73,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,
@@ -77,13 +83,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 swdev_notifier_type {
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 98c631c..4dbd4c4 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -535,8 +535,19 @@ static struct net_device *swdev_get_dev_by_nhs(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)
 {
+	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
@@ -554,15 +565,10 @@ int swdev_fib_ipv4_add(u32 dst, int dst_len, struct fib_info *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;
 }
@@ -583,8 +589,19 @@ EXPORT_SYMBOL_GPL(swdev_fib_ipv4_add);
 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))
@@ -593,14 +610,10 @@ int swdev_fib_ipv4_del(u32 dst, int dst_len, struct fib_info *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;
 }
-- 
1.7.10.4

[PATCH net-next v2 26/26] 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 |  420 +++++++++++++++++++++++++++-----
 1 file changed, 361 insertions(+), 59 deletions(-)

diff --git a/Documentation/networking/switchdev.txt b/Documentation/networking/switchdev.txt
index f981a92..a111dc9 100644
--- a/Documentation/networking/switchdev.txt
+++ b/Documentation/networking/switchdev.txt
@@ -1,59 +1,361 @@
-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_HW_SWITCH_OFFLOAD
+
+The switchdev driver should set this feature flag on each port to mark the
+port as participating in switchdev offloads.  The user can toggle the flag
+using ethtool -K to enable/disable a port from offload.
+
+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 v2 00/26] switchdev: spring cleanup

[Jiri Pirko]

Wed, Apr 01, 2015 at 12:07:36PM CEST, sfeldma@gmail.com wrote:
>From: Scott Feldman <sfeldma@gmail.com>
>
>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

I do not understand why this was not addresses. Can you please provide some
justification?

Re: [PATCH net-next v2 25/26] switchdev: convert swdev_fib_ipv4_add/del over to swdev_port_obj_add/del

[Jiri Pirko]

Wed, Apr 01, 2015 at 12:08:01PM CEST, sfeldma@gmail.com wrote:
>From: Scott Feldman <sfeldma@gmail.com>

...

>@@ -63,10 +73,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,
>@@ -77,13 +83,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);

Can you please elaborate more why generic add/del functions are better
than specific ones? Me personally I don't like this kind of approach. It
makes sense for atts, where we will probably end up in bigger number of
attrs and having setter/getter for each would not be optimal.

But for objs, what other objs do you expect? Why don't have a set of SDOs
for each. Speaking of SDOs, I thought that having many of them was the
reason you ripped them out of NDO struct. Now it looks like there will
be 4 SDOs forever. Maybe I'm missing something...

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

[Jamal Hadi Salim]

On 04/01/15 06:07, 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.

So this GET is very specific? All underlying children have to have the
exact same value?
My impression of GET is, depending on the target object, i retrieve
one thing or a lot of things that probably user space is asking for.
I wasnt getting that impression looking at this code.

  For sets, set the same attr value for all lower devs.  If
> something goes wrong on one of the lower devs, revert all back to previous attr
> value.

And from staring at the code - the reverting could fail as well..

>
> If lower dev recusion isn't desired, allow a flag SWDEV_F_NO_RECURSE to
> indicate get/set only work on port (lowest) device.
>
> On set, allow a flag SWDEV_F_NO_RECOVER to turn off automatic err recovery.
>


uh-oh did i read that last one correctly?;-> I dont see it in the code
but are you now allowing for policy to dictate whether we want something
atomic or not? That should be a generic much higher level flag imo.
Note, I am for supporting the different scenarios but i thought
the initial approach was all transactions are atomic (all-or-nothing).

Also why invent new namespace for these switch attributes? They all seem
to have netlink IDs already. I note in patches afterwards are using the
netlink IDs.

cheers,
jamal

Re: [PATCH net-next v2 25/26] switchdev: convert swdev_fib_ipv4_add/del over to swdev_port_obj_add/del

[roopa]

On 4/1/15, 3:08 AM, sfeldma@gmail.com wrote:
> 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              |   21 +++++++--------
>   net/switchdev/switchdev.c            |   49 +++++++++++++++++++++-------------
>   3 files changed, 56 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
> index b7e54c3..b34efd8 100644
> --- a/drivers/net/ethernet/rocker/rocker.c
> +++ b/drivers/net/ethernet/rocker/rocker.c
> @@ -4244,12 +4244,19 @@ 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;
>   
>   	switch (obj->id) {
>   	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;
> @@ -4289,12 +4296,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;
> @@ -4303,38 +4318,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 37a2607..efaac55 100644
> --- a/include/net/switchdev.h
> +++ b/include/net/switchdev.h
> @@ -39,6 +39,7 @@ struct fib_info;
>   enum swdev_obj_id {
>   	SWDEV_OBJ_UNDEFINED,
>   	SWDEV_OBJ_PORT_VLAN,
> +	SWDEV_OBJ_IPV4_FIB,
>   };
>   
>   struct swdev_obj {
> @@ -50,6 +51,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;
>   	};
>   };
>   
> @@ -63,10 +73,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,
> @@ -77,13 +83,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 swdev_notifier_type {
> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
> index 98c631c..4dbd4c4 100644
> --- a/net/switchdev/switchdev.c
> +++ b/net/switchdev/switchdev.c
> @@ -535,8 +535,19 @@ static struct net_device *swdev_get_dev_by_nhs(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)
>   {
> +	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
> @@ -554,15 +565,10 @@ int swdev_fib_ipv4_add(u32 dst, int dst_len, struct fib_info *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;
>   }
> @@ -583,8 +589,19 @@ EXPORT_SYMBOL_GPL(swdev_fib_ipv4_add);
>   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,
> +		},
> +	};
>   
This looks nice. However, my only concern is we have now ended up adding 
a whole layer of abstraction to all objects. The api abstraction seemed 
fine.
But, carrying it to objects and duplicating every object in the swdev 
layer seems too much duplication. Maybe its just me.
we will end up adding this for bond attributes...vxlan...fdb and 
nftables etc.

The advantage of switchdev in the kernel was to have access the existing 
kernel api and objects directly from switchdev layer.
In which case, to me, it would be better to skip this extra layer of 
objects. And keep the way you had it originally.

Re: [PATCH net-next v2 25/26] switchdev: convert swdev_fib_ipv4_add/del over to swdev_port_obj_add/del

[Jiri Pirko]

Wed, Apr 01, 2015 at 05:05:49PM CEST, roopa@cumulusnetworks.com wrote:
>On 4/1/15, 3:08 AM, sfeldma@gmail.com wrote:
>>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              |   21 +++++++--------
>>  net/switchdev/switchdev.c            |   49 +++++++++++++++++++++-------------
>>  3 files changed, 56 insertions(+), 56 deletions(-)
>>
>>diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
>>index b7e54c3..b34efd8 100644
>>--- a/drivers/net/ethernet/rocker/rocker.c
>>+++ b/drivers/net/ethernet/rocker/rocker.c
>>@@ -4244,12 +4244,19 @@ 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;
>>  	switch (obj->id) {
>>  	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;
>>@@ -4289,12 +4296,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;
>>@@ -4303,38 +4318,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 37a2607..efaac55 100644
>>--- a/include/net/switchdev.h
>>+++ b/include/net/switchdev.h
>>@@ -39,6 +39,7 @@ struct fib_info;
>>  enum swdev_obj_id {
>>  	SWDEV_OBJ_UNDEFINED,
>>  	SWDEV_OBJ_PORT_VLAN,
>>+	SWDEV_OBJ_IPV4_FIB,
>>  };
>>  struct swdev_obj {
>>@@ -50,6 +51,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;
>>  	};
>>  };
>>@@ -63,10 +73,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,
>>@@ -77,13 +83,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 swdev_notifier_type {
>>diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
>>index 98c631c..4dbd4c4 100644
>>--- a/net/switchdev/switchdev.c
>>+++ b/net/switchdev/switchdev.c
>>@@ -535,8 +535,19 @@ static struct net_device *swdev_get_dev_by_nhs(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)
>>  {
>>+	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
>>@@ -554,15 +565,10 @@ int swdev_fib_ipv4_add(u32 dst, int dst_len, struct fib_info *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;
>>  }
>>@@ -583,8 +589,19 @@ EXPORT_SYMBOL_GPL(swdev_fib_ipv4_add);
>>  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,
>>+		},
>>+	};
>This looks nice. However, my only concern is we have now ended up adding a
>whole layer of abstraction to all objects. The api abstraction seemed fine.
>But, carrying it to objects and duplicating every object in the swdev layer
>seems too much duplication. Maybe its just me.
>we will end up adding this for bond attributes...vxlan...fdb and nftables

+1

>etc.
>
>The advantage of switchdev in the kernel was to have access the existing
>kernel api and objects directly from switchdev layer.
>In which case, to me, it would be better to skip this extra layer of objects.
>And keep the way you had it originally.


I agree.
>

Re: [PATCH net-next v2 00/26] switchdev: spring cleanup

[Scott Feldman]

On Wed, Apr 1, 2015 at 4:40 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Wed, Apr 01, 2015 at 12:07:36PM CEST, sfeldma@gmail.com wrote:
>>From: Scott Feldman <sfeldma@gmail.com>
>>
>>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
>
> I do not understand why this was not addresses. Can you please provide some
> justification?

1) It was 3AM in the morning, and I didn't want to fight a rename that
touched 26 patches in the set.

2) I like "swdev_" prefix because it's short and sweet and I don't
like long-winded symbols when a shorter one will do, so I had little
motivation for 1)

3) This is one of those labeling exercises where we can't get
consensus on the name.  So I'm ignoring to get the important stuff in,
and someone else can sponsor a name change.

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

[David Miller]

From: sfeldma@gmail.com
Date: Wed,  1 Apr 2015 03:07:37 -0700

> +int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
> +{
> +	struct swdev_attr prev = *attr;
> +	int err, get_err, revert_err;
> +
> +	get_err = swdev_port_attr_get(dev, &prev);
> +
> +	err = __swdev_port_attr_set(dev, attr);
> +	if (err && !get_err && !(attr->flags & SWDEV_F_NO_RECOVER)) {
> +		/* Some err on set: revert to previous value */
> +		revert_err = __swdev_port_attr_set(dev, &prev);
> +		if (revert_err)
> +			netdev_err(dev, "Reverting swdev port attr %d failed\n",
> +				   attr->id);
> +	}
> +
> +	return err;

This style of error recovery doesn't work.

You have to have a prepare/commit model to do this sanely, because
otherwise:

1) Partial state updates can be seen by the data plane (and other
   code paths) that do not use RTNL mutex protection.

2) It is absoultely expected that if some resource allocation failed
   when switching to the new attribute value, the same exactly thing
   is extremely likely during the rollback.

So you have to code this in a way that no partial state updates are
ever visible, and also that rollbacks don't fail.

I also agree with Jamal that you should use the netlink attribute
values for these attributes rather than inventing yet another
namespace of ID numbers.

Re: [PATCH net-next v2 00/26] switchdev: spring cleanup

[Jiri Pirko]

Wed, Apr 01, 2015 at 08:02:48PM CEST, sfeldma@gmail.com wrote:
>On Wed, Apr 1, 2015 at 4:40 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Wed, Apr 01, 2015 at 12:07:36PM CEST, sfeldma@gmail.com wrote:
>>>From: Scott Feldman <sfeldma@gmail.com>
>>>
>>>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
>>
>> I do not understand why this was not addresses. Can you please provide some
>> justification?
>
>1) It was 3AM in the morning, and I didn't want to fight a rename that
>touched 26 patches in the set.
>
>2) I like "swdev_" prefix because it's short and sweet and I don't
>like long-winded symbols when a shorter one will do, so I had little
>motivation for 1)
>
>3) This is one of those labeling exercises where we can't get
>consensus on the name.  So I'm ignoring to get the important stuff in,
>and someone else can sponsor a name change.

So just please avoid the name channge for now. Thanks.

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

[Scott Feldman]

On Wed, Apr 1, 2015 at 11:08 AM, David Miller <davem@davemloft.net> wrote:
> From: sfeldma@gmail.com
> Date: Wed,  1 Apr 2015 03:07:37 -0700
>
>> +int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
>> +{
>> +     struct swdev_attr prev = *attr;
>> +     int err, get_err, revert_err;
>> +
>> +     get_err = swdev_port_attr_get(dev, &prev);
>> +
>> +     err = __swdev_port_attr_set(dev, attr);
>> +     if (err && !get_err && !(attr->flags & SWDEV_F_NO_RECOVER)) {
>> +             /* Some err on set: revert to previous value */
>> +             revert_err = __swdev_port_attr_set(dev, &prev);
>> +             if (revert_err)
>> +                     netdev_err(dev, "Reverting swdev port attr %d failed\n",
>> +                                attr->id);
>> +     }
>> +
>> +     return err;
>
> This style of error recovery doesn't work.
>
> You have to have a prepare/commit model to do this sanely, because
> otherwise:
>
> 1) Partial state updates can be seen by the data plane (and other
>    code paths) that do not use RTNL mutex protection.
>
> 2) It is absoultely expected that if some resource allocation failed
>    when switching to the new attribute value, the same exactly thing
>    is extremely likely during the rollback.
>
> So you have to code this in a way that no partial state updates are
> ever visible, and also that rollbacks don't fail.

I'm sending v3 with a prepare/commit model, for attr sets and obj
adds.  The prepare phase asks driver(s) if set/add will work (is
supported and device resource is available).  If yes, then do commit
phase.   Commit could still fail due to failures outside the control
of the driver, like ENOMEM.  In that case, WARN and return err.

> I also agree with Jamal that you should use the netlink attribute
> values for these attributes rather than inventing yet another
> namespace of ID numbers.

That doesn't work.  The netlink ids are distributed across multiple
enum lists, so you'll inevitably have collisions in ID number.

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

[David Miller]

From: Scott Feldman <sfeldma@gmail.com>
Date: Thu, 2 Apr 2015 00:43:57 -0700

> On Wed, Apr 1, 2015 at 11:08 AM, David Miller <davem@davemloft.net> wrote:
>> From: sfeldma@gmail.com
>> Date: Wed,  1 Apr 2015 03:07:37 -0700
>>
>>> +int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
>>> +{
>>> +     struct swdev_attr prev = *attr;
>>> +     int err, get_err, revert_err;
>>> +
>>> +     get_err = swdev_port_attr_get(dev, &prev);
>>> +
>>> +     err = __swdev_port_attr_set(dev, attr);
>>> +     if (err && !get_err && !(attr->flags & SWDEV_F_NO_RECOVER)) {
>>> +             /* Some err on set: revert to previous value */
>>> +             revert_err = __swdev_port_attr_set(dev, &prev);
>>> +             if (revert_err)
>>> +                     netdev_err(dev, "Reverting swdev port attr %d failed\n",
>>> +                                attr->id);
>>> +     }
>>> +
>>> +     return err;
>>
>> This style of error recovery doesn't work.
>>
>> You have to have a prepare/commit model to do this sanely, because
>> otherwise:
>>
>> 1) Partial state updates can be seen by the data plane (and other
>>    code paths) that do not use RTNL mutex protection.
>>
>> 2) It is absoultely expected that if some resource allocation failed
>>    when switching to the new attribute value, the same exactly thing
>>    is extremely likely during the rollback.
>>
>> So you have to code this in a way that no partial state updates are
>> ever visible, and also that rollbacks don't fail.
> 
> I'm sending v3 with a prepare/commit model, for attr sets and obj
> adds.  The prepare phase asks driver(s) if set/add will work (is
> supported and device resource is available).  If yes, then do commit
> phase.   Commit could still fail due to failures outside the control
> of the driver, like ENOMEM.  In that case, WARN and return err.

Scott, the whole purpose of the prepare phase is the allocate any
necessary resources so that they are available for the commit phase.
It needs to do this in addition to validating the incoming arguments.

If some part of the prepare phase fails, you go back and release any
pre-allocated resources.

The commit phase must not fail.

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

[Scott Feldman]

On Thu, Apr 2, 2015 at 9:05 AM, David Miller <davem@davemloft.net> wrote:
> From: Scott Feldman <sfeldma@gmail.com>
> Date: Thu, 2 Apr 2015 00:43:57 -0700
>
>> On Wed, Apr 1, 2015 at 11:08 AM, David Miller <davem@davemloft.net> wrote:
>>> From: sfeldma@gmail.com
>>> Date: Wed,  1 Apr 2015 03:07:37 -0700
>>>
>>>> +int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
>>>> +{
>>>> +     struct swdev_attr prev = *attr;
>>>> +     int err, get_err, revert_err;
>>>> +
>>>> +     get_err = swdev_port_attr_get(dev, &prev);
>>>> +
>>>> +     err = __swdev_port_attr_set(dev, attr);
>>>> +     if (err && !get_err && !(attr->flags & SWDEV_F_NO_RECOVER)) {
>>>> +             /* Some err on set: revert to previous value */
>>>> +             revert_err = __swdev_port_attr_set(dev, &prev);
>>>> +             if (revert_err)
>>>> +                     netdev_err(dev, "Reverting swdev port attr %d failed\n",
>>>> +                                attr->id);
>>>> +     }
>>>> +
>>>> +     return err;
>>>
>>> This style of error recovery doesn't work.
>>>
>>> You have to have a prepare/commit model to do this sanely, because
>>> otherwise:
>>>
>>> 1) Partial state updates can be seen by the data plane (and other
>>>    code paths) that do not use RTNL mutex protection.
>>>
>>> 2) It is absoultely expected that if some resource allocation failed
>>>    when switching to the new attribute value, the same exactly thing
>>>    is extremely likely during the rollback.
>>>
>>> So you have to code this in a way that no partial state updates are
>>> ever visible, and also that rollbacks don't fail.
>>
>> I'm sending v3 with a prepare/commit model, for attr sets and obj
>> adds.  The prepare phase asks driver(s) if set/add will work (is
>> supported and device resource is available).  If yes, then do commit
>> phase.   Commit could still fail due to failures outside the control
>> of the driver, like ENOMEM.  In that case, WARN and return err.
>
> Scott, the whole purpose of the prepare phase is the allocate any
> necessary resources so that they are available for the commit phase.
> It needs to do this in addition to validating the incoming arguments.
>
> If some part of the prepare phase fails, you go back and release any
> pre-allocated resources.
>
> The commit phase must not fail.

I was afraid you were going to say that :(

I looked at doing that with rocker for setting STP state.  The driver
does allocate some system memory blocks, conditionally, about 4 call
levels down, and then it may or may not free them it.  To do that all
up-front in prepare, we'd need to traverse all the same code paths
allocating memory as we go and stashing it in some transaction object
passed from above, but skip any device accesses.  And then in commit,
go thru the same paths, but use stash memory from transaction object
and this time do call into device.  All of this to avoid failing
commit due to OOM.  So I reasoned OOM isn't a case worth the work
because the system is pretty much hosed at that point anyway, and went
with a prepare phase that only validates and checks for device
resources, but skip system resources.

It's not impossible, but my little Spring Cleanup patch is going to
turn into Spring Renovation patch.

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

[David Miller]

From: Scott Feldman <sfeldma@gmail.com>
Date: Thu, 2 Apr 2015 10:38:56 -0700

> I looked at doing that with rocker for setting STP state.  The driver
> does allocate some system memory blocks, conditionally, about 4 call
> levels down, and then it may or may not free them it.  To do that all
> up-front in prepare, we'd need to traverse all the same code paths
> allocating memory as we go and stashing it in some transaction object
> passed from above, but skip any device accesses.  And then in commit,
> go thru the same paths, but use stash memory from transaction object
> and this time do call into device.  All of this to avoid failing
> commit due to OOM.  So I reasoned OOM isn't a case worth the work
> because the system is pretty much hosed at that point anyway, and went
> with a prepare phase that only validates and checks for device
> resources, but skip system resources.
> 
> It's not impossible, but my little Spring Cleanup patch is going to
> turn into Spring Renovation patch.

I'm sorry if implementing this properly is not easy.

You'll thank me later for forcing you to do this properly, I promise.
:-)

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

[Andrew Lunn]

On Thu, Apr 02, 2015 at 10:38:56AM -0700, Scott Feldman wrote:
> On Thu, Apr 2, 2015 at 9:05 AM, David Miller <davem@davemloft.net> wrote:
> > From: Scott Feldman <sfeldma@gmail.com>
> > Date: Thu, 2 Apr 2015 00:43:57 -0700
> >
> >> On Wed, Apr 1, 2015 at 11:08 AM, David Miller <davem@davemloft.net> wrote:
> >>> From: sfeldma@gmail.com
> >>> Date: Wed,  1 Apr 2015 03:07:37 -0700
> >>>
> >>>> +int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
> >>>> +{
> >>>> +     struct swdev_attr prev = *attr;
> >>>> +     int err, get_err, revert_err;
> >>>> +
> >>>> +     get_err = swdev_port_attr_get(dev, &prev);
> >>>> +
> >>>> +     err = __swdev_port_attr_set(dev, attr);
> >>>> +     if (err && !get_err && !(attr->flags & SWDEV_F_NO_RECOVER)) {
> >>>> +             /* Some err on set: revert to previous value */
> >>>> +             revert_err = __swdev_port_attr_set(dev, &prev);
> >>>> +             if (revert_err)
> >>>> +                     netdev_err(dev, "Reverting swdev port attr %d failed\n",
> >>>> +                                attr->id);
> >>>> +     }
> >>>> +
> >>>> +     return err;
> >>>
> >>> This style of error recovery doesn't work.
> >>>
> >>> You have to have a prepare/commit model to do this sanely, because
> >>> otherwise:
> >>>
> >>> 1) Partial state updates can be seen by the data plane (and other
> >>>    code paths) that do not use RTNL mutex protection.
> >>>
> >>> 2) It is absoultely expected that if some resource allocation failed
> >>>    when switching to the new attribute value, the same exactly thing
> >>>    is extremely likely during the rollback.
> >>>
> >>> So you have to code this in a way that no partial state updates are
> >>> ever visible, and also that rollbacks don't fail.
> >>
> >> I'm sending v3 with a prepare/commit model, for attr sets and obj
> >> adds.  The prepare phase asks driver(s) if set/add will work (is
> >> supported and device resource is available).  If yes, then do commit
> >> phase.   Commit could still fail due to failures outside the control
> >> of the driver, like ENOMEM.  In that case, WARN and return err.
> >
> > Scott, the whole purpose of the prepare phase is the allocate any
> > necessary resources so that they are available for the commit phase.
> > It needs to do this in addition to validating the incoming arguments.
> >
> > If some part of the prepare phase fails, you go back and release any
> > pre-allocated resources.
> >
> > The commit phase must not fail.
> 
> I was afraid you were going to say that :(
> 
> I looked at doing that with rocker for setting STP state.  The driver
> does allocate some system memory blocks, conditionally, about 4 call
> levels down, and then it may or may not free them it.

Probably a dumb question:

How many of the problems are limited to rocker, and not other switch
devices? DSA allocates all its memory at probe time. There is no
runtime memory allocation. So ENOMEM, with the current code, is not
going to be an issue. Setting STP state is only going to fail if the
switch chip stops responding to the MDIO bus etc. Joining a port to a
bridge will only fail if there is a bug in the DSA code such that
masks don't make sense.

Do we need a generic, complex rollback system, or can we push that
complexity down into rocker?

	   Andrew

Re: [PATCH net-next v2 25/26] switchdev: convert swdev_fib_ipv4_add/del over to swdev_port_obj_add/del

[Florian Fainelli]

On 01/04/15 09:05, Jiri Pirko wrote:
>> This looks nice. However, my only concern is we have now ended up adding a
>> whole layer of abstraction to all objects. The api abstraction seemed fine.
>> But, carrying it to objects and duplicating every object in the swdev layer
>> seems too much duplication. Maybe its just me.
>> we will end up adding this for bond attributes...vxlan...fdb and nftables
> 
> +1

+1.

> 
>> etc.
>>
>> The advantage of switchdev in the kernel was to have access the existing
>> kernel api and objects directly from switchdev layer.
>> In which case, to me, it would be better to skip this extra layer of objects.
>> And keep the way you had it originally.

Same here, I am not exactly sure how much good the abstract object is
giving us here since we are already in the kernel, and we cannot/do not
necessarily want to eliminate a large number of swdev_ops, as these are
precisely the API we want to define.

NB: if we were to do that for swdev_ops, the next step would be doing
the same thing for netdev_ops to reduce their number, would not we?

Maybe for now solving the stacked device problem and
prepare/commit/rollback is good enough to keep you busy that adding the
object layer is a completely secondary problem to solve ;)?
-- 
Florian

Re: [PATCH net-next v2 25/26] switchdev: convert swdev_fib_ipv4_add/del over to swdev_port_obj_add/del

[Scott Feldman]

On Fri, Apr 3, 2015 at 11:00 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 01/04/15 09:05, Jiri Pirko wrote:
>>> This looks nice. However, my only concern is we have now ended up adding a
>>> whole layer of abstraction to all objects. The api abstraction seemed fine.
>>> But, carrying it to objects and duplicating every object in the swdev layer
>>> seems too much duplication. Maybe its just me.
>>> we will end up adding this for bond attributes...vxlan...fdb and nftables
>>
>> +1
>
> +1.
>
>>
>>> etc.
>>>
>>> The advantage of switchdev in the kernel was to have access the existing
>>> kernel api and objects directly from switchdev layer.
>>> In which case, to me, it would be better to skip this extra layer of objects.
>>> And keep the way you had it originally.
>
> Same here, I am not exactly sure how much good the abstract object is
> giving us here since we are already in the kernel, and we cannot/do not
> necessarily want to eliminate a large number of swdev_ops, as these are
> precisely the API we want to define.
>
> NB: if we were to do that for swdev_ops, the next step would be doing
> the same thing for netdev_ops to reduce their number, would not we?
>
> Maybe for now solving the stacked device problem and
> prepare/commit/rollback is good enough to keep you busy that adding the
> object layer is a completely secondary problem to solve ;)?

To solve the stacked device problem and support
prepare/commit/rollback, for both attrs and objs, but using a
parameter list call structure for each attr/obj type will result in a
lot of duplicated code.  You'll end up replicating the basic
lower-dev-walk recursion algo for each, and also replicating the
prepare/commit/rollback logic.  It's the typed parameter list unique
for each obj or attr that messes things up.  Going to the attr/obj
model let's us write the algos once.

We're also eliminating the swdev ops which pass netlink args.  There
are cases where we want to originate the swdev ops call within the
kernel, outside of a netlink message context.  For example, flushing
FDB entries from the device.  Today, to flush FDB entries from the
device, we'd need to call ndo_fdb_del() and pass in netlink arguments
that are made-up.  We create a fake struct ndmsg *ndm and a fake
struct nlattr *tb[] and pass those in to ndo_fdb_del() and hope no-one
expects anything useful in those fake structures.  Moving to the
attr/obj model, we can make the direct in-kernel call to flush the FDB
entries from the device without having to fake a netlink call inside
the kernel.

FDB entry obj would be the next obj to add.  The stacked support and
transaction support come for free.

-scott