[PATCH v6 0/3] Add 802.1AD protocol support for dsa switch and ocelot driver

[PATCH v6 0/3] Add 802.1AD protocol support for dsa switch and ocelot driver

[hongbo.wang]

From: "hongbo.wang" <hongbo.wang@nxp.com>

1. Overview 
a) 0001* is for support to set dsa slave into 802.1AD(QinQ) mode.
b) 0002* is for vlan_proto support for br_switchdev_port_vlan_add and br_switchdev_port_vlan_del.
c) 0003* is for setting QinQ related registers in ocelot switch driver, after applying this patch, the switch(VSC99599)'s port can enable or disable QinQ mode.

2. Version log
v6:
a) put the code for switchdev into single patch
b) change code according to latest mainline

v5:
a) add devlink to enable qinq_mode of ocelot's single port 
b) modify br_switchdev_port_vlan_add to pass bridge's vlan_proto to port driver 
c) enable NETIF_F_HW_VLAN_STAG_FILTER in ocelot driver

v4:
a) modify slave.c to support "ip set br0 type bridge vlan_protocol 802.1ad"
b) modify ocelot.c, if enable QinQ, set VLAN_AWARE_ENA and VLAN_POP_CNT per
   port when vlan_filter=1

v3: combine two patches to one post

hongbo.wang (3):
  net: dsa: Add protocol support for 802.1AD when adding or deleting
    vlan for dsa switch port
  net: switchdev: Add VLAN protocol support for switchdev port
  net: dsa: ocelot: Add support for QinQ Operation

 drivers/net/dsa/ocelot/felix.c     | 123 +++++++++++++++++++++++++++++
 drivers/net/ethernet/mscc/ocelot.c |  39 +++++++--
 include/net/switchdev.h            |   1 +
 include/soc/mscc/ocelot.h          |   4 +
 net/bridge/br_switchdev.c          |  24 ++++++
 net/dsa/slave.c                    |  51 ++++++++----
 6 files changed, 221 insertions(+), 21 deletions(-)

-- 
2.17.1

[PATCH v6 1/3] net: dsa: Add protocol support for 802.1AD when adding or deleting vlan for dsa switch port

[hongbo.wang]

From: "hongbo.wang" <hongbo.wang@nxp.com>

the following command will be supported:

Set bridge's vlan protocol:
    ip link set br0 type bridge vlan_protocol 802.1ad
Add VLAN:
    ip link add link swp1 name swp1.100 type vlan protocol 802.1ad id 100
Delete VLAN:
    ip link del link swp1 name swp1.100

Signed-off-by: hongbo.wang <hongbo.wang@nxp.com>
---
 include/net/switchdev.h |  1 +
 net/dsa/slave.c         | 51 +++++++++++++++++++++++++++++------------
 2 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 53e8b4994296..d93532201ec2 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -97,6 +97,7 @@ struct switchdev_obj_port_vlan {
 	u16 flags;
 	u16 vid_begin;
 	u16 vid_end;
+	u16 proto;
 };
 
 #define SWITCHDEV_OBJ_PORT_VLAN(OBJ) \
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 66a5268398a5..731ab9e2aacc 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -328,6 +328,7 @@ static int dsa_slave_vlan_add(struct net_device *dev,
 	 * it doesn't make sense to program a PVID, so clear this flag.
 	 */
 	vlan.flags &= ~BRIDGE_VLAN_INFO_PVID;
+	vlan.proto = ETH_P_8021Q;
 
 	err = dsa_port_vlan_add(dp->cpu_dp, &vlan, trans);
 	if (err)
@@ -1229,18 +1230,46 @@ static int dsa_slave_get_ts_info(struct net_device *dev,
 	return ds->ops->get_ts_info(ds, p->dp->index, ts);
 }
 
+static bool dsa_slave_skip_vlan_configuration(struct dsa_port *dp,
+					      u16 vlan_proto, u16 vid)
+{
+	struct bridge_vlan_info info;
+	bool change_proto = false;
+	u16 br_proto = 0;
+	int ret;
+
+	/* when changing bridge's vlan protocol, it will change bridge
+	 * port's protocol firstly, then set bridge's protocol. if it's
+	 * changing vlan protocol, should not return -EBUSY.
+	 */
+	ret = br_vlan_get_proto(dp->bridge_dev, &br_proto);
+	if (ret == 0 && br_proto != vlan_proto)
+		change_proto = true;
+
+	/* br_vlan_get_info() returns -EINVAL or -ENOENT if the
+	 * device, respectively the VID is not found, returning
+	 * 0 means success, which is a failure for us here.
+	 */
+	ret = br_vlan_get_info(dp->bridge_dev, vid, &info);
+	if (ret == 0 && !change_proto)
+		return true;
+	else
+		return false;
+}
+
 static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
 				     u16 vid)
 {
 	struct dsa_port *dp = dsa_slave_to_port(dev);
+	u16 vlan_proto = ntohs(proto);
 	struct switchdev_obj_port_vlan vlan = {
 		.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
 		.vid_begin = vid,
 		.vid_end = vid,
 		/* This API only allows programming tagged, non-PVID VIDs */
 		.flags = 0,
+		.proto = vlan_proto,
 	};
-	struct bridge_vlan_info info;
 	struct switchdev_trans trans;
 	int ret;
 
@@ -1251,12 +1280,7 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
 		if (dsa_port_skip_vlan_configuration(dp))
 			return 0;
 
-		/* br_vlan_get_info() returns -EINVAL or -ENOENT if the
-		 * device, respectively the VID is not found, returning
-		 * 0 means success, which is a failure for us here.
-		 */
-		ret = br_vlan_get_info(dp->bridge_dev, vid, &info);
-		if (ret == 0)
+		if (dsa_slave_skip_vlan_configuration(dp, vlan_proto, vid))
 			return -EBUSY;
 	}
 
