diff mbox series

[RFC,net-next,1/3] net: dsa: allow for multiple CPU ports

Message ID 20190824024251.4542-2-marek.behun@nic.cz
State RFC
Delegated to: David Miller
Headers show
Series Multi-CPU DSA support | expand

Commit Message

Marek Behún Aug. 24, 2019, 2:42 a.m. UTC 
Allow for multiple CPU ports in a DSA switch tree. By default assign the
CPU ports to user ports in a round robin way, ie. if there are two CPU
ports connected to eth0 and eth1, and five user ports (lan1..lan5),
assign them as:
  lan1 <-> eth0
  lan2 <-> eth1
  lan3 <-> eth0
  lan4 <-> eth1
  lan5 <-> eth0

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 include/net/dsa.h |  5 +--
 net/dsa/dsa2.c    | 84 +++++++++++++++++++++++++++++++----------------
 2 files changed, 58 insertions(+), 31 deletions(-)

Comments

Andrew Lunn Aug. 24, 2019, 3:43 p.m. UTC | #1 
> +static int dsa_tree_setup_default_cpus(struct dsa_switch_tree *dst)
>  {
>  	struct dsa_switch *ds;
>  	struct dsa_port *dp;
> -	int device, port;
> +	int device, port, i;
>  
> -	/* DSA currently only supports a single CPU port */
> -	dst->cpu_dp = dsa_tree_find_first_cpu(dst);
> -	if (!dst->cpu_dp) {
> +	dsa_tree_fill_cpu_ports(dst);
> +	if (!dst->num_cpu_dps) {
>  		pr_warn("Tree has no master device\n");
>  		return -EINVAL;
>  	}
>  
> -	/* Assign the default CPU port to all ports of the fabric */
> +	/* Assign the default CPU port to all ports of the fabric in a round
> +	 * robin way. This should work nicely for all sane switch tree designs.
> +	 */
> +	i = 0;
> +
>  	for (device = 0; device < DSA_MAX_SWITCHES; device++) {
>  		ds = dst->ds[device];
>  		if (!ds)
> @@ -238,18 +249,20 @@ static int dsa_tree_setup_default_cpu(struct dsa_switch_tree *dst)
>  		for (port = 0; port < ds->num_ports; port++) {
>  			dp = &ds->ports[port];
>  
> -			if (dsa_port_is_user(dp) || dsa_port_is_dsa(dp))
> -				dp->cpu_dp = dst->cpu_dp;
> +			if (dsa_port_is_user(dp) || dsa_port_is_dsa(dp)) {
> +				dp->cpu_dp = dst->cpu_dps[i++];
> +				if (i == dst->num_cpu_dps)
> +					i = 0;
> +			}

Hi Marek

For a single switch, i think this is O.K, but when you have a cluster,
maybe a different allocation should be considered? If this switch has
a local CPU port, use it. Only round robing between remote CPU ports
when there is no local CPU port?

For a two switch setup and each switch having its own CPU port, your
allocation will cause half the CPU traffic to go across the DSA link
between the two switches. But we really want to keep the DSA link for
traffic between user ports on different switches.

But i don't know if it is worth the effort. I've never seen a D in DSA
setup with multiple CPUs ports. I've only ever seen an single switch
with multiple CPU ports.
Marek Behún Aug. 24, 2019, 5:41 p.m. UTC | #2 
On Sat, 24 Aug 2019 17:43:02 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> But i don't know if it is worth the effort. I've never seen a D in DSA
> setup with multiple CPUs ports. I've only ever seen an single switch
> with multiple CPU ports.

Yes, that exactly. I was thinking about the most optimal algorithm, but
such would need to consider speeds between links too. For example the
DSA port between two switches can be linked at 1 GB, but cpu can be
connected to switch with 2.5G. What assignment is best in that case?

I think that we should try to solve such issue when it arises, if ever.
Such cases are more reason to add the ability to change cpu ports for
given ports.

Marek
diff mbox series

Patch

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 147b757ef8ea..64bd70608f2f 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -123,9 +123,10 @@  struct dsa_switch_tree {
 	struct dsa_platform_data	*pd;
 
 	/*
-	 * The switch port to which the CPU is attached.
+	 * The switch ports to which the CPU is attached.
 	 */
-	struct dsa_port		*cpu_dp;
+	size_t			num_cpu_dps;
+	struct dsa_port		*cpu_dps[DSA_MAX_PORTS];
 
 	/*
 	 * Data for the individual switch chips.
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 8c4eccb0cfe6..c5af89079a6b 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -194,11 +194,12 @@  static bool dsa_tree_setup_routing_table(struct dsa_switch_tree *dst)
 	return complete;
 }
 
-static struct dsa_port *dsa_tree_find_first_cpu(struct dsa_switch_tree *dst)
+static void dsa_tree_fill_cpu_ports(struct dsa_switch_tree *dst)
 {
 	struct dsa_switch *ds;
 	struct dsa_port *dp;
 	int device, port;
+	int count = 0;
 
 	for (device = 0; device < DSA_MAX_SWITCHES; device++) {
 		ds = dst->ds[device];
@@ -208,28 +209,38 @@  static struct dsa_port *dsa_tree_find_first_cpu(struct dsa_switch_tree *dst)
 		for (port = 0; port < ds->num_ports; port++) {
 			dp = &ds->ports[port];
 
-			if (dsa_port_is_cpu(dp))
-				return dp;
+			if (dsa_port_is_cpu(dp)) {
+				if (count == ARRAY_SIZE(dst->cpu_dps)) {
+					pr_warn("Tree has too many CPU ports\n");
+					dst->num_cpu_dps = count;
+					return;
+				}
+
+				dst->cpu_dps[count++] = dp;
+			}
 		}
 	}
 
-	return NULL;
+	dst->num_cpu_dps = count;
 }
 
-static int dsa_tree_setup_default_cpu(struct dsa_switch_tree *dst)
+static int dsa_tree_setup_default_cpus(struct dsa_switch_tree *dst)
 {
 	struct dsa_switch *ds;
 	struct dsa_port *dp;
-	int device, port;
+	int device, port, i;
 
-	/* DSA currently only supports a single CPU port */
-	dst->cpu_dp = dsa_tree_find_first_cpu(dst);
-	if (!dst->cpu_dp) {
+	dsa_tree_fill_cpu_ports(dst);
+	if (!dst->num_cpu_dps) {
 		pr_warn("Tree has no master device\n");
 		return -EINVAL;
 	}
 
-	/* Assign the default CPU port to all ports of the fabric */
+	/* Assign the default CPU port to all ports of the fabric in a round
+	 * robin way. This should work nicely for all sane switch tree designs.
+	 */
+	i = 0;
+
 	for (device = 0; device < DSA_MAX_SWITCHES; device++) {
 		ds = dst->ds[device];
 		if (!ds)
@@ -238,18 +249,20 @@  static int dsa_tree_setup_default_cpu(struct dsa_switch_tree *dst)
 		for (port = 0; port < ds->num_ports; port++) {
 			dp = &ds->ports[port];
 
-			if (dsa_port_is_user(dp) || dsa_port_is_dsa(dp))
-				dp->cpu_dp = dst->cpu_dp;
+			if (dsa_port_is_user(dp) || dsa_port_is_dsa(dp)) {
+				dp->cpu_dp = dst->cpu_dps[i++];
+				if (i == dst->num_cpu_dps)
+					i = 0;
+			}
 		}
 	}
 
 	return 0;
 }
 
-static void dsa_tree_teardown_default_cpu(struct dsa_switch_tree *dst)
+static void dsa_tree_teardown_default_cpus(struct dsa_switch_tree *dst)
 {
-	/* DSA currently only supports a single CPU port */
-	dst->cpu_dp = NULL;
+	dst->num_cpu_dps = 0;
 }
 
 static int dsa_port_setup(struct dsa_port *dp)
@@ -493,21 +506,34 @@  static void dsa_tree_teardown_switches(struct dsa_switch_tree *dst)
 	}
 }
 
-static int dsa_tree_setup_master(struct dsa_switch_tree *dst)
+static int dsa_tree_setup_masters(struct dsa_switch_tree *dst)
 {
-	struct dsa_port *cpu_dp = dst->cpu_dp;
-	struct net_device *master = cpu_dp->master;
+	int i;
+	int err;
 
-	/* DSA currently supports a single pair of CPU port and master device */
-	return dsa_master_setup(master, cpu_dp);
+	for (i = 0; i < dst->num_cpu_dps; ++i) {
+		struct dsa_port *cpu_dp = dst->cpu_dps[i];
+		struct net_device *master = cpu_dp->master;
+
+		err = dsa_master_setup(master, cpu_dp);
+		if (err)
+			goto teardown;
+	}
+
+	return 0;
+teardown:
+	for (--i; i >= 0; --i)
+		dsa_master_teardown(dst->cpu_dps[i]->master);
+
+	return err;
 }
 
-static void dsa_tree_teardown_master(struct dsa_switch_tree *dst)
+static void dsa_tree_teardown_masters(struct dsa_switch_tree *dst)
 {
-	struct dsa_port *cpu_dp = dst->cpu_dp;
-	struct net_device *master = cpu_dp->master;
+	int i;
 
-	return dsa_master_teardown(master);
+	for (i = 0; i < dst->num_cpu_dps; ++i)
+		dsa_master_teardown(dst->cpu_dps[i]->master);
 }
 
 static int dsa_tree_setup(struct dsa_switch_tree *dst)
@@ -525,7 +551,7 @@  static int dsa_tree_setup(struct dsa_switch_tree *dst)
 	if (!complete)
 		return 0;
 
-	err = dsa_tree_setup_default_cpu(dst);
+	err = dsa_tree_setup_default_cpus(dst);
 	if (err)
 		return err;
 
@@ -533,7 +559,7 @@  static int dsa_tree_setup(struct dsa_switch_tree *dst)
 	if (err)
 		goto teardown_default_cpu;
 
-	err = dsa_tree_setup_master(dst);
+	err = dsa_tree_setup_masters(dst);
 	if (err)
 		goto teardown_switches;
 
@@ -546,7 +572,7 @@  static int dsa_tree_setup(struct dsa_switch_tree *dst)
 teardown_switches:
 	dsa_tree_teardown_switches(dst);
 teardown_default_cpu:
-	dsa_tree_teardown_default_cpu(dst);
+	dsa_tree_teardown_default_cpus(dst);
 
 	return err;
 }
@@ -556,11 +582,11 @@  static void dsa_tree_teardown(struct dsa_switch_tree *dst)
 	if (!dst->setup)
 		return;
 
-	dsa_tree_teardown_master(dst);
+	dsa_tree_teardown_masters(dst);
 
 	dsa_tree_teardown_switches(dst);
 
-	dsa_tree_teardown_default_cpu(dst);
+	dsa_tree_teardown_default_cpus(dst);
 
 	pr_info("DSA: tree %d torn down\n", dst->index);