[PATCH net-next 00/13] Improvements to DSA core VLAN manipulation

[PATCH net-next 00/13] Improvements to DSA core VLAN manipulation

[Vladimir Oltean]

In preparation of submitting the NXP SJA1105 driver, the Broadcom b53
and Mediatek mt7530 drivers have been found to apply some VLAN
workarounds that are needed in the new driver as well.

Therefore this patchset is mostly simply promoting the DSA driver
workarounds for VLAN to the generic code.

The b53 driver was applying a few workarounds in order to convince DSA
that its vlan_filtering setting is not really per-port. This is now
simply set by the driver via a DSA variable at probe time. The sja1105
driver will be a second user of this.

The mt7530 was also keeping track of when the .port_vlan_filtering
callback was being called. Remove the kept state from this driver
and simplify dealing with vlan_filtering in the generic case.

TODO:

Find the best way to deal generically with the situation described below
(discussion at https://lkml.org/lkml/2019/4/16/1355):

> > +Segregating the switch ports in multiple bridges is supported (e.g. 2 + 2), but
> > +all bridges should have the same level of VLAN awareness (either both have
> > +``vlan_filtering`` 0, or both 1). Also an inevitable limitation of the fact
> > +that VLAN awareness is global at the switch level is that once a bridge with
> > +``vlan_filtering`` enslaves at least one switch port, the other un-bridged
> > +ports are no longer available for standalone traffic termination.
> 
> That is quite a limitation that I don't think I had fully grasped until
> reading your different patches. Since enslaving ports into a bridge
> comes after the network device was already made available for use, maybe
> you should force the carrier down or something along those lines as soon
> as a port is enslaved into a bridge with vlan_filtering=1 to make this
> more predictable for the user?

Vladimir Oltean (13):
  net: dsa: Fix pharse -> phase typo
  net: dsa: Store vlan_filtering as a property of dsa_port
  net: dsa: Be aware of switches where VLAN filtering is a global
    setting
  net: dsa: b53: Let DSA handle mismatched VLAN filtering settings
  net: dsa: Unset vlan_filtering when ports leave the bridge
  net: dsa: mt7530: Let DSA handle the unsetting of vlan_filtering
  net: dsa: Keep the vlan_filtering setting in dsa_switch if it's global
  net: dsa: Add helper function to retrieve VLAN awareness setting
  net: dsa: mt7530: Use the DSA vlan_filtering helper function
  net: dsa: Skip calling .port_vlan_filtering on no change
  net: dsa: b53: Let DSA call .port_vlan_filtering only when necessary
  net: dsa: b53: Use vlan_filtering property from dsa_switch
  net: dsa: Add more convenient functions for installing port VLANs

 drivers/net/dsa/b53/b53_common.c | 34 +++++--------
 drivers/net/dsa/b53/b53_priv.h   |  1 -
 drivers/net/dsa/mt7530.c         | 20 +++-----
 drivers/net/dsa/mt7530.h         |  1 -
 include/net/dsa.h                | 21 ++++++++
 net/dsa/dsa_priv.h               |  2 +
 net/dsa/port.c                   | 85 ++++++++++++++++++++++++++++++--
 net/dsa/slave.c                  | 24 ++-------
 net/dsa/switch.c                 | 31 +++++++++++-
 9 files changed, 156 insertions(+), 63 deletions(-)

-- 
2.17.1

[PATCH net-next 01/13] net: dsa: Fix pharse -> phase typo

[Vladimir Oltean]

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/switch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index e1fae969aa73..fde4e9195709 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -196,7 +196,7 @@ static int dsa_port_vlan_check(struct dsa_switch *ds, int port,
 	if (!dp->bridge_dev)
 		return err;
 
-	/* dsa_slave_vlan_rx_{add,kill}_vid() cannot use the prepare pharse and
+	/* dsa_slave_vlan_rx_{add,kill}_vid() cannot use the prepare phase and
 	 * already checks whether there is an overlapping bridge VLAN entry
 	 * with the same VID, so here we only need to check that if we are
 	 * adding a bridge VLAN entry there is not an overlapping VLAN device
-- 
2.17.1

[PATCH net-next 02/13] net: dsa: Store vlan_filtering as a property of dsa_port

[Vladimir Oltean]

This allows drivers to query the VLAN setting imposed by the bridge
driver directly from DSA, instead of keeping their own state based on
the .port_vlan_filtering callback.

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 include/net/dsa.h |  1 +
 net/dsa/port.c    | 12 ++++++++----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index b550f7bb5314..79a87913126c 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -161,6 +161,7 @@ struct dsa_port {
 	const char		*mac;
 	struct device_node	*dn;
 	unsigned int		ageing_time;
+	bool			vlan_filtering;
 	u8			stp_state;
 	struct net_device	*bridge_dev;
 	struct devlink_port	devlink_port;
diff --git a/net/dsa/port.c b/net/dsa/port.c
index ea848596afe3..0caf7f9bfb57 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -158,15 +158,19 @@ int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
 			    struct switchdev_trans *trans)
 {
 	struct dsa_switch *ds = dp->ds;
+	int err;
 
 	/* bridge skips -EOPNOTSUPP, so skip the prepare phase */
 	if (switchdev_trans_ph_prepare(trans))
 		return 0;
 
-	if (ds->ops->port_vlan_filtering)
-		return ds->ops->port_vlan_filtering(ds, dp->index,
-						    vlan_filtering);
-
+	if (ds->ops->port_vlan_filtering) {
+		err = ds->ops->port_vlan_filtering(ds, dp->index,
+						   vlan_filtering);
+		if (err)
+			return err;
+		dp->vlan_filtering = vlan_filtering;
+	}
 	return 0;
 }
 
-- 
2.17.1

[PATCH net-next 03/13] net: dsa: Be aware of switches where VLAN filtering is a global setting

[Vladimir Oltean]

On some switches, the action of whether to parse VLAN frame headers and use
that information for ingress admission is configurable, but not per
port. Such is the case for the Broadcom BCM53xx and the NXP SJA1105
families, for example. In that case, DSA can prevent the bridge core
from trying to apply different VLAN filtering settings on net devices
that belong to the same switch.

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Suggested-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 include/net/dsa.h |  5 +++++
 net/dsa/port.c    | 52 ++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 79a87913126c..aab3c2029edd 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -228,6 +228,11 @@ struct dsa_switch {
 	/* Number of switch port queues */
 	unsigned int		num_tx_queues;
 
+	/* Disallow bridge core from requesting different VLAN awareness
+	 * settings on ports if not hardware-supported
+	 */
+	bool			vlan_filtering_is_global;
+
 	unsigned long		*bitmap;
 	unsigned long		_bitmap;
 
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 0caf7f9bfb57..67bfad30cd40 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -154,6 +154,39 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
 	dsa_port_set_state_now(dp, BR_STATE_FORWARDING);
 }
 
+static bool dsa_port_can_apply_vlan_filtering(struct dsa_port *dp,
+					      bool vlan_filtering)
+{
+	struct dsa_switch *ds = dp->ds;
+	int i;
+
+	if (!ds->vlan_filtering_is_global)
+		return true;
+
+	/* For cases where enabling/disabling VLAN awareness is global to the
+	 * switch, we need to handle the case where multiple bridges span
+	 * different ports of the same switch device and one of them has a
+	 * different setting than what is being requested.
+	 */
+	for (i = 0; i < ds->num_ports; i++) {
+		struct net_device *other_bridge;
+
+		other_bridge = dsa_to_port(ds, i)->bridge_dev;
+		if (!other_bridge)
+			continue;
+		/* If it's the same bridge, it also has same
+		 * vlan_filtering setting => no need to check
+		 */
+		if (other_bridge == dp->bridge_dev)
+			continue;
+		if (br_vlan_enabled(other_bridge) != vlan_filtering) {
+			dev_err(ds->dev, "VLAN filtering is a global setting\n");
+			return false;
+		}
+	}
+	return true;
+}
+
 int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
 			    struct switchdev_trans *trans)
 {
@@ -164,13 +197,18 @@ int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
 	if (switchdev_trans_ph_prepare(trans))
 		return 0;
 
-	if (ds->ops->port_vlan_filtering) {
-		err = ds->ops->port_vlan_filtering(ds, dp->index,
-						   vlan_filtering);
-		if (err)
-			return err;
-		dp->vlan_filtering = vlan_filtering;
-	}
+	if (!ds->ops->port_vlan_filtering)
+		return 0;
+
+	if (!dsa_port_can_apply_vlan_filtering(dp, vlan_filtering))
+		return -EINVAL;
+
+	err = ds->ops->port_vlan_filtering(ds, dp->index,
+					   vlan_filtering);
+	if (err)
+		return err;
+
+	dp->vlan_filtering = vlan_filtering;
 	return 0;
 }
 
