Date:   Sat, 12 Sep 2020 02:16:19 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     kuba@...nel.org, davem@...emloft.net, roopa@...dia.com,
        nikolay@...dia.com, stephen@...workplumber.org
Cc:     andrew@...n.ch, f.fainelli@...il.com, netdev@...r.kernel.org
Subject: [PATCH net-next] net: bridge: pop vlan from skb if filtering is disabled but it's a pvid

Currently the bridge untags VLANs from its VLAN group in
__allowed_ingress() only when VLAN filtering is enabled.

When installing a pvid in egress-tagged mode, DSA switches have a
problem:

ip link add dev br0 type bridge vlan_filtering 0
ip link set swp0 master br0
bridge vlan del dev swp0 vid 1
bridge vlan add dev swp0 vid 1 pvid

When adding a VLAN on a DSA switch interface, DSA configures the VLAN
membership of the CPU port using the same flags as swp0 (in this case
"pvid and not untagged"), in an attempt to copy the frame as-is from
ingress to the CPU.

However, in this case, the packet may arrive untagged on ingress, it
will be pvid-tagged by the ingress port, and will be sent as
egress-tagged towards the CPU. Otherwise stated, the CPU will see a VLAN
tag where there was none to speak of on ingress.

When vlan_filtering is 1, this is not a problem, as stated in the first
paragraph, because __allowed_ingress() will pop it. But currently, when
vlan_filtering is 0 and we have such a VLAN configuration, we need an
8021q upper (br0.1) to be able to ping over that VLAN.

Make the 2 cases (vlan_filtering 0 and 1) behave the same way by popping
the pvid, if the skb happens to be tagged with it, when vlan_filtering
is 0.

There was an attempt to resolve this issue locally within the DSA
receive data path, but even though we can determine that we are under a
bridge with vlan_filtering=0, there are still some challenges:
- we cannot be certain that the skb will end up in the software bridge's
  data path, and for that reason, we may be popping the VLAN for
  nothing. Example: there might exist an 8021q upper with the same VLAN,
  or this interface might be a DSA master for another switch. In that
  case, the VLAN should definitely not be popped even if it is equal to
  the default_pvid of the bridge, because it will be consumed about the
  DSA layer below.
- the bridge API only offers a race-free API for determining the pvid of
  a port, br_vlan_get_pvid(), under RTNL.

And in fact this might not even be a situation unique to DSA. Any driver
that receives untagged frames as pvid-tagged is now able to communicate
without needing an 8021q upper for the pvid.

Signed-off-by: Vladimir Oltean <olteanv@...il.com>
Tested-by: Florian Fainelli <f.fainelli@...il.com>
---
 net/bridge/br_vlan.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index d2b8737f9fc0..ecfdb9cd3183 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -580,7 +580,23 @@ bool br_allowed_ingress(const struct net_bridge *br,
 	 * permitted.
 	 */
 	if (!br_opt_get(br, BROPT_VLAN_ENABLED)) {
+		u16 v;
+
 		BR_INPUT_SKB_CB(skb)->vlan_filtered = false;
+
+		/* See comment in __allowed_ingress about how skb can end up
+		 * here not having a hwaccel tag
+		 */
+		if (unlikely(!skb_vlan_tag_present(skb) &&
+			     skb->protocol == br->vlan_proto)) {
+			skb = skb_vlan_untag(skb);
+			if (unlikely(!skb))
+				return false;
+		}
+
+		if (!br_vlan_get_tag(skb, &v) && v == br_get_pvid(vg))
+			__vlan_hwaccel_clear_tag(skb);
+
 		return true;
 	}
 
-- 
2.25.1

Powered by blists - more mailing lists