[PATCH net-next 0/5] net: dsa: use per-port upstream port

[PATCH net-next 0/5] net: dsa: use per-port upstream port

[Vivien Didelot]

An upstream port is a local switch port used to reach a CPU port.

DSA still considers a unique CPU port in the whole switch fabric and
thus return a unique upstream port for a given switch. This is wrong in
a multiple CPU ports environment.

We are now switching to using the dedicated CPU port assigned to each
port in order to get rid of the deprecated unique tree CPU port.

This patchset makes the dsa_upstream_port() helper take a port argument
and goes one step closer complete support for multiple CPU ports.

Vivien Didelot (5):
  net: dsa: mv88e6xxx: egress floods all DSA ports
  net: dsa: mv88e6xxx: helper to setup upstream port
  net: dsa: mv88e6xxx: setup global upstream port
  net: dsa: assign a CPU port to DSA port
  net: dsa: return per-port upstream port

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

-- 
2.15.1

[PATCH net-next 1/5] net: dsa: mv88e6xxx: egress floods all DSA ports

[Vivien Didelot]

The mv88e6xxx driver currently assumes a single CPU port in the fabric
and thus floods frames with unknown DA on a single DSA port, the one
that is one hop closer to the CPU port.

With multiple CPU ports in mind, this isn't true anymore because CPU
ports could be found behind both DSA ports of a device in-between
others.

For example in a A <-> B <-> C fabric, both A and C having CPU ports,
device B will have to flood such frame to its two DSA ports.

This patch considers both CPU and DSA ports of a device as upstream
ports, where to flood frames with unknown DA addresses.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index b5e0987c88f0..15475dbb738b 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1723,7 +1723,8 @@ static int mv88e6xxx_setup_message_port(struct mv88e6xxx_chip *chip, int port)
 
 static int mv88e6xxx_setup_egress_floods(struct mv88e6xxx_chip *chip, int port)
 {
-	bool flood = port == dsa_upstream_port(chip->ds);
+	struct dsa_switch *ds = chip->ds;
+	bool flood = dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port);
 
 	/* Upstream ports flood frames with unknown unicast or multicast DA */
 	if (chip->info->ops->port_set_egress_floods)
-- 
2.15.1

[PATCH net-next 2/5] net: dsa: mv88e6xxx: helper to setup upstream port

[Vivien Didelot]

Add a helper function to setup the upstream port of a given port.

This is the port used to reach the dedicated CPU port. This function
will be extended later to setup the global upstream port as well.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 15475dbb738b..3dce1ab19582 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1743,6 +1743,22 @@ static int mv88e6xxx_serdes_power(struct mv88e6xxx_chip *chip, int port,
 	return 0;
 }
 
+static int mv88e6xxx_setup_upstream_port(struct mv88e6xxx_chip *chip, int port)
+{
+	struct dsa_switch *ds = chip->ds;
+	int upstream_port = dsa_upstream_port(ds);
+	int err;
+
+	if (chip->info->ops->port_set_upstream_port) {
+		err = chip->info->ops->port_set_upstream_port(chip, port,
+							      upstream_port);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
 static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
 {
 	struct dsa_switch *ds = chip->ds;
@@ -1813,13 +1829,9 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
 	if (err)
 		return err;
 
-	reg = 0;
-	if (chip->info->ops->port_set_upstream_port) {
-		err = chip->info->ops->port_set_upstream_port(
-			chip, port, dsa_upstream_port(ds));
-		if (err)
-			return err;
-	}
+	err = mv88e6xxx_setup_upstream_port(chip, port);
+	if (err)
+		return err;
 
 	err = mv88e6xxx_port_set_8021q_mode(chip, port,
 				MV88E6XXX_PORT_CTL2_8021Q_MODE_DISABLED);
-- 
2.15.1

[PATCH net-next 3/5] net: dsa: mv88e6xxx: setup global upstream port

[Vivien Didelot]

Move the setup of the global upstream port within the
mv88e6xxx_setup_upstream_port function.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 3dce1ab19582..5de8596b01ad 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1756,6 +1756,22 @@ static int mv88e6xxx_setup_upstream_port(struct mv88e6xxx_chip *chip, int port)
 			return err;
 	}
 
+	if (port == upstream_port) {
+		if (chip->info->ops->set_cpu_port) {
+			err = chip->info->ops->set_cpu_port(chip,
+							    upstream_port);
+			if (err)
+				return err;
+		}
+
+		if (chip->info->ops->set_egress_port) {
+			err = chip->info->ops->set_egress_port(chip,
+							       upstream_port);
+			if (err)
+				return err;
+		}
+	}
+
 	return 0;
 }
 
@@ -1957,21 +1973,8 @@ static int mv88e6xxx_set_ageing_time(struct dsa_switch *ds,
 static int mv88e6xxx_g1_setup(struct mv88e6xxx_chip *chip)
 {
 	struct dsa_switch *ds = chip->ds;
-	u32 upstream_port = dsa_upstream_port(ds);
 	int err;
 
-	if (chip->info->ops->set_cpu_port) {
-		err = chip->info->ops->set_cpu_port(chip, upstream_port);
-		if (err)
-			return err;
-	}
-
-	if (chip->info->ops->set_egress_port) {
-		err = chip->info->ops->set_egress_port(chip, upstream_port);
-		if (err)
-			return err;
-	}
-
 	/* Disable remote management, and set the switch's DSA device number. */
 	err = mv88e6xxx_g1_write(chip, MV88E6XXX_G1_CTL2,
 				 MV88E6XXX_G1_CTL2_MULTIPLE_CASCADE |
-- 
2.15.1

[PATCH net-next 4/5] net: dsa: assign a CPU port to DSA port

[Vivien Didelot]

DSA ports also need to have a dedicated CPU port assigned to them,
because they need to know where to egress frames targeting the CPU,
e.g. To_Cpu frames received on a Marvell Tag port.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/dsa2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 1e287420ff49..21f9bed11988 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -241,7 +241,7 @@ 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))
+			if (dsa_port_is_user(dp) || dsa_port_is_dsa(dp))
 				dp->cpu_dp = dst->cpu_dp;
 		}
 	}
