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] [day] [month] [year] [list]

Date:   Tue, 9 Mar 2021 00:52:27 +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 11:32:57PM +0100, Tobias Waldekranz wrote:
> I get the sense that one reason that the mentioned cases are not caught
> by the existing validation logic, is that checks are scattered in
> multiple places (primarily dsa_slave_check_8021q_upper and
> dsa_port_can_apply_vlan_filtering).
>
> Ideally we should have a single function that answers the question
> "given the current VLAN config, is it OK to make this one modification?"
>
> This is all still very hand-waivy though, it might not be possible.

Nope, they are not caught because they are odd corner cases that we
haven't encountered in real life usage.

> >> - Remove 1/2.
> >> - Rework 2/2 to:
> >>   - `return 0` when adding a VLAN to a non-bridged port, not -EOPNOTSUPP.
> >
> > Still in mv88e6xxx you mean? Well, if mv88e6xxx is still not going to
> > install the VLAN to hardware, why would it lie to DSA and return 0?
>
> Because we want the core to add the `struct vlan_vid_info` to our port's
> vlan list so that we can lazy load it if/when needed. But maybe your
> dynamic rx-vlan-filter patch will render that unnecessary?

Does struct mv88e6xxx_port have a VLAN list that I'm not aware of (a la
struct sja1105_private :: bridge_vlans)? If it doesn't, I was going to
send some patches to the DSA core anyway which manage the VLAN RX
filtering of the user interfaces dynamically.

> >>   - Lazy load/unload VIDs from VLAN uppers when toggling filtering on a
> >>     port using vlan_for_each or similar.
> >
> > How do you plan to do it exactly? Hook into dsa_port_vlan_filtering and:
> > if vlan_filtering == false, then do vlan_for_each(..., dsa_slave_vlan_rx_kill_vid)
> > if vlan_filtering == true, then do vlan_for_each(..., dsa_slave_vlan_rx_add_vid)?
>
> I was just going to hook in to mv88e6xxx_port_vlan_filtering, call
> vlan_for_each and generate an mv88e6xxx-internal call to add the
> VIDs to the port and the CPU port. This does rely on rx-vlan-filter
> always being enabled as the VLAN will be setup on all DSA ports at that
> point, just not on any user ports.

:D
Could you wait for a few hours and see what I'm cooking up first? There
is nothing there that is specific to mv88e6xxx, it is a problem we
should solve generally. The only issue with mv88e6xxx is that it has a
pet peeve to not offload VLANs in standalone mode, while others don't
care so much.

> > Basically when VLAN filtering is disabled, the VTU will only contain the
> > bridge VLANs and none of the 8021q VLANs?
>
> Yes, the switch won't be able to use the 1Q VLANs for anything useful
> anyway.
>
> > If we make this happen, then my patches for runtime toggling
> > 'rx-vlan-filter' should also be needed.
>
> I am fine with that, but I think that means that we need to solve the
> replay at the DSA layer in order to setup DSA ports correctly.

Yup, testing them right now.