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

2020-10-09 Thread 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
> 33917654330sdata=kk8WnA0iH9E5yPrLKC8BwSInD6jqm0qGftJ8Jw1Etk
> 8%3Dreserved=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: [EXT] Re: [PATCH v6 0/3] Add 802.1AD protocol support for dsa switch and ocelot driver

2020-09-20 Thread 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


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

2020-09-16 Thread 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

2020-09-16 Thread Hongbo Wang
Hi Vladimir,

> -Original Message-
> From: Vladimir Oltean 
> Sent: 2020年9月16日 18:00
> To: Hongbo Wang 
> Cc: Xiaoliang Yang ; Po Liu ;
> Mingkai Hu ; allan.niel...@microchip.com; Claudiu
> Manoil ; Alexandru Marginean
> ; Vladimir Oltean
> ; Leo Li ; and...@lunn.ch;
> f.faine...@gmail.com; vivien.dide...@gmail.com; da...@davemloft.net;
> j...@resnulli.us; ido...@idosch.org; k...@kernel.org;
> vinicius.go...@intel.com; niko...@cumulusnetworks.com;
> ro...@cumulusnetworks.com; net...@vger.kernel.org;
> linux-kernel@vger.kernel.org; horatiu.vul...@microchip.com;
> alexandre.bell...@bootlin.com; unglinuxdri...@microchip.com;
> ivec...@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.w...@nxp.com wrote:
> > From: "hongbo.wang" 
> >
> > 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/: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 
> > ---
> 
> 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



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

2020-09-16 Thread hongbo . wang
From: "hongbo.wang" 

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/: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 
---
 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(>qinq_refcount);
+   } else {
+   kref_get(>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(>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, >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, 

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

2020-09-16 Thread hongbo . wang
From: "hongbo.wang" 

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 
---
 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, , 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, _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, );
+   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, );
-   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, , );
@@ -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, 

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

2020-09-16 Thread hongbo . wang
From: "hongbo.wang" 

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 2/3] net: switchdev: Add VLAN protocol support for switchdev port

2020-09-16 Thread hongbo . wang
From: "hongbo.wang" 

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 
---
 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, _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, , 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, );
 }
-- 
2.17.1



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

2020-08-07 Thread hongbo . wang
From: "hongbo.wang" 

1. the patch 0001* is for setting single port into 802.1AD(QinQ) mode,
before this patch, the function dsa_slave_vlan_rx_add_vid didn't pass 
the parameter "proto" to next port level, so switch's port can't get
parameter "proto"
  after applying this patch, the following command can be supported:
  ip link set br0 type bridge vlan_protocol 802.1ad
  ip link add link swp1 name swp1.100 type vlan protocol 802.1ad id 100

2. the patch 0002* 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.

3. Version log
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 (2):
  net: dsa: Add protocol support for 802.1AD when adding or  deleting
vlan for dsa switch port
  net: dsa: ocelot: Add support for QinQ Operation

 drivers/net/dsa/ocelot/felix.c | 124 +
 drivers/net/ethernet/mscc/ocelot.c |  40 --
 include/net/switchdev.h|   1 +
 include/soc/mscc/ocelot.h  |   4 +
 net/bridge/br_switchdev.c  |  22 +
 net/dsa/dsa_priv.h |   4 +-
 net/dsa/port.c |   6 +-
 net/dsa/slave.c|  53 +++-
 net/dsa/tag_8021q.c|   4 +-
 9 files changed, 228 insertions(+), 30 deletions(-)

-- 
2.17.1



[PATCH v5 2/2] net: dsa: ocelot: Add support for QinQ Operation

2020-08-07 Thread hongbo . wang
From: "hongbo.wang" 

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/:00:00.5 name qinq_port_bitmap \
value 2 cmode runtime

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

2.
ip link set dev br0 type bridge vlan_filtering 1
bridge vlan add dev swp0 vid 100 pvid
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)

3.
bridge vlan del dev swp0 vid 1 pvid
bridge vlan add dev swp0 vid 100 pvid untagged
Result:
ISP(tpid:88A8 vid:100 tpid:8100 vid:222) -> swp1 -> swp0 ->\
Customer(tpid:8100 vid:222)

Signed-off-by: hongbo.wang 
---
 drivers/net/dsa/ocelot/felix.c | 124 +
 drivers/net/ethernet/mscc/ocelot.c |  40 --
 include/soc/mscc/ocelot.h  |   4 +
 3 files changed, 162 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index c69d9592a2b7..f9d50af4be65 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -147,9 +147,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(>qinq_refcount);
+   } else {
+   kref_get(>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)
 {
@@ -164,7 +181,11 @@ 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(>qinq_refcount, 
felix_vlan_qinq_release);
}
+
return 0;
 }
 
@@ -172,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;
 }
 
@@ -568,6 +593,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, >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 

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

2020-08-07 Thread hongbo . wang
From: "hongbo.wang" 

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 
---
 include/net/switchdev.h   |  1 +
 net/bridge/br_switchdev.c | 22 
 net/dsa/dsa_priv.h|  4 +--
 net/dsa/port.c|  6 +++--
 net/dsa/slave.c   | 53 ++-
 net/dsa/tag_8021q.c   |  4 +--
 6 files changed, 66 insertions(+), 24 deletions(-)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index ff2246914301..7594ea82879f 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/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
index 015209bf44aa..bcfa00d6d5eb 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(struct net_device *dev)
+{
+   u16 vlan_proto = ETH_P_8021Q;
+   struct net_device *br = NULL;
+   struct net_bridge_port *p;
+
+   if (netif_is_bridge_master(dev)) {
+   br = dev;
+   } else if (netif_is_bridge_port(dev)) {
+   p = br_port_get_rcu(dev);
+   if (p && p->br)
+   br = p->br->dev;
+   }
+
+   if (br)
+   br_vlan_get_proto(br, _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,7 @@ 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, , extack);
 }
 
@@ -169,5 +190,6 @@ 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, );
 }
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 1653e3377cb3..52685b9875e5 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -164,8 +164,8 @@ int dsa_port_vlan_add(struct dsa_port *dp,
  struct switchdev_trans *trans);
 int dsa_port_vlan_del(struct dsa_port *dp,
  const struct switchdev_obj_port_vlan *vlan);
-int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 flags);
-int dsa_port_vid_del(struct dsa_port *dp, u16 vid);
+int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 proto, u16 flags);
+int dsa_port_vid_del(struct dsa_port *dp, u16 vid, u16 proto);
 int dsa_port_link_register_of(struct dsa_port *dp);
 void dsa_port_link_unregister_of(struct dsa_port *dp);
 extern const struct phylink_mac_ops dsa_port_phylink_mac_ops;
diff --git a/net/dsa/port.c b/net/dsa/port.c
index e23ece229c7e..c98bbac3980a 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -433,13 +433,14 @@ int dsa_port_vlan_del(struct dsa_port *dp,
return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, );
 }
 
-int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 flags)
+int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 proto, u16 flags)
 {
struct switchdev_obj_port_vlan vlan = {
.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
.flags = flags,
.vid_begin = vid,
.vid_end = vid,
+   .proto = proto,
};
struct switchdev_trans trans;
int err;
@@ -454,12 +455,13 @@ int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 
flags)
 }
 EXPORT_SYMBOL(dsa_port_vid_add);
 
