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: <20220408200337.718067-5-vladimir.oltean@nxp.com>
Date:   Fri,  8 Apr 2022 23:03:35 +0300
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>,
        UNGLinuxDriver@...rochip.com, Paolo Abeni <pabeni@...hat.com>,
        Roopa Prabhu <roopa@...dia.com>,
        Nikolay Aleksandrov <nikolay@...dia.com>,
        Jiri Pirko <jiri@...dia.com>, Ido Schimmel <idosch@...dia.com>,
        Tobias Waldekranz <tobias@...dekranz.com>,
        Mattias Forsblad <mattias.forsblad@...il.com>
Subject: [PATCH net-next 4/6] net: dsa: track whether bridges have foreign interfaces in them

DSA switches send packets to the CPU for 2 reasons, either for local
termination or for software forwarding.

If the switch driver satisfies the requirements for IFF_UNICAST_FLT and
also offloads the bridge local FDB entries, then there is no reason to
send unknown traffic to the CPU for the purpose of local termination.

This leaves software forwarding as the sole concern, and a place where
certain optimizations can be made. In the case of a bridge where all
bridge ports are offloaded DSA interfaces, there isn't any need to do
software forwarding (and therefore, to enable host flooding).

We approximate the need for a DSA port to enable host flooding by managing
IFF_PROMISC, which ends up triggering dsa_port_manage_cpu_flood().
This isn't ideal, because IFF_PROMISC/dev->uc/dev->mc deal only with the
standalone address database of a port, and not with the bridging
database, but right now DSA doesn't have the poster-child hardware where
flooding is a per-FID setting rather than per-port. So in current
practice, there is no reason to make dsa_port_manage_cpu_flood() more
complex by looking at anything other than IFF_PROMISC.

To enable those optimizations, create a function called
dsa_bridge_foreign_dev_update() which updates a new boolean of struct
dsa_bridge called "have_foreign" whenever a DSA port joins/leaves a
bridge, or a LAG that is already in a bridge, or any other interface
joins/leaves a bridge in which a DSA port or LAG offloaded by a DSA port
exists.

