netdev - Re: [PATCH net 1/2] net: dsa: Accept software VLANs for stacked interfaces

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210308154446.ceqp56bh65bsarlt@skbuf>
Date:   Mon, 8 Mar 2021 17:44:46 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     Tobias Waldekranz <tobias@...dekranz.com>
Cc:     davem@...emloft.net, kuba@...nel.org, andrew@...n.ch,
        vivien.didelot@...il.com, f.fainelli@...il.com,
        netdev@...r.kernel.org
Subject: Re: [PATCH net 1/2] net: dsa: Accept software VLANs for stacked
 interfaces

On Mon, Mar 08, 2021 at 04:04:04PM +0100, Tobias Waldekranz wrote:
> The dsa_slave_vlan_rx_{add,kill}_vid ndos are required for hardware
> that can not control VLAN filtering per port, rather it is a device
> global setting, in order to support VLAN uppers on non-bridged ports.
> 
> For hardware that can control VLAN filtering per port, it is perfectly
> fine to fallback to software VLANs in this scenario. So, make sure
> that this "error" does not leave the DSA layer as vlan_add_vid does
> not know the meaning of it.
> 
> The blamed commit removed this exemption by not advertising the
> feature if the driver did not implement VLAN offloading. But as we
> know see, the assumption that if a driver supports VLAN offloading, it
> will always use it, does not hold in certain edge cases.
> 
> Fixes: 9b236d2a69da ("net: dsa: Advertise the VLAN offload netdev ability only if switch supports it")
> Signed-off-by: Tobias Waldekranz <tobias@...dekranz.com>
> ---

So these NDOs exist for drivers that need the 'rx-vlan-filter: on'
feature in ethtool -k, which can be due to any of the following reasons:
1. vlan_filtering_is_global = true, some ports are under a VLAN-aware
   bridge while others are standalone (this is what you described)
2. Hellcreek. This driver needs it because in standalone mode, it uses
   unique VLANs per port to ensure separation. For separation of untagged
   traffic, it uses different PVIDs for each port, and for separation of
   VLAN-tagged traffic, it never accepts 8021q uppers with the same vid
   on two ports.
3. the ports that are under a VLAN-aware bridge should also set this
   feature, for 8021q uppers having a VID not claimed by the bridge.
   In this case, the driver will essentially not even know that the VID
   is coming from the 8021q layer and not the bridge.

If a driver does not fall under any of the above 3 categories, there is
no reason why it should advertise the 'rx-vlan-filter' feature, therefore
no reason why it should implement these NDOs, and return -EOPNOTSUPP.

We are essentially saying the same thing, except what I propose is to
better manage the 'rx-vlan-filter' feature of the DSA net devices. After
your patches, the network stack still thinks that mv88e6xxx ports in
standalone mode have VLAN filtering enabled, which they don't. That
might be confusing. Not only that, but any other driver that is
VLAN-unaware in standalone mode will similarly have to ignore VLANs
coming from the 8021q layer, which may add uselessly add to their
complexity. Let me prepare an alternative patch series and let's see how
they compare against each other.

As far as I see, mv88e6xxx needs to treat the VLAN NDOs in case 3 only,
and DSA will do that without any sort of driver-level awareness. It's
all the other cases (standalone ports mode) that are bothering you.