Re: [PATCH net-next 3/6] net: dsa: vsc73xx: Add dsa tagging based on 8021q

Simon Horman
2023-06-22T07:41:57Z

On Wed, Jun 21, 2023 at 09:12:59PM +0200, Pawel Dembicki wrote:
> This patch is simple implementation of 8021q tagging in vsc73xx driver.
> At this moment devices with DSA_TAG_PROTO_NONE are useless. VSC73XX
> family doesn't provide any tag support for external ethernet ports.
>
> The only way is vlan-based tagging. It require constant hardware vlan
> filtering. VSC73XX family support provider bridging but QinQ only without
> fully implemented 802.1AD. It allow only doubled 0x8100 TPID.
>
> In simple port mode QinQ is enabled to preserve forwarding vlan tagged
> frames.
>
> Tag driver introduce most simple funcionality required for proper taging
> support.
>
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>

Hi Pawel,

a few minor nits to consider if you end up respining this series.

> diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
> index 3ed5391bb18d..4cf0166fef7b 100644
> --- a/drivers/net/dsa/Kconfig
> +++ b/drivers/net/dsa/Kconfig
> @@ -125,7 +125,7 @@ config NET_DSA_SMSC_LAN9303_MDIO
>
> config NET_DSA_VITESSE_VSC73XX
> tristate
> - select NET_DSA_TAG_NONE
> + select NET_DSA_TAG_VSC73XX
> select FIXED_PHY
> select VITESSE_PHY
> select GPIOLIB
> diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c

...

> +static int vsc73xx_vlan_set_pvid(struct dsa_switch *ds, int port, u16 vid,
> + bool port_vlan)
> +{
> + struct vsc73xx *vsc = ds->priv;
> + struct dsa_port *dsa_port = dsa_to_port(ds, port);
> + u16 vlan_no;
> + u32 val;

nit: Please use reverse xmas tree - longest line to shortest,
for local variable declarations in new Networking code.

Also, once space is enough after '='.

...

> diff --git a/net/dsa/tag_vsc73xx_8021q.c b/net/dsa/tag_vsc73xx_8021q.c
> new file mode 100644
> index 000000000000..3d83dfecde31
> --- /dev/null
> +++ b/net/dsa/tag_vsc73xx_8021q.c
> @@ -0,0 +1,87 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/* Copyright (C) 2022 Pawel Dembicki <paweldembicki@gmail.com>
> + * Based on tag_sja1105.c:
> + * Copyright (c) 2019, Vladimir Oltean <olteanv@gmail.com>
> + */
> +#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_slave_to_port(netdev);
> + u16 queue_mapping = skb_get_queue_mapping(skb);
> + u8 pcp = netdev_txq_to_tc(netdev, queue_mapping);
> + u16 tx_vid = dsa_tag_8021q_standalone_vid(dp);
> + struct net_device *br = dsa_port_bridge_dev_get(dp);

nit: reverse xmas tree here too.

...