-- 
2.17.1

[PATCH net-next 04/13] net: dsa: b53: Let DSA handle mismatched VLAN filtering settings

[Vladimir Oltean]

The DSA core is now able to do this check prior to calling the
.port_vlan_filtering callback, so tell it that VLAN filtering is global
for this particular hardware.

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Suggested-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 0852e5e08177..a779b9c3ab6e 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -966,6 +966,13 @@ static int b53_setup(struct dsa_switch *ds)
 			b53_disable_port(ds, port);
 	}
 
+	/* Let DSA handle the case were multiple bridges span the same switch
+	 * device and different VLAN awareness settings are requested, which
+	 * would be breaking filtering semantics for any of the other bridge
+	 * devices. (not hardware supported)
+	 */
+	ds->vlan_filtering_is_global = true;
+
 	return ret;
 }
 
@@ -1275,26 +1282,8 @@ EXPORT_SYMBOL(b53_phylink_mac_link_up);
 int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering)
 {
 	struct b53_device *dev = ds->priv;
-	struct net_device *bridge_dev;
-	unsigned int i;
 	u16 pvid, new_pvid;
 
-	/* Handle the case were multiple bridges span the same switch device
-	 * and one of them has a different setting than what is being requested
-	 * which would be breaking filtering semantics for any of the other
-	 * bridge devices.
-	 */
-	b53_for_each_port(dev, i) {
-		bridge_dev = dsa_to_port(ds, i)->bridge_dev;
-		if (bridge_dev &&
-		    bridge_dev != dsa_to_port(ds, port)->bridge_dev &&
-		    br_vlan_enabled(bridge_dev) != vlan_filtering) {
-			netdev_err(bridge_dev,
-				   "VLAN filtering is global to the switch!\n");
-			return -EINVAL;
-		}
-	}
-
 	b53_read16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(port), &pvid);
 	new_pvid = pvid;
 	if (dev->vlan_filtering_enabled && !vlan_filtering) {
-- 
2.17.1

[PATCH net-next 05/13] net: dsa: Unset vlan_filtering when ports leave the bridge

[Vladimir Oltean]

When ports are standalone (after they left the bridge), they should have
no VLAN filtering semantics (they should pass all traffic to the CPU).
Currently this is not true for switchdev drivers, because the bridge
"forgets" to unset that.

Normally one would think that doing this at the bridge layer would be a
better idea, i.e. call br_vlan_filter_toggle() from br_del_if(), similar
to how nbp_vlan_init() is called from br_add_if().

However what complicates that approach, and makes this one preferable,
is the fact that for the bridge core, vlan_filtering is a per-bridge
setting, whereas for switchdev/DSA it is per-port. Also there are
switches where the setting is per the entire device, and unsetting
vlan_filtering one by one, for each leaving port, would not be possible
from the bridge core without a certain level of awareness. So do this in
DSA and let drivers be unaware of it.

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/switch.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index fde4e9195709..7d8cd9bc0ecc 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -10,6 +10,7 @@
  * (at your option) any later version.
  */
 
+#include <linux/if_bridge.h>
 #include <linux/netdevice.h>
 #include <linux/notifier.h>
 #include <linux/if_vlan.h>
@@ -71,6 +72,9 @@ static int dsa_switch_bridge_join(struct dsa_switch *ds,
 static int dsa_switch_bridge_leave(struct dsa_switch *ds,
 				   struct dsa_notifier_bridge_info *info)
 {
+	bool unset_vlan_filtering = br_vlan_enabled(info->br);
+	int err, i;
+
 	if (ds->index == info->sw_index && ds->ops->port_bridge_leave)
 		ds->ops->port_bridge_leave(ds, info->port, info->br);
 
@@ -78,6 +82,31 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds,
 		ds->ops->crosschip_bridge_leave(ds, info->sw_index, info->port,
 						info->br);
 
+	/* If the bridge was vlan_filtering, the bridge core doesn't trigger an
+	 * event for changing vlan_filtering setting upon slave ports leaving
+	 * it. That is a good thing, because that lets us handle it and also
+	 * handle the case where the switch's vlan_filtering setting is global
+	 * (not per port). When that happens, the correct moment to trigger the
+	 * vlan_filtering callback is only when the last port left this bridge.
+	 */
+	if (unset_vlan_filtering && ds->vlan_filtering_is_global) {
+		for (i = 0; i < ds->num_ports; i++) {
+			if (i == info->port)
+				continue;
+			if (dsa_to_port(ds, i)->bridge_dev == info->br) {
+				unset_vlan_filtering = false;
+				break;
+			}
+		}
+	}
+	if (unset_vlan_filtering) {
+		struct switchdev_trans trans = {0};
+
+		err = dsa_port_vlan_filtering(&ds->ports[info->port],
+					      false, &trans);
+		if (err && err != EOPNOTSUPP)
+			return err;
+	}
 	return 0;
 }
 
-- 
2.17.1

[PATCH net-next 06/13] net: dsa: mt7530: Let DSA handle the unsetting of vlan_filtering

[Vladimir Oltean]

The driver, recognizing that the .port_vlan_filtering callback was never
coming after the port left its parent bridge, decided to take that duty
in its own hands. DSA now takes care of this condition, so fix that.

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/mt7530.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 7357b4fc0185..3530b6f38428 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -910,8 +910,6 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
 			   PCR_MATRIX(BIT(MT7530_CPU_PORT)));
 	priv->ports[port].pm = PCR_MATRIX(BIT(MT7530_CPU_PORT));
 
-	mt7530_port_set_vlan_unaware(ds, port);
-
 	mutex_unlock(&priv->reg_mutex);
 }
 
