| 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: <20250825221507.vfvnuaxs7hh2jy7d@skbuf>
Date: Tue, 26 Aug 2025 01:15:07 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: David Yang <mmyangfl@...il.com>
Cc: netdev@...r.kernel.org, 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>, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v6 2/3] net: dsa: tag_yt921x: add support for
Motorcomm YT921x tags
On Sun, Aug 24, 2025 at 08:51:10AM +0800, David Yang wrote:
> 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..ab7f97367e76
> --- /dev/null
> +++ b/net/dsa/tag_yt921x.c
> @@ -0,0 +1,126 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Motorcomm YT921x Switch Extended CPU Port Tagging
> + *
> + * Copyright (c) 2025 David Yang <mmyangfl@...il.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>
Why include list.h and slab.h?
> +
> +#include "tag.h"
> +
> +#define YT921X_NAME "yt921x"
> +
> +#define YT921X_TAG_LEN 8
> +
> +#define ETH_P_YT921X 0x9988
You can add a header in include/linux/dsa/ which is shared with the
switch driver, to avoid duplicate definitions.
> +
> +#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;
> + __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?
The tag format sort of becomes fixed ABI as soon as user space is able
to run "cat /sys/class/net/eth0/dsa/tagging", see "yt921x", and record
it to a pcap file. Unless the EtherType bears some other meaning rather
than being a fixed value, then if you change it later to some other
value than 0x9988, you'd better also change the protocol name to
distinguish it from "yt921x".
Also, you can _not_ use yt921x_priv :: tag_eth_p, because doing so would
assume that typeof(ds->priv) == struct yt921x_priv. In principle we
would like to be able to run the tagging protocols on the dsa_loop
driver as well, which can be attached to any network interface. Very
few, if any, tagging protocol drivers don't work on dsa_loop.
> + */
> + 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);
> + tag[3] = htons(tx);
> +
> + /* Now tell the conduit network device about the desired output queue
> + * as well
> + */
> + skb_set_queue_mapping(skb, port);
This is generally used for integrated DSA switches, for lossless
backpressure during CPU transmission, where the conduit interface driver
is known, and has set up its queues in a special way, as a result of the
fact that it is attached to a known DSA switch (made by the same vendor).
What do you need it for, in a discrete MDIO-controlled switch?
> +
> + 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;
Use dsa_etype_header_pos_rx() and validate the CPU_TAG_TPID_TPID as well.
> +
> + /* Locate which port this is coming from */
> + rx = ntohs(tag[1]);
> + if (unlikely((rx & YT921X_TAG_PORT_EN) == 0)) {
> + dev_warn_ratelimited(&netdev->dev,
> + "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)) {
> + dev_warn_ratelimited(&netdev->dev,
> + "Couldn't decode source 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
>
Powered by blists - more mailing lists