mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net v3] net: ti: am65-cpsw-switchdev: flush dynamic FDB entries by port on delete
@ 2026-10-01  9:08 MD Danish Anwar
  2026-10-01  9:08 ` MD Danish Anwar
  2026-10-01  9:13 ` netdev-bot+sinfo
  0 siblings, 2 replies; 4+ messages in thread
From: MD Danish Anwar @ 2026-10-01  9:08 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Siddharth Vadapalli, Roger Quadros, MD Danish Anwar,
	Kees Cook, Stefan Wiehler, Arnd Bergmann, Simon Horman,
	Chintan Vankar, Vignesh Raghavendra
  Cc: netdev, linux-kernel, linux-omap

Dynamic (hardware-learned) ALE entries are not removed on FDB flush and
sit stale until the ALE hardware ages them out ~30 seconds later.

The delete path gated on added_by_user, the same check used for ADD, so
every dynamic delete was dropped before reaching cpsw_ale_del_ucast().
Dynamic deletes are the only way to remove a hardware-learned entry
early, so drop that gate for the delete path.

Removing the gate alone is not enough: ALE_VLAN_AWARE is always on in
switch mode, so a dynamic entry is stored under its real, nonzero vid.
With the bridge's vlan_filtering off, the bridge core never learns that
vid and reports vid=0 on delete. cpsw_ale_del_ucast()'s exact (addr, vid)
match then never finds the row, returns -ENOENT, and the entry stays.

Add cpsw_ale_del_ucast_dynamic_by_port(), which matches by (addr, port)
instead of the usual (addr, vid). When the caller passes vid=0 it is
treated as "no vid filter" and deletes every dynamic entry for that MAC
on that port across all VLANs. vid=0 is only ever passed when
vlan_filtering is off, and in that case which vid the entry was learned
under does not matter since the bridge is not separating traffic by
VLAN anyway. When vlan_filtering is on and a trunk port learns the same
MAC under multiple VLANs, the delete call comes with the real, nonzero
vid, so only that one entry is removed. User-added entries are
unaffected and keep using the existing exact-match cpsw_ale_del_ucast();
this change only touches dynamic learned entries.

The host-MAC-to-HOST_PORT_NUM remap used to run before this new by-port
lookup, so a dynamically learned copy of the port's own slave MAC (e.g.
from a loop) would get redirected to the host port instead of the port
that actually learned it. Move the remap into the user-added branch
only.

Take a runtime PM reference around the ALE accesses in the switchdev
event work. On port-down these work items can run after the last PM
reference has been dropped, so an ADD/DEL that reaches the ALE needs its
own reference; log if the resume fails. The reference is taken only
after the is_local check, since is_local events never touch the ALE and
do not need to resume the device.

Apply the same fix to cpsw_switchdev.c (older CPSW driver), which has
the identical issue.

Fixes: 86e8b070b25e ("net: ti: am65-cpsw-nuss: Add switchdev support")
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
v3 - v2:
Addressed comments from Sashiko [1]
Sashiko gave 2 medium genuine comments, 1 pre-existing high and 1 pre-existing
medium comment.

Addressed comments
1) This patch exposes a ageing related side effect, which is a trade-off.
The same has been documented in the commit message.
2) Runtime PM reference has been added.
3) The same fix has been extended to cpsw_switchdev.c driver as well.
4) A pre-existing race was highlighted by Sashiko, that has not been fixed
and can be done via a follow up patch.
5) Local testing with Sashiko highlighted some changes which has been added
to the commit.

v2 - v1:
Address comments recieved from Sashiko [2]

Sashiko had 1 High and 2 Medium comments on v1. 1 High and 1 medium is addressed
in this patch. 1 Medium is a acceptable behaviour and not an actual issue.
Sashiko also had 1 Low and 1 Medium pre-existing issues. Those two
pre-existing issues are still there and can be planned to fix later but
not as part of this patch.

Addressed comments
1) Dynamic delete with non zero vid was going through cpsw_ale_del_ucast()
doesn't use port based matching and no ucast_type filtering happens, so
the row that gets blanked may belong to a different port or be an
ALE_UCAST_PERSISTANT row. This is fixed by calling
cpsw_ale_del_ucast_dynamic_by_port() for all dynamic entries. vid handling
is taken care by this API.

2) Added EXPORT_SYMBOL_GPL() for cpsw_ale_del_ucast_dynamic_by_port()

