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

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

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

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 (18):
  switchdev: introduce get/set attrs ops
  switchdev: flesh out get/set attr 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: 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: remove unused netdev_switch_port_bridge_dellink
  switchdev: remove unused NETIF_F_HW_SWITCH_OFFLOAD
  switchdev: add new swdev_port_bridge_getlink
  rocker: 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: bring documentation up-to-date

 Documentation/networking/switchdev.txt |  426 +++++++++++++++++++++++++++-----
 drivers/net/bonding/bond_main.c        |    8 +-
 drivers/net/ethernet/rocker/rocker.c   |  108 +++-----
 drivers/net/team/team.c                |    5 +-
 include/linux/netdev_features.h        |    5 +-
 include/net/switchdev.h                |  144 ++++++-----
 net/bridge/br.c                        |   22 +-
 net/bridge/br_netlink.c                |   24 +-
 net/bridge/br_stp.c                    |    6 +-
 net/core/ethtool.c                     |    1 -
 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              |  334 ++++++++++++++-----------
 15 files changed, 750 insertions(+), 425 deletions(-)

-- 
1.7.10.4

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

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

Add two new swdev ops for get/set switch port attributes.  We'll flesh these
out in the next patch.  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.

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

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index d2e69ee..7b72a4f 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -14,6 +14,14 @@
 #include <linux/netdevice.h>
 #include <linux/notifier.h>
 
+enum swdev_attr_id {
+	SWDEV_ATTR_UNDEFINED,
+};
+
+struct swdev_attr {
+	enum swdev_attr_id attr;
+};
+
 struct fib_info;
 
 /**
@@ -23,6 +31,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 +45,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 +84,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 +113,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..a894fa5 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -37,6 +37,30 @@ 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)
+{
+	return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL_GPL(swdev_port_attr_get);
+
+/**
+ *	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)
+{
+	return -EOPNOTSUPP;
+}
+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 02/18] switchdev: flesh out get/set attr ops

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

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_ATTR_F_NO_RECURSE to
indicate get/set only work on port (lowest) device.

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

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

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 7b72a4f..dcf0cb7 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -18,8 +18,12 @@ enum swdev_attr_id {
 	SWDEV_ATTR_UNDEFINED,
 };
 
+#define SWDEV_ATTR_F_NO_RECURSE		BIT(0)
+#define SWDEV_ATTR_F_NO_RECOVER		BIT(1)
+
 struct swdev_attr {
 	enum swdev_attr_id attr;
+	u32 flags;
 };
 
 struct fib_info;
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index a894fa5..f3cac92 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -44,10 +44,67 @@ EXPORT_SYMBOL_GPL(netdev_switch_parent_id_get);
  */
 int swdev_port_attr_get(struct net_device *dev, struct swdev_attr *attr)
 {
-	return -EOPNOTSUPP;
+	const struct swdev_ops *ops = dev->swdev_ops;
+	struct net_device *lower_dev;
+	struct list_head *iter;
+	struct swdev_attr first = {
+		.attr = SWDEV_ATTR_UNDEFINED
+	};
+	int err = -EOPNOTSUPP;
+
+	if (ops && ops->swdev_port_attr_get)
+		return ops->swdev_port_attr_get(dev, attr);
+
+	if (attr->flags & SWDEV_ATTR_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.attr == SWDEV_ATTR_UNDEFINED)
+			first = *attr;
+		else if (memcmp(&first, attr, sizeof(*attr)))
+			return -ENODATA;
+	}
+
+	return err;
 }
 EXPORT_SYMBOL_GPL(swdev_port_attr_get);
 
