[PATCH net-next 0/9] net: dsa: mv88e6xxx: implement VLAN filtering

[PATCH net-next 0/9] net: dsa: mv88e6xxx: implement VLAN filtering

[Vivien Didelot]

This patchset fixes hardware bridging for non 802.1Q aware systems.

The mv88e6xxx DSA driver currently depends on CONFIG_VLAN_8021Q and
CONFIG_BRIDGE_VLAN_FILTERING enabled for correct bridging between switch ports.

Patch 1/9 adds support for the VLAN filtering switchdev attribute in DSA.

Patchs 2/9 and 3/9 add helper functions for the following patches.

Patchs 4/9 to 6/9 assign dynamic address databases to VLANs, ports, and
bridge groups (the lowest available FID is cleared and assigned), and thus
restore support for per-port FDB operations.

Patchs 7/9 to 9/9 refine ports isolation and setup 802.1Q on user demand.

With this patchset, ports get correctly bridged and the driver behaves as
expected, with or without 802.1Q support.

With CONFIG_VLAN_8021Q enabled, setting a default PVID to the bridge correctly
propagates the corresponding VLAN, in addition to the hardware bridging:

    # echo 42 > /sys/class/net/<bridge>/bridge/default_pvid

But considering CONFIG_BRIDGE_VLAN_FILTERING enabled, the hardware VLAN
filtering is enabled on all bridge members only when the user requests it:

    # echo 1 > /sys/class/net/<bridge>/bridge/vlan_filtering

Vivien Didelot (9):
  net: dsa: support VLAN filtering switchdev attr
  net: dsa: mv88e6xxx: extract single VLAN retrieval
  net: dsa: mv88e6xxx: extract single FDB dump
  net: dsa: mv88e6xxx: assign dynamic FDB to VLANs
  net: dsa: mv88e6xxx: assign default FDB to ports
  net: dsa: mv88e6xxx: assign dynamic FDB to bridges
  net: dsa: mv88e6xxx: restore VLANTable map control
  net: dsa: mv88e6xxx: remove reserved VLANs
  net: dsa: mv88e6xxx: support VLAN filtering

 drivers/net/dsa/mv88e6171.c |   1 +
 drivers/net/dsa/mv88e6352.c |   1 +
 drivers/net/dsa/mv88e6xxx.c | 441 ++++++++++++++++++++++++++++++++++----------
 drivers/net/dsa/mv88e6xxx.h |   6 +
 include/net/dsa.h           |   2 +
 net/dsa/slave.c             |  21 +++
 6 files changed, 370 insertions(+), 102 deletions(-)

-- 
2.7.1

[PATCH net-next 1/9] net: dsa: support VLAN filtering switchdev attr

[Vivien Didelot]

When a user explicitly requests VLAN filtering with something like:

    # echo 1 > /sys/class/net/<bridge>/bridge/vlan_filtering

Switchdev propagates a SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING port
attribute.

Add support for it in the DSA layer with a new port_vlan_filtering
function to let drivers toggle 802.1Q filtering on user demand.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 include/net/dsa.h |  2 ++
 net/dsa/slave.c   | 21 +++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 3dd5486..26c0a3f 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -305,6 +305,8 @@ struct dsa_switch_driver {
 	/*
 	 * VLAN support
 	 */
+	int	(*port_vlan_filtering)(struct dsa_switch *ds, int port,
+				       bool vlan_filtering);
 	int	(*port_vlan_prepare)(struct dsa_switch *ds, int port,
 				     const struct switchdev_obj_port_vlan *vlan,
 				     struct switchdev_trans *trans);
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index cde2923..27bf03d 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -317,6 +317,24 @@ static int dsa_slave_stp_update(struct net_device *dev, u8 state)
 	return ret;
 }
 
+static int dsa_slave_vlan_filtering(struct net_device *dev,
+				    const struct switchdev_attr *attr,
+				    struct switchdev_trans *trans)
+{
+	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_switch *ds = p->parent;
+
+	/* bridge skips -EOPNOTSUPP, so skip the prepare phase */
+	if (switchdev_trans_ph_prepare(trans))
+		return 0;
+
+	if (ds->drv->port_vlan_filtering)
+		return ds->drv->port_vlan_filtering(ds, p->port,
+						    attr->u.vlan_filtering);
+
+	return 0;
+}
+
 static int dsa_slave_port_attr_set(struct net_device *dev,
 				   const struct switchdev_attr *attr,
 				   struct switchdev_trans *trans)
@@ -333,6 +351,9 @@ static int dsa_slave_port_attr_set(struct net_device *dev,
 			ret = ds->drv->port_stp_update(ds, p->port,
 						       attr->u.stp_state);
 		break;
+	case SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING:
+		ret = dsa_slave_vlan_filtering(dev, attr, trans);
+		break;
 	default:
 		ret = -EOPNOTSUPP;
 		break;
-- 
2.7.1

[PATCH net-next 2/9] net: dsa: mv88e6xxx: extract single VLAN retrieval

[Vivien Didelot]

Rename _mv88e6xxx_vlan_init in _mv88e6xxx_vtu_new, eventually called
from a new _mv88e6xxx_vtu_get function, which abstracts the VTU GetNext
VID-1 trick to retrieve a single entry.

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

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index d98dc63..e9e9922 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -1458,8 +1458,8 @@ loadpurge:
 	return _mv88e6xxx_vtu_cmd(ds, GLOBAL_VTU_OP_STU_LOAD_PURGE);
 }
 
