[PATCH net-next 1/3] net: dsa: vsc73xx: implement transmit via control interface

[PATCH net-next 1/3] net: dsa: vsc73xx: implement transmit via control interface

[Pawel Dembicki]

Some types of packets can be forwarded only to and from the PI/SI
interface. For more information, see Chapter 2.7.1 (CPU Forwarding) in
the datasheet.

This patch implements the routines required for link-local transmission.
This kind of traffic can't be transferred through the RGMII interface in
vsc73xx.

It uses a method similar to the sja1005 driver, where the DSA tagger
checks if the packet is link-local and uses a special deferred transmit
route for that kind of packet.

The vsc73xx uses an "Internal Frame Header" (IFH) in communication via the
PI/SI interface. Every packet must be prefixed with an IFH. The hardware
fixes the checksums, so there's no need to calculate the FCS in the
driver.

Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
---
 drivers/net/dsa/vitesse-vsc73xx-core.c | 172 +++++++++++++++++++++++++
 drivers/net/dsa/vitesse-vsc73xx.h      |   1 +
 include/linux/dsa/vsc73xx.h            |  20 +++
 net/dsa/tag_vsc73xx_8021q.c            |  73 +++++++++++
 4 files changed, 266 insertions(+)
 create mode 100644 include/linux/dsa/vsc73xx.h

diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
index f18aa321053d..21ab3f214490 100644
--- a/drivers/net/dsa/vitesse-vsc73xx-core.c
+++ b/drivers/net/dsa/vitesse-vsc73xx-core.c
@@ -73,6 +73,9 @@
 #define VSC73XX_CAT_PR_USR_PRIO	0x75
 #define VSC73XX_CAT_VLAN_MISC	0x79
 #define VSC73XX_CAT_PORT_VLAN	0x7a
+#define VSC73XX_CPUTXDAT	0xc0
+#define VSC73XX_MISCFIFO	0xc4
+#define VSC73XX_MISCSTAT	0xc8
 #define VSC73XX_Q_MISC_CONF	0xdf
 
 /* MAC_CFG register bits */
@@ -166,6 +169,14 @@
 #define VSC73XX_CAT_PORT_VLAN_VLAN_USR_PRIO GENMASK(14, 12)
 #define VSC73XX_CAT_PORT_VLAN_VLAN_VID GENMASK(11, 0)
 
+/* MISCFIFO Miscellaneous Control Register */
+#define VSC73XX_MISCFIFO_REWIND_CPU_TX	BIT(1)
+#define VSC73XX_MISCFIFO_CPU_TX		BIT(0)
+
+/* MISCSTAT Miscellaneous Status */
+#define VSC73XX_MISCSTAT_CPU_TX_DATA_PENDING	BIT(8)
+#define VSC73XX_MISCSTAT_CPU_TX_DATA_OVERFLOW	BIT(7)
+
 /* Frame analyzer block 2 registers */
 #define VSC73XX_STORMLIMIT	0x02
 #define VSC73XX_ADVLEARN	0x03
@@ -363,6 +374,9 @@
 #define VSC73XX_MDIO_POLL_SLEEP_US	5
 #define VSC73XX_POLL_TIMEOUT_US		10000
 
+#define VSC73XX_IFH_MAGIC		0x52
+#define VSC73XX_IFH_SIZE		8
+
 struct vsc73xx_counter {
 	u8 counter;
 	const char *name;
@@ -375,6 +389,31 @@ struct vsc73xx_fdb {
 	bool valid;
 };
 
+/* Internal frame header structure */
+struct vsc73xx_ifh {
+	union {
+		u32 datah;
+		struct {
+		u32 wt:1, /* Frame was tagged but tag has removed from frame */
+		    : 1,
+		    frame_length:14, /* Frame Length including CRC */
+		    : 11,
+		    port:5; /* SRC port of switch */
+		};
+	};
+	union {
+		u32 datal;
+		struct {
+		u32 vid:16, /* VLAN ID */
+		    : 3,
+		    magic:9, /* IFH magic field */
+		    lpa:1, /* SMAC is subject of learning */
+		    : 1,
+		    priority:2; /* Switch categorizer assigned priority */
+		};
+	};
+};
+
 /* Counters are named according to the MIB standards where applicable.
  * Some counters are custom, non-standard. The standard counters are
  * named in accordance with RFC2819, RFC2021 and IEEE Std 802.3-2002 Annex
@@ -683,6 +722,133 @@ static int vsc73xx_phy_write(struct dsa_switch *ds, int phy, int regnum,
 	return 0;
 }
 
+static int vsc73xx_tx_fifo_busy_check(struct vsc73xx *vsc, int port)
+{
+	int ret, err;
+	u32 val;
+
+	ret = read_poll_timeout(vsc73xx_read, err,
+				err < 0 ||
+				!(val & VSC73XX_MISCSTAT_CPU_TX_DATA_PENDING),
+				VSC73XX_POLL_SLEEP_US,
+				VSC73XX_POLL_TIMEOUT_US, false, vsc,
+				VSC73XX_BLOCK_MAC, port, VSC73XX_MISCSTAT,
+				&val);
+	if (ret)
+		return ret;
+	return err;
+}
+
+static int
+vsc73xx_write_tx_fifo(struct vsc73xx *vsc, int port, u32 data0, u32 data1)
+{
+	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CPUTXDAT, data0);
+	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CPUTXDAT, data1);
+
+	return vsc73xx_tx_fifo_busy_check(vsc, port);
+}
+
+static int
+vsc73xx_inject_frame(struct vsc73xx *vsc, int port, struct sk_buff *skb)
+{
+	struct vsc73xx_ifh *ifh;
+	u32 length, i, count;
+	u32 *buf;
+	int ret;
+
+	if (skb->len + VSC73XX_IFH_SIZE < 64)
+		length = 64;
+	else
+		length = skb->len + VSC73XX_IFH_SIZE;
+
+	count = DIV_ROUND_UP(length, 8);
+	buf = kzalloc(count * 8, GFP_KERNEL);
+	memset(buf, 0, sizeof(buf));
+
+	ifh = (struct vsc73xx_ifh *)buf;
+	ifh->frame_length = skb->len;
+	ifh->magic = VSC73XX_IFH_MAGIC;
+
+	skb_copy_and_csum_dev(skb, (u8 *)(buf + 2));
+
+	for (i = 0; i < count; i++) {
+		ret = vsc73xx_write_tx_fifo(vsc, port, buf[2 * i],
+					    buf[2 * i + 1]);
+		if (ret) {
+			/* Clear buffer after error */
+			vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
+					    VSC73XX_MISCFIFO,
+					    VSC73XX_MISCFIFO_REWIND_CPU_TX,
+					    VSC73XX_MISCFIFO_REWIND_CPU_TX);
+			goto err;
+		}
+	}
+
+	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MISCFIFO,
+		      VSC73XX_MISCFIFO_CPU_TX);
+
+	skb_tx_timestamp(skb);
+
+	skb->dev->stats.tx_packets++;
+	skb->dev->stats.tx_bytes += skb->len;
+err:
+	kfree(buf);
+	return ret;
+}
+
+#define work_to_xmit_work(w) \
+		container_of((w), struct vsc73xx_deferred_xmit_work, work)
+
+static void vsc73xx_deferred_xmit(struct kthread_work *work)
+{
+	struct vsc73xx_deferred_xmit_work *xmit_work = work_to_xmit_work(work);
+	struct dsa_switch *ds = xmit_work->dp->ds;
+	struct sk_buff *skb = xmit_work->skb;
+	int port = xmit_work->dp->index;
+	struct vsc73xx *vsc = ds->priv;
+	int ret;
+
+	if (vsc73xx_tx_fifo_busy_check(vsc, port)) {
+		dev_err(vsc->dev, "port %d failed to inject skb\n",
+			port);
+
+		/* Clear buffer after error */
+		vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
+				    VSC73XX_MISCFIFO,
+				    VSC73XX_MISCFIFO_REWIND_CPU_TX,
+				    VSC73XX_MISCFIFO_REWIND_CPU_TX);
+
+		kfree_skb(skb);
+		return;
+	}
+
+	ret = vsc73xx_inject_frame(vsc, port, skb);
+
+	if (ret) {
+		dev_err(vsc->dev, "port %d failed to inject skb\n",
+			port);
+		return;
+	}
+
+	consume_skb(skb);
+	kfree(xmit_work);
+}
+
+static int
+vsc73xx_connect_tag_protocol(struct dsa_switch *ds, enum dsa_tag_protocol proto)
+{
+	struct vsc73xx_8021q_tagger_data *tagger_data;
+
+	switch (proto) {
+	case DSA_TAG_PROTO_VSC73XX_8021Q:
+		tagger_data = ds->tagger_data;
+		tagger_data->xmit_work_fn = vsc73xx_deferred_xmit;
+		return 0;
+	default:
+		return -EPROTONOSUPPORT;
+	}
+}
+
 static enum dsa_tag_protocol vsc73xx_get_tag_protocol(struct dsa_switch *ds,
 						      int port,
 						      enum dsa_tag_protocol mp)
@@ -1026,6 +1192,11 @@ static void vsc73xx_init_port(struct vsc73xx *vsc, int port)
 		      VSC73XX_CAT_DROP,
 		      VSC73XX_CAT_DROP_FWD_PAUSE_ENA);
 
+	/* Allow switch to recalculate CRC of CPU packets */
+	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_TXUPDCFG,
+			    VSC73XX_TXUPDCFG_TX_UPDATE_CRC_CPU_ENA,
+			    VSC73XX_TXUPDCFG_TX_UPDATE_CRC_CPU_ENA);
+
 	/* Clear all counters */
 	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC,
 		      port, VSC73XX_C_RX0, 0);
