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.
