[PATCH v3 0/3] net: dsa: mv88e6xxx: add support for VLAN Table Unit

[PATCH v3 0/3] net: dsa: mv88e6xxx: add support for VLAN Table Unit

[Vivien Didelot]

Hi all,

This patchset brings full support for hardware VLANs in DSA, and the Marvell
88E6xxx compatible switch chips.

The first patch adds the VTU operations to the mv88e6xxx code, as well as a
"vtu" debugfs file to read and modify the hardware VLAN table.

The second patch adds the glue between DSA and the switchdev VLAN objects.

The third patch finally implements the necessary functions in the mv88e6xxx
code to interact with the hardware VLAN through switchdev, from userspace
commands such as "bridge vlan".

Below is an example of what can be done with this patchset.

    "VID 550: 1t 3u"
    "VID 1000: 2t"
    "VID 1200: 2t 4t"

The VLAN setup above can be achieved with the following bridge commands:

    bridge vlan add vid 550 dev swp1 master
    bridge vlan add vid 550 dev swp3 master untagged pvid
    bridge vlan add vid 1000 dev swp2 master
    bridge vlan add vid 1200 dev swp2 master
    bridge vlan add vid 1200 dev swp4 master

Removing the port 1 from VLAN 550 is done with:

    bridge vlan del vid 550 dev swp1

The bridge command would output the following setup:

    # bridge vlan
    port	vlan ids
    swp0	None
    swp0
    swp1	None
    swp1
    swp2	1000
		1200

    swp2	1000
		1200

    swp3	550 PVID Egress Untagged

    swp3	550 PVID Egress Untagged

    swp4	1200

    swp4	1200

    br0	None

Assuming that swp5 is the CPU port, the "vtu" debugfs file would show:

    # cat /sys/kernel/debug/dsa0/vtu
    VID  FID  SID  0  1  2  3  4  5  6
    550  550  0    x  x  x  u  x  t  x
    1000 1000 0    x  x  t  x  x  t  x
    1200 1200 0    x  x  t  x  t  t  x

Cheers,
  -v


Vivien Didelot (3):
  net: dsa: mv88e6xxx: add debugfs interface for VTU
  net: dsa: add support for switchdev VLAN objects
  net: dsa: mv88e6xxx: add switchdev VLAN operations

 drivers/net/dsa/mv88e6123_61_65.c |   3 +
 drivers/net/dsa/mv88e6131.c       |   3 +
 drivers/net/dsa/mv88e6171.c       |   3 +
 drivers/net/dsa/mv88e6352.c       |   3 +
 drivers/net/dsa/mv88e6xxx.c       | 463 ++++++++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx.h       |  29 +++
 include/net/dsa.h                 |   9 +
 net/dsa/dsa_priv.h                |   6 +
 net/dsa/slave.c                   | 137 +++++++++++
 9 files changed, 656 insertions(+)

-- 
2.4.5

[PATCH v3 1/3] net: dsa: mv88e6xxx: add debugfs interface for VTU

[Vivien Didelot]

Implement the Get Next and Load Purge operations for the VLAN Table
Unit, and a "vtu" debugfs file to read and write the hardware VLANs.

A populated VTU look like this:

    # cat /sys/kernel/debug/dsa0/vtu
     VID  FID  SID  0  1  2  3  4  5  6
     550  562    0  x  x  x  u  x  t  x
    1000 1012    0  x  x  t  x  x  t  x
    1200 1212    0  x  x  t  x  t  t  x

Where "t", "u", "x", "-", respectively means that the port is tagged,
untagged, excluded or unmodified, for a given VLAN entry.

VTU entries can be added by echoing the same format:

    echo 1300 1312 0 x x t x t t x > vtu

and can be deleted by echoing only the VID:

    echo 1000 > vtu

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

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 8c130c0..ffd9fc6 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -2,6 +2,9 @@
  * net/dsa/mv88e6xxx.c - Marvell 88e6xxx switch chip support
  * Copyright (c) 2008 Marvell Semiconductor
  *
+ * Copyright (c) 2015 CMC Electronics, Inc.
+ *	Added support for 802.1Q VLAN Table Unit
+ *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; either version 2 of the License, or
@@ -1366,6 +1369,181 @@ static void mv88e6xxx_bridge_work(struct work_struct *work)
 	}
 }
 
+static int _mv88e6xxx_vtu_wait(struct dsa_switch *ds)
+{
+	return _mv88e6xxx_wait(ds, REG_GLOBAL, GLOBAL_VTU_OP,
+			       GLOBAL_VTU_OP_BUSY);
+}
+
+static int _mv88e6xxx_vtu_cmd(struct dsa_switch *ds, u16 op)
+{
+	int ret;
+
+	ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_OP, op);
+	if (ret < 0)
+		return ret;
+
+	return _mv88e6xxx_vtu_wait(ds);
+}
+
+static int _mv88e6xxx_stu_loadpurge(struct dsa_switch *ds, u8 sid, bool valid)
+{
+	int ret, data;
+
+	ret = _mv88e6xxx_vtu_wait(ds);
+	if (ret < 0)
+		return ret;
+
+	data = sid & GLOBAL_VTU_SID_MASK;
+	if (valid)
+		data |= GLOBAL_VTU_VID_VALID;
+
+	ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_VID, data);
+	if (ret < 0)
+		return ret;
+
+	/* Unused (yet) data registers */
+	ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_DATA_0_3, 0);
+	if (ret < 0)
+		return ret;
+
+	ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_DATA_4_7, 0);
+	if (ret < 0)
+		return ret;
+
+	ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_DATA_8_11, 0);
+	if (ret < 0)
+		return ret;
+
+	return _mv88e6xxx_vtu_cmd(ds, GLOBAL_VTU_OP_STU_LOAD_PURGE);
+}
+
+static int _mv88e6xxx_vtu_getnext(struct dsa_switch *ds, u16 vid,
+				  struct mv88e6xxx_vtu_entry *entry)
+{
+	int ret, i;
+
+	ret = _mv88e6xxx_vtu_wait(ds);
+	if (ret < 0)
+		return ret;
+
+	ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_VID,
+				   vid & GLOBAL_VTU_VID_MASK);
+	if (ret < 0)
+		return ret;
+
+	ret = _mv88e6xxx_vtu_cmd(ds, GLOBAL_VTU_OP_VTU_GET_NEXT);
+	if (ret < 0)
+		return ret;
+
+	ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_VID);
+	if (ret < 0)
+		return ret;
+
+	entry->vid = ret & GLOBAL_VTU_VID_MASK;
+	entry->valid = !!(ret & GLOBAL_VTU_VID_VALID);
+
+	if (entry->valid) {
+		/* Ports 0-3, offsets 0, 4, 8, 12 */
+		ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_DATA_0_3);
+		if (ret < 0)
+			return ret;
+
+		for (i = 0; i < 4; ++i)
+			entry->tags[i] = (ret >> (i * 4)) & 3;
+
+		/* Ports 4-6, offsets 0, 4, 8 */
+		ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_DATA_4_7);
+		if (ret < 0)
+			return ret;
+
+		for (i = 4; i < 7; ++i)
+			entry->tags[i] = (ret >> ((i - 4) * 4)) & 3;
+
+		if (mv88e6xxx_6097_family(ds) || mv88e6xxx_6165_family(ds) ||
+		    mv88e6xxx_6351_family(ds) || mv88e6xxx_6352_family(ds)) {
+			ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL,
+						  GLOBAL_VTU_FID);
+			if (ret < 0)
+				return ret;
+
+			entry->fid = ret & GLOBAL_VTU_FID_MASK;
+
+			ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL,
+						  GLOBAL_VTU_SID);
+			if (ret < 0)
+				return ret;
+
+			entry->sid = ret & GLOBAL_VTU_SID_MASK;
+		} else {
+			entry->fid = 0;
+			entry->sid = 0;
+		}
+	}
+
+	return 0;
+}
+
+static int _mv88e6xxx_vtu_loadpurge(struct dsa_switch *ds,
+				    struct mv88e6xxx_vtu_entry *entry)
+{
+	u16 data = 0;
+	int ret, i;
+
+	ret = _mv88e6xxx_vtu_wait(ds);
+	if (ret < 0)
+		return ret;
+
+	if (entry->valid) {
+		/* Set Data Register, ports 0-3, offsets 0, 4, 8, 12 */
+		for (data = i = 0; i < 4; ++i)
+			data |= entry->tags[i] << (i * 4);
+		ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_DATA_0_3,
+					   data);
+		if (ret < 0)
+			return ret;
+
+		/* Set Data Register, ports 4-6, offsets 0, 4, 8 */
+		for (data = 0, i = 4; i < 7; ++i)
+			data |= entry->tags[i] << ((i - 4) * 4);
+		ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_DATA_4_7,
+					   data);
+		if (ret < 0)
+			return ret;
+
+		/* Unused Data Register */
+		ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_DATA_8_11,
+					   0);
+		if (ret < 0)
+			return ret;
+
+		if (mv88e6xxx_6097_family(ds) || mv88e6xxx_6165_family(ds) ||
+		    mv88e6xxx_6351_family(ds) || mv88e6xxx_6352_family(ds)) {
+			data = entry->sid & GLOBAL_VTU_SID_MASK;
+			ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL,
+						   GLOBAL_VTU_SID, data);
+			if (ret < 0)
+				return ret;
+
+			data = entry->fid & GLOBAL_VTU_FID_MASK;
+			ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL,
+						   GLOBAL_VTU_FID, data);
+			if (ret < 0)
+				return ret;
+		}
+
+		/* Valid bit set means load, unset means purge */
+		data = GLOBAL_VTU_VID_VALID;
+	}
+
+	data |= entry->vid & GLOBAL_VTU_VID_MASK;
+	ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_VID, data);
+	if (ret < 0)
+		return ret;
+
+	return _mv88e6xxx_vtu_cmd(ds, GLOBAL_VTU_OP_VTU_LOAD_PURGE);
+}
+
 static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