@@ -1271,6 +1295,8 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
 	if (ret)
 		return ret;
 
+	vlan.proto = ETH_P_8021Q;
+
 	/* And CPU port... */
 	trans.ph_prepare = true;
 	ret = dsa_port_vlan_add(dp->cpu_dp, &vlan, &trans);
@@ -1289,14 +1315,14 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
 				      u16 vid)
 {
 	struct dsa_port *dp = dsa_slave_to_port(dev);
+	u16 vlan_proto = ntohs(proto);
 	struct switchdev_obj_port_vlan vlan = {
 		.vid_begin = vid,
 		.vid_end = vid,
 		/* This API only allows programming tagged, non-PVID VIDs */
 		.flags = 0,
+		.proto = vlan_proto,
 	};
-	struct bridge_vlan_info info;
-	int ret;
 
 	/* Check for a possible bridge VLAN entry now since there is no
 	 * need to emulate the switchdev prepare + commit phase.
@@ -1305,12 +1331,7 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
 		if (dsa_port_skip_vlan_configuration(dp))
 			return 0;
 
-		/* br_vlan_get_info() returns -EINVAL or -ENOENT if the
-		 * device, respectively the VID is not found, returning
-		 * 0 means success, which is a failure for us here.
-		 */
-		ret = br_vlan_get_info(dp->bridge_dev, vid, &info);
-		if (ret == 0)
+		if (dsa_slave_skip_vlan_configuration(dp, vlan_proto, vid))
 			return -EBUSY;
 	}
 
-- 
2.17.1

[PATCH v6 2/3] net: switchdev: Add VLAN protocol support for switchdev port

[hongbo.wang]

From: "hongbo.wang" <hongbo.wang@nxp.com>

Add VLAN protocol support when adding or deleting VLAN for switchdev
port, get current bridge's VLAN protocol and pass it to port driver.

Signed-off-by: hongbo.wang <hongbo.wang@nxp.com>
---
 net/bridge/br_switchdev.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
index 015209bf44aa..7712da3e7912 100644
--- a/net/bridge/br_switchdev.c
+++ b/net/bridge/br_switchdev.c
@@ -146,6 +146,26 @@ br_switchdev_fdb_notify(const struct net_bridge_fdb_entry *fdb, int type)
 	}
 }
 
+static u16 br_switchdev_get_bridge_vlan_proto(const struct net_device *dev)
+{
+	const struct net_device *br = NULL;
+	u16 vlan_proto = ETH_P_8021Q;
+	struct net_bridge_port *p;
+
+	if (netif_is_bridge_master(dev)) {
+		br = dev;
+	} else {
+		p = br_port_get_rtnl_rcu(dev);
+		if (p)
+			br = p->br->dev;
+	}
+
+	if (br)
+		br_vlan_get_proto(br, &vlan_proto);
+
+	return vlan_proto;
+}
+
 int br_switchdev_port_vlan_add(struct net_device *dev, u16 vid, u16 flags,
 			       struct netlink_ext_ack *extack)
 {
@@ -157,6 +177,8 @@ int br_switchdev_port_vlan_add(struct net_device *dev, u16 vid, u16 flags,
 		.vid_end = vid,
 	};
 
+	v.proto = br_switchdev_get_bridge_vlan_proto(dev);
+
 	return switchdev_port_obj_add(dev, &v.obj, extack);
 }
 
@@ -169,5 +191,7 @@ int br_switchdev_port_vlan_del(struct net_device *dev, u16 vid)
 		.vid_end = vid,
 	};
 
+	v.proto = br_switchdev_get_bridge_vlan_proto(dev);
+
 	return switchdev_port_obj_del(dev, &v.obj);
 }
-- 
2.17.1

[PATCH v6 3/3] net: dsa: ocelot: Add support for QinQ Operation

[hongbo.wang]

From: "hongbo.wang" <hongbo.wang@nxp.com>

This feature can be test in the following case:
Customer <-----> swp0  <-----> swp1 <-----> ISP

Customer will send and receive packets with single VLAN tag(CTAG),
ISP will send and receive packets with double VLAN tag(STAG and CTAG).
This refers to "4.3.3 Provider Bridges and Q-in-Q Operation" in
VSC99599_1_00_TS.pdf.

The related test commands:
1.
devlink dev param set pci/0000:00:00.5 name qinq_port_bitmap \
value 2 cmode runtime
2.
ip link add dev br0 type bridge vlan_protocol 802.1ad
ip link set dev swp0 master br0
ip link set dev swp1 master br0
ip link set dev br0 type bridge vlan_filtering 1
3.
bridge vlan del dev swp0 vid 1 pvid
bridge vlan add dev swp0 vid 100 pvid untagged
bridge vlan add dev swp1 vid 100
Result:
Customer(tpid:8100 vid:111) -> swp0 -> swp1 -> ISP(STAG \
            tpid:88A8 vid:100, CTAG tpid:8100 vid:111)
ISP(tpid:88A8 vid:100 tpid:8100 vid:222) -> swp1 -> swp0 ->\
            Customer(tpid:8100 vid:222)

