[RFC PATCH net-next 00/10] Use robust notifiers in DSA

[RFC PATCH net-next 00/10] Use robust notifiers in DSA

[Vladimir Oltean]

The DSA framework manages groups of switch devices called trees, and
when an event occurs on one switch, it notifies all the other switches
in that tree through something called cross-chip notifiers, so that they
can react on that change too.

Sometimes switches other than the one who originated a change can reject
that change, and DSA must restore the tree to the previous state. Right
now, it either doesn't always do that consistently, or it does that, by
emitting a second cross-chip notifier with the previous state (which is
handled even by switches which already *are* at the previous state,
since the notifier chain broke earlier).

The status quo has caused bugs like the one fixed in commit 4c46bb49460e
("net: dsa: felix: suppress non-changes to the tagging protocol"), and
this has led to me trying to improve things.

I am introducing a _robust() variant of the functions that emit
cross-chip notifiers, which performs better rollback. But we still need
the non-robust notifiers for things that are not expected to fail, so I
am also making the non-robust variants return void.

I am posting this as RFC because something still feels off, but I can't
exactly pinpoint what, and I'm looking for some feedback. Since most DSA
switches are behind I/O protocols that can fail or time out (SPI, I2C,
MDIO etc), everything can fail; that's a fact. On the other hand, when
a network device or the entire system is torn down, nobody cares that
SPI I/O failed - the system is still shutting down; that is also a fact.
I'm not quite sure how to reconcile the two. On one hand we're
suppressing errors emitted by DSA drivers in the non-robust form of
notifiers, and on the other hand there's nothing we can do about them
either way (upper layers don't necessarily care).

Vladimir Oltean (10):
  notifier: allow robust variants to take a different void *v argument
    on rollback
  net: dsa: introduce and use robust form of dsa_tree_notify()
  net: dsa: introduce and use robust form of dsa_broadcast()
  net: dsa: introduce and use robust form of dsa_port_notify()
  Revert "net: dsa: felix: suppress non-changes to the tagging protocol"
  net: dsa: convert switch.c functions to return void if they can
  net: dsa: remove "breaking chain" comment from dsa_switch_event
  net: dsa: introduce a robust form of MTU cross-chip notifiers
  net: dsa: make dsa_tree_notify() and derivatives return void
  net: dsa: make _del variants of functions return void

 drivers/base/power/domain.c    |   4 +-
 drivers/net/dsa/ocelot/felix.c |   3 -
 include/linux/notifier.h       |   8 +-
 kernel/cpu_pm.c                |   3 +-
 kernel/module/main.c           |   4 +-
 kernel/notifier.c              |  21 ++--
 kernel/power/main.c            |   3 +-
 net/core/dev.c                 |   2 +-
 net/dsa/dsa2.c                 | 156 ++++++++++++++++++++----
 net/dsa/dsa_priv.h             |  56 +++++----
 net/dsa/port.c                 | 213 ++++++++++++++++++---------------
 net/dsa/slave.c                |  81 +++++--------
 net/dsa/switch.c               |  27 ++---
 net/dsa/tag_8021q.c            |   8 +-
 14 files changed, 348 insertions(+), 241 deletions(-)

-- 
2.34.1

[RFC PATCH net-next 01/10] notifier: allow robust variants to take a different void *v argument on rollback

[Vladimir Oltean]

More notifier call chain users would like to convert to the robust form
to simplify the rollback, but sometimes the events are of "SET" type,
and the void *v describes the object to set. In that case, the rollback
will also be a "SET" event, but the void *v has to describe the old
object. This pattern is not possible using the current
notifier_call_chain_robust() function.

Expand the prototypes of blocking_notifier_call_chain_robust() and
raw_notifier_call_chain_robust() by explicitly asking for the void *v to
use on rollback. Modify all callers to supply the same "void *v" as for
the normal call.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/base/power/domain.c |  4 ++--
 include/linux/notifier.h    |  8 ++++++--
 kernel/cpu_pm.c             |  3 ++-
 kernel/module/main.c        |  4 +++-
 kernel/notifier.c           | 21 +++++++++++++--------
 kernel/power/main.c         |  3 ++-
 net/core/dev.c              |  2 +-
 7 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 5a2e0232862e..0b85236249c7 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -504,7 +504,7 @@ static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed)
 	/* Notify consumers that we are about to power on. */
 	ret = raw_notifier_call_chain_robust(&genpd->power_notifiers,
 					     GENPD_NOTIFY_PRE_ON,
-					     GENPD_NOTIFY_OFF, NULL);
+					     GENPD_NOTIFY_OFF, NULL, NULL);
 	ret = notifier_to_errno(ret);
 	if (ret)
 		return ret;
@@ -554,7 +554,7 @@ static int _genpd_power_off(struct generic_pm_domain *genpd, bool timed)
 	/* Notify consumers that we are about to power off. */
 	ret = raw_notifier_call_chain_robust(&genpd->power_notifiers,
 					     GENPD_NOTIFY_PRE_OFF,
-					     GENPD_NOTIFY_ON, NULL);
+					     GENPD_NOTIFY_ON, NULL, NULL);
 	ret = notifier_to_errno(ret);
 	if (ret)
 		return ret;
diff --git a/include/linux/notifier.h b/include/linux/notifier.h
index aef88c2d1173..d5e9c0aea933 100644
--- a/include/linux/notifier.h
+++ b/include/linux/notifier.h
@@ -174,9 +174,13 @@ extern int srcu_notifier_call_chain(struct srcu_notifier_head *nh,
 		unsigned long val, void *v);
 
 extern int blocking_notifier_call_chain_robust(struct blocking_notifier_head *nh,
-		unsigned long val_up, unsigned long val_down, void *v);
+					       unsigned long val_up,
+					       unsigned long val_down,
+					       void *v_up, void *v_down);
 extern int raw_notifier_call_chain_robust(struct raw_notifier_head *nh,
-		unsigned long val_up, unsigned long val_down, void *v);
+					  unsigned long val_up,
+					  unsigned long val_down,
+					  void *v_up, void *v_down);
 
 extern bool atomic_notifier_call_chain_is_empty(struct atomic_notifier_head *nh);
 
diff --git a/kernel/cpu_pm.c b/kernel/cpu_pm.c
index ba4ba71facf9..32ca243f0306 100644
--- a/kernel/cpu_pm.c
+++ b/kernel/cpu_pm.c
@@ -51,7 +51,8 @@ static int cpu_pm_notify_robust(enum cpu_pm_event event_up, enum cpu_pm_event ev
 
 	ct_irq_enter_irqson();
 	raw_spin_lock_irqsave(&cpu_pm_notifier.lock, flags);
-	ret = raw_notifier_call_chain_robust(&cpu_pm_notifier.chain, event_up, event_down, NULL);
+	ret = raw_notifier_call_chain_robust(&cpu_pm_notifier.chain, event_up, event_down,
+					     NULL, NULL);
 	raw_spin_unlock_irqrestore(&cpu_pm_notifier.lock, flags);
 	ct_irq_exit_irqson();
 
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 6a477c622544..d92d6e142d11 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2638,7 +2638,9 @@ static int prepare_coming_module(struct module *mod)
 		return err;
 
 	err = blocking_notifier_call_chain_robust(&module_notify_list,
-			MODULE_STATE_COMING, MODULE_STATE_GOING, mod);
+						  MODULE_STATE_COMING,
+						  MODULE_STATE_GOING,
+						  mod, mod);
 	err = notifier_to_errno(err);
 	if (err)
 		klp_module_going(mod);
diff --git a/kernel/notifier.c b/kernel/notifier.c
index 0d5bd62c480e..db71eb12ad98 100644
--- a/kernel/notifier.c
+++ b/kernel/notifier.c
@@ -114,14 +114,14 @@ NOKPROBE_SYMBOL(notifier_call_chain);
  * Returns:	the return value of the @val_up call.
  */
 static int notifier_call_chain_robust(struct notifier_block **nl,
-				     unsigned long val_up, unsigned long val_down,
-				     void *v)
+				      unsigned long val_up, unsigned long val_down,
+				      void *v_up, void *v_down)
 {
 	int ret, nr = 0;
 
-	ret = notifier_call_chain(nl, val_up, v, -1, &nr);
+	ret = notifier_call_chain(nl, val_up, v_up, -1, &nr);
 	if (ret & NOTIFY_STOP_MASK)
-		notifier_call_chain(nl, val_down, v, nr-1, NULL);
+		notifier_call_chain(nl, val_down, v_down, nr-1, NULL);
 
 	return ret;
 }
