linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/5] net: dsa: add cross-chip VLAN support
@ 2017-06-06 20:56 Vivien Didelot
  2017-06-06 20:56 ` [PATCH net-next 1/5] net: dsa: mv88e6xxx: define membership on VLAN add Vivien Didelot
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Vivien Didelot @ 2017-06-06 20:56 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot

The current code in DSA does not support cross-chip VLAN. This means
that in a multi-chip environment such as this one (similar to ZII Rev B)

         [CPU].................... (mdio)
    (eth0) |   :       :          :
          _|_____    _______    _______
         [__sw0__]--[__sw1__]--[__sw2__]
          |  |  |    |  |  |    |  |  |
          v  v  v    v  v  v    v  v  v
          p1 p2 p3   p4 p5 p6   p7 p8 p9 

adding a VLAN to p9 won't be enough to reach the CPU, until at least one
port of sw0 and sw1 join the VLAN as well and become aware of the VID.

This patchset makes the DSA core program the VLAN on the CPU and DSA
links itself, which brings seamlessly cross-chip VLAN support to DSA.

With this series applied*, the hardware VLAN tables of a 3-switch setup
look like this after adding a VLAN to only one port of the end switch:

    # cat /sys/class/net/br0/bridge/default_pvid 
    42
    # cat /sys/kernel/debug/mv88e6xxx/sw{0,1,2}/vtu
    # ip link set up master br0 dev lan6
    # cat /sys/kernel/debug/mv88e6xxx/sw{0,1,2}/vtu
     VID  FID  SID  0  1  2  3  4  5  6
      42    1    0  x  x  x  x  x  =  =
     VID  FID  SID  0  1  2  3  4  5  6
      42    1    0  x  x  x  x  x  =  =
     VID  FID  SID  0  1  2  3  4  5  6  7  8  9
      42    1    0  u  x  x  x  x  x  x  x  x  =

('x' is excluded, 'u' is untagged, '=' is unmodified DSA and CPU ports.)

Completely removing a VLAN entry (which is currently the responsibility
of drivers anyway) is not supported yet since it requires some caching.

(*) the output is shown from this out-of-tree debugfs patch:
https://github.com/vivien/linux/commit/7b61a684b9d6b6a499135a587c7f62a1fddceb8b.patch

Vivien Didelot (5):
  net: dsa: mv88e6xxx: define membership on VLAN add
  net: dsa: check VLAN capability of every switch
  net: dsa: add CPU and DSA ports as VLAN members
  net: dsa: mv88e6xxx: exclude all ports in new VLAN
  net: dsa: mv88e6xxx: do not purge a VTU entry

 drivers/net/dsa/mv88e6xxx/chip.c | 38 +++++++++++++++-----------------------
 net/dsa/switch.c                 | 30 ++++++++++++++++++++----------
 2 files changed, 35 insertions(+), 33 deletions(-)

-- 
2.13.0

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

* [PATCH net-next 1/5] net: dsa: mv88e6xxx: define membership on VLAN add
  2017-06-06 20:56 [PATCH net-next 0/5] net: dsa: add cross-chip VLAN support Vivien Didelot
@ 2017-06-06 20:56 ` Vivien Didelot
  2017-06-07 19:31   ` Florian Fainelli
  2017-06-06 20:56 ` [PATCH net-next 2/5] net: dsa: check VLAN capability of every switch Vivien Didelot
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Vivien Didelot @ 2017-06-06 20:56 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot

Define the target port membership of the VLAN entry in
mv88e6xxx_port_vlan_add where ds is scoped.