Signed-off-by: hongbo.wang <hongbo.wang@nxp.com>
---
 drivers/net/dsa/ocelot/felix.c     | 123 +++++++++++++++++++++++++++++
 drivers/net/ethernet/mscc/ocelot.c |  39 +++++++--
 include/soc/mscc/ocelot.h          |   4 +
 3 files changed, 160 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index a1e1d3824110..5888b0fa5669 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -148,9 +148,26 @@ static void felix_vlan_add(struct dsa_switch *ds, int port,
 				vid, port, err);
 			return;
 		}
+
+		if (vlan->proto == ETH_P_8021AD) {
+			if (!ocelot->qinq_enable) {
+				ocelot->qinq_enable = true;
+				kref_init(&ocelot->qinq_refcount);
+			} else {
+				kref_get(&ocelot->qinq_refcount);
+			}
+		}
 	}
 }
 
+static void felix_vlan_qinq_release(struct kref *ref)
+{
+	struct ocelot *ocelot;
+
+	ocelot = container_of(ref, struct ocelot, qinq_refcount);
+	ocelot->qinq_enable = false;
+}
+
 static int felix_vlan_del(struct dsa_switch *ds, int port,
 			  const struct switchdev_obj_port_vlan *vlan)
 {
@@ -165,6 +182,9 @@ static int felix_vlan_del(struct dsa_switch *ds, int port,
 				vid, port, err);
 			return err;
 		}
+
+		if (ocelot->qinq_enable && vlan->proto == ETH_P_8021AD)
+			kref_put(&ocelot->qinq_refcount, felix_vlan_qinq_release);
 	}
 	return 0;
 }
@@ -173,9 +193,13 @@ static int felix_port_enable(struct dsa_switch *ds, int port,
 			     struct phy_device *phy)
 {
 	struct ocelot *ocelot = ds->priv;
+	struct net_device *slave;
 
 	ocelot_port_enable(ocelot, port, phy);
 
+	slave = dsa_to_port(ds, port)->slave;
+	slave->features |= NETIF_F_HW_VLAN_STAG_FILTER;
+
 	return 0;
 }
 
@@ -555,6 +579,97 @@ static struct ptp_clock_info ocelot_ptp_clock_info = {
 	.enable		= ocelot_ptp_enable,
 };
 
+static int felix_qinq_port_bitmap_get(struct dsa_switch *ds, u32 *bitmap)
+{
+	struct ocelot *ocelot = ds->priv;
+	struct ocelot_port *ocelot_port;
+	int port;
+
+	*bitmap = 0;
+	for (port = 0; port < ds->num_ports; port++) {
+		ocelot_port = ocelot->ports[port];
+		if (ocelot_port->qinq_mode)
+			*bitmap |= 0x01 << port;
+	}
+
+	return 0;
+}
+
+static int felix_qinq_port_bitmap_set(struct dsa_switch *ds, u32 bitmap)
+{
+	struct ocelot *ocelot = ds->priv;
+	struct ocelot_port *ocelot_port;
+	int port;
+
+	for (port = 0; port < ds->num_ports; port++) {
+		ocelot_port = ocelot->ports[port];
+		if (bitmap & (0x01 << port))
+			ocelot_port->qinq_mode = true;
+		else
+			ocelot_port->qinq_mode = false;
+	}
+
+	return 0;
+}
+
+enum felix_devlink_param_id {
+	FELIX_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
+	FELIX_DEVLINK_PARAM_ID_QINQ_PORT_BITMAP,
+};
+
+static int felix_devlink_param_get(struct dsa_switch *ds, u32 id,
+				   struct devlink_param_gset_ctx *ctx)
+{
+	int err;
+
+	switch (id) {
+	case FELIX_DEVLINK_PARAM_ID_QINQ_PORT_BITMAP:
+		err = felix_qinq_port_bitmap_get(ds, &ctx->val.vu32);
+		break;
+	default:
+		err = -EOPNOTSUPP;
+		break;
+	}
+
+	return err;
+}
+
+static int felix_devlink_param_set(struct dsa_switch *ds, u32 id,
+				   struct devlink_param_gset_ctx *ctx)
+{
+	int err;
+
+	switch (id) {
+	case FELIX_DEVLINK_PARAM_ID_QINQ_PORT_BITMAP:
+		err = felix_qinq_port_bitmap_set(ds, ctx->val.vu32);
+		break;
+	default:
+		err = -EOPNOTSUPP;
+		break;
+	}
+
+	return err;
+}
+
+static const struct devlink_param felix_devlink_params[] = {
+	DSA_DEVLINK_PARAM_DRIVER(FELIX_DEVLINK_PARAM_ID_QINQ_PORT_BITMAP,
+				 "qinq_port_bitmap",
+				 DEVLINK_PARAM_TYPE_U32,
+				 BIT(DEVLINK_PARAM_CMODE_RUNTIME)),
+};
+
+static int felix_setup_devlink_params(struct dsa_switch *ds)
+{
+	return dsa_devlink_params_register(ds, felix_devlink_params,
+					   ARRAY_SIZE(felix_devlink_params));
+}
+
+static void felix_teardown_devlink_params(struct dsa_switch *ds)
+{
+	dsa_devlink_params_unregister(ds, felix_devlink_params,
+				      ARRAY_SIZE(felix_devlink_params));
+}
+
 /* Hardware initialization done here so that we can allocate structures with
  * devm without fear of dsa_register_switch returning -EPROBE_DEFER and causing
  * us to allocate structures twice (leak memory) and map PCI memory twice
@@ -614,6 +729,10 @@ static int felix_setup(struct dsa_switch *ds)
 	ds->mtu_enforcement_ingress = true;
 	ds->configure_vlan_while_not_filtering = true;
 
+	err = felix_setup_devlink_params(ds);
+	if (err < 0)
+		return err;
+
 	return 0;
 }
 
@@ -625,6 +744,8 @@ static void felix_teardown(struct dsa_switch *ds)
 	if (felix->info->mdio_bus_free)
 		felix->info->mdio_bus_free(ocelot);
 
+	felix_teardown_devlink_params(ds);
+
 	ocelot_deinit_timestamp(ocelot);
 	/* stop workqueue thread */
 	ocelot_deinit(ocelot);
