Message ID | 20241020205452.2660042-1-paweldembicki@gmail.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | [net-next,1/3] net: dsa: vsc73xx: implement transmit via control interface | expand |
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); > +}
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, > };
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
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
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, };
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