| Message ID | 20200602205456.2392024-1-linus.walleij@linaro.org |
|---|---|
| State | Deferred |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next,1/5] net: dsa: tag_rtl4_a: Implement Realtek 4 byte A tag | expand |
net-next is closed, sorry
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
> +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
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
> > 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
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
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
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
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
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);
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