@@ -1025,6 +1023,8 @@ mt7530_port_vlan_filtering(struct dsa_switch *ds, int port,
 		 */
 		mt7530_port_set_vlan_aware(ds, port);
 		mt7530_port_set_vlan_aware(ds, MT7530_CPU_PORT);
+	} else {
+		mt7530_port_set_vlan_unaware(ds, port);
 	}
 
 	return 0;
-- 
2.17.1

[PATCH net-next 07/13] net: dsa: Keep the vlan_filtering setting in dsa_switch if it's global

[Vladimir Oltean]

The current behavior is not as obvious as one would assume (which is
that, if the driver set vlan_filtering_is_global = 1, then checking any
dp->vlan_filtering would yield the same result). Only the ports which
are actively enslaved into a bridge would have vlan_filtering set.

This makes it tricky for drivers to check what the global state is.
So fix this and make the struct dsa_switch hold this global setting.

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 include/net/dsa.h | 5 +++++
 net/dsa/port.c    | 5 ++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index aab3c2029edd..4e0f7e9c5aa1 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -233,6 +233,11 @@ struct dsa_switch {
 	 */
 	bool			vlan_filtering_is_global;
 
+	/* In case vlan_filtering_is_global is set, the VLAN awareness state
+	 * should be retrieved from here and not from the per-port settings.
+	 */
+	bool			vlan_filtering;
+
 	unsigned long		*bitmap;
 	unsigned long		_bitmap;
 
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 67bfad30cd40..555fdbf0a3f4 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -208,7 +208,10 @@ int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
 	if (err)
 		return err;
 
-	dp->vlan_filtering = vlan_filtering;
+	if (ds->vlan_filtering_is_global)
+		ds->vlan_filtering = vlan_filtering;
+	else
+		dp->vlan_filtering = vlan_filtering;
 	return 0;
 }
 
