All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] net: dsa: mv88e6xxx: do not leave reserved VLANs
@ 2016-02-05 19:07 Vivien Didelot
  2016-02-06 18:27 ` Andrew Lunn
  2016-02-13 11:08 ` David Miller
  0 siblings, 2 replies; 3+ messages in thread
From: Vivien Didelot @ 2016-02-05 19:07 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn, Kevin Smith,
	Vivien Didelot

BRIDGE_VLAN_FILTERING automatically adds a newly bridged port to the
VLAN with the bridge's default_pvid.

The mv88e6xxx driver currently reserves VLANs 4000+ for unbridged ports
isolation. When a port joins a bridge, it leaves its reserved VLAN. When
a port leaves a bridge, it joins again its reserved VLAN.

But if the VLAN filtering is disabled, or if this hardware VLAN is
already in use, the bridged port ends up with no default VLAN, and the
communication with the CPU is thus broken.

To fix this, make a port join its reserved VLAN once on setup, never
leave it, and restore its PVID after another one was eventually used.

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

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 1cb3d15..4196dd8 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -1572,6 +1572,7 @@ int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port,
 			    const struct switchdev_obj_port_vlan *vlan)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	const u16 defpvid = 4000 + ds->index * DSA_MAX_PORTS + port;
 	u16 pvid, vid;
 	int err = 0;
 
@@ -1587,7 +1588,8 @@ int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port,
 			goto unlock;
 
 		if (vid == pvid) {
-			err = _mv88e6xxx_port_pvid_set(ds, port, 0);
+			/* restore reserved VLAN ID */
+			err = _mv88e6xxx_port_pvid_set(ds, port, defpvid);
 			if (err)
 				goto unlock;
 		}
@@ -1879,26 +1881,20 @@ unlock:
 
 int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port, u32 members)
 {
-	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
-	const u16 pvid = 4000 + ds->index * DSA_MAX_PORTS + port;
-	int err;
-
-	/* The port joined a bridge, so leave its reserved VLAN */
-	mutex_lock(&ps->smi_mutex);
-	err = _mv88e6xxx_port_vlan_del(ds, port, pvid);
-	if (!err)
-		err = _mv88e6xxx_port_pvid_set(ds, port, 0);
-	mutex_unlock(&ps->smi_mutex);
-	return err;
+	return 0;
 }
 
 int mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port, u32 members)
 {
+	return 0;
+}
+
+static int mv88e6xxx_setup_port_default_vlan(struct dsa_switch *ds, int port)
+{
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	const u16 pvid = 4000 + ds->index * DSA_MAX_PORTS + port;
 	int err;
 
-	/* The port left the bridge, so join its reserved VLAN */
 	mutex_lock(&ps->smi_mutex);
 	err = _mv88e6xxx_port_vlan_add(ds, port, pvid, true);
 	if (!err)
@@ -2182,8 +2178,7 @@ int mv88e6xxx_setup_ports(struct dsa_switch *ds)
 		if (dsa_is_cpu_port(ds, i) || dsa_is_dsa_port(ds, i))
 			continue;
 
-		/* setup the unbridged state */
-		ret = mv88e6xxx_port_bridge_leave(ds, i, 0);
+		ret = mv88e6xxx_setup_port_default_vlan(ds, i);
 		if (ret < 0)
 			return ret;
 	}
-- 
2.7.0

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

* Re: [PATCH net] net: dsa: mv88e6xxx: do not leave reserved VLANs
  2016-02-05 19:07 [PATCH net] net: dsa: mv88e6xxx: do not leave reserved VLANs Vivien Didelot
@ 2016-02-06 18:27 ` Andrew Lunn
  2016-02-13 11:08 ` David Miller
  1 sibling, 0 replies; 3+ messages in thread
From: Andrew Lunn @ 2016-02-06 18:27 UTC (permalink / raw)
  To: Vivien Didelot; +Cc: netdev, linux-kernel, kernel, David S. Miller, Kevin Smith

On Fri, Feb 05, 2016 at 02:07:14PM -0500, Vivien Didelot wrote:
> BRIDGE_VLAN_FILTERING automatically adds a newly bridged port to the
> VLAN with the bridge's default_pvid.
> 
> The mv88e6xxx driver currently reserves VLANs 4000+ for unbridged ports
> isolation. When a port joins a bridge, it leaves its reserved VLAN. When
> a port leaves a bridge, it joins again its reserved VLAN.
> 
> But if the VLAN filtering is disabled, or if this hardware VLAN is
> already in use, the bridged port ends up with no default VLAN, and the
> communication with the CPU is thus broken.
> 
> To fix this, make a port join its reserved VLAN once on setup, never
> leave it, and restore its PVID after another one was eventually used.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

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