@@ -1739,6 +1917,136 @@ static const struct file_operations mv88e6xxx_atu_fops = {
 	.owner  = THIS_MODULE,
 };
 
+static int mv88e6xxx_vtu_show(struct seq_file *s, void *p)
+{
+	struct dsa_switch *ds = s->private;
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	int port, ret = 0, vid = 0xfff; /* find the first or lowest VID */
+
+	seq_puts(s, " VID  FID  SID");
+	for (port = 0; port < ps->num_ports; ++port)
+		seq_printf(s, " %2d", port);
+	seq_puts(s, "\n");
+
+	mutex_lock(&ps->smi_mutex);
+
+	do {
+		struct mv88e6xxx_vtu_entry next = { 0 };
+
+		ret = _mv88e6xxx_vtu_getnext(ds, vid, &next);
+		if (ret < 0)
+			goto unlock;
+
+		if (!next.valid)
+			break;
+
+		seq_printf(s, "%4d %4d   %2d", next.vid, next.fid, next.sid);
+		for (port = 0; port < ps->num_ports; ++port) {
+			switch (next.tags[port]) {
+			case GLOBAL_VTU_DATA_MEMBER_TAG_UNMODIFIED:
+				seq_puts(s, "  -");
+				break;
+			case GLOBAL_VTU_DATA_MEMBER_TAG_UNTAGGED:
+				seq_puts(s, "  u");
+				break;
+			case GLOBAL_VTU_DATA_MEMBER_TAG_TAGGED:
+				seq_puts(s, "  t");
+				break;
+			case GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER:
+				seq_puts(s, "  x");
+				break;
+			default:
+				seq_puts(s, "  ?");
+				break;
+			}
+		}
+		seq_puts(s, "\n");
+
+		vid = next.vid;
+	} while (vid < 0xfff);
+
+unlock:
+	mutex_unlock(&ps->smi_mutex);
+
+	return ret;
+}
+
+static ssize_t mv88e6xxx_vtu_write(struct file *file, const char __user *buf,
+				   size_t count, loff_t *ppos)
+{
+	struct seq_file *s = file->private_data;
+	struct dsa_switch *ds = s->private;
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	struct mv88e6xxx_vtu_entry entry = { 0 };
+	bool valid = true;
+	char tags[12]; /* DSA_MAX_PORTS */
+	int vid, fid, sid, port, ret;
+
+	/* scan 12 chars instead of num_ports to avoid dynamic scanning... */
+	ret = sscanf(buf, "%d %d %d %c %c %c %c %c %c %c %c %c %c %c %c", &vid,
+		     &fid, &sid, &tags[0], &tags[1], &tags[2], &tags[3],
+		     &tags[4], &tags[5], &tags[6], &tags[7], &tags[8], &tags[9],
+		     &tags[10], &tags[11]);
+	if (ret == 1)
+		valid = false;
+	else if (ret != 3 + ps->num_ports)
+		return -EINVAL;
+
+	entry.vid = vid;
+	entry.valid = valid;
+
+	if (valid) {
+		entry.fid = fid;
+		entry.sid = sid;
+		/* Note: The VTU entry pointed by VID will be loaded but not
+		 * considered valid until the STU entry pointed by SID is valid.
+		 */
+
+		for (port = 0; port < ps->num_ports; ++port) {
+			u8 tag;
+
+			switch (tags[port]) {
+			case 'u':
+				tag = GLOBAL_VTU_DATA_MEMBER_TAG_UNTAGGED;
+				break;
+			case 't':
+				tag = GLOBAL_VTU_DATA_MEMBER_TAG_TAGGED;
+				break;
+			case 'x':
+				tag = GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER;
+				break;
+			case '-':
+				tag = GLOBAL_VTU_DATA_MEMBER_TAG_UNMODIFIED;
+				break;
+			default:
+				return -EINVAL;
+			}
+
+			entry.tags[port] = tag;
+		}
+	}
+
+	mutex_lock(&ps->smi_mutex);
+	ret = _mv88e6xxx_vtu_loadpurge(ds, &entry);
+	mutex_unlock(&ps->smi_mutex);
+
+	return ret < 0 ? ret : count;
+}
+
+static int mv88e6xxx_vtu_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, mv88e6xxx_vtu_show, inode->i_private);
+}
+
+static const struct file_operations mv88e6xxx_vtu_fops = {
+	.open		= mv88e6xxx_vtu_open,
+	.read		= seq_read,
+	.write		= mv88e6xxx_vtu_write,
+	.llseek		= no_llseek,
+	.release	= single_release,
+	.owner		= THIS_MODULE,
+};
+
 static void mv88e6xxx_stats_show_header(struct seq_file *s,
 					struct mv88e6xxx_priv_state *ps)
 {
@@ -1901,6 +2209,9 @@ int mv88e6xxx_setup_common(struct dsa_switch *ds)
 	debugfs_create_file("atu", S_IRUGO, ps->dbgfs, ds,
 			    &mv88e6xxx_atu_fops);
 
+	debugfs_create_file("vtu", S_IRUGO | S_IWUSR, ps->dbgfs, ds,
+			    &mv88e6xxx_vtu_fops);
+
 	debugfs_create_file("stats", S_IRUGO, ps->dbgfs, ds,
 			    &mv88e6xxx_stats_fops);
 
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 1bc5a3e..65645b7 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -124,6 +124,7 @@
 #define PORT_CONTROL_1		0x05
 #define PORT_BASE_VLAN		0x06
 #define PORT_DEFAULT_VLAN	0x07
+#define PORT_DEFAULT_VLAN_MASK	0xfff
 #define PORT_CONTROL_2		0x08
 #define PORT_CONTROL_2_IGNORE_FCS	BIT(15)
 #define PORT_CONTROL_2_VTU_PRI_OVERRIDE	BIT(14)
@@ -170,6 +171,10 @@
 #define GLOBAL_MAC_01		0x01
 #define GLOBAL_MAC_23		0x02
 #define GLOBAL_MAC_45		0x03
+#define GLOBAL_VTU_FID		0x02	/* 6097 6165 6351 6352 */
+#define GLOBAL_VTU_FID_MASK	0xfff
+#define GLOBAL_VTU_SID		0x03	/* 6097 6165 6351 6352 */
+#define GLOBAL_VTU_SID_MASK	0x3f
 #define GLOBAL_CONTROL		0x04
 #define GLOBAL_CONTROL_SW_RESET		BIT(15)
 #define GLOBAL_CONTROL_PPU_ENABLE	BIT(14)
@@ -186,9 +191,20 @@
 #define GLOBAL_CONTROL_TCAM_EN		BIT(1)
 #define GLOBAL_CONTROL_EEPROM_DONE_EN	BIT(0)
 #define GLOBAL_VTU_OP		0x05
+#define GLOBAL_VTU_OP_BUSY	BIT(15)
+#define GLOBAL_VTU_OP_FLUSH_ALL		((1 << 12) | GLOBAL_VTU_OP_BUSY)
+#define GLOBAL_VTU_OP_VTU_LOAD_PURGE	((3 << 12) | GLOBAL_VTU_OP_BUSY)
+#define GLOBAL_VTU_OP_VTU_GET_NEXT	((4 << 12) | GLOBAL_VTU_OP_BUSY)
+#define GLOBAL_VTU_OP_STU_LOAD_PURGE	((5 << 12) | GLOBAL_VTU_OP_BUSY)
 #define GLOBAL_VTU_VID		0x06
+#define GLOBAL_VTU_VID_MASK	0xfff
+#define GLOBAL_VTU_VID_VALID	BIT(12)
 #define GLOBAL_VTU_DATA_0_3	0x07
 #define GLOBAL_VTU_DATA_4_7	0x08
+#define GLOBAL_VTU_DATA_MEMBER_TAG_UNMODIFIED	0
+#define GLOBAL_VTU_DATA_MEMBER_TAG_UNTAGGED	1
+#define GLOBAL_VTU_DATA_MEMBER_TAG_TAGGED	2
+#define GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER	3
 #define GLOBAL_VTU_DATA_8_11	0x09
 #define GLOBAL_ATU_CONTROL	0x0a
 #define GLOBAL_ATU_CONTROL_LEARN2ALL	BIT(3)
@@ -310,6 +326,14 @@
 #define GLOBAL2_QOS_WEIGHT	0x1c
 #define GLOBAL2_MISC		0x1d
 
+struct mv88e6xxx_vtu_entry {
+	u16	vid;
+	u16	fid;
+	u8	sid;
+	bool	valid;
+	u8	tags[DSA_MAX_PORTS];
+};
+
 struct mv88e6xxx_priv_state {
 	/* When using multi-chip addressing, this mutex protects
 	 * access to the indirect access registers.  (In single-chip
-- 
2.4.5

[PATCH v3 2/3] net: dsa: add support for switchdev VLAN objects

[Vivien Didelot]

This patch adds the glue between DSA and switchdev operations to add,
delete and dump SWITCHDEV_OBJ_PORT_VLAN objects.

This is a first step to link the "bridge vlan" command with hardware
entries for DSA compatible switch chips.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 include/net/dsa.h  |   9 ++++
 net/dsa/dsa_priv.h |   6 +++
 net/dsa/slave.c    | 137 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 152 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index fbca63b..cabf2a5 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -302,6 +302,15 @@ struct dsa_switch_driver {
 			   const unsigned char *addr, u16 vid);
 	int	(*fdb_getnext)(struct dsa_switch *ds, int port,
 			       unsigned char *addr, bool *is_static);
+
+	/*
+	 * VLAN support
+	 */
+	int	(*port_vlan_add)(struct dsa_switch *ds, int port, u16 vid,
+				 u16 bridge_flags);
+	int	(*port_vlan_del)(struct dsa_switch *ds, int port, u16 vid);
+	int	(*port_vlan_dump)(struct dsa_switch *ds, int port, u16 vid,
+				  u16 *bridge_flags);
 };
 
 void register_switch_driver(struct dsa_switch_driver *type);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index d5f1f9b..9029717 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -13,6 +13,7 @@
 
 #include <linux/phy.h>
 #include <linux/netdevice.h>
+#include <linux/if_vlan.h>
 
 struct dsa_device_ops {
 	netdev_tx_t (*xmit)(struct sk_buff *skb, struct net_device *dev);
@@ -47,6 +48,11 @@ struct dsa_slave_priv {
 	int			old_duplex;
 
 	struct net_device	*bridge_dev;
+
+	/*
+	 * Which VLANs this port is a member of.
+	 */
+	DECLARE_BITMAP(vlan_bitmap, VLAN_N_VID);
 };
 
 /* dsa.c */
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 04ffad3..47c459b 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -18,6 +18,7 @@
 #include <net/rtnetlink.h>
 #include <net/switchdev.h>
 #include <linux/if_bridge.h>
+#include <linux/if_vlan.h>
 #include "dsa_priv.h"
 
 /* slave mii_bus handling ***************************************************/
@@ -363,6 +364,136 @@ static int dsa_slave_port_attr_set(struct net_device *dev,
 	return ret;
 }
 
+static int dsa_slave_port_vlans_add(struct net_device *dev,
+				     struct switchdev_obj *obj)
+{
+	struct switchdev_obj_vlan *vlan = &obj->u.vlan;
+	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_switch *ds = p->parent;
+	int vid, err = 0;
+
+	if (!ds->drv->port_vlan_add)
+		return -EOPNOTSUPP;
+
+	for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid) {
+		err = ds->drv->port_vlan_add(ds, p->port, vid, vlan->flags);
+		if (err)
+			break;
+		set_bit(vid, p->vlan_bitmap);
+	}
+
+	return err;
+}
+
+static int dsa_slave_port_obj_add(struct net_device *dev,
+				  struct switchdev_obj *obj)
+{
+	int err;
+
+	/*
+	 * Skip the prepare phase, since currently the DSA drivers don't need to
+	 * allocate any memory for operations and they will not fail to HW
+	 * (unless something horrible goes wrong on the MDIO bus, in which case
+	 * the prepare phase wouldn't have been able to predict anyway).
+	 */
+	if (obj->trans != SWITCHDEV_TRANS_COMMIT)
+		return 0;
+
+	switch (obj->id) {
+	case SWITCHDEV_OBJ_PORT_VLAN:
+		err = dsa_slave_port_vlans_add(dev, obj);
+		break;
+	default:
+		err = -EOPNOTSUPP;
+		break;
+	}
+
+	return err;
+}
+
+static int dsa_slave_port_vlans_del(struct net_device *dev,
+				     struct switchdev_obj *obj)
+{
+	struct switchdev_obj_vlan *vlan = &obj->u.vlan;
+	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_switch *ds = p->parent;
+	int vid, err = 0;
+
+	if (!ds->drv->port_vlan_del)
+		return -EOPNOTSUPP;
+
+	for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid) {
+		err = ds->drv->port_vlan_del(ds, p->port, vid);
+		if (err)
+			break;
+		clear_bit(vid, p->vlan_bitmap);
+	}
+
+	return err;
+}
+
+static int dsa_slave_port_obj_del(struct net_device *dev,
+				  struct switchdev_obj *obj)
+{
+	int err;
+
+	switch (obj->id) {
+	case SWITCHDEV_OBJ_PORT_VLAN:
+		err = dsa_slave_port_vlans_del(dev, obj);
+		break;
+	default:
+		err = -EOPNOTSUPP;
+		break;
+	}
+
+	return err;
+}
+
+static int dsa_slave_port_vlans_dump(struct net_device *dev,
+				     struct switchdev_obj *obj)
+{
+	struct switchdev_obj_vlan *vlan = &obj->u.vlan;
+	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_switch *ds = p->parent;
+	int vid, err = 0;
+
+	if (!ds->drv->port_vlan_dump)
+		return -EOPNOTSUPP;
+
+	for_each_set_bit(vid, p->vlan_bitmap, VLAN_N_VID) {
+		u16 flags = 0;
+
+		err = ds->drv->port_vlan_dump(ds, p->port, vid, &flags);
+		if (err)
+			break;
+
+		vlan->flags = flags;
+		vlan->vid_begin = vlan->vid_end = vid;
+		err = obj->cb(dev, obj);
+		if (err)
+			break;
+	}
+
+	return err;
+}
+
+static int dsa_slave_port_obj_dump(struct net_device *dev,
+				   struct switchdev_obj *obj)
+{
+	int err;
+
+	switch (obj->id) {
+	case SWITCHDEV_OBJ_PORT_VLAN:
+		err = dsa_slave_port_vlans_dump(dev, obj);
+		break;
+	default:
+		err = -EOPNOTSUPP;
+		break;
+	}
+
+	return err;
+}
+
 static int dsa_slave_bridge_port_join(struct net_device *dev,
 				      struct net_device *br)
 {
@@ -697,11 +828,17 @@ static const struct net_device_ops dsa_slave_netdev_ops = {
 	.ndo_fdb_dump		= dsa_slave_fdb_dump,
 	.ndo_do_ioctl		= dsa_slave_ioctl,
 	.ndo_get_iflink		= dsa_slave_get_iflink,
+	.ndo_bridge_getlink	= switchdev_port_bridge_getlink,
+	.ndo_bridge_setlink	= switchdev_port_bridge_setlink,
+	.ndo_bridge_dellink	= switchdev_port_bridge_dellink,
 };
 
 static const struct switchdev_ops dsa_slave_switchdev_ops = {
 	.switchdev_port_attr_get	= dsa_slave_port_attr_get,
 	.switchdev_port_attr_set	= dsa_slave_port_attr_set,
+	.switchdev_port_obj_add		= dsa_slave_port_obj_add,
+	.switchdev_port_obj_del		= dsa_slave_port_obj_del,
+	.switchdev_port_obj_dump	= dsa_slave_port_obj_dump,
 };
 
 static void dsa_slave_adjust_link(struct net_device *dev)
-- 
2.4.5

[PATCH v3 3/3] net: dsa: mv88e6xxx: add switchdev VLAN operations

[Vivien Didelot]

This commit implements the switchdev operations to add, delete and dump
VLANs for the Marvell 88E6352 and compatible switch chips.

This allows to access the switch VLAN Table Unit from standard userspace
commands such as "bridge vlan".

A configuration like "1t 2t 3t 4u" for VLAN 10 is achieved like this:

    # bridge vlan add dev swp1 vid 10 master
    # bridge vlan add dev swp2 vid 10 master
    # bridge vlan add dev swp3 vid 10 master
    # bridge vlan add dev swp4 vid 10 master untagged pvid

This calls port_vlan_add() for each command. Removing the port 3 from
VLAN 10 is done with:

    # bridge vlan del dev swp3 vid 10

This calls port_vlan_del() for port 3. Dumping VLANs is done with:

    # bridge vlan show
    port    vlan ids
    swp0    None
    swp0
    swp1     10

    swp1     10

    swp2     10

    swp2     10

    swp3    None
    swp3
    swp4     10 PVID Egress Untagged

    swp4     10 PVID Egress Untagged

    br0     None

This calls port_vlan_dump() for each ports.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6123_61_65.c |   3 +
 drivers/net/dsa/mv88e6131.c       |   3 +
 drivers/net/dsa/mv88e6171.c       |   3 +
 drivers/net/dsa/mv88e6352.c       |   3 +
 drivers/net/dsa/mv88e6xxx.c       | 152 ++++++++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx.h       |   5 ++
 6 files changed, 169 insertions(+)

diff --git a/drivers/net/dsa/mv88e6123_61_65.c b/drivers/net/dsa/mv88e6123_61_65.c
index 71a29a7..8e679ff 100644
--- a/drivers/net/dsa/mv88e6123_61_65.c
+++ b/drivers/net/dsa/mv88e6123_61_65.c
@@ -134,6 +134,9 @@ struct dsa_switch_driver mv88e6123_61_65_switch_driver = {
 #endif
 	.get_regs_len		= mv88e6xxx_get_regs_len,
 	.get_regs		= mv88e6xxx_get_regs,
+	.port_vlan_add		= mv88e6xxx_port_vlan_add,
+	.port_vlan_del		= mv88e6xxx_port_vlan_del,
+	.port_vlan_dump		= mv88e6xxx_port_vlan_dump,
 };
 
 MODULE_ALIAS("platform:mv88e6123");
diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
index 32f4a08..c4d914b 100644
--- a/drivers/net/dsa/mv88e6131.c
+++ b/drivers/net/dsa/mv88e6131.c
@@ -182,6 +182,9 @@ struct dsa_switch_driver mv88e6131_switch_driver = {
 	.get_strings		= mv88e6xxx_get_strings,
 	.get_ethtool_stats	= mv88e6xxx_get_ethtool_stats,
 	.get_sset_count		= mv88e6xxx_get_sset_count,
+	.port_vlan_add		= mv88e6xxx_port_vlan_add,
+	.port_vlan_del		= mv88e6xxx_port_vlan_del,
+	.port_vlan_dump		= mv88e6xxx_port_vlan_dump,
 };
 
 MODULE_ALIAS("platform:mv88e6085");
diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index 1c78084..7701ce6 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -119,6 +119,9 @@ struct dsa_switch_driver mv88e6171_switch_driver = {
 	.fdb_add		= mv88e6xxx_port_fdb_add,
 	.fdb_del		= mv88e6xxx_port_fdb_del,
 	.fdb_getnext		= mv88e6xxx_port_fdb_getnext,
+	.port_vlan_add		= mv88e6xxx_port_vlan_add,
+	.port_vlan_del		= mv88e6xxx_port_vlan_del,
+	.port_vlan_dump		= mv88e6xxx_port_vlan_dump,
 };
 
 MODULE_ALIAS("platform:mv88e6171");
diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
index 632815c..b981be4a 100644
--- a/drivers/net/dsa/mv88e6352.c
+++ b/drivers/net/dsa/mv88e6352.c
@@ -392,6 +392,9 @@ struct dsa_switch_driver mv88e6352_switch_driver = {
 	.fdb_add		= mv88e6xxx_port_fdb_add,
 	.fdb_del		= mv88e6xxx_port_fdb_del,
 	.fdb_getnext		= mv88e6xxx_port_fdb_getnext,
+	.port_vlan_add		= mv88e6xxx_port_vlan_add,
+	.port_vlan_del		= mv88e6xxx_port_vlan_del,
+	.port_vlan_dump		= mv88e6xxx_port_vlan_dump,
 };
 
 MODULE_ALIAS("platform:mv88e6352");
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index ffd9fc6..d5812ba 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -1544,6 +1544,158 @@ static int _mv88e6xxx_vtu_loadpurge(struct dsa_switch *ds,
 	return _mv88e6xxx_vtu_cmd(ds, GLOBAL_VTU_OP_VTU_LOAD_PURGE);
 }
 
+int mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port, u16 vid,
+			    u16 bridge_flags)
+{
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	struct mv88e6xxx_vtu_entry entry = { 0 };
+	int prev_vid = vid ? vid - 1 : 0xfff;
+	int i, ret;
+
+	mutex_lock(&ps->smi_mutex);
+	ret = _mv88e6xxx_vtu_getnext(ds, prev_vid, &entry);
+	if (ret < 0)
+		goto unlock;
+
+	/* If the VLAN does not exist, re-initialize the entry for addition */
+	if (entry.vid != vid || !entry.valid) {
+		memset(&entry, 0, sizeof(entry));
+		entry.valid = true;
+		entry.vid = vid;
+		entry.fid = vid; /* We use one FID per VLAN at the moment */
+		entry.sid = 0; /* We don't use 802.1s (yet) */
+
+		/* The DSA port-based VLAN setup also reserves one FID per port;
+		 * thus, warn when requesting one of the first num_ports VIDs.
+		 */
+		if (entry.fid < ps->num_ports)
+			netdev_warn(ds->ports[vid], "using reserved FID\n");
+
+		if (mv88e6xxx_6097_family(ds) || mv88e6xxx_6165_family(ds) ||
+		    mv88e6xxx_6351_family(ds) || mv88e6xxx_6352_family(ds)) {
+			/* A VTU entry need a valid STU entry (undocumented).
+			 * The default STU pointer for a VTU entry is 0. If per
+			 * VLAN spanning tree is not used then only one STU
+			 * entry is needed to cover all VTU entries. Thus,
+			 * validate the STU entry 0.
+			 */
+			ret = _mv88e6xxx_stu_loadpurge(ds, 0, true);
+			if (ret < 0)
+				goto unlock;
+		}
+
+		for (i = 0; i < ps->num_ports; ++i)
+			entry.tags[i] = dsa_is_cpu_port(ds, i) ?
+				GLOBAL_VTU_DATA_MEMBER_TAG_TAGGED :
+				GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER;
+	}
+
+	entry.tags[port] = bridge_flags & BRIDGE_VLAN_INFO_UNTAGGED ?
+		GLOBAL_VTU_DATA_MEMBER_TAG_UNTAGGED :
+		GLOBAL_VTU_DATA_MEMBER_TAG_TAGGED;
+
+	ret = _mv88e6xxx_vtu_loadpurge(ds, &entry);
+
+	/* Set port default VID */
+	if (bridge_flags & BRIDGE_VLAN_INFO_PVID)
+		ret = _mv88e6xxx_reg_write(ds, REG_PORT(port),
+					   PORT_DEFAULT_VLAN,
+					   vid & PORT_DEFAULT_VLAN_MASK);
+unlock:
+	mutex_unlock(&ps->smi_mutex);
+
+	return ret;
+}
+
+int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port, u16 vid)
+{
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	struct mv88e6xxx_vtu_entry entry = { 0 };
+	int i, ret, prev_vid = vid ? vid - 1 : 0xfff;
+	bool keep = false;
+
+	mutex_lock(&ps->smi_mutex);
+	ret = _mv88e6xxx_vtu_getnext(ds, prev_vid, &entry);
+	if (ret < 0)
+		goto unlock;
+
+	if (entry.vid != vid || !entry.valid) {
+		ret = -ENOENT;
+		goto unlock;
+	}
+
+	entry.tags[port] = GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER;
+
+	/* keep the VLAN unless all ports are excluded */
+	for (i = 0; i < ps->num_ports; ++i) {
+		if (dsa_is_cpu_port(ds, i))
+			continue;
+
+		if (entry.tags[i] != GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER) {
+			keep = true;
+			break;
+		}
+	}
+
+	entry.valid = keep;
+	ret = _mv88e6xxx_vtu_loadpurge(ds, &entry);
+	if (ret < 0)
+		goto unlock;
+
+	/* TODO reset PVID if it was this VID? */
+
+	if (!keep)
+		ret = _mv88e6xxx_update_bridge_config(ds, entry.fid);
+unlock:
+	mutex_unlock(&ps->smi_mutex);
+
+	return ret;
+}
+
+int mv88e6xxx_port_vlan_dump(struct dsa_switch *ds, int port, u16 vid,
+			     u16 *bridge_flags)
+{
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	struct mv88e6xxx_vtu_entry entry = { 0 };
+	int prev_vid = vid ? vid - 1 : 0xfff;
+	int ret;
+
+	mutex_lock(&ps->smi_mutex);
+	ret = _mv88e6xxx_vtu_getnext(ds, prev_vid, &entry);
+	if (ret < 0)
+		goto unlock;
+
+	if (entry.vid != vid || !entry.valid) {
+		ret = -ENOENT;
+		goto unlock;
+	}
+
+	switch (entry.tags[port]) {
+	case GLOBAL_VTU_DATA_MEMBER_TAG_UNTAGGED:
+		*bridge_flags |= BRIDGE_VLAN_INFO_UNTAGGED;
+		break;
+	case GLOBAL_VTU_DATA_MEMBER_TAG_TAGGED:
+		break;
+	default:
+		ret = -EINVAL;
+		goto unlock;
+	}
+
+	/* check PVID */
+	ret = _mv88e6xxx_reg_read(ds, REG_PORT(port), PORT_DEFAULT_VLAN);
+	if (ret < 0)
+		goto unlock;
+
+	if ((ret & PORT_DEFAULT_VLAN_MASK) == vid)
+		*bridge_flags |= BRIDGE_VLAN_INFO_PVID;
+
+	ret = 0;
+unlock:
+	mutex_unlock(&ps->smi_mutex);
+
+	return ret;
+}
+
 static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 65645b7..be8b6cc 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -440,6 +440,11 @@ int mv88e6xxx_port_fdb_getnext(struct dsa_switch *ds, int port,
 int mv88e6xxx_phy_page_read(struct dsa_switch *ds, int port, int page, int reg);
 int mv88e6xxx_phy_page_write(struct dsa_switch *ds, int port, int page,
 			     int reg, int val);
+int mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port, u16 vid,
+			    u16 bridge_flags);
+int mv88e6xxx_port_vlan_del(struct dsa_switch *ds,int port, u16 vid);
+int mv88e6xxx_port_vlan_dump(struct dsa_switch *ds,int port, u16 vid,
+			     u16 *bridge_flags);
 extern struct dsa_switch_driver mv88e6131_switch_driver;
 extern struct dsa_switch_driver mv88e6123_61_65_switch_driver;
 extern struct dsa_switch_driver mv88e6352_switch_driver;