@@ -2217,6 +2388,7 @@ static const struct phylink_mac_ops vsc73xx_phylink_mac_ops = {
 
 static const struct dsa_switch_ops vsc73xx_ds_ops = {
 	.get_tag_protocol = vsc73xx_get_tag_protocol,
+	.connect_tag_protocol = vsc73xx_connect_tag_protocol,
 	.setup = vsc73xx_setup,
 	.teardown = vsc73xx_teardown,
 	.phy_read = vsc73xx_phy_read,
diff --git a/drivers/net/dsa/vitesse-vsc73xx.h b/drivers/net/dsa/vitesse-vsc73xx.h
index 3c30e143c14f..bf55a20f07f3 100644
--- a/drivers/net/dsa/vitesse-vsc73xx.h
+++ b/drivers/net/dsa/vitesse-vsc73xx.h
@@ -2,6 +2,7 @@
 #include <linux/device.h>
 #include <linux/etherdevice.h>
 #include <linux/gpio/driver.h>
+#include <linux/dsa/vsc73xx.h>
 
 /* The VSC7395 switch chips have 5+1 ports which means 5 ordinary ports and
  * a sixth CPU port facing the processor with an RGMII interface. These ports
diff --git a/include/linux/dsa/vsc73xx.h b/include/linux/dsa/vsc73xx.h
new file mode 100644
index 000000000000..901eeb1da120
--- /dev/null
+++ b/include/linux/dsa/vsc73xx.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0
+ * Copyright (c) 2024, Pawel Dembicki <paweldembicki@gmail.com>
+ */
+
+/* Included by drivers/net/dsa/vitesse-vsc73xx.h and net/dsa/tag_vsc73xx_8021q.c */
+
+#ifndef _NET_DSA_VSC73XX_H
+#define _NET_DSA_VSC73XX_H
+
+struct vsc73xx_deferred_xmit_work {
+	struct dsa_port *dp;
+	struct sk_buff *skb;
+	struct kthread_work work;
+};
+
+struct vsc73xx_8021q_tagger_data {
+	void (*xmit_work_fn)(struct kthread_work *work);
+};
+
+#endif /* _NET_DSA_VSC73XX_H */
diff --git a/net/dsa/tag_vsc73xx_8021q.c b/net/dsa/tag_vsc73xx_8021q.c
index af121a9aff7f..d1d7a860a76e 100644
--- a/net/dsa/tag_vsc73xx_8021q.c
+++ b/net/dsa/tag_vsc73xx_8021q.c
@@ -2,20 +2,61 @@
 /* Copyright (C) 2024 Pawel Dembicki <paweldembicki@gmail.com>
  */
 #include <linux/dsa/8021q.h>
+#include <linux/dsa/vsc73xx.h>
 
 #include "tag.h"
 #include "tag_8021q.h"
 
 #define VSC73XX_8021Q_NAME "vsc73xx-8021q"
 
+struct vsc73xx_8021q_tagger_private {
+	struct vsc73xx_8021q_tagger_data data; /* Must be first */
+	struct kthread_worker *xmit_worker;
+};
+
+static struct sk_buff *vsc73xx_defer_xmit(struct dsa_port *dp, struct sk_buff *skb)
+{
+	struct vsc73xx_8021q_tagger_private *priv = dp->ds->tagger_data;
+	struct vsc73xx_8021q_tagger_data *data = &priv->data;
+	void (*xmit_work_fn)(struct kthread_work *work);
+	struct vsc73xx_deferred_xmit_work *xmit_work;
+	struct kthread_worker *xmit_worker;
+
+	xmit_work_fn = data->xmit_work_fn;
+	xmit_worker = priv->xmit_worker;
+
+	if (!xmit_work_fn || !xmit_worker)
+		return NULL;
+
+	xmit_work = kzalloc(sizeof(*xmit_work), GFP_ATOMIC);
+	if (!xmit_work)
+		return NULL;
+
+	/* Calls vsc73xx_port_deferred_xmit in vitesse-vsc73xx-core.c */
+	kthread_init_work(&xmit_work->work, xmit_work_fn);
+	/* Increase refcount so the kfree_skb in dsa_slave_xmit
+	 * won't really free the packet.
+	 */
+	xmit_work->dp = dp;
+	xmit_work->skb = skb_get(skb);
+
+	kthread_queue_work(xmit_worker, &xmit_work->work);
+
+	return NULL;
+}
+
 static struct sk_buff *
 vsc73xx_xmit(struct sk_buff *skb, struct net_device *netdev)
 {
 	struct dsa_port *dp = dsa_user_to_port(netdev);
 	u16 queue_mapping = skb_get_queue_mapping(skb);
 	u16 tx_vid = dsa_tag_8021q_standalone_vid(dp);
+	struct ethhdr *hdr = eth_hdr(skb);
 	u8 pcp;
 
+	if (is_link_local_ether_addr(hdr->h_dest))
+		return vsc73xx_defer_xmit(dp, skb);
+
 	if (skb->offload_fwd_mark) {
 		unsigned int bridge_num = dsa_port_bridge_num_get(dp);
 		struct net_device *br = dsa_port_bridge_dev_get(dp);
@@ -52,11 +93,43 @@ vsc73xx_rcv(struct sk_buff *skb, struct net_device *netdev)
 	return skb;
 }
 
+static void vsc73xx_disconnect(struct dsa_switch *ds)
+{
+	struct vsc73xx_8021q_tagger_private *priv = ds->tagger_data;
+
+	kthread_destroy_worker(priv->xmit_worker);
+	kfree(priv);
+	ds->tagger_data = NULL;
+}
+
+static int vsc73xx_connect(struct dsa_switch *ds)
+{
+	struct vsc73xx_8021q_tagger_private *priv;
+	int err;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->xmit_worker = kthread_create_worker(0, "vsc73xx_xmit");
+	if (IS_ERR(priv->xmit_worker)) {
+		err = PTR_ERR(priv->xmit_worker);
+		kfree(priv);
+		return err;
+	}
+
+	ds->tagger_data = priv;
+
+	return 0;
+}
+
 static const struct dsa_device_ops vsc73xx_8021q_netdev_ops = {
 	.name			= VSC73XX_8021Q_NAME,
 	.proto			= DSA_TAG_PROTO_VSC73XX_8021Q,
 	.xmit			= vsc73xx_xmit,
 	.rcv			= vsc73xx_rcv,
+	.connect		= vsc73xx_connect,
+	.disconnect		= vsc73xx_disconnect,
 	.needed_headroom	= VLAN_HLEN,
 	.promisc_on_conduit	= true,
 };
-- 
2.34.1

[PATCH net-next 2/3] net: dsa: vsc73xx: implement packet reception via control interface

[Pawel Dembicki]

Some types of packets can be forwarded only to and from the PI/SI
interface. For more information, see Chapter 2.7.1 (CPU Forwarding) in
the datasheet.

This patch implements the routines required for link-local reception.
This kind of traffic can't be transferred through the RGMII interface in
vsc73xx.

The packet receiver poller uses a kthread worker, which checks if a packet
has arrived in the CPU buffer. If the header is valid, the packet is
transferred to the correct DSA conduit interface.

Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
---
 drivers/net/dsa/vitesse-vsc73xx-core.c | 174 +++++++++++++++++++++++++
 drivers/net/dsa/vitesse-vsc73xx.h      |   4 +
 2 files changed, 178 insertions(+)

diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
index 21ab3f214490..596b11c4d672 100644
--- a/drivers/net/dsa/vitesse-vsc73xx-core.c
+++ b/drivers/net/dsa/vitesse-vsc73xx-core.c
@@ -45,6 +45,15 @@
 #define VSC73XX_BLOCK_MII_INTERNAL	0x0 /* Internal MDIO subblock */
 #define VSC73XX_BLOCK_MII_EXTERNAL	0x1 /* External MDIO subblock */
 
+/* CAPTURE Block subblock */
+#define VSC73XX_BLOCK_CAPT_FRAME0	0x0 /* Frame 0 subblock */
+#define VSC73XX_BLOCK_CAPT_FRAME1	0x1 /* Frame 1 subblock */
+#define VSC73XX_BLOCK_CAPT_FRAME2	0x2 /* Frame 2 subblock */
+#define VSC73XX_BLOCK_CAPT_FRAME3	0x3 /* Frame 3 subblock */
+#define VSC73XX_BLOCK_CAPT_Q0		0x4 /* Queue 0 subblock */
+#define VSC73XX_BLOCK_CAPT_Q1		0x6 /* Queue 0 subblock */
+#define VSC73XX_BLOCK_CAPT_RST		0x7 /* Capture reset subblock */
+
 #define CPU_PORT	6 /* CPU port */
 #define VSC73XX_NUM_FDB_ROWS	2048
 #define VSC73XX_NUM_BUCKETS	4
@@ -244,6 +253,13 @@
 #define VSC73XX_MACTINDX_BUCKET_MSK		GENMASK(12, 11)
 #define VSC73XX_MACTINDX_INDEX_MSK		GENMASK(10, 0)
 
+#define VSC73XX_CAPENAB_IPMC			BIT(20)
+#define VSC73XX_CAPENAB_ARPBC			BIT(19)
+#define VSC73XX_CAPENAB_IGMP			BIT(18)
+#define VSC73XX_CAPENAB_ALLBRIDGE		BIT(17)
+#define VSC73XX_CAPENAB_BPDU			BIT(16)
+#define VSC73XX_CAPENAB_GARP			GENMASK(15, 0)
+
 #define VSC73XX_MACACCESS_CPU_COPY		BIT(14)
 #define VSC73XX_MACACCESS_FWD_KILL		BIT(13)
 #define VSC73XX_MACACCESS_IGNORE_VLAN		BIT(12)
@@ -297,6 +313,13 @@
 
 #define VSC73XX_MII_STAT_BUSY	BIT(3)
 
+/* Capture block 4 registers */
+#define VSC73XX_CAPT_FRAME_DATA		0x0
+#define VSC73XX_CAPT_FRAME_DATA_MAX	0xff
+#define VSC73XX_CAPT_CAPREADP		0x0
+#define VSC73XX_CAPT_CAPWRP		0x3
+#define VSC73XX_CAPT_CAPRST		0xff
+
 /* Arbiter block 5 registers */
 #define VSC73XX_ARBEMPTY		0x0c
 #define VSC73XX_ARBDISC			0x0e
@@ -316,6 +339,7 @@
 #define VSC73XX_ICPU_MBOX_SET		0x16
 #define VSC73XX_ICPU_MBOX_CLR		0x17
 #define VSC73XX_CHIPID			0x18
+#define VSC73XX_CAPCTRL			0x31
 #define VSC73XX_GPIO			0x34
 
 #define VSC73XX_GMIIDELAY_GMII0_GTXDELAY_NONE	0
@@ -364,6 +388,24 @@
 				 VSC73XX_ICPU_CTRL_CLK_EN | \
 				 VSC73XX_ICPU_CTRL_SRST)
 
+#define VSC73XX_CAPCTRL_QUEUE1_READY		BIT(31)
+#define VSC73XX_CAPCTRL_QUEUE0_READY		BIT(30)
+#define VSC73XX_CAPCTRL_ARPBC_Q			BIT(18)
+#define VSC73XX_CAPCTRL_IPMC_Q			BIT(17)
+#define VSC73XX_CAPCTRL_IGMP_Q			BIT(16)
+#define VSC73XX_CAPCTRL_ALLBRIDGE_Q		BIT(15)
+#define VSC73XX_CAPCTRL_GARP_Q			BIT(14)
+#define VSC73XX_CAPCTRL_BPDU_Q			BIT(13)
+#define VSC73XX_CAPCTRL_FIFO_MODE		BIT(12)
+#define VSC73XX_CAPCTRL_QUEUE1_ENA		BIT(11)
+#define VSC73XX_CAPCTRL_Q1_IRQ_EN		BIT(6)
+#define VSC73XX_CAPCTRL_Q0_IRQ_EN		BIT(5)
+#define VSC73XX_CAPCTRL_Q1_IRQ_PIN		BIT(4)
+#define VSC73XX_CAPCTRL_Q0_IRQ_PIN		BIT(3)
+#define VSC73XX_CAPCTRL_LEARN_TRUNCATE		BIT(2)
+#define VSC73XX_CAPCTRL_LEARN_Q			BIT(1)
+#define VSC73XX_CAPCTRL_MACB_Q			BIT(0)
+
 #define IS_7385(a) ((a)->chipid == VSC73XX_CHIPID_ID_7385)
 #define IS_7388(a) ((a)->chipid == VSC73XX_CHIPID_ID_7388)
 #define IS_7395(a) ((a)->chipid == VSC73XX_CHIPID_ID_7395)
@@ -373,6 +415,7 @@
 #define VSC73XX_POLL_SLEEP_US		1000
 #define VSC73XX_MDIO_POLL_SLEEP_US	5
 #define VSC73XX_POLL_TIMEOUT_US		10000
+#define VSC73XX_RCV_POLL_INTERVAL	100
 
 #define VSC73XX_IFH_MAGIC		0x52
 #define VSC73XX_IFH_SIZE		8
@@ -834,6 +877,115 @@ static void vsc73xx_deferred_xmit(struct kthread_work *work)
 	kfree(xmit_work);
 }
 
+static void vsc73xx_polled_rcv(struct kthread_work *work)
+{
+	struct vsc73xx *vsc = container_of(work, struct vsc73xx, dwork.work);
+	u16 ptr = VSC73XX_CAPT_FRAME_DATA;
+	struct dsa_switch *ds = vsc->ds;
+	int ret, buf_len, len, part;
+	struct vsc73xx_ifh ifh;
+	struct net_device *dev;
+	struct dsa_port *dp;
+	struct sk_buff *skb;
+	u32 val, *buf;
+	u16 count;
+
+	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_SYSTEM, 0, VSC73XX_CAPCTRL, &val);
+	if (ret)
+		goto queue;
+
+	if (!(val & VSC73XX_CAPCTRL_QUEUE0_READY))
+		/* No frame to read */
+		goto queue;
+
+	/* Initialise reading */
+	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_CAPTURE, VSC73XX_BLOCK_CAPT_Q0,
+			   VSC73XX_CAPT_CAPREADP, &val);
+	if (ret)
+		goto queue;
+
+	/* Get internal frame header */
+	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_CAPTURE,
+			   VSC73XX_BLOCK_CAPT_FRAME0, ptr++, &ifh.datah);
+	if (ret)
+		goto queue;
+
+	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_CAPTURE,
+			   VSC73XX_BLOCK_CAPT_FRAME0, ptr++, &ifh.datal);
+	if (ret)
+		goto queue;
+
+	if (ifh.magic != VSC73XX_IFH_MAGIC) {
+		/* Something goes wrong with buffer. Reset capture block */
+		vsc73xx_write(vsc, VSC73XX_BLOCK_CAPTURE,
+			      VSC73XX_BLOCK_CAPT_RST, VSC73XX_CAPT_CAPRST, 1);
+		goto queue;
+	}
+
+	if (!dsa_is_user_port(ds, ifh.port))
+		goto release_frame;
+
+	dp = dsa_to_port(ds, ifh.port);
+	dev = dp->user;
+	if (!dev)
+		goto release_frame;
+
+	count = (ifh.frame_length + 7 + VSC73XX_IFH_SIZE - ETH_FCS_LEN) >> 2;
+
+	skb = netdev_alloc_skb(dev, len);
+	if (unlikely(!skb)) {
+		netdev_err(dev, "Unable to allocate sk_buff\n");
+		goto release_frame;
+	}
+
+	buf_len = ifh.frame_length - ETH_FCS_LEN;
+	buf = (u32 *)skb_put(skb, buf_len);
+	len = 0;
+	part = 0;
+
+	while (ptr < count) {
+		ret = vsc73xx_read(vsc, VSC73XX_BLOCK_CAPTURE,
+				   VSC73XX_BLOCK_CAPT_FRAME0 + part, ptr++,
+				   buf + len);
+		if (ret)
+			goto free_skb;
+		len++;
+		if (ptr > VSC73XX_CAPT_FRAME_DATA_MAX &&
+		    count != VSC73XX_CAPT_FRAME_DATA_MAX) {
+			ptr = VSC73XX_CAPT_FRAME_DATA;
+			part++;
+			count -= VSC73XX_CAPT_FRAME_DATA_MAX;
+		}
+	}
+
+	/* Get FCS */
+	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_CAPTURE,
+			   VSC73XX_BLOCK_CAPT_FRAME0, ptr++, &val);
+	if (ret)
+		goto free_skb;
+
+	/* Everything we see on an interface that is in the HW bridge
+	 * has already been forwarded.
+	 */
+	if (dp->bridge)
+		skb->offload_fwd_mark = 1;
+
+	skb->protocol = eth_type_trans(skb, dev);
+
+	netif_rx(skb);
+	goto release_frame;
+
+free_skb:
+	kfree_skb(skb);
+release_frame:
+	/* Release the frame from internal buffer */
+	vsc73xx_write(vsc, VSC73XX_BLOCK_CAPTURE, VSC73XX_BLOCK_CAPT_Q0,
+		      VSC73XX_CAPT_CAPREADP, 0);
+queue:
+	kthread_queue_delayed_work(vsc->rcv_worker, &vsc->dwork,
+				   msecs_to_jiffies(VSC73XX_RCV_POLL_INTERVAL));
+}
+
 static int
 vsc73xx_connect_tag_protocol(struct dsa_switch *ds, enum dsa_tag_protocol proto)
 {
@@ -1111,14 +1263,36 @@ static int vsc73xx_setup(struct dsa_switch *ds)
 	ret = dsa_tag_8021q_register(ds, htons(ETH_P_8021Q));
 	rtnl_unlock();
 
+	/* Reset capture block */
+	vsc73xx_write(vsc, VSC73XX_BLOCK_CAPTURE, VSC73XX_BLOCK_CAPT_RST,
+		      VSC73XX_CAPT_CAPRST, 1);
+
+	/* Capture BPDU frames */
+	vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_CAPENAB,
+		      VSC73XX_CAPENAB_BPDU);
+
+	vsc->rcv_worker = kthread_create_worker(0, "vsc73xx_rcv");
+	if (IS_ERR(vsc->rcv_worker))
+		return PTR_ERR(vsc->rcv_worker);
+
+	kthread_init_delayed_work(&vsc->dwork, vsc73xx_polled_rcv);
+
+	kthread_queue_delayed_work(vsc->rcv_worker, &vsc->dwork,
+				   msecs_to_jiffies(VSC73XX_RCV_POLL_INTERVAL));
+
 	return ret;
 }
 
 static void vsc73xx_teardown(struct dsa_switch *ds)
 {
+	struct vsc73xx *vsc = ds->priv;
+
 	rtnl_lock();
 	dsa_tag_8021q_unregister(ds);
 	rtnl_unlock();
+
+	kthread_cancel_delayed_work_sync(&vsc->dwork);
+	kthread_destroy_worker(vsc->rcv_worker);
 }
 
 static void vsc73xx_init_port(struct vsc73xx *vsc, int port)