-static int _mv88e6xxx_vlan_init(struct dsa_switch *ds, u16 vid,
-				struct mv88e6xxx_vtu_stu_entry *entry)
+static int _mv88e6xxx_vtu_new(struct dsa_switch *ds, u16 vid,
+			      struct mv88e6xxx_vtu_stu_entry *entry)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	struct mv88e6xxx_vtu_stu_entry vlan = {
@@ -1509,6 +1509,35 @@ static int _mv88e6xxx_vlan_init(struct dsa_switch *ds, u16 vid,
 	return 0;
 }
 
+static int _mv88e6xxx_vtu_get(struct dsa_switch *ds, u16 vid,
+			      struct mv88e6xxx_vtu_stu_entry *entry, bool creat)
+{
+	int err;
+
+	if (!vid)
+		return -EINVAL;
+
+	err = _mv88e6xxx_vtu_vid_write(ds, vid - 1);
+	if (err)
+		return err;
+
+	err = _mv88e6xxx_vtu_getnext(ds, entry);
+	if (err)
+		return err;
+
+	if (entry->vid != vid || !entry->valid) {
+		if (!creat)
+			return -EOPNOTSUPP;
+		/* -ENOENT would've been more appropriate, but switchdev expects
+		 * -EOPNOTSUPP to inform bridge about an eventual software VLAN.
+		 */
+
+		err = _mv88e6xxx_vtu_new(ds, vid, entry);
+	}
+
+	return err;
+}
+
 static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
 					u16 vid_begin, u16 vid_end)
 {
@@ -1593,20 +1622,10 @@ static int _mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port, u16 vid,
 	struct mv88e6xxx_vtu_stu_entry vlan;
 	int err;
 
-	err = _mv88e6xxx_vtu_vid_write(ds, vid - 1);
-	if (err)
-		return err;
-
-	err = _mv88e6xxx_vtu_getnext(ds, &vlan);
+	err = _mv88e6xxx_vtu_get(ds, vid, &vlan, true);
 	if (err)
 		return err;
 
-	if (vlan.vid != vid || !vlan.valid) {
-		err = _mv88e6xxx_vlan_init(ds, vid, &vlan);
-		if (err)
-			return err;
-	}
-
 	vlan.data[port] = untagged ?
 		GLOBAL_VTU_DATA_MEMBER_TAG_UNTAGGED :
 		GLOBAL_VTU_DATA_MEMBER_TAG_TAGGED;
@@ -1647,16 +1666,12 @@ static int _mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port, u16 vid)
 	struct mv88e6xxx_vtu_stu_entry vlan;
 	int i, err;
 
-	err = _mv88e6xxx_vtu_vid_write(ds, vid - 1);
-	if (err)
-		return err;
-
-	err = _mv88e6xxx_vtu_getnext(ds, &vlan);
+	err = _mv88e6xxx_vtu_get(ds, vid, &vlan, false);
 	if (err)
 		return err;
 
-	if (vlan.vid != vid || !vlan.valid ||
-	    vlan.data[port] == GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER)
+	/* Tell switchdev if this VLAN is handled in software */
+	if (vlan.data[port] == GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER)
 		return -EOPNOTSUPP;
 
 	vlan.data[port] = GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER;
-- 
2.7.1

[PATCH net-next 3/9] net: dsa: mv88e6xxx: extract single FDB dump

[Vivien Didelot]

Move out the code which dumps a single FDB to its own function.

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

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index e9e9922..6329516 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -1895,6 +1895,47 @@ static int _mv88e6xxx_atu_getnext(struct dsa_switch *ds, u16 fid,
 	return 0;
 }
 
+static int _mv88e6xxx_port_fdb_dump_one(struct dsa_switch *ds, u16 fid, u16 vid,
+					int port,
+					struct switchdev_obj_port_fdb *fdb,
+					int (*cb)(struct switchdev_obj *obj))
+{
+	struct mv88e6xxx_atu_entry addr = {
+		.mac = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff },
+	};
+	int err;
+
+	err = _mv88e6xxx_atu_mac_write(ds, addr.mac);
+	if (err)
+		return err;
+
+	do {
+		err = _mv88e6xxx_atu_getnext(ds, fid, &addr);
+		if (err)
+			break;
+
+		if (addr.state == GLOBAL_ATU_DATA_STATE_UNUSED)
+			break;
+
+		if (!addr.trunk && addr.portv_trunkid & BIT(port)) {
+			bool is_static = addr.state ==
+				(is_multicast_ether_addr(addr.mac) ?
+				 GLOBAL_ATU_DATA_STATE_MC_STATIC :
+				 GLOBAL_ATU_DATA_STATE_UC_STATIC);
+
+			fdb->vid = vid;
+			ether_addr_copy(fdb->addr, addr.mac);
+			fdb->ndm_state = is_static ? NUD_NOARP : NUD_REACHABLE;
+
+			err = cb(&fdb->obj);
+			if (err)
+				break;
+		}
+	} while (!is_broadcast_ether_addr(addr.mac));
+
+	return err;
+}
+
 int mv88e6xxx_port_fdb_dump(struct dsa_switch *ds, int port,
 			    struct switchdev_obj_port_fdb *fdb,
 			    int (*cb)(struct switchdev_obj *obj))
@@ -1907,51 +1948,23 @@ int mv88e6xxx_port_fdb_dump(struct dsa_switch *ds, int port,
 
 	mutex_lock(&ps->smi_mutex);
 
+	/* Dump VLANs' Filtering Information Databases */
 	err = _mv88e6xxx_vtu_vid_write(ds, vlan.vid);
 	if (err)
 		goto unlock;
 
 	do {
-		struct mv88e6xxx_atu_entry addr = {
-			.mac = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff },
-		};
-
 		err = _mv88e6xxx_vtu_getnext(ds, &vlan);
 		if (err)
-			goto unlock;
+			break;
 
 		if (!vlan.valid)
 			break;
 
-		err = _mv88e6xxx_atu_mac_write(ds, addr.mac);
+		err = _mv88e6xxx_port_fdb_dump_one(ds, vlan.fid, vlan.vid, port,
+						   fdb, cb);
 		if (err)
-			goto unlock;
-
-		do {
-			err = _mv88e6xxx_atu_getnext(ds, vlan.fid, &addr);
-			if (err)
-				goto unlock;
-
-			if (addr.state == GLOBAL_ATU_DATA_STATE_UNUSED)
-				break;
-
-			if (!addr.trunk && addr.portv_trunkid & BIT(port)) {
-				bool is_static = addr.state ==
-					(is_multicast_ether_addr(addr.mac) ?
-					 GLOBAL_ATU_DATA_STATE_MC_STATIC :
-					 GLOBAL_ATU_DATA_STATE_UC_STATIC);
-
-				fdb->vid = vlan.vid;
-				ether_addr_copy(fdb->addr, addr.mac);
-				fdb->ndm_state = is_static ? NUD_NOARP :
-					NUD_REACHABLE;
-
-				err = cb(&fdb->obj);
-				if (err)
-					goto unlock;
-			}
-		} while (!is_broadcast_ether_addr(addr.mac));
-
+			break;
 	} while (vlan.vid < GLOBAL_VTU_VID_MASK);
 
 unlock:
-- 
2.7.1

[PATCH net-next 4/9] net: dsa: mv88e6xxx: assign dynamic FDB to VLANs

[Vivien Didelot]

Add a _mv88e6xxx_fid_new function which gives and flushes the lowest FID
available. Call it when preparing a new VTU entry.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx.c | 56 +++++++++++++++++++++++++++++++++++++--------
 drivers/net/dsa/mv88e6xxx.h |  2 ++
 2 files changed, 49 insertions(+), 9 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 6329516..b4b2f05 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -1458,6 +1458,41 @@ loadpurge:
 	return _mv88e6xxx_vtu_cmd(ds, GLOBAL_VTU_OP_STU_LOAD_PURGE);
 }
 