-- 
2.4.5

Re: [PATCH v3 0/3] net: dsa: mv88e6xxx: add support for VLAN Table Unit

[Andrew Lunn]

On Sun, Jul 05, 2015 at 10:14:50PM -0400, Vivien Didelot wrote:
> Hi all,
> 
> This patchset brings full support for hardware VLANs in DSA, and the Marvell
> 88E6xxx compatible switch chips.

Hi Vivien

I just booted these patches on my board, and i'm getting WARNINGS:

[   61.111302] WARNING: CPU: 0 PID: 2751 at net/switchdev/switchdev.c:265 switchdev_port_obj_add+0xd4/0xdc()
[   61.119602] lan0: Commit of object (id=2) failed.
[   61.123007] Modules linked in:
[   61.124790] CPU: 0 PID: 2751 Comm: ip Tainted: G        W       4.2.0-rc1-00029-g5b5313438cfd #187
[   61.132484] Hardware name: Freescale Vybrid VF5xx/VF6xx (Device Tree)
[   61.137622] Backtrace: 
[   61.138817] [<80012ecc>] (dump_backtrace) from [<80013118>] (show_stack+0x20/0x24)
[   61.145123]  r6:804e3aa4 r5:00000009 r4:00000000 r3:00000000
[   61.149616] [<800130f8>] (show_stack) from [<804ea2e0>] (dump_stack+0x24/0x28)
[   61.155562] [<804ea2bc>] (dump_stack) from [<8001e4b0>] (warn_slowpath_common+0x98/0xc4)
[   61.162411] [<8001e418>] (warn_slowpath_common) from [<8001e51c>] (warn_slowpath_fmt+0x40/0x48)
[   61.169843]  r8:00000020 r7:00000000 r6:87b4d000 r5:ffffffa1 r4:80647708
[   61.175345] [<8001e4e0>] (warn_slowpath_fmt) from [<804e3aa4>] (switchdev_port_obj_add+0xd4/0xdc)
[   61.182950]  r3:87b4d000 r2:80647708
[   61.185255]  r4:870b3abc
[   61.186525] [<804e39d0>] (switchdev_port_obj_add) from [<804e3b9c>] (switchdev_fib_ipv4_add+0x90/0xbc)
[   61.194564]  r6:00000003 r5:00000c00 r4:87243d80
[   61.197949] [<804e3b0c>] (switchdev_fib_ipv4_add) from [<80491f8c>] (fib_table_insert+0x1cc/0x4cc)
[   61.205642]  r7:870b3b60 r6:c0a80d00 r5:87108794 r4:00000000
[   61.210125] [<80491dc0>] (fib_table_insert) from [<8048ce58>] (fib_magic.isra.18+0xdc/0xf0)
[   61.217169]  r10:8717c6c0 r9:871c9b00 r8:00000020 r7:000da8c0 r6:00000018 r5:040da8c0
[   61.223812]  r4:00000003
[   61.225089] [<8048cd7c>] (fib_magic.isra.18) from [<8048defc>] (fib_add_ifaddr+0x150/0x1a0)
[   61.232168]  r9:87243a00 r8:87a40000 r7:040da8c0 r6:00ffffff r5:000da8c0 r4:87243a00
[   61.238712] [<8048ddac>] (fib_add_ifaddr) from [<8048e500>] (fib_inetaddr_event+0x84/0xc4)
[   61.245707]  r9:00000000 r8:00000000 r7:ffffffff r6:87a40000 r5:00000001 r4:87243a00
[   61.252284] [<8048e47c>] (fib_inetaddr_event) from [<8003892c>] (notifier_call_chain+0x54/0x94)
[   61.259716]  r6:00000001 r5:87243a00 r4:ffffffff r3:8048e47c
[   61.264159] [<800388d8>] (notifier_call_chain) from [<80038cd8>] (__blocking_notifier_call_chain+0x58/0x70)
[   61.272633]  r9:871c9b00 r8:871c9b0c r7:00000001 r6:87243a00 r5:8071a720 r4:ffffffff
[   61.279176] [<80038c80>] (__blocking_notifier_call_chain) from [<80038d18>] (blocking_notifier_call_chain+0x28/0x30)
[   61.288428]  r7:000000c5 r6:87243a00 r5:871c9000 r4:00000abf
[   61.292901] [<80038cf0>] (blocking_notifier_call_chain) from [<80483674>] (__inet_insert_ifa+0x18c/0x238)
[   61.301219] [<804834e8>] (__inet_insert_ifa) from [<80484c24>] (inet_rtm_newaddr+0x188/0x3d0)
[   61.308439]  r9:00000000 r8:ffffffff r7:87243a00 r6:8717c6c0 r5:871c9000 r4:00000000
[   61.315022] [<80484a9c>] (inet_rtm_newaddr) from [<80438130>] (rtnetlink_rcv_msg+0x188/0x228)
[   61.322275]  r9:00000000 r8:00000000 r7:00000002 r6:8717c6c0 r5:871c9000 r4:00000030
[   61.328818] [<80437fa8>] (rtnetlink_rcv_msg) from [<8044a4b0>] (netlink_rcv_skb+0xb0/0xcc)
[   61.335815]  r8:00000000 r7:8717c6c0 r6:8717c6c0 r5:80437fa8 r4:871c9000
[   61.341353] [<8044a400>] (netlink_rcv_skb) from [<80437fa0>] (rtnetlink_rcv+0x34/0x3c)
[   61.347968]  r6:8709c400 r5:00000038 r4:8717c6c0 r3:80437f6c
[   61.352446] [<80437f6c>] (rtnetlink_rcv) from [<80449e74>] (netlink_unicast+0x174/0x1f4)
[   61.359235]  r4:878d3c00 r3:80437f6c
[   61.361596] [<80449d00>] (netlink_unicast) from [<8044a2f0>] (netlink_sendmsg+0x334/0x348)
[   61.368560]  r8:00000000 r7:00000038 r6:00000000 r5:8709c400 r4:870b3f4c
[   61.374100] [<80449fbc>] (netlink_sendmsg) from [<8040e928>] (sock_sendmsg+0x24/0x34)
[   61.380662]  r10:00000000 r9:870b3e30 r8:00000000 r7:8773e3c0 r6:00000000 r5:00000000
[   61.387269]  r4:870b3f4c
[   61.388545] [<8040e904>] (sock_sendmsg) from [<8040f294>] (___sys_sendmsg+0x1dc/0x1e4)
[   61.395218] [<8040f0b8>] (___sys_sendmsg) from [<8040ffe8>] (__sys_sendmsg+0x4c/0x7c)
[   61.401782]  r9:870b2000 r8:8000fa44 r7:00000128 r6:7e9169b4 r5:00000000 r4:8773e3c0
[   61.408331] [<8040ff9c>] (__sys_sendmsg) from [<80410030>] (SyS_sendmsg+0x18/0x1c)
[   61.414630]  r6:7e9169a0 r5:00000010 r4:0000000c
[   61.418019] [<80410018>] (SyS_sendmsg) from [<8000f8a0>] (ret_fast_syscall+0x0/0x3c)
[   61.424494] ---[ end trace 6a5447143fac0dcb ]---

