net: dsa: remove the transactional logic from VLAN objects

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 propagate
errors directly from the .port_vlan_add callback.

It would appear that the code has worse error handling now than it had
before. DSA is the only in-kernel user of switchdev that offloads one
switchdev object to more than one port: for every VLAN object offloaded
to a user port, that VLAN is also offloaded to the CPU port. So the
"prepare for user port -> check for errors -> prepare for CPU port ->
check for errors -> commit for user port -> commit for CPU port"
sequence appears to make more sense than the one we are using now:
"offload to user port -> check for errors -> offload to CPU port ->
check for errors", but it is really a compromise. In the new way, we can
catch errors from the commit phase that we previously had to ignore.
But we have our hands tied and cannot do any rollback now: if we add a
VLAN on the CPU port and it fails, we can't do the rollback by simply
deleting it from the user port, because the switchdev API is not so nice
with us: it could have simply been there already, even with the same
flags. So we don't even attempt to rollback anything on addition error,
just leave whatever VLANs managed to get offloaded right where they are.
This should not be a problem at all in practice.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

1 files changed, 18 insertions, 26 deletions

diff --git a/drivers/net/dsa/rtl8366.c b/drivers/net/dsa/rtl8366.c
index 27f429aa89a6..3b24f2e13200 100644
--- a/drivers/net/dsa/rtl8366.c
+++ b/drivers/net/dsa/rtl8366.c
@@ -374,36 +374,26 @@ 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)
+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);
 	struct realtek_smi *smi = ds->priv;
+	u32 member = 0;
+	u32 untag = 0;
+	int ret;
 
 	if (!smi->ops->is_vlan_valid(smi, vlan->vid))
 		return -EINVAL;
 
-	dev_info(smi->dev, "prepare VLAN %04x\n", vlan->vid);
-
 	/* Enable VLAN in the hardware
 	 * FIXME: what's with this 4k business?
 	 * Just rtl8366_enable_vlan() seems inconclusive.
 	 */
-	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)
-{
-	bool untagged = !!(vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED);
-	bool pvid = !!(vlan->flags & BRIDGE_VLAN_INFO_PVID);
-	struct realtek_smi *smi = ds->priv;
-	u32 member = 0;
-	u32 untag = 0;
-	int ret;
-
-	if (!smi->ops->is_vlan_valid(smi, vlan->vid))
-		return;
+	ret = rtl8366_enable_vlan4k(smi, true);
+	if (ret)
+		return ret;
 
 	dev_info(smi->dev, "add VLAN %d on port %d, %s, %s\n",
 		 vlan->vid, port, untagged ? "untagged" : "tagged",
@@ -418,20 +408,22 @@ void rtl8366_vlan_add(struct dsa_switch *ds, int port,
 		untag |= BIT(port);
 
 	ret = rtl8366_set_vlan(smi, vlan->vid, member, untag, 0);
-	if (ret)
+	if (ret) {
 		dev_err(smi->dev, "failed to set up VLAN %04x", vlan->vid);
+		return ret;
+	}
 
 	if (!pvid)
-		return;
+		return 0;
 
 	ret = rtl8366_set_pvid(smi, port, vlan->vid);
-	if (ret)
+	if (ret) {
 		dev_err(smi->dev, "failed to set PVID on port %d to VLAN %04x",
 			port, vlan->vid);
+		return ret;
+	}
 
-	if (!ret)
-		dev_dbg(smi->dev, "VLAN add: added VLAN %d with PVID on port %d\n",
-			vlan->vid, port);
+	return 0;
 }
 EXPORT_SYMBOL_GPL(rtl8366_vlan_add);