@@ -333,7 +333,9 @@ int blocking_notifier_chain_unregister(struct blocking_notifier_head *nh,
 EXPORT_SYMBOL_GPL(blocking_notifier_chain_unregister);
 
 int blocking_notifier_call_chain_robust(struct blocking_notifier_head *nh,
-		unsigned long val_up, unsigned long val_down, void *v)
+					unsigned long val_up,
+					unsigned long val_down, void *v_up,
+					void *v_down)
 {
 	int ret = NOTIFY_DONE;
 
@@ -344,7 +346,8 @@ int blocking_notifier_call_chain_robust(struct blocking_notifier_head *nh,
 	 */
 	if (rcu_access_pointer(nh->head)) {
 		down_read(&nh->rwsem);
-		ret = notifier_call_chain_robust(&nh->head, val_up, val_down, v);
+		ret = notifier_call_chain_robust(&nh->head, val_up, val_down,
+						 v_up, v_down);
 		up_read(&nh->rwsem);
 	}
 	return ret;
@@ -426,9 +429,11 @@ int raw_notifier_chain_unregister(struct raw_notifier_head *nh,
 EXPORT_SYMBOL_GPL(raw_notifier_chain_unregister);
 
 int raw_notifier_call_chain_robust(struct raw_notifier_head *nh,
-		unsigned long val_up, unsigned long val_down, void *v)
+				   unsigned long val_up, unsigned long val_down,
+				   void *v_up, void *v_down)
 {
-	return notifier_call_chain_robust(&nh->head, val_up, val_down, v);
+	return notifier_call_chain_robust(&nh->head, val_up, val_down, v_up,
+					  v_down);
 }
 EXPORT_SYMBOL_GPL(raw_notifier_call_chain_robust);
 
diff --git a/kernel/power/main.c b/kernel/power/main.c
index e3694034b753..edc49debbecd 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -84,7 +84,8 @@ int pm_notifier_call_chain_robust(unsigned long val_up, unsigned long val_down)
 {
 	int ret;
 
-	ret = blocking_notifier_call_chain_robust(&pm_chain_head, val_up, val_down, NULL);
+	ret = blocking_notifier_call_chain_robust(&pm_chain_head, val_up,
+						  val_down, NULL, NULL);
 
 	return notifier_to_errno(ret);
 }
diff --git a/net/core/dev.c b/net/core/dev.c
index 716df64fcfa5..5820f18c2e1d 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1968,7 +1968,7 @@ call_netdevice_notifiers_info_robust(unsigned long val_up,
 	ASSERT_RTNL();
 
 	return raw_notifier_call_chain_robust(&net->netdev_chain,
-					      val_up, val_down, info);
+					      val_up, val_down, info, info);
 }
 
 static int call_netdevice_notifiers_extack(unsigned long val,
-- 
2.34.1

[RFC PATCH net-next 02/10] net: dsa: introduce and use robust form of dsa_tree_notify()

[Vladimir Oltean]

Cross-chip notifier chains being broken mid-way by a switch returning an
error is a recurring problem, and some callers of dsa_tree_notify() and
derivatives (dsa_port_notify, dsa_broadcast) deal with this more
gracefully than others.

Not dealing gracefully means not doing anything (and letting the tree
have an inconsistent state), while dealing "gracefully" means emitting
one more notifier which contains the old state. However, even this has
its own potential problems, since in some cases, switch drivers do not
expect to receive a call that requests a state change to their existing
state - see commit 0c4e31c0722a ("net: dsa: felix: suppress non-changes
to the tagging protocol").

The right thing would be to roll back the switches that succeeded,
and leave the one that failed, and the ones where the notifier did not
execute, alone. This is achieved using dsa_tree_notify_robust().

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/dsa/dsa2.c     | 56 +++++++++++++++++++++++++++++-----------------
 net/dsa/dsa_priv.h |  2 ++
 2 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index cac48a741f27..50b87419342f 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -46,6 +46,28 @@ int dsa_tree_notify(struct dsa_switch_tree *dst, unsigned long e, void *v)
 	return notifier_to_errno(err);
 }
 
+/**
+ * dsa_tree_notify_robust - Run code for all switches in a tree, with rollback.
+ * @dst: collection of struct dsa_switch devices to notify.
+ * @e: event, must be of type DSA_NOTIFIER_*
+ * @v: event-specific value.
+ * @e_rollback: event, must be of type DSA_NOTIFIER_*
+ * @v_rollback: event-specific value.
+ *
+ * Like dsa_tree_notify(), except makes sure that switches are restored to the
+ * previous state in case the notifier call chain fails mid way.
+ */
+int dsa_tree_notify_robust(struct dsa_switch_tree *dst, unsigned long e,
+			   void *v, unsigned long e_rollback, void *v_rollback)
+{
+	struct raw_notifier_head *nh = &dst->nh;
+	int err;
+
+	err = raw_notifier_call_chain_robust(nh, e, e_rollback, v, v_rollback);
+
+	return notifier_to_errno(err);
+}
+
 /**
  * dsa_broadcast - Notify all DSA trees in the system.
  * @e: event, must be of type DSA_NOTIFIER_*
@@ -1215,22 +1237,18 @@ static int dsa_tree_bind_tag_proto(struct dsa_switch_tree *dst,
 	 * to the new tagger
 	 */
 	info.tag_ops = tag_ops;
-	err = dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO_CONNECT, &info);
-	if (err && err != -EOPNOTSUPP)
-		goto out_disconnect;
+	err = dsa_tree_notify_robust(dst, DSA_NOTIFIER_TAG_PROTO_CONNECT, &info,
+				     DSA_NOTIFIER_TAG_PROTO_DISCONNECT, &info);
+	if (err && err != -EOPNOTSUPP) {
+		dst->tag_ops = old_tag_ops;
+		return err;
+	}
 
 	/* Notify the old tagger about the disconnection from this tree */
 	info.tag_ops = old_tag_ops;
 	dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO_DISCONNECT, &info);
 
 	return 0;
-
-out_disconnect:
-	info.tag_ops = tag_ops;
-	dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO_DISCONNECT, &info);
-	dst->tag_ops = old_tag_ops;
-
-	return err;
 }
 
 /* Since the dsa/tagging sysfs device attribute is per master, the assumption
@@ -1242,7 +1260,7 @@ int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
 			      const struct dsa_device_ops *tag_ops,
 			      const struct dsa_device_ops *old_tag_ops)
 {
-	struct dsa_notifier_tag_proto_info info;
+	struct dsa_notifier_tag_proto_info info, old_info;
 	struct dsa_port *dp;
 	int err = -EBUSY;
 
@@ -1267,23 +1285,19 @@ int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
 
 	/* Notify the tag protocol change */
 	info.tag_ops = tag_ops;
-	err = dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO, &info);
+	old_info.tag_ops = old_tag_ops;
+	err = dsa_tree_notify_robust(dst, DSA_NOTIFIER_TAG_PROTO, &info,
+				     DSA_NOTIFIER_TAG_PROTO, &old_info);
 	if (err)
-		goto out_unwind_tagger;
+		goto out_unlock;
 
 	err = dsa_tree_bind_tag_proto(dst, tag_ops);
 	if (err)
-		goto out_unwind_tagger;
-
-	rtnl_unlock();
+		dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO, &old_info);
 
-	return 0;
-
-out_unwind_tagger:
-	info.tag_ops = old_tag_ops;
-	dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO, &info);
 out_unlock:
 	rtnl_unlock();
+
 	return err;
 }
 
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index d9722e49864b..9db660aeee93 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -543,6 +543,8 @@ void dsa_lag_unmap(struct dsa_switch_tree *dst, struct dsa_lag *lag);
 struct dsa_lag *dsa_tree_lag_find(struct dsa_switch_tree *dst,
 				  const struct net_device *lag_dev);
 int dsa_tree_notify(struct dsa_switch_tree *dst, unsigned long e, void *v);
+int dsa_tree_notify_robust(struct dsa_switch_tree *dst, unsigned long e,
+			   void *v, unsigned long e_rollback, void *v_rollback);
 int dsa_broadcast(unsigned long e, void *v);
 int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
 			      struct net_device *master,
-- 
2.34.1

[RFC PATCH net-next 03/10] net: dsa: introduce and use robust form of dsa_broadcast()

[Vladimir Oltean]

Introduce dsa_broadcast_robust(), which uses dsa_tree_notify_robust(),
and convert the bridge join and tag_8021q VLAN add procedures to use
this.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/dsa/dsa2.c      | 31 +++++++++++++++++++++++++++++++
 net/dsa/dsa_priv.h  |  2 ++
 net/dsa/port.c      |  6 ++++--
 net/dsa/tag_8021q.c |  8 ++++----
 4 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 50b87419342f..40134ed97980 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -94,6 +94,37 @@ int dsa_broadcast(unsigned long e, void *v)
 	return err;
 }
 
+/**
+ * dsa_broadcast_robust - Notify all DSA trees in the system, with rollback.
+ * @e: event, must be of type DSA_NOTIFIER_*
+ * @v: event-specific value.
+ * @e_rollback: event, must be of type DSA_NOTIFIER_*
+ * @v_rollback: event-specific value.
+ *
+ * Like dsa_broadcast(), except makes sure that switches are restored to the
+ * previous state in case the notifier call chain fails mid way.
+ */
+int dsa_broadcast_robust(unsigned long e, void *v, unsigned long e_rollback,
+			 void *v_rollback)
+{
+	struct dsa_switch_tree *dst;
+	int err = 0;
+
+	list_for_each_entry(dst, &dsa_tree_list, list) {
+		err = dsa_tree_notify_robust(dst, e, v, e_rollback, v_rollback);
+		if (err)
+			goto rollback;
+	}
+
+	return 0;
+
+rollback:
+	list_for_each_entry_continue_reverse(dst, &dsa_tree_list, list)
+		dsa_tree_notify(dst, e_rollback, v_rollback);
+
+	return err;
+}
+
 /**
  * dsa_lag_map() - Map LAG structure to a linear LAG array
  * @dst: Tree in which to record the mapping.
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 9db660aeee93..b4545b9ebb64 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -546,6 +546,8 @@ int dsa_tree_notify(struct dsa_switch_tree *dst, unsigned long e, void *v);
 int dsa_tree_notify_robust(struct dsa_switch_tree *dst, unsigned long e,
 			   void *v, unsigned long e_rollback, void *v_rollback);
 int dsa_broadcast(unsigned long e, void *v);
+int dsa_broadcast_robust(unsigned long e, void *v, unsigned long e_rollback,
+			 void *v_rollback);
 int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
 			      struct net_device *master,
 			      const struct dsa_device_ops *tag_ops,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 2dd76eb1621c..6aa6402d3ed9 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -480,7 +480,8 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
 	brport_dev = dsa_port_to_bridge_port(dp);
 
 	info.bridge = *dp->bridge;
-	err = dsa_broadcast(DSA_NOTIFIER_BRIDGE_JOIN, &info);
+	err = dsa_broadcast_robust(DSA_NOTIFIER_BRIDGE_JOIN, &info,
+				   DSA_NOTIFIER_BRIDGE_LEAVE, &info);
 	if (err)
 		goto out_rollback;
 
@@ -1738,7 +1739,8 @@ int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid, bool broadcast)
 	};
 
 	if (broadcast)
-		return dsa_broadcast(DSA_NOTIFIER_TAG_8021Q_VLAN_ADD, &info);
+		return dsa_broadcast_robust(DSA_NOTIFIER_TAG_8021Q_VLAN_ADD, &info,
+					    DSA_NOTIFIER_TAG_8021Q_VLAN_DEL, &info);
 
 	return dsa_port_notify(dp, DSA_NOTIFIER_TAG_8021Q_VLAN_ADD, &info);
 }
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index 01a427800797..d20b9590a2e5 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -205,10 +205,10 @@ int dsa_switch_tag_8021q_vlan_add(struct dsa_switch *ds,
 	struct dsa_port *dp;
 	int err;
 
-	/* Since we use dsa_broadcast(), there might be other switches in other
-	 * trees which don't support tag_8021q, so don't return an error.
-	 * Or they might even support tag_8021q but have not registered yet to
-	 * use it (maybe they use another tagger currently).
+	/* Since we use dsa_broadcast_robust(), there might be other switches
+	 * in other trees which don't support tag_8021q, so don't return an
+	 * error. Or they might even support tag_8021q but have not registered
+	 * yet to use it (maybe they use another tagger currently).
 	 */
 	if (!ds->ops->tag_8021q_vlan_add || !ds->tag_8021q_ctx)
 		return 0;