-- 
2.15.1

[PATCH net-next 5/5] net: dsa: return per-port upstream port

[Vivien Didelot]

The current dsa_upstream_port() helper still assumes a unique CPU port
in the whole switch fabric. This is becoming wrong, as every port in the
fabric has its dedicated CPU port, thus every port has an upstream port.

Add a port argument to the dsa_upstream_port() helper and fetch its CPU
port instead of the deprecated unique fabric CPU port. A CPU or unused
port has no dedicated CPU port, so return itself in this case.

At the same time, change the return value from u8 to unsigned int since
there is no need to limit the size here.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
 include/net/dsa.h                | 9 ++++++---
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 5de8596b01ad..fe4881795906 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1746,7 +1746,7 @@ static int mv88e6xxx_serdes_power(struct mv88e6xxx_chip *chip, int port,
 static int mv88e6xxx_setup_upstream_port(struct mv88e6xxx_chip *chip, int port)
 {
 	struct dsa_switch *ds = chip->ds;
-	int upstream_port = dsa_upstream_port(ds);
+	int upstream_port = dsa_upstream_port(ds, port);
 	int err;
 
 	if (chip->info->ops->port_set_upstream_port) {
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 8198efcc8ced..d29feccaefab 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -307,10 +307,13 @@ static inline unsigned int dsa_towards_port(struct dsa_switch *ds, int device,
 }
 
 /* Return the local port used to reach the dedicated CPU port */
-static inline u8 dsa_upstream_port(struct dsa_switch *ds)
+static inline unsigned int dsa_upstream_port(struct dsa_switch *ds, int port)
 {
-	struct dsa_switch_tree *dst = ds->dst;
-	struct dsa_port *cpu_dp = dst->cpu_dp;
+	const struct dsa_port *dp = dsa_to_port(ds, port);
+	const struct dsa_port *cpu_dp = dp->cpu_dp;
+
+	if (!cpu_dp)
+		return port;
 
 	return dsa_towards_port(ds, cpu_dp->ds->index, cpu_dp->index);
 }
-- 
2.15.1

Re: [PATCH net-next 0/5] net: dsa: use per-port upstream port

[David Miller]

From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Date: Mon,  4 Dec 2017 12:34:52 -0500

> An upstream port is a local switch port used to reach a CPU port.
> 
> DSA still considers a unique CPU port in the whole switch fabric and
> thus return a unique upstream port for a given switch. This is wrong in
> a multiple CPU ports environment.
> 
> We are now switching to using the dedicated CPU port assigned to each
> port in order to get rid of the deprecated unique tree CPU port.
> 
> This patchset makes the dsa_upstream_port() helper take a port argument
> and goes one step closer complete support for multiple CPU ports.

Please adhere to reverse-christmas-tree for variable declarations in
these changes.

I know it can be a pain when there are inter-variable dependencies
wrt. assignments, but just move it below the declaration and into a
real statement.

Thanks.

Re: [PATCH net-next 0/5] net: dsa: use per-port upstream port

[Vivien Didelot]

Hi David,

David Miller <davem@davemloft.net> writes:

> From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> Date: Mon,  4 Dec 2017 12:34:52 -0500
>
>> An upstream port is a local switch port used to reach a CPU port.
>> 
>> DSA still considers a unique CPU port in the whole switch fabric and
>> thus return a unique upstream port for a given switch. This is wrong in
>> a multiple CPU ports environment.
>> 
>> We are now switching to using the dedicated CPU port assigned to each
>> port in order to get rid of the deprecated unique tree CPU port.
>> 
>> This patchset makes the dsa_upstream_port() helper take a port argument
>> and goes one step closer complete support for multiple CPU ports.
>
> Please adhere to reverse-christmas-tree for variable declarations in
> these changes.

Totally makes sense for this month of the year, I'm replanting a v2!


Thanks,

        Vivien