Thanks

	Andrew


> ---
>  drivers/net/dsa/mv88e6xxx.c | 25 ++++++++++---------------
>  1 file changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
> index 1cb3d15..4196dd8 100644
> --- a/drivers/net/dsa/mv88e6xxx.c
> +++ b/drivers/net/dsa/mv88e6xxx.c
> @@ -1572,6 +1572,7 @@ int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port,
>  			    const struct switchdev_obj_port_vlan *vlan)
>  {
>  	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> +	const u16 defpvid = 4000 + ds->index * DSA_MAX_PORTS + port;
>  	u16 pvid, vid;
>  	int err = 0;
>  
> @@ -1587,7 +1588,8 @@ int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port,
>  			goto unlock;
>  
>  		if (vid == pvid) {
> -			err = _mv88e6xxx_port_pvid_set(ds, port, 0);
> +			/* restore reserved VLAN ID */
> +			err = _mv88e6xxx_port_pvid_set(ds, port, defpvid);
>  			if (err)
>  				goto unlock;
>  		}
> @@ -1879,26 +1881,20 @@ unlock:
>  
>  int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port, u32 members)
>  {
> -	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> -	const u16 pvid = 4000 + ds->index * DSA_MAX_PORTS + port;
> -	int err;
> -
> -	/* The port joined a bridge, so leave its reserved VLAN */
> -	mutex_lock(&ps->smi_mutex);
> -	err = _mv88e6xxx_port_vlan_del(ds, port, pvid);
> -	if (!err)
> -		err = _mv88e6xxx_port_pvid_set(ds, port, 0);
> -	mutex_unlock(&ps->smi_mutex);
> -	return err;
> +	return 0;
>  }
>  
>  int mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port, u32 members)
>  {
> +	return 0;
> +}
> +
> +static int mv88e6xxx_setup_port_default_vlan(struct dsa_switch *ds, int port)
> +{
>  	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
>  	const u16 pvid = 4000 + ds->index * DSA_MAX_PORTS + port;
>  	int err;
>  
> -	/* The port left the bridge, so join its reserved VLAN */
>  	mutex_lock(&ps->smi_mutex);
>  	err = _mv88e6xxx_port_vlan_add(ds, port, pvid, true);
>  	if (!err)
> @@ -2182,8 +2178,7 @@ int mv88e6xxx_setup_ports(struct dsa_switch *ds)
>  		if (dsa_is_cpu_port(ds, i) || dsa_is_dsa_port(ds, i))
>  			continue;
>  
> -		/* setup the unbridged state */
> -		ret = mv88e6xxx_port_bridge_leave(ds, i, 0);
> +		ret = mv88e6xxx_setup_port_default_vlan(ds, i);
>  		if (ret < 0)
>  			return ret;
>  	}
> -- 
> 2.7.0
> 

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

* Re: [PATCH net] net: dsa: mv88e6xxx: do not leave reserved VLANs
  2016-02-05 19:07 [PATCH net] net: dsa: mv88e6xxx: do not leave reserved VLANs Vivien Didelot
  2016-02-06 18:27 ` Andrew Lunn
@ 2016-02-13 11:08 ` David Miller
  1 sibling, 0 replies; 3+ messages in thread
From: David Miller @ 2016-02-13 11:08 UTC (permalink / raw)
  To: vivien.didelot; +Cc: netdev, linux-kernel, kernel, andrew, kevin.smith

From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Date: Fri,  5 Feb 2016 14:07:14 -0500

> BRIDGE_VLAN_FILTERING automatically adds a newly bridged port to the
> VLAN with the bridge's default_pvid.
> 
> The mv88e6xxx driver currently reserves VLANs 4000+ for unbridged ports
> isolation. When a port joins a bridge, it leaves its reserved VLAN. When
> a port leaves a bridge, it joins again its reserved VLAN.
> 
> But if the VLAN filtering is disabled, or if this hardware VLAN is
> already in use, the bridged port ends up with no default VLAN, and the
> communication with the CPU is thus broken.
> 
> To fix this, make a port join its reserved VLAN once on setup, never
> leave it, and restore its PVID after another one was eventually used.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Applied.

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

end of thread, other threads:[~2016-02-13 11:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-05 19:07 [PATCH net] net: dsa: mv88e6xxx: do not leave reserved VLANs Vivien Didelot
2016-02-06 18:27 ` Andrew Lunn
2016-02-13 11:08 ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.