Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Russell King - ARM Linux admin <linux@armlinux.org.uk>]

On Wed, Feb 19, 2020 at 04:47:30AM +0100, Andrew Lunn wrote:
> On Wed, Feb 19, 2020 at 12:17:37AM +0000, Russell King - ARM Linux admin wrote:
> > On Tue, Feb 18, 2020 at 04:00:08PM -0800, Florian Fainelli wrote:
> > > On 2/18/20 3:45 AM, Russell King - ARM Linux admin wrote:
> > > > Hi,
> > > > 
> > > > This is a repost of the previously posted RFC back in December, which
> > > > did not get fully reviewed.  I've dropped the RFC tag this time as no
> > > > one really found anything too problematical in the RFC posting.
> > > > 
> > > > I've been trying to configure DSA for VLANs and not having much success.
> > > > The setup is quite simple:
> > > > 
> > > > - The main network is untagged
> > > > - The wifi network is a vlan tagged with id $VN running over the main
> > > >   network.
> > > > 
> > > > I have an Armada 388 Clearfog with a PCIe wifi card which I'm trying to
> > > > setup to provide wifi access to the vlan $VN network, while the switch
> > > > is also part of the main network.
> > > 
> > > Why not just revert 2ea7a679ca2abd251c1ec03f20508619707e1749 ("net: dsa:
> > > Don't add vlans when vlan filtering is disabled")? If a driver wants to
> > > veto the programming of VLANs while it has ports enslaved to a bridge
> > > that does not have VLAN filtering, it should have enough information to
> > > not do that operation.
> > 
> > I do not have the knowledge to know whether reverting that commit
> > would be appropriate; I do not know how the non-Marvell switches will
> > behave with such a revert - what was the reason for the commit in
> > the first place?
> > 
> > The commit says:
> > 
> >     This fixes at least one corner case. There are still issues in other
> >     corners, such as when vlan_filtering is later enabled.
> > 
> > but it doesn't say what that corner case was.  So, presumably reverting
> > it will cause a regression of whatever that corner case was...
> 
> Yes, sorry, bad commit message. I'm not too sure, but it could of been
> that the switch was adding the VLANs to its tables, even though it
> should not because filtering is disabled. And i also think the default
> VLAN was not defined at that point, it only gets defined when
> vlan_filtering is enabled?

It's been too long since I researched all these details, but I seem
to remember that in the Linux software bridge, vlan 1 is always
present even when vlan filtering is not enabled.

Looking at br_vlan_init():

        br->default_pvid = 1;

and nbp_vlan_init() propagates that irrespective of the bridge vlan
enable state to switchdev.  nbp_vlan_init() is called whenever any
interface is added to a bridge (in br_add_if()).

As I believe I mentioned somewhere in the commit messages or covering
message, for at least some of the Marvell DSA switches, it is safe to
add VTU entries - they do not even look at the VTU when the port has
802.1Q disabled.  Whether that is true for all Marvell's DSA switches
I don't know without trawling every functional spec, and I was hoping
that you guys would know.  I guess I need to trawl the specs.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up