diff --git a/drivers/net/dsa/vitesse-vsc73xx.h b/drivers/net/dsa/vitesse-vsc73xx.h
index bf55a20f07f3..5dd458793741 100644
--- a/drivers/net/dsa/vitesse-vsc73xx.h
+++ b/drivers/net/dsa/vitesse-vsc73xx.h
@@ -47,6 +47,8 @@ struct vsc73xx_portinfo {
  *	every vlan configured in port vlan operation. It doesn't cover tag_8021q
  *	vlans.
  * @fdb_lock: Mutex protects fdb access
+ * @rcv_worker: Kthread worker struct for packet reciver poller
+ * @dwork: Work struct for scheduling work to the packet reciver poller
  */
 struct vsc73xx {
 	struct device			*dev;
@@ -60,6 +62,8 @@ struct vsc73xx {
 	struct vsc73xx_portinfo		portinfo[VSC73XX_MAX_NUM_PORTS];
 	struct list_head		vlans;
 	struct mutex			fdb_lock;
+	struct kthread_worker		*rcv_worker;
+	struct kthread_delayed_work	dwork;
 };
 
 /**
-- 
2.34.1

[PATCH net-next 3/3] net: dsa: vsc73xx: Remove FIXME

[Pawel Dembicki]

STP frames can be captured and transmitted now. FIXME can be removed.

Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
---
 drivers/net/dsa/vitesse-vsc73xx-core.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
index 596b11c4d672..0d05e3dcf05f 100644
--- a/drivers/net/dsa/vitesse-vsc73xx-core.c
+++ b/drivers/net/dsa/vitesse-vsc73xx-core.c
@@ -2218,11 +2218,6 @@ static void vsc73xx_refresh_fwd_map(struct dsa_switch *ds, int port, u8 state)
 			    VSC73XX_SRCMASKS_PORTS_MASK, mask);
 }
 
-/* FIXME: STP frames aren't forwarded at this moment. BPDU frames are
- * forwarded only from and to PI/SI interface. For more info see chapter
- * 2.7.1 (CPU Forwarding) in datasheet.
- * This function is required for tag_8021q operations.
- */
 static void vsc73xx_port_stp_state_set(struct dsa_switch *ds, int port,
 				       u8 state)
 {
-- 
2.34.1

Re: [PATCH net-next 2/3] net: dsa: vsc73xx: implement packet reception via control interface

[Simon Horman]

On Sun, Oct 20, 2024 at 10:54:51PM +0200, Pawel Dembicki wrote:
> Some types of packets can be forwarded only to and from the PI/SI
> interface. For more information, see Chapter 2.7.1 (CPU Forwarding) in
> the datasheet.
> 
> This patch implements the routines required for link-local reception.
> This kind of traffic can't be transferred through the RGMII interface in
> vsc73xx.
> 
> The packet receiver poller uses a kthread worker, which checks if a packet
> has arrived in the CPU buffer. If the header is valid, the packet is
> transferred to the correct DSA conduit interface.
> 
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>

Hi Pawel,

This is not a full review, but I noticed a problem that I wanted to bring
to your attention. Please wait a day or so for others to provide a proper
review before posting a v2.

Thanks!

> ---
>  drivers/net/dsa/vitesse-vsc73xx-core.c | 174 +++++++++++++++++++++++++
>  drivers/net/dsa/vitesse-vsc73xx.h      |   4 +
>  2 files changed, 178 insertions(+)
> 
> diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c

...

> @@ -373,6 +415,7 @@
>  #define VSC73XX_POLL_SLEEP_US		1000
>  #define VSC73XX_MDIO_POLL_SLEEP_US	5
>  #define VSC73XX_POLL_TIMEOUT_US		10000
> +#define VSC73XX_RCV_POLL_INTERVAL	100
>  
>  #define VSC73XX_IFH_MAGIC		0x52
>  #define VSC73XX_IFH_SIZE		8
> @@ -834,6 +877,115 @@ static void vsc73xx_deferred_xmit(struct kthread_work *work)
>  	kfree(xmit_work);
>  }
>  
> +static void vsc73xx_polled_rcv(struct kthread_work *work)
> +{
> +	struct vsc73xx *vsc = container_of(work, struct vsc73xx, dwork.work);
> +	u16 ptr = VSC73XX_CAPT_FRAME_DATA;
> +	struct dsa_switch *ds = vsc->ds;
> +	int ret, buf_len, len, part;
> +	struct vsc73xx_ifh ifh;
> +	struct net_device *dev;
> +	struct dsa_port *dp;
> +	struct sk_buff *skb;
> +	u32 val, *buf;
> +	u16 count;
> +
> +	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_SYSTEM, 0, VSC73XX_CAPCTRL, &val);
> +	if (ret)
> +		goto queue;
> +
> +	if (!(val & VSC73XX_CAPCTRL_QUEUE0_READY))
> +		/* No frame to read */
> +		goto queue;
> +
> +	/* Initialise reading */
> +	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_CAPTURE, VSC73XX_BLOCK_CAPT_Q0,
> +			   VSC73XX_CAPT_CAPREADP, &val);
> +	if (ret)
> +		goto queue;
> +
> +	/* Get internal frame header */
> +	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_CAPTURE,
> +			   VSC73XX_BLOCK_CAPT_FRAME0, ptr++, &ifh.datah);
> +	if (ret)
> +		goto queue;
> +
> +	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_CAPTURE,
> +			   VSC73XX_BLOCK_CAPT_FRAME0, ptr++, &ifh.datal);
> +	if (ret)
> +		goto queue;
> +
> +	if (ifh.magic != VSC73XX_IFH_MAGIC) {
> +		/* Something goes wrong with buffer. Reset capture block */
> +		vsc73xx_write(vsc, VSC73XX_BLOCK_CAPTURE,
> +			      VSC73XX_BLOCK_CAPT_RST, VSC73XX_CAPT_CAPRST, 1);
> +		goto queue;
> +	}
> +
> +	if (!dsa_is_user_port(ds, ifh.port))
> +		goto release_frame;
> +
> +	dp = dsa_to_port(ds, ifh.port);
> +	dev = dp->user;
> +	if (!dev)
> +		goto release_frame;
> +
> +	count = (ifh.frame_length + 7 + VSC73XX_IFH_SIZE - ETH_FCS_LEN) >> 2;
> +
> +	skb = netdev_alloc_skb(dev, len);