This is with -rc1.

     Andrew

Re: [PATCH v3 0/3] net: dsa: mv88e6xxx: add support for VLAN Table Unit

[Andrew Lunn]

On Tue, Jul 07, 2015 at 01:38:04AM +0200, Andrew Lunn wrote:
> On Sun, Jul 05, 2015 at 10:14:50PM -0400, Vivien Didelot wrote:
> > Hi all,
> > 
> > This patchset brings full support for hardware VLANs in DSA, and the Marvell
> > 88E6xxx compatible switch chips.
> 
> Hi Vivien
> 
> I just booted these patches on my board, and i'm getting WARNINGS:
> 
> [   61.111302] WARNING: CPU: 0 PID: 2751 at net/switchdev/switchdev.c:265 switchdev_port_obj_add+0xd4/0xdc()

Hi Vivien

I debugged this a bit.

The problem comes from:

static int dsa_slave_port_obj_add(struct net_device *dev,
                                  struct switchdev_obj *obj)
{
        int err;

        /*                                                                                                               
         * Skip the prepare phase, since currently the DSA drivers don't need to                                         
         * allocate any memory for operations and they will not fail to HW                                               
         * (unless something horrible goes wrong on the MDIO bus, in which case                                          
         * the prepare phase wouldn't have been able to predict anyway).                                                 
         */
        if (obj->trans != SWITCHDEV_TRANS_COMMIT)
                return 0;

        switch (obj->id) {
        case SWITCHDEV_OBJ_PORT_VLAN:
                err = dsa_slave_port_vlans_add(dev, obj);
                break;
        default:
                err = -EOPNOTSUPP;
                break;
        }

        return err;
}

It is being called with obj->id of 2, which is
SWITCHDEV_OBJ_IPV4_FIB. This function is called twice. The first time
it is with SWITCHDEV_TRANS_PREPARE and we are allowed to return an
error. The second time, with SWITCHDEV_TRANS_COMMIT, errors are not
allowed.

EOPNOTSUPP is considered an error, so since we don't support
SWITCHDEV_OBJ_IPV4_FIB we error out the COMMIT phase.

Not sure which is cleaner. Test to see if we support the object during
the prepare, or allow the commit to accept EOPNOTSUPP as not being an
error?

	Andrew

Re: [PATCH v3 1/3] net: dsa: mv88e6xxx: add debugfs interface for VTU

[Andrew Lunn]

> +static int _mv88e6xxx_vtu_getnext(struct dsa_switch *ds, u16 vid,
> +				  struct mv88e6xxx_vtu_entry *entry)
> +{
> +	int ret, i;
> +
> +	ret = _mv88e6xxx_vtu_wait(ds);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_VID,
> +				   vid & GLOBAL_VTU_VID_MASK);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = _mv88e6xxx_vtu_cmd(ds, GLOBAL_VTU_OP_VTU_GET_NEXT);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_VID);
> +	if (ret < 0)
> +		return ret;
> +
> +	entry->vid = ret & GLOBAL_VTU_VID_MASK;
> +	entry->valid = !!(ret & GLOBAL_VTU_VID_VALID);
> +
> +	if (entry->valid) {
> +		/* Ports 0-3, offsets 0, 4, 8, 12 */
> +		ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_DATA_0_3);
> +		if (ret < 0)
> +			return ret;
> +
> +		for (i = 0; i < 4; ++i)
> +			entry->tags[i] = (ret >> (i * 4)) & 3;
> +
> +		/* Ports 4-6, offsets 0, 4, 8 */
> +		ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_DATA_4_7);
> +		if (ret < 0)
> +			return ret;
> +
> +		for (i = 4; i < 7; ++i)
> +			entry->tags[i] = (ret >> ((i - 4) * 4)) & 3;

