Add support for Motorcomm YT921x tags, which includes a proper
configurable ethertype field (default to 0x9988).
Signed-off-by: David Yang <mmyangfl@gmail.com>
---
include/net/dsa.h | 2 +
net/dsa/Kconfig | 6 ++
net/dsa/Makefile | 1 +
net/dsa/tag_yt921x.c | 128 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 137 insertions(+)
create mode 100644 net/dsa/tag_yt921x.c
diff --git a/include/net/dsa.h b/include/net/dsa.h
index d73ea0880066..67762fdaf3c7 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -55,6 +55,7 @@ struct tc_action;
#define DSA_TAG_PROTO_LAN937X_VALUE 27
#define DSA_TAG_PROTO_VSC73XX_8021Q_VALUE 28
#define DSA_TAG_PROTO_BRCM_LEGACY_FCS_VALUE 29
+#define DSA_TAG_PROTO_YT921X_VALUE 30
enum dsa_tag_protocol {
DSA_TAG_PROTO_NONE = DSA_TAG_PROTO_NONE_VALUE,
@@ -87,6 +88,7 @@ enum dsa_tag_protocol {
DSA_TAG_PROTO_RZN1_A5PSW = DSA_TAG_PROTO_RZN1_A5PSW_VALUE,
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,
};
struct dsa_switch;
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index 869cbe57162f..6b94028b1fcc 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -190,4 +190,10 @@ config NET_DSA_TAG_XRS700X
Say Y or M if you want to enable support for tagging frames for
Arrow SpeedChips XRS700x switches that use a single byte tag trailer.
+config NET_DSA_TAG_YT921X
+ tristate "Tag driver for Motorcomm YT921x switches"
+ help
+ Say Y or M if you want to enable support for tagging frames for
+ Motorcomm YT921x switches.
+
endif
diff --git a/net/dsa/Makefile b/net/dsa/Makefile
index 555c07cfeb71..4b011a1d5c87 100644
--- a/net/dsa/Makefile
+++ b/net/dsa/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_NET_DSA_TAG_SJA1105) += tag_sja1105.o
obj-$(CONFIG_NET_DSA_TAG_TRAILER) += tag_trailer.o
obj-$(CONFIG_NET_DSA_TAG_VSC73XX_8021Q) += tag_vsc73xx_8021q.o
obj-$(CONFIG_NET_DSA_TAG_XRS700X) += tag_xrs700x.o
+obj-$(CONFIG_NET_DSA_TAG_YT921X) += tag_yt921x.o
# for tracing framework to find trace.h
CFLAGS_trace.o := -I$(src)
diff --git a/net/dsa/tag_yt921x.c b/net/dsa/tag_yt921x.c
new file mode 100644
index 000000000000..d10b9a00c0b6
--- /dev/null
+++ b/net/dsa/tag_yt921x.c
@@ -0,0 +1,128 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Motorcomm YT921x Switch Extend CPU Port
+ *
+ * Copyright (c) 2025 David Yang <mmyangfl@gmail.com>
+ *
+ * +----+----+-------+-----+----+---------
+ * | DA | SA | TagET | Tag | ET | Payload ...
+ * +----+----+-------+-----+----+---------
+ * 6 6 2 6 2 N
+ *
+ * Tag Ethertype: CPU_TAG_TPID_TPID (default: 0x9988)
+ * Tag:
+ * 2: Service VLAN Tag
+ * 2: Rx Port
+ * 15b: Rx Port Valid
+ * 14b-11b: Rx Port
+ * 10b-0b: Unknown value 0x80
+ * 2: Tx Port(s)
+ * 15b: Tx Port(s) Valid
+ * 10b-0b: Tx Port(s) Mask
+ */
+
+#include <linux/etherdevice.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+
+#include "tag.h"
+
+#define YT921X_NAME "yt921x"
+
+#define YT921X_TAG_LEN 8
+
+#define ETH_P_YT921X 0x9988
+
+#define YT921X_TAG_PORT_EN BIT(15)
+#define YT921X_TAG_RX_PORT_M GENMASK(14, 11)
+#define YT921X_TAG_RX_CMD_M GENMASK(10, 0)
+#define YT921X_TAG_RX_CMD(x) FIELD_PREP(YT921X_TAG_RX_CMD_M, (x))
+#define YT921X_TAG_RX_CMD_UNK_NORMAL 0x80
+#define YT921X_TAG_TX_PORTS_M GENMASK(10, 0)
+#define YT921X_TAG_TX_PORTn(port) BIT(port)
+
+static struct sk_buff *
+yt921x_tag_xmit(struct sk_buff *skb, struct net_device *netdev)
+{
+ struct dsa_port *dp = dsa_user_to_port(netdev);
+ unsigned int port = dp->index;
+ struct dsa_port *partner;
+ __be16 *tag;
+ u16 tx;
+
+ skb_push(skb, YT921X_TAG_LEN);
+ dsa_alloc_etype_header(skb, YT921X_TAG_LEN);
+
+ tag = dsa_etype_header_pos_tx(skb);
+
+ /* We might use yt921x_priv::tag_eth_p, but
+ * 1. CPU_TAG_TPID could be configured anyway;
+ * 2. Are you using the right chip?
+ */
+ tag[0] = htons(ETH_P_YT921X);
+ /* Service VLAN tag not used */
+ tag[1] = 0;
+ tag[2] = 0;
+ tx = YT921X_TAG_PORT_EN | YT921X_TAG_TX_PORTn(port);
+ if (dp->hsr_dev)
+ dsa_hsr_foreach_port(partner, dp->ds, dp->hsr_dev)
+ tx |= YT921X_TAG_TX_PORTn(partner->index);
+ tag[3] = htons(tx);
+
+ /* Now tell the conduit network device about the desired output queue
+ * as well
+ */
+ skb_set_queue_mapping(skb, port);
+
+ return skb;
+}
+
+static struct sk_buff *
+yt921x_tag_rcv(struct sk_buff *skb, struct net_device *netdev)
+{
+ unsigned int port;
+ __be16 *tag;
+ u16 rx;
+
+ if (unlikely(!pskb_may_pull(skb, YT921X_TAG_LEN)))
+ return NULL;
+
+ tag = (__be16 *)skb->data;
+
+ /* Locate which port this is coming from */
+ rx = ntohs(tag[1]);
+ if (unlikely((rx & YT921X_TAG_PORT_EN) == 0)) {
+ netdev_err(netdev, "Unexpected rx tag 0x%04x\n", rx);
+ return NULL;
+ }
+
+ port = FIELD_GET(YT921X_TAG_RX_PORT_M, rx);
+ skb->dev = dsa_conduit_find_user(netdev, 0, port);
+ if (unlikely(!skb->dev)) {
+ netdev_err(netdev, "Cannot locate rx port %u\n", port);
+ return NULL;
+ }
+
+ /* Remove YT921x tag and update checksum */
+ skb_pull_rcsum(skb, YT921X_TAG_LEN);
+
+ dsa_default_offload_fwd_mark(skb);
+
+ dsa_strip_etype_header(skb, YT921X_TAG_LEN);
+
+ return skb;
+}
+
+static const struct dsa_device_ops yt921x_netdev_ops = {
+ .name = YT921X_NAME,
+ .proto = DSA_TAG_PROTO_YT921X,
+ .xmit = yt921x_tag_xmit,
+ .rcv = yt921x_tag_rcv,
+ .needed_headroom = YT921X_TAG_LEN,
+};
+
+MODULE_DESCRIPTION("DSA tag driver for Motorcomm YT921x switches");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_YT921X, YT921X_NAME);
+
+module_dsa_tag_driver(yt921x_netdev_ops);
--
2.50.1
> +static struct sk_buff * > +yt921x_tag_xmit(struct sk_buff *skb, struct net_device *netdev) > +{ > + struct dsa_port *dp = dsa_user_to_port(netdev); > + unsigned int port = dp->index; > + struct dsa_port *partner; > + __be16 *tag; > + u16 tx; > + > + skb_push(skb, YT921X_TAG_LEN); > + dsa_alloc_etype_header(skb, YT921X_TAG_LEN); > + > + tag = dsa_etype_header_pos_tx(skb); > + > + /* We might use yt921x_priv::tag_eth_p, but > + * 1. CPU_TAG_TPID could be configured anyway; > + * 2. Are you using the right chip? > + */ > + tag[0] = htons(ETH_P_YT921X); > + /* Service VLAN tag not used */ > + tag[1] = 0; > + tag[2] = 0; > + tx = YT921X_TAG_PORT_EN | YT921X_TAG_TX_PORTn(port); > + if (dp->hsr_dev) > + dsa_hsr_foreach_port(partner, dp->ds, dp->hsr_dev) > + tx |= YT921X_TAG_TX_PORTn(partner->index); As far as i remember, this was not in v1. When i spotting this in v2 that made me comment you should not add new features in revision of a patch. Does the current version of the DSA driver support hsr? Is this useful? Maybe it would be better to add hsr support as a follow up patch? > +static struct sk_buff * > +yt921x_tag_rcv(struct sk_buff *skb, struct net_device *netdev) > +{ > + unsigned int port; > + __be16 *tag; > + u16 rx; > + > + if (unlikely(!pskb_may_pull(skb, YT921X_TAG_LEN))) > + return NULL; > + > + tag = (__be16 *)skb->data; > + > + /* Locate which port this is coming from */ > + rx = ntohs(tag[1]); > + if (unlikely((rx & YT921X_TAG_PORT_EN) == 0)) { > + netdev_err(netdev, "Unexpected rx tag 0x%04x\n", rx); > + return NULL; > + } > + > + port = FIELD_GET(YT921X_TAG_RX_PORT_M, rx); > + skb->dev = dsa_conduit_find_user(netdev, 0, port); > + if (unlikely(!skb->dev)) { > + netdev_err(netdev, "Cannot locate rx port %u\n", port); > + return NULL; > + } O.K. Stop. Think. You changed the rate limiting to an unlimiting netdev_err(). What is the difference? Under what conditions would you want to use rate limiting? When would you not use rate limiting? Please reply and explain why you made this change. Andrew
On Tue, Aug 19, 2025 at 1:07 AM Andrew Lunn <andrew@lunn.ch> wrote: > > > +static struct sk_buff * > > +yt921x_tag_xmit(struct sk_buff *skb, struct net_device *netdev) > > +{ > > + struct dsa_port *dp = dsa_user_to_port(netdev); > > + unsigned int port = dp->index; > > + struct dsa_port *partner; > > + __be16 *tag; > > + u16 tx; > > + > > + skb_push(skb, YT921X_TAG_LEN); > > + dsa_alloc_etype_header(skb, YT921X_TAG_LEN); > > + > > + tag = dsa_etype_header_pos_tx(skb); > > + > > + /* We might use yt921x_priv::tag_eth_p, but > > + * 1. CPU_TAG_TPID could be configured anyway; > > + * 2. Are you using the right chip? > > + */ > > + tag[0] = htons(ETH_P_YT921X); > > + /* Service VLAN tag not used */ > > + tag[1] = 0; > > + tag[2] = 0; > > + tx = YT921X_TAG_PORT_EN | YT921X_TAG_TX_PORTn(port); > > + if (dp->hsr_dev) > > + dsa_hsr_foreach_port(partner, dp->ds, dp->hsr_dev) > > + tx |= YT921X_TAG_TX_PORTn(partner->index); > > As far as i remember, this was not in v1. When i spotting this in v2 > that made me comment you should not add new features in revision of a > patch. > > Does the current version of the DSA driver support hsr? Is this > useful? Maybe it would be better to add hsr support as a follow up > patch? Sorry, this was forgotten to undo. > > +static struct sk_buff * > > +yt921x_tag_rcv(struct sk_buff *skb, struct net_device *netdev) > > +{ > > + unsigned int port; > > + __be16 *tag; > > + u16 rx; > > + > > + if (unlikely(!pskb_may_pull(skb, YT921X_TAG_LEN))) > > + return NULL; > > + > > + tag = (__be16 *)skb->data; > > + > > + /* Locate which port this is coming from */ > > + rx = ntohs(tag[1]); > > + if (unlikely((rx & YT921X_TAG_PORT_EN) == 0)) { > > + netdev_err(netdev, "Unexpected rx tag 0x%04x\n", rx); > > + return NULL; > > + } > > + > > + port = FIELD_GET(YT921X_TAG_RX_PORT_M, rx); > > + skb->dev = dsa_conduit_find_user(netdev, 0, port); > > + if (unlikely(!skb->dev)) { > > + netdev_err(netdev, "Cannot locate rx port %u\n", port); > > + return NULL; > > + } > > O.K. Stop. Think. > > You changed the rate limiting to an unlimiting netdev_err(). > > What is the difference? Under what conditions would you want to use > rate limiting? When would you not use rate limiting? > > Please reply and explain why you made this change. > > Andrew I copied the limited version from tag_vsc73xx_8021q. Under no conditions I expect either of them to appear: it is the case when I did my own tests; unless something really bad happens, like pouring a cup of coffee over your device. I know rate limiting is a way to prevent flooding the same message over dmesg, but if an event never happens, I would consider two methods are exchangeable. Theoretically if an event never happens, no warnings would ever be needed, but I placed one here in case you destroy your device accidentally. Thus if you think rate limiting is not appropriate here, I would fix it with another.
> > > +static struct sk_buff * > > > +yt921x_tag_rcv(struct sk_buff *skb, struct net_device *netdev) > > > +{ > > > + unsigned int port; > > > + __be16 *tag; > > > + u16 rx; > > > + > > > + if (unlikely(!pskb_may_pull(skb, YT921X_TAG_LEN))) > > > + return NULL; > > > + > > > + tag = (__be16 *)skb->data; > > > + > > > + /* Locate which port this is coming from */ > > > + rx = ntohs(tag[1]); > > > + if (unlikely((rx & YT921X_TAG_PORT_EN) == 0)) { > > > + netdev_err(netdev, "Unexpected rx tag 0x%04x\n", rx); > > > + return NULL; > > > + } > > > + > > > + port = FIELD_GET(YT921X_TAG_RX_PORT_M, rx); > > > + skb->dev = dsa_conduit_find_user(netdev, 0, port); > > > + if (unlikely(!skb->dev)) { > > > + netdev_err(netdev, "Cannot locate rx port %u\n", port); > > > + return NULL; > > > + } > > > > O.K. Stop. Think. > > > > You changed the rate limiting to an unlimiting netdev_err(). > > > > What is the difference? Under what conditions would you want to use > > rate limiting? When would you not use rate limiting? > > > > Please reply and explain why you made this change. > > > > Andrew > > I copied the limited version from tag_vsc73xx_8021q. > > Under no conditions I expect either of them to appear: it is the case > when I did my own tests; unless something really bad happens, like > pouring a cup of coffee over your device. > > I know rate limiting is a way to prevent flooding the same message > over dmesg, but if an event never happens, I would consider two > methods are exchangeable. Theoretically if an event never happens, no > warnings would ever be needed, but I placed one here in case you > destroy your device accidentally. > > Thus if you think rate limiting is not appropriate here, I would fix > it with another. My thinking is, every single packet come through here. That could be 100,000 packets per second. We have no control over these packets, they are from other hosts, they can contain anything, included specially crafted packets, ping of death packets, etc. If there is a way to trigger these prints, and they print 100,000 times per second, it makes a good DoS. The box is completely useless. With rate limiting, the box keeps working, there is no DoS, but you get to see the error messages. The basic networking philosophy is that anything per packet related which could cause a print should be rate limited. Andrew
© 2016 - 2025 Red Hat, Inc.