All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH v2 net-next 0/8] Remove the "dsa_to_port in a loop" antipattern
@ 2021-08-10 16:14 Vladimir Oltean
  2021-08-10 16:14 ` [RFC PATCH v2 net-next 1/8] net: dsa: introduce a dsa_port_is_unused helper Vladimir Oltean
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Vladimir Oltean @ 2021-08-10 16:14 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Woojung Huh, UNGLinuxDriver, Sean Wang,
	Landen Chao, DENG Qingfang, Matthias Brugger, Claudiu Manoil,
	Alexandre Belloni, George McCollister

v1->v2: more patches

The DSA core and drivers currently iterate too much through the port
list of a switch. For example, this snippet:

	for (port = 0; port < ds->num_ports; port++) {
		if (!dsa_is_cpu_port(ds, port))
			continue;

		ds->ops->change_tag_protocol(ds, port, tag_ops->proto);
	}

iterates through ds->num_ports once, and then calls dsa_is_cpu_port to
filter out the other types of ports. But that function has a hidden call
to dsa_to_port() in it, which contains:

	list_for_each_entry(dp, &dst->ports, list)
		if (dp->ds == ds && dp->index == p)
			return dp;

where the only thing we wanted to know in the first place was whether
dp->type == DSA_PORT_TYPE_CPU or not.

So it seems that the problem is that we are not iterating with the right
variable. We have an "int port" but in fact need a "struct dsa_port *dp".

This has started being an issue since this patch series:
https://patchwork.ozlabs.org/project/netdev/cover/20191020031941.3805884-1-vivien.didelot@gmail.com/

The currently proposed set of changes iterates like this:

	dsa_switch_for_each_cpu_port(cpu_dp, ds)
		err = ds->ops->change_tag_protocol(ds, cpu_dp->index,
						   tag_ops->proto);

which iterates directly over ds->dst->ports, which is a list of struct
dsa_port *dp. This makes it much easier and more efficient to check
dp->type.

As a nice side effect, with the proposed driver API, driver writers are
now encouraged to use more efficient patterns, and not only due to less
iterations through the port list. For example, something like this:

	for (port = 0; port < ds->num_ports; port++)
		do_something();

probably does not need to do_something() for the ports that are disabled
in the device tree. But adding extra code for that would look like this:

	for (port = 0; port < ds->num_ports; port++) {
		if (!dsa_is_unused_port(ds, port))
			continue;

		do_something();
	}

and therefore, it is understandable that some driver writers may decide
to not bother. This patch series introduces a "dsa_switch_for_each_available_port"
macro which comes at no extra cost in terms of lines of code / number of
braces to the driver writer, but it has the "dsa_is_unused_port" check
embedded within it.

I changed as much code as I could, probably not all, but a start anyway.

Vladimir Oltean (8):
  net: dsa: introduce a dsa_port_is_unused helper
  net: dsa: remove the "dsa_to_port in a loop" antipattern from the core
  net: dsa: remove the "dsa_to_port in a loop" antipattern from drivers
  net: dsa: b53: express b53_for_each_port in terms of
    dsa_switch_for_each_port
  net: dsa: finish conversion to dsa_switch_for_each_port
  net: dsa: remove gratuitous use of dsa_is_{user,dsa,cpu}_port
  net: dsa: convert cross-chip notifiers to iterate using dp
  net: dsa: tag_8021q: finish conversion to dsa_switch_for_each_port

 drivers/net/dsa/b53/b53_common.c              |  26 ++-
 drivers/net/dsa/b53/b53_priv.h                |   6 +-
 drivers/net/dsa/bcm_sf2.c                     |   8 +-
 drivers/net/dsa/hirschmann/hellcreek.c        |  27 +--
 .../net/dsa/hirschmann/hellcreek_hwtstamp.c   |  19 +-
 drivers/net/dsa/microchip/ksz9477.c           |  19 +-
 drivers/net/dsa/microchip/ksz_common.c        |  19 +-
 drivers/net/dsa/mt7530.c                      |  58 +++---
 drivers/net/dsa/mv88e6xxx/chip.c              |  48 ++---
 drivers/net/dsa/mv88e6xxx/hwtstamp.c          |  10 +-
 drivers/net/dsa/mv88e6xxx/port.c              |  12 +-
 drivers/net/dsa/ocelot/felix.c                |  79 +++-----
 drivers/net/dsa/ocelot/felix_vsc9959.c        |  11 +-
 drivers/net/dsa/ocelot/seville_vsc9953.c      |  14 +-
 drivers/net/dsa/qca8k.c                       |  32 ++--
 drivers/net/dsa/sja1105/sja1105_main.c        | 176 ++++++++----------
 drivers/net/dsa/sja1105/sja1105_mdio.c        |  12 +-
 drivers/net/dsa/xrs700x/xrs700x.c             |  37 ++--
 include/net/dsa.h                             |  47 ++++-
 net/dsa/dsa.c                                 |  22 +--
 net/dsa/dsa2.c                                |  38 ++--
 net/dsa/port.c                                |  11 +-
 net/dsa/slave.c                               |   2 +-
 net/dsa/switch.c                              | 170 ++++++++---------
 net/dsa/tag_8021q.c                           | 121 ++++++------
 25 files changed, 501 insertions(+), 523 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC PATCH v2 net-next 1/8] net: dsa: introduce a dsa_port_is_unused helper
  2021-08-10 16:14 [RFC PATCH v2 net-next 0/8] Remove the "dsa_to_port in a loop" antipattern Vladimir Oltean
@ 2021-08-10 16:14 ` Vladimir Oltean
  2021-08-10 16:14 ` [RFC PATCH v2 net-next 2/8] net: dsa: remove the "dsa_to_port in a loop" antipattern from the core Vladimir Oltean
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Vladimir Oltean @ 2021-08-10 16:14 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Woojung Huh, UNGLinuxDriver, Sean Wang,
	Landen Chao, DENG Qingfang, Matthias Brugger, Claudiu Manoil,
	Alexandre Belloni, George McCollister

Similar to the existing dsa_port_is_{cpu,user,dsa} helpers which operate
directly on a struct dsa_port *dp, let's introduce the equivalent of
dsa_is_unused_port. We will use this to create a more efficient iterator
over the available ports of a switch.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
v1->v2: none

 include/net/dsa.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index cd7dc74d0d4c..d05c71a92715 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -431,6 +431,11 @@ static inline struct dsa_port *dsa_to_port(struct dsa_switch *ds, int p)
 	return NULL;
 }
 
+static inline bool dsa_port_is_unused(struct dsa_port *port)
+{
+	return port->type == DSA_PORT_TYPE_UNUSED;
+}
+
 static inline bool dsa_port_is_dsa(struct dsa_port *port)
 {
 	return port->type == DSA_PORT_TYPE_DSA;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [RFC PATCH v2 net-next 2/8] net: dsa: remove the "dsa_to_port in a loop" antipattern from the core
  2021-08-10 16:14 [RFC PATCH v2 net-next 0/8] Remove the "dsa_to_port in a loop" antipattern Vladimir Oltean
  2021-08-10 16:14 ` [RFC PATCH v2 net-next 1/8] net: dsa: introduce a dsa_port_is_unused helper Vladimir Oltean
@ 2021-08-10 16:14 ` Vladimir Oltean
  2021-08-10 16:14 ` [RFC PATCH v2 net-next 3/8] net: dsa: remove the "dsa_to_port in a loop" antipattern from drivers Vladimir Oltean
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Vladimir Oltean @ 2021-08-10 16:14 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Woojung Huh, UNGLinuxDriver, Sean Wang,
	Landen Chao, DENG Qingfang, Matthias Brugger, Claudiu Manoil,
	Alexandre Belloni, George McCollister

Ever since Vivien's conversion of the ds->ports array into a dst->ports
list, and the introduction of dsa_to_port, iterations through the ports
of a switch became quadratic whenever dsa_to_port was needed.

dsa_to_port can either be called directly, or indirectly through the
dsa_is_{user,cpu,dsa,unused}_port helpers.

Introduce a basic switch port iterator macro, dsa_switch_for_each_port,
that works with the iterator variable being a struct dsa_port *dp
directly, and not an int i. It is an expensive variable to go from i to
dp, but cheap to go from dp to i.

This macro iterates through the entire ds->dst->ports list and filters
by the ports belonging just to the switch provided as argument.

While at it, provide some more flavors of that macro which filter for
specific types of ports: at the moment just user ports and CPU ports.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
v1->v2: two more conversions of dsa_port_is_user

 include/net/dsa.h   | 19 +++++++++++++++----
 net/dsa/dsa.c       | 22 +++++++++++-----------
 net/dsa/dsa2.c      | 13 ++++++-------
 net/dsa/port.c      |  7 ++++---
 net/dsa/slave.c     |  2 +-
 net/dsa/switch.c    | 41 ++++++++++++++++++-----------------------
 net/dsa/tag_8021q.c | 29 +++++++++++++----------------
 7 files changed, 68 insertions(+), 65 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index d05c71a92715..4639a82bab66 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -471,14 +471,25 @@ static inline bool dsa_is_user_port(struct dsa_switch *ds, int p)
 	return dsa_to_port(ds, p)->type == DSA_PORT_TYPE_USER;
 }
 
+#define dsa_switch_for_each_port(_dp, _ds) \
+	list_for_each_entry((_dp), &(_ds)->dst->ports, list) \
+		if ((_dp)->ds == (_ds))
+
+#define dsa_switch_for_each_user_port(_dp, _ds) \
+	dsa_switch_for_each_port((_dp), (_ds)) \
+		if (dsa_port_is_user((_dp)))
+
+#define dsa_switch_for_each_cpu_port(_dp, _ds) \
+	dsa_switch_for_each_port((_dp), (_ds)) \
+		if (dsa_port_is_cpu((_dp)))
+
 static inline u32 dsa_user_ports(struct dsa_switch *ds)
 {
+	struct dsa_port *dp;
 	u32 mask = 0;
-	int p;
 
-	for (p = 0; p < ds->num_ports; p++)
-		if (dsa_is_user_port(ds, p))
-			mask |= BIT(p);
+	dsa_switch_for_each_user_port(dp, ds)
+		mask |= BIT(dp->index);
 
 	return mask;
 }
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 5e73332a9707..8104f2382988 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -280,23 +280,22 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
 }
 
 #ifdef CONFIG_PM_SLEEP
