• Vladimir Oltean's avatar
    net: dsa: mv88e6xxx: deny vid 0 on the CPU port and DSA links too · 3e85f580
    Vladimir Oltean authored
    mv88e6xxx apparently has a problem offloading VID 0, which the 8021q
    module tries to install as part of commit ad1afb00 ("vlan_dev: VLAN
    0 should be treated as "no vlan tag" (802.1p packet)"). That mv88e6xxx
    restriction seems to have been introduced by the "VTU GetNext VID-1
    trick to retrieve a single entry" - see commit 2fb5ef09
    
     ("net: dsa:
    mv88e6xxx: extract single VLAN retrieval").
    
    There is one more problem. The mv88e6xxx CPU port and DSA links do not
    report properly in the prepare phase what are the VLANs that they can
    offload. They'll say they can offload everything:
    
    mv88e6xxx_port_vlan_prepare
    -> mv88e6xxx_port_check_hw_vlan:
    
    	/* DSA and CPU ports have to be members of multiple vlans */
    	if (dsa_is_dsa_port(ds, port) || dsa_is_cpu_port(ds, port))
    		return 0;
    
    Except that if you actually try to commit to it, they'll error out and
    print this message:
    
    [   32.802438] mv88e6085 d0032004.mdio-mii:12: p9: failed to add VLAN 0t
    
    which comes from:
    
    mv88e6xxx_port_vlan_add
    -> mv88e6xxx_port_vlan_join:
    
    	if (!vid)
    		return -EOPNOTSUPP;
    
    What prevents this condition from triggering in real life? The fact that
    when a DSA_NOTIFIER_VLAN_ADD is emitted, it never targets a DSA link
    directly. Instead, the notifier will always target either a user port or
    a CPU port. DSA links just happen to get dragged in by:
    
    static bool dsa_switch_vlan_match(struct dsa_switch *ds, int port,
    				  struct dsa_notifier_vlan_info *info)
    {
    	...
    	if (dsa_is_dsa_port(ds, port))
    		return true;
    	...
    }
    
    So for every DSA VLAN notifier, during the prepare phase, it will just
    so happen that there will be somebody to say "no, don't do that".
    
    This will become a problem when the switchdev prepare/commit transactional
    model goes away. Every port needs to think on its own. DSA links can no
    longer bluff and rely on the fact that the prepare phase will not go
    through to the end, because there will be no prepare phase any longer.
    
    Fix this issue before it becomes a problem, by having the "vid == 0"
    check earlier than the check whether we are a CPU port / DSA link or not.
    Also, the "vid == 0" check becomes unnecessary in the .port_vlan_add
    callback, so we can remove it.
    Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
    Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
    Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
    3e85f580
chip.c 163 KB