-- 
2.34.1

[RFC PATCH net-next 04/10] net: dsa: introduce and use robust form of dsa_port_notify()

[Vladimir Oltean]

Introduce dsa_port_notify_robust(), which uses dsa_tree_notify_robust(),
and convert as many call paths to use it. Some notable exceptions are
DSA_NOTIFIER_LAG_CHANGE, for which it isn't clear how to restore the
state (or why this is allowed to return an error for that matter), and
DSA_NOTIFIER_MTU (which we'll convert separately to the robust form).

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/dsa/port.c | 51 +++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 40 insertions(+), 11 deletions(-)

diff --git a/net/dsa/port.c b/net/dsa/port.c
index 6aa6402d3ed9..2fec3df65643 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -30,6 +30,24 @@ static int dsa_port_notify(const struct dsa_port *dp, unsigned long e, void *v)
 	return dsa_tree_notify(dp->ds->dst, e, v);
 }
 
+/**
+ * dsa_port_notify_robust - Notify fabric of changes to port, with rollback
+ * @dp: port on which change occurred
+ * @e: event, must be of type DSA_NOTIFIER_*
+ * @v: event-specific value.
+ * @e_rollback: event, must be of type DSA_NOTIFIER_*
+ * @v_rollback: event-specific value.
+ *
+ * Like dsa_port_notify(), except makes sure that switches are restored to the
+ * previous state in case the notifier call chain fails mid way.
+ */
+static int dsa_port_notify_robust(const struct dsa_port *dp, unsigned long e,
+				  void *v, unsigned long e_rollback,
+				  void *v_rollback)
+{
+	return dsa_tree_notify_robust(dp->ds->dst, e, v, e_rollback, v_rollback);
+}
+
 static void dsa_port_notify_bridge_fdb_flush(const struct dsa_port *dp, u16 vid)
 {
 	struct net_device *brport_dev = dsa_port_to_bridge_port(dp);
@@ -641,7 +659,8 @@ int dsa_port_lag_join(struct dsa_port *dp, struct net_device *lag_dev,
 		goto err_lag_create;
 
 	info.lag = *dp->lag;
-	err = dsa_port_notify(dp, DSA_NOTIFIER_LAG_JOIN, &info);
+	err = dsa_port_notify_robust(dp, DSA_NOTIFIER_LAG_JOIN, &info,
+				     DSA_NOTIFIER_LAG_LEAVE, &info);
 	if (err)
 		goto err_lag_join;
 
@@ -854,12 +873,14 @@ int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock)
 {
 	unsigned long ageing_jiffies = clock_t_to_jiffies(ageing_clock);
 	unsigned int ageing_time = jiffies_to_msecs(ageing_jiffies);
-	struct dsa_notifier_ageing_time_info info;
+	struct dsa_notifier_ageing_time_info info, old_info;
 	int err;
 
 	info.ageing_time = ageing_time;
+	old_info.ageing_time = dp->ageing_time;
 
-	err = dsa_port_notify(dp, DSA_NOTIFIER_AGEING_TIME, &info);
+	err = dsa_port_notify_robust(dp, DSA_NOTIFIER_AGEING_TIME, &info,
+				     DSA_NOTIFIER_AGEING_TIME, &old_info);
 	if (err)
 		return err;
 
@@ -971,7 +992,8 @@ int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 	if (!dp->ds->fdb_isolation)
 		info.db.bridge.num = 0;
 
-	return dsa_port_notify(dp, DSA_NOTIFIER_FDB_ADD, &info);
+	return dsa_port_notify_robust(dp, DSA_NOTIFIER_FDB_ADD, &info,
+				      DSA_NOTIFIER_FDB_DEL, &info);
 }
 
 int dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
@@ -1007,7 +1029,8 @@ static int dsa_port_host_fdb_add(struct dsa_port *dp,
 	if (!dp->ds->fdb_isolation)
 		info.db.bridge.num = 0;
 
-	return dsa_port_notify(dp, DSA_NOTIFIER_HOST_FDB_ADD, &info);
+	return dsa_port_notify_robust(dp, DSA_NOTIFIER_HOST_FDB_ADD, &info,
+				      DSA_NOTIFIER_HOST_FDB_DEL, &info);
 }
 
 int dsa_port_standalone_host_fdb_add(struct dsa_port *dp,
@@ -1107,7 +1130,8 @@ int dsa_port_lag_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 	if (!dp->ds->fdb_isolation)
 		info.db.bridge.num = 0;
 
-	return dsa_port_notify(dp, DSA_NOTIFIER_LAG_FDB_ADD, &info);
+	return dsa_port_notify_robust(dp, DSA_NOTIFIER_LAG_FDB_ADD, &info,
+				      DSA_NOTIFIER_LAG_FDB_DEL, &info);
 }
 
 int dsa_port_lag_fdb_del(struct dsa_port *dp, const unsigned char *addr,
@@ -1155,7 +1179,8 @@ int dsa_port_mdb_add(const struct dsa_port *dp,
 	if (!dp->ds->fdb_isolation)
 		info.db.bridge.num = 0;
 
-	return dsa_port_notify(dp, DSA_NOTIFIER_MDB_ADD, &info);
+	return dsa_port_notify_robust(dp, DSA_NOTIFIER_MDB_ADD, &info,
+				      DSA_NOTIFIER_MDB_DEL, &info);
 }
 
 int dsa_port_mdb_del(const struct dsa_port *dp,
@@ -1189,7 +1214,8 @@ static int dsa_port_host_mdb_add(const struct dsa_port *dp,
 	if (!dp->ds->fdb_isolation)
 		info.db.bridge.num = 0;
 
-	return dsa_port_notify(dp, DSA_NOTIFIER_HOST_MDB_ADD, &info);
+	return dsa_port_notify_robust(dp, DSA_NOTIFIER_HOST_MDB_ADD, &info,
+				      DSA_NOTIFIER_HOST_MDB_DEL, &info);
 }
 
 int dsa_port_standalone_host_mdb_add(const struct dsa_port *dp,
@@ -1274,7 +1300,8 @@ int dsa_port_vlan_add(struct dsa_port *dp,
 		.extack = extack,
 	};
 
-	return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info);
+	return dsa_port_notify_robust(dp, DSA_NOTIFIER_VLAN_ADD, &info,
+				      DSA_NOTIFIER_VLAN_DEL, &info);
 }
 
 int dsa_port_vlan_del(struct dsa_port *dp,
@@ -1300,7 +1327,8 @@ int dsa_port_host_vlan_add(struct dsa_port *dp,
 	struct dsa_port *cpu_dp = dp->cpu_dp;
 	int err;
 
-	err = dsa_port_notify(dp, DSA_NOTIFIER_HOST_VLAN_ADD, &info);
+	err = dsa_port_notify_robust(dp, DSA_NOTIFIER_HOST_VLAN_ADD, &info,
+				     DSA_NOTIFIER_HOST_VLAN_DEL, &info);
 	if (err && err != -EOPNOTSUPP)
 		return err;
 
@@ -1742,7 +1770,8 @@ int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid, bool broadcast)
 		return dsa_broadcast_robust(DSA_NOTIFIER_TAG_8021Q_VLAN_ADD, &info,
 					    DSA_NOTIFIER_TAG_8021Q_VLAN_DEL, &info);
 
-	return dsa_port_notify(dp, DSA_NOTIFIER_TAG_8021Q_VLAN_ADD, &info);
+	return dsa_port_notify_robust(dp, DSA_NOTIFIER_TAG_8021Q_VLAN_ADD, &info,
+				      DSA_NOTIFIER_TAG_8021Q_VLAN_DEL, &info);
 }
 
 void dsa_port_tag_8021q_vlan_del(struct dsa_port *dp, u16 vid, bool broadcast)
-- 
2.34.1

[RFC PATCH net-next 05/10] Revert "net: dsa: felix: suppress non-changes to the tagging protocol"

[Vladimir Oltean]

This reverts commit 4c46bb49460ee14c69629e813640d8b929e88941.

This is no longer necessary now that dsa_tree_change_tag_proto() uses
dsa_tree_notify_robust() and hence, does not bother us with another
notifier for old_tag_ops once we've failed the change to the new one.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index aadb0bd7c24f..859196898a7d 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -610,9 +610,6 @@ static int felix_change_tag_protocol(struct dsa_switch *ds,
 
 	old_proto_ops = felix->tag_proto_ops;
 
-	if (proto_ops == old_proto_ops)
-		return 0;
-
 	err = proto_ops->setup(ds);
 	if (err)
 		goto setup_failed;
-- 
2.34.1

[RFC PATCH net-next 06/10] net: dsa: convert switch.c functions to return void if they can

[Vladimir Oltean]

We try to convert the non-robust cross-chip notifiers (dsa_tree_notify
and derivatives) to return void, and we'll suppress errors inside
dsa_tree_notify() itself.

It makes little sense to force all dsa_switch_event() handlers to return
int especially since they're going to be called from a function that
returns void to its own callers, so stop that and convert functions in
switch.c to return void where possible.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/dsa/switch.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 4dfd68cf61c5..e3a91f38c5db 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -104,8 +104,8 @@ static int dsa_switch_bridge_join(struct dsa_switch *ds,
 	return 0;
 }
 
-static int dsa_switch_bridge_leave(struct dsa_switch *ds,
-				   struct dsa_notifier_bridge_info *info)
+static void dsa_switch_bridge_leave(struct dsa_switch *ds,
+				    struct dsa_notifier_bridge_info *info)
 {
 	if (info->dp->ds == ds && ds->ops->port_bridge_leave)
 		ds->ops->port_bridge_leave(ds, info->dp->index, info->bridge);
@@ -115,8 +115,6 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds,
 						info->dp->ds->index,
 						info->dp->index,
 						info->bridge);
-
-	return 0;
 }
 
 /* Matches for all upstream-facing ports (the CPU port and all upstream-facing
@@ -871,7 +869,7 @@ dsa_switch_connect_tag_proto(struct dsa_switch *ds,
 	return 0;
 }
 
-static int
+static void
 dsa_switch_disconnect_tag_proto(struct dsa_switch *ds,
 				struct dsa_notifier_tag_proto_info *info)
 {
@@ -884,26 +882,23 @@ dsa_switch_disconnect_tag_proto(struct dsa_switch *ds,
 	/* No need to notify the switch, since it shouldn't have any
 	 * resources to tear down
 	 */
-	return 0;
 }
 