+static int _swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
+{
+	const struct swdev_ops *ops = dev->swdev_ops;
+	struct net_device *lower_dev;
+	struct list_head *iter;
+	int err = -EOPNOTSUPP;
+
+	if (ops && ops->swdev_port_attr_set)
+		return ops->swdev_port_attr_set(dev, attr);
+
+	if (attr->flags & SWDEV_ATTR_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
  *
@@ -56,7 +113,17 @@ EXPORT_SYMBOL_GPL(swdev_port_attr_get);
  */
 int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
 {
-	return -EOPNOTSUPP;
+	struct swdev_attr prev = *attr;
+	int err, get_err;
+
+	get_err = swdev_port_attr_get(dev, &prev);
+
+	err = _swdev_port_attr_set(dev, attr);
+	if (err && !get_err && !(attr->flags & SWDEV_ATTR_F_NO_RECOVER))
+		/* Some err on set: revert to previous value */
+		_swdev_port_attr_set(dev, &prev);
+
+	return err;
 }
 EXPORT_SYMBOL_GPL(swdev_port_attr_set);
 
-- 
1.7.10.4

Re: [PATCH net-next 02/18] switchdev: flesh out get/set attr ops

[Jiri Pirko]

Mon, Mar 30, 2015 at 10:40:20AM CEST, sfeldma@gmail.com wrote:
>From: Scott Feldman <sfeldma@gmail.com>
>
>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_ATTR_F_NO_RECURSE to
>indicate get/set only work on port (lowest) device.
>
>On set, allow a flag SWDEV_ATTR_F_NO_RECOVER to turn off automatic err
>recovery.
>
>Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>---
> include/net/switchdev.h   |    4 +++
> net/switchdev/switchdev.c |   71 +++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 73 insertions(+), 2 deletions(-)

I would squash this patch with the previous one.


>
>diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>index 7b72a4f..dcf0cb7 100644
>--- a/include/net/switchdev.h
>+++ b/include/net/switchdev.h
>@@ -18,8 +18,12 @@ enum swdev_attr_id {
> 	SWDEV_ATTR_UNDEFINED,
> };
> 
>+#define SWDEV_ATTR_F_NO_RECURSE		BIT(0)
>+#define SWDEV_ATTR_F_NO_RECOVER		BIT(1)
>+
> struct swdev_attr {
> 	enum swdev_attr_id attr;
>+	u32 flags;
> };
> 
> struct fib_info;
>diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
>index a894fa5..f3cac92 100644
>--- a/net/switchdev/switchdev.c
>+++ b/net/switchdev/switchdev.c
>@@ -44,10 +44,67 @@ EXPORT_SYMBOL_GPL(netdev_switch_parent_id_get);
>  */
> int swdev_port_attr_get(struct net_device *dev, struct swdev_attr *attr)
> {
>-	return -EOPNOTSUPP;
>+	const struct swdev_ops *ops = dev->swdev_ops;
>+	struct net_device *lower_dev;
>+	struct list_head *iter;
>+	struct swdev_attr first = {
>+		.attr = SWDEV_ATTR_UNDEFINED
>+	};
>+	int err = -EOPNOTSUPP;
>+
>+	if (ops && ops->swdev_port_attr_get)
>+		return ops->swdev_port_attr_get(dev, attr);
>+
>+	if (attr->flags & SWDEV_ATTR_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.attr == SWDEV_ATTR_UNDEFINED)
>+			first = *attr;
>+		else if (memcmp(&first, attr, sizeof(*attr)))
>+			return -ENODATA;
>+	}
>+
>+	return err;
> }
> EXPORT_SYMBOL_GPL(swdev_port_attr_get);
> 
>+static int _swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
>+{
>+	const struct swdev_ops *ops = dev->swdev_ops;
>+	struct net_device *lower_dev;
>+	struct list_head *iter;
>+	int err = -EOPNOTSUPP;
>+
>+	if (ops && ops->swdev_port_attr_set)
>+		return ops->swdev_port_attr_set(dev, attr);
>+
>+	if (attr->flags & SWDEV_ATTR_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
>  *
>@@ -56,7 +113,17 @@ EXPORT_SYMBOL_GPL(swdev_port_attr_get);
>  */
> int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
> {
>-	return -EOPNOTSUPP;
>+	struct swdev_attr prev = *attr;
>+	int err, get_err;
>+
>+	get_err = swdev_port_attr_get(dev, &prev);
>+
>+	err = _swdev_port_attr_set(dev, attr);
>+	if (err && !get_err && !(attr->flags & SWDEV_ATTR_F_NO_RECOVER))
>+		/* Some err on set: revert to previous value */
>+		_swdev_port_attr_set(dev, &prev);
>+
>+	return err;
> }
> EXPORT_SYMBOL_GPL(swdev_port_attr_set);
> 
>-- 
>1.7.10.4
>

RE: [PATCH net-next 02/18] switchdev: flesh out get/set attr ops

[Arad, Ronen]

>-----Original Message-----
>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>Behalf Of sfeldma@gmail.com
>Sent: Monday, March 30, 2015 1:40 AM
>To: netdev@vger.kernel.org
>Cc: jiri@resnulli.us; roopa@cumulusnetworks.com; linux@roeck-us.net;
>f.fainelli@gmail.com
>Subject: [PATCH net-next 02/18] switchdev: flesh out get/set attr ops
>
>From: Scott Feldman <sfeldma@gmail.com>
>
>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_ATTR_F_NO_RECURSE to
>indicate get/set only work on port (lowest) device.
>
>On set, allow a flag SWDEV_ATTR_F_NO_RECOVER to turn off automatic err
>recovery.
>
>Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>---
> include/net/switchdev.h   |    4 +++
> net/switchdev/switchdev.c |   71 +++++++++++++++++++++++++++++++++++++++++++-
>-
> 2 files changed, 73 insertions(+), 2 deletions(-)
>
>diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>index 7b72a4f..dcf0cb7 100644
>--- a/include/net/switchdev.h
>+++ b/include/net/switchdev.h
>@@ -18,8 +18,12 @@ enum swdev_attr_id {
> 	SWDEV_ATTR_UNDEFINED,
> };
>
>+#define SWDEV_ATTR_F_NO_RECURSE		BIT(0)
>+#define SWDEV_ATTR_F_NO_RECOVER		BIT(1)
>+
> struct swdev_attr {
> 	enum swdev_attr_id attr;
>+	u32 flags;
> };
>
> struct fib_info;
>diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
>index a894fa5..f3cac92 100644
>--- a/net/switchdev/switchdev.c
>+++ b/net/switchdev/switchdev.c
>@@ -44,10 +44,67 @@ EXPORT_SYMBOL_GPL(netdev_switch_parent_id_get);
>  */
> int swdev_port_attr_get(struct net_device *dev, struct swdev_attr *attr)
> {
>-	return -EOPNOTSUPP;
>+	const struct swdev_ops *ops = dev->swdev_ops;
>+	struct net_device *lower_dev;
>+	struct list_head *iter;
>+	struct swdev_attr first = {
>+		.attr = SWDEV_ATTR_UNDEFINED
>+	};
>+	int err = -EOPNOTSUPP;
>+
>+	if (ops && ops->swdev_port_attr_get)
>+		return ops->swdev_port_attr_get(dev, attr);
>+
>+	if (attr->flags & SWDEV_ATTR_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.attr == SWDEV_ATTR_UNDEFINED)
>+			first = *attr;
>+		else if (memcmp(&first, attr, sizeof(*attr)))
>+			return -ENODATA;
>+	}
>+
>+	return err;
> }
> EXPORT_SYMBOL_GPL(swdev_port_attr_get);
>
>+static int _swdev_port_attr_set(struct net_device *dev, struct swdev_attr
>*attr)
>+{
>+	const struct swdev_ops *ops = dev->swdev_ops;
>+	struct net_device *lower_dev;
>+	struct list_head *iter;
>+	int err = -EOPNOTSUPP;
>+
>+	if (ops && ops->swdev_port_attr_set)
>+		return ops->swdev_port_attr_set(dev, attr);
>+
>+	if (attr->flags & SWDEV_ATTR_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
>  *
>@@ -56,7 +113,17 @@ EXPORT_SYMBOL_GPL(swdev_port_attr_get);
>  */
> int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
> {
>-	return -EOPNOTSUPP;
>+	struct swdev_attr prev = *attr;
>+	int err, get_err;
>+
>+	get_err = swdev_port_attr_get(dev, &prev);
>+
>+	err = _swdev_port_attr_set(dev, attr);
>+	if (err && !get_err && !(attr->flags & SWDEV_ATTR_F_NO_RECOVER))
>+		/* Some err on set: revert to previous value */
>+		_swdev_port_attr_set(dev, &prev);

Could revering to previous value fail?
Should such failure be logged?

>+
>+	return err;
> }
> EXPORT_SYMBOL_GPL(swdev_port_attr_set);
>
>--
>1.7.10.4
>
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next 02/18] switchdev: flesh out get/set attr ops

[Jiri Pirko]

Mon, Mar 30, 2015 at 08:32:09PM CEST, ronen.arad@intel.com wrote:
>
>
>>-----Original Message-----
>>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>>Behalf Of sfeldma@gmail.com
>>Sent: Monday, March 30, 2015 1:40 AM
>>To: netdev@vger.kernel.org
>>Cc: jiri@resnulli.us; roopa@cumulusnetworks.com; linux@roeck-us.net;
>>f.fainelli@gmail.com
>>Subject: [PATCH net-next 02/18] switchdev: flesh out get/set attr ops
>>
>>From: Scott Feldman <sfeldma@gmail.com>
>>
>>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_ATTR_F_NO_RECURSE to
>>indicate get/set only work on port (lowest) device.
>>
>>On set, allow a flag SWDEV_ATTR_F_NO_RECOVER to turn off automatic err
>>recovery.
>>
>>Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>>---
>> include/net/switchdev.h   |    4 +++
>> net/switchdev/switchdev.c |   71 +++++++++++++++++++++++++++++++++++++++++++-
>>-
>> 2 files changed, 73 insertions(+), 2 deletions(-)
>>
>>diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>>index 7b72a4f..dcf0cb7 100644
>>--- a/include/net/switchdev.h
>>+++ b/include/net/switchdev.h
>>@@ -18,8 +18,12 @@ enum swdev_attr_id {
>> 	SWDEV_ATTR_UNDEFINED,
>> };
>>
>>+#define SWDEV_ATTR_F_NO_RECURSE		BIT(0)
>>+#define SWDEV_ATTR_F_NO_RECOVER		BIT(1)
>>+
>> struct swdev_attr {
>> 	enum swdev_attr_id attr;
>>+	u32 flags;
>> };
>>
>> struct fib_info;
>>diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
>>index a894fa5..f3cac92 100644
>>--- a/net/switchdev/switchdev.c
>>+++ b/net/switchdev/switchdev.c
>>@@ -44,10 +44,67 @@ EXPORT_SYMBOL_GPL(netdev_switch_parent_id_get);
>>  */
>> int swdev_port_attr_get(struct net_device *dev, struct swdev_attr *attr)
>> {
>>-	return -EOPNOTSUPP;
>>+	const struct swdev_ops *ops = dev->swdev_ops;
>>+	struct net_device *lower_dev;
>>+	struct list_head *iter;
>>+	struct swdev_attr first = {
>>+		.attr = SWDEV_ATTR_UNDEFINED
>>+	};
>>+	int err = -EOPNOTSUPP;
>>+
>>+	if (ops && ops->swdev_port_attr_get)
>>+		return ops->swdev_port_attr_get(dev, attr);
>>+
>>+	if (attr->flags & SWDEV_ATTR_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.attr == SWDEV_ATTR_UNDEFINED)
>>+			first = *attr;
>>+		else if (memcmp(&first, attr, sizeof(*attr)))
>>+			return -ENODATA;
>>+	}
>>+
>>+	return err;
>> }
>> EXPORT_SYMBOL_GPL(swdev_port_attr_get);
>>
>>+static int _swdev_port_attr_set(struct net_device *dev, struct swdev_attr
>>*attr)
>>+{
>>+	const struct swdev_ops *ops = dev->swdev_ops;
>>+	struct net_device *lower_dev;
>>+	struct list_head *iter;
>>+	int err = -EOPNOTSUPP;
>>+
>>+	if (ops && ops->swdev_port_attr_set)
>>+		return ops->swdev_port_attr_set(dev, attr);
>>+
>>+	if (attr->flags & SWDEV_ATTR_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
>>  *
>>@@ -56,7 +113,17 @@ EXPORT_SYMBOL_GPL(swdev_port_attr_get);
>>  */
>> int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
>> {
>>-	return -EOPNOTSUPP;
>>+	struct swdev_attr prev = *attr;
>>+	int err, get_err;
>>+
>>+	get_err = swdev_port_attr_get(dev, &prev);
>>+
>>+	err = _swdev_port_attr_set(dev, attr);
>>+	if (err && !get_err && !(attr->flags & SWDEV_ATTR_F_NO_RECOVER))
>>+		/* Some err on set: revert to previous value */
>>+		_swdev_port_attr_set(dev, &prev);
>
>Could revering to previous value fail?

I believe that it certainly could

>Should such failure be logged?

That is a good point.

Also, looking at the function name, seems nicer to me to use "__"
instead of "_" as a function prefix. Not sure if that is some rule
somewhere.

>
>>+
>>+	return err;
>> }
>> EXPORT_SYMBOL_GPL(swdev_port_attr_set);
>>
>>--
>>1.7.10.4
>>
>>--
>>To unsubscribe from this list: send the line "unsubscribe netdev" in
>>the body of a message to majordomo@vger.kernel.org
>>More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next 02/18] switchdev: flesh out get/set attr ops

[Scott Feldman]

On Mon, Mar 30, 2015 at 1:46 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Mon, Mar 30, 2015 at 08:32:09PM CEST, ronen.arad@intel.com wrote:
>>
>>
>>>-----Original Message-----
>>>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>>>Behalf Of sfeldma@gmail.com
>>>Sent: Monday, March 30, 2015 1:40 AM
>>>To: netdev@vger.kernel.org
>>>Cc: jiri@resnulli.us; roopa@cumulusnetworks.com; linux@roeck-us.net;
>>>f.fainelli@gmail.com
>>>Subject: [PATCH net-next 02/18] switchdev: flesh out get/set attr ops
>>>
>>>From: Scott Feldman <sfeldma@gmail.com>
>>>

[cut]

>>>+     err = _swdev_port_attr_set(dev, attr);
>>>+     if (err && !get_err && !(attr->flags & SWDEV_ATTR_F_NO_RECOVER))
>>>+             /* Some err on set: revert to previous value */
>>>+             _swdev_port_attr_set(dev, &prev);
>>
>>Could revering to previous value fail?
>
> I believe that it certainly could
>
>>Should such failure be logged?
>
> That is a good point.

I'll add a log msg on revert set failure, good suggestion.


> Also, looking at the function name, seems nicer to me to use "__"
> instead of "_" as a function prefix. Not sure if that is some rule
> somewhere.

Sure.  I'm not sure what the rule is either.  I seems to remember
about using "_" prefix for funcs called under lock, or something like
that.  I'll change it to "__".

RE: [PATCH net-next 02/18] switchdev: flesh out get/set attr ops

[Arad, Ronen]

>-----Original Message-----
>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>Behalf Of sfeldma@gmail.com
>Sent: Monday, March 30, 2015 1:40 AM
>To: netdev@vger.kernel.org
>Cc: jiri@resnulli.us; roopa@cumulusnetworks.com; linux@roeck-us.net;
>f.fainelli@gmail.com
>Subject: [PATCH net-next 02/18] switchdev: flesh out get/set attr ops
>
>From: Scott Feldman <sfeldma@gmail.com>
>
[cut]
>
> int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
> {
>-	return -EOPNOTSUPP;
>+	struct swdev_attr prev = *attr;
>+	int err, get_err;
>+
>+	get_err = swdev_port_attr_get(dev, &prev);
>+
>+	err = _swdev_port_attr_set(dev, attr);
>+	if (err && !get_err && !(attr->flags & SWDEV_ATTR_F_NO_RECOVER))
>+		/* Some err on set: revert to previous value */
>+		_swdev_port_attr_set(dev, &prev);

Netlink requests could contain multiple attributes within a single request.
Reverting to the previous value applies only to the first
swdev_port_attr_set error. It does not rollback all prior changes that
were triggered by the same Netlink request.

>+
>+	return err;
> }
> EXPORT_SYMBOL_GPL(swdev_port_attr_set);
>
>--
>1.7.10.4
>
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next 02/18] switchdev: flesh out get/set attr ops

[Scott Feldman]

On Mon, Mar 30, 2015 at 5:22 PM, Arad, Ronen <ronen.arad@intel.com> wrote:
>
>
>>-----Original Message-----
>>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>>Behalf Of sfeldma@gmail.com
>>Sent: Monday, March 30, 2015 1:40 AM
>>To: netdev@vger.kernel.org
>>Cc: jiri@resnulli.us; roopa@cumulusnetworks.com; linux@roeck-us.net;
>>f.fainelli@gmail.com
>>Subject: [PATCH net-next 02/18] switchdev: flesh out get/set attr ops
>>
>>From: Scott Feldman <sfeldma@gmail.com>
>>
> [cut]
>>
>> int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
>> {
>>-      return -EOPNOTSUPP;
>>+      struct swdev_attr prev = *attr;
>>+      int err, get_err;
>>+
>>+      get_err = swdev_port_attr_get(dev, &prev);
>>+
>>+      err = _swdev_port_attr_set(dev, attr);
>>+      if (err && !get_err && !(attr->flags & SWDEV_ATTR_F_NO_RECOVER))
>>+              /* Some err on set: revert to previous value */
>>+              _swdev_port_attr_set(dev, &prev);
>
> Netlink requests could contain multiple attributes within a single request.
> Reverting to the previous value applies only to the first
> swdev_port_attr_set error. It does not rollback all prior changes that
> were triggered by the same Netlink request.

Since attr_set scope is a single attr across ports, it can only revert
that attr for those ports.  So rewinding multiple attributes needs to
happen above attr_set.  I guess we could have a multi_attr_set that
takes an array of *attrs, but something like this can be built on top
of attr_set.  Maybe baby steps is best first so we can see how things
shape up.

Re: [PATCH net-next 02/18] switchdev: flesh out get/set attr ops

[roopa]

On 3/30/15, 5:38 PM, Scott Feldman wrote:
> On Mon, Mar 30, 2015 at 5:22 PM, Arad, Ronen <ronen.arad@intel.com> wrote:
>>
>>> -----Original Message-----
>>> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>>> Behalf Of sfeldma@gmail.com
>>> Sent: Monday, March 30, 2015 1:40 AM
>>> To: netdev@vger.kernel.org
>>> Cc: jiri@resnulli.us; roopa@cumulusnetworks.com; linux@roeck-us.net;
>>> f.fainelli@gmail.com
>>> Subject: [PATCH net-next 02/18] switchdev: flesh out get/set attr ops
>>>
>>> From: Scott Feldman <sfeldma@gmail.com>
>>>
>> [cut]
>>> int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
>>> {
>>> -      return -EOPNOTSUPP;
>>> +      struct swdev_attr prev = *attr;
>>> +      int err, get_err;
>>> +
>>> +      get_err = swdev_port_attr_get(dev, &prev);
>>> +
>>> +      err = _swdev_port_attr_set(dev, attr);
>>> +      if (err && !get_err && !(attr->flags & SWDEV_ATTR_F_NO_RECOVER))
>>> +              /* Some err on set: revert to previous value */
>>> +              _swdev_port_attr_set(dev, &prev);
>> Netlink requests could contain multiple attributes within a single request.
>> Reverting to the previous value applies only to the first
>> swdev_port_attr_set error. It does not rollback all prior changes that
>> were triggered by the same Netlink request.
> Since attr_set scope is a single attr across ports, it can only revert
> that attr for those ports.  So rewinding multiple attributes needs to
> happen above attr_set.  I guess we could have a multi_attr_set that
> takes an array of *attrs, but something like this can be built on top
> of attr_set.  Maybe baby steps is best first so we can see how things
> shape up.
Having a multi attr set/del is necessary in some cases where the hw api 
may be able to handle more than one attr set/del. I can see this 
especially in the vlan case. Having the driver get all attributes and do 
the necessary optimizations to program hardware might be better.

thanks,
Roopa

Re: [PATCH net-next 02/18] switchdev: flesh out get/set attr ops

[Scott Feldman]

On Tue, Mar 31, 2015 at 8:30 AM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>
>
> On Monday, March 30, 2015, Scott Feldman <sfeldma@gmail.com> wrote:
>>
>> On Mon, Mar 30, 2015 at 5:22 PM, Arad, Ronen <ronen.arad@intel.com> wrote:
>> >
>> >
>> >>-----Original Message-----
>> >>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
>> >> On
>> >>Behalf Of sfeldma@gmail.com
>> >>Sent: Monday, March 30, 2015 1:40 AM
>> >>To: netdev@vger.kernel.org
>> >>Cc: jiri@resnulli.us; roopa@cumulusnetworks.com; linux@roeck-us.net;
>> >>f.fainelli@gmail.com
>> >>Subject: [PATCH net-next 02/18] switchdev: flesh out get/set attr ops
>> >>
>> >>From: Scott Feldman <sfeldma@gmail.com>
>> >>
>> > [cut]
>> >>
>> >> int swdev_port_attr_set(struct net_device *dev, struct swdev_attr
>> >> *attr)
>> >> {
>> >>-      return -EOPNOTSUPP;
>> >>+      struct swdev_attr prev = *attr;
>> >>+      int err, get_err;
>> >>+
>> >>+      get_err = swdev_port_attr_get(dev, &prev);
>> >>+
>> >>+      err = _swdev_port_attr_set(dev, attr);
>> >>+      if (err && !get_err && !(attr->flags & SWDEV_ATTR_F_NO_RECOVER))
>> >>+              /* Some err on set: revert to previous value */
>> >>+              _swdev_port_attr_set(dev, &prev);
>> >
>> > Netlink requests could contain multiple attributes within a single
>> > request.
>> > Reverting to the previous value applies only to the first
>> > swdev_port_attr_set error. It does not rollback all prior changes that
>> > were triggered by the same Netlink request.
>>
>> Since attr_set scope is a single attr across ports, it can only revert
>> that attr for those ports.  So rewinding multiple attributes needs to
>> happen above attr_set.  I guess we could have a multi_attr_set that
>> takes an array of *attrs, but something like this can be built on top
>> of attr_set.  Maybe baby steps is best first so we can see how things
>> shape up.
>
>
>  Having a multi attr set/del is necessary in some cases where the hw api may
> be able to handle more than one attr set/del. I can see this especially for
> vlans. So, it would be nice if the driver can get all attributes and do the
> necessary optimizations to program hardware.

Yup, agreed, I'm adding that for vlans for v2 so it honors the vlan
range work you recently added and the driver gets one call to install
a range-worths of vlans.

[PATCH net-next 03/18] 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_ATTR_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..4bcc555 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->attr) {
+	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 dcf0cb7..fd36b5c 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -16,6 +16,7 @@
 
 enum swdev_attr_id {
 	SWDEV_ATTR_UNDEFINED,
+	SWDEV_ATTR_PORT_PARENT_ID,
 };
 
 #define SWDEV_ATTR_F_NO_RECURSE		BIT(0)
@@ -24,6 +25,9 @@ enum swdev_attr_id {
 struct swdev_attr {
 	enum swdev_attr_id attr;
 	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..dcec629 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 = {
+			.attr = SWDEV_ATTR_PORT_PARENT_ID,
+			.flags = SWDEV_ATTR_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..3ae6052 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 = {
+		.attr = SWDEV_ATTR_PORT_PARENT_ID,
+		.flags = SWDEV_ATTR_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..2a5bfde 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->attr) {
+	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 f3cac92..24440c5 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
@@ -336,11 +318,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) {
@@ -354,8 +335,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 = {
+		.attr = SWDEV_ATTR_PORT_PARENT_ID,
+	};
+	struct swdev_attr prev_attr;
 	struct net_device *dev = NULL;
 	int nhsel;
 
@@ -371,17 +354,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 04/18] 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 4bcc555..224f91d 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->attr) {
+	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 fd36b5c..99050b0 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -17,6 +17,7 @@
 enum swdev_attr_id {
 	SWDEV_ATTR_UNDEFINED,
 	SWDEV_ATTR_PORT_PARENT_ID,
+	SWDEV_ATTR_PORT_STP_STATE,
 };
 
 #define SWDEV_ATTR_F_NO_RECURSE		BIT(0)
@@ -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..8b2ef23 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 = {
+		.attr = 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 2a5bfde..317bb38 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->attr) {
+	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 24440c5..162b011 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -109,34 +109,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

Re: [PATCH net-next 04/18] switchdev: convert STP update to swdev attr set

[Jiri Pirko]

Mon, Mar 30, 2015 at 10:40:22AM CEST, sfeldma@gmail.com wrote:
>From: Scott Feldman <sfeldma@gmail.com>
>
>STP update is just a writable port attribute, so convert swdev_port_stp_update
>to an attr set.
>
>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 4bcc555..224f91d 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->attr) {
>+	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 fd36b5c..99050b0 100644
>--- a/include/net/switchdev.h
>+++ b/include/net/switchdev.h
>@@ -17,6 +17,7 @@
> enum swdev_attr_id {
> 	SWDEV_ATTR_UNDEFINED,
> 	SWDEV_ATTR_PORT_PARENT_ID,
>+	SWDEV_ATTR_PORT_STP_STATE,
> };
> 
> #define SWDEV_ATTR_F_NO_RECURSE		BIT(0)
>@@ -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..8b2ef23 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 = {
>+		.attr = 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);

Seems nicer to me to provide a wrapper for callers so they don't call
directly swdev_port_attr_set and have no knowledge of swdev_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 2a5bfde..317bb38 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->attr) {
>+	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 24440c5..162b011 100644
>--- a/net/switchdev/switchdev.c
>+++ b/net/switchdev/switchdev.c
>@@ -109,34 +109,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
>

Re: [PATCH net-next 04/18] switchdev: convert STP update to swdev attr set

[roopa]

On 3/30/15, 4:54 AM, Jiri Pirko wrote:
> Mon, Mar 30, 2015 at 10:40:22AM CEST, sfeldma@gmail.com wrote:
>> From: Scott Feldman <sfeldma@gmail.com>
>>
>> STP update is just a writable port attribute, so convert swdev_port_stp_update
>> to an attr set.
>>
>> 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 4bcc555..224f91d 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->attr) {
>> +	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 fd36b5c..99050b0 100644
>> --- a/include/net/switchdev.h
>> +++ b/include/net/switchdev.h
>> @@ -17,6 +17,7 @@
>> enum swdev_attr_id {
>> 	SWDEV_ATTR_UNDEFINED,
>> 	SWDEV_ATTR_PORT_PARENT_ID,
>> +	SWDEV_ATTR_PORT_STP_STATE,
>> };
>>
>> #define SWDEV_ATTR_F_NO_RECURSE		BIT(0)
>> @@ -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..8b2ef23 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 = {
>> +		.attr = 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);
> Seems nicer to me to provide a wrapper for callers so they don't call
> directly swdev_port_attr_set and have no knowledge of swdev_attr.
+1. I had a similar patch long back. swdev_port_attr_set can take a 
attrname and void argument.
the caller only needs to know the attrname in that case.

I had used netlink attribute names at that time to not have another set 
of duplicate attribute names in the swdev layer. But then the api has to 
be called swdev_bridge_port_attr_set to match the bridge attribute 
policy name.
but, i have no objections to having  the new attribute names 
SWDEV_ATTR_PORT_*

[PATCH net-next 05/18] 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 99050b0..2ac5240 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -18,6 +18,7 @@ enum swdev_attr_id {
 	SWDEV_ATTR_UNDEFINED,
 	SWDEV_ATTR_PORT_PARENT_ID,
 	SWDEV_ATTR_PORT_STP_STATE,
+	SWDEV_ATTR_PORT_BRIDGE_FLAGS,
 };
 
 #define SWDEV_ATTR_F_NO_RECURSE		BIT(0)
@@ -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 06/18] 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 224f91d..bc4fd33 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->attr) {
 	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

Re: [PATCH net-next 06/18] rocker: use swdev get/set attr for bridge port flags

[Jiri Pirko]

Mon, Mar 30, 2015 at 10:40:24AM CEST, sfeldma@gmail.com wrote:
>From: Scott Feldman <sfeldma@gmail.com>
>
>Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>---
> drivers/net/ethernet/rocker/rocker.c |   10 ++++++++++
> 1 file changed, 10 insertions(+)

I would squash this patch with the previous one.

>
>diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
>index 224f91d..bc4fd33 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->attr) {
> 	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 07/18] 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.  Currently, only bridge port attrs BR_LEARNING and
BR_LEARNING_SYNC are parsed and passed to port driver.

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

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 2ac5240..bf66bcd 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -86,6 +86,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_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,
@@ -118,6 +120,12 @@ static inline int swdev_port_attr_set(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 162b011..f3f457e 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>
 
@@ -197,6 +198,91 @@ int netdev_switch_port_bridge_setlink(struct net_device *dev,
 }
 EXPORT_SYMBOL_GPL(netdev_switch_port_bridge_setlink);
 
+static int swdev_port_bridge_setflag(struct net_device *dev,
+				     struct nlattr *ifla,
+				     unsigned long brport_flag)
+{
+	struct swdev_attr attr = {
+		.attr = SWDEV_ATTR_PORT_BRIDGE_FLAGS,
+	};
+	u8 flag = nla_get_u8(ifla);
+	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 },
+};
+
+/**
+ *	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 *ifla;
+	int rem;
+	int err;
+
+	protinfo = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg),
+				   IFLA_PROTINFO);
+	if (!protinfo)
+		return 0;
+
+	err = nla_validate_nested(protinfo, IFLA_BRPORT_MAX,
+				  swdev_port_bridge_policy);
+	if (err)
+		return err;
+
+	nla_for_each_nested(ifla, protinfo, rem) {
+		switch (nla_type(ifla)) {
+		case IFLA_BRPORT_LEARNING:
+			err = swdev_port_bridge_setflag(dev, ifla,
+							BR_LEARNING);
+			break;
+		case IFLA_BRPORT_LEARNING_SYNC:
+			err = swdev_port_bridge_setflag(dev, ifla,
+							BR_LEARNING_SYNC);
+			break;
+		default:
+			err = -EOPNOTSUPP;
+			break;
+		}
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+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

Re: [PATCH net-next 07/18] switchdev: add new swdev bridge setlink

[Jiri Pirko]

Mon, Mar 30, 2015 at 10:40:25AM CEST, sfeldma@gmail.com wrote:
>From: Scott Feldman <sfeldma@gmail.com>
>
>Add new swdev_port_bridge_setlink that can be used by drivers implementing
>.ndo_bridge_setlink to set swdev bridge attributes.  Basically turn the raw
>rtnl_bridge_setlink netlink into swdev attr sets.  Proper netlink attr policy
>checking is done.  Currently, only bridge port attrs BR_LEARNING and
>BR_LEARNING_SYNC are parsed and passed to port driver.
>
>Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>---
> include/net/switchdev.h   |    8 +++++
> net/switchdev/switchdev.c |   86 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 94 insertions(+)
>

...

>+int swdev_port_bridge_setlink(struct net_device *dev,
>+			      struct nlmsghdr *nlh, u16 flags)
>+{
>+	struct nlattr *protinfo;
>+	struct nlattr *ifla;

	I would prefer just "attr" for example instead of "ifla"

[PATCH net-next 08/18] 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 bc4fd33..475ce93 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 09/18] 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 10/18] 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 11/18] 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 bf66bcd..bad2ec7 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -92,14 +92,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,
@@ -142,13 +138,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)
@@ -163,13 +152,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 f3f457e..0307a45 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -173,31 +173,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_bridge_setflag(struct net_device *dev,
 				     struct nlattr *ifla,
 				     unsigned long brport_flag)
@@ -309,36 +284,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

Re: [PATCH net-next 11/18] switchdev: remove old netdev_switch_port_bridge_setlink

[roopa]

On 3/30/15, 1:40 AM, sfeldma@gmail.com wrote:
> From: Scott Feldman <sfeldma@gmail.com>
>
> New attr-based bridge_setlink can recurse lower devs and recover on err, so
> remove old wrapper.  Also, restore br_setlink back to original and don't call
> into SELF port driver.  rtnetlink.c:bridge_setlink already does a call into
> port driver for SELF.
>
> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
removing this now requires every vlan add to be a two step process, why ?

bridge vlan add dev swp1 vid 10
bridge vlan add dev swp1 vid 10 self

and userspace will get two notifications.

> ---
>   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 bf66bcd..bad2ec7 100644
> --- a/include/net/switchdev.h
> +++ b/include/net/switchdev.h
> @@ -92,14 +92,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,
> @@ -142,13 +138,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)
> @@ -163,13 +152,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 f3f457e..0307a45 100644
> --- a/net/switchdev/switchdev.c
> +++ b/net/switchdev/switchdev.c
> @@ -173,31 +173,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_bridge_setflag(struct net_device *dev,
>   				     struct nlattr *ifla,
>   				     unsigned long brport_flag)
> @@ -309,36 +284,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
>    *

Re: [PATCH net-next 11/18] switchdev: remove old netdev_switch_port_bridge_setlink

[Scott Feldman]

On Mon, Mar 30, 2015 at 6:23 AM, roopa <roopa@cumulusnetworks.com> wrote:
> On 3/30/15, 1:40 AM, sfeldma@gmail.com wrote:
>>
>> From: Scott Feldman <sfeldma@gmail.com>
>>
>> New attr-based bridge_setlink can recurse lower devs and recover on err,
>> so
>> remove old wrapper.  Also, restore br_setlink back to original and don't
>> call
>> into SELF port driver.  rtnetlink.c:bridge_setlink already does a call
>> into
>> port driver for SELF.
>>
>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>
> removing this now requires every vlan add to be a two step process, why ?

No, that's not true.  You want to use
ndo_vlan_rx_add_vid/ndo_vlan_rx_kill_vid in your port driver, and then
using either vlan driver standalone or the bridge driver vlan support
will work.

Try it.  Implement ndo_vlan_rx_add_vid in your port driver and verify
you get called to add VLAN to port with either:

    bridge vlan add dev swp1 vid 10

    -or-

    vconfig add swp1 10

Same for deleting a VLAN, either of these two commands call into the
port driver ndo_vlan_rx_kill_vid:

    bridge vlan del dev swp1 vid 10

    -or-

    vconfig rem swp1 10


> bridge vlan add dev swp1 vid 10
> bridge vlan add dev swp1 vid 10 self

Not necessary.  The first command is sufficient if using
ndo_vlan_rx_add_vid.  If you want the self version to work, add
ndo_bridge_setlink/dellink ops to your port driver.

> and userspace will get two notifications.

Single command (as shown in my examples) generates one user notification.

RE: [PATCH net-next 11/18] switchdev: remove old netdev_switch_port_bridge_setlink

[Arad, Ronen]

>-----Original Message-----
>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>Behalf Of Scott Feldman
>Sent: Monday, March 30, 2015 1:20 PM
>To: roopa
>Cc: Netdev; Jiří Pírko; Guenter Roeck; Florian Fainelli
>Subject: Re: [PATCH net-next 11/18] switchdev: remove old
>netdev_switch_port_bridge_setlink
>
>On Mon, Mar 30, 2015 at 6:23 AM, roopa <roopa@cumulusnetworks.com> wrote:
>> On 3/30/15, 1:40 AM, sfeldma@gmail.com wrote:
>>>
>>> From: Scott Feldman <sfeldma@gmail.com>
>>>
>>> New attr-based bridge_setlink can recurse lower devs and recover on err,
>>> so
>>> remove old wrapper.  Also, restore br_setlink back to original and don't
>>> call
>>> into SELF port driver.  rtnetlink.c:bridge_setlink already does a call
>>> into
>>> port driver for SELF.
>>>
>>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>>
>> removing this now requires every vlan add to be a two step process, why ?
>
>No, that's not true.  You want to use
>ndo_vlan_rx_add_vid/ndo_vlan_rx_kill_vid in your port driver, and then
>using either vlan driver standalone or the bridge driver vlan support
>will work.
>
>Try it.  Implement ndo_vlan_rx_add_vid in your port driver and verify
>you get called to add VLAN to port with either:
>
>    bridge vlan add dev swp1 vid 10
>
>    -or-
>
>    vconfig add swp1 10
>
>Same for deleting a VLAN, either of these two commands call into the
>port driver ndo_vlan_rx_kill_vid:
>
>    bridge vlan del dev swp1 vid 10
>
>    -or-
>
>    vconfig rem swp1 10
>
>
>> bridge vlan add dev swp1 vid 10
>> bridge vlan add dev swp1 vid 10 self
>
>Not necessary.  The first command is sufficient if using
>ndo_vlan_rx_add_vid. 

This is not sufficient for VLAN filtering. Ndo_vlan_rx_add_vid does not
provide the vinfo flags PVID and UNTAGGED. Therefore it is not
an adequate replacement for propagating setlink/dellink messages to the
swithport driver or an alternative via swdev_attr.
  
 If you want the self version to work, add
>ndo_bridge_setlink/dellink ops to your port driver.
>

>> and userspace will get two notifications.
>
>Single command (as shown in my examples) generates one user notification.
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next 11/18] switchdev: remove old netdev_switch_port_bridge_setlink

[Scott Feldman]

On Mon, Mar 30, 2015 at 1:46 PM, Arad, Ronen <ronen.arad@intel.com> wrote:
>
>
>>-----Original Message-----
>>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>>Behalf Of Scott Feldman
>>Sent: Monday, March 30, 2015 1:20 PM
>>To: roopa
>>Cc: Netdev; Jiří Pírko; Guenter Roeck; Florian Fainelli
>>Subject: Re: [PATCH net-next 11/18] switchdev: remove old
>>netdev_switch_port_bridge_setlink
>>
>>On Mon, Mar 30, 2015 at 6:23 AM, roopa <roopa@cumulusnetworks.com> wrote:
>>> On 3/30/15, 1:40 AM, sfeldma@gmail.com wrote:
>>>>
>>>> From: Scott Feldman <sfeldma@gmail.com>
>>>>
>>>> New attr-based bridge_setlink can recurse lower devs and recover on err,
>>>> so
>>>> remove old wrapper.  Also, restore br_setlink back to original and don't
>>>> call
>>>> into SELF port driver.  rtnetlink.c:bridge_setlink already does a call
>>>> into
>>>> port driver for SELF.
>>>>
>>>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>>>
>>> removing this now requires every vlan add to be a two step process, why ?
>>
>>No, that's not true.  You want to use
>>ndo_vlan_rx_add_vid/ndo_vlan_rx_kill_vid in your port driver, and then
>>using either vlan driver standalone or the bridge driver vlan support
>>will work.
>>
>>Try it.  Implement ndo_vlan_rx_add_vid in your port driver and verify
>>you get called to add VLAN to port with either:
>>
>>    bridge vlan add dev swp1 vid 10
>>
>>    -or-
>>
>>    vconfig add swp1 10
>>
>>Same for deleting a VLAN, either of these two commands call into the
>>port driver ndo_vlan_rx_kill_vid:
>>
>>    bridge vlan del dev swp1 vid 10
>>
>>    -or-
>>
>>    vconfig rem swp1 10
>>
>>
>>> bridge vlan add dev swp1 vid 10
>>> bridge vlan add dev swp1 vid 10 self
>>
>>Not necessary.  The first command is sufficient if using
>>ndo_vlan_rx_add_vid.
>
> This is not sufficient for VLAN filtering. Ndo_vlan_rx_add_vid does not
> provide the vinfo flags PVID and UNTAGGED. Therefore it is not
> an adequate replacement for propagating setlink/dellink messages to the
> swithport driver or an alternative via swdev_attr.

Glad you bring that point up.  I think these can get cast as port
attrs and set using swdev_attr.  This is something swdev attr should
open up is allowing more settings to be pushed down to port driver.
I'll look into this one and include it with v2.

RE: [PATCH net-next 11/18] switchdev: remove old netdev_switch_port_bridge_setlink

[Arad, Ronen]

>-----Original Message-----
>From: Scott Feldman [mailto:sfeldma@gmail.com]
>Sent: Monday, March 30, 2015 2:28 PM
>To: Arad, Ronen
>Cc: roopa; Netdev; Jirí Pírko; Guenter Roeck; Florian Fainelli
>Subject: Re: [PATCH net-next 11/18] switchdev: remove old
>netdev_switch_port_bridge_setlink
>
>On Mon, Mar 30, 2015 at 1:46 PM, Arad, Ronen <ronen.arad@intel.com> wrote:
>>
>>
>>>-----Original Message-----
>>>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>>>Behalf Of Scott Feldman
>>>Sent: Monday, March 30, 2015 1:20 PM
>>>To: roopa
>>>Cc: Netdev; Jiří Pírko; Guenter Roeck; Florian Fainelli
>>>Subject: Re: [PATCH net-next 11/18] switchdev: remove old
>>>netdev_switch_port_bridge_setlink
>>>
>>>On Mon, Mar 30, 2015 at 6:23 AM, roopa <roopa@cumulusnetworks.com> wrote:
>>>> On 3/30/15, 1:40 AM, sfeldma@gmail.com wrote:
>>>>>
>>>>> From: Scott Feldman <sfeldma@gmail.com>
>>>>>
>>>>> New attr-based bridge_setlink can recurse lower devs and recover on err,
>>>>> so
>>>>> remove old wrapper.  Also, restore br_setlink back to original and don't
>>>>> call
>>>>> into SELF port driver.  rtnetlink.c:bridge_setlink already does a call
>>>>> into
>>>>> port driver for SELF.
>>>>>
>>>>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>>>>
>>>> removing this now requires every vlan add to be a two step process, why ?
>>>
>>>No, that's not true.  You want to use
>>>ndo_vlan_rx_add_vid/ndo_vlan_rx_kill_vid in your port driver, and then
>>>using either vlan driver standalone or the bridge driver vlan support
>>>will work.
>>>
>>>Try it.  Implement ndo_vlan_rx_add_vid in your port driver and verify
>>>you get called to add VLAN to port with either:
>>>
>>>    bridge vlan add dev swp1 vid 10
>>>
>>>    -or-
>>>
>>>    vconfig add swp1 10
>>>
>>>Same for deleting a VLAN, either of these two commands call into the
>>>port driver ndo_vlan_rx_kill_vid:
>>>
>>>    bridge vlan del dev swp1 vid 10
>>>
>>>    -or-
>>>
>>>    vconfig rem swp1 10
>>>
>>>
>>>> bridge vlan add dev swp1 vid 10
>>>> bridge vlan add dev swp1 vid 10 self
>>>
>>>Not necessary.  The first command is sufficient if using
>>>ndo_vlan_rx_add_vid.
>>
>> This is not sufficient for VLAN filtering. Ndo_vlan_rx_add_vid does not
>> provide the vinfo flags PVID and UNTAGGED. Therefore it is not
>> an adequate replacement for propagating setlink/dellink messages to the
>> swithport driver or an alternative via swdev_attr.
>
>Glad you bring that point up.  I think these can get cast as port
>attrs and set using swdev_attr.  This is something swdev attr should
>open up is allowing more settings to be pushed down to port driver.
>I'll look into this one and include it with v2.

It could be beneficial to build extensibility into swdev_attr.
An experimenter attribute designed to carry arbitrary data could allow
for passing new attributes and implementation specific attributes
without affecting any existing switchdev driver:

enum swdev_attr_id {
 	SWDEV_ATTR_UNDEFINED,
	SWDEV_ATTR_EXPERIMENTER,
 	SWDEV_ATTR_PORT_PARENT_ID,
	SWDEV_ATTR_PORT_STP_STATE,
 };


struct swdev_experimenter_attr {
	u32 exp_id; 	/* - MSB 0: low-order bytes are IEEE OUI */
			/* - MSB != 0: reserved for netdev */
	u32 exp_attr;
	u16 exp_attr_size;
	const void *exp_attr_data;
}

struct swdev_attr {
	enum swdev_attr_id attr;
	u32 flags;

 	union {
 		struct netdev_phys_item_id ppid;	/* PORT_PARENT_ID */
 		u8 stp_state;				/* PORT_STP_STATE */
		unsigned long brport_flags;	/* PORT_BRIDGE_FLAGS */
		/* netdev defined attributes abobe this line */
		struct swdev_experimenter_attr exp_attr;
 	};

Re: [PATCH net-next 11/18] switchdev: remove old netdev_switch_port_bridge_setlink

[Scott Feldman]

On Mon, Mar 30, 2015 at 5:08 PM, Arad, Ronen <ronen.arad@intel.com> wrote:
>
>
>>-----Original Message-----
>>From: Scott Feldman [mailto:sfeldma@gmail.com]
>>Sent: Monday, March 30, 2015 2:28 PM
>>To: Arad, Ronen
>>Cc: roopa; Netdev; Jirí Pírko; Guenter Roeck; Florian Fainelli
>>Subject: Re: [PATCH net-next 11/18] switchdev: remove old
>>netdev_switch_port_bridge_setlink
>>
>>On Mon, Mar 30, 2015 at 1:46 PM, Arad, Ronen <ronen.arad@intel.com> wrote:
>>>
>>>
>>>>-----Original Message-----
>>>>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>>>>Behalf Of Scott Feldman
>>>>Sent: Monday, March 30, 2015 1:20 PM
>>>>To: roopa
>>>>Cc: Netdev; Jiří Pírko; Guenter Roeck; Florian Fainelli
>>>>Subject: Re: [PATCH net-next 11/18] switchdev: remove old
>>>>netdev_switch_port_bridge_setlink
>>>>
>>>>On Mon, Mar 30, 2015 at 6:23 AM, roopa <roopa@cumulusnetworks.com> wrote:
>>>>> On 3/30/15, 1:40 AM, sfeldma@gmail.com wrote:
>>>>>>
>>>>>> From: Scott Feldman <sfeldma@gmail.com>
>>>>>>
>>>>>> New attr-based bridge_setlink can recurse lower devs and recover on err,
>>>>>> so
>>>>>> remove old wrapper.  Also, restore br_setlink back to original and don't
>>>>>> call
>>>>>> into SELF port driver.  rtnetlink.c:bridge_setlink already does a call
>>>>>> into
>>>>>> port driver for SELF.
>>>>>>
>>>>>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>>>>>
>>>>> removing this now requires every vlan add to be a two step process, why ?
>>>>
>>>>No, that's not true.  You want to use
>>>>ndo_vlan_rx_add_vid/ndo_vlan_rx_kill_vid in your port driver, and then
>>>>using either vlan driver standalone or the bridge driver vlan support
>>>>will work.
>>>>
>>>>Try it.  Implement ndo_vlan_rx_add_vid in your port driver and verify
>>>>you get called to add VLAN to port with either:
>>>>
>>>>    bridge vlan add dev swp1 vid 10
>>>>
>>>>    -or-
>>>>
>>>>    vconfig add swp1 10
>>>>
>>>>Same for deleting a VLAN, either of these two commands call into the
>>>>port driver ndo_vlan_rx_kill_vid:
>>>>
>>>>    bridge vlan del dev swp1 vid 10
>>>>
>>>>    -or-
>>>>
>>>>    vconfig rem swp1 10
>>>>
>>>>
>>>>> bridge vlan add dev swp1 vid 10
>>>>> bridge vlan add dev swp1 vid 10 self
>>>>
>>>>Not necessary.  The first command is sufficient if using
>>>>ndo_vlan_rx_add_vid.
>>>
>>> This is not sufficient for VLAN filtering. Ndo_vlan_rx_add_vid does not
>>> provide the vinfo flags PVID and UNTAGGED. Therefore it is not
>>> an adequate replacement for propagating setlink/dellink messages to the
>>> swithport driver or an alternative via swdev_attr.
>>
>>Glad you bring that point up.  I think these can get cast as port
>>attrs and set using swdev_attr.  This is something swdev attr should
>>open up is allowing more settings to be pushed down to port driver.
>>I'll look into this one and include it with v2.
>
> It could be beneficial to build extensibility into swdev_attr.
> An experimenter attribute designed to carry arbitrary data could allow
> for passing new attributes and implementation specific attributes
> without affecting any existing switchdev driver:
>
> enum swdev_attr_id {
>         SWDEV_ATTR_UNDEFINED,
>         SWDEV_ATTR_EXPERIMENTER,
>         SWDEV_ATTR_PORT_PARENT_ID,
>         SWDEV_ATTR_PORT_STP_STATE,
>  };
>
>
> struct swdev_experimenter_attr {
>         u32 exp_id;     /* - MSB 0: low-order bytes are IEEE OUI */
>                         /* - MSB != 0: reserved for netdev */
>         u32 exp_attr;
>         u16 exp_attr_size;
>         const void *exp_attr_data;
> }
>
> struct swdev_attr {
>         enum swdev_attr_id attr;
>         u32 flags;
>
>         union {
>                 struct netdev_phys_item_id ppid;        /* PORT_PARENT_ID */
>                 u8 stp_state;                           /* PORT_STP_STATE */
>                 unsigned long brport_flags;     /* PORT_BRIDGE_FLAGS */
>                 /* netdev defined attributes abobe this line */
>                 struct swdev_experimenter_attr exp_attr;
>         };

I'm scared of void *'s.  How would this be used?  Maybe an example to
illustrate the use-case would help.

Re: [PATCH net-next 11/18] switchdev: remove old netdev_switch_port_bridge_setlink

[Jiri Pirko]

Tue, Mar 31, 2015 at 02:08:34AM CEST, ronen.arad@intel.com wrote:
>
>
>>-----Original Message-----
>>From: Scott Feldman [mailto:sfeldma@gmail.com]
>>Sent: Monday, March 30, 2015 2:28 PM
>>To: Arad, Ronen
>>Cc: roopa; Netdev; Jirí Pírko; Guenter Roeck; Florian Fainelli
>>Subject: Re: [PATCH net-next 11/18] switchdev: remove old
>>netdev_switch_port_bridge_setlink
>>
>>On Mon, Mar 30, 2015 at 1:46 PM, Arad, Ronen <ronen.arad@intel.com> wrote:
>>>
>>>
>>>>-----Original Message-----
>>>>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>>>>Behalf Of Scott Feldman
>>>>Sent: Monday, March 30, 2015 1:20 PM
>>>>To: roopa
>>>>Cc: Netdev; Jiří Pírko; Guenter Roeck; Florian Fainelli
>>>>Subject: Re: [PATCH net-next 11/18] switchdev: remove old
>>>>netdev_switch_port_bridge_setlink
>>>>
>>>>On Mon, Mar 30, 2015 at 6:23 AM, roopa <roopa@cumulusnetworks.com> wrote:
>>>>> On 3/30/15, 1:40 AM, sfeldma@gmail.com wrote:
>>>>>>
>>>>>> From: Scott Feldman <sfeldma@gmail.com>
>>>>>>
>>>>>> New attr-based bridge_setlink can recurse lower devs and recover on err,
>>>>>> so
>>>>>> remove old wrapper.  Also, restore br_setlink back to original and don't
>>>>>> call
>>>>>> into SELF port driver.  rtnetlink.c:bridge_setlink already does a call
>>>>>> into
>>>>>> port driver for SELF.
>>>>>>
>>>>>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>>>>>
>>>>> removing this now requires every vlan add to be a two step process, why ?
>>>>
>>>>No, that's not true.  You want to use
>>>>ndo_vlan_rx_add_vid/ndo_vlan_rx_kill_vid in your port driver, and then
>>>>using either vlan driver standalone or the bridge driver vlan support
>>>>will work.
>>>>
>>>>Try it.  Implement ndo_vlan_rx_add_vid in your port driver and verify
>>>>you get called to add VLAN to port with either:
>>>>
>>>>    bridge vlan add dev swp1 vid 10
>>>>
>>>>    -or-
>>>>
>>>>    vconfig add swp1 10
>>>>
>>>>Same for deleting a VLAN, either of these two commands call into the
>>>>port driver ndo_vlan_rx_kill_vid:
>>>>
>>>>    bridge vlan del dev swp1 vid 10
>>>>
>>>>    -or-
>>>>
>>>>    vconfig rem swp1 10
>>>>
>>>>
>>>>> bridge vlan add dev swp1 vid 10
>>>>> bridge vlan add dev swp1 vid 10 self
>>>>
>>>>Not necessary.  The first command is sufficient if using
>>>>ndo_vlan_rx_add_vid.
>>>
>>> This is not sufficient for VLAN filtering. Ndo_vlan_rx_add_vid does not
>>> provide the vinfo flags PVID and UNTAGGED. Therefore it is not
>>> an adequate replacement for propagating setlink/dellink messages to the
>>> swithport driver or an alternative via swdev_attr.
>>
>>Glad you bring that point up.  I think these can get cast as port
>>attrs and set using swdev_attr.  This is something swdev attr should
>>open up is allowing more settings to be pushed down to port driver.
>>I'll look into this one and include it with v2.
>
>It could be beneficial to build extensibility into swdev_attr.
>An experimenter attribute designed to carry arbitrary data could allow
>for passing new attributes and implementation specific attributes
>without affecting any existing switchdev driver:

Warning sign...

I believe that we don't want this. It is very easy to add attribute for
anything. Having this "universal attribuute" only allows wild things...

Thanks.

Jiri

>
>enum swdev_attr_id {
> 	SWDEV_ATTR_UNDEFINED,
>	SWDEV_ATTR_EXPERIMENTER,
> 	SWDEV_ATTR_PORT_PARENT_ID,
>	SWDEV_ATTR_PORT_STP_STATE,
> };
>
>
>struct swdev_experimenter_attr {
>	u32 exp_id; 	/* - MSB 0: low-order bytes are IEEE OUI */
>			/* - MSB != 0: reserved for netdev */
>	u32 exp_attr;
>	u16 exp_attr_size;
>	const void *exp_attr_data;
>}
>
>struct swdev_attr {
>	enum swdev_attr_id attr;
>	u32 flags;
>
> 	union {
> 		struct netdev_phys_item_id ppid;	/* PORT_PARENT_ID */
> 		u8 stp_state;				/* PORT_STP_STATE */
>		unsigned long brport_flags;	/* PORT_BRIDGE_FLAGS */
>		/* netdev defined attributes abobe this line */
>		struct swdev_experimenter_attr exp_attr;
> 	};

RE: [PATCH net-next 11/18] switchdev: remove old netdev_switch_port_bridge_setlink

[Arad, Ronen]

>-----Original Message-----
>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>Behalf Of Jiri Pirko
>Sent: Monday, March 30, 2015 10:53 PM
>To: Arad, Ronen
>Cc: Scott Feldman; Netdev; roopa; Guenter Roeck; Florian Fainelli
>Subject: Re: [PATCH net-next 11/18] switchdev: remove old
>netdev_switch_port_bridge_setlink
>
>Tue, Mar 31, 2015 at 02:08:34AM CEST, ronen.arad@intel.com wrote:
>>
>>
>>>-----Original Message-----
>>>From: Scott Feldman [mailto:sfeldma@gmail.com]
>>>Sent: Monday, March 30, 2015 2:28 PM
>>>To: Arad, Ronen
>>>Cc: roopa; Netdev; Jirí Pírko; Guenter Roeck; Florian Fainelli
>>>Subject: Re: [PATCH net-next 11/18] switchdev: remove old
>>>netdev_switch_port_bridge_setlink
>>>
>>>On Mon, Mar 30, 2015 at 1:46 PM, Arad, Ronen <ronen.arad@intel.com> wrote:
>>>>
>>>>
>>>>>-----Original Message-----
>>>>>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
>On
>>>>>Behalf Of Scott Feldman
>>>>>Sent: Monday, March 30, 2015 1:20 PM
>>>>>To: roopa
>>>>>Cc: Netdev; Jiří Pírko; Guenter Roeck; Florian Fainelli
>>>>>Subject: Re: [PATCH net-next 11/18] switchdev: remove old
>>>>>netdev_switch_port_bridge_setlink
>>>>>
>>>>>On Mon, Mar 30, 2015 at 6:23 AM, roopa <roopa@cumulusnetworks.com> wrote:
>>>>>> On 3/30/15, 1:40 AM, sfeldma@gmail.com wrote:
>>>>>>>
>>>>>>> From: Scott Feldman <sfeldma@gmail.com>
>>>>>>>
>>>>>>> New attr-based bridge_setlink can recurse lower devs and recover on
>err,
>>>>>>> so
>>>>>>> remove old wrapper.  Also, restore br_setlink back to original and
>don't
>>>>>>> call
>>>>>>> into SELF port driver.  rtnetlink.c:bridge_setlink already does a call
>>>>>>> into
>>>>>>> port driver for SELF.
>>>>>>>
>>>>>>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>>>>>>
>>>>>> removing this now requires every vlan add to be a two step process, why
>?
>>>>>
>>>>>No, that's not true.  You want to use
>>>>>ndo_vlan_rx_add_vid/ndo_vlan_rx_kill_vid in your port driver, and then
>>>>>using either vlan driver standalone or the bridge driver vlan support
>>>>>will work.
>>>>>
>>>>>Try it.  Implement ndo_vlan_rx_add_vid in your port driver and verify
>>>>>you get called to add VLAN to port with either:
>>>>>
>>>>>    bridge vlan add dev swp1 vid 10
>>>>>
>>>>>    -or-
>>>>>
>>>>>    vconfig add swp1 10
>>>>>
>>>>>Same for deleting a VLAN, either of these two commands call into the
>>>>>port driver ndo_vlan_rx_kill_vid:
>>>>>
>>>>>    bridge vlan del dev swp1 vid 10
>>>>>
>>>>>    -or-
>>>>>
>>>>>    vconfig rem swp1 10
>>>>>
>>>>>
>>>>>> bridge vlan add dev swp1 vid 10
>>>>>> bridge vlan add dev swp1 vid 10 self
>>>>>
>>>>>Not necessary.  The first command is sufficient if using
>>>>>ndo_vlan_rx_add_vid.
>>>>
>>>> This is not sufficient for VLAN filtering. Ndo_vlan_rx_add_vid does not
>>>> provide the vinfo flags PVID and UNTAGGED. Therefore it is not
>>>> an adequate replacement for propagating setlink/dellink messages to the
>>>> swithport driver or an alternative via swdev_attr.
>>>
>>>Glad you bring that point up.  I think these can get cast as port
>>>attrs and set using swdev_attr.  This is something swdev attr should
>>>open up is allowing more settings to be pushed down to port driver.
>>>I'll look into this one and include it with v2.
>>
>>It could be beneficial to build extensibility into swdev_attr.
>>An experimenter attribute designed to carry arbitrary data could allow
>>for passing new attributes and implementation specific attributes
>>without affecting any existing switchdev driver:
>
>Warning sign...
>
>I believe that we don't want this. It is very easy to add attribute for
>anything. Having this "universal attribuute" only allows wild things...
>
>Thanks.
>
>Jiri
>
Let's say a switch device has some behavior that is not common to all
switch devices. Defining an explicit attribute might not be desirable
in that case. For example let's say SOMEswitch device implements VLAN
priority markdown using a table. It could be represented as a table of
8 bytes. To support this feature, there is a need to allow a user-space
tool to program such table in SOMEswitch device. What mechanisms are 
available for that?
This could be done via sysfs entries. However, I believe we're trying
to make Netlink and iproute2 the protocol/tool for all switch device
configuration. Therefore, I think that a generic mechanism, integrated
with existing switchdev supported tools would be best. 
Alternative approach would be for SOMEswitch driver to introduce its
own generic netlink family and introduce SOMEswitch version of iproute2
or similar tool to augment SOMEswitch HW configuration where (or as long
as) no common way to control such feature is available


>>
>>enum swdev_attr_id {
>> 	SWDEV_ATTR_UNDEFINED,
>>	SWDEV_ATTR_EXPERIMENTER,
>> 	SWDEV_ATTR_PORT_PARENT_ID,
>>	SWDEV_ATTR_PORT_STP_STATE,
>> };
>>
>>
>>struct swdev_experimenter_attr {
>>	u32 exp_id; 	/* - MSB 0: low-order bytes are IEEE OUI */
>>			/* - MSB != 0: reserved for netdev */
>>	u32 exp_attr;
>>	u16 exp_attr_size;
>>	const void *exp_attr_data;
>>}
>>
>>struct swdev_attr {
>>	enum swdev_attr_id attr;
>>	u32 flags;
>>
>> 	union {
>> 		struct netdev_phys_item_id ppid;	/* PORT_PARENT_ID */
>> 		u8 stp_state;				/* PORT_STP_STATE */
>>		unsigned long brport_flags;	/* PORT_BRIDGE_FLAGS */
>>		/* netdev defined attributes abobe this line */
>>		struct swdev_experimenter_attr exp_attr;
>> 	};
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next 11/18] switchdev: remove old netdev_switch_port_bridge_setlink

[Jiri Pirko]

Tue, Mar 31, 2015 at 09:15:35PM CEST, ronen.arad@intel.com wrote:
>
>
>>-----Original Message-----
>>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>>Behalf Of Jiri Pirko
>>Sent: Monday, March 30, 2015 10:53 PM
>>To: Arad, Ronen
>>Cc: Scott Feldman; Netdev; roopa; Guenter Roeck; Florian Fainelli
>>Subject: Re: [PATCH net-next 11/18] switchdev: remove old
>>netdev_switch_port_bridge_setlink
>>
>>Tue, Mar 31, 2015 at 02:08:34AM CEST, ronen.arad@intel.com wrote:
>>>
>>>
>>>>-----Original Message-----
>>>>From: Scott Feldman [mailto:sfeldma@gmail.com]
>>>>Sent: Monday, March 30, 2015 2:28 PM
>>>>To: Arad, Ronen
>>>>Cc: roopa; Netdev; Jirí Pírko; Guenter Roeck; Florian Fainelli
>>>>Subject: Re: [PATCH net-next 11/18] switchdev: remove old
>>>>netdev_switch_port_bridge_setlink
>>>>
>>>>On Mon, Mar 30, 2015 at 1:46 PM, Arad, Ronen <ronen.arad@intel.com> wrote:
>>>>>
>>>>>
>>>>>>-----Original Message-----
>>>>>>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
>>On
>>>>>>Behalf Of Scott Feldman
>>>>>>Sent: Monday, March 30, 2015 1:20 PM
>>>>>>To: roopa
>>>>>>Cc: Netdev; Jiří Pírko; Guenter Roeck; Florian Fainelli
>>>>>>Subject: Re: [PATCH net-next 11/18] switchdev: remove old
>>>>>>netdev_switch_port_bridge_setlink
>>>>>>
>>>>>>On Mon, Mar 30, 2015 at 6:23 AM, roopa <roopa@cumulusnetworks.com> wrote:
>>>>>>> On 3/30/15, 1:40 AM, sfeldma@gmail.com wrote:
>>>>>>>>
>>>>>>>> From: Scott Feldman <sfeldma@gmail.com>
>>>>>>>>
>>>>>>>> New attr-based bridge_setlink can recurse lower devs and recover on
>>err,
>>>>>>>> so
>>>>>>>> remove old wrapper.  Also, restore br_setlink back to original and
>>don't
>>>>>>>> call
>>>>>>>> into SELF port driver.  rtnetlink.c:bridge_setlink already does a call
>>>>>>>> into
>>>>>>>> port driver for SELF.
>>>>>>>>
>>>>>>>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>>>>>>>
>>>>>>> removing this now requires every vlan add to be a two step process, why
>>?
>>>>>>
>>>>>>No, that's not true.  You want to use
>>>>>>ndo_vlan_rx_add_vid/ndo_vlan_rx_kill_vid in your port driver, and then
>>>>>>using either vlan driver standalone or the bridge driver vlan support
>>>>>>will work.
>>>>>>
>>>>>>Try it.  Implement ndo_vlan_rx_add_vid in your port driver and verify
>>>>>>you get called to add VLAN to port with either:
>>>>>>
>>>>>>    bridge vlan add dev swp1 vid 10
>>>>>>
>>>>>>    -or-
>>>>>>
>>>>>>    vconfig add swp1 10
>>>>>>
>>>>>>Same for deleting a VLAN, either of these two commands call into the
>>>>>>port driver ndo_vlan_rx_kill_vid:
>>>>>>
>>>>>>    bridge vlan del dev swp1 vid 10
>>>>>>
>>>>>>    -or-
>>>>>>
>>>>>>    vconfig rem swp1 10
>>>>>>
>>>>>>
>>>>>>> bridge vlan add dev swp1 vid 10
>>>>>>> bridge vlan add dev swp1 vid 10 self
>>>>>>
>>>>>>Not necessary.  The first command is sufficient if using
>>>>>>ndo_vlan_rx_add_vid.
>>>>>
>>>>> This is not sufficient for VLAN filtering. Ndo_vlan_rx_add_vid does not
>>>>> provide the vinfo flags PVID and UNTAGGED. Therefore it is not
>>>>> an adequate replacement for propagating setlink/dellink messages to the
>>>>> swithport driver or an alternative via swdev_attr.
>>>>
>>>>Glad you bring that point up.  I think these can get cast as port
>>>>attrs and set using swdev_attr.  This is something swdev attr should
>>>>open up is allowing more settings to be pushed down to port driver.
>>>>I'll look into this one and include it with v2.
>>>
>>>It could be beneficial to build extensibility into swdev_attr.
>>>An experimenter attribute designed to carry arbitrary data could allow
>>>for passing new attributes and implementation specific attributes
>>>without affecting any existing switchdev driver:
>>
>>Warning sign...
>>
>>I believe that we don't want this. It is very easy to add attribute for
>>anything. Having this "universal attribuute" only allows wild things...
>>
>>Thanks.
>>
>>Jiri
>>
>Let's say a switch device has some behavior that is not common to all
>switch devices. Defining an explicit attribute might not be desirable
>in that case. For example let's say SOMEswitch device implements VLAN
>priority markdown using a table. It could be represented as a table of
>8 bytes. To support this feature, there is a need to allow a user-space
>tool to program such table in SOMEswitch device. What mechanisms are 
>available for that?

If you want to expose feature X that's ok, just do it. I see no problem
in that. Please, just do not introduce kernel bypass. That simply won't
fly...


>This could be done via sysfs entries. However, I believe we're trying
>to make Netlink and iproute2 the protocol/tool for all switch device

It does not matter if universal attr is exposed via netlink or sysfs.
Either way, I believe it is unacceptable.

>configuration. Therefore, I think that a generic mechanism, integrated
>with existing switchdev supported tools would be best. 
>Alternative approach would be for SOMEswitch driver to introduce its
>own generic netlink family and introduce SOMEswitch version of iproute2
>or similar tool to augment SOMEswitch HW configuration where (or as long
>as) no common way to control such feature is available

This is definitelly unacceptable :/

RE: [PATCH net-next 11/18] switchdev: remove old netdev_switch_port_bridge_setlink

[Arad, Ronen]

>-----Original Message-----
>From: Jiri Pirko [mailto:jiri@resnulli.us]
>Sent: Tuesday, March 31, 2015 2:53 PM
>To: Arad, Ronen
>Cc: Scott Feldman; Netdev; roopa; Guenter Roeck; Florian Fainelli
>Subject: Re: [PATCH net-next 11/18] switchdev: remove old
>netdev_switch_port_bridge_setlink
>
>Tue, Mar 31, 2015 at 09:15:35PM CEST, ronen.arad@intel.com wrote:
>>
>>
>>>-----Original Message-----
>>>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>>>Behalf Of Jiri Pirko
>>>Sent: Monday, March 30, 2015 10:53 PM
>>>To: Arad, Ronen
>>>Cc: Scott Feldman; Netdev; roopa; Guenter Roeck; Florian Fainelli
>>>Subject: Re: [PATCH net-next 11/18] switchdev: remove old
>>>netdev_switch_port_bridge_setlink
>>>
>>>Tue, Mar 31, 2015 at 02:08:34AM CEST, ronen.arad@intel.com wrote:
>>>>
[cut]
>>>>
>>>>It could be beneficial to build extensibility into swdev_attr.
>>>>An experimenter attribute designed to carry arbitrary data could allow
>>>>for passing new attributes and implementation specific attributes
>>>>without affecting any existing switchdev driver:
>>>
>>>Warning sign...
>>>
>>>I believe that we don't want this. It is very easy to add attribute for
>>>anything. Having this "universal attribuute" only allows wild things...
>>>
>>>Thanks.
>>>
>>>Jiri
>>>
>>Let's say a switch device has some behavior that is not common to all
>>switch devices. Defining an explicit attribute might not be desirable
>>in that case. For example let's say SOMEswitch device implements VLAN
>>priority markdown using a table. It could be represented as a table of
>>8 bytes. To support this feature, there is a need to allow a user-space
>>tool to program such table in SOMEswitch device. What mechanisms are
>>available for that?
>
>If you want to expose feature X that's ok, just do it. I see no problem
>in that. Please, just do not introduce kernel bypass.

This is not a kernel bypass as long as the target device is an in-tree
device driver open for the community scrutiny. 
 
 That simply won't
>fly...

Let's say vendor X wants to expose features X1, X2, ...Xm,
vendor Y wants to expose features Y1, Y2, ...Yn, etc.
How this would be visible to the users via iproute2?
We'll need keywords Kx1..Kxm, Ky1..Kn, etc. There is scalability issue here.
The user has to figure out the keyword Kxj goes with feature Xj etc.
There must be a better way.

What I have in mind is a way for switchdevX driver to define the its options
and the keywords that the user could use to control them in a generic way.
The kernel will provide the mechanism to export that to user space such that
A tool like iproute2 could present user-friendly interface which is context
aware. 

It is important to allow for a generic interface that will not require a new
patch to the user-space tool or to the kernel net core (or switchdev) infrastructure, whenever any such new feature is being exposed.
Only the switchdev driver which exposes a new feature need a patch.

The kernel could have sufficient information about the information that
switchdevX is expecting such that it could validate size and type and could
reject information that is not supported by the device.

This is very similar to the options interface the team driver exposes.
Team driver provides the mechanism for user-space to configure options in the
kernel. Team driver does not understand or interpret the options that are
intended for team modes. It only provides the mechanism to communicate them
over generic netlink and process them in the kernel and user-space.
Those options do not show up in any header file. You don't need a new kernel
version to introduce a new team option for a new or existing team mode.
All you need is to register the option (identified by a string) with team
driver and set/get it from the teamd or any other user-space tool over
generic netlink.
The team protocol is stable and does not require changes for this level of
extensibility.

The same level of extensibility is needed for switchdev port or device
attributes.
   
>
>
>>This could be done via sysfs entries. However, I believe we're trying
>>to make Netlink and iproute2 the protocol/tool for all switch device
>
>It does not matter if universal attr is exposed via netlink or sysfs.
>Either way, I believe it is unacceptable.
>
>>configuration. Therefore, I think that a generic mechanism, integrated
>>with existing switchdev supported tools would be best.
>>Alternative approach would be for SOMEswitch driver to introduce its
>>own generic netlink family and introduce SOMEswitch version of iproute2
>>or similar tool to augment SOMEswitch HW configuration where (or as long
>>as) no common way to control such feature is available
>
>This is definitelly unacceptable :/

Re: [PATCH net-next 11/18] switchdev: remove old netdev_switch_port_bridge_setlink

[Scott Feldman]

On Tue, Mar 31, 2015 at 4:32 PM, Arad, Ronen <ronen.arad@intel.com> wrote:
>
>
>>-----Original Message-----
>>From: Jiri Pirko [mailto:jiri@resnulli.us]
>>Sent: Tuesday, March 31, 2015 2:53 PM
>>To: Arad, Ronen
>>Cc: Scott Feldman; Netdev; roopa; Guenter Roeck; Florian Fainelli
>>Subject: Re: [PATCH net-next 11/18] switchdev: remove old
>>netdev_switch_port_bridge_setlink
>>
>>Tue, Mar 31, 2015 at 09:15:35PM CEST, ronen.arad@intel.com wrote:
>>>
>>>
>>>>-----Original Message-----
>>>>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>>>>Behalf Of Jiri Pirko
>>>>Sent: Monday, March 30, 2015 10:53 PM
>>>>To: Arad, Ronen
>>>>Cc: Scott Feldman; Netdev; roopa; Guenter Roeck; Florian Fainelli
>>>>Subject: Re: [PATCH net-next 11/18] switchdev: remove old
>>>>netdev_switch_port_bridge_setlink
>>>>
>>>>Tue, Mar 31, 2015 at 02:08:34AM CEST, ronen.arad@intel.com wrote:
>>>>>
> [cut]
>>>>>
>>>>>It could be beneficial to build extensibility into swdev_attr.
>>>>>An experimenter attribute designed to carry arbitrary data could allow
>>>>>for passing new attributes and implementation specific attributes
>>>>>without affecting any existing switchdev driver:
>>>>
>>>>Warning sign...
>>>>
>>>>I believe that we don't want this. It is very easy to add attribute for
>>>>anything. Having this "universal attribuute" only allows wild things...
>>>>
>>>>Thanks.
>>>>
>>>>Jiri
>>>>
>>>Let's say a switch device has some behavior that is not common to all
>>>switch devices. Defining an explicit attribute might not be desirable
>>>in that case. For example let's say SOMEswitch device implements VLAN
>>>priority markdown using a table. It could be represented as a table of
>>>8 bytes. To support this feature, there is a need to allow a user-space
>>>tool to program such table in SOMEswitch device. What mechanisms are
>>>available for that?
>>
>>If you want to expose feature X that's ok, just do it. I see no problem
>>in that. Please, just do not introduce kernel bypass.
>
> This is not a kernel bypass as long as the target device is an in-tree
> device driver open for the community scrutiny.
>
>  That simply won't
>>fly...
>
> Let's say vendor X wants to expose features X1, X2, ...Xm,
> vendor Y wants to expose features Y1, Y2, ...Yn, etc.
> How this would be visible to the users via iproute2?
> We'll need keywords Kx1..Kxm, Ky1..Kn, etc. There is scalability issue here.
> The user has to figure out the keyword Kxj goes with feature Xj etc.
> There must be a better way.
>
> What I have in mind is a way for switchdevX driver to define the its options
> and the keywords that the user could use to control them in a generic way.
> The kernel will provide the mechanism to export that to user space such that
> A tool like iproute2 could present user-friendly interface which is context
> aware.
>
> It is important to allow for a generic interface that will not require a new
> patch to the user-space tool or to the kernel net core (or switchdev) infrastructure, whenever any such new feature is being exposed.
> Only the switchdev driver which exposes a new feature need a patch.
>
> The kernel could have sufficient information about the information that
> switchdevX is expecting such that it could validate size and type and could
> reject information that is not supported by the device.
>
> This is very similar to the options interface the team driver exposes.
> Team driver provides the mechanism for user-space to configure options in the
> kernel. Team driver does not understand or interpret the options that are
> intended for team modes. It only provides the mechanism to communicate them
> over generic netlink and process them in the kernel and user-space.
> Those options do not show up in any header file. You don't need a new kernel
> version to introduce a new team option for a new or existing team mode.
> All you need is to register the option (identified by a string) with team
> driver and set/get it from the teamd or any other user-space tool over
> generic netlink.
> The team protocol is stable and does not require changes for this level of
> extensibility.
>
> The same level of extensibility is needed for switchdev port or device
> attributes.

It sounds like vendor extensions need to be supported someway,
someday.  Maybe let's table vendor extensions for the time being and
continue our focus on shared infrastructure: the things we know are
common for all vendors.  We still have a lot of work just getting the
common bits working.  Then we can take additional features one at a
time and see if they're shared or vendor-specific.  In the meantime,
there are always back doors such as genl for vendor to use; there is
nothing preventing that.

-scott

Re: [PATCH net-next 11/18] switchdev: remove old netdev_switch_port_bridge_setlink

[Jamal Hadi Salim]

On 03/31/15 22:38, Scott Feldman wrote:

>
> It sounds like vendor extensions need to be supported someway,
> someday.


I agree. This is one of those impendance mismatch things. There are
scenarios where the feature may not be generic enough. But whatever
feature it is - eventually we should get it back into mainstream.
The danger of backdoors that Jiri alludes to is there.


> Maybe let's table vendor extensions for the time being and
> continue our focus on shared infrastructure: the things we know are
> common for all vendors.  We still have a lot of work just getting the
> common bits working.  Then we can take additional features one at a
> time and see if they're shared or vendor-specific.  In the meantime,
> there are always back doors such as genl for vendor to use; there is
> nothing preventing that.
>

And now i disagree. Did you read the back of your netdev tshirt? Quote:
---
I also don't buy the argument that "people can put arbitrary changes
into their kernel to do stuff like that".
---

Vendors can do whatever they want. We just should not be aiding them to
put proprietary shit. I want to stop coding around SDKs. Help me.

cheers,
jamal

Re: [PATCH net-next 11/18] switchdev: remove old netdev_switch_port_bridge_setlink

[Scott Feldman]

On Wed, Apr 1, 2015 at 5:03 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> On 03/31/15 22:38, Scott Feldman wrote:
>
>>
>> It sounds like vendor extensions need to be supported someway,
>> someday.
>
>
>
> I agree. This is one of those impendance mismatch things. There are
> scenarios where the feature may not be generic enough. But whatever
> feature it is - eventually we should get it back into mainstream.
> The danger of backdoors that Jiri alludes to is there.
>
>
>> Maybe let's table vendor extensions for the time being and
>> continue our focus on shared infrastructure: the things we know are
>> common for all vendors.  We still have a lot of work just getting the
>> common bits working.  Then we can take additional features one at a
>> time and see if they're shared or vendor-specific.  In the meantime,
>> there are always back doors such as genl for vendor to use; there is
>> nothing preventing that.
>>
>
> And now i disagree. Did you read the back of your netdev tshirt? Quote:
> ---
> I also don't buy the argument that "people can put arbitrary changes
> into their kernel to do stuff like that".
> ---
>
> Vendors can do whatever they want. We just should not be aiding them to
> put proprietary shit. I want to stop coding around SDKs. Help me.

I'm not sure what we're disagreeing on...I thought I said the same
thing.  I just want to defer the vendor extension discussion for now
and refocus on common stuff.  Trust me, there is no one on the planet
that wants to stop coding around the SDKs more than myself.

-scott

Re: [PATCH net-next 11/18] switchdev: remove old netdev_switch_port_bridge_setlink

[Florian Fainelli]

2015-03-31 16:32 GMT-07:00 Arad, Ronen <ronen.arad@intel.com>:
> Let's say vendor X wants to expose features X1, X2, ...Xm,
> vendor Y wants to expose features Y1, Y2, ...Yn, etc.
> How this would be visible to the users via iproute2?
> We'll need keywords Kx1..Kxm, Ky1..Kn, etc. There is scalability issue here.
> The user has to figure out the keyword Kxj goes with feature Xj etc.
> There must be a better way.
>
> What I have in mind is a way for switchdevX driver to define the its options
> and the keywords that the user could use to control them in a generic way.
> The kernel will provide the mechanism to export that to user space such that
> A tool like iproute2 could present user-friendly interface which is context
> aware.
>
> It is important to allow for a generic interface that will not require a new
> patch to the user-space tool or to the kernel net core (or switchdev) infrastructure, whenever any such new feature is being exposed.
> Only the switchdev driver which exposes a new feature need a patch.

You are essentially proposing to do the same thing as OpenWrt's
swconfig is doing [1]: each driver exposes both "generic" (to some
degree) and custom attributes, which are discovered by user-space in a
generic way. I think this is part of what do not want here, because
that leaves room for vendors to implement feature XYZ, using the
kernel as a programming interface, but without making the in-kernel
API and code any consistent, hence failing part of its abstraction
role.

Things like this have existed for a long time, like NL80211 testmode,
or wext private ioctl()s, etc.. etc.. I don't think that leads to
better APIs being defined unfortunately.

[1]: https://lwn.net/Articles/571390/
-- 
Florian

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

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

There are no port driver using bridge_dellink, so remove these swdev wrappers.
Something was fishy here anyway because dellink isn't used for deleting port
attributes (not sure how you delete a port attribute?).  VLAN deletes are
already handled via ndo_vlan_rx_add_vid/ndo_vlan_rx_kill_vid.

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

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 99ab282..9d823b6 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4037,7 +4037,6 @@ 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_features_check	= passthru_features_check,
 };
 
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 6a3debc..101b341 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1978,7 +1978,6 @@ 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_features_check	= passthru_features_check,
 };
 
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index bad2ec7..97196ab 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -92,10 +92,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,
@@ -138,20 +134,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 0307a45..8a07096 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -258,61 +258,6 @@ int swdev_port_bridge_setlink(struct net_device *dev,
 }
 EXPORT_SYMBOL_GPL(swdev_port_bridge_setlink);
 
-/**
- *	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

Re: [PATCH net-next 12/18] switchdev: remove unused netdev_switch_port_bridge_dellink

[roopa]

On 3/30/15, 1:40 AM, sfeldma@gmail.com wrote:
> From: Scott Feldman <sfeldma@gmail.com>
>
> There are no port driver using bridge_dellink, so remove these swdev wrappers.
> Something was fishy here anyway because dellink isn't used for deleting port
> attributes (not sure how you delete a port attribute?).  VLAN deletes are
> already handled via ndo_vlan_rx_add_vid/ndo_vlan_rx_kill_vid.
>
> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
dellink is used for vlan deletes in the new bridge vlan filtering bridge.

> ---
>   drivers/net/bonding/bond_main.c |    1 -
>   drivers/net/team/team.c         |    1 -
>   include/net/switchdev.h         |   18 -------------
>   net/bridge/br_netlink.c         |   12 +--------
>   net/switchdev/switchdev.c       |   55 ---------------------------------------
>   5 files changed, 1 insertion(+), 86 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 99ab282..9d823b6 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -4037,7 +4037,6 @@ 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_features_check	= passthru_features_check,
>   };
>   
> diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
> index 6a3debc..101b341 100644
> --- a/drivers/net/team/team.c
> +++ b/drivers/net/team/team.c
> @@ -1978,7 +1978,6 @@ 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_features_check	= passthru_features_check,
>   };
>   
> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
> index bad2ec7..97196ab 100644
> --- a/include/net/switchdev.h
> +++ b/include/net/switchdev.h
> @@ -92,10 +92,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,
> @@ -138,20 +134,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 0307a45..8a07096 100644
> --- a/net/switchdev/switchdev.c
> +++ b/net/switchdev/switchdev.c
> @@ -258,61 +258,6 @@ int swdev_port_bridge_setlink(struct net_device *dev,
>   }
>   EXPORT_SYMBOL_GPL(swdev_port_bridge_setlink);
>   
> -/**
> - *	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;

[PATCH net-next 13/18] switchdev: remove unused NETIF_F_HW_SWITCH_OFFLOAD

[sfeldma]

From: Scott Feldman <sfeldma@gmail.com>

Flag is no longer needed so remove it.  Using the attr get/set recurse algo
obsoletes the flag.  Setting the flag on bond/team interface, even when the
consitient member ports didn't have flag set was confusing.

Signed-off-by: Scott Feldman <sfeldma@gmail.com>
---
 drivers/net/bonding/bond_main.c      |    5 +----
 drivers/net/ethernet/rocker/rocker.c |    3 +--
 drivers/net/team/team.c              |    2 +-
 include/linux/netdev_features.h      |    5 +----
 net/core/ethtool.c                   |    1 -
 5 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 9d823b6..f6f4c9c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1013,10 +1013,7 @@ static netdev_features_t bond_fix_features(struct net_device *dev,
 	netdev_features_t mask;
 	struct slave *slave;
 
-	/* If any slave has the offload feature flag set,
-	 * set the offload flag on the bond.
-	 */
-	mask = features | NETIF_F_HW_SWITCH_OFFLOAD;
+	mask = features;
 
 	features &= ~NETIF_F_ONE_FOR_ALL;
 	features |= NETIF_F_ALL_FOR_ALL;
diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index 475ce93..9510525 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -4617,8 +4617,7 @@ static int rocker_probe_port(struct rocker *rocker, unsigned int port_number)
 	rocker_carrier_init(rocker_port);
 
 	dev->features |= NETIF_F_NETNS_LOCAL |
-			 NETIF_F_HW_VLAN_CTAG_FILTER |
-			 NETIF_F_HW_SWITCH_OFFLOAD;
+			 NETIF_F_HW_VLAN_CTAG_FILTER;
 
 	err = register_netdev(dev);
 	if (err) {
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 101b341..fecc6fe 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1924,7 +1924,7 @@ static netdev_features_t team_fix_features(struct net_device *dev,
 	struct team *team = netdev_priv(dev);
 	netdev_features_t mask;
 
-	mask = features | NETIF_F_HW_SWITCH_OFFLOAD;
+	mask = features;
 	features &= ~NETIF_F_ONE_FOR_ALL;
 	features |= NETIF_F_ALL_FOR_ALL;
 
diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
index 7d59dc6..9672781 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h
@@ -66,7 +66,6 @@ enum {
 	NETIF_F_HW_VLAN_STAG_FILTER_BIT,/* Receive filtering on VLAN STAGs */
 	NETIF_F_HW_L2FW_DOFFLOAD_BIT,	/* Allow L2 Forwarding in Hardware */
 	NETIF_F_BUSY_POLL_BIT,		/* Busy poll */
-	NETIF_F_HW_SWITCH_OFFLOAD_BIT,  /* HW switch offload */
 
 	/*
 	 * Add your fresh new feature above and remember to update
@@ -125,7 +124,6 @@ enum {
 #define NETIF_F_HW_VLAN_STAG_TX	__NETIF_F(HW_VLAN_STAG_TX)
 #define NETIF_F_HW_L2FW_DOFFLOAD	__NETIF_F(HW_L2FW_DOFFLOAD)
 #define NETIF_F_BUSY_POLL	__NETIF_F(BUSY_POLL)
-#define NETIF_F_HW_SWITCH_OFFLOAD	__NETIF_F(HW_SWITCH_OFFLOAD)
 
 /* Features valid for ethtool to change */
 /* = all defined minus driver/device-class-related */
@@ -161,8 +159,7 @@ enum {
  */
 #define NETIF_F_ONE_FOR_ALL	(NETIF_F_GSO_SOFTWARE | NETIF_F_GSO_ROBUST | \
 				 NETIF_F_SG | NETIF_F_HIGHDMA |		\
-				 NETIF_F_FRAGLIST | NETIF_F_VLAN_CHALLENGED | \
-				 NETIF_F_HW_SWITCH_OFFLOAD)
+				 NETIF_F_FRAGLIST | NETIF_F_VLAN_CHALLENGED)
 
 /*
  * If one device doesn't support one of these features, then disable it
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 1d00b89..eb0c3ac 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -98,7 +98,6 @@ static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]
 	[NETIF_F_RXALL_BIT] =            "rx-all",
 	[NETIF_F_HW_L2FW_DOFFLOAD_BIT] = "l2-fwd-offload",
 	[NETIF_F_BUSY_POLL_BIT] =        "busy-poll",
-	[NETIF_F_HW_SWITCH_OFFLOAD_BIT] = "hw-switch-offload",
 };
 
 static const char
-- 
1.7.10.4

Re: [PATCH net-next 13/18] switchdev: remove unused NETIF_F_HW_SWITCH_OFFLOAD

[roopa]

On 3/30/15, 1:40 AM, sfeldma@gmail.com wrote:
> From: Scott Feldman <sfeldma@gmail.com>
>
> Flag is no longer needed so remove it.  Using the attr get/set recurse algo
> obsoletes the flag.  Setting the flag on bond/team interface, even when the
> consitient member ports didn't have flag set was confusing.
>
> Signed-off-by: Scott Feldman <sfeldma@gmail.com>

The flag was there to avoid the recursive lowerdev walk where possible.
bond will have the flag if any one slave can have it. You don't walk the 
bond if it does not have the flag.
Also, this allows the user to disable the feature from userspace if 
required and move everything to software.

Re: [PATCH net-next 13/18] switchdev: remove unused NETIF_F_HW_SWITCH_OFFLOAD

[Samudrala, Sridhar]

On 3/30/2015 6:38 AM, roopa wrote:
> On 3/30/15, 1:40 AM, sfeldma@gmail.com wrote:
>> From: Scott Feldman <sfeldma@gmail.com>
>>
>> Flag is no longer needed so remove it.  Using the attr get/set 
>> recurse algo
>> obsoletes the flag.  Setting the flag on bond/team interface, even 
>> when the
>> consitient member ports didn't have flag set was confusing.
>>
>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>
> The flag was there to avoid the recursive lowerdev walk where possible.
> bond will have the flag if any one slave can have it. You don't walk 
> the bond if it does not have the flag.
> Also, this allows the user to disable the feature from userspace if 
> required and move everything to software.
>
I was also planning to use this flag to enable lowerdev walk for fdb 
add/delete on a team device with switchdev
member ports and not attached to a bridge.
Will submit a RFC patch for review.

Thanks
Sridhar

Re: [PATCH net-next 13/18] switchdev: remove unused NETIF_F_HW_SWITCH_OFFLOAD

[Scott Feldman]

On Mon, Mar 30, 2015 at 6:38 AM, roopa <roopa@cumulusnetworks.com> wrote:
> On 3/30/15, 1:40 AM, sfeldma@gmail.com wrote:
>>
>> From: Scott Feldman <sfeldma@gmail.com>
>>
>> Flag is no longer needed so remove it.  Using the attr get/set recurse
>> algo
>> obsoletes the flag.  Setting the flag on bond/team interface, even when
>> the
>> consitient member ports didn't have flag set was confusing.
>>
>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>
>
> The flag was there to avoid the recursive lowerdev walk where possible.
> bond will have the flag if any one slave can have it. You don't walk the
> bond if it does not have the flag.

Ok, I see.  I'll remove this patch from set.  Should I put the
NETIF_F_HW_SWITCH_OFFLOAD check in the get/set attr wrappers?

> Also, this allows the user to disable the feature from userspace if required
> and move everything to software.
>

Re: [PATCH net-next 13/18] switchdev: remove unused NETIF_F_HW_SWITCH_OFFLOAD

[roopa]

On 3/30/15, 2:20 PM, Scott Feldman wrote:
> On Mon, Mar 30, 2015 at 6:38 AM, roopa <roopa@cumulusnetworks.com> wrote:
>> On 3/30/15, 1:40 AM, sfeldma@gmail.com wrote:
>>> From: Scott Feldman <sfeldma@gmail.com>
>>>
>>> Flag is no longer needed so remove it.  Using the attr get/set recurse
>>> algo
>>> obsoletes the flag.  Setting the flag on bond/team interface, even when
>>> the
>>> consitient member ports didn't have flag set was confusing.
>>>
>>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>>
>> The flag was there to avoid the recursive lowerdev walk where possible.
>> bond will have the flag if any one slave can have it. You don't walk the
>> bond if it does not have the flag.
> Ok, I see.  I'll remove this patch from set.  Should I put the
> NETIF_F_HW_SWITCH_OFFLOAD check in the get/set attr wrappers?
>
that would be nice. Thanks scott

Re: [PATCH net-next 13/18] switchdev: remove unused NETIF_F_HW_SWITCH_OFFLOAD

[Scott Feldman]

On Tue, Mar 31, 2015 at 8:31 AM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>
>
> On Monday, March 30, 2015, Scott Feldman <sfeldma@gmail.com> wrote:
>>
>> On Mon, Mar 30, 2015 at 6:38 AM, roopa <roopa@cumulusnetworks.com> wrote:
>> > On 3/30/15, 1:40 AM, sfeldma@gmail.com wrote:
>> >>
>> >> From: Scott Feldman <sfeldma@gmail.com>
>> >>
>> >> Flag is no longer needed so remove it.  Using the attr get/set recurse
>> >> algo
>> >> obsoletes the flag.  Setting the flag on bond/team interface, even when
>> >> the
>> >> consitient member ports didn't have flag set was confusing.
>> >>
>> >> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>> >
>> >
>> > The flag was there to avoid the recursive lowerdev walk where possible.
>> > bond will have the flag if any one slave can have it. You don't walk the
>> > bond if it does not have the flag.
>>
>> Ok, I see.  I'll remove this patch from set.  Should I put the
>> NETIF_F_HW_SWITCH_OFFLOAD check in the get/set attr wrappers?
>
>
> would be nice. Thanks scott.

Ok, it's back.  I'll send v2 tonight which addresses the review
comments and solves the vlan issues once and for all (I hope).

[PATCH net-next 14/18] 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 97196ab..bf77f66 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -86,6 +86,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_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 register_netdev_switch_notifier(struct notifier_block *nb);
@@ -112,6 +114,13 @@ static inline int swdev_port_attr_set(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 8a07096..d681931 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -173,6 +173,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 = {
+		.attr = 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_bridge_setflag(struct net_device *dev,
 				     struct nlattr *ifla,
 				     unsigned long brport_flag)
-- 
1.7.10.4

[PATCH net-next 15/18] 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 |   14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index 9510525..b65c634 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,7 +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		= rocker_port_bridge_getlink,
+	.ndo_bridge_getlink		= swdev_port_bridge_getlink,
 	.ndo_get_phys_port_name		= rocker_port_get_phys_port_name,
 };
 
-- 
1.7.10.4

[PATCH net-next 16/18] 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 bf77f66..9cda17e 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -94,11 +94,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
 
@@ -143,22 +143,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 d681931..e59fe10 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -285,7 +285,7 @@ int swdev_port_bridge_setlink(struct net_device *dev,
 }
 EXPORT_SYMBOL_GPL(swdev_port_bridge_setlink);
 
-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;
@@ -300,7 +300,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;
 	}
@@ -308,7 +308,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 = {
 		.attr = SWDEV_ATTR_PORT_PARENT_ID,
@@ -325,7 +325,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;
 
@@ -347,7 +347,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)
@@ -359,8 +359,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;
@@ -378,7 +378,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;
@@ -393,10 +393,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)
@@ -407,8 +407,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;
@@ -417,7 +417,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;
@@ -431,14 +431,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
@@ -451,4 +451,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 17/18] 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 b65c634..81038cf 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 9cda17e..fbe33f3 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -61,23 +61,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;
 }
@@ -90,10 +90,10 @@ 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 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,
@@ -127,18 +127,19 @@ static inline int swdev_port_bridge_setlink(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 e59fe10..42a5338 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -114,14 +114,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;
 
@@ -130,16 +130,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;
 
@@ -148,10 +148,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
@@ -160,8 +160,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;
 
@@ -171,7 +171,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 18/18] 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 |  426 +++++++++++++++++++++++++++-----
 1 file changed, 367 insertions(+), 59 deletions(-)

diff --git a/Documentation/networking/switchdev.txt b/Documentation/networking/switchdev.txt
index f981a92..2dcc8d8 100644
--- a/Documentation/networking/switchdev.txt
+++ b/Documentation/networking/switchdev.txt
@@ -1,59 +1,367 @@
-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.
+
+
+                             User-space tools                                 
+                                                                              
+       user space                   |                                         
+      +-------------------------------------------------------------------+   
+       kernel                       | Netlink                                 
+                                    |                                         
+                     +--------------+-------------------------------+         
+                     |         Network stack                        |         
+                     |           (Linux)                            |         
+                     |                                              |         
+                     +----------------------------------------------+         
+                                                                              
+                           sw1p2     sw1p4     sw1p6
+                      sw1p1  +  sw1p3  +  sw1p5  +          eth1             
+                        +    |    +    |    +    |            +               
+                        |    |    |    |    |    |            |               
+                     +--+----+----+----+-+--+----+---+  +-----+-----+         
+                     |         Switch driver         |  |    mgmt   |         
+                     |        (this document)        |  |   driver  |         
+                     |                               |  |           |         
+                     +--------------+----------------+  +-----------+         
+                                    |                                         
+       kernel                       | HW bus (eg PCI)                         
+      +-------------------------------------------------------------------+   
+       hardware                     |                                         
+                     +--------------+---+------------+                        
+                     |         Switch device (sw1)   |                        
+                     |  +----+                       +--------+               
+                     |  |    v offloaded data path   | mgmt port              
+                     |  |    |                       |                        
+                     +--|----|----+----+----+----+---+                        
+                        |    |    |    |    |    |                            
+                        +    +    +    +    +    +                            
+                       p1   p2   p3   p4   p5   p6
+                                       
+                             front-panel ports                                
+                                                                              
+
+                                    Fig 1.
+
+
+Include Files
+-------------
+
+#include <linux/netdevice.h>
+#include <net/switchdev.h>
+
+
+Configuration
+-------------
+
+Use "depends NET_SWITCHDEV" in driver's Kconfig to ensure switchdev model
+support is built for driver.
+
+
+Switch Ports
+------------
+
+On switchdev driver initialization, the driver will allocate and register a
+struct net_device (using register_netdev()) for each enumerated physical switch
+port, called the port netdev.  A port netdev is the software representation of
+the physical port and provides a conduit for control traffic to/from the
+controller (the kernel) and the network, as well as an anchor point for higher
+level constructs such as bridges, bonds, VLANs, tunnels, and L3 routers.  Using
+standard netdev tools (iproute2, ethtool, etc), the port netdev can also
+provide to the user access to the physical properties of the switch port such
+as PHY link state and I/O statistics.
+
+There is (currently) no higher-level kernel object for the switch beyond the
+port netdevs.  All of the switchdev driver ops are netdev ops or swdev ops.
+
+A switch management port is outside the scope of the switchdev driver model.
+Typically, the management port is not participating in offloaded data plane and
+is loaded with a different driver, such as a NIC driver, on the management port
+device.
+
+Port Netdev Naming
+^^^^^^^^^^^^^^^^^^
+
+Udev rules should be used for port netdev naming, using some unique attribute
+of the port as a key, for example the port MAC address or the port PHYS name.
+Hard-coding of kernel netdev names within the driver is discouraged; let the
+kernel pick the default netdev name, and let udev set the final name based on a
+port attribute.
+
+Using port PHYS name (ndo_get_phys_port_name) for the key is particularly
+useful for dynically-named ports where the device names it's ports based on
+external configuration.  For example, if a physical 40G port is split logically
+into 4 10G ports, resulting in 4 port netdevs, the device can give a unique
+name for each port using port PHYS name.  The udev rule would be:
+
+SUBSYSTEM=="net", ACTION=="add", DRIVER="<driver>", ATTR{phys_port_name}!="", \
+	NAME="$attr{phys_port_name}"
+
+Suggested naming convention is "swXpYsZ", where X is the switch name or ID, Y
+is the port name or ID, and Z is the sub-port name or ID.  For example, sw1p1s0
+would be sub-port 0 on port 1 on switch 1.
+
+Switch ID
+^^^^^^^^^
+
+The switchdev driver must implement the swdev op swdev_port_attr_get for
+SWDEV_ATTR_PORT_PARENT_ID for each port netdev, returning the same physical ID
+for each port of a switch.  The ID must be unique between switches on the same
+system.  The ID does not need to be unique between switches on different
+systems.
+
+The switch ID is used to locate ports on a switch and to know if aggregated
+ports belong to the same switch.
+
+Port Features
+^^^^^^^^^^^^^
+
+NETIF_F_NETNS_LOCAL
+
+If the switchdev driver (and device) only supports offloading of the default
+network namespace (netns), the driver should set this feature flag to prevent
+the port netdev from being moved out of the default netns.  A netns-aware
+driver/device would not set this flag and be resposible for partitioning
+hardware to preserve netns containment.  This means hardware cannot forward
+traffic from a port in one namespace to another port in another namespace.
+
+NETIF_F_HW_VLAN_CTAG_FILTER
+
+The switchdev driver should set this flag if offloading VLANs on the port
+netdev.  When set, the driver will be called when a VLAN is added or removed
+from a port netdev using the standard ndo_vlan_rx_add_vid and
+ndo_vlan_rx_kill_vid netdev ops.
+
+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 driver, the VLAN 8021q
+driver, and netlink-based tools such as iproute2.
+
+The driver can know a particular port's position in the topology by monitoring
+NETDEV_CHANGEUPPER notifications, with the exception of VLAN membership, which
+we'll get to next.  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.
+
+VLAN membership for a particular port can be know by setting
+NETIF_F_HW_VLAN_CTAG_FILTER and implementing .ndo_vlan_rx_add_vid and
+.ndo_vlan_rx_kill_vid.  This works if using the standalone 8021q VLAN driver or
+if creating VLANs on ports in a bridge.
+
+
+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 prefered 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 00/18] switchdev: spring cleanup

[Jiri Pirko]

Mon, Mar 30, 2015 at 10:40:18AM CEST, sfeldma@gmail.com wrote:
>From: Scott Feldman <sfeldma@gmail.com>
>
>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.

As you can see in the switchdev patch submission history, I originally
pushed this with "swdev_" previx. Turned out people did not like that
because "sw" can be easily misunderstood as "software". Therefore the
naming prefix settled to "netdev_switch_". Me personally don't care, I like
both.

>
>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 (18):
>  switchdev: introduce get/set attrs ops
>  switchdev: flesh out get/set attr 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: 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: remove unused netdev_switch_port_bridge_dellink
>  switchdev: remove unused NETIF_F_HW_SWITCH_OFFLOAD
>  switchdev: add new swdev_port_bridge_getlink
>  rocker: 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: bring documentation up-to-date
>
> Documentation/networking/switchdev.txt |  426 +++++++++++++++++++++++++++-----
> drivers/net/bonding/bond_main.c        |    8 +-
> drivers/net/ethernet/rocker/rocker.c   |  108 +++-----
> drivers/net/team/team.c                |    5 +-
> include/linux/netdev_features.h        |    5 +-
> include/net/switchdev.h                |  144 ++++++-----
> net/bridge/br.c                        |   22 +-
> net/bridge/br_netlink.c                |   24 +-
> net/bridge/br_stp.c                    |    6 +-
> net/core/ethtool.c                     |    1 -
> 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              |  334 ++++++++++++++-----------
> 15 files changed, 750 insertions(+), 425 deletions(-)
>
>-- 
>1.7.10.4
>

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

[Andy Gospodarek]

On Mon, Mar 30, 2015 at 02:00:25PM +0200, Jiri Pirko wrote:
> Mon, Mar 30, 2015 at 10:40:18AM CEST, sfeldma@gmail.com wrote:
> >From: Scott Feldman <sfeldma@gmail.com>
> >
> >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.
> 
> As you can see in the switchdev patch submission history, I originally
> pushed this with "swdev_" previx. Turned out people did not like that
> because "sw" can be easily misunderstood as "software". 

I agree with this.

> 
> >
> >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 (18):
> >  switchdev: introduce get/set attrs ops
> >  switchdev: flesh out get/set attr 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: 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: remove unused netdev_switch_port_bridge_dellink
> >  switchdev: remove unused NETIF_F_HW_SWITCH_OFFLOAD
> >  switchdev: add new swdev_port_bridge_getlink
> >  rocker: 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: bring documentation up-to-date
> >
> > Documentation/networking/switchdev.txt |  426 +++++++++++++++++++++++++++-----
> > drivers/net/bonding/bond_main.c        |    8 +-
> > drivers/net/ethernet/rocker/rocker.c   |  108 +++-----
> > drivers/net/team/team.c                |    5 +-
> > include/linux/netdev_features.h        |    5 +-
> > include/net/switchdev.h                |  144 ++++++-----
> > net/bridge/br.c                        |   22 +-
> > net/bridge/br_netlink.c                |   24 +-
> > net/bridge/br_stp.c                    |    6 +-
> > net/core/ethtool.c                     |    1 -
> > 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              |  334 ++++++++++++++-----------
> > 15 files changed, 750 insertions(+), 425 deletions(-)
> >
> >-- 
> >1.7.10.4
> >
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[roopa]

On 3/30/15, 6:11 AM, Andy Gospodarek wrote:
> On Mon, Mar 30, 2015 at 02:00:25PM +0200, Jiri Pirko wrote:
>> Mon, Mar 30, 2015 at 10:40:18AM CEST, sfeldma@gmail.com wrote:
>>> From: Scott Feldman <sfeldma@gmail.com>
>>>
>>> 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.
>> As you can see in the switchdev patch submission history, I originally
>> pushed this with "swdev_" previx. Turned out people did not like that
>> because "sw" can be easily misunderstood as "software".
> I agree with this.
>
agree. maybe just "switchdev_" will be better.

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

[Or Gerlitz]

On Mon, Mar 30, 2015 at 6:00 PM, roopa <roopa@cumulusnetworks.com> wrote:
> On 3/30/15, 6:11 AM, Andy Gospodarek wrote:
>>
>> On Mon, Mar 30, 2015 at 02:00:25PM +0200, Jiri Pirko wrote:
>>>
>>> Mon, Mar 30, 2015 at 10:40:18AM CEST, sfeldma@gmail.com wrote:
>>>>
>>>> From: Scott Feldman <sfeldma@gmail.com>
>>>>
>>>> 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.
>>>
>>> As you can see in the switchdev patch submission history, I originally
>>> pushed this with "swdev_" previx. Turned out people did not like that
>>> because "sw" can be easily misunderstood as "software".
>>
>> I agree with this.
>>
> agree. maybe just "switchdev_" will be better.

Also, how about prefixing the ops names supported by switchdev with
"sdo_" e.g as we have ndo_ prefix for "plain" netdevs?