+static int _mv88e6xxx_fid_new(struct dsa_switch *ds, u16 *fid)
+{
+	DECLARE_BITMAP(fid_bitmap, MV88E6XXX_N_FID);
+	struct mv88e6xxx_vtu_stu_entry vlan;
+	int err;
+
+	bitmap_zero(fid_bitmap, MV88E6XXX_N_FID);
+
+	/* Set every FID bit used by the VLAN entries */
+	err = _mv88e6xxx_vtu_vid_write(ds, GLOBAL_VTU_VID_MASK);
+	if (err)
+		return err;
+
+	do {
+		err = _mv88e6xxx_vtu_getnext(ds, &vlan);
+		if (err)
+			return err;
+
+		if (!vlan.valid)
+			break;
+
+		set_bit(vlan.fid, fid_bitmap);
+	} while (vlan.vid < GLOBAL_VTU_VID_MASK);
+
+	/* The reset value 0x000 is used to indicate that multiple address
+	 * databases are not needed. Return the next positive available.
+	 */
+	*fid = find_next_zero_bit(fid_bitmap, MV88E6XXX_N_FID, 1);
+	if (unlikely(*fid == MV88E6XXX_N_FID))
+		return -ENOSPC;
+
+	/* Clear the database */
+	return _mv88e6xxx_atu_flush(ds, *fid, true);
+}
+
 static int _mv88e6xxx_vtu_new(struct dsa_switch *ds, u16 vid,
 			      struct mv88e6xxx_vtu_stu_entry *entry)
 {
@@ -1465,9 +1500,12 @@ static int _mv88e6xxx_vtu_new(struct dsa_switch *ds, u16 vid,
 	struct mv88e6xxx_vtu_stu_entry vlan = {
 		.valid = true,
 		.vid = vid,
-		.fid = vid, /* We use one FID per VLAN */
 	};
-	int i;
+	int i, err;
+
+	err = _mv88e6xxx_fid_new(ds, &vlan.fid);
+	if (err)
+		return err;
 
 	/* exclude all ports except the CPU and DSA ports */
 	for (i = 0; i < ps->num_ports; ++i)
@@ -1478,7 +1516,6 @@ static int _mv88e6xxx_vtu_new(struct dsa_switch *ds, u16 vid,
 	if (mv88e6xxx_6097_family(ds) || mv88e6xxx_6165_family(ds) ||
 	    mv88e6xxx_6351_family(ds) || mv88e6xxx_6352_family(ds)) {
 		struct mv88e6xxx_vtu_stu_entry vstp;
-		int err;
 
 		/* Adding a VTU entry requires a valid STU entry. As VSTP is not
 		 * implemented, only one STU entry is needed to cover all VTU
@@ -1498,11 +1535,6 @@ static int _mv88e6xxx_vtu_new(struct dsa_switch *ds, u16 vid,
 			if (err)
 				return err;
 		}
-
-		/* Clear all MAC addresses from the new database */
-		err = _mv88e6xxx_atu_flush(ds, vlan.fid, true);
-		if (err)
-			return err;
 	}
 
 	*entry = vlan;
@@ -1789,8 +1821,14 @@ static int _mv88e6xxx_port_fdb_load(struct dsa_switch *ds, int port,
 				    u8 state)
 {
 	struct mv88e6xxx_atu_entry entry = { 0 };
+	struct mv88e6xxx_vtu_stu_entry vlan;
+	int err;
+
+	err = _mv88e6xxx_vtu_get(ds, vid, &vlan, false);
+	if (err)
+		return err;
 
-	entry.fid = vid; /* We use one FID per VLAN */
+	entry.fid = vlan.fid;
 	entry.state = state;
 	ether_addr_copy(entry.mac, addr);
 	if (state != GLOBAL_ATU_DATA_STATE_UNUSED) {
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 6a30bda..9df331e 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -355,6 +355,8 @@
 #define GLOBAL2_QOS_WEIGHT	0x1c
 #define GLOBAL2_MISC		0x1d
 
+#define MV88E6XXX_N_FID		4096
+
 struct mv88e6xxx_switch_id {
 	u16 id;
 	char *name;
-- 
2.7.1

[PATCH net-next 5/9] net: dsa: mv88e6xxx: assign default FDB to ports

[Vivien Didelot]

Restore per-port FDB. Assign them on setup, allow adding and deleting
addresses into them, and dump them.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx.c | 96 +++++++++++++++++++++++++++++++++++++++++----
 drivers/net/dsa/mv88e6xxx.h |  2 +
 2 files changed, 91 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index b4b2f05..0f06488 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -1458,14 +1458,82 @@ loadpurge:
 	return _mv88e6xxx_vtu_cmd(ds, GLOBAL_VTU_OP_STU_LOAD_PURGE);
 }
 
+static int _mv88e6xxx_port_fid(struct dsa_switch *ds, int port, u16 *new,
+			       u16 *old)
+{
+	u16 fid;
+	int ret;
+
+	/* Port's default FID bits 3:0 are located in reg 0x06, offset 12 */
+	ret = _mv88e6xxx_reg_read(ds, REG_PORT(port), PORT_BASE_VLAN);
+	if (ret < 0)
+		return ret;
+
+	fid = (ret & PORT_BASE_VLAN_FID_3_0_MASK) >> 12;
+
+	if (new) {
+		ret &= ~PORT_BASE_VLAN_FID_3_0_MASK;
+		ret |= (*new << 12) & PORT_BASE_VLAN_FID_3_0_MASK;
+
+		ret = _mv88e6xxx_reg_write(ds, REG_PORT(port), PORT_BASE_VLAN,
+					   ret);
+		if (ret < 0)
+			return ret;
+	}
+
+	/* Port's default FID bits 11:4 are located in reg 0x05, offset 0 */
+	ret = _mv88e6xxx_reg_read(ds, REG_PORT(port), PORT_CONTROL_1);
+	if (ret < 0)
+		return ret;
+
+	fid |= (ret & PORT_CONTROL_1_FID_11_4_MASK) << 4;
+
+	if (new) {
+		ret &= ~PORT_CONTROL_1_FID_11_4_MASK;
+		ret |= (*new >> 4) & PORT_CONTROL_1_FID_11_4_MASK;
+
+		ret = _mv88e6xxx_reg_write(ds, REG_PORT(port), PORT_CONTROL_1,
+					   ret);
+		if (ret < 0)
+			return ret;
+
+		netdev_dbg(ds->ports[port], "FID %d (was %d)\n", *new, fid);
+	}
+
+	if (old)
+		*old = fid;
+
+	return 0;
+}
+
+static int _mv88e6xxx_port_fid_get(struct dsa_switch *ds, int port, u16 *fid)
+{
+	return _mv88e6xxx_port_fid(ds, port, NULL, fid);
+}
+
+static int _mv88e6xxx_port_fid_set(struct dsa_switch *ds, int port, u16 fid)
+{
+	return _mv88e6xxx_port_fid(ds, port, &fid, NULL);
+}
+
 static int _mv88e6xxx_fid_new(struct dsa_switch *ds, u16 *fid)
 {
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	DECLARE_BITMAP(fid_bitmap, MV88E6XXX_N_FID);
 	struct mv88e6xxx_vtu_stu_entry vlan;
-	int err;
+	int i, err;
 
 	bitmap_zero(fid_bitmap, MV88E6XXX_N_FID);
 
+	/* Set every FID bit used by the (un)bridged ports */
+	for (i = 0; i < ps->num_ports; ++i) {
+		err = _mv88e6xxx_port_fid_get(ds, i, fid);
+		if (err)
+			return err;
+
+		set_bit(*fid, fid_bitmap);
+	}
+
 	/* Set every FID bit used by the VLAN entries */
 	err = _mv88e6xxx_vtu_vid_write(ds, GLOBAL_VTU_VID_MASK);
 	if (err)
@@ -1824,7 +1892,11 @@ static int _mv88e6xxx_port_fdb_load(struct dsa_switch *ds, int port,
 	struct mv88e6xxx_vtu_stu_entry vlan;
 	int err;
 
-	err = _mv88e6xxx_vtu_get(ds, vid, &vlan, false);
+	/* Null VLAN ID corresponds to the port private database */
+	if (vid == 0)
+		err = _mv88e6xxx_port_fid_get(ds, port, &vlan.fid);
+	else
+		err = _mv88e6xxx_vtu_get(ds, vid, &vlan, false);
 	if (err)
 		return err;
 
@@ -1843,10 +1915,6 @@ int mv88e6xxx_port_fdb_prepare(struct dsa_switch *ds, int port,
 			       const struct switchdev_obj_port_fdb *fdb,
 			       struct switchdev_trans *trans)
 {
-	/* We don't use per-port FDB */
-	if (fdb->vid == 0)
-		return -EOPNOTSUPP;
-
 	/* We don't need any dynamic resource from the kernel (yet),
 	 * so skip the prepare phase.
 	 */
@@ -1982,10 +2050,20 @@ int mv88e6xxx_port_fdb_dump(struct dsa_switch *ds, int port,
 	struct mv88e6xxx_vtu_stu_entry vlan = {
 		.vid = GLOBAL_VTU_VID_MASK, /* all ones */
 	};
+	u16 fid;
 	int err;
 
 	mutex_lock(&ps->smi_mutex);
 
+	/* Dump port's default Filtering Information Database (VLAN ID 0) */
+	err = _mv88e6xxx_port_fid_get(ds, port, &fid);
+	if (err)
+		goto unlock;
+
+	err = _mv88e6xxx_port_fdb_dump_one(ds, fid, 0, port, fdb, cb);
+	if (err)
+		goto unlock;
+
 	/* Dump VLANs' Filtering Information Databases */
 	err = _mv88e6xxx_vtu_vid_write(ds, vlan.vid);
 	if (err)
@@ -2286,9 +2364,13 @@ static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port)
 	if (ret)
 		goto abort;
 
-	/* Port based VLAN map: do not give each port its own address
+	/* Port based VLAN map: give each port its own address
 	 * database, and allow every port to egress frames on all other ports.
 	 */
+	ret = _mv88e6xxx_port_fid_set(ds, port, port + 1);
+	if (ret)
+		goto abort;
+
 	reg = BIT(ps->num_ports) - 1; /* all ports */
 	reg &= ~BIT(port); /* except itself */
 	ret = _mv88e6xxx_port_vlan_map_set(ds, port, reg);
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 9df331e..85a4166 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -133,7 +133,9 @@
 #define PORT_CONTROL_STATE_LEARNING	0x02
 #define PORT_CONTROL_STATE_FORWARDING	0x03
 #define PORT_CONTROL_1		0x05
+#define PORT_CONTROL_1_FID_11_4_MASK	(0xff << 0)
 #define PORT_BASE_VLAN		0x06
+#define PORT_BASE_VLAN_FID_3_0_MASK	(0xf << 12)
 #define PORT_DEFAULT_VLAN	0x07
 #define PORT_DEFAULT_VLAN_MASK	0xfff
 #define PORT_CONTROL_2		0x08
-- 
2.7.1

[PATCH net-next 6/9] net: dsa: mv88e6xxx: assign dynamic FDB to bridges

[Vivien Didelot]

Give a new bridge a fresh FDB, assign it to its members, and restore a
fresh FDB to a port leaving a bridge.

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

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 0f06488..0f16911 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -2093,19 +2093,56 @@ int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
 			       struct net_device *bridge)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	u16 fid;
+	int i, err;
+
+	mutex_lock(&ps->smi_mutex);
+
+	/* Get or create the bridge FID and assign it to the port */
+	for (i = 0; i < ps->num_ports; ++i)
+		if (ps->ports[i].bridge_dev == bridge)
+			break;
+
+	if (i < ps->num_ports)
+		err = _mv88e6xxx_port_fid_get(ds, i, &fid);
+	else
+		err = _mv88e6xxx_fid_new(ds, &fid);
+	if (err)
+		goto unlock;
+
+	err = _mv88e6xxx_port_fid_set(ds, port, fid);
+	if (err)
+		goto unlock;
 
 	ps->ports[port].bridge_dev = bridge;
+unlock:
+	mutex_unlock(&ps->smi_mutex);
 
-	return 0;
+	return err;
 }
 
 int mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	u16 fid;
+	int err;
+
+	mutex_lock(&ps->smi_mutex);
+
+	/* Give the port a fresh Filtering Information Database */
+	err = _mv88e6xxx_fid_new(ds, &fid);
+	if (err)
+		goto unlock;
+
+	err = _mv88e6xxx_port_fid_set(ds, port, fid);
+	if (err)
+		goto unlock;
 
 	ps->ports[port].bridge_dev = NULL;
+unlock:
+	mutex_unlock(&ps->smi_mutex);
 
-	return 0;
+	return err;
 }
 
 static int mv88e6xxx_setup_port_default_vlan(struct dsa_switch *ds, int port)
-- 
2.7.1

[PATCH net-next 7/9] net: dsa: mv88e6xxx: restore VLANTable map control

[Vivien Didelot]

The In Chip Port Based VLAN Table contains bits used to restrict which
output ports this input port can send frames to.

With the VLAN filtering enabled, these tables work in conjunction with
the VLAN Table Unit to allow egressing frames.

In order to remove the current dependency to BRIDGE_VLAN_FILTERING for
basic hardware bridging to work, it is necessary to restore a fine
control of each port's VLANTable, on setup and when a port joins or
leaves a bridge.

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

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 0f16911..7f3036b 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -1087,12 +1087,32 @@ abort:
 	return ret;
 }
 
-static int _mv88e6xxx_port_vlan_map_set(struct dsa_switch *ds, int port,
-					u16 output_ports)
+static int _mv88e6xxx_port_based_vlan_map(struct dsa_switch *ds, int port)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	struct net_device *bridge = ps->ports[port].bridge_dev;
 	const u16 mask = (1 << ps->num_ports) - 1;
+	u16 output_ports = 0;
 	int reg;
+	int i;
+
+	/* allow CPU port or DSA link(s) to send frames to every port */
+	if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)) {
+		output_ports = mask;
+	} else {
+		for (i = 0; i < ps->num_ports; ++i) {
+			/* allow sending frames to every group member */
+			if (bridge && ps->ports[i].bridge_dev == bridge)
+				output_ports |= BIT(i);
+
+			/* allow sending frames to CPU port and DSA link(s) */
+			if (dsa_is_cpu_port(ds, i) || dsa_is_dsa_port(ds, i))
+				output_ports |= BIT(i);
+		}
+	}
+
+	/* prevent frames from going back out of the port they came in on */
+	output_ports &= ~BIT(port);
 
 	reg = _mv88e6xxx_reg_read(ds, REG_PORT(port), PORT_BASE_VLAN);
 	if (reg < 0)
@@ -2114,7 +2134,17 @@ int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
 	if (err)
 		goto unlock;
 
+	/* Assign the bridge and remap each port's VLANTable */
 	ps->ports[port].bridge_dev = bridge;
+
+	for (i = 0; i < ps->num_ports; ++i) {
+		if (ps->ports[i].bridge_dev == bridge) {
+			err = _mv88e6xxx_port_based_vlan_map(ds, i);
+			if (err)
+				break;
+		}
+	}
+
 unlock:
 	mutex_unlock(&ps->smi_mutex);
 
@@ -2124,8 +2154,9 @@ unlock:
 int mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	struct net_device *bridge = ps->ports[port].bridge_dev;
 	u16 fid;
-	int err;
+	int i, err;
 
 	mutex_lock(&ps->smi_mutex);
 
@@ -2138,7 +2169,17 @@ int mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port)
 	if (err)
 		goto unlock;
 
+	/* Unassign the bridge and remap each port's VLANTable */
 	ps->ports[port].bridge_dev = NULL;
+
+	for (i = 0; i < ps->num_ports; ++i) {
+		if (i == port || ps->ports[i].bridge_dev == bridge) {
+			err = _mv88e6xxx_port_based_vlan_map(ds, i);
+			if (err)
+				break;
+		}
+	}
+
 unlock:
 	mutex_unlock(&ps->smi_mutex);
 
@@ -2402,15 +2443,14 @@ static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port)
 		goto abort;
 
 	/* Port based VLAN map: give each port its own address
-	 * database, and allow every port to egress frames on all other ports.
+	 * database, and allow bidirectional communication between the
+	 * CPU and DSA port(s), and the other ports.
 	 */
 	ret = _mv88e6xxx_port_fid_set(ds, port, port + 1);
 	if (ret)
 		goto abort;
 