-static int
+static void
 dsa_switch_master_state_change(struct dsa_switch *ds,
 			       struct dsa_notifier_master_state_info *info)
 {
 	if (!ds->ops->master_state_change)
-		return 0;
+		return;
 
 	ds->ops->master_state_change(ds, info->master, info->operational);
-
-	return 0;
 }
 
 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;
+	int err = 0;
 
 	switch (event) {
 	case DSA_NOTIFIER_AGEING_TIME:
@@ -913,7 +908,7 @@ static int dsa_switch_event(struct notifier_block *nb,
 		err = dsa_switch_bridge_join(ds, info);
 		break;
 	case DSA_NOTIFIER_BRIDGE_LEAVE:
-		err = dsa_switch_bridge_leave(ds, info);
+		dsa_switch_bridge_leave(ds, info);
 		break;
 	case DSA_NOTIFIER_FDB_ADD:
 		err = dsa_switch_fdb_add(ds, info);
@@ -976,7 +971,7 @@ static int dsa_switch_event(struct notifier_block *nb,
 		err = dsa_switch_connect_tag_proto(ds, info);
 		break;
 	case DSA_NOTIFIER_TAG_PROTO_DISCONNECT:
-		err = dsa_switch_disconnect_tag_proto(ds, info);
+		dsa_switch_disconnect_tag_proto(ds, info);
 		break;
 	case DSA_NOTIFIER_TAG_8021Q_VLAN_ADD:
 		err = dsa_switch_tag_8021q_vlan_add(ds, info);
@@ -985,7 +980,7 @@ static int dsa_switch_event(struct notifier_block *nb,
 		err = dsa_switch_tag_8021q_vlan_del(ds, info);
 		break;
 	case DSA_NOTIFIER_MASTER_STATE_CHANGE:
-		err = dsa_switch_master_state_change(ds, info);
+		dsa_switch_master_state_change(ds, info);
 		break;
 	default:
 		err = -EOPNOTSUPP;
-- 
2.34.1

[RFC PATCH net-next 07/10] net: dsa: remove "breaking chain" comment from dsa_switch_event

[Vladimir Oltean]

With the introduction of robust notifier chains, it is certainly no
longer true to simply say in dsa_switch_event() that the chain is
broken.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/dsa/switch.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index e3a91f38c5db..37875960d3f0 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -987,10 +987,6 @@ static int dsa_switch_event(struct notifier_block *nb,
 		break;
 	}
 
-	if (err)
-		dev_dbg(ds->dev, "breaking chain for DSA event %lu (%d)\n",
-			event, err);
-
 	return notifier_from_errno(err);
 }
 
-- 
2.34.1

[RFC PATCH net-next 08/10] net: dsa: introduce a robust form of MTU cross-chip notifiers

[Vladimir Oltean]

Add rollback to the MTU cross-chip change procedure, which allows us to
leave the tree in a consistent state at all times.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/dsa/dsa_priv.h |  3 ++-
 net/dsa/port.c     | 19 +++++++++++++++++--
 net/dsa/slave.c    |  7 ++++---
 3 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index b4545b9ebb64..6c935f151864 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -230,7 +230,8 @@ int dsa_port_mst_enable(struct dsa_port *dp, bool on,
 			struct netlink_ext_ack *extack);
 int dsa_port_vlan_msti(struct dsa_port *dp,
 		       const struct switchdev_vlan_msti *msti);
-int dsa_port_mtu_change(struct dsa_port *dp, int new_mtu);
+void dsa_port_mtu_change(struct dsa_port *dp, int new_mtu);
+int dsa_port_mtu_change_robust(struct dsa_port *dp, int new_mtu, int old_mtu);
 int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 		     u16 vid);
 int dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 2fec3df65643..4095592c4790 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -962,14 +962,29 @@ int dsa_port_vlan_msti(struct dsa_port *dp,
 	return ds->ops->vlan_msti_set(ds, *dp->bridge, msti);
 }
 
-int dsa_port_mtu_change(struct dsa_port *dp, int new_mtu)
+void dsa_port_mtu_change(struct dsa_port *dp, int new_mtu)
 {
 	struct dsa_notifier_mtu_info info = {
 		.dp = dp,
 		.mtu = new_mtu,
 	};
 
-	return dsa_port_notify(dp, DSA_NOTIFIER_MTU, &info);
+	dsa_port_notify(dp, DSA_NOTIFIER_MTU, &info);
+}
+
+int dsa_port_mtu_change_robust(struct dsa_port *dp, int new_mtu, int old_mtu)
+{
+	struct dsa_notifier_mtu_info old_info = {
+		.dp = dp,
+		.mtu = old_mtu,
+	};
+	struct dsa_notifier_mtu_info info = {
+		.dp = dp,
+		.mtu = new_mtu,
+	};
+
+	return dsa_port_notify_robust(dp, DSA_NOTIFIER_MTU, &info,
+				      DSA_NOTIFIER_MTU, &old_info);
 }
 
 int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index ad6a6663feeb..02cc1774888a 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1810,6 +1810,7 @@ int dsa_slave_change_mtu(struct net_device *dev, int new_mtu)
 	int largest_mtu = 0;
 	int new_master_mtu;
 	int old_master_mtu;
+	int old_cpu_mtu;
 	int mtu_limit;
 	int cpu_mtu;
 	int err;
@@ -1849,6 +1850,7 @@ int dsa_slave_change_mtu(struct net_device *dev, int new_mtu)
 	 * MTU, since that surely isn't either.
 	 */
 	cpu_mtu = largest_mtu;
+	old_cpu_mtu = old_master_mtu - dsa_tag_protocol_overhead(cpu_dp->tag_ops);
 
 	/* Start applying stuff */
 	if (new_master_mtu != old_master_mtu) {
@@ -1859,7 +1861,7 @@ int dsa_slave_change_mtu(struct net_device *dev, int new_mtu)
 		/* We only need to propagate the MTU of the CPU port to
 		 * upstream switches, so emit a notifier which updates them.
 		 */
-		err = dsa_port_mtu_change(cpu_dp, cpu_mtu);
+		err = dsa_port_mtu_change_robust(cpu_dp, cpu_mtu, old_cpu_mtu);
 		if (err)
 			goto out_cpu_failed;
 	}
@@ -1876,8 +1878,7 @@ int dsa_slave_change_mtu(struct net_device *dev, int new_mtu)
 
 out_port_failed:
 	if (new_master_mtu != old_master_mtu)
-		dsa_port_mtu_change(cpu_dp, old_master_mtu -
-				    dsa_tag_protocol_overhead(cpu_dp->tag_ops));
+		dsa_port_mtu_change(cpu_dp, old_cpu_mtu);
 out_cpu_failed:
 	if (new_master_mtu != old_master_mtu)
 		dev_set_mtu(master, old_master_mtu);
-- 
2.34.1

[RFC PATCH net-next 09/10] net: dsa: make dsa_tree_notify() and derivatives return void

[Vladimir Oltean]

Now that all cross-chip notifiers where we do care for the error code
were converted to use the robust variant, suppress errors coming from
the rest, giving a clear indication as to what we expect to fail and
what we don't.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/dsa/dsa2.c     | 83 +++++++++++++++++++++++++++++++++++++++-------
 net/dsa/dsa_priv.h |  5 +--
 net/dsa/port.c     | 62 ++++++++++++++++------------------
 3 files changed, 103 insertions(+), 47 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 40134ed97980..6596e7c2831d 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -26,6 +26,68 @@ LIST_HEAD(dsa_tree_list);
 /* Track the bridges with forwarding offload enabled */
 static unsigned long dsa_fwd_offloading_bridges;
 
+static const char *dsa_event_name(unsigned long e)
+{
+	switch (e) {
+	case DSA_NOTIFIER_AGEING_TIME:
+		return "DSA_NOTIFIER_AGEING_TIME";
+	case DSA_NOTIFIER_BRIDGE_JOIN:
+		return "DSA_NOTIFIER_BRIDGE_JOIN";
+	case DSA_NOTIFIER_BRIDGE_LEAVE:
+		return "DSA_NOTIFIER_BRIDGE_LEAVE";
+	case DSA_NOTIFIER_FDB_ADD:
+		return "DSA_NOTIFIER_FDB_ADD";
+	case DSA_NOTIFIER_FDB_DEL:
+		return "DSA_NOTIFIER_FDB_DEL";
+	case DSA_NOTIFIER_HOST_FDB_ADD:
+		return "DSA_NOTIFIER_HOST_FDB_ADD";
+	case DSA_NOTIFIER_HOST_FDB_DEL:
+		return "DSA_NOTIFIER_HOST_FDB_DEL";
+	case DSA_NOTIFIER_LAG_FDB_ADD:
+		return "DSA_NOTIFIER_LAG_FDB_ADD";
+	case DSA_NOTIFIER_LAG_FDB_DEL:
+		return "DSA_NOTIFIER_LAG_FDB_DEL";
+	case DSA_NOTIFIER_LAG_CHANGE:
+		return "DSA_NOTIFIER_LAG_CHANGE";
+	case DSA_NOTIFIER_LAG_JOIN:
+		return "DSA_NOTIFIER_LAG_JOIN";
+	case DSA_NOTIFIER_LAG_LEAVE:
+		return "DSA_NOTIFIER_LAG_LEAVE";
+	case DSA_NOTIFIER_MDB_ADD:
+		return "DSA_NOTIFIER_MDB_ADD";
+	case DSA_NOTIFIER_MDB_DEL:
+		return "DSA_NOTIFIER_MDB_DEL";
+	case DSA_NOTIFIER_HOST_MDB_ADD:
+		return "DSA_NOTIFIER_HOST_MDB_ADD";
+	case DSA_NOTIFIER_HOST_MDB_DEL:
+		return "DSA_NOTIFIER_HOST_MDB_DEL";
+	case DSA_NOTIFIER_VLAN_ADD:
+		return "DSA_NOTIFIER_VLAN_ADD";
+	case DSA_NOTIFIER_VLAN_DEL:
+		return "DSA_NOTIFIER_VLAN_DEL";
+	case DSA_NOTIFIER_HOST_VLAN_ADD:
+		return "DSA_NOTIFIER_HOST_VLAN_ADD";
+	case DSA_NOTIFIER_HOST_VLAN_DEL:
+		return "DSA_NOTIFIER_HOST_VLAN_DEL";
+	case DSA_NOTIFIER_MTU:
+		return "DSA_NOTIFIER_MTU";
+	case DSA_NOTIFIER_TAG_PROTO:
+		return "DSA_NOTIFIER_TAG_PROTO";
+	case DSA_NOTIFIER_TAG_PROTO_CONNECT:
+		return "DSA_NOTIFIER_TAG_PROTO_CONNECT";
+	case DSA_NOTIFIER_TAG_PROTO_DISCONNECT:
+		return "DSA_NOTIFIER_TAG_PROTO_DISCONNECT";
+	case DSA_NOTIFIER_TAG_8021Q_VLAN_ADD:
+		return "DSA_NOTIFIER_TAG_8021Q_VLAN_ADD";
+	case DSA_NOTIFIER_TAG_8021Q_VLAN_DEL:
+		return "DSA_NOTIFIER_TAG_8021Q_VLAN_DEL";
+	case DSA_NOTIFIER_MASTER_STATE_CHANGE:
+		return "DSA_NOTIFIER_MASTER_STATE_CHANGE";
+	default:
+		return "unknown";
+	}
+}
+
 /**
  * dsa_tree_notify - Execute code for all switches in a DSA switch tree.
  * @dst: collection of struct dsa_switch devices to notify.
@@ -36,14 +98,17 @@ static unsigned long dsa_fwd_offloading_bridges;
  * each member DSA switch. The other alternative of traversing the tree is only
  * through its ports list, which does not uniquely list the switches.
  */
-int dsa_tree_notify(struct dsa_switch_tree *dst, unsigned long e, void *v)
+void dsa_tree_notify(struct dsa_switch_tree *dst, unsigned long e, void *v)
 {
 	struct raw_notifier_head *nh = &dst->nh;
 	int err;
 
 	err = raw_notifier_call_chain(nh, e, v);
 
-	return notifier_to_errno(err);
+	err = notifier_to_errno(err);
+	if (err)
+		pr_err("DSA tree %d failed to notify event %s: %pe\n",
+		       dst->index, dsa_event_name(e), ERR_PTR(err));
 }
 
 /**
@@ -80,18 +145,12 @@ int dsa_tree_notify_robust(struct dsa_switch_tree *dst, unsigned long e,
  * WARNING: this function is not reliable during probe time, because probing
  * between trees is asynchronous and not all DSA trees might have probed.
  */
-int dsa_broadcast(unsigned long e, void *v)
+void dsa_broadcast(unsigned long e, void *v)
 {
 	struct dsa_switch_tree *dst;
-	int err = 0;
 
-	list_for_each_entry(dst, &dsa_tree_list, list) {
-		err = dsa_tree_notify(dst, e, v);
-		if (err)
-			break;
-	}
-
-	return err;
+	list_for_each_entry(dst, &dsa_tree_list, list)
+		dsa_tree_notify(dst, e, v);
 }
 
 /**
@@ -108,7 +167,7 @@ int dsa_broadcast_robust(unsigned long e, void *v, unsigned long e_rollback,
 			 void *v_rollback)
 {
 	struct dsa_switch_tree *dst;
-	int err = 0;
+	int err;
 
 	list_for_each_entry(dst, &dsa_tree_list, list) {
 		err = dsa_tree_notify_robust(dst, e, v, e_rollback, v_rollback);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 6c935f151864..263a07152b07 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -17,6 +17,7 @@
 
 #define DSA_MAX_NUM_OFFLOADING_BRIDGES		BITS_PER_LONG
 
+/* Please update dsa_event_name() when adding new elements to this array */
 enum {
 	DSA_NOTIFIER_AGEING_TIME,
 	DSA_NOTIFIER_BRIDGE_JOIN,
@@ -543,10 +544,10 @@ void dsa_lag_map(struct dsa_switch_tree *dst, struct dsa_lag *lag);
 void dsa_lag_unmap(struct dsa_switch_tree *dst, struct dsa_lag *lag);
 struct dsa_lag *dsa_tree_lag_find(struct dsa_switch_tree *dst,
 				  const struct net_device *lag_dev);
-int dsa_tree_notify(struct dsa_switch_tree *dst, unsigned long e, void *v);
+void dsa_tree_notify(struct dsa_switch_tree *dst, unsigned long e, void *v);
 int dsa_tree_notify_robust(struct dsa_switch_tree *dst, unsigned long e,
 			   void *v, unsigned long e_rollback, void *v_rollback);
-int dsa_broadcast(unsigned long e, void *v);
+void dsa_broadcast(unsigned long e, void *v);
 int dsa_broadcast_robust(unsigned long e, void *v, unsigned long e_rollback,
 			 void *v_rollback);
 int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 4095592c4790..1452f818263a 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -25,9 +25,9 @@
  * reconfigure ports without net_devices (CPU ports, DSA links) whenever
  * a user port's state changes.
  */
-static int dsa_port_notify(const struct dsa_port *dp, unsigned long e, void *v)
+static void dsa_port_notify(const struct dsa_port *dp, unsigned long e, void *v)
 {
-	return dsa_tree_notify(dp->ds->dst, e, v);
+	dsa_tree_notify(dp->ds->dst, e, v);
 }
 
 /**
@@ -551,7 +551,6 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
 	struct dsa_notifier_bridge_info info = {
 		.dp = dp,
 	};
-	int err;
 
 	/* If the port could not be offloaded to begin with, then
 	 * there is nothing to do.
@@ -566,11 +565,7 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
 	 */
 	dsa_port_bridge_destroy(dp, br);
 
-	err = dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info);
-	if (err)
-		dev_err(dp->ds->dev,
-			"port %d failed to notify DSA_NOTIFIER_BRIDGE_LEAVE: %pe\n",
-			dp->index, ERR_PTR(err));
+	dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info);
 
 	dsa_port_switchdev_unsync_attrs(dp, info.bridge);
 }
