netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH net-next 0/9] Get rid of the switchdev transactional model
@ 2020-12-17  1:58 Vladimir Oltean
  2020-12-17  1:58 ` [RFC PATCH net-next 1/9] net: switchdev: remove the transaction structure from port object notifiers Vladimir Oltean
                   ` (10 more replies)
  0 siblings, 11 replies; 26+ messages in thread
From: Vladimir Oltean @ 2020-12-17  1:58 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh,
	Microchip Linux Driver Support, Sean Wang, Landen Chao,
	Claudiu Manoil, Alexandre Belloni, Linus Walleij, Vadym Kochan,
	Taras Chornyi, Jiri Pirko, Ido Schimmel, Grygorii Strashko,
	Ioana Ciornei, Ivan Vecera

This series comes after the late realization that the prepare/commit
separation imposed by switchdev does not help literally anybody:
https://patchwork.kernel.org/project/netdevbpf/patch/20201212203901.351331-1-vladimir.oltean@nxp.com/

We should kill it before it inflicts even more damage to the error
handling logic in drivers.

Vladimir Oltean (9):
  net: switchdev: remove the transaction structure from port object
    notifiers
  net: switchdev: delete switchdev_port_obj_add_now
  net: switchdev: remove the transaction structure from port attributes
  net: dsa: remove the transactional logic from ageing time notifiers
  net: dsa: remove the transactional logic from MDB entries
  net: dsa: remove the transactional logic from VLAN objects
  net: dsa: remove obsolete comment about switchdev transactions
  mlxsw: spectrum_switchdev: remove transactional logic for VLAN objects
  net: switchdev: delete the transaction object

 drivers/net/dsa/b53/b53_common.c              |  42 +++----
 drivers/net/dsa/b53/b53_priv.h                |  15 +--
 drivers/net/dsa/bcm_sf2.c                     |   2 -
 drivers/net/dsa/bcm_sf2_cfp.c                 |   7 +-
 drivers/net/dsa/dsa_loop.c                    |  32 ++----
 drivers/net/dsa/hirschmann/hellcreek.c        |  18 +--
 drivers/net/dsa/lan9303-core.c                |  12 +-
 drivers/net/dsa/lantiq_gswip.c                |  40 +++----
 drivers/net/dsa/microchip/ksz8795.c           |  14 +--
 drivers/net/dsa/microchip/ksz9477.c           |  40 +++----
 drivers/net/dsa/microchip/ksz_common.c        |  23 +---
 drivers/net/dsa/microchip/ksz_common.h        |   8 +-
 drivers/net/dsa/mt7530.c                      |  20 +---
 drivers/net/dsa/mv88e6xxx/chip.c              |  72 ++++++------
 drivers/net/dsa/ocelot/felix.c                |  32 +++---
 drivers/net/dsa/qca8k.c                       |  20 +---
 drivers/net/dsa/realtek-smi-core.h            |   9 +-
 drivers/net/dsa/rtl8366.c                     |  36 +++---
 drivers/net/dsa/rtl8366rb.c                   |   1 -
 drivers/net/dsa/sja1105/sja1105.h             |   3 +-
 drivers/net/dsa/sja1105/sja1105_devlink.c     |   9 +-
 drivers/net/dsa/sja1105/sja1105_main.c        |  48 +++-----
 .../marvell/prestera/prestera_switchdev.c     |  44 ++------
 .../mellanox/mlxsw/spectrum_switchdev.c       | 102 ++++-------------
 drivers/net/ethernet/mscc/ocelot.c            |  32 ++----
 drivers/net/ethernet/mscc/ocelot_net.c        |  39 ++-----
 drivers/net/ethernet/rocker/rocker.h          |   6 +-
 drivers/net/ethernet/rocker/rocker_main.c     |  61 +++--------
 drivers/net/ethernet/rocker/rocker_ofdpa.c    |  23 ++--
 drivers/net/ethernet/ti/cpsw_switchdev.c      |  37 ++-----
 drivers/staging/fsl-dpaa2/ethsw/ethsw.c       |  80 +++++---------
 include/net/dsa.h                             |  11 +-
 include/net/switchdev.h                       |  24 +---
 include/soc/mscc/ocelot.h                     |   3 +-
 net/dsa/dsa_priv.h                            |  27 ++---
 net/dsa/port.c                                | 103 +++++++-----------
 net/dsa/slave.c                               |  56 +++-------
 net/dsa/switch.c                              |  74 ++-----------
 net/switchdev/switchdev.c                     | 101 ++---------------
 39 files changed, 408 insertions(+), 918 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [RFC PATCH net-next 1/9] net: switchdev: remove the transaction structure from port object notifiers
  2020-12-17  1:58 [RFC PATCH net-next 0/9] Get rid of the switchdev transactional model Vladimir Oltean
@ 2020-12-17  1:58 ` Vladimir Oltean
  2020-12-17  2:14   ` Florian Fainelli
  2020-12-17  1:58 ` [RFC PATCH net-next 2/9] net: switchdev: delete switchdev_port_obj_add_now Vladimir Oltean
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 26+ messages in thread
From: Vladimir Oltean @ 2020-12-17  1:58 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh,
	Microchip Linux Driver Support, Sean Wang, Landen Chao,
	Claudiu Manoil, Alexandre Belloni, Linus Walleij, Vadym Kochan,
	Taras Chornyi, Jiri Pirko, Ido Schimmel, Grygorii Strashko,
	Ioana Ciornei, Ivan Vecera

Since the introduction of the switchdev API, port objects were
transmitted to drivers for offloading using a two-step transactional
model, with a prepare phase that was supposed to catch all errors, and a
commit phase that was supposed to never fail.

Some classes of failures can never be avoided, like hardware access, or
memory allocation. In the latter case, merely attempting to move the
memory allocation to the preparation phase makes it impossible to avoid
memory leaks, since commit 91cf8eceffc1 ("switchdev: Remove unused
transaction item queue") which has removed the unused mechanism of
passing on the allocated memory between one phase and another.

It is time we admit that separating the preparation from the commit
phase is something that is best left for the driver to decide, and not
something that should be baked into the API, especially since there are
no switchdev callers that depend on this.

This patch removes the struct switchdev_trans member from switchdev port
object notifier structures, and converts drivers to not look at this
member.

Where driver conversion is trivial (like in the case of the Marvell
Prestera driver, NXP DPAA2 switch, TI CPSW, and Rocker drivers), it is
done in this patch.

Where driver conversion needs more attention (DSA, Mellanox Spectrum),
the conversion is left for subsequent patches and here we only fake the
prepare/commit phases at a lower level, just not in the switchdev
notifier itself.

Where the code has a natural structure that is best left alone as a
preparation and a commit phase (as in the case of the Ocelot switch),
that structure is left in place, just made to not depend upon the
switchdev transactional model.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 .../marvell/prestera/prestera_switchdev.c     |  7 +--
 .../mellanox/mlxsw/spectrum_switchdev.c       | 51 +++++++++-------
 drivers/net/ethernet/mscc/ocelot_net.c        | 26 +++-----
 drivers/net/ethernet/rocker/rocker_main.c     | 15 ++---
 drivers/net/ethernet/ti/cpsw_switchdev.c      | 17 ++----
 drivers/staging/fsl-dpaa2/ethsw/ethsw.c       | 59 ++++++++-----------
 include/net/switchdev.h                       |  3 -
 net/dsa/dsa_priv.h                            |  6 +-
 net/dsa/port.c                                | 28 +++++++--
 net/dsa/slave.c                               | 34 +++--------
 net/switchdev/switchdev.c                     | 42 ++-----------
 11 files changed, 109 insertions(+), 179 deletions(-)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c b/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c
index 7d83e1f91ef1..e92fa4329de2 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c
@@ -1020,7 +1020,6 @@ prestera_bridge_port_vlan_del(struct prestera_port *port,
 
 static int prestera_port_vlans_add(struct prestera_port *port,
 				   const struct switchdev_obj_port_vlan *vlan,
-				   struct switchdev_trans *trans,
 				   struct netlink_ext_ack *extack)
 {
 	bool flag_untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
@@ -1034,9 +1033,6 @@ static int prestera_port_vlans_add(struct prestera_port *port,
 	if (netif_is_bridge_master(dev))
 		return 0;
 
-	if (switchdev_trans_ph_commit(trans))
-		return 0;
-
 	br_port = prestera_bridge_port_by_dev(sw->swdev, dev);
 	if (WARN_ON(!br_port))
 		return -EINVAL;
@@ -1060,7 +1056,6 @@ static int prestera_port_vlans_add(struct prestera_port *port,
 
 static int prestera_port_obj_add(struct net_device *dev,
 				 const struct switchdev_obj *obj,
-				 struct switchdev_trans *trans,
 				 struct netlink_ext_ack *extack)
 {
 	struct prestera_port *port = netdev_priv(dev);
@@ -1069,7 +1064,7 @@ static int prestera_port_obj_add(struct net_device *dev,
 	switch (obj->id) {
 	case SWITCHDEV_OBJ_ID_PORT_VLAN:
 		vlan = SWITCHDEV_OBJ_PORT_VLAN(obj);
-		return prestera_port_vlans_add(port, vlan, trans, extack);
+		return prestera_port_vlans_add(port, vlan, extack);
 	default:
 		return -EOPNOTSUPP;
 	}
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index cea42f6ed89b..16129edf1751 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -1716,8 +1716,7 @@ static int mlxsw_sp_port_remove_from_mid(struct mlxsw_sp_port *mlxsw_sp_port,
 }
 
 static int mlxsw_sp_port_mdb_add(struct mlxsw_sp_port *mlxsw_sp_port,
-				 const struct switchdev_obj_port_mdb *mdb,
-				 struct switchdev_trans *trans)
+				 const struct switchdev_obj_port_mdb *mdb)
 {
 	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
 	struct net_device *orig_dev = mdb->obj.orig_dev;
@@ -1729,9 +1728,6 @@ static int mlxsw_sp_port_mdb_add(struct mlxsw_sp_port *mlxsw_sp_port,
 	u16 fid_index;
 	int err = 0;
 
-	if (switchdev_trans_ph_commit(trans))
-		return 0;
-
 	bridge_port = mlxsw_sp_bridge_port_find(mlxsw_sp->bridge, orig_dev);
 	if (!bridge_port)
 		return 0;
@@ -1813,32 +1809,37 @@ mlxsw_sp_port_mrouter_update_mdb(struct mlxsw_sp_port *mlxsw_sp_port,
 
 static int mlxsw_sp_port_obj_add(struct net_device *dev,
 				 const struct switchdev_obj *obj,
-				 struct switchdev_trans *trans,
 				 struct netlink_ext_ack *extack)
 {
 	struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(dev);
 	const struct switchdev_obj_port_vlan *vlan;
+	struct switchdev_trans trans;
 	int err = 0;
 
 	switch (obj->id) {
 	case SWITCHDEV_OBJ_ID_PORT_VLAN:
 		vlan = SWITCHDEV_OBJ_PORT_VLAN(obj);
-		err = mlxsw_sp_port_vlans_add(mlxsw_sp_port, vlan, trans,
+
+		trans.ph_prepare = true;
+		err = mlxsw_sp_port_vlans_add(mlxsw_sp_port, vlan, &trans,
 					      extack);
+		if (err)
+			break;
 
-		if (switchdev_trans_ph_prepare(trans)) {
-			/* The event is emitted before the changes are actually
-			 * applied to the bridge. Therefore schedule the respin
-			 * call for later, so that the respin logic sees the
-			 * updated bridge state.
-			 */
-			mlxsw_sp_span_respin(mlxsw_sp_port->mlxsw_sp);
-		}
+		/* The event is emitted before the changes are actually
+		 * applied to the bridge. Therefore schedule the respin
+		 * call for later, so that the respin logic sees the
+		 * updated bridge state.
+		 */
+		mlxsw_sp_span_respin(mlxsw_sp_port->mlxsw_sp);
+
+		trans.ph_prepare = false;
+		err = mlxsw_sp_port_vlans_add(mlxsw_sp_port, vlan, &trans,
+					      extack);
 		break;
 	case SWITCHDEV_OBJ_ID_PORT_MDB:
 		err = mlxsw_sp_port_mdb_add(mlxsw_sp_port,
-					    SWITCHDEV_OBJ_PORT_MDB(obj),
-					    trans);
+					    SWITCHDEV_OBJ_PORT_MDB(obj));
 		break;
 	default:
 		err = -EOPNOTSUPP;
@@ -3400,13 +3401,13 @@ mlxsw_sp_switchdev_vxlan_vlan_del(struct mlxsw_sp *mlxsw_sp,
 static int
 mlxsw_sp_switchdev_vxlan_vlans_add(struct net_device *vxlan_dev,
 				   struct switchdev_notifier_port_obj_info *
-				   port_obj_info)
+				   port_obj_info,
+				   struct switchdev_trans *trans)
 {
 	struct switchdev_obj_port_vlan *vlan =
 		SWITCHDEV_OBJ_PORT_VLAN(port_obj_info->obj);
 	bool flag_untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
 	bool flag_pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
-	struct switchdev_trans *trans = port_obj_info->trans;
 	struct mlxsw_sp_bridge_device *bridge_device;
 	struct netlink_ext_ack *extack;
 	struct mlxsw_sp *mlxsw_sp;
@@ -3487,12 +3488,22 @@ mlxsw_sp_switchdev_handle_vxlan_obj_add(struct net_device *vxlan_dev,
 					struct switchdev_notifier_port_obj_info *
 					port_obj_info)
 {
+	struct switchdev_trans trans;
 	int err = 0;
 
 	switch (port_obj_info->obj->id) {
 	case SWITCHDEV_OBJ_ID_PORT_VLAN:
+		trans.ph_prepare = true;
+		err = mlxsw_sp_switchdev_vxlan_vlans_add(vxlan_dev,
+							 port_obj_info,
+							 &trans);
+		if (err)
+			break;
+
+		trans.ph_prepare = false;
 		err = mlxsw_sp_switchdev_vxlan_vlans_add(vxlan_dev,
-							 port_obj_info);
+							 port_obj_info,
+							 &trans);
 		break;
 	default:
 		break;
diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index 2bd2840d88bd..5babe2a38041 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -890,8 +890,7 @@ static int ocelot_port_attr_set(struct net_device *dev,
 }
 
 static int ocelot_port_obj_add_vlan(struct net_device *dev,
-				    const struct switchdev_obj_port_vlan *vlan,
-				    struct switchdev_trans *trans)
+				    const struct switchdev_obj_port_vlan *vlan)
 {
 	int ret;
 	u16 vid;
@@ -900,11 +899,11 @@ static int ocelot_port_obj_add_vlan(struct net_device *dev,
 		bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
 		bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
 
-		if (switchdev_trans_ph_prepare(trans))
-			ret = ocelot_vlan_vid_prepare(dev, vid, pvid,
-						      untagged);
-		else
-			ret = ocelot_vlan_vid_add(dev, vid, pvid, untagged);
+		ret = ocelot_vlan_vid_prepare(dev, vid, pvid, untagged);
+		if (ret)
+			return ret;
+
+		ret = ocelot_vlan_vid_add(dev, vid, pvid, untagged);
 		if (ret)
 			return ret;
 	}
@@ -929,17 +928,13 @@ static int ocelot_port_vlan_del_vlan(struct net_device *dev,
 }
 
 static int ocelot_port_obj_add_mdb(struct net_device *dev,
-				   const struct switchdev_obj_port_mdb *mdb,
-				   struct switchdev_trans *trans)
+				   const struct switchdev_obj_port_mdb *mdb)
 {
 	struct ocelot_port_private *priv = netdev_priv(dev);
 	struct ocelot_port *ocelot_port = &priv->port;
 	struct ocelot *ocelot = ocelot_port->ocelot;
 	int port = priv->chip_port;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	return ocelot_port_mdb_add(ocelot, port, mdb);
 }
 
@@ -956,7 +951,6 @@ static int ocelot_port_obj_del_mdb(struct net_device *dev,
 
 static int ocelot_port_obj_add(struct net_device *dev,
 			       const struct switchdev_obj *obj,
-			       struct switchdev_trans *trans,
 			       struct netlink_ext_ack *extack)
 {
 	int ret = 0;
@@ -964,12 +958,10 @@ static int ocelot_port_obj_add(struct net_device *dev,
 	switch (obj->id) {
 	case SWITCHDEV_OBJ_ID_PORT_VLAN:
 		ret = ocelot_port_obj_add_vlan(dev,
-					       SWITCHDEV_OBJ_PORT_VLAN(obj),
-					       trans);
+					       SWITCHDEV_OBJ_PORT_VLAN(obj));
 		break;
 	case SWITCHDEV_OBJ_ID_PORT_MDB:
-		ret = ocelot_port_obj_add_mdb(dev, SWITCHDEV_OBJ_PORT_MDB(obj),
-					      trans);
+		ret = ocelot_port_obj_add_mdb(dev, SWITCHDEV_OBJ_PORT_MDB(obj));
 		break;
 	default:
 		return -EOPNOTSUPP;
diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c
index dd0bc7f0aaee..1018d3759316 100644
--- a/drivers/net/ethernet/rocker/rocker_main.c
+++ b/drivers/net/ethernet/rocker/rocker_main.c
@@ -1638,17 +1638,13 @@ rocker_world_port_attr_bridge_ageing_time_set(struct rocker_port *rocker_port,
 
 static int
 rocker_world_port_obj_vlan_add(struct rocker_port *rocker_port,
-			       const struct switchdev_obj_port_vlan *vlan,
-			       struct switchdev_trans *trans)
+			       const struct switchdev_obj_port_vlan *vlan)
 {
 	struct rocker_world_ops *wops = rocker_port->rocker->wops;
 
 	if (!wops->port_obj_vlan_add)
 		return -EOPNOTSUPP;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	return wops->port_obj_vlan_add(rocker_port, vlan);
 }
 
@@ -2102,8 +2098,7 @@ static int rocker_port_attr_set(struct net_device *dev,
 }
 
 static int rocker_port_obj_add(struct net_device *dev,
-			       const struct switchdev_obj *obj,
-			       struct switchdev_trans *trans)
+			       const struct switchdev_obj *obj)
 {
 	struct rocker_port *rocker_port = netdev_priv(dev);
 	int err = 0;
@@ -2111,8 +2106,7 @@ static int rocker_port_obj_add(struct net_device *dev,
 	switch (obj->id) {
 	case SWITCHDEV_OBJ_ID_PORT_VLAN:
 		err = rocker_world_port_obj_vlan_add(rocker_port,
-						     SWITCHDEV_OBJ_PORT_VLAN(obj),
-						     trans);
+						     SWITCHDEV_OBJ_PORT_VLAN(obj));
 		break;
 	default:
 		err = -EOPNOTSUPP;
@@ -2847,8 +2841,7 @@ rocker_switchdev_port_obj_event(unsigned long event, struct net_device *netdev,
 
 	switch (event) {
 	case SWITCHDEV_PORT_OBJ_ADD:
-		err = rocker_port_obj_add(netdev, port_obj_info->obj,
-					  port_obj_info->trans);
+		err = rocker_port_obj_add(netdev, port_obj_info->obj);
 		break;
 	case SWITCHDEV_PORT_OBJ_DEL:
 		err = rocker_port_obj_del(netdev, port_obj_info->obj);
diff --git a/drivers/net/ethernet/ti/cpsw_switchdev.c b/drivers/net/ethernet/ti/cpsw_switchdev.c
index 29747da5c514..48a4f277ee31 100644
--- a/drivers/net/ethernet/ti/cpsw_switchdev.c
+++ b/drivers/net/ethernet/ti/cpsw_switchdev.c
@@ -253,8 +253,7 @@ static int cpsw_port_vlan_del(struct cpsw_priv *priv, u16 vid,
 }
 
 static int cpsw_port_vlans_add(struct cpsw_priv *priv,
-			       const struct switchdev_obj_port_vlan *vlan,
-			       struct switchdev_trans *trans)
+			       const struct switchdev_obj_port_vlan *vlan)
 {
 	bool untag = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
 	struct net_device *orig_dev = vlan->obj.orig_dev;
@@ -268,9 +267,6 @@ static int cpsw_port_vlans_add(struct cpsw_priv *priv,
 	if (cpu_port && !(vlan->flags & BRIDGE_VLAN_INFO_BRENTRY))
 		return 0;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
 		int err;
 
@@ -301,8 +297,7 @@ static int cpsw_port_vlans_del(struct cpsw_priv *priv,
 }
 
 static int cpsw_port_mdb_add(struct cpsw_priv *priv,
-			     struct switchdev_obj_port_mdb *mdb,
-			     struct switchdev_trans *trans)
+			     struct switchdev_obj_port_mdb *mdb)
 
 {
 	struct net_device *orig_dev = mdb->obj.orig_dev;
@@ -311,9 +306,6 @@ static int cpsw_port_mdb_add(struct cpsw_priv *priv,
 	int port_mask;
 	int err;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	if (cpu_port)
 		port_mask = BIT(HOST_PORT_NUM);
 	else
@@ -352,7 +344,6 @@ static int cpsw_port_mdb_del(struct cpsw_priv *priv,
 
 static int cpsw_port_obj_add(struct net_device *ndev,
 			     const struct switchdev_obj *obj,
-			     struct switchdev_trans *trans,
 			     struct netlink_ext_ack *extack)
 {
 	struct switchdev_obj_port_vlan *vlan = SWITCHDEV_OBJ_PORT_VLAN(obj);
@@ -365,11 +356,11 @@ static int cpsw_port_obj_add(struct net_device *ndev,
 
 	switch (obj->id) {
 	case SWITCHDEV_OBJ_ID_PORT_VLAN:
-		err = cpsw_port_vlans_add(priv, vlan, trans);
+		err = cpsw_port_vlans_add(priv, vlan);
 		break;
 	case SWITCHDEV_OBJ_ID_PORT_MDB:
 	case SWITCHDEV_OBJ_ID_HOST_MDB:
-		err = cpsw_port_mdb_add(priv, mdb, trans);
+		err = cpsw_port_mdb_add(priv, mdb);
 		break;
 	default:
 		err = -EOPNOTSUPP;
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index d524e92051a3..d7e6e06418ed 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -975,38 +975,33 @@ static int dpaa2_switch_port_attr_set(struct net_device *netdev,
 }
 
 static int dpaa2_switch_port_vlans_add(struct net_device *netdev,
-				       const struct switchdev_obj_port_vlan *vlan,
-				       struct switchdev_trans *trans)
+				       const struct switchdev_obj_port_vlan *vlan)
 {
 	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
 	struct ethsw_core *ethsw = port_priv->ethsw_data;
 	struct dpsw_attr *attr = &ethsw->sw_attr;
 	int vid, err = 0, new_vlans = 0;
 
-	if (switchdev_trans_ph_prepare(trans)) {
-		for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
-			if (!port_priv->ethsw_data->vlans[vid])
-				new_vlans++;
-
-			/* Make sure that the VLAN is not already configured
-			 * on the switch port
-			 */
-			if (port_priv->vlans[vid] & ETHSW_VLAN_MEMBER)
-				return -EEXIST;
-		}
+	for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
+		if (!port_priv->ethsw_data->vlans[vid])
+			new_vlans++;
 
-		/* Check if there is space for a new VLAN */
-		err = dpsw_get_attributes(ethsw->mc_io, 0, ethsw->dpsw_handle,
-					  &ethsw->sw_attr);
-		if (err) {
-			netdev_err(netdev, "dpsw_get_attributes err %d\n", err);
-			return err;
-		}
-		if (attr->max_vlans - attr->num_vlans < new_vlans)
-			return -ENOSPC;
+		/* Make sure that the VLAN is not already configured
+		 * on the switch port
+		 */
+		if (port_priv->vlans[vid] & ETHSW_VLAN_MEMBER)
+			return -EEXIST;
+	}
 
-		return 0;
+	/* Check if there is space for a new VLAN */
+	err = dpsw_get_attributes(ethsw->mc_io, 0, ethsw->dpsw_handle,
+				  &ethsw->sw_attr);
+	if (err) {
+		netdev_err(netdev, "dpsw_get_attributes err %d\n", err);
+		return err;
 	}
+	if (attr->max_vlans - attr->num_vlans < new_vlans)
+		return -ENOSPC;
 
 	for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
 		if (!port_priv->ethsw_data->vlans[vid]) {
@@ -1043,15 +1038,11 @@ static int dpaa2_switch_port_lookup_address(struct net_device *netdev, int is_uc
 }
 
 static int dpaa2_switch_port_mdb_add(struct net_device *netdev,
-				     const struct switchdev_obj_port_mdb *mdb,
-				     struct switchdev_trans *trans)
+				     const struct switchdev_obj_port_mdb *mdb)
 {
 	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
 	int err;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	/* Check if address is already set on this port */
 	if (dpaa2_switch_port_lookup_address(netdev, 0, mdb->addr))
 		return -EEXIST;
@@ -1070,21 +1061,18 @@ static int dpaa2_switch_port_mdb_add(struct net_device *netdev,
 }
 
 static int dpaa2_switch_port_obj_add(struct net_device *netdev,
-				     const struct switchdev_obj *obj,
-				     struct switchdev_trans *trans)
+				     const struct switchdev_obj *obj)
 {
 	int err;
 
 	switch (obj->id) {
 	case SWITCHDEV_OBJ_ID_PORT_VLAN:
 		err = dpaa2_switch_port_vlans_add(netdev,
-						  SWITCHDEV_OBJ_PORT_VLAN(obj),
-						  trans);
+						  SWITCHDEV_OBJ_PORT_VLAN(obj));
 		break;
 	case SWITCHDEV_OBJ_ID_PORT_MDB:
 		err = dpaa2_switch_port_mdb_add(netdev,
-						SWITCHDEV_OBJ_PORT_MDB(obj),
-						trans);
+						SWITCHDEV_OBJ_PORT_MDB(obj));
 		break;
 	default:
 		err = -EOPNOTSUPP;
@@ -1411,8 +1399,7 @@ static int dpaa2_switch_port_obj_event(unsigned long event,
 
 	switch (event) {
 	case SWITCHDEV_PORT_OBJ_ADD:
-		err = dpaa2_switch_port_obj_add(netdev, port_obj_info->obj,
-						port_obj_info->trans);
+		err = dpaa2_switch_port_obj_add(netdev, port_obj_info->obj);
 		break;
 	case SWITCHDEV_PORT_OBJ_DEL:
 		err = dpaa2_switch_port_obj_del(netdev, port_obj_info->obj);
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 99cd538d6519..daca7a14c1db 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -234,7 +234,6 @@ struct switchdev_notifier_fdb_info {
 struct switchdev_notifier_port_obj_info {
 	struct switchdev_notifier_info info; /* must be first */
 	const struct switchdev_obj *obj;
-	struct switchdev_trans *trans;
 	bool handled;
 };
 
@@ -289,7 +288,6 @@ int switchdev_handle_port_obj_add(struct net_device *dev,
 			bool (*check_cb)(const struct net_device *dev),
 			int (*add_cb)(struct net_device *dev,
 				      const struct switchdev_obj *obj,
-				      struct switchdev_trans *trans,
 				      struct netlink_ext_ack *extack));
 int switchdev_handle_port_obj_del(struct net_device *dev,
 			struct switchdev_notifier_port_obj_info *port_obj_info,
@@ -373,7 +371,6 @@ switchdev_handle_port_obj_add(struct net_device *dev,
 			bool (*check_cb)(const struct net_device *dev),
 			int (*add_cb)(struct net_device *dev,
 				      const struct switchdev_obj *obj,
-				      struct switchdev_trans *trans,
 				      struct netlink_ext_ack *extack))
 {
 	return 0;
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 7c96aae9062c..ec70066413c7 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -148,8 +148,7 @@ int dsa_port_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,
-		     struct switchdev_trans *trans);
+		     const struct switchdev_obj_port_mdb *mdb);
 int dsa_port_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, unsigned long flags,
@@ -159,8 +158,7 @@ int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags,
 int dsa_port_mrouter(struct dsa_port *dp, bool mrouter,
 		     struct switchdev_trans *trans);
 int dsa_port_vlan_add(struct dsa_port *dp,
-		      const struct switchdev_obj_port_vlan *vlan,
-		      struct switchdev_trans *trans);
+		      const struct switchdev_obj_port_vlan *vlan);
 int dsa_port_vlan_del(struct dsa_port *dp,
 		      const struct switchdev_obj_port_vlan *vlan);
 int dsa_port_link_register_of(struct dsa_port *dp);
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 73569c9af3cc..8d173064539c 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -425,16 +425,24 @@ 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,
-		     struct switchdev_trans *trans)
+		     const struct switchdev_obj_port_mdb *mdb)
 {
 	struct dsa_notifier_mdb_info info = {
 		.sw_index = dp->ds->index,
 		.port = dp->index,
-		.trans = trans,
 		.mdb = mdb,
 	};
+	struct switchdev_trans trans;
+	int err;
+
+	info.trans = &trans;
+
+	trans.ph_prepare = true;
+	err = dsa_port_notify(dp, DSA_NOTIFIER_MDB_ADD, &info);
+	if (err)
+		return err;
 
+	trans.ph_prepare = false;
 	return dsa_port_notify(dp, DSA_NOTIFIER_MDB_ADD, &info);
 }
 
@@ -451,16 +459,24 @@ int dsa_port_mdb_del(const struct dsa_port *dp,
 }
 
 int dsa_port_vlan_add(struct dsa_port *dp,
-		      const struct switchdev_obj_port_vlan *vlan,
-		      struct switchdev_trans *trans)
+		      const struct switchdev_obj_port_vlan *vlan)
 {
 	struct dsa_notifier_vlan_info info = {
 		.sw_index = dp->ds->index,
 		.port = dp->index,
-		.trans = trans,
 		.vlan = vlan,
 	};
+	struct switchdev_trans trans;
+	int err;
+
+	info.trans = &trans;
+
+	trans.ph_prepare = true;
+	err = dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info);
+	if (err)
+		return err;
 
+	trans.ph_prepare = false;
 	return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info);
 }
 
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 4a0498bf6c65..d32fda1fed6f 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -326,8 +326,7 @@ dsa_slave_vlan_check_for_8021q_uppers(struct net_device *slave,
 }
 
 static int dsa_slave_vlan_add(struct net_device *dev,
-			      const struct switchdev_obj *obj,
-			      struct switchdev_trans *trans)
+			      const struct switchdev_obj *obj)
 {
 	struct net_device *master = dsa_slave_to_master(dev);
 	struct dsa_port *dp = dsa_slave_to_port(dev);
@@ -345,7 +344,7 @@ static int dsa_slave_vlan_add(struct net_device *dev,
 	/* Deny adding a bridge VLAN when there is already an 802.1Q upper with
 	 * the same VID.
 	 */
-	if (trans->ph_prepare && br_vlan_enabled(dp->bridge_dev)) {
+	if (br_vlan_enabled(dp->bridge_dev)) {
 		rcu_read_lock();
 		err = dsa_slave_vlan_check_for_8021q_uppers(dev, &vlan);
 		rcu_read_unlock();
@@ -353,7 +352,7 @@ static int dsa_slave_vlan_add(struct net_device *dev,
 			return err;
 	}
 
-	err = dsa_port_vlan_add(dp, &vlan, trans);
+	err = dsa_port_vlan_add(dp, &vlan);
 	if (err)
 		return err;
 
@@ -363,7 +362,7 @@ static int dsa_slave_vlan_add(struct net_device *dev,
 	 */
 	vlan.flags &= ~BRIDGE_VLAN_INFO_PVID;
 
-	err = dsa_port_vlan_add(dp->cpu_dp, &vlan, trans);
+	err = dsa_port_vlan_add(dp->cpu_dp, &vlan);
 	if (err)
 		return err;
 
@@ -378,7 +377,6 @@ static int dsa_slave_vlan_add(struct net_device *dev,
 
 static int dsa_slave_port_obj_add(struct net_device *dev,
 				  const struct switchdev_obj *obj,
-				  struct switchdev_trans *trans,
 				  struct netlink_ext_ack *extack)
 {
 	struct dsa_port *dp = dsa_slave_to_port(dev);
@@ -393,17 +391,16 @@ static int dsa_slave_port_obj_add(struct net_device *dev,
 	case SWITCHDEV_OBJ_ID_PORT_MDB:
 		if (obj->orig_dev != dev)
 			return -EOPNOTSUPP;
-		err = dsa_port_mdb_add(dp, SWITCHDEV_OBJ_PORT_MDB(obj), trans);
+		err = dsa_port_mdb_add(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
 		break;
 	case SWITCHDEV_OBJ_ID_HOST_MDB:
 		/* DSA can directly translate this to a normal MDB add,
 		 * but on the CPU port.
 		 */
-		err = dsa_port_mdb_add(dp->cpu_dp, SWITCHDEV_OBJ_PORT_MDB(obj),
-				       trans);
+		err = dsa_port_mdb_add(dp->cpu_dp, SWITCHDEV_OBJ_PORT_MDB(obj));
 		break;
 	case SWITCHDEV_OBJ_ID_PORT_VLAN:
-		err = dsa_slave_vlan_add(dev, obj, trans);
+		err = dsa_slave_vlan_add(dev, obj);
 		break;
 	default:
 		err = -EOPNOTSUPP;
@@ -1294,28 +1291,15 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
 		/* This API only allows programming tagged, non-PVID VIDs */
 		.flags = 0,
 	};
-	struct switchdev_trans trans;
 	int ret;
 
 	/* User port... */
-	trans.ph_prepare = true;
-	ret = dsa_port_vlan_add(dp, &vlan, &trans);
-	if (ret)
-		return ret;
-
-	trans.ph_prepare = false;
-	ret = dsa_port_vlan_add(dp, &vlan, &trans);
+	ret = dsa_port_vlan_add(dp, &vlan);
 	if (ret)
 		return ret;
 
 	/* And CPU port... */
-	trans.ph_prepare = true;
-	ret = dsa_port_vlan_add(dp->cpu_dp, &vlan, &trans);
-	if (ret)
-		return ret;
-
-	trans.ph_prepare = false;
-	ret = dsa_port_vlan_add(dp->cpu_dp, &vlan, &trans);
+	ret = dsa_port_vlan_add(dp->cpu_dp, &vlan);
 	if (ret)
 		return ret;
 
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 23d868545362..a575bb33ee6c 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -221,7 +221,6 @@ static size_t switchdev_obj_size(const struct switchdev_obj *obj)
 static int switchdev_port_obj_notify(enum switchdev_notifier_type nt,
 				     struct net_device *dev,
 				     const struct switchdev_obj *obj,
-				     struct switchdev_trans *trans,
 				     struct netlink_ext_ack *extack)
 {
 	int rc;
@@ -229,7 +228,6 @@ static int switchdev_port_obj_notify(enum switchdev_notifier_type nt,
 
 	struct switchdev_notifier_port_obj_info obj_info = {
 		.obj = obj,
-		.trans = trans,
 		.handled = false,
 	};
 
@@ -248,35 +246,10 @@ static int switchdev_port_obj_add_now(struct net_device *dev,
 				      const struct switchdev_obj *obj,
 				      struct netlink_ext_ack *extack)
 {
-	struct switchdev_trans trans;
-	int err;
-
 	ASSERT_RTNL();
 
-	/* Phase I: prepare for obj add. Driver/device should fail
-	 * here if there are going to be issues in the commit phase,
-	 * such as lack of resources or support.  The driver/device
-	 * should reserve resources needed for the commit phase here,
-	 * but should not commit the obj.
-	 */
-
-	trans.ph_prepare = true;
-	err = switchdev_port_obj_notify(SWITCHDEV_PORT_OBJ_ADD,
-					dev, obj, &trans, extack);
-	if (err)
-		return err;
-
-	/* Phase II: commit obj add.  This cannot fail as a fault
-	 * of driver/device.  If it does, it's a bug in the driver/device
-	 * because the driver said everythings was OK in phase I.
-	 */
-
-	trans.ph_prepare = false;
-	err = switchdev_port_obj_notify(SWITCHDEV_PORT_OBJ_ADD,
-					dev, obj, &trans, extack);
-	WARN(err, "%s: Commit of object (id=%d) failed.\n", dev->name, obj->id);
-
-	return err;
+	return switchdev_port_obj_notify(SWITCHDEV_PORT_OBJ_ADD,
+					 dev, obj, extack);
 }
 
 static void switchdev_port_obj_add_deferred(struct net_device *dev,
@@ -307,10 +280,6 @@ static int switchdev_port_obj_add_defer(struct net_device *dev,
  *	@obj: object to add
  *	@extack: netlink extended ack
  *
- *	Use a 2-phase prepare-commit transaction model to ensure
- *	system is not left in a partially updated state due to
- *	failure from driver/device.
- *
  *	rtnl_lock must be held and must not be in atomic section,
  *	in case SWITCHDEV_F_DEFER flag is not set.
  */
@@ -329,7 +298,7 @@ static int switchdev_port_obj_del_now(struct net_device *dev,
 				      const struct switchdev_obj *obj)
 {
 	return switchdev_port_obj_notify(SWITCHDEV_PORT_OBJ_DEL,
-					 dev, obj, NULL, NULL);
+					 dev, obj, NULL);
 }
 
 static void switchdev_port_obj_del_deferred(struct net_device *dev,
@@ -449,7 +418,6 @@ static int __switchdev_handle_port_obj_add(struct net_device *dev,
 			bool (*check_cb)(const struct net_device *dev),
 			int (*add_cb)(struct net_device *dev,
 				      const struct switchdev_obj *obj,
-				      struct switchdev_trans *trans,
 				      struct netlink_ext_ack *extack))
 {
 	struct netlink_ext_ack *extack;
@@ -462,8 +430,7 @@ static int __switchdev_handle_port_obj_add(struct net_device *dev,
 	if (check_cb(dev)) {
 		/* This flag is only checked if the return value is success. */
 		port_obj_info->handled = true;
-		return add_cb(dev, port_obj_info->obj, port_obj_info->trans,
-			      extack);
+		return add_cb(dev, port_obj_info->obj, extack);
 	}
 
 	/* Switch ports might be stacked under e.g. a LAG. Ignore the
@@ -491,7 +458,6 @@ int switchdev_handle_port_obj_add(struct net_device *dev,
 			bool (*check_cb)(const struct net_device *dev),
 			int (*add_cb)(struct net_device *dev,
 				      const struct switchdev_obj *obj,
-				      struct switchdev_trans *trans,
 				      struct netlink_ext_ack *extack))
 {
 	int err;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [RFC PATCH net-next 2/9] net: switchdev: delete switchdev_port_obj_add_now
  2020-12-17  1:58 [RFC PATCH net-next 0/9] Get rid of the switchdev transactional model Vladimir Oltean
  2020-12-17  1:58 ` [RFC PATCH net-next 1/9] net: switchdev: remove the transaction structure from port object notifiers Vladimir Oltean
