netdev - [RFC PATCH net-next 1/5] net: bridge: vlan: br_vlan

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20220209213044.2353153-2-vladimir.oltean@nxp.com>
Date:   Wed,  9 Feb 2022 23:30:39 +0200
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     netdev@...r.kernel.org
Cc:     Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        Roopa Prabhu <roopa@...dia.com>,
        Nikolay Aleksandrov <nikolay@...dia.com>,
        Jiri Pirko <jiri@...dia.com>, Ido Schimmel <idosch@...dia.com>,
        Rafael Richter <rafael.richter@....de>,
        Daniel Klauer <daniel.klauer@....de>,
        Tobias Waldekranz <tobias@...dekranz.com>
Subject: [RFC PATCH net-next 1/5] net: bridge: vlan: br_vlan_add: notify switchdev only when changed

When a VLAN entry is added multiple times in a row to a bridge:

bridge vlan add dev br0 vid 100 self

the bridge notifies switchdev each time, even if nothing changed, which
makes driver-level accounting impossible.

Move br_switchdev_port_vlan_add() out of br_vlan_add_existing(), keep
track of exactly what br_vlan_add_existing() changed, only call
br_switchdev_port_vlan_add() afterwards and if anything changed at all,
and restore whatever br_vlan_add_existing() may have done on the error
path of br_switchdev_port_vlan_add().

Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
---
 net/bridge/br_vlan.c | 68 ++++++++++++++++++++++++++++++++------------
 1 file changed, 50 insertions(+), 18 deletions(-)

diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 1402d5ca242d..c7cadc1b4f71 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -667,44 +667,56 @@ bool br_should_learn(struct net_bridge_port *p, struct sk_buff *skb, u16 *vid)
 static int br_vlan_add_existing(struct net_bridge *br,
 				struct net_bridge_vlan_group *vg,
 				struct net_bridge_vlan *vlan,
-				u16 flags, bool *changed,
+				u16 flags, bool *brentry_created,
+				bool *flags_changed,
 				struct netlink_ext_ack *extack)
 {
 	int err;
 
-	err = br_switchdev_port_vlan_add(br->dev, vlan->vid, flags, extack);
-	if (err && err != -EOPNOTSUPP)
-		return err;
+	*brentry_created = false;
+	*flags_changed = false;
 
 	if (!br_vlan_is_brentry(vlan)) {
 		/* Trying to change flags of non-existent bridge vlan */
-		if (!(flags & BRIDGE_VLAN_INFO_BRENTRY)) {
-			err = -EINVAL;
-			goto err_flags;
-		}
+		if (!(flags & BRIDGE_VLAN_INFO_BRENTRY))
+			return -EINVAL;
+
 		/* It was only kept for port vlans, now make it real */
 		err = br_fdb_add_local(br, NULL, br->dev->dev_addr, vlan->vid);
 		if (err) {
 			br_err(br, "failed to insert local address into bridge forwarding table\n");
-			goto err_fdb_insert;
+			return err;
 		}
 
 		refcount_inc(&vlan->refcnt);
 		vlan->flags |= BRIDGE_VLAN_INFO_BRENTRY;
 		vg->num_vlans++;
-		*changed = true;
+		*brentry_created = true;
 		br_multicast_toggle_one_vlan(vlan, true);
 	}
 
 	if (__vlan_add_flags(vlan, flags))
-		*changed = true;
+		*flags_changed = true;
 
 	return 0;
+}
 
-err_fdb_insert:
-err_flags:
-	br_switchdev_port_vlan_del(br->dev, vlan->vid);
-	return err;
+static void br_vlan_restore_existing(struct net_bridge *br,
+				     struct net_bridge_vlan_group *vg,
+				     struct net_bridge_vlan *vlan,
+				     u16 flags, bool del_brentry,
+				     bool restore_flags)
+{
+	if (del_brentry) {
+		br_fdb_find_delete_local(br, NULL, br->dev->dev_addr, vlan->vid);
+
+		refcount_dec(&vlan->refcnt);
+		vlan->flags &= ~BRIDGE_VLAN_INFO_BRENTRY;
+		vg->num_vlans--;
+	}
+
+	if (restore_flags)
+		__vlan_add_flags(vlan, flags);
 }
 
 /* Must be protected by RTNL.
@@ -723,9 +735,29 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags, bool *changed,
 	*changed = false;
 	vg = br_vlan_group(br);
 	vlan = br_vlan_find(vg, vid);
-	if (vlan)
-		return br_vlan_add_existing(br, vg, vlan, flags, changed,
-					    extack);
+	if (vlan) {
+		bool brentry_created, flags_changed;
+		u16 old_flags = vlan->flags;
+
+		ret = br_vlan_add_existing(br, vg, vlan, flags,
+					   &brentry_created, &flags_changed,
+					   extack);
+		if (ret)
+			return ret;
+
+		*changed = brentry_created || flags_changed;
+		if (*changed) {
+			ret = br_switchdev_port_vlan_add(br->dev, vlan->vid,
+							 flags, extack);
+			if (ret && ret != -EOPNOTSUPP) {
+				br_vlan_restore_existing(br, vg, vlan, old_flags,
+							 brentry_created, flags_changed);
+				return ret;
+			}
+		}
+
+		return 0;
+	}
 
 	vlan = kzalloc(sizeof(*vlan), GFP_KERNEL);
 	if (!vlan)
-- 
2.25.1