-static bool dsa_is_port_initialized(struct dsa_switch *ds, int p)
+static bool dsa_port_is_initialized(const struct dsa_port *dp)
 {
-	const struct dsa_port *dp = dsa_to_port(ds, p);
-
 	return dp->type == DSA_PORT_TYPE_USER && dp->slave;
 }
 
 int dsa_switch_suspend(struct dsa_switch *ds)
 {
-	int i, ret = 0;
+	struct dsa_port *dp;
+	int ret = 0;
 
 	/* Suspend slave network devices */
-	for (i = 0; i < ds->num_ports; i++) {
-		if (!dsa_is_port_initialized(ds, i))
+	dsa_switch_for_each_port(dp, ds) {
+		if (!dsa_port_is_initialized(dp))
 			continue;
 
-		ret = dsa_slave_suspend(dsa_to_port(ds, i)->slave);
+		ret = dsa_slave_suspend(dp->slave);
 		if (ret)
 			return ret;
 	}
@@ -310,7 +309,8 @@ EXPORT_SYMBOL_GPL(dsa_switch_suspend);
 
 int dsa_switch_resume(struct dsa_switch *ds)
 {
-	int i, ret = 0;
+	struct dsa_port *dp;
+	int ret = 0;
 
 	if (ds->ops->resume)
 		ret = ds->ops->resume(ds);
@@ -319,11 +319,11 @@ int dsa_switch_resume(struct dsa_switch *ds)
 		return ret;
 
 	/* Resume slave network devices */
-	for (i = 0; i < ds->num_ports; i++) {
-		if (!dsa_is_port_initialized(ds, i))
+	dsa_switch_for_each_port(dp, ds) {
+		if (!dsa_port_is_initialized(dp))
 			continue;
 
-		ret = dsa_slave_resume(dsa_to_port(ds, i)->slave);
+		ret = dsa_slave_resume(dp->slave);
 		if (ret)
 			return ret;
 	}
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 8150e16aaa55..cf9810d55611 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -707,16 +707,15 @@ static int dsa_switch_setup_tag_protocol(struct dsa_switch *ds)
 {
 	const struct dsa_device_ops *tag_ops = ds->dst->tag_ops;
 	struct dsa_switch_tree *dst = ds->dst;
-	int port, err;
+	struct dsa_port *cpu_dp;
+	int err;
 
 	if (tag_ops->proto == dst->default_proto)
 		return 0;
 
-	for (port = 0; port < ds->num_ports; port++) {
-		if (!dsa_is_cpu_port(ds, port))
-			continue;
-
-		err = ds->ops->change_tag_protocol(ds, port, tag_ops->proto);
+	dsa_switch_for_each_cpu_port(cpu_dp, ds) {
+		err = ds->ops->change_tag_protocol(ds, cpu_dp->index,
+						   tag_ops->proto);
 		if (err) {
 			dev_err(ds->dev, "Unable to use tag protocol \"%s\": %pe\n",
 				tag_ops->name, ERR_PTR(err));
@@ -1040,7 +1039,7 @@ int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
 		goto out_unlock;
 
 	list_for_each_entry(dp, &dst->ports, list) {
-		if (!dsa_is_user_port(dp->ds, dp->index))
+		if (!dsa_port_is_user(dp))
 			continue;
 
 		if (dp->slave->flags & IFF_UP)
diff --git a/net/dsa/port.c b/net/dsa/port.c
index eedd9881e1ba..05b902d69863 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -540,7 +540,8 @@ static bool dsa_port_can_apply_vlan_filtering(struct dsa_port *dp,
 					      struct netlink_ext_ack *extack)
 {
 	struct dsa_switch *ds = dp->ds;
-	int err, i;
+	struct dsa_port *other_dp;
+	int err;
 
 	/* VLAN awareness was off, so the question is "can we turn it on".
 	 * We may have had 8021q uppers, those need to go. Make sure we don't
@@ -582,10 +583,10 @@ static bool dsa_port_can_apply_vlan_filtering(struct dsa_port *dp,
 	 * different ports of the same switch device and one of them has a
 	 * different setting than what is being requested.
 	 */
-	for (i = 0; i < ds->num_ports; i++) {
+	dsa_switch_for_each_port(other_dp, ds) {
 		struct net_device *other_bridge;
 
-		other_bridge = dsa_to_port(ds, i)->bridge_dev;
+		other_bridge = other_dp->bridge_dev;
 		if (!other_bridge)
 			continue;
 		/* If it's the same bridge, it also has same
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 7674f788d563..2f6a2e7d24c0 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -2266,7 +2266,7 @@ static int dsa_slave_netdevice_event(struct notifier_block *nb,
 		dst = cpu_dp->ds->dst;
 
 		list_for_each_entry(dp, &dst->ports, list) {
-			if (!dsa_is_user_port(dp->ds, dp->index))
+			if (!dsa_port_is_user(dp))
 				continue;
 
 			list_add(&dp->slave->close_list, &close_list);
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index fd1a1c6bf9cf..5ddcf37ecfa5 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -17,14 +17,11 @@
 static unsigned int dsa_switch_fastest_ageing_time(struct dsa_switch *ds,
 						   unsigned int ageing_time)
 {
-	int i;
-
-	for (i = 0; i < ds->num_ports; ++i) {
-		struct dsa_port *dp = dsa_to_port(ds, i);
+	struct dsa_port *dp;
 
+	dsa_switch_for_each_port(dp, ds)
 		if (dp->ageing_time && dp->ageing_time < ageing_time)
 			ageing_time = dp->ageing_time;
-	}
 
 	return ageing_time;
 }
@@ -117,7 +114,8 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds,
 	bool unset_vlan_filtering = br_vlan_enabled(info->br);
 	struct dsa_switch_tree *dst = ds->dst;
 	struct netlink_ext_ack extack = {0};
-	int err, port;
+	struct dsa_port *dp;
+	int err;
 
 	if (dst->index == info->tree_index && ds->index == info->sw_index &&
 	    ds->ops->port_bridge_leave)
@@ -138,10 +136,10 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds,
 	 * VLAN-aware bridge.
 	 */
 	if (unset_vlan_filtering && ds->vlan_filtering_is_global) {
-		for (port = 0; port < ds->num_ports; port++) {
+		dsa_switch_for_each_port(dp, ds) {
 			struct net_device *bridge_dev;
 
-			bridge_dev = dsa_to_port(ds, port)->bridge_dev;
+			bridge_dev = dp->bridge_dev;
 
 			if (bridge_dev && br_vlan_enabled(bridge_dev)) {
 				unset_vlan_filtering = false;
@@ -566,38 +564,35 @@ static int dsa_switch_change_tag_proto(struct dsa_switch *ds,
 				       struct dsa_notifier_tag_proto_info *info)
 {
 	const struct dsa_device_ops *tag_ops = info->tag_ops;
-	int port, err;
+	struct dsa_port *dp, *cpu_dp;
+	int err;
 
 	if (!ds->ops->change_tag_protocol)
 		return -EOPNOTSUPP;
 
 	ASSERT_RTNL();
 
-	for (port = 0; port < ds->num_ports; port++) {
-		if (!dsa_is_cpu_port(ds, port))
-			continue;
-
-		err = ds->ops->change_tag_protocol(ds, port, tag_ops->proto);
+	dsa_switch_for_each_cpu_port(cpu_dp, ds) {
+		err = ds->ops->change_tag_protocol(ds, cpu_dp->index,
+						   tag_ops->proto);
 		if (err)
 			return err;
 
-		dsa_port_set_tag_protocol(dsa_to_port(ds, port), tag_ops);
+		dsa_port_set_tag_protocol(cpu_dp, tag_ops);
 	}
 
 	/* Now that changing the tag protocol can no longer fail, let's update
 	 * the remaining bits which are "duplicated for faster access", and the
 	 * bits that depend on the tagger, such as the MTU.
 	 */
-	for (port = 0; port < ds->num_ports; port++) {
-		if (dsa_is_user_port(ds, port)) {
-			struct net_device *slave;
+	dsa_switch_for_each_user_port(dp, ds) {
+		struct net_device *slave;
 
-			slave = dsa_to_port(ds, port)->slave;
-			dsa_slave_setup_tagger(slave);
+		slave = dp->slave;
+		dsa_slave_setup_tagger(slave);
 
-			/* rtnl_mutex is held in dsa_tree_change_tag_proto */
-			dsa_slave_change_mtu(slave, slave->mtu);
-		}
+		/* rtnl_mutex is held in dsa_tree_change_tag_proto */
+		dsa_slave_change_mtu(slave, slave->mtu);
 	}
 
 	return 0;
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index 654697ebb6f3..b29f4eb9aed1 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -322,15 +322,13 @@ int dsa_switch_tag_8021q_vlan_del(struct dsa_switch *ds,
  * +-+-----+-+-----+-+-----+-+-----+-+    +-+-----+-+-----+-+-----+-+-----+-+
  *   swp0    swp1    swp2    swp3           swp0    swp1    swp2    swp3
  */
-static bool dsa_tag_8021q_bridge_match(struct dsa_switch *ds, int port,
+static bool dsa_tag_8021q_bridge_match(struct dsa_port *dp,
 				       struct dsa_notifier_bridge_info *info)
 {
-	struct dsa_port *dp = dsa_to_port(ds, port);
-
 	/* Don't match on self */
-	if (ds->dst->index == info->tree_index &&
-	    ds->index == info->sw_index &&
-	    port == info->port)
+	if (dp->ds->dst->index == info->tree_index &&
+	    dp->ds->index == info->sw_index &&
+	    dp->index == info->port)
 		return false;
 
 	if (dsa_port_is_user(dp))
@@ -344,8 +342,9 @@ int dsa_tag_8021q_bridge_join(struct dsa_switch *ds,
 {
 	struct dsa_switch *targeted_ds;
 	struct dsa_port *targeted_dp;
+	struct dsa_port *dp;
 	u16 targeted_rx_vid;
-	int err, port;
+	int err;
 
 	if (!ds->tag_8021q_ctx)
 		return 0;
@@ -354,11 +353,10 @@ int dsa_tag_8021q_bridge_join(struct dsa_switch *ds,
 	targeted_dp = dsa_to_port(targeted_ds, info->port);
 	targeted_rx_vid = dsa_8021q_rx_vid(targeted_ds, info->port);
 
-	for (port = 0; port < ds->num_ports; port++) {
-		struct dsa_port *dp = dsa_to_port(ds, port);
-		u16 rx_vid = dsa_8021q_rx_vid(ds, port);
+	dsa_switch_for_each_port(dp, ds) {
+		u16 rx_vid = dsa_8021q_rx_vid(ds, dp->index);
 
-		if (!dsa_tag_8021q_bridge_match(ds, port, info))
+		if (!dsa_tag_8021q_bridge_match(dp, info))
 			continue;
 
 		/* Install the RX VID of the targeted port in our VLAN table */
@@ -380,8 +378,8 @@ int dsa_tag_8021q_bridge_leave(struct dsa_switch *ds,
 {
 	struct dsa_switch *targeted_ds;
 	struct dsa_port *targeted_dp;
+	struct dsa_port *dp;
 	u16 targeted_rx_vid;
-	int port;
 
 	if (!ds->tag_8021q_ctx)
 		return 0;
@@ -390,11 +388,10 @@ int dsa_tag_8021q_bridge_leave(struct dsa_switch *ds,
 	targeted_dp = dsa_to_port(targeted_ds, info->port);
 	targeted_rx_vid = dsa_8021q_rx_vid(targeted_ds, info->port);
 
-	for (port = 0; port < ds->num_ports; port++) {
-		struct dsa_port *dp = dsa_to_port(ds, port);
-		u16 rx_vid = dsa_8021q_rx_vid(ds, port);
+	dsa_switch_for_each_port(dp, ds) {
+		u16 rx_vid = dsa_8021q_rx_vid(ds, dp->index);
 
-		if (!dsa_tag_8021q_bridge_match(ds, port, info))
+		if (!dsa_tag_8021q_bridge_match(dp, info))
 			continue;
 
 		/* Remove the RX VID of the targeted port from our VLAN table */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [RFC PATCH v2 net-next 3/8] net: dsa: remove the "dsa_to_port in a loop" antipattern from drivers
  2021-08-10 16:14 [RFC PATCH v2 net-next 0/8] Remove the "dsa_to_port in a loop" antipattern Vladimir Oltean
  2021-08-10 16:14 ` [RFC PATCH v2 net-next 1/8] net: dsa: introduce a dsa_port_is_unused helper Vladimir Oltean
  2021-08-10 16:14 ` [RFC PATCH v2 net-next 2/8] net: dsa: remove the "dsa_to_port in a loop" antipattern from the core Vladimir Oltean
@ 2021-08-10 16:14 ` Vladimir Oltean
  2021-08-11  8:57   ` kernel test robot
  2021-08-10 16:14 ` [RFC PATCH v2 net-next 4/8] net: dsa: b53: express b53_for_each_port in terms of dsa_switch_for_each_port Vladimir Oltean
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 11+ messages in thread
From: Vladimir Oltean @ 2021-08-10 16:14 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Woojung Huh, UNGLinuxDriver, Sean Wang,
	Landen Chao, DENG Qingfang, Matthias Brugger, Claudiu Manoil,
	Alexandre Belloni, George McCollister

Since the DSA conversion from the ds->ports array into the dst->ports
list, the DSA API has encouraged driver writers to write inefficient
code.

Currently, switch drivers which want to filter by a specific type of
port when iterating, like {!unused, user, cpu, dsa}, use the
dsa_is_*_port helper. Under the hood, this uses dsa_to_port which
iterates again through dst->ports. But the driver iterates through the
port list already, so the complexity is quadratic for the typical case
of a single-switch tree.

Many drivers also call dsa_to_port many times while iterating and do not
even cache the result, probably unaware of the hidden complexity of this
function.

When drivers need to iterate between pairs of {to, from} ports, and use
any of the functions above, the complexity is even higher.

Use the newly introduced DSA port iterators, and also introduce some
more which are not directly needed by the DSA core.

Note that the b53_br_{join,leave} functions have been converted to use
the dp-based iterators, but to preserve the original behavior, the
dev->enabled_ports check from b53_for_each_port has been split out and
open-coded. This will be addressed in the next patch.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: remove an unused "int port" variable in felix_vsc9959.c

 drivers/net/dsa/b53/b53_common.c              |  22 ++-
 drivers/net/dsa/bcm_sf2.c                     |   8 +-
 drivers/net/dsa/hirschmann/hellcreek.c        |  27 +--
 .../net/dsa/hirschmann/hellcreek_hwtstamp.c   |  19 +-
 drivers/net/dsa/microchip/ksz9477.c           |  19 +-
 drivers/net/dsa/microchip/ksz_common.c        |  19 +-
 drivers/net/dsa/mt7530.c                      |  58 +++---
 drivers/net/dsa/mv88e6xxx/chip.c              |  37 ++--
 drivers/net/dsa/mv88e6xxx/hwtstamp.c          |  10 +-
 drivers/net/dsa/mv88e6xxx/port.c              |  12 +-
 drivers/net/dsa/ocelot/felix.c                |  79 +++-----
 drivers/net/dsa/ocelot/felix_vsc9959.c        |  11 +-
 drivers/net/dsa/ocelot/seville_vsc9953.c      |  14 +-
 drivers/net/dsa/qca8k.c                       |  32 ++--
 drivers/net/dsa/sja1105/sja1105_main.c        | 176 ++++++++----------
 drivers/net/dsa/sja1105/sja1105_mdio.c        |  12 +-
 drivers/net/dsa/xrs700x/xrs700x.c             |  37 ++--
 include/net/dsa.h                             |  19 ++
 18 files changed, 284 insertions(+), 327 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index bd1417a66cbf..ccd93147d994 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1851,6 +1851,7 @@ int b53_br_join(struct dsa_switch *ds, int port, struct net_device *br)
 {
 	struct b53_device *dev = ds->priv;
 	s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
+	struct dsa_port *dp;
 	u16 pvlan, reg;
 	unsigned int i;
 
@@ -1873,8 +1874,13 @@ int b53_br_join(struct dsa_switch *ds, int port, struct net_device *br)
 
 	b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), &pvlan);
 
-	b53_for_each_port(dev, i) {
-		if (dsa_to_port(ds, i)->bridge_dev != br)
+	dsa_switch_for_each_port(dp, ds) {
+		i = dp->index;
+
+		if (!(dev->enabled_ports & BIT(i)))
+			continue;
+
+		if (dp->bridge_dev != br)
 			continue;
 
 		/* Add this local port to the remote port VLAN control
@@ -1903,14 +1909,20 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct net_device *br)
 	struct b53_device *dev = ds->priv;
 	struct b53_vlan *vl = &dev->vlans[0];
 	s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
-	unsigned int i;
 	u16 pvlan, reg, pvid;
+	struct dsa_port *dp;
+	unsigned int i;
 
 	b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), &pvlan);
 
-	b53_for_each_port(dev, i) {
+	dsa_switch_for_each_port(dp, ds) {
+		i = dp->index;
+
+		if (!(dev->enabled_ports & BIT(i)))
+			continue;
+
 		/* Don't touch the remaining ports */
-		if (dsa_to_port(ds, i)->bridge_dev != br)
+		if (dp->bridge_dev != br)
 			continue;
 
 		b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(i), &reg);
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 6ce9ec1283e0..2f78fb88ed9e 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -913,7 +913,7 @@ static void bcm_sf2_enable_acb(struct dsa_switch *ds)
 static int bcm_sf2_sw_suspend(struct dsa_switch *ds)
 {
 	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
-	unsigned int port;
+	struct dsa_port *dp;
 
 	bcm_sf2_intr_disable(priv);
 
@@ -921,10 +921,8 @@ static int bcm_sf2_sw_suspend(struct dsa_switch *ds)
 	 * port, the other ones have already been disabled during
 	 * bcm_sf2_sw_setup
 	 */
-	for (port = 0; port < ds->num_ports; port++) {
-		if (dsa_is_user_port(ds, port) || dsa_is_cpu_port(ds, port))
-			bcm_sf2_port_disable(ds, port);
-	}
+	dsa_switch_for_each_available_port(dp, ds)
+		bcm_sf2_port_disable(ds, dp->index);
 
 	if (!priv->wol_ports_mask)
 		clk_disable_unprepare(priv->clk);
diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index 5c54ae1be62c..c439c4b09211 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -345,7 +345,7 @@ static int hellcreek_vlan_prepare(struct dsa_switch *ds, int port,
 				  struct netlink_ext_ack *extack)
 {
 	struct hellcreek *hellcreek = ds->priv;
-	int i;
+	struct dsa_port *dp;
 
 	dev_dbg(hellcreek->dev, "VLAN prepare for port %d\n", port);
 
@@ -353,11 +353,8 @@ static int hellcreek_vlan_prepare(struct dsa_switch *ds, int port,
 	 * VLANs are internally used by the driver to ensure port
 	 * separation. Thus, they cannot be used by someone else.
 	 */
-	for (i = 0; i < hellcreek->pdata->num_ports; ++i) {
-		const u16 restricted_vid = hellcreek_private_vid(i);
-
-		if (!dsa_is_user_port(ds, i))
-			continue;
+	dsa_switch_for_each_user_port(dp, ds) {
+		const u16 restricted_vid = hellcreek_private_vid(dp->index);
 
 		if (vlan->vid == restricted_vid) {
 			NL_SET_ERR_MSG_MOD(extack, "VID restricted by driver");
@@ -1304,8 +1301,9 @@ static void hellcreek_teardown_devlink_regions(struct dsa_switch *ds)
 static int hellcreek_setup(struct dsa_switch *ds)
 {
 	struct hellcreek *hellcreek = ds->priv;
+	struct dsa_port *dp;
 	u16 swcfg = 0;
-	int ret, i;
+	int ret;
 
 	dev_dbg(hellcreek->dev, "Set up the switch\n");
 
@@ -1331,12 +1329,8 @@ static int hellcreek_setup(struct dsa_switch *ds)
 	hellcreek_write(hellcreek, swcfg, HR_SWCFG);
 
 	/* Initial vlan membership to reflect port separation */
-	for (i = 0; i < ds->num_ports; ++i) {
-		if (!dsa_is_user_port(ds, i))
-			continue;
-
-		hellcreek_setup_vlan_membership(ds, i, true);
-	}
+	dsa_switch_for_each_user_port(dp, ds)
+		hellcreek_setup_vlan_membership(ds, dp->index, true);
 
 	/* Configure PCP <-> TC mapping */
 	hellcreek_setup_tc_identity_mapping(hellcreek);
@@ -1413,10 +1407,10 @@ hellcreek_port_prechangeupper(struct dsa_switch *ds, int port,
 			      struct netdev_notifier_changeupper_info *info)
 {
 	struct hellcreek *hellcreek = ds->priv;
+	struct dsa_port *dp;
 	bool used = true;
 	int ret = -EBUSY;
 	u16 vid;
-	int i;
 
 	dev_dbg(hellcreek->dev, "Pre change upper for port %d\n", port);
 
@@ -1435,9 +1429,8 @@ hellcreek_port_prechangeupper(struct dsa_switch *ds, int port,
 
 	/* For all ports, check bitmaps */
 	mutex_lock(&hellcreek->vlan_lock);
-	for (i = 0; i < hellcreek->pdata->num_ports; ++i) {
-		if (!dsa_is_user_port(ds, i))
-			continue;
+	dsa_switch_for_each_user_port(dp, ds) {
+		int i = dp->index;
 
 		if (port == i)
 			continue;
diff --git a/drivers/net/dsa/hirschmann/hellcreek_hwtstamp.c b/drivers/net/dsa/hirschmann/hellcreek_hwtstamp.c
index 40b41c794dfa..1cbae1654dfe 100644
--- a/drivers/net/dsa/hirschmann/hellcreek_hwtstamp.c
+++ b/drivers/net/dsa/hirschmann/hellcreek_hwtstamp.c
@@ -354,13 +354,12 @@ long hellcreek_hwtstamp_work(struct ptp_clock_info *ptp)
 {
 	struct hellcreek *hellcreek = ptp_to_hellcreek(ptp);
 	struct dsa_switch *ds = hellcreek->ds;
-	int i, restart = 0;
+	struct dsa_port *dp;
+	int restart = 0;
 
-	for (i = 0; i < ds->num_ports; i++) {
+	dsa_switch_for_each_user_port(dp, ds) {
 		struct hellcreek_port_hwtstamp *ps;
-
-		if (!dsa_is_user_port(ds, i))
-			continue;
+		int i = dp->index;
 
 		ps = &hellcreek->ports[i].port_hwtstamp;
 
@@ -459,15 +458,11 @@ static void hellcreek_hwtstamp_port_setup(struct hellcreek *hellcreek, int port)
 int hellcreek_hwtstamp_setup(struct hellcreek *hellcreek)
 {
 	struct dsa_switch *ds = hellcreek->ds;
-	int i;
+	struct dsa_port *dp;
 
 	/* Initialize timestamping ports. */
-	for (i = 0; i < ds->num_ports; ++i) {
-		if (!dsa_is_user_port(ds, i))
-			continue;
-
-		hellcreek_hwtstamp_port_setup(hellcreek, i);
-	}
+	dsa_switch_for_each_user_port(dp, ds)
+		hellcreek_hwtstamp_port_setup(hellcreek, dp->index);
 
 	/* Select the synchronized clock as the source timekeeper for the
 	 * timestamps and enable inline timestamping.
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 854e25f43fa7..a7435c581e49 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -1266,11 +1266,14 @@ static void ksz9477_port_setup(struct ksz_device *dev, int port, bool cpu_port)
 static void ksz9477_config_cpu_port(struct dsa_switch *ds)
 {
 	struct ksz_device *dev = ds->priv;
+	struct dsa_port *dp;
 	struct ksz_port *p;
 	int i;
 
-	for (i = 0; i < dev->port_cnt; i++) {
-		if (dsa_is_cpu_port(ds, i) && (dev->cpu_ports & (1 << i))) {
+	dsa_switch_for_each_port(dp, ds) {
+		i = dp->index;
+
+		if (dsa_port_is_cpu(dp) && (dev->cpu_ports & (1 << i))) {
 			phy_interface_t interface;
 			const char *prev_msg;
 			const char *prev_mode;
@@ -1609,18 +1612,22 @@ static const struct ksz_dev_ops ksz9477_dev_ops = {
 
 int ksz9477_switch_register(struct ksz_device *dev)
 {
-	int ret, i;
 	struct phy_device *phydev;
+	struct dsa_switch *ds;
+	struct dsa_port *dp;
+	int ret;
 
 	ret = ksz_switch_register(dev, &ksz9477_dev_ops);
 	if (ret)
 		return ret;
 
-	for (i = 0; i < dev->phy_port_cnt; ++i) {
-		if (!dsa_is_user_port(dev->ds, i))
+	ds = dev->ds;
+
+	dsa_switch_for_each_user_port(dp, ds) {
+		if (dp->index >= dev->phy_port_cnt)
 			continue;
 
-		phydev = dsa_to_port(dev->ds, i)->slave->phydev;
+		phydev = dp->slave->phydev;
 
 		/* The MAC actually cannot run in 1000 half-duplex mode. */
 		phy_remove_link_mode(phydev,
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 1542bfb8b5e5..2b188f998115 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -68,28 +68,23 @@ static void ksz_mib_read_work(struct work_struct *work)
 {
 	struct ksz_device *dev = container_of(work, struct ksz_device,
 					      mib_read.work);
+	struct dsa_switch *ds = dev->ds;
 	struct ksz_port_mib *mib;
+	struct dsa_port *dp;
 	struct ksz_port *p;
-	int i;
-
-	for (i = 0; i < dev->port_cnt; i++) {
-		if (dsa_is_unused_port(dev->ds, i))
-			continue;
 
-		p = &dev->ports[i];
+	dsa_switch_for_each_available_port(dp, ds) {
+		p = &dev->ports[dp->index];
 		mib = &p->mib;
 		mutex_lock(&mib->cnt_mutex);
 
 		/* Only read MIB counters when the port is told to do.
 		 * If not, read only dropped counters when link is not up.
 		 */
-		if (!p->read) {
-			const struct dsa_port *dp = dsa_to_port(dev->ds, i);
+		if (!p->read && !netif_carrier_ok(dp->slave))
+			mib->cnt_ptr = dev->reg_mib_cnt;
 
-			if (!netif_carrier_ok(dp->slave))
-				mib->cnt_ptr = dev->reg_mib_cnt;
-		}
-		port_r_cnt(dev, i);
+		port_r_cnt(dev, dp->index);
 		p->read = false;
 		mutex_unlock(&mib->cnt_mutex);
 	}
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 53e6150e95b6..010d4bb7794f 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1195,25 +1195,27 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
 {
 	struct mt7530_priv *priv = ds->priv;
 	u32 port_bitmap = BIT(MT7530_CPU_PORT);
-	int i;
+	struct dsa_port *dp;
 
 	mutex_lock(&priv->reg_mutex);
 
-	for (i = 0; i < MT7530_NUM_PORTS; i++) {
+	dsa_switch_for_each_user_port(dp, ds) {
+		if (dp->index == port)
+			continue;
+
+		if (dp->bridge_dev != bridge)
+			continue;
+
 		/* Add this port to the port matrix of the other ports in the
 		 * same bridge. If the port is disabled, port matrix is kept
 		 * and not being setup until the port becomes enabled.
 		 */
-		if (dsa_is_user_port(ds, i) && i != port) {
-			if (dsa_to_port(ds, i)->bridge_dev != bridge)
-				continue;
-			if (priv->ports[i].enable)
-				mt7530_set(priv, MT7530_PCR_P(i),
-					   PCR_MATRIX(BIT(port)));
-			priv->ports[i].pm |= PCR_MATRIX(BIT(port));
+		if (priv->ports[dp->index].enable)
+			mt7530_set(priv, MT7530_PCR_P(dp->index),
+				   PCR_MATRIX(BIT(port)));
+		priv->ports[dp->index].pm |= PCR_MATRIX(BIT(port));
 
-			port_bitmap |= BIT(i);
-		}
+		port_bitmap |= BIT(dp->index);
 	}
 
 	/* Add the all other ports to this port matrix. */
@@ -1236,7 +1238,7 @@ mt7530_port_set_vlan_unaware(struct dsa_switch *ds, int port)
 {
 	struct mt7530_priv *priv = ds->priv;
 	bool all_user_ports_removed = true;
-	int i;
+	struct dsa_port *dp;
 
 	/* This is called after .port_bridge_leave when leaving a VLAN-aware
 	 * bridge. Don't set standalone ports to fallback mode.
@@ -1255,9 +1257,8 @@ mt7530_port_set_vlan_unaware(struct dsa_switch *ds, int port)
 	mt7530_rmw(priv, MT7530_PPBV1_P(port), G0_PORT_VID_MASK,
 		   G0_PORT_VID_DEF);
 
-	for (i = 0; i < MT7530_NUM_PORTS; i++) {
-		if (dsa_is_user_port(ds, i) &&
-		    dsa_port_is_vlan_filtering(dsa_to_port(ds, i))) {
+	dsa_switch_for_each_user_port(dp, ds) {
+		if (dsa_port_is_vlan_filtering(dp)) {
 			all_user_ports_removed = false;
 			break;
 		}
@@ -1307,26 +1308,31 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
 			 struct net_device *bridge)
 {
 	struct mt7530_priv *priv = ds->priv;
-	int i;
+	struct dsa_port *dp;
 
 	mutex_lock(&priv->reg_mutex);
 
-	for (i = 0; i < MT7530_NUM_PORTS; i++) {
+	dsa_switch_for_each_user_port(dp, ds) {
 		/* Remove this port from the port matrix of the other ports
 		 * in the same bridge. If the port is disabled, port matrix
 		 * is kept and not being setup until the port becomes enabled.
 		 * And the other port's port matrix cannot be broken when the
 		 * other port is still a VLAN-aware port.
 		 */
-		if (dsa_is_user_port(ds, i) && i != port &&
-		   !dsa_port_is_vlan_filtering(dsa_to_port(ds, i))) {
-			if (dsa_to_port(ds, i)->bridge_dev != bridge)
-				continue;
-			if (priv->ports[i].enable)
-				mt7530_clear(priv, MT7530_PCR_P(i),
-					     PCR_MATRIX(BIT(port)));
-			priv->ports[i].pm &= ~PCR_MATRIX(BIT(port));
-		}
+		if (dp->index == port)
+			continue;
+
+		if (dsa_port_is_vlan_filtering(dp))
+			continue;
+
+		if (dp->bridge_dev != bridge)
+			continue;
+
+		if (priv->ports[dp->index].enable)
+			mt7530_clear(priv, MT7530_PCR_P(dp->index),
+				     PCR_MATRIX(BIT(port)));
+
+		priv->ports[dp->index].pm &= ~PCR_MATRIX(BIT(port));
 	}
 
 	/* Set the cpu port to be the only one in the port matrix of
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index c45ca2473743..1525415aca46 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1635,9 +1635,11 @@ static int mv88e6xxx_atu_new(struct mv88e6xxx_chip *chip, u16 *fid)
 static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
 					u16 vid)
 {
+	struct dsa_port *dp = dsa_to_port(ds, port);
 	struct mv88e6xxx_chip *chip = ds->priv;
 	struct mv88e6xxx_vtu_entry vlan;
-	int i, err;
+	struct dsa_port *other_dp;
+	int err;
 
 	/* DSA and CPU ports have to be members of multiple vlans */
 	if (dsa_is_dsa_port(ds, port) || dsa_is_cpu_port(ds, port))
@@ -1650,27 +1652,20 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
 	if (!vlan.valid)
 		return 0;
 
-	for (i = 0; i < mv88e6xxx_num_ports(chip); ++i) {
-		if (dsa_is_dsa_port(ds, i) || dsa_is_cpu_port(ds, i))
-			continue;
-
-		if (!dsa_to_port(ds, i)->slave)
-			continue;
-
-		if (vlan.member[i] ==
+	dsa_switch_for_each_user_port(other_dp, ds) {
+		if (vlan.member[other_dp->index] ==
 		    MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_NON_MEMBER)
 			continue;
 
-		if (dsa_to_port(ds, i)->bridge_dev ==
-		    dsa_to_port(ds, port)->bridge_dev)
+		if (other_dp->bridge_dev == dp->bridge_dev)
 			break; /* same bridge, check next VLAN */
 
-		if (!dsa_to_port(ds, i)->bridge_dev)
+		if (!other_dp->bridge_dev)
 			continue;
 
 		dev_err(ds->dev, "p%d: hw VLAN %d already used by port %d in %s\n",
-			port, vlan.vid, i,
-			netdev_name(dsa_to_port(ds, i)->bridge_dev));
+			port, vlan.vid, other_dp->index,
+			netdev_name(other_dp->bridge_dev));
 		return -EOPNOTSUPP;
 	}
 
@@ -1996,16 +1991,14 @@ static int mv88e6xxx_port_add_broadcast(struct mv88e6xxx_chip *chip, int port,
 
 static int mv88e6xxx_broadcast_setup(struct mv88e6xxx_chip *chip, u16 vid)
 {
+	struct dsa_switch *ds = chip->ds;
+	struct dsa_port *dp;
 	int port;
 	int err;
 
-	for (port = 0; port < mv88e6xxx_num_ports(chip); port++) {
-		struct dsa_port *dp = dsa_to_port(chip->ds, port);
+	dsa_switch_for_each_available_port(dp, ds) {
 		struct net_device *brport;
 
-		if (dsa_is_unused_port(chip->ds, port))
-			continue;
-
 		brport = dsa_port_to_bridge_port(dp);
 		if (brport && !br_port_flag_is_set(brport, BR_BCAST_FLOOD))
 			/* Skip bridged user ports where broadcast
@@ -3077,6 +3070,7 @@ static void mv88e6xxx_teardown(struct dsa_switch *ds)
 static int mv88e6xxx_setup(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
+	struct dsa_port *dp;
 	u8 cmode;
 	int err;
 	int i;
@@ -3113,9 +3107,8 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
 	}
 
 	/* Setup Switch Port Registers */
-	for (i = 0; i < mv88e6xxx_num_ports(chip); i++) {
-		if (dsa_is_unused_port(ds, i))
-			continue;
+	dsa_switch_for_each_available_port(dp, ds) {
+		i = dp->index;
 
 		/* Prevent the use of an invalid port. */
 		if (mv88e6xxx_is_invalid_port(chip, i)) {
diff --git a/drivers/net/dsa/mv88e6xxx/hwtstamp.c b/drivers/net/dsa/mv88e6xxx/hwtstamp.c
index 8f74ffc7a279..73153249b4c0 100644
--- a/drivers/net/dsa/mv88e6xxx/hwtstamp.c
+++ b/drivers/net/dsa/mv88e6xxx/hwtstamp.c
@@ -452,13 +452,11 @@ long mv88e6xxx_hwtstamp_work(struct ptp_clock_info *ptp)
 	struct mv88e6xxx_chip *chip = ptp_to_chip(ptp);
 	struct dsa_switch *ds = chip->ds;
 	struct mv88e6xxx_port_hwtstamp *ps;
-	int i, restart = 0;
+	struct dsa_port *dp;
+	int restart = 0;
 
-	for (i = 0; i < ds->num_ports; i++) {
-		if (!dsa_is_user_port(ds, i))
-			continue;
-
-		ps = &chip->port_hwtstamp[i];
+	dsa_switch_for_each_user_port(dp, ds) {
+		ps = &chip->port_hwtstamp[dp->index];
 		if (test_bit(MV88E6XXX_HWTSTAMP_TX_IN_PROGRESS, &ps->state))
 			restart |= mv88e6xxx_txtstamp_work(chip, ps);
 
diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
index f77e2ee64a60..2c300ab70002 100644
--- a/drivers/net/dsa/mv88e6xxx/port.c
+++ b/drivers/net/dsa/mv88e6xxx/port.c
@@ -1381,13 +1381,13 @@ static int mv88e6393x_port_policy_write(struct mv88e6xxx_chip *chip, int port,
 static int mv88e6393x_port_policy_write_all(struct mv88e6xxx_chip *chip,
 					    u16 pointer, u8 data)
 {
-	int err, port;
-
-	for (port = 0; port < mv88e6xxx_num_ports(chip); port++) {
-		if (dsa_is_unused_port(chip->ds, port))
-			continue;
+	struct dsa_switch *ds = chip->ds;
+	struct dsa_port *dp;
+	int err;
 
-		err = mv88e6393x_port_policy_write(chip, port, pointer, data);
+	dsa_switch_for_each_available_port(dp, ds) {
+		err = mv88e6393x_port_policy_write(chip, dp->index, pointer,
+						   data);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 9505f9b3ac90..f4bd43545f04 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -266,12 +266,12 @@ static void felix_8021q_cpu_port_deinit(struct ocelot *ocelot, int port)
  */
 static int felix_setup_mmio_filtering(struct felix *felix)
 {
-	unsigned long user_ports = 0, cpu_ports = 0;
+	unsigned long user_ports = dsa_user_ports(felix->ds);
+	unsigned long cpu_ports = dsa_cpu_ports(felix->ds);
 	struct ocelot_vcap_filter *redirect_rule;
 	struct ocelot_vcap_filter *tagging_rule;
 	struct ocelot *ocelot = &felix->ocelot;
-	struct dsa_switch *ds = felix->ds;
-	int port, ret;
+	int ret;
 
 	tagging_rule = kzalloc(sizeof(struct ocelot_vcap_filter), GFP_KERNEL);
 	if (!tagging_rule)
@@ -283,13 +283,6 @@ static int felix_setup_mmio_filtering(struct felix *felix)
 		return -ENOMEM;
 	}
 
-	for (port = 0; port < ocelot->num_phys_ports; port++) {
-		if (dsa_is_user_port(ds, port))
-			user_ports |= BIT(port);
-		if (dsa_is_cpu_port(ds, port))
-			cpu_ports |= BIT(port);
-	}
-
 	tagging_rule->key_type = OCELOT_VCAP_KEY_ETYPE;
 	*(__be16 *)tagging_rule->key.etype.etype.value = htons(ETH_P_1588);
 	*(__be16 *)tagging_rule->key.etype.etype.mask = htons(0xffff);
@@ -388,14 +381,12 @@ static int felix_setup_tag_8021q(struct dsa_switch *ds, int cpu)
 	struct ocelot *ocelot = ds->priv;
 	struct felix *felix = ocelot_to_felix(ocelot);
 	unsigned long cpu_flood;
-	int port, err;
+	struct dsa_port *dp;
+	int err;
 
 	felix_8021q_cpu_port_init(ocelot, cpu);
 
-	for (port = 0; port < ds->num_ports; port++) {
-		if (dsa_is_unused_port(ds, port))
-			continue;
-
+	dsa_switch_for_each_available_port(dp, ds) {
 		/* This overwrites ocelot_init():
 		 * Do not forward BPDU frames to the CPU port module,
 		 * for 2 reasons:
@@ -408,7 +399,7 @@ static int felix_setup_tag_8021q(struct dsa_switch *ds, int cpu)
 		 */
 		ocelot_write_gix(ocelot,
 				 ANA_PORT_CPU_FWD_BPDU_CFG_BPDU_REDIR_ENA(0),
-				 ANA_PORT_CPU_FWD_BPDU_CFG, port);
+				 ANA_PORT_CPU_FWD_BPDU_CFG, dp->index);
 	}
 
 	/* In tag_8021q mode, the CPU port module is unused, except for PTP
@@ -439,7 +430,8 @@ static void felix_teardown_tag_8021q(struct dsa_switch *ds, int cpu)
 {
 	struct ocelot *ocelot = ds->priv;
 	struct felix *felix = ocelot_to_felix(ocelot);
-	int err, port;
+	struct dsa_port *dp;
+	int err;
 
 	err = felix_teardown_mmio_filtering(felix);
 	if (err)
@@ -448,17 +440,14 @@ static void felix_teardown_tag_8021q(struct dsa_switch *ds, int cpu)
 
 	dsa_tag_8021q_unregister(ds);
 
-	for (port = 0; port < ds->num_ports; port++) {
-		if (dsa_is_unused_port(ds, port))
-			continue;
-
+	dsa_switch_for_each_available_port(dp, ds) {
 		/* Restore the logic from ocelot_init:
 		 * do not forward BPDU frames to the front ports.
 		 */
 		ocelot_write_gix(ocelot,
 				 ANA_PORT_CPU_FWD_BPDU_CFG_BPDU_REDIR_ENA(0xffff),
 				 ANA_PORT_CPU_FWD_BPDU_CFG,
-				 port);
+				 dp->index);
 	}
 
 	felix_8021q_cpu_port_deinit(ocelot, cpu);
@@ -1178,7 +1167,8 @@ static int felix_setup(struct dsa_switch *ds)
 {
 	struct ocelot *ocelot = ds->priv;
 	struct felix *felix = ocelot_to_felix(ocelot);
-	int port, err;
+	struct dsa_port *dp;
+	int err;
 
 	err = felix_init_structs(felix, ds->num_ports);
 	if (err)
@@ -1197,31 +1187,24 @@ static int felix_setup(struct dsa_switch *ds)
 		}
 	}
 
-	for (port = 0; port < ds->num_ports; port++) {
-		if (dsa_is_unused_port(ds, port))
-			continue;
-
-		ocelot_init_port(ocelot, port);
+	dsa_switch_for_each_available_port(dp, ds) {
+		ocelot_init_port(ocelot, dp->index);
 
 		/* Set the default QoS Classification based on PCP and DEI
 		 * bits of vlan tag.
 		 */
-		felix_port_qos_map_init(ocelot, port);
+		felix_port_qos_map_init(ocelot, dp->index);
 	}
 
 	err = ocelot_devlink_sb_register(ocelot);
 	if (err)
 		goto out_deinit_ports;
 
-	for (port = 0; port < ds->num_ports; port++) {
-		if (!dsa_is_cpu_port(ds, port))
-			continue;
-
+	dsa_switch_for_each_cpu_port(dp, ds)
 		/* The initial tag protocol is NPI which always returns 0, so
 		 * there's no real point in checking for errors.
 		 */
-		felix_set_tag_protocol(ds, port, felix->tag_proto);
-	}
+		felix_set_tag_protocol(ds, dp->index, felix->tag_proto);
 
 	ds->mtu_enforcement_ingress = true;
 	ds->assisted_learning_on_cpu_port = true;
@@ -1229,12 +1212,8 @@ static int felix_setup(struct dsa_switch *ds)
 	return 0;
 
 out_deinit_ports:
-	for (port = 0; port < ocelot->num_phys_ports; port++) {
-		if (dsa_is_unused_port(ds, port))
-			continue;
-
-		ocelot_deinit_port(ocelot, port);
-	}
+	dsa_switch_for_each_available_port(dp, ds)
+		ocelot_deinit_port(ocelot, dp->index);
 
 	ocelot_deinit_timestamp(ocelot);
 	ocelot_deinit(ocelot);
@@ -1250,25 +1229,17 @@ static void felix_teardown(struct dsa_switch *ds)
 {
 	struct ocelot *ocelot = ds->priv;
 	struct felix *felix = ocelot_to_felix(ocelot);
-	int port;
-
-	for (port = 0; port < ds->num_ports; port++) {
-		if (!dsa_is_cpu_port(ds, port))
-			continue;
+	struct dsa_port *dp;
 
-		felix_del_tag_protocol(ds, port, felix->tag_proto);
-	}
+	dsa_switch_for_each_cpu_port(dp, ds)
+		felix_del_tag_protocol(ds, dp->index, felix->tag_proto);
 
 	ocelot_devlink_sb_unregister(ocelot);
 	ocelot_deinit_timestamp(ocelot);
 	ocelot_deinit(ocelot);
 
-	for (port = 0; port < ocelot->num_phys_ports; port++) {
-		if (dsa_is_unused_port(ds, port))
-			continue;
-
-		ocelot_deinit_port(ocelot, port);
-	}
+	dsa_switch_for_each_available_port(dp, ds)
+		ocelot_deinit_port(ocelot, dp->index);
 
 	if (felix->info->mdio_bus_free)
 		felix->info->mdio_bus_free(ocelot);
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index f966a253d1c7..493ffa40ee40 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -1034,13 +1034,14 @@ static const struct ocelot_ops vsc9959_ops = {
 static int vsc9959_mdio_bus_alloc(struct ocelot *ocelot)
 {
 	struct felix *felix = ocelot_to_felix(ocelot);
+	struct dsa_switch *ds = felix->ds;
 	struct enetc_mdio_priv *mdio_priv;
 	struct device *dev = ocelot->dev;
 	void __iomem *imdio_regs;
 	struct resource res;
 	struct enetc_hw *hw;
 	struct mii_bus *bus;
-	int port;
+	struct dsa_port *dp;
 	int rc;
 
 	felix->pcs = devm_kcalloc(dev, felix->info->num_ports,
@@ -1091,13 +1092,11 @@ static int vsc9959_mdio_bus_alloc(struct ocelot *ocelot)
 
 	felix->imdio = bus;
 
-	for (port = 0; port < felix->info->num_ports; port++) {
-		struct ocelot_port *ocelot_port = ocelot->ports[port];
+	dsa_switch_for_each_available_port(dp, ds) {
+		struct ocelot_port *ocelot_port = ocelot->ports[dp->index];
 		struct mdio_device *pcs;
 		struct lynx_pcs *lynx;
-
-		if (dsa_is_unused_port(felix->ds, port))
-			continue;
+		int port = dp->index;
 
 		if (ocelot_port->phy_mode == PHY_INTERFACE_MODE_INTERNAL)
 			continue;
diff --git a/drivers/net/dsa/ocelot/seville_vsc9953.c b/drivers/net/dsa/ocelot/seville_vsc9953.c
index deae923c8b7a..fe7828c402bd 100644
--- a/drivers/net/dsa/ocelot/seville_vsc9953.c
+++ b/drivers/net/dsa/ocelot/seville_vsc9953.c
@@ -1085,9 +1085,10 @@ static const struct ocelot_ops vsc9953_ops = {
 static int vsc9953_mdio_bus_alloc(struct ocelot *ocelot)
 {
 	struct felix *felix = ocelot_to_felix(ocelot);
+	struct dsa_switch *ds = felix->ds;
 	struct device *dev = ocelot->dev;
+	struct dsa_port *dp;
 	struct mii_bus *bus;
-	int port;
 	int rc;
 
 	felix->pcs = devm_kcalloc(dev, felix->info->num_ports,
@@ -1118,15 +1119,12 @@ static int vsc9953_mdio_bus_alloc(struct ocelot *ocelot)
 
 	felix->imdio = bus;
 
-	for (port = 0; port < felix->info->num_ports; port++) {
-		struct ocelot_port *ocelot_port = ocelot->ports[port];
-		int addr = port + 4;
+	dsa_switch_for_each_available_port(dp, ds) {
+		struct ocelot_port *ocelot_port = ocelot->ports[dp->index];
+		int addr = dp->index + 4;
 		struct mdio_device *pcs;
 		struct lynx_pcs *lynx;
 
-		if (dsa_is_unused_port(felix->ds, port))
-			continue;
-
 		if (ocelot_port->phy_mode == PHY_INTERFACE_MODE_INTERNAL)
 			continue;
 
@@ -1140,7 +1138,7 @@ static int vsc9953_mdio_bus_alloc(struct ocelot *ocelot)
 			continue;
 		}
 
-		felix->pcs[port] = lynx;
+		felix->pcs[dp->index] = lynx;
 
 		dev_info(dev, "Found PCS at internal MDIO address %d\n", addr);
 	}
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 1f63f50f73f1..f2c1369c2bff 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -940,6 +940,7 @@ static int
 qca8k_setup(struct dsa_switch *ds)
 {
 	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
+	struct dsa_port *dp;
 	int ret, i;
 	u32 mask;
 
@@ -1009,9 +1010,11 @@ qca8k_setup(struct dsa_switch *ds)
 		return ret;
 
 	/* Setup connection between CPU port & user ports */
-	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
+	dsa_switch_for_each_port(dp, ds) {
+		i = dp->index;
+
 		/* CPU port gets connected to all user ports of the switch */
-		if (dsa_is_cpu_port(ds, i)) {
+		if (dsa_port_is_cpu(dp)) {
 			ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(QCA8K_CPU_PORT),
 					QCA8K_PORT_LOOKUP_MEMBER, dsa_user_ports(ds));
 			if (ret)
@@ -1019,7 +1022,7 @@ qca8k_setup(struct dsa_switch *ds)
 		}
 
 		/* Individual user ports get connected to CPU port only */
-		if (dsa_is_user_port(ds, i)) {
+		if (dsa_port_is_user(dp)) {
 			int shift = 16 * (i % 2);
 
 			ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(i),
@@ -1509,21 +1512,23 @@ qca8k_port_bridge_join(struct dsa_switch *ds, int port, struct net_device *br)
 {
 	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
 	int port_mask = BIT(QCA8K_CPU_PORT);
-	int i, ret;
+	struct dsa_port *dp;
+	int ret;
 
-	for (i = 1; i < QCA8K_NUM_PORTS; i++) {
-		if (dsa_to_port(ds, i)->bridge_dev != br)
+	dsa_switch_for_each_port(dp, ds) {
+		if (dp->bridge_dev != br)
 			continue;
+
 		/* Add this port to the portvlan mask of the other ports
 		 * in the bridge
 		 */
 		ret = qca8k_reg_set(priv,
-				    QCA8K_PORT_LOOKUP_CTRL(i),
+				    QCA8K_PORT_LOOKUP_CTRL(dp->index),
 				    BIT(port));
 		if (ret)
 			return ret;
-		if (i != port)
-			port_mask |= BIT(i);
+		if (dp->index != port)
+			port_mask |= BIT(dp->index);
 	}
 
 	/* Add all other ports to this ports portvlan mask */
@@ -1537,16 +1542,17 @@ static void
 qca8k_port_bridge_leave(struct dsa_switch *ds, int port, struct net_device *br)
 {
 	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
-	int i;
+	struct dsa_port *dp;
 
-	for (i = 1; i < QCA8K_NUM_PORTS; i++) {
-		if (dsa_to_port(ds, i)->bridge_dev != br)
+	dsa_switch_for_each_port(dp, ds) {
+		if (dp->bridge_dev != br)
 			continue;
+
 		/* Remove this port to the portvlan mask of the other ports
 		 * in the bridge
 		 */
 		qca8k_reg_clear(priv,
-				QCA8K_PORT_LOOKUP_CTRL(i),
+				QCA8K_PORT_LOOKUP_CTRL(dp->index),
 				BIT(port));
 	}
 
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index f7219bca45e9..cd6e3f40d474 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -207,10 +207,7 @@ static int sja1105_init_mac_settings(struct sja1105_private *priv)
 
 	mac = table->entries;
 
-	list_for_each_entry(dp, &ds->dst->ports, list) {
-		if (dp->ds != ds)
-			continue;
-
+	dsa_switch_for_each_available_port(dp, ds) {
 		mac[dp->index] = default_mac;
 
 		/* Let sja1105_bridge_stp_state_set() keep address learning
@@ -239,7 +236,7 @@ static int sja1105_init_mii_settings(struct sja1105_private *priv)
 	struct sja1105_xmii_params_entry *mii;
 	struct dsa_switch *ds = priv->ds;
 	struct sja1105_table *table;
-	int i;
+	struct dsa_port *dp;
 
 	table = &priv->static_config.tables[BLK_IDX_XMII_PARAMS];
 
@@ -259,11 +256,9 @@ static int sja1105_init_mii_settings(struct sja1105_private *priv)
 
 	mii = table->entries;
 
-	for (i = 0; i < ds->num_ports; i++) {
+	dsa_switch_for_each_available_port(dp, ds) {
 		sja1105_mii_role_t role = XMII_MAC;
-
-		if (dsa_is_unused_port(priv->ds, i))
-			continue;
+		int i = dp->index;
 
 		switch (priv->phy_mode[i]) {
 		case PHY_INTERFACE_MODE_INTERNAL:
@@ -331,8 +326,9 @@ static int sja1105_init_mii_settings(struct sja1105_private *priv)
 static int sja1105_init_static_fdb(struct sja1105_private *priv)
 {
 	struct sja1105_l2_lookup_entry *l2_lookup;
+	struct dsa_switch *ds = priv->ds;
 	struct sja1105_table *table;
-	int port;
+	struct dsa_port *dp;
 
 	table = &priv->static_config.tables[BLK_IDX_L2_LOOKUP];
 
@@ -363,9 +359,8 @@ static int sja1105_init_static_fdb(struct sja1105_private *priv)
 	l2_lookup[0].index = SJA1105_MAX_L2_LOOKUP_COUNT - 1;
 
 	/* Flood multicast to every port by default */
-	for (port = 0; port < priv->ds->num_ports; port++)
-		if (!dsa_is_unused_port(priv->ds, port))
-			l2_lookup[0].destports |= BIT(port);
+	dsa_switch_for_each_available_port(dp, ds)
+		l2_lookup[0].destports |= BIT(dp->index);
 
 	return 0;
 }
@@ -405,20 +400,16 @@ static int sja1105_init_l2_lookup_params(struct sja1105_private *priv)
 	struct dsa_switch *ds = priv->ds;
 	int port, num_used_ports = 0;
 	struct sja1105_table *table;
+	struct dsa_port *dp;
 	u64 max_fdb_entries;
 
-	for (port = 0; port < ds->num_ports; port++)
-		if (!dsa_is_unused_port(ds, port))
-			num_used_ports++;
+	dsa_switch_for_each_available_port(dp, ds)
+		num_used_ports++;
 
 	max_fdb_entries = SJA1105_MAX_L2_LOOKUP_COUNT / num_used_ports;
 
-	for (port = 0; port < ds->num_ports; port++) {
-		if (dsa_is_unused_port(ds, port))
-			continue;
-
+	dsa_switch_for_each_available_port(dp, ds)
 		default_l2_lookup_params.maxaddrp[port] = max_fdb_entries;
-	}
 
 	table = &priv->static_config.tables[BLK_IDX_L2_LOOKUP_PARAMS];
 
@@ -461,7 +452,7 @@ static int sja1105_init_static_vlan(struct sja1105_private *priv)
 		.vlanid = SJA1105_DEFAULT_VLAN,
 	};
 	struct dsa_switch *ds = priv->ds;
-	int port;
+	struct dsa_port *dp;
 
 	table = &priv->static_config.tables[BLK_IDX_VLAN_LOOKUP];
 
@@ -477,15 +468,14 @@ static int sja1105_init_static_vlan(struct sja1105_private *priv)
 
 	table->entry_count = 1;
 
-	for (port = 0; port < ds->num_ports; port++) {
-		if (dsa_is_unused_port(ds, port))
-			continue;
+	dsa_switch_for_each_available_port(dp, ds) {
+		int port = dp->index;
 
 		pvid.vmemb_port |= BIT(port);
 		pvid.vlan_bc |= BIT(port);
 		pvid.tag_port &= ~BIT(port);
 
-		if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)) {
+		if (dsa_port_is_cpu(dp) || dsa_port_is_dsa(dp)) {
 			priv->tag_8021q_pvid[port] = SJA1105_DEFAULT_VLAN;
 			priv->bridge_pvid[port] = SJA1105_DEFAULT_VLAN;
 		}
@@ -498,12 +488,12 @@ static int sja1105_init_static_vlan(struct sja1105_private *priv)
 static int sja1105_init_l2_forwarding(struct sja1105_private *priv)
 {
 	struct sja1105_l2_forwarding_entry *l2fwd;
+	struct dsa_port *dp, *from_dp, *to_dp;
 	struct dsa_switch *ds = priv->ds;
 	struct dsa_switch_tree *dst;
 	struct sja1105_table *table;
 	struct dsa_link *dl;
-	int port, tc;
-	int from, to;
+	int tc;
 
 	table = &priv->static_config.tables[BLK_IDX_L2_FORWARDING];
 
@@ -525,24 +515,21 @@ static int sja1105_init_l2_forwarding(struct sja1105_private *priv)
 	 * rules and the VLAN PCP to ingress queue mapping.
 	 * Set up the ingress queue mapping first.
 	 */
-	for (port = 0; port < ds->num_ports; port++) {
-		if (dsa_is_unused_port(ds, port))
-			continue;
-
+	dsa_switch_for_each_available_port(dp, ds)
 		for (tc = 0; tc < SJA1105_NUM_TC; tc++)
-			l2fwd[port].vlan_pmap[tc] = tc;
-	}
+			l2fwd[dp->index].vlan_pmap[tc] = tc;
 
 	/* Then manage the forwarding domain for user ports. These can forward
 	 * only to the always-on domain (CPU port and DSA links)
 	 */
-	for (from = 0; from < ds->num_ports; from++) {
-		if (!dsa_is_user_port(ds, from))
-			continue;
+	dsa_switch_for_each_user_port(from_dp, ds) {
+		int from = from_dp->index;
 
-		for (to = 0; to < ds->num_ports; to++) {
-			if (!dsa_is_cpu_port(ds, to) &&
-			    !dsa_is_dsa_port(ds, to))
+		dsa_switch_for_each_port(to_dp, ds) {
+			int to = to_dp->index;
+
+			if (!dsa_port_is_cpu(to_dp) &&
+			    !dsa_port_is_dsa(to_dp))
 				continue;
 
 			l2fwd[from].bc_domain |= BIT(to);
@@ -556,13 +543,14 @@ static int sja1105_init_l2_forwarding(struct sja1105_private *priv)
 	 * always-on domain). These can send packets to any enabled port except
 	 * themselves.
 	 */
-	for (from = 0; from < ds->num_ports; from++) {
-		if (!dsa_is_cpu_port(ds, from) && !dsa_is_dsa_port(ds, from))
+	dsa_switch_for_each_port(from_dp, ds) {
+		int from = from_dp->index;
+
+		if (!dsa_port_is_cpu(from_dp) && !dsa_port_is_dsa(from_dp))
 			continue;
 
-		for (to = 0; to < ds->num_ports; to++) {
-			if (dsa_is_unused_port(ds, to))
-				continue;
+		dsa_switch_for_each_available_port(to_dp, ds) {
+			int to = to_dp->index;
 
 			if (from == to)
 				continue;
@@ -585,6 +573,8 @@ static int sja1105_init_l2_forwarding(struct sja1105_private *priv)
 	dst = ds->dst;
 
 	list_for_each_entry(dl, &dst->rtable, list) {
+		int from, to;
+
 		if (dl->dp->ds != ds || dl->link_dp->cpu_dp == dl->dp->cpu_dp)
 			continue;
 
@@ -604,24 +594,17 @@ static int sja1105_init_l2_forwarding(struct sja1105_private *priv)
 	/* Finally, manage the egress flooding domain. All ports start up with
 	 * flooding enabled, including the CPU port and DSA links.
 	 */
-	for (port = 0; port < ds->num_ports; port++) {
-		if (dsa_is_unused_port(ds, port))
-			continue;
-
-		priv->ucast_egress_floods |= BIT(port);
-		priv->bcast_egress_floods |= BIT(port);
+	dsa_switch_for_each_available_port(dp, ds) {
+		priv->ucast_egress_floods |= BIT(dp->index);
+		priv->bcast_egress_floods |= BIT(dp->index);
 	}
 
 	/* Next 8 entries define VLAN PCP mapping from ingress to egress.
 	 * Create a one-to-one mapping.
 	 */
 	for (tc = 0; tc < SJA1105_NUM_TC; tc++) {
-		for (port = 0; port < ds->num_ports; port++) {
-			if (dsa_is_unused_port(ds, port))
-				continue;
-
-			l2fwd[ds->num_ports + tc].vlan_pmap[port] = tc;
-		}
+		dsa_switch_for_each_available_port(dp, ds)
+			l2fwd[ds->num_ports + tc].vlan_pmap[dp->index] = tc;
 
 		l2fwd[ds->num_ports + tc].type_egrpcp2outputq = true;
 	}
@@ -634,7 +617,8 @@ static int sja1110_init_pcp_remapping(struct sja1105_private *priv)
 	struct sja1110_pcp_remapping_entry *pcp_remap;
 	struct dsa_switch *ds = priv->ds;
 	struct sja1105_table *table;
-	int port, tc;
+	struct dsa_port *dp;
+	int tc;
 
 	table = &priv->static_config.tables[BLK_IDX_PCP_REMAPPING];
 
@@ -657,13 +641,9 @@ static int sja1110_init_pcp_remapping(struct sja1105_private *priv)
 	pcp_remap = table->entries;
 
 	/* Repeat the configuration done for vlan_pmap */
-	for (port = 0; port < ds->num_ports; port++) {
-		if (dsa_is_unused_port(ds, port))
-			continue;
-
+	dsa_switch_for_each_available_port(dp, ds)
 		for (tc = 0; tc < SJA1105_NUM_TC; tc++)
-			pcp_remap[port].egrpcp[tc] = tc;
-	}
+			pcp_remap[dp->index].egrpcp[tc] = tc;
 
 	return 0;
 }
@@ -999,7 +979,8 @@ static int sja1105_init_l2_policing(struct sja1105_private *priv)
 	struct sja1105_l2_policing_entry *policing;
 	struct dsa_switch *ds = priv->ds;
 	struct sja1105_table *table;
-	int port, tc;
+	struct dsa_port *dp;
+	int tc;
 
 	table = &priv->static_config.tables[BLK_IDX_L2_POLICING];
 
@@ -1019,9 +1000,10 @@ static int sja1105_init_l2_policing(struct sja1105_private *priv)
 	policing = table->entries;
 
 	/* Setup shared indices for the matchall policers */
-	for (port = 0; port < ds->num_ports; port++) {
-		int mcast = (ds->num_ports * (SJA1105_NUM_TC + 1)) + port;
-		int bcast = (ds->num_ports * SJA1105_NUM_TC) + port;
+	dsa_switch_for_each_available_port(dp, ds) {
+		int mcast = (ds->num_ports * (SJA1105_NUM_TC + 1)) + dp->index;
+		int bcast = (ds->num_ports * SJA1105_NUM_TC) + dp->index;
+		int port = dp->index;
 
 		for (tc = 0; tc < SJA1105_NUM_TC; tc++)
 			policing[port * SJA1105_NUM_TC + tc].sharindx = port;
@@ -1033,10 +1015,11 @@ static int sja1105_init_l2_policing(struct sja1105_private *priv)
 	}
 
 	/* Setup the matchall policer parameters */
-	for (port = 0; port < ds->num_ports; port++) {
+	dsa_switch_for_each_available_port(dp, ds) {
 		int mtu = VLAN_ETH_FRAME_LEN + ETH_FCS_LEN;
+		int port = dp->index;
 
-		if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port))
+		if (dsa_port_is_cpu(dp) || dsa_port_is_dsa(dp))
 			mtu += VLAN_HLEN;
 
 		policing[port].smax = 65535; /* Burst size in bytes */
@@ -1996,16 +1979,17 @@ static int sja1105_bridge_member(struct dsa_switch *ds, int port,
 {
 	struct sja1105_l2_forwarding_entry *l2_fwd;
 	struct sja1105_private *priv = ds->priv;
-	int i, rc;
+	struct dsa_port *dp;
+	int rc;
 
 	l2_fwd = priv->static_config.tables[BLK_IDX_L2_FORWARDING].entries;
 
-	for (i = 0; i < ds->num_ports; i++) {
-		/* Add this port to the forwarding matrix of the
-		 * other ports in the same bridge, and viceversa.
-		 */
-		if (!dsa_is_user_port(ds, i))
-			continue;
+	/* Add this port to the forwarding matrix of the
+	 * other ports in the same bridge, and viceversa.
+	 */
+	dsa_switch_for_each_user_port(dp, ds) {
+		int i = dp->index;
+
 		/* For the ports already under the bridge, only one thing needs
 		 * to be done, and that is to add this port to their
 		 * reachability domain. So we can perform the SPI write for
@@ -2017,8 +2001,10 @@ static int sja1105_bridge_member(struct dsa_switch *ds, int port,
 		 */
 		if (i == port)
 			continue;
-		if (dsa_to_port(ds, i)->bridge_dev != br)
+
+		if (dp->bridge_dev != br)
 			continue;
+
 		sja1105_port_allow_traffic(l2_fwd, i, port, member);
 		sja1105_port_allow_traffic(l2_fwd, port, i, member);
 
@@ -2366,6 +2352,7 @@ int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled,
 	struct sja1105_private *priv = ds->priv;
 	struct sja1105_table *table;
 	struct sja1105_rule *rule;
+	struct dsa_port *dp;
 	u16 tpid, tpid2;
 	int rc;
 
@@ -2435,11 +2422,8 @@ int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled,
 	l2_lookup_params = table->entries;
 	l2_lookup_params->shared_learn = !priv->vlan_aware;
 
-	for (port = 0; port < ds->num_ports; port++) {
-		if (dsa_is_unused_port(ds, port))
-			continue;
-
-		rc = sja1105_commit_pvid(ds, port);
+	dsa_switch_for_each_available_port(dp, ds) {
+		rc = sja1105_commit_pvid(ds, dp->index);
 		if (rc)
 			return rc;
 	}
@@ -2753,17 +2737,14 @@ static int sja1105_setup(struct dsa_switch *ds)
 static void sja1105_teardown(struct dsa_switch *ds)
 {
 	struct sja1105_private *priv = ds->priv;
-	int port;
+	struct dsa_port *dp;
 
 	rtnl_lock();
 	dsa_tag_8021q_unregister(ds);
 	rtnl_unlock();
 
-	for (port = 0; port < ds->num_ports; port++) {
-		struct sja1105_port *sp = &priv->ports[port];
-
-		if (!dsa_is_user_port(ds, port))
-			continue;
+	dsa_switch_for_each_user_port(dp, ds) {
+		struct sja1105_port *sp = &priv->ports[dp->index];
 
 		if (sp->xmit_worker)
 			kthread_destroy_worker(sp->xmit_worker);
@@ -3284,7 +3265,8 @@ static int sja1105_probe(struct spi_device *spi)
 	struct sja1105_private *priv;
 	size_t max_xfer, max_msg;
 	struct dsa_switch *ds;
-	int rc, port;
+	struct dsa_port *dp;
+	int rc;
 
 	if (!dev->of_node) {
 		dev_err(dev, "No DTS bindings for SJA1105 driver\n");
@@ -3389,14 +3371,10 @@ static int sja1105_probe(struct spi_device *spi)
 	}
 
 	/* Connections between dsa_port and sja1105_port */
-	for (port = 0; port < ds->num_ports; port++) {
-		struct sja1105_port *sp = &priv->ports[port];
-		struct dsa_port *dp = dsa_to_port(ds, port);
+	dsa_switch_for_each_user_port(dp, ds) {
+		struct sja1105_port *sp = &priv->ports[dp->index];
 		struct net_device *slave;
 
-		if (!dsa_is_user_port(ds, port))
-			continue;
-
 		dp->priv = sp;
 		sp->dp = dp;
 		sp->data = tagger_data;
@@ -3418,10 +3396,10 @@ static int sja1105_probe(struct spi_device *spi)
 	return 0;
 
 out_destroy_workers:
-	while (port-- > 0) {
-		struct sja1105_port *sp = &priv->ports[port];
+	dsa_switch_for_each_port_continue_reverse(dp, ds) {
+		struct sja1105_port *sp = &priv->ports[dp->index];
 
-		if (!dsa_is_user_port(ds, port))
+		if (!dsa_port_is_user(dp))
 			continue;
 
 		kthread_destroy_worker(sp->xmit_worker);
diff --git a/drivers/net/dsa/sja1105/sja1105_mdio.c b/drivers/net/dsa/sja1105/sja1105_mdio.c
index 19aea8fb76f6..6f8d20a5bc37 100644
--- a/drivers/net/dsa/sja1105/sja1105_mdio.c
+++ b/drivers/net/dsa/sja1105/sja1105_mdio.c
@@ -390,9 +390,9 @@ static int sja1105_mdiobus_pcs_register(struct sja1105_private *priv)
 {
 	struct sja1105_mdio_private *mdio_priv;
 	struct dsa_switch *ds = priv->ds;
+	struct dsa_port *dp;
 	struct mii_bus *bus;
 	int rc = 0;
-	int port;
 
 	if (!priv->info->pcs_mdio_read || !priv->info->pcs_mdio_write)
 		return 0;
@@ -420,12 +420,10 @@ static int sja1105_mdiobus_pcs_register(struct sja1105_private *priv)
 		return rc;
 	}
 
-	for (port = 0; port < ds->num_ports; port++) {
+	dsa_switch_for_each_available_port(dp, ds) {
 		struct mdio_device *mdiodev;
 		struct dw_xpcs *xpcs;
-
-		if (dsa_is_unused_port(ds, port))
-			continue;
+		int port = dp->index;
 
 		if (priv->phy_mode[port] != PHY_INTERFACE_MODE_SGMII &&
 		    priv->phy_mode[port] != PHY_INTERFACE_MODE_2500BASEX)
@@ -451,7 +449,9 @@ static int sja1105_mdiobus_pcs_register(struct sja1105_private *priv)
 	return 0;
 
 out_pcs_free:
-	for (port = 0; port < ds->num_ports; port++) {
+	dsa_switch_for_each_port_continue_reverse(dp, ds) {
+		int port = dp->index;
+
 		if (!priv->xpcs[port])
 			continue;
 
diff --git a/drivers/net/dsa/xrs700x/xrs700x.c b/drivers/net/dsa/xrs700x/xrs700x.c
index 130abb0f1438..7465be144cc9 100644
--- a/drivers/net/dsa/xrs700x/xrs700x.c
+++ b/drivers/net/dsa/xrs700x/xrs700x.c
@@ -297,7 +297,6 @@ static void xrs700x_port_stp_state_set(struct dsa_switch *ds, int port,
 static int xrs700x_port_add_bpdu_ipf(struct dsa_switch *ds, int port)
 {
 	struct xrs700x *priv = ds->priv;
-	unsigned int val = 0;
 	int i = 0;
 	int ret;
 
@@ -316,12 +315,8 @@ static int xrs700x_port_add_bpdu_ipf(struct dsa_switch *ds, int port)
 	}
 
 	/* Mirror BPDU to CPU port */
-	for (i = 0; i < ds->num_ports; i++) {
-		if (dsa_is_cpu_port(ds, i))
-			val |= BIT(i);
-	}
-
-	ret = regmap_write(priv->regmap, XRS_ETH_ADDR_FWD_MIRROR(port, 0), val);
+	ret = regmap_write(priv->regmap, XRS_ETH_ADDR_FWD_MIRROR(port, 0),
+			   dsa_cpu_ports(ds));
 	if (ret)
 		return ret;
 
@@ -340,8 +335,8 @@ static int xrs700x_port_add_bpdu_ipf(struct dsa_switch *ds, int port)
 static int xrs700x_port_add_hsrsup_ipf(struct dsa_switch *ds, int port,
 				       int fwdport)
 {
+	unsigned int val = dsa_cpu_ports(ds);
 	struct xrs700x *priv = ds->priv;
-	unsigned int val = 0;
 	int i = 0;
 	int ret;
 
@@ -360,11 +355,6 @@ static int xrs700x_port_add_hsrsup_ipf(struct dsa_switch *ds, int port,
 	}
 
 	/* Mirror HSR/PRP supervision to CPU port */
-	for (i = 0; i < ds->num_ports; i++) {
-		if (dsa_is_cpu_port(ds, i))
-			val |= BIT(i);
-	}
-
 	ret = regmap_write(priv->regmap, XRS_ETH_ADDR_FWD_MIRROR(port, 1), val);
 	if (ret)
 		return ret;
@@ -505,28 +495,27 @@ static void xrs700x_mac_link_up(struct dsa_switch *ds, int port,
 static int xrs700x_bridge_common(struct dsa_switch *ds, int port,
 				 struct net_device *bridge, bool join)
 {
-	unsigned int i, cpu_mask = 0, mask = 0;
+	unsigned int cpu_mask = 0, mask = 0;
 	struct xrs700x *priv = ds->priv;
+	struct dsa_port *dp;
 	int ret;
 
-	for (i = 0; i < ds->num_ports; i++) {
-		if (dsa_is_cpu_port(ds, i))
-			continue;
-
-		cpu_mask |= BIT(i);
+	dsa_switch_for_each_user_port(dp, ds) {
+		cpu_mask |= BIT(dp->index);
 
-		if (dsa_to_port(ds, i)->bridge_dev == bridge)
+		if (dp->bridge_dev == bridge)
 			continue;
 
-		mask |= BIT(i);
+		mask |= BIT(dp->index);
 	}
 
-	for (i = 0; i < ds->num_ports; i++) {
-		if (dsa_to_port(ds, i)->bridge_dev != bridge)
+	dsa_switch_for_each_port(dp, ds) {
+		if (dp->bridge_dev != bridge)
 			continue;
 
 		/* 1 = Disable forwarding to the port */
-		ret = regmap_write(priv->regmap, XRS_PORT_FWD_MASK(i), mask);
+		ret = regmap_write(priv->regmap, XRS_PORT_FWD_MASK(dp->index),
+				   mask);
 		if (ret)
 			return ret;
 	}
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 4639a82bab66..379ad8183639 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -475,6 +475,14 @@ static inline bool dsa_is_user_port(struct dsa_switch *ds, int p)
 	list_for_each_entry((_dp), &(_ds)->dst->ports, list) \
 		if ((_dp)->ds == (_ds))
 
+#define dsa_switch_for_each_port_continue_reverse(_dp, _ds) \
+	list_for_each_entry_continue_reverse((_dp), &(_ds)->dst->ports, list) \
+		if ((_dp)->ds == (_ds))
+
+#define dsa_switch_for_each_available_port(_dp, _ds) \
+	dsa_switch_for_each_port((_dp), (_ds)) \
+		if (!dsa_port_is_unused((_dp)))
+
 #define dsa_switch_for_each_user_port(_dp, _ds) \
 	dsa_switch_for_each_port((_dp), (_ds)) \
 		if (dsa_port_is_user((_dp)))
@@ -494,6 +502,17 @@ static inline u32 dsa_user_ports(struct dsa_switch *ds)
 	return mask;
 }
 
+static inline u32 dsa_cpu_ports(struct dsa_switch *ds)
+{
+	struct dsa_port *dp;
+	u32 mask = 0;
+
+	dsa_switch_for_each_cpu_port(dp, ds)
+		mask |= BIT(dp->index);
+
+	return mask;
+}
+
 /* Return the local port used to reach an arbitrary switch device */
 static inline unsigned int dsa_routing_port(struct dsa_switch *ds, int device)
 {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [RFC PATCH v2 net-next 4/8] net: dsa: b53: express b53_for_each_port in terms of dsa_switch_for_each_port
  2021-08-10 16:14 [RFC PATCH v2 net-next 0/8] Remove the "dsa_to_port in a loop" antipattern Vladimir Oltean
                   ` (2 preceding siblings ...)
  2021-08-10 16:14 ` [RFC PATCH v2 net-next 3/8] net: dsa: remove the "dsa_to_port in a loop" antipattern from drivers Vladimir Oltean
@ 2021-08-10 16:14 ` Vladimir Oltean
  2021-08-11  8:40   ` Florian Fainelli
  2021-08-10 16:14 ` [RFC PATCH v2 net-next 5/8] net: dsa: finish conversion to dsa_switch_for_each_port Vladimir Oltean
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 11+ messages in thread
From: Vladimir Oltean @ 2021-08-10 16:14 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Woojung Huh, UNGLinuxDriver, Sean Wang,
	Landen Chao, DENG Qingfang, Matthias Brugger, Claudiu Manoil,
	Alexandre Belloni, George McCollister

Merging the two allows us to remove the open-coded
"dev->enabled_ports & BIT(i)" check from b53_br_join and b53_br_leave,
while still avoiding a quadratic iteration through the switch's ports.

Sadly I don't know if it's possible to completely get rid of
b53_for_each_port and replace it with dsa_switch_for_each_available_port,
especially for the platforms that use pdata and not OF bindings.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: none

 drivers/net/dsa/b53/b53_common.c | 20 ++++++++++----------
 drivers/net/dsa/b53/b53_priv.h   |  6 +++---
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index ccd93147d994..5351d1f65ed9 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -498,6 +498,7 @@ static int b53_fast_age_vlan(struct b53_device *dev, u16 vid)
 void b53_imp_vlan_setup(struct dsa_switch *ds, int cpu_port)
 {
 	struct b53_device *dev = ds->priv;
+	struct dsa_port *dp;
 	unsigned int i;
 	u16 pvlan;
 
@@ -505,7 +506,9 @@ void b53_imp_vlan_setup(struct dsa_switch *ds, int cpu_port)
 	 * on a per-port basis such that we only have Port i and IMP in
 	 * the same VLAN.
 	 */
-	b53_for_each_port(dev, i) {
+	b53_for_each_port(dp, dev) {
+		i = dp->index;
+
 		b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(i), &pvlan);
 		pvlan |= BIT(cpu_port);
 		b53_write16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(i), pvlan);
@@ -739,6 +742,7 @@ int b53_configure_vlan(struct dsa_switch *ds)
 {
 	struct b53_device *dev = ds->priv;
 	struct b53_vlan vl = { 0 };
+	struct dsa_port *dp;
 	struct b53_vlan *v;
 	int i, def_vid;
 	u16 vid;
@@ -761,7 +765,9 @@ int b53_configure_vlan(struct dsa_switch *ds)
 	 * entry. Do this only when the tagging protocol is not
 	 * DSA_TAG_PROTO_NONE
 	 */
-	b53_for_each_port(dev, i) {
+	b53_for_each_port(dp, dev) {
+		i = dp->index;
+
 		v = &dev->vlans[def_vid];
 		v->members |= BIT(i);
 		if (!b53_vlan_port_needs_forced_tagged(ds, i))
@@ -1874,12 +1880,9 @@ int b53_br_join(struct dsa_switch *ds, int port, struct net_device *br)
 
 	b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), &pvlan);
 
-	dsa_switch_for_each_port(dp, ds) {
+	b53_for_each_port(dp, dev) {
 		i = dp->index;
 
-		if (!(dev->enabled_ports & BIT(i)))
-			continue;
-
 		if (dp->bridge_dev != br)
 			continue;
 
@@ -1915,12 +1918,9 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct net_device *br)
 
 	b53_read16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(port), &pvlan);
 
-	dsa_switch_for_each_port(dp, ds) {
+	b53_for_each_port(dp, dev) {
 		i = dp->index;
 
-		if (!(dev->enabled_ports & BIT(i)))
-			continue;
-
 		/* Don't touch the remaining ports */
 		if (dp->bridge_dev != br)
 			continue;
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index 9bf8319342b0..aec4b1176be9 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -145,10 +145,10 @@ struct b53_device {
 	struct b53_port *ports;
 };
 
-#define b53_for_each_port(dev, i) \
-	for (i = 0; i < B53_N_PORTS; i++) \
-		if (dev->enabled_ports & BIT(i))
 
+#define b53_for_each_port(_dp, _dev) \
+	dsa_switch_for_each_port((_dp), (_dev)->ds) \
+		if ((_dev)->enabled_ports & BIT((_dp)->index))
 
 static inline int is5325(struct b53_device *dev)
 {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [RFC PATCH v2 net-next 5/8] net: dsa: finish conversion to dsa_switch_for_each_port
  2021-08-10 16:14 [RFC PATCH v2 net-next 0/8] Remove the "dsa_to_port in a loop" antipattern Vladimir Oltean
                   ` (3 preceding siblings ...)
  2021-08-10 16:14 ` [RFC PATCH v2 net-next 4/8] net: dsa: b53: express b53_for_each_port in terms of dsa_switch_for_each_port Vladimir Oltean
@ 2021-08-10 16:14 ` Vladimir Oltean
  2021-08-10 16:14 ` [RFC PATCH v2 net-next 6/8] net: dsa: remove gratuitous use of dsa_is_{user,dsa,cpu}_port Vladimir Oltean
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Vladimir Oltean @ 2021-08-10 16:14 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Woojung Huh, UNGLinuxDriver, Sean Wang,
	Landen Chao, DENG Qingfang, Matthias Brugger, Claudiu Manoil,
	Alexandre Belloni, George McCollister

What is left is a 'beauty' conversion, since the performance is in no
way different than before.

Find the remaining iterators over dst->ports that only filter for the
ports belonging to a certain switch, and replace those with the
dsa_switch_for_each_port helper that we have now.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: patch is new

 drivers/net/dsa/mv88e6xxx/chip.c |  9 ++++-----
 include/net/dsa.h                |  4 ++++
 net/dsa/dsa2.c                   | 25 +++++++++----------------
 3 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 1525415aca46..5ce3c9129307 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1263,9 +1263,8 @@ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port)
 	/* Frames from user ports can egress any local DSA links and CPU ports,
 	 * as well as any local member of their bridge group.
 	 */
-	list_for_each_entry(dp, &dst->ports, list)
-		if (dp->ds == ds &&
-		    (dp->type == DSA_PORT_TYPE_CPU ||
+	dsa_switch_for_each_port(dp, ds)
+		if ((dp->type == DSA_PORT_TYPE_CPU ||
 		     dp->type == DSA_PORT_TYPE_DSA ||
 		     (br && dp->bridge_dev == br)))
 			pvlan |= BIT(dp->index);
@@ -5938,8 +5937,8 @@ static int mv88e6xxx_lag_sync_masks(struct dsa_switch *ds)
 	ivec = BIT(mv88e6xxx_num_ports(chip)) - 1;
 
 	/* Disable all masks for ports that _are_ members of a LAG. */
-	list_for_each_entry(dp, &ds->dst->ports, list) {
-		if (!dp->lag_dev || dp->ds != ds)
+	dsa_switch_for_each_port(dp, ds) {
+		if (!dp->lag_dev)
 			continue;
 
 		ivec &= ~BIT(dp->index);
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 379ad8183639..3203b200cc38 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -475,6 +475,10 @@ static inline bool dsa_is_user_port(struct dsa_switch *ds, int p)
 	list_for_each_entry((_dp), &(_ds)->dst->ports, list) \
 		if ((_dp)->ds == (_ds))
 
+#define dsa_switch_for_each_port_safe(_dp, _next, _ds) \
+	list_for_each_entry_safe((_dp), (_next), &(_ds)->dst->ports, list) \
+		if ((_dp)->ds == (_ds))
+
 #define dsa_switch_for_each_port_continue_reverse(_dp, _ds) \
 	list_for_each_entry_continue_reverse((_dp), &(_ds)->dst->ports, list) \
 		if ((_dp)->ds == (_ds))
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index cf9810d55611..67222f634876 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -759,12 +759,10 @@ static int dsa_switch_setup(struct dsa_switch *ds)
 	/* Setup devlink port instances now, so that the switch
 	 * setup() can register regions etc, against the ports
 	 */
-	list_for_each_entry(dp, &ds->dst->ports, list) {
-		if (dp->ds == ds) {
-			err = dsa_port_devlink_setup(dp);
-			if (err)
-				goto unregister_devlink_ports;
-		}
+	dsa_switch_for_each_port(dp, ds) {
+		err = dsa_port_devlink_setup(dp);
+		if (err)
+			goto unregister_devlink_ports;
 	}
 
 	err = dsa_switch_register_notifier(ds);
@@ -807,9 +805,8 @@ static int dsa_switch_setup(struct dsa_switch *ds)
 unregister_notifier:
 	dsa_switch_unregister_notifier(ds);
 unregister_devlink_ports:
-	list_for_each_entry(dp, &ds->dst->ports, list)
-		if (dp->ds == ds)
-			dsa_port_devlink_teardown(dp);
+	dsa_switch_for_each_port(dp, ds)
+		dsa_port_devlink_teardown(dp);
 	devlink_unregister(ds->devlink);
 free_devlink:
 	devlink_free(ds->devlink);
@@ -834,9 +831,8 @@ static void dsa_switch_teardown(struct dsa_switch *ds)
 		ds->ops->teardown(ds);
 
 	if (ds->devlink) {
-		list_for_each_entry(dp, &ds->dst->ports, list)
-			if (dp->ds == ds)
-				dsa_port_devlink_teardown(dp);
+		dsa_switch_for_each_port(dp, ds)
+			dsa_port_devlink_teardown(dp);
 		devlink_unregister(ds->devlink);
 		devlink_free(ds->devlink);
 		ds->devlink = NULL;
@@ -1412,12 +1408,9 @@ static int dsa_switch_parse(struct dsa_switch *ds, struct dsa_chip_data *cd)
 
 static void dsa_switch_release_ports(struct dsa_switch *ds)
 {
-	struct dsa_switch_tree *dst = ds->dst;
 	struct dsa_port *dp, *next;
 
-	list_for_each_entry_safe(dp, next, &dst->ports, list) {
-		if (dp->ds != ds)
-			continue;
+	dsa_switch_for_each_port_safe(dp, next, ds) {
 		list_del(&dp->list);
 		kfree(dp);
 	}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [RFC PATCH v2 net-next 6/8] net: dsa: remove gratuitous use of dsa_is_{user,dsa,cpu}_port
  2021-08-10 16:14 [RFC PATCH v2 net-next 0/8] Remove the "dsa_to_port in a loop" antipattern Vladimir Oltean
                   ` (4 preceding siblings ...)
  2021-08-10 16:14 ` [RFC PATCH v2 net-next 5/8] net: dsa: finish conversion to dsa_switch_for_each_port Vladimir Oltean
@ 2021-08-10 16:14 ` Vladimir Oltean
  2021-08-10 16:14 ` [RFC PATCH v2 net-next 7/8] net: dsa: convert cross-chip notifiers to iterate using dp Vladimir Oltean
  2021-08-10 16:14 ` [RFC PATCH v2 net-next 8/8] net: dsa: tag_8021q: finish conversion to dsa_switch_for_each_port Vladimir Oltean
  7 siblings, 0 replies; 11+ messages in thread
From: Vladimir Oltean @ 2021-08-10 16:14 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Woojung Huh, UNGLinuxDriver, Sean Wang,
	Landen Chao, DENG Qingfang, Matthias Brugger, Claudiu Manoil,
	Alexandre Belloni, George McCollister

Find the occurrences of dsa_is_{user,dsa,cpu}_port where a struct
dsa_port *dp was already available in the function scope, and replace
them with the dsa_port_is_{user,dsa,cpu} equivalent function which uses
that dp directly and does not perform another hidden dsa_to_port().

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: patch is new

 drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
 net/dsa/port.c                   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 5ce3c9129307..edd8eaa22bc7 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1641,7 +1641,7 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
 	int err;
 
 	/* DSA and CPU ports have to be members of multiple vlans */
-	if (dsa_is_dsa_port(ds, port) || dsa_is_cpu_port(ds, port))
+	if (dsa_port_is_dsa(dp) || dsa_port_is_cpu(dp))
 		return 0;
 
 	err = mv88e6xxx_vtu_get(chip, vid, &vlan);
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 05b902d69863..e035650a30ab 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -548,7 +548,7 @@ static bool dsa_port_can_apply_vlan_filtering(struct dsa_port *dp,
 	 * enter an inconsistent state: deny changing the VLAN awareness state
 	 * as long as we have 8021q uppers.
 	 */
-	if (vlan_filtering && dsa_is_user_port(ds, dp->index)) {
+	if (vlan_filtering && dsa_port_is_user(dp)) {
 		struct net_device *upper_dev, *slave = dp->slave;
 		struct net_device *br = dp->bridge_dev;
 		struct list_head *iter;
@@ -1046,7 +1046,7 @@ static void dsa_port_phylink_mac_link_down(struct phylink_config *config,
 	struct phy_device *phydev = NULL;
 	struct dsa_switch *ds = dp->ds;
 
-	if (dsa_is_user_port(ds, dp->index))
+	if (dsa_port_is_user(dp))
 		phydev = dp->slave->phydev;
 
 	if (!ds->ops->phylink_mac_link_down) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [RFC PATCH v2 net-next 7/8] net: dsa: convert cross-chip notifiers to iterate using dp
  2021-08-10 16:14 [RFC PATCH v2 net-next 0/8] Remove the "dsa_to_port in a loop" antipattern Vladimir Oltean
                   ` (5 preceding siblings ...)
  2021-08-10 16:14 ` [RFC PATCH v2 net-next 6/8] net: dsa: remove gratuitous use of dsa_is_{user,dsa,cpu}_port Vladimir Oltean
@ 2021-08-10 16:14 ` Vladimir Oltean
  2021-08-10 16:14 ` [RFC PATCH v2 net-next 8/8] net: dsa: tag_8021q: finish conversion to dsa_switch_for_each_port Vladimir Oltean
  7 siblings, 0 replies; 11+ messages in thread
From: Vladimir Oltean @ 2021-08-10 16:14 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Woojung Huh, UNGLinuxDriver, Sean Wang,
	Landen Chao, DENG Qingfang, Matthias Brugger, Claudiu Manoil,
	Alexandre Belloni, George McCollister

The majority of cross-chip switch notifiers need to filter in some way
over the type of ports: some install VLANs etc on all cascade ports.

The difference is that the matching function, which filters by port
type, is separate from the function where the iteration happens. So this
patch needs to refactor the matching functions' prototypes as well, to
take the dp as argument.

In a future patch/series, I might convert dsa_towards_port to return a
struct dsa_port *dp too, but at the moment it is a bit entangled with
dsa_routing_port which is also used by mv88e6xxx and they both return an
int port. So keep dsa_towards_port the way it is and convert it into a
dp using dsa_to_port.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2: patch is new

 net/dsa/switch.c    | 129 +++++++++++++++++++++++---------------------
 net/dsa/tag_8021q.c |  53 +++++++++---------
 2 files changed, 97 insertions(+), 85 deletions(-)

diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 5ddcf37ecfa5..b18480f333bb 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -46,10 +46,10 @@ static int dsa_switch_ageing_time(struct dsa_switch *ds,
 	return 0;
 }
 
-static bool dsa_switch_mtu_match(struct dsa_switch *ds, int port,
-				 struct dsa_notifier_mtu_info *info)
+static bool dsa_port_mtu_match(struct dsa_port *dp,
+			       struct dsa_notifier_mtu_info *info)
 {
-	if (ds->index == info->sw_index && port == info->port)
+	if (dp->ds->index == info->sw_index && dp->index == info->port)
 		return true;
 
 	/* Do not propagate to other switches in the tree if the notifier was
@@ -58,7 +58,7 @@ static bool dsa_switch_mtu_match(struct dsa_switch *ds, int port,
 	if (info->targeted_match)
 		return false;
 
-	if (dsa_is_dsa_port(ds, port) || dsa_is_cpu_port(ds, port))
+	if (dsa_port_is_dsa(dp) || dsa_port_is_cpu(dp))
 		return true;
 
 	return false;
@@ -67,14 +67,16 @@ static bool dsa_switch_mtu_match(struct dsa_switch *ds, int port,
 static int dsa_switch_mtu(struct dsa_switch *ds,
 			  struct dsa_notifier_mtu_info *info)
 {
-	int port, ret;
+	struct dsa_port *dp;
+	int ret;
 
 	if (!ds->ops->port_change_mtu)
 		return -EOPNOTSUPP;
 
-	for (port = 0; port < ds->num_ports; port++) {
-		if (dsa_switch_mtu_match(ds, port, info)) {
-			ret = ds->ops->port_change_mtu(ds, port, info->mtu);
+	dsa_switch_for_each_port(dp, ds) {
+		if (dsa_port_mtu_match(dp, info)) {
+			ret = ds->ops->port_change_mtu(ds, dp->index,
+						       info->mtu);
 			if (ret)
 				return ret;
 		}
@@ -164,19 +166,19 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds,
  * DSA links) that sit between the targeted port on which the notifier was
  * emitted and its dedicated CPU port.
  */
-static bool dsa_switch_host_address_match(struct dsa_switch *ds, int port,
-					  int info_sw_index, int info_port)
+static bool dsa_port_host_address_match(struct dsa_port *dp,
+					int info_sw_index, int info_port)
 {
 	struct dsa_port *targeted_dp, *cpu_dp;
 	struct dsa_switch *targeted_ds;
 
-	targeted_ds = dsa_switch_find(ds->dst->index, info_sw_index);
+	targeted_ds = dsa_switch_find(dp->ds->dst->index, info_sw_index);
 	targeted_dp = dsa_to_port(targeted_ds, info_port);
 	cpu_dp = targeted_dp->cpu_dp;
 
-	if (dsa_switch_is_upstream_of(ds, targeted_ds))
-		return port == dsa_towards_port(ds, cpu_dp->ds->index,
-						cpu_dp->index);
+	if (dsa_switch_is_upstream_of(dp->ds, targeted_ds))
+		return dp->index == dsa_towards_port(dp->ds, cpu_dp->ds->index,
+						     cpu_dp->index);
 
 	return false;
 }
@@ -194,11 +196,12 @@ static struct dsa_mac_addr *dsa_mac_addr_find(struct list_head *addr_list,
 	return NULL;
 }
 
-static int dsa_switch_do_mdb_add(struct dsa_switch *ds, int port,
-				 const struct switchdev_obj_port_mdb *mdb)
+static int dsa_port_do_mdb_add(struct dsa_port *dp,
+			       const struct switchdev_obj_port_mdb *mdb)
 {
-	struct dsa_port *dp = dsa_to_port(ds, port);
+	struct dsa_switch *ds = dp->ds;
 	struct dsa_mac_addr *a;
+	int port = dp->index;
 	int err;
 
 	/* No need to bother with refcounting for user ports */
@@ -229,11 +232,12 @@ static int dsa_switch_do_mdb_add(struct dsa_switch *ds, int port,
 	return 0;
 }
 
-static int dsa_switch_do_mdb_del(struct dsa_switch *ds, int port,
-				 const struct switchdev_obj_port_mdb *mdb)
+static int dsa_port_do_mdb_del(struct dsa_port *dp,
+			       const struct switchdev_obj_port_mdb *mdb)
 {
-	struct dsa_port *dp = dsa_to_port(ds, port);
+	struct dsa_switch *ds = dp->ds;
 	struct dsa_mac_addr *a;
+	int port = dp->index;
 	int err;
 
 	/* No need to bother with refcounting for user ports */
@@ -259,11 +263,12 @@ static int dsa_switch_do_mdb_del(struct dsa_switch *ds, int port,
 	return 0;
 }
 
-static int dsa_switch_do_fdb_add(struct dsa_switch *ds, int port,
-				 const unsigned char *addr, u16 vid)
+static int dsa_port_do_fdb_add(struct dsa_port *dp, const unsigned char *addr,
+			       u16 vid)
 {
-	struct dsa_port *dp = dsa_to_port(ds, port);
+	struct dsa_switch *ds = dp->ds;
 	struct dsa_mac_addr *a;
+	int port = dp->index;
 	int err;
 
 	/* No need to bother with refcounting for user ports */
@@ -294,11 +299,12 @@ static int dsa_switch_do_fdb_add(struct dsa_switch *ds, int port,
 	return 0;
 }
 
-static int dsa_switch_do_fdb_del(struct dsa_switch *ds, int port,
-				 const unsigned char *addr, u16 vid)
+static int dsa_port_do_fdb_del(struct dsa_port *dp, const unsigned char *addr,
+			       u16 vid)
 {
-	struct dsa_port *dp = dsa_to_port(ds, port);
+	struct dsa_switch *ds = dp->ds;
 	struct dsa_mac_addr *a;
+	int port = dp->index;
 	int err;
 
 	/* No need to bother with refcounting for user ports */
@@ -327,17 +333,16 @@ static int dsa_switch_do_fdb_del(struct dsa_switch *ds, int port,
 static int dsa_switch_host_fdb_add(struct dsa_switch *ds,
 				   struct dsa_notifier_fdb_info *info)
 {
+	struct dsa_port *dp;
 	int err = 0;
-	int port;
 
 	if (!ds->ops->port_fdb_add)
 		return -EOPNOTSUPP;
 
-	for (port = 0; port < ds->num_ports; port++) {
-		if (dsa_switch_host_address_match(ds, port, info->sw_index,
-						  info->port)) {
-			err = dsa_switch_do_fdb_add(ds, port, info->addr,
-						    info->vid);
+	dsa_switch_for_each_port(dp, ds) {
+		if (dsa_port_host_address_match(dp, info->sw_index,
+						info->port)) {
+			err = dsa_port_do_fdb_add(dp, info->addr, info->vid);
 			if (err)
 				break;
 		}
@@ -349,17 +354,16 @@ static int dsa_switch_host_fdb_add(struct dsa_switch *ds,
 static int dsa_switch_host_fdb_del(struct dsa_switch *ds,
 				   struct dsa_notifier_fdb_info *info)
 {
+	struct dsa_port *dp;
 	int err = 0;
-	int port;
 
 	if (!ds->ops->port_fdb_del)
 		return -EOPNOTSUPP;
 
-	for (port = 0; port < ds->num_ports; port++) {
-		if (dsa_switch_host_address_match(ds, port, info->sw_index,
-						  info->port)) {
-			err = dsa_switch_do_fdb_del(ds, port, info->addr,
-						    info->vid);
+	dsa_switch_for_each_port(dp, ds) {
+		if (dsa_port_host_address_match(dp, info->sw_index,
+						info->port)) {
+			err = dsa_port_do_fdb_del(dp, info->addr, info->vid);
 			if (err)
 				break;
 		}
@@ -372,22 +376,24 @@ static int dsa_switch_fdb_add(struct dsa_switch *ds,
 			      struct dsa_notifier_fdb_info *info)
 {
 	int port = dsa_towards_port(ds, info->sw_index, info->port);
+	struct dsa_port *dp = dsa_to_port(ds, port);
 
 	if (!ds->ops->port_fdb_add)
 		return -EOPNOTSUPP;
 
-	return dsa_switch_do_fdb_add(ds, port, info->addr, info->vid);
+	return dsa_port_do_fdb_add(dp, info->addr, info->vid);
 }
 
 static int dsa_switch_fdb_del(struct dsa_switch *ds,
 			      struct dsa_notifier_fdb_info *info)
 {
 	int port = dsa_towards_port(ds, info->sw_index, info->port);
+	struct dsa_port *dp = dsa_to_port(ds, port);
 
 	if (!ds->ops->port_fdb_del)
 		return -EOPNOTSUPP;
 
-	return dsa_switch_do_fdb_del(ds, port, info->addr, info->vid);
+	return dsa_port_do_fdb_del(dp, info->addr, info->vid);
 }
 
 static int dsa_switch_hsr_join(struct dsa_switch *ds,
@@ -453,37 +459,39 @@ static int dsa_switch_mdb_add(struct dsa_switch *ds,
 			      struct dsa_notifier_mdb_info *info)
 {
 	int port = dsa_towards_port(ds, info->sw_index, info->port);
+	struct dsa_port *dp = dsa_to_port(ds, port);
 
 	if (!ds->ops->port_mdb_add)
 		return -EOPNOTSUPP;
 
-	return dsa_switch_do_mdb_add(ds, port, info->mdb);
+	return dsa_port_do_mdb_add(dp, info->mdb);
 }
 
 static int dsa_switch_mdb_del(struct dsa_switch *ds,
 			      struct dsa_notifier_mdb_info *info)
 {
 	int port = dsa_towards_port(ds, info->sw_index, info->port);
+	struct dsa_port *dp = dsa_to_port(ds, port);
 
 	if (!ds->ops->port_mdb_del)
 		return -EOPNOTSUPP;
 
-	return dsa_switch_do_mdb_del(ds, port, info->mdb);
+	return dsa_port_do_mdb_del(dp, info->mdb);
 }
 
 static int dsa_switch_host_mdb_add(struct dsa_switch *ds,
 				   struct dsa_notifier_mdb_info *info)
 {
+	struct dsa_port *dp;
 	int err = 0;
-	int port;
 
 	if (!ds->ops->port_mdb_add)
 		return -EOPNOTSUPP;
 
-	for (port = 0; port < ds->num_ports; port++) {
-		if (dsa_switch_host_address_match(ds, port, info->sw_index,
-						  info->port)) {
-			err = dsa_switch_do_mdb_add(ds, port, info->mdb);
+	dsa_switch_for_each_port(dp, ds) {
+		if (dsa_port_host_address_match(dp, info->sw_index,
+						info->port)) {
+			err = dsa_port_do_mdb_add(dp, info->mdb);
 			if (err)
 				break;
 		}
@@ -495,16 +503,16 @@ static int dsa_switch_host_mdb_add(struct dsa_switch *ds,
 static int dsa_switch_host_mdb_del(struct dsa_switch *ds,
 				   struct dsa_notifier_mdb_info *info)
 {
+	struct dsa_port *dp;
 	int err = 0;
-	int port;
 
 	if (!ds->ops->port_mdb_del)
 		return -EOPNOTSUPP;
 
-	for (port = 0; port < ds->num_ports; port++) {
-		if (dsa_switch_host_address_match(ds, port, info->sw_index,
-						  info->port)) {
-			err = dsa_switch_do_mdb_del(ds, port, info->mdb);
+	dsa_switch_for_each_port(dp, ds) {
+		if (dsa_port_host_address_match(dp, info->sw_index,
+						info->port)) {
+			err = dsa_port_do_mdb_del(dp, info->mdb);
 			if (err)
 				break;
 		}
@@ -513,13 +521,13 @@ static int dsa_switch_host_mdb_del(struct dsa_switch *ds,
 	return err;
 }
 
-static bool dsa_switch_vlan_match(struct dsa_switch *ds, int port,
-				  struct dsa_notifier_vlan_info *info)
+static bool dsa_port_vlan_match(struct dsa_port *dp,
+				struct dsa_notifier_vlan_info *info)
 {
-	if (ds->index == info->sw_index && port == info->port)
+	if (dp->ds->index == info->sw_index && dp->index == info->port)
 		return true;
 
-	if (dsa_is_dsa_port(ds, port))
+	if (dsa_port_is_dsa(dp))
 		return true;
 
 	return false;
@@ -528,14 +536,15 @@ static bool dsa_switch_vlan_match(struct dsa_switch *ds, int port,
 static int dsa_switch_vlan_add(struct dsa_switch *ds,
 			       struct dsa_notifier_vlan_info *info)
 {
-	int port, err;
+	struct dsa_port *dp;
+	int err;
 
 	if (!ds->ops->port_vlan_add)
 		return -EOPNOTSUPP;
 
-	for (port = 0; port < ds->num_ports; port++) {
-		if (dsa_switch_vlan_match(ds, port, info)) {
-			err = ds->ops->port_vlan_add(ds, port, info->vlan,
+	dsa_switch_for_each_port(dp, ds) {
+		if (dsa_port_vlan_match(dp, info)) {
+			err = ds->ops->port_vlan_add(ds, dp->index, info->vlan,
 						     info->extack);
 			if (err)
 				return err;
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index b29f4eb9aed1..8e4fbbfc6d86 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -139,12 +139,13 @@ dsa_tag_8021q_vlan_find(struct dsa_8021q_context *ctx, int port, u16 vid)
 	return NULL;
 }
 
-static int dsa_switch_do_tag_8021q_vlan_add(struct dsa_switch *ds, int port,
-					    u16 vid, u16 flags)
+static int dsa_port_do_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid,
+					  u16 flags)
 {
-	struct dsa_8021q_context *ctx = ds->tag_8021q_ctx;
-	struct dsa_port *dp = dsa_to_port(ds, port);
+	struct dsa_8021q_context *ctx = dp->ds->tag_8021q_ctx;
+	struct dsa_switch *ds = dp->ds;
 	struct dsa_tag_8021q_vlan *v;
+	int port = dp->index;
 	int err;
 
 	/* No need to bother with refcounting for user ports */
@@ -175,12 +176,12 @@ static int dsa_switch_do_tag_8021q_vlan_add(struct dsa_switch *ds, int port,
 	return 0;
 }
 
-static int dsa_switch_do_tag_8021q_vlan_del(struct dsa_switch *ds, int port,
-					    u16 vid)
+static int dsa_port_do_tag_8021q_vlan_del(struct dsa_port *dp, u16 vid)
 {
-	struct dsa_8021q_context *ctx = ds->tag_8021q_ctx;
-	struct dsa_port *dp = dsa_to_port(ds, port);
+	struct dsa_8021q_context *ctx = dp->ds->tag_8021q_ctx;
+	struct dsa_switch *ds = dp->ds;
 	struct dsa_tag_8021q_vlan *v;
+	int port = dp->index;
 	int err;
 
 	/* No need to bother with refcounting for user ports */
@@ -207,14 +208,16 @@ static int dsa_switch_do_tag_8021q_vlan_del(struct dsa_switch *ds, int port,
 }
 
 static bool
-dsa_switch_tag_8021q_vlan_match(struct dsa_switch *ds, int port,
-				struct dsa_notifier_tag_8021q_vlan_info *info)
+dsa_port_tag_8021q_vlan_match(struct dsa_port *dp,
+			      struct dsa_notifier_tag_8021q_vlan_info *info)
 {
-	if (dsa_is_dsa_port(ds, port) || dsa_is_cpu_port(ds, port))
+	struct dsa_switch *ds = dp->ds;
+
+	if (dsa_port_is_dsa(dp) || dsa_port_is_cpu(dp))
 		return true;
 
 	if (ds->dst->index == info->tree_index && ds->index == info->sw_index)
-		return port == info->port;
+		return dp->index == info->port;
 
 	return false;
 }
@@ -222,7 +225,8 @@ dsa_switch_tag_8021q_vlan_match(struct dsa_switch *ds, int port,
 int dsa_switch_tag_8021q_vlan_add(struct dsa_switch *ds,
 				  struct dsa_notifier_tag_8021q_vlan_info *info)
 {
-	int port, err;
+	struct dsa_port *dp;
+	int err;
 
 	/* Since we use dsa_broadcast(), there might be other switches in other
 	 * trees which don't support tag_8021q, so don't return an error.
@@ -232,21 +236,20 @@ int dsa_switch_tag_8021q_vlan_add(struct dsa_switch *ds,
 	if (!ds->ops->tag_8021q_vlan_add || !ds->tag_8021q_ctx)
 		return 0;
 
-	for (port = 0; port < ds->num_ports; port++) {
-		if (dsa_switch_tag_8021q_vlan_match(ds, port, info)) {
+	dsa_switch_for_each_port(dp, ds) {
+		if (dsa_port_tag_8021q_vlan_match(dp, info)) {
 			u16 flags = 0;
 
-			if (dsa_is_user_port(ds, port))
+			if (dsa_port_is_user(dp))
 				flags |= BRIDGE_VLAN_INFO_UNTAGGED;
 
 			if (vid_is_dsa_8021q_rxvlan(info->vid) &&
 			    dsa_8021q_rx_switch_id(info->vid) == ds->index &&
-			    dsa_8021q_rx_source_port(info->vid) == port)
+			    dsa_8021q_rx_source_port(info->vid) == dp->index)
 				flags |= BRIDGE_VLAN_INFO_PVID;
 
-			err = dsa_switch_do_tag_8021q_vlan_add(ds, port,
-							       info->vid,
-							       flags);
+			err = dsa_port_do_tag_8021q_vlan_add(dp, info->vid,
+							     flags);
 			if (err)
 				return err;
 		}
@@ -258,15 +261,15 @@ int dsa_switch_tag_8021q_vlan_add(struct dsa_switch *ds,
 int dsa_switch_tag_8021q_vlan_del(struct dsa_switch *ds,
 				  struct dsa_notifier_tag_8021q_vlan_info *info)
 {
-	int port, err;
+	struct dsa_port *dp;
+	int err;
 
 	if (!ds->ops->tag_8021q_vlan_del || !ds->tag_8021q_ctx)
 		return 0;
 
-	for (port = 0; port < ds->num_ports; port++) {
-		if (dsa_switch_tag_8021q_vlan_match(ds, port, info)) {
-			err = dsa_switch_do_tag_8021q_vlan_del(ds, port,
-							       info->vid);
+	dsa_switch_for_each_port(dp, ds) {
+		if (dsa_port_tag_8021q_vlan_match(dp, info)) {
+			err = dsa_port_do_tag_8021q_vlan_del(dp, info->vid);
 			if (err)
 				return err;
 		}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [RFC PATCH v2 net-next 8/8] net: dsa: tag_8021q: finish conversion to dsa_switch_for_each_port
  2021-08-10 16:14 [RFC PATCH v2 net-next 0/8] Remove the "dsa_to_port in a loop" antipattern Vladimir Oltean
                   ` (6 preceding siblings ...)
  2021-08-10 16:14 ` [RFC PATCH v2 net-next 7/8] net: dsa: convert cross-chip notifiers to iterate using dp Vladimir Oltean
@ 2021-08-10 16:14 ` Vladimir Oltean
  7 siblings, 0 replies; 11+ messages in thread
From: Vladimir Oltean @ 2021-08-10 16:14 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, David S. Miller
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Kurt Kanzenbach, Woojung Huh, UNGLinuxDriver, Sean Wang,
	Landen Chao, DENG Qingfang, Matthias Brugger, Claudiu Manoil,
	Alexandre Belloni, George McCollister

The tag_8021q cross-chip notifiers have been converted to iterate using
dp, but the setup and teardown functions have not yet. This patch makes
that change.

Note that we jump directly to the "for_each_available_port" variant
because it didn't make too much sense to set up tag_8021q for unused
ports even before, but skipping them was too much of a hassle.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/dsa/tag_8021q.c | 39 ++++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index 8e4fbbfc6d86..9e7c6b46d62a 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -428,13 +428,13 @@ void dsa_tag_8021q_bridge_tx_fwd_unoffload(struct dsa_switch *ds, int port,
 EXPORT_SYMBOL_GPL(dsa_tag_8021q_bridge_tx_fwd_unoffload);
 
 /* Set up a port's tag_8021q RX and TX VLAN for standalone mode operation */
-static int dsa_tag_8021q_port_setup(struct dsa_switch *ds, int port)
+static int dsa_port_tag_8021q_setup(struct dsa_port *dp)
 {
-	struct dsa_8021q_context *ctx = ds->tag_8021q_ctx;
-	struct dsa_port *dp = dsa_to_port(ds, port);
-	u16 rx_vid = dsa_8021q_rx_vid(ds, port);
-	u16 tx_vid = dsa_8021q_tx_vid(ds, port);
+	struct dsa_8021q_context *ctx = dp->ds->tag_8021q_ctx;
+	struct dsa_switch *ds = dp->ds;
 	struct net_device *master;
+	int port = dp->index;
+	u16 rx_vid, tx_vid;
 	int err;
 
 	/* The CPU port is implicitly configured by
@@ -443,6 +443,8 @@ static int dsa_tag_8021q_port_setup(struct dsa_switch *ds, int port)
 	if (!dsa_port_is_user(dp))
 		return 0;
 
+	rx_vid = dsa_8021q_rx_vid(ds, port);
+	tx_vid = dsa_8021q_tx_vid(ds, port);
 	master = dp->cpu_dp->master;
 
 	/* Add this user port's RX VID to the membership list of all others
@@ -473,13 +475,13 @@ static int dsa_tag_8021q_port_setup(struct dsa_switch *ds, int port)
 	return err;
 }
 
-static void dsa_tag_8021q_port_teardown(struct dsa_switch *ds, int port)
+static void dsa_port_tag_8021q_teardown(struct dsa_port *dp)
 {
-	struct dsa_8021q_context *ctx = ds->tag_8021q_ctx;
-	struct dsa_port *dp = dsa_to_port(ds, port);
-	u16 rx_vid = dsa_8021q_rx_vid(ds, port);
-	u16 tx_vid = dsa_8021q_tx_vid(ds, port);
+	struct dsa_8021q_context *ctx = dp->ds->tag_8021q_ctx;
+	struct dsa_switch *ds = dp->ds;
 	struct net_device *master;
+	int port = dp->index;
+	u16 rx_vid, tx_vid;
 
 	/* The CPU port is implicitly configured by
 	 * configuring the front-panel ports
@@ -487,6 +489,8 @@ static void dsa_tag_8021q_port_teardown(struct dsa_switch *ds, int port)
 	if (!dsa_port_is_user(dp))
 		return;
 
+	rx_vid = dsa_8021q_rx_vid(ds, port);
+	tx_vid = dsa_8021q_tx_vid(ds, port);
 	master = dp->cpu_dp->master;
 
 	dsa_port_tag_8021q_vlan_del(dp, rx_vid);
@@ -498,16 +502,17 @@ static void dsa_tag_8021q_port_teardown(struct dsa_switch *ds, int port)
 
 static int dsa_tag_8021q_setup(struct dsa_switch *ds)
 {
-	int err, port;
+	struct dsa_port *dp;
+	int err;
 
 	ASSERT_RTNL();
 
-	for (port = 0; port < ds->num_ports; port++) {
-		err = dsa_tag_8021q_port_setup(ds, port);
+	dsa_switch_for_each_available_port(dp, ds) {
+		err = dsa_port_tag_8021q_setup(dp);
 		if (err < 0) {
 			dev_err(ds->dev,
 				"Failed to setup VLAN tagging for port %d: %pe\n",
-				port, ERR_PTR(err));
+				dp->index, ERR_PTR(err));
 			return err;
 		}
 	}
@@ -517,12 +522,12 @@ static int dsa_tag_8021q_setup(struct dsa_switch *ds)
 
 static void dsa_tag_8021q_teardown(struct dsa_switch *ds)
 {
-	int port;
+	struct dsa_port *dp;
 
 	ASSERT_RTNL();
 
-	for (port = 0; port < ds->num_ports; port++)
-		dsa_tag_8021q_port_teardown(ds, port);
+	dsa_switch_for_each_available_port(dp, ds)
+		dsa_port_tag_8021q_teardown(dp);
 }
 
 int dsa_tag_8021q_register(struct dsa_switch *ds, __be16 proto)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH v2 net-next 4/8] net: dsa: b53: express b53_for_each_port in terms of dsa_switch_for_each_port
  2021-08-10 16:14 ` [RFC PATCH v2 net-next 4/8] net: dsa: b53: express b53_for_each_port in terms of dsa_switch_for_each_port Vladimir Oltean
@ 2021-08-11  8:40   ` Florian Fainelli
  0 siblings, 0 replies; 11+ messages in thread
From: Florian Fainelli @ 2021-08-11  8:40 UTC (permalink / raw)
  To: Vladimir Oltean, netdev, Jakub Kicinski, David S. Miller
  Cc: Andrew Lunn, Vivien Didelot, Vladimir Oltean, Kurt Kanzenbach,
	Woojung Huh, UNGLinuxDriver, Sean Wang, Landen Chao,
	DENG Qingfang, Matthias Brugger, Claudiu Manoil,
	Alexandre Belloni, George McCollister



On 8/10/2021 9:14 AM, Vladimir Oltean wrote:
> Merging the two allows us to remove the open-coded
> "dev->enabled_ports & BIT(i)" check from b53_br_join and b53_br_leave,
> while still avoiding a quadratic iteration through the switch's ports.
> 
> Sadly I don't know if it's possible to completely get rid of
> b53_for_each_port and replace it with dsa_switch_for_each_available_port,
> especially for the platforms that use pdata and not OF bindings.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC PATCH v2 net-next 3/8] net: dsa: remove the "dsa_to_port in a loop" antipattern from drivers
  2021-08-10 16:14 ` [RFC PATCH v2 net-next 3/8] net: dsa: remove the "dsa_to_port in a loop" antipattern from drivers Vladimir Oltean
@ 2021-08-11  8:57   ` kernel test robot
  0 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2021-08-11  8:57 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4367 bytes --]

Hi Vladimir,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Vladimir-Oltean/Remove-the-dsa_to_port-in-a-loop-antipattern/20210811-002035
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 4ef3960ea19c3b2bced37405b251f05fd4b35545
config: x86_64-randconfig-a016-20210810 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d39ebdae674c8efc84ebe8dc32716ec353220530)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/ed3baa791b1827372c9df651fca62a9152f2c852
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Vladimir-Oltean/Remove-the-dsa_to_port-in-a-loop-antipattern/20210811-002035
        git checkout ed3baa791b1827372c9df651fca62a9152f2c852
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/net/dsa/mv88e6xxx/chip.c:2009:44: warning: variable 'port' is uninitialized when used here [-Wuninitialized]
                   err = mv88e6xxx_port_add_broadcast(chip, port, vid);
                                                            ^~~~
   drivers/net/dsa/mv88e6xxx/chip.c:1996:10: note: initialize the variable 'port' to silence this warning
           int port;
                   ^
                    = 0
   1 warning generated.
--
>> drivers/net/dsa/sja1105/sja1105_main.c:394:37: warning: variable 'port' is uninitialized when used here [-Wuninitialized]
                   default_l2_lookup_params.maxaddrp[port] = max_fdb_entries;
                                                     ^~~~
   drivers/net/dsa/sja1105/sja1105_main.c:383:10: note: initialize the variable 'port' to silence this warning
           int port, num_used_ports = 0;
                   ^
                    = 0
   1 warning generated.


vim +/port +2009 drivers/net/dsa/mv88e6xxx/chip.c

87fa886e1fb7d0 Andrew Lunn       2017-11-09  1991  
87fa886e1fb7d0 Andrew Lunn       2017-11-09  1992  static int mv88e6xxx_broadcast_setup(struct mv88e6xxx_chip *chip, u16 vid)
87fa886e1fb7d0 Andrew Lunn       2017-11-09  1993  {
ed3baa791b1827 Vladimir Oltean   2021-08-10  1994  	struct dsa_switch *ds = chip->ds;
ed3baa791b1827 Vladimir Oltean   2021-08-10  1995  	struct dsa_port *dp;
87fa886e1fb7d0 Andrew Lunn       2017-11-09  1996  	int port;
87fa886e1fb7d0 Andrew Lunn       2017-11-09  1997  	int err;
87fa886e1fb7d0 Andrew Lunn       2017-11-09  1998  
ed3baa791b1827 Vladimir Oltean   2021-08-10  1999  	dsa_switch_for_each_available_port(dp, ds) {
8d1d8298eb0075 Tobias Waldekranz 2021-03-18  2000  		struct net_device *brport;
8d1d8298eb0075 Tobias Waldekranz 2021-03-18  2001  
8d1d8298eb0075 Tobias Waldekranz 2021-03-18  2002  		brport = dsa_port_to_bridge_port(dp);
8d1d8298eb0075 Tobias Waldekranz 2021-03-18  2003  		if (brport && !br_port_flag_is_set(brport, BR_BCAST_FLOOD))
8d1d8298eb0075 Tobias Waldekranz 2021-03-18  2004  			/* Skip bridged user ports where broadcast
8d1d8298eb0075 Tobias Waldekranz 2021-03-18  2005  			 * flooding is disabled.
8d1d8298eb0075 Tobias Waldekranz 2021-03-18  2006  			 */
8d1d8298eb0075 Tobias Waldekranz 2021-03-18  2007  			continue;
8d1d8298eb0075 Tobias Waldekranz 2021-03-18  2008  
87fa886e1fb7d0 Andrew Lunn       2017-11-09 @2009  		err = mv88e6xxx_port_add_broadcast(chip, port, vid);
87fa886e1fb7d0 Andrew Lunn       2017-11-09  2010  		if (err)
87fa886e1fb7d0 Andrew Lunn       2017-11-09  2011  			return err;
87fa886e1fb7d0 Andrew Lunn       2017-11-09  2012  	}
87fa886e1fb7d0 Andrew Lunn       2017-11-09  2013  
87fa886e1fb7d0 Andrew Lunn       2017-11-09  2014  	return 0;
87fa886e1fb7d0 Andrew Lunn       2017-11-09  2015  }
87fa886e1fb7d0 Andrew Lunn       2017-11-09  2016  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 37724 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-08-11  8:57 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-10 16:14 [RFC PATCH v2 net-next 0/8] Remove the "dsa_to_port in a loop" antipattern Vladimir Oltean
2021-08-10 16:14 ` [RFC PATCH v2 net-next 1/8] net: dsa: introduce a dsa_port_is_unused helper Vladimir Oltean
2021-08-10 16:14 ` [RFC PATCH v2 net-next 2/8] net: dsa: remove the "dsa_to_port in a loop" antipattern from the core Vladimir Oltean
2021-08-10 16:14 ` [RFC PATCH v2 net-next 3/8] net: dsa: remove the "dsa_to_port in a loop" antipattern from drivers Vladimir Oltean
2021-08-11  8:57   ` kernel test robot
2021-08-10 16:14 ` [RFC PATCH v2 net-next 4/8] net: dsa: b53: express b53_for_each_port in terms of dsa_switch_for_each_port Vladimir Oltean
2021-08-11  8:40   ` Florian Fainelli
2021-08-10 16:14 ` [RFC PATCH v2 net-next 5/8] net: dsa: finish conversion to dsa_switch_for_each_port Vladimir Oltean
2021-08-10 16:14 ` [RFC PATCH v2 net-next 6/8] net: dsa: remove gratuitous use of dsa_is_{user,dsa,cpu}_port Vladimir Oltean
2021-08-10 16:14 ` [RFC PATCH v2 net-next 7/8] net: dsa: convert cross-chip notifiers to iterate using dp Vladimir Oltean
2021-08-10 16:14 ` [RFC PATCH v2 net-next 8/8] net: dsa: tag_8021q: finish conversion to dsa_switch_for_each_port Vladimir Oltean

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.