@ 2020-12-17  1:58 ` Vladimir Oltean
  2020-12-17  2:14   ` Florian Fainelli
  2020-12-17  1:58 ` [RFC PATCH net-next 3/9] net: switchdev: remove the transaction structure from port attributes Vladimir Oltean
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 26+ messages in thread
From: Vladimir Oltean @ 2020-12-17  1:58 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh,
	Microchip Linux Driver Support, Sean Wang, Landen Chao,
	Claudiu Manoil, Alexandre Belloni, Linus Walleij, Vadym Kochan,
	Taras Chornyi, Jiri Pirko, Ido Schimmel, Grygorii Strashko,
	Ioana Ciornei, Ivan Vecera

After the removal of the transactional model inside
switchdev_port_obj_add_now, it has no added value and we can just call
switchdev_port_obj_notify directly, bypassing this function. Let's
delete it.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/switchdev/switchdev.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index a575bb33ee6c..3509d362056d 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -242,23 +242,15 @@ static int switchdev_port_obj_notify(enum switchdev_notifier_type nt,
 	return 0;
 }
 
-static int switchdev_port_obj_add_now(struct net_device *dev,
-				      const struct switchdev_obj *obj,
-				      struct netlink_ext_ack *extack)
-{
-	ASSERT_RTNL();
-
-	return switchdev_port_obj_notify(SWITCHDEV_PORT_OBJ_ADD,
-					 dev, obj, extack);
-}
-
 static void switchdev_port_obj_add_deferred(struct net_device *dev,
 					    const void *data)
 {
 	const struct switchdev_obj *obj = data;
 	int err;
 
-	err = switchdev_port_obj_add_now(dev, obj, NULL);
+	ASSERT_RTNL();
+	err = switchdev_port_obj_notify(SWITCHDEV_PORT_OBJ_ADD,
+					dev, obj, NULL);
 	if (err && err != -EOPNOTSUPP)
 		netdev_err(dev, "failed (err=%d) to add object (id=%d)\n",
 			   err, obj->id);
@@ -290,7 +282,8 @@ int switchdev_port_obj_add(struct net_device *dev,
 	if (obj->flags & SWITCHDEV_F_DEFER)
 		return switchdev_port_obj_add_defer(dev, obj);
 	ASSERT_RTNL();
-	return switchdev_port_obj_add_now(dev, obj, extack);
+	return switchdev_port_obj_notify(SWITCHDEV_PORT_OBJ_ADD,
+					 dev, obj, extack);
 }
 EXPORT_SYMBOL_GPL(switchdev_port_obj_add);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [RFC PATCH net-next 3/9] net: switchdev: remove the transaction structure from port attributes
  2020-12-17  1:58 [RFC PATCH net-next 0/9] Get rid of the switchdev transactional model Vladimir Oltean
  2020-12-17  1:58 ` [RFC PATCH net-next 1/9] net: switchdev: remove the transaction structure from port object notifiers Vladimir Oltean
  2020-12-17  1:58 ` [RFC PATCH net-next 2/9] net: switchdev: delete switchdev_port_obj_add_now Vladimir Oltean
@ 2020-12-17  1:58 ` Vladimir Oltean
  2020-12-18  8:46   ` Kurt Kanzenbach
  2020-12-27 13:26   ` Linus Walleij
  2020-12-17  1:58 ` [RFC PATCH net-next 4/9] net: dsa: remove the transactional logic from ageing time notifiers Vladimir Oltean
                   ` (7 subsequent siblings)
  10 siblings, 2 replies; 26+ messages in thread
From: Vladimir Oltean @ 2020-12-17  1:58 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh,
	Microchip Linux Driver Support, Sean Wang, Landen Chao,
	Claudiu Manoil, Alexandre Belloni, Linus Walleij, Vadym Kochan,
	Taras Chornyi, Jiri Pirko, Ido Schimmel, Grygorii Strashko,
	Ioana Ciornei, Ivan Vecera

Since the introduction of the switchdev API, port attributes were
transmitted to drivers for offloading using a two-step transactional
model, with a prepare phase that was supposed to catch all errors, and a
commit phase that was supposed to never fail.