@@ -598,7 +593,9 @@ int dsa_port_lag_change(struct dsa_port *dp,
 
 	dp->lag_tx_enabled = tx_enabled;
 
-	return dsa_port_notify(dp, DSA_NOTIFIER_LAG_CHANGE, &info);
+	dsa_port_notify(dp, DSA_NOTIFIER_LAG_CHANGE, &info);
+
+	return 0;
 }
 
 static int dsa_port_lag_create(struct dsa_port *dp,
@@ -696,7 +693,6 @@ void dsa_port_lag_leave(struct dsa_port *dp, struct net_device *lag_dev)
 	struct dsa_notifier_lag_info info = {
 		.dp = dp,
 	};
-	int err;
 
 	if (!dp->lag)
 		return;
@@ -711,11 +707,7 @@ void dsa_port_lag_leave(struct dsa_port *dp, struct net_device *lag_dev)
 
 	dsa_port_lag_destroy(dp);
 
-	err = dsa_port_notify(dp, DSA_NOTIFIER_LAG_LEAVE, &info);
-	if (err)
-		dev_err(dp->ds->dev,
-			"port %d failed to notify DSA_NOTIFIER_LAG_LEAVE: %pe\n",
-			dp->index, ERR_PTR(err));
+	dsa_port_notify(dp, DSA_NOTIFIER_LAG_LEAVE, &info);
 }
 
 /* Must be called under rcu_read_lock() */
@@ -1027,7 +1019,9 @@ int dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
 	if (!dp->ds->fdb_isolation)
 		info.db.bridge.num = 0;
 
-	return dsa_port_notify(dp, DSA_NOTIFIER_FDB_DEL, &info);
+	dsa_port_notify(dp, DSA_NOTIFIER_FDB_DEL, &info);
+
+	return 0;
 }
 
 static int dsa_port_host_fdb_add(struct dsa_port *dp,
@@ -1096,7 +1090,9 @@ static int dsa_port_host_fdb_del(struct dsa_port *dp,
 	if (!dp->ds->fdb_isolation)
 		info.db.bridge.num = 0;
 
-	return dsa_port_notify(dp, DSA_NOTIFIER_HOST_FDB_DEL, &info);
+	dsa_port_notify(dp, DSA_NOTIFIER_HOST_FDB_DEL, &info);
+
+	return 0;
 }
 
 int dsa_port_standalone_host_fdb_del(struct dsa_port *dp,
@@ -1165,7 +1161,9 @@ int dsa_port_lag_fdb_del(struct dsa_port *dp, const unsigned char *addr,
 	if (!dp->ds->fdb_isolation)
 		info.db.bridge.num = 0;
 
-	return dsa_port_notify(dp, DSA_NOTIFIER_LAG_FDB_DEL, &info);
+	dsa_port_notify(dp, DSA_NOTIFIER_LAG_FDB_DEL, &info);
+
+	return 0;
 }
 
 int dsa_port_fdb_dump(struct dsa_port *dp, dsa_fdb_dump_cb_t *cb, void *data)
@@ -1213,7 +1211,9 @@ int dsa_port_mdb_del(const struct dsa_port *dp,
 	if (!dp->ds->fdb_isolation)
 		info.db.bridge.num = 0;
 
-	return dsa_port_notify(dp, DSA_NOTIFIER_MDB_DEL, &info);
+	dsa_port_notify(dp, DSA_NOTIFIER_MDB_DEL, &info);
+
+	return 0;
 }
 
 static int dsa_port_host_mdb_add(const struct dsa_port *dp,
@@ -1274,7 +1274,9 @@ static int dsa_port_host_mdb_del(const struct dsa_port *dp,
 	if (!dp->ds->fdb_isolation)
 		info.db.bridge.num = 0;
 
-	return dsa_port_notify(dp, DSA_NOTIFIER_HOST_MDB_DEL, &info);
+	dsa_port_notify(dp, DSA_NOTIFIER_HOST_MDB_DEL, &info);
+
+	return 0;
 }
 
 int dsa_port_standalone_host_mdb_del(const struct dsa_port *dp,
@@ -1327,7 +1329,9 @@ int dsa_port_vlan_del(struct dsa_port *dp,
 		.vlan = vlan,
 	};
 
-	return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, &info);
+	dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, &info);
+
+	return 0;
 }
 
 int dsa_port_host_vlan_add(struct dsa_port *dp,
@@ -1360,15 +1364,12 @@ int dsa_port_host_vlan_del(struct dsa_port *dp,
 		.vlan = vlan,
 	};
 	struct dsa_port *cpu_dp = dp->cpu_dp;
-	int err;
 
-	err = dsa_port_notify(dp, DSA_NOTIFIER_HOST_VLAN_DEL, &info);
-	if (err && err != -EOPNOTSUPP)
-		return err;
+	dsa_port_notify(dp, DSA_NOTIFIER_HOST_VLAN_DEL, &info);
 
 	vlan_vid_del(cpu_dp->master, htons(ETH_P_8021Q), vlan->vid);
 
-	return err;
+	return 0;
 }
 
 int dsa_port_mrp_add(const struct dsa_port *dp,
@@ -1795,14 +1796,9 @@ void dsa_port_tag_8021q_vlan_del(struct dsa_port *dp, u16 vid, bool broadcast)
 		.dp = dp,
 		.vid = vid,
 	};
-	int err;
 
 	if (broadcast)
-		err = dsa_broadcast(DSA_NOTIFIER_TAG_8021Q_VLAN_DEL, &info);
+		dsa_broadcast(DSA_NOTIFIER_TAG_8021Q_VLAN_DEL, &info);
 	else
-		err = dsa_port_notify(dp, DSA_NOTIFIER_TAG_8021Q_VLAN_DEL, &info);
-	if (err)
-		dev_err(dp->ds->dev,
-			"port %d failed to notify tag_8021q VLAN %d deletion: %pe\n",
-			dp->index, vid, ERR_PTR(err));
+		dsa_port_notify(dp, DSA_NOTIFIER_TAG_8021Q_VLAN_DEL, &info);
 }