-int dsa_port_vid_del(struct dsa_port *dp, u16 vid)
+int dsa_port_vid_del(struct dsa_port *dp, u16 vid, u16 proto)
 {
struct switchdev_obj_port_vlan vlan = {
.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
.vid_begin = vid,
.vid_end = vid,
+   .proto = proto,
};
 
return dsa_port_vlan_del(dp, );
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 41d60eeefdbd..f01deda00492 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, , trans);
if (err)
@@ -1229,11 +1230,38 @@ static int dsa_slave_get_ts_info(struct net_device *dev,
return ds->ops->get_ts_info(ds, p->dp->index, ts);
 

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

2020-08-05 Thread Hongbo Wang
> On 8/3/2020 11:36 PM, Hongbo Wang wrote:
> >>> + if (vlan->proto == ETH_P_8021AD) {
> >>> + ocelot->enable_qinq = true;
> >>> + ocelot_port->qinq_mode = true;
> >>> + }
> >>  ...
> >>> + if (vlan->proto == ETH_P_8021AD) {
> >>> + ocelot->enable_qinq = false;
> >>> + ocelot_port->qinq_mode = false;
> >>> + }
> >>> +
> >>
> >> I don't understand how this can work just by using a boolean to track
> >> the state.
> >>
> >> This won't work properly if you are handling multiple QinQ VLAN entries.
> >>
> >> Also, I need Andrew and Florian to review and ACK the DSA layer
> >> changes that add the protocol value to the device notifier block.
> >
> > Hi David,
> > Thanks for reply.
> >
> > When setting bridge's VLAN protocol to 802.1AD by the command "ip link
> > set br0 type bridge vlan_protocol 802.1ad", it will call
> > dsa_slave_vlan_rx_add(dev, proto, vid) for every port in the bridge,
> > the parameter vid is port's pvid 1, if pvid's proto is 802.1AD, I will
> > enable switch's enable_qinq, and the related port's qinq_mode,
> >
> > When there are multiple QinQ VLAN entries, If one VLAN's proto is 802.1AD,
> I will enable switch and the related port into QinQ mode.
> 
> The enabling appears fine, the problem is the disabling, the first 802.1AD 
> VLAN
> entry that gets deleted will lead to the port and switch no longer being in 
> QinQ
> mode, and this does not look intended.
> --
> Florian

When I try to add reference counter, I found that:
1.
the command "ip link set br0 type bridge vlan_protocol 802.1ad" call path is:
br_changelink -> __br_vlan_set_proto -> vlan_vid_add -> ... -> 
ndo_vlan_rx_add_vid -> dsa_slave_vlan_rx_add_vid(dev, proto, vid) -> 
felix_vlan_add

dsa_slave_vlan_rx_add_vid can pass correct protocol and vid(1) to ocelot driver.

vlan_vid_add is in net/8021q/vlan_core.c, it maintains a vid_list that stores 
the map of vid and protocol,
the function vlan_vid_info_get can read the map.

but when deleting bridge using "ip link del dev br0 type bridge", the call path 
is:
br_dev_delete -> ... -> br_switchdev_port_vlan_del -> ... -> 
dsa_slave_port_obj_del -> dsa_slave_vlan_del -> ... -> felix_vlan_del

br_switchdev_port_vlan_del is in net/bridge/br_switchdev.c, it didn't have the 
list for map vid and protocol,
so it can't pass correct protocol that corresponding with vid to ocelot driver.

2.
For ocelot QinQ case, the switch port linked to customer has different actions 
with the port for ISP,

uplink: Customer LAN(CTAG) -> swp0(vlan_aware:0 pop_cnt:0) -> swp1(add STAG) -> 
ISP MAN(STAG + CTAG)
downlink: ISP MAN(STAG + CTAG) -> swp1(vlan_aware:1 pop_cnt:1, pop STAG) -> 
swp0(only CTAG) -> Customer LAN

the different action is descripted in "4.3.3 Provider Bridges and Q-in-Q 
Operation" in VSC99599_1_00_TS.pdf

so I need a standard command to set swp0 and swp1 for different mode, 
but "ip link set br0 type bridge vlan_protocol 802.1ad" will set all ports into 
the same mode, it's not my intent.

3.
I thought some ways to resovle the above issue:
a. br_switchdev_port_vlan_del will pass default value ETH_P_8021Q, but don't 
care it in felix_vlan_del.
b. In felix_vlan_add and felix_vlan_del, only when vid is ocelot_port's pvid, 
it enable or disable switch's enable_qinq.
c. Maybe I can use devlink to set swp0 and swp1 into different mode.
d. let br_switchdev_port_vlan_del call vlan_vid_info_get to get protocol for 
vid, but vlan_vid_info_get is static in vlan_core.c, so this need to add 
related functions in br_switchdev.c.

Any comments is welcome!

Thanks
Hongbo



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

2020-08-04 Thread Hongbo Wang
> Caution: EXT Email
> 
> On 8/3/2020 11:36 PM, Hongbo Wang wrote:
> >>> + if (vlan->proto == ETH_P_8021AD) {
> >>> + ocelot->enable_qinq = true;
> >>> + ocelot_port->qinq_mode = true;
> >>> + }
> >>  ...
> >>> + if (vlan->proto == ETH_P_8021AD) {
> >>> + ocelot->enable_qinq = false;
> >>> + ocelot_port->qinq_mode = false;
> >>> + }
> >>> +
> >>
> >> I don't understand how this can work just by using a boolean to track
> >> the state.
> >>
> >> This won't work properly if you are handling multiple QinQ VLAN entries.
> >>
> >> Also, I need Andrew and Florian to review and ACK the DSA layer
> >> changes that add the protocol value to the device notifier block.
> >
> > Hi David,
> > Thanks for reply.
> >
> > When setting bridge's VLAN protocol to 802.1AD by the command "ip link
> > set br0 type bridge vlan_protocol 802.1ad", it will call
> > dsa_slave_vlan_rx_add(dev, proto, vid) for every port in the bridge,
> > the parameter vid is port's pvid 1, if pvid's proto is 802.1AD, I will
> > enable switch's enable_qinq, and the related port's qinq_mode,
> >
> > When there are multiple QinQ VLAN entries, If one VLAN's proto is 802.1AD,
> I will enable switch and the related port into QinQ mode.
> 
> The enabling appears fine, the problem is the disabling, the first 802.1AD 
> VLAN
> entry that gets deleted will lead to the port and switch no longer being in 
> QinQ
> mode, and this does not look intended.
> --
> Florian

Thanks, Florian

I will add reference counter.
When deleting VLAN entry, only if the counter is zero, I will set the switch to 
exit QinQ mode.

hongbo



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

2020-08-04 Thread Hongbo Wang
> > This featue can be test using network test tools
> 
> mispelled: feature, can be used to test network test tools? or can be used to
> exercise network test tool?

when testing this feature, I need network tool to send packet with VLAN tag(pcp 
proto and vid), I will change it to avoid ambiguity.

> 
> >   struct ocelot *ocelot = ds->priv;
> > + struct ocelot_port *ocelot_port = ocelot->ports[port];
> >   u16 vid;
> >   int err;
> >
> > + if (vlan->proto == ETH_P_8021AD) {
> > + ocelot->enable_qinq = false;
> > + ocelot_port->qinq_mode = false;
> > + }
> 
> You need the delete part to be reference counted, otherwise the first 802.1AD
> VLAN delete request that comes in, regardless of whether other 802.1AD VLAN
> entries are installed will disable qinq_mode and enable_qinq for the entire
> port and switch, that does not sound like what you want.

I will add reference count in next version.
maybe I can disable qinq_mode and enable_qinq only when deleting pvid 1, I will 
test and change it.

> 
> Is not ocelot->enable_qinq the logical or of all ocelo_port instances's
> qinq_mode as well?

enable_qinq is flag of switch, and qinq_mode is flag of single port,
if switch is in working in QinQ mode, some ports that linked to ISP networking 
should enable qinq_mode,
other ports that linked to customer networking don't need set qinq_mode. these 
two types of port have different action.

> >   else
> >   /* Tag all frames */
> >   val = REW_TAG_CFG_TAG_CFG(3);
> > +
> > + if (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);
> > + if (ocelot_port->qinq_mode) {
> > + if (ocelot_port->vid)
> > + val = REW_TAG_CFG_TAG_CFG(1);
> > + else
> > + val = REW_TAG_CFG_TAG_CFG(3);
> > +> +  tag_tpid = REW_TAG_CFG_TAG_TPID_CFG(1);
> 
> This is nearly the same branch as the one above, can you merge the conditions
> vlan_aware || qinq_mode and just set an appropriate TAG_CFG() register value
> based on either booleans?

this feature needs vlan_filter=1, so the branch if (vlan_filter == 0 && 
qinq_mode) can be deleted now.
I will optimize the related code.

> 
> Are you also not possibly missing a if (untaged || qinq_mode) check in
> ocelot_vlan_add() to call into ocelot_port_set_native_vlan()?

The qinq_mode action can be triggered by ocelot_port_vlan_filtering, so don't 
need if (untagged || qinq_mode).

Thanks!
Hongbo



RE: [EXT] Re: [PATCH v4 1/2] net: dsa: Add protocol support for 802.1AD when adding or deleting vlan for dsa switch port

2020-08-04 Thread Hongbo Wang
> You are adding a new member to the switchdev VLAN object, so you should
> make sure that all call paths creating and parsing that object get updated as
> well, for now, you are doing this solely within DSA which is probably
> reasonable if we assume proto is uninitialized and unused elsewhere, there is
> no change of functionality.

I will review the related code, and confirm it.

> [snip]
> > + ret = br_vlan_get_proto(dp->bridge_dev, _proto);
> > + if (ret == 0 && br_proto != vlan_proto)
> > + change_proto = true;
> 
> 
> This deserves a comment, because the change_proto variable is not really
> explaining what this is about, maybe more like "incompatible_proto" would?
> 
> First you query the VLAN protocol currently configured on the bridge master
> device, and if this VLAN protocol is different than the one being requested,
> then you treat this as an error. It might make sense to also print a message
> towards the user that the bridge device protocol should be changed, or that
> the bridge device should be removed and re-created accordingly.
> 
> Does it not work if we have a bridge currently configured with 802.1ad and a
> 802.1q VLAN programming request comes in? In premise it should, right?
> Likewise, if we had a 802.1ad bridge configured already and we want to
> configure a 802.1Q VLAN on a bridged port, there should be a way for this
> configuration to work.
> 
> And both cases, it ought to be possible to configure the switch in double
> tagged mode and just make sure that there is no S-tag being added unless
> requested.

Thanks for long reply.

When create a bridge br0, it's default protocol is 802.1Q, then add a port swp1 
to bridge, the bridge will call add_vlan add a default VLAN to swp1, its vid is 
pvid 1, the vlan's protocol is 802.1Q. the related command is:
# ip link add dev br0 type bridge
# ip link set dev swp1 master br0

After testing port's 802.1Q mode, If want to test 802.1AD mode, we can use the 
following command to set switch and its  ports into QinQ mode, 
# ip link set br0 type bridge vlan_protocol 802.1ad
it will call vlan_vid_add(dev, proto, vid) for every port, the parameter proto 
is 802.1AD and the vid is also 1, after that, it will set br->vlan_proto to 
802.1AD, the related code is __br_vlan_set_proto in br_vlan.c

vlan_vid_add will call dsa_slave_vlan_rx_add_vid.
But in dsa_slave_vlan_rx_add_vid, because we had added vlan 1 before, so 
br_vlan_get_info will return 0, then the function return -EBUSY, it can't call 
dsa_port_vid_add, so I add the code to check protocol changing.

I will add code to print the message for protocol changing.

> > + ret = br_vlan_get_proto(dp->bridge_dev, _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, );
> > - if (ret == 0)
> > + if (ret == 0 && !change_proto)
> >   return -EBUSY;
> 
> Since we are copying the same code than in the add_vid path, it might make
> sense to extract this to a helper function eventually.

I will extract the code to helper function.

> >   slave_dev->features = master->vlan_features | NETIF_F_HW_TC;
> >   if (ds->ops->port_vlan_add && ds->ops->port_vlan_del)
> > - slave_dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
> > + slave_dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER |
> > +
> NETIF_F_HW_VLAN_STAG_FILTER;
> 
> You cannot advertise this netdev feature for *all* DSA switch driver unless 
> you
> have verified that each DSA driver implementing
> port_vlan_add() will work correctly. Please assign this flag from within the
> ocelot driver for now.

I will change it in next version.

> 
> >
> >   if (enabled)
> > - return dsa_port_vid_add(dp, vid, flags);
> > + return dsa_port_vid_add(dp, vid, 0, flags);
> 
> Why not pass ETH_P_8021Q here to indicate we want a 802.1Q, not .AD
> configuration request?
> --
I will change it in next version.




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

2020-08-04 Thread Hongbo Wang
> > + if (vlan->proto == ETH_P_8021AD) {
> > + ocelot->enable_qinq = true;
> > + ocelot_port->qinq_mode = true;
> > + }
>  ...
> > + if (vlan->proto == ETH_P_8021AD) {
> > + ocelot->enable_qinq = false;
> > + ocelot_port->qinq_mode = false;
> > + }
> > +
> 
> I don't understand how this can work just by using a boolean to track the
> state.
> 
> This won't work properly if you are handling multiple QinQ VLAN entries.
> 
> Also, I need Andrew and Florian to review and ACK the DSA layer changes that
> add the protocol value to the device notifier block.

Hi David,
Thanks for reply.

When setting bridge's VLAN protocol to 802.1AD by the command "ip link set br0 
type bridge vlan_protocol 802.1ad", it will call dsa_slave_vlan_rx_add(dev, 
proto, vid) for every port in the bridge, the parameter vid is port's pvid 1,
if pvid's proto is 802.1AD, I will enable switch's enable_qinq, and the related 
port's qinq_mode,

When there are multiple QinQ VLAN entries, If one VLAN's proto is 802.1AD, I 
will enable switch and the related port into QinQ mode.

Thanks,
hongbo



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

2020-07-30 Thread hongbo . wang
From: "hongbo.wang" 

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 
---
 include/net/switchdev.h |  1 +
 net/dsa/dsa_priv.h  |  4 ++--
 net/dsa/port.c  |  6 --
 net/dsa/slave.c | 27 +--
 net/dsa/tag_8021q.c |  4 ++--
 5 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index ff2246914301..7594ea82879f 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/dsa_priv.h b/net/dsa/dsa_priv.h
index 1653e3377cb3..52685b9875e5 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -164,8 +164,8 @@ int dsa_port_vlan_add(struct dsa_port *dp,
  struct switchdev_trans *trans);
 int dsa_port_vlan_del(struct dsa_port *dp,
  const struct switchdev_obj_port_vlan *vlan);
-int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 flags);
-int dsa_port_vid_del(struct dsa_port *dp, u16 vid);
+int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 proto, u16 flags);
+int dsa_port_vid_del(struct dsa_port *dp, u16 vid, u16 proto);
 int dsa_port_link_register_of(struct dsa_port *dp);
 void dsa_port_link_unregister_of(struct dsa_port *dp);
 extern const struct phylink_mac_ops dsa_port_phylink_mac_ops;
