Re: [RFC PATCH net-next] net: dsa: tag_qca: Check for upstream VLAN tag

[Matthew Hagan <mnhagan88@gmail.com>]

On 06/06/2021 01:53, Vladimir Oltean wrote:

> It is a bit unconventional for the upstream Broadcom switch, which is a
> DSA master of its own, to insert a VLAN ID of zero out of the blue,
> especially if it operates in standalone mode. Supposedly sw0 and sw1 are
> not under a bridge net device, are they?

sw0 and sw1 are brought up but otherwise left unconfigured. The bridge
consists of the user ports only (wanN and swNpN). A side note here is that
your "net: dsa: don't set skb->offload_fwd_mark when not offloading the
bridge" patch is also in use. Would setting up a bridge for sw0/sw1 not
have implications for receiving unknown frames on one port, that have been
sent from another port of the same switch? Since unknown frames will go to
the CPU, dp->bridge_dev would return the bridge name, setting
offload_fwd_mark=1 thus preventing those frames being sent back out
sw0/sw1 to its other ports.

>
> If I'm not mistaken, this patch should solve your problem?
>
> -----------------------------[ cut here ]-----------------------------
> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> index 3ca6b394dd5f..d6655b516bd8 100644
> --- a/drivers/net/dsa/b53/b53_common.c
> +++ b/drivers/net/dsa/b53/b53_common.c
> @@ -1462,6 +1462,7 @@ int b53_vlan_add(struct dsa_switch *ds, int port,
>  	struct b53_device *dev = ds->priv;
>  	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
>  	bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
> +	bool really_untagged = false;
>  	struct b53_vlan *vl;
>  	int err;
>  
> @@ -1474,10 +1475,10 @@ int b53_vlan_add(struct dsa_switch *ds, int port,
>  	b53_get_vlan_entry(dev, vlan->vid, vl);
>  
>  	if (vlan->vid == 0 && vlan->vid == b53_default_pvid(dev))
> -		untagged = true;
> +		really_untagged = true;
>  
>  	vl->members |= BIT(port);
> -	if (untagged && !dsa_is_cpu_port(ds, port))
> +	if (really_untagged || (untagged && !dsa_is_cpu_port(ds, port)))
>  		vl->untag |= BIT(port);
>  	else
>  		vl->untag &= ~BIT(port);
> -----------------------------[ cut here ]-----------------------------
>
This does seem to sort the issue as well in this case. Thanks!

Matthew