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
v4: return -EOPNOTSUPP in switchdev prepare phase for unsupported objects;
handle num_ports in VTU GetNext / LoadPurge operations, instead of hardcoded 7.
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 | 476 ++++++++++++++++++++++++++++++++++++++
drivers/net/dsa/mv88e6xxx.h | 36 +++
include/net/dsa.h | 9 +
net/dsa/dsa_priv.h | 6 +
net/dsa/slave.c | 142 ++++++++++++
9 files changed, 681 insertions(+)
--
2.4.5
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 <[email protected]>
---
drivers/net/dsa/mv88e6xxx.c | 322 ++++++++++++++++++++++++++++++++++++++++++++
drivers/net/dsa/mv88e6xxx.h | 31 +++++
2 files changed, 353 insertions(+)
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 8c130c0..049553c 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,192 @@ 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)
+{
+ struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+ struct mv88e6xxx_vtu_entry next = { 0 };
+ int ret;
+
+ 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;
+
+ next.vid = ret & GLOBAL_VTU_VID_MASK;
+ next.valid = !!(ret & GLOBAL_VTU_VID_VALID);
+
+ if (next.valid) {
+ u16 data[3];
+ int port;
+
+ ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_DATA_0_3);
+ if (ret < 0)
+ return ret;
+ data[0] = ret;
+ ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_DATA_4_7);
+ if (ret < 0)
+ return ret;
+ data[1] = ret;
+ ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_DATA_8_11);
+ if (ret < 0)
+ return ret;
+ data[2] = ret;
+
+ for (port = 0; port < ps->num_ports; ++port) {
+ int reg = data[port / 4];
+
+ next.tags[port] =
+ GLOBAL_VTU_DATA_MEMBER_TAG_UNMASK(port, reg);
+ }
+
+ 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;
+
+ next.fid = ret & GLOBAL_VTU_FID_MASK;
+
+ ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL,
+ GLOBAL_VTU_SID);
+ if (ret < 0)
+ return ret;
+
+ next.sid = ret & GLOBAL_VTU_SID_MASK;
+ }
+ }
+
+ *entry = next;
+ return 0;
+}
+
+static int _mv88e6xxx_vtu_loadpurge(struct dsa_switch *ds,
+ struct mv88e6xxx_vtu_entry *entry)
+{
+ struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+ u16 reg = 0;
+ int ret;
+
+ ret = _mv88e6xxx_vtu_wait(ds);
+ if (ret < 0)
+ return ret;
+
+ if (entry->valid) {
+ u16 data[3] = { 0 };
+ int port;
+
+ for (port = 0; port < ps->num_ports; ++port) {
+ u8 tag = entry->tags[port];
+
+ data[port / 4] |= GLOBAL_VTU_DATA_MEMBER_TAG_MASK(port,
+ tag);
+ }
+
+ ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_DATA_0_3,
+ data[0]);
+ if (ret < 0)
+ return ret;
+
+ ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_DATA_4_7,
+ data[1]);
+ if (ret < 0)
+ return ret;
+
+ ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_DATA_8_11,
+ data[2]);
+ if (ret < 0)
+ return ret;
+
+ if (mv88e6xxx_6097_family(ds) || mv88e6xxx_6165_family(ds) ||
+ mv88e6xxx_6351_family(ds) || mv88e6xxx_6352_family(ds)) {
+ reg = entry->sid & GLOBAL_VTU_SID_MASK;
+ ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL,
+ GLOBAL_VTU_SID, reg);
+ if (ret < 0)
+ return ret;
+
+ reg = entry->fid & GLOBAL_VTU_FID_MASK;
+ ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL,
+ GLOBAL_VTU_FID, reg);
+ if (ret < 0)
+ return ret;
+ }
+
+ /* Valid bit set means load, unset means purge */
+ reg = GLOBAL_VTU_VID_VALID;
+ }
+
+ reg |= entry->vid & GLOBAL_VTU_VID_MASK;
+ ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_VID, reg);
+ 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 +1928,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 +2220,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..27d1913 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,10 +191,28 @@
#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_8_11 0x09
+#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
+/* Port member tags 0-11 are at offsets 0, 4, 8, 12 of VTU Data registers 1-3 */
+#define GLOBAL_VTU_DATA_MEMBER_TAG_SHIFT(_port) \
+ (((_port) % 4) * 4)
+#define GLOBAL_VTU_DATA_MEMBER_TAG_MASK(_port, _tag) \
+ (((_tag) & 0x03) << GLOBAL_VTU_DATA_MEMBER_TAG_SHIFT(_port))
+#define GLOBAL_VTU_DATA_MEMBER_TAG_UNMASK(_port, _reg) \
+ (((_reg) >> GLOBAL_VTU_DATA_MEMBER_TAG_SHIFT(_port)) & 0x03)
#define GLOBAL_ATU_CONTROL 0x0a
#define GLOBAL_ATU_CONTROL_LEARN2ALL BIT(3)
#define GLOBAL_ATU_OP 0x0b
@@ -310,6 +333,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
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 <[email protected]>
---
include/net/dsa.h | 9 ++++
net/dsa/dsa_priv.h | 6 +++
net/dsa/slave.c | 142 +++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 157 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..1da861e 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,141 @@ 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 = -EOPNOTSUPP;
+
+ /*
+ * The DSA drivers don't need to allocate any memory for operations on
+ * prepare phase, and they won't fail to HW on commit phase (unless
+ * something terrible goes wrong on the MDIO bus, in which case the
+ * commit phase wouldn't have been able to predict anyway).
+ *
+ * If an object is supported, skip the prepare phase by returning 0,
+ * otherwise return -EOPNOTSUPP.
+ */
+
+ switch (obj->id) {
+ case SWITCHDEV_OBJ_PORT_VLAN:
+ if (obj->trans == SWITCHDEV_TRANS_PREPARE)
+ return 0;
+
+ if (obj->trans == SWITCHDEV_TRANS_COMMIT)
+ 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 +833,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
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 <[email protected]>
---
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 | 154 ++++++++++++++++++++++++++++++++++++++
drivers/net/dsa/mv88e6xxx.h | 5 ++
6 files changed, 171 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 049553c..c7cd5f4 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -1555,6 +1555,160 @@ 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);
+ if (ret < 0)
+ goto unlock;
+
+ /* 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 27d1913..1577222 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -447,6 +447,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
On Tue, Jul 07, 2015 at 05:18:17PM -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 would like to do a proper review and testing of these patchset, but
i go on vacation this afternoon. So it will be in about 2 weeks time.
I spent 15 minutes tests just now. I spotted two things:
1) I played with a configuration, and then rebooted the machine. After
login i see:
Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
permitted by applicable law.
# cat /sys/kernel/debug/dsa0/vtu
VID FID SID 0 1 2 3 4 5 6
1 1 0 u u u u x x t
500 500 0 t t t t x x t
550 550 0 t x x x x x t
# bridge vlan show
port vlan ids
lan0 1 PVID Egress Untagged
lan0 1 PVID Egress Untagged
lan1
lan2
lan3
lan4
lan5
lan6
lan7
lan8 1 PVID Egress Untagged
lan8 1 PVID Egress Untagged
optical3
optical4
br0 1 PVID Egress Untagged
So the switch seems to have some VTU table entries, but the bridge
command does not show them. I suspect that a warm boot does not clear
out the VTU entries in the switch.
Until recently we had a similar problem with the statistics
counters. I wounder if we have the same problem with other tables? Do
static ATU entries get removed on a reboot?
2) I cold booted the machine, to be sure to have a clean state. Then:
# cat /sys/kernel/debug/dsa0/vtu
VID FID SID 0 1 2 3 4 5 6
1 1 0 u x x x x x t
So a good initial state. I then configure two bridges:
# brctl show
bridge name bridge id STP enabled interfaces
br0 8000.92647a2160c4 yes lan0
lan1
br1 8000.92647a2160c4 yes lan2
lan3
and then add vlan 500 to the four interfaces.
# bridge vlan add vid 500 dev lan0 master
# bridge vlan add vid 500 dev lan1 master
# bridge vlan add vid 500 dev lan2 master
# bridge vlan add vid 500 dev lan3 master
# cat /sys/kernel/debug/dsa0/vtu
VID FID SID 0 1 2 3 4 5 6
1 1 0 u u u u x x t
500 500 0 t t t t x x t
Does this mean we have one hardware bridge? All four ports can talk to
each other? I've not actually sent any frames to test this, so i'm
just speculating. Given that i have two software bridges, this is not
what i would expect, if frames from lan0 or lan1, also went out lan2
or lan3.
Thanks
Andrew
Hi Andrew,
On Jul 8, 2015, at 10:38 AM, Andrew Lunn [email protected] wrote:
> On Tue, Jul 07, 2015 at 05:18:17PM -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 would like to do a proper review and testing of these patchset, but
> i go on vacation this afternoon. So it will be in about 2 weeks time.
>
> I spent 15 minutes tests just now. I spotted two things:
>
> 1) I played with a configuration, and then rebooted the machine. After
> login i see:
>
> Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
> permitted by applicable law.
> # cat /sys/kernel/debug/dsa0/vtu
> VID FID SID 0 1 2 3 4 5 6
> 1 1 0 u u u u x x t
> 500 500 0 t t t t x x t
> 550 550 0 t x x x x x t
> # bridge vlan show
> port vlan ids
> lan0 1 PVID Egress Untagged
>
> lan0 1 PVID Egress Untagged
>
> lan1
> lan2
> lan3
> lan4
> lan5
> lan6
> lan7
> lan8 1 PVID Egress Untagged
>
> lan8 1 PVID Egress Untagged
>
> optical3
> optical4
> br0 1 PVID Egress Untagged
>
>
> So the switch seems to have some VTU table entries, but the bridge
> command does not show them. I suspect that a warm boot does not clear
> out the VTU entries in the switch.
>
> Until recently we had a similar problem with the statistics
> counters. I wounder if we have the same problem with other tables? Do
> static ATU entries get removed on a reboot?
>
You're right. There's a single operation to clear the STU and VTU. I
will send a follow-up patch to send this command during the switch
setup.
> 2) I cold booted the machine, to be sure to have a clean state. Then:
>
> # cat /sys/kernel/debug/dsa0/vtu
> VID FID SID 0 1 2 3 4 5 6
> 1 1 0 u x x x x x t
>
> So a good initial state. I then configure two bridges:
>
> # brctl show
> bridge name bridge id STP enabled interfaces
> br0 8000.92647a2160c4 yes lan0
> lan1
> br1 8000.92647a2160c4 yes lan2
> lan3
>
> and then add vlan 500 to the four interfaces.
>
> # bridge vlan add vid 500 dev lan0 master
> # bridge vlan add vid 500 dev lan1 master
> # bridge vlan add vid 500 dev lan2 master
> # bridge vlan add vid 500 dev lan3 master
>
> # cat /sys/kernel/debug/dsa0/vtu
> VID FID SID 0 1 2 3 4 5 6
> 1 1 0 u u u u x x t
> 500 500 0 t t t t x x t
>
> Does this mean we have one hardware bridge? All four ports can talk to
> each other? I've not actually sent any frames to test this, so i'm
> just speculating. Given that i have two software bridges, this is not
> what i would expect, if frames from lan0 or lan1, also went out lan2
> or lan3.
Indeed, with the "master" keyword, we ask switchdev to populate the
parent's (i.e. the switch chip) to create VLANs. Marvell switch such as
the 88E66352 can only have a single VLAN table entry for a given VID.
Please note that this patchset only brings support to access the
hardware VLAN Table Unit through switchdev, that is to say add, delete,
and dump entries in this table. To have fully functional VLAN support in
Linux with this switch family, there is certainly more work to do.
>From what I see, the patchset works fine for you (from bridge commands
and debugfs).
If the patchset looks good to you guys, may I suggest to get some
acknowledgments from you, so it can get merged? Then we could continue
to work on better synchronization with the FDB, STU (already in
progress) and other switchdev objects.
Regards,
-v
Vivien Didelot wrote:
> Hi Andrew,
>
> On Jul 8, 2015, at 10:38 AM, Andrew Lunn [email protected] wrote:
>
> > On Tue, Jul 07, 2015 at 05:18:17PM -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 would like to do a proper review and testing of these patchset, but
> > i go on vacation this afternoon. So it will be in about 2 weeks time.
> >
> > I spent 15 minutes tests just now. I spotted two things:
> >
> > 1) I played with a configuration, and then rebooted the machine. After
> > login i see:
> >
> > Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
> > permitted by applicable law.
> > # cat /sys/kernel/debug/dsa0/vtu
> > VID FID SID 0 1 2 3 4 5 6
> > 1 1 0 u u u u x x t
> > 500 500 0 t t t t x x t
> > 550 550 0 t x x x x x t
> > # bridge vlan show
> > port vlan ids
> > lan0 1 PVID Egress Untagged
> >
> > lan0 1 PVID Egress Untagged
> >
> > lan1
> > lan2
> > lan3
> > lan4
> > lan5
> > lan6
> > lan7
> > lan8 1 PVID Egress Untagged
> >
> > lan8 1 PVID Egress Untagged
> >
> > optical3
> > optical4
> > br0 1 PVID Egress Untagged
> >
> >
> > So the switch seems to have some VTU table entries, but the bridge
> > command does not show them. I suspect that a warm boot does not clear
> > out the VTU entries in the switch.
> >
> > Until recently we had a similar problem with the statistics
> > counters. I wounder if we have the same problem with other tables? Do
> > static ATU entries get removed on a reboot?
> >
>
> You're right. There's a single operation to clear the STU and VTU. I
> will send a follow-up patch to send this command during the switch
> setup.
>
> > 2) I cold booted the machine, to be sure to have a clean state. Then:
> >
> > # cat /sys/kernel/debug/dsa0/vtu
> > VID FID SID 0 1 2 3 4 5 6
> > 1 1 0 u x x x x x t
> >
> > So a good initial state. I then configure two bridges:
> >
> > # brctl show
> > bridge name bridge id STP enabled interfaces
> > br0 8000.92647a2160c4 yes lan0
> > lan1
> > br1 8000.92647a2160c4 yes lan2
> > lan3
> >
> > and then add vlan 500 to the four interfaces.
> >
> > # bridge vlan add vid 500 dev lan0 master
> > # bridge vlan add vid 500 dev lan1 master
> > # bridge vlan add vid 500 dev lan2 master
> > # bridge vlan add vid 500 dev lan3 master
> >
> > # cat /sys/kernel/debug/dsa0/vtu
> > VID FID SID 0 1 2 3 4 5 6
> > 1 1 0 u u u u x x t
> > 500 500 0 t t t t x x t
> >
> > Does this mean we have one hardware bridge? All four ports can talk to
> > each other? I've not actually sent any frames to test this, so i'm
> > just speculating. Given that i have two software bridges, this is not
> > what i would expect, if frames from lan0 or lan1, also went out lan2
> > or lan3.
>
> Indeed, with the "master" keyword, we ask switchdev to populate the
> parent's (i.e. the switch chip) to create VLANs. Marvell switch such as
> the 88E66352 can only have a single VLAN table entry for a given VID.
Hi Vivien
We are using the switch to perform hardware acceleration of things
that Linux does already in software. We have to keep with the
semantics of what is already supported in software. The patch in its
current state breaks the accepted behaviour.
This is a limitation of the switch. So the driver needs to keep track
of which bridge a VLAN belongs to, if it is asked to accelerate the
same VLAN for a different bridge, it needs to say to the kernel,
sorry, cannot do that, and leave the kernel to do it in software.
Andrew
Hi Andrew,
On Jul 8, 2015, at 1:32 PM, Andrew Lunn [email protected] wrote:
> Vivien Didelot wrote:
>> Hi Andrew,
>>
>> On Jul 8, 2015, at 10:38 AM, Andrew Lunn [email protected] wrote:
>>
>> > On Tue, Jul 07, 2015 at 05:18:17PM -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 would like to do a proper review and testing of these patchset, but
>> > i go on vacation this afternoon. So it will be in about 2 weeks time.
>> >
>> > I spent 15 minutes tests just now. I spotted two things:
>> >
>> > 1) I played with a configuration, and then rebooted the machine. After
>> > login i see:
>> >
>> > Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
>> > permitted by applicable law.
>> > # cat /sys/kernel/debug/dsa0/vtu
>> > VID FID SID 0 1 2 3 4 5 6
>> > 1 1 0 u u u u x x t
>> > 500 500 0 t t t t x x t
>> > 550 550 0 t x x x x x t
>> > # bridge vlan show
>> > port vlan ids
>> > lan0 1 PVID Egress Untagged
>> >
>> > lan0 1 PVID Egress Untagged
>> >
>> > lan1
>> > lan2
>> > lan3
>> > lan4
>> > lan5
>> > lan6
>> > lan7
>> > lan8 1 PVID Egress Untagged
>> >
>> > lan8 1 PVID Egress Untagged
>> >
>> > optical3
>> > optical4
>> > br0 1 PVID Egress Untagged
>> >
>> >
>> > So the switch seems to have some VTU table entries, but the bridge
>> > command does not show them. I suspect that a warm boot does not clear
>> > out the VTU entries in the switch.
>> >
>> > Until recently we had a similar problem with the statistics
>> > counters. I wounder if we have the same problem with other tables? Do
>> > static ATU entries get removed on a reboot?
>> >
>>
>> You're right. There's a single operation to clear the STU and VTU. I
>> will send a follow-up patch to send this command during the switch
>> setup.
>>
>> > 2) I cold booted the machine, to be sure to have a clean state. Then:
>> >
>> > # cat /sys/kernel/debug/dsa0/vtu
>> > VID FID SID 0 1 2 3 4 5 6
>> > 1 1 0 u x x x x x t
>> >
>> > So a good initial state. I then configure two bridges:
>> >
>> > # brctl show
>> > bridge name bridge id STP enabled interfaces
>> > br0 8000.92647a2160c4 yes lan0
>> > lan1
>> > br1 8000.92647a2160c4 yes lan2
>> > lan3
>> >
>> > and then add vlan 500 to the four interfaces.
>> >
>> > # bridge vlan add vid 500 dev lan0 master
>> > # bridge vlan add vid 500 dev lan1 master
>> > # bridge vlan add vid 500 dev lan2 master
>> > # bridge vlan add vid 500 dev lan3 master
>> >
>> > # cat /sys/kernel/debug/dsa0/vtu
>> > VID FID SID 0 1 2 3 4 5 6
>> > 1 1 0 u u u u x x t
>> > 500 500 0 t t t t x x t
>> >
>> > Does this mean we have one hardware bridge? All four ports can talk to
>> > each other? I've not actually sent any frames to test this, so i'm
>> > just speculating. Given that i have two software bridges, this is not
>> > what i would expect, if frames from lan0 or lan1, also went out lan2
>> > or lan3.
>>
>> Indeed, with the "master" keyword, we ask switchdev to populate the
>> parent's (i.e. the switch chip) to create VLANs. Marvell switch such as
>> the 88E66352 can only have a single VLAN table entry for a given VID.
>
> Hi Vivien
>
> We are using the switch to perform hardware acceleration of things
> that Linux does already in software. We have to keep with the
> semantics of what is already supported in software. The patch in its
> current state breaks the accepted behaviour.
I understand. However this whole VLAN thing represents a lot of code.
Some other work depends on portions of it. Do you think it'd be OK if I
resend the patch 1/3 alone? Having only the VTU operations and "vtu"
debugfs file does not break the actual behavior, and will lighten up the
following patchsets.
The patch 2/3 is ready and doesn't break anything either, but Jiri and
David suggested to send this patch with some actual implementation. Even
if the patch 3/3 shows that this switchdev/DSA glue is functional, I
understand that both have to be sent together later.
> This is a limitation of the switch. So the driver needs to keep track
> of which bridge a VLAN belongs to, if it is asked to accelerate the
> same VLAN for a different bridge, it needs to say to the kernel,
> sorry, cannot do that, and leave the kernel to do it in software.
Scott, how do you think this must be done? Returning a different error
code when trying to add a SWITCHDEV_OBJ_PORT_VLAN object?
Not sure how to query this fallback. Is -EOPNOTSUPP enough?
Thanks,
-v
> > This is a limitation of the switch. So the driver needs to keep track
> > of which bridge a VLAN belongs to, if it is asked to accelerate the
> > same VLAN for a different bridge, it needs to say to the kernel,
> > sorry, cannot do that, and leave the kernel to do it in software.
>
> Scott, how do you think this must be done? Returning a different error
> code when trying to add a SWITCHDEV_OBJ_PORT_VLAN object?
> Not sure how to query this fallback. Is -EOPNOTSUPP enough?
There was quite a bit of discussion about this a while back. It
should return an error during the prepare phase. Looking at
__vlan_vid_add() and switchdev_fib_ipv4_add, it should return
EOPNOTSUPP.
Andrew
> I understand. However this whole VLAN thing represents a lot of code.
> Some other work depends on portions of it. Do you think it'd be OK if I
> resend the patch 1/3 alone? Having only the VTU operations and "vtu"
> debugfs file does not break the actual behavior, and will lighten up the
> following patchsets.
It might be a bit early for that. All the previous versions of 1/3
have had issues with number of ports. I've not had time to take a
close look to see if there are other problems.
If somebody else does a proper review and says its O.K, them i'm
O.K. with that. But until that happens, i don't think it should go in.
Andrew
Hi Andrew,
----- On Jul 8, 2015, at 4:12 PM, Andrew Lunn [email protected] wrote:
>> I understand. However this whole VLAN thing represents a lot of code.
>> Some other work depends on portions of it. Do you think it'd be OK if I
>> resend the patch 1/3 alone? Having only the VTU operations and "vtu"
>> debugfs file does not break the actual behavior, and will lighten up the
>> following patchsets.
>
> It might be a bit early for that. All the previous versions of 1/3
> have had issues with number of ports. I've not had time to take a
> close look to see if there are other problems.
>
> If somebody else does a proper review and says its O.K, them i'm
> O.K. with that. But until that happens, i don't think it should go in.
>
> Andrew
OK, then I'll send right away smaller debugfs patches that I need.
Hopefully you'll have time to review it before your holidays ;-)
Also, I'll resend the VTU debugfs support later then.
Thanks,
-v
From: Vivien Didelot <[email protected]>
Date: Wed, 8 Jul 2015 13:13:16 -0400 (EDT)
> You're right. There's a single operation to clear the STU and VTU. I
> will send a follow-up patch to send this command during the switch
> setup.
Fix known bugs in the patch series, within the patch series, since it
hasn't been merged yet.
Don't say "I'll fix this bug later", because we can fix it now.
Also as others have noted, this patch set creates an intermediate
state where functionality is effectively broken. So it is illogical
to knowingly merge this patch series as-is. You'll have to piece
together all of the changes necessary such that the series is fully
bisectable, which means at every patch along the way things function
properly and do not break.
Thanks.
Hi David,
----- On Jul 9, 2015, at 2:01 PM, David [email protected] wrote:
> From: Vivien Didelot <[email protected]>
> Date: Wed, 8 Jul 2015 13:13:16 -0400 (EDT)
>
>> You're right. There's a single operation to clear the STU and VTU. I
>> will send a follow-up patch to send this command during the switch
>> setup.
>
> Fix known bugs in the patch series, within the patch series, since it
> hasn't been merged yet.
>
> Don't say "I'll fix this bug later", because we can fix it now.
>
> Also as others have noted, this patch set creates an intermediate
> state where functionality is effectively broken. So it is illogical
> to knowingly merge this patch series as-is. You'll have to piece
> together all of the changes necessary such that the series is fully
> bisectable, which means at every patch along the way things function
> properly and do not break.
Dully noted.
Thank you,
-v