| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 5 Jul 2022 20:31:14 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: netdev@...r.kernel.org
Cc: "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Xiaoliang Yang <xiaoliang.yang_1@....com>,
Claudiu Manoil <claudiu.manoil@....com>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
UNGLinuxDriver@...rochip.com, Andrew Lunn <andrew@...n.ch>,
Vivien Didelot <vivien.didelot@...il.com>,
Florian Fainelli <f.fainelli@...il.com>,
Petr Machata <petrm@...dia.com>,
Ido Schimmel <idosch@...dia.com>,
Woojung Huh <woojung.huh@...rochip.com>,
Oleksij Rempel <linux@...pel-privat.de>,
Arun Ramadoss <arun.ramadoss@...rochip.com>,
Hauke Mehrtens <hauke@...ke-m.de>,
Martin Blumenstingl <martin.blumenstingl@...glemail.com>
Subject: [RFC PATCH net-next 3/3] net: dsa: never skip VLAN configuration
Stop protecting DSA drivers from switchdev VLAN notifications emitted
while the bridge has vlan_filtering 0, by deleting the deprecated bool
ds->configure_vlan_while_not_filtering opt-in. Now all DSA drivers see
all notifications and should save the bridge PVID until they need to
commit it to hardware.
The 2 remaining unconverted drivers are the gswip and the Microchip KSZ
family. They are both probably broken and would need changing as far as
I can see:
- For lantiq_gswip, after the initial call path
-> gswip_port_bridge_join
-> gswip_vlan_add_unaware
-> gswip_switch_w(priv, 0, GSWIP_PCE_DEFPVID(port));
nobody seems to prevent a future call path
-> gswip_port_vlan_add
-> gswip_vlan_add_aware
-> gswip_switch_w(priv, idx, GSWIP_PCE_DEFPVID(port));
- For ksz9477, REG_PORT_DEFAULT_VID seems to be only modified by the
bridge VLAN add/del handlers, so there is no logic to update it on
VLAN awareness change. I don't know exactly how the switch behaves
after ksz9477_port_vlan_filtering() is called with "false".
If packets are classified to the REG_PORT_DEFAULT_VID, then it is
broken. Similar thing can be said for KSZ8.
In any case, see commit 8b6836d82470 ("net: dsa: mv88e6xxx: keep the
pvid at 0 when VLAN-unaware") for an example of how to deal with the
problem, and test pvid_change() in
tools/testing/selftests/drivers/net/dsa/bridge_vlan_unaware.sh for how
to check whether the driver behaves correctly. I don't have the hardware
to test any changes.
Cc: Arun Ramadoss <arun.ramadoss@...rochip.com>
Cc: Hauke Mehrtens <hauke@...ke-m.de>
Cc: Martin Blumenstingl <martin.blumenstingl@...glemail.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
---
drivers/net/dsa/lantiq_gswip.c | 2 --
drivers/net/dsa/microchip/ksz_common.c | 2 --
include/net/dsa.h | 7 -------
net/dsa/dsa2.c | 2 --
net/dsa/dsa_priv.h | 1 -
net/dsa/port.c | 14 --------------
net/dsa/slave.c | 22 +---------------------
7 files changed, 1 insertion(+), 49 deletions(-)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index e531b93f3cb2..86acf8a4e53c 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -893,8 +893,6 @@ static int gswip_setup(struct dsa_switch *ds)
gswip_port_enable(ds, cpu_port, NULL);
- ds->configure_vlan_while_not_filtering = false;
-
return 0;
}
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 28d7cb2ce98f..561a515c7cb8 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -927,8 +927,6 @@ static int ksz_setup(struct dsa_switch *ds)
ksz_init_mib_timer(dev);
- ds->configure_vlan_while_not_filtering = false;
-
if (dev->dev_ops->setup) {
ret = dev->dev_ops->setup(ds);
if (ret)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index b902b31bebce..2ed1c2db4037 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -400,13 +400,6 @@ struct dsa_switch {
/* Keep VLAN filtering enabled on ports not offloading any upper */
u32 needs_standalone_vlan_filtering:1;
- /* Pass .port_vlan_add and .port_vlan_del to drivers even for bridges
- * that have vlan_filtering=0. All drivers should ideally set this (and
- * then the option would get removed), but it is unknown whether this
- * would break things or not.
- */
- u32 configure_vlan_while_not_filtering:1;
-
/* If the switch driver always programs the CPU port as egress tagged
* despite the VLAN configuration indicating otherwise, then setting
* @untag_bridge_pvid will force the DSA receive path to pop the
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index cac48a741f27..47a2e60b6080 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -890,8 +890,6 @@ static int dsa_switch_setup(struct dsa_switch *ds)
if (err)
goto unregister_devlink_ports;
- ds->configure_vlan_while_not_filtering = true;
-
err = ds->ops->setup(ds);
if (err < 0)
goto unregister_notifier;
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index d9722e49864b..63191db45d02 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -224,7 +224,6 @@ void dsa_port_pre_lag_leave(struct dsa_port *dp, struct net_device *lag_dev);
void dsa_port_lag_leave(struct dsa_port *dp, struct net_device *lag_dev);
int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
struct netlink_ext_ack *extack);
-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_mst_enable(struct dsa_port *dp, bool on,
struct netlink_ext_ack *extack);
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 3738f2d40a0b..79853f673b65 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -834,20 +834,6 @@ int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
return err;
}
-/* This enforces legacy behavior for switch drivers which assume they can't
- * receive VLAN configuration when enslaved to a bridge with vlan_filtering=0
- */
-bool dsa_port_skip_vlan_configuration(struct dsa_port *dp)
-{
- struct net_device *br = dsa_port_bridge_dev_get(dp);
- struct dsa_switch *ds = dp->ds;
-
- if (!br)
- return false;
-
- return !ds->configure_vlan_while_not_filtering && !br_vlan_enabled(br);
-}
-
int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock)
{
unsigned long ageing_jiffies = clock_t_to_jiffies(ageing_clock);
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index ad6a6663feeb..d1284f8684d8 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -533,11 +533,6 @@ static int dsa_slave_vlan_add(struct net_device *dev,
struct switchdev_obj_port_vlan *vlan;
int err;
- if (dsa_port_skip_vlan_configuration(dp)) {
- NL_SET_ERR_MSG_MOD(extack, "skipping configuration of VLAN");
- return 0;
- }
-
vlan = SWITCHDEV_OBJ_PORT_VLAN(obj);
/* Deny adding a bridge VLAN when there is already an 802.1Q upper with
@@ -571,11 +566,6 @@ static int dsa_slave_host_vlan_add(struct net_device *dev,
if (!dp->bridge)
return -EOPNOTSUPP;
- if (dsa_port_skip_vlan_configuration(dp)) {
- NL_SET_ERR_MSG_MOD(extack, "skipping configuration of VLAN");
- return 0;
- }
-
vlan = *SWITCHDEV_OBJ_PORT_VLAN(obj);
/* Even though drivers often handle CPU membership in special ways,
@@ -642,9 +632,6 @@ static int dsa_slave_vlan_del(struct net_device *dev,
struct dsa_port *dp = dsa_slave_to_port(dev);
struct switchdev_obj_port_vlan *vlan;
- if (dsa_port_skip_vlan_configuration(dp))
- return 0;
-
vlan = SWITCHDEV_OBJ_PORT_VLAN(obj);
return dsa_port_vlan_del(dp, vlan);
@@ -660,9 +647,6 @@ static int dsa_slave_host_vlan_del(struct net_device *dev,
if (!dp->bridge)
return -EOPNOTSUPP;
- if (dsa_port_skip_vlan_configuration(dp))
- return 0;
-
vlan = SWITCHDEV_OBJ_PORT_VLAN(obj);
return dsa_port_host_vlan_del(dp, vlan);
@@ -1655,11 +1639,7 @@ static int dsa_slave_clear_vlan(struct net_device *vdev, int vid, void *arg)
* - no VLAN (any 8021q upper is a software VLAN)
*
* - If under a vlan_filtering=0 bridge which it offload:
- * - if ds->configure_vlan_while_not_filtering = true (default):
- * - the bridge VLANs. These VLANs are committed to hardware but inactive.
- * - else (deprecated):
- * - no VLAN. The bridge VLANs are not restored when VLAN awareness is
- * enabled, so this behavior is broken and discouraged.
+ * - the bridge VLANs. These VLANs are committed to hardware but inactive.
*
* - If under a vlan_filtering=1 bridge which it offload:
* - the bridge VLANs
--
2.25.1
Powered by blists - more mailing lists