| Message ID | 20210217062139.7893-2-dqfext@gmail.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | DSA driver for Realtek RTL8366S/SR | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 7 of 7 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 14 this patch: 14 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: please write a paragraph that describes the config symbol fully |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 14 this patch: 14 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On 17.02.2021 07:21, DENG Qingfang wrote: > Add support for Realtek RTL8366S switch tag > > Signed-off-by: DENG Qingfang <dqfext@gmail.com> > --- > include/net/dsa.h | 2 + > net/dsa/Kconfig | 6 +++ > net/dsa/Makefile | 1 + > net/dsa/tag_rtl8366s.c | 98 ++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 107 insertions(+) > create mode 100644 net/dsa/tag_rtl8366s.c > > diff --git a/include/net/dsa.h b/include/net/dsa.h > index 83a933e563fe..14fedf832f27 100644 > --- a/include/net/dsa.h > +++ b/include/net/dsa.h > @@ -49,6 +49,7 @@ struct phylink_link_state; > #define DSA_TAG_PROTO_XRS700X_VALUE 19 > #define DSA_TAG_PROTO_OCELOT_8021Q_VALUE 20 > #define DSA_TAG_PROTO_SEVILLE_VALUE 21 > +#define DSA_TAG_PROTO_RTL8366S_VALUE 22 > > enum dsa_tag_protocol { > DSA_TAG_PROTO_NONE = DSA_TAG_PROTO_NONE_VALUE, > @@ -73,6 +74,7 @@ enum dsa_tag_protocol { > DSA_TAG_PROTO_XRS700X = DSA_TAG_PROTO_XRS700X_VALUE, > DSA_TAG_PROTO_OCELOT_8021Q = DSA_TAG_PROTO_OCELOT_8021Q_VALUE, > DSA_TAG_PROTO_SEVILLE = DSA_TAG_PROTO_SEVILLE_VALUE, > + DSA_TAG_PROTO_RTL8366S = DSA_TAG_PROTO_RTL8366S_VALUE, > }; > > struct packet_type; > diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig > index a45572cfb71a..303228e0ad8b 100644 > --- a/net/dsa/Kconfig > +++ b/net/dsa/Kconfig > @@ -104,6 +104,12 @@ config NET_DSA_TAG_RTL4_A > Realtek switches with 4 byte protocol A tags, sich as found in > the Realtek RTL8366RB. > > +config NET_DSA_TAG_RTL8366S > + tristate "Tag driver for Realtek RTL8366S switch tags" > + help > + Say Y or M if you want to enable support for tagging frames for the > + Realtek RTL8366S switch. > + > config NET_DSA_TAG_OCELOT > tristate "Tag driver for Ocelot family of switches, using NPI port" > select PACKING > diff --git a/net/dsa/Makefile b/net/dsa/Makefile > index 44bc79952b8b..df158e73a30b 100644 > --- a/net/dsa/Makefile > +++ b/net/dsa/Makefile > @@ -12,6 +12,7 @@ obj-$(CONFIG_NET_DSA_TAG_GSWIP) += tag_gswip.o > obj-$(CONFIG_NET_DSA_TAG_HELLCREEK) += tag_hellcreek.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_RTL8366S) += tag_rtl8366s.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_rtl8366s.c b/net/dsa/tag_rtl8366s.c > new file mode 100644 > index 000000000000..6c6c66853e4c > --- /dev/null > +++ b/net/dsa/tag_rtl8366s.c > @@ -0,0 +1,98 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Handler for Realtek RTL8366S switch tags > + * Copyright (c) 2021 DENG, Qingfang <dqfext@gmail.com> > + * > + * This tag header looks like: > + * > + * ------------------------------------------------- > + * | MAC DA | MAC SA | 0x8899 | 2-byte tag | Type | > + * ------------------------------------------------- > + * > + * The 2-byte tag format in tag_rcv: > + * +------+------+------+------+------+------+------+------+ > + * 15: 8 | Protocol number (0x9) | Priority | Reserved | > + * +------+------+------+------+------+------+------+------+ > + * 7: 0 | Reserved | Source port number | > + * +------+------+------+------+------+------+------+------+ > + * > + * The 2-byte tag format in tag_xmit: > + * +------+------+------+------+------+------+------+------+ > + * 15: 8 | Protocol number (0x9) | Priority | Reserved | > + * +------+------+------+------+------+------+------+------+ > + * 7: 0 | Reserved | Destination port mask | > + * +------+------+------+------+------+------+------+------+ > + */ > + > +#include <linux/etherdevice.h> > + > +#include "dsa_priv.h" > + > +#define RTL8366S_HDR_LEN 4 > +#define RTL8366S_ETHERTYPE 0x8899 I found this protocol referenced as Realtek Remote Control Protocol (RRCP) and it seems to be used by few Realtek chips. Not sure whether this protocol is officially registered. If yes, then it should be added to the list of ethernet protocol id's in include/uapi/linux/if_ether.h. If not, then it may be better to define it using the usual naming scheme as ETH_P_RRCP in realtek-smi-core.h. > +#define RTL8366S_PROTOCOL_SHIFT 12 > +#define RTL8366S_PROTOCOL 0x9 > +#define RTL8366S_TAG \ > + (RTL8366S_PROTOCOL << RTL8366S_PROTOCOL_SHIFT) > + > +static struct sk_buff *rtl8366s_tag_xmit(struct sk_buff *skb, > + struct net_device *dev) > +{ > + struct dsa_port *dp = dsa_slave_to_port(dev); > + __be16 *tag; > + > + /* Make sure the frame is at least 60 bytes long _before_ > + * inserting the CPU tag, or it will be dropped by the switch. > + */ > + if (unlikely(eth_skb_pad(skb))) > + return NULL; > + > + skb_push(skb, RTL8366S_HDR_LEN); > + memmove(skb->data, skb->data + RTL8366S_HDR_LEN, > + 2 * ETH_ALEN); > + > + tag = (__be16 *)(skb->data + 2 * ETH_ALEN); > + tag[0] = htons(RTL8366S_ETHERTYPE); > + tag[1] = htons(RTL8366S_TAG | BIT(dp->index)); > + > + return skb; > +} > + > +static struct sk_buff *rtl8366s_tag_rcv(struct sk_buff *skb, > + struct net_device *dev, > + struct packet_type *pt) > +{ > + u8 *tag; > + u8 port; > + > + if (unlikely(!pskb_may_pull(skb, RTL8366S_HDR_LEN))) > + return NULL; > + > + tag = skb->data - 2; > + port = tag[3] & 0x7; > + > + skb->dev = dsa_master_find_slave(dev, 0, port); > + if (unlikely(!skb->dev)) > + return NULL; > + > + skb_pull_rcsum(skb, RTL8366S_HDR_LEN); > + memmove(skb->data - ETH_HLEN, > + skb->data - ETH_HLEN - RTL8366S_HDR_LEN, > + 2 * ETH_ALEN); > + > + skb->offload_fwd_mark = 1; > + > + return skb; > +} > + > +static const struct dsa_device_ops rtl8366s_netdev_ops = { > + .name = "rtl8366s", > + .proto = DSA_TAG_PROTO_RTL8366S, > + .xmit = rtl8366s_tag_xmit, > + .rcv = rtl8366s_tag_rcv, > + .overhead = RTL8366S_HDR_LEN, > +}; > +module_dsa_tag_driver(rtl8366s_netdev_ops); > + > +MODULE_LICENSE("GPL"); > +MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_RTL8366S); >
On Wed, Feb 17, 2021 at 7:21 AM DENG Qingfang <dqfext@gmail.com> wrote: > Add support for Realtek RTL8366S switch tag > > Signed-off-by: DENG Qingfang <dqfext@gmail.com> I suppose this switch can just use the existing RTL4 A tag code after the recent patch so this one is not needed? Yours, Linus Walleij
On Wed, Feb 17, 2021 at 8:08 AM Heiner Kallweit <hkallweit1@gmail.com> wrote: > > +#define RTL8366S_HDR_LEN 4 > > +#define RTL8366S_ETHERTYPE 0x8899 > > I found this protocol referenced as Realtek Remote Control Protocol (RRCP) > and it seems to be used by few Realtek chips. Not sure whether this > protocol is officially registered. If yes, then it should be added to > the list of ethernet protocol id's in include/uapi/linux/if_ether.h. > If not, then it may be better to define it using the usual naming > scheme as ETH_P_RRCP in realtek-smi-core.h. It's actually quite annoying, Realtek use type 0x8899 for all their custom stuff, including RRCP and internal DSA tagging inside switches, which are two completely different use cases. When I expose raw DSA frames to wireshark it identifies it as "Realtek RRCP" and then naturally cannot decode the frames since this is not RRCP but another protocol identified by the same ethertype. Inside DSA it works as we explicitly asks tells the kernel using the tagging code in net/dsa/tag_rtl4_a.c that this is the DSA version of ethertype 0x8899 and it then goes to dissect the actual 4 bytes tag. There are at least four protocols out there using ethertype 0x8899. Yours, Linus Walleij
On 17.02.2021 12:01, Linus Walleij wrote: > On Wed, Feb 17, 2021 at 8:08 AM Heiner Kallweit <hkallweit1@gmail.com> wrote: > >>> +#define RTL8366S_HDR_LEN 4 >>> +#define RTL8366S_ETHERTYPE 0x8899 >> >> I found this protocol referenced as Realtek Remote Control Protocol (RRCP) >> and it seems to be used by few Realtek chips. Not sure whether this >> protocol is officially registered. If yes, then it should be added to >> the list of ethernet protocol id's in include/uapi/linux/if_ether.h. >> If not, then it may be better to define it using the usual naming >> scheme as ETH_P_RRCP in realtek-smi-core.h. > > It's actually quite annoying, Realtek use type 0x8899 for all their > custom stuff, including RRCP and internal DSA tagging inside > switches, which are two completely different use cases. > > When I expose raw DSA frames to wireshark it identifies it > as "Realtek RRCP" and then naturally cannot decode the > frames since this is not RRCP but another protocol identified > by the same ethertype. > > Inside DSA it works as we explicitly asks tells the kernel using the > tagging code in net/dsa/tag_rtl4_a.c that this is the DSA version > of ethertype 0x8899 and it then goes to dissect the actual > 4 bytes tag. > > There are at least four protocols out there using ethertype 0x8899. > Ugly .. Thanks for the details! > Yours, > Linus Walleij >
On Wed, Feb 17, 2021 at 6:55 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > I suppose this switch can just use the existing RTL4 A tag > code after the recent patch so this one is not needed? No. Different protocol (0x9 instead of 0xA) and different xmit tag format (port bit mask instead of port number). Please take a closer look. > > Yours, > Linus Walleij
On Wed, Feb 17, 2021 at 7:21 AM DENG Qingfang <dqfext@gmail.com> wrote: > Add support for Realtek RTL8366S switch tag > > Signed-off-by: DENG Qingfang <dqfext@gmail.com> I understand this switch tag now sorry for confusion. > @@ -104,6 +104,12 @@ config NET_DSA_TAG_RTL4_A > Realtek switches with 4 byte protocol A tags, sich as found in > the Realtek RTL8366RB. > > +config NET_DSA_TAG_RTL8366S > + tristate "Tag driver for Realtek RTL8366S switch tags" > + help > + Say Y or M if you want to enable support for tagging frames for the > + Realtek RTL8366S switch. I names the previous protocol "RTL4 A" after a 4-byte tag with protocol indicted as "A", what about naming this "RTL2 9" in the same vein? It will be good if some other switch is using the same protocol. (If any...) > obj-$(CONFIG_NET_DSA_TAG_RTL4_A) += tag_rtl4_a.o > +obj-$(CONFIG_NET_DSA_TAG_RTL8366S) += tag_rtl8366s.o So tag_rtl2_9.o etc. Yours, Linus Walleij
On Sun, Feb 28, 2021 at 7:47 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > I names the previous protocol "RTL4 A" after a 4-byte tag > with protocol indicted as "A", what about naming this > "RTL2 9" in the same vein? It will be good if some other > switch is using the same protocol. (If any...) RTL8306 uses 0x9 too, but the tag format is different... > > > obj-$(CONFIG_NET_DSA_TAG_RTL4_A) += tag_rtl4_a.o > > +obj-$(CONFIG_NET_DSA_TAG_RTL8366S) += tag_rtl8366s.o > > So tag_rtl2_9.o etc. > > Yours, > Linus Walleij
On Sun, Feb 28, 2021 at 12:32 PM DENG Qingfang <dqfext@gmail.com> wrote: > On Sun, Feb 28, 2021 at 7:47 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > > > I names the previous protocol "RTL4 A" after a 4-byte tag > > with protocol indicted as "A", what about naming this > > "RTL2 9" in the same vein? It will be good if some other > > switch is using the same protocol. (If any...) > > RTL8306 uses 0x9 too, but the tag format is different... Oh what a mess. :O OK go with this. Maybe I should rename my tagger to tag_rtl8366rb.c instead... Yours, Linus Walleij
diff --git a/include/net/dsa.h b/include/net/dsa.h index 83a933e563fe..14fedf832f27 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -49,6 +49,7 @@ struct phylink_link_state; #define DSA_TAG_PROTO_XRS700X_VALUE 19 #define DSA_TAG_PROTO_OCELOT_8021Q_VALUE 20 #define DSA_TAG_PROTO_SEVILLE_VALUE 21 +#define DSA_TAG_PROTO_RTL8366S_VALUE 22 enum dsa_tag_protocol { DSA_TAG_PROTO_NONE = DSA_TAG_PROTO_NONE_VALUE, @@ -73,6 +74,7 @@ enum dsa_tag_protocol { DSA_TAG_PROTO_XRS700X = DSA_TAG_PROTO_XRS700X_VALUE, DSA_TAG_PROTO_OCELOT_8021Q = DSA_TAG_PROTO_OCELOT_8021Q_VALUE, DSA_TAG_PROTO_SEVILLE = DSA_TAG_PROTO_SEVILLE_VALUE, + DSA_TAG_PROTO_RTL8366S = DSA_TAG_PROTO_RTL8366S_VALUE, }; struct packet_type; diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig index a45572cfb71a..303228e0ad8b 100644 --- a/net/dsa/Kconfig +++ b/net/dsa/Kconfig @@ -104,6 +104,12 @@ config NET_DSA_TAG_RTL4_A Realtek switches with 4 byte protocol A tags, sich as found in the Realtek RTL8366RB. +config NET_DSA_TAG_RTL8366S + tristate "Tag driver for Realtek RTL8366S switch tags" + help + Say Y or M if you want to enable support for tagging frames for the + Realtek RTL8366S switch. + config NET_DSA_TAG_OCELOT tristate "Tag driver for Ocelot family of switches, using NPI port" select PACKING diff --git a/net/dsa/Makefile b/net/dsa/Makefile index 44bc79952b8b..df158e73a30b 100644 --- a/net/dsa/Makefile +++ b/net/dsa/Makefile @@ -12,6 +12,7 @@ obj-$(CONFIG_NET_DSA_TAG_GSWIP) += tag_gswip.o obj-$(CONFIG_NET_DSA_TAG_HELLCREEK) += tag_hellcreek.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_RTL8366S) += tag_rtl8366s.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_rtl8366s.c b/net/dsa/tag_rtl8366s.c new file mode 100644 index 000000000000..6c6c66853e4c --- /dev/null +++ b/net/dsa/tag_rtl8366s.c @@ -0,0 +1,98 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Handler for Realtek RTL8366S switch tags + * Copyright (c) 2021 DENG, Qingfang <dqfext@gmail.com> + * + * This tag header looks like: + * + * ------------------------------------------------- + * | MAC DA | MAC SA | 0x8899 | 2-byte tag | Type | + * ------------------------------------------------- + * + * The 2-byte tag format in tag_rcv: + * +------+------+------+------+------+------+------+------+ + * 15: 8 | Protocol number (0x9) | Priority | Reserved | + * +------+------+------+------+------+------+------+------+ + * 7: 0 | Reserved | Source port number | + * +------+------+------+------+------+------+------+------+ + * + * The 2-byte tag format in tag_xmit: + * +------+------+------+------+------+------+------+------+ + * 15: 8 | Protocol number (0x9) | Priority | Reserved | + * +------+------+------+------+------+------+------+------+ + * 7: 0 | Reserved | Destination port mask | + * +------+------+------+------+------+------+------+------+ + */ + +#include <linux/etherdevice.h> + +#include "dsa_priv.h" + +#define RTL8366S_HDR_LEN 4 +#define RTL8366S_ETHERTYPE 0x8899 +#define RTL8366S_PROTOCOL_SHIFT 12 +#define RTL8366S_PROTOCOL 0x9 +#define RTL8366S_TAG \ + (RTL8366S_PROTOCOL << RTL8366S_PROTOCOL_SHIFT) + +static struct sk_buff *rtl8366s_tag_xmit(struct sk_buff *skb, + struct net_device *dev) +{ + struct dsa_port *dp = dsa_slave_to_port(dev); + __be16 *tag; + + /* Make sure the frame is at least 60 bytes long _before_ + * inserting the CPU tag, or it will be dropped by the switch. + */ + if (unlikely(eth_skb_pad(skb))) + return NULL; + + skb_push(skb, RTL8366S_HDR_LEN); + memmove(skb->data, skb->data + RTL8366S_HDR_LEN, + 2 * ETH_ALEN); + + tag = (__be16 *)(skb->data + 2 * ETH_ALEN); + tag[0] = htons(RTL8366S_ETHERTYPE); + tag[1] = htons(RTL8366S_TAG | BIT(dp->index)); + + return skb; +} + +static struct sk_buff *rtl8366s_tag_rcv(struct sk_buff *skb, + struct net_device *dev, + struct packet_type *pt) +{ + u8 *tag; + u8 port; + + if (unlikely(!pskb_may_pull(skb, RTL8366S_HDR_LEN))) + return NULL; + + tag = skb->data - 2; + port = tag[3] & 0x7; + + skb->dev = dsa_master_find_slave(dev, 0, port); + if (unlikely(!skb->dev)) + return NULL; + + skb_pull_rcsum(skb, RTL8366S_HDR_LEN); + memmove(skb->data - ETH_HLEN, + skb->data - ETH_HLEN - RTL8366S_HDR_LEN, + 2 * ETH_ALEN); + + skb->offload_fwd_mark = 1; + + return skb; +} + +static const struct dsa_device_ops rtl8366s_netdev_ops = { + .name = "rtl8366s", + .proto = DSA_TAG_PROTO_RTL8366S, + .xmit = rtl8366s_tag_xmit, + .rcv = rtl8366s_tag_rcv, + .overhead = RTL8366S_HDR_LEN, +}; +module_dsa_tag_driver(rtl8366s_netdev_ops); + +MODULE_LICENSE("GPL"); +MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_RTL8366S);
Add support for Realtek RTL8366S switch tag Signed-off-by: DENG Qingfang <dqfext@gmail.com> --- include/net/dsa.h | 2 + net/dsa/Kconfig | 6 +++ net/dsa/Makefile | 1 + net/dsa/tag_rtl8366s.c | 98 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 107 insertions(+) create mode 100644 net/dsa/tag_rtl8366s.c