diff --git a/net/dsa/port.c b/net/dsa/port.c
index e23ece229c7e..c98bbac3980a 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -433,13 +433,14 @@ int dsa_port_vlan_del(struct dsa_port *dp,
return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, );
 }
 
-int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 flags)
+int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 proto, u16 flags)
 {
struct switchdev_obj_port_vlan vlan = {
.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
.flags = flags,
.vid_begin = vid,
.vid_end = vid,
+   .proto = proto,
};
struct switchdev_trans trans;
int err;
@@ -454,12 +455,13 @@ int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 
flags)
 }
 EXPORT_SYMBOL(dsa_port_vid_add);
 
-int dsa_port_vid_del(struct dsa_port *dp, u16 vid)
+int dsa_port_vid_del(struct dsa_port *dp, u16 vid, u16 proto)
 {
struct switchdev_obj_port_vlan vlan = {
.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
.vid_begin = vid,
.vid_end = vid,
+   .proto = proto,
};
 
return dsa_port_vlan_del(dp, );
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 41d60eeefdbd..2a03da92af0a 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1233,7 +1233,10 @@ 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 bridge_vlan_info info;
+   bool change_proto = false;
+   u16 br_proto = 0;
int ret;
 
/* Check for a possible bridge VLAN entry now since there is no
@@ -1243,20 +1246,24 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device 
*dev, __be16 proto,
if (dsa_port_skip_vlan_configuration(dp))
return 0;
 
+   ret = br_vlan_get_proto(dp->bridge_dev, _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, );
-   if (ret == 0)
+   if (ret == 0 && !change_proto)
return -EBUSY;
}
 
-   ret = dsa_port_vid_add(dp, vid, 0);
+   ret = dsa_port_vid_add(dp, vid, vlan_proto, 0);
if (ret)
return ret;
 
-   ret = dsa_port_vid_add(dp->cpu_dp, vid, 0);
+   ret = dsa_port_vid_add(dp->cpu_dp, vid, 0, 0);
if (ret)
return ret;
 
@@ -1267,7 +1274,10 @@ 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 bridge_vlan_info info;
+   bool change_proto = false;
+   u16 br_proto = 0;
int ret;
 
/* Check for a possible bridge VLAN entry now since there is no
@@ -1277,19 

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

2020-07-30 Thread hongbo . wang
From: "hongbo.wang" 

1. the patch 0001* is for setting single port into 802.1AD(QinQ) mode,
before this patch, the function dsa_slave_vlan_rx_add_vid didn't pass 
the parameter "proto" to next port level, so switch's port can't get
parameter "proto"
  after applying this patch, the following command can be supported:
  ip link set br0 type bridge vlan_protocol 802.1ad
  ip link add link swp1 name swp1.100 type vlan protocol 802.1ad id 100

2. the patch 0002* 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.

3. Version log
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 (2):
  net: dsa: Add protocol support for 802.1AD when adding or   deleting
vlan for dsa switch port
  net: dsa: ocelot: Add support for QinQ Operation

 drivers/net/dsa/ocelot/felix.c | 12 +++
 drivers/net/ethernet/mscc/ocelot.c | 53 +-
 include/net/switchdev.h|  1 +
 include/soc/mscc/ocelot.h  |  2 ++
 net/dsa/dsa_priv.h |  4 +--
 net/dsa/port.c |  6 ++--
 net/dsa/slave.c| 27 +++
 net/dsa/tag_8021q.c|  4 +--
 8 files changed, 89 insertions(+), 20 deletions(-)

-- 
2.17.1



[PATCH v4 2/2] net: dsa: ocelot: Add support for QinQ Operation

2020-07-30 Thread hongbo . wang
From: "hongbo.wang" 

This featue can be test using network test tools
TX-tool -> swp0  -> swp1 -> RX-tool

TX-tool simulates Customer that will send and receive packets with single
VLAN tag(CTAG), RX-tool simulates Service-Provider that 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.
ip link add dev br0 type bridge
ip link set dev swp1 master br0
ip link set br0 type bridge vlan_protocol 802.1ad
ip link set dev swp0 master br0

2.
ip link set dev br0 type bridge vlan_filtering 1
bridge vlan add dev swp0 vid 100 pvid
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)

3.
bridge vlan del dev swp0 vid 1 pvid
bridge vlan add dev swp0 vid 100 pvid untagged
Result:
ISP(tpid:88A8 vid:100 tpid:8100 vid:222) -> swp1 -> swp0 ->\
Customer(tpid:8100 vid:222)

Signed-off-by: hongbo.wang 
---
 drivers/net/dsa/ocelot/felix.c | 12 +++
 drivers/net/ethernet/mscc/ocelot.c | 53 +-
 include/soc/mscc/ocelot.h  |  2 ++
 3 files changed, 59 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index c69d9592a2b7..72a27b61080e 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -131,10 +131,16 @@ static void felix_vlan_add(struct dsa_switch *ds, int 
port,
   const struct switchdev_obj_port_vlan *vlan)
 {
struct ocelot *ocelot = ds->priv;
+   struct ocelot_port *ocelot_port = ocelot->ports[port];
u16 flags = vlan->flags;
u16 vid;
int err;
 
+   if (vlan->proto == ETH_P_8021AD) {
+   ocelot->enable_qinq = true;
+   ocelot_port->qinq_mode = true;
+   }
+
if (dsa_is_cpu_port(ds, port))
flags &= ~BRIDGE_VLAN_INFO_UNTAGGED;
 
@@ -154,9 +160,15 @@ static int felix_vlan_del(struct dsa_switch *ds, int port,
  const struct switchdev_obj_port_vlan *vlan)
 {
struct ocelot *ocelot = ds->priv;
+   struct ocelot_port *ocelot_port = ocelot->ports[port];
u16 vid;
int err;
 
+   if (vlan->proto == ETH_P_8021AD) {
+   ocelot->enable_qinq = false;
+   ocelot_port->qinq_mode = false;
+   }
+
for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
err = ocelot_vlan_del(ocelot, port, vid);
if (err) {
diff --git a/drivers/net/ethernet/mscc/ocelot.c 
b/drivers/net/ethernet/mscc/ocelot.c
index f2d94b026d88..b5fec6855afd 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_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,28 @@ 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_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);
+   if (ocelot_port->qinq_mode) {
+   if (ocelot_port->vid)
+   val = REW_TAG_CFG_TAG_CFG(1);
+   else
+   val = REW_TAG_CFG_TAG_CFG(3);
+
+   tag_tpid = REW_TAG_CFG_TAG_TPID_CFG(1);
+   } else {
+   /* Port tagging disabled. */
+   val = REW_TAG_CFG_TAG_CFG(0);
+   tag_tpid = REW_TAG_CFG_TAG_TPID_CFG(0);
+   }
}
-   ocelot_rmw_gix(ocelot, 

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

2020-07-22 Thread Hongbo Wang
> Instead of writing a long email, let me just say this.
> I ran your commands on 2 random network cards (not ocelot/felix ports).
> They don't produce the same results as you. In fact, no frame with VLAN
> 111 C-TAG is forwarded (or received) at all by the bridge, not to mention that
> no VLAN 1000 S-TAG is pushed on egress.
> 
> 
> Have you tried playing with these commands?
> 
> ip link add dev br0 type bridge vlan_filtering 1 vlan_protocol 802.1ad ip link
> set eth0 master br0 ip link set eth1 master br0 bridge vlan add dev eth0 vid
> 100 pvid bridge vlan add dev eth1 vid 100
> 
> They produce the same output as yours, but have the benefit of using the
> network stack's abstractions and not glue between the 802.1q and the bridge
> module, hidden in the network driver.
> 
> I am sending the following packet towards eth0:
> 
> 00:04:9f:05:f4:ad > 00:01:02:03:04:05, ethertype 802.1Q (0x8100), length
> 102: \
> vlan 111, p 0, ethertype IPv4, 10.0.111.1 > 10.0.111.3: \
> ICMP echo request, id 63493, seq 991, length 64
> 
> and collecting it on the partner of eth1 as follows:
> 
> 00:04:9f:05:f4:ad > 00:01:02:03:04:05, ethertype 802.1Q-QinQ (0x88a8),
> length 106: \
> vlan 100, p 0, ethertype 802.1Q, vlan 111, p 0, ethertype IPv4, \
> 10.0.111.1 > 10.0.111.3: ICMP echo request, id 63493, seq 991,
> length 64
> 
> Thanks,
> -Vladimir

Hi Vladimir,
  the command " ip link add dev br0 type bridge vlan_filtering 1 vlan_protocol 
802.1ad " will influence all ports within the bridge, it will enable all ports 
vlan_filtering
flag and 802.1ad mode, if ocelot port enable vlan_filtering, it will set 
VLAN_AWARE_ENA
and VLAN_POP_CNT(1), the code is in ocelot_port_vlan_filtering in ocelot.c. it 
will
pop one tag from ingress frame, it's not my need, so I don't set vlan_filtering.

  If enable vlan_filtering, it needs enable VCAP ES0 push double VLAN tag, the 
code
is in another patch, it's based on VCAP ES0 related code, I will post it after 
ES0 code
be accepted.

  In this case, I only want the egress port(swp1) in QinQ mode, the mode will 
change swp1's
REW_TAG value, don't need swp0 enter QinQ mode, another issue is that if use " 
ip link add dev 
br0 type bridge ...", it can't pass proto to port driver, in 
dsa_slave_vlan_rx_add_vid, it will walk
into here:
ret = br_vlan_get_info(dp->bridge_dev, vid, );
if (ret == 0)  // ret is 0
return -EBUSY;
so I use "ip link add link swp1 name swp1.111 type vlan protocol 802.1ad id 
111" to enable only
port swp1's QinQ mode.

Thanks,
hongbo




[PATCH v3 2/2] net: dsa: ocelot: Add support for QinQ Operation

2020-07-22 Thread hongbo . wang
From: "hongbo.wang" 

This featue can be test using network test tools
TX-tool -> swp0  -> swp1 -> RX-tool

TX-tool simulates Customer that will send and receive packets with single
VLAN tag(CTAG), RX-tool simulates Service-Provider that 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.
ip link add dev br0 type bridge
ip link set dev swp0 master br0
ip link set dev swp1 master br0
2.
ip link add link swp0 name swp0.111 type vlan id 111
ip link add link swp1 name swp1.111 type vlan protocol 802.1ad id 111
3.
bridge vlan add dev swp0 vid 100 pvid
bridge vlan add dev swp1 vid 100
bridge vlan del dev swp1 vid 1 pvid
bridge vlan add dev swp1 vid 200 pvid untagged
Result:
Customer(tpid:8100 vid:111) -> swp0 -> swp1 -> Service-Provider(STAG \
tpid:88A8 vid:100, CTAG tpid:8100 vid:111)

Signed-off-by: hongbo.wang 
---
 drivers/net/dsa/ocelot/felix.c |  8 ++
 drivers/net/ethernet/mscc/ocelot.c | 44 --
 include/soc/mscc/ocelot.h  |  1 +
 3 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index c69d9592a2b7..12b46cb6549c 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -132,9 +132,13 @@ static void felix_vlan_add(struct dsa_switch *ds, int port,
 {
struct ocelot *ocelot = ds->priv;
u16 flags = vlan->flags;
+   struct ocelot_port *ocelot_port = ocelot->ports[port];
u16 vid;
int err;
 
+   if (vlan->proto == ETH_P_8021AD)
+   ocelot_port->qinq_mode = true;
+
if (dsa_is_cpu_port(ds, port))
flags &= ~BRIDGE_VLAN_INFO_UNTAGGED;
 
@@ -154,9 +158,13 @@ static int felix_vlan_del(struct dsa_switch *ds, int port,
  const struct switchdev_obj_port_vlan *vlan)
 {
struct ocelot *ocelot = ds->priv;
+   struct ocelot_port *ocelot_port = ocelot->ports[port];
u16 vid;
int err;
 
+   if (vlan->proto == ETH_P_8021AD)
+   ocelot_port->qinq_mode = false;
+
for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
err = ocelot_vlan_del(ocelot, port, vid);
if (err) {
diff --git a/drivers/net/ethernet/mscc/ocelot.c 
b/drivers/net/ethernet/mscc/ocelot.c
index f2d94b026d88..621277e875a5 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -144,6 +144,8 @@ static int ocelot_port_set_native_vlan(struct ocelot 
*ocelot, int port,
 {
struct ocelot_port *ocelot_port = ocelot->ports[port];
u32 val = 0;
+   u32 tag_tpid = 0;
+   u32 port_tpid = 0;
 
if (ocelot_port->vid != vid) {
/* Always permit deleting the native VLAN (vid = 0) */
@@ -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_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,28 @@ 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_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);
+   if (ocelot_port->qinq_mode) {
+   if (ocelot_port->vid)
+   val = REW_TAG_CFG_TAG_CFG(1);
+   else
+   val = REW_TAG_CFG_TAG_CFG(3);
+
+   tag_tpid = REW_TAG_CFG_TAG_TPID_CFG(1);
+   } 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 

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

2020-07-22 Thread hongbo . wang
From: "hongbo.wang" 

the following command will be supported:
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

when adding vlan, this patch only set protocol for user port,
cpu port don't care it, so set parameter proto to 0 for cpu port.

Signed-off-by: hongbo.wang 
---
 include/net/switchdev.h | 1 +
 net/dsa/dsa_priv.h  | 4 ++--
 net/dsa/port.c  | 6 --
 net/dsa/slave.c | 9 +
 net/dsa/tag_8021q.c | 4 ++--
 5 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index ff2246914301..7594ea82879f 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/dsa_priv.h b/net/dsa/dsa_priv.h
index 1653e3377cb3..52685b9875e5 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -164,8 +164,8 @@ int dsa_port_vlan_add(struct dsa_port *dp,
  struct switchdev_trans *trans);
 int dsa_port_vlan_del(struct dsa_port *dp,
  const struct switchdev_obj_port_vlan *vlan);
-int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 flags);
-int dsa_port_vid_del(struct dsa_port *dp, u16 vid);
+int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 proto, u16 flags);
+int dsa_port_vid_del(struct dsa_port *dp, u16 vid, u16 proto);
 int dsa_port_link_register_of(struct dsa_port *dp);
 void dsa_port_link_unregister_of(struct dsa_port *dp);
 extern const struct phylink_mac_ops dsa_port_phylink_mac_ops;
diff --git a/net/dsa/port.c b/net/dsa/port.c
index e23ece229c7e..c98bbac3980a 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -433,13 +433,14 @@ int dsa_port_vlan_del(struct dsa_port *dp,
return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, );
 }
 
-int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 flags)
+int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 proto, u16 flags)
 {
struct switchdev_obj_port_vlan vlan = {
.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
.flags = flags,
.vid_begin = vid,
.vid_end = vid,
+   .proto = proto,
};
struct switchdev_trans trans;
int err;
@@ -454,12 +455,13 @@ int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 
flags)
 }
 EXPORT_SYMBOL(dsa_port_vid_add);
 
-int dsa_port_vid_del(struct dsa_port *dp, u16 vid)
+int dsa_port_vid_del(struct dsa_port *dp, u16 vid, u16 proto)
 {
struct switchdev_obj_port_vlan vlan = {
.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
.vid_begin = vid,
.vid_end = vid,
+   .proto = proto,
};
 
return dsa_port_vlan_del(dp, );
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 41d60eeefdbd..ba3984565f7e 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1252,11 +1252,11 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device 
*dev, __be16 proto,
return -EBUSY;
}
 
-   ret = dsa_port_vid_add(dp, vid, 0);
+   ret = dsa_port_vid_add(dp, vid, ntohs(proto), 0);
if (ret)
return ret;
 
-   ret = dsa_port_vid_add(dp->cpu_dp, vid, 0);
+   ret = dsa_port_vid_add(dp->cpu_dp, vid, 0, 0);
if (ret)
return ret;
 
@@ -1289,7 +1289,7 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device 
*dev, __be16 proto,
/* Do not deprogram the CPU port as it may be shared with other user
 * ports which can be members of this VLAN as well.
 */
-   return dsa_port_vid_del(dp, vid);
+   return dsa_port_vid_del(dp, vid, ntohs(proto));
 }
 
 struct dsa_hw_port {
@@ -1744,7 +1744,8 @@ int dsa_slave_create(struct dsa_port *port)
 
slave_dev->features = master->vlan_features | NETIF_F_HW_TC;
if (ds->ops->port_vlan_add && ds->ops->port_vlan_del)
-   slave_dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
+   slave_dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER |
+  NETIF_F_HW_VLAN_STAG_FILTER;
slave_dev->hw_features |= NETIF_F_HW_TC;
slave_dev->features |= NETIF_F_LLTX;
slave_dev->ethtool_ops = _slave_ethtool_ops;
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index 780b2a15ac9b..848f85ed5c0f 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -152,9 +152,9 @@ static int dsa_8021q_vid_apply(struct dsa_switch *ds, int 
port, u16 vid,
struct dsa_port *dp = dsa_to_port(ds, port);
 
if (enabled)
-   return dsa_port_vid_add(dp, vid, flags);
+   return dsa_port_vid_add(dp, vid, 0, flags);
 
-   return dsa_port_vid_del(dp, vid);
+   return dsa_port_vid_del(dp, vid, 0);
 }
 
 /* RX VLAN tagging (left) and TX VLAN 

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

2020-07-22 Thread hongbo . wang
From: "hongbo.wang" 

1. the patch 0001* is for setting single port into 802.1AD(QinQ) mode,
before this patch, the function dsa_slave_vlan_rx_add_vid didn't pass 
the parameter "proto" to next port level, so switch's port can't get
parameter "proto"
  after applying this patch, we can use the following commands to set port 
to enable or disable QinQ mode:
  ip link add link swp1 name swp1.100 type vlan protocol 802.1ad id 100
  ip link del link swp1 name swp1.100

2. the patch 0002* 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.

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

 drivers/net/dsa/ocelot/felix.c |  8 ++
 drivers/net/ethernet/mscc/ocelot.c | 44 --
 include/net/switchdev.h|  1 +
 include/soc/mscc/ocelot.h  |  1 +
 net/dsa/dsa_priv.h |  4 +--
 net/dsa/port.c |  6 ++--
 net/dsa/slave.c|  9 +++---
 net/dsa/tag_8021q.c|  4 +--
 8 files changed, 59 insertions(+), 18 deletions(-)

-- 
2.17.1



RE: [EXT] Re: [PATCH] net: dsa: Add protocol support for 802.1AD when adding or deleting vlan for dsa switch and port

2020-07-21 Thread Hongbo Wang
Hi Florian,

Thanks for your reply!

I had posted my patch for switch port driver, the email title is "net: dsa: 
ocelot: Add support for QinQ Operation",

Best Regards!
hongbo

-Original Message-
From: Florian Fainelli  
Sent: 2020年7月22日 1:55
To: Hongbo Wang ; Xiaoliang Yang 
; allan.niel...@microchip.com; Po Liu 
; Claudiu Manoil ; Alexandru Marginean 
; Vladimir Oltean ; Leo 
Li ; Mingkai Hu ; and...@lunn.ch; 
vivien.dide...@gmail.com; da...@davemloft.net; j...@resnulli.us; 
ido...@idosch.org; k...@kernel.org; vinicius.go...@intel.com; 
niko...@cumulusnetworks.com; ro...@cumulusnetworks.com; net...@vger.kernel.org; 
linux-kernel@vger.kernel.org; horatiu.vul...@microchip.com; 
alexandre.bell...@bootlin.com; unglinuxdri...@microchip.com; 
linux-de...@linux.nxdi.nxp.com
Subject: [EXT] Re: [PATCH] net: dsa: Add protocol support for 802.1AD when 
adding or deleting vlan for dsa switch and port

Caution: EXT Email

On 7/21/20 4:02 AM, hongbo.w...@nxp.com wrote:
> From: "hongbo.wang" 
>
> the following command will be supported:
> 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
>
> when adding vlan, this patch only set protocol for user port, cpu port 
> don't care it, so set parameter proto to 0 for cpu port.

My previous feedback has been partially addressed, can you also post the switch 
driver changes that are going to implement the driver side changes? Presumably 
you must act on the 802.1AD programming request in the switch driver somehow, 
right?
--
Florian


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

2020-07-21 Thread hongbo . wang
From: "hongbo.wang" 

This featue can be test using network test tools
TX-tool -> swp0  -> swp1 -> RX-tool

TX-tool simulates Customer that will send and receive packets with single
VLAN tag(CTAG), RX-tool simulates Service-Provider that 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.
ip link add dev br0 type bridge
ip link set dev swp0 master br0
ip link set dev swp1 master br0
2.
ip link add link swp0 name swp0.111 type vlan id 111
ip link add link swp1 name swp1.111 type vlan protocol 802.1ad id 111
3.
bridge vlan add dev swp0 vid 100 pvid
bridge vlan add dev swp1 vid 100
bridge vlan del dev swp1 vid 1 pvid
bridge vlan add dev swp1 vid 200 pvid untagged
Result:
Customer(tpid:8100 vid:111) -> swp0 -> swp1 -> Service-Provider(STAG \
tpid:88A8 vid:100, CTAG tpid:8100 vid:111)

Signed-off-by: hongbo.wang 
---
 drivers/net/dsa/ocelot/felix.c |  8 ++
 drivers/net/ethernet/mscc/ocelot.c | 44 --
 include/soc/mscc/ocelot.h  |  1 +
 3 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index c69d9592a2b7..12b46cb6549c 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -132,9 +132,13 @@ static void felix_vlan_add(struct dsa_switch *ds, int port,
 {
struct ocelot *ocelot = ds->priv;
u16 flags = vlan->flags;
+   struct ocelot_port *ocelot_port = ocelot->ports[port];
u16 vid;
int err;
 
+   if (vlan->proto == ETH_P_8021AD)
+   ocelot_port->qinq_mode = true;
+
if (dsa_is_cpu_port(ds, port))
flags &= ~BRIDGE_VLAN_INFO_UNTAGGED;
 
@@ -154,9 +158,13 @@ static int felix_vlan_del(struct dsa_switch *ds, int port,
  const struct switchdev_obj_port_vlan *vlan)
 {
struct ocelot *ocelot = ds->priv;
+   struct ocelot_port *ocelot_port = ocelot->ports[port];
u16 vid;
int err;
 
+   if (vlan->proto == ETH_P_8021AD)
+   ocelot_port->qinq_mode = false;
+
for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
err = ocelot_vlan_del(ocelot, port, vid);
if (err) {
diff --git a/drivers/net/ethernet/mscc/ocelot.c 
b/drivers/net/ethernet/mscc/ocelot.c
index f2d94b026d88..621277e875a5 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -144,6 +144,8 @@ static int ocelot_port_set_native_vlan(struct ocelot 
*ocelot, int port,
 {
struct ocelot_port *ocelot_port = ocelot->ports[port];
u32 val = 0;
+   u32 tag_tpid = 0;
+   u32 port_tpid = 0;
 
if (ocelot_port->vid != vid) {
/* Always permit deleting the native VLAN (vid = 0) */
@@ -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_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,28 @@ 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_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);
+   if (ocelot_port->qinq_mode) {
+   if (ocelot_port->vid)
+   val = REW_TAG_CFG_TAG_CFG(1);
+   else
+   val = REW_TAG_CFG_TAG_CFG(3);
+
+   tag_tpid = REW_TAG_CFG_TAG_TPID_CFG(1);
+   } 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 

