[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87h7lkow44.fsf@waldekranz.com>
Date: Tue, 09 Mar 2021 22:28:11 +0100
From: Tobias Waldekranz <tobias@...dekranz.com>
To: Florian Fainelli <f.fainelli@...il.com>, davem@...emloft.net,
kuba@...nel.org
Cc: andrew@...n.ch, vivien.didelot@...il.com, olteanv@...il.com,
netdev@...r.kernel.org
Subject: Re: [RFC net] net: dsa: Centralize validation of VLAN configuration
On Tue, Mar 09, 2021 at 12:40, Florian Fainelli <f.fainelli@...il.com> wrote:
> On 3/9/21 10:42 AM, Tobias Waldekranz wrote:
>> There are three kinds of events that have an inpact on VLAN
>> configuration of DSA ports:
>>
>> - Adding of stacked VLANs
>> (ip link add dev swp0.1 link swp0 type vlan id 1)
>>
>> - Adding of bridged VLANs
>> (bridge vlan add dev swp0 vid 1)
>>
>> - Changes to a bridge's VLAN filtering setting
>> (ip link set dev br0 type bridge vlan_filtering 1)
>>
>> For all of these events, we want to ensure that some invariants are
>> upheld:
>>
>> - For hardware where VLAN filtering is a global setting, either all
>> bridges must use VLAN filtering, or no bridge can.
>
> I suppose that is true, given that a non-VLAN filtering bridge must not
> perform ingress VID checking, OK.
>
>>
>> - For all filtering bridges, no stacked VLAN on any port may be
>> configured on multiple ports.
>
> You need to qualify multiple ports a bit more here, are you saying
> multiple ports that are part of said bridge, or?
Yeah sorry, I can imagine that makes no sense whatsoever without the
context of the recent discussions. It is basically guarding against this
situation:
.100 br0 .100
\ / \ /
lan0 lan1
$ ip link add dev br0 type bridge vlan_filtering 1
$ ip link add dev lan0.100 link lan0 type vlan id 100
$ ip link add dev lan1.100 link lan1 type vlan id 100
$ ip link set dev lan0 master br0
$ ip link set dev lan1 master br0 # This should fail
>> - For all filtering bridges, no stacked VLAN may be configured in the
>> bridge.
>
> Being stacked in the bridge does not really compute for me, you mean, no
> VLAN upper must be configured on the bridge master device(s)? Why would
> that be a problem though?
Again sorry, I relize that this message needs a lot of work. It guards
against this scenario:
.100 br0
\ / \
lan0 lan1
$ ip link add dev br0 type bridge vlan_filtering 1
$ ip link add dev lan0.100 link lan0 type vlan id 100
$ ip link set dev lan0 master br0
$ ip link set dev lan1 master br0
$ bridge vlan add dev lan1 vid 100 # This should fail
>> Move the validation of these invariants to a central function, and use
>> it from all sites where these events are handled. This way, we ensure
>> that all invariants are always checked, avoiding certain configs being
>> allowed or disallowed depending on the order in which commands are
>> given.
>>
>> Signed-off-by: Tobias Waldekranz <tobias@...dekranz.com>
>> ---
>>
>> There is still testing left to do on this, but I wanted to send early
>> in order show what I meant by "generic" VLAN validation in this
>> discussion:
>>
>> https://lore.kernel.org/netdev/87mtvdp97q.fsf@waldekranz.com/
>>
>> This is basically an alternative implementation of 1/4 and 2/4 from
>> this series by Vladimir:
>>
>> https://lore.kernel.org/netdev/20210309021657.3639745-1-olteanv@gmail.com/
>
> I really have not been able to keep up with your discussion, and I am
> not sure if I will given how quickly you guys can spin patches (not a
> criticism, this is welcome).
Yeah I know, it has been a bit of a whirlwind.
Maybe I should just have posted this inline in the other thread, since
it was mostly to show Vladimir my idea, and it seemed easier to write it
in C than in English :)
Powered by blists - more mailing lists