Hi Vivien

It looks like you still have up to 7 ports, rather than use
ps->num_ports. I have a ten port switch i would like to use VLANs with
:-)

> +
> +		if (mv88e6xxx_6097_family(ds) || mv88e6xxx_6165_family(ds) ||
> +		    mv88e6xxx_6351_family(ds) || mv88e6xxx_6352_family(ds)) {
> +			ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL,
> +						  GLOBAL_VTU_FID);
> +			if (ret < 0)
> +				return ret;
> +
> +			entry->fid = ret & GLOBAL_VTU_FID_MASK;
> +
> +			ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL,
> +						  GLOBAL_VTU_SID);
> +			if (ret < 0)
> +				return ret;
> +
> +			entry->sid = ret & GLOBAL_VTU_SID_MASK;
> +		} else {
> +			entry->fid = 0;
> +			entry->sid = 0;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int _mv88e6xxx_vtu_loadpurge(struct dsa_switch *ds,
> +				    struct mv88e6xxx_vtu_entry *entry)
> +{
> +	u16 data = 0;
> +	int ret, i;
> +
> +	ret = _mv88e6xxx_vtu_wait(ds);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (entry->valid) {
> +		/* Set Data Register, ports 0-3, offsets 0, 4, 8, 12 */
> +		for (data = i = 0; i < 4; ++i)
> +			data |= entry->tags[i] << (i * 4);
> +		ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_DATA_0_3,
> +					   data);
> +		if (ret < 0)
> +			return ret;
> +
> +		/* Set Data Register, ports 4-6, offsets 0, 4, 8 */
> +		for (data = 0, i = 4; i < 7; ++i)
> +			data |= entry->tags[i] << ((i - 4) * 4);
> +		ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_DATA_4_7,
> +					   data);

Same again here.

     Andrew

Re: [PATCH v3 0/3] net: dsa: mv88e6xxx: add support for VLAN Table Unit

[Scott Feldman]

On Mon, Jul 6, 2015 at 7:00 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Tue, Jul 07, 2015 at 01:38:04AM +0200, Andrew Lunn wrote:
>> On Sun, Jul 05, 2015 at 10:14:50PM -0400, Vivien Didelot wrote:
>> > Hi all,
>> >
>> > This patchset brings full support for hardware VLANs in DSA, and the Marvell
>> > 88E6xxx compatible switch chips.
>>
>> Hi Vivien
>>
>> I just booted these patches on my board, and i'm getting WARNINGS:
>>
>> [   61.111302] WARNING: CPU: 0 PID: 2751 at net/switchdev/switchdev.c:265 switchdev_port_obj_add+0xd4/0xdc()
>
> Hi Vivien
>
> I debugged this a bit.
>
> The problem comes from:
>
> static int dsa_slave_port_obj_add(struct net_device *dev,
>                                   struct switchdev_obj *obj)
> {
>         int err;
>
>         /*
>          * Skip the prepare phase, since currently the DSA drivers don't need to
>          * allocate any memory for operations and they will not fail to HW
>          * (unless something horrible goes wrong on the MDIO bus, in which case
>          * the prepare phase wouldn't have been able to predict anyway).
>          */
>         if (obj->trans != SWITCHDEV_TRANS_COMMIT)
>                 return 0;
>
>         switch (obj->id) {
>         case SWITCHDEV_OBJ_PORT_VLAN:
>                 err = dsa_slave_port_vlans_add(dev, obj);
>                 break;
>         default:
>                 err = -EOPNOTSUPP;
>                 break;
>         }
>
>         return err;
> }
>
> It is being called with obj->id of 2, which is
> SWITCHDEV_OBJ_IPV4_FIB. This function is called twice. The first time
> it is with SWITCHDEV_TRANS_PREPARE and we are allowed to return an
> error. The second time, with SWITCHDEV_TRANS_COMMIT, errors are not
> allowed.
>
> EOPNOTSUPP is considered an error, so since we don't support
> SWITCHDEV_OBJ_IPV4_FIB we error out the COMMIT phase.
>
> Not sure which is cleaner. Test to see if we support the object during
> the prepare, or allow the commit to accept EOPNOTSUPP as not being an
> error?

I think we should return EOPNOTSUPP on PREPARE, so move the trans !=
COMMIT test inside the case for PORT_VLAN.  That would future-proof
the func when new objects are added to switchdev (and not supported by
dsa_slave).

Does that sound OK?

-scott

Re: [PATCH v3 1/3] net: dsa: mv88e6xxx: add debugfs interface for VTU

[Vivien Didelot]

Hi Andrew,

On Jul 6, 2015, at 10:08 PM, Andrew Lunn andrew@lunn.ch wrote:

>> +static int _mv88e6xxx_vtu_getnext(struct dsa_switch *ds, u16 vid,
>> +				  struct mv88e6xxx_vtu_entry *entry)
>> +{
>> +	int ret, i;
>> +
>> +	ret = _mv88e6xxx_vtu_wait(ds);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_VID,
>> +				   vid & GLOBAL_VTU_VID_MASK);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = _mv88e6xxx_vtu_cmd(ds, GLOBAL_VTU_OP_VTU_GET_NEXT);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_VID);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	entry->vid = ret & GLOBAL_VTU_VID_MASK;
>> +	entry->valid = !!(ret & GLOBAL_VTU_VID_VALID);
>> +
>> +	if (entry->valid) {
>> +		/* Ports 0-3, offsets 0, 4, 8, 12 */
>> +		ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_DATA_0_3);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		for (i = 0; i < 4; ++i)
>> +			entry->tags[i] = (ret >> (i * 4)) & 3;
>> +
>> +		/* Ports 4-6, offsets 0, 4, 8 */
>> +		ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_DATA_4_7);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		for (i = 4; i < 7; ++i)
>> +			entry->tags[i] = (ret >> ((i - 4) * 4)) & 3;
> 
> Hi Vivien
> 
> It looks like you still have up to 7 ports, rather than use
> ps->num_ports. I have a ten port switch i would like to use VLANs with
> :-)
> 
>> +
>> +		if (mv88e6xxx_6097_family(ds) || mv88e6xxx_6165_family(ds) ||
>> +		    mv88e6xxx_6351_family(ds) || mv88e6xxx_6352_family(ds)) {
>> +			ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL,
>> +						  GLOBAL_VTU_FID);
>> +			if (ret < 0)
>> +				return ret;
>> +
>> +			entry->fid = ret & GLOBAL_VTU_FID_MASK;
>> +
>> +			ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL,
>> +						  GLOBAL_VTU_SID);
>> +			if (ret < 0)
>> +				return ret;
>> +
>> +			entry->sid = ret & GLOBAL_VTU_SID_MASK;
>> +		} else {
>> +			entry->fid = 0;
>> +			entry->sid = 0;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int _mv88e6xxx_vtu_loadpurge(struct dsa_switch *ds,
>> +				    struct mv88e6xxx_vtu_entry *entry)
>> +{
>> +	u16 data = 0;
>> +	int ret, i;
>> +
>> +	ret = _mv88e6xxx_vtu_wait(ds);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	if (entry->valid) {
>> +		/* Set Data Register, ports 0-3, offsets 0, 4, 8, 12 */
>> +		for (data = i = 0; i < 4; ++i)
>> +			data |= entry->tags[i] << (i * 4);
>> +		ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_DATA_0_3,
>> +					   data);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		/* Set Data Register, ports 4-6, offsets 0, 4, 8 */
>> +		for (data = 0, i = 4; i < 7; ++i)
>> +			data |= entry->tags[i] << ((i - 4) * 4);
>> +		ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_DATA_4_7,
>> +					   data);
> 
> Same again here.
> 
>      Andrew

Indeed, I intentionally kept it as is, since the 88E6352 datasheet is not
really clear about this. I see that the register 0x09 (called
GLOBAL_VTU_DATA_8_11 in mv88e6xxx.h) only contains VID priority related bits in
15:12, in my case. As bits 11:0 are reserved, I suspect that the offsets of
Member TagP7, Member TagP8 and Member TagP9 are respectively 0, 4 and 8 for
you. Can you confirm? If so, I'll make that generic with these values.

Thanks,
-v

Re: [PATCH v3 1/3] net: dsa: mv88e6xxx: add debugfs interface for VTU

[Andrew Lunn]

> Indeed, I intentionally kept it as is, since the 88E6352 datasheet is not
> really clear about this. I see that the register 0x09 (called
> GLOBAL_VTU_DATA_8_11 in mv88e6xxx.h) only contains VID priority related bits in
> 15:12, in my case. As bits 11:0 are reserved, I suspect that the offsets of
> Member TagP7, Member TagP8 and Member TagP9 are respectively 0, 4 and 8 for
> you. Can you confirm? If so, I'll make that generic with these values.

Yes, the pattern continues for up to 11 ports, if you look at the DSDT
sources from Marvell. The mv88e6131.c driver has one device, the 6095,
with 11 ports.

     Thanks
          Andrew

Re: [PATCH v3 1/3] net: dsa: mv88e6xxx: add debugfs interface for VTU

[Guenter Roeck]

On 07/07/2015 08:52 AM, Vivien Didelot wrote:
> Hi Andrew,
>
> On Jul 6, 2015, at 10:08 PM, Andrew Lunn andrew@lunn.ch wrote:
>
>>> +static int _mv88e6xxx_vtu_getnext(struct dsa_switch *ds, u16 vid,
>>> +				  struct mv88e6xxx_vtu_entry *entry)
>>> +{
>>> +	int ret, i;
>>> +
>>> +	ret = _mv88e6xxx_vtu_wait(ds);
>>> +	if (ret < 0)
>>> +		return ret;
>>> +
>>> +	ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_VID,
>>> +				   vid & GLOBAL_VTU_VID_MASK);
>>> +	if (ret < 0)
>>> +		return ret;
>>> +
>>> +	ret = _mv88e6xxx_vtu_cmd(ds, GLOBAL_VTU_OP_VTU_GET_NEXT);
>>> +	if (ret < 0)
>>> +		return ret;
>>> +
>>> +	ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_VID);
>>> +	if (ret < 0)
>>> +		return ret;
>>> +
>>> +	entry->vid = ret & GLOBAL_VTU_VID_MASK;
>>> +	entry->valid = !!(ret & GLOBAL_VTU_VID_VALID);
>>> +
>>> +	if (entry->valid) {
>>> +		/* Ports 0-3, offsets 0, 4, 8, 12 */
>>> +		ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_DATA_0_3);
>>> +		if (ret < 0)
>>> +			return ret;
>>> +
>>> +		for (i = 0; i < 4; ++i)
>>> +			entry->tags[i] = (ret >> (i * 4)) & 3;
>>> +
>>> +		/* Ports 4-6, offsets 0, 4, 8 */
>>> +		ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_DATA_4_7);
>>> +		if (ret < 0)
>>> +			return ret;
>>> +
>>> +		for (i = 4; i < 7; ++i)
>>> +			entry->tags[i] = (ret >> ((i - 4) * 4)) & 3;
>>
>> Hi Vivien
>>
>> It looks like you still have up to 7 ports, rather than use
>> ps->num_ports. I have a ten port switch i would like to use VLANs with
>> :-)
>>
>>> +
>>> +		if (mv88e6xxx_6097_family(ds) || mv88e6xxx_6165_family(ds) ||
>>> +		    mv88e6xxx_6351_family(ds) || mv88e6xxx_6352_family(ds)) {
>>> +			ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL,
>>> +						  GLOBAL_VTU_FID);
>>> +			if (ret < 0)
>>> +				return ret;
>>> +
>>> +			entry->fid = ret & GLOBAL_VTU_FID_MASK;
>>> +
>>> +			ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL,
>>> +						  GLOBAL_VTU_SID);
>>> +			if (ret < 0)
>>> +				return ret;
>>> +
>>> +			entry->sid = ret & GLOBAL_VTU_SID_MASK;
>>> +		} else {
>>> +			entry->fid = 0;
>>> +			entry->sid = 0;
>>> +		}
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int _mv88e6xxx_vtu_loadpurge(struct dsa_switch *ds,
>>> +				    struct mv88e6xxx_vtu_entry *entry)
>>> +{
>>> +	u16 data = 0;
>>> +	int ret, i;
>>> +
>>> +	ret = _mv88e6xxx_vtu_wait(ds);
>>> +	if (ret < 0)
>>> +		return ret;
>>> +
>>> +	if (entry->valid) {
>>> +		/* Set Data Register, ports 0-3, offsets 0, 4, 8, 12 */
>>> +		for (data = i = 0; i < 4; ++i)
>>> +			data |= entry->tags[i] << (i * 4);
>>> +		ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_DATA_0_3,
>>> +					   data);
>>> +		if (ret < 0)
>>> +			return ret;
>>> +
>>> +		/* Set Data Register, ports 4-6, offsets 0, 4, 8 */
>>> +		for (data = 0, i = 4; i < 7; ++i)
>>> +			data |= entry->tags[i] << ((i - 4) * 4);
>>> +		ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_DATA_4_7,
>>> +					   data);
>>
>> Same again here.
>>
>>       Andrew
>
> Indeed, I intentionally kept it as is, since the 88E6352 datasheet is not
> really clear about this. I see that the register 0x09 (called
> GLOBAL_VTU_DATA_8_11 in mv88e6xxx.h) only contains VID priority related bits in
> 15:12, in my case. As bits 11:0 are reserved, I suspect that the offsets of
> Member TagP7, Member TagP8 and Member TagP9 are respectively 0, 4 and 8 for
> you. Can you confirm? If so, I'll make that generic with these values.
>

