net: dsa: sja1105: update existing VLANs from the bridge VLAN list

When running this sequence of operations:

ip link add br0 type bridge vlan_filtering 1
ip link set swp4 master br0
bridge vlan add dev swp4 vid 1

We observe the traffic sent on swp4 is still untagged, even though the
bridge has overwritten the existing VLAN entry:

port    vlan ids
swp4     1 PVID

br0      1 PVID Egress Untagged

This happens because we didn't consider that the 'bridge vlan add'
command just overwrites VLANs like it's nothing. We treat the 'vid 1
pvid untagged' and the 'vid 1' as two separate VLANs, and the first
still has precedence when calling sja1105_build_vlan_table. Obviously
there is a disagreement regarding semantics, and we end up doing
something unexpected from the PoV of the bridge.

Let's actually consider an "existing VLAN" to be one which is on the
same port, and has the same VLAN ID, as one we already have, and update
it if it has different flags than we do.

The first blamed commit is the one introducing the bug, the second one
is the latest on top of which the bugfix still applies.

Fixes: ec5ae61076d0 ("net: dsa: sja1105: save/restore VLANs using a delta commit method")
Fixes: 5899ee367ab3 ("net: dsa: tag_8021q: add a context structure")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index dffa7dd838770176071012c2022064f5d5a6f9ac..b88d9ef45a1f1d138861bc454dcf197171dd0e6b 100644 (file)
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2816,11 +2816,22 @@ static int sja1105_vlan_add_one(struct dsa_switch *ds, int port, u16 vid,
        bool pvid = flags & BRIDGE_VLAN_INFO_PVID;
        struct sja1105_bridge_vlan *v;
 
-       list_for_each_entry(v, vlan_list, list)
-               if (v->port == port && v->vid == vid &&
-                   v->untagged == untagged && v->pvid == pvid)
+       list_for_each_entry(v, vlan_list, list) {
+               if (v->port == port && v->vid == vid) {
                        /* Already added */
-                       return 0;
+                       if (v->untagged == untagged && v->pvid == pvid)
+                               /* Nothing changed */
+                               return 0;
+
+                       /* It's the same VLAN, but some of the flags changed
+                        * and the user did not bother to delete it first.
+                        * Update it and trigger sja1105_build_vlan_table.
+                        */
+                       v->untagged = untagged;
+                       v->pvid = pvid;
+                       return 1;
+               }
+       }
 
        v = kzalloc(sizeof(*v), GFP_KERNEL);
        if (!v) {