[PATCH] net: dsa: Add protocol support for 802.1AD when adding or deleting vlan for dsa switch and port

2020-07-21 Thread hongbo . wang
From: "hongbo.wang" 

the following command will be supported:
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

when adding vlan, this patch only set protocol for user port,
cpu port don't care it, so set parameter proto to 0 for cpu port.

Signed-off-by: hongbo.wang 
---
 include/net/switchdev.h | 1 +
 net/dsa/dsa_priv.h  | 4 ++--
 net/dsa/port.c  | 7 ---
 net/dsa/slave.c | 9 +
 net/dsa/tag_8021q.c | 4 ++--
 5 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index ff2246914301..7594ea82879f 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/dsa_priv.h b/net/dsa/dsa_priv.h
index 1653e3377cb3..52685b9875e5 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -164,8 +164,8 @@ int dsa_port_vlan_add(struct dsa_port *dp,
  struct switchdev_trans *trans);
 int dsa_port_vlan_del(struct dsa_port *dp,
  const struct switchdev_obj_port_vlan *vlan);
-int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 flags);
-int dsa_port_vid_del(struct dsa_port *dp, u16 vid);
+int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 proto, u16 flags);
+int dsa_port_vid_del(struct dsa_port *dp, u16 vid, u16 proto);
 int dsa_port_link_register_of(struct dsa_port *dp);
 void dsa_port_link_unregister_of(struct dsa_port *dp);
 extern const struct phylink_mac_ops dsa_port_phylink_mac_ops;