Note that when dsa_port_bridge_create() is first called,
dsa_bridge_foreign_dev_update() is not called right away. It is called
slightly later (still under rtnl_mutex), leading to some DSA switch
callbacks (->port_bridge_join) being called with a potentially not
up-to-date "have_foreign" property. This can be changed if necessary,
I deem it as "not a problem" for now.

Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
---
 include/net/dsa.h  |  3 +-
 net/dsa/dsa_priv.h |  1 +
 net/dsa/port.c     |  7 ++++
 net/dsa/slave.c    | 80 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 90 insertions(+), 1 deletion(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index f2352d82e37b..0ea45a4acc80 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -240,8 +240,9 @@ struct dsa_mall_tc_entry {
 struct dsa_bridge {
 	struct net_device *dev;
 	unsigned int num;
-	bool tx_fwd_offload;
 	refcount_t refcount;
+	u8 tx_fwd_offload:1;
+	u8 have_foreign:1;
 };
 
 struct dsa_port {
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 5d3f4a67dce1..d610776ecd76 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -320,6 +320,7 @@ void dsa_slave_setup_tagger(struct net_device *slave);
 int dsa_slave_change_mtu(struct net_device *dev, int new_mtu);
 int dsa_slave_manage_vlan_filtering(struct net_device *dev,
 				    bool vlan_filtering);
+int dsa_bridge_foreign_dev_update(struct net_device *bridge_dev);
 
 static inline struct dsa_port *dsa_slave_to_port(const struct net_device *dev)
 {
diff --git a/net/dsa/port.c b/net/dsa/port.c
index af9a815c2639..cbee564e1c22 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -656,8 +656,15 @@ int dsa_port_lag_join(struct dsa_port *dp, struct net_device *lag_dev,
 	if (err)
 		goto err_bridge_join;
 
+	err = dsa_bridge_foreign_dev_update(bridge_dev);
+	if (err)
+		goto err_foreign_update;
+
 	return 0;
 
+err_foreign_update:
+	dsa_port_pre_bridge_leave(dp, bridge_dev);
+	dsa_port_bridge_leave(dp, bridge_dev);
 err_bridge_join:
 	dsa_port_notify(dp, DSA_NOTIFIER_LAG_LEAVE, &info);
 err_lag_join:
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index f87109e7696d..1bc8d830fb46 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -2595,6 +2595,18 @@ dsa_slave_lag_prechangeupper(struct net_device *dev,
 	return err;
 }
 
+static int dsa_bridge_changelower(struct net_device *dev,
+				  struct netdev_notifier_changeupper_info *info)
+{
+	int err;
+
+	if (!netif_is_bridge_master(info->upper_dev))
+		return NOTIFY_DONE;
+
+	err = dsa_bridge_foreign_dev_update(info->upper_dev);
+	return notifier_from_errno(err);
+}
+
 static int
 dsa_prevent_bridging_8021q_upper(struct net_device *dev,
 				 struct netdev_notifier_changeupper_info *info)
@@ -2720,6 +2732,10 @@ static int dsa_slave_netdevice_event(struct notifier_block *nb,
 		if (notifier_to_errno(err))
 			return err;
 
+		err = dsa_bridge_changelower(dev, ptr);
+		if (notifier_to_errno(err))
+			return err;
+
 		break;
 	}
 	case NETDEV_CHANGELOWERSTATE: {
@@ -2874,6 +2890,70 @@ static bool dsa_foreign_dev_check(const struct net_device *dev,
 	return true;
 }
 
+/* We need to keep flooding towards the CPU enabled as long as software
+ * forwarding is required.
+ */
+static void dsa_bridge_host_flood_change(struct dsa_bridge *bridge,
+					 bool have_foreign)
+{
+	bool host_flood_was_enabled = bridge->have_foreign;
+	bool host_flood_enabled = have_foreign;
+	int inc = host_flood_enabled ? 1 : -1;
+	struct net_device *brport_dev;
+	struct dsa_switch_tree *dst;
+	struct dsa_port *dp;
+
+	if (host_flood_was_enabled == host_flood_enabled)
+		goto out;
+
+	list_for_each_entry(dst, &dsa_tree_list, list) {
+		dsa_tree_for_each_user_port(dp, dst) {
+			if (dsa_port_offloads_bridge(dp, bridge)) {
+				brport_dev = dsa_port_to_bridge_port(dp);
+				dev_set_promiscuity(brport_dev, inc);
+			}
+		}
+	}
+
+out:
+	bridge->have_foreign = have_foreign;
+}
+
+int dsa_bridge_foreign_dev_update(struct net_device *bridge_dev)
+{
+	struct net_device *first_slave, *lower;
+	struct dsa_bridge *bridge = NULL;
+	struct dsa_switch_tree *dst;
+	bool have_foreign = false;
+	struct list_head *iter;
+	struct dsa_port *dp;
+
+	list_for_each_entry(dst, &dsa_tree_list, list) {
+		dsa_tree_for_each_user_port(dp, dst) {
+			if (dsa_port_offloads_bridge_dev(dp, bridge_dev)) {
+				bridge = dp->bridge;
+				first_slave = dp->slave;
+				break;
+			}
+		}
+	}
+
+	/* Bridge with no DSA interface in it */
+	if (!bridge)
+		return 0;
+
+	netdev_for_each_lower_dev(bridge_dev, lower, iter) {
+		if (dsa_foreign_dev_check(first_slave, lower)) {
+			have_foreign = true;
+			break;
+		}
+	}
+
+	dsa_bridge_host_flood_change(bridge, have_foreign);
+
+	return 0;
+}
+
 static int dsa_slave_fdb_event(struct net_device *dev,
 			       struct net_device *orig_dev,
 			       unsigned long event, const void *ctx,
-- 
2.25.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