Message-ID: <CA+h21hqMRG0Ai+KBsft_0uKDSBr4FQaO+tM+sNtm32_ekwdziA@mail.gmail.com>
Date:   Tue, 20 Aug 2019 13:18:15 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Vivien Didelot <vivien.didelot@...il.com>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>, Ido Schimmel <idosch@...sch.org>,
        Roopa Prabhu <roopa@...ulusnetworks.com>,
        nikolay@...ulusnetworks.com,
        "David S. Miller" <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 0/6] Dynamic toggling of vlan_filtering for
 SJA1105 DSA

On Tue, 20 Aug 2019 at 11:04, Vivien Didelot <vivien.didelot@...il.com> wrote:
>
> On Tue, 20 Aug 2019 02:59:56 +0300, Vladimir Oltean <olteanv@...il.com> wrote:
> > This patchset addresses a few limitations in DSA and the bridge core
> > that made it impossible for this sequence of commands to work:
> >
> >   ip link add name br0 type bridge
> >   ip link set dev swp2 master br0
> >   echo 1 > /sys/class/net/br0/bridge/vlan_filtering
> >
> > Only this sequence was previously working:
> >
> >   ip link add name br0 type bridge vlan_filtering 1
> >   ip link set dev swp2 master br0
>
> This is not quite true, these sequences of commands do "work". What I see
> though is that with the first sequence, the PVID 1 won't be programmed in
> the hardware. But the second sequence does program the hardware.
>
> But following bridge members will be correctly programmed with the VLAN
> though. The sequence below programs the hardware with VLAN 1 for swp3 as
> well as CPU and DSA ports, but not for swp2:
>
>     ip link add name br0 type bridge
>     ip link set dev swp2 master br0
>     echo 1 > /sys/class/net/br0/bridge/vlan_filtering
>     ip link set dev swp3 master br0
>
> This is unfortunately also true for any 802.1Q VLANs. For example, only VID
> 43 is programmed with the following sequence, but not VID 1 and VID 42:
>
>     ip link add name br0 type bridge
>     ip link set dev swp2 master br0
>     bridge vlan add dev swp2 vid 42
>     echo 1 > /sys/class/net/br0/bridge/vlan_filtering
>     bridge vlan add dev swp2 vid 43
>
> So I understand that because VLANs are not propagated by DSA to the hardware
> when VLAN filtering is disabled, a port may not be programmed with its
> bridge's default PVID, and this is causing a problem for tag_8021q.
>
> Please reword so that we understand better what is the issue being fixed here.
>

Not exactly, no.
The fact that a port in DSA is not programmed with the bridge's
default_pvid is just a fact.
Right now, letting the bridge install the default_pvid into the ports
even breaks dsa_8021q when enslaving the ports to a vlan_filtering=0
bridge, but that is perhaps only a timing issue and can be resolved in
other ways.

To understand my issue I need to make a side-note.
The sja1105 needs to be reset at runtime for changing some settings. I
am working on a separate patchset that tries to make those switch
resets as seamless as possible. The end goal is to not disrupt
linuxptp (ptp4l, phc2sys) operation when enabling the tc-taprio
offload. That means:
- saving and restoring the PTP time after reset
- preventing switch reset to happen during TX timestamping
- preventing any clock operation on the PTP timer during switch reset
Since toggling vlan_filtering also needs to reset the switch, I am
simply using that as a handy tool to test how seamless the switch
reset is.
But currently, toggling vlan_filtering breaks traffic, due to nobody
restoring the correct (from the bridge's perspective) pvid on the
ports. So I need to fix this to continue the testing.
On the user ports, I can just query the bridge, and that's exactly
what I'm doing.
On the CPU and DSA ports, I can't query the bridge because the bridge
knows nothing about them. And I also can't query the pvid from the DSA
core - it can change it, but not know what it is. So I need to be
proactive and make DSA avoid changing their pvid needlessly.
With this set of changes, the goal was for traffic to 'just work' when
changing the vlan_filtering setting back and forth. This would also
benefit other future users of dsa_8021q.
There is still work to be done. dsa_8021q uses VID range 1024-2559
privately. If the bridge had been using any VLAN in that range during
vlan_filtering=1, then dsa_8021q should also restore that VLAN.
Currently I'm not doing that because I'm still not clear whether it's
just as simple as calling br_vlan_get_info(slave, {rx,tx}_vid,
&vinfo); and adding that back (I don't completely understand the
interaction with the implicit rules on the CPU ports - I am concerned
that if the switch driver sees the VID is already installed on the CPU
port, it will just skip the command).
Ido Schimmel mentioned that static FDB entries with VLAN != PVID need
to be preserved. I am already doing something in that direction there
- in vlan_filtering=0 mode I am switching to shared VLAN learning
mode. This means the switch is ignoring the VLAN from the FDB entries,
and just uses the DMAC for L2 routing. When going back to the
vlan_filtering=1 mode, the VLAN from the static FDB entries goes back
in use. I think that's ok.
There is also stuff that cannot be reasonably expected to be preserved
across switch resets. Learned FDB entries is one thing. Traffic
(including regular, but not management, traffic originated from the
CPU) will be temporarily dropped.

> >
> > On SJA1105, the situation is further complicated by the fact that
> > toggling vlan_filtering is causing a switch reset. However, the hardware
> > state restoration logic is already there in the driver. It is a matter
> > of the layers above which need a few fixups.
> >
> > Also see this discussion thread:
> > https://www.spinics.net/lists/netdev/msg581042.html
> >
> > Patch 1/6 is not functionally related but also related to dsa_8021q
> > handling of VLANs and this is a good opportunity to bring up the subject
> > for discussion.
>
> So please send 1/6 as a separate patch and bring up the discussion there.
>

Ok.

>
> Thanks,
>
>         Vivien

Regards,
-Vladimir

Powered by blists - more mailing lists