diff mbox series

[net-next,4/6] net: dsa: track whether bridges have foreign interfaces in them

Message ID 20220408200337.718067-5-vladimir.oltean@nxp.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series Disable host flooding for DSA ports under a bridge | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 19 this patch: 19
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 17 this patch: 17
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 19 this patch: 19
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 130 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Vladimir Oltean April 8, 2022, 8:03 p.m. UTC
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@nxp.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 mbox series

Patch

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,