-- 
2.34.1

[RFC PATCH net-next 10/10] net: dsa: make _del variants of functions return void

[Vladimir Oltean]

Now that we suppress return codes from the cross-chip notifier layer
where we don't have anything sane to do, we can propagate the prototype
chain to void one layer higher, which is to the netdev notifiers and
switchdev notifiers.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/dsa/dsa_priv.h |  44 ++++++++++----------
 net/dsa/port.c     | 101 +++++++++++++++++----------------------------
 net/dsa/slave.c    |  74 ++++++++++++---------------------
 3 files changed, 86 insertions(+), 133 deletions(-)

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 263a07152b07..75aa1bcc432b 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -235,33 +235,33 @@ void dsa_port_mtu_change(struct dsa_port *dp, int new_mtu);
 int dsa_port_mtu_change_robust(struct dsa_port *dp, int new_mtu, int old_mtu);
 int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 		     u16 vid);
-int dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
-		     u16 vid);
+void dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
+		      u16 vid);
 int dsa_port_standalone_host_fdb_add(struct dsa_port *dp,
 				     const unsigned char *addr, u16 vid);
-int dsa_port_standalone_host_fdb_del(struct dsa_port *dp,
-				     const unsigned char *addr, u16 vid);
+void dsa_port_standalone_host_fdb_del(struct dsa_port *dp,
+				      const unsigned char *addr, u16 vid);
 int dsa_port_bridge_host_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 				 u16 vid);
-int dsa_port_bridge_host_fdb_del(struct dsa_port *dp, const unsigned char *addr,
-				 u16 vid);
+void dsa_port_bridge_host_fdb_del(struct dsa_port *dp, const unsigned char *addr,
+				  u16 vid);
 int dsa_port_lag_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 			 u16 vid);
-int dsa_port_lag_fdb_del(struct dsa_port *dp, const unsigned char *addr,
-			 u16 vid);
+void dsa_port_lag_fdb_del(struct dsa_port *dp, const unsigned char *addr,
+			  u16 vid);
 int dsa_port_fdb_dump(struct dsa_port *dp, dsa_fdb_dump_cb_t *cb, void *data);
 int dsa_port_mdb_add(const struct dsa_port *dp,
 		     const struct switchdev_obj_port_mdb *mdb);
-int dsa_port_mdb_del(const struct dsa_port *dp,
-		     const struct switchdev_obj_port_mdb *mdb);
+void dsa_port_mdb_del(const struct dsa_port *dp,
+		      const struct switchdev_obj_port_mdb *mdb);
 int dsa_port_standalone_host_mdb_add(const struct dsa_port *dp,
 				     const struct switchdev_obj_port_mdb *mdb);
-int dsa_port_standalone_host_mdb_del(const struct dsa_port *dp,
-				     const struct switchdev_obj_port_mdb *mdb);
+void dsa_port_standalone_host_mdb_del(const struct dsa_port *dp,
+				      const struct switchdev_obj_port_mdb *mdb);
 int dsa_port_bridge_host_mdb_add(const struct dsa_port *dp,
 				 const struct switchdev_obj_port_mdb *mdb);
-int dsa_port_bridge_host_mdb_del(const struct dsa_port *dp,
-				 const struct switchdev_obj_port_mdb *mdb);
+void dsa_port_bridge_host_mdb_del(const struct dsa_port *dp,
+				  const struct switchdev_obj_port_mdb *mdb);
 int dsa_port_pre_bridge_flags(const struct dsa_port *dp,
 			      struct switchdev_brport_flags flags,
 			      struct netlink_ext_ack *extack);
@@ -271,21 +271,21 @@ int dsa_port_bridge_flags(struct dsa_port *dp,
 int dsa_port_vlan_add(struct dsa_port *dp,
 		      const struct switchdev_obj_port_vlan *vlan,
 		      struct netlink_ext_ack *extack);
-int dsa_port_vlan_del(struct dsa_port *dp,
-		      const struct switchdev_obj_port_vlan *vlan);
+void dsa_port_vlan_del(struct dsa_port *dp,
+		       const struct switchdev_obj_port_vlan *vlan);
 int dsa_port_host_vlan_add(struct dsa_port *dp,
 			   const struct switchdev_obj_port_vlan *vlan,
 			   struct netlink_ext_ack *extack);
-int dsa_port_host_vlan_del(struct dsa_port *dp,
-			   const struct switchdev_obj_port_vlan *vlan);
+void dsa_port_host_vlan_del(struct dsa_port *dp,
+			    const struct switchdev_obj_port_vlan *vlan);
 int dsa_port_mrp_add(const struct dsa_port *dp,
 		     const struct switchdev_obj_mrp *mrp);
-int dsa_port_mrp_del(const struct dsa_port *dp,
-		     const struct switchdev_obj_mrp *mrp);
+void dsa_port_mrp_del(const struct dsa_port *dp,
+		      const struct switchdev_obj_mrp *mrp);
 int dsa_port_mrp_add_ring_role(const struct dsa_port *dp,
 			       const struct switchdev_obj_ring_role_mrp *mrp);
-int dsa_port_mrp_del_ring_role(const struct dsa_port *dp,
-			       const struct switchdev_obj_ring_role_mrp *mrp);
+void dsa_port_mrp_del_ring_role(const struct dsa_port *dp,
+				const struct switchdev_obj_ring_role_mrp *mrp);
 int dsa_port_phylink_create(struct dsa_port *dp);
 int dsa_port_link_register_of(struct dsa_port *dp);
 void dsa_port_link_unregister_of(struct dsa_port *dp);
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 1452f818263a..8ad9261a074e 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -1003,8 +1003,8 @@ int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 				      DSA_NOTIFIER_FDB_DEL, &info);
 }
 