3) There was a comment about cost associated with full ALE walk for each
dynamic delete. The ALE size is small (max 512 entries on the largest
supported device), so no change is done here.

[1] https://lore.kernel.org/all/179057463349.3145.13175831323497920715@kernel.org/
[2] https://lore.kernel.org/all/179006497666.2160803.14768308117153644313@kernel.org/
v1 https://lore.kernel.org/all/20260918075926.3616434-1-danishanwar@ti.com/
v2 https://lore.kernel.org/all/20260924052146.594157-1-danishanwar@ti.com/#t

 drivers/net/ethernet/ti/am65-cpsw-switchdev.c | 34 +++++++++++--
 drivers/net/ethernet/ti/cpsw_ale.c            | 49 ++++++++++++++++++-
 drivers/net/ethernet/ti/cpsw_ale.h            |  3 ++
 drivers/net/ethernet/ti/cpsw_switchdev.c      | 34 +++++++++++--
 4 files changed, 109 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-switchdev.c b/drivers/net/ethernet/ti/am65-cpsw-switchdev.c
index 53cdac272b583..bc4e19e2373f5 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-switchdev.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-switchdev.c
@@ -9,6 +9,7 @@
 #include <linux/if_bridge.h>
 #include <linux/netdevice.h>
 #include <linux/workqueue.h>
+#include <linux/pm_runtime.h>
 #include <net/switchdev.h>
 
 #include "am65-cpsw-nuss.h"
@@ -371,6 +372,7 @@ static void am65_cpsw_switchdev_event_work(struct work_struct *work)
 	struct switchdev_notifier_fdb_info *fdb;
 	struct am65_cpsw_common *cpsw = port->common;
 	int port_id = port->port_id;
+	int ret;
 
 	rtnl_lock();
 	switch (switchdev_work->event) {
@@ -383,12 +385,20 @@ static void am65_cpsw_switchdev_event_work(struct work_struct *work)
 
 		if (!fdb->added_by_user || fdb->is_local)
 			break;
+
+		ret = pm_runtime_resume_and_get(cpsw->dev);
+		if (ret < 0) {
+			netdev_err(port->ndev, "%s: failed to resume device: %d\n", __func__, ret);
+			break;
+		}
+
 		if (memcmp(port->slave.mac_addr, (u8 *)fdb->addr, ETH_ALEN) == 0)
 			port_id = HOST_PORT_NUM;
 
 		cpsw_ale_add_ucast(cpsw->ale, (u8 *)fdb->addr, port_id,
 				   fdb->vid ? ALE_VLAN : 0, fdb->vid);
 		am65_cpsw_fdb_offload_notify(port->ndev, fdb);
+		pm_runtime_put(cpsw->dev);
 		break;
 	case SWITCHDEV_FDB_DEL_TO_DEVICE:
 		fdb = &switchdev_work->fdb_info;
@@ -397,13 +407,27 @@ static void am65_cpsw_switchdev_event_work(struct work_struct *work)
 			   fdb->addr, fdb->vid, fdb->added_by_user,
 			   fdb->offloaded, port_id);
 
-		if (!fdb->added_by_user || fdb->is_local)
+		if (fdb->is_local)
 			break;
-		if (memcmp(port->slave.mac_addr, (u8 *)fdb->addr, ETH_ALEN) == 0)
-			port_id = HOST_PORT_NUM;
 
-		cpsw_ale_del_ucast(cpsw->ale, (u8 *)fdb->addr, port_id,
-				   fdb->vid ? ALE_VLAN : 0, fdb->vid);
+		ret = pm_runtime_resume_and_get(cpsw->dev);
+		if (ret < 0) {
+			netdev_err(port->ndev, "%s: failed to resume device: %d\n", __func__, ret);
+			break;
+		}
+
+		if (!fdb->added_by_user) {
+			cpsw_ale_del_ucast_dynamic_by_port(cpsw->ale,
+							   (u8 *)fdb->addr,
+							   port_id,
+							   fdb->vid);
+		} else {
+			if (memcmp(port->slave.mac_addr, (u8 *)fdb->addr, ETH_ALEN) == 0)
+				port_id = HOST_PORT_NUM;
+			cpsw_ale_del_ucast(cpsw->ale, (u8 *)fdb->addr, port_id,
+					   fdb->vid ? ALE_VLAN : 0, fdb->vid);
+		}
+		pm_runtime_put(cpsw->dev);
 		break;
 	default:
 		break;
