[net-next PATCH 1/5] net: dsa: tag_rtl4_a: Implement Realtek 4 byte A tag

[net-next PATCH 1/5] net: dsa: tag_rtl4_a: Implement Realtek 4 byte A tag

[Linus Walleij]

This implements the known parts of the Realtek 4 byte
tag protocol version 0xA, as found in the RTL8366RB
DSA switch.

It is designated as protocol version 0xA as a
different Realtek 4 byte tag format with protocol
version 0x9 is known to exist in the Realtek RTL8306
chips.

The tag and switch chip lacks public documentation, so
the tag format has been reverse-engineered from
packet dumps. As only ingress traffic has been available
for analysis an egress tag has not been possible to
develop (even using educated guesses about bit fields)
so this is as far as it gets. It is not know if the
switch even supports egress tagging.

Using these ingress tags however, the switch
functionality is vastly improved and the packets find
their way into the destination port without any
tricky configuration. On the D-Link DIR-685 the
LAN ports now come up and respond to ping without
any command line configuration so this is a real
improvement for users.

Egress packets need to be restricted to the proper
target ports using VLAN, which the DSA switch driver
already sets up.

Cc: DENG Qingfang <dqfext@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 include/net/dsa.h    |   2 +
 net/dsa/Kconfig      |   7 +++
 net/dsa/Makefile     |   1 +
 net/dsa/tag_rtl4_a.c | 134 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 144 insertions(+)
 create mode 100644 net/dsa/tag_rtl4_a.c

diff --git a/include/net/dsa.h b/include/net/dsa.h
index fb3f9222f2a1..7a6a922a509e 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -44,6 +44,7 @@ struct phylink_link_state;
 #define DSA_TAG_PROTO_KSZ8795_VALUE		14
 #define DSA_TAG_PROTO_OCELOT_VALUE		15
 #define DSA_TAG_PROTO_AR9331_VALUE		16
+#define DSA_TAG_PROTO_RTL4_A_VALUE		17
 
 enum dsa_tag_protocol {
 	DSA_TAG_PROTO_NONE		= DSA_TAG_PROTO_NONE_VALUE,
@@ -63,6 +64,7 @@ enum dsa_tag_protocol {
 	DSA_TAG_PROTO_KSZ8795		= DSA_TAG_PROTO_KSZ8795_VALUE,
 	DSA_TAG_PROTO_OCELOT		= DSA_TAG_PROTO_OCELOT_VALUE,
 	DSA_TAG_PROTO_AR9331		= DSA_TAG_PROTO_AR9331_VALUE,
+	DSA_TAG_PROTO_RTL4_A		= DSA_TAG_PROTO_RTL4_A_VALUE,
 };
 
 struct packet_type;
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index 92663dcb3aa2..0ec29e49683f 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -85,6 +85,13 @@ config NET_DSA_TAG_KSZ
 	  Say Y if you want to enable support for tagging frames for the
 	  Microchip 8795/9477/9893 families of switches.
 
+config NET_DSA_TAG_RTL4_A
+	tristate "Tag driver for Realtek 4 byte protocol A tags"
+	help
+	  Say Y or M if you want to enable support for tagging frames for the
+	  Realtek switches with 4 byte protocol A tags, sich as found in
+	  the Realtek RTL8366RB.
+
 config NET_DSA_TAG_OCELOT
 	tristate "Tag driver for Ocelot family of switches"
 	select PACKING
diff --git a/net/dsa/Makefile b/net/dsa/Makefile
index 108486cfdeef..4f47b2025ff5 100644
--- a/net/dsa/Makefile
+++ b/net/dsa/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_NET_DSA_TAG_DSA) += tag_dsa.o
 obj-$(CONFIG_NET_DSA_TAG_EDSA) += tag_edsa.o
 obj-$(CONFIG_NET_DSA_TAG_GSWIP) += tag_gswip.o
 obj-$(CONFIG_NET_DSA_TAG_KSZ) += tag_ksz.o
+obj-$(CONFIG_NET_DSA_TAG_RTL4_A) += tag_rtl4_a.o
 obj-$(CONFIG_NET_DSA_TAG_LAN9303) += tag_lan9303.o
 obj-$(CONFIG_NET_DSA_TAG_MTK) += tag_mtk.o
 obj-$(CONFIG_NET_DSA_TAG_OCELOT) += tag_ocelot.o