len does not appear to be initialised here.

Flagged by W=1 builds.

> +	if (unlikely(!skb)) {
> +		netdev_err(dev, "Unable to allocate sk_buff\n");
> +		goto release_frame;
> +	}
> +
> +	buf_len = ifh.frame_length - ETH_FCS_LEN;
> +	buf = (u32 *)skb_put(skb, buf_len);
> +	len = 0;
> +	part = 0;
> +
> +	while (ptr < count) {
> +		ret = vsc73xx_read(vsc, VSC73XX_BLOCK_CAPTURE,
> +				   VSC73XX_BLOCK_CAPT_FRAME0 + part, ptr++,
> +				   buf + len);
> +		if (ret)
> +			goto free_skb;
> +		len++;
> +		if (ptr > VSC73XX_CAPT_FRAME_DATA_MAX &&
> +		    count != VSC73XX_CAPT_FRAME_DATA_MAX) {
> +			ptr = VSC73XX_CAPT_FRAME_DATA;
> +			part++;
> +			count -= VSC73XX_CAPT_FRAME_DATA_MAX;
> +		}
> +	}
> +
> +	/* Get FCS */
> +	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_CAPTURE,
> +			   VSC73XX_BLOCK_CAPT_FRAME0, ptr++, &val);
> +	if (ret)
> +		goto free_skb;
> +
> +	/* Everything we see on an interface that is in the HW bridge
> +	 * has already been forwarded.
> +	 */
> +	if (dp->bridge)
> +		skb->offload_fwd_mark = 1;
> +
> +	skb->protocol = eth_type_trans(skb, dev);
> +
> +	netif_rx(skb);
> +	goto release_frame;
> +
> +free_skb:
> +	kfree_skb(skb);
> +release_frame:
> +	/* Release the frame from internal buffer */
> +	vsc73xx_write(vsc, VSC73XX_BLOCK_CAPTURE, VSC73XX_BLOCK_CAPT_Q0,
> +		      VSC73XX_CAPT_CAPREADP, 0);
> +queue:
> +	kthread_queue_delayed_work(vsc->rcv_worker, &vsc->dwork,
> +				   msecs_to_jiffies(VSC73XX_RCV_POLL_INTERVAL));
> +}

...

-- 
pw-bot: changes-requested

Re: [PATCH net-next 2/3] net: dsa: vsc73xx: implement packet reception via control interface

[Vladimir Oltean]

On Sun, Oct 20, 2024 at 10:54:51PM +0200, Pawel Dembicki wrote:
> Some types of packets can be forwarded only to and from the PI/SI
> interface. For more information, see Chapter 2.7.1 (CPU Forwarding) in
> the datasheet.
> 
> This patch implements the routines required for link-local reception.
> This kind of traffic can't be transferred through the RGMII interface in
> vsc73xx.
> 
> The packet receiver poller uses a kthread worker, which checks if a packet
> has arrived in the CPU buffer. If the header is valid, the packet is
> transferred to the correct DSA conduit interface.
> 
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> ---

Is there no way to use an IRQ for packet reception from the PI/SI interface?
And if not, is there no way to use any workaround to do event-based reception?
Like for example how felix has the quirk_no_xtr_irq mechanism through which
it replicates all traffic that would go to the PI/SI interface to also go
over Ethernet, and use the reception of the packet as the trigger for something
being available to read from PI/SI?

> +static void vsc73xx_polled_rcv(struct kthread_work *work)
> +{
> +	struct vsc73xx *vsc = container_of(work, struct vsc73xx, dwork.work);
> +	u16 ptr = VSC73XX_CAPT_FRAME_DATA;
> +	struct dsa_switch *ds = vsc->ds;
> +	int ret, buf_len, len, part;
> +	struct vsc73xx_ifh ifh;
> +	struct net_device *dev;
> +	struct dsa_port *dp;
> +	struct sk_buff *skb;
> +	u32 val, *buf;
> +	u16 count;
> +
> +	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_SYSTEM, 0, VSC73XX_CAPCTRL, &val);
> +	if (ret)
> +		goto queue;
> +
> +	if (!(val & VSC73XX_CAPCTRL_QUEUE0_READY))
> +		/* No frame to read */
> +		goto queue;
> +
> +	/* Initialise reading */
> +	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_CAPTURE, VSC73XX_BLOCK_CAPT_Q0,
> +			   VSC73XX_CAPT_CAPREADP, &val);
> +	if (ret)
> +		goto queue;
> +
> +	/* Get internal frame header */
> +	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_CAPTURE,
> +			   VSC73XX_BLOCK_CAPT_FRAME0, ptr++, &ifh.datah);
> +	if (ret)
> +		goto queue;
> +
> +	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_CAPTURE,
> +			   VSC73XX_BLOCK_CAPT_FRAME0, ptr++, &ifh.datal);
> +	if (ret)
> +		goto queue;
> +
> +	if (ifh.magic != VSC73XX_IFH_MAGIC) {
> +		/* Something goes wrong with buffer. Reset capture block */
> +		vsc73xx_write(vsc, VSC73XX_BLOCK_CAPTURE,
> +			      VSC73XX_BLOCK_CAPT_RST, VSC73XX_CAPT_CAPRST, 1);

Log that?

> +		goto queue;
> +	}
> +
> +	if (!dsa_is_user_port(ds, ifh.port))
> +		goto release_frame;

First do the dsa_to_port(), and then convert this to dsa_port_is_user().

> +
> +	dp = dsa_to_port(ds, ifh.port);
> +	dev = dp->user;
> +	if (!dev)
> +		goto release_frame;
> +
> +	count = (ifh.frame_length + 7 + VSC73XX_IFH_SIZE - ETH_FCS_LEN) >> 2;

What's "(.. + 7) >> 2" doing? Some sort of DIV_ROUND_UP(..., 4)? But why 7?
Please don't be afraid to use the arithmetic macros that make the code
more readable. You can confirm with "make drivers/net/dsa/vitesse-vsc73xx-core.lst"
that they should end up generating code that is just as efficient as the
"optimized" bit shift.

> +
> +	skb = netdev_alloc_skb(dev, len);
> +	if (unlikely(!skb)) {
> +		netdev_err(dev, "Unable to allocate sk_buff\n");
> +		goto release_frame;
> +	}
> +
> +	buf_len = ifh.frame_length - ETH_FCS_LEN;
> +	buf = (u32 *)skb_put(skb, buf_len);
> +	len = 0;
> +	part = 0;
> +
> +	while (ptr < count) {
> +		ret = vsc73xx_read(vsc, VSC73XX_BLOCK_CAPTURE,
> +				   VSC73XX_BLOCK_CAPT_FRAME0 + part, ptr++,
> +				   buf + len);
> +		if (ret)
> +			goto free_skb;
> +		len++;
> +		if (ptr > VSC73XX_CAPT_FRAME_DATA_MAX &&
> +		    count != VSC73XX_CAPT_FRAME_DATA_MAX) {
> +			ptr = VSC73XX_CAPT_FRAME_DATA;
> +			part++;
> +			count -= VSC73XX_CAPT_FRAME_DATA_MAX;
> +		}
> +	}
> +
> +	/* Get FCS */
> +	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_CAPTURE,
> +			   VSC73XX_BLOCK_CAPT_FRAME0, ptr++, &val);
> +	if (ret)
> +		goto free_skb;
> +
> +	/* Everything we see on an interface that is in the HW bridge
> +	 * has already been forwarded.
> +	 */
> +	if (dp->bridge)
> +		skb->offload_fwd_mark = 1;
> +
> +	skb->protocol = eth_type_trans(skb, dev);
> +
> +	netif_rx(skb);
> +	goto release_frame;
> +
> +free_skb:
> +	kfree_skb(skb);
> +release_frame:
> +	/* Release the frame from internal buffer */
> +	vsc73xx_write(vsc, VSC73XX_BLOCK_CAPTURE, VSC73XX_BLOCK_CAPT_Q0,
> +		      VSC73XX_CAPT_CAPREADP, 0);
> +queue:

Log errors with dev_err_ratelimited() maybe?

