Re: [PATCH net-next] net: dsa: loop: Print when registration is successful
On Tue, 7 Jul 2020 21:45:13 -0700, Florian Fainelli wrote: > We have a number of error conditions that can lead to the driver not > probing successfully, move the print when we are sure > dsa_register_switch() has suceeded. This avoids repeated prints in case > of probe deferral for instance. > > Signed-off-by: Florian Fainelli Reviewed-by: Vivien Didelot
Re: vim: linux-kernel: Add "fallthrough" as a keyword
Hi Joe, On Sat, 14 Mar 2020 12:08:40 -0700, Joe Perches wrote: > Add fallthrough as a keyword for source code highlighting as > fallthrough was added as a pseudo-keyword macro to replace the > various forms of switch/case /* fallthrough */ comments. > > Signed-off-by: Joe Perches > --- > plugin/linuxsty.vim | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/plugin/linuxsty.vim b/plugin/linuxsty.vim > index 18b28..44824 100644 > --- a/plugin/linuxsty.vim > +++ b/plugin/linuxsty.vim > @@ -69,6 +69,7 @@ function s:LinuxFormatting() > endfunction > > function s:LinuxKeywords() > +syn keyword cStatement fallthrough > syn keyword cOperator likely unlikely > syn keyword cType u8 u16 u32 u64 s8 s16 s32 s64 > syn keyword cType __u8 __u16 __u32 __u64 __s8 __s16 __s32 __s64 > I've applied the patch, sorry for the delay. Thanks, Vivien
Re: [RFC net] net: dsa: Add missing reference counting
On Tue, 5 May 2020 14:02:53 -0700, Florian Fainelli
wrote:
> If we are probed through platform_data we would be intentionally
> dropping the reference count on master after dev_to_net_device()
> incremented it. If we are probed through Device Tree,
> of_find_net_device() does not do a dev_hold() at all.
>
> Ensure that the DSA master device is properly reference counted by
> holding it as soon as the CPU port is successfully initialized and later
> released during dsa_switch_release_ports(). dsa_get_tag_protocol() does
> a short de-reference, so we hold and release the master at that time,
> too.
>
> Fixes: 83c0afaec7b7 ("net: dsa: Add new binding implementation")
> Signed-off-by: Florian Fainelli
Reviewed-by: Vivien Didelot
[PATCH net-next v2 02/16] net: dsa: add ports list in the switch fabric
Add a list of switch ports within the switch fabric. This will help the
lookup of a port inside the whole fabric, and it is the first step
towards supporting multiple CPU ports, before deprecating the usage of
the unique dst->cpu_dp pointer.
In preparation for a future allocation of the dsa_port structures,
return -ENOMEM in case no structure is returned, even though this
error cannot be reached yet.
Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
Reviewed-by: Andrew Lunn
---
include/net/dsa.h | 5 +
net/dsa/dsa2.c| 48 +--
2 files changed, 47 insertions(+), 6 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 2e4fe2f8962b..6ff6dfcdc61d 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -125,6 +125,9 @@ struct dsa_switch_tree {
*/
struct dsa_port *cpu_dp;
+ /* List of switch ports */
+ struct list_head ports;
+
/*
* Data for the individual switch chips.
*/
@@ -195,6 +198,8 @@ struct dsa_port {
struct work_struct xmit_work;
struct sk_buff_head xmit_queue;
+ struct list_head list;
+
/*
* Give the switch driver somewhere to hang its per-port private data
* structures (accessible from the tagger).
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 1716535167ee..ba27ff8b4445 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -45,6 +45,8 @@ static struct dsa_switch_tree *dsa_tree_alloc(int index)
dst->index = index;
+ INIT_LIST_HEAD(&dst->ports);
+
INIT_LIST_HEAD(&dst->list);
list_add_tail(&dst->list, &dsa_tree_list);
@@ -616,6 +618,22 @@ static int dsa_tree_add_switch(struct dsa_switch_tree *dst,
return err;
}
+static struct dsa_port *dsa_port_touch(struct dsa_switch *ds, int index)
+{
+ struct dsa_switch_tree *dst = ds->dst;
+ struct dsa_port *dp;
+
+ dp = &ds->ports[index];
+
+ dp->ds = ds;
+ dp->index = index;
+
+ INIT_LIST_HEAD(&dp->list);
+ list_add_tail(&dp->list, &dst->ports);
+
+ return dp;
+}
+
static int dsa_port_parse_user(struct dsa_port *dp, const char *name)
{
if (!name)
@@ -742,6 +760,20 @@ static int dsa_switch_parse_member_of(struct dsa_switch
*ds,
return 0;
}
+static int dsa_switch_touch_ports(struct dsa_switch *ds)
+{
+ struct dsa_port *dp;
+ int port;
+
+ for (port = 0; port < ds->num_ports; port++) {
+ dp = dsa_port_touch(ds, port);
+ if (!dp)
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
static int dsa_switch_parse_of(struct dsa_switch *ds, struct device_node *dn)
{
int err;
@@ -750,6 +782,10 @@ static int dsa_switch_parse_of(struct dsa_switch *ds,
struct device_node *dn)
if (err)
return err;
+ err = dsa_switch_touch_ports(ds);
+ if (err)
+ return err;
+
return dsa_switch_parse_ports_of(ds, dn);
}
@@ -807,6 +843,8 @@ static int dsa_switch_parse_ports(struct dsa_switch *ds,
static int dsa_switch_parse(struct dsa_switch *ds, struct dsa_chip_data *cd)
{
+ int err;
+
ds->cd = cd;
/* We don't support interconnected switches nor multiple trees via
@@ -817,6 +855,10 @@ static int dsa_switch_parse(struct dsa_switch *ds, struct
dsa_chip_data *cd)
if (!ds->dst)
return -ENOMEM;
+ err = dsa_switch_touch_ports(ds);
+ if (err)
+ return err;
+
return dsa_switch_parse_ports(ds, cd);
}
@@ -849,7 +891,6 @@ static int dsa_switch_probe(struct dsa_switch *ds)
struct dsa_switch *dsa_switch_alloc(struct device *dev, size_t n)
{
struct dsa_switch *ds;
- int i;
ds = devm_kzalloc(dev, struct_size(ds, ports, n), GFP_KERNEL);
if (!ds)
@@ -858,11 +899,6 @@ struct dsa_switch *dsa_switch_alloc(struct device *dev,
size_t n)
ds->dev = dev;
ds->num_ports = n;
- for (i = 0; i < ds->num_ports; ++i) {
- ds->ports[i].index = i;
- ds->ports[i].ds = ds;
- }
-
return ds;
}
EXPORT_SYMBOL_GPL(dsa_switch_alloc);
--
2.23.0
[PATCH net-next v2 08/16] net: dsa: use ports list to setup multiple master devices
Now that we have a potential list of CPU ports, make use of it instead
of only configuring the master device of an unique CPU port.
Signed-off-by: Vivien Didelot
---
net/dsa/dsa2.c | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index a4de7ff8b19b..514c0195e2e8 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -493,19 +493,27 @@ static void dsa_tree_teardown_switches(struct
dsa_switch_tree *dst)
static int dsa_tree_setup_master(struct dsa_switch_tree *dst)
{
- struct dsa_port *cpu_dp = dst->cpu_dp;
- struct net_device *master = cpu_dp->master;
+ struct dsa_port *dp;
+ int err;
- /* DSA currently supports a single pair of CPU port and master device */
- return dsa_master_setup(master, cpu_dp);
+ list_for_each_entry(dp, &dst->ports, list) {
+ if (dsa_port_is_cpu(dp)) {
+ err = dsa_master_setup(dp->master, dp);
+ if (err)
+ return err;
+ }
+ }
+
+ return 0;
}
static void dsa_tree_teardown_master(struct dsa_switch_tree *dst)
{
- struct dsa_port *cpu_dp = dst->cpu_dp;
- struct net_device *master = cpu_dp->master;
+ struct dsa_port *dp;
- return dsa_master_teardown(master);
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dsa_port_is_cpu(dp))
+ dsa_master_teardown(dp->master);
}
static int dsa_tree_setup(struct dsa_switch_tree *dst)
--
2.23.0
[PATCH net-next v2 11/16] net: dsa: mv88e6xxx: silently skip PVT ops
Since mv88e6xxx_pvt_map is a static helper, no need to return
-EOPNOTSUPP if the chip has no PVT, simply silently skip the operation.
Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
---
drivers/net/dsa/mv88e6xxx/chip.c | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index d67deec77452..510ccdc2d03c 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1253,7 +1253,7 @@ static int mv88e6xxx_pvt_map(struct mv88e6xxx_chip *chip,
int dev, int port)
u16 pvlan = 0;
if (!mv88e6xxx_has_pvt(chip))
- return -EOPNOTSUPP;
+ return 0;
/* Skip the local source device, which uses in-chip port VLAN */
if (dev != chip->ds->index)
@@ -2049,9 +2049,6 @@ static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip
*chip,
}
}
- if (!mv88e6xxx_has_pvt(chip))
- return 0;
-
/* Remap the Port VLAN of each cross-chip bridge group member */
for (dev = 0; dev < DSA_MAX_SWITCHES; ++dev) {
ds = chip->ds->dst->ds[dev];
@@ -2101,9 +2098,6 @@ static int mv88e6xxx_crosschip_bridge_join(struct
dsa_switch *ds, int dev,
struct mv88e6xxx_chip *chip = ds->priv;
int err;
- if (!mv88e6xxx_has_pvt(chip))
- return 0;
-
mv88e6xxx_reg_lock(chip);
err = mv88e6xxx_pvt_map(chip, dev, port);
mv88e6xxx_reg_unlock(chip);
@@ -2116,9 +2110,6 @@ static void mv88e6xxx_crosschip_bridge_leave(struct
dsa_switch *ds, int dev,
{
struct mv88e6xxx_chip *chip = ds->priv;
- if (!mv88e6xxx_has_pvt(chip))
- return;
-
mv88e6xxx_reg_lock(chip);
if (mv88e6xxx_pvt_map(chip, dev, port))
dev_err(ds->dev, "failed to remap cross-chip Port VLAN\n");
--
2.23.0
[PATCH net-next v2 06/16] net: dsa: use ports list for routing table setup
Use the new ports list instead of accessing the dsa_switch array
of ports when iterating over DSA ports of a switch to set up the
routing table.
Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
Reviewed-by: Andrew Lunn
---
net/dsa/dsa2.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 01b6047d9b7b..623805ba8e1a 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -157,6 +157,7 @@ static bool dsa_port_setup_routing_table(struct dsa_port
*dp)
static bool dsa_switch_setup_routing_table(struct dsa_switch *ds)
{
+ struct dsa_switch_tree *dst = ds->dst;
bool complete = true;
struct dsa_port *dp;
int i;
@@ -164,10 +165,8 @@ static bool dsa_switch_setup_routing_table(struct
dsa_switch *ds)
for (i = 0; i < DSA_MAX_SWITCHES; i++)
ds->rtable[i] = DSA_RTABLE_NONE;
- for (i = 0; i < ds->num_ports; i++) {
- dp = &ds->ports[i];
-
- if (dsa_port_is_dsa(dp)) {
+ list_for_each_entry(dp, &dst->ports, list) {
+ if (dp->ds == ds && dsa_port_is_dsa(dp)) {
complete = dsa_port_setup_routing_table(dp);
if (!complete)
break;
--
2.23.0
[PATCH net-next v2 07/16] net: dsa: use ports list to find a port by node
Use the new ports list instead of iterating over switches and their
ports to find a port from a given node.
Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
---
net/dsa/dsa2.c | 17 +++--
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 623805ba8e1a..a4de7ff8b19b 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -113,22 +113,11 @@ static bool dsa_port_is_user(struct dsa_port *dp)
static struct dsa_port *dsa_tree_find_port_by_node(struct dsa_switch_tree *dst,
struct device_node *dn)
{
- struct dsa_switch *ds;
struct dsa_port *dp;
- int device, port;
-
- for (device = 0; device < DSA_MAX_SWITCHES; device++) {
- ds = dst->ds[device];
- if (!ds)
- continue;
- for (port = 0; port < ds->num_ports; port++) {
- dp = &ds->ports[port];
-
- if (dp->dn == dn)
- return dp;
- }
- }
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dp->dn == dn)
+ return dp;
return NULL;
}
--
2.23.0
[PATCH net-next v2 05/16] net: dsa: use ports list to setup switches
Use the new ports list instead of iterating over switches and their
ports when setting up the switches and their ports.
At the same time, provide setup states and messages for ports and
switches as it is done for the trees.
Signed-off-by: Vivien Didelot
---
include/net/dsa.h | 4 ++
net/dsa/dsa2.c| 93 +--
2 files changed, 45 insertions(+), 52 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index d2b7ee28f3fd..bd08bdee8341 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -215,9 +215,13 @@ struct dsa_port {
* Original copy of the master netdev net_device_ops
*/
const struct net_device_ops *orig_ndo_ops;
+
+ bool setup;
};
struct dsa_switch {
+ bool setup;
+
struct device *dev;
/*
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index ba27ff8b4445..01b6047d9b7b 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -267,6 +267,9 @@ static int dsa_port_setup(struct dsa_port *dp)
bool dsa_port_enabled = false;
int err = 0;
+ if (dp->setup)
+ return 0;
+
switch (dp->type) {
case DSA_PORT_TYPE_UNUSED:
dsa_port_disable(dp);
@@ -335,14 +338,21 @@ static int dsa_port_setup(struct dsa_port *dp)
dsa_port_link_unregister_of(dp);
if (err && devlink_port_registered)
devlink_port_unregister(dlp);
+ if (err)
+ return err;
- return err;
+ dp->setup = true;
+
+ return 0;
}
static void dsa_port_teardown(struct dsa_port *dp)
{
struct devlink_port *dlp = &dp->devlink_port;
+ if (!dp->setup)
+ return;
+
switch (dp->type) {
case DSA_PORT_TYPE_UNUSED:
break;
@@ -365,11 +375,16 @@ static void dsa_port_teardown(struct dsa_port *dp)
}
break;
}
+
+ dp->setup = false;
}
static int dsa_switch_setup(struct dsa_switch *ds)
{
- int err = 0;
+ int err;
+
+ if (ds->setup)
+ return 0;
/* Initialize ds->phys_mii_mask before registering the slave MDIO bus
* driver and before ops->setup() has run, since the switch drivers and
@@ -411,6 +426,8 @@ static int dsa_switch_setup(struct dsa_switch *ds)
goto unregister_notifier;
}
+ ds->setup = true;
+
return 0;
unregister_notifier:
@@ -426,6 +443,9 @@ static int dsa_switch_setup(struct dsa_switch *ds)
static void dsa_switch_teardown(struct dsa_switch *ds)
{
+ if (!ds->setup)
+ return;
+
if (ds->slave_mii_bus && ds->ops->phy_read)
mdiobus_unregister(ds->slave_mii_bus);
@@ -440,78 +460,47 @@ static void dsa_switch_teardown(struct dsa_switch *ds)
ds->devlink = NULL;
}
+ ds->setup = false;
}
static int dsa_tree_setup_switches(struct dsa_switch_tree *dst)
{
- struct dsa_switch *ds;
struct dsa_port *dp;
- int device, port, i;
- int err = 0;
-
- for (device = 0; device < DSA_MAX_SWITCHES; device++) {
- ds = dst->ds[device];
- if (!ds)
- continue;
+ int err;
- err = dsa_switch_setup(ds);
+ list_for_each_entry(dp, &dst->ports, list) {
+ err = dsa_switch_setup(dp->ds);
if (err)
- goto switch_teardown;
-
- for (port = 0; port < ds->num_ports; port++) {
- dp = &ds->ports[port];
+ goto teardown;
+ }
- err = dsa_port_setup(dp);
- if (err)
- goto ports_teardown;
- }
+ list_for_each_entry(dp, &dst->ports, list) {
+ err = dsa_port_setup(dp);
+ if (err)
+ goto teardown;
}
return 0;
-ports_teardown:
- for (i = 0; i < port; i++)
- dsa_port_teardown(&ds->ports[i]);
-
- dsa_switch_teardown(ds);
-
-switch_teardown:
- for (i = 0; i < device; i++) {
- ds = dst->ds[i];
- if (!ds)
- continue;
-
- for (port = 0; port < ds->num_ports; port++) {
- dp = &ds->ports[port];
-
- dsa_port_teardown(dp);
- }
+teardown:
+ list_for_each_entry(dp, &dst->ports, list)
+ dsa_port_teardown(dp);
- dsa_switch_teardown(ds);
- }
+ list_for_each_entry(dp, &dst->ports, list)
+ dsa_switch_teardown(dp->ds);
return err;
}
static void dsa_tree_teardown_switches(struct dsa_switch_tree *dst)
{
- struct dsa_s
[PATCH net-next v2 13/16] net: dsa: mv88e6xxx: use ports list to map bridge
Instead of digging into the other dsa_switch structures of the fabric
and relying too much on the dsa_to_port helper, use the new list
of switch fabric ports to remap the Port VLAN Map of local bridge
group members or remap the Port VLAN Table entry of external bridge
group members.
Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
---
drivers/net/dsa/mv88e6xxx/chip.c | 39 +++-
1 file changed, 18 insertions(+), 21 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index af8943142053..826ae82ed727 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2043,29 +2043,26 @@ static int mv88e6xxx_port_fdb_dump(struct dsa_switch
*ds, int port,
static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip *chip,
struct net_device *br)
{
- struct dsa_switch *ds;
- int port;
- int dev;
+ struct dsa_switch *ds = chip->ds;
+ struct dsa_switch_tree *dst = ds->dst;
+ struct dsa_port *dp;
int err;
- /* Remap the Port VLAN of each local bridge group member */
- for (port = 0; port < mv88e6xxx_num_ports(chip); ++port) {
- if (dsa_to_port(chip->ds, port)->bridge_dev == br) {
- err = mv88e6xxx_port_vlan_map(chip, port);
- if (err)
- return err;
- }
- }
-
- /* Remap the Port VLAN of each cross-chip bridge group member */
- for (dev = 0; dev < DSA_MAX_SWITCHES; ++dev) {
- ds = chip->ds->dst->ds[dev];
- if (!ds)
- break;
-
- for (port = 0; port < ds->num_ports; ++port) {
- if (dsa_to_port(ds, port)->bridge_dev == br) {
- err = mv88e6xxx_pvt_map(chip, dev, port);
+ list_for_each_entry(dp, &dst->ports, list) {
+ if (dp->bridge_dev == br) {
+ if (dp->ds == ds) {
+ /* This is a local bridge group member,
+* remap its Port VLAN Map.
+*/
+ err = mv88e6xxx_port_vlan_map(chip, dp->index);
+ if (err)
+ return err;
+ } else {
+ /* This is an external bridge group member,
+* remap its cross-chip Port VLAN Table entry.
+*/
+ err = mv88e6xxx_pvt_map(chip, dp->ds->index,
+ dp->index);
if (err)
return err;
}
--
2.23.0
[PATCH net-next v2 10/16] net: dsa: use ports list to setup default CPU port
Use the new ports list instead of iterating over switches and their
ports when setting up the default CPU port. Unassign it on teardown.
Now that we can iterate over multiple CPU ports, remove dst->cpu_dp.
At the same time, provide a better error message for CPU-less tree.
Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
---
include/net/dsa.h | 5 -
net/dsa/dsa2.c| 33 -
2 files changed, 12 insertions(+), 26 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index bd08bdee8341..f572134eb5de 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -120,11 +120,6 @@ struct dsa_switch_tree {
*/
struct dsa_platform_data*pd;
- /*
-* The switch port to which the CPU is attached.
-*/
- struct dsa_port *cpu_dp;
-
/* List of switch ports */
struct list_head ports;
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 80191c7702a9..bf8b4e0fcb4f 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -197,38 +197,29 @@ static struct dsa_port *dsa_tree_find_first_cpu(struct
dsa_switch_tree *dst)
static int dsa_tree_setup_default_cpu(struct dsa_switch_tree *dst)
{
- struct dsa_switch *ds;
- struct dsa_port *dp;
- int device, port;
+ struct dsa_port *cpu_dp, *dp;
- /* DSA currently only supports a single CPU port */
- dst->cpu_dp = dsa_tree_find_first_cpu(dst);
- if (!dst->cpu_dp) {
- pr_warn("Tree has no master device\n");
+ cpu_dp = dsa_tree_find_first_cpu(dst);
+ if (!cpu_dp) {
+ pr_err("DSA: tree %d has no CPU port\n", dst->index);
return -EINVAL;
}
/* Assign the default CPU port to all ports of the fabric */
- for (device = 0; device < DSA_MAX_SWITCHES; device++) {
- ds = dst->ds[device];
- if (!ds)
- continue;
-
- 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;
- }
- }
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dsa_port_is_user(dp) || dsa_port_is_dsa(dp))
+ dp->cpu_dp = cpu_dp;
return 0;
}
static void dsa_tree_teardown_default_cpu(struct dsa_switch_tree *dst)
{
- /* DSA currently only supports a single CPU port */
- dst->cpu_dp = NULL;
+ struct dsa_port *dp;
+
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dsa_port_is_user(dp) || dsa_port_is_dsa(dp))
+ dp->cpu_dp = NULL;
}
static int dsa_port_setup(struct dsa_port *dp)
--
2.23.0
[PATCH net-next v2 14/16] net: dsa: sja1105: register switch before assigning port private data
Like the dsa_switch_tree structures, the dsa_port structures will be
allocated on switch registration.
The SJA1105 driver is the only one accessing the dsa_port structure
after the switch allocation and before the switch registration.
For that reason, move switch registration prior to assigning the priv
member of the dsa_port structures.
Signed-off-by: Vivien Didelot
---
drivers/net/dsa/sja1105/sja1105_main.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c
b/drivers/net/dsa/sja1105/sja1105_main.c
index 4b0cb779f187..0ebbda5ca665 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2057,6 +2057,15 @@ static int sja1105_probe(struct spi_device *spi)
tagger_data = &priv->tagger_data;
+ mutex_init(&priv->ptp_data.lock);
+ mutex_init(&priv->mgmt_lock);
+
+ sja1105_tas_setup(ds);
+
+ rc = dsa_register_switch(priv->ds);
+ if (rc)
+ return rc;
+
/* Connections between dsa_port and sja1105_port */
for (i = 0; i < SJA1105_NUM_PORTS; i++) {
struct sja1105_port *sp = &priv->ports[i];
@@ -2065,12 +2074,8 @@ static int sja1105_probe(struct spi_device *spi)
sp->dp = dsa_to_port(ds, i);
sp->data = tagger_data;
}
- mutex_init(&priv->ptp_data.lock);
- mutex_init(&priv->mgmt_lock);
- sja1105_tas_setup(ds);
-
- return dsa_register_switch(priv->ds);
+ return 0;
}
static int sja1105_remove(struct spi_device *spi)
--
2.23.0
[PATCH net-next v2 09/16] net: dsa: use ports list to find first CPU port
Use the new ports list instead of iterating over switches and their
ports when looking up the first CPU port in the tree.
Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
---
net/dsa/dsa2.c | 17 +++--
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 514c0195e2e8..80191c7702a9 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -186,22 +186,11 @@ static bool dsa_tree_setup_routing_table(struct
dsa_switch_tree *dst)
static struct dsa_port *dsa_tree_find_first_cpu(struct dsa_switch_tree *dst)
{
- struct dsa_switch *ds;
struct dsa_port *dp;
- int device, port;
-
- for (device = 0; device < DSA_MAX_SWITCHES; device++) {
- ds = dst->ds[device];
- if (!ds)
- continue;
- for (port = 0; port < ds->num_ports; port++) {
- dp = &ds->ports[port];
-
- if (dsa_port_is_cpu(dp))
- return dp;
- }
- }
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dsa_port_is_cpu(dp))
+ return dp;
return NULL;
}
--
2.23.0
[PATCH net-next v2 16/16] net: dsa: remove dsa_switch_alloc helper
Now that ports are dynamically listed in the fabric, there is no need
to provide a special helper to allocate the dsa_switch structure. This
will give more flexibility to drivers to embed this structure as they
wish in their private structure.
Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
---
drivers/net/dsa/b53/b53_common.c | 5 -
drivers/net/dsa/dsa_loop.c | 5 -
drivers/net/dsa/lan9303-core.c | 4 +++-
drivers/net/dsa/lantiq_gswip.c | 4 +++-
drivers/net/dsa/microchip/ksz_common.c | 5 -
drivers/net/dsa/mt7530.c | 5 -
drivers/net/dsa/mv88e6060.c| 4 +++-
drivers/net/dsa/mv88e6xxx/chip.c | 4 +++-
drivers/net/dsa/qca8k.c| 5 -
drivers/net/dsa/realtek-smi-core.c | 5 -
drivers/net/dsa/sja1105/sja1105_main.c | 4 +++-
drivers/net/dsa/vitesse-vsc73xx-core.c | 5 -
include/net/dsa.h | 1 -
net/dsa/dsa2.c | 21 ++---
14 files changed, 49 insertions(+), 28 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index aef9b56781ef..baadf622ac55 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -2341,10 +2341,13 @@ struct b53_device *b53_switch_alloc(struct device *base,
struct dsa_switch *ds;
struct b53_device *dev;
- ds = dsa_switch_alloc(base, DSA_MAX_PORTS);
+ ds = devm_kzalloc(base, sizeof(*ds), GFP_KERNEL);
if (!ds)
return NULL;
+ ds->dev = base;
+ ds->num_ports = DSA_MAX_PORTS;
+
dev = devm_kzalloc(base, sizeof(*dev), GFP_KERNEL);
if (!dev)
return NULL;
diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
index 925ed135a4d9..c8d7ef27fd72 100644
--- a/drivers/net/dsa/dsa_loop.c
+++ b/drivers/net/dsa/dsa_loop.c
@@ -286,10 +286,13 @@ static int dsa_loop_drv_probe(struct mdio_device *mdiodev)
dev_info(&mdiodev->dev, "%s: 0x%0x\n",
pdata->name, pdata->enabled_ports);
- ds = dsa_switch_alloc(&mdiodev->dev, DSA_MAX_PORTS);
+ ds = devm_kzalloc(&mdiodev->dev, sizeof(*ds), GFP_KERNEL);
if (!ds)
return -ENOMEM;
+ ds->dev = &mdiodev->dev;
+ ds->num_ports = DSA_MAX_PORTS;
+
ps = devm_kzalloc(&mdiodev->dev, sizeof(*ps), GFP_KERNEL);
if (!ps)
return -ENOMEM;
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index bbec86b9418e..e3c333a8f45d 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -1283,10 +1283,12 @@ static int lan9303_register_switch(struct lan9303 *chip)
{
int base;
- chip->ds = dsa_switch_alloc(chip->dev, LAN9303_NUM_PORTS);
+ chip->ds = devm_kzalloc(chip->dev, sizeof(*chip->ds), GFP_KERNEL);
if (!chip->ds)
return -ENOMEM;
+ chip->ds->dev = chip->dev;
+ chip->ds->num_ports = LAN9303_NUM_PORTS;
chip->ds->priv = chip;
chip->ds->ops = &lan9303_switch_ops;
base = chip->phy_addr_base;
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index a69c9b9878b7..955324968b74 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1854,10 +1854,12 @@ static int gswip_probe(struct platform_device *pdev)
if (!priv->hw_info)
return -EINVAL;
- priv->ds = dsa_switch_alloc(dev, priv->hw_info->max_ports);
+ priv->ds = devm_kzalloc(dev, sizeof(*priv->ds), GFP_KERNEL);
if (!priv->ds)
return -ENOMEM;
+ priv->ds->dev = dev;
+ priv->ds->num_ports = priv->hw_info->max_ports;
priv->ds->priv = priv;
priv->ds->ops = &gswip_switch_ops;
priv->dev = dev;
diff --git a/drivers/net/dsa/microchip/ksz_common.c
b/drivers/net/dsa/microchip/ksz_common.c
index fe47180c908b..5d08e4430824 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -398,10 +398,13 @@ struct ksz_device *ksz_switch_alloc(struct device *base,
void *priv)
struct dsa_switch *ds;
struct ksz_device *swdev;
- ds = dsa_switch_alloc(base, DSA_MAX_PORTS);
+ ds = devm_kzalloc(base, sizeof(*ds), GFP_KERNEL);
if (!ds)
return NULL;
+ ds->dev = base;
+ ds->num_ports = DSA_MAX_PORTS;
+
swdev = devm_kzalloc(base, sizeof(*swdev), GFP_KERNEL);
if (!swdev)
return NULL;
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index a91293e47a57..add9e4279176 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1632,10 +1632,13 @@ mt7530_probe(struct mdi
[PATCH net-next v2 12/16] net: dsa: mv88e6xxx: use ports list to map port VLAN
Instead of digging into the other dsa_switch structures of the fabric
and relying too much on the dsa_to_port helper, use the new list of
switch fabric ports to define the mask of the local ports allowed to
receive frames from another port of the fabric.
Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
---
drivers/net/dsa/mv88e6xxx/chip.c | 32
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 510ccdc2d03c..af8943142053 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1057,35 +1057,43 @@ static int mv88e6xxx_set_mac_eee(struct dsa_switch *ds,
int port,
return 0;
}
+/* Mask of the local ports allowed to receive frames from a given fabric port
*/
static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port)
{
- struct dsa_switch *ds = NULL;
+ struct dsa_switch *ds = chip->ds;
+ struct dsa_switch_tree *dst = ds->dst;
struct net_device *br;
+ struct dsa_port *dp;
+ bool found = false;
u16 pvlan;
- int i;
- if (dev < DSA_MAX_SWITCHES)
- ds = chip->ds->dst->ds[dev];
+ list_for_each_entry(dp, &dst->ports, list) {
+ if (dp->ds->index == dev && dp->index == port) {
+ found = true;
+ break;
+ }
+ }
/* Prevent frames from unknown switch or port */
- if (!ds || port >= ds->num_ports)
+ if (!found)
return 0;
/* Frames from DSA links and CPU ports can egress any local port */
- if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port))
+ if (dp->type == DSA_PORT_TYPE_CPU || dp->type == DSA_PORT_TYPE_DSA)
return mv88e6xxx_port_mask(chip);
- br = dsa_to_port(ds, port)->bridge_dev;
+ br = dp->bridge_dev;
pvlan = 0;
/* Frames from user ports can egress any local DSA links and CPU ports,
* as well as any local member of their bridge group.
*/
- for (i = 0; i < mv88e6xxx_num_ports(chip); ++i)
- if (dsa_is_cpu_port(chip->ds, i) ||
- dsa_is_dsa_port(chip->ds, i) ||
- (br && dsa_to_port(chip->ds, i)->bridge_dev == br))
- pvlan |= BIT(i);
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dp->ds == ds &&
+ (dp->type == DSA_PORT_TYPE_CPU ||
+dp->type == DSA_PORT_TYPE_DSA ||
+(br && dp->bridge_dev == br)))
+ pvlan |= BIT(dp->index);
return pvlan;
}
--
2.23.0
[PATCH net-next v2 04/16] net: dsa: use ports list to find slave
Use the new ports list instead of iterating over switches and their
ports when looking for a slave device from a given master interface.
Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
Reviewed-by: Andrew Lunn
---
net/dsa/dsa_priv.h | 23 ++-
1 file changed, 6 insertions(+), 17 deletions(-)
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 12f8c7ee4dd8..53e7577896b6 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -104,25 +104,14 @@ static inline struct net_device
*dsa_master_find_slave(struct net_device *dev,
{
struct dsa_port *cpu_dp = dev->dsa_ptr;
struct dsa_switch_tree *dst = cpu_dp->dst;
- struct dsa_switch *ds;
- struct dsa_port *slave_port;
+ struct dsa_port *dp;
- if (device < 0 || device >= DSA_MAX_SWITCHES)
- return NULL;
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dp->ds->index == device && dp->index == port &&
+ dp->type == DSA_PORT_TYPE_USER)
+ return dp->slave;
- ds = dst->ds[device];
- if (!ds)
- return NULL;
-
- if (port < 0 || port >= ds->num_ports)
- return NULL;
-
- slave_port = &ds->ports[port];
-
- if (unlikely(slave_port->type != DSA_PORT_TYPE_USER))
- return NULL;
-
- return slave_port->slave;
+ return NULL;
}
/* port.c */
--
2.23.0
[PATCH net-next v2 15/16] net: dsa: allocate ports on touch
Allocate the struct dsa_port the first time it is accessed with
dsa_port_touch, and remove the static dsa_port array from the
dsa_switch structure.
Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
---
include/net/dsa.h | 2 --
net/dsa/dsa2.c| 16 ++--
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index f572134eb5de..9bc1d3f71f89 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -277,9 +277,7 @@ struct dsa_switch {
*/
boolvlan_filtering;
- /* Dynamically allocated ports, keep last */
size_t num_ports;
- struct dsa_port ports[];
};
static inline struct dsa_port *dsa_to_port(struct dsa_switch *ds, int p)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index bf8b4e0fcb4f..83cba4623698 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -588,7 +588,13 @@ static struct dsa_port *dsa_port_touch(struct dsa_switch
*ds, int index)
struct dsa_switch_tree *dst = ds->dst;
struct dsa_port *dp;
- dp = &ds->ports[index];
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dp->ds == ds && dp->index == index)
+ return dp;
+
+ dp = kzalloc(sizeof(*dp), GFP_KERNEL);
+ if (!dp)
+ return NULL;
dp->ds = ds;
dp->index = index;
@@ -857,7 +863,7 @@ struct dsa_switch *dsa_switch_alloc(struct device *dev,
size_t n)
{
struct dsa_switch *ds;
- ds = devm_kzalloc(dev, struct_size(ds, ports, n), GFP_KERNEL);
+ ds = devm_kzalloc(dev, sizeof(*ds), GFP_KERNEL);
if (!ds)
return NULL;
@@ -885,6 +891,12 @@ static void dsa_switch_remove(struct dsa_switch *ds)
{
struct dsa_switch_tree *dst = ds->dst;
unsigned int index = ds->index;
+ struct dsa_port *dp, *next;
+
+ list_for_each_entry_safe(dp, next, &dst->ports, list) {
+ list_del(&dp->list);
+ kfree(dp);
+ }
dsa_tree_remove_switch(dst, index);
}
--
2.23.0
[PATCH net-next v2 00/16] net: dsa: turn arrays of ports into a list
The dsa_switch structure represents the physical switch device itself, and is allocated by the driver. The dsa_switch_tree and dsa_port structures represent the logical switch fabric (eventually composed of multiple switch devices) and its ports, and are allocated by the DSA core. This branch lists the logical ports directly in the fabric which simplifies the iteration over all ports when assigning the default CPU port or configuring the D in DSA in drivers like mv88e6xxx. This also removes the unique dst->cpu_dp pointer and is a first step towards supporting multiple CPU ports and dropping the DSA_MAX_PORTS limitation. Because the dsa_port structures are not tight to the dsa_switch structure anymore, we do not need to provide an helper for the drivers to allocate a switch structure. Like in many other subsystems, drivers can now embed their dsa_switch structure as they wish into their private structure. This will be particularly interesting for the Broadcom drivers which were currently limited by the dynamically allocated array of DSA ports. The series implements the list of dsa_port structures, makes use of it, then drops dst->cpu_dp and the dsa_switch_alloc helper. Changes in v2: - use list_add_tail instead of list_add to respect ports order - use a single return statement for dsa_to_port - remove pr_info messages - put comments under appropriate branches - add Git tags from reviewers Vivien Didelot (16): net: dsa: use dsa_to_port helper everywhere net: dsa: add ports list in the switch fabric net: dsa: use ports list in dsa_to_port net: dsa: use ports list to find slave net: dsa: use ports list to setup switches net: dsa: use ports list for routing table setup net: dsa: use ports list to find a port by node net: dsa: use ports list to setup multiple master devices net: dsa: use ports list to find first CPU port net: dsa: use ports list to setup default CPU port net: dsa: mv88e6xxx: silently skip PVT ops net: dsa: mv88e6xxx: use ports list to map port VLAN net: dsa: mv88e6xxx: use ports list to map bridge net: dsa: sja1105: register switch before assigning port private data net: dsa: allocate ports on touch net: dsa: remove dsa_switch_alloc helper drivers/net/dsa/b53/b53_common.c | 11 +- drivers/net/dsa/bcm_sf2.c | 8 +- drivers/net/dsa/bcm_sf2_cfp.c | 6 +- drivers/net/dsa/dsa_loop.c | 5 +- drivers/net/dsa/lan9303-core.c | 4 +- drivers/net/dsa/lantiq_gswip.c | 4 +- drivers/net/dsa/microchip/ksz_common.c | 5 +- drivers/net/dsa/mt7530.c | 17 +- drivers/net/dsa/mv88e6060.c| 4 +- drivers/net/dsa/mv88e6xxx/chip.c | 90 drivers/net/dsa/qca8k.c| 7 +- drivers/net/dsa/realtek-smi-core.c | 5 +- drivers/net/dsa/sja1105/sja1105_main.c | 37 ++-- drivers/net/dsa/vitesse-vsc73xx-core.c | 5 +- include/net/dsa.h | 26 ++- net/dsa/dsa.c | 8 +- net/dsa/dsa2.c | 274 + net/dsa/dsa_priv.h | 23 +-- net/dsa/switch.c | 4 +- net/dsa/tag_8021q.c| 6 +- 20 files changed, 292 insertions(+), 257 deletions(-) -- 2.23.0
[PATCH net-next v2 03/16] net: dsa: use ports list in dsa_to_port
Use the new ports list instead of accessing the dsa_switch array
of ports in the dsa_to_port helper.
Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
Reviewed-by: Andrew Lunn
---
include/net/dsa.h | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 6ff6dfcdc61d..d2b7ee28f3fd 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -285,7 +285,14 @@ struct dsa_switch {
static inline struct dsa_port *dsa_to_port(struct dsa_switch *ds, int p)
{
- return &ds->ports[p];
+ struct dsa_switch_tree *dst = ds->dst;
+ struct dsa_port *dp = NULL;
+
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dp->ds == ds && dp->index == p)
+ break;
+
+ return dp;
}
static inline bool dsa_is_unused_port(struct dsa_switch *ds, int p)
--
2.23.0
[PATCH net-next v2 01/16] net: dsa: use dsa_to_port helper everywhere
Do not let the drivers access the ds->ports static array directly
while there is a dsa_to_port helper for this purpose.
At the same time, un-const this helper since the SJA1105 driver
assigns the priv member of the returned dsa_port structure.
Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
Reviewed-by: Andrew Lunn
---
drivers/net/dsa/b53/b53_common.c | 6 +++---
drivers/net/dsa/bcm_sf2.c | 8
drivers/net/dsa/bcm_sf2_cfp.c | 6 +++---
drivers/net/dsa/mt7530.c | 12 ++--
drivers/net/dsa/mv88e6xxx/chip.c | 10 +-
drivers/net/dsa/qca8k.c| 2 +-
drivers/net/dsa/sja1105/sja1105_main.c | 18 +-
include/net/dsa.h | 2 +-
net/dsa/dsa.c | 8 +---
net/dsa/dsa2.c | 4 ++--
net/dsa/switch.c | 4 ++--
net/dsa/tag_8021q.c| 6 +++---
12 files changed, 44 insertions(+), 42 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index cc3536315eff..aef9b56781ef 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -524,7 +524,7 @@ int b53_enable_port(struct dsa_switch *ds, int port, struct
phy_device *phy)
if (!dsa_is_user_port(ds, port))
return 0;
- cpu_port = ds->ports[port].cpu_dp->index;
+ cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
if (dev->ops->irq_enable)
ret = dev->ops->irq_enable(dev, port);
@@ -1629,7 +1629,7 @@ EXPORT_SYMBOL(b53_fdb_dump);
int b53_br_join(struct dsa_switch *ds, int port, struct net_device *br)
{
struct b53_device *dev = ds->priv;
- s8 cpu_port = ds->ports[port].cpu_dp->index;
+ s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
u16 pvlan, reg;
unsigned int i;
@@ -1675,7 +1675,7 @@ 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 = ds->ports[port].cpu_dp->index;
+ s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
unsigned int i;
u16 pvlan, reg, pvid;
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 26509fa37a50..c068a3b7207b 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -662,7 +662,7 @@ static void bcm_sf2_sw_fixed_state(struct dsa_switch *ds,
int port,
* state machine and make it go in PHY_FORCING state instead.
*/
if (!status->link)
- netif_carrier_off(ds->ports[port].slave);
+ netif_carrier_off(dsa_to_port(ds, port)->slave);
status->duplex = DUPLEX_FULL;
} else {
status->link = true;
@@ -728,7 +728,7 @@ static int bcm_sf2_sw_resume(struct dsa_switch *ds)
static void bcm_sf2_sw_get_wol(struct dsa_switch *ds, int port,
struct ethtool_wolinfo *wol)
{
- struct net_device *p = ds->ports[port].cpu_dp->master;
+ struct net_device *p = dsa_to_port(ds, port)->cpu_dp->master;
struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
struct ethtool_wolinfo pwol = { };
@@ -752,9 +752,9 @@ static void bcm_sf2_sw_get_wol(struct dsa_switch *ds, int
port,
static int bcm_sf2_sw_set_wol(struct dsa_switch *ds, int port,
struct ethtool_wolinfo *wol)
{
- struct net_device *p = ds->ports[port].cpu_dp->master;
+ struct net_device *p = dsa_to_port(ds, port)->cpu_dp->master;
struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
- s8 cpu_port = ds->ports[port].cpu_dp->index;
+ s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
struct ethtool_wolinfo pwol = { };
if (p->ethtool_ops->get_wol)
diff --git a/drivers/net/dsa/bcm_sf2_cfp.c b/drivers/net/dsa/bcm_sf2_cfp.c
index d264776a95a3..f3f0c3f07391 100644
--- a/drivers/net/dsa/bcm_sf2_cfp.c
+++ b/drivers/net/dsa/bcm_sf2_cfp.c
@@ -821,7 +821,7 @@ static int bcm_sf2_cfp_rule_insert(struct dsa_switch *ds,
int port,
struct ethtool_rx_flow_spec *fs)
{
struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
- s8 cpu_port = ds->ports[port].cpu_dp->index;
+ s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
__u64 ring_cookie = fs->ring_cookie;
unsigned int queue_num, port_num;
int ret;
@@ -1049,7 +1049,7 @@ static int bcm_sf2_cfp_rule_get_all(struct bcm_sf2_priv
*priv,
int bcm_sf2_get_rxnfc(struct dsa_switch *ds, int port,
struct ethtool_rxnfc *nfc, u32 *rule_locs)
{
- struct net_device *p = ds->ports[port].cpu_dp->master;
+
Re: [PATCH net-next 02/16] net: dsa: add ports list in the switch fabric
On Mon, 21 Oct 2019 14:37:40 +0200, Andrew Lunn wrote:
> > +static struct dsa_port *dsa_port_touch(struct dsa_switch *ds, int index)
> > +{
> > + struct dsa_switch_tree *dst = ds->dst;
> > + struct dsa_port *dp;
> > +
> > + dp = &ds->ports[index];
> > +
> > + dp->ds = ds;
> > + dp->index = index;
> > +
> > + INIT_LIST_HEAD(&dp->list);
> > + list_add(&dp->list, &dst->ports);
> > +
> > + return dp;
> > +}
>
> Bike shedding, but i don't particularly like the name touch. How
> about list. The opposite would then be delist, if we ever need it?
The fabric code uses "touch" for "get or create" already, so I used the same
semantics for ports as well. But I'm not strongly attached to this naming
anyway, so I will polish them all together in a future series.
Thanks,
Vivien
Re: [PATCH net-next 05/16] net: dsa: use ports list to setup switches
On Mon, 21 Oct 2019 14:49:02 +0200, Andrew Lunn wrote: > On Sun, Oct 20, 2019 at 07:42:15PM -0700, Florian Fainelli wrote: > > > > > > On 10/19/2019 8:19 PM, Vivien Didelot wrote: > > > Use the new ports list instead of iterating over switches and their > > > ports when setting up the switches and their ports. > > > > > > At the same time, provide setup states and messages for ports and > > > switches as it is done for the trees. > > > > Humm, that becomes quite noisy, would it make sense to have those > > messages only for non-user ports that are not already visible because > > they do not have a net_device? > > I agree, it looks noise. Maybe change them to _dbg()? > > > If you have multiple switches in a fabric, it might be convenient to use > > dev_info(dp->ds->dev, ...) to print your message so you can clearly > > identify which port belongs to which switch, which becomes even more > > important as it is all flattened thanks to lists now. What do you think? > > I do think it needs to identify both the dst and the ds. It is noise indeed and doesn't add much value, I'll remove them. Thanks, Vivien
Re: [PATCH net-next 01/16] net: dsa: use dsa_to_port helper everywhere
Hi Andrew, On Mon, 21 Oct 2019 14:31:49 +0200, Andrew Lunn wrote: > On Sat, Oct 19, 2019 at 11:19:26PM -0400, Vivien Didelot wrote: > > Do not let the drivers access the ds->ports static array directly > > while there is a dsa_to_port helper for this purpose. > > > > At the same time, un-const this helper since the SJA1105 driver > > assigns the priv member of the returned dsa_port structure. > > Hi Vivien > > Is priv the only member we expect drivers to change? Is the rest > private to the core/RO? Rather then remove the const, i wonder if it > would be better to add a helper to set priv? > > Otherwise: > > Reviewed-by: Andrew Lunn I had the same thought but actually since the SJA1105 driver is the only user, I was thinking about maybe getting rid of it, I don't really see the point of having some "dp->priv = priv, priv->dp = dp" kind of code... In the meantime I kept the eventual helper or priv removal for a future series. Thank you, Vivien
[PATCH net-next 10/16] net: dsa: use ports list to setup default CPU port
Use the new ports list instead of iterating over switches and their
ports when setting up the default CPU port. Unassign it on teardown.
Now that we can iterate over multiple CPU ports, remove dst->cpu_dp.
At the same time, provide a better error message for CPU-less tree.
Signed-off-by: Vivien Didelot
---
include/net/dsa.h | 5 -
net/dsa/dsa2.c| 33 -
2 files changed, 12 insertions(+), 26 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index b199a8ca6393..020f5db8666b 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -120,11 +120,6 @@ struct dsa_switch_tree {
*/
struct dsa_platform_data*pd;
- /*
-* The switch port to which the CPU is attached.
-*/
- struct dsa_port *cpu_dp;
-
/* List of switch ports */
struct list_head ports;
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 99f5dab06787..772deacc33d3 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -197,38 +197,29 @@ static struct dsa_port *dsa_tree_find_first_cpu(struct
dsa_switch_tree *dst)
static int dsa_tree_setup_default_cpu(struct dsa_switch_tree *dst)
{
- struct dsa_switch *ds;
- struct dsa_port *dp;
- int device, port;
+ struct dsa_port *cpu_dp, *dp;
- /* DSA currently only supports a single CPU port */
- dst->cpu_dp = dsa_tree_find_first_cpu(dst);
- if (!dst->cpu_dp) {
- pr_warn("Tree has no master device\n");
+ cpu_dp = dsa_tree_find_first_cpu(dst);
+ if (!cpu_dp) {
+ pr_err("DSA: tree %d has no CPU port\n", dst->index);
return -EINVAL;
}
/* Assign the default CPU port to all ports of the fabric */
- for (device = 0; device < DSA_MAX_SWITCHES; device++) {
- ds = dst->ds[device];
- if (!ds)
- continue;
-
- 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;
- }
- }
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dsa_port_is_user(dp) || dsa_port_is_dsa(dp))
+ dp->cpu_dp = cpu_dp;
return 0;
}
static void dsa_tree_teardown_default_cpu(struct dsa_switch_tree *dst)
{
- /* DSA currently only supports a single CPU port */
- dst->cpu_dp = NULL;
+ struct dsa_port *dp;
+
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dsa_port_is_user(dp) || dsa_port_is_dsa(dp))
+ dp->cpu_dp = NULL;
}
static int dsa_port_setup(struct dsa_port *dp)
--
2.23.0
[PATCH net-next 01/16] net: dsa: use dsa_to_port helper everywhere
Do not let the drivers access the ds->ports static array directly
while there is a dsa_to_port helper for this purpose.
At the same time, un-const this helper since the SJA1105 driver
assigns the priv member of the returned dsa_port structure.
Signed-off-by: Vivien Didelot
---
drivers/net/dsa/b53/b53_common.c | 6 +++---
drivers/net/dsa/bcm_sf2.c | 8
drivers/net/dsa/bcm_sf2_cfp.c | 6 +++---
drivers/net/dsa/mt7530.c | 12 ++--
drivers/net/dsa/mv88e6xxx/chip.c | 10 +-
drivers/net/dsa/qca8k.c| 2 +-
drivers/net/dsa/sja1105/sja1105_main.c | 18 +-
include/net/dsa.h | 2 +-
net/dsa/dsa.c | 8 +---
net/dsa/dsa2.c | 4 ++--
net/dsa/switch.c | 4 ++--
net/dsa/tag_8021q.c| 6 +++---
12 files changed, 44 insertions(+), 42 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 526ba2ab66f1..9ba91f1370ac 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -524,7 +524,7 @@ int b53_enable_port(struct dsa_switch *ds, int port, struct
phy_device *phy)
if (!dsa_is_user_port(ds, port))
return 0;
- cpu_port = ds->ports[port].cpu_dp->index;
+ cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
if (dev->ops->irq_enable)
ret = dev->ops->irq_enable(dev, port);
@@ -1629,7 +1629,7 @@ EXPORT_SYMBOL(b53_fdb_dump);
int b53_br_join(struct dsa_switch *ds, int port, struct net_device *br)
{
struct b53_device *dev = ds->priv;
- s8 cpu_port = ds->ports[port].cpu_dp->index;
+ s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
u16 pvlan, reg;
unsigned int i;
@@ -1675,7 +1675,7 @@ 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 = ds->ports[port].cpu_dp->index;
+ s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
unsigned int i;
u16 pvlan, reg, pvid;
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 26509fa37a50..c068a3b7207b 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -662,7 +662,7 @@ static void bcm_sf2_sw_fixed_state(struct dsa_switch *ds,
int port,
* state machine and make it go in PHY_FORCING state instead.
*/
if (!status->link)
- netif_carrier_off(ds->ports[port].slave);
+ netif_carrier_off(dsa_to_port(ds, port)->slave);
status->duplex = DUPLEX_FULL;
} else {
status->link = true;
@@ -728,7 +728,7 @@ static int bcm_sf2_sw_resume(struct dsa_switch *ds)
static void bcm_sf2_sw_get_wol(struct dsa_switch *ds, int port,
struct ethtool_wolinfo *wol)
{
- struct net_device *p = ds->ports[port].cpu_dp->master;
+ struct net_device *p = dsa_to_port(ds, port)->cpu_dp->master;
struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
struct ethtool_wolinfo pwol = { };
@@ -752,9 +752,9 @@ static void bcm_sf2_sw_get_wol(struct dsa_switch *ds, int
port,
static int bcm_sf2_sw_set_wol(struct dsa_switch *ds, int port,
struct ethtool_wolinfo *wol)
{
- struct net_device *p = ds->ports[port].cpu_dp->master;
+ struct net_device *p = dsa_to_port(ds, port)->cpu_dp->master;
struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
- s8 cpu_port = ds->ports[port].cpu_dp->index;
+ s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
struct ethtool_wolinfo pwol = { };
if (p->ethtool_ops->get_wol)
diff --git a/drivers/net/dsa/bcm_sf2_cfp.c b/drivers/net/dsa/bcm_sf2_cfp.c
index d264776a95a3..f3f0c3f07391 100644
--- a/drivers/net/dsa/bcm_sf2_cfp.c
+++ b/drivers/net/dsa/bcm_sf2_cfp.c
@@ -821,7 +821,7 @@ static int bcm_sf2_cfp_rule_insert(struct dsa_switch *ds,
int port,
struct ethtool_rx_flow_spec *fs)
{
struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
- s8 cpu_port = ds->ports[port].cpu_dp->index;
+ s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
__u64 ring_cookie = fs->ring_cookie;
unsigned int queue_num, port_num;
int ret;
@@ -1049,7 +1049,7 @@ static int bcm_sf2_cfp_rule_get_all(struct bcm_sf2_priv
*priv,
int bcm_sf2_get_rxnfc(struct dsa_switch *ds, int port,
struct ethtool_rxnfc *nfc, u32 *rule_locs)
{
- struct net_device *p = ds->ports[port].cpu_dp->master;
+ struct net_device *p = dsa_to_port(ds, port)->cpu_dp->m
[PATCH net-next 09/16] net: dsa: use ports list to find first CPU port
Use the new ports list instead of iterating over switches and their
ports when looking up the first CPU port in the tree.
Signed-off-by: Vivien Didelot
---
net/dsa/dsa2.c | 17 +++--
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 3b8de155bc0b..99f5dab06787 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -186,22 +186,11 @@ static bool dsa_tree_setup_routing_table(struct
dsa_switch_tree *dst)
static struct dsa_port *dsa_tree_find_first_cpu(struct dsa_switch_tree *dst)
{
- struct dsa_switch *ds;
struct dsa_port *dp;
- int device, port;
-
- for (device = 0; device < DSA_MAX_SWITCHES; device++) {
- ds = dst->ds[device];
- if (!ds)
- continue;
- for (port = 0; port < ds->num_ports; port++) {
- dp = &ds->ports[port];
-
- if (dsa_port_is_cpu(dp))
- return dp;
- }
- }
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dsa_port_is_cpu(dp))
+ return dp;
return NULL;
}
--
2.23.0
[PATCH net-next 04/16] net: dsa: use ports list to find slave
Use the new ports list instead of iterating over switches and their
ports when looking for a slave device from a given master interface.
Signed-off-by: Vivien Didelot
---
net/dsa/dsa_priv.h | 23 ++-
1 file changed, 6 insertions(+), 17 deletions(-)
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 12f8c7ee4dd8..53e7577896b6 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -104,25 +104,14 @@ static inline struct net_device
*dsa_master_find_slave(struct net_device *dev,
{
struct dsa_port *cpu_dp = dev->dsa_ptr;
struct dsa_switch_tree *dst = cpu_dp->dst;
- struct dsa_switch *ds;
- struct dsa_port *slave_port;
+ struct dsa_port *dp;
- if (device < 0 || device >= DSA_MAX_SWITCHES)
- return NULL;
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dp->ds->index == device && dp->index == port &&
+ dp->type == DSA_PORT_TYPE_USER)
+ return dp->slave;
- ds = dst->ds[device];
- if (!ds)
- return NULL;
-
- if (port < 0 || port >= ds->num_ports)
- return NULL;
-
- slave_port = &ds->ports[port];
-
- if (unlikely(slave_port->type != DSA_PORT_TYPE_USER))
- return NULL;
-
- return slave_port->slave;
+ return NULL;
}
/* port.c */
--
2.23.0
[PATCH net-next 08/16] net: dsa: use ports list to setup multiple master devices
Now that we have a potential list of CPU ports, make use of it instead
of only configuring the master device of an unique CPU port.
Signed-off-by: Vivien Didelot
---
net/dsa/dsa2.c | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 8b038cc56769..3b8de155bc0b 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -501,19 +501,27 @@ static void dsa_tree_teardown_switches(struct
dsa_switch_tree *dst)
static int dsa_tree_setup_master(struct dsa_switch_tree *dst)
{
- struct dsa_port *cpu_dp = dst->cpu_dp;
- struct net_device *master = cpu_dp->master;
+ struct dsa_port *dp;
+ int err;
- /* DSA currently supports a single pair of CPU port and master device */
- return dsa_master_setup(master, cpu_dp);
+ list_for_each_entry(dp, &dst->ports, list) {
+ if (dsa_port_is_cpu(dp)) {
+ err = dsa_master_setup(dp->master, dp);
+ if (err)
+ return err;
+ }
+ }
+
+ return 0;
}
static void dsa_tree_teardown_master(struct dsa_switch_tree *dst)
{
- struct dsa_port *cpu_dp = dst->cpu_dp;
- struct net_device *master = cpu_dp->master;
+ struct dsa_port *dp;
- return dsa_master_teardown(master);
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dsa_port_is_cpu(dp))
+ dsa_master_teardown(dp->master);
}
static int dsa_tree_setup(struct dsa_switch_tree *dst)
--
2.23.0
[PATCH net-next 07/16] net: dsa: use ports list to find a port by node
Use the new ports list instead of iterating over switches and their
ports to find a port from a given node.
Signed-off-by: Vivien Didelot
---
net/dsa/dsa2.c | 17 +++--
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 84afeaeef141..8b038cc56769 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -113,22 +113,11 @@ static bool dsa_port_is_user(struct dsa_port *dp)
static struct dsa_port *dsa_tree_find_port_by_node(struct dsa_switch_tree *dst,
struct device_node *dn)
{
- struct dsa_switch *ds;
struct dsa_port *dp;
- int device, port;
-
- for (device = 0; device < DSA_MAX_SWITCHES; device++) {
- ds = dst->ds[device];
- if (!ds)
- continue;
- for (port = 0; port < ds->num_ports; port++) {
- dp = &ds->ports[port];
-
- if (dp->dn == dn)
- return dp;
- }
- }
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dp->dn == dn)
+ return dp;
return NULL;
}
--
2.23.0
[PATCH net-next 05/16] net: dsa: use ports list to setup switches
Use the new ports list instead of iterating over switches and their
ports when setting up the switches and their ports.
At the same time, provide setup states and messages for ports and
switches as it is done for the trees.
Signed-off-by: Vivien Didelot
---
include/net/dsa.h | 4 ++
net/dsa/dsa2.c| 101 ++
2 files changed, 53 insertions(+), 52 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 938de9518c61..b199a8ca6393 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -215,9 +215,13 @@ struct dsa_port {
* Original copy of the master netdev net_device_ops
*/
const struct net_device_ops *orig_ndo_ops;
+
+ bool setup;
};
struct dsa_switch {
+ bool setup;
+
struct device *dev;
/*
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index b6536641ac99..fd2b7f157f97 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -267,6 +267,9 @@ static int dsa_port_setup(struct dsa_port *dp)
bool dsa_port_enabled = false;
int err = 0;
+ if (dp->setup)
+ return 0;
+
switch (dp->type) {
case DSA_PORT_TYPE_UNUSED:
dsa_port_disable(dp);
@@ -335,14 +338,23 @@ static int dsa_port_setup(struct dsa_port *dp)
dsa_port_link_unregister_of(dp);
if (err && devlink_port_registered)
devlink_port_unregister(dlp);
+ if (err)
+ return err;
- return err;
+ dp->setup = true;
+
+ pr_info("DSA: switch %d port %d setup\n", dp->ds->index, dp->index);
+
+ return 0;
}
static void dsa_port_teardown(struct dsa_port *dp)
{
struct devlink_port *dlp = &dp->devlink_port;
+ if (!dp->setup)
+ return;
+
switch (dp->type) {
case DSA_PORT_TYPE_UNUSED:
break;
@@ -365,11 +377,18 @@ static void dsa_port_teardown(struct dsa_port *dp)
}
break;
}
+
+ dp->setup = false;
+
+ pr_info("DSA: switch %d port %d torn down\n", dp->ds->index, dp->index);
}
static int dsa_switch_setup(struct dsa_switch *ds)
{
- int err = 0;
+ int err;
+
+ if (ds->setup)
+ return 0;
/* Initialize ds->phys_mii_mask before registering the slave MDIO bus
* driver and before ops->setup() has run, since the switch drivers and
@@ -411,6 +430,10 @@ static int dsa_switch_setup(struct dsa_switch *ds)
goto unregister_notifier;
}
+ ds->setup = true;
+
+ pr_info("DSA: switch %d setup\n", ds->index);
+
return 0;
unregister_notifier:
@@ -426,6 +449,9 @@ static int dsa_switch_setup(struct dsa_switch *ds)
static void dsa_switch_teardown(struct dsa_switch *ds)
{
+ if (!ds->setup)
+ return;
+
if (ds->slave_mii_bus && ds->ops->phy_read)
mdiobus_unregister(ds->slave_mii_bus);
@@ -440,78 +466,49 @@ static void dsa_switch_teardown(struct dsa_switch *ds)
ds->devlink = NULL;
}
+ ds->setup = false;
+
+ pr_info("DSA: switch %d torn down\n", ds->index);
}
static int dsa_tree_setup_switches(struct dsa_switch_tree *dst)
{
- struct dsa_switch *ds;
struct dsa_port *dp;
- int device, port, i;
- int err = 0;
-
- for (device = 0; device < DSA_MAX_SWITCHES; device++) {
- ds = dst->ds[device];
- if (!ds)
- continue;
+ int err;
- err = dsa_switch_setup(ds);
+ list_for_each_entry(dp, &dst->ports, list) {
+ err = dsa_switch_setup(dp->ds);
if (err)
- goto switch_teardown;
-
- for (port = 0; port < ds->num_ports; port++) {
- dp = &ds->ports[port];
+ goto teardown;
+ }
- err = dsa_port_setup(dp);
- if (err)
- goto ports_teardown;
- }
+ list_for_each_entry(dp, &dst->ports, list) {
+ err = dsa_port_setup(dp);
+ if (err)
+ goto teardown;
}
return 0;
-ports_teardown:
- for (i = 0; i < port; i++)
- dsa_port_teardown(&ds->ports[i]);
-
- dsa_switch_teardown(ds);
-
-switch_teardown:
- for (i = 0; i < device; i++) {
- ds = dst->ds[i];
- if (!ds)
- continue;
-
- for (port = 0; port < ds->num_ports; port++) {
- dp = &ds->ports[port];
-
- dsa_port_teardown(dp);
- }
+teardown:
+ list_for_each_entry(d
[PATCH net-next 14/16] net: dsa: sja1105: register switch before assigning port private data
Like the dsa_switch_tree structures, the dsa_port structures will be
allocated on switch registration.
The SJA1105 driver is the only one accessing the dsa_port structure
after the switch allocation and before the switch registration.
For that reason, move switch registration prior to assigning the priv
member of the dsa_port structures.
Signed-off-by: Vivien Didelot
---
drivers/net/dsa/sja1105/sja1105_main.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c
b/drivers/net/dsa/sja1105/sja1105_main.c
index 4b0cb779f187..0ebbda5ca665 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2057,6 +2057,15 @@ static int sja1105_probe(struct spi_device *spi)
tagger_data = &priv->tagger_data;
+ mutex_init(&priv->ptp_data.lock);
+ mutex_init(&priv->mgmt_lock);
+
+ sja1105_tas_setup(ds);
+
+ rc = dsa_register_switch(priv->ds);
+ if (rc)
+ return rc;
+
/* Connections between dsa_port and sja1105_port */
for (i = 0; i < SJA1105_NUM_PORTS; i++) {
struct sja1105_port *sp = &priv->ports[i];
@@ -2065,12 +2074,8 @@ static int sja1105_probe(struct spi_device *spi)
sp->dp = dsa_to_port(ds, i);
sp->data = tagger_data;
}
- mutex_init(&priv->ptp_data.lock);
- mutex_init(&priv->mgmt_lock);
- sja1105_tas_setup(ds);
-
- return dsa_register_switch(priv->ds);
+ return 0;
}
static int sja1105_remove(struct spi_device *spi)
--
2.23.0
[PATCH net-next 13/16] net: dsa: mv88e6xxx: use ports list to map bridge
Instead of digging into the other dsa_switch structures of the fabric
and relying too much on the dsa_to_port helper, use the new list
of switch fabric ports to remap the Port VLAN Map of local bridge
group members or remap the Port VLAN Table entry of external bridge
group members.
Signed-off-by: Vivien Didelot
---
drivers/net/dsa/mv88e6xxx/chip.c | 36 +---
1 file changed, 15 insertions(+), 21 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index af8943142053..8771f2525932 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2043,29 +2043,23 @@ static int mv88e6xxx_port_fdb_dump(struct dsa_switch
*ds, int port,
static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip *chip,
struct net_device *br)
{
- struct dsa_switch *ds;
- int port;
- int dev;
+ struct dsa_switch *ds = chip->ds;
+ struct dsa_switch_tree *dst = ds->dst;
+ struct dsa_port *dp;
int err;
- /* Remap the Port VLAN of each local bridge group member */
- for (port = 0; port < mv88e6xxx_num_ports(chip); ++port) {
- if (dsa_to_port(chip->ds, port)->bridge_dev == br) {
- err = mv88e6xxx_port_vlan_map(chip, port);
- if (err)
- return err;
- }
- }
-
- /* Remap the Port VLAN of each cross-chip bridge group member */
- for (dev = 0; dev < DSA_MAX_SWITCHES; ++dev) {
- ds = chip->ds->dst->ds[dev];
- if (!ds)
- break;
-
- for (port = 0; port < ds->num_ports; ++port) {
- if (dsa_to_port(ds, port)->bridge_dev == br) {
- err = mv88e6xxx_pvt_map(chip, dev, port);
+ list_for_each_entry(dp, &dst->ports, list) {
+ /* Remap the Port VLAN Map of local bridge group members and
+* remap the PVT entry of external bridge group members.
+*/
+ if (dp->bridge_dev == br) {
+ if (dp->ds == ds) {
+ err = mv88e6xxx_port_vlan_map(chip, dp->index);
+ if (err)
+ return err;
+ } else {
+ err = mv88e6xxx_pvt_map(chip, dp->ds->index,
+ dp->index);
if (err)
return err;
}
--
2.23.0
[PATCH net-next 12/16] net: dsa: mv88e6xxx: use ports list to map port VLAN
Instead of digging into the other dsa_switch structures of the fabric
and relying too much on the dsa_to_port helper, use the new list of
switch fabric ports to define the mask of the local ports allowed to
receive frames from another port of the fabric.
Signed-off-by: Vivien Didelot
---
drivers/net/dsa/mv88e6xxx/chip.c | 32
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 510ccdc2d03c..af8943142053 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1057,35 +1057,43 @@ static int mv88e6xxx_set_mac_eee(struct dsa_switch *ds,
int port,
return 0;
}
+/* Mask of the local ports allowed to receive frames from a given fabric port
*/
static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port)
{
- struct dsa_switch *ds = NULL;
+ struct dsa_switch *ds = chip->ds;
+ struct dsa_switch_tree *dst = ds->dst;
struct net_device *br;
+ struct dsa_port *dp;
+ bool found = false;
u16 pvlan;
- int i;
- if (dev < DSA_MAX_SWITCHES)
- ds = chip->ds->dst->ds[dev];
+ list_for_each_entry(dp, &dst->ports, list) {
+ if (dp->ds->index == dev && dp->index == port) {
+ found = true;
+ break;
+ }
+ }
/* Prevent frames from unknown switch or port */
- if (!ds || port >= ds->num_ports)
+ if (!found)
return 0;
/* Frames from DSA links and CPU ports can egress any local port */
- if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port))
+ if (dp->type == DSA_PORT_TYPE_CPU || dp->type == DSA_PORT_TYPE_DSA)
return mv88e6xxx_port_mask(chip);
- br = dsa_to_port(ds, port)->bridge_dev;
+ br = dp->bridge_dev;
pvlan = 0;
/* Frames from user ports can egress any local DSA links and CPU ports,
* as well as any local member of their bridge group.
*/
- for (i = 0; i < mv88e6xxx_num_ports(chip); ++i)
- if (dsa_is_cpu_port(chip->ds, i) ||
- dsa_is_dsa_port(chip->ds, i) ||
- (br && dsa_to_port(chip->ds, i)->bridge_dev == br))
- pvlan |= BIT(i);
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dp->ds == ds &&
+ (dp->type == DSA_PORT_TYPE_CPU ||
+dp->type == DSA_PORT_TYPE_DSA ||
+(br && dp->bridge_dev == br)))
+ pvlan |= BIT(dp->index);
return pvlan;
}
--
2.23.0
[PATCH net-next 15/16] net: dsa: allocate ports on touch
Allocate the struct dsa_port the first time it is accessed with
dsa_port_touch, and remove the static dsa_port array from the
dsa_switch structure.
Signed-off-by: Vivien Didelot
---
include/net/dsa.h | 2 --
net/dsa/dsa2.c| 16 ++--
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 020f5db8666b..d28ac54cb8c4 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -277,9 +277,7 @@ struct dsa_switch {
*/
boolvlan_filtering;
- /* Dynamically allocated ports, keep last */
size_t num_ports;
- struct dsa_port ports[];
};
static inline struct dsa_port *dsa_to_port(struct dsa_switch *ds, int p)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 772deacc33d3..7669a6278c40 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -596,7 +596,13 @@ static struct dsa_port *dsa_port_touch(struct dsa_switch
*ds, int index)
struct dsa_switch_tree *dst = ds->dst;
struct dsa_port *dp;
- dp = &ds->ports[index];
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dp->ds == ds && dp->index == index)
+ return dp;
+
+ dp = kzalloc(sizeof(*dp), GFP_KERNEL);
+ if (!dp)
+ return NULL;
dp->ds = ds;
dp->index = index;
@@ -865,7 +871,7 @@ struct dsa_switch *dsa_switch_alloc(struct device *dev,
size_t n)
{
struct dsa_switch *ds;
- ds = devm_kzalloc(dev, struct_size(ds, ports, n), GFP_KERNEL);
+ ds = devm_kzalloc(dev, sizeof(*ds), GFP_KERNEL);
if (!ds)
return NULL;
@@ -893,6 +899,12 @@ static void dsa_switch_remove(struct dsa_switch *ds)
{
struct dsa_switch_tree *dst = ds->dst;
unsigned int index = ds->index;
+ struct dsa_port *dp, *next;
+
+ list_for_each_entry_safe(dp, next, &dst->ports, list) {
+ list_del(&dp->list);
+ kfree(dp);
+ }
dsa_tree_remove_switch(dst, index);
}
--
2.23.0
[PATCH net-next 16/16] net: dsa: remove dsa_switch_alloc helper
Now that ports are dynamically listed in the fabric, there is no need
to provide a special helper to allocate the dsa_switch structure. This
will give more flexibility to drivers to embed this structure as they
wish in their private structure.
Signed-off-by: Vivien Didelot
---
drivers/net/dsa/b53/b53_common.c | 5 -
drivers/net/dsa/dsa_loop.c | 5 -
drivers/net/dsa/lan9303-core.c | 4 +++-
drivers/net/dsa/lantiq_gswip.c | 4 +++-
drivers/net/dsa/microchip/ksz_common.c | 5 -
drivers/net/dsa/mt7530.c | 5 -
drivers/net/dsa/mv88e6060.c| 4 +++-
drivers/net/dsa/mv88e6xxx/chip.c | 4 +++-
drivers/net/dsa/qca8k.c| 5 -
drivers/net/dsa/realtek-smi-core.c | 5 -
drivers/net/dsa/sja1105/sja1105_main.c | 4 +++-
drivers/net/dsa/vitesse-vsc73xx-core.c | 5 -
include/net/dsa.h | 1 -
net/dsa/dsa2.c | 21 ++---
14 files changed, 49 insertions(+), 28 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 9ba91f1370ac..0a5ab2ce74e3 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -2342,10 +2342,13 @@ struct b53_device *b53_switch_alloc(struct device *base,
struct dsa_switch *ds;
struct b53_device *dev;
- ds = dsa_switch_alloc(base, DSA_MAX_PORTS);
+ ds = devm_kzalloc(base, sizeof(*ds), GFP_KERNEL);
if (!ds)
return NULL;
+ ds->dev = base;
+ ds->num_ports = DSA_MAX_PORTS;
+
dev = devm_kzalloc(base, sizeof(*dev), GFP_KERNEL);
if (!dev)
return NULL;
diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
index 925ed135a4d9..c8d7ef27fd72 100644
--- a/drivers/net/dsa/dsa_loop.c
+++ b/drivers/net/dsa/dsa_loop.c
@@ -286,10 +286,13 @@ static int dsa_loop_drv_probe(struct mdio_device *mdiodev)
dev_info(&mdiodev->dev, "%s: 0x%0x\n",
pdata->name, pdata->enabled_ports);
- ds = dsa_switch_alloc(&mdiodev->dev, DSA_MAX_PORTS);
+ ds = devm_kzalloc(&mdiodev->dev, sizeof(*ds), GFP_KERNEL);
if (!ds)
return -ENOMEM;
+ ds->dev = &mdiodev->dev;
+ ds->num_ports = DSA_MAX_PORTS;
+
ps = devm_kzalloc(&mdiodev->dev, sizeof(*ps), GFP_KERNEL);
if (!ps)
return -ENOMEM;
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index bbec86b9418e..e3c333a8f45d 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -1283,10 +1283,12 @@ static int lan9303_register_switch(struct lan9303 *chip)
{
int base;
- chip->ds = dsa_switch_alloc(chip->dev, LAN9303_NUM_PORTS);
+ chip->ds = devm_kzalloc(chip->dev, sizeof(*chip->ds), GFP_KERNEL);
if (!chip->ds)
return -ENOMEM;
+ chip->ds->dev = chip->dev;
+ chip->ds->num_ports = LAN9303_NUM_PORTS;
chip->ds->priv = chip;
chip->ds->ops = &lan9303_switch_ops;
base = chip->phy_addr_base;
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index a69c9b9878b7..955324968b74 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1854,10 +1854,12 @@ static int gswip_probe(struct platform_device *pdev)
if (!priv->hw_info)
return -EINVAL;
- priv->ds = dsa_switch_alloc(dev, priv->hw_info->max_ports);
+ priv->ds = devm_kzalloc(dev, sizeof(*priv->ds), GFP_KERNEL);
if (!priv->ds)
return -ENOMEM;
+ priv->ds->dev = dev;
+ priv->ds->num_ports = priv->hw_info->max_ports;
priv->ds->priv = priv;
priv->ds->ops = &gswip_switch_ops;
priv->dev = dev;
diff --git a/drivers/net/dsa/microchip/ksz_common.c
b/drivers/net/dsa/microchip/ksz_common.c
index b0b870f0c252..c755d9e1c4f6 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -398,10 +398,13 @@ struct ksz_device *ksz_switch_alloc(struct device *base,
void *priv)
struct dsa_switch *ds;
struct ksz_device *swdev;
- ds = dsa_switch_alloc(base, DSA_MAX_PORTS);
+ ds = devm_kzalloc(base, sizeof(*ds), GFP_KERNEL);
if (!ds)
return NULL;
+ ds->dev = base;
+ ds->num_ports = DSA_MAX_PORTS;
+
swdev = devm_kzalloc(base, sizeof(*swdev), GFP_KERNEL);
if (!swdev)
return NULL;
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index a91293e47a57..add9e4279176 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1632,10 +1632,13 @@ mt7530_probe(struct mdio_device *mdiodev)
[PATCH net-next 11/16] net: dsa: mv88e6xxx: silently skip PVT ops
Since mv88e6xxx_pvt_map is a static helper, no need to return
-EOPNOTSUPP if the chip has no PVT, simply silently skip the operation.
Signed-off-by: Vivien Didelot
---
drivers/net/dsa/mv88e6xxx/chip.c | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index d67deec77452..510ccdc2d03c 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1253,7 +1253,7 @@ static int mv88e6xxx_pvt_map(struct mv88e6xxx_chip *chip,
int dev, int port)
u16 pvlan = 0;
if (!mv88e6xxx_has_pvt(chip))
- return -EOPNOTSUPP;
+ return 0;
/* Skip the local source device, which uses in-chip port VLAN */
if (dev != chip->ds->index)
@@ -2049,9 +2049,6 @@ static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip
*chip,
}
}
- if (!mv88e6xxx_has_pvt(chip))
- return 0;
-
/* Remap the Port VLAN of each cross-chip bridge group member */
for (dev = 0; dev < DSA_MAX_SWITCHES; ++dev) {
ds = chip->ds->dst->ds[dev];
@@ -2101,9 +2098,6 @@ static int mv88e6xxx_crosschip_bridge_join(struct
dsa_switch *ds, int dev,
struct mv88e6xxx_chip *chip = ds->priv;
int err;
- if (!mv88e6xxx_has_pvt(chip))
- return 0;
-
mv88e6xxx_reg_lock(chip);
err = mv88e6xxx_pvt_map(chip, dev, port);
mv88e6xxx_reg_unlock(chip);
@@ -2116,9 +2110,6 @@ static void mv88e6xxx_crosschip_bridge_leave(struct
dsa_switch *ds, int dev,
{
struct mv88e6xxx_chip *chip = ds->priv;
- if (!mv88e6xxx_has_pvt(chip))
- return;
-
mv88e6xxx_reg_lock(chip);
if (mv88e6xxx_pvt_map(chip, dev, port))
dev_err(ds->dev, "failed to remap cross-chip Port VLAN\n");
--
2.23.0
[PATCH net-next 06/16] net: dsa: use ports list for routing table setup
Use the new ports list instead of accessing the dsa_switch array
of ports when iterating over DSA ports of a switch to set up the
routing table.
Signed-off-by: Vivien Didelot
---
net/dsa/dsa2.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index fd2b7f157f97..84afeaeef141 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -157,6 +157,7 @@ static bool dsa_port_setup_routing_table(struct dsa_port
*dp)
static bool dsa_switch_setup_routing_table(struct dsa_switch *ds)
{
+ struct dsa_switch_tree *dst = ds->dst;
bool complete = true;
struct dsa_port *dp;
int i;
@@ -164,10 +165,8 @@ static bool dsa_switch_setup_routing_table(struct
dsa_switch *ds)
for (i = 0; i < DSA_MAX_SWITCHES; i++)
ds->rtable[i] = DSA_RTABLE_NONE;
- for (i = 0; i < ds->num_ports; i++) {
- dp = &ds->ports[i];
-
- if (dsa_port_is_dsa(dp)) {
+ list_for_each_entry(dp, &dst->ports, list) {
+ if (dp->ds == ds && dsa_port_is_dsa(dp)) {
complete = dsa_port_setup_routing_table(dp);
if (!complete)
break;
--
2.23.0
[PATCH net-next 02/16] net: dsa: add ports list in the switch fabric
Add a list of switch ports within the switch fabric. This will help the
lookup of a port inside the whole fabric, and it is the first step
towards supporting multiple CPU ports, before deprecating the usage of
the unique dst->cpu_dp pointer.
In preparation for a future allocation of the dsa_port structures,
return -ENOMEM in case no structure is returned, even though this
error cannot be reached yet.
Signed-off-by: Vivien Didelot
---
include/net/dsa.h | 5 +
net/dsa/dsa2.c| 48 +--
2 files changed, 47 insertions(+), 6 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 2e4fe2f8962b..6ff6dfcdc61d 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -125,6 +125,9 @@ struct dsa_switch_tree {
*/
struct dsa_port *cpu_dp;
+ /* List of switch ports */
+ struct list_head ports;
+
/*
* Data for the individual switch chips.
*/
@@ -195,6 +198,8 @@ struct dsa_port {
struct work_struct xmit_work;
struct sk_buff_head xmit_queue;
+ struct list_head list;
+
/*
* Give the switch driver somewhere to hang its per-port private data
* structures (accessible from the tagger).
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 1716535167ee..b6536641ac99 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -45,6 +45,8 @@ static struct dsa_switch_tree *dsa_tree_alloc(int index)
dst->index = index;
+ INIT_LIST_HEAD(&dst->ports);
+
INIT_LIST_HEAD(&dst->list);
list_add_tail(&dst->list, &dsa_tree_list);
@@ -616,6 +618,22 @@ static int dsa_tree_add_switch(struct dsa_switch_tree *dst,
return err;
}
+static struct dsa_port *dsa_port_touch(struct dsa_switch *ds, int index)
+{
+ struct dsa_switch_tree *dst = ds->dst;
+ struct dsa_port *dp;
+
+ dp = &ds->ports[index];
+
+ dp->ds = ds;
+ dp->index = index;
+
+ INIT_LIST_HEAD(&dp->list);
+ list_add(&dp->list, &dst->ports);
+
+ return dp;
+}
+
static int dsa_port_parse_user(struct dsa_port *dp, const char *name)
{
if (!name)
@@ -742,6 +760,20 @@ static int dsa_switch_parse_member_of(struct dsa_switch
*ds,
return 0;
}
+static int dsa_switch_touch_ports(struct dsa_switch *ds)
+{
+ struct dsa_port *dp;
+ int port;
+
+ for (port = 0; port < ds->num_ports; port++) {
+ dp = dsa_port_touch(ds, port);
+ if (!dp)
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
static int dsa_switch_parse_of(struct dsa_switch *ds, struct device_node *dn)
{
int err;
@@ -750,6 +782,10 @@ static int dsa_switch_parse_of(struct dsa_switch *ds,
struct device_node *dn)
if (err)
return err;
+ err = dsa_switch_touch_ports(ds);
+ if (err)
+ return err;
+
return dsa_switch_parse_ports_of(ds, dn);
}
@@ -807,6 +843,8 @@ static int dsa_switch_parse_ports(struct dsa_switch *ds,
static int dsa_switch_parse(struct dsa_switch *ds, struct dsa_chip_data *cd)
{
+ int err;
+
ds->cd = cd;
/* We don't support interconnected switches nor multiple trees via
@@ -817,6 +855,10 @@ static int dsa_switch_parse(struct dsa_switch *ds, struct
dsa_chip_data *cd)
if (!ds->dst)
return -ENOMEM;
+ err = dsa_switch_touch_ports(ds);
+ if (err)
+ return err;
+
return dsa_switch_parse_ports(ds, cd);
}
@@ -849,7 +891,6 @@ static int dsa_switch_probe(struct dsa_switch *ds)
struct dsa_switch *dsa_switch_alloc(struct device *dev, size_t n)
{
struct dsa_switch *ds;
- int i;
ds = devm_kzalloc(dev, struct_size(ds, ports, n), GFP_KERNEL);
if (!ds)
@@ -858,11 +899,6 @@ struct dsa_switch *dsa_switch_alloc(struct device *dev,
size_t n)
ds->dev = dev;
ds->num_ports = n;
- for (i = 0; i < ds->num_ports; ++i) {
- ds->ports[i].index = i;
- ds->ports[i].ds = ds;
- }
-
return ds;
}
EXPORT_SYMBOL_GPL(dsa_switch_alloc);
--
2.23.0
[PATCH net-next 03/16] net: dsa: use ports list in dsa_to_port
Use the new ports list instead of accessing the dsa_switch array
of ports in the dsa_to_port helper.
Signed-off-by: Vivien Didelot
---
include/net/dsa.h | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 6ff6dfcdc61d..938de9518c61 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -285,7 +285,14 @@ struct dsa_switch {
static inline struct dsa_port *dsa_to_port(struct dsa_switch *ds, int p)
{
- return &ds->ports[p];
+ struct dsa_switch_tree *dst = ds->dst;
+ struct dsa_port *dp;
+
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dp->ds == ds && dp->index == p)
+ return dp;
+
+ return NULL;
}
static inline bool dsa_is_unused_port(struct dsa_switch *ds, int p)
--
2.23.0
[PATCH net-next 00/16] net: dsa: turn arrays of ports into a list
The dsa_switch structure represents the physical switch device itself, and is allocated by the driver. The dsa_switch_tree and dsa_port structures represent the logical switch fabric (eventually composed of multiple switch devices) and its ports, and are allocated by the DSA core. This branch lists the logical ports directly in the fabric which simplifies the iteration over all ports when assigning the default CPU port or configuring the D in DSA in drivers like mv88e6xxx. This also removes the unique dst->cpu_dp pointer and is a first step towards supporting multiple CPU ports and dropping the DSA_MAX_PORTS limitation. Because the dsa_port structures are not tight to the dsa_switch structure anymore, we do not need to provide an helper for the drivers to allocate a switch structure. Like in many other subsystems, drivers can now embed their dsa_switch structure as they wish into their private structure. This will be particularly interesting for the Broadcom drivers which were currently limited by the dynamically allocated array of DSA ports. The series implements the list of dsa_port structures, makes use of it, then drops dst->cpu_dp and the dsa_switch_alloc helper. Vivien Didelot (16): net: dsa: use dsa_to_port helper everywhere net: dsa: add ports list in the switch fabric net: dsa: use ports list in dsa_to_port net: dsa: use ports list to find slave net: dsa: use ports list to setup switches net: dsa: use ports list for routing table setup net: dsa: use ports list to find a port by node net: dsa: use ports list to setup multiple master devices net: dsa: use ports list to find first CPU port net: dsa: use ports list to setup default CPU port net: dsa: mv88e6xxx: silently skip PVT ops net: dsa: mv88e6xxx: use ports list to map port VLAN net: dsa: mv88e6xxx: use ports list to map bridge net: dsa: sja1105: register switch before assigning port private data net: dsa: allocate ports on touch net: dsa: remove dsa_switch_alloc helper drivers/net/dsa/b53/b53_common.c | 11 +- drivers/net/dsa/bcm_sf2.c | 8 +- drivers/net/dsa/bcm_sf2_cfp.c | 6 +- drivers/net/dsa/dsa_loop.c | 5 +- drivers/net/dsa/lan9303-core.c | 4 +- drivers/net/dsa/lantiq_gswip.c | 4 +- drivers/net/dsa/microchip/ksz_common.c | 5 +- drivers/net/dsa/mt7530.c | 17 +- drivers/net/dsa/mv88e6060.c| 4 +- drivers/net/dsa/mv88e6xxx/chip.c | 87 drivers/net/dsa/qca8k.c| 7 +- drivers/net/dsa/realtek-smi-core.c | 5 +- drivers/net/dsa/sja1105/sja1105_main.c | 37 ++-- drivers/net/dsa/vitesse-vsc73xx-core.c | 5 +- include/net/dsa.h | 26 ++- net/dsa/dsa.c | 8 +- net/dsa/dsa2.c | 282 + net/dsa/dsa_priv.h | 23 +- net/dsa/switch.c | 4 +- net/dsa/tag_8021q.c| 6 +- 20 files changed, 297 insertions(+), 257 deletions(-) -- 2.23.0
Re: [PATCH net] net: dsa: b53: Do not clear existing mirrored port mask
On Sat, 5 Oct 2019 15:05:18 -0700, Florian Fainelli
wrote:
> Clearing the existing bitmask of mirrored ports essentially prevents us
> from capturing more than one port at any given time. This is clearly
> wrong, do not clear the bitmask prior to setting up the new port.
>
> Reported-by: Hubert Feurstein
> Fixes: ed3af5fd08eb ("net: dsa: b53: Add support for port mirroring")
> Signed-off-by: Florian Fainelli
Reviewed-by: Vivien Didelot
Re: [PATCH] tty: serial: fsl_lpuart: Fix lpuart_flush_buffer()
Hi Andrey,
On Fri, 4 Oct 2019 14:55:37 -0700, Andrey Smirnov
wrote:
> Fix incorrect read-modify-write sequence in lpuart_flush_buffer() that
> was reading from UARTPFIFO and writing to UARTCFIFO instead of
> operating solely on the latter.
>
> Fixes: 9bc19af9dacb ("tty: serial: fsl_lpuart: Flush HW FIFOs in
> .flush_buffer")
> Signed-off-by: Andrey Smirnov
This fixes the TTY on my ZII Devel Boards Rev B and C:
Reported-by: Vivien Didelot
Tested-by: Vivien Didelot
Thank you!
Vivien
Re: [PATCH] net: dsa: b53: Use the correct style for SPDX License Identifier
On Sat, 21 Sep 2019 19:00:16 +0530, Nishad Kamdar wrote: > This patch corrects the SPDX License Identifier style > in header file for Broadcom BCM53xx managed switch driver. > For C header files Documentation/process/license-rules.rst > mandates C-like comments (opposed to C source files where > C++ style should be used) > > Changes made by using a script provided by Joe Perches here: > https://lkml.org/lkml/2019/2/7/46. > > Suggested-by: Joe Perches > Signed-off-by: Nishad Kamdar Reviewed-by: Vivien Didelot
Re: [PATCH] net: dsa: Use the correct style for SPDX License Identifier
On Sat, 21 Sep 2019 19:15:25 +0530, Nishad Kamdar wrote: > This patch corrects the SPDX License Identifier style > in header file for Distributed Switch Architecture drivers. > For C header files Documentation/process/license-rules.rst > mandates C-like comments (opposed to C source files where > C++ style should be used) > > Changes made by using a script provided by Joe Perches here: > https://lkml.org/lkml/2019/2/7/46. > > Suggested-by: Joe Perches > Signed-off-by: Nishad Kamdar Reviewed-by: Vivien Didelot
Re: [PATCH net-next 2/3] net: dsa: mv88e6xxx: extend PTP gettime function to read system clock
On Mon, 19 Aug 2019 15:27:33 +0200, Andrew Lunn wrote:
> > @@ -45,7 +45,8 @@ static int mv88e6xxx_smi_direct_write(struct
> > mv88e6xxx_chip *chip,
> > {
> > int ret;
> >
> > - ret = mdiobus_write_nested(chip->bus, dev, reg, data);
> > + ret = mdiobus_write_sts_nested(chip->bus, dev, reg, data,
> > + chip->ptp_sts);
> > if (ret < 0)
> > return ret;
> >
>
> Please also make a similar change to mv88e6xxx_smi_indirect_write().
> The last write in that function should be timestamped.
>
> Vivien, please could you think about these changes with respect to
> RMU. We probably want to skip the RMU in this case, so we get slow but
> uniform jitter, vs fast and unpredictable jitter from using the RMU.
The RMU will have its own mv88e6xxx_bus_ops.
Re: [PATCH net v3] net: dsa: Check existence of .port_mdb_add callback before calling it
t; [] (switchdev_deferred_process_work) from []
> (process_one_work+0x218/0x50c)
> [] (process_one_work) from []
> (worker_thread+0x44/0x5bc)
> [] (worker_thread) from [] (kthread+0x148/0x150)
> [] (kthread) from [] (ret_from_fork+0x14/0x2c)
> Exception stack(0xee871fb0 to 0xee871ff8)
> 1fa0:
>
> 1fc0:
>
> 1fe0: 0013
> Code: bad PC value
> ---[ end trace 1292c61abd17b130 ]---
>
> [] (dsa_switch_event) from []
> (notifier_call_chain+0x48/0x84)
> corresponds to
>
> $ arm-linux-gnueabihf-addr2line -C -i -e vmlinux c08533ec
>
> linux/net/dsa/switch.c:156
> linux/net/dsa/switch.c:178
> linux/net/dsa/switch.c:328
>
> Fixes: e6db98db8a95 ("net: dsa: add switch mdb bitmap functions")
> Signed-off-by: Chen-Yu Tsai
Thanks for your patience,
Reviewed-by: Vivien Didelot
Re: [PATCH net-next] net: dsa: sja1105: remove set but not used variables 'tx_vid' and 'rx_vid'
On Wed, 7 Aug 2019 21:08:56 +0800, YueHaibing wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/dsa/sja1105/sja1105_main.c: In function sja1105_fdb_dump:
> drivers/net/dsa/sja1105/sja1105_main.c:1226:14: warning:
> variable tx_vid set but not used [-Wunused-but-set-variable]
> drivers/net/dsa/sja1105/sja1105_main.c:1226:6: warning:
> variable rx_vid set but not used [-Wunused-but-set-variable]
>
> They are not used since commit 6d7c7d948a2e ("net: dsa:
> sja1105: Fix broken learning with vlan_filtering disabled")
>
> Reported-by: Hulk Robot
> Signed-off-by: YueHaibing
Reviewed-by: Vivien Didelot
Re: [PATCH net v2] net: dsa: Check existence of .port_mdb_add callback before calling it
Hi Chen-Yu,
On Wed, 7 Aug 2019 11:18:28 +0800, Chen-Yu Tsai wrote:
> On Wed, Aug 7, 2019 at 4:34 AM Vivien Didelot
> wrote:
> >
> > Hi Chen-Yu,
> >
> > On Wed, 7 Aug 2019 01:49:37 +0800, Chen-Yu Tsai wrote:
> > > On Wed, Aug 7, 2019 at 1:15 AM Vivien Didelot
> > > wrote:
> > > >
> > > > Hi Chen-Yu,
> > > >
> > > > On Tue, 6 Aug 2019 15:53:25 +0800, Chen-Yu Tsai
> > > > wrote:
> > > > > From: Chen-Yu Tsai
> > > > >
> > > > > With the recent addition of commit 75dad2520fc3 ("net: dsa: b53:
> > > > > Disable
> > > > > all ports on setup"), users of b53 (BCM53125 on Lamobo R1 in my case)
> > > > > are forced to use the dsa subsystem to enable the switch, instead of
> > > > > having it in the default transparent "forward-to-all" mode.
> > > > >
> > > > > The b53 driver does not support mdb bitmap functions. However the dsa
> > > > > layer does not check for the existence of the .port_mdb_add callback
> > > > > before actually using it. This results in a NULL pointer dereference,
> > > > > as shown in the kernel oops below.
> > > > >
> > > > > The other functions seem to be properly guarded. Do the same for
> > > > > .port_mdb_add in dsa_switch_mdb_add_bitmap() as well.
> > > > >
> > > > > b53 is not the only driver that doesn't support mdb bitmap functions.
> > > > > Others include bcm_sf2, dsa_loop, lantiq_gswip, mt7530, mv88e6060,
> > > > > qca8k, realtek-smi, and vitesse-vsc73xx.
> > > >
> > > > I don't know what you mean by that, there's no "mdb bitmap function"
> > > > support for drivers, only the port_mdb_{prepare,add,del} callbacks...
> > >
> > > The term was coined from commit e6db98db8a95 ("net: dsa: add switch mdb
> > > bitmap functions"). But yeah, .port_mdb_* ops/callbacks would be more
> > > appropriate.
> > >
> > > > > 8<--- cut here ---
> > > > > Unable to handle kernel NULL pointer dereference at virtual
> > > > > address
> > > > > pgd = (ptrval)
> > > > > [] *pgd=
> > > > > Internal error: Oops: 8005 [#1] SMP ARM
> > > > > Modules linked in: rtl8xxxu rtl8192cu rtl_usb rtl8192c_common
> > > > > rtlwifi mac80211 cfg80211
> > > > > CPU: 1 PID: 134 Comm: kworker/1:2 Not tainted
> > > > > 5.3.0-rc1-00247-gd3519030752a #1
> > > > > Hardware name: Allwinner sun7i (A20) Family
> > > > > Workqueue: events switchdev_deferred_process_work
> > > > > PC is at 0x0
> > > > > LR is at dsa_switch_event+0x570/0x620
> > > > > pc : [<>]lr : []psr: 80070013
> > > > > sp : ee871db8 ip : fp : ee98d0a4
> > > > > r10: 000c r9 : 0008 r8 : ee89f710
> > > > > r7 : ee98d040 r6 : ee98d088 r5 : c0f04c48 r4 : ee98d04c
> > > > > r3 : r2 : ee89f710 r1 : 0008 r0 : ee98d040
> > > > > Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
> > > > > Control: 10c5387d Table: 6deb406a DAC: 0051
> > > > > Process kworker/1:2 (pid: 134, stack limit = 0x(ptrval))
> > > > > Stack: (0xee871db8 to 0xee872000)
> > > > > 1da0:
> > > > > ee871e14 103ace2d
> > > > > 1dc0: ee871e14 0005
> > > > > c08524a0
> > > > > 1de0: e000 c014bdfc c0f04c48 ee871e98 c0f04c48 ee9e5000
> > > > > c0851120 c014bef0
> > > > > 1e00: b643aea2 ee9b4068 c08509a8 ee2bf940 ee89f710
> > > > > ee871ecb
> > > > > 1e20: 0008 103ace2d c087e248 ee29c868 103ace2d
> > > > > 0001
> > > > > 1e40: ee871e98 0006 c0fb2a50 c087e2d0
> > > > > c08523c4
> > > > > 1e60: c014bdfc 0006 c0fad2d0 ee871e98 ee89f710
> > > > > c014c500
> > > > > 1e80: ee89f3c0 c0f04c48 ee9e5000 c087dfb4
> > > > > ee9e5000
> >
Re: [PATCH net v2] net: dsa: Check existence of .port_mdb_add callback before calling it
Hi Chen-Yu,
On Wed, 7 Aug 2019 01:49:37 +0800, Chen-Yu Tsai wrote:
> On Wed, Aug 7, 2019 at 1:15 AM Vivien Didelot
> wrote:
> >
> > Hi Chen-Yu,
> >
> > On Tue, 6 Aug 2019 15:53:25 +0800, Chen-Yu Tsai wrote:
> > > From: Chen-Yu Tsai
> > >
> > > With the recent addition of commit 75dad2520fc3 ("net: dsa: b53: Disable
> > > all ports on setup"), users of b53 (BCM53125 on Lamobo R1 in my case)
> > > are forced to use the dsa subsystem to enable the switch, instead of
> > > having it in the default transparent "forward-to-all" mode.
> > >
> > > The b53 driver does not support mdb bitmap functions. However the dsa
> > > layer does not check for the existence of the .port_mdb_add callback
> > > before actually using it. This results in a NULL pointer dereference,
> > > as shown in the kernel oops below.
> > >
> > > The other functions seem to be properly guarded. Do the same for
> > > .port_mdb_add in dsa_switch_mdb_add_bitmap() as well.
> > >
> > > b53 is not the only driver that doesn't support mdb bitmap functions.
> > > Others include bcm_sf2, dsa_loop, lantiq_gswip, mt7530, mv88e6060,
> > > qca8k, realtek-smi, and vitesse-vsc73xx.
> >
> > I don't know what you mean by that, there's no "mdb bitmap function"
> > support for drivers, only the port_mdb_{prepare,add,del} callbacks...
>
> The term was coined from commit e6db98db8a95 ("net: dsa: add switch mdb
> bitmap functions"). But yeah, .port_mdb_* ops/callbacks would be more
> appropriate.
>
> > > 8<--- cut here ---
> > > Unable to handle kernel NULL pointer dereference at virtual address
> > >
> > > pgd = (ptrval)
> > > [] *pgd=
> > > Internal error: Oops: 8005 [#1] SMP ARM
> > > Modules linked in: rtl8xxxu rtl8192cu rtl_usb rtl8192c_common rtlwifi
> > > mac80211 cfg80211
> > > CPU: 1 PID: 134 Comm: kworker/1:2 Not tainted
> > > 5.3.0-rc1-00247-gd3519030752a #1
> > > Hardware name: Allwinner sun7i (A20) Family
> > > Workqueue: events switchdev_deferred_process_work
> > > PC is at 0x0
> > > LR is at dsa_switch_event+0x570/0x620
> > > pc : [<>]lr : []psr: 80070013
> > > sp : ee871db8 ip : fp : ee98d0a4
> > > r10: 000c r9 : 0008 r8 : ee89f710
> > > r7 : ee98d040 r6 : ee98d088 r5 : c0f04c48 r4 : ee98d04c
> > > r3 : r2 : ee89f710 r1 : 0008 r0 : ee98d040
> > > Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
> > > Control: 10c5387d Table: 6deb406a DAC: 0051
> > > Process kworker/1:2 (pid: 134, stack limit = 0x(ptrval))
> > > Stack: (0xee871db8 to 0xee872000)
> > > 1da0: ee871e14
> > > 103ace2d
> > > 1dc0: ee871e14 0005 c08524a0
> > >
> > > 1de0: e000 c014bdfc c0f04c48 ee871e98 c0f04c48 ee9e5000 c0851120
> > > c014bef0
> > > 1e00: b643aea2 ee9b4068 c08509a8 ee2bf940 ee89f710 ee871ecb
> > >
> > > 1e20: 0008 103ace2d c087e248 ee29c868 103ace2d 0001
> > >
> > > 1e40: ee871e98 0006 c0fb2a50 c087e2d0
> > > c08523c4
> > > 1e60: c014bdfc 0006 c0fad2d0 ee871e98 ee89f710
> > > c014c500
> > > 1e80: ee89f3c0 c0f04c48 ee9e5000 c087dfb4 ee9e5000
> > >
> > > 1ea0: ee89f710 ee871ecb 0001 103ace2d c0f04c48
> > > c087e0a8
> > > 1ec0: efd9a3e0 0089f3c0 103ace2d ee89f700 ee89f710 ee9e5000
> > > 0122
> > > 1ee0: 0100 c087e130 ee89f700 c0fad2c8 c1003ef0 c087de4c 2e928000
> > > c0fad2ec
> > > 1f00: c0fad2ec ee839580 ef7a62c0 ef7a9400 c087def8 c0fad2ec
> > > c01447dc
> > > 1f20: ef315640 ef7a62c0 0008 ee839580 ee839594 ef7a62c0 0008
> > > c0f03d00
> > > 1f40: ef7a62d8 ef7a62c0 e000 c0145b84 e000 c0fb2420 c0bfaa8c
> > >
> > > 1f60: e000 ee84b600 ee84b5c0 ee87 ee839580 c0145b40
> > > ef0e5ea4
> > > 1f80: ee84b61c c014a6f8 0001 ee84b5c0 c014a5b0
> > >
> > > 1fa0: 0
Re: [PATCH net v2] net: dsa: Check existence of .port_mdb_add callback before calling it
Hi Chen-Yu,
On Tue, 6 Aug 2019 15:53:25 +0800, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai
>
> With the recent addition of commit 75dad2520fc3 ("net: dsa: b53: Disable
> all ports on setup"), users of b53 (BCM53125 on Lamobo R1 in my case)
> are forced to use the dsa subsystem to enable the switch, instead of
> having it in the default transparent "forward-to-all" mode.
>
> The b53 driver does not support mdb bitmap functions. However the dsa
> layer does not check for the existence of the .port_mdb_add callback
> before actually using it. This results in a NULL pointer dereference,
> as shown in the kernel oops below.
>
> The other functions seem to be properly guarded. Do the same for
> .port_mdb_add in dsa_switch_mdb_add_bitmap() as well.
>
> b53 is not the only driver that doesn't support mdb bitmap functions.
> Others include bcm_sf2, dsa_loop, lantiq_gswip, mt7530, mv88e6060,
> qca8k, realtek-smi, and vitesse-vsc73xx.
I don't know what you mean by that, there's no "mdb bitmap function"
support for drivers, only the port_mdb_{prepare,add,del} callbacks...
>
> 8<--- cut here ---
> Unable to handle kernel NULL pointer dereference at virtual address
>
> pgd = (ptrval)
> [] *pgd=
> Internal error: Oops: 8005 [#1] SMP ARM
> Modules linked in: rtl8xxxu rtl8192cu rtl_usb rtl8192c_common rtlwifi
> mac80211 cfg80211
> CPU: 1 PID: 134 Comm: kworker/1:2 Not tainted
> 5.3.0-rc1-00247-gd3519030752a #1
> Hardware name: Allwinner sun7i (A20) Family
> Workqueue: events switchdev_deferred_process_work
> PC is at 0x0
> LR is at dsa_switch_event+0x570/0x620
> pc : [<>]lr : []psr: 80070013
> sp : ee871db8 ip : fp : ee98d0a4
> r10: 000c r9 : 0008 r8 : ee89f710
> r7 : ee98d040 r6 : ee98d088 r5 : c0f04c48 r4 : ee98d04c
> r3 : r2 : ee89f710 r1 : 0008 r0 : ee98d040
> Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
> Control: 10c5387d Table: 6deb406a DAC: 0051
> Process kworker/1:2 (pid: 134, stack limit = 0x(ptrval))
> Stack: (0xee871db8 to 0xee872000)
> 1da0: ee871e14
> 103ace2d
> 1dc0: ee871e14 0005 c08524a0
>
> 1de0: e000 c014bdfc c0f04c48 ee871e98 c0f04c48 ee9e5000 c0851120
> c014bef0
> 1e00: b643aea2 ee9b4068 c08509a8 ee2bf940 ee89f710 ee871ecb
>
> 1e20: 0008 103ace2d c087e248 ee29c868 103ace2d 0001
>
> 1e40: ee871e98 0006 c0fb2a50 c087e2d0
> c08523c4
> 1e60: c014bdfc 0006 c0fad2d0 ee871e98 ee89f710
> c014c500
> 1e80: ee89f3c0 c0f04c48 ee9e5000 c087dfb4 ee9e5000
>
> 1ea0: ee89f710 ee871ecb 0001 103ace2d c0f04c48
> c087e0a8
> 1ec0: efd9a3e0 0089f3c0 103ace2d ee89f700 ee89f710 ee9e5000
> 0122
> 1ee0: 0100 c087e130 ee89f700 c0fad2c8 c1003ef0 c087de4c 2e928000
> c0fad2ec
> 1f00: c0fad2ec ee839580 ef7a62c0 ef7a9400 c087def8 c0fad2ec
> c01447dc
> 1f20: ef315640 ef7a62c0 0008 ee839580 ee839594 ef7a62c0 0008
> c0f03d00
> 1f40: ef7a62d8 ef7a62c0 e000 c0145b84 e000 c0fb2420 c0bfaa8c
>
> 1f60: e000 ee84b600 ee84b5c0 ee87 ee839580 c0145b40
> ef0e5ea4
> 1f80: ee84b61c c014a6f8 0001 ee84b5c0 c014a5b0
>
> 1fa0: c01010e8
>
> 1fc0:
>
> 1fe0: 0013
>
> [] (dsa_switch_event) from []
> (notifier_call_chain+0x48/0x84)
> [] (notifier_call_chain) from []
> (raw_notifier_call_chain+0x18/0x20)
> [] (raw_notifier_call_chain) from []
> (dsa_port_mdb_add+0x48/0x74)
> [] (dsa_port_mdb_add) from []
> (__switchdev_handle_port_obj_add+0x54/0xd4)
> [] (__switchdev_handle_port_obj_add) from []
> (switchdev_handle_port_obj_add+0x8/0x14)
> [] (switchdev_handle_port_obj_add) from []
> (dsa_slave_switchdev_blocking_event+0x94/0xa4)
> [] (dsa_slave_switchdev_blocking_event) from []
> (notifier_call_chain+0x48/0x84)
> [] (notifier_call_chain) from []
> (blocking_notifier_call_chain+0x50/0x68)
> [] (blocking_notifier_call_chain) from []
> (switchdev_port_obj_notify+0x44/0xa8)
> [] (switchdev_port_obj_notify) from []
> (switchdev_port_obj_add_now+0x90/0x104)
> [] (switchdev_port_obj_add_now) from []
> (switchdev_port_obj_add_deferred+0x14/0x5c)
> [] (switchdev_port_obj_add_deferred) from []
> (switchdev_deferred_process+0x64/0x104)
> [] (switchdev_deferred_process) from []
> (switchdev_deferred_process_work+0xc/0x14)
> [] (switchdev_deferred_process_work) from []
Re: [PATCH] net: bridge: Allow bridge to joing multicast groups
Hi Horatiu,
On Thu, 25 Jul 2019 13:44:04 +0200, Horatiu Vultur
wrote:
> There is no way to configure the bridge, to receive only specific link
> layer multicast addresses. From the description of the command 'bridge
> fdb append' is supposed to do that, but there was no way to notify the
> network driver that the bridge joined a group, because LLADDR was added
> to the unicast netdev_hw_addr_list.
>
> Therefore update fdb_add_entry to check if the NLM_F_APPEND flag is set
> and if the source is NULL, which represent the bridge itself. Then add
> address to multicast netdev_hw_addr_list for each bridge interfaces.
> And then the .ndo_set_rx_mode function on the driver is called. To notify
> the driver that the list of multicast mac addresses changed.
>
> Signed-off-by: Horatiu Vultur
> ---
> net/bridge/br_fdb.c | 49 ++---
> 1 file changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index b1d3248..d93746d 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -175,6 +175,29 @@ static void fdb_add_hw_addr(struct net_bridge *br, const
> unsigned char *addr)
> }
> }
>
> +static void fdb_add_hw_maddr(struct net_bridge *br, const unsigned char
> *addr)
> +{
> + int err;
> + struct net_bridge_port *p;
> +
> + ASSERT_RTNL();
> +
> + list_for_each_entry(p, &br->port_list, list) {
> + if (!br_promisc_port(p)) {
> + err = dev_mc_add(p->dev, addr);
> + if (err)
> + goto undo;
> + }
> + }
> +
> + return;
> +undo:
> + list_for_each_entry_continue_reverse(p, &br->port_list, list) {
> + if (!br_promisc_port(p))
> + dev_mc_del(p->dev, addr);
> + }
> +}
> +
> /* When a static FDB entry is deleted, the HW address from that entry is
> * also removed from the bridge private HW address list and updates all
> * the ports with needed information.
> @@ -192,13 +215,27 @@ static void fdb_del_hw_addr(struct net_bridge *br,
> const unsigned char *addr)
> }
> }
>
> +static void fdb_del_hw_maddr(struct net_bridge *br, const unsigned char
> *addr)
> +{
> + struct net_bridge_port *p;
> +
> + ASSERT_RTNL();
> +
> + list_for_each_entry(p, &br->port_list, list) {
> + if (!br_promisc_port(p))
> + dev_mc_del(p->dev, addr);
> + }
> +}
> +
> static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f,
> bool swdev_notify)
> {
> trace_fdb_delete(br, f);
>
> - if (f->is_static)
> + if (f->is_static) {
> fdb_del_hw_addr(br, f->key.addr.addr);
> + fdb_del_hw_maddr(br, f->key.addr.addr);
> + }
>
> hlist_del_init_rcu(&f->fdb_node);
> rhashtable_remove_fast(&br->fdb_hash_tbl, &f->rhnode,
> @@ -843,13 +880,19 @@ static int fdb_add_entry(struct net_bridge *br, struct
> net_bridge_port *source,
> fdb->is_local = 1;
> if (!fdb->is_static) {
> fdb->is_static = 1;
> - fdb_add_hw_addr(br, addr);
> + if (flags & NLM_F_APPEND && !source)
> + fdb_add_hw_maddr(br, addr);
> + else
> + fdb_add_hw_addr(br, addr);
> }
> } else if (state & NUD_NOARP) {
> fdb->is_local = 0;
> if (!fdb->is_static) {
> fdb->is_static = 1;
> - fdb_add_hw_addr(br, addr);
> + if (flags & NLM_F_APPEND && !source)
> + fdb_add_hw_maddr(br, addr);
> + else
> + fdb_add_hw_addr(br, addr);
> }
> } else {
> fdb->is_local = 0;
> --
> 2.7.4
>
I'm a bit late in the conversation. Isn't this what you want?
ip address add dev br0 autojoin
Thanks,
Vivien
[PATCH net-next 2/5] net: dsa: mv88e6xxx: explicit entry passed to vtu_getnext
mv88e6xxx_vtu_getnext interprets two members from the input
mv88e6xxx_vtu_entry structure: the (excluded) vid member to start
the iteration from, and the valid argument specifying whether the VID
must be written or not (only required once at the start of a loop).
Explicit the assignation of these two fields right before calling
mv88e6xxx_vtu_getnext, as it is done in the mv88e6xxx_vtu_get wrapper.
Signed-off-by: Vivien Didelot
---
drivers/net/dsa/mv88e6xxx/chip.c | 21 -
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 1b2cb46d3b53..c825fa3477fa 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1361,9 +1361,7 @@ static int mv88e6xxx_vtu_loadpurge(struct mv88e6xxx_chip
*chip,
static int mv88e6xxx_atu_new(struct mv88e6xxx_chip *chip, u16 *fid)
{
DECLARE_BITMAP(fid_bitmap, MV88E6XXX_N_FID);
- struct mv88e6xxx_vtu_entry vlan = {
- .vid = chip->info->max_vid,
- };
+ struct mv88e6xxx_vtu_entry vlan;
int i, err;
bitmap_zero(fid_bitmap, MV88E6XXX_N_FID);
@@ -1378,6 +1376,9 @@ static int mv88e6xxx_atu_new(struct mv88e6xxx_chip *chip,
u16 *fid)
}
/* Set every FID bit used by the VLAN entries */
+ vlan.vid = chip->info->max_vid;
+ vlan.valid = false;
+
do {
err = mv88e6xxx_vtu_getnext(chip, &vlan);
if (err)
@@ -1441,9 +1442,7 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch
*ds, int port,
u16 vid_begin, u16 vid_end)
{
struct mv88e6xxx_chip *chip = ds->priv;
- struct mv88e6xxx_vtu_entry vlan = {
- .vid = vid_begin - 1,
- };
+ struct mv88e6xxx_vtu_entry vlan;
int i, err;
/* DSA and CPU ports have to be members of multiple vlans */
@@ -1453,6 +1452,9 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch
*ds, int port,
if (!vid_begin)
return -EOPNOTSUPP;
+ vlan.vid = vid_begin - 1;
+ vlan.valid = false;
+
do {
err = mv88e6xxx_vtu_getnext(chip, &vlan);
if (err)
@@ -1789,9 +1791,7 @@ static int mv88e6xxx_port_db_dump_fid(struct
mv88e6xxx_chip *chip,
static int mv88e6xxx_port_db_dump(struct mv88e6xxx_chip *chip, int port,
dsa_fdb_dump_cb_t *cb, void *data)
{
- struct mv88e6xxx_vtu_entry vlan = {
- .vid = chip->info->max_vid,
- };
+ struct mv88e6xxx_vtu_entry vlan;
u16 fid;
int err;
@@ -1805,6 +1805,9 @@ static int mv88e6xxx_port_db_dump(struct mv88e6xxx_chip
*chip, int port,
return err;
/* Dump VLANs' Filtering Information Databases */
+ vlan.vid = chip->info->max_vid;
+ vlan.valid = false;
+
do {
err = mv88e6xxx_vtu_getnext(chip, &vlan);
if (err)
--
2.22.0
[PATCH net-next 5/5] net: dsa: mv88e6xxx: call vtu_getnext directly in vlan_add
Wrapping mv88e6xxx_vtu_getnext makes the code less easy to read and
_mv88e6xxx_port_vlan_add is the only function requiring the preparation
of a new VLAN entry.
To simplify things up, remove the mv88e6xxx_vtu_get wrapper and
explicit the VLAN lookup in _mv88e6xxx_port_vlan_add. This rework
also avoids programming the broadcast entries again when changing a
port's membership, e.g. from tagged to untagged.
At the same time, rename the helper using an old underscore convention.
Signed-off-by: Vivien Didelot
---
drivers/net/dsa/mv88e6xxx/chip.c | 93 +++-
1 file changed, 44 insertions(+), 49 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 50a6dbcc669c..8c4216e7a4bb 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1401,43 +1401,6 @@ static int mv88e6xxx_atu_new(struct mv88e6xxx_chip
*chip, u16 *fid)
return mv88e6xxx_g1_atu_flush(chip, *fid, true);
}
-static int mv88e6xxx_vtu_get(struct mv88e6xxx_chip *chip, u16 vid,
-struct mv88e6xxx_vtu_entry *entry, bool new)
-{
- int err;
-
- if (!vid)
- return -EOPNOTSUPP;
-
- entry->vid = vid - 1;
- entry->valid = false;
-
- err = mv88e6xxx_vtu_getnext(chip, entry);
- if (err)
- return err;
-
- if (entry->vid == vid && entry->valid)
- return 0;
-
- if (new) {
- int i;
-
- /* Initialize a fresh VLAN entry */
- memset(entry, 0, sizeof(*entry));
- entry->vid = vid;
-
- /* Exclude all ports */
- for (i = 0; i < mv88e6xxx_num_ports(chip); ++i)
- entry->member[i] =
- MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_NON_MEMBER;
-
- return mv88e6xxx_atu_new(chip, &entry->fid);
- }
-
- /* switchdev expects -EOPNOTSUPP to honor software VLANs */
- return -EOPNOTSUPP;
-}
-
static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
u16 vid_begin, u16 vid_end)
{
@@ -1616,26 +1579,58 @@ static int mv88e6xxx_broadcast_setup(struct
mv88e6xxx_chip *chip, u16 vid)
return 0;
}
-static int _mv88e6xxx_port_vlan_add(struct mv88e6xxx_chip *chip, int port,
+static int mv88e6xxx_port_vlan_join(struct mv88e6xxx_chip *chip, int port,
u16 vid, u8 member)
{
+ const u8 non_member = MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_NON_MEMBER;
struct mv88e6xxx_vtu_entry vlan;
- int err;
+ int i, err;
- err = mv88e6xxx_vtu_get(chip, vid, &vlan, true);
+ if (!vid)
+ return -EOPNOTSUPP;
+
+ vlan.vid = vid - 1;
+ vlan.valid = false;
+
+ err = mv88e6xxx_vtu_getnext(chip, &vlan);
if (err)
return err;
- if (vlan.valid && vlan.member[port] == member)
- return 0;
- vlan.valid = true;
- vlan.member[port] = member;
+ if (vlan.vid != vid || !vlan.valid) {
+ memset(&vlan, 0, sizeof(vlan));
- err = mv88e6xxx_vtu_loadpurge(chip, &vlan);
- if (err)
- return err;
+ err = mv88e6xxx_atu_new(chip, &vlan.fid);
+ if (err)
+ return err;
- return mv88e6xxx_broadcast_setup(chip, vid);
+ for (i = 0; i < mv88e6xxx_num_ports(chip); ++i)
+ if (i == port)
+ vlan.member[i] = member;
+ else
+ vlan.member[i] = non_member;
+
+ vlan.vid = vid;
+ vlan.valid = true;
+
+ err = mv88e6xxx_vtu_loadpurge(chip, &vlan);
+ if (err)
+ return err;
+
+ err = mv88e6xxx_broadcast_setup(chip, vlan.vid);
+ if (err)
+ return err;
+ } else if (vlan.member[port] != member) {
+ vlan.member[port] = member;
+
+ err = mv88e6xxx_vtu_loadpurge(chip, &vlan);
+ if (err)
+ return err;
+ } else {
+ dev_info(chip->dev, "p%d: already a member of VLAN %d\n",
+port, vid);
+ }
+
+ return 0;
}
static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
@@ -1660,7 +1655,7 @@ static void mv88e6xxx_port_vlan_add(struct dsa_switch
*ds, int port,
mv88e6xxx_reg_lock(chip);
for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid)
- if (_mv88e6xxx_port_vlan_add(chip, port, vid, member))
+ if (mv88e6xxx_port_vlan_join(chip, port, vid, member))
dev_err(ds->dev, "p%d: failed to add VLAN %d%c\n", port,
vid, untagged ? 'u' : 't');
--
2.22.0
[PATCH net-next 4/5] net: dsa: mv88e6xxx: call vtu_getnext directly in vlan_del
Wrapping mv88e6xxx_vtu_getnext makes the code less easy to read.
Explicit the call to mv88e6xxx_vtu_getnext in _mv88e6xxx_port_vlan_del
and the return value expected by switchdev in case of software VLANs.
At the same time, rename the helper using an old underscore convention.
Signed-off-by: Vivien Didelot
---
drivers/net/dsa/mv88e6xxx/chip.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 42ab57dbc790..50a6dbcc669c 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1671,18 +1671,27 @@ static void mv88e6xxx_port_vlan_add(struct dsa_switch
*ds, int port,
mv88e6xxx_reg_unlock(chip);
}
-static int _mv88e6xxx_port_vlan_del(struct mv88e6xxx_chip *chip,
- int port, u16 vid)
+static int mv88e6xxx_port_vlan_leave(struct mv88e6xxx_chip *chip,
+int port, u16 vid)
{
struct mv88e6xxx_vtu_entry vlan;
int i, err;
- err = mv88e6xxx_vtu_get(chip, vid, &vlan, false);
+ if (!vid)
+ return -EOPNOTSUPP;
+
+ vlan.vid = vid - 1;
+ vlan.valid = false;
+
+ err = mv88e6xxx_vtu_getnext(chip, &vlan);
if (err)
return err;
- /* Tell switchdev if this VLAN is handled in software */
- if (vlan.member[port] == MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_NON_MEMBER)
+ /* If the VLAN doesn't exist in hardware or the port isn't a member,
+* tell switchdev that this VLAN is likely handled in software.
+*/
+ if (vlan.vid != vid || !vlan.valid ||
+ vlan.member[port] == MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_NON_MEMBER)
return -EOPNOTSUPP;
vlan.member[port] = MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_NON_MEMBER;
@@ -1721,7 +1730,7 @@ static int mv88e6xxx_port_vlan_del(struct dsa_switch *ds,
int port,
goto unlock;
for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid) {
- err = _mv88e6xxx_port_vlan_del(chip, port, vid);
+ err = mv88e6xxx_port_vlan_leave(chip, port, vid);
if (err)
goto unlock;
--
2.22.0
[PATCH net-next 3/5] net: dsa: mv88e6xxx: call vtu_getnext directly in db load/purge
mv88e6xxx_vtu_getnext is simple enough to call it directly in the
mv88e6xxx_port_db_load_purge function and explicit the return code
expected by switchdev for software VLANs when an hardware VLAN does
not exist.
Signed-off-by: Vivien Didelot
---
drivers/net/dsa/mv88e6xxx/chip.c | 31 ++-
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index c825fa3477fa..42ab57dbc790 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1540,23 +1540,36 @@ static int mv88e6xxx_port_db_load_purge(struct
mv88e6xxx_chip *chip, int port,
const unsigned char *addr, u16 vid,
u8 state)
{
- struct mv88e6xxx_vtu_entry vlan;
struct mv88e6xxx_atu_entry entry;
+ struct mv88e6xxx_vtu_entry vlan;
+ u16 fid;
int err;
/* Null VLAN ID corresponds to the port private database */
- if (vid == 0)
- err = mv88e6xxx_port_get_fid(chip, port, &vlan.fid);
- else
- err = mv88e6xxx_vtu_get(chip, vid, &vlan, false);
- if (err)
- return err;
+ if (vid == 0) {
+ err = mv88e6xxx_port_get_fid(chip, port, &fid);
+ if (err)
+ return err;
+ } else {
+ vlan.vid = vid - 1;
+ vlan.valid = false;
+
+ err = mv88e6xxx_vtu_getnext(chip, &vlan);
+ if (err)
+ return err;
+
+ /* switchdev expects -EOPNOTSUPP to honor software VLANs */
+ if (vlan.vid != vid || !vlan.valid)
+ return -EOPNOTSUPP;
+
+ fid = vlan.fid;
+ }
entry.state = MV88E6XXX_G1_ATU_DATA_STATE_UNUSED;
ether_addr_copy(entry.mac, addr);
eth_addr_dec(entry.mac);
- err = mv88e6xxx_g1_atu_getnext(chip, vlan.fid, &entry);
+ err = mv88e6xxx_g1_atu_getnext(chip, fid, &entry);
if (err)
return err;
@@ -1577,7 +1590,7 @@ static int mv88e6xxx_port_db_load_purge(struct
mv88e6xxx_chip *chip, int port,
entry.state = state;
}
- return mv88e6xxx_g1_atu_loadpurge(chip, vlan.fid, &entry);
+ return mv88e6xxx_g1_atu_loadpurge(chip, fid, &entry);
}
static int mv88e6xxx_port_add_broadcast(struct mv88e6xxx_chip *chip, int port,
--
2.22.0
[PATCH net-next 1/5] net: dsa: mv88e6xxx: lock mutex in vlan_prepare
Lock the mutex in the mv88e6xxx_port_vlan_prepare function
called by the DSA stack, instead of doing it in the internal
mv88e6xxx_port_check_hw_vlan helper.
Signed-off-by: Vivien Didelot
---
drivers/net/dsa/mv88e6xxx/chip.c | 18 ++
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 2e500428670c..1b2cb46d3b53 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1453,12 +1453,10 @@ static int mv88e6xxx_port_check_hw_vlan(struct
dsa_switch *ds, int port,
if (!vid_begin)
return -EOPNOTSUPP;
- mv88e6xxx_reg_lock(chip);
-
do {
err = mv88e6xxx_vtu_getnext(chip, &vlan);
if (err)
- goto unlock;
+ return err;
if (!vlan.valid)
break;
@@ -1487,15 +1485,11 @@ static int mv88e6xxx_port_check_hw_vlan(struct
dsa_switch *ds, int port,
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));
- err = -EOPNOTSUPP;
- goto unlock;
+ return -EOPNOTSUPP;
}
} while (vlan.vid < vid_end);
-unlock:
- mv88e6xxx_reg_unlock(chip);
-
- return err;
+ return 0;
}
static int mv88e6xxx_port_vlan_filtering(struct dsa_switch *ds, int port,
@@ -1529,15 +1523,15 @@ mv88e6xxx_port_vlan_prepare(struct dsa_switch *ds, int
port,
/* If the requested port doesn't belong to the same bridge as the VLAN
* members, do not support it (yet) and fallback to software VLAN.
*/
+ mv88e6xxx_reg_lock(chip);
err = mv88e6xxx_port_check_hw_vlan(ds, port, vlan->vid_begin,
vlan->vid_end);
- if (err)
- return err;
+ mv88e6xxx_reg_unlock(chip);
/* We don't need any dynamic resource from the kernel (yet),
* so skip the prepare phase.
*/
- return 0;
+ return err;
}
static int mv88e6xxx_port_db_load_purge(struct mv88e6xxx_chip *chip, int port,
--
2.22.0
[PATCH net-next 0/5] net: dsa: mv88e6xxx: avoid some redundant VTU operations
The mv88e6xxx driver currently uses a mv88e6xxx_vtu_get wrapper to get a single entry and uses a boolean to eventually initialize a fresh one. However the fresh entry is only needed in one place and mv88e6xxx_vtu_getnext is simple enough to call it directly. Doing so makes the code easier to read, especially for the return code expected by switchdev to honor software VLANs. In addition to not loading the VTU again when an entry is already correctly programmed, this also allows to avoid programming the broadcast entries again when updating a port's membership, from e.g. tagged to untagged. This patch series removes the mv88e6xxx_vtu_get wrapper in favor of direct calls to mv88e6xxx_vtu_getnext, and also renames the _mv88e6xxx_port_vlan_add and _mv88e6xxx_port_vlan_del helpers using an old underscore prefix convention. In case the port's membership is already correctly programmed in hardware, the following debug message may be printed: [ 745.989884] mv88e6085 2188000.ethernet-1:00: p4: already a member of VLAN 42 Vivien Didelot (5): net: dsa: mv88e6xxx: lock mutex in vlan_prepare net: dsa: mv88e6xxx: explicit entry passed to vtu_getnext net: dsa: mv88e6xxx: call vtu_getnext directly in db load/purge net: dsa: mv88e6xxx: call vtu_getnext directly in vlan_del net: dsa: mv88e6xxx: call vtu_getnext directly in vlan_add drivers/net/dsa/mv88e6xxx/chip.c | 182 +-- 1 file changed, 98 insertions(+), 84 deletions(-) -- 2.22.0
Re: [PATCH net-next] net: dsa: mv88e6xxx: avoid some redundant vtu load/purge operations
Hi Rasmus, On Mon, 22 Jul 2019 23:37:26 +, Rasmus Villemoes wrote: > We have an ERPS (Ethernet Ring Protection Switching) setup involving > mv88e6250 switches which we're in the process of switching to a BSP > based on the mainline driver. Breaking any link in the ring works as > expected, with the ring reconfiguring itself quickly and traffic > continuing with almost no noticable drops. However, when plugging back > the cable, we see 5+ second stalls. > > This has been tracked down to the userspace application in charge of > the protocol missing a few CCM messages on the good link (the one that > was not unplugged), causing it to broadcast a "signal fail". That > message eventually reaches its link partner, which responds by > blocking the port. Meanwhile, the first node has continued to block > the port with the just plugged-in cable, breaking the network. And the > reason for those missing CCM messages has in turn been tracked down to > the VTU apparently being too busy servicing load/purge operations that > the normal lookups are delayed. > > Initial state, the link between C and D is blocked in software. > > _ > / \ >| | >A - B - C * D > > Unplug the cable between C and D. > > _ > / \ >| | >A - B - C * * D > > Reestablish the link between C and D. > _ > / \ >| | >A - B - C * D > > Somehow, enough VTU/ATU operations happen inside C that prevents > the application from receving the CCM messages from B in a timely > manner, so a Signal Fail message is sent by C. When B receives > that, it responds by blocking its port. > > _ > / \ >| | >A - B *---* C * D > > Very shortly after this, the signal fail condition clears on the > BC link (some CCM messages finally make it through), so C > unblocks the port. However, a guard timer inside B prevents it > from removing the blocking before 5 seconds have elapsed. > > It is not unlikely that our userspace ERPS implementation could be > smarter and/or is simply buggy. However, this patch fixes the symptoms > we see, and is a small optimization that should not break anything > (knock wood). The idea is simply to avoid doing an VTU load of an > entry identical to the one already present. To do that, we need to > know whether mv88e6xxx_vtu_get() actually found an existing entry, or > has just prepared a struct mv88e6xxx_vtu_entry for us to load. To that > end, let vlan->valid be an output parameter. The other two callers of > mv88e6xxx_vtu_get() are not affected by this patch since they pass > new=false. > > Signed-off-by: Rasmus Villemoes > --- > drivers/net/dsa/mv88e6xxx/chip.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c > b/drivers/net/dsa/mv88e6xxx/chip.c > index 6b17cd961d06..2e500428670c 100644 > --- a/drivers/net/dsa/mv88e6xxx/chip.c > +++ b/drivers/net/dsa/mv88e6xxx/chip.c > @@ -1423,7 +1423,6 @@ static int mv88e6xxx_vtu_get(struct mv88e6xxx_chip > *chip, u16 vid, > > /* Initialize a fresh VLAN entry */ > memset(entry, 0, sizeof(*entry)); > - entry->valid = true; > entry->vid = vid; > > /* Exclude all ports */ > @@ -1618,6 +1617,9 @@ static int _mv88e6xxx_port_vlan_add(struct > mv88e6xxx_chip *chip, int port, > if (err) > return err; > > + if (vlan.valid && vlan.member[port] == member) > + return 0; > + vlan.valid = true; > vlan.member[port] = member; > > err = mv88e6xxx_vtu_loadpurge(chip, &vlan); I'd prefer not to use the mv88e6xxx_vtu_entry structure for output parameters, this can be confusing. As you correctly mentioned, this initialization is only done for _mv88e6xxx_port_vlan_add, so I'll prepare a patch which gets rid of the boolean parameter and move that logic up. Thanks, Vivien
Re: [PATCH net-next] net: dsa: mv88e6xxx: introduce helpers for handling chip->reg_lock
On Thu, 20 Jun 2019 13:50:42 +, Rasmus Villemoes wrote: > This is a no-op that simply moves all locking and unlocking of > ->reg_lock into trivial helpers. I did that to be able to easily add > some ad hoc instrumentation to those helpers to get some information > on contention and hold times of the mutex. Perhaps others want to do > something similar at some point, so this frees them from doing the > 'sed -i' yoga, and have a much smaller 'git diff' while fiddling. > > Signed-off-by: Rasmus Villemoes Reviewed-by: Vivien Didelot
[PATCH net-next v2 2/4] net: dsa: make cpu_dp non const
A port may trigger operations on its dedicated CPU port, so using
cpu_dp as const will raise warnings. Make cpu_dp non const.
Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
---
include/net/dsa.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 82a2baa2dc48..1e8650fa8acc 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -181,7 +181,7 @@ struct dsa_port {
struct dsa_switch *ds;
unsigned intindex;
const char *name;
- const struct dsa_port *cpu_dp;
+ struct dsa_port *cpu_dp;
const char *mac;
struct device_node *dn;
unsigned intageing_time;
--
2.21.0
[PATCH net-next v2 0/4] net: dsa: use switchdev attr and obj handlers
This series reduces boilerplate in the handling of switchdev attribute and object operations by using the switchdev_handle_* helpers, which check the targeted devices and recurse into their lower devices. This also brings back the ability to inspect operations targeting the bridge device itself (where .orig_dev and .dev were originally the bridge device), even though that is of no use yet and skipped by this series. Changes in v2: Only VLAN and (non-host) MDB objects not directly targeting the slave device are unsupported at the moment, so only skip these cases. Vivien Didelot (4): net: dsa: do not check orig_dev in vlan del net: dsa: make cpu_dp non const net: dsa: make dsa_slave_dev_check use const net: dsa: use switchdev handle helpers include/net/dsa.h | 2 +- net/dsa/port.c| 9 -- net/dsa/slave.c | 80 --- 3 files changed, 35 insertions(+), 56 deletions(-) -- 2.21.0
[PATCH net-next v2 3/4] net: dsa: make dsa_slave_dev_check use const
The switchdev handle helpers make use of a device checking helper
requiring a const net_device. Make dsa_slave_dev_check compliant
to this.
Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
---
net/dsa/slave.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 289a6aa4b51c..cb436a05c9a8 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -22,7 +22,7 @@
#include "dsa_priv.h"
-static bool dsa_slave_dev_check(struct net_device *dev);
+static bool dsa_slave_dev_check(const struct net_device *dev);
/* slave mii_bus handling ***/
static int dsa_slave_phy_read(struct mii_bus *bus, int addr, int reg)
@@ -1408,7 +1408,7 @@ void dsa_slave_destroy(struct net_device *slave_dev)
free_netdev(slave_dev);
}
-static bool dsa_slave_dev_check(struct net_device *dev)
+static bool dsa_slave_dev_check(const struct net_device *dev)
{
return dev->netdev_ops == &dsa_slave_netdev_ops;
}
--
2.21.0
[PATCH net-next v2 1/4] net: dsa: do not check orig_dev in vlan del
The current DSA code handling switchdev objects does not recurse into the lower devices thus is never called with an orig_dev member being a bridge device, hence remove this useless check. At the same time, remove the comments about the callers, which is unlikely to be updated if the code changes and thus will be confusing. Signed-off-by: Vivien Didelot Reviewed-by: Florian Fainelli --- net/dsa/port.c | 9 - 1 file changed, 9 deletions(-) diff --git a/net/dsa/port.c b/net/dsa/port.c index 585b6b9a9433..d2b65e8dc60c 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -336,9 +336,6 @@ int dsa_port_vlan_add(struct dsa_port *dp, .vlan = vlan, }; - /* Can be called from dsa_slave_port_obj_add() or -* dsa_slave_vlan_rx_add_vid() -*/ if (!dp->bridge_dev || br_vlan_enabled(dp->bridge_dev)) return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info); @@ -354,12 +351,6 @@ int dsa_port_vlan_del(struct dsa_port *dp, .vlan = vlan, }; - if (vlan->obj.orig_dev && netif_is_bridge_master(vlan->obj.orig_dev)) - return -EOPNOTSUPP; - - /* Can be called from dsa_slave_port_obj_del() or -* dsa_slave_vlan_rx_kill_vid() -*/ if (!dp->bridge_dev || br_vlan_enabled(dp->bridge_dev)) return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, &info); -- 2.21.0
[PATCH net-next v2 4/4] net: dsa: use switchdev handle helpers
Get rid of the dsa_slave_switchdev_port_{attr_set,obj}_event functions
in favor of the switchdev_handle_port_{attr_set,obj_add,obj_del}
helpers which recurse into the lower devices of the target interface.
This has the benefit of being aware of the operations made on the
bridge device itself, where orig_dev is the bridge, and dev is the
slave. This can be used later to configure the hardware switches.
Only VLAN and (port) MDB objects not directly targeting the slave
device are unsupported at the moment, so skip this case in their
respective case statements.
Signed-off-by: Vivien Didelot
---
net/dsa/slave.c | 76 +
1 file changed, 32 insertions(+), 44 deletions(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index cb436a05c9a8..99673f6b07f6 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -311,7 +311,8 @@ static int dsa_slave_port_attr_set(struct net_device *dev,
static int dsa_slave_port_obj_add(struct net_device *dev,
const struct switchdev_obj *obj,
- struct switchdev_trans *trans)
+ struct switchdev_trans *trans,
+ struct netlink_ext_ack *extack)
{
struct dsa_port *dp = dsa_slave_to_port(dev);
int err;
@@ -323,6 +324,8 @@ static int dsa_slave_port_obj_add(struct net_device *dev,
switch (obj->id) {
case SWITCHDEV_OBJ_ID_PORT_MDB:
+ if (obj->orig_dev != dev)
+ return -EOPNOTSUPP;
err = dsa_port_mdb_add(dp, SWITCHDEV_OBJ_PORT_MDB(obj), trans);
break;
case SWITCHDEV_OBJ_ID_HOST_MDB:
@@ -333,6 +336,8 @@ static int dsa_slave_port_obj_add(struct net_device *dev,
trans);
break;
case SWITCHDEV_OBJ_ID_PORT_VLAN:
+ if (obj->orig_dev != dev)
+ return -EOPNOTSUPP;
err = dsa_port_vlan_add(dp, SWITCHDEV_OBJ_PORT_VLAN(obj),
trans);
break;
@@ -352,6 +357,8 @@ static int dsa_slave_port_obj_del(struct net_device *dev,
switch (obj->id) {
case SWITCHDEV_OBJ_ID_PORT_MDB:
+ if (obj->orig_dev != dev)
+ return -EOPNOTSUPP;
err = dsa_port_mdb_del(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
break;
case SWITCHDEV_OBJ_ID_HOST_MDB:
@@ -361,6 +368,8 @@ static int dsa_slave_port_obj_del(struct net_device *dev,
err = dsa_port_mdb_del(dp->cpu_dp, SWITCHDEV_OBJ_PORT_MDB(obj));
break;
case SWITCHDEV_OBJ_ID_PORT_VLAN:
+ if (obj->orig_dev != dev)
+ return -EOPNOTSUPP;
err = dsa_port_vlan_del(dp, SWITCHDEV_OBJ_PORT_VLAN(obj));
break;
default:
@@ -1479,19 +1488,6 @@ static int dsa_slave_netdevice_event(struct
notifier_block *nb,
return NOTIFY_DONE;
}
-static int
-dsa_slave_switchdev_port_attr_set_event(struct net_device *netdev,
- struct switchdev_notifier_port_attr_info *port_attr_info)
-{
- int err;
-
- err = dsa_slave_port_attr_set(netdev, port_attr_info->attr,
- port_attr_info->trans);
-
- port_attr_info->handled = true;
- return notifier_from_errno(err);
-}
-
struct dsa_switchdev_event_work {
struct work_struct work;
struct switchdev_notifier_fdb_info fdb_info;
@@ -1566,13 +1562,18 @@ static int dsa_slave_switchdev_event(struct
notifier_block *unused,
{
struct net_device *dev = switchdev_notifier_info_to_dev(ptr);
struct dsa_switchdev_event_work *switchdev_work;
+ int err;
+
+ if (event == SWITCHDEV_PORT_ATTR_SET) {
+ err = switchdev_handle_port_attr_set(dev, ptr,
+dsa_slave_dev_check,
+dsa_slave_port_attr_set);
+ return notifier_from_errno(err);
+ }
if (!dsa_slave_dev_check(dev))
return NOTIFY_DONE;
- if (event == SWITCHDEV_PORT_ATTR_SET)
- return dsa_slave_switchdev_port_attr_set_event(dev, ptr);
-
switchdev_work = kzalloc(sizeof(*switchdev_work), GFP_ATOMIC);
if (!switchdev_work)
return NOTIFY_BAD;
@@ -1602,41 +1603,28 @@ static int dsa_slave_switchdev_event(struct
notifier_block *unused,
return NOTIFY_BAD;
}
-static int
-dsa_slave_switchdev_port_obj_event(unsigned long event,
- struct net_device *netdev,
- struct switchdev_notifier_port_obj_info *port_obj_info)
-{
- int err = -EOPNOTSUPP;
-
- switch (event) {
- case SWITCHDEV_PORT_OBJ_ADD:
- err = dsa_slave_port_obj_
Re: [PATCH net-next 0/4] net: dsa: use switchdev attr and obj handlers
Hi David, On Tue, 11 Jun 2019 17:47:43 -0400, Vivien Didelot wrote: > This series reduces boilerplate in the handling of switchdev attribute and > object operations by using the switchdev_handle_* helpers, which check the > targeted devices and recurse into their lower devices. > > This also brings back the ability to inspect operations targeting the bridge > device itself (where .orig_dev is the bridge device and .dev is the slave), > even though that is of no use yet and skipped by this series. > > Vivien Didelot (4): > net: dsa: do not check orig_dev in vlan del > net: dsa: make cpu_dp non const > net: dsa: make dsa_slave_dev_check use const > net: dsa: use switchdev handle helpers > > include/net/dsa.h | 2 +- > net/dsa/port.c| 9 -- > net/dsa/slave.c | 81 --- > 3 files changed, 36 insertions(+), 56 deletions(-) Please do not merge. The orig_dev != dev test in patch 4 is not correct, because it skips the programming of the HOST_MDB object. I'll respin in a few. Thanks, Vivien
[PATCH net-next 2/4] net: dsa: make cpu_dp non const
A port may trigger operations on its dedicated CPU port, so using
cpu_dp as const will raise warnings. Make cpu_dp non const.
Signed-off-by: Vivien Didelot
---
include/net/dsa.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 82a2baa2dc48..1e8650fa8acc 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -181,7 +181,7 @@ struct dsa_port {
struct dsa_switch *ds;
unsigned intindex;
const char *name;
- const struct dsa_port *cpu_dp;
+ struct dsa_port *cpu_dp;
const char *mac;
struct device_node *dn;
unsigned intageing_time;
--
2.21.0
[PATCH net-next 1/4] net: dsa: do not check orig_dev in vlan del
The current DSA code handling switchdev objects does not recurse into the lower devices thus is never called with an orig_dev member being a bridge device, hence remove this useless check. At the same time, remove the comments about the callers, which is unlikely to be updated if the code changes and thus confusing. Signed-off-by: Vivien Didelot --- net/dsa/port.c | 9 - 1 file changed, 9 deletions(-) diff --git a/net/dsa/port.c b/net/dsa/port.c index 70744fec9717..f83756eaf8a5 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -336,9 +336,6 @@ int dsa_port_vlan_add(struct dsa_port *dp, .vlan = vlan, }; - /* Can be called from dsa_slave_port_obj_add() or -* dsa_slave_vlan_rx_add_vid() -*/ if (!dp->bridge_dev || br_vlan_enabled(dp->bridge_dev)) return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info); @@ -354,12 +351,6 @@ int dsa_port_vlan_del(struct dsa_port *dp, .vlan = vlan, }; - if (vlan->obj.orig_dev && netif_is_bridge_master(vlan->obj.orig_dev)) - return -EOPNOTSUPP; - - /* Can be called from dsa_slave_port_obj_del() or -* dsa_slave_vlan_rx_kill_vid() -*/ if (!dp->bridge_dev || br_vlan_enabled(dp->bridge_dev)) return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, &info); -- 2.21.0
[PATCH net-next 0/4] net: dsa: use switchdev attr and obj handlers
This series reduces boilerplate in the handling of switchdev attribute and object operations by using the switchdev_handle_* helpers, which check the targeted devices and recurse into their lower devices. This also brings back the ability to inspect operations targeting the bridge device itself (where .orig_dev is the bridge device and .dev is the slave), even though that is of no use yet and skipped by this series. Vivien Didelot (4): net: dsa: do not check orig_dev in vlan del net: dsa: make cpu_dp non const net: dsa: make dsa_slave_dev_check use const net: dsa: use switchdev handle helpers include/net/dsa.h | 2 +- net/dsa/port.c| 9 -- net/dsa/slave.c | 81 --- 3 files changed, 36 insertions(+), 56 deletions(-) -- 2.21.0
[PATCH net-next 4/4] net: dsa: use switchdev handle helpers
Get rid of the dsa_slave_switchdev_port_{attr_set,obj}_event functions
in favor of the switchdev_handle_port_{attr_set,obj_add,obj_del}
helpers which recurse into the lower devices of the target interface.
This has the benefit of being aware of the operations made on the
bridge device itself, where orig_dev is the bridge, and dev is the
slave. This can be used later to configure bridge-wide attributes on
the hardware switches.
Signed-off-by: Vivien Didelot
---
net/dsa/slave.c | 77 +
1 file changed, 33 insertions(+), 44 deletions(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index cb436a05c9a8..915dd43873f9 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -283,6 +283,9 @@ static int dsa_slave_port_attr_set(struct net_device *dev,
struct dsa_port *dp = dsa_slave_to_port(dev);
int ret;
+ if (attr->orig_dev != dev)
+ return -EOPNOTSUPP;
+
switch (attr->id) {
case SWITCHDEV_ATTR_ID_PORT_STP_STATE:
ret = dsa_port_set_state(dp, attr->u.stp_state, trans);
@@ -311,11 +314,15 @@ static int dsa_slave_port_attr_set(struct net_device *dev,
static int dsa_slave_port_obj_add(struct net_device *dev,
const struct switchdev_obj *obj,
- struct switchdev_trans *trans)
+ struct switchdev_trans *trans,
+ struct netlink_ext_ack *extack)
{
struct dsa_port *dp = dsa_slave_to_port(dev);
int err;
+ if (obj->orig_dev != dev)
+ return -EOPNOTSUPP;
+
/* For the prepare phase, ensure the full set of changes is feasable in
* one go in order to signal a failure properly. If an operation is not
* supported, return -EOPNOTSUPP.
@@ -350,6 +357,9 @@ static int dsa_slave_port_obj_del(struct net_device *dev,
struct dsa_port *dp = dsa_slave_to_port(dev);
int err;
+ if (obj->orig_dev != dev)
+ return -EOPNOTSUPP;
+
switch (obj->id) {
case SWITCHDEV_OBJ_ID_PORT_MDB:
err = dsa_port_mdb_del(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
@@ -1479,19 +1489,6 @@ static int dsa_slave_netdevice_event(struct
notifier_block *nb,
return NOTIFY_DONE;
}
-static int
-dsa_slave_switchdev_port_attr_set_event(struct net_device *netdev,
- struct switchdev_notifier_port_attr_info *port_attr_info)
-{
- int err;
-
- err = dsa_slave_port_attr_set(netdev, port_attr_info->attr,
- port_attr_info->trans);
-
- port_attr_info->handled = true;
- return notifier_from_errno(err);
-}
-
struct dsa_switchdev_event_work {
struct work_struct work;
struct switchdev_notifier_fdb_info fdb_info;
@@ -1566,13 +1563,18 @@ static int dsa_slave_switchdev_event(struct
notifier_block *unused,
{
struct net_device *dev = switchdev_notifier_info_to_dev(ptr);
struct dsa_switchdev_event_work *switchdev_work;
+ int err;
+
+ if (event == SWITCHDEV_PORT_ATTR_SET) {
+ err = switchdev_handle_port_attr_set(dev, ptr,
+dsa_slave_dev_check,
+dsa_slave_port_attr_set);
+ return notifier_from_errno(err);
+ }
if (!dsa_slave_dev_check(dev))
return NOTIFY_DONE;
- if (event == SWITCHDEV_PORT_ATTR_SET)
- return dsa_slave_switchdev_port_attr_set_event(dev, ptr);
-
switchdev_work = kzalloc(sizeof(*switchdev_work), GFP_ATOMIC);
if (!switchdev_work)
return NOTIFY_BAD;
@@ -1602,41 +1604,28 @@ static int dsa_slave_switchdev_event(struct
notifier_block *unused,
return NOTIFY_BAD;
}
-static int
-dsa_slave_switchdev_port_obj_event(unsigned long event,
- struct net_device *netdev,
- struct switchdev_notifier_port_obj_info *port_obj_info)
-{
- int err = -EOPNOTSUPP;
-
- switch (event) {
- case SWITCHDEV_PORT_OBJ_ADD:
- err = dsa_slave_port_obj_add(netdev, port_obj_info->obj,
-port_obj_info->trans);
- break;
- case SWITCHDEV_PORT_OBJ_DEL:
- err = dsa_slave_port_obj_del(netdev, port_obj_info->obj);
- break;
- }
-
- port_obj_info->handled = true;
- return notifier_from_errno(err);
-}
-
static int dsa_slave_switchdev_blocking_event(struct notifier_block *unused,
unsigned long event, void *ptr)
{
struct net_device *dev = switchdev_notifier_info_to_dev(ptr);
-
- if (!dsa_slave_dev_check(dev))
- return NOTIFY_DONE;
+ int err;
switch (event) {
- case SWITCHDEV_PORT_O
[PATCH net-next 3/4] net: dsa: make dsa_slave_dev_check use const
The switchdev handle helpers make use of a device checking helper
requiring a const net_device. Make dsa_slave_dev_check compliant
to this.
Signed-off-by: Vivien Didelot
---
net/dsa/slave.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 289a6aa4b51c..cb436a05c9a8 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -22,7 +22,7 @@
#include "dsa_priv.h"
-static bool dsa_slave_dev_check(struct net_device *dev);
+static bool dsa_slave_dev_check(const struct net_device *dev);
/* slave mii_bus handling ***/
static int dsa_slave_phy_read(struct mii_bus *bus, int addr, int reg)
@@ -1408,7 +1408,7 @@ void dsa_slave_destroy(struct net_device *slave_dev)
free_netdev(slave_dev);
}
-static bool dsa_slave_dev_check(struct net_device *dev)
+static bool dsa_slave_dev_check(const struct net_device *dev)
{
return dev->netdev_ops == &dsa_slave_netdev_ops;
}
--
2.21.0
Re: [PATCH net-next v3 01/10] net: dsa: mv88e6xxx: add mv88e6250_g1_ieee_pri_map
Hi Rasmus,
On Mon, 3 Jun 2019 14:42:12 +, Rasmus Villemoes
wrote:
> Quite a few of the existing supported chips that use
> mv88e6085_g1_ieee_pri_map as ->ieee_pri_map (including, incidentally,
> mv88e6085 itself) actually have a reset value of 0xfa50 in the
> G1_IEEE_PRI register.
>
> The data sheet for the mv88e6095, however, does describe a reset value
> of 0xfa41.
>
> So rather than changing the value in the existing callback, introduce
> a new variant with the 0xfa50 value. That will be used by the upcoming
> mv88e6250, and existing chips can be switched over one by one,
> preferably double-checking both the data sheet and actual hardware in
> each case - if anybody actually feels this is important enough to
> care.
Given your previous thread on this topic, I'd prefer that you include
a first patch which implements mv88e6095_g1_ieee_pri_map() using 0xfa41
and update mv88e{6092,6095}_ops to use it, then a second one which fixes
mv88e6085_g1_ieee_pri_map to use 0xfa50. Then mv88e6250_ops can use it.
Thanks,
Vivien
Re: [PATCH] net: dsa: mv88e6xxx: make mv88e6xxx_g1_stats_wait static
On Mon, 3 Jun 2019 08:04:09 +, Rasmus Villemoes wrote: > mv88e6xxx_g1_stats_wait has no users outside global1.c, so make it > static. > > Signed-off-by: Rasmus Villemoes Reviewed-by: Vivien Didelot
Re: [PATCH] net: dsa: mv88e6xxx: avoid error message on remove from VLAN 0
On Fri, 31 May 2019 10:35:14 +0300, Nikita Yushchenko wrote: > When non-bridged, non-vlan'ed mv88e6xxx port is moving down, error > message is logged: > > failed to kill vid 0081/0 for device eth_cu_1000_4 > > This is caused by call from __vlan_vid_del() with vin set to zero, over > call chain this results into _mv88e6xxx_port_vlan_del() called with > vid=0, and mv88e6xxx_vtu_get() called from there returns -EINVAL. > > On symmetric path moving port up, call goes through > mv88e6xxx_port_vlan_prepare() that calls mv88e6xxx_port_check_hw_vlan() > that returns -EOPNOTSUPP for zero vid. > > This patch changes mv88e6xxx_vtu_get() to also return -EOPNOTSUPP for > zero vid, then this error code is explicitly cleared in > dsa_slave_vlan_rx_kill_vid() and error message is no longer logged. > > Signed-off-by: Nikita Yushchenko Reviewed-by: Vivien Didelot
Re: [PATCH] net: dsa: mv88e6xxx: avoid error message on remove from VLAN 0
Hi Florian,
On Fri, 31 May 2019 09:34:03 -0700, Florian Fainelli
wrote:
> On 5/31/19 7:37 AM, Andrew Lunn wrote:
> >> I'm not sure that I like the semantic of it, because the driver can
> >> actually
> >> support VID 0 per-se, only the kernel does not use VLAN 0. Thus I would
> >> avoid
> >> calling the port_vlan_del() ops for VID 0, directly into the upper DSA
> >> layer.
> >>
> >> Florian, Andrew, wouldn't the following patch be more adequate?
> >>
> >> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> >> index 1e2ae9d59b88..80f228258a92 100644
> >> --- a/net/dsa/slave.c
> >> +++ b/net/dsa/slave.c
> >> @@ -1063,6 +1063,10 @@ static int dsa_slave_vlan_rx_kill_vid(struct
> >> net_device *dev, __be16 proto,
> >> struct bridge_vlan_info info;
> >> int ret;
> >>
> >> + /* VID 0 has a special meaning and is never programmed in
> >> hardware */
> >> + if (!vid)
> >> + return 0;
> >> +
> >> /* Check for a possible bridge VLAN entry now since there is no
> >> * need to emulate the switchdev prepare + commit phase.
> >> */
> >
> > Hi Vivien
> >
> > If we put this in rx_kill_vid, we should probably have something
> > similar in rx_add_vid, just in case the kernel does start using VID 0.
>
> We use the prepare/commit model in the rx_add_vid() path so we deal with
> -EOPNOTSUPP, that was caught fairly early on by Heiner when I added
> programming of VLAN filtering through rx_{add,kill}_vid.
>
> Nikita's patcha s it stands is correct and would make both
> mv88e6xxx_port_check_hw_vlan() and mv88e6xxx_vtu_get() consistent.
OK, I'll double check if I can simplify the management of VID 0 in mv88e6xxx to
match what other switches do. In the meantime, Nikita's approach is consistent.
Thank you,
Vivien
Re: [PATCH] net: dsa: mv88e6xxx: avoid error message on remove from VLAN 0
Hi Florian, On Fri, 31 May 2019 09:36:13 -0700, Florian Fainelli wrote: > > But VID 0 has a special meaning for the kernel, it means the port's private > > database (when it is isolated, non-bridged), it is not meant to be > > programmed > > in the switch. That's why I would've put that knowledge into the DSA layer, > > which job is to translate the kernel operations to the (dumb) DSA drivers. > > > > I hope I'm seeing things correctly here. > > Your first part about the fact that it's the port private database is > true, the fact that it is not programmed into the HW actually depends on > what the switch is capable of doing. With mv88e6xxx you have per-port > VLAN filtering controls, but other switches that do not have that > capability need to program VID == 0 into the HW to continue maintaining > VLAN filtering on a non bridged port while a bridge has enslaved other > ports of the switch. Are you saying that switches without per-port VLAN filtering controls will program VID 0, and thus put all non bridged ports into the same VLAN, allowing them to talk to each other? Thanks, Vivien
Re: [PATCH] net: dsa: mv88e6xxx: avoid error message on remove from VLAN 0
Hi Nikita, On Fri, 31 May 2019 17:46:29 +0300, Nikita Yushchenko wrote: > > > 31.05.2019 17:37, Andrew Lunn wrote: > >> I'm not sure that I like the semantic of it, because the driver can > >> actually > >> support VID 0 per-se, only the kernel does not use VLAN 0. Thus I would > >> avoid > >> calling the port_vlan_del() ops for VID 0, directly into the upper DSA > >> layer. > >> > >> Florian, Andrew, wouldn't the following patch be more adequate? > >> > >> diff --git a/net/dsa/slave.c b/net/dsa/slave.c > >> index 1e2ae9d59b88..80f228258a92 100644 > >> --- a/net/dsa/slave.c > >> +++ b/net/dsa/slave.c > >> @@ -1063,6 +1063,10 @@ static int dsa_slave_vlan_rx_kill_vid(struct > >> net_device *dev, __be16 proto, > >> struct bridge_vlan_info info; > >> int ret; > >> > >> + /* VID 0 has a special meaning and is never programmed in > >> hardware */ > >> + if (!vid) > >> + return 0; > >> + > >> /* Check for a possible bridge VLAN entry now since there is no > >> * need to emulate the switchdev prepare + commit phase. > >> */ > > > Kernel currently does, but it is caught in > mv88e6xxx_port_check_hw_vlan() and returns -ENOTSUPP from there. But VID 0 has a special meaning for the kernel, it means the port's private database (when it is isolated, non-bridged), it is not meant to be programmed in the switch. That's why I would've put that knowledge into the DSA layer, which job is to translate the kernel operations to the (dumb) DSA drivers. I hope I'm seeing things correctly here. Thanks, Vivien
Re: [PATCH net-next] ethtool: do not use regs->len after ops->get_regs
Hi Michal,
On Fri, 31 May 2019 16:27:50 +0200, Michal Kubecek wrote:
> On Fri, May 31, 2019 at 10:15:01AM -0400, Vivien Didelot wrote:
> > Hi Michal,
> >
> > On Fri, 31 May 2019 08:54:32 +0200, Michal Kubecek wrote:
> > > On Thu, May 30, 2019 at 07:54:50PM -0400, Vivien Didelot wrote:
> > > > The kernel allocates a buffer of size ops->get_regs_len(), and pass
> > > > it to the kernel driver via ops->get_regs() for filling.
> > > >
> > > > There is no restriction about what the kernel drivers can or cannot
> > > > do with the regs->len member. Drivers usually ignore it or set
> > > > the same size again. However, ethtool_get_regs() must not use this
> > > > value when copying the buffer back to the user, because userspace may
> > > > have allocated a smaller buffer. For instance ethtool does that when
> > > > dumping the raw registers directly into a fixed-size file.
> > > >
> > > > Software may still make use of the regs->len value updated by the
> > > > kernel driver, but ethtool_get_regs() must use the original regs->len
> > > > given by userspace, up to ops->get_regs_len(), when copying the buffer.
> > > >
> > > > Also no need to check regbuf twice.
> > > >
> > > > Signed-off-by: Vivien Didelot
> > > > ---
> > > > net/core/ethtool.c | 4 +++-
> > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> > > > index 4a593853cbf2..8f95c7b7cafe 100644
> > > > --- a/net/core/ethtool.c
> > > > +++ b/net/core/ethtool.c
> > > > @@ -1338,38 +1338,40 @@ static noinline_for_stack int
> > > > ethtool_set_rxfh(struct net_device *dev,
> > > > static int ethtool_get_regs(struct net_device *dev, char __user
> > > > *useraddr)
> > > > {
> > > > struct ethtool_regs regs;
> > > > const struct ethtool_ops *ops = dev->ethtool_ops;
> > > > void *regbuf;
> > > > int reglen, ret;
> > > >
> > > > if (!ops->get_regs || !ops->get_regs_len)
> > > > return -EOPNOTSUPP;
> > > >
> > > > if (copy_from_user(®s, useraddr, sizeof(regs)))
> > > > return -EFAULT;
> > > >
> > > > reglen = ops->get_regs_len(dev);
> > > > if (reglen <= 0)
> > > > return reglen;
> > > >
> > > > if (regs.len > reglen)
> > > > regs.len = reglen;
> > > > + else
> > > > + reglen = regs.len;
> > >
> > > This seems wrong. Most drivers do not check regs.len in their get_regs()
> > > handler (I'm not sure if there are any that do) and simply write as much
> > > data as they have. Thus if userspace passes too short regs.len, this
> > > would replace overflow of a userspace buffer for few drivers by overflow
> > > of a kernel buffer for (almost) all drivers.
> > >
> > > So while we should use the original regs.len from userspace for final
> > > copy_to_user(), we have to allocate the buffer for driver ->get_regs()
> > > callback with size returned by its ->get_regs_len() callback.
> >
> > Either I've completely screwed my patch, or you have misread it. This patch
> > actually just stores the original value of regs.len passed by userspace to
> > the kernel into reglen, before calling ops->get_regs().
> >
> > But the kernel still allocates ops->get_regs_len() and passes that to the
> > kernel drivers, as this is the only size drivers must care about.
> >
> > Then the kernel only copies what the userspace (originally) requested,
> > up to ops->get_regs_len().
> >
> > In other words, if userspace requested a bigger buffer only
> > ops->get_regs_len()
> > get copied, if the userspace requested a smaller buffer only that length
> > get copied.
>
> The problem with this patch is not with what gets copied to userspace
> but with the buffer allocated for driver callback. With this patch, the
> code looks like this:
>
> reglen = ops->get_regs_len(dev);
> if (reglen <= 0)
> return reglen;
>
> Here we get actual dump size from driver and put it into reglen.
>
> if (regs.len > reglen)
> regs.l
Re: [PATCH] net: dsa: mv88e6xxx: avoid error message on remove from VLAN 0
Hi Nikita, On Fri, 31 May 2019 10:35:14 +0300, Nikita Yushchenko wrote: > When non-bridged, non-vlan'ed mv88e6xxx port is moving down, error > message is logged: > > failed to kill vid 0081/0 for device eth_cu_1000_4 > > This is caused by call from __vlan_vid_del() with vin set to zero, over > call chain this results into _mv88e6xxx_port_vlan_del() called with > vid=0, and mv88e6xxx_vtu_get() called from there returns -EINVAL. > > On symmetric path moving port up, call goes through > mv88e6xxx_port_vlan_prepare() that calls mv88e6xxx_port_check_hw_vlan() > that returns -EOPNOTSUPP for zero vid. > > This patch changes mv88e6xxx_vtu_get() to also return -EOPNOTSUPP for > zero vid, then this error code is explicitly cleared in > dsa_slave_vlan_rx_kill_vid() and error message is no longer logged. > > Signed-off-by: Nikita Yushchenko > --- > drivers/net/dsa/mv88e6xxx/chip.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c > b/drivers/net/dsa/mv88e6xxx/chip.c > index 28414db979b0..6b77fde5f0e4 100644 > --- a/drivers/net/dsa/mv88e6xxx/chip.c > +++ b/drivers/net/dsa/mv88e6xxx/chip.c > @@ -1392,7 +1392,7 @@ static int mv88e6xxx_vtu_get(struct mv88e6xxx_chip > *chip, u16 vid, > int err; > > if (!vid) > - return -EINVAL; > + return -EOPNOTSUPP; > > entry->vid = vid - 1; > entry->valid = false; I'm not sure that I like the semantic of it, because the driver can actually support VID 0 per-se, only the kernel does not use VLAN 0. Thus I would avoid calling the port_vlan_del() ops for VID 0, directly into the upper DSA layer. Florian, Andrew, wouldn't the following patch be more adequate? diff --git a/net/dsa/slave.c b/net/dsa/slave.c index 1e2ae9d59b88..80f228258a92 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -1063,6 +1063,10 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, struct bridge_vlan_info info; int ret; + /* VID 0 has a special meaning and is never programmed in hardware */ + if (!vid) + return 0; + /* Check for a possible bridge VLAN entry now since there is no * need to emulate the switchdev prepare + commit phase. */ Thanks, Vivien
Re: [PATCH net-next] ethtool: do not use regs->len after ops->get_regs
Hi Michal,
On Fri, 31 May 2019 08:54:32 +0200, Michal Kubecek wrote:
> On Thu, May 30, 2019 at 07:54:50PM -0400, Vivien Didelot wrote:
> > The kernel allocates a buffer of size ops->get_regs_len(), and pass
> > it to the kernel driver via ops->get_regs() for filling.
> >
> > There is no restriction about what the kernel drivers can or cannot
> > do with the regs->len member. Drivers usually ignore it or set
> > the same size again. However, ethtool_get_regs() must not use this
> > value when copying the buffer back to the user, because userspace may
> > have allocated a smaller buffer. For instance ethtool does that when
> > dumping the raw registers directly into a fixed-size file.
> >
> > Software may still make use of the regs->len value updated by the
> > kernel driver, but ethtool_get_regs() must use the original regs->len
> > given by userspace, up to ops->get_regs_len(), when copying the buffer.
> >
> > Also no need to check regbuf twice.
> >
> > Signed-off-by: Vivien Didelot
> > ---
> > net/core/ethtool.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> > index 4a593853cbf2..8f95c7b7cafe 100644
> > --- a/net/core/ethtool.c
> > +++ b/net/core/ethtool.c
> > @@ -1338,38 +1338,40 @@ static noinline_for_stack int
> > ethtool_set_rxfh(struct net_device *dev,
> > static int ethtool_get_regs(struct net_device *dev, char __user *useraddr)
> > {
> > struct ethtool_regs regs;
> > const struct ethtool_ops *ops = dev->ethtool_ops;
> > void *regbuf;
> > int reglen, ret;
> >
> > if (!ops->get_regs || !ops->get_regs_len)
> > return -EOPNOTSUPP;
> >
> > if (copy_from_user(®s, useraddr, sizeof(regs)))
> > return -EFAULT;
> >
> > reglen = ops->get_regs_len(dev);
> > if (reglen <= 0)
> > return reglen;
> >
> > if (regs.len > reglen)
> > regs.len = reglen;
> > + else
> > + reglen = regs.len;
>
> This seems wrong. Most drivers do not check regs.len in their get_regs()
> handler (I'm not sure if there are any that do) and simply write as much
> data as they have. Thus if userspace passes too short regs.len, this
> would replace overflow of a userspace buffer for few drivers by overflow
> of a kernel buffer for (almost) all drivers.
>
> So while we should use the original regs.len from userspace for final
> copy_to_user(), we have to allocate the buffer for driver ->get_regs()
> callback with size returned by its ->get_regs_len() callback.
Either I've completely screwed my patch, or you have misread it. This patch
actually just stores the original value of regs.len passed by userspace to
the kernel into reglen, before calling ops->get_regs().
But the kernel still allocates ops->get_regs_len() and passes that to the
kernel drivers, as this is the only size drivers must care about.
Then the kernel only copies what the userspace (originally) requested,
up to ops->get_regs_len().
In other words, if userspace requested a bigger buffer only ops->get_regs_len()
get copied, if the userspace requested a smaller buffer only that length
get copied.
Thanks,
Vivien
[PATCH net-next] ethtool: do not use regs->len after ops->get_regs
The kernel allocates a buffer of size ops->get_regs_len(), and pass
it to the kernel driver via ops->get_regs() for filling.
There is no restriction about what the kernel drivers can or cannot
do with the regs->len member. Drivers usually ignore it or set
the same size again. However, ethtool_get_regs() must not use this
value when copying the buffer back to the user, because userspace may
have allocated a smaller buffer. For instance ethtool does that when
dumping the raw registers directly into a fixed-size file.
Software may still make use of the regs->len value updated by the
kernel driver, but ethtool_get_regs() must use the original regs->len
given by userspace, up to ops->get_regs_len(), when copying the buffer.
Also no need to check regbuf twice.
Signed-off-by: Vivien Didelot
---
net/core/ethtool.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 4a593853cbf2..8f95c7b7cafe 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1338,38 +1338,40 @@ static noinline_for_stack int ethtool_set_rxfh(struct
net_device *dev,
static int ethtool_get_regs(struct net_device *dev, char __user *useraddr)
{
struct ethtool_regs regs;
const struct ethtool_ops *ops = dev->ethtool_ops;
void *regbuf;
int reglen, ret;
if (!ops->get_regs || !ops->get_regs_len)
return -EOPNOTSUPP;
if (copy_from_user(®s, useraddr, sizeof(regs)))
return -EFAULT;
reglen = ops->get_regs_len(dev);
if (reglen <= 0)
return reglen;
if (regs.len > reglen)
regs.len = reglen;
+ else
+ reglen = regs.len;
regbuf = vzalloc(reglen);
if (!regbuf)
return -ENOMEM;
ops->get_regs(dev, ®s, regbuf);
ret = -EFAULT;
if (copy_to_user(useraddr, ®s, sizeof(regs)))
goto out;
useraddr += offsetof(struct ethtool_regs, data);
- if (regbuf && copy_to_user(useraddr, regbuf, regs.len))
+ if (copy_to_user(useraddr, regbuf, reglen))
goto out;
ret = 0;
out:
vfree(regbuf);
return ret;
}
--
2.21.0
Re: [PATCH net-next] ethtool: copy reglen to userspace
Hi Michal, David, On Thu, 30 May 2019 11:26:30 -0700 (PDT), David Miller wrote: > From: Michal Kubecek > Date: Thu, 30 May 2019 10:27:22 +0200 > > > I believe this should be handled by ethtool_get_regs(), either by > > returning an error or by only copying data up to original regs.len > > passed by userspace. The former seems more correct but broken userspace > > software would suddenly start to fail where it "used to work". The > > latter would be closer to current behaviour but it would mean that > > broken userspace software might nerver notice there is something wrong. > > I therefore think we need to meticulously fixup all of these adjustments > of regs.len before adjusting the copy call here in generic ethtool. Indeed there are cases where userspace may allocate a smaller buffer (even though there's no way for the kernel to ensure the size that was really allocated, but well...). The kernel must still allocates a buffer of size ops->get_regs_len() and pass it to the driver. It's OK if the kernel driver messes with regs->len, since it's an open structure. Software may actually make use of it. However the kernel must use the original regs->len passed by userspace (up to ops->get_regs_len()) when copying the data. I'm sending a new patch right away. Thanks, Vivien
Re: [PATCH net v2] net: dsa: mv88e6xxx: fix handling of upper half of STATS_TYPE_PORT
Hi Rasmus,
On Wed, 29 May 2019 07:02:11 +, Rasmus Villemoes
wrote:
> Currently, the upper half of a 4-byte STATS_TYPE_PORT statistic ends
> up in bits 47:32 of the return value, instead of bits 31:16 as they
> should.
>
> Fixes: 6e46e2d821bb ("net: dsa: mv88e6xxx: Fix u64 statistics")
> Signed-off-by: Rasmus Villemoes
Correct, this 4-byte stat must be stored in the lower half of the
8-byte returned value:
Reviewed-by: Vivien Didelot
Re: [PATCH] net: dsa: mv88e6xxx: fix handling of upper half of STATS_TYPE_PORT
Hi Rasmus, Andrew,
On Wed, 29 May 2019 06:42:53 +, Rasmus Villemoes
wrote:
> On 28/05/2019 15.44, Andrew Lunn wrote:
> > On Tue, May 28, 2019 at 01:17:10PM +, Rasmus Villemoes wrote:
> >> Currently, the upper half of a 4-byte STATS_TYPE_PORT statistic ends
> >> up in bits 47:32 of the return value, instead of bits 31:16 as they
> >> should.
> >>
> >> Signed-off-by: Rasmus Villemoes
> >
> > Hi Rasmus
> >
> > Please include a Fixes tag, to indicate where the problem was
> > introduced. In this case, i think it was:
> >
> > Fixes: 6e46e2d821bb ("net: dsa: mv88e6xxx: Fix u64 statistics")
> >
> > And set the Subject to [PATCH net] to indicate this should be applied
> > to the net tree.
>
> Will do.
>
> >> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c
> >> b/drivers/net/dsa/mv88e6xxx/chip.c
> >> index 370434bdbdab..317553d2cb21 100644
> >> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> >> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> >> @@ -785,7 +785,7 @@ static uint64_t _mv88e6xxx_get_ethtool_stat(struct
> >> mv88e6xxx_chip *chip,
> >>err = mv88e6xxx_port_read(chip, port, s->reg + 1, ®);
> >>if (err)
> >>return U64_MAX;
> >> - high = reg;
> >> + low |= ((u32)reg) << 16;
> >>}
> >>break;
> >>case STATS_TYPE_BANK1:
> >
> > What i don't like about this is how the function finishes:
> >
> > }
> > value = (((u64)high) << 32) | low;
> > return value;
> > }
> >
> > A better fix might be
> >
> > - break
> > + value = (((u64)high) << 16 | low;
> > + return value;
>
> Why? It's odd to have the u32 "high" sometimes represent the high 32
> bits, sometimes the third 16 bits. It would make it harder to support an
> 8-byte STATS_TYPE_PORT statistic. I think the code is much cleaner if
> each case is just responsible for providing the upper/lower 32 bits,
> then have the common case combine them; . It's just that in the
> STATS_TYPE_BANK cases, the 32 bits are assembled from two 16 bit values
> by a helper (mv88e6xxx_g1_stats_read), while it is "open-coded" in the
> first case.
Here "low" and "high" are u32 and refer to the upper and lower halves
of the u64 returned value. For a 4-byte STATS_TYPE_PORT value, the
second read refers to the bits 31:16, thus belongs to (the upper half
of) the lower half of the 64-bit returned value, i.e. "low".
The current return value is correct, as well as your patch.
Thanks,
Vivien
[PATCH net-next] ethtool: copy reglen to userspace
ethtool_get_regs() allocates a buffer of size reglen obtained from
ops->get_regs_len(), thus only this value must be used when copying
the buffer back to userspace. Also no need to check regbuf twice.
Signed-off-by: Vivien Didelot
---
net/core/ethtool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 4a593853cbf2..f3369f31d93a 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1338,38 +1338,38 @@ static noinline_for_stack int ethtool_set_rxfh(struct
net_device *dev,
static int ethtool_get_regs(struct net_device *dev, char __user *useraddr)
{
struct ethtool_regs regs;
const struct ethtool_ops *ops = dev->ethtool_ops;
void *regbuf;
int reglen, ret;
if (!ops->get_regs || !ops->get_regs_len)
return -EOPNOTSUPP;
if (copy_from_user(®s, useraddr, sizeof(regs)))
return -EFAULT;
reglen = ops->get_regs_len(dev);
if (reglen <= 0)
return reglen;
if (regs.len > reglen)
regs.len = reglen;
regbuf = vzalloc(reglen);
if (!regbuf)
return -ENOMEM;
ops->get_regs(dev, ®s, regbuf);
ret = -EFAULT;
if (copy_to_user(useraddr, ®s, sizeof(regs)))
goto out;
useraddr += offsetof(struct ethtool_regs, data);
- if (regbuf && copy_to_user(useraddr, regbuf, regs.len))
+ if (copy_to_user(useraddr, regbuf, reglen))
goto out;
ret = 0;
out:
vfree(regbuf);
return ret;
}
--
2.21.0
Re: [PATCH v2 5/5] net: dsa: add support for mv88e6250
Hi Rasmus, On Fri, 24 May 2019 09:00:31 +, Rasmus Villemoes wrote: > This is a very rough attempt at adding support for the Marvell > 88E6250. The _info and _ops structures are based on those for 6240 (as > I have data sheets for both the 6240 and 6250), fixing the things that > I have determined to be different for the two chips - but some things > are almost certain to still be wrong. The idea is that for things that you're not certain about, simply don't add the corresponding ops. The driver would simply return -EOPNOTSUPP for the related features (if it doesn't behave like this, we must fix this.) Thanks, Vivien
Re: [PATCH v2 4/5] net: dsa: mv88e6xxx: implement watchdog_ops for mv88e6250
On Fri, 24 May 2019 09:00:29 +, Rasmus Villemoes wrote: > The MV88E6352_G2_WDOG_CTL_* bits almost, but not quite, describe the > watchdog control register on the mv88e6250. Among those actually > referenced in the code, only QC_ENABLE differs (bit 6 rather than bit > 5). > > Signed-off-by: Rasmus Villemoes Clean patch, Reviewed-by: Vivien Didelot Thanks, Vivien
Re: [PATCH v2 3/5] net: dsa: implement vtu_getnext and vtu_loadpurge for mv88e6250
On Fri, 24 May 2019 09:00:27 +, Rasmus Villemoes wrote: > These are almost identical to the 6185 variants, but have fewer bits > for the FID. > > Bit 10 of the VTU_OP register (offset 0x05) is the VidPolicy bit, > which one should probably preserve in mv88e6xxx_g1_vtu_op(), instead > of always writing a 0. However, on the 6352 family, that bit is > located at bit 12 in the VTU FID register (offset 0x02), and is always > unconditionally cleared by the mv88e6xxx_g1_vtu_fid_write() > function. > > Since nothing in the existing driver seems to know or care about that > bit, it seems reasonable to not add the boilerplate to preserve it for > the 6250 (which would require adding a chip-specific vtu_op function, > or adding chip-quirks to the existing one). > > Reviewed-by: Andrew Lunn > Signed-off-by: Rasmus Villemoes Reviewed-by: Vivien Didelot
Re: [PATCH v2 2/5] net: dsa: prepare mv88e6xxx_g1_atu_op() for the mv88e6250
On Fri, 24 May 2019 09:00:26 +, Rasmus Villemoes wrote: > All the currently supported chips have .num_databases either 256 or > 4096, so this patch does not change behaviour for any of those. The > mv88e6250, however, has .num_databases == 64, and it does not put the > upper two bits in ATU control 13:12, but rather in ATU Operation > 9:8. So change the logic to prepare for supporting mv88e6250. > > Reviewed-by: Andrew Lunn > Signed-off-by: Rasmus Villemoes Reviewed-by: Vivien Didelot
Re: [PATCH v2 1/5] net: dsa: mv88e6xxx: introduce support for two chips using direct smi addressing
Hi Rasmus,
On Fri, 24 May 2019 09:00:24 +, Rasmus Villemoes
wrote:
> The 88e6250 (as well as 6220, 6071, 6070, 6020) do not support
> multi-chip (indirect) addressing. However, one can still have two of
> them on the same mdio bus, since the device only uses 16 of the 32
> possible addresses, either addresses 0x00-0x0F or 0x10-0x1F depending
> on the ADDR4 pin at reset [since ADDR4 is internally pulled high, the
> latter is the default].
>
> In order to prepare for supporting the 88e6250 and friends, introduce
> mv88e6xxx_info::dual_chip to allow having a non-zero sw_addr while
> still using direct addressing.
>
> Signed-off-by: Rasmus Villemoes
> ---
> drivers/net/dsa/mv88e6xxx/chip.h | 6 ++
> drivers/net/dsa/mv88e6xxx/smi.c | 25 -
> 2 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.h
> b/drivers/net/dsa/mv88e6xxx/chip.h
> index faa3fa889f19..74777c3bc313 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.h
> +++ b/drivers/net/dsa/mv88e6xxx/chip.h
> @@ -112,6 +112,12 @@ struct mv88e6xxx_info {
>* when it is non-zero, and use indirect access to internal registers.
>*/
> bool multi_chip;
> + /* Dual-chip Addressing Mode
> + * Some chips respond to only half of the 32 SMI addresses,
> + * allowing two to coexist on the same SMI interface.
> + */
> + bool dual_chip;
> +
> enum dsa_tag_protocol tag_protocol;
>
> /* Mask for FromPort and ToPort value of PortVec used in ATU Move
> diff --git a/drivers/net/dsa/mv88e6xxx/smi.c b/drivers/net/dsa/mv88e6xxx/smi.c
> index 96f7d2685bdc..1151b5b493ea 100644
> --- a/drivers/net/dsa/mv88e6xxx/smi.c
> +++ b/drivers/net/dsa/mv88e6xxx/smi.c
> @@ -24,6 +24,10 @@
> * When ADDR is non-zero, the chip uses Multi-chip Addressing Mode, allowing
> * multiple devices to share the SMI interface. In this mode it responds to
> only
> * 2 registers, used to indirectly access the internal SMI devices.
> + *
> + * Some chips use a different scheme: Only the ADDR4 pin is used for
> + * configuration, and the device responds to 16 of the 32 SMI
> + * addresses, allowing two to coexist on the same SMI interface.
> */
>
> static int mv88e6xxx_smi_direct_read(struct mv88e6xxx_chip *chip,
> @@ -76,6 +80,23 @@ static const struct mv88e6xxx_bus_ops
> mv88e6xxx_smi_direct_ops = {
> .write = mv88e6xxx_smi_direct_write,
> };
>
> +static int mv88e6xxx_smi_dual_direct_read(struct mv88e6xxx_chip *chip,
> + int dev, int reg, u16 *data)
> +{
> + return mv88e6xxx_smi_direct_read(chip, dev + chip->sw_addr, reg, data);
Using chip->sw_addr + dev seems more idiomatic to me than dev + chip->sw_addr.
> +}
> +
> +static int mv88e6xxx_smi_dual_direct_write(struct mv88e6xxx_chip *chip,
> +int dev, int reg, u16 data)
> +{
> + return mv88e6xxx_smi_direct_write(chip, dev + chip->sw_addr, reg, data);
> +}
> +
> +static const struct mv88e6xxx_bus_ops mv88e6xxx_smi_dual_direct_ops = {
> + .read = mv88e6xxx_smi_dual_direct_read,
> + .write = mv88e6xxx_smi_dual_direct_write,
> +};
> +
> /* Offset 0x00: SMI Command Register
> * Offset 0x01: SMI Data Register
> */
> @@ -144,7 +165,9 @@ static const struct mv88e6xxx_bus_ops
> mv88e6xxx_smi_indirect_ops = {
> int mv88e6xxx_smi_init(struct mv88e6xxx_chip *chip,
> struct mii_bus *bus, int sw_addr)
> {
> - if (sw_addr == 0)
> + if (chip->info->dual_chip)
> + chip->smi_ops = &mv88e6xxx_smi_dual_direct_ops;
> + else if (sw_addr == 0)
> chip->smi_ops = &mv88e6xxx_smi_direct_ops;
> else if (chip->info->multi_chip)
> chip->smi_ops = &mv88e6xxx_smi_indirect_ops;
Please submit respins (v2, v3, and so on) as independent threads,
not as a reply to the previous version.
Otherwise this looks good to me:
Reviewed-by: Vivien Didelot
Thanks,
Vivien
Re: [RFC PATCH 1/5] net: dsa: mv88e6xxx: introduce support for two chips using direct smi addressing
Hi Rasmus,
On Wed, 8 May 2019 13:47:15 +0200, Andrew Lunn wrote:
> > >
> > > This works, but i think i prefer adding mv88e6xxx_smi_dual_chip_write,
> > > mv88e6xxx_smi_dual_chip_read, and create a
> > > mv88e6xxx_smi_single_chip_ops.
> >
> > Now that Vivien's "net: dsa: mv88e6xxx: refine SMI support" is in
> > master, do you still prefer introducing a third bus_ops structure
> > (mv88e6xxx_smi_dual_direct_ops ?), or would the approach of adding
> > chip->sw_addr in the smi_direct_{read/write} functions be ok (which
> > would then require changing the indirect callers to pass 0 instead of
> > chip->swaddr).
>
> I would still prefer a new bus_ops.
Even though those are direct read and write operations, having 3
mv88e6xxx_bus_ops structures will make it clear that there are 3 ways
for accessible the internal switch registers through SMI, depending
on the Marvell chip model. So I would prefer a third ops as well.
Thanks,
Vivien
Re: [RFC PATCH 2/5] net: dsa: mv88e6xxx: rename smi read/write functions
Hi Rasmus, On Mon, 6 May 2019 05:57:11 +, Rasmus Villemoes wrote: > > I have a preparatory patch which does almost exactly that. I'm sending it > > to simplify this patchset. > > OK, I'll hold off sending a v2 until I see how 1/5 and 2/5 are obsoleted > by your patch(es). You may rebase your patches now and add your new implementation of register access through SMI in the smi.c file if that is necessary. Thanks, Vivien
Re: [PATCH net-next] net: dsa: use struct_size() in devm_kzalloc()
On Thu, 7 Feb 2019 19:16:03 -0600, "Gustavo A. R. Silva"
wrote:
> One of the more common cases of allocation size calculations is finding
> the size of a structure that has a zero-sized array at the end, along
> with memory for some number of elements for that array. For example:
>
> struct foo {
> int stuff;
> struct boo entry[];
> };
>
> size = sizeof(struct foo) + count * sizeof(struct boo);
> instance = alloc(size, GFP_KERNEL)
>
> Instead of leaving these open-coded and prone to type mistakes, we can
> now use the new struct_size() helper:
>
> instance = alloc(struct_size(instance, entry, count), GFP_KERNEL)
>
> Notice that, in this case, variable size is not necessary, hence it is
> removed.
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva
Reviewed-by: Vivien Didelot
Re: [PATCH net-next v3 11/12] net: dsa: Implement ndo_get_port_parent_id()
Hi Florian,
On Tue, 5 Feb 2019 15:53:25 -0800, Florian Fainelli
wrote:
> DSA implements SWITCHDEV_ATTR_ID_PORT_PARENT_ID and we want to get rid
> of switchdev_ops eventually, ease that migration by implementing a
> ndo_get_port_parent_id() function which returns what
> switchdev_port_attr_get() would do.
>
> Signed-off-by: Florian Fainelli
> ---
> net/dsa/slave.c | 18 --
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 91de3a663226..70395a0ae52e 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -362,18 +362,23 @@ static int dsa_slave_port_obj_del(struct net_device
> *dev,
> return err;
> }
>
> -static int dsa_slave_port_attr_get(struct net_device *dev,
> -struct switchdev_attr *attr)
> +static int dsa_slave_get_port_parent_id(struct net_device *dev,
> + struct netdev_phys_item_id *ppid)
> {
> struct dsa_port *dp = dsa_slave_to_port(dev);
> struct dsa_switch *ds = dp->ds;
> struct dsa_switch_tree *dst = ds->dst;
>
> + ppid->id_len = sizeof(dst->index);
> + memcpy(&ppid->id, &dst->index, ppid->id_len);
> +
> + return 0;
> +}
Finally this will give us a way to distinguish two ports with the same switch
and port IDs on a system with two disjoint switch trees, thanks!
Reviewed-by: Vivien Didelot
Re: [PATCH] net: dsa: Fix lockdep false positive splat
On Sat, 2 Feb 2019 17:53:29 +, Marc Zyngier wrote:
> Creating a macvtap on a DSA-backed interface results in the following
> splat when lockdep is enabled:
>
> [ 19.638080] IPv6: ADDRCONF(NETDEV_CHANGE): lan0: link becomes ready
> [ 23.041198] device lan0 entered promiscuous mode
> [ 23.043445] device eth0 entered promiscuous mode
> [ 23.049255]
> [ 23.049557]
> [ 23.055021] WARNING: possible recursive locking detected
> [ 23.060490] 5.0.0-rc3-00013-g56c857a1b8d3 #118 Not tainted
> [ 23.066132]
> [ 23.071598] ip/2861 is trying to acquire lock:
> [ 23.076171] f61990cb (_xmit_ETHER){+...}, at:
> dev_set_rx_mode+0x1c/0x38
> [ 23.083693]
> [ 23.083693] but task is already holding lock:
> [ 23.089696] ecf0c3b4 (_xmit_ETHER){+...}, at: dev_uc_add+0x24/0x70
> [ 23.096774]
> [ 23.096774] other info that might help us debug this:
> [ 23.103494] Possible unsafe locking scenario:
> [ 23.103494]
> [ 23.109584]CPU0
> [ 23.112093]
> [ 23.114601] lock(_xmit_ETHER);
> [ 23.117917] lock(_xmit_ETHER);
> [ 23.121233]
> [ 23.121233] *** DEADLOCK ***
> [ 23.121233]
> [ 23.127325] May be due to missing lock nesting notation
> [ 23.127325]
> [ 23.134315] 2 locks held by ip/2861:
> [ 23.137987] #0: 3b766c72 (rtnl_mutex){+.+.}, at:
> rtnetlink_rcv_msg+0x338/0x4e0
> [ 23.146231] #1: ecf0c3b4 (_xmit_ETHER){+...}, at:
> dev_uc_add+0x24/0x70
> [ 23.153757]
> [ 23.153757] stack backtrace:
> [ 23.158243] CPU: 0 PID: 2861 Comm: ip Not tainted
> 5.0.0-rc3-00013-g56c857a1b8d3 #118
> [ 23.166212] Hardware name: Globalscale Marvell ESPRESSOBin Board (DT)
> [ 23.172843] Call trace:
> [ 23.175358] dump_backtrace+0x0/0x188
> [ 23.179116] show_stack+0x14/0x20
> [ 23.182524] dump_stack+0xb4/0xec
> [ 23.185928] __lock_acquire+0x123c/0x1860
> [ 23.190048] lock_acquire+0xc8/0x248
> [ 23.193724] _raw_spin_lock_bh+0x40/0x58
> [ 23.197755] dev_set_rx_mode+0x1c/0x38
> [ 23.201607] dev_set_promiscuity+0x3c/0x50
> [ 23.205820] dsa_slave_change_rx_flags+0x5c/0x70
> [ 23.210567] __dev_set_promiscuity+0x148/0x1e0
> [ 23.215136] __dev_set_rx_mode+0x74/0x98
> [ 23.219167] dev_uc_add+0x54/0x70
> [ 23.222575] macvlan_open+0x170/0x1d0
> [ 23.226336] __dev_open+0xe0/0x160
> [ 23.229830] __dev_change_flags+0x16c/0x1b8
> [ 23.234132] dev_change_flags+0x20/0x60
> [ 23.238074] do_setlink+0x2d0/0xc50
> [ 23.241658] __rtnl_newlink+0x5f8/0x6e8
> [ 23.245601] rtnl_newlink+0x50/0x78
> [ 23.249184] rtnetlink_rcv_msg+0x360/0x4e0
> [ 23.253397] netlink_rcv_skb+0xe8/0x130
> [ 23.257338] rtnetlink_rcv+0x14/0x20
> [ 23.261012] netlink_unicast+0x190/0x210
> [ 23.265043] netlink_sendmsg+0x288/0x350
> [ 23.269075] sock_sendmsg+0x18/0x30
> [ 23.272659] ___sys_sendmsg+0x29c/0x2c8
> [ 23.276602] __sys_sendmsg+0x60/0xb8
> [ 23.280276] __arm64_sys_sendmsg+0x1c/0x28
> [ 23.284488] el0_svc_common+0xd8/0x138
> [ 23.288340] el0_svc_handler+0x24/0x80
> [ 23.292192] el0_svc+0x8/0xc
>
> This looks fairly harmless (no actual deadlock occurs), and is
> fixed in a similar way to c6894dec8ea9 ("bridge: fix lockdep
> addr_list_lock false positive splat") by putting the addr_list_lock
> in its own lockdep class.
>
> Signed-off-by: Marc Zyngier
Reviewed-by: Vivien Didelot
Re: [PATCH net-next v3] net: dsa: mv88e6xxx: Prevent suspend to RAM
Hi Miquel,
On Tue, 5 Feb 2019 12:07:28 +0100, Miquel Raynal
wrote:
> +/* There is no suspend to RAM support at DSA level yet, the switch
> configuration
> + * would be lost after a power cycle so prevent it to be suspended.
> + */
> +static int __maybe_unused mv88e6xxx_suspend(struct device *dev)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static int __maybe_unused mv88e6xxx_resume(struct device *dev)
> +{
> + return 0;
> +}
The code looks good but my only concern is -EOPNOTSUPP. In this
context this code is specific to callbacks targeting bridge and
switchdev, while the dev_pm_ops are completely parallel to DSA.
It is intuitive but given Documentation/power/runtime_pm.txt, this
will default to being interpreted as a fatal error, while -EBUSY
seems to keep the device in an 'active' state in a saner way.
I don't understand yet how to properly tell PM core that suspend to RAM
isn't supported. If an error code different from -EAGAIN or -EBUSY
is the way to go, I'm good with it:
Reviewed-by: Vivien Didelot
Thanks,
Vivien
Re: [PATCH net-next v2 1/2] net: dsa: mv88e6xxx: Save switch rules
Hi Miquèl, On Wed, 30 Jan 2019 10:46:06 +0100, Miquel Raynal wrote: > > > > Today, there is no S2RAM support for switches. First, I proposed to add > > > > suspend/resume callbacks to the mv88e6xxx driver - just enough to avoid > > > > crashing the kernel. > > > > > > Then i would suggest the mv88e6xxx refuses the suspend. Actually that > > > probably is the first correct step. We don't have suspend support, so > > > stop the suspend happening, so preventing the kernel crash. Actually can you show me the crash that is happening?
Re: [PATCH net-next v2 1/2] net: dsa: mv88e6xxx: Save switch rules
Hi Miquèl, On Tue, 29 Jan 2019 15:51:57 +0100, Andrew Lunn wrote: > > Today, there is no S2RAM support for switches. First, I proposed to add > > suspend/resume callbacks to the mv88e6xxx driver - just enough to avoid > > crashing the kernel. > > Then i would suggest the mv88e6xxx refuses the suspend. Actually that > probably is the first correct step. We don't have suspend support, so > stop the suspend happening, so preventing the kernel crash. I am not confortable with adding support for S2RAM in mv88e6xxx yet because as it was explained, we are aware of much complicated scenarios out there to pretend that DSA /partly/ supports suspend-resume. The prefered approach for the moment is to keep things simple and not supporting this feature yet, especially at the mv88e6xxx driver level. However crashing is unacceptable so I'm backing Andrew's point here, please submit a fix to prevent the suspend (and crash) for the moment. Sorry if you felt that your work is being delayed, it is much appreciated! Thanks, Vivien
Re: [PATCH 1/2] regulator: mc13xxx: Use lowercase regulator names to match the DT
Hi Rob,
On Fri, 25 Jan 2019 09:37:04 -0600, Rob Herring wrote:
> Since c32569e358ad ("regulator: Use of_node_name_eq for node name
> comparisons") Vivien reported the mc13892-regulator complaining about
> not being able to find regulators.
>
> This is because prior to that commit we used of_node_cmp() to compare
> the regulator array passed from mc13892_regulators down to
> mc13xxx_parse_regulators_dt() and they are all defined in uppercase
> letters by the MC13892_*_DEFINE* macros, whereas they are defined as
> lowercase in the DTS.
>
> Fix this by using a lowercase regulator name to match the DT node name.
>
> Fixes: c32569e358ad ("regulator: Use of_node_name_eq for node name
> comparisons")
> Reported-by: Vivien Didelot
> Reported-by: Florian Fainelli
> Cc: Liam Girdwood
> Cc: Mark Brown
> Signed-off-by: Rob Herring
This fixes the boot with arch/arm/boot/dts/imx51-zii-rdu1.dts,
Tested-by: Vivien Didelot
Thanks,
Vivien
Re: [PATCH] of: Make of_node_name_eq() case insensitive
Hi,
On Thu, 24 Jan 2019 12:08:25 -0800, Florian Fainelli
wrote:
> Since c32569e358ad ("regulator: Use of_node_name_eq for node name
> comparisons") Vivien reported the mc13892-regulator complaining about
> not being able to find regulators.
>
> This is because prior to that commit we used of_node_cmp() to compare
> the regulator array passed from mc13892_regulators down to
> mc13xxx_parse_regulators_dt() and they are all defined in uppercase
> letters by the MC13892_*_DEFINE* macros, whereas they are defined as
> lowercase in the DTS.
>
> Fix this by use strncasecmp() since that makes sure the comparison is
> case insensitive like what of_node_cmp() did.
>
> Reported-by: Vivien Didelot
> Fixes: c32569e358ad ("regulator: Use of_node_name_eq for node name
> comparisons")
> Signed-off-by: Florian Fainelli
This fixes the boot on i.MX51 ZII RDU1, which was printing this:
[2.895302] imx-ipuv3 4000.ipu: IPUv3EX probed
[2.903869] spi_imx 7001.spi: dma setup error -19, use pio
[2.911943] mc13xxx spi0.0: mc13892: rev: 2.4, fin: 2, fab: 0, icid: 7/2
[2.921463] mc13892-regulator mc13892-regulator: Unknown regulator: sw1
[2.928207] mc13892-regulator mc13892-regulator: Unknown regulator: sw2
[2.934896] mc13892-regulator mc13892-regulator: Unknown regulator: sw3
[2.941575] mc13892-regulator mc13892-regulator: Unknown regulator: sw4
[2.948263] mc13892-regulator mc13892-regulator: Unknown regulator: vpll
[2.955050] mc13892-regulator mc13892-regulator: Unknown regulator: vdig
[2.961820] mc13892-regulator mc13892-regulator: Unknown regulator: vsd
[2.968464] mc13892-regulator mc13892-regulator: Unknown regulator: vusb
[2.975251] mc13892-regulator mc13892-regulator: Unknown regulator: vusb2
[2.982110] mc13892-regulator mc13892-regulator: Unknown regulator:
vvideo
[2.989039] mc13892-regulator mc13892-regulator: Unknown regulator:
vaudio
[2.995983] mc13892-regulator mc13892-regulator: Unknown regulator: vcam
[3.002754] mc13892-regulator mc13892-regulator: Unknown regulator: vgen1
[3.009597] mc13892-regulator mc13892-regulator: Unknown regulator: vgen2
[3.016458] mc13892-regulator mc13892-regulator: Unknown regulator: vgen3
before looping forever on the defered probe of the Marvell switch.
Tested-by: Vivien Didelot
Thanks,
Vivien
Re: [PATCH net-next] net: dsa: mv88e6xxx: Add suspend/resume callbacks
On Thu, 17 Jan 2019 10:00:46 -0800, Florian Fainelli wrote: > A possible approach could be to call the port_disable, port_enable > callbacks from dsa_slave_suspend() and dsa_slave_resume(), I might have > some patches doing that already somewhere. > >>> > >>> I expect it is also on Viviens TODO list, since this really could be > >>> in the core. > >> > >> Indeed that is! > > > > So, shall I wait for Vivien's patches (adding port_disable/enable() > > in dsa_slave_suspend/resume()) and keep the driver as-is or do you want > > me to manually call port_disable/enable() from the mv88e6xxx driver? > > Up to you guys, the only thing that I an tell you is that my platform > loses its register contents during suspend/resume, therefore you must > make sure the driver re-applies the entire switch configuration, > identical to how it was prior to suspend. If you need me to test > something, please holler. Let's wait then. Thanks, Vivien