diff --git a/net/dsa/tag_rtl4_a.c b/net/dsa/tag_rtl4_a.c
new file mode 100644
index 000000000000..45f24e5cdde2
--- /dev/null
+++ b/net/dsa/tag_rtl4_a.c
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Handler for Realtek 4 byte DSA switch tags
+ * Currently only supports protocol "A" found in RTL8366RB
+ * Copyright (c) 2020 Linus Walleij <linus.walleij@linaro.org>
+ *
+ * This "proprietary tag" header looks like so:
+ *
+ * -------------------------------------------------
+ * | MAC DA | MAC SA | 0x8899 | 2 bytes tag | Type |
+ * -------------------------------------------------
+ *
+ * The 2 bytes tag form a 16 bit big endian word. The exact
+ * meaning has been guess from packet dumps from ingress
+ * frames, as no working egress traffic has been available
+ * we do not know the format of the egress tags or if they
+ * are even supported.
+ */
+
+#include <linux/etherdevice.h>
+#include <linux/bits.h>
+
+#include "dsa_priv.h"
+
+#define RTL4_A_HDR_LEN		4
+#define RTL4_A_ETHERTYPE	0x8899
+#define RTL4_A_PROTOCOL_SHIFT	12
+/*
+ * 0x1 = Realtek Remote Control protocol (RRCP)
+ * 0x2/0x3 seems to be used for loopback testing
+ * 0x9 = RTL8306 DSA protocol
+ * 0xa = RTL8366RB DSA protocol
+ */
+#define RTL4_A_PROTOCOL_RTL8366RB	0xa
+
+static struct sk_buff *rtl4a_tag_xmit(struct sk_buff *skb,
+				      struct net_device *dev)
+{
+	/*
+	 * Just let it pass thru, we don't know if it is possible
+	 * to tag a frame with the 0x8899 ethertype and direct it
+	 * to a specific port, all attempts at reverse-engineering have
+	 * ended up with the frames getting dropped.
+	 *
+	 * The VLAN set-up needs to restrict the frames to the right port.
+	 *
+	 * If you have documentation on the tagging format for RTL8366RB
+	 * (tag type A) then please contribute.
+	 */
+	return skb;
+}
+
+static struct sk_buff *rtl4a_tag_rcv(struct sk_buff *skb,
+				     struct net_device *dev,
+				     struct packet_type *pt)
+{
+	u16 protport;
+	__be16 *p;
+	u16 etype;
+	u8 flags;
+	u8 *tag;
+	u8 prot;
+	u8 port;
+
+	if (unlikely(!pskb_may_pull(skb, RTL4_A_HDR_LEN)))
+		return NULL;
+
+	/* The RTL4 header has its own custom Ethertype 0x8899 and that
+	 * starts right at the beginning of the packet, after the src
+	 * ethernet addr. Apparantly skb->data always points 2 bytes in,
+	 * behind the Ethertype.
+	 */
+	tag = skb->data - 2;
+	p = (__be16 *)tag;
+	etype = ntohs(*p);
+	if (etype != RTL4_A_ETHERTYPE) {
+		/* Not custom, just pass through */
+		netdev_dbg(dev, "non-realtek ethertype 0x%04x\n", etype);
+		return skb;
+	}
+	p = (__be16 *)(tag + 2);
+	protport = ntohs(*p);
+	/* The 4 upper bits are the protocol */
+	prot = (protport >> RTL4_A_PROTOCOL_SHIFT) & 0x0f;
+	if (prot != RTL4_A_PROTOCOL_RTL8366RB) {
+		netdev_err(dev, "unknown realtek protocol 0x%01x\n", prot);
+		return NULL;
+	}
+	netdev_dbg(dev, "realtek protocol 0x%02x\n", prot);
+	port = protport & 0xff;
+	netdev_dbg(dev, "realtek port origin 0x%02x\n", port);
+
+	/* Remove RTL4 tag and recalculate checksum */
+	skb_pull_rcsum(skb, RTL4_A_HDR_LEN);
+
+	/* Move ethernet DA and SA in front of the data */
+	memmove(skb->data - ETH_HLEN,
+		skb->data - ETH_HLEN - RTL4_A_HDR_LEN,
+		2 * ETH_ALEN);
+
+	skb->dev = dsa_master_find_slave(dev, 0, port);
+	if (!skb->dev) {
+		netdev_dbg(dev, "could not find slave for port %d\n", port);
+		return NULL;
+	}
+	netdev_dbg(skb->dev, "forwarded packet to slave port %d\n", port);
+
+	skb->offload_fwd_mark = 1;
+
+	return skb;
+}
+
+static int rtl4a_tag_flow_dissect(const struct sk_buff *skb, __be16 *proto,
+				  int *offset)
+{
+	*offset = RTL4_A_HDR_LEN;
+	/* Skip past the tag and fetch the encapsulated Ethertype */
+	*proto = ((__be16 *)skb->data)[1];
+
+	return 0;
+}
+
+static const struct dsa_device_ops rtl4a_netdev_ops = {
+	.name	= "rtl4a",
+	.proto	= DSA_TAG_PROTO_RTL4_A,
+	.xmit	= rtl4a_tag_xmit,
+	.rcv	= rtl4a_tag_rcv,
+	.flow_dissect = rtl4a_tag_flow_dissect,
+	.overhead = RTL4_A_HDR_LEN,
+};
+module_dsa_tag_driver(rtl4a_netdev_ops);
+
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_RTL4_A);
-- 
2.26.2

[net-next PATCH 2/5] net: dsa: rtl8366rb: Support the CPU DSA tag

[Linus Walleij]

This activates the support to use the CPU tag to properly
direct ingress traffic to the right port.

After this e.g. ping works out-of-the-box with the
RTL8366RB.

Cc: DENG Qingfang <dqfext@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/net/dsa/Kconfig     |  1 +
 drivers/net/dsa/rtl8366rb.c | 31 ++++++++-----------------------
 2 files changed, 9 insertions(+), 23 deletions(-)

diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index 2d38dbc9dd8c..3a4485651f1d 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -70,6 +70,7 @@ config NET_DSA_QCA8K
 config NET_DSA_REALTEK_SMI
 	tristate "Realtek SMI Ethernet switch family support"
 	depends on NET_DSA
+	select NET_DSA_TAG_RTL4_A
 	select FIXED_PHY
 	select IRQ_DOMAIN
 	select REALTEK_PHY
diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c
index fd1977590cb4..48f1ff746799 100644
--- a/drivers/net/dsa/rtl8366rb.c
+++ b/drivers/net/dsa/rtl8366rb.c
@@ -109,8 +109,8 @@
 /* CPU port control reg */
 #define RTL8368RB_CPU_CTRL_REG		0x0061
 #define RTL8368RB_CPU_PORTS_MSK		0x00FF
-/* Enables inserting custom tag length/type 0x8899 */
-#define RTL8368RB_CPU_INSTAG		BIT(15)
+/* Disables inserting custom tag length/type 0x8899 */
+#define RTL8368RB_CPU_NO_TAG		BIT(15)
 
 #define RTL8366RB_SMAR0			0x0070 /* bits 0..15 */
 #define RTL8366RB_SMAR1			0x0071 /* bits 16..31 */
@@ -844,16 +844,14 @@ static int rtl8366rb_setup(struct dsa_switch *ds)
 	if (ret)
 		return ret;
 
-	/* Enable CPU port and enable inserting CPU tag
+	/* Enable CPU port with custom DSA tag 8899.
 	 *
-	 * Disabling RTL8368RB_CPU_INSTAG here will change the behaviour
-	 * of the switch totally and it will start talking Realtek RRCP
-	 * internally. It is probably possible to experiment with this,
-	 * but then the kernel needs to understand and handle RRCP first.
+	 * If you set RTL8368RB_CPU_NO_TAG (bit 15) in this registers
+	 * the custom tag is turned off.
 	 */
 	ret = regmap_update_bits(smi->map, RTL8368RB_CPU_CTRL_REG,
 				 0xFFFF,
-				 RTL8368RB_CPU_INSTAG | BIT(smi->cpu_port));
+				 BIT(smi->cpu_port));
 	if (ret)
 		return ret;
 
@@ -967,21 +965,8 @@ static enum dsa_tag_protocol rtl8366_get_tag_protocol(struct dsa_switch *ds,
 						      int port,
 						      enum dsa_tag_protocol mp)
 {
-	/* For now, the RTL switches are handled without any custom tags.
-	 *
-	 * It is possible to turn on "custom tags" by removing the
-	 * RTL8368RB_CPU_INSTAG flag when enabling the port but what it
-	 * does is unfamiliar to DSA: ethernet frames of type 8899, the Realtek
-	 * Remote Control Protocol (RRCP) start to appear on the CPU port of
-	 * the device. So this is not the ordinary few extra bytes in the
-	 * frame. Instead it appears that the switch starts to talk Realtek
-	 * RRCP internally which means a pretty complex RRCP implementation
-	 * decoding and responding the RRCP protocol is needed to exploit this.
-	 *
-	 * The OpenRRCP project (dormant since 2009) have reverse-egineered
-	 * parts of the protocol.
-	 */
-	return DSA_TAG_PROTO_NONE;
+	/* This switch uses the 4 byte protocol A Realtek DSA tag */
+	return DSA_TAG_PROTO_RTL4_A;
 }
 
 static void rtl8366rb_adjust_link(struct dsa_switch *ds, int port,
-- 
2.26.2

[net-next PATCH 3/5] net: dsa: rtl8366: Split out default VLAN config

[Linus Walleij]

We loop over the ports to initialize the default VLAN
and PVID for each port. As we need to reuse the
code to reinitialize a single port, break out the
function rtl8366_set_default_vlan_and_pvid().

Cc: DENG Qingfang <dqfext@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/net/dsa/rtl8366.c | 70 ++++++++++++++++++++++++---------------
 1 file changed, 43 insertions(+), 27 deletions(-)

diff --git a/drivers/net/dsa/rtl8366.c b/drivers/net/dsa/rtl8366.c
index ac88caca5ad4..66bd1241204c 100644
--- a/drivers/net/dsa/rtl8366.c
+++ b/drivers/net/dsa/rtl8366.c
@@ -253,6 +253,48 @@ int rtl8366_reset_vlan(struct realtek_smi *smi)
 }
 EXPORT_SYMBOL_GPL(rtl8366_reset_vlan);
 