-- 
2.17.1

[PATCH net-next 08/13] net: dsa: Add helper function to retrieve VLAN awareness setting

[Vladimir Oltean]

Since different types of hardware may or may not support this setting
per-port, DSA keeps it either in dsa_switch or in dsa_port.

While drivers may know the characteristics of their hardware and
retrieve it from the correct place without the need of helpers, it is
cumbersone to find out an unambigous answer from generic DSA code.

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 include/net/dsa.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 4e0f7e9c5aa1..1e6b4efc80b9 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -305,6 +305,16 @@ static inline unsigned int dsa_upstream_port(struct dsa_switch *ds, int port)
 	return dsa_towards_port(ds, cpu_dp->ds->index, cpu_dp->index);
 }
 
+static inline bool dsa_port_is_vlan_filtering(const struct dsa_port *dp)
+{
+	const struct dsa_switch *ds = dp->ds;
+
+	if (ds->vlan_filtering_is_global)
+		return ds->vlan_filtering;
+	else
+		return dp->vlan_filtering;
+}
+
 typedef int dsa_fdb_dump_cb_t(const unsigned char *addr, u16 vid,
 			      bool is_static, void *data);
 struct dsa_switch_ops {
-- 
2.17.1

Re: [PATCH net-next 08/13] net: dsa: Add helper function to retrieve VLAN awareness setting

[Andrew Lunn]

On Sun, Apr 28, 2019 at 09:45:49PM +0300, Vladimir Oltean wrote:
> Since different types of hardware may or may not support this setting
> per-port, DSA keeps it either in dsa_switch or in dsa_port.
> 
> While drivers may know the characteristics of their hardware and
> retrieve it from the correct place without the need of helpers, it is
> cumbersone to find out an unambigous answer from generic DSA code.
> 
> Signed-off-by: Vladimir Oltean <olteanv@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 09/13] net: dsa: mt7530: Use the DSA vlan_filtering helper function

