[PATCH net-next 0/6] net: dsa: add fabric notifier

[PATCH net-next 0/6] net: dsa: add fabric notifier

[Vivien Didelot]

When a switch fabric is composed of multiple switch chips, these chips
must be programmed accordingly when an event occurred on one of them.

Examples of such event include hardware bridging: when a Linux bridge
spans interconnected chips, they must be programmed to allow external
ports to ingress frames on their internal ports.

Another example is cross-chip hardware VLANs. Switch chips in-between
interconnected bridge ports must also configure a given VLAN to allow
packets to pass through them.

In order to support that, this patchset introduces a non-intrusive
notifier mechanism. It adds a notifier head in every DSA switch tree
(the said fabric), and a notifier block in every DSA switch chip.

When an even occurs, it is chained to all notifiers of the fabric.
Switch chips can react accordingly if they are cross-chip capable.

On a dynamic debug enabled system, bridging a port in a multi-chip
fabric will print something like this (ZII Rev B board):

    # brctl addif br0 lan3
    mv88e6085 0.1:00: crosschip DSA port 1.0 bridged to br0
    mv88e6085 0.4:00: crosschip DSA port 1.0 bridged to br0
    # brctl delif br0 lan3
    mv88e6085 0.1:00: crosschip DSA port 1.0 unbridged from br0
    mv88e6085 0.4:00: crosschip DSA port 1.0 unbridged from br0

Currently only bridging events are added. A patchset introducing support
for cross-chip hardware bridging configuration in mv88e6xxx will follow
right after. Then events for switchdev operations are next on the line.

We should note that non-switchdev events do not support rolling-back
switch-wide operations. We'll have to work on closer integration with
switchdev for that, like introducing new attributes or objects, to
benefit from the prepare and commit phases.

Vivien Didelot (6):
  net: dsa: move netdevice notifier registration
  net: dsa: simplify netdevice events handling
  net: dsa: rollback bridging on error
  net: dsa: change state setter scope
  net: dsa: add switch notifier
  net: dsa: introduce bridge notifier

 include/net/dsa.h  |  17 ++++++++
 net/dsa/Makefile   |   1 +
 net/dsa/dsa.c      |  16 ++++---
 net/dsa/dsa2.c     |   6 +++
 net/dsa/dsa_priv.h |   8 +++-
 net/dsa/slave.c    | 121 +++++++++++++++++++++++++++++++++++------------------
 net/dsa/switch.c   |  85 +++++++++++++++++++++++++++++++++++++
 7 files changed, 205 insertions(+), 49 deletions(-)
 create mode 100644 net/dsa/switch.c

-- 
2.11.0

[PATCH net-next 1/6] net: dsa: move netdevice notifier registration

[Vivien Didelot]

Move the netdevice notifier block register code in slave.c and provide
helpers for dsa.c to register and unregister it.

At the same time, check for errors since (un)register_netdevice_notifier
may fail.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/dsa.c      | 10 ++++------
 net/dsa/dsa_priv.h |  4 ++--
 net/dsa/slave.c    | 22 ++++++++++++++++++++--
 3 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 619e57a44d1d..beb79ccf0f59 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -903,10 +903,6 @@ static struct packet_type dsa_pack_type __read_mostly = {
 	.func	= dsa_switch_rcv,
 };
 
