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.

quoted

quoted

- 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.

quoted

quoted

  - 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.

quoted

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.

quoted

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.