| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20251028002841.zja7km3oesczrlo3@skbuf>
Date: Tue, 28 Oct 2025 02:28:41 +0200
From: Vladimir Oltean <olteanv@...il.com>
To: Daniel Golle <daniel@...rotopia.org>
Cc: Hauke Mehrtens <hauke@...ke-m.de>, Andrew Lunn <andrew@...n.ch>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Simon Horman <horms@...nel.org>,
Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
Andreas Schirm <andreas.schirm@...mens.com>,
Lukas Stockmann <lukas.stockmann@...mens.com>,
Alexander Sverdlin <alexander.sverdlin@...mens.com>,
Peter Christen <peter.christen@...mens.com>,
Avinash Jayaraman <ajayaraman@...linear.com>,
Bing tao Xu <bxu@...linear.com>, Liang Xu <lxu@...linear.com>,
Juraj Povazanec <jpovazanec@...linear.com>,
"Fanni (Fang-Yi) Chan" <fchan@...linear.com>,
"Benny (Ying-Tsan) Weng" <yweng@...linear.com>,
"Livia M. Rosu" <lrosu@...linear.com>,
John Crispin <john@...ozen.org>
Subject: Re: [PATCH net-next v3 11/12] net: dsa: add tagging driver for
MaxLinear GSW1xx switch family
On Sun, Oct 26, 2025 at 11:48:23PM +0000, Daniel Golle wrote:
> Add support for a new DSA tagging protocol driver for the MaxLinear
> GSW1xx switch family. The GSW1xx switches use a proprietary 8-byte
> special tag inserted between the source MAC address and the EtherType
> field to indicate the source and destination ports for frames
> traversing the CPU port.
>
> Implement the tag handling logic to insert the special tag on transmit
> and parse it on receive.
>
> Signed-off-by: Daniel Golle <daniel@...rotopia.org>
> ---
> since RFC:
> * use dsa etype header macros instead of open coding them
> * maintain alphabetic order in Kconfig and Makefile
>
> MAINTAINERS | 3 +-
> include/net/dsa.h | 2 +
> net/dsa/Kconfig | 8 +++
> net/dsa/Makefile | 1 +
> net/dsa/tag_mxl-gsw1xx.c | 141 +++++++++++++++++++++++++++++++++++++++
> 5 files changed, 154 insertions(+), 1 deletion(-)
> create mode 100644 net/dsa/tag_mxl-gsw1xx.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d652f4f27756..4ddff0b0a547 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14038,7 +14038,7 @@ F: tools/testing/selftests/landlock/
> K: landlock
> K: LANDLOCK
>
> -LANTIQ / INTEL Ethernet drivers
> +LANTIQ / MAXLINEAR / INTEL Ethernet DSA drivers
> M: Hauke Mehrtens <hauke@...ke-m.de>
> L: netdev@...r.kernel.org
> S: Maintained
> @@ -14046,6 +14046,7 @@ F: Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml
> F: drivers/net/dsa/lantiq/*
> F: drivers/net/ethernet/lantiq_xrx200.c
> F: net/dsa/tag_gswip.c
> +F: net/dsa/tag_mxl-gsw1xx.c
>
> LANTIQ MIPS ARCHITECTURE
> M: John Crispin <john@...ozen.org>
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 67762fdaf3c7..2df2e2ead9a8 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -56,6 +56,7 @@ struct tc_action;
> #define DSA_TAG_PROTO_VSC73XX_8021Q_VALUE 28
> #define DSA_TAG_PROTO_BRCM_LEGACY_FCS_VALUE 29
> #define DSA_TAG_PROTO_YT921X_VALUE 30
> +#define DSA_TAG_PROTO_MXL_GSW1XX_VALUE 31
>
> enum dsa_tag_protocol {
> DSA_TAG_PROTO_NONE = DSA_TAG_PROTO_NONE_VALUE,
> @@ -89,6 +90,7 @@ enum dsa_tag_protocol {
> DSA_TAG_PROTO_LAN937X = DSA_TAG_PROTO_LAN937X_VALUE,
> DSA_TAG_PROTO_VSC73XX_8021Q = DSA_TAG_PROTO_VSC73XX_8021Q_VALUE,
> DSA_TAG_PROTO_YT921X = DSA_TAG_PROTO_YT921X_VALUE,
> + DSA_TAG_PROTO_MXL_GSW1XX = DSA_TAG_PROTO_MXL_GSW1XX_VALUE,
> };
>
> struct dsa_switch;
> diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
> index 6b94028b1fcc..f86b30742122 100644
> --- a/net/dsa/Kconfig
> +++ b/net/dsa/Kconfig
> @@ -104,6 +104,14 @@ config NET_DSA_TAG_MTK
> Say Y or M if you want to enable support for tagging frames for
> Mediatek switches.
>
> +config NET_DSA_TAG_MXL_GSW1XX
> + tristate "Tag driver for MaxLinear GSW1xx switches"
> + help
> + The GSW1xx family of switches supports an 8-byte special tag which
> + can be used on the CPU port of the switch.
> + Say Y or M if you want to enable support for tagging frames for
> + MaxLinear GSW1xx switches.
> +
> config NET_DSA_TAG_KSZ
> tristate "Tag driver for Microchip 8795/937x/9477/9893 families of switches"
> help
> diff --git a/net/dsa/Makefile b/net/dsa/Makefile
> index 4b011a1d5c87..42d173f5a701 100644
> --- a/net/dsa/Makefile
> +++ b/net/dsa/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_NET_DSA_TAG_HELLCREEK) += tag_hellcreek.o
> obj-$(CONFIG_NET_DSA_TAG_KSZ) += tag_ksz.o
> obj-$(CONFIG_NET_DSA_TAG_LAN9303) += tag_lan9303.o
> obj-$(CONFIG_NET_DSA_TAG_MTK) += tag_mtk.o
> +obj-$(CONFIG_NET_DSA_TAG_MXL_GSW1XX) += tag_mxl-gsw1xx.o
> obj-$(CONFIG_NET_DSA_TAG_NONE) += tag_none.o
> obj-$(CONFIG_NET_DSA_TAG_OCELOT) += tag_ocelot.o
> obj-$(CONFIG_NET_DSA_TAG_OCELOT_8021Q) += tag_ocelot_8021q.o
> diff --git a/net/dsa/tag_mxl-gsw1xx.c b/net/dsa/tag_mxl-gsw1xx.c
> new file mode 100644
> index 000000000000..9efec6deb494
> --- /dev/null
> +++ b/net/dsa/tag_mxl-gsw1xx.c
> @@ -0,0 +1,141 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * DSA driver Special Tag support for MaxLinear GSW1xx switch chips
> + *
> + * Copyright (C) 2025 Daniel Golle <daniel@...rotopia.org>
> + * Copyright (C) 2023 - 2024 MaxLinear Inc.
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/etherdevice.h>
> +#include <linux/skbuff.h>
> +#include <net/dsa.h>
> +
> +#include "tag.h"
> +
> +/* To define the outgoing port and to discover the incoming port a special
> + * tag is used by the GSW1xx.
> + *
> + * Dest MAC Src MAC special TAG EtherType
> + * ...| 1 2 3 4 5 6 | 1 2 3 4 5 6 | 1 2 3 4 5 6 7 8 | 1 2 |...
> + * |<--------------->|
> + */
> +
> +#define GSW1XX_TAG_NAME "gsw1xx"
> +
> +/* special tag in TX path header */
> +#define GSW1XX_TX_HEADER_LEN 8
> +
> +/* Byte 0 = Ethertype byte 1 -> 0x88 */
> +/* Byte 1 = Ethertype byte 2 -> 0xC3*/
> +
> +/* Byte 2 */
> +#define GSW1XX_TX_PORT_MAP_EN BIT(7)
> +#define GSW1XX_TX_CLASS_EN BIT(6)
> +#define GSW1XX_TX_TIME_STAMP_EN BIT(5)
> +#define GSW1XX_TX_LRN_DIS BIT(4)
> +#define GSW1XX_TX_CLASS_SHIFT 0
> +#define GSW1XX_TX_CLASS_MASK GENMASK(3, 0)
Using FIELD_PREP() would eliminate these _SHIFT definitions and _MASK
would also go away from the macro names.
> +
> +/* Byte 3 */
> +#define GSW1XX_TX_PORT_MAP_LOW_SHIFT 0
> +#define GSW1XX_TX_PORT_MAP_LOW_MASK GENMASK(7, 0)
> +
> +/* Byte 4 */
> +#define GSW1XX_TX_PORT_MAP_HIGH_SHIFT 0
> +#define GSW1XX_TX_PORT_MAP_HIGH_MASK GENMASK(7, 0)
> +
> +#define GSW1XX_RX_HEADER_LEN 8
Usually you use two separate macros when the lengths are not equal, and
you set .needed_headroom to the largest value.
> +
> +/* special tag in RX path header */
> +/* Byte 4 */
> +#define GSW1XX_RX_PORT_MAP_LOW_SHIFT 0
> +#define GSW1XX_RX_PORT_MAP_LOW_MASK GENMASK(7, 0)
> +
> +/* Byte 5 */
> +#define GSW1XX_RX_PORT_MAP_HIGH_SHIFT 0
> +#define GSW1XX_RX_PORT_MAP_HIGH_MASK GENMASK(7, 0)
> +
> +static struct sk_buff *gsw1xx_tag_xmit(struct sk_buff *skb,
> + struct net_device *dev)
> +{
> + struct dsa_port *dp = dsa_user_to_port(dev);
> + u8 *gsw1xx_tag;
> +
> + /* provide additional space 'GSW1XX_TX_HEADER_LEN' bytes */
> + skb_push(skb, GSW1XX_TX_HEADER_LEN);
> +
> + /* add space between MAC address and Ethertype */
> + dsa_alloc_etype_header(skb, GSW1XX_TX_HEADER_LEN);
> +
> + /* special tag ingress */
> + gsw1xx_tag = dsa_etype_header_pos_tx(skb);
> + gsw1xx_tag[0] = 0x88;
> + gsw1xx_tag[1] = 0xc3;
Could you write this as a u16 pointer, to make it obvious to everyone
it's an EtherType, and define the EtherType constant in
include/uapi/linux/if_ether.h, to make it a bit more visible that it's
in use?
> + gsw1xx_tag[2] = GSW1XX_TX_PORT_MAP_EN | GSW1XX_TX_LRN_DIS;
> + gsw1xx_tag[3] = BIT(dp->index + GSW1XX_TX_PORT_MAP_LOW_SHIFT) &
> + GSW1XX_TX_PORT_MAP_LOW_MASK;
> + gsw1xx_tag[4] = 0;
> + gsw1xx_tag[5] = 0;
> + gsw1xx_tag[6] = 0;
> + gsw1xx_tag[7] = 0;
> +
> + return skb;
> +}
> +
> +static struct sk_buff *gsw1xx_tag_rcv(struct sk_buff *skb,
> + struct net_device *dev)
> +{
> + int port;
> + u8 *gsw1xx_tag;
> +
> + if (unlikely(!pskb_may_pull(skb, GSW1XX_RX_HEADER_LEN))) {
> + dev_warn_ratelimited(&dev->dev, "Dropping packet, cannot pull SKB\n");
> + return NULL;
> + }
> +
> + gsw1xx_tag = dsa_etype_header_pos_rx(skb);
> +
> + if (gsw1xx_tag[0] != 0x88 && gsw1xx_tag[1] != 0xc3) {
> + dev_warn_ratelimited(&dev->dev, "Dropping packet due to invalid special tag\n");
> + dev_warn_ratelimited(&dev->dev,
> + "Tag: 0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x\n",
> + gsw1xx_tag[0], gsw1xx_tag[1], gsw1xx_tag[2], gsw1xx_tag[3],
> + gsw1xx_tag[4], gsw1xx_tag[5], gsw1xx_tag[6], gsw1xx_tag[7]);
I think you could print the tag with %*ph, according to
https://elixir.bootlin.com/linux/v6.17.5/source/lib/vsprintf.c#L2453
(needs testing)
> + return NULL;
> + }
> +
> + /* Get source port information */
> + port = (gsw1xx_tag[2] & GSW1XX_RX_PORT_MAP_LOW_MASK) >> GSW1XX_RX_PORT_MAP_LOW_SHIFT;
> + skb->dev = dsa_conduit_find_user(dev, 0, port);
> + if (!skb->dev) {
> + dev_warn_ratelimited(&dev->dev, "Dropping packet due to invalid source port\n");
> + dev_warn_ratelimited(&dev->dev,
> + "Tag: 0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x\n",
> + gsw1xx_tag[0], gsw1xx_tag[1], gsw1xx_tag[2], gsw1xx_tag[3],
> + gsw1xx_tag[4], gsw1xx_tag[5], gsw1xx_tag[6], gsw1xx_tag[7]);
> + return NULL;
> + }
> +
> + /* remove the GSW1xx special tag between MAC addresses and the current
> + * ethertype field.
> + */
> + skb_pull_rcsum(skb, GSW1XX_RX_HEADER_LEN);
> + dsa_strip_etype_header(skb, GSW1XX_RX_HEADER_LEN);
You're not setting skb->offload_fwd_mark but you implement
port_bridge_join() so you offload L2 switching. If a packet gets flooded
from port A to the CPU and also to port B, don't you see that the
software bridge also creates a packet copy that it sends to port B a
second time?
> +
> + return skb;
> +}
> +
> +static const struct dsa_device_ops gsw1xx_netdev_ops = {
> + .name = GSW1XX_TAG_NAME,
> + .proto = DSA_TAG_PROTO_MXL_GSW1XX,
> + .xmit = gsw1xx_tag_xmit,
> + .rcv = gsw1xx_tag_rcv,
> + .needed_headroom = GSW1XX_RX_HEADER_LEN,
> +};
> +
> +MODULE_DESCRIPTION("DSA tag driver for MaxLinear GSW1xx 8 byte protocol");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_MXL_GSW1XX, GSW1XX_TAG_NAME);
> +
> +module_dsa_tag_driver(gsw1xx_netdev_ops);
> --
> 2.51.1
Powered by blists - more mailing lists