[PATCH net v1] net: dsa: lantiq_gswip: Fix FDB add/remove on the CPU port

Martin Blumenstingl
2022-06-30T21:27:37Z

There's no bridge available when adding or removing a static FDB entry
for (towards) the CPU port. This is intentional because the CPU port is
always considered standalone, even if technically for the GSWIP driver
it's part of every bridge.

Handling FDB add/remove on the CPU port fixes the following message
during boot in OpenWrt:
port 4 failed to add <LAN MAC address> vid 1 to fdb: -22
as well as the following message during system shutdown:
port 4 failed to delete <LAN MAC address> vid 1 from fdb: -22

Use FID 0 (which is also the "default" FID) when adding/removing an FDB
entry for the CPU port. Testing with a BT Home Hub 5A shows that this
"default" FID works as expected:
- traffic from/to LAN (ports in a bridge) is not seen on the WAN port
(standalone port)
- traffic from/to the WAN port (standalone port) is not seen on the LAN
(ports in a bridge) ports
- traffic from/to LAN is not seen on another LAN port with a different
VLAN
- traffic from/to one LAN port to another is still seen

Fixes: 58c59ef9e930c4 ("net: dsa: lantiq: Add Forwarding Database access")
Cc: stable@vger.kernel.org
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
This patch is "minimalistic" on purpose: the goal is to have it
backported to Linux 5.15. Linux 5.15 doesn't have
dsa_fdb_present_in_other_db() or struct dsa_db yet. Once this patch has
been accepted I will work on implementing FDB isolation for the Lantiq
GSWIP driver.

Hauke, I hope I considered all test-cases which you find relevant. If not
then please let me know.

drivers/net/dsa/lantiq_gswip.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index e531b93f3cb2..9dab28903af0 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1365,19 +1365,26 @@ static int gswip_port_fdb(struct dsa_switch *ds, int port,
int i;
int err;

- if (!bridge)
- return -EINVAL;
-
- for (i = max_ports; i < ARRAY_SIZE(priv->vlans); i++) {
- if (priv->vlans[i].bridge == bridge) {
- fid = priv->vlans[i].fid;
- break;
+ if (bridge) {
+ for (i = max_ports; i < ARRAY_SIZE(priv->vlans); i++) {
+ if (priv->vlans[i].bridge == bridge) {
+ fid = priv->vlans[i].fid;
+ break;
+ }
}
- }

- if (fid == -1) {
- dev_err(priv->dev, "Port not part of a bridge\n");
- return -EINVAL;
+ if (fid == -1) {
+ dev_err(priv->dev, "Port not part of a bridge\n");
+ return -EINVAL;
+ }
+ } else if (dsa_is_cpu_port(ds, port)) {
+ /* Use FID 0 which is the "default" and used as fallback. This
+ * is not used by any standalone port or a bridge, so we can
+ * safely use it for the CPU port.
+ */
+ fid = 0;
+ } else {
+ return -EOPNOTSUPP;
}

mac_bridge.table = GSWIP_TABLE_MAC_BRIDGE;
--
2.37.0