From 8674f8d310215d2bb7469b80e6cfccc044f717b3 Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Mon, 21 Jun 2021 19:42:14 +0300
Subject: [PATCH 1/6] net: dsa: assert uniqueness of dsa,member properties
The cross-chip notifiers work by comparing each ds->index against the
info->sw_index value from the notifier. The ds->index is retrieved from
the device tree dsa,member property.
If a single tree cross-chip topology does not declare unique switch IDs,
this will result in hard-to-debug issues/voodoo effects such as the
cross-chip notifier for one switch port also matching the port with the
same number from another switch.
Check in dsa_switch_parse_member_of() whether the DSA switch tree
contains a DSA switch with the index we're preparing to add, before
actually adding it.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/dsa/dsa2.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index b71e87909f0e3..ba244fbd9646d 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -1259,6 +1259,13 @@ static int dsa_switch_parse_member_of(struct dsa_switch *ds,
if (!ds->dst)
return -ENOMEM;
+ if (dsa_switch_find(ds->dst->index, ds->index)) {
+ dev_err(ds->dev,
+ "A DSA switch with index %d already exists in tree %d\n",
+ ds->index, ds->dst->index);
+ return -EEXIST;
+ }
+
return 0;
}
From a8986681ccada614a30df7248390780e7708a763 Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Mon, 21 Jun 2021 19:42:15 +0300
Subject: [PATCH 2/6] net: dsa: export the dsa_port_is_{user,cpu,dsa} helpers
The difference between dsa_is_user_port and dsa_port_is_user is that the
former needs to look up the list of ports of the DSA switch tree in
order to find the struct dsa_port, while the latter directly receives it
as an argument.
dsa_is_user_port is already in widespread use and has its place, so
there isn't any chance of converting all callers to a single form.
But being able to do:
dsa_port_is_user(dp)
instead of
dsa_is_user_port(dp->ds, dp->index)
is much more efficient too, especially when the "dp" comes from an
iterator over the DSA switch tree - this reduces the complexity from
quadratic to linear.
Move these helpers from dsa2.c to include/net/dsa.h so that others can
use them too.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
include/net/dsa.h | 15 +++++++++++++++
net/dsa/dsa2.c | 15 ---------------
2 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 289d68e82da02..ea47783d56951 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -409,6 +409,21 @@ static inline struct dsa_port *dsa_to_port(struct dsa_switch *ds, int p)
return NULL;
}
+static inline bool dsa_port_is_dsa(struct dsa_port *port)
+{
+ return port->type == DSA_PORT_TYPE_DSA;
+}
+
+static inline bool dsa_port_is_cpu(struct dsa_port *port)
+{
+ return port->type == DSA_PORT_TYPE_CPU;
+}
+
+static inline bool dsa_port_is_user(struct dsa_port *dp)
+{
+ return dp->type == DSA_PORT_TYPE_USER;
+}
+
static inline bool dsa_is_unused_port(struct dsa_switch *ds, int p)
{
return dsa_to_port(ds, p)->type == DSA_PORT_TYPE_UNUSED;
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index ba244fbd9646d..9000a8c84bafe 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -219,21 +219,6 @@ static void dsa_tree_put(struct dsa_switch_tree *dst)
kref_put(&dst->refcount, dsa_tree_release);
}
-static bool dsa_port_is_dsa(struct dsa_port *port)
-{
- return port->type == DSA_PORT_TYPE_DSA;
-}
-
-static bool dsa_port_is_cpu(struct dsa_port *port)
-{
- return port->type == DSA_PORT_TYPE_CPU;
-}
-
-static bool dsa_port_is_user(struct dsa_port *dp)
-{
- return dp->type == DSA_PORT_TYPE_USER;
-}
-
static struct dsa_port *dsa_tree_find_port_by_node(struct dsa_switch_tree *dst,
struct device_node *dn)
{
From abd49535c3801f33c3ca42d81271d7e535adce81 Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Mon, 21 Jun 2021 19:42:16 +0300
Subject: [PATCH 3/6] net: dsa: execute dsa_switch_mdb_add only for routing
port in cross-chip topologies
Currently, the notifier for adding a multicast MAC address matches on
the targeted port and on all DSA links in the system, be they upstream
or downstream links.
This leads to a considerable amount of useless traffic.
Consider this daisy chain topology, and a MDB add notifier emitted on
sw0p0. It matches on sw0p0, sw0p3, sw1p3 and sw2p4.
sw0p0 sw0p1 sw0p2 sw0p3 sw0p4
[ user ] [ user ] [ user ] [ dsa ] [ cpu ]
[ x ] [ ] [ ] [ x ] [ ]
|
+---------+
|
sw1p0 sw1p1 sw1p2 sw1p3 sw1p4
[ user ] [ user ] [ user ] [ dsa ] [ dsa ]
[ ] [ ] [ ] [ x ] [ x ]
|
+---------+
|
sw2p0 sw2p1 sw2p2 sw2p3 sw2p4
[ user ] [ user ] [ user ] [ user ] [ dsa ]
[ ] [ ] [ ] [ ] [ x ]
But switch 0 has no reason to send the multicast traffic for that MAC
address on sw0p3, which is how it reaches switches 1 and 2. Those
switches don't expect, according to the user configuration, to receive
this multicast address from switch 1, and they will drop it anyway,
because the only valid destination is the port they received it on.
They only need to configure themselves to deliver that multicast address
_towards_ switch 1, where the MDB entry is installed.
Similarly, switch 1 should not send this multicast traffic towards
sw1p3, because that is how it reaches switch 2.
With this change, the heat map for this MDB notifier changes as follows:
sw0p0 sw0p1 sw0p2 sw0p3 sw0p4
[ user ] [ user ] [ user ] [ dsa ] [ cpu ]
[ x ] [ ] [ ] [ ] [ ]
|
+---------+
|
sw1p0 sw1p1 sw1p2 sw1p3 sw1p4
[ user ] [ user ] [ user ] [ dsa ] [ dsa ]
[ ] [ ] [ ] [ ] [ x ]
|
+---------+
|
sw2p0 sw2p1 sw2p2 sw2p3 sw2p4
[ user ] [ user ] [ user ] [ user ] [ dsa ]
[ ] [ ] [ ] [ ] [ x ]
Now the mdb notifier behaves the same as the fdb notifier.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/dsa/switch.c | 25 ++-----------------------
1 file changed, 2 insertions(+), 23 deletions(-)
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 9bf8e20ecdf38..8b601ced6b45f 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -232,36 +232,15 @@ static int dsa_switch_lag_leave(struct dsa_switch *ds,
return 0;
}
-static bool dsa_switch_mdb_match(struct dsa_switch *ds, int port,
- struct dsa_notifier_mdb_info *info)
-{
- if (ds->index == info->sw_index && port == info->port)
- return true;
-
- if (dsa_is_dsa_port(ds, port))
- return true;
-
- return false;
-}
-
static int dsa_switch_mdb_add(struct dsa_switch *ds,
struct dsa_notifier_mdb_info *info)
{
- int err = 0;
- int port;
+ int port = dsa_towards_port(ds, info->sw_index, info->port);
if (!ds->ops->port_mdb_add)
return -EOPNOTSUPP;
- for (port = 0; port < ds->num_ports; port++) {
- if (dsa_switch_mdb_match(ds, port, info)) {
- err = ds->ops->port_mdb_add(ds, port, info->mdb);
- if (err)
- break;
- }
- }
-
- return err;
+ return ds->ops->port_mdb_add(ds, port, info->mdb);
}
static int dsa_switch_mdb_del(struct dsa_switch *ds,
From 4e4ab7950044d195f6e3d4dac328f506badb6efa Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Mon, 21 Jun 2021 19:42:17 +0300
Subject: [PATCH 4/6] net: dsa: calculate the largest_mtu across all ports in
the tree
If we have a cross-chip topology like this:
sw0p0 sw0p1 sw0p2 sw0p3 sw0p4
[ cpu ] [ user ] [ user ] [ dsa ] [ user ]
|
+---------+
|
sw1p0 sw1p1 sw1p2 sw1p3 sw1p4
[ user ] [ user ] [ user ] [ dsa ] [ dsa ]
and we issue the following commands:
1. ip link set sw0p1 mtu 1700
2. ip link set sw1p1 mtu 1600
we notice the following happening:
Command 1. emits a non-targeted MTU notifier for the CPU port (sw0p0)
with the largest_mtu calculated across switch 0, of 1700. This matches
sw0p0, sw0p3 and sw1p4 (all CPU ports and DSA links).
Then, it emits a targeted MTU notifier for the user port (sw0p1), again
with MTU 1700 (this doesn't matter).
Command 2. emits a non-targeted MTU notifier for the CPU port (sw0p0)
with the largest_mtu calculated across switch 1, of 1600. This matches
the same group of ports as above, and decreases the MTU for the CPU port
and the DSA links from 1700 to 1600.
As a result, the sw0p1 user port can no longer communicate with its CPU
port at MTU 1700.
To address this, we should calculate the largest_mtu across all switches
that may share a CPU port, and only emit MTU notifiers with that value.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/dsa/slave.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 798944aa847a0..ac2ca5f75af35 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1528,6 +1528,7 @@ int dsa_slave_change_mtu(struct net_device *dev, int new_mtu)
struct dsa_port *dp = dsa_slave_to_port(dev);
struct dsa_slave_priv *p = netdev_priv(dev);
struct dsa_switch *ds = p->dp->ds;
+ struct dsa_port *dp_iter;
struct dsa_port *cpu_dp;
int port = p->dp->index;
int largest_mtu = 0;
@@ -1535,31 +1536,31 @@ int dsa_slave_change_mtu(struct net_device *dev, int new_mtu)
int old_master_mtu;
int mtu_limit;
int cpu_mtu;
- int err, i;
+ int err;
if (!ds->ops->port_change_mtu)
return -EOPNOTSUPP;
- for (i = 0; i < ds->num_ports; i++) {
+ list_for_each_entry(dp_iter, &ds->dst->ports, list) {
int slave_mtu;
- if (!dsa_is_user_port(ds, i))
+ if (!dsa_port_is_user(dp_iter))
continue;
/* During probe, this function will be called for each slave
* device, while not all of them have been allocated. That's
* ok, it doesn't change what the maximum is, so ignore it.
*/
- if (!dsa_to_port(ds, i)->slave)
+ if (!dp_iter->slave)
continue;
/* Pretend that we already applied the setting, which we
* actually haven't (still haven't done all integrity checks)
*/
- if (i == port)
+ if (dp_iter == dp)
slave_mtu = new_mtu;
else
- slave_mtu = dsa_to_port(ds, i)->slave->mtu;
+ slave_mtu = dp_iter->slave->mtu;
if (largest_mtu < slave_mtu)
largest_mtu = slave_mtu;
From 88faba20e2100c1f367133af56612742ad37df08 Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Mon, 21 Jun 2021 19:42:18 +0300
Subject: [PATCH 5/6] net: dsa: targeted MTU notifiers should only match on one
port
dsa_slave_change_mtu() calls dsa_port_mtu_change() twice:
- it sends a cross-chip notifier with the MTU of the CPU port which is
used to update the DSA links.
- it sends one targeted MTU notifier which is supposed to only match the
user port on which we are changing the MTU. The "propagate_upstream"
variable is used here to bypass the cross-chip notifier system from
switch.c
But due to a mistake, the second, targeted notifier matches not only on
the user port, but also on the DSA link which is a member of the same
switch, if that exists.
And because the DSA links of the entire dst were programmed in a
previous round to the largest_mtu via a "propagate_upstream == true"
notification, then the dsa_port_mtu_change(propagate_upstream == false)
call that is immediately upcoming will break the MTU on the one DSA link
which is chip-wise local to the dp whose MTU is changing right now.
Example given this daisy chain topology:
sw0p0 sw0p1 sw0p2 sw0p3 sw0p4
[ cpu ] [ user ] [ user ] [ dsa ] [ user ]
[ x ] [ ] [ ] [ x ] [ ]
|
+---------+
|
sw1p0 sw1p1 sw1p2 sw1p3 sw1p4
[ user ] [ user ] [ user ] [ dsa ] [ dsa ]
[ ] [ ] [ ] [ ] [ x ]
ip link set sw0p1 mtu 9000
ip link set sw1p1 mtu 9000 # at this stage, sw0p1 and sw1p1 can talk
# to one another using jumbo frames
ip link set sw0p2 mtu 1500 # this programs the sw0p3 DSA link first to
# the largest_mtu of 9000, then reprograms it to
# 1500 with the "propagate_upstream == false"
# notifier, breaking communication between
# sw0p1 and sw1p1
To escape from this situation, make the targeted match really match on a
single port - the user port, and rename the "propagate_upstream"
variable to "targeted_match" to clarify the intention and avoid future
issues.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/dsa/dsa_priv.h | 4 ++--
net/dsa/port.c | 4 ++--
net/dsa/slave.c | 9 +++++----
net/dsa/switch.c | 9 ++++++---
4 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index b8b17474b72b1..b0811253d1018 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -84,7 +84,7 @@ struct dsa_notifier_vlan_info {
/* DSA_NOTIFIER_MTU */
struct dsa_notifier_mtu_info {
- bool propagate_upstream;
+ bool targeted_match;
int sw_index;
int port;
int mtu;
@@ -200,7 +200,7 @@ int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
bool dsa_port_skip_vlan_configuration(struct dsa_port *dp);
int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock);
int dsa_port_mtu_change(struct dsa_port *dp, int new_mtu,
- bool propagate_upstream);
+ bool targeted_match);
int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
u16 vid);
int dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 6379d66a6bb32..5c93f1e1a03d6 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -567,11 +567,11 @@ int dsa_port_mrouter(struct dsa_port *dp, bool mrouter,
}
int dsa_port_mtu_change(struct dsa_port *dp, int new_mtu,
- bool propagate_upstream)
+ bool targeted_match)
{
struct dsa_notifier_mtu_info info = {
.sw_index = dp->ds->index,
- .propagate_upstream = propagate_upstream,
+ .targeted_match = targeted_match,
.port = dp->index,
.mtu = new_mtu,
};
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index ac2ca5f75af35..5e668e529575d 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1586,14 +1586,15 @@ int dsa_slave_change_mtu(struct net_device *dev, int new_mtu)
goto out_master_failed;
/* We only need to propagate the MTU of the CPU port to
- * upstream switches.
+ * upstream switches, so create a non-targeted notifier which
+ * updates all switches.
*/
- err = dsa_port_mtu_change(cpu_dp, cpu_mtu, true);
+ err = dsa_port_mtu_change(cpu_dp, cpu_mtu, false);
if (err)
goto out_cpu_failed;
}
- err = dsa_port_mtu_change(dp, new_mtu, false);
+ err = dsa_port_mtu_change(dp, new_mtu, true);
if (err)
goto out_port_failed;
@@ -1607,7 +1608,7 @@ int dsa_slave_change_mtu(struct net_device *dev, int new_mtu)
if (new_master_mtu != old_master_mtu)
dsa_port_mtu_change(cpu_dp, old_master_mtu -
dsa_tag_protocol_overhead(cpu_dp->tag_ops),
- true);
+ false);
out_cpu_failed:
if (new_master_mtu != old_master_mtu)
dev_set_mtu(master, old_master_mtu);
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 8b601ced6b45f..75f567390a6b6 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -52,10 +52,13 @@ static int dsa_switch_ageing_time(struct dsa_switch *ds,
static bool dsa_switch_mtu_match(struct dsa_switch *ds, int port,
struct dsa_notifier_mtu_info *info)
{
- if (ds->index == info->sw_index)
- return (port == info->port) || dsa_is_dsa_port(ds, port);
+ if (ds->index == info->sw_index && port == info->port)
+ return true;
- if (!info->propagate_upstream)
+ /* Do not propagate to other switches in the tree if the notifier was
+ * targeted for a single switch.
+ */
+ if (info->targeted_match)
return false;
if (dsa_is_dsa_port(ds, port) || dsa_is_cpu_port(ds, port))
From f9bcdc362c7776b875c0f390e982cbac597d660f Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Mon, 21 Jun 2021 19:42:19 +0300
Subject: [PATCH 6/6] net: dsa: remove cross-chip support from the MRP
notifiers
With MRP hardware assist being supported only by the ocelot switch
family, which by design does not support cross-chip bridging, the
current match functions are at best a guess and have not been confirmed
in any way to do anything relevant in a multi-switch topology.
Drop the code and make the notifiers match only on the targeted switch
port.
Cc: Horatiu Vultur <horatiu.vultur@microchip.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/dsa/switch.c | 55 ++++++------------------------------------------
1 file changed, 7 insertions(+), 48 deletions(-)
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 75f567390a6b6..c1e5afafe6332 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -346,36 +346,16 @@ static int dsa_switch_change_tag_proto(struct dsa_switch *ds,
return 0;
}
-static bool dsa_switch_mrp_match(struct dsa_switch *ds, int port,
- struct dsa_notifier_mrp_info *info)
-{
- if (ds->index == info->sw_index && port == info->port)
- return true;
-
- if (dsa_is_dsa_port(ds, port))
- return true;
-
- return false;
-}
-
static int dsa_switch_mrp_add(struct dsa_switch *ds,
struct dsa_notifier_mrp_info *info)
{
- int err = 0;
- int port;
-
if (!ds->ops->port_mrp_add)
return -EOPNOTSUPP;
- for (port = 0; port < ds->num_ports; port++) {
- if (dsa_switch_mrp_match(ds, port, info)) {
- err = ds->ops->port_mrp_add(ds, port, info->mrp);
- if (err)
- break;
- }
- }
+ if (ds->index == info->sw_index)
+ return ds->ops->port_mrp_add(ds, info->port, info->mrp);
- return err;
+ return 0;
}
static int dsa_switch_mrp_del(struct dsa_switch *ds,
@@ -390,39 +370,18 @@ static int dsa_switch_mrp_del(struct dsa_switch *ds,
return 0;
}
-static bool
-dsa_switch_mrp_ring_role_match(struct dsa_switch *ds, int port,
- struct dsa_notifier_mrp_ring_role_info *info)
-{
- if (ds->index == info->sw_index && port == info->port)
- return true;
-
- if (dsa_is_dsa_port(ds, port))
- return true;
-
- return false;
-}
-
static int
dsa_switch_mrp_add_ring_role(struct dsa_switch *ds,
struct dsa_notifier_mrp_ring_role_info *info)
{
- int err = 0;
- int port;
-
if (!ds->ops->port_mrp_add)
return -EOPNOTSUPP;
- for (port = 0; port < ds->num_ports; port++) {
- if (dsa_switch_mrp_ring_role_match(ds, port, info)) {
- err = ds->ops->port_mrp_add_ring_role(ds, port,
- info->mrp);
- if (err)
- break;
- }
- }
+ if (ds->index == info->sw_index)
+ return ds->ops->port_mrp_add_ring_role(ds, info->port,
+ info->mrp);
- return err;
+ return 0;
}
static int