| Message ID | 20220504093000.132579-3-clement.leger@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | add support for Renesas RZ/N1 ethernet subsystem devices | expand |
On Wed, May 04, 2022 at 11:29:50AM +0200, Clément Léger wrote: > The switch that is present on the Renesas RZ/N1 SoC uses a specific > VLAN value followed by 6 bytes which contains forwarding configuration. > > Signed-off-by: Clément Léger <clement.leger@bootlin.com> > --- > include/net/dsa.h | 2 + > net/dsa/Kconfig | 7 +++ > net/dsa/Makefile | 1 + > net/dsa/tag_rzn1_a5psw.c | 114 +++++++++++++++++++++++++++++++++++++++ > 4 files changed, 124 insertions(+) > create mode 100644 net/dsa/tag_rzn1_a5psw.c > > diff --git a/include/net/dsa.h b/include/net/dsa.h > index d9da32aacbf1..9aaaa7deb102 100644 > --- a/include/net/dsa.h > +++ b/include/net/dsa.h > @@ -53,6 +53,7 @@ struct phylink_link_state; > #define DSA_TAG_PROTO_SJA1110_VALUE 23 > #define DSA_TAG_PROTO_RTL8_4_VALUE 24 > #define DSA_TAG_PROTO_RTL8_4T_VALUE 25 > +#define DSA_TAG_PROTO_RZN1_A5PSW_VALUE 26 > > enum dsa_tag_protocol { > DSA_TAG_PROTO_NONE = DSA_TAG_PROTO_NONE_VALUE, > @@ -81,6 +82,7 @@ enum dsa_tag_protocol { > DSA_TAG_PROTO_SJA1110 = DSA_TAG_PROTO_SJA1110_VALUE, > DSA_TAG_PROTO_RTL8_4 = DSA_TAG_PROTO_RTL8_4_VALUE, > DSA_TAG_PROTO_RTL8_4T = DSA_TAG_PROTO_RTL8_4T_VALUE, > + DSA_TAG_PROTO_RZN1_A5PSW = DSA_TAG_PROTO_RZN1_A5PSW_VALUE, > }; > > struct dsa_switch; > diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig > index 8cb87b5067ee..e22f33d7cf60 100644 > --- a/net/dsa/Kconfig > +++ b/net/dsa/Kconfig > @@ -132,6 +132,13 @@ config NET_DSA_TAG_RTL8_4 > Say Y or M if you want to enable support for tagging frames for Realtek > switches with 8 byte protocol 4 tags, such as the Realtek RTL8365MB-VC. > > +config NET_DSA_TAG_RZN1_A5PSW > + tristate "Tag driver for Renesas RZ/N1 A5PSW switch" > + help > + Say Y or M if you want to enable support for tagging frames for > + Renesas RZ/N1 embedded switch that uses a 8 byte tag located after "an" 8 byte tag > + destination MAC address. > + > config NET_DSA_TAG_LAN9303 > tristate "Tag driver for SMSC/Microchip LAN9303 family of switches" > help > diff --git a/net/dsa/Makefile b/net/dsa/Makefile > index 9f75820e7c98..af28c24ead18 100644 > --- a/net/dsa/Makefile > +++ b/net/dsa/Makefile > @@ -17,6 +17,7 @@ obj-$(CONFIG_NET_DSA_TAG_OCELOT_8021Q) += tag_ocelot_8021q.o > obj-$(CONFIG_NET_DSA_TAG_QCA) += tag_qca.o > obj-$(CONFIG_NET_DSA_TAG_RTL4_A) += tag_rtl4_a.o > obj-$(CONFIG_NET_DSA_TAG_RTL8_4) += tag_rtl8_4.o > +obj-$(CONFIG_NET_DSA_TAG_RZN1_A5PSW) += tag_rzn1_a5psw.o > obj-$(CONFIG_NET_DSA_TAG_SJA1105) += tag_sja1105.o > obj-$(CONFIG_NET_DSA_TAG_TRAILER) += tag_trailer.o > obj-$(CONFIG_NET_DSA_TAG_XRS700X) += tag_xrs700x.o > diff --git a/net/dsa/tag_rzn1_a5psw.c b/net/dsa/tag_rzn1_a5psw.c > new file mode 100644 > index 000000000000..87177682eb54 > --- /dev/null > +++ b/net/dsa/tag_rzn1_a5psw.c > @@ -0,0 +1,114 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2022 Schneider Electric > + * > + * Clément Léger <clement.leger@bootlin.com> > + */ > + > +#include <linux/bitfield.h> > +#include <linux/etherdevice.h> > +#include <net/dsa.h> > + > +#include "dsa_priv.h" > + > +/* To define the outgoing port and to discover the incoming port a TAG is > + * inserted after Src MAC : > + * > + * Dest MAC Src MAC TAG Type > + * ...| 1 2 3 4 5 6 | 1 2 3 4 5 6 | 1 2 3 4 5 6 7 8 | 1 2 |... > + * |<--------------->| > + * > + * See struct a5psw_tag for layout > + */ > + > +#define A5PSW_TAG_VALUE 0xE001 Maybe an ETH_P_DSA_A5PSW definition in include/uapi/linux/if_ether.h would be appropriate? I'm not sure. > +#define A5PSW_TAG_LEN 8 > +#define A5PSW_CTRL_DATA_FORCE_FORWARD BIT(0) > +/* This is both used for xmit tag and rcv tagging */ > +#define A5PSW_CTRL_DATA_PORT GENMASK(3, 0) > + > +struct a5psw_tag { > + __be16 ctrl_tag; > + __be16 ctrl_data; > + __be16 ctrl_data2_hi; > + __be16 ctrl_data2_lo; > +}; > + > +static struct sk_buff *a5psw_tag_xmit(struct sk_buff *skb, struct net_device *dev) > +{ > + struct dsa_port *dp = dsa_slave_to_port(dev); > + struct a5psw_tag *ptag; > + u32 data2_val; > + > + BUILD_BUG_ON(sizeof(*ptag) != A5PSW_TAG_LEN); > + > + /* The Ethernet switch we are interfaced with needs packets to be at > + * least 64 bytes (including FCS) otherwise they will be discarded when > + * they enter the switch port logic. When tagging is enabled, we need > + * to make sure that packets are at least 70 bytes (including FCS and > + * tag). > + */ > + if (__skb_put_padto(skb, ETH_ZLEN + ETH_FCS_LEN + A5PSW_TAG_LEN, false)) > + return NULL; I'm confused by the inclusion of the FCS length in this calculation, since the FCS space isn't present in the skb buffer as far as I know? "64 bytes including FCS" means "60 bytes excluding FCS". And ETH_ZLEN is 60... And I'm also not sure how we got to the number 70? A5PSW_TAG_LEN is 8. If we add it to 60 we get 68. If we add it to 64 we get 72? > + > + /* provide 'A5PSW_TAG_LEN' bytes additional space */ > + skb_push(skb, A5PSW_TAG_LEN); > + > + /* make room between MACs and Ether-Type to insert tag */ > + dsa_alloc_etype_header(skb, A5PSW_TAG_LEN); > + > + ptag = dsa_etype_header_pos_tx(skb); > + > + data2_val = FIELD_PREP(A5PSW_CTRL_DATA_PORT, BIT(dp->index)); > + ptag->ctrl_tag = htons(A5PSW_TAG_VALUE); > + ptag->ctrl_data = htons(A5PSW_CTRL_DATA_FORCE_FORWARD); > + ptag->ctrl_data2_lo = htons(data2_val); > + ptag->ctrl_data2_hi = 0; > + > + return skb; > +} > + > +static struct sk_buff *a5psw_tag_rcv(struct sk_buff *skb, > + struct net_device *dev) > +{ > + struct a5psw_tag *tag; > + int port; > + > + if (unlikely(!pskb_may_pull(skb, A5PSW_TAG_LEN))) { > + dev_warn_ratelimited(&dev->dev, > + "Dropping packet, cannot pull\n"); > + return NULL; > + } > + > + tag = dsa_etype_header_pos_rx(skb); > + > + if (tag->ctrl_tag != htons(A5PSW_TAG_VALUE)) { > + dev_warn_ratelimited(&dev->dev, "Dropping packet due to invalid TAG marker\n"); > + return NULL; > + } > + > + port = FIELD_GET(A5PSW_CTRL_DATA_PORT, ntohs(tag->ctrl_data)); > + > + skb->dev = dsa_master_find_slave(dev, 0, port); > + if (!skb->dev) > + return NULL; > + > + skb_pull_rcsum(skb, A5PSW_TAG_LEN); > + dsa_strip_etype_header(skb, A5PSW_TAG_LEN); > + > + dsa_default_offload_fwd_mark(skb); > + > + return skb; > +} > + > +static const struct dsa_device_ops a5psw_netdev_ops = { > + .name = "a5psw", > + .proto = DSA_TAG_PROTO_RZN1_A5PSW, > + .xmit = a5psw_tag_xmit, > + .rcv = a5psw_tag_rcv, > + .needed_headroom = A5PSW_TAG_LEN, > +}; > + > +MODULE_LICENSE("GPL v2"); > +MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_A5PSW); > +module_dsa_tag_driver(a5psw_netdev_ops); > -- > 2.34.1 >
Le Wed, 4 May 2022 19:00:39 +0300, Vladimir Oltean <olteanv@gmail.com> a écrit : > > +/* To define the outgoing port and to discover the incoming port a TAG is > > + * inserted after Src MAC : > > + * > > + * Dest MAC Src MAC TAG Type > > + * ...| 1 2 3 4 5 6 | 1 2 3 4 5 6 | 1 2 3 4 5 6 7 8 | 1 2 |... > > + * |<--------------->| > > + * > > + * See struct a5psw_tag for layout > > + */ > > + > > +#define A5PSW_TAG_VALUE 0xE001 > > Maybe an ETH_P_DSA_A5PSW definition in include/uapi/linux/if_ether.h > would be appropriate? I'm not sure. That's a good question. Actually, this value is the default but is configurable in the hardware so I'm not sure this should be a reserved value. Maybe it would make sense to add it anyway to have the define shared between the switch driver and the tag driver. > > > +#define A5PSW_TAG_LEN 8 > > +#define A5PSW_CTRL_DATA_FORCE_FORWARD BIT(0) > > +/* This is both used for xmit tag and rcv tagging */ > > +#define A5PSW_CTRL_DATA_PORT GENMASK(3, 0) > > + > > +struct a5psw_tag { > > + __be16 ctrl_tag; > > + __be16 ctrl_data; > > + __be16 ctrl_data2_hi; > > + __be16 ctrl_data2_lo; > > +}; > > + > > +static struct sk_buff *a5psw_tag_xmit(struct sk_buff *skb, struct net_device *dev) > > +{ > > + struct dsa_port *dp = dsa_slave_to_port(dev); > > + struct a5psw_tag *ptag; > > + u32 data2_val; > > + > > + BUILD_BUG_ON(sizeof(*ptag) != A5PSW_TAG_LEN); > > + > > + /* The Ethernet switch we are interfaced with needs packets to be at > > + * least 64 bytes (including FCS) otherwise they will be discarded when > > + * they enter the switch port logic. When tagging is enabled, we need > > + * to make sure that packets are at least 70 bytes (including FCS and > > + * tag). > > + */ > > + if (__skb_put_padto(skb, ETH_ZLEN + ETH_FCS_LEN + A5PSW_TAG_LEN, false)) > > + return NULL; > > I'm confused by the inclusion of the FCS length in this calculation, > since the FCS space isn't present in the skb buffer as far as I know? I'm not sure either, the documentation is not really clear on what is the requirement for the minimal size of a packet. This was the closest thing I could find about that requirement: "A frame has a valid length if it contains at least 64 octets and does not exceed the programmed maximum length" And the figure associated to the frame show that the frame length includes the FCS which lead to a 64bytes frame. > > "64 bytes including FCS" means "60 bytes excluding FCS". > And ETH_ZLEN is 60... > > And I'm also not sure how we got to the number 70? A5PSW_TAG_LEN is 8. > If we add it to 60 we get 68. If we add it to 64 we get 72? > I'll check all these numbers and find the correct size that is expected by the switch.
diff --git a/include/net/dsa.h b/include/net/dsa.h index d9da32aacbf1..9aaaa7deb102 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -53,6 +53,7 @@ struct phylink_link_state; #define DSA_TAG_PROTO_SJA1110_VALUE 23 #define DSA_TAG_PROTO_RTL8_4_VALUE 24 #define DSA_TAG_PROTO_RTL8_4T_VALUE 25 +#define DSA_TAG_PROTO_RZN1_A5PSW_VALUE 26 enum dsa_tag_protocol { DSA_TAG_PROTO_NONE = DSA_TAG_PROTO_NONE_VALUE, @@ -81,6 +82,7 @@ enum dsa_tag_protocol { DSA_TAG_PROTO_SJA1110 = DSA_TAG_PROTO_SJA1110_VALUE, DSA_TAG_PROTO_RTL8_4 = DSA_TAG_PROTO_RTL8_4_VALUE, DSA_TAG_PROTO_RTL8_4T = DSA_TAG_PROTO_RTL8_4T_VALUE, + DSA_TAG_PROTO_RZN1_A5PSW = DSA_TAG_PROTO_RZN1_A5PSW_VALUE, }; struct dsa_switch; diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig index 8cb87b5067ee..e22f33d7cf60 100644 --- a/net/dsa/Kconfig +++ b/net/dsa/Kconfig @@ -132,6 +132,13 @@ config NET_DSA_TAG_RTL8_4 Say Y or M if you want to enable support for tagging frames for Realtek switches with 8 byte protocol 4 tags, such as the Realtek RTL8365MB-VC. +config NET_DSA_TAG_RZN1_A5PSW + tristate "Tag driver for Renesas RZ/N1 A5PSW switch" + help + Say Y or M if you want to enable support for tagging frames for + Renesas RZ/N1 embedded switch that uses a 8 byte tag located after + destination MAC address. + config NET_DSA_TAG_LAN9303 tristate "Tag driver for SMSC/Microchip LAN9303 family of switches" help diff --git a/net/dsa/Makefile b/net/dsa/Makefile index 9f75820e7c98..af28c24ead18 100644 --- a/net/dsa/Makefile +++ b/net/dsa/Makefile @@ -17,6 +17,7 @@ obj-$(CONFIG_NET_DSA_TAG_OCELOT_8021Q) += tag_ocelot_8021q.o obj-$(CONFIG_NET_DSA_TAG_QCA) += tag_qca.o obj-$(CONFIG_NET_DSA_TAG_RTL4_A) += tag_rtl4_a.o obj-$(CONFIG_NET_DSA_TAG_RTL8_4) += tag_rtl8_4.o +obj-$(CONFIG_NET_DSA_TAG_RZN1_A5PSW) += tag_rzn1_a5psw.o obj-$(CONFIG_NET_DSA_TAG_SJA1105) += tag_sja1105.o obj-$(CONFIG_NET_DSA_TAG_TRAILER) += tag_trailer.o obj-$(CONFIG_NET_DSA_TAG_XRS700X) += tag_xrs700x.o diff --git a/net/dsa/tag_rzn1_a5psw.c b/net/dsa/tag_rzn1_a5psw.c new file mode 100644 index 000000000000..87177682eb54 --- /dev/null +++ b/net/dsa/tag_rzn1_a5psw.c @@ -0,0 +1,114 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2022 Schneider Electric + * + * Clément Léger <clement.leger@bootlin.com> + */ + +#include <linux/bitfield.h> +#include <linux/etherdevice.h> +#include <net/dsa.h> + +#include "dsa_priv.h" + +/* To define the outgoing port and to discover the incoming port a TAG is + * inserted after Src MAC : + * + * Dest MAC Src MAC TAG Type + * ...| 1 2 3 4 5 6 | 1 2 3 4 5 6 | 1 2 3 4 5 6 7 8 | 1 2 |... + * |<--------------->| + * + * See struct a5psw_tag for layout + */ + +#define A5PSW_TAG_VALUE 0xE001 +#define A5PSW_TAG_LEN 8 +#define A5PSW_CTRL_DATA_FORCE_FORWARD BIT(0) +/* This is both used for xmit tag and rcv tagging */ +#define A5PSW_CTRL_DATA_PORT GENMASK(3, 0) + +struct a5psw_tag { + __be16 ctrl_tag; + __be16 ctrl_data; + __be16 ctrl_data2_hi; + __be16 ctrl_data2_lo; +}; + +static struct sk_buff *a5psw_tag_xmit(struct sk_buff *skb, struct net_device *dev) +{ + struct dsa_port *dp = dsa_slave_to_port(dev); + struct a5psw_tag *ptag; + u32 data2_val; + + BUILD_BUG_ON(sizeof(*ptag) != A5PSW_TAG_LEN); + + /* The Ethernet switch we are interfaced with needs packets to be at + * least 64 bytes (including FCS) otherwise they will be discarded when + * they enter the switch port logic. When tagging is enabled, we need + * to make sure that packets are at least 70 bytes (including FCS and + * tag). + */ + if (__skb_put_padto(skb, ETH_ZLEN + ETH_FCS_LEN + A5PSW_TAG_LEN, false)) + return NULL; + + /* provide 'A5PSW_TAG_LEN' bytes additional space */ + skb_push(skb, A5PSW_TAG_LEN); + + /* make room between MACs and Ether-Type to insert tag */ + dsa_alloc_etype_header(skb, A5PSW_TAG_LEN); + + ptag = dsa_etype_header_pos_tx(skb); + + data2_val = FIELD_PREP(A5PSW_CTRL_DATA_PORT, BIT(dp->index)); + ptag->ctrl_tag = htons(A5PSW_TAG_VALUE); + ptag->ctrl_data = htons(A5PSW_CTRL_DATA_FORCE_FORWARD); + ptag->ctrl_data2_lo = htons(data2_val); + ptag->ctrl_data2_hi = 0; + + return skb; +} + +static struct sk_buff *a5psw_tag_rcv(struct sk_buff *skb, + struct net_device *dev) +{ + struct a5psw_tag *tag; + int port; + + if (unlikely(!pskb_may_pull(skb, A5PSW_TAG_LEN))) { + dev_warn_ratelimited(&dev->dev, + "Dropping packet, cannot pull\n"); + return NULL; + } + + tag = dsa_etype_header_pos_rx(skb); + + if (tag->ctrl_tag != htons(A5PSW_TAG_VALUE)) { + dev_warn_ratelimited(&dev->dev, "Dropping packet due to invalid TAG marker\n"); + return NULL; + } + + port = FIELD_GET(A5PSW_CTRL_DATA_PORT, ntohs(tag->ctrl_data)); + + skb->dev = dsa_master_find_slave(dev, 0, port); + if (!skb->dev) + return NULL; + + skb_pull_rcsum(skb, A5PSW_TAG_LEN); + dsa_strip_etype_header(skb, A5PSW_TAG_LEN); + + dsa_default_offload_fwd_mark(skb); + + return skb; +} + +static const struct dsa_device_ops a5psw_netdev_ops = { + .name = "a5psw", + .proto = DSA_TAG_PROTO_RZN1_A5PSW, + .xmit = a5psw_tag_xmit, + .rcv = a5psw_tag_rcv, + .needed_headroom = A5PSW_TAG_LEN, +}; + +MODULE_LICENSE("GPL v2"); +MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_A5PSW); +module_dsa_tag_driver(a5psw_netdev_ops);
The switch that is present on the Renesas RZ/N1 SoC uses a specific VLAN value followed by 6 bytes which contains forwarding configuration. Signed-off-by: Clément Léger <clement.leger@bootlin.com> --- include/net/dsa.h | 2 + net/dsa/Kconfig | 7 +++ net/dsa/Makefile | 1 + net/dsa/tag_rzn1_a5psw.c | 114 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 124 insertions(+) create mode 100644 net/dsa/tag_rzn1_a5psw.c