From: Vladimir Oltean olteanv@gmail.com
[ Upstream commit 1c44ce560b4de639f237b458be1729489ff44d0a ]
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: 7142529f1688 ("net: mscc: ocelot: add VLAN filtering") Signed-off-by: Vladimir Oltean olteanv@gmail.com Reviewed-by: Florian Fainelli f.fainelli@gmail.com Acked-by: Alexandre Belloni alexandre.belloni@bootlin.com Reviewed-by: Horatiu Vultur horatiu.vultur@microchip.com Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/net/ethernet/mscc/ocelot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c index dc78940f08fb1..07ca3e0cdf92b 100644 --- a/drivers/net/ethernet/mscc/ocelot.c +++ b/drivers/net/ethernet/mscc/ocelot.c @@ -877,7 +877,7 @@ end: static int ocelot_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid) { - return ocelot_vlan_vid_add(dev, vid, false, true); + return ocelot_vlan_vid_add(dev, vid, false, false); }
static int ocelot_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,