-	reg = BIT(ps->num_ports) - 1; /* all ports */
-	reg &= ~BIT(port); /* except itself */
-	ret = _mv88e6xxx_port_vlan_map_set(ds, port, reg);
+	ret = _mv88e6xxx_port_based_vlan_map(ds, port);
 	if (ret)
 		goto abort;
 
-- 
2.7.1

[PATCH net-next 8/9] net: dsa: mv88e6xxx: remove reserved VLANs

[Vivien Didelot]

Now that ports isolation is correctly configured when joining or leaving
a bridge, there is no need to rely on reserved VLANs to isolate
unbridged ports anymore. Thus remove them, and disable 802.1Q on setup.

This restores the expected behavior of hardware bridging for systems
without 802.1Q or VLAN filtering enabled.

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

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 7f3036b..27a19dc 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -1718,10 +1718,6 @@ int mv88e6xxx_port_vlan_prepare(struct dsa_switch *ds, int port,
 {
 	int err;
 
-	/* We reserve a few VLANs to isolate unbridged ports */
-	if (vlan->vid_end >= 4000)
-		return -EOPNOTSUPP;
-
 	/* If the requested port doesn't belong to the same bridge as the VLAN
 	 * members, do not support it (yet) and fallback to software VLAN.
 	 */
@@ -1819,7 +1815,6 @@ 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;
 
@@ -1835,8 +1830,7 @@ int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port,
 			goto unlock;
 
 		if (vid == pvid) {
-			/* restore reserved VLAN ID */
-			err = _mv88e6xxx_port_pvid_set(ds, port, defpvid);
+			err = _mv88e6xxx_port_pvid_set(ds, port, 0);
 			if (err)
 				goto unlock;
 		}
@@ -2186,20 +2180,6 @@ unlock:
 	return err;
 }
 
-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;
-
-	mutex_lock(&ps->smi_mutex);
-	err = _mv88e6xxx_port_vlan_add(ds, port, pvid, true);
-	if (!err)
-		err = _mv88e6xxx_port_pvid_set(ds, port, pvid);
-	mutex_unlock(&ps->smi_mutex);
-	return err;
-}
-
 static void mv88e6xxx_bridge_work(struct work_struct *work)
 {
 	struct mv88e6xxx_priv_state *ps;
@@ -2320,7 +2300,7 @@ static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port)
 	}
 
 	/* Port Control 2: don't force a good FCS, set the maximum frame size to
-	 * 10240 bytes, enable secure 802.1q tags, don't discard tagged or
+	 * 10240 bytes, disable 802.1q tags checking, don't discard tagged or
 	 * untagged frames on this port, do a destination address lookup on all
 	 * received packets as usual, disable ARP mirroring and don't send a
 	 * copy of all transmitted/received frames on this port to the CPU.
@@ -2345,7 +2325,7 @@ static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port)
 			reg |= PORT_CONTROL_2_FORWARD_UNKNOWN;
 	}
 
-	reg |= PORT_CONTROL_2_8021Q_SECURE;
+	reg |= PORT_CONTROL_2_8021Q_DISABLED;
 
 	if (reg) {
 		ret = _mv88e6xxx_reg_write(ds, REG_PORT(port),
@@ -2474,13 +2454,6 @@ int mv88e6xxx_setup_ports(struct dsa_switch *ds)
 		ret = mv88e6xxx_setup_port(ds, i);
 		if (ret < 0)
 			return ret;
-
-		if (dsa_is_cpu_port(ds, i) || dsa_is_dsa_port(ds, i))
-			continue;
-
-		ret = mv88e6xxx_setup_port_default_vlan(ds, i);
-		if (ret < 0)
-			return ret;
 	}
 	return 0;
 }
-- 
2.7.1

[PATCH net-next 9/9] net: dsa: mv88e6xxx: support VLAN filtering

[Vivien Didelot]

Implement port_vlan_filtering in the driver to toggle the related port
802.1Q mode between DISABLED and SECURE, on user request.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6171.c |  1 +
 drivers/net/dsa/mv88e6352.c |  1 +
 drivers/net/dsa/mv88e6xxx.c | 39 +++++++++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx.h |  2 ++
 4 files changed, 43 insertions(+)

diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index dd1ebaf..d72ccbd 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -106,6 +106,7 @@ struct dsa_switch_driver mv88e6171_switch_driver = {
 	.port_join_bridge	= mv88e6xxx_port_bridge_join,
 	.port_leave_bridge	= mv88e6xxx_port_bridge_leave,
 	.port_stp_update        = mv88e6xxx_port_stp_update,
+	.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,
diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
index bbca36a..a41fa50 100644
--- a/drivers/net/dsa/mv88e6352.c
+++ b/drivers/net/dsa/mv88e6352.c
@@ -327,6 +327,7 @@ struct dsa_switch_driver mv88e6352_switch_driver = {
 	.port_join_bridge	= mv88e6xxx_port_bridge_join,
 	.port_leave_bridge	= mv88e6xxx_port_bridge_leave,
 	.port_stp_update	= mv88e6xxx_port_stp_update,
+	.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,
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 27a19dc..d11c9d5 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -1712,6 +1712,45 @@ unlock:
 	return err;
 }
 
+static const char * const mv88e6xxx_port_8021q_mode_names[] = {
+	[PORT_CONTROL_2_8021Q_DISABLED] = "Disabled",
+	[PORT_CONTROL_2_8021Q_FALLBACK] = "Fallback",
+	[PORT_CONTROL_2_8021Q_CHECK] = "Check",
+	[PORT_CONTROL_2_8021Q_SECURE] = "Secure",
+};
+
+int mv88e6xxx_port_vlan_filtering(struct dsa_switch *ds, int port,
+				  bool vlan_filtering)
+{
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	u16 old, new = vlan_filtering ? PORT_CONTROL_2_8021Q_SECURE :
+		PORT_CONTROL_2_8021Q_DISABLED;
+	int ret;
+
+	mutex_lock(&ps->smi_mutex);
+
+	ret = _mv88e6xxx_reg_read(ds, REG_PORT(port), PORT_CONTROL_2);
+	if (ret < 0)
+		goto unlock;
+
+	old = ret & PORT_CONTROL_2_8021Q_MASK;
+
+	ret &= ~PORT_CONTROL_2_8021Q_MASK;
+	ret |= new & PORT_CONTROL_2_8021Q_MASK;
+
+	ret = _mv88e6xxx_reg_write(ds, REG_PORT(port), PORT_CONTROL_2, ret);
+	if (ret < 0)
+		goto unlock;
+
+	netdev_dbg(ds->ports[port], "802.1Q Mode: %s (was %s)\n",
+		   mv88e6xxx_port_8021q_mode_names[new],
+		   mv88e6xxx_port_8021q_mode_names[old]);
+unlock:
+	mutex_unlock(&ps->smi_mutex);
+
+	return ret;
+}
+
 int mv88e6xxx_port_vlan_prepare(struct dsa_switch *ds, int port,
 				const struct switchdev_obj_port_vlan *vlan,
 				struct switchdev_trans *trans)
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 85a4166..d7b088d 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -490,6 +490,8 @@ int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
 			       struct net_device *bridge);
 int mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port);
 int mv88e6xxx_port_stp_update(struct dsa_switch *ds, int port, u8 state);
+int mv88e6xxx_port_vlan_filtering(struct dsa_switch *ds, int port,
+				  bool vlan_filtering);
 int mv88e6xxx_port_vlan_prepare(struct dsa_switch *ds, int port,
 				const struct switchdev_obj_port_vlan *vlan,
 				struct switchdev_trans *trans);
-- 
2.7.1

Re: [PATCH net-next 7/9] net: dsa: mv88e6xxx: restore VLANTable map control

[Kevin Smith]

Hi Vivien,

On 02/26/2016 12:16 PM, Vivien Didelot wrote:
> +	/* allow CPU port or DSA link(s) to send frames to every port */
> +	if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)) {
> +		output_ports = mask;
> +	} else {
Is this always correct?  Are there situations where a CPU or neighboring 
switch should not be allowed to access another port? (e.g. Figure 6 or 7 
in the 88E6352 functional specification).

Thanks,
Kevin

Re: [PATCH net-next 7/9] net: dsa: mv88e6xxx: restore VLANTable map control

[Andrew Lunn]

On Fri, Feb 26, 2016 at 08:45:28PM +0000, Kevin Smith wrote:
> Hi Vivien,
> 
> On 02/26/2016 12:16 PM, Vivien Didelot wrote:
> > +	/* allow CPU port or DSA link(s) to send frames to every port */
> > +	if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)) {
> > +		output_ports = mask;
> > +	} else {

> Is this always correct?  Are there situations where a CPU or neighboring 
> switch should not be allowed to access another port? (e.g. Figure 6 or 7 
> in the 88E6352 functional specification).

What do these figures show?

The CPU port needs to be able to send to each external port. The whole
DSA concept is that Linux has a netdev per external port, and can send
frames using the netdev out a specific port. Such frames have a DSA
header indicating which port they are destined to.  When you have a
multi chip setup, the frame needs to traverse DSA ports.

      Andrew

Re: [PATCH net-next 7/9] net: dsa: mv88e6xxx: restore VLANTable map control

[Vivien Didelot]

Hi Kevin, Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

> On Fri, Feb 26, 2016 at 08:45:28PM +0000, Kevin Smith wrote:
>> Hi Vivien,
>> 
>> On 02/26/2016 12:16 PM, Vivien Didelot wrote:
>> > +	/* allow CPU port or DSA link(s) to send frames to every port */
>> > +	if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)) {
>> > +		output_ports = mask;
>> > +	} else {
>
>> Is this always correct?  Are there situations where a CPU or neighboring 
>> switch should not be allowed to access another port? (e.g. Figure 6 or 7 
>> in the 88E6352 functional specification).

Given Linux expectations (described below by Andrew) I'd say yes, this
is always correct. But I'd be curious to know if someone has counter
examples for this.

> What do these figures show?

The figure shows the following VLANTable config:

Port  0  1  2  3  4  5  6
  0   -  *  *  *  -  -  *
  1   *  -  *  *  -  -  *
  2   *  *  -  *  -  -  *
  3   *  *  *  -  -  -  *
  4   -  -  -  -  -  *  -
  5   -  -  -  -  *  -  -
  6   *  *  *  *  -  -  -

There is two independant groups: 0, 1, 2, 3, 6 (LAN, 6 is CPU/Router),
and 4, 5 (4 is WAN and 5 is CPU/Router):

Port #   Port Type     VLANTable Setting
0        LAN           0x4E
1        LAN           0x4D
2        LAN           0x4B
3        LAN           0x47
4        WAN           0x20
5        CPU           0x10
6        CPU           0x0F

> The CPU port needs to be able to send to each external port. The whole
> DSA concept is that Linux has a netdev per external port, and can send
> frames using the netdev out a specific port. Such frames have a DSA
> header indicating which port they are destined to.  When you have a
> multi chip setup, the frame needs to traverse DSA ports.

This current patch produces to following setup at setup:

Port  0  1  2  3  4  5  6
  0   -  -  -  -  -  *  *
  1   -  -  -  -  -  *  *
  2   -  -  -  -  -  *  *
  3   -  -  -  -  -  *  *
  4   -  -  -  -  -  *  *
  5   *  *  *  *  *  -  *
  6   *  *  *  *  *  *  -

Here, 5 is the CPU port and 6 is a DSA port.

After joining ports 0, 1, 2 in the same bridge, we end up with:

Port  0  1  2  3  4  5  6
  0   -  *  *  -  -  *  *
  1   *  -  *  -  -  *  *
  2   *  *  -  -  -  *  *
  3   -  -  -  -  -  *  *
  4   -  -  -  -  -  *  *
  5   *  *  *  *  *  -  *
  6   *  *  *  *  *  *  -

Thanks,
-v

Re: [PATCH net-next 7/9] net: dsa: mv88e6xxx: restore VLANTable map control

[Andrew Lunn]

On Fri, Feb 26, 2016 at 04:37:39PM -0500, Vivien Didelot wrote:
> Hi Kevin, Andrew,
> 
> Andrew Lunn <andrew@lunn.ch> writes:
> 
> > On Fri, Feb 26, 2016 at 08:45:28PM +0000, Kevin Smith wrote:
> >> Hi Vivien,
> >> 
> >> On 02/26/2016 12:16 PM, Vivien Didelot wrote:
> >> > +	/* allow CPU port or DSA link(s) to send frames to every port */
> >> > +	if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)) {
> >> > +		output_ports = mask;
> >> > +	} else {
> >
> >> Is this always correct?  Are there situations where a CPU or neighboring 
> >> switch should not be allowed to access another port? (e.g. Figure 6 or 7 
> >> in the 88E6352 functional specification).
> 
> Given Linux expectations (described below by Andrew) I'd say yes, this
> is always correct. But I'd be curious to know if someone has counter
> examples for this.
> 
> > What do these figures show?
> 
> The figure shows the following VLANTable config:
> 
> Port  0  1  2  3  4  5  6
>   0   -  *  *  *  -  -  *
>   1   *  -  *  *  -  -  *
>   2   *  *  -  *  -  -  *
>   3   *  *  *  -  -  -  *
>   4   -  -  -  -  -  *  -
>   5   -  -  -  -  *  -  -
>   6   *  *  *  *  -  -  -
> 
> There is two independant groups: 0, 1, 2, 3, 6 (LAN, 6 is CPU/Router),
> and 4, 5 (4 is WAN and 5 is CPU/Router):

Ah, two CPU interfaces. We don't support that yet.  I do have patches,
but i took a different approach. They just load balance, by some
definition of 'load balance' between the two CPU ports.

	   Andrew

Re: [PATCH net-next 7/9] net: dsa: mv88e6xxx: restore VLANTable map control

[Kevin Smith]

Hi Vivien, Andrew,

On 02/26/2016 03:37 PM, Vivien Didelot wrote:
> Here, 5 is the CPU port and 6 is a DSA port.
>
> After joining ports 0, 1, 2 in the same bridge, we end up with:
>
> Port  0  1  2  3  4  5  6
>    0   -  *  *  -  -  *  *
>    1   *  -  *  -  -  *  *
>    2   *  *  -  -  -  *  *
>    3   -  -  -  -  -  *  *
>    4   -  -  -  -  -  *  *
>    5   *  *  *  *  *  -  *
>    6   *  *  *  *  *  *  -
The case I am concerned about is if the switch connected over DSA in 
this example has a WAN port on it, which can legitimately route to the 
CPU on port 5 but should not route to the LAN ports 0, 1, and 2.  Does 
this VLAN allow direct communication between the WAN and LAN?  Or is 
this prevented by DSA or some other mechanism?

Thanks,
Kevin

Re: [PATCH net-next 7/9] net: dsa: mv88e6xxx: restore VLANTable map control

[Andrew Lunn]

On Fri, Feb 26, 2016 at 10:12:28PM +0000, Kevin Smith wrote:
> Hi Vivien, Andrew,
> 
> On 02/26/2016 03:37 PM, Vivien Didelot wrote:
> > Here, 5 is the CPU port and 6 is a DSA port.
> >
> > After joining ports 0, 1, 2 in the same bridge, we end up with:
> >
> > Port  0  1  2  3  4  5  6
> >    0   -  *  *  -  -  *  *
> >    1   *  -  *  -  -  *  *
> >    2   *  *  -  -  -  *  *
> >    3   -  -  -  -  -  *  *
> >    4   -  -  -  -  -  *  *
> >    5   *  *  *  *  *  -  *
> >    6   *  *  *  *  *  *  -
> The case I am concerned about is if the switch connected over DSA in 
> this example has a WAN port on it, which can legitimately route to the 
> CPU on port 5 but should not route to the LAN ports 0, 1, and 2.  Does 
> this VLAN allow direct communication between the WAN and LAN?  Or is 
> this prevented by DSA or some other mechanism?

A typical WIFI access point with a connection to a cable modem.

So in linux you have interfaces like

lan0, lan1, lan2, lan3, wan0

DSA provides you these interface. And by default they are all
separated. There is no path between them. You can consider them as
being separate physical ethernet cards, just like all other interfaces
in linux.

What you would typically do is:

brctl addbr br0
brctl addif br0 lan0
brctl addif br0 lan1
brctl addif br0 lan2
brctl addif br0 lan3

to create a bridge between the lan ports. The linux kernel will then
push this bridge configuration down into the hardware, so the switch
can forward frames between these ports.

The wan port is not part of the bridge, so there is no L2 path to the
WAN port. You need to do IP routing on the CPU.

Linux takes the stance that switch ports interfaces should act just
like any other linux interface and you configure them in the normal
linux way.

    Andrew

Re: [PATCH net-next 7/9] net: dsa: mv88e6xxx: restore VLANTable map control

[Kevin Smith]

Hi Andrew,

On 02/26/2016 04:35 PM, Andrew Lunn wrote:
> On Fri, Feb 26, 2016 at 10:12:28PM +0000, Kevin Smith wrote:
>> Hi Vivien, Andrew,
>>
>> On 02/26/2016 03:37 PM, Vivien Didelot wrote:
>>> Here, 5 is the CPU port and 6 is a DSA port.
>>>
>>> After joining ports 0, 1, 2 in the same bridge, we end up with:
>>>
>>> Port  0  1  2  3  4  5  6
>>>     0   -  *  *  -  -  *  *
>>>     1   *  -  *  -  -  *  *
>>>     2   *  *  -  -  -  *  *
>>>     3   -  -  -  -  -  *  *
>>>     4   -  -  -  -  -  *  *
>>>     5   *  *  *  *  *  -  *
>>>     6   *  *  *  *  *  *  -
>> The case I am concerned about is if the switch connected over DSA in
>> this example has a WAN port on it, which can legitimately route to the
>> CPU on port 5 but should not route to the LAN ports 0, 1, and 2.  Does
>> this VLAN allow direct communication between the WAN and LAN?  Or is
>> this prevented by DSA or some other mechanism?
> A typical WIFI access point with a connection to a cable modem.
>
> So in linux you have interfaces like
>
> lan0, lan1, lan2, lan3, wan0
>
> DSA provides you these interface. And by default they are all
> separated. There is no path between them. You can consider them as
> being separate physical ethernet cards, just like all other interfaces
> in linux.
>
> What you would typically do is:
>
> brctl addbr br0
> brctl addif br0 lan0
> brctl addif br0 lan1
> brctl addif br0 lan2
> brctl addif br0 lan3
>
> to create a bridge between the lan ports. The linux kernel will then
> push this bridge configuration down into the hardware, so the switch
> can forward frames between these ports.
>
> The wan port is not part of the bridge, so there is no L2 path to the
> WAN port. You need to do IP routing on the CPU.
>
> Linux takes the stance that switch ports interfaces should act just
> like any other linux interface and you configure them in the normal
> linux way.
>
>      Andrew

Thanks for the explanation.  I am a bit befuddled by the combination of 
all the possible configurations of the switch and how they interact with 
Linux.  :)  I think I understand what is happening now.

