Re: [PATCH net-next v4 12/15] net: dsa: vsc73xx: introduce tag 8021q for vsc73xx

[Vladimir Oltean <olteanv@gmail.com>]

On Tue, Feb 13, 2024 at 11:03:25PM +0100, Pawel Dembicki wrote:
> This commit introduces a new tagger based on 802.1q tagging.
> It's designed for the vsc73xx driver. The VSC73xx family doesn't have
> any tag support for the RGMII port, but it could be based on VLANs.
> 
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> ---
> v4:
>   - rebase to net-next/main
> v3:
>   - Introduce a patch after the tagging patch split
> 
>  include/net/dsa.h           |  2 ++
>  net/dsa/Kconfig             |  6 ++++
>  net/dsa/Makefile            |  1 +
>  net/dsa/tag_vsc73xx_8021q.c | 69 +++++++++++++++++++++++++++++++++++++
>  4 files changed, 78 insertions(+)
>  create mode 100644 net/dsa/tag_vsc73xx_8021q.c
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 7c0da9effe4e..b79e136e4c41 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -56,6 +56,7 @@ struct phylink_link_state;
>  #define DSA_TAG_PROTO_RTL8_4T_VALUE		25
>  #define DSA_TAG_PROTO_RZN1_A5PSW_VALUE		26
>  #define DSA_TAG_PROTO_LAN937X_VALUE		27
> +#define DSA_TAG_PROTO_VSC73XX_8021Q_VALUE	28
>  
>  enum dsa_tag_protocol {
>  	DSA_TAG_PROTO_NONE		= DSA_TAG_PROTO_NONE_VALUE,
> @@ -86,6 +87,7 @@ enum dsa_tag_protocol {
>  	DSA_TAG_PROTO_RTL8_4T		= DSA_TAG_PROTO_RTL8_4T_VALUE,
>  	DSA_TAG_PROTO_RZN1_A5PSW	= DSA_TAG_PROTO_RZN1_A5PSW_VALUE,
>  	DSA_TAG_PROTO_LAN937X		= DSA_TAG_PROTO_LAN937X_VALUE,
> +	DSA_TAG_PROTO_VSC73XX_8021Q	= DSA_TAG_PROTO_VSC73XX_8021Q_VALUE,
>  };
>  
>  struct dsa_switch;
> diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
> index 8e698bea99a3..e59360071c67 100644
> --- a/net/dsa/Kconfig
> +++ b/net/dsa/Kconfig
> @@ -166,6 +166,12 @@ config NET_DSA_TAG_TRAILER
>  	  Say Y or M if you want to enable support for tagging frames at
>  	  with a trailed. e.g. Marvell 88E6060.
>  
> +config NET_DSA_TAG_VSC73XX_8021Q
> +	tristate "Tag driver for Microchip/Vitesse VSC73xx family of switches, using VLAN"
> +	help
> +	  Say Y or M if you want to enable support for tagging frames with a
> +	  custom VLAN-based header.
> +
>  config NET_DSA_TAG_XRS700X
>  	tristate "Tag driver for XRS700x switches"
>  	help
> diff --git a/net/dsa/Makefile b/net/dsa/Makefile
> index 8a1894a42552..555c07cfeb71 100644
> --- a/net/dsa/Makefile
> +++ b/net/dsa/Makefile
> @@ -37,6 +37,7 @@ obj-$(CONFIG_NET_DSA_TAG_RTL8_4) += tag_rtl8_4.o
>  obj-$(CONFIG_NET_DSA_TAG_RZN1_A5PSW) += tag_rzn1_a5psw.o
>  obj-$(CONFIG_NET_DSA_TAG_SJA1105) += tag_sja1105.o
>  obj-$(CONFIG_NET_DSA_TAG_TRAILER) += tag_trailer.o
> +obj-$(CONFIG_NET_DSA_TAG_VSC73XX_8021Q) += tag_vsc73xx_8021q.o
>  obj-$(CONFIG_NET_DSA_TAG_XRS700X) += tag_xrs700x.o
>  
>  # for tracing framework to find trace.h
> diff --git a/net/dsa/tag_vsc73xx_8021q.c b/net/dsa/tag_vsc73xx_8021q.c
> new file mode 100644
> index 000000000000..0bf150a10576
> --- /dev/null
> +++ b/net/dsa/tag_vsc73xx_8021q.c
> @@ -0,0 +1,69 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/* Copyright (C) 2023 Pawel Dembicki <paweldembicki@gmail.com>

It's 2024 already :)

> + */
> +#include <linux/dsa/8021q.h>
> +
> +#include "tag.h"
> +#include "tag_8021q.h"
> +
> +#define VSC73XX_8021Q_NAME "vsc73xx-8021q"
> +
> +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);
> +	u8 pcp;
> +
> +	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);
> +
> +		if (br_vlan_enabled(br))
> +			return skb;
> +
> +		tx_vid = dsa_tag_8021q_bridge_vid(bridge_num);
> +	}
> +
> +	pcp = netdev_txq_to_tc(netdev, queue_mapping);
> +
> +	return dsa_8021q_xmit(skb, netdev, ETH_P_8021Q,
> +			      ((pcp << VLAN_PRIO_SHIFT) | tx_vid));
> +}
> +
> +static struct sk_buff *vsc73xx_rcv(struct sk_buff *skb, struct net_device *netdev)
> +{
> +	int src_port = -1, switch_id = -1, vbid = -1, vid = -1;
> +
> +	if (skb_vlan_tag_present(skb)) {
> +		/* Normal traffic path. */
> +		dsa_8021q_rcv(skb, &src_port, &switch_id, &vbid, &vid);

dsa_8021q_rcv() also works with VLAN tags in the skb head, not in the
hwaccel area. So please remove "if (skb_vlan_tag_present())" and the
"else" clause, and let dsa_tag_8021q_find_user() below fail if it will.

> +	} else {
> +		netdev_warn(netdev, "Couldn't decode source port\n");
> +		return NULL;
> +	}
> +
> +	skb->dev = dsa_tag_8021q_find_user(netdev, src_port, switch_id, vid, vbid);
> +	if (!skb->dev) {
> +		netdev_warn(netdev, "Couldn't decode source port\n");
> +		return NULL;
> +	}
> +
> +	dsa_default_offload_fwd_mark(skb);
> +
> +	return skb;
> +}
> +
> +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,
> +	.needed_headroom	= VLAN_HLEN,
> +	.promisc_on_conduit	= true,
> +};
> +
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_VSC73XX_8021Q, VSC73XX_8021Q_NAME);
> +
> +module_dsa_tag_driver(vsc73xx_8021q_netdev_ops);
> -- 
> 2.34.1
>