> +	kthread_queue_delayed_work(vsc->rcv_worker, &vsc->dwork,
> +				   msecs_to_jiffies(VSC73XX_RCV_POLL_INTERVAL));
> +}
> +
>  static int
>  vsc73xx_connect_tag_protocol(struct dsa_switch *ds, enum dsa_tag_protocol proto)
>  {
> @@ -1111,14 +1263,36 @@ static int vsc73xx_setup(struct dsa_switch *ds)
>  	ret = dsa_tag_8021q_register(ds, htons(ETH_P_8021Q));
>  	rtnl_unlock();
>  
> +	/* Reset capture block */
> +	vsc73xx_write(vsc, VSC73XX_BLOCK_CAPTURE, VSC73XX_BLOCK_CAPT_RST,
> +		      VSC73XX_CAPT_CAPRST, 1);
> +
> +	/* Capture BPDU frames */
> +	vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_CAPENAB,
> +		      VSC73XX_CAPENAB_BPDU);
> +
> +	vsc->rcv_worker = kthread_create_worker(0, "vsc73xx_rcv");
> +	if (IS_ERR(vsc->rcv_worker))
> +		return PTR_ERR(vsc->rcv_worker);

There's teardown work to do on error here.

> +
> +	kthread_init_delayed_work(&vsc->dwork, vsc73xx_polled_rcv);
> +
> +	kthread_queue_delayed_work(vsc->rcv_worker, &vsc->dwork,
> +				   msecs_to_jiffies(VSC73XX_RCV_POLL_INTERVAL));
> +
>  	return ret;

This "return ret" is the error code of dsa_tag_8021q_register(). The new
code block is very badly placed.

>  }
>  
>  static void vsc73xx_teardown(struct dsa_switch *ds)
>  {
> +	struct vsc73xx *vsc = ds->priv;
> +
>  	rtnl_lock();
>  	dsa_tag_8021q_unregister(ds);
>  	rtnl_unlock();
> +
> +	kthread_cancel_delayed_work_sync(&vsc->dwork);
> +	kthread_destroy_worker(vsc->rcv_worker);

This needs to be the reverse process of vsc73xx_setup().

>  }
>  
>  static void vsc73xx_init_port(struct vsc73xx *vsc, int port)
> diff --git a/drivers/net/dsa/vitesse-vsc73xx.h b/drivers/net/dsa/vitesse-vsc73xx.h
> index bf55a20f07f3..5dd458793741 100644
> --- a/drivers/net/dsa/vitesse-vsc73xx.h
> +++ b/drivers/net/dsa/vitesse-vsc73xx.h
> @@ -47,6 +47,8 @@ struct vsc73xx_portinfo {
>   *	every vlan configured in port vlan operation. It doesn't cover tag_8021q
>   *	vlans.
>   * @fdb_lock: Mutex protects fdb access
> + * @rcv_worker: Kthread worker struct for packet reciver poller
> + * @dwork: Work struct for scheduling work to the packet reciver poller
>   */
>  struct vsc73xx {
>  	struct device			*dev;
> @@ -60,6 +62,8 @@ struct vsc73xx {
>  	struct vsc73xx_portinfo		portinfo[VSC73XX_MAX_NUM_PORTS];
>  	struct list_head		vlans;
>  	struct mutex			fdb_lock;
> +	struct kthread_worker		*rcv_worker;
> +	struct kthread_delayed_work	dwork;
>  };
>  
>  /**
> -- 
> 2.34.1
> 

Re: [PATCH net-next 2/3] net: dsa: vsc73xx: implement packet reception via control interface

[Vladimir Oltean]

On Sun, Oct 20, 2024 at 10:54:51PM +0200, Pawel Dembicki wrote:
> The packet receiver poller uses a kthread worker, which checks if a packet
> has arrived in the CPU buffer. If the header is valid, the packet is
> transferred to the correct DSA conduit interface.

s/conduit/user/

Re: [PATCH net-next 1/3] net: dsa: vsc73xx: implement transmit via control interface

[Vladimir Oltean]

On Sun, Oct 20, 2024 at 10:54:50PM +0200, Pawel Dembicki wrote:
> Some types of packets can be forwarded only to and from the PI/SI
> interface. For more information, see Chapter 2.7.1 (CPU Forwarding) in
> the datasheet.
> 
> This patch implements the routines required for link-local transmission.
> This kind of traffic can't be transferred through the RGMII interface in
> vsc73xx.
> 
> It uses a method similar to the sja1005 driver, where the DSA tagger
> checks if the packet is link-local and uses a special deferred transmit
> route for that kind of packet.
> 
> The vsc73xx uses an "Internal Frame Header" (IFH) in communication via the
> PI/SI interface. Every packet must be prefixed with an IFH. The hardware
> fixes the checksums, so there's no need to calculate the FCS in the
> driver.
> 
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> ---
>  drivers/net/dsa/vitesse-vsc73xx-core.c | 172 +++++++++++++++++++++++++
>  drivers/net/dsa/vitesse-vsc73xx.h      |   1 +
>  include/linux/dsa/vsc73xx.h            |  20 +++
>  net/dsa/tag_vsc73xx_8021q.c            |  73 +++++++++++
>  4 files changed, 266 insertions(+)
>  create mode 100644 include/linux/dsa/vsc73xx.h
> 
> diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
> index f18aa321053d..21ab3f214490 100644
> --- a/drivers/net/dsa/vitesse-vsc73xx-core.c
> +++ b/drivers/net/dsa/vitesse-vsc73xx-core.c
> @@ -73,6 +73,9 @@
>  #define VSC73XX_CAT_PR_USR_PRIO	0x75
>  #define VSC73XX_CAT_VLAN_MISC	0x79
>  #define VSC73XX_CAT_PORT_VLAN	0x7a
> +#define VSC73XX_CPUTXDAT	0xc0
> +#define VSC73XX_MISCFIFO	0xc4
> +#define VSC73XX_MISCSTAT	0xc8
>  #define VSC73XX_Q_MISC_CONF	0xdf
>  
>  /* MAC_CFG register bits */
> @@ -166,6 +169,14 @@
>  #define VSC73XX_CAT_PORT_VLAN_VLAN_USR_PRIO GENMASK(14, 12)
>  #define VSC73XX_CAT_PORT_VLAN_VLAN_VID GENMASK(11, 0)
>  
> +/* MISCFIFO Miscellaneous Control Register */
> +#define VSC73XX_MISCFIFO_REWIND_CPU_TX	BIT(1)
> +#define VSC73XX_MISCFIFO_CPU_TX		BIT(0)
> +
> +/* MISCSTAT Miscellaneous Status */
> +#define VSC73XX_MISCSTAT_CPU_TX_DATA_PENDING	BIT(8)
> +#define VSC73XX_MISCSTAT_CPU_TX_DATA_OVERFLOW	BIT(7)
> +
>  /* Frame analyzer block 2 registers */
>  #define VSC73XX_STORMLIMIT	0x02
>  #define VSC73XX_ADVLEARN	0x03
> @@ -363,6 +374,9 @@
>  #define VSC73XX_MDIO_POLL_SLEEP_US	5
>  #define VSC73XX_POLL_TIMEOUT_US		10000
>  
> +#define VSC73XX_IFH_MAGIC		0x52
> +#define VSC73XX_IFH_SIZE		8
> +
>  struct vsc73xx_counter {
>  	u8 counter;
>  	const char *name;
> @@ -375,6 +389,31 @@ struct vsc73xx_fdb {
>  	bool valid;
>  };
>  
> +/* Internal frame header structure */
> +struct vsc73xx_ifh {
> +	union {
> +		u32 datah;
> +		struct {
> +		u32 wt:1, /* Frame was tagged but tag has removed from frame */
> +		    : 1,
> +		    frame_length:14, /* Frame Length including CRC */
> +		    : 11,
> +		    port:5; /* SRC port of switch */

Please indent the struct field members.

> +		};
> +	};
> +	union {
> +		u32 datal;

Is the union with datah/datal actually useful in any way? Just a comment
about high word/low word should suffice?

Is there any field that crosses the word boundary? Or is the IFH nicely
arranged?

Does CPU endianness affect the correct bit layout?

> +		struct {
> +		u32 vid:16, /* VLAN ID */
> +		    : 3,
> +		    magic:9, /* IFH magic field */
> +		    lpa:1, /* SMAC is subject of learning */
> +		    : 1,
> +		    priority:2; /* Switch categorizer assigned priority */
> +		};
> +	};
> +};

__packed

> +
>  /* Counters are named according to the MIB standards where applicable.
>   * Some counters are custom, non-standard. The standard counters are
>   * named in accordance with RFC2819, RFC2021 and IEEE Std 802.3-2002 Annex
> @@ -683,6 +722,133 @@ static int vsc73xx_phy_write(struct dsa_switch *ds, int phy, int regnum,
>  	return 0;
>  }
>  
> +static int vsc73xx_tx_fifo_busy_check(struct vsc73xx *vsc, int port)
> +{
> +	int ret, err;
> +	u32 val;
> +
> +	ret = read_poll_timeout(vsc73xx_read, err,
> +				err < 0 ||
> +				!(val & VSC73XX_MISCSTAT_CPU_TX_DATA_PENDING),
> +				VSC73XX_POLL_SLEEP_US,
> +				VSC73XX_POLL_TIMEOUT_US, false, vsc,
> +				VSC73XX_BLOCK_MAC, port, VSC73XX_MISCSTAT,
> +				&val);
> +	if (ret)
> +		return ret;
> +	return err;
> +}
> +
> +static int
> +vsc73xx_write_tx_fifo(struct vsc73xx *vsc, int port, u32 data0, u32 data1)
> +{
> +	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CPUTXDAT, data0);
> +	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CPUTXDAT, data1);
> +
> +	return vsc73xx_tx_fifo_busy_check(vsc, port);
> +}
> +
> +static int
> +vsc73xx_inject_frame(struct vsc73xx *vsc, int port, struct sk_buff *skb)
> +{
> +	struct vsc73xx_ifh *ifh;
> +	u32 length, i, count;
> +	u32 *buf;
> +	int ret;
> +
> +	if (skb->len + VSC73XX_IFH_SIZE < 64)
> +		length = 64;
> +	else
> +		length = skb->len + VSC73XX_IFH_SIZE;

length = min_t(u32, 64, skb->len + VSC73XX_IFH_SIZE)?
Also, what does 64 represent? ETH_ZLEN + ?

> +
> +	count = DIV_ROUND_UP(length, 8);
> +	buf = kzalloc(count * 8, GFP_KERNEL);

this can return NULL

> +	memset(buf, 0, sizeof(buf));
> +
> +	ifh = (struct vsc73xx_ifh *)buf;
> +	ifh->frame_length = skb->len;
> +	ifh->magic = VSC73XX_IFH_MAGIC;
> +
> +	skb_copy_and_csum_dev(skb, (u8 *)(buf + 2));

Do you really _have_ to allocate dynamically a buffer and copy the skb
to it? Can't you write_tx_fifo() based on a pointer from skb->data, and
allocate the IFH as a separate on-stack structure?

For the checksum calculation, you could add the same logic as ocelot_defer_xmit():

	if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb))
		return NULL;

> +
> +	for (i = 0; i < count; i++) {
> +		ret = vsc73xx_write_tx_fifo(vsc, port, buf[2 * i],
> +					    buf[2 * i + 1]);
> +		if (ret) {
> +			/* Clear buffer after error */
> +			vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
> +					    VSC73XX_MISCFIFO,
> +					    VSC73XX_MISCFIFO_REWIND_CPU_TX,
> +					    VSC73XX_MISCFIFO_REWIND_CPU_TX);
> +			goto err;
> +		}
> +	}
> +
> +	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MISCFIFO,
> +		      VSC73XX_MISCFIFO_CPU_TX);
> +
> +	skb_tx_timestamp(skb);