diff --git a/net/dsa/port.c b/net/dsa/port.c
index e23ece229c7e..c07c6ee136a9 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -364,7 +364,6 @@ int dsa_port_fdb_del(struct dsa_port *dp, const unsigned 
char *addr,
.port = dp->index,
.addr = addr,
.vid = vid,
-
};
 
return dsa_port_notify(dp, DSA_NOTIFIER_FDB_DEL, );
@@ -433,13 +432,14 @@ int dsa_port_vlan_del(struct dsa_port *dp,
return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, );
 }
 
-int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 flags)
+int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 proto, u16 flags)
 {
struct switchdev_obj_port_vlan vlan = {
.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
.flags = flags,
.vid_begin = vid,
.vid_end = vid,
+   .proto = proto,
};
struct switchdev_trans trans;
int err;
@@ -454,12 +454,13 @@ int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 
flags)
 }
 EXPORT_SYMBOL(dsa_port_vid_add);
 
-int dsa_port_vid_del(struct dsa_port *dp, u16 vid)
+int dsa_port_vid_del(struct dsa_port *dp, u16 vid, u16 proto)
 {
struct switchdev_obj_port_vlan vlan = {
.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
.vid_begin = vid,
.vid_end = vid,
+   .proto = proto,
};
 
return dsa_port_vlan_del(dp, );
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 41d60eeefdbd..ba3984565f7e 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1252,11 +1252,11 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device 
*dev, __be16 proto,
return -EBUSY;
}
 