Some classes of failures can never be avoided, like hardware access, or
memory allocation. In the latter case, merely attempting to move the
memory allocation to the preparation phase makes it impossible to avoid
memory leaks, since commit 91cf8eceffc1 ("switchdev: Remove unused
transaction item queue") which has removed the unused mechanism of
passing on the allocated memory between one phase and another.

It is time we admit that separating the preparation from the commit
phase is something that is best left for the driver to decide, and not
something that should be baked into the API, especially since there are
no switchdev callers that depend on this.

This patch removes the struct switchdev_trans member from switchdev port
attribute notifier structures, and converts drivers to not look at this
member.

In part, this patch contains a revert of my previous commit 2e554a7a5d8a
("net: dsa: propagate switchdev vlan_filtering prepare phase to
drivers").

For the most part, the conversion was trivial except for:
- Rocker's world implementation based on Broadcom OF-DPA had an odd
  implementation of ofdpa_port_attr_bridge_flags_set. The conversion was
  done mechanically, by pasting the implementation twice, then only
  keeping the code that would get executed during prepare phase on top,
  then only keeping the code that gets executed during the commit phase
  on bottom, then simplifying the resulting code until this was obtained.
- DSA's offloading of STP state, bridge flags, VLAN filtering and
  multicast router could be converted right away. But the ageing time
  could not, so a shim was introduced and this was left for a further
  commit.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/b53/b53_common.c              |  6 +-
 drivers/net/dsa/b53/b53_priv.h                |  3 +-
 drivers/net/dsa/dsa_loop.c                    |  3 +-
 drivers/net/dsa/hirschmann/hellcreek.c        |  6 +-
 drivers/net/dsa/lantiq_gswip.c                | 26 +----
 drivers/net/dsa/microchip/ksz8795.c           |  6 +-
 drivers/net/dsa/microchip/ksz9477.c           |  6 +-
 drivers/net/dsa/mt7530.c                      |  6 +-
 drivers/net/dsa/mv88e6xxx/chip.c              |  7 +-
 drivers/net/dsa/ocelot/felix.c                |  5 +-
 drivers/net/dsa/qca8k.c                       |  6 +-
 drivers/net/dsa/realtek-smi-core.h            |  3 +-
 drivers/net/dsa/rtl8366.c                     | 11 +--
 drivers/net/dsa/sja1105/sja1105.h             |  3 +-
 drivers/net/dsa/sja1105/sja1105_devlink.c     |  9 +-
 drivers/net/dsa/sja1105/sja1105_main.c        | 17 ++--
 .../marvell/prestera/prestera_switchdev.c     | 37 ++-----
 .../mellanox/mlxsw/spectrum_switchdev.c       | 63 +++---------
 drivers/net/ethernet/mscc/ocelot.c            | 32 ++----
 drivers/net/ethernet/mscc/ocelot_net.c        | 13 +--
 drivers/net/ethernet/rocker/rocker.h          |  6 +-
 drivers/net/ethernet/rocker/rocker_main.c     | 46 +++------
 drivers/net/ethernet/rocker/rocker_ofdpa.c    | 23 ++---
 drivers/net/ethernet/ti/cpsw_switchdev.c      | 20 +---
 drivers/staging/fsl-dpaa2/ethsw/ethsw.c       | 21 +---
 include/net/dsa.h                             |  3 +-
 include/net/switchdev.h                       |  7 +-
 include/soc/mscc/ocelot.h                     |  3 +-
 net/dsa/dsa_priv.h                            | 18 ++--
 net/dsa/port.c                                | 97 ++++++++-----------
 net/dsa/slave.c                               | 17 ++--
 net/dsa/switch.c                              | 11 +--
 net/switchdev/switchdev.c                     | 46 +--------
 33 files changed, 162 insertions(+), 424 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 288b5a5c3e0d..ce18ba0b74eb 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1374,14 +1374,10 @@ void b53_phylink_mac_link_up(struct dsa_switch *ds, int port,
 }
 EXPORT_SYMBOL(b53_phylink_mac_link_up);
 
-int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
-		       struct switchdev_trans *trans)
+int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering)
 {
 	struct b53_device *dev = ds->priv;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	b53_enable_vlan(dev, dev->vlan_enabled, vlan_filtering);
 
 	return 0;
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index 7c67409bb186..24893b592216 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -347,8 +347,7 @@ void b53_phylink_mac_link_up(struct dsa_switch *ds, int port,
 			     struct phy_device *phydev,
 			     int speed, int duplex,
 			     bool tx_pause, bool rx_pause);
-int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
-		       struct switchdev_trans *trans);
+int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering);
 int b53_vlan_prepare(struct dsa_switch *ds, int port,
 		     const struct switchdev_obj_port_vlan *vlan);
 void b53_vlan_add(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
index e38906ae8f23..b588614d1e5e 100644
--- a/drivers/net/dsa/dsa_loop.c
+++ b/drivers/net/dsa/dsa_loop.c
@@ -190,8 +190,7 @@ static void dsa_loop_port_stp_state_set(struct dsa_switch *ds, int port,
 }
 
 static int dsa_loop_port_vlan_filtering(struct dsa_switch *ds, int port,
-					bool vlan_filtering,
-					struct switchdev_trans *trans)
+					bool vlan_filtering)
 {
 	dev_dbg(ds->dev, "%s: port: %d, vlan_filtering: %d\n",
 		__func__, port, vlan_filtering);
diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index 6420b76ea37c..88d4f602b21c 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -866,14 +866,10 @@ static int hellcreek_fdb_dump(struct dsa_switch *ds, int port,
 }
 
 static int hellcreek_vlan_filtering(struct dsa_switch *ds, int port,
-				    bool vlan_filtering,
-				    struct switchdev_trans *trans)
+				    bool vlan_filtering)
 {
 	struct hellcreek *hellcreek = ds->priv;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	dev_dbg(hellcreek->dev, "%s VLAN filtering on port %d\n",
 		vlan_filtering ? "Enable" : "Disable", port);
 
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 09701c17f3f6..6f8e46d21981 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -737,23 +737,14 @@ static int gswip_pce_load_microcode(struct gswip_priv *priv)
 }
 
 static int gswip_port_vlan_filtering(struct dsa_switch *ds, int port,
-				     bool vlan_filtering,
-				     struct switchdev_trans *trans)
+				     bool vlan_filtering)
 {
+	struct net_device *bridge = dsa_to_port(ds, port)->bridge_dev;
 	struct gswip_priv *priv = ds->priv;
 
 	/* Do not allow changing the VLAN filtering options while in bridge */
-	if (switchdev_trans_ph_prepare(trans)) {
-		struct net_device *bridge = dsa_to_port(ds, port)->bridge_dev;
-
-		if (!bridge)
-			return 0;
-
-		if (!!(priv->port_vlan_filter & BIT(port)) != vlan_filtering)
-			return -EIO;
-
-		return 0;
-	}
+	if (bridge && !!(priv->port_vlan_filter & BIT(port)) != vlan_filtering)
+		return -EIO;
 
 	if (vlan_filtering) {
 		/* Use port based VLAN tag */
@@ -791,15 +782,8 @@ static int gswip_setup(struct dsa_switch *ds)
 
 	/* disable port fetch/store dma on all ports */
 	for (i = 0; i < priv->hw_info->max_ports; i++) {
-		struct switchdev_trans trans;
-
-		/* Skip the prepare phase, this shouldn't return an error
-		 * during setup.
-		 */
-		trans.ph_prepare = false;
-
 		gswip_port_disable(ds, i);
-		gswip_port_vlan_filtering(ds, i, false, &trans);
+		gswip_port_vlan_filtering(ds, i, false);
 	}
 
 	/* enable Switch */
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index c973db101b72..754a2377989f 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -783,14 +783,10 @@ static void ksz8795_flush_dyn_mac_table(struct ksz_device *dev, int port)
 }
 
 static int ksz8795_port_vlan_filtering(struct dsa_switch *ds, int port,
-				       bool flag,
-				       struct switchdev_trans *trans)
+				       bool flag)
 {
 	struct ksz_device *dev = ds->priv;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	ksz_cfg(dev, S_MIRROR_CTRL, SW_VLAN_ENABLE, flag);
 
 	return 0;
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 42e647b67abd..4e181c09f286 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -493,14 +493,10 @@ static void ksz9477_flush_dyn_mac_table(struct ksz_device *dev, int port)
 }
 
 static int ksz9477_port_vlan_filtering(struct dsa_switch *ds, int port,
-				       bool flag,
-				       struct switchdev_trans *trans)
+				       bool flag)
 {
 	struct ksz_device *dev = ds->priv;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	if (flag) {
 		ksz_port_cfg(dev, port, REG_PORT_LUE_CTRL,
 			     PORT_VLAN_LOOKUP_VID_0, true);
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index a67cac15a724..95ea7d1ecb05 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1376,12 +1376,8 @@ mt7530_vlan_cmd(struct mt7530_priv *priv, enum mt7530_vlan_cmd cmd, u16 vid)
 
 static int
 mt7530_port_vlan_filtering(struct dsa_switch *ds, int port,
-			   bool vlan_filtering,
-			   struct switchdev_trans *trans)
+			   bool vlan_filtering)
 {
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	if (vlan_filtering) {
 		/* The port is being kept as VLAN-unaware port when bridge is
 		 * set up with vlan_filtering not being set, Otherwise, the
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index eafe6bedc692..9206ba37d4de 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1585,16 +1585,15 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
 }
 
 static int mv88e6xxx_port_vlan_filtering(struct dsa_switch *ds, int port,
-					 bool vlan_filtering,
-					 struct switchdev_trans *trans)
+					 bool vlan_filtering)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
 	u16 mode = vlan_filtering ? MV88E6XXX_PORT_CTL2_8021Q_MODE_SECURE :
 		MV88E6XXX_PORT_CTL2_8021Q_MODE_DISABLED;
 	int err;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return mv88e6xxx_max_vid(chip) ? 0 : -EOPNOTSUPP;
+	if (!mv88e6xxx_max_vid(chip))
+		return -EOPNOTSUPP;
 
 	mv88e6xxx_reg_lock(chip);
 	err = mv88e6xxx_port_set_8021q_mode(chip, port, mode);
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 7dc230677b78..acf9507b1af3 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -141,12 +141,11 @@ static int felix_vlan_prepare(struct dsa_switch *ds, int port,
 	return 0;
 }
 
-static int felix_vlan_filtering(struct dsa_switch *ds, int port, bool enabled,
-				struct switchdev_trans *trans)
+static int felix_vlan_filtering(struct dsa_switch *ds, int port, bool enabled)
 {
 	struct ocelot *ocelot = ds->priv;
 
-	return ocelot_port_vlan_filtering(ocelot, port, enabled, trans);
+	return ocelot_port_vlan_filtering(ocelot, port, enabled);
 }
 
 static void felix_vlan_add(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 5bdac669a339..7adf71ebd9fe 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1294,14 +1294,10 @@ qca8k_port_fdb_dump(struct dsa_switch *ds, int port,
 }
 
 static int
-qca8k_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
-			  struct switchdev_trans *trans)
+qca8k_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering)
 {
 	struct qca8k_priv *priv = ds->priv;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	if (vlan_filtering) {
 		qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(port),
 			  QCA8K_PORT_LOOKUP_VLAN_MODE,
diff --git a/drivers/net/dsa/realtek-smi-core.h b/drivers/net/dsa/realtek-smi-core.h
index 6b6a3dec0984..bc7bd47fb037 100644
--- a/drivers/net/dsa/realtek-smi-core.h
+++ b/drivers/net/dsa/realtek-smi-core.h
@@ -131,8 +131,7 @@ int rtl8366_enable_vlan(struct realtek_smi *smi, bool enable);
 int rtl8366_reset_vlan(struct realtek_smi *smi);
 int rtl8366_init_vlan(struct realtek_smi *smi);
 int rtl8366_vlan_filtering(struct dsa_switch *ds, int port,
-			   bool vlan_filtering,
-			   struct switchdev_trans *trans);
+			   bool vlan_filtering);
 int rtl8366_vlan_prepare(struct dsa_switch *ds, int port,
 			 const struct switchdev_obj_port_vlan *vlan);
 void rtl8366_vlan_add(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/rtl8366.c b/drivers/net/dsa/rtl8366.c
index 83d481ef9273..212c2bf87aac 100644
--- a/drivers/net/dsa/rtl8366.c
+++ b/drivers/net/dsa/rtl8366.c
@@ -340,20 +340,15 @@ int rtl8366_init_vlan(struct realtek_smi *smi)
 }
 EXPORT_SYMBOL_GPL(rtl8366_init_vlan);
 
-int rtl8366_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
-			   struct switchdev_trans *trans)
+int rtl8366_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering)
 {
 	struct realtek_smi *smi = ds->priv;
 	struct rtl8366_vlan_4k vlan4k;
 	int ret;
 
 	/* Use VLAN nr port + 1 since VLAN0 is not valid */
-	if (switchdev_trans_ph_prepare(trans)) {
-		if (!smi->ops->is_vlan_valid(smi, port + 1))
-			return -EINVAL;
-
-		return 0;
-	}
+	if (!smi->ops->is_vlan_valid(smi, port + 1))
+		return -EINVAL;
 
 	dev_info(smi->dev, "%s filtering on port %d\n",
 		 vlan_filtering ? "enable" : "disable",
diff --git a/drivers/net/dsa/sja1105/sja1105.h b/drivers/net/dsa/sja1105/sja1105.h
index 4ebc4a5a7b35..d582308c2401 100644
--- a/drivers/net/dsa/sja1105/sja1105.h
+++ b/drivers/net/dsa/sja1105/sja1105.h
@@ -245,8 +245,7 @@ enum sja1105_reset_reason {
 
 int sja1105_static_config_reload(struct sja1105_private *priv,
 				 enum sja1105_reset_reason reason);
-int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled,
-			   struct switchdev_trans *trans);
+int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled);
 void sja1105_frame_memory_partitioning(struct sja1105_private *priv);
 
 /* From sja1105_devlink.c */
diff --git a/drivers/net/dsa/sja1105/sja1105_devlink.c b/drivers/net/dsa/sja1105/sja1105_devlink.c
index 4a2ec395bcb0..b4bf1b10e66c 100644
--- a/drivers/net/dsa/sja1105/sja1105_devlink.c
+++ b/drivers/net/dsa/sja1105/sja1105_devlink.c
@@ -135,7 +135,6 @@ static int sja1105_best_effort_vlan_filtering_set(struct sja1105_private *priv,
 
 	rtnl_lock();
 	for (port = 0; port < ds->num_ports; port++) {
-		struct switchdev_trans trans;
 		struct dsa_port *dp;
 
 		if (!dsa_is_user_port(ds, port))
@@ -144,13 +143,7 @@ static int sja1105_best_effort_vlan_filtering_set(struct sja1105_private *priv,
 		dp = dsa_to_port(ds, port);
 		vlan_filtering = dsa_port_is_vlan_filtering(dp);
 
-		trans.ph_prepare = true;
-		rc = sja1105_vlan_filtering(ds, port, vlan_filtering, &trans);
-		if (rc)
-			break;
-
-		trans.ph_prepare = false;
-		rc = sja1105_vlan_filtering(ds, port, vlan_filtering, &trans);
+		rc = sja1105_vlan_filtering(ds, port, vlan_filtering);
 		if (rc)
 			break;
 	}
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 4ca029650993..dc82c71d1dbf 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2634,8 +2634,7 @@ static int sja1105_vlan_prepare(struct dsa_switch *ds, int port,
  * which can only be partially reconfigured at runtime (and not the TPID).
  * So a switch reset is required.
  */
-int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled,
-			   struct switchdev_trans *trans)
+int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled)
 {
 	struct sja1105_l2_lookup_params_entry *l2_lookup_params;
 	struct sja1105_general_params_entry *general_params;
@@ -2647,16 +2646,12 @@ int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled,
 	u16 tpid, tpid2;
 	int rc;
 
-	if (switchdev_trans_ph_prepare(trans)) {
-		list_for_each_entry(rule, &priv->flow_block.rules, list) {
-			if (rule->type == SJA1105_RULE_VL) {
-				dev_err(ds->dev,
-					"Cannot change VLAN filtering with active VL rules\n");
-				return -EBUSY;
-			}
+	list_for_each_entry(rule, &priv->flow_block.rules, list) {
+		if (rule->type == SJA1105_RULE_VL) {
+			dev_err(ds->dev,
+				"Cannot change VLAN filtering with active VL rules\n");
+			return -EBUSY;
 		}
-
-		return 0;
 	}
 
 	if (enabled) {
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c b/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c
index e92fa4329de2..eb33f473daaf 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c
@@ -580,16 +580,12 @@ int prestera_bridge_port_event(struct net_device *dev, unsigned long event,
 }
 
 static int prestera_port_attr_br_flags_set(struct prestera_port *port,
-					   struct switchdev_trans *trans,
 					   struct net_device *dev,
 					   unsigned long flags)
 {
 	struct prestera_bridge_port *br_port;
 	int err;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	br_port = prestera_bridge_port_by_dev(port->sw->swdev, dev);
 	if (!br_port)
 		return 0;
@@ -608,35 +604,26 @@ static int prestera_port_attr_br_flags_set(struct prestera_port *port,
 }
 
 static int prestera_port_attr_br_ageing_set(struct prestera_port *port,
-					    struct switchdev_trans *trans,
 					    unsigned long ageing_clock_t)
 {
 	unsigned long ageing_jiffies = clock_t_to_jiffies(ageing_clock_t);
 	u32 ageing_time_ms = jiffies_to_msecs(ageing_jiffies);
 	struct prestera_switch *sw = port->sw;
 
-	if (switchdev_trans_ph_prepare(trans)) {
-		if (ageing_time_ms < PRESTERA_MIN_AGEING_TIME_MS ||
-		    ageing_time_ms > PRESTERA_MAX_AGEING_TIME_MS)
-			return -ERANGE;
-		else
-			return 0;
-	}
+	if (ageing_time_ms < PRESTERA_MIN_AGEING_TIME_MS ||
+	    ageing_time_ms > PRESTERA_MAX_AGEING_TIME_MS)
+		return -ERANGE;
 
 	return prestera_hw_switch_ageing_set(sw, ageing_time_ms);
 }
 
 static int prestera_port_attr_br_vlan_set(struct prestera_port *port,
-					  struct switchdev_trans *trans,
 					  struct net_device *dev,
 					  bool vlan_enabled)
 {
 	struct prestera_switch *sw = port->sw;
 	struct prestera_bridge *bridge;
 
-	if (!switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	bridge = prestera_bridge_by_dev(sw->swdev, dev);
 	if (WARN_ON(!bridge))
 		return -EINVAL;
@@ -666,7 +653,6 @@ static int prestera_port_bridge_vlan_stp_set(struct prestera_port *port,
 }
 
 static int presterar_port_attr_stp_state_set(struct prestera_port *port,
-					     struct switchdev_trans *trans,
 					     struct net_device *dev,
 					     u8 state)
 {
@@ -675,9 +661,6 @@ static int presterar_port_attr_stp_state_set(struct prestera_port *port,
 	int err;
 	u16 vid;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	br_port = prestera_bridge_port_by_dev(port->sw->swdev, dev);
 	if (!br_port)
 		return 0;
@@ -712,16 +695,14 @@ static int presterar_port_attr_stp_state_set(struct prestera_port *port,
 }
 
 static int prestera_port_obj_attr_set(struct net_device *dev,
-				      const struct switchdev_attr *attr,
-				      struct switchdev_trans *trans)
+				      const struct switchdev_attr *attr)
 {
 	struct prestera_port *port = netdev_priv(dev);
 	int err = 0;
 
 	switch (attr->id) {
 	case SWITCHDEV_ATTR_ID_PORT_STP_STATE:
-		err = presterar_port_attr_stp_state_set(port, trans,
-							attr->orig_dev,
+		err = presterar_port_attr_stp_state_set(port, attr->orig_dev,
 							attr->u.stp_state);
 		break;
 	case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS:
@@ -730,17 +711,15 @@ static int prestera_port_obj_attr_set(struct net_device *dev,
 			err = -EINVAL;
 		break;
 	case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
-		err = prestera_port_attr_br_flags_set(port, trans,
-						      attr->orig_dev,
+		err = prestera_port_attr_br_flags_set(port, attr->orig_dev,
 						      attr->u.brport_flags);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
-		err = prestera_port_attr_br_ageing_set(port, trans,
+		err = prestera_port_attr_br_ageing_set(port,
 						       attr->u.ageing_time);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING:
-		err = prestera_port_attr_br_vlan_set(port, trans,
-						     attr->orig_dev,
+		err = prestera_port_attr_br_vlan_set(port, attr->orig_dev,
 						     attr->u.vlan_filtering);
 		break;
 	default:
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index 16129edf1751..e248f9deb2ec 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -527,7 +527,6 @@ mlxsw_sp_port_bridge_vlan_stp_set(struct mlxsw_sp_port *mlxsw_sp_port,
 }
 
 static int mlxsw_sp_port_attr_stp_state_set(struct mlxsw_sp_port *mlxsw_sp_port,
-					    struct switchdev_trans *trans,
 					    struct net_device *orig_dev,
 					    u8 state)
 {
@@ -535,9 +534,6 @@ static int mlxsw_sp_port_attr_stp_state_set(struct mlxsw_sp_port *mlxsw_sp_port,
 	struct mlxsw_sp_bridge_vlan *bridge_vlan;
 	int err;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	/* It's possible we failed to enslave the port, yet this
 	 * operation is executed due to it being deferred.
 	 */
@@ -659,7 +655,6 @@ mlxsw_sp_bridge_port_learning_set(struct mlxsw_sp_port *mlxsw_sp_port,
 
 static int mlxsw_sp_port_attr_br_pre_flags_set(struct mlxsw_sp_port
 					       *mlxsw_sp_port,
-					       struct switchdev_trans *trans,
 					       unsigned long brport_flags)
 {
 	if (brport_flags & ~(BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD))
@@ -669,16 +664,12 @@ static int mlxsw_sp_port_attr_br_pre_flags_set(struct mlxsw_sp_port
 }
 
 static int mlxsw_sp_port_attr_br_flags_set(struct mlxsw_sp_port *mlxsw_sp_port,
-					   struct switchdev_trans *trans,
 					   struct net_device *orig_dev,
 					   unsigned long brport_flags)
 {
 	struct mlxsw_sp_bridge_port *bridge_port;
 	int err;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	bridge_port = mlxsw_sp_bridge_port_find(mlxsw_sp_port->mlxsw_sp->bridge,
 						orig_dev);
 	if (!bridge_port)
@@ -724,35 +715,26 @@ static int mlxsw_sp_ageing_set(struct mlxsw_sp *mlxsw_sp, u32 ageing_time)
 }
 
 static int mlxsw_sp_port_attr_br_ageing_set(struct mlxsw_sp_port *mlxsw_sp_port,
-					    struct switchdev_trans *trans,
 					    unsigned long ageing_clock_t)
 {
 	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
 	unsigned long ageing_jiffies = clock_t_to_jiffies(ageing_clock_t);
 	u32 ageing_time = jiffies_to_msecs(ageing_jiffies) / 1000;
 
-	if (switchdev_trans_ph_prepare(trans)) {
-		if (ageing_time < MLXSW_SP_MIN_AGEING_TIME ||
-		    ageing_time > MLXSW_SP_MAX_AGEING_TIME)
-			return -ERANGE;
-		else
-			return 0;
-	}
+	if (ageing_time < MLXSW_SP_MIN_AGEING_TIME ||
+	    ageing_time > MLXSW_SP_MAX_AGEING_TIME)
+		return -ERANGE;
 
 	return mlxsw_sp_ageing_set(mlxsw_sp, ageing_time);
 }
 
 static int mlxsw_sp_port_attr_br_vlan_set(struct mlxsw_sp_port *mlxsw_sp_port,
-					  struct switchdev_trans *trans,
 					  struct net_device *orig_dev,
 					  bool vlan_enabled)
 {
 	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
 	struct mlxsw_sp_bridge_device *bridge_device;
 
-	if (!switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	bridge_device = mlxsw_sp_bridge_device_find(mlxsw_sp->bridge, orig_dev);
 	if (WARN_ON(!bridge_device))
 		return -EINVAL;
@@ -765,16 +747,12 @@ static int mlxsw_sp_port_attr_br_vlan_set(struct mlxsw_sp_port *mlxsw_sp_port,
 }
 
 static int mlxsw_sp_port_attr_br_vlan_proto_set(struct mlxsw_sp_port *mlxsw_sp_port,
-						struct switchdev_trans *trans,
 						struct net_device *orig_dev,
 						u16 vlan_proto)
 {
 	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
 	struct mlxsw_sp_bridge_device *bridge_device;
 
-	if (!switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	bridge_device = mlxsw_sp_bridge_device_find(mlxsw_sp->bridge, orig_dev);
 	if (WARN_ON(!bridge_device))
 		return -EINVAL;
@@ -784,16 +762,12 @@ static int mlxsw_sp_port_attr_br_vlan_proto_set(struct mlxsw_sp_port *mlxsw_sp_p
 }
 
 static int mlxsw_sp_port_attr_mrouter_set(struct mlxsw_sp_port *mlxsw_sp_port,
-					  struct switchdev_trans *trans,
 					  struct net_device *orig_dev,
 					  bool is_port_mrouter)
 {
 	struct mlxsw_sp_bridge_port *bridge_port;
 	int err;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	bridge_port = mlxsw_sp_bridge_port_find(mlxsw_sp_port->mlxsw_sp->bridge,
 						orig_dev);
 	if (!bridge_port)
@@ -825,7 +799,6 @@ static bool mlxsw_sp_mc_flood(const struct mlxsw_sp_bridge_port *bridge_port)
 }
 
 static int mlxsw_sp_port_mc_disabled_set(struct mlxsw_sp_port *mlxsw_sp_port,
-					 struct switchdev_trans *trans,
 					 struct net_device *orig_dev,
 					 bool mc_disabled)
 {
@@ -834,9 +807,6 @@ static int mlxsw_sp_port_mc_disabled_set(struct mlxsw_sp_port *mlxsw_sp_port,
 	struct mlxsw_sp_bridge_port *bridge_port;
 	int err;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	/* It's possible we failed to enslave the port, yet this
 	 * operation is executed due to it being deferred.
 	 */
@@ -896,16 +866,12 @@ mlxsw_sp_bridge_mrouter_update_mdb(struct mlxsw_sp *mlxsw_sp,
 
 static int
 mlxsw_sp_port_attr_br_mrouter_set(struct mlxsw_sp_port *mlxsw_sp_port,
-				  struct switchdev_trans *trans,
 				  struct net_device *orig_dev,
 				  bool is_mrouter)
 {
 	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
 	struct mlxsw_sp_bridge_device *bridge_device;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	/* It's possible we failed to enslave the port, yet this
 	 * operation is executed due to it being deferred.
 	 */
@@ -921,54 +887,52 @@ mlxsw_sp_port_attr_br_mrouter_set(struct mlxsw_sp_port *mlxsw_sp_port,
 }
 
 static int mlxsw_sp_port_attr_set(struct net_device *dev,
-				  const struct switchdev_attr *attr,
-				  struct switchdev_trans *trans)
+				  const struct switchdev_attr *attr)
 {
 	struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(dev);
 	int err;
 
 	switch (attr->id) {
 	case SWITCHDEV_ATTR_ID_PORT_STP_STATE:
-		err = mlxsw_sp_port_attr_stp_state_set(mlxsw_sp_port, trans,
+		err = mlxsw_sp_port_attr_stp_state_set(mlxsw_sp_port,
 						       attr->orig_dev,
 						       attr->u.stp_state);
 		break;
 	case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS:
 		err = mlxsw_sp_port_attr_br_pre_flags_set(mlxsw_sp_port,
-							  trans,
 							  attr->u.brport_flags);
 		break;
 	case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
-		err = mlxsw_sp_port_attr_br_flags_set(mlxsw_sp_port, trans,
+		err = mlxsw_sp_port_attr_br_flags_set(mlxsw_sp_port,
 						      attr->orig_dev,
 						      attr->u.brport_flags);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
-		err = mlxsw_sp_port_attr_br_ageing_set(mlxsw_sp_port, trans,
+		err = mlxsw_sp_port_attr_br_ageing_set(mlxsw_sp_port,
 						       attr->u.ageing_time);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING:
-		err = mlxsw_sp_port_attr_br_vlan_set(mlxsw_sp_port, trans,
+		err = mlxsw_sp_port_attr_br_vlan_set(mlxsw_sp_port,
 						     attr->orig_dev,
 						     attr->u.vlan_filtering);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_VLAN_PROTOCOL:
-		err = mlxsw_sp_port_attr_br_vlan_proto_set(mlxsw_sp_port, trans,
+		err = mlxsw_sp_port_attr_br_vlan_proto_set(mlxsw_sp_port,
 							   attr->orig_dev,
 							   attr->u.vlan_protocol);
 		break;
 	case SWITCHDEV_ATTR_ID_PORT_MROUTER:
-		err = mlxsw_sp_port_attr_mrouter_set(mlxsw_sp_port, trans,
+		err = mlxsw_sp_port_attr_mrouter_set(mlxsw_sp_port,
 						     attr->orig_dev,
 						     attr->u.mrouter);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED:
-		err = mlxsw_sp_port_mc_disabled_set(mlxsw_sp_port, trans,
+		err = mlxsw_sp_port_mc_disabled_set(mlxsw_sp_port,
 						    attr->orig_dev,
 						    attr->u.mc_disabled);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_MROUTER:
-		err = mlxsw_sp_port_attr_br_mrouter_set(mlxsw_sp_port, trans,
+		err = mlxsw_sp_port_attr_br_mrouter_set(mlxsw_sp_port,
 							attr->orig_dev,
 							attr->u.mrouter);
 		break;
@@ -977,8 +941,7 @@ static int mlxsw_sp_port_attr_set(struct net_device *dev,
 		break;
 	}
 
-	if (switchdev_trans_ph_commit(trans))
-		mlxsw_sp_span_respin(mlxsw_sp_port->mlxsw_sp);
+	mlxsw_sp_span_respin(mlxsw_sp_port->mlxsw_sp);
 
 	return err;
 }
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 0b9992bd6626..af620f7ce469 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -208,25 +208,20 @@ static void ocelot_port_set_pvid(struct ocelot *ocelot, int port,
 }
 
 int ocelot_port_vlan_filtering(struct ocelot *ocelot, int port,
-			       bool vlan_aware, struct switchdev_trans *trans)
+			       bool vlan_aware)
 {
+	struct ocelot_vcap_block *block = &ocelot->block[VCAP_IS1];
 	struct ocelot_port *ocelot_port = ocelot->ports[port];
+	struct ocelot_vcap_filter *filter;
 	u32 val;
 
-	if (switchdev_trans_ph_prepare(trans)) {
-		struct ocelot_vcap_block *block = &ocelot->block[VCAP_IS1];
-		struct ocelot_vcap_filter *filter;
-
-		list_for_each_entry(filter, &block->rules, list) {
-			if (filter->ingress_port_mask & BIT(port) &&
-			    filter->action.vid_replace_ena) {
-				dev_err(ocelot->dev,
-					"Cannot change VLAN state with vlan modify rules active\n");
-				return -EBUSY;
-			}
+	list_for_each_entry(filter, &block->rules, list) {
+		if (filter->ingress_port_mask & BIT(port) &&
+		    filter->action.vid_replace_ena) {
+			dev_err(ocelot->dev,
+				"Cannot change VLAN state with vlan modify rules active\n");
+			return -EBUSY;
 		}
-
-		return 0;
 	}
 
 	ocelot_port->vlan_aware = vlan_aware;
@@ -1179,7 +1174,6 @@ int ocelot_port_bridge_leave(struct ocelot *ocelot, int port,
 			     struct net_device *bridge)
 {
 	struct ocelot_vlan pvid = {0}, native_vlan = {0};
-	struct switchdev_trans trans;
 	int ret;
 
 	ocelot->bridge_mask &= ~BIT(port);
@@ -1187,13 +1181,7 @@ int ocelot_port_bridge_leave(struct ocelot *ocelot, int port,
 	if (!ocelot->bridge_mask)
 		ocelot->hw_bridge_dev = NULL;
 
-	trans.ph_prepare = true;
-	ret = ocelot_port_vlan_filtering(ocelot, port, false, &trans);
-	if (ret)
-		return ret;
-
-	trans.ph_prepare = false;
-	ret = ocelot_port_vlan_filtering(ocelot, port, false, &trans);
+	ret = ocelot_port_vlan_filtering(ocelot, port, false);
 	if (ret)
 		return ret;
 
diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index 5babe2a38041..95e89cf4495c 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -825,12 +825,8 @@ static const struct ethtool_ops ocelot_ethtool_ops = {
 };
 
 static void ocelot_port_attr_stp_state_set(struct ocelot *ocelot, int port,
-					   struct switchdev_trans *trans,
 					   u8 state)
 {
-	if (switchdev_trans_ph_prepare(trans))
-		return;
-
 	ocelot_bridge_stp_state_set(ocelot, port, state);
 }
 
@@ -858,8 +854,7 @@ static void ocelot_port_attr_mc_set(struct ocelot *ocelot, int port, bool mc)
 }
 
 static int ocelot_port_attr_set(struct net_device *dev,
-				const struct switchdev_attr *attr,
-				struct switchdev_trans *trans)
+				const struct switchdev_attr *attr)
 {
 	struct ocelot_port_private *priv = netdev_priv(dev);
 	struct ocelot *ocelot = priv->port.ocelot;
@@ -868,15 +863,13 @@ static int ocelot_port_attr_set(struct net_device *dev,
 
 	switch (attr->id) {
 	case SWITCHDEV_ATTR_ID_PORT_STP_STATE:
-		ocelot_port_attr_stp_state_set(ocelot, port, trans,
-					       attr->u.stp_state);
+		ocelot_port_attr_stp_state_set(ocelot, port, attr->u.stp_state);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
 		ocelot_port_attr_ageing_set(ocelot, port, attr->u.ageing_time);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING:
-		ocelot_port_vlan_filtering(ocelot, port,
-					   attr->u.vlan_filtering, trans);
+		ocelot_port_vlan_filtering(ocelot, port, attr->u.vlan_filtering);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED:
 		ocelot_port_attr_mc_set(ocelot, port, !attr->u.mc_disabled);
diff --git a/drivers/net/ethernet/rocker/rocker.h b/drivers/net/ethernet/rocker/rocker.h
index 6fad25321dc5..315a6e5c0f59 100644
--- a/drivers/net/ethernet/rocker/rocker.h
+++ b/drivers/net/ethernet/rocker/rocker.h
@@ -103,15 +103,13 @@ struct rocker_world_ops {
 	int (*port_attr_stp_state_set)(struct rocker_port *rocker_port,
 				       u8 state);
 	int (*port_attr_bridge_flags_set)(struct rocker_port *rocker_port,
-					  unsigned long brport_flags,
-					  struct switchdev_trans *trans);
+					  unsigned long brport_flags);
 	int (*port_attr_bridge_flags_support_get)(const struct rocker_port *
 						  rocker_port,
 						  unsigned long *
 						  p_brport_flags);
 	int (*port_attr_bridge_ageing_time_set)(struct rocker_port *rocker_port,
-						u32 ageing_time,
-						struct switchdev_trans *trans);
+						u32 ageing_time);
 	int (*port_obj_vlan_add)(struct rocker_port *rocker_port,
 				 const struct switchdev_obj_port_vlan *vlan);
 	int (*port_obj_vlan_del)(struct rocker_port *rocker_port,
diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c
index 1018d3759316..740a715c49c6 100644
--- a/drivers/net/ethernet/rocker/rocker_main.c
+++ b/drivers/net/ethernet/rocker/rocker_main.c
@@ -1550,17 +1550,13 @@ static void rocker_world_port_stop(struct rocker_port *rocker_port)
 }
 
 static int rocker_world_port_attr_stp_state_set(struct rocker_port *rocker_port,
-						u8 state,
-						struct switchdev_trans *trans)
+						u8 state)
 {
 	struct rocker_world_ops *wops = rocker_port->rocker->wops;
 
 	if (!wops->port_attr_stp_state_set)
 		return -EOPNOTSUPP;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	return wops->port_attr_stp_state_set(rocker_port, state);
 }
 
@@ -1580,8 +1576,7 @@ rocker_world_port_attr_bridge_flags_support_get(const struct rocker_port *
 
 static int
 rocker_world_port_attr_pre_bridge_flags_set(struct rocker_port *rocker_port,
-					    unsigned long brport_flags,
-					    struct switchdev_trans *trans)
+					    unsigned long brport_flags)
 {
 	struct rocker_world_ops *wops = rocker_port->rocker->wops;
 	unsigned long brport_flags_s;
@@ -1603,37 +1598,26 @@ rocker_world_port_attr_pre_bridge_flags_set(struct rocker_port *rocker_port,
 
 static int
 rocker_world_port_attr_bridge_flags_set(struct rocker_port *rocker_port,
-					unsigned long brport_flags,
-					struct switchdev_trans *trans)
+					unsigned long brport_flags)
 {
 	struct rocker_world_ops *wops = rocker_port->rocker->wops;
 
 	if (!wops->port_attr_bridge_flags_set)
 		return -EOPNOTSUPP;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
-	return wops->port_attr_bridge_flags_set(rocker_port, brport_flags,
-						trans);
+	return wops->port_attr_bridge_flags_set(rocker_port, brport_flags);
 }
 
 static int
 rocker_world_port_attr_bridge_ageing_time_set(struct rocker_port *rocker_port,
-					      u32 ageing_time,
-					      struct switchdev_trans *trans)
-
+					      u32 ageing_time)
 {
 	struct rocker_world_ops *wops = rocker_port->rocker->wops;
 
 	if (!wops->port_attr_bridge_ageing_time_set)
 		return -EOPNOTSUPP;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
-	return wops->port_attr_bridge_ageing_time_set(rocker_port, ageing_time,
-						      trans);
+	return wops->port_attr_bridge_ageing_time_set(rocker_port, ageing_time);
 }
 
 static int
@@ -2062,8 +2046,7 @@ static const struct net_device_ops rocker_port_netdev_ops = {
  ********************/
 
 static int rocker_port_attr_set(struct net_device *dev,
-				const struct switchdev_attr *attr,
-				struct switchdev_trans *trans)
+				const struct switchdev_attr *attr)
 {
 	struct rocker_port *rocker_port = netdev_priv(dev);
 	int err = 0;
@@ -2071,23 +2054,19 @@ static int rocker_port_attr_set(struct net_device *dev,
 	switch (attr->id) {
 	case SWITCHDEV_ATTR_ID_PORT_STP_STATE:
 		err = rocker_world_port_attr_stp_state_set(rocker_port,
-							   attr->u.stp_state,
-							   trans);
+							   attr->u.stp_state);
 		break;
 	case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS:
 		err = rocker_world_port_attr_pre_bridge_flags_set(rocker_port,
-							      attr->u.brport_flags,
-							      trans);
+							      attr->u.brport_flags);
 		break;
 	case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
 		err = rocker_world_port_attr_bridge_flags_set(rocker_port,
-							      attr->u.brport_flags,
-							      trans);
+							      attr->u.brport_flags);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
 		err = rocker_world_port_attr_bridge_ageing_time_set(rocker_port,
-								    attr->u.ageing_time,
-								    trans);
+								    attr->u.ageing_time);
 		break;
 	default:
 		err = -EOPNOTSUPP;
@@ -2719,8 +2698,7 @@ rocker_switchdev_port_attr_set_event(struct net_device *netdev,
 {
 	int err;
 
-	err = rocker_port_attr_set(netdev, port_attr_info->attr,
-				   port_attr_info->trans);
+	err = rocker_port_attr_set(netdev, port_attr_info->attr);
 
 	port_attr_info->handled = true;
 	return notifier_from_errno(err);
diff --git a/drivers/net/ethernet/rocker/rocker_ofdpa.c b/drivers/net/ethernet/rocker/rocker_ofdpa.c
index 7072b249c8bd..0dc5710adb45 100644
--- a/drivers/net/ethernet/rocker/rocker_ofdpa.c
+++ b/drivers/net/ethernet/rocker/rocker_ofdpa.c
@@ -2488,8 +2488,7 @@ static int ofdpa_port_attr_stp_state_set(struct rocker_port *rocker_port,
 }
 
 static int ofdpa_port_attr_bridge_flags_set(struct rocker_port *rocker_port,
-					    unsigned long brport_flags,
-					    struct switchdev_trans *trans)
+					    unsigned long brport_flags)
 {
 	struct ofdpa_port *ofdpa_port = rocker_port->wpriv;
 	unsigned long orig_flags;
@@ -2497,14 +2496,11 @@ static int ofdpa_port_attr_bridge_flags_set(struct rocker_port *rocker_port,
 
 	orig_flags = ofdpa_port->brport_flags;
 	ofdpa_port->brport_flags = brport_flags;
-	if ((orig_flags ^ ofdpa_port->brport_flags) & BR_LEARNING &&
-	    !switchdev_trans_ph_prepare(trans))
+
+	if ((orig_flags ^ ofdpa_port->brport_flags) & BR_LEARNING)
 		err = rocker_port_set_learning(ofdpa_port->rocker_port,
 					       !!(ofdpa_port->brport_flags & BR_LEARNING));
 
-	if (switchdev_trans_ph_prepare(trans))
-		ofdpa_port->brport_flags = orig_flags;
-
 	return err;
 }
 
@@ -2520,18 +2516,15 @@ ofdpa_port_attr_bridge_flags_support_get(const struct rocker_port *
 
 static int
 ofdpa_port_attr_bridge_ageing_time_set(struct rocker_port *rocker_port,
-				       u32 ageing_time,
-				       struct switchdev_trans *trans)
+				       u32 ageing_time)
 {
 	struct ofdpa_port *ofdpa_port = rocker_port->wpriv;
 	struct ofdpa *ofdpa = ofdpa_port->ofdpa;
 
-	if (!switchdev_trans_ph_prepare(trans)) {
-		ofdpa_port->ageing_time = clock_t_to_jiffies(ageing_time);
-		if (ofdpa_port->ageing_time < ofdpa->ageing_time)
-			ofdpa->ageing_time = ofdpa_port->ageing_time;
-		mod_timer(&ofdpa_port->ofdpa->fdb_cleanup_timer, jiffies);
-	}
+	ofdpa_port->ageing_time = clock_t_to_jiffies(ageing_time);
+	if (ofdpa_port->ageing_time < ofdpa->ageing_time)
+		ofdpa->ageing_time = ofdpa_port->ageing_time;
+	mod_timer(&ofdpa_port->ofdpa->fdb_cleanup_timer, jiffies);
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/ti/cpsw_switchdev.c b/drivers/net/ethernet/ti/cpsw_switchdev.c
index 48a4f277ee31..25eaff8e7bb8 100644
--- a/drivers/net/ethernet/ti/cpsw_switchdev.c
+++ b/drivers/net/ethernet/ti/cpsw_switchdev.c
@@ -24,16 +24,12 @@ struct cpsw_switchdev_event_work {
 	unsigned long event;
 };
 
-static int cpsw_port_stp_state_set(struct cpsw_priv *priv,
-				   struct switchdev_trans *trans, u8 state)
+static int cpsw_port_stp_state_set(struct cpsw_priv *priv, u8 state)
 {
 	struct cpsw_common *cpsw = priv->cpsw;
 	u8 cpsw_state;
 	int ret = 0;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	switch (state) {
 	case BR_STATE_FORWARDING:
 		cpsw_state = ALE_PORT_STATE_FORWARD;
@@ -60,16 +56,12 @@ static int cpsw_port_stp_state_set(struct cpsw_priv *priv,
 }
 
 static int cpsw_port_attr_br_flags_set(struct cpsw_priv *priv,
-				       struct switchdev_trans *trans,
 				       struct net_device *orig_dev,
 				       unsigned long brport_flags)
 {
 	struct cpsw_common *cpsw = priv->cpsw;
 	bool unreg_mcast_add = false;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	if (brport_flags & BR_MCAST_FLOOD)
 		unreg_mcast_add = true;
 	dev_dbg(priv->dev, "BR_MCAST_FLOOD: %d port %u\n",
@@ -82,7 +74,6 @@ static int cpsw_port_attr_br_flags_set(struct cpsw_priv *priv,
 }
 
 static int cpsw_port_attr_br_flags_pre_set(struct net_device *netdev,
-					   struct switchdev_trans *trans,
 					   unsigned long flags)
 {
 	if (flags & ~(BR_LEARNING | BR_MCAST_FLOOD))
@@ -92,8 +83,7 @@ static int cpsw_port_attr_br_flags_pre_set(struct net_device *netdev,
 }
 
 static int cpsw_port_attr_set(struct net_device *ndev,
-			      const struct switchdev_attr *attr,
-			      struct switchdev_trans *trans)
+			      const struct switchdev_attr *attr)
 {
 	struct cpsw_priv *priv = netdev_priv(ndev);
 	int ret;
@@ -102,15 +92,15 @@ static int cpsw_port_attr_set(struct net_device *ndev,
 
 	switch (attr->id) {
 	case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS:
-		ret = cpsw_port_attr_br_flags_pre_set(ndev, trans,
+		ret = cpsw_port_attr_br_flags_pre_set(ndev,
 						      attr->u.brport_flags);
 		break;
 	case SWITCHDEV_ATTR_ID_PORT_STP_STATE:
-		ret = cpsw_port_stp_state_set(priv, trans, attr->u.stp_state);
+		ret = cpsw_port_stp_state_set(priv, attr->u.stp_state);
 		dev_dbg(priv->dev, "stp state: %u\n", attr->u.stp_state);
 		break;
 	case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
-		ret = cpsw_port_attr_br_flags_set(priv, trans, attr->orig_dev,
+		ret = cpsw_port_attr_br_flags_set(priv, attr->orig_dev,
 						  attr->u.brport_flags);
 		break;
 	default:
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index d7e6e06418ed..0475c5e38312 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -901,19 +901,14 @@ static void dpaa2_switch_teardown_irqs(struct fsl_mc_device *sw_dev)
 }
 
 static int dpaa2_switch_port_attr_stp_state_set(struct net_device *netdev,
-						struct switchdev_trans *trans,
 						u8 state)
 {
 	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	return dpaa2_switch_port_set_stp_state(port_priv, state);
 }
 
 static int dpaa2_switch_port_attr_br_flags_pre_set(struct net_device *netdev,
-						   struct switchdev_trans *trans,
 						   unsigned long flags)
 {
 	if (flags & ~(BR_LEARNING | BR_FLOOD))
@@ -923,15 +918,11 @@ static int dpaa2_switch_port_attr_br_flags_pre_set(struct net_device *netdev,
 }
 
 static int dpaa2_switch_port_attr_br_flags_set(struct net_device *netdev,
-					       struct switchdev_trans *trans,
 					       unsigned long flags)
 {
 	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
 	int err = 0;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	/* Learning is enabled per switch */
 	err = dpaa2_switch_set_learning(port_priv->ethsw_data,
 					!!(flags & BR_LEARNING));
@@ -945,22 +936,21 @@ static int dpaa2_switch_port_attr_br_flags_set(struct net_device *netdev,
 }
 
 static int dpaa2_switch_port_attr_set(struct net_device *netdev,
-				      const struct switchdev_attr *attr,
-				      struct switchdev_trans *trans)
+				      const struct switchdev_attr *attr)
 {
 	int err = 0;
 
 	switch (attr->id) {
 	case SWITCHDEV_ATTR_ID_PORT_STP_STATE:
-		err = dpaa2_switch_port_attr_stp_state_set(netdev, trans,
+		err = dpaa2_switch_port_attr_stp_state_set(netdev,
 							   attr->u.stp_state);
 		break;
 	case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS:
-		err = dpaa2_switch_port_attr_br_flags_pre_set(netdev, trans,
+		err = dpaa2_switch_port_attr_br_flags_pre_set(netdev,
 							      attr->u.brport_flags);
 		break;
 	case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
-		err = dpaa2_switch_port_attr_br_flags_set(netdev, trans,
+		err = dpaa2_switch_port_attr_br_flags_set(netdev,
 							  attr->u.brport_flags);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING:
@@ -1204,8 +1194,7 @@ static int dpaa2_switch_port_attr_set_event(struct net_device *netdev,
 {
 	int err;
 
-	err = dpaa2_switch_port_attr_set(netdev, port_attr_info->attr,
-					 port_attr_info->trans);
+	err = dpaa2_switch_port_attr_set(netdev, port_attr_info->attr);
 
 	port_attr_info->handled = true;
 	return notifier_from_errno(err);
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 4e60d2610f20..7727b56a4eee 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -560,8 +560,7 @@ struct dsa_switch_ops {
 	 * VLAN support
 	 */
 	int	(*port_vlan_filtering)(struct dsa_switch *ds, int port,
-				       bool vlan_filtering,
-				       struct switchdev_trans *trans);
+				       bool vlan_filtering);
 	int (*port_vlan_prepare)(struct dsa_switch *ds, int port,
 				 const struct switchdev_obj_port_vlan *vlan);
 	void (*port_vlan_add)(struct dsa_switch *ds, int port,
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index daca7a14c1db..2df84ffe6ba4 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -240,7 +240,6 @@ struct switchdev_notifier_port_obj_info {
 struct switchdev_notifier_port_attr_info {
 	struct switchdev_notifier_info info; /* must be first */
 	const struct switchdev_attr *attr;
-	struct switchdev_trans *trans;
 	bool handled;
 };
 
@@ -299,8 +298,7 @@ int switchdev_handle_port_attr_set(struct net_device *dev,
 			struct switchdev_notifier_port_attr_info *port_attr_info,
 			bool (*check_cb)(const struct net_device *dev),
 			int (*set_cb)(struct net_device *dev,
-				      const struct switchdev_attr *attr,
-				      struct switchdev_trans *trans));
+				      const struct switchdev_attr *attr));
 #else
 
 static inline void switchdev_deferred_process(void)
@@ -391,8 +389,7 @@ switchdev_handle_port_attr_set(struct net_device *dev,
 			struct switchdev_notifier_port_attr_info *port_attr_info,
 			bool (*check_cb)(const struct net_device *dev),
 			int (*set_cb)(struct net_device *dev,
-				      const struct switchdev_attr *attr,
-				      struct switchdev_trans *trans))
+				      const struct switchdev_attr *attr))
 {
 	return 0;
 }
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index 2f4cd3288bcc..f3db75c0172b 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -739,8 +739,7 @@ int ocelot_get_ts_info(struct ocelot *ocelot, int port,
 void ocelot_set_ageing_time(struct ocelot *ocelot, unsigned int msecs);
 void ocelot_adjust_link(struct ocelot *ocelot, int port,
 			struct phy_device *phydev);
-int ocelot_port_vlan_filtering(struct ocelot *ocelot, int port, bool enabled,
-			       struct switchdev_trans *trans);
+int ocelot_port_vlan_filtering(struct ocelot *ocelot, int port, bool enabled);
 void ocelot_bridge_stp_state_set(struct ocelot *ocelot, int port, u8 state);
 int ocelot_port_bridge_join(struct ocelot *ocelot, int port,
 			    struct net_device *bridge);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index ec70066413c7..54e2ee1a2131 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -127,19 +127,16 @@ static inline struct net_device *dsa_master_find_slave(struct net_device *dev,
 }
 
 /* port.c */
-int dsa_port_set_state(struct dsa_port *dp, u8 state,
-		       struct switchdev_trans *trans);
+int dsa_port_set_state(struct dsa_port *dp, u8 state);
 int dsa_port_enable_rt(struct dsa_port *dp, struct phy_device *phy);
 int dsa_port_enable(struct dsa_port *dp, struct phy_device *phy);
 void dsa_port_disable_rt(struct dsa_port *dp);
 void dsa_port_disable(struct dsa_port *dp);
 int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br);
 void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br);
-int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
-			    struct switchdev_trans *trans);
+int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering);
 bool dsa_port_skip_vlan_configuration(struct dsa_port *dp);
-int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock,
-			 struct switchdev_trans *trans);
+int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock);
 int dsa_port_mtu_change(struct dsa_port *dp, int new_mtu,
 			bool propagate_upstream);
 int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
@@ -151,12 +148,9 @@ 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);
-int dsa_port_pre_bridge_flags(const struct dsa_port *dp, unsigned long flags,
-			      struct switchdev_trans *trans);
-int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags,
-			  struct switchdev_trans *trans);
-int dsa_port_mrouter(struct dsa_port *dp, bool mrouter,
-		     struct switchdev_trans *trans);
+int dsa_port_pre_bridge_flags(const struct dsa_port *dp, unsigned long flags);
+int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags);
+int dsa_port_mrouter(struct dsa_port *dp, bool mrouter);
 int dsa_port_vlan_add(struct dsa_port *dp,
 		      const struct switchdev_obj_port_vlan *vlan);
 int dsa_port_vlan_del(struct dsa_port *dp,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 8d173064539c..62f38a36386f 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -40,17 +40,15 @@ static int dsa_port_notify(const struct dsa_port *dp, unsigned long e, void *v)
 	return notifier_to_errno(err);
 }
 
-int dsa_port_set_state(struct dsa_port *dp, u8 state,
-		       struct switchdev_trans *trans)
+int dsa_port_set_state(struct dsa_port *dp, u8 state)
 {
 	struct dsa_switch *ds = dp->ds;
 	int port = dp->index;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return ds->ops->port_stp_state_set ? 0 : -EOPNOTSUPP;
+	if (!ds->ops->port_stp_state_set)
+		return -EOPNOTSUPP;
 
-	if (ds->ops->port_stp_state_set)
-		ds->ops->port_stp_state_set(ds, port, state);
+	ds->ops->port_stp_state_set(ds, port, state);
 
 	if (ds->ops->port_fast_age) {
 		/* Fast age FDB entries or flush appropriate forwarding database
@@ -75,7 +73,7 @@ static void dsa_port_set_state_now(struct dsa_port *dp, u8 state)
 {
 	int err;
 
-	err = dsa_port_set_state(dp, state, NULL);
+	err = dsa_port_set_state(dp, state);
 	if (err)
 		pr_err("DSA: failed to set STP state %u (%d)\n", state, err);
 }
@@ -145,7 +143,7 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br)
 	int err;
 
 	/* Set the flooding mode before joining the port in the switch */
-	err = dsa_port_bridge_flags(dp, BR_FLOOD | BR_MCAST_FLOOD, NULL);
+	err = dsa_port_bridge_flags(dp, BR_FLOOD | BR_MCAST_FLOOD);
 	if (err)
 		return err;
 
@@ -158,7 +156,7 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br)
 
 	/* The bridging is rolled back on error */
 	if (err) {
-		dsa_port_bridge_flags(dp, 0, NULL);
+		dsa_port_bridge_flags(dp, 0);
 		dp->bridge_dev = NULL;
 	}
 
@@ -185,7 +183,7 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
 		pr_err("DSA: failed to notify DSA_NOTIFIER_BRIDGE_LEAVE\n");
 
 	/* Port is leaving the bridge, disable flooding */
-	dsa_port_bridge_flags(dp, 0, NULL);
+	dsa_port_bridge_flags(dp, 0);
 
 	/* 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
@@ -259,43 +257,36 @@ static bool dsa_port_can_apply_vlan_filtering(struct dsa_port *dp,
 	return true;
 }
 
-int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
-			    struct switchdev_trans *trans)
+int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering)
 {
 	struct dsa_switch *ds = dp->ds;
+	bool apply;
 	int err;
 
-	if (switchdev_trans_ph_prepare(trans)) {
-		bool apply;
-
-		if (!ds->ops->port_vlan_filtering)
-			return -EOPNOTSUPP;
+	if (!ds->ops->port_vlan_filtering)
+		return -EOPNOTSUPP;
 
-		/* We are called from dsa_slave_switchdev_blocking_event(),
-		 * which is not under rcu_read_lock(), unlike
-		 * dsa_slave_switchdev_event().
-		 */
-		rcu_read_lock();
-		apply = dsa_port_can_apply_vlan_filtering(dp, vlan_filtering);
-		rcu_read_unlock();
-		if (!apply)
-			return -EINVAL;
-	}
+	/* We are called from dsa_slave_switchdev_blocking_event(),
+	 * which is not under rcu_read_lock(), unlike
+	 * dsa_slave_switchdev_event().
+	 */
+	rcu_read_lock();
+	apply = dsa_port_can_apply_vlan_filtering(dp, vlan_filtering);
+	rcu_read_unlock();
+	if (!apply)
+		return -EINVAL;
 
 	if (dsa_port_is_vlan_filtering(dp) == vlan_filtering)
 		return 0;
 
-	err = ds->ops->port_vlan_filtering(ds, dp->index, vlan_filtering,
-					   trans);
+	err = ds->ops->port_vlan_filtering(ds, dp->index, vlan_filtering);
 	if (err)
 		return err;
 
-	if (switchdev_trans_ph_commit(trans)) {
-		if (ds->vlan_filtering_is_global)
-			ds->vlan_filtering = vlan_filtering;
-		else
-			dp->vlan_filtering = vlan_filtering;
-	}
+	if (ds->vlan_filtering_is_global)
+		ds->vlan_filtering = vlan_filtering;
+	else
+		dp->vlan_filtering = vlan_filtering;
 
 	return 0;
 }
@@ -314,26 +305,29 @@ bool dsa_port_skip_vlan_configuration(struct dsa_port *dp)
 		!br_vlan_enabled(dp->bridge_dev));
 }
 
-int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock,
-			 struct switchdev_trans *trans)
+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 = {
-		.ageing_time = ageing_time,
-		.trans = trans,
-	};
+	struct dsa_notifier_ageing_time_info info;
+	struct switchdev_trans trans;
+	int err;
+
+	info.ageing_time = ageing_time;
+	info.trans = &trans;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return dsa_port_notify(dp, DSA_NOTIFIER_AGEING_TIME, &info);
+	trans.ph_prepare = true;
+	err = dsa_port_notify(dp, DSA_NOTIFIER_AGEING_TIME, &info);
+	if (err)
+		return err;
 
 	dp->ageing_time = ageing_time;
 
+	trans.ph_prepare = false;
 	return dsa_port_notify(dp, DSA_NOTIFIER_AGEING_TIME, &info);
 }
 
-int dsa_port_pre_bridge_flags(const struct dsa_port *dp, unsigned long flags,
-			      struct switchdev_trans *trans)
+int dsa_port_pre_bridge_flags(const struct dsa_port *dp, unsigned long flags)
 {
 	struct dsa_switch *ds = dp->ds;
 
@@ -344,16 +338,12 @@ int dsa_port_pre_bridge_flags(const struct dsa_port *dp, unsigned long flags,
 	return 0;
 }
 
-int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags,
-			  struct switchdev_trans *trans)
+int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags)
 {
 	struct dsa_switch *ds = dp->ds;
 	int port = dp->index;
 	int err = 0;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return 0;
-
 	if (ds->ops->port_egress_floods)
 		err = ds->ops->port_egress_floods(ds, port, flags & BR_FLOOD,
 						  flags & BR_MCAST_FLOOD);
@@ -361,14 +351,13 @@ int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags,
 	return err;
 }
 
-int dsa_port_mrouter(struct dsa_port *dp, bool mrouter,
-		     struct switchdev_trans *trans)
+int dsa_port_mrouter(struct dsa_port *dp, bool mrouter)
 {
 	struct dsa_switch *ds = dp->ds;
 	int port = dp->index;
 
-	if (switchdev_trans_ph_prepare(trans))
-		return ds->ops->port_egress_floods ? 0 : -EOPNOTSUPP;
+	if (!ds->ops->port_egress_floods)
+		return -EOPNOTSUPP;
 
 	return ds->ops->port_egress_floods(ds, port, true, mrouter);
 }
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index d32fda1fed6f..ffad4324c736 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -268,32 +268,29 @@ static int dsa_slave_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 }
 
 static int dsa_slave_port_attr_set(struct net_device *dev,
-				   const struct switchdev_attr *attr,
-				   struct switchdev_trans *trans)
+				   const struct switchdev_attr *attr)
 {
 	struct dsa_port *dp = dsa_slave_to_port(dev);
 	int ret;
 
 	switch (attr->id) {
 	case SWITCHDEV_ATTR_ID_PORT_STP_STATE:
-		ret = dsa_port_set_state(dp, attr->u.stp_state, trans);
+		ret = dsa_port_set_state(dp, attr->u.stp_state);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING:
-		ret = dsa_port_vlan_filtering(dp, attr->u.vlan_filtering,
-					      trans);
+		ret = dsa_port_vlan_filtering(dp, attr->u.vlan_filtering);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
-		ret = dsa_port_ageing_time(dp, attr->u.ageing_time, trans);
+		ret = dsa_port_ageing_time(dp, attr->u.ageing_time);
 		break;
 	case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS:
-		ret = dsa_port_pre_bridge_flags(dp, attr->u.brport_flags,
-						trans);
+		ret = dsa_port_pre_bridge_flags(dp, attr->u.brport_flags);
 		break;
 	case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
-		ret = dsa_port_bridge_flags(dp, attr->u.brport_flags, trans);
+		ret = dsa_port_bridge_flags(dp, attr->u.brport_flags);
 		break;
 	case SWITCHDEV_ATTR_ID_BRIDGE_MROUTER:
-		ret = dsa_port_mrouter(dp->cpu_dp, attr->u.mrouter, trans);
+		ret = dsa_port_mrouter(dp->cpu_dp, attr->u.mrouter);
 		break;
 	default:
 		ret = -EOPNOTSUPP;
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 3fb362b6874e..54f8861e937e 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -139,17 +139,8 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds,
 		}
 	}
 	if (unset_vlan_filtering) {
-		struct switchdev_trans trans;
-
-		trans.ph_prepare = true;
-		err = dsa_port_vlan_filtering(dsa_to_port(ds, info->port),
-					      false, &trans);
-		if (err && err != EOPNOTSUPP)
-			return err;
-
-		trans.ph_prepare = false;
 		err = dsa_port_vlan_filtering(dsa_to_port(ds, info->port),
-					      false, &trans);
+					      false);
 		if (err && err != EOPNOTSUPP)
 			return err;
 	}
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 3509d362056d..855a10feef3d 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -100,15 +100,13 @@ static int switchdev_deferred_enqueue(struct net_device *dev,
 
 static int switchdev_port_attr_notify(enum switchdev_notifier_type nt,
 				      struct net_device *dev,
-				      const struct switchdev_attr *attr,
-				      struct switchdev_trans *trans)
+				      const struct switchdev_attr *attr)
 {
 	int err;
 	int rc;
 
 	struct switchdev_notifier_port_attr_info attr_info = {
 		.attr = attr,
-		.trans = trans,
 		.handled = false,
 	};
 
@@ -129,34 +127,7 @@ static int switchdev_port_attr_notify(enum switchdev_notifier_type nt,
 static int switchdev_port_attr_set_now(struct net_device *dev,
 				       const struct switchdev_attr *attr)
 {
-	struct switchdev_trans trans;
-	int err;
-
-	/* Phase I: prepare for attr set. Driver/device should fail
-	 * here if there are going to be issues in the commit phase,
-	 * such as lack of resources or support.  The driver/device
-	 * should reserve resources needed for the commit phase here,
-	 * but should not commit the attr.
-	 */
-
-	trans.ph_prepare = true;
-	err = switchdev_port_attr_notify(SWITCHDEV_PORT_ATTR_SET, dev, attr,
-					 &trans);
-	if (err)
-		return err;
-
-	/* Phase II: commit attr set.  This cannot fail as a fault
-	 * of driver/device.  If it does, it's a bug in the driver/device
-	 * because the driver said everythings was OK in phase I.
-	 */
-
-	trans.ph_prepare = false;
-	err = switchdev_port_attr_notify(SWITCHDEV_PORT_ATTR_SET, dev, attr,
-					 &trans);
-	WARN(err, "%s: Commit of attribute (id=%d) failed.\n",
-	     dev->name, attr->id);
-
-	return err;
+	return switchdev_port_attr_notify(SWITCHDEV_PORT_ATTR_SET, dev, attr);
 }
 
 static void switchdev_port_attr_set_deferred(struct net_device *dev,
@@ -186,10 +157,6 @@ static int switchdev_port_attr_set_defer(struct net_device *dev,
  *	@dev: port device
  *	@attr: attribute to set
  *
- *	Use a 2-phase prepare-commit transaction model to ensure
- *	system is not left in a partially updated state due to
- *	failure from driver/device.
- *
  *	rtnl_lock must be held and must not be in atomic section,
  *	in case SWITCHDEV_F_DEFER flag is not set.
  */
@@ -519,8 +486,7 @@ static int __switchdev_handle_port_attr_set(struct net_device *dev,
 			struct switchdev_notifier_port_attr_info *port_attr_info,
 			bool (*check_cb)(const struct net_device *dev),
 			int (*set_cb)(struct net_device *dev,
-				      const struct switchdev_attr *attr,
-				      struct switchdev_trans *trans))
+				      const struct switchdev_attr *attr))
 {
 	struct net_device *lower_dev;
 	struct list_head *iter;
@@ -528,8 +494,7 @@ static int __switchdev_handle_port_attr_set(struct net_device *dev,
 
 	if (check_cb(dev)) {
 		port_attr_info->handled = true;
-		return set_cb(dev, port_attr_info->attr,
-			      port_attr_info->trans);
+		return set_cb(dev, port_attr_info->attr);
 	}
 
 	/* Switch ports might be stacked under e.g. a LAG. Ignore the
@@ -556,8 +521,7 @@ int switchdev_handle_port_attr_set(struct net_device *dev,
 			struct switchdev_notifier_port_attr_info *port_attr_info,
 			bool (*check_cb)(const struct net_device *dev),
 			int (*set_cb)(struct net_device *dev,
-				      const struct switchdev_attr *attr,
-				      struct switchdev_trans *trans))
+				      const struct switchdev_attr *attr))
 {
 	int err;
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [RFC PATCH net-next 4/9] net: dsa: remove the transactional logic from ageing time notifiers
  2020-12-17  1:58 [RFC PATCH net-next 0/9] Get rid of the switchdev transactional model Vladimir Oltean
                   ` (2 preceding siblings ...)
  2020-12-17  1:58 ` [RFC PATCH net-next 3/9] net: switchdev: remove the transaction structure from port attributes Vladimir Oltean
@ 2020-12-17  1:58 ` Vladimir Oltean
  2020-12-17  2:06   ` Florian Fainelli
  2020-12-17  1:58 ` [RFC PATCH net-next 5/9] net: dsa: remove the transactional logic from MDB entries Vladimir Oltean
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 26+ messages in thread
From: Vladimir Oltean @ 2020-12-17  1:58 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh,
	Microchip Linux Driver Support, Sean Wang, Landen Chao,
	Claudiu Manoil, Alexandre Belloni, Linus Walleij, Vadym Kochan,
	Taras Chornyi, Jiri Pirko, Ido Schimmel, Grygorii Strashko,
	Ioana Ciornei, Ivan Vecera

Remove the shim introduced in DSA for offloading the bridge ageing time
from switchdev, by first checking whether the ageing time is within the
range limits requested by the driver.

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

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 54e2ee1a2131..90a879322b21 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -29,7 +29,6 @@ enum {
 
 /* DSA_NOTIFIER_AGEING_TIME */
 struct dsa_notifier_ageing_time_info {
-	struct switchdev_trans *trans;
 	unsigned int ageing_time;
 };
 
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 62f38a36386f..b39a5eebbe27 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -310,21 +310,17 @@ 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 switchdev_trans trans;
 	int err;
 
 	info.ageing_time = ageing_time;
-	info.trans = &trans;
 
-	trans.ph_prepare = true;
 	err = dsa_port_notify(dp, DSA_NOTIFIER_AGEING_TIME, &info);
 	if (err)
 		return err;
 
 	dp->ageing_time = ageing_time;
 
-	trans.ph_prepare = false;
-	return dsa_port_notify(dp, DSA_NOTIFIER_AGEING_TIME, &info);
+	return 0;
 }
 
 int dsa_port_pre_bridge_flags(const struct dsa_port *dp, unsigned long flags)
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 54f8861e937e..39848eac1da8 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -33,15 +33,12 @@ static int dsa_switch_ageing_time(struct dsa_switch *ds,
 				  struct dsa_notifier_ageing_time_info *info)
 {
 	unsigned int ageing_time = info->ageing_time;
-	struct switchdev_trans *trans = info->trans;
 
-	if (switchdev_trans_ph_prepare(trans)) {
-		if (ds->ageing_time_min && ageing_time < ds->ageing_time_min)
-			return -ERANGE;
-		if (ds->ageing_time_max && ageing_time > ds->ageing_time_max)
-			return -ERANGE;
-		return 0;
-	}
+	if (ds->ageing_time_min && ageing_time < ds->ageing_time_min)
+		return -ERANGE;
+
+	if (ds->ageing_time_max && ageing_time > ds->ageing_time_max)
+		return -ERANGE;
 
 	/* Program the fastest ageing time in case of multiple bridges */
 	ageing_time = dsa_switch_fastest_ageing_time(ds, ageing_time);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [RFC PATCH net-next 5/9] net: dsa: remove the transactional logic from MDB entries
  2020-12-17  1:58 [RFC PATCH net-next 0/9] Get rid of the switchdev transactional model Vladimir Oltean
                   ` (3 preceding siblings ...)
  2020-12-17  1:58 ` [RFC PATCH net-next 4/9] net: dsa: remove the transactional logic from ageing time notifiers Vladimir Oltean
@ 2020-12-17  1:58 ` Vladimir Oltean
  2020-12-17  2:08   ` Florian Fainelli
  2020-12-17 11:03   ` Vladimir Oltean
  2020-12-17  1:58 ` [RFC PATCH net-next 6/9] net: dsa: remove the transactional logic from VLAN objects Vladimir Oltean
                   ` (5 subsequent siblings)
  10 siblings, 2 replies; 26+ messages in thread
From: Vladimir Oltean @ 2020-12-17  1:58 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh,
	Microchip Linux Driver Support, Sean Wang, Landen Chao,
	Claudiu Manoil, Alexandre Belloni, Linus Walleij, Vadym Kochan,
	Taras Chornyi, Jiri Pirko, Ido Schimmel, Grygorii Strashko,
	Ioana Ciornei, Ivan Vecera

For many drivers, the .port_mdb_prepare callback was not a good opportunity
to avoid any error condition, and they would suppress errors found during
the actual commit phase.

Where a logical separation between the prepare and the commit phase
existed, the function that used to implement the .port_mdb_prepare
callback still exists, but now it is called directly from .port_mdb_add,
which was modified to return an int code.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/b53/b53_common.c       | 19 +++----------------
 drivers/net/dsa/b53/b53_priv.h         |  6 ++----
 drivers/net/dsa/bcm_sf2.c              |  1 -
 drivers/net/dsa/lan9303-core.c         | 12 ++++++++----
 drivers/net/dsa/microchip/ksz8795.c    |  1 -
 drivers/net/dsa/microchip/ksz9477.c    | 14 +++++++++-----
 drivers/net/dsa/microchip/ksz_common.c | 14 ++++----------
 drivers/net/dsa/microchip/ksz_common.h |  6 ++----
 drivers/net/dsa/mv88e6xxx/chip.c       | 24 +++++++-----------------
 drivers/net/dsa/ocelot/felix.c         | 14 +++-----------
 drivers/net/dsa/sja1105/sja1105_main.c | 14 +++-----------
 include/net/dsa.h                      |  4 +---
 net/dsa/dsa_priv.h                     |  1 -
 net/dsa/port.c                         | 10 ----------
 net/dsa/switch.c                       | 24 +-----------------------
 15 files changed, 43 insertions(+), 121 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index ce18ba0b74eb..9865e734150e 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1747,8 +1747,8 @@ int b53_fdb_dump(struct dsa_switch *ds, int port,
 }
 EXPORT_SYMBOL(b53_fdb_dump);
 
-int b53_mdb_prepare(struct dsa_switch *ds, int port,
-		    const struct switchdev_obj_port_mdb *mdb)
+int b53_mdb_add(struct dsa_switch *ds, int port,
+		const struct switchdev_obj_port_mdb *mdb)
 {
 	struct b53_device *priv = ds->priv;
 
@@ -1758,19 +1758,7 @@ int b53_mdb_prepare(struct dsa_switch *ds, int port,
 	if (is5325(priv) || is5365(priv))
 		return -EOPNOTSUPP;
 
-	return 0;
-}
-EXPORT_SYMBOL(b53_mdb_prepare);
-
-void b53_mdb_add(struct dsa_switch *ds, int port,
-		 const struct switchdev_obj_port_mdb *mdb)
-{
-	struct b53_device *priv = ds->priv;
-	int ret;
-
-	ret = b53_arl_op(priv, 0, port, mdb->addr, mdb->vid, true);
-	if (ret)
-		dev_err(ds->dev, "failed to add MDB entry\n");
+	return b53_arl_op(priv, 0, port, mdb->addr, mdb->vid, true);
 }
 EXPORT_SYMBOL(b53_mdb_add);
 
@@ -2211,7 +2199,6 @@ static const struct dsa_switch_ops b53_switch_ops = {
 	.port_fdb_del		= b53_fdb_del,
 	.port_mirror_add	= b53_mirror_add,
 	.port_mirror_del	= b53_mirror_del,
-	.port_mdb_prepare	= b53_mdb_prepare,
 	.port_mdb_add		= b53_mdb_add,
 	.port_mdb_del		= b53_mdb_del,
 	.port_max_mtu		= b53_get_max_mtu,
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index 24893b592216..224423ab0682 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -360,10 +360,8 @@ int b53_fdb_del(struct dsa_switch *ds, int port,
 		const unsigned char *addr, u16 vid);
 int b53_fdb_dump(struct dsa_switch *ds, int port,
 		 dsa_fdb_dump_cb_t *cb, void *data);
-int b53_mdb_prepare(struct dsa_switch *ds, int port,
-		    const struct switchdev_obj_port_mdb *mdb);
-void b53_mdb_add(struct dsa_switch *ds, int port,
-		 const struct switchdev_obj_port_mdb *mdb);
+int b53_mdb_add(struct dsa_switch *ds, int port,
+		const struct switchdev_obj_port_mdb *mdb);
 int b53_mdb_del(struct dsa_switch *ds, int port,
 		const struct switchdev_obj_port_mdb *mdb);
 int b53_mirror_add(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 1e9a0adda2d6..4c493bb47d30 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -1123,7 +1123,6 @@ static const struct dsa_switch_ops bcm_sf2_ops = {
 	.set_rxnfc		= bcm_sf2_set_rxnfc,
 	.port_mirror_add	= b53_mirror_add,
 	.port_mirror_del	= b53_mirror_del,
-	.port_mdb_prepare	= b53_mdb_prepare,
 	.port_mdb_add		= b53_mdb_add,
 	.port_mdb_del		= b53_mdb_del,
 };
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index aa1142d6a9f5..344374025426 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -1232,14 +1232,19 @@ static int lan9303_port_mdb_prepare(struct dsa_switch *ds, int port,
 	return 0;
 }
 
-static void lan9303_port_mdb_add(struct dsa_switch *ds, int port,
-				 const struct switchdev_obj_port_mdb *mdb)
+static int lan9303_port_mdb_add(struct dsa_switch *ds, int port,
+				const struct switchdev_obj_port_mdb *mdb)
 {
 	struct lan9303 *chip = ds->priv;
+	int err;
+
+	err = lan9303_port_mdb_prepare(ds, port, mdb);
+	if (err)
+		return err;
 
 	dev_dbg(chip->dev, "%s(%d, %pM, %d)\n", __func__, port, mdb->addr,
 		mdb->vid);
-	lan9303_alr_add_port(chip, mdb->addr, port, false);
+	return lan9303_alr_add_port(chip, mdb->addr, port, false);
 }
 
 static int lan9303_port_mdb_del(struct dsa_switch *ds, int port,
@@ -1274,7 +1279,6 @@ static const struct dsa_switch_ops lan9303_switch_ops = {
 	.port_fdb_add           = lan9303_port_fdb_add,
 	.port_fdb_del           = lan9303_port_fdb_del,
 	.port_fdb_dump          = lan9303_port_fdb_dump,
-	.port_mdb_prepare       = lan9303_port_mdb_prepare,
 	.port_mdb_add           = lan9303_port_mdb_add,
 	.port_mdb_del           = lan9303_port_mdb_del,
 };
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 754a2377989f..49cd82109ee9 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -1116,7 +1116,6 @@ static const struct dsa_switch_ops ksz8795_switch_ops = {
 	.port_vlan_add		= ksz8795_port_vlan_add,
 	.port_vlan_del		= ksz8795_port_vlan_del,
 	.port_fdb_dump		= ksz_port_fdb_dump,
-	.port_mdb_prepare       = ksz_port_mdb_prepare,
 	.port_mdb_add           = ksz_port_mdb_add,
 	.port_mdb_del           = ksz_port_mdb_del,
 	.port_mirror_add	= ksz8795_port_mirror_add,
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 4e181c09f286..8e7439e91f60 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -780,14 +780,15 @@ static int ksz9477_port_fdb_dump(struct dsa_switch *ds, int port,
 	return ret;
 }
 
-static void ksz9477_port_mdb_add(struct dsa_switch *ds, int port,
-				 const struct switchdev_obj_port_mdb *mdb)
+static int ksz9477_port_mdb_add(struct dsa_switch *ds, int port,
+				const struct switchdev_obj_port_mdb *mdb)
 {
 	struct ksz_device *dev = ds->priv;
 	u32 static_table[4];
 	u32 data;
 	int index;
 	u32 mac_hi, mac_lo;
+	int err = 0;
 
 	mac_hi = ((mdb->addr[0] << 8) | mdb->addr[1]);
 	mac_lo = ((mdb->addr[2] << 24) | (mdb->addr[3] << 16));
@@ -802,7 +803,8 @@ static void ksz9477_port_mdb_add(struct dsa_switch *ds, int port,
 		ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
 
 		/* wait to be finished */
-		if (ksz9477_wait_alu_sta_ready(dev)) {
+		err = ksz9477_wait_alu_sta_ready(dev);
+		if (err) {
 			dev_dbg(dev->dev, "Failed to read ALU STATIC\n");
 			goto exit;
 		}
@@ -825,8 +827,10 @@ static void ksz9477_port_mdb_add(struct dsa_switch *ds, int port,
 	}
 
 	/* no available entry */
-	if (index == dev->num_statics)
+	if (index == dev->num_statics) {
+		err = -ENOSPC;
 		goto exit;
+	}
 
 	/* add entry */
 	static_table[0] = ALU_V_STATIC_VALID;
@@ -848,6 +852,7 @@ static void ksz9477_port_mdb_add(struct dsa_switch *ds, int port,
 
 exit:
 	mutex_unlock(&dev->alu_mutex);
+	return err;
 }
 
 static int ksz9477_port_mdb_del(struct dsa_switch *ds, int port,
@@ -1401,7 +1406,6 @@ static const struct dsa_switch_ops ksz9477_switch_ops = {
 	.port_fdb_dump		= ksz9477_port_fdb_dump,
 	.port_fdb_add		= ksz9477_port_fdb_add,
 	.port_fdb_del		= ksz9477_port_fdb_del,
-	.port_mdb_prepare       = ksz_port_mdb_prepare,
 	.port_mdb_add           = ksz9477_port_mdb_add,
 	.port_mdb_del           = ksz9477_port_mdb_del,
 	.port_mirror_add	= ksz9477_port_mirror_add,
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index cf743133b0b9..874cdc38fad2 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -253,15 +253,7 @@ int ksz_port_fdb_dump(struct dsa_switch *ds, int port, dsa_fdb_dump_cb_t *cb,
 }
 EXPORT_SYMBOL_GPL(ksz_port_fdb_dump);
 
-int ksz_port_mdb_prepare(struct dsa_switch *ds, int port,
-			 const struct switchdev_obj_port_mdb *mdb)
-{
-	/* nothing to do */
-	return 0;
-}
-EXPORT_SYMBOL_GPL(ksz_port_mdb_prepare);
-
-void ksz_port_mdb_add(struct dsa_switch *ds, int port,
+int ksz_port_mdb_add(struct dsa_switch *ds, int port,
 		      const struct switchdev_obj_port_mdb *mdb)
 {
 	struct ksz_device *dev = ds->priv;
@@ -284,7 +276,7 @@ void ksz_port_mdb_add(struct dsa_switch *ds, int port,
 
 	/* no available entry */
 	if (index == dev->num_statics && !empty)
-		return;
+		return -ENOSPC;
 
 	/* add entry */
 	if (index == dev->num_statics) {
@@ -301,6 +293,8 @@ void ksz_port_mdb_add(struct dsa_switch *ds, int port,
 		alu.fid = mdb->vid;
 	}
 	dev->dev_ops->w_sta_mac_table(dev, index, &alu);
+
+	return 0;
 }
 EXPORT_SYMBOL_GPL(ksz_port_mdb_add);
 
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 720f22275c84..a1f0929d45a0 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -165,10 +165,8 @@ int ksz_port_vlan_prepare(struct dsa_switch *ds, int port,
 			  const struct switchdev_obj_port_vlan *vlan);
 int ksz_port_fdb_dump(struct dsa_switch *ds, int port, dsa_fdb_dump_cb_t *cb,
 		      void *data);
-int ksz_port_mdb_prepare(struct dsa_switch *ds, int port,
-			 const struct switchdev_obj_port_mdb *mdb);
-void ksz_port_mdb_add(struct dsa_switch *ds, int port,
-		      const struct switchdev_obj_port_mdb *mdb);
+int ksz_port_mdb_add(struct dsa_switch *ds, int port,
+		     const struct switchdev_obj_port_mdb *mdb);
 int ksz_port_mdb_del(struct dsa_switch *ds, int port,
 		     const struct switchdev_obj_port_mdb *mdb);
 int ksz_enable_port(struct dsa_switch *ds, int port, struct phy_device *phy);
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 9206ba37d4de..dc9da2d46786 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -5248,27 +5248,18 @@ static enum dsa_tag_protocol mv88e6xxx_get_tag_protocol(struct dsa_switch *ds,
 	return chip->info->tag_protocol;
 }
 
-static int mv88e6xxx_port_mdb_prepare(struct dsa_switch *ds, int port,
-				      const struct switchdev_obj_port_mdb *mdb)
-{
-	/* We don't need any dynamic resource from the kernel (yet),
-	 * so skip the prepare phase.
-	 */
-
-	return 0;
-}
-
-static void mv88e6xxx_port_mdb_add(struct dsa_switch *ds, int port,
-				   const struct switchdev_obj_port_mdb *mdb)
+static int mv88e6xxx_port_mdb_add(struct dsa_switch *ds, int port,
+				  const struct switchdev_obj_port_mdb *mdb)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
+	int err;
 
 	mv88e6xxx_reg_lock(chip);
-	if (mv88e6xxx_port_db_load_purge(chip, port, mdb->addr, mdb->vid,
-					 MV88E6XXX_G1_ATU_DATA_STATE_MC_STATIC))
-		dev_err(ds->dev, "p%d: failed to load multicast MAC address\n",
-			port);
+	err = mv88e6xxx_port_db_load_purge(chip, port, mdb->addr, mdb->vid,
+					   MV88E6XXX_G1_ATU_DATA_STATE_MC_STATIC);
 	mv88e6xxx_reg_unlock(chip);
+
+	return err;
 }
 
 static int mv88e6xxx_port_mdb_del(struct dsa_switch *ds, int port,
@@ -5413,7 +5404,6 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 	.port_fdb_add           = mv88e6xxx_port_fdb_add,
 	.port_fdb_del           = mv88e6xxx_port_fdb_del,
 	.port_fdb_dump          = mv88e6xxx_port_fdb_dump,
-	.port_mdb_prepare       = mv88e6xxx_port_mdb_prepare,
 	.port_mdb_add           = mv88e6xxx_port_mdb_add,
 	.port_mdb_del           = mv88e6xxx_port_mdb_del,
 	.port_mirror_add	= mv88e6xxx_port_mirror_add,
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index acf9507b1af3..d3e0693ae2df 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -65,19 +65,12 @@ static int felix_fdb_del(struct dsa_switch *ds, int port,
 	return ocelot_fdb_del(ocelot, port, addr, vid);
 }
 
-/* This callback needs to be present */
-static int felix_mdb_prepare(struct dsa_switch *ds, int port,
-			     const struct switchdev_obj_port_mdb *mdb)
-{
-	return 0;
-}
-
-static void felix_mdb_add(struct dsa_switch *ds, int port,
-			  const struct switchdev_obj_port_mdb *mdb)
+static int felix_mdb_add(struct dsa_switch *ds, int port,
+			 const struct switchdev_obj_port_mdb *mdb)
 {
 	struct ocelot *ocelot = ds->priv;
 
-	ocelot_port_mdb_add(ocelot, port, mdb);
+	return ocelot_port_mdb_add(ocelot, port, mdb);
 }
 
 static int felix_mdb_del(struct dsa_switch *ds, int port,
@@ -797,7 +790,6 @@ const struct dsa_switch_ops felix_switch_ops = {
 	.port_fdb_dump		= felix_fdb_dump,
 	.port_fdb_add		= felix_fdb_add,
 	.port_fdb_del		= felix_fdb_del,
-	.port_mdb_prepare	= felix_mdb_prepare,
 	.port_mdb_add		= felix_mdb_add,
 	.port_mdb_del		= felix_mdb_del,
 	.port_bridge_join	= felix_bridge_join,
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index dc82c71d1dbf..3dfcb750e169 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -1524,17 +1524,10 @@ static int sja1105_fdb_dump(struct dsa_switch *ds, int port,
 	return 0;
 }
 
-/* This callback needs to be present */
-static int sja1105_mdb_prepare(struct dsa_switch *ds, int port,
-			       const struct switchdev_obj_port_mdb *mdb)
-{
-	return 0;
-}
-
-static void sja1105_mdb_add(struct dsa_switch *ds, int port,
-			    const struct switchdev_obj_port_mdb *mdb)
+static int sja1105_mdb_add(struct dsa_switch *ds, int port,
+			   const struct switchdev_obj_port_mdb *mdb)
 {
-	sja1105_fdb_add(ds, port, mdb->addr, mdb->vid);
+	return sja1105_fdb_add(ds, port, mdb->addr, mdb->vid);
 }
 
 static int sja1105_mdb_del(struct dsa_switch *ds, int port,
@@ -3297,7 +3290,6 @@ static const struct dsa_switch_ops sja1105_switch_ops = {
 	.port_vlan_filtering	= sja1105_vlan_filtering,
 	.port_vlan_add		= sja1105_vlan_add,
 	.port_vlan_del		= sja1105_vlan_del,
-	.port_mdb_prepare	= sja1105_mdb_prepare,
 	.port_mdb_add		= sja1105_mdb_add,
 	.port_mdb_del		= sja1105_mdb_del,
 	.port_hwtstamp_get	= sja1105_hwtstamp_get,
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 7727b56a4eee..2d715b0502e3 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -580,10 +580,8 @@ struct dsa_switch_ops {
 	/*
 	 * Multicast database
 	 */
-	int (*port_mdb_prepare)(struct dsa_switch *ds, int port,
+	int	(*port_mdb_add)(struct dsa_switch *ds, int port,
 				const struct switchdev_obj_port_mdb *mdb);
-	void (*port_mdb_add)(struct dsa_switch *ds, int port,
-			     const struct switchdev_obj_port_mdb *mdb);
 	int	(*port_mdb_del)(struct dsa_switch *ds, int port,
 				const struct switchdev_obj_port_mdb *mdb);
 	/*
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 90a879322b21..f8362860cdaa 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -51,7 +51,6 @@ struct dsa_notifier_fdb_info {
 /* DSA_NOTIFIER_MDB_* */
 struct dsa_notifier_mdb_info {
 	const struct switchdev_obj_port_mdb *mdb;
-	struct switchdev_trans *trans;
 	int sw_index;
 	int port;
 };
diff --git a/net/dsa/port.c b/net/dsa/port.c
index b39a5eebbe27..c89e3c0c65ed 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -417,17 +417,7 @@ int dsa_port_mdb_add(const struct dsa_port *dp,
 		.port = dp->index,
 		.mdb = mdb,
 	};
-	struct switchdev_trans trans;
-	int err;
 
-	info.trans = &trans;
-
-	trans.ph_prepare = true;
-	err = dsa_port_notify(dp, DSA_NOTIFIER_MDB_ADD, &info);
-	if (err)
-		return err;
-
-	trans.ph_prepare = false;
 	return dsa_port_notify(dp, DSA_NOTIFIER_MDB_ADD, &info);
 }
 
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 39848eac1da8..65124bc3ddfb 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -178,35 +178,13 @@ static bool dsa_switch_mdb_match(struct dsa_switch *ds, int port,
 	return false;
 }
 
-static int dsa_switch_mdb_prepare(struct dsa_switch *ds,
-				  struct dsa_notifier_mdb_info *info)
-{
-	int port, err;
-
-	if (!ds->ops->port_mdb_prepare || !ds->ops->port_mdb_add)
-		return -EOPNOTSUPP;
-
-	for (port = 0; port < ds->num_ports; port++) {
-		if (dsa_switch_mdb_match(ds, port, info)) {
-			err = ds->ops->port_mdb_prepare(ds, port, info->mdb);
-			if (err)
-				return err;
-		}
-	}
-
-	return 0;
-}
-
 static int dsa_switch_mdb_add(struct dsa_switch *ds,
 			      struct dsa_notifier_mdb_info *info)
 {
 	int port;
 
-	if (switchdev_trans_ph_prepare(info->trans))
-		return dsa_switch_mdb_prepare(ds, info);
-
 	if (!ds->ops->port_mdb_add)
-		return 0;
+		return -EOPNOTSUPP;
 
 	for (port = 0; port < ds->num_ports; port++)
 		if (dsa_switch_mdb_match(ds, port, info))
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [RFC PATCH net-next 6/9] net: dsa: remove the transactional logic from VLAN objects
  2020-12-17  1:58 [RFC PATCH net-next 0/9] Get rid of the switchdev transactional model Vladimir Oltean
                   ` (4 preceding siblings ...)
  2020-12-17  1:58 ` [RFC PATCH net-next 5/9] net: dsa: remove the transactional logic from MDB entries Vladimir Oltean
@ 2020-12-17  1:58 ` Vladimir Oltean
  2020-12-17  2:11   ` Florian Fainelli
                     ` (3 more replies)
  2020-12-17  1:58 ` [RFC PATCH net-next 7/9] net: dsa: remove obsolete comment about switchdev transactions Vladimir Oltean
                   ` (4 subsequent siblings)
  10 siblings, 4 replies; 26+ messages in thread
From: Vladimir Oltean @ 2020-12-17  1:58 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh,
	Microchip Linux Driver Support, Sean Wang, Landen Chao,
	Claudiu Manoil, Alexandre Belloni, Linus Walleij, Vadym Kochan,
	Taras Chornyi, Jiri Pirko, Ido Schimmel, Grygorii Strashko,
	Ioana Ciornei, Ivan Vecera

It should be the driver's business to logically separate its VLAN
offloading into a preparation and a commit phase, and some drivers don't
need / can't do this.

So remove the transactional shim from DSA and let drivers to propagate
errors directly from the .port_vlan_add callback.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/b53/b53_common.c       | 17 +++++++----
 drivers/net/dsa/b53/b53_priv.h         |  6 ++--
 drivers/net/dsa/bcm_sf2.c              |  1 -
 drivers/net/dsa/bcm_sf2_cfp.c          |  7 ++---
 drivers/net/dsa/dsa_loop.c             | 29 +++++-------------
 drivers/net/dsa/hirschmann/hellcreek.c | 12 ++++++--
 drivers/net/dsa/lantiq_gswip.c         | 14 ++++++---
 drivers/net/dsa/microchip/ksz8795.c    |  7 +++--
 drivers/net/dsa/microchip/ksz9477.c    | 20 ++++++-------
 drivers/net/dsa/microchip/ksz_common.c |  9 ------
 drivers/net/dsa/microchip/ksz_common.h |  2 --
 drivers/net/dsa/mt7530.c               | 14 ++-------
 drivers/net/dsa/mv88e6xxx/chip.c       | 41 ++++++++++++++++----------
 drivers/net/dsa/ocelot/felix.c         | 13 +++++---
 drivers/net/dsa/qca8k.c                | 14 ++++-----
 drivers/net/dsa/realtek-smi-core.h     |  6 ++--
 drivers/net/dsa/rtl8366.c              | 25 +++++++++++-----
 drivers/net/dsa/rtl8366rb.c            |  1 -
 drivers/net/dsa/sja1105/sja1105_main.c | 17 ++++++-----
 include/net/dsa.h                      |  4 +--
 net/dsa/dsa_priv.h                     |  1 -
 net/dsa/port.c                         | 10 -------
 net/dsa/switch.c                       | 24 +--------------
 23 files changed, 128 insertions(+), 166 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 9865e734150e..e8fe8ba226c4 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1384,8 +1384,8 @@ int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering)
 }
 EXPORT_SYMBOL(b53_vlan_filtering);
 
-int b53_vlan_prepare(struct dsa_switch *ds, int port,
-		     const struct switchdev_obj_port_vlan *vlan)
+static int b53_vlan_prepare(struct dsa_switch *ds, int port,
+			    const struct switchdev_obj_port_vlan *vlan)
 {
 	struct b53_device *dev = ds->priv;
 
@@ -1407,16 +1407,20 @@ int b53_vlan_prepare(struct dsa_switch *ds, int port,
 
 	return 0;
 }
-EXPORT_SYMBOL(b53_vlan_prepare);
 
-void b53_vlan_add(struct dsa_switch *ds, int port,
-		  const struct switchdev_obj_port_vlan *vlan)
+int b53_vlan_add(struct dsa_switch *ds, int port,
+		 const struct switchdev_obj_port_vlan *vlan)
 {
 	struct b53_device *dev = ds->priv;
 	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
 	bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
 	struct b53_vlan *vl;
 	u16 vid;
+	int err;
+
+	err = b53_vlan_prepare(ds, port, vlan);
+	if (err)
+		return err;
 
 	for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid) {
 		vl = &dev->vlans[vid];
@@ -1441,6 +1445,8 @@ void b53_vlan_add(struct dsa_switch *ds, int port,
 			    vlan->vid_end);
 		b53_fast_age_vlan(dev, vid);
 	}
+
+	return 0;
 }
 EXPORT_SYMBOL(b53_vlan_add);
 
@@ -2191,7 +2197,6 @@ static const struct dsa_switch_ops b53_switch_ops = {
 	.port_fast_age		= b53_br_fast_age,
 	.port_egress_floods	= b53_br_egress_floods,
 	.port_vlan_filtering	= b53_vlan_filtering,
-	.port_vlan_prepare	= b53_vlan_prepare,
 	.port_vlan_add		= b53_vlan_add,
 	.port_vlan_del		= b53_vlan_del,
 	.port_fdb_dump		= b53_fdb_dump,
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index 224423ab0682..7cdf36755a2b 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -348,10 +348,8 @@ void b53_phylink_mac_link_up(struct dsa_switch *ds, int port,
 			     int speed, int duplex,
 			     bool tx_pause, bool rx_pause);
 int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering);
-int b53_vlan_prepare(struct dsa_switch *ds, int port,
-		     const struct switchdev_obj_port_vlan *vlan);
-void b53_vlan_add(struct dsa_switch *ds, int port,
-		  const struct switchdev_obj_port_vlan *vlan);
+int b53_vlan_add(struct dsa_switch *ds, int port,
+		 const struct switchdev_obj_port_vlan *vlan);
 int b53_vlan_del(struct dsa_switch *ds, int port,
 		 const struct switchdev_obj_port_vlan *vlan);
 int b53_fdb_add(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 4c493bb47d30..e377ab142e41 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -1113,7 +1113,6 @@ static const struct dsa_switch_ops bcm_sf2_ops = {
 	.port_stp_state_set	= b53_br_set_stp_state,
 	.port_fast_age		= b53_br_fast_age,
 	.port_vlan_filtering	= b53_vlan_filtering,
-	.port_vlan_prepare	= b53_vlan_prepare,
 	.port_vlan_add		= b53_vlan_add,
 	.port_vlan_del		= b53_vlan_del,
 	.port_fdb_dump		= b53_fdb_dump,
diff --git a/drivers/net/dsa/bcm_sf2_cfp.c b/drivers/net/dsa/bcm_sf2_cfp.c
index d82cee5d9202..764503fd8bcb 100644
--- a/drivers/net/dsa/bcm_sf2_cfp.c
+++ b/drivers/net/dsa/bcm_sf2_cfp.c
@@ -892,11 +892,9 @@ static int bcm_sf2_cfp_rule_insert(struct dsa_switch *ds, int port,
 		else
 			vlan.flags = 0;
 
-		ret = ds->ops->port_vlan_prepare(ds, port_num, &vlan);
+		ret = ds->ops->port_vlan_add(ds, port_num, &vlan);
 		if (ret)
 			return ret;
-
-		ds->ops->port_vlan_add(ds, port_num, &vlan);
 	}
 
 	/*
@@ -942,8 +940,7 @@ static int bcm_sf2_cfp_rule_set(struct dsa_switch *ds, int port,
 		return -EINVAL;
 
 	if ((fs->flow_type & FLOW_EXT) &&
-	    !(ds->ops->port_vlan_prepare || ds->ops->port_vlan_add ||
-	      ds->ops->port_vlan_del))
+	    !(ds->ops->port_vlan_add || ds->ops->port_vlan_del))
 		return -EOPNOTSUPP;
 
 	if (fs->location != RX_CLS_LOC_ANY &&
diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
index b588614d1e5e..7c16fc66c730 100644
--- a/drivers/net/dsa/dsa_loop.c
+++ b/drivers/net/dsa/dsa_loop.c
@@ -198,27 +198,8 @@ static int dsa_loop_port_vlan_filtering(struct dsa_switch *ds, int port,
 	return 0;
 }
 
-static int
-dsa_loop_port_vlan_prepare(struct dsa_switch *ds, int port,
-			   const struct switchdev_obj_port_vlan *vlan)
-{
-	struct dsa_loop_priv *ps = ds->priv;
-	struct mii_bus *bus = ps->bus;
-
-	dev_dbg(ds->dev, "%s: port: %d, vlan: %d-%d",
-		__func__, port, vlan->vid_begin, vlan->vid_end);
-
-	/* Just do a sleeping operation to make lockdep checks effective */
-	mdiobus_read(bus, ps->port_base + port, MII_BMSR);
-
-	if (vlan->vid_end > ARRAY_SIZE(ps->vlans))
-		return -ERANGE;
-
-	return 0;
-}
-
-static void dsa_loop_port_vlan_add(struct dsa_switch *ds, int port,
-				   const struct switchdev_obj_port_vlan *vlan)
+static int dsa_loop_port_vlan_add(struct dsa_switch *ds, int port,
+				  const struct switchdev_obj_port_vlan *vlan)
 {
 	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
 	bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
@@ -227,6 +208,9 @@ static void dsa_loop_port_vlan_add(struct dsa_switch *ds, int port,
 	struct dsa_loop_vlan *vl;
 	u16 vid;
 
+	if (vlan->vid_end > ARRAY_SIZE(ps->vlans))
+		return -ERANGE;
+
 	/* Just do a sleeping operation to make lockdep checks effective */
 	mdiobus_read(bus, ps->port_base + port, MII_BMSR);
 
@@ -245,6 +229,8 @@ static void dsa_loop_port_vlan_add(struct dsa_switch *ds, int port,
 
 	if (pvid)
 		ps->ports[port].pvid = vid;
+
+	return 0;
 }
 
 static int dsa_loop_port_vlan_del(struct dsa_switch *ds, int port,
@@ -306,7 +292,6 @@ static const struct dsa_switch_ops dsa_loop_driver = {
 	.port_bridge_leave	= dsa_loop_port_bridge_leave,
 	.port_stp_state_set	= dsa_loop_port_stp_state_set,
 	.port_vlan_filtering	= dsa_loop_port_vlan_filtering,
-	.port_vlan_prepare	= dsa_loop_port_vlan_prepare,
 	.port_vlan_add		= dsa_loop_port_vlan_add,
 	.port_vlan_del		= dsa_loop_port_vlan_del,
 	.port_change_mtu	= dsa_loop_port_change_mtu,
diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index 88d4f602b21c..f107185419ea 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -440,13 +440,18 @@ static void hellcreek_unapply_vlan(struct hellcreek *hellcreek, int port,
 	mutex_unlock(&hellcreek->reg_lock);
 }
 
-static void hellcreek_vlan_add(struct dsa_switch *ds, int port,
-			       const struct switchdev_obj_port_vlan *vlan)
+static int hellcreek_vlan_add(struct dsa_switch *ds, int port,
+			      const struct switchdev_obj_port_vlan *vlan)
 {
 	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
 	bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
 	struct hellcreek *hellcreek = ds->priv;
 	u16 vid;
+	int err;
+
+	err = hellcreek_vlan_prepare(ds, port, vlan);
+	if (err)
+		return err;
 
 	dev_dbg(hellcreek->dev, "Add VLANs (%d -- %d) on port %d, %s, %s\n",
 		vlan->vid_begin, vlan->vid_end, port,
@@ -455,6 +460,8 @@ static void hellcreek_vlan_add(struct dsa_switch *ds, int port,
 
 	for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid)
 		hellcreek_apply_vlan(hellcreek, port, vid, pvid, untagged);
+
+	return 0;
 }
 
 static int hellcreek_vlan_del(struct dsa_switch *ds, int port,
@@ -1154,7 +1161,6 @@ static const struct dsa_switch_ops hellcreek_ds_ops = {
 	.port_vlan_add	     = hellcreek_vlan_add,
 	.port_vlan_del	     = hellcreek_vlan_del,
 	.port_vlan_filtering = hellcreek_vlan_filtering,
-	.port_vlan_prepare   = hellcreek_vlan_prepare,
 	.setup		     = hellcreek_setup,
 };
 
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 6f8e46d21981..dcfcb17eac48 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1183,14 +1183,19 @@ static int gswip_port_vlan_prepare(struct dsa_switch *ds, int port,
 	return 0;
 }
 
-static void gswip_port_vlan_add(struct dsa_switch *ds, int port,
-				const struct switchdev_obj_port_vlan *vlan)
+static int gswip_port_vlan_add(struct dsa_switch *ds, int port,
+			       const struct switchdev_obj_port_vlan *vlan)
 {
 	struct gswip_priv *priv = ds->priv;
 	struct net_device *bridge = dsa_to_port(ds, port)->bridge_dev;
 	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
 	bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
 	u16 vid;
+	int err;
+
+	err = gswip_port_vlan_prepare(ds, port, vlan);
+	if (err)
+		return err;
 
 	/* We have to receive all packets on the CPU port and should not
 	 * do any VLAN filtering here. This is also called with bridge
@@ -1198,10 +1203,12 @@ static void gswip_port_vlan_add(struct dsa_switch *ds, int port,
 	 * this.
 	 */
 	if (dsa_is_cpu_port(ds, port))
-		return;
+		return 0;
 
 	for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid)
 		gswip_vlan_add_aware(priv, bridge, port, vid, untagged, pvid);
+
+	return 0;
 }
 
 static int gswip_port_vlan_del(struct dsa_switch *ds, int port,
@@ -1607,7 +1614,6 @@ static const struct dsa_switch_ops gswip_switch_ops = {
 	.port_bridge_leave	= gswip_port_bridge_leave,
 	.port_fast_age		= gswip_port_fast_age,
 	.port_vlan_filtering	= gswip_port_vlan_filtering,
-	.port_vlan_prepare	= gswip_port_vlan_prepare,
 	.port_vlan_add		= gswip_port_vlan_add,
 	.port_vlan_del		= gswip_port_vlan_del,
 	.port_stp_state_set	= gswip_port_stp_state_set,
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 49cd82109ee9..9b59cf9ef840 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -792,8 +792,8 @@ static int ksz8795_port_vlan_filtering(struct dsa_switch *ds, int port,
 	return 0;
 }
 
-static void ksz8795_port_vlan_add(struct dsa_switch *ds, int port,
-				  const struct switchdev_obj_port_vlan *vlan)
+static int ksz8795_port_vlan_add(struct dsa_switch *ds, int port,
+				 const struct switchdev_obj_port_vlan *vlan)
 {
 	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
 	struct ksz_device *dev = ds->priv;
@@ -828,6 +828,8 @@ static void ksz8795_port_vlan_add(struct dsa_switch *ds, int port,
 		vid |= new_pvid;
 		ksz_pwrite16(dev, port, REG_PORT_CTRL_VID, vid);
 	}
+
+	return 0;
 }
 
 static int ksz8795_port_vlan_del(struct dsa_switch *ds, int port,
@@ -1112,7 +1114,6 @@ static const struct dsa_switch_ops ksz8795_switch_ops = {
 	.port_stp_state_set	= ksz8795_port_stp_state_set,
 	.port_fast_age		= ksz_port_fast_age,
 	.port_vlan_filtering	= ksz8795_port_vlan_filtering,
-	.port_vlan_prepare	= ksz_port_vlan_prepare,
 	.port_vlan_add		= ksz8795_port_vlan_add,
 	.port_vlan_del		= ksz8795_port_vlan_del,
 	.port_fdb_dump		= ksz_port_fdb_dump,
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 8e7439e91f60..5ca826079af6 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -510,19 +510,19 @@ static int ksz9477_port_vlan_filtering(struct dsa_switch *ds, int port,
 	return 0;
 }
 
-static void ksz9477_port_vlan_add(struct dsa_switch *ds, int port,
+static int ksz9477_port_vlan_add(struct dsa_switch *ds, int port,
 				  const struct switchdev_obj_port_vlan *vlan)
 {
 	struct ksz_device *dev = ds->priv;
 	u32 vlan_table[3];
 	u16 vid;
 	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
+	int err;
 
 	for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
-		if (ksz9477_get_vlan_table(dev, vid, vlan_table)) {
-			dev_dbg(dev->dev, "Failed to get vlan table\n");
-			return;
-		}
+		err = ksz9477_get_vlan_table(dev, vid, vlan_table);
+		if (err)
+			return err;
 
 		vlan_table[0] = VLAN_VALID | (vid & VLAN_FID_M);
 		if (untagged)
@@ -533,15 +533,16 @@ static void ksz9477_port_vlan_add(struct dsa_switch *ds, int port,
 
 		vlan_table[2] |= BIT(port) | BIT(dev->cpu_port);
 
-		if (ksz9477_set_vlan_table(dev, vid, vlan_table)) {
-			dev_dbg(dev->dev, "Failed to set vlan table\n");
-			return;
-		}
+		err = ksz9477_set_vlan_table(dev, vid, vlan_table);
+		if (err)
+			return err;
 
 		/* change PVID */
 		if (vlan->flags & BRIDGE_VLAN_INFO_PVID)
 			ksz_pwrite16(dev, port, REG_PORT_DEFAULT_VID, vid);
 	}
+
+	return 0;
 }
 
 static int ksz9477_port_vlan_del(struct dsa_switch *ds, int port,
@@ -1400,7 +1401,6 @@ static const struct dsa_switch_ops ksz9477_switch_ops = {
 	.port_stp_state_set	= ksz9477_port_stp_state_set,
 	.port_fast_age		= ksz_port_fast_age,
 	.port_vlan_filtering	= ksz9477_port_vlan_filtering,
-	.port_vlan_prepare	= ksz_port_vlan_prepare,
 	.port_vlan_add		= ksz9477_port_vlan_add,
 	.port_vlan_del		= ksz9477_port_vlan_del,
 	.port_fdb_dump		= ksz9477_port_fdb_dump,
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 874cdc38fad2..0a5878c01091 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -213,15 +213,6 @@ void ksz_port_fast_age(struct dsa_switch *ds, int port)
 }
 EXPORT_SYMBOL_GPL(ksz_port_fast_age);
 
-int ksz_port_vlan_prepare(struct dsa_switch *ds, int port,
-			  const struct switchdev_obj_port_vlan *vlan)
-{
-	/* nothing needed */
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(ksz_port_vlan_prepare);
-
 int ksz_port_fdb_dump(struct dsa_switch *ds, int port, dsa_fdb_dump_cb_t *cb,
 		      void *data)
 {
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index a1f0929d45a0..f212775372ce 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -161,8 +161,6 @@ int ksz_port_bridge_join(struct dsa_switch *ds, int port,
 void ksz_port_bridge_leave(struct dsa_switch *ds, int port,
 			   struct net_device *br);
 void ksz_port_fast_age(struct dsa_switch *ds, int port);
-int ksz_port_vlan_prepare(struct dsa_switch *ds, int port,
-			  const struct switchdev_obj_port_vlan *vlan);
 int ksz_port_fdb_dump(struct dsa_switch *ds, int port, dsa_fdb_dump_cb_t *cb,
 		      void *data);
 int ksz_port_mdb_add(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 95ea7d1ecb05..5be38faf0463 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1393,15 +1393,6 @@ mt7530_port_vlan_filtering(struct dsa_switch *ds, int port,
 	return 0;
 }
 
-static int
-mt7530_port_vlan_prepare(struct dsa_switch *ds, int port,
-			 const struct switchdev_obj_port_vlan *vlan)
-{
-	/* nothing needed */
-
-	return 0;
-}
-
 static void
 mt7530_hw_vlan_add(struct mt7530_priv *priv,
 		   struct mt7530_hw_vlan_entry *entry)
@@ -1489,7 +1480,7 @@ mt7530_hw_vlan_update(struct mt7530_priv *priv, u16 vid,
 	mt7530_vlan_cmd(priv, MT7530_VTCR_WR_VID, vid);
 }
 
-static void
+static int
 mt7530_port_vlan_add(struct dsa_switch *ds, int port,
 		     const struct switchdev_obj_port_vlan *vlan)
 {
@@ -1514,6 +1505,8 @@ mt7530_port_vlan_add(struct dsa_switch *ds, int port,
 	}
 
 	mutex_unlock(&priv->reg_mutex);
+
+	return 0;
 }
 
 static int
@@ -2614,7 +2607,6 @@ static const struct dsa_switch_ops mt7530_switch_ops = {
 	.port_fdb_del		= mt7530_port_fdb_del,
 	.port_fdb_dump		= mt7530_port_fdb_dump,
 	.port_vlan_filtering	= mt7530_port_vlan_filtering,
-	.port_vlan_prepare	= mt7530_port_vlan_prepare,
 	.port_vlan_add		= mt7530_port_vlan_add,
 	.port_vlan_del		= mt7530_port_vlan_del,
 	.port_mirror_add	= mt753x_port_mirror_add,
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index dc9da2d46786..2aae7e82f9cd 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1620,9 +1620,6 @@ mv88e6xxx_port_vlan_prepare(struct dsa_switch *ds, int port,
 					   vlan->vid_end);
 	mv88e6xxx_reg_unlock(chip);
 
-	/* We don't need any dynamic resource from the kernel (yet),
-	 * so skip the prepare phase.
-	 */
 	return err;
 }
 
@@ -1969,8 +1966,8 @@ static int mv88e6xxx_port_vlan_join(struct mv88e6xxx_chip *chip, int port,
 	return 0;
 }
 
-static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
-				    const struct switchdev_obj_port_vlan *vlan)
+static int mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
+				   const struct switchdev_obj_port_vlan *vlan)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
 	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
@@ -1978,9 +1975,11 @@ static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
 	bool warn;
 	u8 member;
 	u16 vid;
+	int err;
 
-	if (!mv88e6xxx_max_vid(chip))
-		return;
+	err = mv88e6xxx_port_vlan_prepare(ds, port, vlan);
+	if (err)
+		return err;
 
 	if (dsa_is_dsa_port(ds, port) || dsa_is_cpu_port(ds, port))
 		member = MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_UNMODIFIED;
@@ -1996,16 +1995,29 @@ static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
 
 	mv88e6xxx_reg_lock(chip);
 
-	for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid)
-		if (mv88e6xxx_port_vlan_join(chip, port, vid, member, warn))
-			dev_err(ds->dev, "p%d: failed to add VLAN %d%c\n", port,
-				vid, untagged ? 'u' : 't');
+	for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid) {
+		err = mv88e6xxx_port_vlan_join(chip, port, vid, member, warn);
+		if (err)
+			break;
+	}
+
+	if (err) {
+		dev_err(ds->dev, "p%d: failed to add VLAN %d%c\n", port,
+			vid, untagged ? 'u' : 't');
+		return err;
+	}
 
-	if (pvid && mv88e6xxx_port_set_pvid(chip, port, vlan->vid_end))
-		dev_err(ds->dev, "p%d: failed to set PVID %d\n", port,
-			vlan->vid_end);
+	if (pvid) {
+		err = mv88e6xxx_port_set_pvid(chip, port, vlan->vid_end);
+		if (err) {
+			dev_err(ds->dev, "p%d: failed to set PVID %d\n", port,
+				vlan->vid_end);
+		}
+	}
 
 	mv88e6xxx_reg_unlock(chip);
+
+	return err;
 }
 
 static int mv88e6xxx_port_vlan_leave(struct mv88e6xxx_chip *chip,
@@ -5398,7 +5410,6 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 	.port_stp_state_set	= mv88e6xxx_port_stp_state_set,
 	.port_fast_age		= mv88e6xxx_port_fast_age,
 	.port_vlan_filtering	= mv88e6xxx_port_vlan_filtering,
-	.port_vlan_prepare	= mv88e6xxx_port_vlan_prepare,
 	.port_vlan_add		= mv88e6xxx_port_vlan_add,
 	.port_vlan_del		= mv88e6xxx_port_vlan_del,
 	.port_fdb_add           = mv88e6xxx_port_fdb_add,
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index d3e0693ae2df..76edbfd7b8e9 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -141,14 +141,18 @@ static int felix_vlan_filtering(struct dsa_switch *ds, int port, bool enabled)
 	return ocelot_port_vlan_filtering(ocelot, port, enabled);
 }
 
-static void felix_vlan_add(struct dsa_switch *ds, int port,
-			   const struct switchdev_obj_port_vlan *vlan)
+static int felix_vlan_add(struct dsa_switch *ds, int port,
+			  const struct switchdev_obj_port_vlan *vlan)
 {
 	struct ocelot *ocelot = ds->priv;
 	u16 flags = vlan->flags;
 	u16 vid;
 	int err;
 
+	err = felix_vlan_prepare(ds, port, vlan);
+	if (err)
+		return err;
+
 	for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
 		err = ocelot_vlan_add(ocelot, port, vid,
 				      flags & BRIDGE_VLAN_INFO_PVID,
@@ -156,9 +160,11 @@ static void felix_vlan_add(struct dsa_switch *ds, int port,
 		if (err) {
 			dev_err(ds->dev, "Failed to add VLAN %d to port %d: %d\n",
 				vid, port, err);
-			return;
+			return err;
 		}
 	}
+
+	return 0;
 }
 
 static int felix_vlan_del(struct dsa_switch *ds, int port,
@@ -795,7 +801,6 @@ const struct dsa_switch_ops felix_switch_ops = {
 	.port_bridge_join	= felix_bridge_join,
 	.port_bridge_leave	= felix_bridge_leave,
 	.port_stp_state_set	= felix_bridge_stp_state_set,
-	.port_vlan_prepare	= felix_vlan_prepare,
 	.port_vlan_filtering	= felix_vlan_filtering,
 	.port_vlan_add		= felix_vlan_add,
 	.port_vlan_del		= felix_vlan_del,
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 7adf71ebd9fe..7978027aded8 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1312,13 +1312,6 @@ qca8k_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering)
 }
 
 static int
-qca8k_port_vlan_prepare(struct dsa_switch *ds, int port,
-			const struct switchdev_obj_port_vlan *vlan)
-{
-	return 0;
-}
-
-static void
 qca8k_port_vlan_add(struct dsa_switch *ds, int port,
 		    const struct switchdev_obj_port_vlan *vlan)
 {
@@ -1331,8 +1324,10 @@ qca8k_port_vlan_add(struct dsa_switch *ds, int port,
 	for (vid = vlan->vid_begin; vid <= vlan->vid_end && !ret; ++vid)
 		ret = qca8k_vlan_add(priv, port, vid, untagged);
 
-	if (ret)
+	if (ret) {
 		dev_err(priv->dev, "Failed to add VLAN to port %d (%d)", port, ret);
+		return ret;
+	}
 
 	if (pvid) {
 		int shift = 16 * (port % 2);
@@ -1344,6 +1339,8 @@ qca8k_port_vlan_add(struct dsa_switch *ds, int port,
 			    QCA8K_PORT_VLAN_CVID(vlan->vid_end) |
 			    QCA8K_PORT_VLAN_SVID(vlan->vid_end));
 	}
+
+	return 0;
 }
 
 static int
@@ -1389,7 +1386,6 @@ static const struct dsa_switch_ops qca8k_switch_ops = {
 	.port_fdb_del		= qca8k_port_fdb_del,
 	.port_fdb_dump		= qca8k_port_fdb_dump,
 	.port_vlan_filtering	= qca8k_port_vlan_filtering,
-	.port_vlan_prepare	= qca8k_port_vlan_prepare,
 	.port_vlan_add		= qca8k_port_vlan_add,
 	.port_vlan_del		= qca8k_port_vlan_del,
 	.phylink_validate	= qca8k_phylink_validate,
diff --git a/drivers/net/dsa/realtek-smi-core.h b/drivers/net/dsa/realtek-smi-core.h
index bc7bd47fb037..26376b052594 100644
--- a/drivers/net/dsa/realtek-smi-core.h
+++ b/drivers/net/dsa/realtek-smi-core.h
@@ -132,10 +132,8 @@ int rtl8366_reset_vlan(struct realtek_smi *smi);
 int rtl8366_init_vlan(struct realtek_smi *smi);
 int rtl8366_vlan_filtering(struct dsa_switch *ds, int port,
 			   bool vlan_filtering);
-int rtl8366_vlan_prepare(struct dsa_switch *ds, int port,
-			 const struct switchdev_obj_port_vlan *vlan);
-void rtl8366_vlan_add(struct dsa_switch *ds, int port,
-		      const struct switchdev_obj_port_vlan *vlan);
+int rtl8366_vlan_add(struct dsa_switch *ds, int port,
+		     const struct switchdev_obj_port_vlan *vlan);
 int rtl8366_vlan_del(struct dsa_switch *ds, int port,
 		     const struct switchdev_obj_port_vlan *vlan);
 void rtl8366_get_strings(struct dsa_switch *ds, int port, u32 stringset,
diff --git a/drivers/net/dsa/rtl8366.c b/drivers/net/dsa/rtl8366.c
index 212c2bf87aac..30dba9e76c66 100644
--- a/drivers/net/dsa/rtl8366.c
+++ b/drivers/net/dsa/rtl8366.c
@@ -374,8 +374,8 @@ int rtl8366_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering)
 }
 EXPORT_SYMBOL_GPL(rtl8366_vlan_filtering);
 
-int rtl8366_vlan_prepare(struct dsa_switch *ds, int port,
-			 const struct switchdev_obj_port_vlan *vlan)
+static int rtl8366_vlan_prepare(struct dsa_switch *ds, int port,
+				const struct switchdev_obj_port_vlan *vlan)
 {
 	struct realtek_smi *smi = ds->priv;
 	u16 vid;
@@ -393,10 +393,9 @@ int rtl8366_vlan_prepare(struct dsa_switch *ds, int port,
 	 */
 	return rtl8366_enable_vlan4k(smi, true);
 }
-EXPORT_SYMBOL_GPL(rtl8366_vlan_prepare);
 
-void rtl8366_vlan_add(struct dsa_switch *ds, int port,
-		      const struct switchdev_obj_port_vlan *vlan)
+int rtl8366_vlan_add(struct dsa_switch *ds, int port,
+		     const struct switchdev_obj_port_vlan *vlan)
 {
 	bool untagged = !!(vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED);
 	bool pvid = !!(vlan->flags & BRIDGE_VLAN_INFO_PVID);
@@ -406,9 +405,13 @@ void rtl8366_vlan_add(struct dsa_switch *ds, int port,
 	u16 vid;
 	int ret;
 
+	ret = rtl8366_vlan_prepare(ds, port, vlan);
+	if (ret)
+		return ret;
+
 	for (vid = vlan->vid_begin; vid < vlan->vid_end; vid++)
 		if (!smi->ops->is_vlan_valid(smi, vid))
-			return;
+			return -EINVAL;
 
 	dev_info(smi->dev, "add VLAN %d on port %d, %s, %s\n",
 		 vlan->vid_begin,
@@ -426,24 +429,30 @@ void rtl8366_vlan_add(struct dsa_switch *ds, int port,
 			untag |= BIT(port);
 
 		ret = rtl8366_set_vlan(smi, vid, member, untag, 0);
-		if (ret)
+		if (ret) {
 			dev_err(smi->dev,
 				"failed to set up VLAN %04x",
 				vid);
+			return ret;
+		}
 
 		if (!pvid)
 			continue;
 
 		ret = rtl8366_set_pvid(smi, port, vid);
-		if (ret)
+		if (ret) {
 			dev_err(smi->dev,
 				"failed to set PVID on port %d to VLAN %04x",
 				port, vid);
+			return ret;
+		}
 
 		if (!ret)
 			dev_dbg(smi->dev, "VLAN add: added VLAN %d with PVID on port %d\n",
 				vid, port);
 	}
+
+	return 0;
 }
 EXPORT_SYMBOL_GPL(rtl8366_vlan_add);
 
diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c
index cfe56960f44b..896978568716 100644
--- a/drivers/net/dsa/rtl8366rb.c
+++ b/drivers/net/dsa/rtl8366rb.c
@@ -1504,7 +1504,6 @@ static const struct dsa_switch_ops rtl8366rb_switch_ops = {
 	.get_ethtool_stats = rtl8366_get_ethtool_stats,
 	.get_sset_count = rtl8366_get_sset_count,
 	.port_vlan_filtering = rtl8366_vlan_filtering,
-	.port_vlan_prepare = rtl8366_vlan_prepare,
 	.port_vlan_add = rtl8366_vlan_add,
 	.port_vlan_del = rtl8366_vlan_del,
 	.port_enable = rtl8366rb_port_enable,
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 3dfcb750e169..7f960ecc6b6b 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2782,29 +2782,31 @@ static int sja1105_vlan_del_one(struct dsa_switch *ds, int port, u16 vid,
 	return 0;
 }
 
-static void sja1105_vlan_add(struct dsa_switch *ds, int port,
-			     const struct switchdev_obj_port_vlan *vlan)
+static int sja1105_vlan_add(struct dsa_switch *ds, int port,
+			    const struct switchdev_obj_port_vlan *vlan)
 {
 	struct sja1105_private *priv = ds->priv;
 	bool vlan_table_changed = false;
 	u16 vid;
 	int rc;
 
+	rc = sja1105_vlan_prepare(ds, port, vlan);
+	if (rc)
+		return rc;
+
 	for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
 		rc = sja1105_vlan_add_one(ds, port, vid, vlan->flags,
 					  &priv->bridge_vlans);
 		if (rc < 0)
-			return;
+			return rc;
 		if (rc > 0)
 			vlan_table_changed = true;
 	}
 
 	if (!vlan_table_changed)
-		return;
+		return 0;
 
-	rc = sja1105_build_vlan_table(priv, true);
-	if (rc)
-		dev_err(ds->dev, "Failed to build VLAN table: %d\n", rc);
+	return sja1105_build_vlan_table(priv, true);
 }
 
 static int sja1105_vlan_del(struct dsa_switch *ds, int port,
@@ -3286,7 +3288,6 @@ static const struct dsa_switch_ops sja1105_switch_ops = {
 	.port_bridge_join	= sja1105_bridge_join,
 	.port_bridge_leave	= sja1105_bridge_leave,
 	.port_stp_state_set	= sja1105_bridge_stp_state_set,
-	.port_vlan_prepare	= sja1105_vlan_prepare,
 	.port_vlan_filtering	= sja1105_vlan_filtering,
 	.port_vlan_add		= sja1105_vlan_add,
 	.port_vlan_del		= sja1105_vlan_del,
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 2d715b0502e3..b5d04adc09b0 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -561,10 +561,8 @@ struct dsa_switch_ops {
 	 */
 	int	(*port_vlan_filtering)(struct dsa_switch *ds, int port,
 				       bool vlan_filtering);
-	int (*port_vlan_prepare)(struct dsa_switch *ds, int port,
+	int	(*port_vlan_add)(struct dsa_switch *ds, int port,
 				 const struct switchdev_obj_port_vlan *vlan);
-	void (*port_vlan_add)(struct dsa_switch *ds, int port,
-			      const struct switchdev_obj_port_vlan *vlan);
 	int	(*port_vlan_del)(struct dsa_switch *ds, int port,
 				 const struct switchdev_obj_port_vlan *vlan);
 	/*
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index f8362860cdaa..5bd2327d519f 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -58,7 +58,6 @@ struct dsa_notifier_mdb_info {
 /* DSA_NOTIFIER_VLAN_* */
 struct dsa_notifier_vlan_info {
 	const struct switchdev_obj_port_vlan *vlan;
-	struct switchdev_trans *trans;
 	int sw_index;
 	int port;
 };
diff --git a/net/dsa/port.c b/net/dsa/port.c
index c89e3c0c65ed..e59bf66c4c0d 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -441,17 +441,7 @@ int dsa_port_vlan_add(struct dsa_port *dp,
 		.port = dp->index,
 		.vlan = vlan,
 	};
-	struct switchdev_trans trans;
-	int err;
-
-	info.trans = &trans;
-
-	trans.ph_prepare = true;
-	err = dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info);
-	if (err)
-		return err;
 
-	trans.ph_prepare = false;
 	return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info);
 }
 
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 65124bc3ddfb..bd00ef6296f9 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -217,35 +217,13 @@ static bool dsa_switch_vlan_match(struct dsa_switch *ds, int port,
 	return false;
 }
 
-static int dsa_switch_vlan_prepare(struct dsa_switch *ds,
-				   struct dsa_notifier_vlan_info *info)
-{
-	int port, err;
-
-	if (!ds->ops->port_vlan_prepare || !ds->ops->port_vlan_add)
-		return -EOPNOTSUPP;
-
-	for (port = 0; port < ds->num_ports; port++) {
-		if (dsa_switch_vlan_match(ds, port, info)) {
-			err = ds->ops->port_vlan_prepare(ds, port, info->vlan);
-			if (err)
-				return err;
-		}
-	}
-
-	return 0;
-}
-
 static int dsa_switch_vlan_add(struct dsa_switch *ds,
 			       struct dsa_notifier_vlan_info *info)
 {
 	int port;
 
-	if (switchdev_trans_ph_prepare(info->trans))
-		return dsa_switch_vlan_prepare(ds, info);
-
 	if (!ds->ops->port_vlan_add)
-		return 0;
+		return -EOPNOTSUPP;
 
 	for (port = 0; port < ds->num_ports; port++)
 		if (dsa_switch_vlan_match(ds, port, info))
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [RFC PATCH net-next 7/9] net: dsa: remove obsolete comment about switchdev transactions
  2020-12-17  1:58 [RFC PATCH net-next 0/9] Get rid of the switchdev transactional model Vladimir Oltean
                   ` (5 preceding siblings ...)
  2020-12-17  1:58 ` [RFC PATCH net-next 6/9] net: dsa: remove the transactional logic from VLAN objects Vladimir Oltean
@ 2020-12-17  1:58 ` Vladimir Oltean
  2020-12-17  2:11   ` Florian Fainelli
  2020-12-17  1:58 ` [RFC PATCH net-next 8/9] mlxsw: spectrum_switchdev: remove transactional logic for VLAN objects Vladimir Oltean
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 26+ messages in thread
From: Vladimir Oltean @ 2020-12-17  1:58 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh,
	Microchip Linux Driver Support, Sean Wang, Landen Chao,
	Claudiu Manoil, Alexandre Belloni, Linus Walleij, Vadym Kochan,
	Taras Chornyi, Jiri Pirko, Ido Schimmel, Grygorii Strashko,
	Ioana Ciornei, Ivan Vecera

Now that all port object notifiers were converted to be non-transactional,
we can remove the comment that says otherwise.

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

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index ffad4324c736..c5f38bcaa5af 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -379,11 +379,6 @@ static int dsa_slave_port_obj_add(struct net_device *dev,
 	struct dsa_port *dp = dsa_slave_to_port(dev);
 	int err;
 
-	/* For the prepare phase, ensure the full set of changes is feasable in
-	 * one go in order to signal a failure properly. If an operation is not
-	 * supported, return -EOPNOTSUPP.
-	 */
-
 	switch (obj->id) {
 	case SWITCHDEV_OBJ_ID_PORT_MDB:
 		if (obj->orig_dev != dev)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [RFC PATCH net-next 8/9] mlxsw: spectrum_switchdev: remove transactional logic for VLAN objects
  2020-12-17  1:58 [RFC PATCH net-next 0/9] Get rid of the switchdev transactional model Vladimir Oltean
                   ` (6 preceding siblings ...)
  2020-12-17  1:58 ` [RFC PATCH net-next 7/9] net: dsa: remove obsolete comment about switchdev transactions Vladimir Oltean
@ 2020-12-17  1:58 ` Vladimir Oltean
  2020-12-17  1:58 ` [RFC PATCH net-next 9/9] net: switchdev: delete the transaction object Vladimir Oltean
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 26+ messages in thread
From: Vladimir Oltean @ 2020-12-17  1:58 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh,
	Microchip Linux Driver Support, Sean Wang, Landen Chao,
	Claudiu Manoil, Alexandre Belloni, Linus Walleij, Vadym Kochan,
	Taras Chornyi, Jiri Pirko, Ido Schimmel, Grygorii Strashko,
	Ioana Ciornei, Ivan Vecera

As of commit 457e20d65924 ("mlxsw: spectrum_switchdev: Avoid returning
errors in commit phase"), the mlxsw driver performs the VLAN object
offloading during the prepare phase. So conversion just seems to be a
matter of removing the code that was running in the commit phase.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 .../mellanox/mlxsw/spectrum_switchdev.c       | 38 +++----------------
 1 file changed, 5 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index e248f9deb2ec..c775c9edea78 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -1199,7 +1199,6 @@ mlxsw_sp_br_ban_rif_pvid_change(struct mlxsw_sp *mlxsw_sp,
 
 static int mlxsw_sp_port_vlans_add(struct mlxsw_sp_port *mlxsw_sp_port,
 				   const struct switchdev_obj_port_vlan *vlan,
-				   struct switchdev_trans *trans,
 				   struct netlink_ext_ack *extack)
 {
 	bool flag_untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
@@ -1213,8 +1212,7 @@ static int mlxsw_sp_port_vlans_add(struct mlxsw_sp_port *mlxsw_sp_port,
 		int err = 0;
 
 		if ((vlan->flags & BRIDGE_VLAN_INFO_BRENTRY) &&
-		    br_vlan_enabled(orig_dev) &&
-		    switchdev_trans_ph_prepare(trans))
+		    br_vlan_enabled(orig_dev))
 			err = mlxsw_sp_br_ban_rif_pvid_change(mlxsw_sp,
 							      orig_dev, vlan);
 		if (!err)
@@ -1222,9 +1220,6 @@ static int mlxsw_sp_port_vlans_add(struct mlxsw_sp_port *mlxsw_sp_port,
 		return err;
 	}
 
-	if (switchdev_trans_ph_commit(trans))
-		return 0;
-
 	bridge_port = mlxsw_sp_bridge_port_find(mlxsw_sp->bridge, orig_dev);
 	if (WARN_ON(!bridge_port))
 		return -EINVAL;
@@ -1776,29 +1771,20 @@ static int mlxsw_sp_port_obj_add(struct net_device *dev,
 {
 	struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(dev);
 	const struct switchdev_obj_port_vlan *vlan;
-	struct switchdev_trans trans;
 	int err = 0;
 
 	switch (obj->id) {
 	case SWITCHDEV_OBJ_ID_PORT_VLAN:
 		vlan = SWITCHDEV_OBJ_PORT_VLAN(obj);
 
-		trans.ph_prepare = true;
-		err = mlxsw_sp_port_vlans_add(mlxsw_sp_port, vlan, &trans,
-					      extack);
+		err = mlxsw_sp_port_vlans_add(mlxsw_sp_port, vlan, extack);
 		if (err)
 			break;
 
-		/* The event is emitted before the changes are actually
-		 * applied to the bridge. Therefore schedule the respin
-		 * call for later, so that the respin logic sees the
+		/* Schedule the respin call, so that the respin logic sees the
 		 * updated bridge state.
 		 */
 		mlxsw_sp_span_respin(mlxsw_sp_port->mlxsw_sp);
-
-		trans.ph_prepare = false;
-		err = mlxsw_sp_port_vlans_add(mlxsw_sp_port, vlan, &trans,
-					      extack);
 		break;
 	case SWITCHDEV_OBJ_ID_PORT_MDB:
 		err = mlxsw_sp_port_mdb_add(mlxsw_sp_port,
@@ -3364,8 +3350,7 @@ mlxsw_sp_switchdev_vxlan_vlan_del(struct mlxsw_sp *mlxsw_sp,
 static int
 mlxsw_sp_switchdev_vxlan_vlans_add(struct net_device *vxlan_dev,
 				   struct switchdev_notifier_port_obj_info *
-				   port_obj_info,
-				   struct switchdev_trans *trans)
+				   port_obj_info)
 {
 	struct switchdev_obj_port_vlan *vlan =
 		SWITCHDEV_OBJ_PORT_VLAN(port_obj_info->obj);
@@ -3388,9 +3373,6 @@ mlxsw_sp_switchdev_vxlan_vlans_add(struct net_device *vxlan_dev,
 
 	port_obj_info->handled = true;
 
-	if (switchdev_trans_ph_commit(trans))
-		return 0;
-
 	bridge_device = mlxsw_sp_bridge_device_find(mlxsw_sp->bridge, br_dev);
 	if (!bridge_device)
 		return -EINVAL;
@@ -3451,22 +3433,12 @@ mlxsw_sp_switchdev_handle_vxlan_obj_add(struct net_device *vxlan_dev,
 					struct switchdev_notifier_port_obj_info *
 					port_obj_info)
 {
-	struct switchdev_trans trans;
 	int err = 0;
 
 	switch (port_obj_info->obj->id) {
 	case SWITCHDEV_OBJ_ID_PORT_VLAN:
-		trans.ph_prepare = true;
-		err = mlxsw_sp_switchdev_vxlan_vlans_add(vxlan_dev,
-							 port_obj_info,
-							 &trans);
-		if (err)
-			break;
-
-		trans.ph_prepare = false;
 		err = mlxsw_sp_switchdev_vxlan_vlans_add(vxlan_dev,
-							 port_obj_info,
-							 &trans);
+							 port_obj_info);
 		break;
 	default:
 		break;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [RFC PATCH net-next 9/9] net: switchdev: delete the transaction object
  2020-12-17  1:58 [RFC PATCH net-next 0/9] Get rid of the switchdev transactional model Vladimir Oltean
                   ` (7 preceding siblings ...)
  2020-12-17  1:58 ` [RFC PATCH net-next 8/9] mlxsw: spectrum_switchdev: remove transactional logic for VLAN objects Vladimir Oltean
@ 2020-12-17  1:58 ` Vladimir Oltean
  2020-12-17  2:11   ` Florian Fainelli
  2020-12-27 13:32 ` [RFC PATCH net-next 0/9] Get rid of the switchdev transactional model Linus Walleij
  2020-12-28  9:59 ` Jiri Pirko
  10 siblings, 1 reply; 26+ messages in thread
From: Vladimir Oltean @ 2020-12-17  1:58 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh,
	Microchip Linux Driver Support, Sean Wang, Landen Chao,
	Claudiu Manoil, Alexandre Belloni, Linus Walleij, Vadym Kochan,
	Taras Chornyi, Jiri Pirko, Ido Schimmel, Grygorii Strashko,
	Ioana Ciornei, Ivan Vecera

Now that all users of struct switchdev_trans have been modified to do
without it, we can remove this structure and the two helpers to determine
the phase.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 include/net/switchdev.h | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 2df84ffe6ba4..eb45a0bbc79f 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -16,20 +16,6 @@
 #define SWITCHDEV_F_SKIP_EOPNOTSUPP	BIT(1)
 #define SWITCHDEV_F_DEFER		BIT(2)
 
-struct switchdev_trans {
-	bool ph_prepare;
-};
-
-static inline bool switchdev_trans_ph_prepare(struct switchdev_trans *trans)
-{
-	return trans && trans->ph_prepare;
-}
-
-static inline bool switchdev_trans_ph_commit(struct switchdev_trans *trans)
-{
-	return trans && !trans->ph_prepare;
-}
-
 enum switchdev_attr_id {
 	SWITCHDEV_ATTR_ID_UNDEFINED,
 	SWITCHDEV_ATTR_ID_PORT_STP_STATE,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [RFC PATCH net-next 4/9] net: dsa: remove the transactional logic from ageing time notifiers
  2020-12-17  1:58 ` [RFC PATCH net-next 4/9] net: dsa: remove the transactional logic from ageing time notifiers Vladimir Oltean
@ 2020-12-17  2:06   ` Florian Fainelli
  0 siblings, 0 replies; 26+ messages in thread
From: Florian Fainelli @ 2020-12-17  2:06 UTC (permalink / raw)
  To: Vladimir Oltean, David S. Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Vladimir Oltean, Kurt Kanzenbach,
	Hauke Mehrtens, Woojung Huh, Microchip Linux Driver Support,
	Sean Wang, Landen Chao, Claudiu Manoil, Alexandre Belloni,
	Linus Walleij, Vadym Kochan, Taras Chornyi, Jiri Pirko,
	Ido Schimmel, Grygorii Strashko, Ioana Ciornei, Ivan Vecera



On 12/16/2020 5:58 PM, Vladimir Oltean wrote:
> Remove the shim introduced in DSA for offloading the bridge ageing time
> from switchdev, by first checking whether the ageing time is within the
> range limits requested by the driver.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC PATCH net-next 5/9] net: dsa: remove the transactional logic from MDB entries
  2020-12-17  1:58 ` [RFC PATCH net-next 5/9] net: dsa: remove the transactional logic from MDB entries Vladimir Oltean
@ 2020-12-17  2:08   ` Florian Fainelli
  2020-12-17 11:03   ` Vladimir Oltean
  1 sibling, 0 replies; 26+ messages in thread
From: Florian Fainelli @ 2020-12-17  2:08 UTC (permalink / raw)
  To: Vladimir Oltean, David S. Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Vladimir Oltean, Kurt Kanzenbach,
	Hauke Mehrtens, Woojung Huh, Microchip Linux Driver Support,
	Sean Wang, Landen Chao, Claudiu Manoil, Alexandre Belloni,
	Linus Walleij, Vadym Kochan, Taras Chornyi, Jiri Pirko,
	Ido Schimmel, Grygorii Strashko, Ioana Ciornei, Ivan Vecera



On 12/16/2020 5:58 PM, Vladimir Oltean wrote:
> For many drivers, the .port_mdb_prepare callback was not a good opportunity
> to avoid any error condition, and they would suppress errors found during
> the actual commit phase.
> 
> Where a logical separation between the prepare and the commit phase
> existed, the function that used to implement the .port_mdb_prepare
> callback still exists, but now it is called directly from .port_mdb_add,
> which was modified to return an int code.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC PATCH net-next 6/9] net: dsa: remove the transactional logic from VLAN objects
  2020-12-17  1:58 ` [RFC PATCH net-next 6/9] net: dsa: remove the transactional logic from VLAN objects Vladimir Oltean
@ 2020-12-17  2:11   ` Florian Fainelli
  2020-12-17 11:04   ` Vladimir Oltean
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 26+ messages in thread
From: Florian Fainelli @ 2020-12-17  2:11 UTC (permalink / raw)
  To: Vladimir Oltean, David S. Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Vladimir Oltean, Kurt Kanzenbach,
	Hauke Mehrtens, Woojung Huh, Microchip Linux Driver Support,
	Sean Wang, Landen Chao, Claudiu Manoil, Alexandre Belloni,
	Linus Walleij, Vadym Kochan, Taras Chornyi, Jiri Pirko,
	Ido Schimmel, Grygorii Strashko, Ioana Ciornei, Ivan Vecera



On 12/16/2020 5:58 PM, Vladimir Oltean wrote:
> It should be the driver's business to logically separate its VLAN
> offloading into a preparation and a commit phase, and some drivers don't
> need / can't do this.
> 
> So remove the transactional shim from DSA and let drivers to propagate
> errors directly from the .port_vlan_add callback.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC PATCH net-next 7/9] net: dsa: remove obsolete comment about switchdev transactions
  2020-12-17  1:58 ` [RFC PATCH net-next 7/9] net: dsa: remove obsolete comment about switchdev transactions Vladimir Oltean
@ 2020-12-17  2:11   ` Florian Fainelli
  0 siblings, 0 replies; 26+ messages in thread
From: Florian Fainelli @ 2020-12-17  2:11 UTC (permalink / raw)
  To: Vladimir Oltean, David S. Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Vladimir Oltean, Kurt Kanzenbach,
	Hauke Mehrtens, Woojung Huh, Microchip Linux Driver Support,
	Sean Wang, Landen Chao, Claudiu Manoil, Alexandre Belloni,
	Linus Walleij, Vadym Kochan, Taras Chornyi, Jiri Pirko,
	Ido Schimmel, Grygorii Strashko, Ioana Ciornei, Ivan Vecera



On 12/16/2020 5:58 PM, Vladimir Oltean wrote:
> Now that all port object notifiers were converted to be non-transactional,
> we can remove the comment that says otherwise.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC PATCH net-next 9/9] net: switchdev: delete the transaction object
  2020-12-17  1:58 ` [RFC PATCH net-next 9/9] net: switchdev: delete the transaction object Vladimir Oltean
@ 2020-12-17  2:11   ` Florian Fainelli
  0 siblings, 0 replies; 26+ messages in thread
From: Florian Fainelli @ 2020-12-17  2:11 UTC (permalink / raw)
  To: Vladimir Oltean, David S. Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Vladimir Oltean, Kurt Kanzenbach,
	Hauke Mehrtens, Woojung Huh, Microchip Linux Driver Support,
	Sean Wang, Landen Chao, Claudiu Manoil, Alexandre Belloni,
	Linus Walleij, Vadym Kochan, Taras Chornyi, Jiri Pirko,
	Ido Schimmel, Grygorii Strashko, Ioana Ciornei, Ivan Vecera



On 12/16/2020 5:58 PM, Vladimir Oltean wrote:
> Now that all users of struct switchdev_trans have been modified to do
> without it, we can remove this structure and the two helpers to determine
> the phase.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC PATCH net-next 1/9] net: switchdev: remove the transaction structure from port object notifiers
  2020-12-17  1:58 ` [RFC PATCH net-next 1/9] net: switchdev: remove the transaction structure from port object notifiers Vladimir Oltean
@ 2020-12-17  2:14   ` Florian Fainelli
  0 siblings, 0 replies; 26+ messages in thread
From: Florian Fainelli @ 2020-12-17  2:14 UTC (permalink / raw)
  To: Vladimir Oltean, David S. Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Vladimir Oltean, Kurt Kanzenbach,
	Hauke Mehrtens, Woojung Huh, Microchip Linux Driver Support,
	Sean Wang, Landen Chao, Claudiu Manoil, Alexandre Belloni,
	Linus Walleij, Vadym Kochan, Taras Chornyi, Jiri Pirko,
	Ido Schimmel, Grygorii Strashko, Ioana Ciornei, Ivan Vecera



On 12/16/2020 5:58 PM, Vladimir Oltean wrote:
> Since the introduction of the switchdev API, port objects were
> transmitted to drivers for offloading using a two-step transactional
> model, with a prepare phase that was supposed to catch all errors, and a
> commit phase that was supposed to never fail.
> 
> Some classes of failures can never be avoided, like hardware access, or
> memory allocation. In the latter case, merely attempting to move the
> memory allocation to the preparation phase makes it impossible to avoid
> memory leaks, since commit 91cf8eceffc1 ("switchdev: Remove unused
> transaction item queue") which has removed the unused mechanism of
> passing on the allocated memory between one phase and another.
> 
> It is time we admit that separating the preparation from the commit
> phase is something that is best left for the driver to decide, and not
> something that should be baked into the API, especially since there are
> no switchdev callers that depend on this.
> 
> This patch removes the struct switchdev_trans member from switchdev port
> object notifier structures, and converts drivers to not look at this
> member.
> 
> Where driver conversion is trivial (like in the case of the Marvell
> Prestera driver, NXP DPAA2 switch, TI CPSW, and Rocker drivers), it is
> done in this patch.
> 
> Where driver conversion needs more attention (DSA, Mellanox Spectrum),
> the conversion is left for subsequent patches and here we only fake the
> prepare/commit phases at a lower level, just not in the switchdev
> notifier itself.
> 
> Where the code has a natural structure that is best left alone as a
> preparation and a commit phase (as in the case of the Ocelot switch),
> that structure is left in place, just made to not depend upon the
> switchdev transactional model.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC PATCH net-next 2/9] net: switchdev: delete switchdev_port_obj_add_now
  2020-12-17  1:58 ` [RFC PATCH net-next 2/9] net: switchdev: delete switchdev_port_obj_add_now Vladimir Oltean
@ 2020-12-17  2:14   ` Florian Fainelli
  0 siblings, 0 replies; 26+ messages in thread
From: Florian Fainelli @ 2020-12-17  2:14 UTC (permalink / raw)
  To: Vladimir Oltean, David S. Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Vivien Didelot, Vladimir Oltean, Kurt Kanzenbach,
	Hauke Mehrtens, Woojung Huh, Microchip Linux Driver Support,
	Sean Wang, Landen Chao, Claudiu Manoil, Alexandre Belloni,
	Linus Walleij, Vadym Kochan, Taras Chornyi, Jiri Pirko,
	Ido Schimmel, Grygorii Strashko, Ioana Ciornei, Ivan Vecera



On 12/16/2020 5:58 PM, Vladimir Oltean wrote:
> After the removal of the transactional model inside
> switchdev_port_obj_add_now, it has no added value and we can just call
> switchdev_port_obj_notify directly, bypassing this function. Let's
> delete it.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC PATCH net-next 5/9] net: dsa: remove the transactional logic from MDB entries
  2020-12-17  1:58 ` [RFC PATCH net-next 5/9] net: dsa: remove the transactional logic from MDB entries Vladimir Oltean
  2020-12-17  2:08   ` Florian Fainelli
@ 2020-12-17 11:03   ` Vladimir Oltean
  1 sibling, 0 replies; 26+ messages in thread
From: Vladimir Oltean @ 2020-12-17 11:03 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh,
	Microchip Linux Driver Support, Sean Wang, Landen Chao,
	Claudiu Manoil, Alexandre Belloni, Linus Walleij, Vadym Kochan,
	Taras Chornyi, Jiri Pirko, Ido Schimmel, Grygorii Strashko,
	Ioana Ciornei, Ivan Vecera

On Thu, Dec 17, 2020 at 03:58:18AM +0200, Vladimir Oltean wrote:
> For many drivers, the .port_mdb_prepare callback was not a good opportunity
> to avoid any error condition, and they would suppress errors found during
> the actual commit phase.
> 
> Where a logical separation between the prepare and the commit phase
> existed, the function that used to implement the .port_mdb_prepare
> callback still exists, but now it is called directly from .port_mdb_add,
> which was modified to return an int code.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
[...]
> diff --git a/net/dsa/switch.c b/net/dsa/switch.c
> index 39848eac1da8..65124bc3ddfb 100644
> --- a/net/dsa/switch.c
> +++ b/net/dsa/switch.c
> @@ -178,35 +178,13 @@ static bool dsa_switch_mdb_match(struct dsa_switch *ds, int port,
>  	return false;
>  }
>  
> -static int dsa_switch_mdb_prepare(struct dsa_switch *ds,
> -				  struct dsa_notifier_mdb_info *info)
> -{
> -	int port, err;
> -
> -	if (!ds->ops->port_mdb_prepare || !ds->ops->port_mdb_add)
> -		return -EOPNOTSUPP;
> -
> -	for (port = 0; port < ds->num_ports; port++) {
> -		if (dsa_switch_mdb_match(ds, port, info)) {
> -			err = ds->ops->port_mdb_prepare(ds, port, info->mdb);
> -			if (err)
> -				return err;
> -		}
> -	}
> -
> -	return 0;
> -}
> -
>  static int dsa_switch_mdb_add(struct dsa_switch *ds,
>  			      struct dsa_notifier_mdb_info *info)
>  {
>  	int port;
>  
> -	if (switchdev_trans_ph_prepare(info->trans))
> -		return dsa_switch_mdb_prepare(ds, info);
> -
>  	if (!ds->ops->port_mdb_add)
> -		return 0;
> +		return -EOPNOTSUPP;
>  
>  	for (port = 0; port < ds->num_ports; port++)
>  		if (dsa_switch_mdb_match(ds, port, info))
> -- 
> 2.25.1
> 

For anybody who wants to test, I forgot to update dsa_switch_mdb_add to
propagate the errors. It should look like this:

static int dsa_switch_mdb_add(struct dsa_switch *ds,
			      struct dsa_notifier_mdb_info *info)
{
	int err = 0;
	int port;

	if (!ds->ops->port_mdb_add)
		return -EOPNOTSUPP;

	for (port = 0; port < ds->num_ports; port++) {
		if (dsa_switch_mdb_match(ds, port, info)) {
			err = ds->ops->port_mdb_add(ds, port, info->mdb);
			if (err)
				break;
		}
	}

	return err;
}

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC PATCH net-next 6/9] net: dsa: remove the transactional logic from VLAN objects
  2020-12-17  1:58 ` [RFC PATCH net-next 6/9] net: dsa: remove the transactional logic from VLAN objects Vladimir Oltean
  2020-12-17  2:11   ` Florian Fainelli
@ 2020-12-17 11:04   ` Vladimir Oltean
  2020-12-17 11:43     ` Vladimir Oltean
  2020-12-18  8:49   ` Kurt Kanzenbach
  2020-12-27 13:30   ` Linus Walleij
  3 siblings, 1 reply; 26+ messages in thread
From: Vladimir Oltean @ 2020-12-17 11:04 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh,
	Microchip Linux Driver Support, Sean Wang, Landen Chao,
	Claudiu Manoil, Alexandre Belloni, Linus Walleij, Vadym Kochan,
	Taras Chornyi, Jiri Pirko, Ido Schimmel, Grygorii Strashko,
	Ioana Ciornei, Ivan Vecera

On Thu, Dec 17, 2020 at 03:58:19AM +0200, Vladimir Oltean wrote:
> It should be the driver's business to logically separate its VLAN
> offloading into a preparation and a commit phase, and some drivers don't
> need / can't do this.
> 
> So remove the transactional shim from DSA and let drivers to propagate
> errors directly from the .port_vlan_add callback.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
[...]
> diff --git a/net/dsa/switch.c b/net/dsa/switch.c
> index 65124bc3ddfb..bd00ef6296f9 100644
> --- a/net/dsa/switch.c
> +++ b/net/dsa/switch.c
> @@ -217,35 +217,13 @@ static bool dsa_switch_vlan_match(struct dsa_switch *ds, int port,
>  	return false;
>  }
>  
> -static int dsa_switch_vlan_prepare(struct dsa_switch *ds,
> -				   struct dsa_notifier_vlan_info *info)
> -{
> -	int port, err;
> -
> -	if (!ds->ops->port_vlan_prepare || !ds->ops->port_vlan_add)
> -		return -EOPNOTSUPP;
> -
> -	for (port = 0; port < ds->num_ports; port++) {
> -		if (dsa_switch_vlan_match(ds, port, info)) {
> -			err = ds->ops->port_vlan_prepare(ds, port, info->vlan);
> -			if (err)
> -				return err;
> -		}
> -	}
> -
> -	return 0;
> -}
> -
>  static int dsa_switch_vlan_add(struct dsa_switch *ds,
>  			       struct dsa_notifier_vlan_info *info)
>  {
>  	int port;
>  
> -	if (switchdev_trans_ph_prepare(info->trans))
> -		return dsa_switch_vlan_prepare(ds, info);
> -
>  	if (!ds->ops->port_vlan_add)
> -		return 0;
> +		return -EOPNOTSUPP;
>  
>  	for (port = 0; port < ds->num_ports; port++)
>  		if (dsa_switch_vlan_match(ds, port, info))
> -- 
> 2.25.1
> 

For anybody who wants to test, please paste this instead of the existing
dsa_switch_vlan_add, to propagate the errors:

static int dsa_switch_vlan_add(struct dsa_switch *ds,
			       struct dsa_notifier_vlan_info *info)
{
	int err = 0;
	int port;

	if (!ds->ops->port_vlan_add)
		return -EOPNOTSUPP;

	for (port = 0; port < ds->num_ports; port++) {
		if (dsa_switch_vlan_match(ds, port, info)) {
			err = ds->ops->port_vlan_add(ds, port, info->vlan);
			if (err)
				break;
		}
	}

	return err;
}

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC PATCH net-next 6/9] net: dsa: remove the transactional logic from VLAN objects
  2020-12-17 11:04   ` Vladimir Oltean
@ 2020-12-17 11:43     ` Vladimir Oltean
  0 siblings, 0 replies; 26+ messages in thread
From: Vladimir Oltean @ 2020-12-17 11:43 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Hauke Mehrtens, Woojung Huh,
	Microchip Linux Driver Support, Sean Wang, Landen Chao,
	Claudiu Manoil, Alexandre Belloni, Linus Walleij, Vadym Kochan,
	Taras Chornyi, Jiri Pirko, Ido Schimmel, Grygorii Strashko,
	Ioana Ciornei, Ivan Vecera

On Thu, Dec 17, 2020 at 01:04:26PM +0200, Vladimir Oltean wrote:
> On Thu, Dec 17, 2020 at 03:58:19AM +0200, Vladimir Oltean wrote:
> > It should be the driver's business to logically separate its VLAN
> > offloading into a preparation and a commit phase, and some drivers don't
> > need / can't do this.
> > 
> > So remove the transactional shim from DSA and let drivers to propagate
> > errors directly from the .port_vlan_add callback.
> > 
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > ---
> [...]
> > diff --git a/net/dsa/switch.c b/net/dsa/switch.c
> > index 65124bc3ddfb..bd00ef6296f9 100644
> > --- a/net/dsa/switch.c
> > +++ b/net/dsa/switch.c
> > @@ -217,35 +217,13 @@ static bool dsa_switch_vlan_match(struct dsa_switch *ds, int port,
> >  	return false;
> >  }
> >  
> > -static int dsa_switch_vlan_prepare(struct dsa_switch *ds,
> > -				   struct dsa_notifier_vlan_info *info)
> > -{
> > -	int port, err;
> > -
> > -	if (!ds->ops->port_vlan_prepare || !ds->ops->port_vlan_add)
> > -		return -EOPNOTSUPP;
> > -
> > -	for (port = 0; port < ds->num_ports; port++) {
> > -		if (dsa_switch_vlan_match(ds, port, info)) {
> > -			err = ds->ops->port_vlan_prepare(ds, port, info->vlan);
> > -			if (err)
> > -				return err;
> > -		}
> > -	}
> > -
> > -	return 0;
> > -}
> > -
> >  static int dsa_switch_vlan_add(struct dsa_switch *ds,
> >  			       struct dsa_notifier_vlan_info *info)
> >  {
> >  	int port;
> >  
> > -	if (switchdev_trans_ph_prepare(info->trans))
> > -		return dsa_switch_vlan_prepare(ds, info);
> > -
> >  	if (!ds->ops->port_vlan_add)
> > -		return 0;
> > +		return -EOPNOTSUPP;
> >  
> >  	for (port = 0; port < ds->num_ports; port++)
> >  		if (dsa_switch_vlan_match(ds, port, info))
> > -- 
> > 2.25.1
> > 
> 
> For anybody who wants to test, please paste this instead of the existing
> dsa_switch_vlan_add, to propagate the errors:
> 
> static int dsa_switch_vlan_add(struct dsa_switch *ds,
> 			       struct dsa_notifier_vlan_info *info)
> {
> 	int err = 0;
> 	int port;
> 
> 	if (!ds->ops->port_vlan_add)
> 		return -EOPNOTSUPP;
> 
> 	for (port = 0; port < ds->num_ports; port++) {
> 		if (dsa_switch_vlan_match(ds, port, info)) {
> 			err = ds->ops->port_vlan_add(ds, port, info->vlan);
> 			if (err)
> 				break;
> 		}
> 	}
> 
> 	return err;
> }

Having said that, now I finally understand one of the ways in which the
prepare phase was actually useful for something. Sometimes it takes
removing all the code before things like this are visible...

DSA uses notifiers to propagate a switchdev object to multiple ports.
For example, every VLAN is installed not only on the front-panel user
port, but also on the CPU port. Iterating through the port list, we find
that the addition on the user port may succeed, but the addition on the
CPU port might fail.

So, at the very least, we should attempt a rollback on error (iterating
backwards through the port list).

But there is a second problem. Nobody prevents you from adding a VLAN
twice on the same port.

bridge vlan add dev swp0 vid 100
bridge vlan add dev swp0 vid 90-110

If the second command fails at VLAN 110 and we attempt a rollback, we
would end up deleting VLAN 100 too, which was set up by the first command.

This is where the prepare/commit phase could come in handy. First all
the ports confirm that the VLAN range 90-110 is ok, and only then we
proceed to commit it. That would avoid the first problem.

However there's the other problem, which is that artificially splitting
into a prepare and a commit phase will still inevitably leave some
"catastrophic" failures unhandled in the commit phase. We typically
throw our hands up in the air when it comes to these, but normally if we
bother to do something about consistency in the first place, we should
have rollback logic too. It's just that the rollback will not be very
simple. We would need to know, for each VLAN, if it already existed or
not. And if it existed, what flags it had. Either that, or we start
disallowing the user, from the bridge code, to add VLAN objects that
already exist.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC PATCH net-next 3/9] net: switchdev: remove the transaction structure from port attributes
  2020-12-17  1:58 ` [RFC PATCH net-next 3/9] net: switchdev: remove the transaction structure from port attributes Vladimir Oltean
@ 2020-12-18  8:46   ` Kurt Kanzenbach
  2020-12-27 13:26   ` Linus Walleij
  1 sibling, 0 replies; 26+ messages in thread
From: Kurt Kanzenbach @ 2020-12-18  8:46 UTC (permalink / raw)
  To: Vladimir Oltean, David S. Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Hauke Mehrtens, Woojung Huh, Microchip Linux Driver Support,
	Sean Wang, Landen Chao, Claudiu Manoil, Alexandre Belloni,
	Linus Walleij, Vadym Kochan, Taras Chornyi, Jiri Pirko,
	Ido Schimmel, Grygorii Strashko, Ioana Ciornei, Ivan Vecera

[-- Attachment #1: Type: text/plain, Size: 4214 bytes --]

Hi Vladimir,

On Thu Dec 17 2020, Vladimir Oltean wrote:
> Since the introduction of the switchdev API, port attributes were
> transmitted to drivers for offloading using a two-step transactional
> model, with a prepare phase that was supposed to catch all errors, and a
> commit phase that was supposed to never fail.
>
> Some classes of failures can never be avoided, like hardware access, or
> memory allocation. In the latter case, merely attempting to move the
> memory allocation to the preparation phase makes it impossible to avoid
> memory leaks, since commit 91cf8eceffc1 ("switchdev: Remove unused
> transaction item queue") which has removed the unused mechanism of
> passing on the allocated memory between one phase and another.
>
> It is time we admit that separating the preparation from the commit
> phase is something that is best left for the driver to decide, and not
> something that should be baked into the API, especially since there are
> no switchdev callers that depend on this.
>
> This patch removes the struct switchdev_trans member from switchdev port
> attribute notifier structures, and converts drivers to not look at this
> member.
>
> In part, this patch contains a revert of my previous commit 2e554a7a5d8a
> ("net: dsa: propagate switchdev vlan_filtering prepare phase to
> drivers").
>
> For the most part, the conversion was trivial except for:
> - Rocker's world implementation based on Broadcom OF-DPA had an odd
>   implementation of ofdpa_port_attr_bridge_flags_set. The conversion was
>   done mechanically, by pasting the implementation twice, then only
>   keeping the code that would get executed during prepare phase on top,
>   then only keeping the code that gets executed during the commit phase
>   on bottom, then simplifying the resulting code until this was obtained.
> - DSA's offloading of STP state, bridge flags, VLAN filtering and
>   multicast router could be converted right away. But the ageing time
>   could not, so a shim was introduced and this was left for a further
>   commit.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  drivers/net/dsa/b53/b53_common.c              |  6 +-
>  drivers/net/dsa/b53/b53_priv.h                |  3 +-
>  drivers/net/dsa/dsa_loop.c                    |  3 +-
>  drivers/net/dsa/hirschmann/hellcreek.c        |  6 +-
>  drivers/net/dsa/lantiq_gswip.c                | 26 +----
>  drivers/net/dsa/microchip/ksz8795.c           |  6 +-
>  drivers/net/dsa/microchip/ksz9477.c           |  6 +-
>  drivers/net/dsa/mt7530.c                      |  6 +-
>  drivers/net/dsa/mv88e6xxx/chip.c              |  7 +-
>  drivers/net/dsa/ocelot/felix.c                |  5 +-
>  drivers/net/dsa/qca8k.c                       |  6 +-
>  drivers/net/dsa/realtek-smi-core.h            |  3 +-
>  drivers/net/dsa/rtl8366.c                     | 11 +--
>  drivers/net/dsa/sja1105/sja1105.h             |  3 +-
>  drivers/net/dsa/sja1105/sja1105_devlink.c     |  9 +-
>  drivers/net/dsa/sja1105/sja1105_main.c        | 17 ++--
>  .../marvell/prestera/prestera_switchdev.c     | 37 ++-----
>  .../mellanox/mlxsw/spectrum_switchdev.c       | 63 +++---------
>  drivers/net/ethernet/mscc/ocelot.c            | 32 ++----
>  drivers/net/ethernet/mscc/ocelot_net.c        | 13 +--
>  drivers/net/ethernet/rocker/rocker.h          |  6 +-
>  drivers/net/ethernet/rocker/rocker_main.c     | 46 +++------
>  drivers/net/ethernet/rocker/rocker_ofdpa.c    | 23 ++---
>  drivers/net/ethernet/ti/cpsw_switchdev.c      | 20 +---
>  drivers/staging/fsl-dpaa2/ethsw/ethsw.c       | 21 +---
>  include/net/dsa.h                             |  3 +-
>  include/net/switchdev.h                       |  7 +-
>  include/soc/mscc/ocelot.h                     |  3 +-
>  net/dsa/dsa_priv.h                            | 18 ++--
>  net/dsa/port.c                                | 97 ++++++++-----------
>  net/dsa/slave.c                               | 17 ++--
>  net/dsa/switch.c                              | 11 +--
>  net/switchdev/switchdev.c                     | 46 +--------
>  33 files changed, 162 insertions(+), 424 deletions(-)
>

Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>

for the hellcreek part.

Thanks,
Kurt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC PATCH net-next 6/9] net: dsa: remove the transactional logic from VLAN objects
  2020-12-17  1:58 ` [RFC PATCH net-next 6/9] net: dsa: remove the transactional logic from VLAN objects Vladimir Oltean
  2020-12-17  2:11   ` Florian Fainelli
  2020-12-17 11:04   ` Vladimir Oltean
@ 2020-12-18  8:49   ` Kurt Kanzenbach
  2020-12-27 13:30   ` Linus Walleij
  3 siblings, 0 replies; 26+ messages in thread
From: Kurt Kanzenbach @ 2020-12-18  8:49 UTC (permalink / raw)
  To: Vladimir Oltean, David S. Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Hauke Mehrtens, Woojung Huh, Microchip Linux Driver Support,
	Sean Wang, Landen Chao, Claudiu Manoil, Alexandre Belloni,
	Linus Walleij, Vadym Kochan, Taras Chornyi, Jiri Pirko,
	Ido Schimmel, Grygorii Strashko, Ioana Ciornei, Ivan Vecera

[-- Attachment #1: Type: text/plain, Size: 502 bytes --]

Hi Vladimir,

On Thu Dec 17 2020, Vladimir Oltean wrote:
> It should be the driver's business to logically separate its VLAN
> offloading into a preparation and a commit phase, and some drivers don't
> need / can't do this.
>
> So remove the transactional shim from DSA and let drivers to propagate
> errors directly from the .port_vlan_add callback.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>

for the hellcreek part.

Thanks,
Kurt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC PATCH net-next 3/9] net: switchdev: remove the transaction structure from port attributes
  2020-12-17  1:58 ` [RFC PATCH net-next 3/9] net: switchdev: remove the transaction structure from port attributes Vladimir Oltean
  2020-12-18  8:46   ` Kurt Kanzenbach
@ 2020-12-27 13:26   ` Linus Walleij
  1 sibling, 0 replies; 26+ messages in thread
From: Linus Walleij @ 2020-12-27 13:26 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: David S. Miller, Jakub Kicinski, netdev, Florian Fainelli,
	Andrew Lunn, Vivien Didelot, Vladimir Oltean, Kurt Kanzenbach,
	Hauke Mehrtens, Woojung Huh, Microchip Linux Driver Support,
	Sean Wang, Landen Chao, Claudiu Manoil, Alexandre Belloni,
	Vadym Kochan, Taras Chornyi, Jiri Pirko, Ido Schimmel,
	Grygorii Strashko, Ioana Ciornei, Ivan Vecera

On Thu, Dec 17, 2020 at 2:59 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:

> Since the introduction of the switchdev API, port attributes were
> transmitted to drivers for offloading using a two-step transactional
> model, with a prepare phase that was supposed to catch all errors, and a
> commit phase that was supposed to never fail.
>
> Some classes of failures can never be avoided, like hardware access, or
> memory allocation. In the latter case, merely attempting to move the
> memory allocation to the preparation phase makes it impossible to avoid
> memory leaks, since commit 91cf8eceffc1 ("switchdev: Remove unused
> transaction item queue") which has removed the unused mechanism of
> passing on the allocated memory between one phase and another.
>
> It is time we admit that separating the preparation from the commit
> phase is something that is best left for the driver to decide, and not
> something that should be baked into the API, especially since there are
> no switchdev callers that depend on this.
>
> This patch removes the struct switchdev_trans member from switchdev port
> attribute notifier structures, and converts drivers to not look at this
> member.
>
> In part, this patch contains a revert of my previous commit 2e554a7a5d8a
> ("net: dsa: propagate switchdev vlan_filtering prepare phase to
> drivers").
>
> For the most part, the conversion was trivial except for:
> - Rocker's world implementation based on Broadcom OF-DPA had an odd
>   implementation of ofdpa_port_attr_bridge_flags_set. The conversion was
>   done mechanically, by pasting the implementation twice, then only
>   keeping the code that would get executed during prepare phase on top,
>   then only keeping the code that gets executed during the commit phase
>   on bottom, then simplifying the resulting code until this was obtained.
> - DSA's offloading of STP state, bridge flags, VLAN filtering and
>   multicast router could be converted right away. But the ageing time
>   could not, so a shim was introduced and this was left for a further
>   commit.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

For RTL8366RB it seems to do what the commit text says so:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC PATCH net-next 6/9] net: dsa: remove the transactional logic from VLAN objects
  2020-12-17  1:58 ` [RFC PATCH net-next 6/9] net: dsa: remove the transactional logic from VLAN objects Vladimir Oltean
                     ` (2 preceding siblings ...)
  2020-12-18  8:49   ` Kurt Kanzenbach
@ 2020-12-27 13:30   ` Linus Walleij
  3 siblings, 0 replies; 26+ messages in thread
From: Linus Walleij @ 2020-12-27 13:30 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: David S. Miller, Jakub Kicinski, netdev, Florian Fainelli,
	Andrew Lunn, Vivien Didelot, Vladimir Oltean, Kurt Kanzenbach,
	Hauke Mehrtens, Woojung Huh, Microchip Linux Driver Support,
	Sean Wang, Landen Chao, Claudiu Manoil, Alexandre Belloni,
	Vadym Kochan, Taras Chornyi, Jiri Pirko, Ido Schimmel,
	Grygorii Strashko, Ioana Ciornei, Ivan Vecera

On Thu, Dec 17, 2020 at 2:59 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:

> It should be the driver's business to logically separate its VLAN
> offloading into a preparation and a commit phase, and some drivers don't
> need / can't do this.
>
> So remove the transactional shim from DSA and let drivers to propagate
> errors directly from the .port_vlan_add callback.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

This makes the RTL8366 handle errors in a strict way so:
Reviewed-by: Linus Wallei <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC PATCH net-next 0/9] Get rid of the switchdev transactional model
  2020-12-17  1:58 [RFC PATCH net-next 0/9] Get rid of the switchdev transactional model Vladimir Oltean
                   ` (8 preceding siblings ...)
  2020-12-17  1:58 ` [RFC PATCH net-next 9/9] net: switchdev: delete the transaction object Vladimir Oltean
@ 2020-12-27 13:32 ` Linus Walleij
  2020-12-28  9:59 ` Jiri Pirko
  10 siblings, 0 replies; 26+ messages in thread
From: Linus Walleij @ 2020-12-27 13:32 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: David S. Miller, Jakub Kicinski, netdev, Florian Fainelli,
	Andrew Lunn, Vivien Didelot, Vladimir Oltean, Kurt Kanzenbach,
	Hauke Mehrtens, Woojung Huh, Microchip Linux Driver Support,
	Sean Wang, Landen Chao, Claudiu Manoil, Alexandre Belloni,
	Vadym Kochan, Taras Chornyi, Jiri Pirko, Ido Schimmel,
	Grygorii Strashko, Ioana Ciornei, Ivan Vecera

On Thu, Dec 17, 2020 at 2:59 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:

> This series comes after the late realization that the prepare/commit
> separation imposed by switchdev does not help literally anybody:
> https://patchwork.kernel.org/project/netdevbpf/patch/20201212203901.351331-1-vladimir.oltean@nxp.com/
>
> We should kill it before it inflicts even more damage to the error
> handling logic in drivers.

I agree with the goal and the series make the kernel less
complex so:
Acked-by: Linus Walleij <linus.walleij@linaro.org>
for the series.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [RFC PATCH net-next 0/9] Get rid of the switchdev transactional model
  2020-12-17  1:58 [RFC PATCH net-next 0/9] Get rid of the switchdev transactional model Vladimir Oltean
                   ` (9 preceding siblings ...)
  2020-12-27 13:32 ` [RFC PATCH net-next 0/9] Get rid of the switchdev transactional model Linus Walleij
@ 2020-12-28  9:59 ` Jiri Pirko
  10 siblings, 0 replies; 26+ messages in thread
From: Jiri Pirko @ 2020-12-28  9:59 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: David S. Miller, Jakub Kicinski, netdev, Florian Fainelli,
	Andrew Lunn, Vivien Didelot, Vladimir Oltean, Kurt Kanzenbach,
	Hauke Mehrtens, Woojung Huh, Microchip Linux Driver Support,
	Sean Wang, Landen Chao, Claudiu Manoil, Alexandre Belloni,
	Linus Walleij, Vadym Kochan, Taras Chornyi, Jiri Pirko,
	Ido Schimmel, Grygorii Strashko, Ioana Ciornei, Ivan Vecera

Thu, Dec 17, 2020 at 02:58:13AM CET, vladimir.oltean@nxp.com wrote:
>This series comes after the late realization that the prepare/commit
>separation imposed by switchdev does not help literally anybody:
>https://patchwork.kernel.org/project/netdevbpf/patch/20201212203901.351331-1-vladimir.oltean@nxp.com/
>
>We should kill it before it inflicts even more damage to the error
>handling logic in drivers.

Awesome, I totally like this patchset. To be honest, I didn't see much
or a point to do the transaction model from the start, yet I remember
there were people requiring it. I guess they are fine now without
it or they don't care anymore.

Acked-by: Jiri Pirko <jiri@nvidia.com>

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2020-12-28 10:02 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-17  1:58 [RFC PATCH net-next 0/9] Get rid of the switchdev transactional model Vladimir Oltean
2020-12-17  1:58 ` [RFC PATCH net-next 1/9] net: switchdev: remove the transaction structure from port object notifiers Vladimir Oltean
2020-12-17  2:14   ` Florian Fainelli
2020-12-17  1:58 ` [RFC PATCH net-next 2/9] net: switchdev: delete switchdev_port_obj_add_now Vladimir Oltean
2020-12-17  2:14   ` Florian Fainelli
2020-12-17  1:58 ` [RFC PATCH net-next 3/9] net: switchdev: remove the transaction structure from port attributes Vladimir Oltean
2020-12-18  8:46   ` Kurt Kanzenbach
2020-12-27 13:26   ` Linus Walleij
2020-12-17  1:58 ` [RFC PATCH net-next 4/9] net: dsa: remove the transactional logic from ageing time notifiers Vladimir Oltean
2020-12-17  2:06   ` Florian Fainelli
2020-12-17  1:58 ` [RFC PATCH net-next 5/9] net: dsa: remove the transactional logic from MDB entries Vladimir Oltean
2020-12-17  2:08   ` Florian Fainelli
2020-12-17 11:03   ` Vladimir Oltean
2020-12-17  1:58 ` [RFC PATCH net-next 6/9] net: dsa: remove the transactional logic from VLAN objects Vladimir Oltean
2020-12-17  2:11   ` Florian Fainelli
2020-12-17 11:04   ` Vladimir Oltean
2020-12-17 11:43     ` Vladimir Oltean
2020-12-18  8:49   ` Kurt Kanzenbach
2020-12-27 13:30   ` Linus Walleij
2020-12-17  1:58 ` [RFC PATCH net-next 7/9] net: dsa: remove obsolete comment about switchdev transactions Vladimir Oltean
2020-12-17  2:11   ` Florian Fainelli
2020-12-17  1:58 ` [RFC PATCH net-next 8/9] mlxsw: spectrum_switchdev: remove transactional logic for VLAN objects Vladimir Oltean
2020-12-17  1:58 ` [RFC PATCH net-next 9/9] net: switchdev: delete the transaction object Vladimir Oltean
2020-12-17  2:11   ` Florian Fainelli
2020-12-27 13:32 ` [RFC PATCH net-next 0/9] Get rid of the switchdev transactional model Linus Walleij
2020-12-28  9:59 ` Jiri Pirko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).