When is the packet transmission actually started? At VSC73XX_MISCFIFO_CPU_TX
time? skb_tx_timestamp() should be done prior to that. PHY TX
timestamping is also hooked into this call, and will be completely
broken if it is racing with the packet transmission.

> +
> +	skb->dev->stats.tx_packets++;
> +	skb->dev->stats.tx_bytes += skb->len;
> +err:
> +	kfree(buf);
> +	return ret;
> +}
> +
> +#define work_to_xmit_work(w) \
> +		container_of((w), struct vsc73xx_deferred_xmit_work, work)
> +
> +static void vsc73xx_deferred_xmit(struct kthread_work *work)
> +{
> +	struct vsc73xx_deferred_xmit_work *xmit_work = work_to_xmit_work(work);
> +	struct dsa_switch *ds = xmit_work->dp->ds;
> +	struct sk_buff *skb = xmit_work->skb;
> +	int port = xmit_work->dp->index;
> +	struct vsc73xx *vsc = ds->priv;
> +	int ret;
> +
> +	if (vsc73xx_tx_fifo_busy_check(vsc, port)) {
> +		dev_err(vsc->dev, "port %d failed to inject skb\n",
> +			port);
> +
> +		/* Clear buffer after error */
> +		vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
> +				    VSC73XX_MISCFIFO,
> +				    VSC73XX_MISCFIFO_REWIND_CPU_TX,
> +				    VSC73XX_MISCFIFO_REWIND_CPU_TX);
> +
> +		kfree_skb(skb);
> +		return;
> +	}
> +
> +	ret = vsc73xx_inject_frame(vsc, port, skb);
> +

extraneous blank line

> +	if (ret) {
> +		dev_err(vsc->dev, "port %d failed to inject skb\n",

dev_err_ratelimited(... %pe, ERR_PTR(ret))?

> +			port);
> +		return;
> +	}

Is this hardware procedure completely reentrant (can it simultaneously
inject packets towards multiple ports) or should there be a spinlock
serializing the access?

> +
> +	consume_skb(skb);
> +	kfree(xmit_work);
> +}

Re: [PATCH net-next 1/3] net: dsa: vsc73xx: implement transmit via control interface

[Vadim Fedorenko]

