Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1344537 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH net-next 0/9] net: dsa: mv88e6xxx: implement VLAN filtering Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-02-26 19:30 +0100
    [PATCH net-next 2/9] net: dsa: mv88e6xxx: extract single VLAN retrieval Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-02-26 19:30 +0100
    [PATCH net-next 4/9] net: dsa: mv88e6xxx: assign dynamic FDB to VLANs Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-02-26 19:30 +0100
    [PATCH net-next 6/9] net: dsa: mv88e6xxx: assign dynamic FDB to bridges Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-02-26 19:30 +0100
    [PATCH net-next 7/9] net: dsa: mv88e6xxx: restore VLANTable map control Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-02-26 19:30 +0100
      Re: [PATCH net-next 7/9] net: dsa: mv88e6xxx: restore VLANTable map  control Kevin Smith <kevin.smith@elecsyscorp.com> - 2016-02-26 21:50 +0100
        Re: [PATCH net-next 7/9] net: dsa: mv88e6xxx: restore VLANTable map  control Andrew Lunn <andrew@lunn.ch> - 2016-02-26 22:10 +0100
          Re: [PATCH net-next 7/9] net: dsa: mv88e6xxx: restore VLANTable map control Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-02-26 22:40 +0100
            Re: [PATCH net-next 7/9] net: dsa: mv88e6xxx: restore VLANTable map  control Andrew Lunn <andrew@lunn.ch> - 2016-02-26 23:10 +0100
            Re: [PATCH net-next 7/9] net: dsa: mv88e6xxx: restore VLANTable map  control Kevin Smith <kevin.smith@elecsyscorp.com> - 2016-02-26 23:30 +0100
              Re: [PATCH net-next 7/9] net: dsa: mv88e6xxx: restore VLANTable map  control Andrew Lunn <andrew@lunn.ch> - 2016-02-26 23:40 +0100
                Re: [PATCH net-next 7/9] net: dsa: mv88e6xxx: restore VLANTable map  control Kevin Smith <kevin.smith@elecsyscorp.com> - 2016-02-26 23:50 +0100
                  Re: [PATCH net-next 7/9] net: dsa: mv88e6xxx: restore VLANTable map  control Andrew Lunn <andrew@lunn.ch> - 2016-02-27 04:20 +0100
    [PATCH net-next 3/9] net: dsa: mv88e6xxx: extract single FDB dump Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-02-26 19:30 +0100
    Re: [PATCH net-next 0/9] net: dsa: mv88e6xxx: implement VLAN  filtering David Miller <davem@davemloft.net> - 2016-03-01 22:30 +0100

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

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

[toc] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web