Message-ID: <20200317212453.GV25745@shell.armlinux.org.uk>
Date:   Tue, 17 Mar 2020 21:24:53 +0000
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Vivien Didelot <vivien.didelot@...il.com>
Cc:     Vladimir Oltean <olteanv@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Ido Schimmel <idosch@...sch.org>,
        "David S. Miller" <davem@...emloft.net>,
        Ivan Vecera <ivecera@...hat.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Jiri Pirko <jiri@...nulli.us>, netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

On Tue, Mar 17, 2020 at 02:49:06PM -0400, Vivien Didelot wrote:
> Hi Russell,
> 
> On Tue, 17 Mar 2020 15:12:38 +0000, Russell King - ARM Linux admin <linux@...linux.org.uk> wrote:
> > On Tue, Mar 17, 2020 at 04:21:00PM +0200, Vladimir Oltean wrote:
> > > Hi Russell,
> > > 
> > > On Tue, 17 Mar 2020 at 14:00, Russell King - ARM Linux admin
> > > <linux@...linux.org.uk> wrote:
> > > >
> > > > On Mon, Mar 16, 2020 at 11:15:24AM +0000, Russell King - ARM Linux admin wrote:
> > > > > On Fri, Feb 21, 2020 at 12:21:10AM +0000, Russell King - ARM Linux admin wrote:
> > > > > > On Thu, Feb 20, 2020 at 10:56:17AM -0800, Florian Fainelli wrote:
> > > > > > > Let's get your patch series merged. If you re-spin while addressing
> > > > > > > Vivien's comment not to use the term "vtu", I think I would be fine with
> > > > > > > the current approach of having to go after each driver and enabling them
> > > > > > > where necessary.
> > > > > >
> > > > > > The question then becomes what to call it.  "always_allow_vlans" or
> > > > > > "always_allow_vlan_config" maybe?
> > > > >
> > > > > Please note that I still have this patch pending (i.o.w., the problem
> > > > > with vlans remains unfixed) as I haven't received a reply to this,
> > > > > although the first two patches have been merged.
> > > >
> > > > Okay, I think three and a half weeks is way beyond a reasonable time
> > > > period to expect any kind of reply.
> > > >
> > > > Since no one seems to have any idea what to name this, but can only
> > > > offer "we don't like the vtu" term, it's difficult to see what would
> > > > actually be acceptable.  So, I propose that we go with the existing
> > > > naming.
> > > >
> > > > If you only know what you don't want, but not what you want, and aren't
> > > > even willing to discuss it, it makes it very much impossible to
> > > > progress.
> > > >
> > > > --
> > > > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > > > FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
> > > 
> > > As I said, I know why I need this blocking of bridge vlan
> > > configuration for sja1105, but not more. For sja1105 in particular, I
> > > fully admit that the hardware is quirky, but I can work around that
> > > within the driver. The concern is for the other drivers where we don't
> > > really "remember" why this workaround is in place. I think, while your
> > > patch is definitely a punctual fix for one case that doesn't need the
> > > workaround, it might be better for maintenance to just see exactly
> > > what breaks, instead of introducing this opaque property.
> > > While I don't want to speak on behalf of the maintainers, I think that
> > > may be at least part of the reason why there is little progress being
> > > made. Introducing some breakage which is going to be fixed better next
> > > time might be the more appropriate thing to do.
> > 
> > The conclusion on 21st February was that all patches should be merged,
> > complete with the boolean control, but there was an open question about
> > the name of the boolean used to enable this behaviour.
> > 
> > That question has not been resolved, so I'm trying to re-open discussion
> > of that point.  I've re-posted the patch.
> > 
> > -- 
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
> 
> In response to your 3/3 patch, I suggested commands to test setting up a
> VLAN filtering aware bridge with your own default PVID before enslaving
> DSA ports. Unfortunately you left this unanswered.

I don't believe I left it unanswered.  However, I'm not about to rip
apart my network to try an experiment with specific set of commands.
I did, however, experiment a lot to work out what was going on, so I
already have the knowledge.

I believe I explained in the series description that the problem only
happens when vlan filtering is enabled with a pre-existing vlan
configuration present, even the default configuration.

Enabling vlan filtering *immediately* blocks all traffic on the Marvell
switch, whether it has vlan tags or not.  Any *new* vlan modifications
then get entered into the VTU, and the Marvell switch then behaves
accordingly with those new entries as one would expect.

Any setup done *before* vlan filtering is enabled is not present in the
VTU, and so remains non-existent as far as the DSA switch is concerned.

As soon as vlan filtering is enabled, the ports are switched to "802.1Q
secure" state, which means:

- The switch will consult the VTU for the incoming packet; if no entry
  is found for the vlan number either tagged to the packet, or the
  default vlan if not, then the packet will be discarded by the switch.

So, the setup done *before* vlan filtering is enabled, which is not
programmed into the VTU, results in that traffic being lost.

> I think this would be
> much more interesting in order to tackle the issue you're having here with
> mv88e6xxx and bridge, instead of pointing out the lack of response regarding
> an alternative boolean name.

It is my understanding that Florian actively wants this merged.  No
one objected to his email.

It seems there's a disconnect *between* the DSA maintainers - I think
you need to be more effectively communicating with each other and
reading each other's emails, and pro-actively replying to stuff you
may have other views on.

> That being said, "force_vlan_programming",
> "always_allow_vlans", or whatever explicit non-"vtu" term is fine by me.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up

Powered by blists - more mailing lists