@@ -798,6 +919,8 @@ const struct dsa_switch_ops felix_switch_ops = {
 	.cls_flower_del		= felix_cls_flower_del,
 	.cls_flower_stats	= felix_cls_flower_stats,
 	.port_setup_tc          = felix_port_setup_tc,
+	.devlink_param_get	= felix_devlink_param_get,
+	.devlink_param_set	= felix_devlink_param_set,
 };
 
 static int __init felix_init(void)
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 5abb7d2b0a9e..9186501cef03 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -143,6 +143,8 @@ static int ocelot_port_set_native_vlan(struct ocelot *ocelot, int port,
 				       u16 vid)
 {
 	struct ocelot_port *ocelot_port = ocelot->ports[port];
+	u32 port_tpid = 0;
+	u32 tag_tpid = 0;
 	u32 val = 0;
 
 	if (ocelot_port->vid != vid) {
@@ -156,8 +158,14 @@ static int ocelot_port_set_native_vlan(struct ocelot *ocelot, int port,
 		ocelot_port->vid = vid;
 	}
 
-	ocelot_rmw_gix(ocelot, REW_PORT_VLAN_CFG_PORT_VID(vid),
-		       REW_PORT_VLAN_CFG_PORT_VID_M,
+	if (ocelot->qinq_enable && ocelot_port->qinq_mode)
+		port_tpid = REW_PORT_VLAN_CFG_PORT_TPID(ETH_P_8021AD);
+	else
+		port_tpid = REW_PORT_VLAN_CFG_PORT_TPID(ETH_P_8021Q);
+
+	ocelot_rmw_gix(ocelot, REW_PORT_VLAN_CFG_PORT_VID(vid) | port_tpid,
+		       REW_PORT_VLAN_CFG_PORT_VID_M |
+		       REW_PORT_VLAN_CFG_PORT_TPID_M,
 		       REW_PORT_VLAN_CFG, port);
 
 	if (ocelot_port->vlan_aware && !ocelot_port->vid)
@@ -180,12 +188,18 @@ static int ocelot_port_set_native_vlan(struct ocelot *ocelot, int port,
 		else
 			/* Tag all frames */
 			val = REW_TAG_CFG_TAG_CFG(3);
+
+		if (ocelot->qinq_enable && ocelot_port->qinq_mode)
+			tag_tpid = REW_TAG_CFG_TAG_TPID_CFG(1);
+		else
+			tag_tpid = REW_TAG_CFG_TAG_TPID_CFG(0);
 	} else {
 		/* Port tagging disabled. */
 		val = REW_TAG_CFG_TAG_CFG(0);
+		tag_tpid = REW_TAG_CFG_TAG_TPID_CFG(0);
 	}
-	ocelot_rmw_gix(ocelot, val,
-		       REW_TAG_CFG_TAG_CFG_M,
+	ocelot_rmw_gix(ocelot, val | tag_tpid,
+		       REW_TAG_CFG_TAG_CFG_M | REW_TAG_CFG_TAG_TPID_CFG_M,
 		       REW_TAG_CFG, port);
 
 	return 0;
@@ -204,6 +218,15 @@ void ocelot_port_vlan_filtering(struct ocelot *ocelot, int port,
 		      ANA_PORT_VLAN_CFG_VLAN_POP_CNT(1);
 	else
 		val = 0;
+
+	/* if switch is enabled for QinQ, the port for LAN should set
+	 * VLAN_CFG.VLAN_POP_CNT=0 && VLAN_CFG.VLAN_AWARE_ENA=0.
+	 * the port for MAN should set VLAN_CFG.VLAN_POP_CNT=1 &&
+	 * VLAN_CFG.VLAN_AWARE_ENA=1. referring to 4.3.3 in VSC9959_1_00_TS.pdf
+	 */
+	if (ocelot->qinq_enable && !ocelot_port->qinq_mode)
+		val = 0;
+
 	ocelot_rmw_gix(ocelot, val,
 		       ANA_PORT_VLAN_CFG_VLAN_AWARE_ENA |
 		       ANA_PORT_VLAN_CFG_VLAN_POP_CNT_M,
@@ -217,10 +240,14 @@ EXPORT_SYMBOL(ocelot_port_vlan_filtering);
 static void ocelot_port_set_pvid(struct ocelot *ocelot, int port, u16 pvid)
 {
 	struct ocelot_port *ocelot_port = ocelot->ports[port];
+	u32 tag_type = 0;
+
+	if (ocelot->qinq_enable && ocelot_port->qinq_mode)
+		tag_type = ANA_PORT_VLAN_CFG_VLAN_TAG_TYPE;
 
 	ocelot_rmw_gix(ocelot,
-		       ANA_PORT_VLAN_CFG_VLAN_VID(pvid),
-		       ANA_PORT_VLAN_CFG_VLAN_VID_M,
+		       ANA_PORT_VLAN_CFG_VLAN_VID(pvid) | tag_type,
+		       ANA_PORT_VLAN_CFG_VLAN_VID_M | tag_type,
 		       ANA_PORT_VLAN_CFG, port);
 
 	ocelot_port->pvid = pvid;
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index da369b12005f..8d0f9f9ec0b2 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -556,6 +556,7 @@ struct ocelot_port {
 	struct regmap			*target;
 
 	bool				vlan_aware;
+	bool				qinq_mode;
 
 	/* Ingress default VLAN (pvid) */
 	u16				pvid;
@@ -632,6 +633,9 @@ struct ocelot {
 	/* Protects the PTP clock */
 	spinlock_t			ptp_clock_lock;
 	struct ptp_pin_desc		ptp_pins[OCELOT_PTP_PINS_NUM];
+
+	bool				qinq_enable;
+	struct kref			qinq_refcount;
 };
 
 struct ocelot_policer {
-- 
2.17.1

Re: [PATCH v6 3/3] net: dsa: ocelot: Add support for QinQ Operation

[Vladimir Oltean]

Hi Hongbo,

On Wed, Sep 16, 2020 at 05:48:45PM +0800, hongbo.wang@nxp.com wrote:
> From: "hongbo.wang" <hongbo.wang@nxp.com>
>
> This feature can be test in the following case:
> Customer <-----> swp0  <-----> swp1 <-----> ISP
>
> Customer will send and receive packets with single VLAN tag(CTAG),
> ISP will send and receive packets with double VLAN tag(STAG and CTAG).
> This refers to "4.3.3 Provider Bridges and Q-in-Q Operation" in
> VSC99599_1_00_TS.pdf.
>
> The related test commands:
> 1.
> devlink dev param set pci/0000:00:00.5 name qinq_port_bitmap \
> value 2 cmode runtime
> 2.
> ip link add dev br0 type bridge vlan_protocol 802.1ad
> ip link set dev swp0 master br0
> ip link set dev swp1 master br0
> ip link set dev br0 type bridge vlan_filtering 1
> 3.
> bridge vlan del dev swp0 vid 1 pvid
> bridge vlan add dev swp0 vid 100 pvid untagged
> bridge vlan add dev swp1 vid 100
> Result:
> Customer(tpid:8100 vid:111) -> swp0 -> swp1 -> ISP(STAG \
>             tpid:88A8 vid:100, CTAG tpid:8100 vid:111)
> ISP(tpid:88A8 vid:100 tpid:8100 vid:222) -> swp1 -> swp0 ->\
>             Customer(tpid:8100 vid:222)
>
> Signed-off-by: hongbo.wang <hongbo.wang@nxp.com>
> ---

Can you please explain what is the purpose of the devlink parameter
command? As far as I understand, the commands from step 2 and 3 should
behave like that, even without running the command at step 1.

Thanks,
-Vladimir

RE: [EXT] Re: [PATCH v6 3/3] net: dsa: ocelot: Add support for QinQ Operation

[Hongbo Wang]

Hi Vladimir,

> -----Original Message-----
> From: Vladimir Oltean <olteanv@gmail.com>
> Sent: 2020年9月16日 18:00
> To: Hongbo Wang <hongbo.wang@nxp.com>
> Cc: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>; Po Liu <po.liu@nxp.com>;
> Mingkai Hu <mingkai.hu@nxp.com>; allan.nielsen@microchip.com; Claudiu
> Manoil <claudiu.manoil@nxp.com>; Alexandru Marginean
> <alexandru.marginean@nxp.com>; Vladimir Oltean
> <vladimir.oltean@nxp.com>; Leo Li <leoyang.li@nxp.com>; andrew@lunn.ch;
> f.fainelli@gmail.com; vivien.didelot@gmail.com; davem@davemloft.net;
> jiri@resnulli.us; idosch@idosch.org; kuba@kernel.org;
> vinicius.gomes@intel.com; nikolay@cumulusnetworks.com;
> roopa@cumulusnetworks.com; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org; horatiu.vultur@microchip.com;
> alexandre.belloni@bootlin.com; UNGLinuxDriver@microchip.com;
> ivecera@redhat.com
> Subject: [EXT] Re: [PATCH v6 3/3] net: dsa: ocelot: Add support for QinQ
> Operation
> 
> Caution: EXT Email
> 
> Hi Hongbo,
> 
> On Wed, Sep 16, 2020 at 05:48:45PM +0800, hongbo.wang@nxp.com wrote:
> > From: "hongbo.wang" <hongbo.wang@nxp.com>
> >
> > This feature can be test in the following case:
> > Customer <-----> swp0  <-----> swp1 <-----> ISP
> >
> > Customer will send and receive packets with single VLAN tag(CTAG), ISP
> > will send and receive packets with double VLAN tag(STAG and CTAG).
> > This refers to "4.3.3 Provider Bridges and Q-in-Q Operation" in
> > VSC99599_1_00_TS.pdf.
> >
> > The related test commands:
> > 1.
> > devlink dev param set pci/0000:00:00.5 name qinq_port_bitmap \ value 2
> > cmode runtime 2.
> > ip link add dev br0 type bridge vlan_protocol 802.1ad ip link set dev
> > swp0 master br0 ip link set dev swp1 master br0 ip link set dev br0
> > type bridge vlan_filtering 1 3.
> > bridge vlan del dev swp0 vid 1 pvid
> > bridge vlan add dev swp0 vid 100 pvid untagged bridge vlan add dev
> > swp1 vid 100
> > Result:
> > Customer(tpid:8100 vid:111) -> swp0 -> swp1 -> ISP(STAG \
> >             tpid:88A8 vid:100, CTAG tpid:8100 vid:111)
> > ISP(tpid:88A8 vid:100 tpid:8100 vid:222) -> swp1 -> swp0 ->\
> >             Customer(tpid:8100 vid:222)
> >
> > Signed-off-by: hongbo.wang <hongbo.wang@nxp.com>
> > ---
> 
> Can you please explain what is the purpose of the devlink parameter command?
> As far as I understand, the commands from step 2 and 3 should behave like
> that, even without running the command at step 1.

if swp0 connects with customer, and swp1 connects with ISP, According to the VSC99599_1_00_TS.pdf,
swp0 and swp1 will have different VLAN_POP_CNT && VLAN_AWARE_ENA, 

swp0 should set VLAN_CFG.VLAN_POP_CNT=0 && VLAN_CFG.VLAN_AWARE_ENA=0
swp1 should set VLAN_CFG.VLAN_POP_CNT=1 && VLAN_CFG.VLAN_AWARE_ENA=1

but when set vlan_filter=1, current code will set same value for both swp0 and swp1, 
for compatibility with existing code(802.1Q mode), so add devlink to set swp0 and swp1 into different modes.

Thanks,
hongbo

Re: [EXT] Re: [PATCH v6 3/3] net: dsa: ocelot: Add support for QinQ Operation

[Vladimir Oltean]

On Wed, Sep 16, 2020 at 10:28:38AM +0000, Hongbo Wang wrote:
> Hi Vladimir,
>
> if swp0 connects with customer, and swp1 connects with ISP, According
> to the VSC99599_1_00_TS.pdf, swp0 and swp1 will have different
> VLAN_POP_CNT && VLAN_AWARE_ENA,
>
> swp0 should set VLAN_CFG.VLAN_POP_CNT=0 && VLAN_CFG.VLAN_AWARE_ENA=0
> swp1 should set VLAN_CFG.VLAN_POP_CNT=1 && VLAN_CFG.VLAN_AWARE_ENA=1
>
> but when set vlan_filter=1, current code will set same value for both
> swp0 and swp1, for compatibility with existing code(802.1Q mode), so
> add devlink to set swp0 and swp1 into different modes.

But if you make VLAN_CFG.VLAN_AWARE_ENA=0, does that mean the switch
will accept any 802.1ad VLAN, not only those configured in the VLAN
database of the bridge? Otherwise said, after running the commands
above, and I send a packet to swp0 having tpid:88A8 vid:101, then the
bridge should not accept it.

I might be wrong, but I thought that an 802.1ad bridge with
vlan_filtering=1 behaves the same as an 802.1q bridge, except that it
should filter VLANs using a different TPID (0x88a8 instead of 0x8100).
I don't think the driver, in the way you're configuring it, does that,
does it?

Thanks,
-Vladimir

RE: [EXT] Re: [PATCH v6 3/3] net: dsa: ocelot: Add support for QinQ Operation

[Hongbo Wang]

> On Wed, Sep 16, 2020 at 10:28:38AM +0000, Hongbo Wang wrote:
> > Hi Vladimir,
> >
> > if swp0 connects with customer, and swp1 connects with ISP, According
> > to the VSC99599_1_00_TS.pdf, swp0 and swp1 will have different
> > VLAN_POP_CNT && VLAN_AWARE_ENA,
> >
> > swp0 should set VLAN_CFG.VLAN_POP_CNT=0 &&
> VLAN_CFG.VLAN_AWARE_ENA=0
> > swp1 should set VLAN_CFG.VLAN_POP_CNT=1 &&
> VLAN_CFG.VLAN_AWARE_ENA=1
> >
> > but when set vlan_filter=1, current code will set same value for both
> > swp0 and swp1, for compatibility with existing code(802.1Q mode), so
> > add devlink to set swp0 and swp1 into different modes.
> 
> But if you make VLAN_CFG.VLAN_AWARE_ENA=0, does that mean the switch
> will accept any 802.1ad VLAN, not only those configured in the VLAN database
> of the bridge? Otherwise said, after running the commands above, and I send a
> packet to swp0 having tpid:88A8 vid:101, then the bridge should not accept it.
> 
> I might be wrong, but I thought that an 802.1ad bridge with
> vlan_filtering=1 behaves the same as an 802.1q bridge, except that it should
> filter VLANs using a different TPID (0x88a8 instead of 0x8100).
> I don't think the driver, in the way you're configuring it, does that, does it?

hi Vladimir,
you can refer to "4.3.3.0.1 MAN Access Switch Example" in VSC99599_1_00_TS.pdf,
By testing the case, if don't set VLAN_AWARE_ENA=0 for customer's port swp0,
the Q-in-Q feature can't work well.

In order to distinguish the port for customer and for ISP, I add devlink command,
Actually, I can modify the driver config directly, not using devlink, but it will be not compatible with current 
code and user guide. 

Thanks,
hongbo

Re: [EXT] Re: [PATCH v6 3/3] net: dsa: ocelot: Add support for QinQ Operation

[Vladimir Oltean]

Hi Hongbo,

On Thu, Sep 17, 2020 at 02:37:59AM +0000, Hongbo Wang wrote:
> > On Wed, Sep 16, 2020 at 10:28:38AM +0000, Hongbo Wang wrote:
> > > Hi Vladimir,
> > >
> > > if swp0 connects with customer, and swp1 connects with ISP, According
> > > to the VSC99599_1_00_TS.pdf, swp0 and swp1 will have different
> > > VLAN_POP_CNT && VLAN_AWARE_ENA,
> > >
> > > swp0 should set VLAN_CFG.VLAN_POP_CNT=0 &&
> > VLAN_CFG.VLAN_AWARE_ENA=0
> > > swp1 should set VLAN_CFG.VLAN_POP_CNT=1 &&
> > VLAN_CFG.VLAN_AWARE_ENA=1
> > >
> > > but when set vlan_filter=1, current code will set same value for both
> > > swp0 and swp1, for compatibility with existing code(802.1Q mode), so
> > > add devlink to set swp0 and swp1 into different modes.
> >
> > But if you make VLAN_CFG.VLAN_AWARE_ENA=0, does that mean the switch
> > will accept any 802.1ad VLAN, not only those configured in the VLAN database
> > of the bridge? Otherwise said, after running the commands above, and I send a
> > packet to swp0 having tpid:88A8 vid:101, then the bridge should not accept it.
> >
> > I might be wrong, but I thought that an 802.1ad bridge with
> > vlan_filtering=1 behaves the same as an 802.1q bridge, except that it should
> > filter VLANs using a different TPID (0x88a8 instead of 0x8100).
> > I don't think the driver, in the way you're configuring it, does that, does it?
>
> hi Vladimir,
> you can refer to "4.3.3.0.1 MAN Access Switch Example" in VSC99599_1_00_TS.pdf,
> By testing the case, if don't set VLAN_AWARE_ENA=0 for customer's port swp0,
> the Q-in-Q feature can't work well.
>
> In order to distinguish the port for customer and for ISP, I add devlink command,
> Actually, I can modify the driver config directly, not using devlink,
> but it will be not compatible with current code and user guide.

I asked this on the Microchip Support portal:

-----------------------------[cut here]-----------------------------

VLAN filtering only on specific TPID
------------------------------------

I would like to configure a port with the following behavior:
- The VLAN table should contain 802.1ad VLANs 1 and 10. VLAN ingress filtering
  should be enabled.
- An untagged frame on ingress should be classified to 802.1ad (TAG_TYPE=1)
  VLAN ID 1 (the port-based VLAN). The frame should be accepted because 802.1ad
  VLAN 1 is in the VLAN table.
- An ingress frame with 802.1Q (0x8100) header VLAN ID 100 should be classified
  to 802.1ad (TAG_TYPE=1) VLAN ID 1 (the port-based VLAN). The frame should be
  accepted because 802.1ad VLAN 1 is in the VLAN table.
- An ingress frame with 802.1ad (0x88a8) header VLAN ID 10 should be classified
  to 802.1ad (TAG_TYPE=1) VLAN ID 10. The frame should be accepted because
  802.1ad VLAN 10 is in the VLAN table.
- An ingress frame with 802.1ad (0x88a8) header VLAN ID 100 should be
  classified to 802.1ad (TAG_TYPE=1) VLAN ID 100. The frame should be dropped
  because 802.1ad VLAN 100 is not in the VLAN table.
How do I configure the switch to obtain this behavior? This is not what the
"Provider Bridges and Q-in-Q Operation" chapter in the reference manual is
explaining how to do. Instead, that chapter suggests to make
VLAN_CFG.VLAN_AWARE_ENA = 0. But I don't want to do this, because I need to be
able to drop the frames with 802.1ad VLAN ID 100 in the example above.

-----------------------------[cut here]-----------------------------

Judging from the fact that I received no answer whatsoever, I can only
deduce that offloading an 8021ad bridge, at least one that has the
semantics that Toshiaki Makita described here,
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=1a0b20b257326523ec2a6cb51dd6f26ef179eb84
is not possible with this hardware.

So I think there's little left to do here.

If it helps, I am fairly certain that the sja1105 can offer the
requested services, if you play a little bit with the TPID and TPID2
values. Maybe that's a path forward for your patches, if you still want
to add the generic support in switchdev and in DSA.

Thanks,
-Vladimir

RE: [EXT] Re: [PATCH v6 3/3] net: dsa: ocelot: Add support for QinQ Operation

[Hongbo Wang]

Hi Vladimir,

> 
> I asked this on the Microchip Support portal:
> 
> -----------------------------[cut here]-----------------------------
> 
> VLAN filtering only on specific TPID
> ------------------------------------
> 
> I would like to configure a port with the following behavior:
> - The VLAN table should contain 802.1ad VLANs 1 and 10. VLAN ingress
> filtering
>   should be enabled.
> - An untagged frame on ingress should be classified to 802.1ad (TAG_TYPE=1)
>   VLAN ID 1 (the port-based VLAN). The frame should be accepted because
> 802.1ad
>   VLAN 1 is in the VLAN table.
> - An ingress frame with 802.1Q (0x8100) header VLAN ID 100 should be
> classified
>   to 802.1ad (TAG_TYPE=1) VLAN ID 1 (the port-based VLAN). The frame
> should be
>   accepted because 802.1ad VLAN 1 is in the VLAN table.
> - An ingress frame with 802.1ad (0x88a8) header VLAN ID 10 should be
> classified
>   to 802.1ad (TAG_TYPE=1) VLAN ID 10. The frame should be accepted
> because
>   802.1ad VLAN 10 is in the VLAN table.
> - An ingress frame with 802.1ad (0x88a8) header VLAN ID 100 should be
>   classified to 802.1ad (TAG_TYPE=1) VLAN ID 100. The frame should be
> dropped
>   because 802.1ad VLAN 100 is not in the VLAN table.
> How do I configure the switch to obtain this behavior? This is not what the
> "Provider Bridges and Q-in-Q Operation" chapter in the reference manual is
> explaining how to do. Instead, that chapter suggests to make
> VLAN_CFG.VLAN_AWARE_ENA = 0. But I don't want to do this, because I need
> to be able to drop the frames with 802.1ad VLAN ID 100 in the example above.
> 
> -----------------------------[cut here]-----------------------------
> 
> Judging from the fact that I received no answer whatsoever, I can only deduce
> that offloading an 8021ad bridge, at least one that has the semantics that
> Toshiaki Makita described here,
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kern
> el.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fnetdev%2Fnet-next.git%2F
> commit%2F%3Fid%3D1a0b20b257326523ec2a6cb51dd6f26ef179eb84&
> data=02%7C01%7Chongbo.wang%40nxp.com%7Cdcfd5e5df4e24455726408d
> 86c4f0493%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6373784
> 33917654330&sdata=kk8WnA0iH9E5yPrLKC8BwSInD6jqm0qGftJ8Jw1Etk
> 8%3D&reserved=0
> is not possible with this hardware.
> 
> So I think there's little left to do here.
> 
> If it helps, I am fairly certain that the sja1105 can offer the requested services,
> if you play a little bit with the TPID and TPID2 values. Maybe that's a path
> forward for your patches, if you still want to add the generic support in
> switchdev and in DSA.
> 

Thanks for your suggestion,
I will research the related code, and will optimize my patches.

Thanks,
hongbo

Re: [PATCH v6 0/3] Add 802.1AD protocol support for dsa switch and ocelot driver

[David Miller]

From: hongbo.wang@nxp.com
Date: Wed, 16 Sep 2020 17:48:42 +0800

> 1. Overview 
> a) 0001* is for support to set dsa slave into 802.1AD(QinQ) mode.
> b) 0002* is for vlan_proto support for br_switchdev_port_vlan_add and br_switchdev_port_vlan_del.
> c) 0003* is for setting QinQ related registers in ocelot switch driver, after applying this patch, the switch(VSC99599)'s port can enable or disable QinQ mode.

You're going to have to update every single SWITCHDEV_PORT_ADD_OBJ handler
and subsequent helpers to check the validate the protocol value.

You also are going to have to make sure that every instantiated
switchdev_obj_port_vlan object initializes the vlan protocol field
properly.

Basically, now that this structure has a new member, everything that
operates on that object must be updated to handle the new protocol
value.

And I do mean everything.

You can't just add the protocol handling to the locations you care
about for bridging and DSA.

You also have to more fully address the feedback given by Vladimir
in patch #3.  Are the expectations on the user side a Linux based
expectation, or one specific about how this ASIC is expected to
behave by default.  It is very unclear what you are talking about
when you say customer and ISP etc.

Thanks.

Re: [PATCH v6 0/3] Add 802.1AD protocol support for dsa switch and ocelot driver

[Florian Fainelli]

On 9/18/2020 5:20 PM, David Miller wrote:
> From: hongbo.wang@nxp.com
> Date: Wed, 16 Sep 2020 17:48:42 +0800
> 
>> 1. Overview
>> a) 0001* is for support to set dsa slave into 802.1AD(QinQ) mode.
>> b) 0002* is for vlan_proto support for br_switchdev_port_vlan_add and br_switchdev_port_vlan_del.
>> c) 0003* is for setting QinQ related registers in ocelot switch driver, after applying this patch, the switch(VSC99599)'s port can enable or disable QinQ mode.
> 
> You're going to have to update every single SWITCHDEV_PORT_ADD_OBJ handler
> and subsequent helpers to check the validate the protocol value.
> 
> You also are going to have to make sure that every instantiated
> switchdev_obj_port_vlan object initializes the vlan protocol field
> properly.
> 
> Basically, now that this structure has a new member, everything that
> operates on that object must be updated to handle the new protocol
> value.
> 
> And I do mean everything.
> 
> You can't just add the protocol handling to the locations you care
> about for bridging and DSA.
> 
> You also have to more fully address the feedback given by Vladimir
> in patch #3.  Are the expectations on the user side a Linux based
> expectation, or one specific about how this ASIC is expected to
> behave by default.  It is very unclear what you are talking about
> when you say customer and ISP etc.

Switch vendors like to refer to the outer tag as ISP VLAN tag and inner 
tag as customer VLAN tag, sometimes S-TAG and C-TAG terms are used, too...
-- 
Florian

RE: [EXT] Re: [PATCH v6 0/3] Add 802.1AD protocol support for dsa switch and ocelot driver

[Hongbo Wang]

> You're going to have to update every single SWITCHDEV_PORT_ADD_OBJ
> handler and subsequent helpers to check the validate the protocol value.
> 
> You also are going to have to make sure that every instantiated
> switchdev_obj_port_vlan object initializes the vlan protocol field properly.
> 
> Basically, now that this structure has a new member, everything that operates
> on that object must be updated to handle the new protocol value.
> 
> And I do mean everything.
> 
> You can't just add the protocol handling to the locations you care about for
> bridging and DSA.
> 
> You also have to more fully address the feedback given by Vladimir in patch #3.
> Are the expectations on the user side a Linux based expectation, or one specific
> about how this ASIC is expected to behave by default.  It is very unclear what
> you are talking about when you say customer and ISP etc.
> 

Hi David,

Thanks for your comments.

I understand your concerns, I will review the code.
Generally, the packets from customer port will have only C-TAG, but the packets form ISP port will have both S-TAG and C-TAG. For ocelot switch, these two ports have different register config.


Thanks
hongbo