-   ret = dsa_port_vid_add(dp, vid, 0);
+   ret = dsa_port_vid_add(dp, vid, ntohs(proto), 0);
if (ret)
return ret;
 
-   ret = dsa_port_vid_add(dp->cpu_dp, vid, 0);
+   ret = dsa_port_vid_add(dp->cpu_dp, vid, 0, 0);
if (ret)
return ret;
 
@@ -1289,7 +1289,7 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device 
*dev, __be16 proto,
/* Do not deprogram the CPU port as it may be shared with other user
 * ports which can be members of this VLAN as well.
 */
-   return dsa_port_vid_del(dp, vid);
+   return dsa_port_vid_del(dp, vid, ntohs(proto));
 }
 
 struct dsa_hw_port {
@@ -1744,7 +1744,8 @@ int dsa_slave_create(struct dsa_port *port)
 
slave_dev->features = master->vlan_features | NETIF_F_HW_TC;
if (ds->ops->port_vlan_add && ds->ops->port_vlan_del)
-   slave_dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
+   slave_dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER |
+  NETIF_F_HW_VLAN_STAG_FILTER;
slave_dev->hw_features |= NETIF_F_HW_TC;
slave_dev->features |= NETIF_F_LLTX;
slave_dev->ethtool_ops = _slave_ethtool_ops;
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index 780b2a15ac9b..848f85ed5c0f 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -152,9 +152,9 @@ static int dsa_8021q_vid_apply(struct dsa_switch *ds, int 
port, u16 vid,
struct dsa_port *dp = dsa_to_port(ds, port);
 
if 

RE: [EXT] Re: [PATCH 1/2] net: dsa: Add flag for 802.1AD when adding VLAN for dsa switch and port

2020-07-21 Thread Hongbo Wang
Thanks for your suggestion!
I will change the code.
thanks.

-Original Message-
From: Florian Fainelli  
Sent: 2020年7月21日 12:57
To: Hongbo Wang ; Xiaoliang Yang 
; allan.niel...@microchip.com; Po Liu 
; Claudiu Manoil ; Alexandru Marginean 
; Vladimir Oltean ; Leo 
Li ; Mingkai Hu ; and...@lunn.ch; 
vivien.dide...@gmail.com; da...@davemloft.net; j...@resnulli.us; 
ido...@idosch.org; k...@kernel.org; vinicius.go...@intel.com; 
niko...@cumulusnetworks.com; ro...@cumulusnetworks.com; net...@vger.kernel.org; 
linux-kernel@vger.kernel.org; horatiu.vul...@microchip.com; 
alexandre.bell...@bootlin.com; unglinuxdri...@microchip.com; 
linux-de...@linux.nxdi.nxp.com
Subject: [EXT] Re: [PATCH 1/2] net: dsa: Add flag for 802.1AD when adding VLAN 
for dsa switch and port

Caution: EXT Email

On 7/20/2020 3:41 AM, hongbo.w...@nxp.com wrote:
> From: "hongbo.wang" 
>
> the following command can be supported:
> ip link add link swp1 name swp1.100 type vlan protocol 802.1ad id 100

You should probably include the switch driver that is going to be benefiting 
from doing these changes in the patch series, provide a cover letter when 
sending more than one patch, and also combine both the add and delete parts.

Since you already have visibility into proto, it may not be necessary at all 
for now to define a BRIDGE_VLAN_INFO_8021AD bit in order to pass that flag down 
to DSA for programming the VLAN, just pass proto to
dsa_port_vid_add() or a boolean flag which indicates whether this is the 
customer or service tag that you are trying to program?
--
Florian


RE: [EXT] Re: [PATCH 1/2] net: dsa: Add flag for 802.1AD when adding VLAN for dsa switch and port

2020-07-20 Thread Hongbo Wang
Hi Nikolay,

 Thanks for your comments.

The original intention is that I want to run a command to set single port into 
QinQ mode,
the related commands are:
ip link set br0 type bridge vlan_protocol 802.1ad  // this command will set all 
ports under the bridge br0
ip link add link swp1 name swp1.100 type vlan protocol 802.1ad id 100  // this 
command can set single port for vlan

I trace the related code of these two commands, find the same issue that 
dsa_slave_vlan_rx_add_vid didn't pass the parameter "proto" to next port level, 
so I create this patch.

I understand your concern, If don't use the flags for bridge, another way is 
that add new item "u16 proto" in struct switchdev_obj_port_vlan, the slave port 
can get proto from that, like that:

struct switchdev_obj_port_vlan {
struct switchdev_obj obj;
u16 flags;
u16 vid_begin;
u16 vid_end;
   + u16 proto;
};

The related modification has:
int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 flags, u16 proto);  // 
add parameter proto
int dsa_port_vid_del(struct dsa_port *dp, u16 vid, u16 proto);  // add 
parameter proto

Any comments?
Thanks


-Original Message-
From: Nikolay Aleksandrov  
Sent: 2020年7月20日 18:45
To: Hongbo Wang ; Xiaoliang Yang 
; allan.niel...@microchip.com; Po Liu 
; Claudiu Manoil ; Alexandru Marginean 
; Vladimir Oltean ; Leo 
Li ; Mingkai Hu ; and...@lunn.ch; 
f.faine...@gmail.com; vivien.dide...@gmail.com; da...@davemloft.net; 
j...@resnulli.us; ido...@idosch.org; k...@kernel.org; vinicius.go...@intel.com; 
ro...@cumulusnetworks.com; net...@vger.kernel.org; 
linux-kernel@vger.kernel.org; horatiu.vul...@microchip.com; 
alexandre.bell...@bootlin.com; unglinuxdri...@microchip.com; 
linux-de...@linux.nxdi.nxp.com
Subject: [EXT] Re: [PATCH 1/2] net: dsa: Add flag for 802.1AD when adding VLAN 
for dsa switch and port

Caution: EXT Email

On 20/07/2020 13:41, hongbo.w...@nxp.com wrote:
> From: "hongbo.wang" 
>
> the following command can be supported:
> ip link add link swp1 name swp1.100 type vlan protocol 802.1ad id 100
>
> Signed-off-by: hongbo.wang 
> ---
>  include/uapi/linux/if_bridge.h | 1 +
>  net/dsa/slave.c| 9 +++--
>  2 files changed, 8 insertions(+), 2 deletions(-)
>

This is not bridge related at all, please leave its flags out of it.

Nacked-by: Nikolay Aleksandrov 



> diff --git a/include/uapi/linux/if_bridge.h 
> b/include/uapi/linux/if_bridge.h index caa6914a3e53..ecd960aa65c7 
> 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -132,6 +132,7 @@ enum {
>  #define BRIDGE_VLAN_INFO_RANGE_END   (1<<4) /* VLAN is end of vlan range */
>  #define BRIDGE_VLAN_INFO_BRENTRY (1<<5) /* Global bridge VLAN entry */
>  #define BRIDGE_VLAN_INFO_ONLY_OPTS   (1<<6) /* Skip create/delete/flags */
> +#define BRIDGE_VLAN_INFO_8021AD  (1<<7) /* VLAN is 802.1AD protocol */
>
>  struct bridge_vlan_info {
>   __u16 flags;
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c index 
> 4c7f086a047b..376d7ac5f1e5 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -1232,6 +1232,7 @@ static int dsa_slave_get_ts_info(struct 
> net_device *dev,  static int dsa_slave_vlan_rx_add_vid(struct net_device 
> *dev, __be16 proto,
>u16 vid)  {
> + u16 flags = 0;
>   struct dsa_port *dp = dsa_slave_to_port(dev);
>   struct bridge_vlan_info info;
>   int ret;
> @@ -1252,7 +1253,10 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device 
> *dev, __be16 proto,
>   return -EBUSY;
>   }
>
> - ret = dsa_port_vid_add(dp, vid, 0);
> + if (ntohs(proto) == ETH_P_8021AD)
> + flags |= BRIDGE_VLAN_INFO_8021AD;
> +
> + ret = dsa_port_vid_add(dp, vid, flags);
>   if (ret)
>   return ret;
>
> @@ -1744,7 +1748,8 @@ int dsa_slave_create(struct dsa_port *port)
>
>   slave_dev->features = master->vlan_features | NETIF_F_HW_TC;
>   if (ds->ops->port_vlan_add && ds->ops->port_vlan_del)
> - slave_dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
> + slave_dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER |
> +NETIF_F_HW_VLAN_STAG_FILTER;
>   slave_dev->hw_features |= NETIF_F_HW_TC;
>   slave_dev->features |= NETIF_F_LLTX;
>   slave_dev->ethtool_ops = _slave_ethtool_ops;
>



[PATCH 2/2] net: dsa: Set flag for 802.1AD when deleting vlan for dsa switch and port

2020-07-20 Thread hongbo . wang
From: "hongbo.wang" 

the following command will be supported:
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 
---
 net/dsa/dsa_priv.h  | 2 +-
 net/dsa/port.c  | 3 ++-
 net/dsa/slave.c | 6 +-
 net/dsa/tag_8021q.c | 2 +-
 4 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index adecf73bd608..5cd804c1d7e3 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -165,7 +165,7 @@ int dsa_port_vlan_add(struct dsa_port *dp,
 int dsa_port_vlan_del(struct dsa_port *dp,
  const struct switchdev_obj_port_vlan *vlan);
 int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 flags);
-int dsa_port_vid_del(struct dsa_port *dp, u16 vid);
+int dsa_port_vid_del(struct dsa_port *dp, u16 vid, u16 flags);
 int dsa_port_link_register_of(struct dsa_port *dp);
 void dsa_port_link_unregister_of(struct dsa_port *dp);
 extern const struct phylink_mac_ops dsa_port_phylink_mac_ops;
diff --git a/net/dsa/port.c b/net/dsa/port.c
index e23ece229c7e..8a8ecb91a030 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -454,10 +454,11 @@ int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 
flags)
 }
 EXPORT_SYMBOL(dsa_port_vid_add);
 
-int dsa_port_vid_del(struct dsa_port *dp, u16 vid)
+int dsa_port_vid_del(struct dsa_port *dp, u16 vid, u16 flags)
 {
struct switchdev_obj_port_vlan vlan = {
.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
+   .flags = flags,
.vid_begin = vid,
.vid_end = vid,
};
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 376d7ac5f1e5..14784a6718a9 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1270,6 +1270,7 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device 
*dev, __be16 proto,
 static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
  u16 vid)
 {
+   u16 flags = 0;
struct dsa_port *dp = dsa_slave_to_port(dev);
struct bridge_vlan_info info;
int ret;
@@ -1290,10 +1291,13 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device 
*dev, __be16 proto,
return -EBUSY;
}
 
+   if (ntohs(proto) == ETH_P_8021AD)
+   flags |= BRIDGE_VLAN_INFO_8021AD;
+
/* Do not deprogram the CPU port as it may be shared with other user
 * ports which can be members of this VLAN as well.
 */
-   return dsa_port_vid_del(dp, vid);
+   return dsa_port_vid_del(dp, vid, flags);
 }
 
 struct dsa_hw_port {
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index 780b2a15ac9b..87b732c5cccf 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -154,7 +154,7 @@ static int dsa_8021q_vid_apply(struct dsa_switch *ds, int 
port, u16 vid,
if (enabled)
return dsa_port_vid_add(dp, vid, flags);
 
-   return dsa_port_vid_del(dp, vid);
+   return dsa_port_vid_del(dp, vid, flags);
 }
 
 /* RX VLAN tagging (left) and TX VLAN tagging (right) setup shown for a single
-- 
2.17.1



[PATCH 1/2] net: dsa: Add flag for 802.1AD when adding VLAN for dsa switch and port

2020-07-20 Thread hongbo . wang
From: "hongbo.wang" 

the following command can be supported:
ip link add link swp1 name swp1.100 type vlan protocol 802.1ad id 100

Signed-off-by: hongbo.wang 
---
 include/uapi/linux/if_bridge.h | 1 +
 net/dsa/slave.c| 9 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index caa6914a3e53..ecd960aa65c7 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -132,6 +132,7 @@ enum {
 #define BRIDGE_VLAN_INFO_RANGE_END (1<<4) /* VLAN is end of vlan range */
 #define BRIDGE_VLAN_INFO_BRENTRY   (1<<5) /* Global bridge VLAN entry */
 #define BRIDGE_VLAN_INFO_ONLY_OPTS (1<<6) /* Skip create/delete/flags */
+#define BRIDGE_VLAN_INFO_8021AD(1<<7) /* VLAN is 802.1AD protocol */
 
 struct bridge_vlan_info {
__u16 flags;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 4c7f086a047b..376d7ac5f1e5 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1232,6 +1232,7 @@ static int dsa_slave_get_ts_info(struct net_device *dev,
 static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
 u16 vid)
 {
+   u16 flags = 0;
struct dsa_port *dp = dsa_slave_to_port(dev);
struct bridge_vlan_info info;
int ret;
@@ -1252,7 +1253,10 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device 
*dev, __be16 proto,
return -EBUSY;
}
 
-   ret = dsa_port_vid_add(dp, vid, 0);
+   if (ntohs(proto) == ETH_P_8021AD)
+   flags |= BRIDGE_VLAN_INFO_8021AD;
+
+   ret = dsa_port_vid_add(dp, vid, flags);
if (ret)
return ret;
 
@@ -1744,7 +1748,8 @@ int dsa_slave_create(struct dsa_port *port)
 
slave_dev->features = master->vlan_features | NETIF_F_HW_TC;
if (ds->ops->port_vlan_add && ds->ops->port_vlan_del)
-   slave_dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
+   slave_dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER |
+  NETIF_F_HW_VLAN_STAG_FILTER;
slave_dev->hw_features |= NETIF_F_HW_TC;
slave_dev->features |= NETIF_F_LLTX;
slave_dev->ethtool_ops = _slave_ethtool_ops;
-- 
2.17.1