Allow the DSA core to call later the port_vlan_add operation for CPU or
DSA ports, by using the Unmodified membership for these ports, as in the
current behavior.

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

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 117f275e3fb6..93078bbe3cb5 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1274,7 +1274,7 @@ mv88e6xxx_port_vlan_prepare(struct dsa_switch *ds, int port,
 }
 
 static int _mv88e6xxx_port_vlan_add(struct mv88e6xxx_chip *chip, int port,
-				    u16 vid, bool untagged)
+				    u16 vid, u8 member)
 {
 	struct mv88e6xxx_vtu_entry vlan;
 	int err;
@@ -1283,9 +1283,7 @@ static int _mv88e6xxx_port_vlan_add(struct mv88e6xxx_chip *chip, int port,
 	if (err)
 		return err;
 
-	vlan.member[port] = untagged ?
-		GLOBAL_VTU_DATA_MEMBER_TAG_UNTAGGED :
-		GLOBAL_VTU_DATA_MEMBER_TAG_TAGGED;
+	vlan.member[port] = member;
 
 	return mv88e6xxx_vtu_loadpurge(chip, &vlan);
 }
@@ -1297,15 +1295,23 @@ static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
 	struct mv88e6xxx_chip *chip = ds->priv;
 	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
 	bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
+	u8 member;
 	u16 vid;
 
 	if (!chip->info->max_vid)
 		return;
 
+	if (dsa_is_dsa_port(ds, port) || dsa_is_cpu_port(ds, port))
+		member = GLOBAL_VTU_DATA_MEMBER_TAG_UNMODIFIED;
+	else if (untagged)
+		member = GLOBAL_VTU_DATA_MEMBER_TAG_UNTAGGED;
+	else
+		member = GLOBAL_VTU_DATA_MEMBER_TAG_TAGGED;
+
 	mutex_lock(&chip->reg_lock);
 
 	for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid)
-		if (_mv88e6xxx_port_vlan_add(chip, port, vid, untagged))
+		if (_mv88e6xxx_port_vlan_add(chip, port, vid, member))
 			netdev_err(ds->ports[port].netdev,
 				   "failed to add VLAN %d%c\n",
 				   vid, untagged ? 'u' : 't');
-- 
2.13.0

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

* [PATCH net-next 2/5] net: dsa: check VLAN capability of every switch
  2017-06-06 20:56 [PATCH net-next 0/5] net: dsa: add cross-chip VLAN support Vivien Didelot
  2017-06-06 20:56 ` [PATCH net-next 1/5] net: dsa: mv88e6xxx: define membership on VLAN add Vivien Didelot
@ 2017-06-06 20:56 ` Vivien Didelot
  2017-06-07 19:39   ` Florian Fainelli
  2017-06-06 20:56 ` [PATCH net-next 3/5] net: dsa: add CPU and DSA ports as VLAN members Vivien Didelot
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Vivien Didelot @ 2017-06-06 20:56 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot

Now that the VLAN object is propagated to every switch chip of the
switch fabric, we can easily ensure that they all support the required
VLAN operations before modifying an entry on a single switch.

To achieve that, remove the condition skipping other target switches,
and add a bitmap of VLAN members, eventually containing the target port,
if we are programming the switch target.

This will allow us to easily add other VLAN members, such as the DSA or
CPU ports (to introduce cross-chip VLAN support) or the other port
members if we want to reduce hardware accesses later.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/switch.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index d8e5c311ee7c..f235ae1e9777 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -159,19 +159,27 @@ static int dsa_switch_vlan_add(struct dsa_switch *ds,
 {
 	const struct switchdev_obj_port_vlan *vlan = info->vlan;
 	struct switchdev_trans *trans = info->trans;
+	DECLARE_BITMAP(members, ds->num_ports);
+	int port, err;
 
-	/* Do not care yet about other switch chips of the fabric */
-	if (ds->index != info->sw_index)
-		return 0;
+	/* Build a mask of VLAN members */
+	bitmap_zero(members, ds->num_ports);
+	if (ds->index == info->sw_index)
+		set_bit(info->port, members);
 
 	if (switchdev_trans_ph_prepare(trans)) {
 		if (!ds->ops->port_vlan_prepare || !ds->ops->port_vlan_add)
 			return -EOPNOTSUPP;
 
-		return ds->ops->port_vlan_prepare(ds, info->port, vlan, trans);
+		for_each_set_bit(port, members, ds->num_ports) {
+			err = ds->ops->port_vlan_prepare(ds, port, vlan, trans);
+			if (err)
+				return err;
+		}
 	}
 
-	ds->ops->port_vlan_add(ds, info->port, vlan, trans);
+	for_each_set_bit(port, members, ds->num_ports)
+		ds->ops->port_vlan_add(ds, port, vlan, trans);
 
 	return 0;
 }
@@ -181,14 +189,13 @@ static int dsa_switch_vlan_del(struct dsa_switch *ds,
 {
 	const struct switchdev_obj_port_vlan *vlan = info->vlan;
 
-	/* Do not care yet about other switch chips of the fabric */
-	if (ds->index != info->sw_index)
-		return 0;
-
 	if (!ds->ops->port_vlan_del)
 		return -EOPNOTSUPP;
 
-	return ds->ops->port_vlan_del(ds, info->port, vlan);
+	if (ds->index == info->sw_index)
+		return ds->ops->port_vlan_del(ds, info->port, vlan);
+
+	return 0;
 }
 
 static int dsa_switch_event(struct notifier_block *nb,
-- 
2.13.0

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

* [PATCH net-next 3/5] net: dsa: add CPU and DSA ports as VLAN members
  2017-06-06 20:56 [PATCH net-next 0/5] net: dsa: add cross-chip VLAN support Vivien Didelot
  2017-06-06 20:56 ` [PATCH net-next 1/5] net: dsa: mv88e6xxx: define membership on VLAN add Vivien Didelot
  2017-06-06 20:56 ` [PATCH net-next 2/5] net: dsa: check VLAN capability of every switch Vivien Didelot
@ 2017-06-06 20:56 ` Vivien Didelot
  2017-06-07 19:00   ` David Miller
  2017-06-07 19:38   ` Florian Fainelli
  2017-06-06 20:56 ` [PATCH net-next 4/5] net: dsa: mv88e6xxx: exclude all ports in new VLAN Vivien Didelot
  2017-06-06 20:56 ` [PATCH net-next 5/5] net: dsa: mv88e6xxx: do not purge a VTU entry Vivien Didelot
  4 siblings, 2 replies; 14+ messages in thread
From: Vivien Didelot @ 2017-06-06 20:56 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot

In a multi-chip switch fabric, it is currently the responsibility of the
driver to add the CPU or DSA (interconnecting chips together) ports as
members of a new VLAN entry. This makes the drivers more complicated.

We want the DSA drivers to be stupid and the DSA core being the one
responsible for caring about the abstracted switch logic and topology.

Make the DSA core program the CPU and DSA ports as part of the VLAN.

This makes all chips of the data path to be aware of VIDs spanning the
the whole fabric and thus, seamlessly add support for cross-chip VLAN.

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

diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index f235ae1e9777..f913cdfe6585 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -166,6 +166,9 @@ static int dsa_switch_vlan_add(struct dsa_switch *ds,
 	bitmap_zero(members, ds->num_ports);
 	if (ds->index == info->sw_index)
 		set_bit(info->port, members);
+	for (port = 0; port < ds->num_ports; ++port)
+		if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port))
+			set_bit(port, members);
 
 	if (switchdev_trans_ph_prepare(trans)) {
 		if (!ds->ops->port_vlan_prepare || !ds->ops->port_vlan_add)
-- 
2.13.0

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

* [PATCH net-next 4/5] net: dsa: mv88e6xxx: exclude all ports in new VLAN
  2017-06-06 20:56 [PATCH net-next 0/5] net: dsa: add cross-chip VLAN support Vivien Didelot
                   ` (2 preceding siblings ...)
  2017-06-06 20:56 ` [PATCH net-next 3/5] net: dsa: add CPU and DSA ports as VLAN members Vivien Didelot
@ 2017-06-06 20:56 ` Vivien Didelot
  2017-06-07 19:34   ` Florian Fainelli
  2017-06-06 20:56 ` [PATCH net-next 5/5] net: dsa: mv88e6xxx: do not purge a VTU entry Vivien Didelot
  4 siblings, 1 reply; 14+ messages in thread
From: Vivien Didelot @ 2017-06-06 20:56 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot

Now that the DSA core adds the CPU and DSA ports itself to the new VLAN
entry, there is no need to include them as members of this VLAN when
initializing a new VTU entry.

As of now, initialize a new VTU entry with all ports excluded.

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

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 93078bbe3cb5..522f023bb17e 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1159,11 +1159,10 @@ static int mv88e6xxx_vtu_get(struct mv88e6xxx_chip *chip, u16 vid,
 		entry->valid = true;
 		entry->vid = vid;
 
-		/* Include only CPU and DSA ports */
+		/* Exclude all ports */
 		for (i = 0; i < mv88e6xxx_num_ports(chip); ++i)
-			entry->member[i] = dsa_is_normal_port(chip->ds, i) ?
-				GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER :
-				GLOBAL_VTU_DATA_MEMBER_TAG_UNMODIFIED;
+			entry->member[i] =
+				GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER;
 
 		return mv88e6xxx_atu_new(chip, &entry->fid);
 	}
-- 
2.13.0

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

* [PATCH net-next 5/5] net: dsa: mv88e6xxx: do not purge a VTU entry
  2017-06-06 20:56 [PATCH net-next 0/5] net: dsa: add cross-chip VLAN support Vivien Didelot
                   ` (3 preceding siblings ...)
  2017-06-06 20:56 ` [PATCH net-next 4/5] net: dsa: mv88e6xxx: exclude all ports in new VLAN Vivien Didelot
@ 2017-06-06 20:56 ` Vivien Didelot
  2017-06-07 19:37   ` Florian Fainelli
  4 siblings, 1 reply; 14+ messages in thread
From: Vivien Didelot @ 2017-06-06 20:56 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot

The mv88e6xxx driver currently tries to be smart and remove by itself a
VLAN entry from the VTU when the driven switch sees no user ports as
members of the VLAN.

This is bad in a multi-chip switch fabric, since a chip in between
others may have no bridge port members, but still needs to be aware of
the VID in order to correctly pass frames in the data path.

Remove the code purging a VTU entry when updating a port membership.

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

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 522f023bb17e..64c0f88f9e79 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1325,9 +1325,8 @@ static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
 static int _mv88e6xxx_port_vlan_del(struct mv88e6xxx_chip *chip,
 				    int port, u16 vid)
 {
-	struct dsa_switch *ds = chip->ds;
 	struct mv88e6xxx_vtu_entry vlan;
-	int i, err;
+	int err;
 
 	err = mv88e6xxx_vtu_get(chip, vid, &vlan, false);
 	if (err)
@@ -1339,18 +1338,6 @@ static int _mv88e6xxx_port_vlan_del(struct mv88e6xxx_chip *chip,
 
 	vlan.member[port] = GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER;
 
-	/* keep the VLAN unless all ports are excluded */
-	vlan.valid = false;
-	for (i = 0; i < mv88e6xxx_num_ports(chip); ++i) {
-		if (dsa_is_cpu_port(ds, i) || dsa_is_dsa_port(ds, i))
-			continue;
-
-		if (vlan.member[i] != GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER) {
-			vlan.valid = true;
-			break;
-		}
-	}
-
 	err = mv88e6xxx_vtu_loadpurge(chip, &vlan);
 	if (err)
 		return err;
-- 
2.13.0

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

* Re: [PATCH net-next 3/5] net: dsa: add CPU and DSA ports as VLAN members
  2017-06-06 20:56 ` [PATCH net-next 3/5] net: dsa: add CPU and DSA ports as VLAN members Vivien Didelot
@ 2017-06-07 19:00   ` David Miller
  2017-06-07 19:38   ` Florian Fainelli
  1 sibling, 0 replies; 14+ messages in thread
From: David Miller @ 2017-06-07 19:00 UTC (permalink / raw)
  To: vivien.didelot; +Cc: netdev, linux-kernel, kernel, f.fainelli, andrew

From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Date: Tue,  6 Jun 2017 16:56:29 -0400

> @@ -166,6 +166,9 @@ static int dsa_switch_vlan_add(struct dsa_switch *ds,
>  	bitmap_zero(members, ds->num_ports);
>  	if (ds->index == info->sw_index)
>  		set_bit(info->port, members);
> +	for (port = 0; port < ds->num_ports; ++port)
> +		if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port))
> +			set_bit(port, members);

Please use the more canonical "x++" post-increment in the for() statement.

Thank you.

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

* Re: [PATCH net-next 1/5] net: dsa: mv88e6xxx: define membership on VLAN add
  2017-06-06 20:56 ` [PATCH net-next 1/5] net: dsa: mv88e6xxx: define membership on VLAN add Vivien Didelot
@ 2017-06-07 19:31   ` Florian Fainelli
  0 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2017-06-07 19:31 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn

On 06/06/2017 01:56 PM, Vivien Didelot wrote:
> Define the target port membership of the VLAN entry in
> mv88e6xxx_port_vlan_add where ds is scoped.
> 
> Allow the DSA core to call later the port_vlan_add operation for CPU or
> DSA ports, by using the Unmodified membership for these ports, as in the
> current behavior.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

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

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

* Re: [PATCH net-next 4/5] net: dsa: mv88e6xxx: exclude all ports in new VLAN
  2017-06-06 20:56 ` [PATCH net-next 4/5] net: dsa: mv88e6xxx: exclude all ports in new VLAN Vivien Didelot
@ 2017-06-07 19:34   ` Florian Fainelli
  0 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2017-06-07 19:34 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn

On 06/06/2017 01:56 PM, Vivien Didelot wrote:
> Now that the DSA core adds the CPU and DSA ports itself to the new VLAN
> entry, there is no need to include them as members of this VLAN when
> initializing a new VTU entry.
> 
> As of now, initialize a new VTU entry with all ports excluded.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

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

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

* Re: [PATCH net-next 5/5] net: dsa: mv88e6xxx: do not purge a VTU entry
  2017-06-06 20:56 ` [PATCH net-next 5/5] net: dsa: mv88e6xxx: do not purge a VTU entry Vivien Didelot
@ 2017-06-07 19:37   ` Florian Fainelli
  2017-06-07 19:59     ` Vivien Didelot
  0 siblings, 1 reply; 14+ messages in thread
From: Florian Fainelli @ 2017-06-07 19:37 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn

On 06/06/2017 01:56 PM, Vivien Didelot wrote:
> The mv88e6xxx driver currently tries to be smart and remove by itself a
> VLAN entry from the VTU when the driven switch sees no user ports as
> members of the VLAN.
> 
> This is bad in a multi-chip switch fabric, since a chip in between
> others may have no bridge port members, but still needs to be aware of
> the VID in order to correctly pass frames in the data path.
> 
> Remove the code purging a VTU entry when updating a port membership.

In that case the switch sitting between two other chips and passing
traffic would still have at least two of its DSA ports be part of a VTU
entry, right?

So could not we just do ....

> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
>  drivers/net/dsa/mv88e6xxx/chip.c | 15 +--------------
>  1 file changed, 1 insertion(+), 14 deletions(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 522f023bb17e..64c0f88f9e79 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -1325,9 +1325,8 @@ static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
>  static int _mv88e6xxx_port_vlan_del(struct mv88e6xxx_chip *chip,
>  				    int port, u16 vid)
>  {
> -	struct dsa_switch *ds = chip->ds;
>  	struct mv88e6xxx_vtu_entry vlan;
> -	int i, err;
> +	int err;
>  
>  	err = mv88e6xxx_vtu_get(chip, vid, &vlan, false);
>  	if (err)
> @@ -1339,18 +1338,6 @@ static int _mv88e6xxx_port_vlan_del(struct mv88e6xxx_chip *chip,
>  
>  	vlan.member[port] = GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER;
>  
> -	/* keep the VLAN unless all ports are excluded */
> -	vlan.valid = false;
> -	for (i = 0; i < mv88e6xxx_num_ports(chip); ++i) {
> -		if (dsa_is_cpu_port(ds, i) || dsa_is_dsa_port(ds, i))
> -			continue;

... break the loop here?

> -
> -		if (vlan.member[i] != GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER) {
> -			vlan.valid = true;
> -			break;
> -		}
> -	}
> -
>  	err = mv88e6xxx_vtu_loadpurge(chip, &vlan);
>  	if (err)
>  		return err;
> 


-- 
Florian

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

* Re: [PATCH net-next 3/5] net: dsa: add CPU and DSA ports as VLAN members
  2017-06-06 20:56 ` [PATCH net-next 3/5] net: dsa: add CPU and DSA ports as VLAN members Vivien Didelot
  2017-06-07 19:00   ` David Miller
@ 2017-06-07 19:38   ` Florian Fainelli
  1 sibling, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2017-06-07 19:38 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn

On 06/06/2017 01:56 PM, Vivien Didelot wrote:
> In a multi-chip switch fabric, it is currently the responsibility of the
> driver to add the CPU or DSA (interconnecting chips together) ports as
> members of a new VLAN entry. This makes the drivers more complicated.
> 
> We want the DSA drivers to be stupid and the DSA core being the one
> responsible for caring about the abstracted switch logic and topology.
> 
> Make the DSA core program the CPU and DSA ports as part of the VLAN.
> 
> This makes all chips of the data path to be aware of VIDs spanning the
> the whole fabric and thus, seamlessly add support for cross-chip VLAN.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

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

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

* Re: [PATCH net-next 2/5] net: dsa: check VLAN capability of every switch
  2017-06-06 20:56 ` [PATCH net-next 2/5] net: dsa: check VLAN capability of every switch Vivien Didelot
@ 2017-06-07 19:39   ` Florian Fainelli
  2017-06-07 20:03     ` Vivien Didelot
  0 siblings, 1 reply; 14+ messages in thread
From: Florian Fainelli @ 2017-06-07 19:39 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn

On 06/06/2017 01:56 PM, Vivien Didelot wrote:
> Now that the VLAN object is propagated to every switch chip of the
> switch fabric, we can easily ensure that they all support the required
> VLAN operations before modifying an entry on a single switch.
> 
> To achieve that, remove the condition skipping other target switches,
> and add a bitmap of VLAN members, eventually containing the target port,
> if we are programming the switch target.

You could add in the commit message that with this commit, there is not
actually a functional change yet because we have one (and only one) bit
set in the members bitmap.

> 
> This will allow us to easily add other VLAN members, such as the DSA or
> CPU ports (to introduce cross-chip VLAN support) or the other port
> members if we want to reduce hardware accesses later.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

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

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

* Re: [PATCH net-next 5/5] net: dsa: mv88e6xxx: do not purge a VTU entry
  2017-06-07 19:37   ` Florian Fainelli
@ 2017-06-07 19:59     ` Vivien Didelot
  0 siblings, 0 replies; 14+ messages in thread
From: Vivien Didelot @ 2017-06-07 19:59 UTC (permalink / raw)
  To: Florian Fainelli, netdev
  Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn

Hi Florian,

Florian Fainelli <f.fainelli@gmail.com> writes:

> On 06/06/2017 01:56 PM, Vivien Didelot wrote:
>> The mv88e6xxx driver currently tries to be smart and remove by itself a
>> VLAN entry from the VTU when the driven switch sees no user ports as
>> members of the VLAN.
>> 
>> This is bad in a multi-chip switch fabric, since a chip in between
>> others may have no bridge port members, but still needs to be aware of
>> the VID in order to correctly pass frames in the data path.
>> 
>> Remove the code purging a VTU entry when updating a port membership.
>
> In that case the switch sitting between two other chips and passing
> traffic would still have at least two of its DSA ports be part of a VTU
> entry, right?

That is correct.

>
> So could not we just do ....
>
>> 
>> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>> ---
>>  drivers/net/dsa/mv88e6xxx/chip.c | 15 +--------------
>>  1 file changed, 1 insertion(+), 14 deletions(-)
>> 
>> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
>> index 522f023bb17e..64c0f88f9e79 100644
>> --- a/drivers/net/dsa/mv88e6xxx/chip.c
>> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
>> @@ -1325,9 +1325,8 @@ static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
>>  static int _mv88e6xxx_port_vlan_del(struct mv88e6xxx_chip *chip,
>>  				    int port, u16 vid)
>>  {
>> -	struct dsa_switch *ds = chip->ds;
>>  	struct mv88e6xxx_vtu_entry vlan;
>> -	int i, err;
>> +	int err;
>>  
>>  	err = mv88e6xxx_vtu_get(chip, vid, &vlan, false);
>>  	if (err)
>> @@ -1339,18 +1338,6 @@ static int _mv88e6xxx_port_vlan_del(struct mv88e6xxx_chip *chip,
>>  
>>  	vlan.member[port] = GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER;
>>  
>> -	/* keep the VLAN unless all ports are excluded */
>> -	vlan.valid = false;
>> -	for (i = 0; i < mv88e6xxx_num_ports(chip); ++i) {
>> -		if (dsa_is_cpu_port(ds, i) || dsa_is_dsa_port(ds, i))
>> -			continue;
>
> ... break the loop here?

I can remove only the dsa_is_{cpu,dsa}_port condition above, this will
make the code ready for when the DSA core will remove VLAN on DSA ports.

Thanks!

        Vivien

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

* Re: [PATCH net-next 2/5] net: dsa: check VLAN capability of every switch
  2017-06-07 19:39   ` Florian Fainelli
@ 2017-06-07 20:03     ` Vivien Didelot
  0 siblings, 0 replies; 14+ messages in thread
From: Vivien Didelot @ 2017-06-07 20:03 UTC (permalink / raw)
  To: Florian Fainelli, netdev
  Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn

Hi Florian,

Florian Fainelli <f.fainelli@gmail.com> writes:

> On 06/06/2017 01:56 PM, Vivien Didelot wrote:
>> Now that the VLAN object is propagated to every switch chip of the
>> switch fabric, we can easily ensure that they all support the required
>> VLAN operations before modifying an entry on a single switch.
>> 
>> To achieve that, remove the condition skipping other target switches,
>> and add a bitmap of VLAN members, eventually containing the target port,
>> if we are programming the switch target.
>
> You could add in the commit message that with this commit, there is not
> actually a functional change yet because we have one (and only one) bit
> set in the members bitmap.

Hum there is a small functional change though: if one interconnected
switch of the fabric does not support the VLAN operations, -EOPNOTSUPP
is returned (even though the target switch is VLAN capable.)

Thanks,

        Vivien

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

end of thread, other threads:[~2017-06-07 20:05 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-06 20:56 [PATCH net-next 0/5] net: dsa: add cross-chip VLAN support Vivien Didelot
2017-06-06 20:56 ` [PATCH net-next 1/5] net: dsa: mv88e6xxx: define membership on VLAN add Vivien Didelot
2017-06-07 19:31   ` Florian Fainelli
2017-06-06 20:56 ` [PATCH net-next 2/5] net: dsa: check VLAN capability of every switch Vivien Didelot
2017-06-07 19:39   ` Florian Fainelli
2017-06-07 20:03     ` Vivien Didelot
2017-06-06 20:56 ` [PATCH net-next 3/5] net: dsa: add CPU and DSA ports as VLAN members Vivien Didelot
2017-06-07 19:00   ` David Miller
2017-06-07 19:38   ` Florian Fainelli
2017-06-06 20:56 ` [PATCH net-next 4/5] net: dsa: mv88e6xxx: exclude all ports in new VLAN Vivien Didelot
2017-06-07 19:34   ` Florian Fainelli
2017-06-06 20:56 ` [PATCH net-next 5/5] net: dsa: mv88e6xxx: do not purge a VTU entry Vivien Didelot
2017-06-07 19:37   ` Florian Fainelli
2017-06-07 19:59     ` Vivien Didelot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).