-static struct notifier_block dsa_netdevice_nb __read_mostly = {
-	.notifier_call	= dsa_slave_netdevice_event,
-};
-
 #ifdef CONFIG_PM_SLEEP
 static int dsa_suspend(struct device *d)
 {
@@ -964,7 +960,9 @@ static int __init dsa_init_module(void)
 {
 	int rc;
 
-	register_netdevice_notifier(&dsa_netdevice_nb);
+	rc = dsa_slave_register_notifier();
+	if (rc)
+		return rc;
 
 	rc = platform_driver_register(&dsa_driver);
 	if (rc)
@@ -978,7 +976,7 @@ module_init(dsa_init_module);
 
 static void __exit dsa_cleanup_module(void)
 {
-	unregister_netdevice_notifier(&dsa_netdevice_nb);
+	dsa_slave_unregister_notifier();
 	dev_remove_pack(&dsa_pack_type);
 	platform_driver_unregister(&dsa_driver);
 }
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index a5509b765fc0..591a40aea9ca 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -63,8 +63,8 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
 void dsa_slave_destroy(struct net_device *slave_dev);
 int dsa_slave_suspend(struct net_device *slave_dev);
 int dsa_slave_resume(struct net_device *slave_dev);
-int dsa_slave_netdevice_event(struct notifier_block *unused,
-			      unsigned long event, void *ptr);
+int dsa_slave_register_notifier(void);
+void dsa_slave_unregister_notifier(void);
 
 /* tag_dsa.c */
 extern const struct dsa_device_ops dsa_netdev_ops;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 09fc3e9462c1..949644c1dac2 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1524,8 +1524,8 @@ static int dsa_slave_port_event(struct net_device *dev, unsigned long event,
 	return NOTIFY_DONE;
 }
 
-int dsa_slave_netdevice_event(struct notifier_block *unused,
-			      unsigned long event, void *ptr)
+static int dsa_slave_netdevice_event(struct notifier_block *nb,
+				     unsigned long event, void *ptr)
 {
 	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
 
@@ -1534,3 +1534,21 @@ int dsa_slave_netdevice_event(struct notifier_block *unused,
 
 	return NOTIFY_DONE;
 }
+
+static struct notifier_block dsa_slave_nb __read_mostly = {
+	.notifier_call	= dsa_slave_netdevice_event,
+};
+
+int dsa_slave_register_notifier(void)
+{
+	return register_netdevice_notifier(&dsa_slave_nb);
+}
+
+void dsa_slave_unregister_notifier(void)
+{
+	int err;
+
+	err = unregister_netdevice_notifier(&dsa_slave_nb);
+	if (err)
+		pr_err("DSA: failed to unregister slave notifier (%d)\n", err);
+}
-- 
2.11.0

[PATCH net-next 2/6] net: dsa: simplify netdevice events handling

[Vivien Didelot]

Simplify the code handling the slave netdevice notifier call by
providing a dsa_slave_changeupper helper for NETDEV_CHANGEUPPER, and so
on (only this event is supported at the moment.)

Return NOTIFY_DONE when we did not care about an event, and NOTIFY_OK
when we were concerned but no error occurred, as the API suggests.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/slave.c | 44 ++++++++++++++++----------------------------
 1 file changed, 16 insertions(+), 28 deletions(-)

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 949644c1dac2..332eb234dc21 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1491,37 +1491,22 @@ static bool dsa_slave_dev_check(struct net_device *dev)
 	return dev->netdev_ops == &dsa_slave_netdev_ops;
 }
 
-static int dsa_slave_port_upper_event(struct net_device *dev,
-				      unsigned long event, void *ptr)
+static int dsa_slave_changeupper(struct net_device *dev,
+				 struct netdev_notifier_changeupper_info *info)
 {
-	struct netdev_notifier_changeupper_info *info = ptr;
-	struct net_device *upper = info->upper_dev;
-	int err = 0;
+	int err = NOTIFY_DONE;
 
-	switch (event) {
-	case NETDEV_CHANGEUPPER:
-		if (netif_is_bridge_master(upper)) {
-			if (info->linking)
-				err = dsa_slave_bridge_port_join(dev, upper);
-			else
-				dsa_slave_bridge_port_leave(dev, upper);
+	if (netif_is_bridge_master(info->upper_dev)) {
+		if (info->linking) {
+			err = dsa_slave_bridge_port_join(dev, info->upper_dev);
+			err = notifier_from_errno(err);
+		} else {
+			dsa_slave_bridge_port_leave(dev, info->upper_dev);
+			err = NOTIFY_OK;
 		}
-
-		break;
-	}
-
-	return notifier_from_errno(err);
-}
-
-static int dsa_slave_port_event(struct net_device *dev, unsigned long event,
-				void *ptr)
-{
-	switch (event) {
-	case NETDEV_CHANGEUPPER:
-		return dsa_slave_port_upper_event(dev, event, ptr);
 	}
 
-	return NOTIFY_DONE;
+	return err;
 }
 
 static int dsa_slave_netdevice_event(struct notifier_block *nb,
@@ -1529,8 +1514,11 @@ static int dsa_slave_netdevice_event(struct notifier_block *nb,
 {
 	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
 
-	if (dsa_slave_dev_check(dev))
-		return dsa_slave_port_event(dev, event, ptr);
+	if (dev->netdev_ops != &dsa_slave_netdev_ops)
+		return NOTIFY_DONE;
+
+	if (event == NETDEV_CHANGEUPPER)
+		return dsa_slave_changeupper(dev, ptr);
 
 	return NOTIFY_DONE;
 }
-- 
2.11.0

[PATCH net-next 3/6] net: dsa: rollback bridging on error

[Vivien Didelot]

When an error is returned during the bridging of a port in a
NETDEV_CHANGEUPPER event, net/core/dev.c rolls back the operation.

Be consistent and unassign dp->bridge_dev when this happens.

In the meantime, add comments to document this behavior.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/slave.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 332eb234dc21..d726307c7795 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -562,12 +562,21 @@ static int dsa_slave_bridge_port_join(struct net_device *dev,
 	struct dsa_switch *ds = p->dp->ds;
 	int ret = -EOPNOTSUPP;
 
+	/* Here the port is already bridged. Reflect the current configuration
+	 * so that drivers can program their chips accordingly.
+	 */
 	p->dp->bridge_dev = br;
 
 	if (ds->ops->port_bridge_join)
 		ret = ds->ops->port_bridge_join(ds, p->dp->index, br);
 
-	return ret == -EOPNOTSUPP ? 0 : ret;
+	/* The bridging is rolled back on error */
+	if (ret && ret != -EOPNOTSUPP) {
+		p->dp->bridge_dev = NULL;
+		return ret;
+	}
+
+	return 0;
 }
 
 static void dsa_slave_bridge_port_leave(struct net_device *dev,
@@ -576,6 +585,9 @@ static void dsa_slave_bridge_port_leave(struct net_device *dev,
 	struct dsa_slave_priv *p = netdev_priv(dev);
 	struct dsa_switch *ds = p->dp->ds;
 
+	/* Here the port is already unbridged. Reflect the current configuration
+	 * so that drivers can program their chips accordingly.
+	 */
 	p->dp->bridge_dev = NULL;
 
 	if (ds->ops->port_bridge_leave)
-- 
2.11.0

[PATCH net-next 4/6] net: dsa: change state setter scope

[Vivien Didelot]

The scope of the functions inside net/dsa/slave.c must be the slave
net_device pointer. Change to state setter helper accordingly to
simplify callers.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/slave.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index d726307c7795..d8c3c0f00cf3 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -74,9 +74,12 @@ static inline bool dsa_port_is_bridged(struct dsa_port *dp)
 	return !!dp->bridge_dev;
 }
 
-static void dsa_port_set_stp_state(struct dsa_switch *ds, int port, u8 state)
+static void dsa_slave_set_state(struct net_device *dev, u8 state)
 {
-	struct dsa_port *dp = &ds->ports[port];
+	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_port *dp = p->dp;
+	struct dsa_switch *ds = dp->ds;
+	int port = dp->index;
 
 	if (ds->ops->port_stp_state_set)
 		ds->ops->port_stp_state_set(ds, port, state);
@@ -133,7 +136,7 @@ static int dsa_slave_open(struct net_device *dev)
 			goto clear_promisc;
 	}
 
-	dsa_port_set_stp_state(ds, p->dp->index, stp_state);
+	dsa_slave_set_state(dev, stp_state);
 
 	if (p->phy)
 		phy_start(p->phy);
@@ -175,7 +178,7 @@ static int dsa_slave_close(struct net_device *dev)
 	if (ds->ops->port_disable)
 		ds->ops->port_disable(ds, p->dp->index, p->phy);
 
-	dsa_port_set_stp_state(ds, p->dp->index, BR_STATE_DISABLED);
+	dsa_slave_set_state(dev, BR_STATE_DISABLED);
 
 	return 0;
 }
@@ -382,7 +385,7 @@ static int dsa_slave_stp_state_set(struct net_device *dev,
 	if (switchdev_trans_ph_prepare(trans))
 		return ds->ops->port_stp_state_set ? 0 : -EOPNOTSUPP;
 
-	dsa_port_set_stp_state(ds, p->dp->index, attr->u.stp_state);
+	dsa_slave_set_state(dev, attr->u.stp_state);
 
 	return 0;
 }
@@ -596,7 +599,7 @@ static void dsa_slave_bridge_port_leave(struct net_device *dev,
 	/* Port left the bridge, put in BR_STATE_DISABLED by the bridge layer,
 	 * so allow it to be in BR_STATE_FORWARDING to be kept functional
 	 */
-	dsa_port_set_stp_state(ds, p->dp->index, BR_STATE_FORWARDING);
+	dsa_slave_set_state(dev, BR_STATE_FORWARDING);
 }
 
 static int dsa_slave_port_attr_get(struct net_device *dev,
-- 
2.11.0

[PATCH net-next 5/6] net: dsa: add switch notifier

[Vivien Didelot]

Add a notifier block per DSA switch, registered against a notifier head
in the switch fabric they belong to.

This infrastructure will allow to propagate fabric-wide events such as
port bridging, VLAN configuration, etc. If a DSA switch driver cares
about cross-chip configuration, such events can be caught.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 include/net/dsa.h  |  7 +++++++
 net/dsa/Makefile   |  1 +
 net/dsa/dsa.c      |  6 ++++++
 net/dsa/dsa2.c     |  6 ++++++
 net/dsa/dsa_priv.h |  4 ++++
 net/dsa/switch.c   | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 77 insertions(+)
 create mode 100644 net/dsa/switch.c

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 2cb77e64d648..ac4ea7c3a102 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -13,6 +13,7 @@
 
 #include <linux/if_ether.h>
 #include <linux/list.h>
+#include <linux/notifier.h>
 #include <linux/timer.h>
 #include <linux/workqueue.h>
 #include <linux/of.h>
@@ -92,6 +93,9 @@ struct packet_type;
 struct dsa_switch_tree {
 	struct list_head	list;
 
+	/* Notifier chain for switch-wide events */
+	struct raw_notifier_head	nh;
+
 	/* Tree identifier */
 	u32 tree;
 
@@ -182,6 +186,9 @@ struct dsa_switch {
 	struct dsa_switch_tree	*dst;
 	int			index;
 
+	/* Listener for switch fabric events */
+	struct notifier_block	nb;
+
 	/*
 	 * Give the switch driver somewhere to hang its private data
 	 * structure.
diff --git a/net/dsa/Makefile b/net/dsa/Makefile
index a3380ed0e0be..72912982de3d 100644
--- a/net/dsa/Makefile
+++ b/net/dsa/Makefile
@@ -1,6 +1,7 @@
 # the core
 obj-$(CONFIG_NET_DSA) += dsa_core.o
 dsa_core-y += dsa.o slave.o dsa2.o
+dsa_core-y += dsa.o slave.o dsa2.o switch.o
 
 # tagging formats
 dsa_core-$(CONFIG_NET_DSA_TAG_BRCM) += tag_brcm.o
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index beb79ccf0f59..22e44f691ab9 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -275,6 +275,10 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct device *parent)
 	if (ret < 0)
 		return ret;
 
+	ret = dsa_switch_register_notifier(ds);
+	if (ret)
+		return ret;
+
 	if (ops->set_addr) {
 		ret = ops->set_addr(ds, dst->master_netdev->dev_addr);
 		if (ret < 0)
@@ -400,6 +404,8 @@ static void dsa_switch_destroy(struct dsa_switch *ds)
 
 	if (ds->slave_mii_bus && ds->ops->phy_read)
 		mdiobus_unregister(ds->slave_mii_bus);
+
+	dsa_switch_unregister_notifier(ds);
 }
 
 #ifdef CONFIG_PM_SLEEP
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 9f8cc26be9ea..1c546b6621ee 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -294,6 +294,10 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 	if (err < 0)
 		return err;
 
+	err = dsa_switch_register_notifier(ds);
+	if (err)
+		return err;
+
 	if (ds->ops->set_addr) {
 		err = ds->ops->set_addr(ds, dst->master_netdev->dev_addr);
 		if (err < 0)
@@ -364,6 +368,8 @@ static void dsa_ds_unapply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 
 	if (ds->slave_mii_bus && ds->ops->phy_read)
 		mdiobus_unregister(ds->slave_mii_bus);
+
+	dsa_switch_unregister_notifier(ds);
 }
 
 static int dsa_dst_apply(struct dsa_switch_tree *dst)
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 591a40aea9ca..0706a511244e 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -66,6 +66,10 @@ int dsa_slave_resume(struct net_device *slave_dev);
 int dsa_slave_register_notifier(void);
 void dsa_slave_unregister_notifier(void);
 
+/* switch.c */
+int dsa_switch_register_notifier(struct dsa_switch *ds);
+void dsa_switch_unregister_notifier(struct dsa_switch *ds);
+
 /* tag_dsa.c */
 extern const struct dsa_device_ops dsa_netdev_ops;
 
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
new file mode 100644
index 000000000000..e22fa7633d03
--- /dev/null
+++ b/net/dsa/switch.c
@@ -0,0 +1,53 @@
+/*
+ * Handling of a single switch chip, part of a switch fabric
+ *
+ * Copyright (c) 2017 Vivien Didelot <vivien.didelot@savoirfairelinux.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/netdevice.h>
+#include <linux/notifier.h>
+#include <net/dsa.h>
+
+static int dsa_switch_event(struct notifier_block *nb,
+			    unsigned long event, void *info)
+{
+	struct dsa_switch *ds = container_of(nb, struct dsa_switch, nb);
+	int err;
+
+	switch (event) {
+	default:
+		err = -EOPNOTSUPP;
+		break;
+	}
+
+	/* Non-switchdev operations cannot be rolled back. If a DSA driver
+	 * returns an error during the chained call, switch chips may be in an
+	 * inconsistent state.
+	 */
+	if (err)
+		dev_dbg(ds->dev, "breaking chain for DSA event %lu (%d)\n",
+			event, err);
+
+	return notifier_from_errno(err);
+}
+
+int dsa_switch_register_notifier(struct dsa_switch *ds)
+{
+	ds->nb.notifier_call = dsa_switch_event;
+
+	return raw_notifier_chain_register(&ds->dst->nh, &ds->nb);
+}
+
+void dsa_switch_unregister_notifier(struct dsa_switch *ds)
+{
+	int err;
+
+	err = raw_notifier_chain_unregister(&ds->dst->nh, &ds->nb);
+	if (err)
+		dev_err(ds->dev, "failed to unregister notifier (%d)\n", err);
+}
-- 
2.11.0

[PATCH net-next 6/6] net: dsa: introduce bridge notifier

[Vivien Didelot]

A slave device will now notify the switch fabric once its port is
bridged or unbridged, instead of calling directly its switch operations.

This code allows propagating cross-chip bridging events in the fabric.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 include/net/dsa.h | 10 ++++++++++
 net/dsa/slave.c   | 40 +++++++++++++++++++++++++++++-----------
 net/dsa/switch.c  | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+), 11 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index ac4ea7c3a102..e9c940c8936f 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -268,6 +268,16 @@ struct switchdev_obj_port_fdb;
 struct switchdev_obj_port_mdb;
 struct switchdev_obj_port_vlan;
 
+#define DSA_NOTIFIER_BRIDGE_JOIN		1
+#define DSA_NOTIFIER_BRIDGE_LEAVE		2
+
+/* DSA_NOTIFIER_BRIDGE_* */
+struct dsa_notifier_bridge_info {
+	struct net_device *br;
+	int sw_index;
+	int port;
+};
+
 struct dsa_switch_ops {
 	/*
 	 * Probing and setup.
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index d8c3c0f00cf3..061a49c29cef 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -27,6 +27,17 @@
 
 static bool dsa_slave_dev_check(struct net_device *dev);
 
+static int dsa_slave_notify(struct net_device *dev, unsigned long e, void *v)
+{
+	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct raw_notifier_head *nh = &p->dp->ds->dst->nh;
+	int err;
+
+	err = raw_notifier_call_chain(nh, e, v);
+
+	return notifier_to_errno(err);
+}
+
 /* slave mii_bus handling ***************************************************/
 static int dsa_slave_phy_read(struct mii_bus *bus, int addr, int reg)
 {
@@ -562,39 +573,46 @@ static int dsa_slave_bridge_port_join(struct net_device *dev,
 				      struct net_device *br)
 {
 	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_switch *ds = p->dp->ds;
-	int ret = -EOPNOTSUPP;
+	struct dsa_notifier_bridge_info info = {
+		.sw_index = p->dp->ds->index,
+		.port = p->dp->index,
+		.br = br,
+	};
+	int err;
 
 	/* Here the port is already bridged. Reflect the current configuration
 	 * so that drivers can program their chips accordingly.
 	 */
 	p->dp->bridge_dev = br;
 
-	if (ds->ops->port_bridge_join)
-		ret = ds->ops->port_bridge_join(ds, p->dp->index, br);
+	err = dsa_slave_notify(dev, DSA_NOTIFIER_BRIDGE_JOIN, &info);
 
 	/* The bridging is rolled back on error */
-	if (ret && ret != -EOPNOTSUPP) {
+	if (err)
 		p->dp->bridge_dev = NULL;
-		return ret;
-	}
 
-	return 0;
+	return err;
 }
 
 static void dsa_slave_bridge_port_leave(struct net_device *dev,
 					struct net_device *br)
 {
 	struct dsa_slave_priv *p = netdev_priv(dev);
-	struct dsa_switch *ds = p->dp->ds;
+	struct dsa_notifier_bridge_info info = {
+		.sw_index = p->dp->ds->index,
+		.port = p->dp->index,
+		.br = br,
+	};
+	int err;
 
 	/* Here the port is already unbridged. Reflect the current configuration
 	 * so that drivers can program their chips accordingly.
 	 */
 	p->dp->bridge_dev = NULL;
 
-	if (ds->ops->port_bridge_leave)
-		ds->ops->port_bridge_leave(ds, p->dp->index, br);
+	err = dsa_slave_notify(dev, DSA_NOTIFIER_BRIDGE_LEAVE, &info);
+	if (err)
+		netdev_err(dev, "failed to notify DSA_NOTIFIER_BRIDGE_LEAVE\n");
 
 	/* Port left the bridge, put in BR_STATE_DISABLED by the bridge layer,
 	 * so allow it to be in BR_STATE_FORWARDING to be kept functional
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index e22fa7633d03..6456dacf9ae9 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -13,6 +13,32 @@
 #include <linux/notifier.h>
 #include <net/dsa.h>
 
+static int dsa_switch_bridge_join(struct dsa_switch *ds,
+				  struct dsa_notifier_bridge_info *info)
+{
+	if (ds->index == info->sw_index && ds->ops->port_bridge_join)
+		return ds->ops->port_bridge_join(ds, info->port, info->br);
+
+	if (ds->index != info->sw_index)
+		dev_dbg(ds->dev, "crosschip DSA port %d.%d bridged to %s\n",
+			info->sw_index, info->port, netdev_name(info->br));
+
+	return 0;
+}
+
+static int dsa_switch_bridge_leave(struct dsa_switch *ds,
+				   struct dsa_notifier_bridge_info *info)
+{
+	if (ds->index == info->sw_index && ds->ops->port_bridge_leave)
+		ds->ops->port_bridge_leave(ds, info->port, info->br);
+
+	if (ds->index != info->sw_index)
+		dev_dbg(ds->dev, "crosschip DSA port %d.%d unbridged from %s\n",
+			info->sw_index, info->port, netdev_name(info->br));
+
+	return 0;
+}
+
 static int dsa_switch_event(struct notifier_block *nb,
 			    unsigned long event, void *info)
 {
@@ -20,6 +46,12 @@ static int dsa_switch_event(struct notifier_block *nb,
 	int err;
 
 	switch (event) {
+	case DSA_NOTIFIER_BRIDGE_JOIN:
+		err = dsa_switch_bridge_join(ds, info);
+		break;
+	case DSA_NOTIFIER_BRIDGE_LEAVE:
+		err = dsa_switch_bridge_leave(ds, info);
+		break;
 	default:
 		err = -EOPNOTSUPP;
 		break;
-- 
2.11.0

Re: [PATCH net-next 2/6] net: dsa: simplify netdevice events handling

[Florian Fainelli]

On 02/03/2017 10:20 AM, Vivien Didelot wrote:
> Simplify the code handling the slave netdevice notifier call by
> providing a dsa_slave_changeupper helper for NETDEV_CHANGEUPPER, and so
> on (only this event is supported at the moment.)
> 
> Return NOTIFY_DONE when we did not care about an event, and NOTIFY_OK
> when we were concerned but no error occurred, as the API suggests.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---

>  static int dsa_slave_netdevice_event(struct notifier_block *nb,
> @@ -1529,8 +1514,11 @@ static int dsa_slave_netdevice_event(struct notifier_block *nb,
>  {
>  	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
>  
> -	if (dsa_slave_dev_check(dev))
> -		return dsa_slave_port_event(dev, event, ptr);
> +	if (dev->netdev_ops != &dsa_slave_netdev_ops)
> +		return NOTIFY_DONE;

Why not keep the dsa_slave_dev_check() here?


-- 
Florian

Re: [PATCH net-next 2/6] net: dsa: simplify netdevice events handling

[Vivien Didelot]

Hi Florian,

Florian Fainelli <f.fainelli@gmail.com> writes:

>> -	if (dsa_slave_dev_check(dev))
>> -		return dsa_slave_port_event(dev, event, ptr);
>> +	if (dev->netdev_ops != &dsa_slave_netdev_ops)
>> +		return NOTIFY_DONE;
>
> Why not keep the dsa_slave_dev_check() here?

I dropped it because that condition feels more readable to me than
!dsa_slave_dev_check(dev).

Thanks,

        Vivien

Re: [PATCH net-next 6/6] net: dsa: introduce bridge notifier

[Andrew Lunn]

> +#define DSA_NOTIFIER_BRIDGE_JOIN		1
> +#define DSA_NOTIFIER_BRIDGE_LEAVE		2

Hi Vivien

Is one notifier per event sufficient?

I've not looked at what actually needs to happen when a port joins a
bridge, in a D in DSA setup. Do we need to both enable the flow of
frames around the switch fabric, but also block those frames going out
ports they should not? Do we need a first notifier to put in place
the blocks, and then a second notifier to enable the flow of packets?

What we don't want is a window of time during the fabric setup as a
whole is inconsistent, and frames a leaking out ports they should not.

      Andrew

Re: [PATCH net-next 6/6] net: dsa: introduce bridge notifier

[Vivien Didelot]

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

>> +#define DSA_NOTIFIER_BRIDGE_JOIN		1
>> +#define DSA_NOTIFIER_BRIDGE_LEAVE		2
>
> Is one notifier per event sufficient?

Yes. What a switch chip in a fabric needs to know is what action is
currently triggered on another chip, so that it can react accordingly.

> I've not looked at what actually needs to happen when a port joins a
> bridge, in a D in DSA setup. Do we need to both enable the flow of
> frames around the switch fabric, but also block those frames going out
> ports they should not? Do we need a first notifier to put in place
> the blocks, and then a second notifier to enable the flow of packets?

I gave examples in the cover letter as well as the limitation of
non-switchdev operations.

Basic (non VLAN-filtering enabled) cross-chip hardware bridging in
Marvell consists of configuring a cross-chip port based VLAN table (PVT)
in the chip, which basically is a matrice of external chip/port IDs from
which a local port will accept frames.

In mv88e6xxx, this table defaults to being configured with all ones,
meaning that frames from any external ports are allowed to ingress any
local ports of the chip.

My next patchset will change this behavior to configure the PVT as all
zero by default (except for DSA and CPU links). A new switch-wide
operation will be added to inform a chip that an external port 3.2 got
(un)bridged with something like:

    ds->ops->crosschip_bridge_join(ds, 3, 2, br0);

The mv88e6xxx will then write the PVT to allowed the external port 3.2
to ingress frames on local ports member of br0 (if any).

> What we don't want is a window of time during the fabric setup as a
> whole is inconsistent, and frames a leaking out ports they should not.

There is no such window. A switch notifier is registered once the switch
chip is setup. Chips that are not setup yet won't receive fabric
notifier events.

Thanks,

        Vivien

Re: [PATCH net-next 6/6] net: dsa: introduce bridge notifier

[Andrew Lunn]

> > What we don't want is a window of time during the fabric setup as a
> > whole is inconsistent, and frames a leaking out ports they should not.
> 
> There is no such window.

Hi Vivien

Great, that is what i wanted to hear.

Thanks
	Andrew

Re: [PATCH net-next 0/6] net: dsa: add fabric notifier

[David Miller]

From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Date: Fri,  3 Feb 2017 13:20:15 -0500

> When a switch fabric is composed of multiple switch chips, these chips
> must be programmed accordingly when an event occurred on one of them.
> 
> Examples of such event include hardware bridging: when a Linux bridge
> spans interconnected chips, they must be programmed to allow external
> ports to ingress frames on their internal ports.
> 
> Another example is cross-chip hardware VLANs. Switch chips in-between
> interconnected bridge ports must also configure a given VLAN to allow
> packets to pass through them.
> 
> In order to support that, this patchset introduces a non-intrusive
> notifier mechanism. It adds a notifier head in every DSA switch tree
> (the said fabric), and a notifier block in every DSA switch chip.
 ...

Series applied, thanks Vivien.