-int dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
-		     u16 vid)
+void dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
+		      u16 vid)
 {
 	struct dsa_notifier_fdb_info info = {
 		.dp = dp,
@@ -1020,8 +1020,6 @@ int dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
 		info.db.bridge.num = 0;
 
 	dsa_port_notify(dp, DSA_NOTIFIER_FDB_DEL, &info);
-
-	return 0;
 }
 
 static int dsa_port_host_fdb_add(struct dsa_port *dp,
@@ -1076,9 +1074,9 @@ int dsa_port_bridge_host_fdb_add(struct dsa_port *dp,
 	return dsa_port_host_fdb_add(dp, addr, vid, db);
 }
 
-static int dsa_port_host_fdb_del(struct dsa_port *dp,
-				 const unsigned char *addr, u16 vid,
-				 struct dsa_db db)
+static void dsa_port_host_fdb_del(struct dsa_port *dp,
+				  const unsigned char *addr, u16 vid,
+				  struct dsa_db db)
 {
 	struct dsa_notifier_fdb_info info = {
 		.dp = dp,
@@ -1091,38 +1089,32 @@ static int dsa_port_host_fdb_del(struct dsa_port *dp,
 		info.db.bridge.num = 0;
 
 	dsa_port_notify(dp, DSA_NOTIFIER_HOST_FDB_DEL, &info);
-
-	return 0;
 }
 
-int dsa_port_standalone_host_fdb_del(struct dsa_port *dp,
-				     const unsigned char *addr, u16 vid)
+void dsa_port_standalone_host_fdb_del(struct dsa_port *dp,
+				      const unsigned char *addr, u16 vid)
 {
 	struct dsa_db db = {
 		.type = DSA_DB_PORT,
 		.dp = dp,
 	};
 
-	return dsa_port_host_fdb_del(dp, addr, vid, db);
+	dsa_port_host_fdb_del(dp, addr, vid, db);
 }
 
-int dsa_port_bridge_host_fdb_del(struct dsa_port *dp,
-				 const unsigned char *addr, u16 vid)
+void dsa_port_bridge_host_fdb_del(struct dsa_port *dp,
+				  const unsigned char *addr, u16 vid)
 {
 	struct dsa_port *cpu_dp = dp->cpu_dp;
 	struct dsa_db db = {
 		.type = DSA_DB_BRIDGE,
 		.bridge = *dp->bridge,
 	};
-	int err;
 
-	if (cpu_dp->master->priv_flags & IFF_UNICAST_FLT) {
-		err = dev_uc_del(cpu_dp->master, addr);
-		if (err)
-			return err;
-	}
+	if (cpu_dp->master->priv_flags & IFF_UNICAST_FLT)
+		dev_uc_del(cpu_dp->master, addr);
 
-	return dsa_port_host_fdb_del(dp, addr, vid, db);
+	dsa_port_host_fdb_del(dp, addr, vid, db);
 }
 
 int dsa_port_lag_fdb_add(struct dsa_port *dp, const unsigned char *addr,
@@ -1145,8 +1137,8 @@ int dsa_port_lag_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 				      DSA_NOTIFIER_LAG_FDB_DEL, &info);
 }
 
-int dsa_port_lag_fdb_del(struct dsa_port *dp, const unsigned char *addr,
-			 u16 vid)
+void dsa_port_lag_fdb_del(struct dsa_port *dp, const unsigned char *addr,
+			  u16 vid)
 {
 	struct dsa_notifier_lag_fdb_info info = {
 		.lag = dp->lag,
@@ -1162,8 +1154,6 @@ int dsa_port_lag_fdb_del(struct dsa_port *dp, const unsigned char *addr,
 		info.db.bridge.num = 0;
 
 	dsa_port_notify(dp, DSA_NOTIFIER_LAG_FDB_DEL, &info);
-
-	return 0;
 }
 
 int dsa_port_fdb_dump(struct dsa_port *dp, dsa_fdb_dump_cb_t *cb, void *data)
@@ -1196,8 +1186,8 @@ int dsa_port_mdb_add(const struct dsa_port *dp,
 				      DSA_NOTIFIER_MDB_DEL, &info);
 }
 
-int dsa_port_mdb_del(const struct dsa_port *dp,
-		     const struct switchdev_obj_port_mdb *mdb)
+void dsa_port_mdb_del(const struct dsa_port *dp,
+		      const struct switchdev_obj_port_mdb *mdb)
 {
 	struct dsa_notifier_mdb_info info = {
 		.dp = dp,
@@ -1212,8 +1202,6 @@ int dsa_port_mdb_del(const struct dsa_port *dp,
 		info.db.bridge.num = 0;
 
 	dsa_port_notify(dp, DSA_NOTIFIER_MDB_DEL, &info);
-
-	return 0;
 }
 
 static int dsa_port_host_mdb_add(const struct dsa_port *dp,
@@ -1261,9 +1249,9 @@ int dsa_port_bridge_host_mdb_add(const struct dsa_port *dp,
 	return dsa_port_host_mdb_add(dp, mdb, db);
 }
 
-static int dsa_port_host_mdb_del(const struct dsa_port *dp,
-				 const struct switchdev_obj_port_mdb *mdb,
-				 struct dsa_db db)
+static void dsa_port_host_mdb_del(const struct dsa_port *dp,
+				  const struct switchdev_obj_port_mdb *mdb,
+				  struct dsa_db db)
 {
 	struct dsa_notifier_mdb_info info = {
 		.dp = dp,
@@ -1275,36 +1263,31 @@ static int dsa_port_host_mdb_del(const struct dsa_port *dp,
 		info.db.bridge.num = 0;
 
 	dsa_port_notify(dp, DSA_NOTIFIER_HOST_MDB_DEL, &info);
-
-	return 0;
 }
 
-int dsa_port_standalone_host_mdb_del(const struct dsa_port *dp,
-				     const struct switchdev_obj_port_mdb *mdb)
+void dsa_port_standalone_host_mdb_del(const struct dsa_port *dp,
+				      const struct switchdev_obj_port_mdb *mdb)
 {
 	struct dsa_db db = {
 		.type = DSA_DB_PORT,
 		.dp = dp,
 	};
 
-	return dsa_port_host_mdb_del(dp, mdb, db);
+	dsa_port_host_mdb_del(dp, mdb, db);
 }
 
-int dsa_port_bridge_host_mdb_del(const struct dsa_port *dp,
-				 const struct switchdev_obj_port_mdb *mdb)
+void dsa_port_bridge_host_mdb_del(const struct dsa_port *dp,
+				  const struct switchdev_obj_port_mdb *mdb)
 {
 	struct dsa_port *cpu_dp = dp->cpu_dp;
 	struct dsa_db db = {
 		.type = DSA_DB_BRIDGE,
 		.bridge = *dp->bridge,
 	};
-	int err;
 
-	err = dev_mc_del(cpu_dp->master, mdb->addr);
-	if (err)
-		return err;
+	dev_mc_del(cpu_dp->master, mdb->addr);
 
-	return dsa_port_host_mdb_del(dp, mdb, db);
+	dsa_port_host_mdb_del(dp, mdb, db);
 }
 
 int dsa_port_vlan_add(struct dsa_port *dp,
@@ -1321,8 +1304,8 @@ int dsa_port_vlan_add(struct dsa_port *dp,
 				      DSA_NOTIFIER_VLAN_DEL, &info);
 }
 
-int dsa_port_vlan_del(struct dsa_port *dp,
-		      const struct switchdev_obj_port_vlan *vlan)
+void dsa_port_vlan_del(struct dsa_port *dp,
+		       const struct switchdev_obj_port_vlan *vlan)
 {
 	struct dsa_notifier_vlan_info info = {
 		.dp = dp,
@@ -1330,8 +1313,6 @@ int dsa_port_vlan_del(struct dsa_port *dp,
 	};
 
 	dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, &info);
-
-	return 0;
 }
 
 int dsa_port_host_vlan_add(struct dsa_port *dp,
@@ -1356,8 +1337,8 @@ int dsa_port_host_vlan_add(struct dsa_port *dp,
 	return err;
 }
 
-int dsa_port_host_vlan_del(struct dsa_port *dp,
-			   const struct switchdev_obj_port_vlan *vlan)
+void dsa_port_host_vlan_del(struct dsa_port *dp,
+			    const struct switchdev_obj_port_vlan *vlan)
 {
 	struct dsa_notifier_vlan_info info = {
 		.dp = dp,
@@ -1368,8 +1349,6 @@ int dsa_port_host_vlan_del(struct dsa_port *dp,
 	dsa_port_notify(dp, DSA_NOTIFIER_HOST_VLAN_DEL, &info);
 
 	vlan_vid_del(cpu_dp->master, htons(ETH_P_8021Q), vlan->vid);
-
-	return 0;
 }
 
 int dsa_port_mrp_add(const struct dsa_port *dp,
@@ -1383,15 +1362,13 @@ int dsa_port_mrp_add(const struct dsa_port *dp,
 	return ds->ops->port_mrp_add(ds, dp->index, mrp);
 }
 
-int dsa_port_mrp_del(const struct dsa_port *dp,
-		     const struct switchdev_obj_mrp *mrp)
+void dsa_port_mrp_del(const struct dsa_port *dp,
+		      const struct switchdev_obj_mrp *mrp)
 {
 	struct dsa_switch *ds = dp->ds;
 
-	if (!ds->ops->port_mrp_del)
-		return -EOPNOTSUPP;
-
-	return ds->ops->port_mrp_del(ds, dp->index, mrp);
+	if (ds->ops->port_mrp_del)
+		ds->ops->port_mrp_del(ds, dp->index, mrp);
 }
 
 int dsa_port_mrp_add_ring_role(const struct dsa_port *dp,
@@ -1405,15 +1382,13 @@ int dsa_port_mrp_add_ring_role(const struct dsa_port *dp,
 	return ds->ops->port_mrp_add_ring_role(ds, dp->index, mrp);
 }
 
-int dsa_port_mrp_del_ring_role(const struct dsa_port *dp,
-			       const struct switchdev_obj_ring_role_mrp *mrp)
+void dsa_port_mrp_del_ring_role(const struct dsa_port *dp,
+				const struct switchdev_obj_ring_role_mrp *mrp)
 {
 	struct dsa_switch *ds = dp->ds;
 
 	if (!ds->ops->port_mrp_del_ring_role)
-		return -EOPNOTSUPP;
-
-	return ds->ops->port_mrp_del_ring_role(ds, dp->index, mrp);
+		ds->ops->port_mrp_del_ring_role(ds, dp->index, mrp);
 }
 
 void dsa_port_set_tag_protocol(struct dsa_port *cpu_dp,
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 02cc1774888a..776c58a1795b 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -48,14 +48,9 @@ static void dsa_slave_standalone_event_work(struct work_struct *work)
 		break;
 
 	case DSA_UC_DEL:
-		err = dsa_port_standalone_host_fdb_del(dp, addr, vid);
-		if (err) {
-			dev_err(ds->dev,
-				"port %d failed to delete %pM vid %d from fdb: %d\n",
-				dp->index, addr, vid, err);
-		}
-
+		dsa_port_standalone_host_fdb_del(dp, addr, vid);
 		break;
+
 	case DSA_MC_ADD:
 		ether_addr_copy(mdb.addr, addr);
 		mdb.vid = vid;
@@ -72,13 +67,7 @@ static void dsa_slave_standalone_event_work(struct work_struct *work)
 		ether_addr_copy(mdb.addr, addr);
 		mdb.vid = vid;
 
-		err = dsa_port_standalone_host_mdb_del(dp, &mdb);
-		if (err) {
-			dev_err(ds->dev,
-				"port %d failed to delete %pM vid %d from mdb: %d\n",
-				dp->index, addr, vid, err);
-		}
-
+		dsa_port_standalone_host_mdb_del(dp, &mdb);
 		break;
 	}
 
@@ -636,43 +625,42 @@ static int dsa_slave_port_obj_add(struct net_device *dev, const void *ctx,
 	return err;
 }
 
-static int dsa_slave_vlan_del(struct net_device *dev,
-			      const struct switchdev_obj *obj)
+static void dsa_slave_vlan_del(struct net_device *dev,
+			       const struct switchdev_obj *obj)
 {
 	struct dsa_port *dp = dsa_slave_to_port(dev);
 	struct switchdev_obj_port_vlan *vlan;
 
 	if (dsa_port_skip_vlan_configuration(dp))
-		return 0;
+		return;
 
 	vlan = SWITCHDEV_OBJ_PORT_VLAN(obj);
 
-	return dsa_port_vlan_del(dp, vlan);
+	dsa_port_vlan_del(dp, vlan);
 }
 
-static int dsa_slave_host_vlan_del(struct net_device *dev,
-				   const struct switchdev_obj *obj)
+static void dsa_slave_host_vlan_del(struct net_device *dev,
+				    const struct switchdev_obj *obj)
 {
 	struct dsa_port *dp = dsa_slave_to_port(dev);
 	struct switchdev_obj_port_vlan *vlan;
 
 	/* Do nothing if this is a software bridge */
 	if (!dp->bridge)
-		return -EOPNOTSUPP;
+		return;
 
 	if (dsa_port_skip_vlan_configuration(dp))
-		return 0;
+		return;
 
 	vlan = SWITCHDEV_OBJ_PORT_VLAN(obj);
 
-	return dsa_port_host_vlan_del(dp, vlan);
+	dsa_port_host_vlan_del(dp, vlan);
 }
 
 static int dsa_slave_port_obj_del(struct net_device *dev, const void *ctx,
 				  const struct switchdev_obj *obj)
 {
 	struct dsa_port *dp = dsa_slave_to_port(dev);
-	int err;
 
 	if (ctx && ctx != dp)
 		return 0;
@@ -682,39 +670,37 @@ static int dsa_slave_port_obj_del(struct net_device *dev, const void *ctx,
 		if (!dsa_port_offloads_bridge_port(dp, obj->orig_dev))
 			return -EOPNOTSUPP;
 
-		err = dsa_port_mdb_del(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
+		dsa_port_mdb_del(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
 		break;
 	case SWITCHDEV_OBJ_ID_HOST_MDB:
 		if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
 			return -EOPNOTSUPP;
 
-		err = dsa_port_bridge_host_mdb_del(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
+		dsa_port_bridge_host_mdb_del(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
 		break;
 	case SWITCHDEV_OBJ_ID_PORT_VLAN:
 		if (dsa_port_offloads_bridge_port(dp, obj->orig_dev))
-			err = dsa_slave_vlan_del(dev, obj);
+			dsa_slave_vlan_del(dev, obj);
 		else
-			err = dsa_slave_host_vlan_del(dev, obj);
+			dsa_slave_host_vlan_del(dev, obj);
 		break;
 	case SWITCHDEV_OBJ_ID_MRP:
 		if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
 			return -EOPNOTSUPP;
 
-		err = dsa_port_mrp_del(dp, SWITCHDEV_OBJ_MRP(obj));
+		dsa_port_mrp_del(dp, SWITCHDEV_OBJ_MRP(obj));
 		break;
 	case SWITCHDEV_OBJ_ID_RING_ROLE_MRP:
 		if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
 			return -EOPNOTSUPP;
 
-		err = dsa_port_mrp_del_ring_role(dp,
-						 SWITCHDEV_OBJ_RING_ROLE_MRP(obj));
+		dsa_port_mrp_del_ring_role(dp, SWITCHDEV_OBJ_RING_ROLE_MRP(obj));
 		break;
 	default:
-		err = -EOPNOTSUPP;
-		break;
+		return -EOPNOTSUPP;
 	}
 
-	return err;
+	return 0;
 }
 
 static inline netdev_tx_t dsa_slave_netpoll_send_skb(struct net_device *dev,
@@ -1611,13 +1597,11 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
 		/* This API only allows programming tagged, non-PVID VIDs */
 		.flags = 0,
 	};
-	int err;
 
-	err = dsa_port_vlan_del(dp, &vlan);
-	if (err)
-		return err;
+	dsa_port_vlan_del(dp, &vlan);
+	dsa_port_host_vlan_del(dp, &vlan);
 
-	return dsa_port_host_vlan_del(dp, &vlan);
+	return 0;
 }
 
 static int dsa_slave_restore_vlan(struct net_device *vdev, int vid, void *arg)
@@ -2837,17 +2821,11 @@ static void dsa_slave_switchdev_event_work(struct work_struct *work)
 
 	case SWITCHDEV_FDB_DEL_TO_DEVICE:
 		if (switchdev_work->host_addr)
-			err = dsa_port_bridge_host_fdb_del(dp, addr, vid);
+			dsa_port_bridge_host_fdb_del(dp, addr, vid);
 		else if (dp->lag)
-			err = dsa_port_lag_fdb_del(dp, addr, vid);
+			dsa_port_lag_fdb_del(dp, addr, vid);
 		else
-			err = dsa_port_fdb_del(dp, addr, vid);
-		if (err) {
-			dev_err(ds->dev,
-				"port %d failed to delete %pM vid %d from fdb: %d\n",
-				dp->index, addr, vid, err);
-		}
-
+			dsa_port_fdb_del(dp, addr, vid);
 		break;
 	}
 
-- 
2.34.1

Re: [RFC PATCH net-next 00/10] Use robust notifiers in DSA

[Andrew Lunn]

> I am posting this as RFC because something still feels off, but I can't
> exactly pinpoint what, and I'm looking for some feedback. Since most DSA
> switches are behind I/O protocols that can fail or time out (SPI, I2C,
> MDIO etc), everything can fail; that's a fact. On the other hand, when
> a network device or the entire system is torn down, nobody cares that
> SPI I/O failed - the system is still shutting down; that is also a fact.
> I'm not quite sure how to reconcile the two. On one hand we're
> suppressing errors emitted by DSA drivers in the non-robust form of
> notifiers, and on the other hand there's nothing we can do about them
> either way (upper layers don't necessarily care).

I would split it into two classes of errors:

Bus transactions fail. This very likely means the hardware design is
bad, connectors are loose, etc. There is not much we can do about
this, bad things are going to happen no what.

We have consumed all of some sort of resource. Out of memory, the ATU
is full, too many LAGs, etc. We try to roll back in order to get out
of this resource problem.

So i would say -EIO, -ETIMEDOUT, we don't care about too
much. -ENOMEM, -ENOBUF, -EOPNOTSUPP or whatever, we should try to do a
robust rollback.

The original design of switchdev was two phase:

1) Allocate whatever resources are needed, can fail
2) Put those resources into use, must not fail

At some point that all got thrown away.

	Andrew

Re: [RFC PATCH net-next 00/10] Use robust notifiers in DSA

[Vladimir Oltean]

On Thu, Aug 18, 2022 at 11:49:24PM +0200, Andrew Lunn wrote:
> I would split it into two classes of errors:
> 
> Bus transactions fail. This very likely means the hardware design is
> bad, connectors are loose, etc. There is not much we can do about
> this, bad things are going to happen no what.
> 
> We have consumed all of some sort of resource. Out of memory, the ATU
> is full, too many LAGs, etc. We try to roll back in order to get out
> of this resource problem.
> 
> So i would say -EIO, -ETIMEDOUT, we don't care about too
> much. -ENOMEM, -ENOBUF, -EOPNOTSUPP or whatever, we should try to do a
> robust rollback.
> 
> The original design of switchdev was two phase:
> 
> 1) Allocate whatever resources are needed, can fail
> 2) Put those resources into use, must not fail
> 
> At some point that all got thrown away.

So you think that rollback at the cross-chip notifier layer is a new
problem we need to tackle, because we don't have enough transactional
layering in the code?

In case you don't remember how that utopia dug itself into a hole in practice:
nobody (not even DSA) used the switchdev transactional item queue (which
passed allocated resources between the prepare and the commit phase)
from its introduction in 2015 until it was deleted in 2019, and then
drivers were left unable to reclaim the memory they allocated during
preparation, if the code path never came to the commit stage. There was
nothing left to do except to throw it away.

To discover whether the ATU is full, you either need to reserve space
for static entries beforehand, which is inefficient, or just try to add
what you want and see if you could. Which inevitably leads to encouraging
the strategy of doing the work in the preparation phase and nothing in
the commit phase.

"Too many X" where the resource limitation is known beforehand is about
the only case where a prepare/commit phase could avoid useless rollback.
It's also a case which could also be solved by being upfront about the
limitation to your higher layer, then it would not even try at all.
And do note that "less useless rollback" is different than "code gives
more guarantees that system will remain in a known state".

Sadly reality is much more dynamic than "allocate" -> can fail / "use" ->
must not fail. I think when the model fails to describe reality, you
change the model, not reality.

Re: [RFC PATCH net-next 00/10] Use robust notifiers in DSA

[Andrew Lunn]

On Fri, Aug 19, 2022 at 01:28:50AM +0300, Vladimir Oltean wrote:
> On Thu, Aug 18, 2022 at 11:49:24PM +0200, Andrew Lunn wrote:
> > I would split it into two classes of errors:
> > 
> > Bus transactions fail. This very likely means the hardware design is
> > bad, connectors are loose, etc. There is not much we can do about
> > this, bad things are going to happen no what.
> > 
> > We have consumed all of some sort of resource. Out of memory, the ATU
> > is full, too many LAGs, etc. We try to roll back in order to get out
> > of this resource problem.
> > 
> > So i would say -EIO, -ETIMEDOUT, we don't care about too
> > much. -ENOMEM, -ENOBUF, -EOPNOTSUPP or whatever, we should try to do a
> > robust rollback.
> > 
> > The original design of switchdev was two phase:
> > 
> > 1) Allocate whatever resources are needed, can fail
> > 2) Put those resources into use, must not fail
> > 
> > At some point that all got thrown away.
> 
> So you think that rollback at the cross-chip notifier layer is a new
> problem we need to tackle, because we don't have enough transactional
> layering in the code?

No, i don't think it is a new problem, but it might help explain why
you don't feel quite right about it. Some errors we simply don't care
about because we cannot do anything about it. Other errors we should
try to rollback, and hence need robust notifiers for those errors.

    Andrew

Re: [RFC PATCH net-next 00/10] Use robust notifiers in DSA

[Vladimir Oltean]

On Fri, Aug 19, 2022 at 12:35:07AM +0200, Andrew Lunn wrote:
> > So you think that rollback at the cross-chip notifier layer is a new
> > problem we need to tackle, because we don't have enough transactional
> > layering in the code?
> 
> No, i don't think it is a new problem, but it might help explain why
> you don't feel quite right about it. Some errors we simply don't care
> about because we cannot do anything about it. Other errors we should
> try to rollback, and hence need robust notifiers for those errors.

So most of the actual errors I've had to handle in the kernel were
caused by half the code (the callee) expecting one thing, and half the
code (the caller) providing another. That doesn't fit well in neither of
your categories, but it's more like how to best treat the unexpected.
And I'm not talking unexpected as in

  switchdev                               dsa
----------------------------------------------------------------------

- Please add MAC 00:01:02:03:04:05
  to the FDB
                                       - Whoa, after all these years, I
                                         never knew you could speak!

but rather

  switchdev                               dsa
----------------------------------------------------------------------

- Please add MAC 00:01:02:03:04:05
  to the FDB
                                       - Sure thing, man!

- Please delete MAC 00:01:02:03:04:05
  from the FDB
                                       - Aye!

- Please delete MAC 00:01:02:03:04:05
  from the FDB
                                       - Wait, what MAC 00:01:02:03:04:05?
                                         I have no such thing!
- Wha?
                                       - Wha?

There's nothing to do about that except to wait for Mr Developer to come
and debug, and the severity of the problem might be low even though the
problem is just as intractable programmatically as a hardware I/O error.
Nonetheless it's still indicative of a problem worth propagating as high
as possible, because one side of the code had expectations of what the
other side could do that were clearly violated, so their models of the
other side are wrong.

This patch set makes that worse for Mr Developer that gets to debug,
because it makes dsa_port_fdb_del() return void, and the errors will now
get reported to the console at the level of dsa_port_notify() and then
suppressed. dsa_port_notify(), being at the low cross-chip notifier
level, won't print all the gory details of the FDB entry that failed to
be deleted and on what port, it will just say that the operation failed
and return void.

That's what felt wrong for me doing this conversion.