On 20/10/2024 21:54, Pawel Dembicki wrote:
> Some types of packets can be forwarded only to and from the PI/SI
> interface. For more information, see Chapter 2.7.1 (CPU Forwarding) in
> the datasheet.
> 
> This patch implements the routines required for link-local transmission.
> This kind of traffic can't be transferred through the RGMII interface in
> vsc73xx.
> 
> It uses a method similar to the sja1005 driver, where the DSA tagger
> checks if the packet is link-local and uses a special deferred transmit
> route for that kind of packet.
> 
> The vsc73xx uses an "Internal Frame Header" (IFH) in communication via the
> PI/SI interface. Every packet must be prefixed with an IFH. The hardware
> fixes the checksums, so there's no need to calculate the FCS in the
> driver.
> 
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> ---
>   drivers/net/dsa/vitesse-vsc73xx-core.c | 172 +++++++++++++++++++++++++
>   drivers/net/dsa/vitesse-vsc73xx.h      |   1 +
>   include/linux/dsa/vsc73xx.h            |  20 +++
>   net/dsa/tag_vsc73xx_8021q.c            |  73 +++++++++++
>   4 files changed, 266 insertions(+)
>   create mode 100644 include/linux/dsa/vsc73xx.h
> 
> diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
> index f18aa321053d..21ab3f214490 100644
> --- a/drivers/net/dsa/vitesse-vsc73xx-core.c
> +++ b/drivers/net/dsa/vitesse-vsc73xx-core.c
> @@ -73,6 +73,9 @@
>   #define VSC73XX_CAT_PR_USR_PRIO	0x75
>   #define VSC73XX_CAT_VLAN_MISC	0x79
>   #define VSC73XX_CAT_PORT_VLAN	0x7a
> +#define VSC73XX_CPUTXDAT	0xc0
> +#define VSC73XX_MISCFIFO	0xc4
> +#define VSC73XX_MISCSTAT	0xc8
>   #define VSC73XX_Q_MISC_CONF	0xdf
>   
>   /* MAC_CFG register bits */
> @@ -166,6 +169,14 @@
>   #define VSC73XX_CAT_PORT_VLAN_VLAN_USR_PRIO GENMASK(14, 12)
>   #define VSC73XX_CAT_PORT_VLAN_VLAN_VID GENMASK(11, 0)
>   
> +/* MISCFIFO Miscellaneous Control Register */
> +#define VSC73XX_MISCFIFO_REWIND_CPU_TX	BIT(1)
> +#define VSC73XX_MISCFIFO_CPU_TX		BIT(0)
> +
> +/* MISCSTAT Miscellaneous Status */
> +#define VSC73XX_MISCSTAT_CPU_TX_DATA_PENDING	BIT(8)
> +#define VSC73XX_MISCSTAT_CPU_TX_DATA_OVERFLOW	BIT(7)
> +
>   /* Frame analyzer block 2 registers */
>   #define VSC73XX_STORMLIMIT	0x02
>   #define VSC73XX_ADVLEARN	0x03
> @@ -363,6 +374,9 @@
>   #define VSC73XX_MDIO_POLL_SLEEP_US	5
>   #define VSC73XX_POLL_TIMEOUT_US		10000
>   
> +#define VSC73XX_IFH_MAGIC		0x52
> +#define VSC73XX_IFH_SIZE		8
> +
>   struct vsc73xx_counter {
>   	u8 counter;
>   	const char *name;
> @@ -375,6 +389,31 @@ struct vsc73xx_fdb {
>   	bool valid;
>   };
>   
> +/* Internal frame header structure */
> +struct vsc73xx_ifh {
> +	union {
> +		u32 datah;
> +		struct {
> +		u32 wt:1, /* Frame was tagged but tag has removed from frame */
> +		    : 1,
> +		    frame_length:14, /* Frame Length including CRC */
> +		    : 11,
> +		    port:5; /* SRC port of switch */
> +		};
> +	};
> +	union {
> +		u32 datal;
> +		struct {
> +		u32 vid:16, /* VLAN ID */
> +		    : 3,
> +		    magic:9, /* IFH magic field */
> +		    lpa:1, /* SMAC is subject of learning */
> +		    : 1,
> +		    priority:2; /* Switch categorizer assigned priority */
> +		};
> +	};
> +};
> +
>   /* Counters are named according to the MIB standards where applicable.
>    * Some counters are custom, non-standard. The standard counters are
>    * named in accordance with RFC2819, RFC2021 and IEEE Std 802.3-2002 Annex
> @@ -683,6 +722,133 @@ static int vsc73xx_phy_write(struct dsa_switch *ds, int phy, int regnum,
>   	return 0;
>   }
>   
> +static int vsc73xx_tx_fifo_busy_check(struct vsc73xx *vsc, int port)
> +{
> +	int ret, err;
> +	u32 val;
> +
> +	ret = read_poll_timeout(vsc73xx_read, err,
> +				err < 0 ||
> +				!(val & VSC73XX_MISCSTAT_CPU_TX_DATA_PENDING),
> +				VSC73XX_POLL_SLEEP_US,
> +				VSC73XX_POLL_TIMEOUT_US, false, vsc,
> +				VSC73XX_BLOCK_MAC, port, VSC73XX_MISCSTAT,
> +				&val);
> +	if (ret)
> +		return ret;
> +	return err;
> +}
> +
> +static int
> +vsc73xx_write_tx_fifo(struct vsc73xx *vsc, int port, u32 data0, u32 data1)
> +{
> +	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CPUTXDAT, data0);
> +	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CPUTXDAT, data1);
> +
> +	return vsc73xx_tx_fifo_busy_check(vsc, port);
> +}
> +
> +static int
> +vsc73xx_inject_frame(struct vsc73xx *vsc, int port, struct sk_buff *skb)
> +{
> +	struct vsc73xx_ifh *ifh;
> +	u32 length, i, count;
> +	u32 *buf;
> +	int ret;
> +
> +	if (skb->len + VSC73XX_IFH_SIZE < 64)
> +		length = 64;
> +	else
> +		length = skb->len + VSC73XX_IFH_SIZE;

This looks like open-coded length = max(64, skb->len + VSC73XX_IFH_SIZE)

> +
> +	count = DIV_ROUND_UP(length, 8);
> +	buf = kzalloc(count * 8, GFP_KERNEL);
> +	memset(buf, 0, sizeof(buf));

no need to memset, kzalloc initializes memory to zero

> +
> +	ifh = (struct vsc73xx_ifh *)buf;
> +	ifh->frame_length = skb->len;
> +	ifh->magic = VSC73XX_IFH_MAGIC;
> +
> +	skb_copy_and_csum_dev(skb, (u8 *)(buf + 2));
> +
> +	for (i = 0; i < count; i++) {
> +		ret = vsc73xx_write_tx_fifo(vsc, port, buf[2 * i],
> +					    buf[2 * i + 1]);
> +		if (ret) {
> +			/* Clear buffer after error */
> +			vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
> +					    VSC73XX_MISCFIFO,
> +					    VSC73XX_MISCFIFO_REWIND_CPU_TX,
> +					    VSC73XX_MISCFIFO_REWIND_CPU_TX);
> +			goto err;
> +		}
> +	}
> +
> +	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MISCFIFO,
> +		      VSC73XX_MISCFIFO_CPU_TX);
> +
> +	skb_tx_timestamp(skb);
> +
> +	skb->dev->stats.tx_packets++;
> +	skb->dev->stats.tx_bytes += skb->len;
> +err:
> +	kfree(buf);
> +	return ret;
> +}
> +
> +#define work_to_xmit_work(w) \
> +		container_of((w), struct vsc73xx_deferred_xmit_work, work)
> +
> +static void vsc73xx_deferred_xmit(struct kthread_work *work)
> +{
> +	struct vsc73xx_deferred_xmit_work *xmit_work = work_to_xmit_work(work);
> +	struct dsa_switch *ds = xmit_work->dp->ds;
> +	struct sk_buff *skb = xmit_work->skb;
> +	int port = xmit_work->dp->index;
> +	struct vsc73xx *vsc = ds->priv;
> +	int ret;
> +
> +	if (vsc73xx_tx_fifo_busy_check(vsc, port)) {
> +		dev_err(vsc->dev, "port %d failed to inject skb\n",
> +			port);
> +
> +		/* Clear buffer after error */
> +		vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
> +				    VSC73XX_MISCFIFO,
> +				    VSC73XX_MISCFIFO_REWIND_CPU_TX,
> +				    VSC73XX_MISCFIFO_REWIND_CPU_TX);
> +
> +		kfree_skb(skb);
> +		return;
> +	}
> +
> +	ret = vsc73xx_inject_frame(vsc, port, skb);
> +
> +	if (ret) {
> +		dev_err(vsc->dev, "port %d failed to inject skb\n",
> +			port);
> +		return;
> +	}
> +
> +	consume_skb(skb);
> +	kfree(xmit_work);
> +}
> +
> +static int
> +vsc73xx_connect_tag_protocol(struct dsa_switch *ds, enum dsa_tag_protocol proto)
> +{
> +	struct vsc73xx_8021q_tagger_data *tagger_data;
> +
> +	switch (proto) {
> +	case DSA_TAG_PROTO_VSC73XX_8021Q:
> +		tagger_data = ds->tagger_data;
> +		tagger_data->xmit_work_fn = vsc73xx_deferred_xmit;
> +		return 0;
> +	default:
> +		return -EPROTONOSUPPORT;
> +	}
> +}
> +
>   static enum dsa_tag_protocol vsc73xx_get_tag_protocol(struct dsa_switch *ds,
>   						      int port,
>   						      enum dsa_tag_protocol mp)
> @@ -1026,6 +1192,11 @@ static void vsc73xx_init_port(struct vsc73xx *vsc, int port)
>   		      VSC73XX_CAT_DROP,
>   		      VSC73XX_CAT_DROP_FWD_PAUSE_ENA);
>   
> +	/* Allow switch to recalculate CRC of CPU packets */
> +	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_TXUPDCFG,
> +			    VSC73XX_TXUPDCFG_TX_UPDATE_CRC_CPU_ENA,
> +			    VSC73XX_TXUPDCFG_TX_UPDATE_CRC_CPU_ENA);
> +
>   	/* Clear all counters */
>   	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC,
>   		      port, VSC73XX_C_RX0, 0);
> @@ -2217,6 +2388,7 @@ static const struct phylink_mac_ops vsc73xx_phylink_mac_ops = {
>   
>   static const struct dsa_switch_ops vsc73xx_ds_ops = {
>   	.get_tag_protocol = vsc73xx_get_tag_protocol,
> +	.connect_tag_protocol = vsc73xx_connect_tag_protocol,
>   	.setup = vsc73xx_setup,
>   	.teardown = vsc73xx_teardown,
>   	.phy_read = vsc73xx_phy_read,
> diff --git a/drivers/net/dsa/vitesse-vsc73xx.h b/drivers/net/dsa/vitesse-vsc73xx.h
> index 3c30e143c14f..bf55a20f07f3 100644
> --- a/drivers/net/dsa/vitesse-vsc73xx.h
> +++ b/drivers/net/dsa/vitesse-vsc73xx.h
> @@ -2,6 +2,7 @@
>   #include <linux/device.h>
>   #include <linux/etherdevice.h>
>   #include <linux/gpio/driver.h>
> +#include <linux/dsa/vsc73xx.h>
>   
>   /* The VSC7395 switch chips have 5+1 ports which means 5 ordinary ports and
>    * a sixth CPU port facing the processor with an RGMII interface. These ports
> diff --git a/include/linux/dsa/vsc73xx.h b/include/linux/dsa/vsc73xx.h
> new file mode 100644
> index 000000000000..901eeb1da120
> --- /dev/null
> +++ b/include/linux/dsa/vsc73xx.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + * Copyright (c) 2024, Pawel Dembicki <paweldembicki@gmail.com>
> + */
> +
> +/* Included by drivers/net/dsa/vitesse-vsc73xx.h and net/dsa/tag_vsc73xx_8021q.c */
> +
> +#ifndef _NET_DSA_VSC73XX_H
> +#define _NET_DSA_VSC73XX_H
> +
> +struct vsc73xx_deferred_xmit_work {
> +	struct dsa_port *dp;
> +	struct sk_buff *skb;
> +	struct kthread_work work;
> +};
> +
> +struct vsc73xx_8021q_tagger_data {
> +	void (*xmit_work_fn)(struct kthread_work *work);
> +};
> +
> +#endif /* _NET_DSA_VSC73XX_H */
> diff --git a/net/dsa/tag_vsc73xx_8021q.c b/net/dsa/tag_vsc73xx_8021q.c
> index af121a9aff7f..d1d7a860a76e 100644
> --- a/net/dsa/tag_vsc73xx_8021q.c
> +++ b/net/dsa/tag_vsc73xx_8021q.c
> @@ -2,20 +2,61 @@
>   /* Copyright (C) 2024 Pawel Dembicki <paweldembicki@gmail.com>
>    */
>   #include <linux/dsa/8021q.h>
> +#include <linux/dsa/vsc73xx.h>
>   
>   #include "tag.h"
>   #include "tag_8021q.h"
>   
>   #define VSC73XX_8021Q_NAME "vsc73xx-8021q"
>   
> +struct vsc73xx_8021q_tagger_private {
> +	struct vsc73xx_8021q_tagger_data data; /* Must be first */
> +	struct kthread_worker *xmit_worker;
> +};
> +
> +static struct sk_buff *vsc73xx_defer_xmit(struct dsa_port *dp, struct sk_buff *skb)
> +{
> +	struct vsc73xx_8021q_tagger_private *priv = dp->ds->tagger_data;
> +	struct vsc73xx_8021q_tagger_data *data = &priv->data;
> +	void (*xmit_work_fn)(struct kthread_work *work);
> +	struct vsc73xx_deferred_xmit_work *xmit_work;
> +	struct kthread_worker *xmit_worker;
> +
> +	xmit_work_fn = data->xmit_work_fn;
> +	xmit_worker = priv->xmit_worker;
> +
> +	if (!xmit_work_fn || !xmit_worker)
> +		return NULL;
> +
> +	xmit_work = kzalloc(sizeof(*xmit_work), GFP_ATOMIC);
> +	if (!xmit_work)
> +		return NULL;
> +
> +	/* Calls vsc73xx_port_deferred_xmit in vitesse-vsc73xx-core.c */
> +	kthread_init_work(&xmit_work->work, xmit_work_fn);
> +	/* Increase refcount so the kfree_skb in dsa_slave_xmit
> +	 * won't really free the packet.
> +	 */
> +	xmit_work->dp = dp;
> +	xmit_work->skb = skb_get(skb);
> +
> +	kthread_queue_work(xmit_worker, &xmit_work->work);
> +
> +	return NULL;
> +}
> +
>   static struct sk_buff *
>   vsc73xx_xmit(struct sk_buff *skb, struct net_device *netdev)
>   {
>   	struct dsa_port *dp = dsa_user_to_port(netdev);
>   	u16 queue_mapping = skb_get_queue_mapping(skb);
>   	u16 tx_vid = dsa_tag_8021q_standalone_vid(dp);
> +	struct ethhdr *hdr = eth_hdr(skb);
>   	u8 pcp;
>   
> +	if (is_link_local_ether_addr(hdr->h_dest))
> +		return vsc73xx_defer_xmit(dp, skb);
> +
>   	if (skb->offload_fwd_mark) {
>   		unsigned int bridge_num = dsa_port_bridge_num_get(dp);
>   		struct net_device *br = dsa_port_bridge_dev_get(dp);
> @@ -52,11 +93,43 @@ vsc73xx_rcv(struct sk_buff *skb, struct net_device *netdev)
>   	return skb;
>   }
>   
> +static void vsc73xx_disconnect(struct dsa_switch *ds)
> +{
> +	struct vsc73xx_8021q_tagger_private *priv = ds->tagger_data;
> +
> +	kthread_destroy_worker(priv->xmit_worker);
> +	kfree(priv);
> +	ds->tagger_data = NULL;
> +}
> +
> +static int vsc73xx_connect(struct dsa_switch *ds)
> +{
> +	struct vsc73xx_8021q_tagger_private *priv;
> +	int err;
> +
> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->xmit_worker = kthread_create_worker(0, "vsc73xx_xmit");
> +	if (IS_ERR(priv->xmit_worker)) {
> +		err = PTR_ERR(priv->xmit_worker);
> +		kfree(priv);
> +		return err;
> +	}
> +
> +	ds->tagger_data = priv;
> +
> +	return 0;
> +}
> +
>   static const struct dsa_device_ops vsc73xx_8021q_netdev_ops = {
>   	.name			= VSC73XX_8021Q_NAME,
>   	.proto			= DSA_TAG_PROTO_VSC73XX_8021Q,
>   	.xmit			= vsc73xx_xmit,
>   	.rcv			= vsc73xx_rcv,
> +	.connect		= vsc73xx_connect,
> +	.disconnect		= vsc73xx_disconnect,
>   	.needed_headroom	= VLAN_HLEN,
>   	.promisc_on_conduit	= true,
>   };

Re: [PATCH net-next 1/3] net: dsa: vsc73xx: implement transmit via control interface

[kernel test robot]

Hi Pawel,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Pawel-Dembicki/net-dsa-vsc73xx-implement-packet-reception-via-control-interface/20241021-050041
base:   net-next/main
patch link:    https://lore.kernel.org/r/20241020205452.2660042-1-paweldembicki%40gmail.com
patch subject: [PATCH net-next 1/3] net: dsa: vsc73xx: implement transmit via control interface
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20241022/202410220836.TtUmbpFx-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241022/202410220836.TtUmbpFx-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410220836.TtUmbpFx-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/string.h:64,
                    from include/linux/bitmap.h:13,
                    from include/linux/cpumask.h:12,
                    from include/linux/smp.h:13,
                    from include/linux/lockdep.h:14,
                    from include/linux/spinlock.h:63,
                    from include/linux/mmzone.h:8,
                    from include/linux/gfp.h:7,
                    from include/linux/umh.h:4,
                    from include/linux/kmod.h:9,
                    from include/linux/module.h:17,
                    from drivers/net/dsa/vitesse-vsc73xx-core.c:18:
   drivers/net/dsa/vitesse-vsc73xx-core.c: In function 'vsc73xx_inject_frame':
>> drivers/net/dsa/vitesse-vsc73xx-core.c:766:30: warning: argument to 'sizeof' in '__builtin_memset' call is the same expression as the destination; did you mean to dereference it? [-Wsizeof-pointer-memaccess]
     766 |         memset(buf, 0, sizeof(buf));
         |                              ^
   arch/m68k/include/asm/string.h:49:48: note: in definition of macro 'memset'
      49 | #define memset(d, c, n) __builtin_memset(d, c, n)
         |                                                ^

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for GET_FREE_REGION
   Depends on [n]: SPARSEMEM [=n]
   Selected by [m]:
   - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]


vim +766 drivers/net/dsa/vitesse-vsc73xx-core.c

   750	
   751	static int
   752	vsc73xx_inject_frame(struct vsc73xx *vsc, int port, struct sk_buff *skb)
   753	{
   754		struct vsc73xx_ifh *ifh;
   755		u32 length, i, count;
   756		u32 *buf;
   757		int ret;
   758	
   759		if (skb->len + VSC73XX_IFH_SIZE < 64)
   760			length = 64;
   761		else
   762			length = skb->len + VSC73XX_IFH_SIZE;
   763	
   764		count = DIV_ROUND_UP(length, 8);
   765		buf = kzalloc(count * 8, GFP_KERNEL);
 > 766		memset(buf, 0, sizeof(buf));
   767	
   768		ifh = (struct vsc73xx_ifh *)buf;
   769		ifh->frame_length = skb->len;
   770		ifh->magic = VSC73XX_IFH_MAGIC;
   771	
   772		skb_copy_and_csum_dev(skb, (u8 *)(buf + 2));
   773	
   774		for (i = 0; i < count; i++) {
   775			ret = vsc73xx_write_tx_fifo(vsc, port, buf[2 * i],
   776						    buf[2 * i + 1]);
   777			if (ret) {
   778				/* Clear buffer after error */
   779				vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
   780						    VSC73XX_MISCFIFO,
   781						    VSC73XX_MISCFIFO_REWIND_CPU_TX,
   782						    VSC73XX_MISCFIFO_REWIND_CPU_TX);
   783				goto err;
   784			}
   785		}
   786	
   787		vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MISCFIFO,
   788			      VSC73XX_MISCFIFO_CPU_TX);
   789	
   790		skb_tx_timestamp(skb);
   791	
   792		skb->dev->stats.tx_packets++;
   793		skb->dev->stats.tx_bytes += skb->len;
   794	err:
   795		kfree(buf);
   796		return ret;
   797	}
   798	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next 2/3] net: dsa: vsc73xx: implement packet reception via control interface

