• Vladimir Oltean's avatar
    net: mscc: ocelot: fix vlan_filtering when enslaving to bridge before link is up · 1c44ce56
    Vladimir Oltean authored
    Background information: the driver operates the hardware in a mode where
    a single VLAN can be transmitted as untagged on a particular egress
    port. That is the "native VLAN on trunk port" use case. Its value is
    held in port->vid.
    
    Consider the following command sequence (no network manager, all
    interfaces are down, debugging prints added by me):
    
    $ ip link add dev br0 type bridge vlan_filtering 1
    $ ip link set dev swp0 master br0
    
    Kernel code path during last command:
    
    br_add_slave -> ocelot_netdevice_port_event (NETDEV_CHANGEUPPER):
    [   21.401901] ocelot_vlan_port_apply: port 0 vlan aware 0 pvid 0 vid 0
    
    br_add_slave -> nbp_vlan_init -> switchdev_port_attr_set -> ocelot_port_attr_set (SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING):
    [   21.413335] ocelot_vlan_port_apply: port 0 vlan aware 1 pvid 0 vid 0
    
    br_add_slave -> nbp_vlan_init -> nbp_vlan_add -> br_switchdev_port_vlan_add -> switchdev_port_obj_add -> ocelot_port_obj_add -> ocelot_vlan_vid_add
    [   21.667421] ocelot_vlan_port_apply: port 0 vlan aware 1 pvid 1 vid 1
    
    So far so good. The bridge has replaced the driver's default pvid used
    in standalone mode (0) with its own default_pvid (1). The port's vid
    (native VLAN) has also changed from 0 to 1.
    
    $ ip link set dev swp0 up
    
    [   31.722956] 8021q: adding VLAN 0 to HW filter on device swp0
    do_setlink -> dev_change_flags -> vlan_vid_add -> ocelot_vlan_rx_add_vid -> ocelot_vlan_vid_add:
    [   31.728700] ocelot_vlan_port_apply: port 0 vlan aware 1 pvid 1 vid 0
    
    The 8021q module uses the .ndo_vlan_rx_add_vid API on .ndo_open to make
    ports be able to transmit and receive 802.1p-tagged traffic by default.
    This API is supposed to offload a VLAN sub-interface, which for a switch
    port means to add a VLAN that is not a pvid, and tagged on egress.
    
    But the driver implementation of .ndo_vlan_rx_add_vid is wrong: it adds
    back vid 0 as "egress untagged". Now back to the initial paragraph:
    there is a single untagged VID that the driver keeps track of, and that
    has just changed from 1 (the pvid) to 0. So this breaks the bridge
    core's expectation, because it has changed vid 1 from untagged to
    tagged, when what the user sees is.
    
    $ bridge vlan
    port    vlan ids
    swp0     1 PVID Egress Untagged
    
    br0      1 PVID Egress Untagged
    
    But curiously, instead of manifesting itself as "untagged and
    pvid-tagged traffic gets sent as tagged on egress", the bug:
    
    - is hidden when vlan_filtering=0
    - manifests as dropped traffic when vlan_filtering=1, due to this setting:
    
    	if (port->vlan_aware && !port->vid)
    		/* If port is vlan-aware and tagged, drop untagged and priority
    		 * tagged frames.
    		 */
    		val |= ANA_PORT_DROP_CFG_DROP_UNTAGGED_ENA |
    		       ANA_PORT_DROP_CFG_DROP_PRIO_S_TAGGED_ENA |
    		       ANA_PORT_DROP_CFG_DROP_PRIO_C_TAGGED_ENA;
    
    which would have made sense if it weren't for this bug. The setting's
    intention was "this is a trunk port with no native VLAN, so don't accept
    untagged traffic". So the driver was never expecting to set VLAN 0 as
    the value of the native VLAN, 0 was just encoding for "invalid".
    
    So the fix is to not send 802.1p traffic as untagged, because that would
    change the port's native vlan to 0, unbeknownst to the bridge, and
    trigger unexpected code paths in the driver.
    
    Cc: Antoine Tenart <antoine.tenart@bootlin.com>
    Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
    Fixes: 7142529f ("net: mscc: ocelot: add VLAN filtering")
    Signed-off-by: default avatarVladimir Oltean <olteanv@gmail.com>
    Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
    Acked-by: default avatarAlexandre Belloni <alexandre.belloni@bootlin.com>
    Reviewed-by: default avatarHoratiu Vultur <horatiu.vultur@microchip.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    1c44ce56
ocelot.c 59.1 KB