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

From: Florian Fainelli <f.fainelli@gmail.com>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: "Andrew Lunn" <andrew@lunn.ch>,
	"René van Dorst" <opensource@vdorst.com>,
	"Paul Fertser" <fercerpav@gmail.com>,
	netdev <netdev@vger.kernel.org>,
	"Sean Wang" <sean.wang@mediatek.com>,
	"Russell King" <linux@armlinux.org.uk>,
	"Vivien Didelot" <vivien.didelot@gmail.com>,
	"DENG Qingfang" <dqfext@gmail.com>,
	"moderated list:ARM/Mediatek SoC support"
	<linux-mediatek@lists.infradead.org>,
	"Stijn Segers" <foss@volatilesystems.org>,
	"Szabolcs Hubai" <szab.hu@gmail.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	riddlariddla@hotmail.com,
	"David S . Miller" <davem@davemloft.net>,
	"Tom James" <tj17@me.com>
Subject: Re: [PATCH net-next] net: dsa: mt7530: set CPU port to fallback mode
Date: Wed, 13 May 2020 09:22:04 -0700	[thread overview]
Message-ID: <280bce1d-a95d-b2fd-6b46-0e3f46508292@gmail.com> (raw)
In-Reply-To: <CA+h21hr_TyWQyvGukXqS0SocmvOBWUp6keghuhZh6HSaxAGb8A@mail.gmail.com>

On 5/13/2020 9:17 AM, Vladimir Oltean wrote:
> On Wed, 13 May 2020 at 18:49, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>>
>>
>> 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.
>>
> 
> What Qingfang is doing is in effect (but not by intention) removing
> the front panel port sw0p0 from the membership list of the CPU port's
> pvid. What you seem to be thinking of (VLAN of which the CPU is not a
> member of) does not seem to be supported in DSA at the moment.

Indeed and I replied to Qingfang already that I had misunderstood his
patch originally.

> 
> As a fix, there's nothing wrong with the patch actually, I don't even
> know how it would work otherwise. DSA doesn't change the pvid of the
> CPU port when the pvid of a slave changes, because 4 slave ports could
> have 4 different pvids and the CPU port pvid would keep changing.
> Fallback mode should only apply on ingress from CPU, so there's no
> danger really.

Agreed.

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

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