Kevin

Re: [PATCH net-next 7/9] net: dsa: mv88e6xxx: restore VLANTable map control

[Andrew Lunn]

On Fri, Feb 26, 2016 at 10:47:38PM +0000, Kevin Smith wrote:
> Hi Andrew,
> 
> On 02/26/2016 04:35 PM, Andrew Lunn wrote:
> > On Fri, Feb 26, 2016 at 10:12:28PM +0000, Kevin Smith wrote:
> >> Hi Vivien, Andrew,
> >>
> >> On 02/26/2016 03:37 PM, Vivien Didelot wrote:
> >>> Here, 5 is the CPU port and 6 is a DSA port.
> >>>
> >>> After joining ports 0, 1, 2 in the same bridge, we end up with:
> >>>
> >>> Port  0  1  2  3  4  5  6
> >>>     0   -  *  *  -  -  *  *
> >>>     1   *  -  *  -  -  *  *
> >>>     2   *  *  -  -  -  *  *
> >>>     3   -  -  -  -  -  *  *
> >>>     4   -  -  -  -  -  *  *
> >>>     5   *  *  *  *  *  -  *
> >>>     6   *  *  *  *  *  *  -
> >> The case I am concerned about is if the switch connected over DSA in
> >> this example has a WAN port on it, which can legitimately route to the
> >> CPU on port 5 but should not route to the LAN ports 0, 1, and 2.  Does
> >> this VLAN allow direct communication between the WAN and LAN?  Or is
> >> this prevented by DSA or some other mechanism?
> > A typical WIFI access point with a connection to a cable modem.
> >
> > So in linux you have interfaces like
> >
> > lan0, lan1, lan2, lan3, wan0
> >
> > DSA provides you these interface. And by default they are all
> > separated. There is no path between them. You can consider them as
> > being separate physical ethernet cards, just like all other interfaces
> > in linux.
> >
> > What you would typically do is:
> >
> > brctl addbr br0
> > brctl addif br0 lan0
> > brctl addif br0 lan1
> > brctl addif br0 lan2
> > brctl addif br0 lan3
> >
> > to create a bridge between the lan ports. The linux kernel will then
> > push this bridge configuration down into the hardware, so the switch
> > can forward frames between these ports.
> >
> > The wan port is not part of the bridge, so there is no L2 path to the
> > WAN port. You need to do IP routing on the CPU.
> >
> > Linux takes the stance that switch ports interfaces should act just
> > like any other linux interface and you configure them in the normal
> > linux way.
> >
> >      Andrew
> 
> Thanks for the explanation.  I am a bit befuddled by the combination of 
> all the possible configurations of the switch and how they interact with 
> Linux.  :)  I think I understand what is happening now.

You might also be looking at this the wrong way around. It is best to
think of the switch as a hardware accelerator. It offers functions to
the linux network stack to accelerate part of the linux network
stack. We only push out to the hardware functions it is capable of
accelerating. What it cannot accelerate stays in software. Think of it
as a GPU, but for networking...

	      Andrew

Re: [PATCH net-next 0/9] net: dsa: mv88e6xxx: implement VLAN filtering

[David Miller]

From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Date: Fri, 26 Feb 2016 13:15:59 -0500

> This patchset fixes hardware bridging for non 802.1Q aware systems.

Series applied, thanks Vivien.