+static int rtl8366_set_default_vlan_and_pvid(struct realtek_smi *smi,
+					     int port)
+{
+	u32 mask;
+	u16 vid;
+	int ret;
+
+	/* This is the reserved default VLAN for this port */
+	vid = port + 1;
+
+	if (port == smi->cpu_port)
+		/* For the CPU port, make all ports members of this
+		 * VLAN.
+		 */
+		mask = GENMASK(smi->num_ports - 1, 0);
+	else
+		/* For all other ports, enable itself plus the
+		 * CPU port.
+		 */
+		mask = BIT(port) | BIT(smi->cpu_port);
+
+	/* For each port, set the port as member of VLAN (port+1)
+	 * and untagged, except for the CPU port: the CPU port (5) is
+	 * member of VLAN 6 and so are ALL the other ports as well.
+	 * Use filter 0 (no filter).
+	 */
+	dev_info(smi->dev, "Set VLAN %04x portmask to %08x (port %d %s)\n",
+		 vid, mask, port, (port == smi->cpu_port) ?
+		 "CPU PORT and all other ports" : "and CPU port");
+	ret = rtl8366_set_vlan(smi, vid, mask, mask, 0);
+	if (ret)
+		return ret;
+
+	dev_info(smi->dev, "Set PVID %04x on port %d\n",
+		 vid, port);
+	ret = rtl8366_set_pvid(smi, port, vid);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 int rtl8366_init_vlan(struct realtek_smi *smi)
 {
 	int port;
@@ -266,33 +308,7 @@ int rtl8366_init_vlan(struct realtek_smi *smi)
 	 * it with the VLAN (port+1)
 	 */
 	for (port = 0; port < smi->num_ports; port++) {
-		u32 mask;
-
-		if (port == smi->cpu_port)
-			/* For the CPU port, make all ports members of this
-			 * VLAN.
-			 */
-			mask = GENMASK(smi->num_ports - 1, 0);
-		else
-			/* For all other ports, enable itself plus the
-			 * CPU port.
-			 */
-			mask = BIT(port) | BIT(smi->cpu_port);
-
-		/* For each port, set the port as member of VLAN (port+1)
-		 * and untagged, except for the CPU port: the CPU port (5) is
-		 * member of VLAN 6 and so are ALL the other ports as well.
-		 * Use filter 0 (no filter).
-		 */
-		dev_info(smi->dev, "VLAN%d port mask for port %d, %08x\n",
-			 (port + 1), port, mask);
-		ret = rtl8366_set_vlan(smi, (port + 1), mask, mask, 0);
-		if (ret)
-			return ret;
-
-		dev_info(smi->dev, "VLAN%d port %d, PVID set to %d\n",
-			 (port + 1), port, (port + 1));
-		ret = rtl8366_set_pvid(smi, port, (port + 1));
+		ret = rtl8366_set_default_vlan_and_pvid(smi, port);
 		if (ret)
 			return ret;
 	}
-- 
2.26.2

[net-next PATCH 4/5] net: dsa: rtl8366: VLAN 0 as disable tagging

[Linus Walleij]

The code in net/8021q/vlan.c, vlan_device_event() sets
VLAN 0 for a VLAN-capable ethernet device when it
comes up.

Since the RTL8366 DSA switches must have a VLAN and
PVID set up for any packets to come through we have
already set up default VLAN for each port as part of
bringing the switch online.

Make sure that setting VLAN 0 has the same effect
and does not try to actually tell the hardware to use
VLAN 0 on the port because that will not work.

Cc: DENG Qingfang <dqfext@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/net/dsa/rtl8366.c | 65 +++++++++++++++++++++++++++++++--------
 1 file changed, 52 insertions(+), 13 deletions(-)

diff --git a/drivers/net/dsa/rtl8366.c b/drivers/net/dsa/rtl8366.c
index 66bd1241204c..7f0691a6da13 100644
--- a/drivers/net/dsa/rtl8366.c
+++ b/drivers/net/dsa/rtl8366.c
@@ -355,15 +355,25 @@ int rtl8366_vlan_prepare(struct dsa_switch *ds, int port,
 			 const struct switchdev_obj_port_vlan *vlan)
 {
 	struct realtek_smi *smi = ds->priv;
+	u16 vid_begin = vlan->vid_begin;
+	u16 vid_end = vlan->vid_end;
 	u16 vid;
 	int ret;
 
-	for (vid = vlan->vid_begin; vid < vlan->vid_end; vid++)
+	if (vid_begin == 0) {
+		dev_info(smi->dev, "prepare VLAN 0 - ignored\n");
+		if (vid_end == 0)
+			return 0;
+		/* Skip VLAN 0 and start with VLAN 1 */
+		vid_begin = 1;
+	}
+
+	for (vid = vid_begin; vid < vid_end; vid++)
 		if (!smi->ops->is_vlan_valid(smi, vid))
 			return -EINVAL;
 
 	dev_info(smi->dev, "prepare VLANs %04x..%04x\n",
-		 vlan->vid_begin, vlan->vid_end);
+		 vid_begin, vid_end);
 
 	/* Enable VLAN in the hardware
 	 * FIXME: what's with this 4k business?
@@ -383,27 +393,46 @@ void rtl8366_vlan_add(struct dsa_switch *ds, int port,
 	bool untagged = !!(vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED);
 	bool pvid = !!(vlan->flags & BRIDGE_VLAN_INFO_PVID);
 	struct realtek_smi *smi = ds->priv;
+	u16 vid_begin = vlan->vid_begin;
+	u16 vid_end = vlan->vid_end;
 	u32 member = 0;
 	u32 untag = 0;
 	u16 vid;
 	int ret;
 
-	for (vid = vlan->vid_begin; vid < vlan->vid_end; vid++)
-		if (!smi->ops->is_vlan_valid(smi, vid))
+	if (vid_begin == 0) {
+		dev_info(smi->dev, "set VLAN 0 on port %d = default VLAN\n",
+			 port);
+		/* Set up default tagging */
+		ret = rtl8366_set_default_vlan_and_pvid(smi, port);
+		if (ret) {
+			dev_err(smi->dev,
+				"error setting default VLAN on port %d\n",
+				port);
 			return;
+		}
+		if (vid_end == 0)
+			return;
+		/* Skip VLAN 0 and start with VLAN 1 */
+		vid_begin = 1;
+	}
 
-	dev_info(smi->dev, "add VLAN on port %d, %s, %s\n",
-		 port,
-		 untagged ? "untagged" : "tagged",
-		 pvid ? " PVID" : "no PVID");
+	for (vid = vid_begin; vid < vid_end; vid++)
+		if (!smi->ops->is_vlan_valid(smi, vid))
+			return;
 
 	if (dsa_is_dsa_port(ds, port) || dsa_is_cpu_port(ds, port))
 		dev_err(smi->dev, "port is DSA or CPU port\n");
 
-	for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid) {
+	for (vid = vid_begin; vid <= vid_end; ++vid) {
 		int pvid_val = 0;
 
-		dev_info(smi->dev, "add VLAN %04x\n", vid);
+		dev_info(smi->dev, "add VLAN %04x to port %d, %s, %s\n",
+			 vid,
+			 port,
+			 untagged ? "untagged" : "tagged",
+			 pvid ? " PVID" : "no PVID");
+
 		member |= BIT(port);
 
 		if (untagged)
@@ -437,15 +466,25 @@ int rtl8366_vlan_del(struct dsa_switch *ds, int port,
 		     const struct switchdev_obj_port_vlan *vlan)
 {
 	struct realtek_smi *smi = ds->priv;
+	u16 vid_begin = vlan->vid_begin;
+	u16 vid_end = vlan->vid_end;
 	u16 vid;
 	int ret;
 
-	dev_info(smi->dev, "del VLAN on port %d\n", port);
+	if (vid_begin == 0) {
+		dev_info(smi->dev, "remove port %d from VLAN 0 (no-op)\n",
+			 port);
+		if (vid_end == 0)
+			return 0;
+		/* Skip VLAN 0 and start with VLAN 1 */
+		vid_begin = 1;
+	}
 
-	for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid) {
+	for (vid = vid_begin; vid <= vid_end; ++vid) {
 		int i;
 
-		dev_info(smi->dev, "del VLAN %04x\n", vid);
+		dev_info(smi->dev, "remove VLAN %04x from port %d\n",
+			 vid, port);
 
 		for (i = 0; i < smi->num_vlan_mc; i++) {
 			struct rtl8366_vlan_mc vlanmc;
-- 
2.26.2

[net-next PATCH 5/5] net: dsa: rtl8366: Use top VLANs for default

[Linus Walleij]

The RTL8366 DSA switches will not work unless we set
up a default VLAN for each port. We are currently using
e.g. VLAN 1..6 for a 5-port switch as default VLANs.

This is not very helpful for users, move it to allocate
the top VLANs for default instead, for example on
RTL8366RB there are 16 VLANs so instead of using
VLAN 1..6 as default use VLAN 10..15 so VLAN 1
thru VLAN 9 is available for users.

Cc: DENG Qingfang <dqfext@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/net/dsa/rtl8366.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/rtl8366.c b/drivers/net/dsa/rtl8366.c
index 7f0691a6da13..4e7562b41598 100644
--- a/drivers/net/dsa/rtl8366.c
+++ b/drivers/net/dsa/rtl8366.c
@@ -260,8 +260,8 @@ static int rtl8366_set_default_vlan_and_pvid(struct realtek_smi *smi,
 	u16 vid;
 	int ret;
 
-	/* This is the reserved default VLAN for this port */
-	vid = port + 1;
+	/* Use the top VLANs for per-port default VLAN */
+	vid = smi->num_vlan_mc - smi->num_ports + port;
 
 	if (port == smi->cpu_port)
 		/* For the CPU port, make all ports members of this
-- 
2.26.2

Re: [net-next PATCH 1/5] net: dsa: tag_rtl4_a: Implement Realtek 4 byte A tag

[David Miller]

net-next is closed, sorry

Re: [net-next PATCH 5/5] net: dsa: rtl8366: Use top VLANs for default

[DENG Qingfang]

Hi Linus

On Wed, Jun 3, 2020 at 4:55 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> The RTL8366 DSA switches will not work unless we set
> up a default VLAN for each port. We are currently using
> e.g. VLAN 1..6 for a 5-port switch as default VLANs.
>
> This is not very helpful for users, move it to allocate
> the top VLANs for default instead, for example on
> RTL8366RB there are 16 VLANs so instead of using

RTL8366RB has full 4K vlan entry but you have to enable it manually.
https://github.com/openwrt/openwrt/blob/e73c61a978c56d41a2cdae4b5a2ca660aec4931b/target/linux/generic/files/drivers/net/phy/rtl8366rb.c#L43

> VLAN 1..6 as default use VLAN 10..15 so VLAN 1
> thru VLAN 9 is available for users.
>
> Cc: DENG Qingfang <dqfext@gmail.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/net/dsa/rtl8366.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/dsa/rtl8366.c b/drivers/net/dsa/rtl8366.c
> index 7f0691a6da13..4e7562b41598 100644
> --- a/drivers/net/dsa/rtl8366.c
> +++ b/drivers/net/dsa/rtl8366.c
> @@ -260,8 +260,8 @@ static int rtl8366_set_default_vlan_and_pvid(struct realtek_smi *smi,
>         u16 vid;
>         int ret;
>
> -       /* This is the reserved default VLAN for this port */
> -       vid = port + 1;
> +       /* Use the top VLANs for per-port default VLAN */
> +       vid = smi->num_vlan_mc - smi->num_ports + port;
>
>         if (port == smi->cpu_port)
>                 /* For the CPU port, make all ports members of this
> --
> 2.26.2
>

Re: [net-next PATCH 1/5] net: dsa: tag_rtl4_a: Implement Realtek 4 byte A tag

[Linus Walleij]

On Wed, Jun 3, 2020 at 12:48 AM David Miller <davem@davemloft.net> wrote:

> net-next is closed, sorry

No problem I just resend it later (I should have known since
it is merge window).

I'll watch this URL for opening hours:
http://vger.kernel.org/~davem/net-next.html

Yours,
Linus Walleij

Re: [net-next PATCH 5/5] net: dsa: rtl8366: Use top VLANs for default

[Linus Walleij]

On Wed, Jun 3, 2020 at 4:19 AM DENG Qingfang <dqfext@gmail.com> wrote:

> RTL8366RB has full 4K vlan entry but you have to enable it manually.
> https://github.com/openwrt/openwrt/blob/e73c61a978c56d41a2cdae4b5a2ca660aec4931b/target/linux/generic/files/drivers/net/phy/rtl8366rb.c#L43

Yep fixing up the 4K VLAN support is on my (slowly processed) TODO.
I will do that with a separate series later.

Thanks!
Linus Walleij

Re: [net-next PATCH 1/5] net: dsa: tag_rtl4_a: Implement Realtek 4 byte A tag

[Andrew Lunn]

> +static struct sk_buff *rtl4a_tag_rcv(struct sk_buff *skb,
> +				     struct net_device *dev,
> +				     struct packet_type *pt)
> +{
> +	u16 protport;
> +	__be16 *p;
> +	u16 etype;
> +	u8 flags;
> +	u8 *tag;
> +	u8 prot;
> +	u8 port;
> +
> +	if (unlikely(!pskb_may_pull(skb, RTL4_A_HDR_LEN)))
> +		return NULL;
> +
> +	/* The RTL4 header has its own custom Ethertype 0x8899 and that
> +	 * starts right at the beginning of the packet, after the src
> +	 * ethernet addr. Apparantly skb->data always points 2 bytes in,
> +	 * behind the Ethertype.
> +	 */
> +	tag = skb->data - 2;
> +	p = (__be16 *)tag;
> +	etype = ntohs(*p);
> +	if (etype != RTL4_A_ETHERTYPE) {
> +		/* Not custom, just pass through */
> +		netdev_dbg(dev, "non-realtek ethertype 0x%04x\n", etype);
> +		return skb;
> +	}
> +	p = (__be16 *)(tag + 2);
> +	protport = ntohs(*p);
> +	/* The 4 upper bits are the protocol */
> +	prot = (protport >> RTL4_A_PROTOCOL_SHIFT) & 0x0f;
> +	if (prot != RTL4_A_PROTOCOL_RTL8366RB) {
> +		netdev_err(dev, "unknown realtek protocol 0x%01x\n", prot);
> +		return NULL;
> +	}
> +	netdev_dbg(dev, "realtek protocol 0x%02x\n", prot);
> +	port = protport & 0xff;
> +	netdev_dbg(dev, "realtek port origin 0x%02x\n", port);
> +
> +	/* Remove RTL4 tag and recalculate checksum */
> +	skb_pull_rcsum(skb, RTL4_A_HDR_LEN);
> +
> +	/* Move ethernet DA and SA in front of the data */
> +	memmove(skb->data - ETH_HLEN,
> +		skb->data - ETH_HLEN - RTL4_A_HDR_LEN,
> +		2 * ETH_ALEN);
> +
> +	skb->dev = dsa_master_find_slave(dev, 0, port);
> +	if (!skb->dev) {
> +		netdev_dbg(dev, "could not find slave for port %d\n", port);
> +		return NULL;
> +	}
> +	netdev_dbg(skb->dev, "forwarded packet to slave port %d\n", port);
> +
> +	skb->offload_fwd_mark = 1;
> +
> +	return skb;
> +}

Hi Linus

Do you think you are passed basic debug/reverse engineering? There are
a lot of netdev_dbg() statements here. It would be nice to remove most
of them, if you think the code is stable.

Is there any hint in OpenRRPC that tags can be used in the other
direction? Where is spanning tree performed? In the switch, or by the
host? That is one example where the host needs to be able to
send/receive frames on specific ports.

       Andrew

Re: [net-next PATCH 1/5] net: dsa: tag_rtl4_a: Implement Realtek 4 byte A tag

[Linus Walleij]

Hi Andrew!

On Wed, Jun 3, 2020 at 3:52 PM Andrew Lunn <andrew@lunn.ch> wrote:

> Do you think you are passed basic debug/reverse engineering? There are
> a lot of netdev_dbg() statements here. It would be nice to remove most
> of them, if you think the code is stable.

OK

> Is there any hint in OpenRRPC that tags can be used in the other
> direction?

It appears OpenRRPC is something totally different, but yeah
I looked in that code. I have documentation on three other tag
types: a 8 byte tag (not similar) another 4 byte tag (protocol 9)
and I looked at the RTL838x kernel code which has a third
tag format (trailer). None of it is very helpful.

> Where is spanning tree performed? In the switch, or by the
> host?

In the switch I think.
There is a register in the ASIC to set the spanning tree status
to disabled, blocking, learning or forwarding.

> That is one example where the host needs to be able to
> send/receive frames on specific ports.

No luck there I'm afraid :/

Yours,
Linus Walleij

Re: [net-next PATCH 1/5] net: dsa: tag_rtl4_a: Implement Realtek 4 byte A tag

[Andrew Lunn]

> > Where is spanning tree performed? In the switch, or by the
> > host?
> 
> In the switch I think.
> There is a register in the ASIC to set the spanning tree status
> to disabled, blocking, learning or forwarding.

Hi Linus

Spanning tree needs two parts. You need to be able to change the ports
between disabled, blocking, learning, forwarding. And you need to be
able to send/receive frames.

If spanning tree is performed in the ASIC, i don't see why there would
be registers to control the port status. It would do it all itself,
and not export these controls.

So i would not give up on spanning tree as a way to reverse engineer
this.

    Andrew

Re: [net-next PATCH 1/5] net: dsa: tag_rtl4_a: Implement Realtek 4 byte A tag

[Linus Walleij]

On Thu, Jun 4, 2020 at 2:54 AM Andrew Lunn <andrew@lunn.ch> wrote:

> If spanning tree is performed in the ASIC, i don't see why there would
> be registers to control the port status. It would do it all itself,
> and not export these controls.
>
> So i would not give up on spanning tree as a way to reverse engineer
> this.

Hm I guess I have to take out the textbooks and refresh my lacking
knowledge about spanning tree :)

What I have for "documentation" is the code drop inside DD Wrt:
https://svn.dd-wrt.com//browser/src/linux/universal/linux-3.2/drivers/net/ethernet/raeth/rb

The code is a bit messy and seems hacked up by Realtek, also
at one point apparently the ASIC was closely related to  RTL8368s
and then renamed to RTL8366RB...

The code accessing the ASIC is here (under the name RTL8368s):
https://svn.dd-wrt.com/browser/src/linux/universal/linux-3.2/drivers/net/ethernet/raeth/rb/rtl8368s_asicdrv.c

I'm hacking on it but a bit stuck :/

Yours,
Linus Walleij

Re: [net-next PATCH 1/5] net: dsa: tag_rtl4_a: Implement Realtek 4 byte A tag

[Vladimir Oltean]

Hi Linus,

On Thu, 4 Jun 2020 at 12:17, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Thu, Jun 4, 2020 at 2:54 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > If spanning tree is performed in the ASIC, i don't see why there would
> > be registers to control the port status. It would do it all itself,
> > and not export these controls.
> >
> > So i would not give up on spanning tree as a way to reverse engineer
> > this.
>
> Hm I guess I have to take out the textbooks and refresh my lacking
> knowledge about spanning tree :)
>
> What I have for "documentation" is the code drop inside DD Wrt:
> https://svn.dd-wrt.com//browser/src/linux/universal/linux-3.2/drivers/net/ethernet/raeth/rb
>
> The code is a bit messy and seems hacked up by Realtek, also
> at one point apparently the ASIC was closely related to  RTL8368s
> and then renamed to RTL8366RB...
>
> The code accessing the ASIC is here (under the name RTL8368s):
> https://svn.dd-wrt.com/browser/src/linux/universal/linux-3.2/drivers/net/ethernet/raeth/rb/rtl8368s_asicdrv.c
>
> I'm hacking on it but a bit stuck :/
>
> Yours,
> Linus Walleij

In the code you pointed to, there is a potentially relevant comment:

1532//CPU tag: Realtek Ethertype==0x8899(2 bytes)+protocol==0x9(4
MSB)+priority(2 bits)+reserved(4 bits)+portmask(6 LSB)

https://svn.dd-wrt.com/browser/src/linux/universal/linux-3.2/drivers/net/ethernet/raeth/rb/rtl_multicast_snooping.c#L1527
https://svn.dd-wrt.com/browser/src/linux/universal/linux-3.2/drivers/net/ethernet/raeth/rb/rtl_multicast_snooping.c#L5224

This strongly indicates to me that the insertion tag is the same as
the extraction tag.
It is completely opaque to me why in patch "[net-next PATCH 2/5] net:
dsa: rtl8366rb: Support the CPU DSA tag" you are _disabling_ the
injection of these tags via RTL8368RB_CPU_INSTAG. I think it's natural
that the switch drops these packets when CPU tag insertion is
disabled.

Thanks,
-Vladimir

Re: [net-next PATCH 1/5] net: dsa: tag_rtl4_a: Implement Realtek 4 byte A tag

[Linus Walleij]

On Thu, Jun 4, 2020 at 1:23 PM Vladimir Oltean <olteanv@gmail.com> wrote:

> In the code you pointed to, there is a potentially relevant comment:
>
> 1532//CPU tag: Realtek Ethertype==0x8899(2 bytes)+protocol==0x9(4
> MSB)+priority(2 bits)+reserved(4 bits)+portmask(6 LSB)
>
> https://svn.dd-wrt.com/browser/src/linux/universal/linux-3.2/drivers/net/ethernet/raeth/rb/rtl_multicast_snooping.c#L1527
> https://svn.dd-wrt.com/browser/src/linux/universal/linux-3.2/drivers/net/ethernet/raeth/rb/rtl_multicast_snooping.c#L5224
>
> This strongly indicates to me that the insertion tag is the same as
> the extraction tag.

This code is a problem because it is Realtek-development style.
This style seems to be that the hardware people write the drivers
using copy/paste from the previous ASIC and ship is as soon as
possible. Keep this in mind.

The above tag is using protocol 9 and is actually even documented
in a PDF I have for RTL8306. The problem is that the RTL8366RB
(I suspect also RTL8366S) uses protocol "a" (as in hex 10).
Which is of course necessarily different.

I have *really* tried to figure out how the bits in protocol a works
when transmissing from the CPU port to any switch port.

When nothing else worked, I just tried all bit combinations with
0xannp where a is protocol and p is port. I looped through all
values several times trying to get a response from ping.

So this is really how far I can get right now, even with brute
force.

> It is completely opaque to me why in patch "[net-next PATCH 2/5] net:
> dsa: rtl8366rb: Support the CPU DSA tag" you are _disabling_ the
> injection of these tags via RTL8368RB_CPU_INSTAG. I think it's natural
> that the switch drops these packets when CPU tag insertion is
> disabled.

This is another Realtek-ism where they managed to invert the
meaning of a bit.

Bit 15 in register 0x0061 (RTL8368RB_CPU_CTRL_REG) can
be set to 1 and then the special (custom) CPU tag 0x8899
protocol a will be DISABLED. This value Realtek calls
"RTL8368RB_CPU_INSTAG" which makes you think that
the tag will be inserted, it is named "instag" right? But that
is not how it works.

That bit needs to be set to 0 to insert the tag and 1 to disable
insertion of the tag.

For this reason the patch also renames this bit to
RTL8368RB_CPU_NO_TAG which is more to the point.

Yours,
Linus Walleij

Re: [net-next PATCH 1/5] net: dsa: tag_rtl4_a: Implement Realtek 4 byte A tag

[DENG Qingfang]

Hi

On Wed, Jun 17, 2020 at 4:06 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Thu, Jun 4, 2020 at 1:23 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> > In the code you pointed to, there is a potentially relevant comment:
> >
> > 1532//CPU tag: Realtek Ethertype==0x8899(2 bytes)+protocol==0x9(4
> > MSB)+priority(2 bits)+reserved(4 bits)+portmask(6 LSB)
> >
> > https://svn.dd-wrt.com/browser/src/linux/universal/linux-3.2/drivers/net/ethernet/raeth/rb/rtl_multicast_snooping.c#L1527
> > https://svn.dd-wrt.com/browser/src/linux/universal/linux-3.2/drivers/net/ethernet/raeth/rb/rtl_multicast_snooping.c#L5224
> >
> > This strongly indicates to me that the insertion tag is the same as
> > the extraction tag.
>
> This code is a problem because it is Realtek-development style.
> This style seems to be that the hardware people write the drivers
> using copy/paste from the previous ASIC and ship is as soon as
> possible. Keep this in mind.
>
> The above tag is using protocol 9 and is actually even documented
> in a PDF I have for RTL8306. The problem is that the RTL8366RB
> (I suspect also RTL8366S) uses protocol "a" (as in hex 10).
> Which is of course necessarily different.
>
> I have *really* tried to figure out how the bits in protocol a works
> when transmissing from the CPU port to any switch port.
>
> When nothing else worked, I just tried all bit combinations with
> 0xannp where a is protocol and p is port. I looped through all
> values several times trying to get a response from ping.

Have you looped through the whole 32-bit field?

>
> So this is really how far I can get right now, even with brute
> force.
>
> > It is completely opaque to me why in patch "[net-next PATCH 2/5] net:
> > dsa: rtl8366rb: Support the CPU DSA tag" you are _disabling_ the
> > injection of these tags via RTL8368RB_CPU_INSTAG. I think it's natural
> > that the switch drops these packets when CPU tag insertion is
> > disabled.
>
> This is another Realtek-ism where they managed to invert the
> meaning of a bit.
>
> Bit 15 in register 0x0061 (RTL8368RB_CPU_CTRL_REG) can
> be set to 1 and then the special (custom) CPU tag 0x8899
> protocol a will be DISABLED. This value Realtek calls
> "RTL8368RB_CPU_INSTAG" which makes you think that
> the tag will be inserted, it is named "instag" right? But that
> is not how it works.
>
> That bit needs to be set to 0 to insert the tag and 1 to disable
> insertion of the tag.
>
> For this reason the patch also renames this bit to
> RTL8368RB_CPU_NO_TAG which is more to the point.
>
> Yours,
> Linus Walleij