For 6152 and 6185: Port 7 is in bit 12:15 of GLOBAL_VTU_DATA_4_7, port 8 and 9
are in register 9, bit 0:3 and 4:7.

Guenter

Re: [PATCH v3 0/3] net: dsa: mv88e6xxx: add support for VLAN Table Unit

[Vivien Didelot]

Hi Andrew, Scott,

On Jul 6, 2015, at 11:46 PM, Scott Feldman sfeldma@gmail.com wrote:

> On Mon, Jul 6, 2015 at 7:00 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> On Tue, Jul 07, 2015 at 01:38:04AM +0200, Andrew Lunn wrote:
>>> On Sun, Jul 05, 2015 at 10:14:50PM -0400, Vivien Didelot wrote:
>>> > Hi all,
>>> >
>>> > This patchset brings full support for hardware VLANs in DSA, and the Marvell
>>> > 88E6xxx compatible switch chips.
>>>
>>> Hi Vivien
>>>
>>> I just booted these patches on my board, and i'm getting WARNINGS:
>>>
>>> [   61.111302] WARNING: CPU: 0 PID: 2751 at net/switchdev/switchdev.c:265
>>> switchdev_port_obj_add+0xd4/0xdc()
>>
>> Hi Vivien
>>
>> I debugged this a bit.
>>
>> The problem comes from:
>>
>> static int dsa_slave_port_obj_add(struct net_device *dev,
>>                                   struct switchdev_obj *obj)
>> {
>>         int err;
>>
>>         /*
>>          * Skip the prepare phase, since currently the DSA drivers don't need to
>>          * allocate any memory for operations and they will not fail to HW
>>          * (unless something horrible goes wrong on the MDIO bus, in which case
>>          * the prepare phase wouldn't have been able to predict anyway).
>>          */
>>         if (obj->trans != SWITCHDEV_TRANS_COMMIT)
>>                 return 0;
>>
>>         switch (obj->id) {
>>         case SWITCHDEV_OBJ_PORT_VLAN:
>>                 err = dsa_slave_port_vlans_add(dev, obj);
>>                 break;
>>         default:
>>                 err = -EOPNOTSUPP;
>>                 break;
>>         }
>>
>>         return err;
>> }
>>
>> It is being called with obj->id of 2, which is
>> SWITCHDEV_OBJ_IPV4_FIB. This function is called twice. The first time
>> it is with SWITCHDEV_TRANS_PREPARE and we are allowed to return an
>> error. The second time, with SWITCHDEV_TRANS_COMMIT, errors are not
>> allowed.
>>
>> EOPNOTSUPP is considered an error, so since we don't support
>> SWITCHDEV_OBJ_IPV4_FIB we error out the COMMIT phase.
>>
>> Not sure which is cleaner. Test to see if we support the object during
>> the prepare, or allow the commit to accept EOPNOTSUPP as not being an
>> error?
> 
> I think we should return EOPNOTSUPP on PREPARE, so move the trans !=
> COMMIT test inside the case for PORT_VLAN.  That would future-proof
> the func when new objects are added to switchdev (and not supported by
> dsa_slave).