[kernel test robot]

Hi Pawel,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Pawel-Dembicki/net-dsa-vsc73xx-implement-packet-reception-via-control-interface/20241021-050041
base:   net-next/main
patch link:    https://lore.kernel.org/r/20241020205452.2660042-2-paweldembicki%40gmail.com
patch subject: [PATCH net-next 2/3] net: dsa: vsc73xx: implement packet reception via control interface
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20241022/202410220908.uFiUPMGy-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241022/202410220908.uFiUPMGy-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410220908.uFiUPMGy-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/dsa/vitesse-vsc73xx-core.c:935:30: warning: variable 'len' is uninitialized when used here [-Wuninitialized]
     935 |         skb = netdev_alloc_skb(dev, len);
         |                                     ^~~
   drivers/net/dsa/vitesse-vsc73xx-core.c:885:23: note: initialize the variable 'len' to silence this warning
     885 |         int ret, buf_len, len, part;
         |                              ^
         |                               = 0
   1 warning generated.


vim +/len +935 drivers/net/dsa/vitesse-vsc73xx-core.c

   879	
   880	static void vsc73xx_polled_rcv(struct kthread_work *work)
   881	{
   882		struct vsc73xx *vsc = container_of(work, struct vsc73xx, dwork.work);
   883		u16 ptr = VSC73XX_CAPT_FRAME_DATA;
   884		struct dsa_switch *ds = vsc->ds;
   885		int ret, buf_len, len, part;
   886		struct vsc73xx_ifh ifh;
   887		struct net_device *dev;
   888		struct dsa_port *dp;
   889		struct sk_buff *skb;
   890		u32 val, *buf;
   891		u16 count;
   892	
   893		ret = vsc73xx_read(vsc, VSC73XX_BLOCK_SYSTEM, 0, VSC73XX_CAPCTRL, &val);
   894		if (ret)
   895			goto queue;
   896	
   897		if (!(val & VSC73XX_CAPCTRL_QUEUE0_READY))
   898			/* No frame to read */
   899			goto queue;
   900	
   901		/* Initialise reading */
   902		ret = vsc73xx_read(vsc, VSC73XX_BLOCK_CAPTURE, VSC73XX_BLOCK_CAPT_Q0,
   903				   VSC73XX_CAPT_CAPREADP, &val);
   904		if (ret)
   905			goto queue;
   906	
   907		/* Get internal frame header */
   908		ret = vsc73xx_read(vsc, VSC73XX_BLOCK_CAPTURE,
   909				   VSC73XX_BLOCK_CAPT_FRAME0, ptr++, &ifh.datah);
   910		if (ret)
   911			goto queue;
   912	
   913		ret = vsc73xx_read(vsc, VSC73XX_BLOCK_CAPTURE,
   914				   VSC73XX_BLOCK_CAPT_FRAME0, ptr++, &ifh.datal);
   915		if (ret)
   916			goto queue;
   917	
   918		if (ifh.magic != VSC73XX_IFH_MAGIC) {
   919			/* Something goes wrong with buffer. Reset capture block */
   920			vsc73xx_write(vsc, VSC73XX_BLOCK_CAPTURE,
   921				      VSC73XX_BLOCK_CAPT_RST, VSC73XX_CAPT_CAPRST, 1);
   922			goto queue;
   923		}
   924	
   925		if (!dsa_is_user_port(ds, ifh.port))
   926			goto release_frame;
   927	
   928		dp = dsa_to_port(ds, ifh.port);
   929		dev = dp->user;
   930		if (!dev)
   931			goto release_frame;
   932	
   933		count = (ifh.frame_length + 7 + VSC73XX_IFH_SIZE - ETH_FCS_LEN) >> 2;
   934	
 > 935		skb = netdev_alloc_skb(dev, len);
   936		if (unlikely(!skb)) {
   937			netdev_err(dev, "Unable to allocate sk_buff\n");
   938			goto release_frame;
   939		}
   940	
   941		buf_len = ifh.frame_length - ETH_FCS_LEN;
   942		buf = (u32 *)skb_put(skb, buf_len);
   943		len = 0;
   944		part = 0;
   945	
   946		while (ptr < count) {
   947			ret = vsc73xx_read(vsc, VSC73XX_BLOCK_CAPTURE,
   948					   VSC73XX_BLOCK_CAPT_FRAME0 + part, ptr++,
   949					   buf + len);
   950			if (ret)
   951				goto free_skb;
   952			len++;
   953			if (ptr > VSC73XX_CAPT_FRAME_DATA_MAX &&
   954			    count != VSC73XX_CAPT_FRAME_DATA_MAX) {
   955				ptr = VSC73XX_CAPT_FRAME_DATA;
   956				part++;
   957				count -= VSC73XX_CAPT_FRAME_DATA_MAX;
   958			}
   959		}
   960	
   961		/* Get FCS */
   962		ret = vsc73xx_read(vsc, VSC73XX_BLOCK_CAPTURE,
   963				   VSC73XX_BLOCK_CAPT_FRAME0, ptr++, &val);
   964		if (ret)
   965			goto free_skb;
   966	
   967		/* Everything we see on an interface that is in the HW bridge
   968		 * has already been forwarded.
   969		 */
   970		if (dp->bridge)
   971			skb->offload_fwd_mark = 1;
   972	
   973		skb->protocol = eth_type_trans(skb, dev);
   974	
   975		netif_rx(skb);
   976		goto release_frame;
   977	
   978	free_skb:
   979		kfree_skb(skb);
   980	release_frame:
   981		/* Release the frame from internal buffer */
   982		vsc73xx_write(vsc, VSC73XX_BLOCK_CAPTURE, VSC73XX_BLOCK_CAPT_Q0,
   983			      VSC73XX_CAPT_CAPREADP, 0);
   984	queue:
   985		kthread_queue_delayed_work(vsc->rcv_worker, &vsc->dwork,
   986					   msecs_to_jiffies(VSC73XX_RCV_POLL_INTERVAL));
   987	}
   988	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next 1/3] net: dsa: vsc73xx: implement transmit via control interface

[kernel test robot]

Hi Pawel,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Pawel-Dembicki/net-dsa-vsc73xx-implement-packet-reception-via-control-interface/20241021-050041
base:   net-next/main
patch link:    https://lore.kernel.org/r/20241020205452.2660042-1-paweldembicki%40gmail.com
patch subject: [PATCH net-next 1/3] net: dsa: vsc73xx: implement transmit via control interface
config: openrisc-allyesconfig (https://download.01.org/0day-ci/archive/20241022/202410221001.KrzTEU3A-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241022/202410221001.KrzTEU3A-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410221001.KrzTEU3A-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/net/dsa/vitesse-vsc73xx-core.c: In function 'vsc73xx_inject_frame':
>> drivers/net/dsa/vitesse-vsc73xx-core.c:766:30: warning: argument to 'sizeof' in 'memset' call is the same expression as the destination; did you mean to dereference it? [-Wsizeof-pointer-memaccess]
     766 |         memset(buf, 0, sizeof(buf));
         |                              ^

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for GET_FREE_REGION
   Depends on [n]: SPARSEMEM [=n]
   Selected by [y]:
   - RESOURCE_KUNIT_TEST [=y] && RUNTIME_TESTING_MENU [=y] && KUNIT [=y]


vim +766 drivers/net/dsa/vitesse-vsc73xx-core.c

   750	
   751	static int
   752	vsc73xx_inject_frame(struct vsc73xx *vsc, int port, struct sk_buff *skb)
   753	{
   754		struct vsc73xx_ifh *ifh;
   755		u32 length, i, count;
   756		u32 *buf;
   757		int ret;
   758	
   759		if (skb->len + VSC73XX_IFH_SIZE < 64)
   760			length = 64;
   761		else
   762			length = skb->len + VSC73XX_IFH_SIZE;
   763	
   764		count = DIV_ROUND_UP(length, 8);
   765		buf = kzalloc(count * 8, GFP_KERNEL);
 > 766		memset(buf, 0, sizeof(buf));
   767	
   768		ifh = (struct vsc73xx_ifh *)buf;
   769		ifh->frame_length = skb->len;
   770		ifh->magic = VSC73XX_IFH_MAGIC;
   771	
   772		skb_copy_and_csum_dev(skb, (u8 *)(buf + 2));
   773	
   774		for (i = 0; i < count; i++) {
   775			ret = vsc73xx_write_tx_fifo(vsc, port, buf[2 * i],
   776						    buf[2 * i + 1]);
   777			if (ret) {
   778				/* Clear buffer after error */
   779				vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,
   780						    VSC73XX_MISCFIFO,
   781						    VSC73XX_MISCFIFO_REWIND_CPU_TX,
   782						    VSC73XX_MISCFIFO_REWIND_CPU_TX);
   783				goto err;
   784			}
   785		}
   786	
   787		vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MISCFIFO,
   788			      VSC73XX_MISCFIFO_CPU_TX);
   789	
   790		skb_tx_timestamp(skb);
   791	
   792		skb->dev->stats.tx_packets++;
   793		skb->dev->stats.tx_bytes += skb->len;
   794	err:
   795		kfree(buf);
   796		return ret;
   797	}
   798	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki