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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
From: Vivien Didelot <[email protected]>
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.
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 <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
--
Florian
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 <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
--
Florian
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 <[email protected]>
> ---
> 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
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 <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
--
Florian
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 <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
--
Florian
Hi Florian,
Florian Fainelli <[email protected]> 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 <[email protected]>
>> ---
>> 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
Hi Florian,
Florian Fainelli <[email protected]> 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