[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20220215170218.2032432-12-vladimir.oltean@nxp.com>
Date: Tue, 15 Feb 2022 19:02:18 +0200
From: Vladimir Oltean <vladimir.oltean@....com>
To: netdev@...r.kernel.org
Cc: Jakub Kicinski <kuba@...nel.org>,
"David S. Miller" <davem@...emloft.net>,
Florian Fainelli <f.fainelli@...il.com>,
Andrew Lunn <andrew@...n.ch>,
Vivien Didelot <vivien.didelot@...il.com>,
Vladimir Oltean <olteanv@...il.com>,
Roopa Prabhu <roopa@...dia.com>,
Nikolay Aleksandrov <nikolay@...dia.com>,
Jiri Pirko <jiri@...dia.com>, Ido Schimmel <idosch@...dia.com>,
Rafael Richter <rafael.richter@....de>,
Daniel Klauer <daniel.klauer@....de>,
Tobias Waldekranz <tobias@...dekranz.com>
Subject: [PATCH v3 net-next 11/11] net: dsa: offload bridge port VLANs on foreign interfaces
DSA now explicitly handles VLANs installed with the 'self' flag on the
bridge as host VLANs, instead of just replicating every bridge port VLAN
also on the CPU port and never deleting it, which is what it did before.
However, this leaves a corner case uncovered, as explained by
Tobias Waldekranz:
https://patchwork.kernel.org/project/netdevbpf/patch/20220209213044.2353153-6-vladimir.oltean@nxp.com/#24735260
Forwarding towards a bridge port VLAN installed on a bridge port foreign
to DSA (separate NIC, Wi-Fi AP) used to work by virtue of the fact that
DSA itself needed to have at least one port in that VLAN (therefore, it
also had the CPU port in said VLAN). However, now that the CPU port may
not be member of all VLANs that user ports are members of, we need to
ensure this isn't the case if software forwarding to a foreign interface
is required.
The solution is to treat bridge port VLANs on standalone interfaces in
the exact same way as host VLANs. From DSA's perspective, there is no
difference between local termination and software forwarding; packets in
that VLAN must reach the CPU in both cases.
Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
---
v2->v3:
- merge dsa_slave_host_vlan_{add,del}() with
dsa_slave_foreign_vlan_{add,del}(), since now they do the same thing,
because the host_vlan functions no longer need to mangle the vlan
BRENTRY flags and bool changed.
v1->v2:
- patch is new
net/dsa/dsa2.c | 6 ++++++
net/dsa/slave.c | 51 ++++++++++++++++++++++++-------------------------
2 files changed, 31 insertions(+), 26 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 1df8c2356463..408b79a28cd4 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -565,6 +565,7 @@ static void dsa_port_teardown(struct dsa_port *dp)
struct dsa_switch *ds = dp->ds;
struct dsa_mac_addr *a, *tmp;
struct net_device *slave;
+ struct dsa_vlan *v, *n;
if (!dp->setup)
return;
@@ -605,6 +606,11 @@ static void dsa_port_teardown(struct dsa_port *dp)
kfree(a);
}
+ list_for_each_entry_safe(v, n, &dp->vlans, list) {
+ list_del(&v->list);
+ kfree(v);
+ }
+
dp->setup = false;
}
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 734c381f89ca..f61e6b72ffbb 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -376,6 +376,9 @@ static int dsa_slave_vlan_add(struct net_device *dev,
return dsa_port_vlan_add(dp, vlan, extack);
}
+/* Offload a VLAN installed on the bridge or on a foreign interface by
+ * installing it as a VLAN towards the CPU port.
+ */
static int dsa_slave_host_vlan_add(struct net_device *dev,
const struct switchdev_obj *obj,
struct netlink_ext_ack *extack)
@@ -383,6 +386,10 @@ static int dsa_slave_host_vlan_add(struct net_device *dev,
struct dsa_port *dp = dsa_slave_to_port(dev);
struct switchdev_obj_port_vlan vlan;
+ /* Do nothing if this is a software bridge */
+ if (!dp->bridge)
+ return -EOPNOTSUPP;
+
if (dsa_port_skip_vlan_configuration(dp)) {
NL_SET_ERR_MSG_MOD(extack, "skipping configuration of VLAN");
return 0;
@@ -422,17 +429,10 @@ static int dsa_slave_port_obj_add(struct net_device *dev, const void *ctx,
err = dsa_port_host_mdb_add(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
break;
case SWITCHDEV_OBJ_ID_PORT_VLAN:
- if (netif_is_bridge_master(obj->orig_dev)) {
- if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
- return -EOPNOTSUPP;
-
- err = dsa_slave_host_vlan_add(dev, obj, extack);
- } else {
- if (!dsa_port_offloads_bridge_port(dp, obj->orig_dev))
- return -EOPNOTSUPP;
-
+ if (dsa_port_offloads_bridge_port(dp, obj->orig_dev))
err = dsa_slave_vlan_add(dev, obj, extack);
- }
+ else
+ err = dsa_slave_host_vlan_add(dev, obj, extack);
break;
case SWITCHDEV_OBJ_ID_MRP:
if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
@@ -475,6 +475,10 @@ static int dsa_slave_host_vlan_del(struct net_device *dev,
struct dsa_port *dp = dsa_slave_to_port(dev);
struct switchdev_obj_port_vlan *vlan;
+ /* Do nothing if this is a software bridge */
+ if (!dp->bridge)
+ return -EOPNOTSUPP;
+
if (dsa_port_skip_vlan_configuration(dp))
return 0;
@@ -506,17 +510,10 @@ static int dsa_slave_port_obj_del(struct net_device *dev, const void *ctx,
err = dsa_port_host_mdb_del(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
break;
case SWITCHDEV_OBJ_ID_PORT_VLAN:
- if (netif_is_bridge_master(obj->orig_dev)) {
- if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
- return -EOPNOTSUPP;
-
- err = dsa_slave_host_vlan_del(dev, obj);
- } else {
- if (!dsa_port_offloads_bridge_port(dp, obj->orig_dev))
- return -EOPNOTSUPP;
-
+ if (dsa_port_offloads_bridge_port(dp, obj->orig_dev))
err = dsa_slave_vlan_del(dev, obj);
- }
+ else
+ err = dsa_slave_host_vlan_del(dev, obj);
break;
case SWITCHDEV_OBJ_ID_MRP:
if (!dsa_port_offloads_bridge_dev(dp, obj->orig_dev))
@@ -2547,14 +2544,16 @@ static int dsa_slave_switchdev_blocking_event(struct notifier_block *unused,
switch (event) {
case SWITCHDEV_PORT_OBJ_ADD:
- err = switchdev_handle_port_obj_add(dev, ptr,
- dsa_slave_dev_check,
- dsa_slave_port_obj_add);
+ err = switchdev_handle_port_obj_add_foreign(dev, ptr,
+ dsa_slave_dev_check,
+ dsa_foreign_dev_check,
+ dsa_slave_port_obj_add);
return notifier_from_errno(err);
case SWITCHDEV_PORT_OBJ_DEL:
- err = switchdev_handle_port_obj_del(dev, ptr,
- dsa_slave_dev_check,
- dsa_slave_port_obj_del);
+ err = switchdev_handle_port_obj_del_foreign(dev, ptr,
+ dsa_slave_dev_check,
+ dsa_foreign_dev_check,
+ dsa_slave_port_obj_del);
return notifier_from_errno(err);
case SWITCHDEV_PORT_ATTR_SET:
err = switchdev_handle_port_attr_set(dev, ptr,
--
2.25.1
Powered by blists - more mailing lists