diff --git a/drivers/net/ethernet/ti/cpsw_ale.c b/drivers/net/ethernet/ti/cpsw_ale.c
index e202bba494807..d3d6a1ef894ac 100644
--- a/drivers/net/ethernet/ti/cpsw_ale.c
+++ b/drivers/net/ethernet/ti/cpsw_ale.c
@@ -249,7 +249,7 @@ DEFINE_ALE_FIELD_SET(mcast_state,	62,	2)
 DEFINE_ALE_FIELD1(port_mask,		66)
 DEFINE_ALE_FIELD(super,			65,	1)
 DEFINE_ALE_FIELD(ucast_type,		62,     2)
-DEFINE_ALE_FIELD1_SET(port_num,		66)
+DEFINE_ALE_FIELD1(port_num,		66)
 DEFINE_ALE_FIELD_SET(blocked,		65,     1)
 DEFINE_ALE_FIELD_SET(secure,		64,     1)
 DEFINE_ALE_FIELD_GET(mcast,		40,	1)
@@ -441,6 +441,53 @@ static int cpsw_ale_find_ageable(struct cpsw_ale *ale)
 	return -ENOENT;
 }
 
+/* Delete dynamic ucast entries matching addr+port. vid is an exact match
+ * when nonzero; vid == 0 is a wildcard that deletes every matching dynamic
+ * entry for addr+port across all vlans, instead of matching vlan id 0
+ * literally.
+ */
+int cpsw_ale_del_ucast_dynamic_by_port(struct cpsw_ale *ale, const u8 *addr,
+				       int port, u16 vid)
+{
+	u32 ale_entry[ALE_ENTRY_WORDS];
+	int type, ucast_type, idx;
+	u8 entry_addr[6];
+	int deleted = 0;
+	int entry_vid;
+
+	for (idx = 0; idx < ale->params.ale_entries; idx++) {
+		cpsw_ale_read(ale, idx, ale_entry);
+		type = cpsw_ale_get_entry_type(ale_entry);
+		if (type != ALE_TYPE_ADDR && type != ALE_TYPE_VLAN_ADDR)
+			continue;
+		if (cpsw_ale_get_mcast(ale_entry))
+			continue;
+		ucast_type = cpsw_ale_get_ucast_type(ale_entry);
+		if (ucast_type == ALE_UCAST_PERSISTANT ||
+		    ucast_type == ALE_UCAST_OUI)
+			continue;
+		if (cpsw_ale_get_port_num(ale_entry, ale->port_num_bits) != port)
+			continue;
+		cpsw_ale_get_addr(ale_entry, entry_addr);
+		if (!ether_addr_equal(entry_addr, addr))
+			continue;
+		entry_vid = cpsw_ale_get_vlan_id(ale_entry);
+		if (vid && entry_vid != vid)
+			continue;
+
+		memset(ale_entry, 0, sizeof(ale_entry));
+		cpsw_ale_set_entry_type(ale_entry, ALE_TYPE_FREE);
+		cpsw_ale_write(ale, idx, ale_entry);
+		deleted++;
+
+		if (vid)
+			break;
+	}
+
+	return deleted ? 0 : -ENOENT;
+}
+EXPORT_SYMBOL_GPL(cpsw_ale_del_ucast_dynamic_by_port);
+
 static void cpsw_ale_flush_mcast(struct cpsw_ale *ale, u32 *ale_entry,
 				 int port_mask)
 {
diff --git a/drivers/net/ethernet/ti/cpsw_ale.h b/drivers/net/ethernet/ti/cpsw_ale.h
index 87b7d1b3a34a9..ed1921b428e50 100644
--- a/drivers/net/ethernet/ti/cpsw_ale.h
+++ b/drivers/net/ethernet/ti/cpsw_ale.h
@@ -166,6 +166,9 @@ int cpsw_ale_add_ucast(struct cpsw_ale *ale, const u8 *addr, int port,
 		       int flags, u16 vid);
 int cpsw_ale_del_ucast(struct cpsw_ale *ale, const u8 *addr, int port,
 		       int flags, u16 vid);
+/* vid == 0 wildcards across all vlans; see cpsw_ale.c for details */
+int cpsw_ale_del_ucast_dynamic_by_port(struct cpsw_ale *ale, const u8 *addr,
+				       int port, u16 vid);
 int cpsw_ale_add_mcast(struct cpsw_ale *ale, const u8 *addr, int port_mask,
 		       int flags, u16 vid, int mcast_state);
 int cpsw_ale_del_mcast(struct cpsw_ale *ale, const u8 *addr, int port_mask,
diff --git a/drivers/net/ethernet/ti/cpsw_switchdev.c b/drivers/net/ethernet/ti/cpsw_switchdev.c
index 7e06aac388a60..7fbebe666da81 100644
--- a/drivers/net/ethernet/ti/cpsw_switchdev.c
+++ b/drivers/net/ethernet/ti/cpsw_switchdev.c
@@ -10,6 +10,7 @@
 #include <linux/if_bridge.h>
 #include <linux/netdevice.h>
 #include <linux/workqueue.h>
+#include <linux/pm_runtime.h>
 #include <net/switchdev.h>
 
 #include "cpsw.h"
@@ -381,6 +382,7 @@ static void cpsw_switchdev_event_work(struct work_struct *work)
 	struct switchdev_notifier_fdb_info *fdb;
 	struct cpsw_common *cpsw = priv->cpsw;
 	int port = priv->emac_port;
+	int ret;
 
 	rtnl_lock();
 	switch (switchdev_work->event) {
@@ -393,12 +395,20 @@ static void cpsw_switchdev_event_work(struct work_struct *work)
 
 		if (!fdb->added_by_user || fdb->is_local)
 			break;
+
+		ret = pm_runtime_resume_and_get(cpsw->dev);
+		if (ret < 0) {
+			dev_err(cpsw->dev, "%s: failed to resume device: %d\n", __func__, ret);
+			break;
+		}
+
 		if (memcmp(priv->mac_addr, (u8 *)fdb->addr, ETH_ALEN) == 0)
 			port = HOST_PORT_NUM;
 
 		cpsw_ale_add_ucast(cpsw->ale, (u8 *)fdb->addr, port,
 				   fdb->vid ? ALE_VLAN : 0, fdb->vid);
 		cpsw_fdb_offload_notify(priv->ndev, fdb);
+		pm_runtime_put(cpsw->dev);
 		break;
 	case SWITCHDEV_FDB_DEL_TO_DEVICE:
 		fdb = &switchdev_work->fdb_info;
@@ -407,13 +417,27 @@ static void cpsw_switchdev_event_work(struct work_struct *work)
 			fdb->addr, fdb->vid, fdb->added_by_user,
 			fdb->offloaded, port);
 
-		if (!fdb->added_by_user || fdb->is_local)
+		if (fdb->is_local)
 			break;
-		if (memcmp(priv->mac_addr, (u8 *)fdb->addr, ETH_ALEN) == 0)
-			port = HOST_PORT_NUM;
 
-		cpsw_ale_del_ucast(cpsw->ale, (u8 *)fdb->addr, port,
-				   fdb->vid ? ALE_VLAN : 0, fdb->vid);
+		ret = pm_runtime_resume_and_get(cpsw->dev);
+		if (ret < 0) {
+			dev_err(cpsw->dev, "%s: failed to resume device: %d\n", __func__, ret);
+			break;
+		}
+
+		if (!fdb->added_by_user) {
+			cpsw_ale_del_ucast_dynamic_by_port(cpsw->ale,
+							   (u8 *)fdb->addr,
+							   port,
+							   fdb->vid);
+		} else {
+			if (memcmp(priv->mac_addr, (u8 *)fdb->addr, ETH_ALEN) == 0)
+				port = HOST_PORT_NUM;
+			cpsw_ale_del_ucast(cpsw->ale, (u8 *)fdb->addr, port,
+					   fdb->vid ? ALE_VLAN : 0, fdb->vid);
+		}
+		pm_runtime_put(cpsw->dev);
 		break;
 	default:
 		break;

base-commit: 7375d38364a9aa66fb31716bcefef38aecad75d8
-- 
2.34.1


^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-01  9:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-01  9:08 [PATCH net v3] net: ti: am65-cpsw-switchdev: flush dynamic FDB entries by port on delete MD Danish Anwar
2026-10-01  9:08 ` MD Danish Anwar
2026-10-01  9:13   ` netdev-bot+sinfo
2026-10-01  9:13 ` netdev-bot+sinfo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®