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]
Date:   Mon, 12 Jul 2021 18:21:30 +0300
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     netdev@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Jiri Pirko <jiri@...nulli.us>,
        Ido Schimmel <idosch@...sch.org>,
        Tobias Waldekranz <tobias@...dekranz.com>,
        Roopa Prabhu <roopa@...dia.com>,
        Nikolay Aleksandrov <nikolay@...dia.com>,
        Stephen Hemminger <stephen@...workplumber.org>,
        bridge@...ts.linux-foundation.org,
        Grygorii Strashko <grygorii.strashko@...com>
Subject: [RFC PATCH v3 net-next 12/24] net: bridge: drop context pointer from br_fdb_replay

As opposed to the port objects (mdb, vlan), the FDB entries on a LAG are
a bit special.

While a VLAN installed on a bridge port that is a LAG can reasonably be
modeled from the bridge's perspective as individual VLANs being
installed on all physical ports that are beneath that LAG (and similar
for multicast addresses), the same cannot really be said about a unicast
forwarding destination MAC address.

Actually there is no driver today that makes meaningful use of FDB
entries towards bridge ports that are LAG (bond/team) interfaces, so it
is hard to assume anything. But intuitively, since FDB entries are
usually exclusive to a single destination port, replicating them on all
LAG lowers sounds like a bad idea. Maybe, instead, the switchdev driver
models the LAG as a logical port, and the FDB entries associated with
the LAG target that.

Anyway, do not assume anything and drop the context pointer from the fdb
replay helpers. The context pointer was introduced specifically for the
case where the bridge port is a LAG, beneath which there are multiple
switchdev lowers, all of which must do the same thing when offloading a
given switchdev object, and none of the ports must act on the same
object twice. It appears that in the case of FDB entries it is not
useful: the driver appears to be required to be able to do something
more elaborate even though it is not clear what.

The trouble, really, is that call_switchdev_notifiers() is not able
today to pass the context pointer, but br_fdb_replay calls a hand-coded
version of that function which is. Refactoring call_switchdev_notifiers
does not appear really worth it without at least knowing the requrements,
so drop the functionality with no users for now.

Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
---
 include/linux/if_bridge.h | 4 ++--
 net/bridge/br_fdb.c       | 8 +++-----
 net/dsa/port.c            | 8 ++++----
 3 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index d0bec83488b9..13acc1ff476c 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -168,7 +168,7 @@ bool br_port_flag_is_set(const struct net_device *dev, unsigned long flag);
 u8 br_port_get_stp_state(const struct net_device *dev);
 clock_t br_get_ageing_time(const struct net_device *br_dev);
 int br_fdb_replay(const struct net_device *br_dev, const struct net_device *dev,
-		  const void *ctx, bool adding, struct notifier_block *nb);
+		  bool adding, struct notifier_block *nb);
 #else
 static inline struct net_device *
 br_fdb_find_port(const struct net_device *br_dev,
@@ -199,7 +199,7 @@ static inline clock_t br_get_ageing_time(const struct net_device *br_dev)
 }
 
 static inline int br_fdb_replay(const struct net_device *br_dev,
-				const struct net_device *dev, const void *ctx,
+				const struct net_device *dev,
 				bool adding, struct notifier_block *nb)
 {
 	return -EOPNOTSUPP;
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 2b862cffc03a..c93a2b3a0ad8 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -734,8 +734,7 @@ static inline size_t fdb_nlmsg_size(void)
 
 static int br_fdb_replay_one(struct notifier_block *nb,
 			     const struct net_bridge_fdb_entry *fdb,
-			     struct net_device *dev, unsigned long action,
-			     const void *ctx)
+			     struct net_device *dev, unsigned long action)
 {
 	struct switchdev_notifier_fdb_info item;
 	int err;
@@ -746,14 +745,13 @@ static int br_fdb_replay_one(struct notifier_block *nb,
 	item.offloaded = test_bit(BR_FDB_OFFLOADED, &fdb->flags);
 	item.is_local = test_bit(BR_FDB_LOCAL, &fdb->flags);
 	item.info.dev = dev;
-	item.info.ctx = ctx;
 
 	err = nb->notifier_call(nb, action, &item);
 	return notifier_to_errno(err);
 }
 
 int br_fdb_replay(const struct net_device *br_dev, const struct net_device *dev,
-		  const void *ctx, bool adding, struct notifier_block *nb)
+		  bool adding, struct notifier_block *nb)
 {
 	struct net_bridge_fdb_entry *fdb;
 	struct net_bridge *br;
@@ -783,7 +781,7 @@ int br_fdb_replay(const struct net_device *br_dev, const struct net_device *dev,
 		if (dst_dev != br_dev && dst_dev != dev)
 			continue;
 
-		err = br_fdb_replay_one(nb, fdb, dst_dev, action, ctx);
+		err = br_fdb_replay_one(nb, fdb, dst_dev, action);
 		if (err)
 			break;
 	}
diff --git a/net/dsa/port.c b/net/dsa/port.c
index b824b6f8aa84..34b7f64348c2 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -200,13 +200,13 @@ static int dsa_port_switchdev_sync(struct dsa_port *dp,
 		return err;
 
 	/* Forwarding and termination FDB entries on the port */
-	err = br_fdb_replay(br, brport_dev, dp, true,
+	err = br_fdb_replay(br, brport_dev, true,
 			    &dsa_slave_switchdev_notifier);
 	if (err && err != -EOPNOTSUPP)
 		return err;
 
 	/* Termination FDB entries on the bridge itself */
-	err = br_fdb_replay(br, br, dp, true, &dsa_slave_switchdev_notifier);
+	err = br_fdb_replay(br, br, true, &dsa_slave_switchdev_notifier);
 	if (err && err != -EOPNOTSUPP)
 		return err;
 
@@ -232,13 +232,13 @@ static int dsa_port_switchdev_unsync_objs(struct dsa_port *dp,
 		return err;
 
 	/* Forwarding and termination FDB entries on the port */
-	err = br_fdb_replay(br, brport_dev, dp, false,
+	err = br_fdb_replay(br, brport_dev, false,
 			    &dsa_slave_switchdev_notifier);
 	if (err && err != -EOPNOTSUPP)
 		return err;
 
 	/* Termination FDB entries on the bridge itself */
-	err = br_fdb_replay(br, br, dp, false, &dsa_slave_switchdev_notifier);
+	err = br_fdb_replay(br, br, false, &dsa_slave_switchdev_notifier);
 	if (err && err != -EOPNOTSUPP)
 		return err;
 
-- 
2.25.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