[Vladimir Oltean]

This was recently introduced, so keeping state inside the driver is no
longer necessary.

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Suggested-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/mt7530.c | 16 +++++-----------
 drivers/net/dsa/mt7530.h |  1 -
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 3530b6f38428..8d531c5f21f3 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -828,11 +828,9 @@ mt7530_port_set_vlan_unaware(struct dsa_switch *ds, int port)
 	mt7530_rmw(priv, MT7530_PVC_P(port), VLAN_ATTR_MASK,
 		   VLAN_ATTR(MT7530_VLAN_TRANSPARENT));
 
-	priv->ports[port].vlan_filtering = false;
-
 	for (i = 0; i < MT7530_NUM_PORTS; i++) {
 		if (dsa_is_user_port(ds, i) &&
-		    priv->ports[i].vlan_filtering) {
+		    dsa_port_is_vlan_filtering(&ds->ports[i])) {
 			all_user_ports_removed = false;
 			break;
 		}
@@ -891,8 +889,8 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
 		 * And the other port's port matrix cannot be broken when the
 		 * other port is still a VLAN-aware port.
 		 */
-		if (!priv->ports[i].vlan_filtering &&
-		    dsa_is_user_port(ds, i) && i != port) {
+		if (dsa_is_user_port(ds, i) && i != port &&
+		   !dsa_port_is_vlan_filtering(&ds->ports[i])) {
 			if (dsa_to_port(ds, i)->bridge_dev != bridge)
 				continue;
 			if (priv->ports[i].enable)
@@ -1011,10 +1009,6 @@ static int
 mt7530_port_vlan_filtering(struct dsa_switch *ds, int port,
 			   bool vlan_filtering)
 {
-	struct mt7530_priv *priv = ds->priv;
-
-	priv->ports[port].vlan_filtering = vlan_filtering;
-
 	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
@@ -1139,7 +1133,7 @@ mt7530_port_vlan_add(struct dsa_switch *ds, int port,
 	/* The port is kept as VLAN-unaware if bridge with vlan_filtering not
 	 * being set.
 	 */
-	if (!priv->ports[port].vlan_filtering)
+	if (!dsa_port_is_vlan_filtering(&ds->ports[port]))
 		return;
 
 	mutex_lock(&priv->reg_mutex);
@@ -1170,7 +1164,7 @@ mt7530_port_vlan_del(struct dsa_switch *ds, int port,
 	/* The port is kept as VLAN-unaware if bridge with vlan_filtering not
 	 * being set.
 	 */
-	if (!priv->ports[port].vlan_filtering)
+	if (!dsa_port_is_vlan_filtering(&ds->ports[port]))
 		return 0;
 
 	mutex_lock(&priv->reg_mutex);
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index a95ed958df5b..1eec7bdc283a 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -410,7 +410,6 @@ struct mt7530_port {
 	bool enable;
 	u32 pm;
 	u16 pvid;
-	bool vlan_filtering;
 };
 
 /* struct mt7530_priv -	This is the main data structure for holding the state
-- 
2.17.1

[PATCH net-next 10/13] net: dsa: Skip calling .port_vlan_filtering on no change

[Vladimir Oltean]

Even if VLAN filtering is global, DSA will call this callback once per
each port. Drivers should not have to compare the global state with the
requested change. So let DSA do it.

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 net/dsa/port.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/dsa/port.c b/net/dsa/port.c
index 555fdbf0a3f4..da5922ba7d33 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -203,6 +203,9 @@ int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
 	if (!dsa_port_can_apply_vlan_filtering(dp, vlan_filtering))
 		return -EINVAL;
 
+	if (dsa_port_is_vlan_filtering(dp) == vlan_filtering)
+		return 0;
+
 	err = ds->ops->port_vlan_filtering(ds, dp->index,
 					   vlan_filtering);
 	if (err)
-- 
2.17.1

Re: [PATCH net-next 10/13] net: dsa: Skip calling .port_vlan_filtering on no change

[Andrew Lunn]

On Sun, Apr 28, 2019 at 09:45:51PM +0300, Vladimir Oltean wrote:
> Even if VLAN filtering is global, DSA will call this callback once per
> each port. Drivers should not have to compare the global state with the
> requested change. So let DSA do it.
> 
> Signed-off-by: Vladimir Oltean <olteanv@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 11/13] net: dsa: b53: Let DSA call .port_vlan_filtering only when necessary

[Vladimir Oltean]

Since DSA has recently learned to treat better with drivers that set
vlan_filtering_is_global, doing this is no longer required.

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index a779b9c3ab6e..9fbeb20ba263 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1286,13 +1286,13 @@ int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering)
 
 	b53_read16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(port), &pvid);
 	new_pvid = pvid;
-	if (dev->vlan_filtering_enabled && !vlan_filtering) {
+	if (!vlan_filtering) {
 		/* Filtering is currently enabled, use the default PVID since
 		 * the bridge does not expect tagging anymore
 		 */
 		dev->ports[port].pvid = pvid;
 		new_pvid = b53_default_pvid(dev);
-	} else if (!dev->vlan_filtering_enabled && vlan_filtering) {
+	} else {
 		/* Filtering is currently disabled, restore the previous PVID */
 		new_pvid = dev->ports[port].pvid;
 	}
-- 
2.17.1

Re: [PATCH net-next 11/13] net: dsa: b53: Let DSA call .port_vlan_filtering only when necessary

[Andrew Lunn]

On Sun, Apr 28, 2019 at 09:45:52PM +0300, Vladimir Oltean wrote:
> Since DSA has recently learned to treat better with drivers that set
> vlan_filtering_is_global, doing this is no longer required.
> 
> Signed-off-by: Vladimir Oltean <olteanv@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 12/13] net: dsa: b53: Use vlan_filtering property from dsa_switch

[Vladimir Oltean]

While possible (and safe) to use the newly introduced
dsa_port_is_vlan_filtering helper, fabricating a dsa_port pointer is a
bit awkward, so simply retrieve this from the dsa_switch structure.

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 5 ++---
 drivers/net/dsa/b53/b53_priv.h   | 1 -
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 9fbeb20ba263..c8040ecf4425 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -428,7 +428,6 @@ static void b53_enable_vlan(struct b53_device *dev, bool enable,
 	b53_write8(dev, B53_CTRL_PAGE, B53_SWITCH_MODE, mgmt);
 
 	dev->vlan_enabled = enable;
-	dev->vlan_filtering_enabled = enable_filtering;
 }
 
 static int b53_set_jumbo(struct b53_device *dev, bool enable, bool allow_10_100)
@@ -665,7 +664,7 @@ int b53_configure_vlan(struct dsa_switch *ds)
 		b53_do_vlan_op(dev, VTA_CMD_CLEAR);
 	}
 
-	b53_enable_vlan(dev, false, dev->vlan_filtering_enabled);
+	b53_enable_vlan(dev, false, ds->vlan_filtering);
 
 	b53_for_each_port(dev, i)
 		b53_write16(dev, B53_VLAN_PAGE,
@@ -1318,7 +1317,7 @@ int b53_vlan_prepare(struct dsa_switch *ds, int port,
 	if (vlan->vid_end > dev->num_vlans)
 		return -ERANGE;
 
-	b53_enable_vlan(dev, true, dev->vlan_filtering_enabled);
+	b53_enable_vlan(dev, true, ds->vlan_filtering);
 
 	return 0;
 }
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index e3441dcf2d21..f25bc80c4ffc 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -139,7 +139,6 @@ struct b53_device {
 	unsigned int num_vlans;
 	struct b53_vlan *vlans;
 	bool vlan_enabled;
-	bool vlan_filtering_enabled;
 	unsigned int num_ports;
 	struct b53_port *ports;
 };
-- 
2.17.1

Re: [PATCH net-next 12/13] net: dsa: b53: Use vlan_filtering property from dsa_switch

[Andrew Lunn]

On Sun, Apr 28, 2019 at 09:45:53PM +0300, Vladimir Oltean wrote:
> While possible (and safe) to use the newly introduced
> dsa_port_is_vlan_filtering helper, fabricating a dsa_port pointer is a
> bit awkward, so simply retrieve this from the dsa_switch structure.
> 
> Signed-off-by: Vladimir Oltean <olteanv@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 13/13] net: dsa: Add more convenient functions for installing port VLANs

[Vladimir Oltean]

This hides the need to perform a two-phase transaction and construct a
switchdev_obj_port_vlan struct.

Call graph (including a function that will be introduced in a follow-up
patch) looks like this now (same for the *_vlan_del function):

dsa_slave_vlan_rx_add_vid   dsa_port_setup_8021q_tagging
            |                        |
            |                        |
            |          +-------------+
            |          |
            v          v
           dsa_port_vid_add      dsa_slave_port_obj_add
                  |                         |
                  +-------+         +-------+
                          |         |
                          v         v
                       dsa_port_vlan_add

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/dsa_priv.h |  2 ++
 net/dsa/port.c     | 31 +++++++++++++++++++++++++++++++
 net/dsa/slave.c    | 24 +++---------------------
 3 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index e860512d673a..37751b505572 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -171,6 +171,8 @@ int dsa_port_vlan_add(struct dsa_port *dp,
 		      struct switchdev_trans *trans);
 int dsa_port_vlan_del(struct dsa_port *dp,
 		      const struct switchdev_obj_port_vlan *vlan);
+int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 flags);
+int dsa_port_vid_del(struct dsa_port *dp, u16 vid);
 int dsa_port_link_register_of(struct dsa_port *dp);
 void dsa_port_link_unregister_of(struct dsa_port *dp);
 
diff --git a/net/dsa/port.c b/net/dsa/port.c
index da5922ba7d33..e6ad19df5cd8 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -370,6 +370,37 @@ int dsa_port_vlan_del(struct dsa_port *dp,
 	return 0;
 }
 
+int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 flags)
+{
+	struct switchdev_obj_port_vlan vlan = {
+		.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
+		.flags = flags,
+		.vid_begin = vid,
+		.vid_end = vid,
+	};
+	struct switchdev_trans trans;
+	int err;
+
+	trans.ph_prepare = true;
+	err = dsa_port_vlan_add(dp, &vlan, &trans);
+	if (err == -EOPNOTSUPP)
+		return 0;
+
+	trans.ph_prepare = false;
+	return dsa_port_vlan_add(dp, &vlan, &trans);
+}
+
+int dsa_port_vid_del(struct dsa_port *dp, u16 vid)
+{
+	struct switchdev_obj_port_vlan vlan = {
+		.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
+		.vid_begin = vid,
+		.vid_end = vid,
+	};
+
+	return dsa_port_vlan_del(dp, &vlan);
+}
+
 static struct phy_device *dsa_port_get_phy_device(struct dsa_port *dp)
 {
 	struct device_node *phy_dn;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index ce26dddc8270..8ad9bf957da1 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1001,13 +1001,6 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
 				     u16 vid)
 {
 	struct dsa_port *dp = dsa_slave_to_port(dev);
-	struct switchdev_obj_port_vlan vlan = {
-		.vid_begin = vid,
-		.vid_end = vid,
-		/* This API only allows programming tagged, non-PVID VIDs */
-		.flags = 0,
-	};
-	struct switchdev_trans trans;
 	struct bridge_vlan_info info;
 	int ret;
 
@@ -1024,25 +1017,14 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
 			return -EBUSY;
 	}
 
-	trans.ph_prepare = true;
-	ret = dsa_port_vlan_add(dp, &vlan, &trans);
-	if (ret == -EOPNOTSUPP)
-		return 0;
-
-	trans.ph_prepare = false;
-	return dsa_port_vlan_add(dp, &vlan, &trans);
+	/* This API only allows programming tagged, non-PVID VIDs */
+	return dsa_port_vid_add(dp, vid, 0);
 }
 
 static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
 				      u16 vid)
 {
 	struct dsa_port *dp = dsa_slave_to_port(dev);
-	struct switchdev_obj_port_vlan vlan = {
-		.vid_begin = vid,
-		.vid_end = vid,
-		/* This API only allows programming tagged, non-PVID VIDs */
-		.flags = 0,
-	};
 	struct bridge_vlan_info info;
 	int ret;
 
@@ -1059,7 +1041,7 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
 			return -EBUSY;
 	}
 
-	ret = dsa_port_vlan_del(dp, &vlan);
+	ret = dsa_port_vid_del(dp, vid);
 	if (ret == -EOPNOTSUPP)
 		ret = 0;
 
-- 
2.17.1

Re: [PATCH net-next 00/13] Improvements to DSA core VLAN manipulation

[David Miller]

From: Vladimir Oltean <olteanv@gmail.com>
Date: Sun, 28 Apr 2019 21:45:41 +0300

> In preparation of submitting the NXP SJA1105 driver, the Broadcom b53
> and Mediatek mt7530 drivers have been found to apply some VLAN
> workarounds that are needed in the new driver as well.
> 
> Therefore this patchset is mostly simply promoting the DSA driver
> workarounds for VLAN to the generic code.
 ...

Series applied, thanks.