Does this fixup http://ix.io/jxq look good to you?

Thanks,
-v

Re: [PATCH v3 1/3] net: dsa: mv88e6xxx: add debugfs interface for VTU

[Vivien Didelot]

Hi Guenter, Andrew,

----- On Jul 7, 2015, at 12:07 PM, Andrew Lunn andrew@lunn.ch wrote:

>> Indeed, I intentionally kept it as is, since the 88E6352 datasheet is not
>> really clear about this. I see that the register 0x09 (called
>> GLOBAL_VTU_DATA_8_11 in mv88e6xxx.h) only contains VID priority related bits in
>> 15:12, in my case. As bits 11:0 are reserved, I suspect that the offsets of
>> Member TagP7, Member TagP8 and Member TagP9 are respectively 0, 4 and 8 for
>> you. Can you confirm? If so, I'll make that generic with these values.
> 
> Yes, the pattern continues for up to 11 ports, if you look at the DSDT
> sources from Marvell. The mv88e6131.c driver has one device, the 6095,
> with 11 ports.

Thanks for the details. I make the change and send a v4 asap.

Best,
-v

Re: [PATCH v3 0/3] net: dsa: mv88e6xxx: add support for VLAN Table Unit

[Andrew Lunn]

On Tue, Jul 07, 2015 at 12:17:57PM -0400, Vivien Didelot wrote:
> Hi Andrew, Scott,
> Does this fixup http://ix.io/jxq look good to you?

Yes, that looks good.

     Andrew

Re: [PATCH v3 0/3] net: dsa: mv88e6xxx: add support for VLAN Table Unit

[Scott Feldman]

On Tue, Jul 7, 2015 at 9:17 AM, Vivien Didelot
<vivien.didelot@savoirfairelinux.com> wrote:
> Hi Andrew, Scott,
> Does this fixup http://ix.io/jxq look good to you?

Yes.