Re: [PATCH net-next] net: dsa: mt7530: set CPU port to fallback mode

From: Florian Fainelli <f.fainelli@gmail.com>
To: DENG Qingfang <dqfext@gmail.com>, netdev@vger.kernel.org
Cc: "Andrew Lunn" <andrew@lunn.ch>,
	riddlariddla@hotmail.com, "Paul Fertser" <fercerpav@gmail.com>,
	"Sean Wang" <sean.wang@mediatek.com>,
	"Russell King" <linux@armlinux.org.uk>,
	"David S . Miller" <davem@davemloft.net>,
	"René van Dorst" <opensource@vdorst.com>,
	linux-mediatek@lists.infradead.org,
	"Stijn Segers" <foss@volatilesystems.org>,
	"Szabolcs Hubai" <szab.hu@gmail.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"Vivien Didelot" <vivien.didelot@gmail.com>,
	"Tom James" <tj17@me.com>
Subject: Re: [PATCH net-next] net: dsa: mt7530: set CPU port to fallback mode
Date: Wed, 13 May 2020 08:46:35 -0700	[thread overview]
Message-ID: <5d77da58-694a-7f9c-53fb-9d107e271d40@gmail.com> (raw)
In-Reply-To: <20200513153717.15599-1-dqfext@gmail.com>

On 5/13/2020 8:37 AM, DENG Qingfang wrote:
> Currently, setting a bridge's self PVID to other value and deleting
> the default VID 1 renders untagged ports of that VLAN unable to talk to
> the CPU port:
> 
> 	bridge vlan add dev br0 vid 2 pvid untagged self
> 	bridge vlan del dev br0 vid 1 self
> 	bridge vlan add dev sw0p0 vid 2 pvid untagged
> 	bridge vlan del dev sw0p0 vid 1
> 	# br0 cannot send untagged frames out of sw0p0 anymore
> 
> That is because the CPU port is set to security mode and its PVID is
> still 1, and untagged frames are dropped due to VLAN member violation.
> 
> Set the CPU port to fallback mode so untagged frames can pass through.

How about if the bridge has vlan_filtering=1? The use case you present
seems to be valid to me, that is, you may create a VLAN just for the
user ports and not have the CPU port be part of it at all.

> 
> Fixes: 83163f7dca56 ("net: dsa: mediatek: add VLAN support for MT7530")
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>
> ---
>  drivers/net/dsa/mt7530.c | 11 ++++++++---
>  drivers/net/dsa/mt7530.h |  6 ++++++
>  2 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 5c444cd722bd..a063d914c23f 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -810,10 +810,15 @@ mt7530_port_set_vlan_aware(struct dsa_switch *ds, int port)
>  		   PCR_MATRIX_MASK, PCR_MATRIX(MT7530_ALL_MEMBERS));
>  
>  	/* Trapped into security mode allows packet forwarding through VLAN
> -	 * table lookup.
> +	 * table lookup. CPU port is set to fallback mode to let untagged
> +	 * frames pass through.
>  	 */
> -	mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
> -		   MT7530_PORT_SECURITY_MODE);
> +	if (dsa_is_cpu_port(ds, port))
> +		mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
> +			   MT7530_PORT_FALLBACK_MODE);
> +	else
> +		mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
> +			   MT7530_PORT_SECURITY_MODE);
>  
>  	/* Set the port as a user port which is to be able to recognize VID
>  	 * from incoming packets before fetching entry within the VLAN table.
> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> index 979bb6374678..d45eb7540703 100644
> --- a/drivers/net/dsa/mt7530.h
> +++ b/drivers/net/dsa/mt7530.h
> @@ -152,6 +152,12 @@ enum mt7530_port_mode {
>  	/* Port Matrix Mode: Frames are forwarded by the PCR_MATRIX members. */
>  	MT7530_PORT_MATRIX_MODE = PORT_VLAN(0),
>  
> +	/* Fallback Mode: Forward received frames with ingress ports that do
> +	 * not belong to the VLAN member. Frames whose VID is not listed on
> +	 * the VLAN table are forwarded by the PCR_MATRIX members.
> +	 */
> +	MT7530_PORT_FALLBACK_MODE = PORT_VLAN(1),
> +
>  	/* Security Mode: Discard any frame due to ingress membership
>  	 * violation or VID missed on the VLAN table.
>  	 */
> 

-- 
Florian

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek