Skip to content

Commit

Permalink
net: dsa: sja1105: use 4095 as the private VLAN for untagged traffic
Browse files Browse the repository at this point in the history
commit ed040ab upstream.

One thing became visible when writing the blamed commit, and that was
that STP and PTP frames injected by net/dsa/tag_sja1105.c using the
deferred xmit mechanism are always classified to the pvid of the CPU
port, regardless of whatever VLAN there might be in these packets.

So a decision needed to be taken regarding the mechanism through which
we should ensure that delivery of STP and PTP traffic is possible when
we are in a VLAN awareness mode that involves tag_8021q. This is because
tag_8021q is not concerned with managing the pvid of the CPU port, since
as far as tag_8021q is concerned, no traffic should be sent as untagged
from the CPU port. So we end up not actually having a pvid on the CPU
port if we only listen to tag_8021q, and unless we do something about it.

The decision taken at the time was to keep VLAN 1 in the list of
priv->dsa_8021q_vlans, and make it a pvid of the CPU port. This ensures
that STP and PTP frames can always be sent to the outside world.

However there is a problem. If we do the following while we are in
the best_effort_vlan_filtering=true mode:

ip link add br0 type bridge vlan_filtering 1
ip link set swp2 master br0
bridge vlan del dev swp2 vid 1

Then untagged and pvid-tagged frames should be dropped. But we observe
that they aren't, and this is because of the precaution we took that VID
1 is always installed on all ports.

So clearly VLAN 1 is not good for this purpose. What about VLAN 0?
Well, VLAN 0 is managed by the 8021q module, and that module wants to
ensure that 802.1p tagged frames are always received by a port, and are
always transmitted as VLAN-tagged (with VLAN ID 0). Whereas we want our
STP and PTP frames to be untagged if the stack sent them as untagged -
we don't want the driver to just decide out of the blue that it adds
VID 0 to some packets.

So what to do?

Well, there is one other VLAN that is reserved, and that is 4095:
$ ip link add link swp2 name swp2.4095 type vlan id 4095
Error: 8021q: Invalid VLAN id.
$ bridge vlan add dev swp2 vid 4095
Error: bridge: Vlan id is invalid.

After we made this change, VLAN 1 is indeed forwarded and/or dropped
according to the bridge VLAN table, there are no further alterations
done by the sja1105 driver.

Fixes: ec5ae61 ("net: dsa: sja1105: save/restore VLANs using a delta commit method")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
vladimiroltean authored and gregkh committed Jun 3, 2021
1 parent 6f4b792 commit 4ef506c
Showing 1 changed file with 11 additions and 12 deletions.
23 changes: 11 additions & 12 deletions drivers/net/dsa/sja1105/sja1105_main.c
Expand Up @@ -25,6 +25,8 @@
#include "sja1105_sgmii.h"
#include "sja1105_tas.h"

#define SJA1105_DEFAULT_VLAN (VLAN_N_VID - 1)

static const struct dsa_switch_ops sja1105_switch_ops;

static void sja1105_hw_reset(struct gpio_desc *gpio, unsigned int pulse_len,
Expand Down Expand Up @@ -292,6 +294,13 @@ static int sja1105_init_l2_lookup_params(struct sja1105_private *priv)
return 0;
}

/* Set up a default VLAN for untagged traffic injected from the CPU
* using management routes (e.g. STP, PTP) as opposed to tag_8021q.
* All DT-defined ports are members of this VLAN, and there are no
* restrictions on forwarding (since the CPU selects the destination).
* Frames from this VLAN will always be transmitted as untagged, and
* neither the bridge nor the 8021q module cannot create this VLAN ID.
*/
static int sja1105_init_static_vlan(struct sja1105_private *priv)
{
struct sja1105_table *table;
Expand All @@ -301,17 +310,13 @@ static int sja1105_init_static_vlan(struct sja1105_private *priv)
.vmemb_port = 0,
.vlan_bc = 0,
.tag_port = 0,
.vlanid = 1,
.vlanid = SJA1105_DEFAULT_VLAN,
};
struct dsa_switch *ds = priv->ds;
int port;

table = &priv->static_config.tables[BLK_IDX_VLAN_LOOKUP];

/* The static VLAN table will only contain the initial pvid of 1.
* All other VLANs are to be configured through dynamic entries,
* and kept in the static configuration table as backing memory.
*/
if (table->entry_count) {
kfree(table->entries);
table->entry_count = 0;
Expand All @@ -324,9 +329,6 @@ static int sja1105_init_static_vlan(struct sja1105_private *priv)

table->entry_count = 1;

/* VLAN 1: all DT-defined ports are members; no restrictions on
* forwarding; always transmit as untagged.
*/
for (port = 0; port < ds->num_ports; port++) {
struct sja1105_bridge_vlan *v;

Expand All @@ -337,15 +339,12 @@ static int sja1105_init_static_vlan(struct sja1105_private *priv)
pvid.vlan_bc |= BIT(port);
pvid.tag_port &= ~BIT(port);

/* Let traffic that don't need dsa_8021q (e.g. STP, PTP) be
* transmitted as untagged.
*/
v = kzalloc(sizeof(*v), GFP_KERNEL);
if (!v)
return -ENOMEM;

v->port = port;
v->vid = 1;
v->vid = SJA1105_DEFAULT_VLAN;
v->untagged = true;
if (dsa_is_cpu_port(ds, port))
v->pvid = true;
Expand Down

0 comments on commit 4ef506c

Please sign in to comment.