[PATCH net-next 0/2] net: dsa: fix host fdb refcount when an entry roams

[PATCH net-next 0/2] net: dsa: fix host fdb refcount when an entry roams

[Julius Bairaktaris]

When a bridge FDB entry roams between ports, br_fdb_update() notifies
switchdev of the deletion and the addition without a lock. With two
CPUs learning one address on different ports, DSA counts the host
address on its CPU port more or fewer times than the bridge has it,
which ends in "failed to delete ... from fdb: -2" or an address left on
the CPU port.

The earlier attempt took hash_lock in br_fdb_update(); Nikolay rejected
it because it slows down learning and roaming. This series leaves
br_fdb_update() and the rest of the bridge's learning path unchanged
and fixes DSA instead: each user port records the host addresses it
holds and, on every event, checks where the bridge has the entry now.
Patch 1 is the only bridge change: an RCU lookup that DSA calls from
its workqueue. Patch 2 replaces the counting.

The counting has been exposed to the race since commit 90dc8fd36078
("net: bridge: notify switchdev of disappearance of old FDB entry upon
migration"). The change redesigns how DSA tracks host addresses, so it
goes to net-next without a Fixes: tag.

Tested in QEMU with dsa_loop given FDB operations and assisted
learning, PROVE_LOCKING, PROVE_RCU and DEBUG_ATOMIC_SLEEP enabled, one
source address sent from two veth ports on two CPUs for 5 s, 5 boots
per kernel: net-next logs the -2 error and leaves the address on the
CPU port in 4 of 5 boots, this series in none, and it removes the
address when the user ports leave the bridge. A "bridge fdb replace" of
the address onto a user port leaves it on the CPU port in 5 of 5 boots
on net-next and removes it in 5 of 5 with this series. The kernel log
of every boot has no lockdep, RCU or atomic sleep reports.

Changes since the hash_lock patch:
- Leave br_fdb_update() unchanged (Nikolay).
- Sashiko's finding on that patch, a deletion that races the locked
  roam, has no counterpart here: DSA looks the entry up when it handles
  each event and treats a deleted entry as gone.
- Of the three findings it lists as pre-existing, the MAB and
  BR_FDB_LOCAL ones are in the bridge, which this series does not
  change; the peer-site one, a "bridge fdb replace" onto a DSA user port
  that notifies no deletion, is handled in patch 2.

Link: https://lore.kernel.org/netdev/20261004124254.3525496-1-julius@bairaktaris.de/

I used Claude (Anthropic) to help write these patches.

Julius Bairaktaris (2):
  net: bridge: switchdev: add br_switchdev_fdb_find_port()
  net: dsa: fix host fdb refcount when a bridge fdb entry roams

 include/linux/if_bridge.h |  13 +++
 net/bridge/br_switchdev.c |  63 +++++++++--
 net/dsa/port.c            |  36 +++++--
 net/dsa/port.h            |   6 ++
 net/dsa/user.c            | 216 +++++++++++++++++++++++++++++++++++---
 net/dsa/user.h            |   8 ++
 6 files changed, 309 insertions(+), 33 deletions(-)


base-commit: fa5196392691051963726787944db872420dfa75
-- 
2.53.0

[PATCH net-next 1/2] net: bridge: switchdev: add br_switchdev_fdb_find_port()

[Julius Bairaktaris]

Add an RCU lookup that returns the port of a non-local FDB entry that
switchdev is notified of: not locked, not added by the user as a
dynamic entry, and not learned by a switchdev driver.

A switchdev driver that handles FDB events from a workqueue cannot take
RTNL for br_fdb_find_port(), and has to ignore the entries whose
deletion it will not be notified of. The filter of
br_switchdev_fdb_notify() moves into a helper that both use, with no
functional change; the learning path does not change.

Signed-off-by: Julius Bairaktaris <julius@bairaktaris.de>
---
 include/linux/if_bridge.h | 13 ++++++++
 net/bridge/br_switchdev.c | 63 +++++++++++++++++++++++++++++++++------
 2 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index 75673b8bffcb8..e050b7e698fc9 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -201,6 +201,19 @@ static inline int br_mst_get_state(const struct net_device *dev, u16 msti,
 }
 #endif
 
+#if IS_ENABLED(CONFIG_BRIDGE) && IS_ENABLED(CONFIG_NET_SWITCHDEV)
+struct net_device *br_switchdev_fdb_find_port(const struct net_device *br_dev,
+					      const unsigned char *addr,
+					      u16 vid);
+#else
+static inline struct net_device *
+br_switchdev_fdb_find_port(const struct net_device *br_dev,
+			   const unsigned char *addr, u16 vid)
+{
+	return NULL;
+}
+#endif
+
 #if IS_ENABLED(CONFIG_BRIDGE)
 struct net_device *br_fdb_find_port(const struct net_device *br_dev,
 				    const unsigned char *addr,
diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
index 10f3aea6a1f82..02ca1cbcfede0 100644
--- a/net/bridge/br_switchdev.c
+++ b/net/bridge/br_switchdev.c
@@ -141,14 +141,10 @@ static void br_switchdev_fdb_populate(struct net_bridge *br,
 	item->info.ctx = ctx;
 }
 
-void
-br_switchdev_fdb_notify(struct net_bridge *br,
-			const struct net_bridge_fdb_entry *fdb, int type)
+static bool br_switchdev_fdb_notifies(const struct net_bridge_fdb_entry *fdb)
 {
-	struct switchdev_notifier_fdb_info item;
-
 	if (test_bit(BR_FDB_LOCKED, &fdb->flags))
-		return;
+		return false;
 
 	/* Entries with these flags were created using ndm_state == NUD_REACHABLE,
 	 * ndm_flags == NTF_MASTER( | NTF_STICKY), ext_flags == 0 by something
@@ -156,9 +152,58 @@ br_switchdev_fdb_notify(struct net_bridge *br,
 	 * Drivers don't know how to deal with these, so don't notify them to
 	 * avoid confusing them.
 	 */
-	if (test_bit(BR_FDB_ADDED_BY_USER, &fdb->flags) &&
-	    !test_bit(BR_FDB_STATIC, &fdb->flags) &&
-	    !test_bit(BR_FDB_ADDED_BY_EXT_LEARN, &fdb->flags))
+	return !test_bit(BR_FDB_ADDED_BY_USER, &fdb->flags) ||
+	       test_bit(BR_FDB_STATIC, &fdb->flags) ||
+	       test_bit(BR_FDB_ADDED_BY_EXT_LEARN, &fdb->flags);
+}
+
+/**
+ * br_switchdev_fdb_find_port - find the port of an entry switchdev knows of
+ * @br_dev: the bridge device
+ * @addr: the MAC address of the entry
+ * @vid: the VLAN of the entry
+ *
+ * Must be called under RCU; the device is valid until the read-side critical
+ * section ends.
+ *
+ * Return: the port of the non-local entry for @addr and @vid, or NULL if
+ * there is none or switchdev is not notified of it.
+ */
+struct net_device *br_switchdev_fdb_find_port(const struct net_device *br_dev,
+					      const unsigned char *addr,
+					      u16 vid)
+{
+	const struct net_bridge_port *p;
+	struct net_bridge_fdb_entry *f;
+
+	RCU_LOCKDEP_WARN(!rcu_read_lock_held(), "suspicious use outside RCU");
+
+	if (!netif_is_bridge_master(br_dev))
+		return NULL;
+
+	f = br_fdb_find_rcu(netdev_priv(br_dev), addr, vid);
+	if (!f || test_bit(BR_FDB_LOCAL, &f->flags) ||
+	    !br_switchdev_fdb_notifies(f))
+		return NULL;
+
+	/* learned by a switchdev driver, which is not notified back */
+	if (test_bit(BR_FDB_ADDED_BY_EXT_LEARN, &f->flags) &&
+	    !test_bit(BR_FDB_ADDED_BY_USER, &f->flags))
+		return NULL;
+
+	p = br_fdb_dst_port(f);
+
+	return p ? p->dev : NULL;
+}
+EXPORT_SYMBOL_GPL(br_switchdev_fdb_find_port);
+
+void
+br_switchdev_fdb_notify(struct net_bridge *br,
+			const struct net_bridge_fdb_entry *fdb, int type)
+{
+	struct switchdev_notifier_fdb_info item;
+
+	if (!br_switchdev_fdb_notifies(fdb))
 		return;
 
 	br_switchdev_fdb_populate(br, &item, fdb, NULL);
-- 
2.53.0

[PATCH net-next 2/2] net: dsa: fix host fdb refcount when a bridge fdb entry roams

[Julius Bairaktaris]

A bridge FDB entry on a foreign port is a host address on every user
port in the bridge. When the entry roams, br_fdb_update() notifies the
deletion from the old port and the addition on the new one without a
lock. With two CPUs receiving the address on different ports, DSA can
get two deletions for one addition, an addition for a port the entry
already left, or the deletion before the addition. Counting these
events either makes a deletion find no reference left on the CPU port,
or leaks a reference, which keeps the address on the CPU port after the
entry moved to a user port or was deleted. The first ends in this
error, here from dsa_loop:

  port 1 failed to delete 02:5a:0b:a2:1a:46 vid 0 from fdb: -2

Instead of counting, each user port records the host addresses it
holds, and on every event holds the address while the bridge has the
entry on a foreign port and notifies switchdev of it. A deletion that
still finds the entry on its own port was sent before the bridge
changed the port, so the address is released. An entry on a user port,
also one of another tree, is notified only to that port and is not
held; an addition there releases the address on the other ports of the
bridge, which also covers a "bridge fdb replace" onto a user port that
notifies no deletion. Replayed deletions release the address without
the lookup. When the port leaves the bridge, or fails to join it, the
workqueue is flushed before the bridge is freed and the records left
are released. Addresses local to the bridge do not roam and are still
counted. The software bridge is not changed.

In QEMU, with dsa_loop given FDB operations and assisted learning, two
veth ports in its bridge and one source address sent from both on two
CPUs for 5 s, the deletion fails and the address stays on the CPU port
after both veth ports go down in 4 of 5 boots. With this change neither
happens in 5 of 5 boots, and the address is removed when the user ports
leave the bridge. A "bridge fdb replace" of the address onto a user port
leaves it on the CPU port in 5 of 5 boots without this change and
removes it in 5 of 5 with it.

Signed-off-by: Julius Bairaktaris <julius@bairaktaris.de>
---
 net/dsa/port.c |  36 +++++++--
 net/dsa/port.h |   6 ++
 net/dsa/user.c | 216 +++++++++++++++++++++++++++++++++++++++++++++----
 net/dsa/user.h |   8 ++
 4 files changed, 242 insertions(+), 24 deletions(-)

diff --git a/net/dsa/port.c b/net/dsa/port.c
index 1f5536c0dffc2..e270a0c5e6641 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -457,7 +457,13 @@ static void dsa_port_bridge_destroy(struct dsa_port *dp,
 {
 	struct dsa_bridge *bridge = dp->bridge;
 
-	dp->bridge = NULL;
+	WRITE_ONCE(dp->bridge, NULL);
+
+	/* Wait for the FDB events that still see the bridge, then release the
+	 * host addresses the port holds, before the drivers are told it left.
+	 */
+	dsa_flush_workqueue();
+	dsa_user_bridge_host_fdb_purge(dp, bridge);
 
 	if (!refcount_dec_and_test(&bridge->refcount))
 		return;
@@ -528,7 +534,9 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br,
 					&dsa_user_switchdev_blocking_notifier);
 	dsa_flush_workqueue();
 out_rollback_unbridge:
+	dsa_port_bridge_destroy(dp, br);
 	dsa_broadcast(DSA_NOTIFIER_BRIDGE_LEAVE, &info);
+	return err;
 out_rollback:
 	dsa_port_bridge_destroy(dp, br);
 	return err;
@@ -1040,13 +1048,14 @@ int dsa_port_standalone_host_fdb_add(struct dsa_port *dp,
 	return dsa_port_host_fdb_add(dp, addr, vid, db);
 }
 
-int dsa_port_bridge_host_fdb_add(struct dsa_port *dp,
-				 const unsigned char *addr, u16 vid)
+int __dsa_port_bridge_host_fdb_add(struct dsa_port *dp,
+				   const struct dsa_bridge *bridge,
+				   const unsigned char *addr, u16 vid)
 {
 	struct net_device *conduit = dsa_port_to_conduit(dp);
 	struct dsa_db db = {
 		.type = DSA_DB_BRIDGE,
-		.bridge = *dp->bridge,
+		.bridge = *bridge,
 	};
 	int err;
 
@@ -1066,6 +1075,12 @@ int dsa_port_bridge_host_fdb_add(struct dsa_port *dp,
 	return dsa_port_host_fdb_add(dp, addr, vid, db);
 }
 
+int dsa_port_bridge_host_fdb_add(struct dsa_port *dp,
+				 const unsigned char *addr, u16 vid)
+{
+	return __dsa_port_bridge_host_fdb_add(dp, dp->bridge, addr, vid);
+}
+
 static int dsa_port_host_fdb_del(struct dsa_port *dp,
 				 const unsigned char *addr, u16 vid,
 				 struct dsa_db db)
@@ -1091,13 +1106,14 @@ int dsa_port_standalone_host_fdb_del(struct dsa_port *dp,
 	return dsa_port_host_fdb_del(dp, addr, vid, db);
 }
 
-int dsa_port_bridge_host_fdb_del(struct dsa_port *dp,
-				 const unsigned char *addr, u16 vid)
+int __dsa_port_bridge_host_fdb_del(struct dsa_port *dp,
+				   const struct dsa_bridge *bridge,
+				   const unsigned char *addr, u16 vid)
 {
 	struct net_device *conduit = dsa_port_to_conduit(dp);
 	struct dsa_db db = {
 		.type = DSA_DB_BRIDGE,
-		.bridge = *dp->bridge,
+		.bridge = *bridge,
 	};
 	int err;
 
@@ -1113,6 +1129,12 @@ int dsa_port_bridge_host_fdb_del(struct dsa_port *dp,
 	return dsa_port_host_fdb_del(dp, addr, vid, db);
 }
 
+int dsa_port_bridge_host_fdb_del(struct dsa_port *dp,
+				 const unsigned char *addr, u16 vid)
+{
+	return __dsa_port_bridge_host_fdb_del(dp, dp->bridge, addr, vid);
+}
+
 int dsa_port_lag_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 			 u16 vid)
 {
diff --git a/net/dsa/port.h b/net/dsa/port.h
index 6bc3291573c02..6524f67a37a20 100644
--- a/net/dsa/port.h
+++ b/net/dsa/port.h
@@ -54,8 +54,14 @@ int dsa_port_standalone_host_fdb_add(struct dsa_port *dp,
 				     const unsigned char *addr, u16 vid);
 int dsa_port_standalone_host_fdb_del(struct dsa_port *dp,
 				     const unsigned char *addr, u16 vid);
+int __dsa_port_bridge_host_fdb_add(struct dsa_port *dp,
+				   const struct dsa_bridge *bridge,
+				   const unsigned char *addr, u16 vid);
 int dsa_port_bridge_host_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 				 u16 vid);
+int __dsa_port_bridge_host_fdb_del(struct dsa_port *dp,
+				   const struct dsa_bridge *bridge,
+				   const unsigned char *addr, u16 vid);
 int dsa_port_bridge_host_fdb_del(struct dsa_port *dp, const unsigned char *addr,
 				 u16 vid);
 int dsa_port_lag_fdb_add(struct dsa_port *dp, const unsigned char *addr,
diff --git a/net/dsa/user.c b/net/dsa/user.c
index 041f9060c8ef0..7f18c7885b9c5 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -43,6 +43,8 @@ struct dsa_switchdev_event_work {
 	unsigned char addr[ETH_ALEN];
 	u16 vid;
 	bool host_addr;
+	bool sync;
+	bool replay;
 };
 
 enum dsa_standalone_event {
@@ -2813,6 +2815,7 @@ int dsa_user_create(struct dsa_port *port)
 
 	p->dp = port;
 	INIT_LIST_HEAD(&p->mall_tc_list);
+	INIT_LIST_HEAD(&p->bridge_host_fdbs);
 	port->user = user_dev;
 	dsa_user_setup_tagger(user_dev);
 
@@ -3627,6 +3630,22 @@ static int dsa_user_netdevice_event(struct notifier_block *nb,
 	return NOTIFY_DONE;
 }
 
+static bool dsa_foreign_dev_check(const struct net_device *dev,
+				  const struct net_device *foreign_dev)
+{
+	const struct dsa_port *dp = dsa_user_to_port(dev);
+	struct dsa_switch_tree *dst = dp->ds->dst;
+
+	if (netif_is_bridge_master(foreign_dev))
+		return !dsa_tree_offloads_bridge_dev(dst, foreign_dev);
+
+	if (netif_is_bridge_port(foreign_dev))
+		return !dsa_tree_offloads_bridge_port(dst, foreign_dev);
+
+	/* Everything else is foreign */
+	return true;
+}
+
 static void
 dsa_fdb_offload_notify(struct dsa_switchdev_event_work *switchdev_work)
 {
@@ -3639,6 +3658,171 @@ dsa_fdb_offload_notify(struct dsa_switchdev_event_work *switchdev_work)
 				 switchdev_work->orig_dev, &info.info, NULL);
 }
 
+/* The bridge notifies a roaming entry without a lock, so with two CPUs the
+ * deletion from the old port and the addition on the new one can arrive
+ * twice or out of order. Hold the host address while the bridge has the
+ * entry on a foreign port. A deletion that still finds the entry on its own
+ * port was sent before the bridge changed the port. An entry on a user port,
+ * also one of another tree, is notified only to that port.
+ */
+static bool dsa_user_bridge_host_fdb_wanted(struct net_device *dev,
+					    const struct dsa_bridge *bridge,
+					    const unsigned char *addr, u16 vid,
+					    const struct net_device *leaving)
+{
+	struct net_device *brport;
+	bool want;
+
+	rcu_read_lock();
+	brport = br_switchdev_fdb_find_port(bridge->dev, addr, vid);
+	want = brport && brport != leaving && !dsa_user_dev_check(brport) &&
+	       dsa_foreign_dev_check(dev, brport);
+	rcu_read_unlock();
+
+	return want;
+}
+
+static void dsa_user_bridge_host_fdb_set(struct net_device *dev,
+					 const struct dsa_bridge *bridge,
+					 const unsigned char *addr, u16 vid,
+					 bool want)
+{
+	struct dsa_user_priv *p = netdev_priv(dev);
+	struct dsa_port *dp = p->dp;
+	struct dsa_mac_addr *a;
+	bool held = false;
+	int err;
+
+	list_for_each_entry(a, &p->bridge_host_fdbs, list) {
+		if (ether_addr_equal(a->addr, addr) && a->vid == vid) {
+			held = true;
+			break;
+		}
+	}
+
+	if (want == held)
+		return;
+
+	if (want) {
+		a = kzalloc_obj(*a);
+		if (!a)
+			return;
+
+		err = __dsa_port_bridge_host_fdb_add(dp, bridge, addr, vid);
+		if (err) {
+			dev_err(dp->ds->dev,
+				"port %d failed to add %pM vid %d to fdb: %d\n",
+				dp->index, addr, vid, err);
+			kfree(a);
+			return;
+		}
+
+		ether_addr_copy(a->addr, addr);
+		a->vid = vid;
+		list_add(&a->list, &p->bridge_host_fdbs);
+	} else {
+		err = __dsa_port_bridge_host_fdb_del(dp, bridge, addr, vid);
+		if (err) {
+			dev_err(dp->ds->dev,
+				"port %d failed to delete %pM vid %d from fdb: %d\n",
+				dp->index, addr, vid, err);
+			return;
+		}
+
+		list_del(&a->list);
+		kfree(a);
+	}
+}
+
+static void
+dsa_user_bridge_host_fdb_sync(struct dsa_switchdev_event_work *switchdev_work)
+{
+	bool del = switchdev_work->event == SWITCHDEV_FDB_DEL_TO_DEVICE;
+	struct net_device *dev = switchdev_work->dev;
+	const struct dsa_bridge *bridge;
+	bool want = false;
+
+	/* Cleared before the workqueue is flushed and the bridge is freed */
+	bridge = READ_ONCE(dsa_user_to_port(dev)->bridge);
+	if (!bridge)
+		return;
+
+	if (!(del && switchdev_work->replay))
+		want = dsa_user_bridge_host_fdb_wanted(dev, bridge,
+						       switchdev_work->addr,
+						       switchdev_work->vid,
+						       del ? switchdev_work->orig_dev : NULL);
+
+	dsa_user_bridge_host_fdb_set(dev, bridge, switchdev_work->addr,
+				     switchdev_work->vid, want);
+	if (want && !del)
+		dsa_fdb_offload_notify(switchdev_work);
+}
+
+/* Only the user port an entry is on gets its events, so when the entry roams
+ * there, release it on the ports of the bridge that still hold it.
+ */
+static void dsa_user_bridge_host_fdb_release_work(struct work_struct *work)
+{
+	struct dsa_switchdev_event_work *switchdev_work =
+		container_of(work, struct dsa_switchdev_event_work, work);
+	struct dsa_port *dp = dsa_user_to_port(switchdev_work->dev);
+	const unsigned char *addr = switchdev_work->addr;
+	const struct dsa_bridge *bridge = READ_ONCE(dp->bridge);
+	u16 vid = switchdev_work->vid;
+	struct dsa_port *other;
+
+	if (bridge) {
+		dsa_tree_for_each_user_port(other, dp->ds->dst) {
+			if (dsa_port_offloads_bridge_dev(other, bridge->dev) &&
+			    !dsa_user_bridge_host_fdb_wanted(other->user, bridge,
+							     addr, vid, NULL))
+				dsa_user_bridge_host_fdb_set(other->user, bridge,
+							     addr, vid, false);
+		}
+	}
+
+	kfree(switchdev_work);
+}
+
+static int dsa_user_bridge_host_fdb_release(struct net_device *dev,
+					    const unsigned char *addr, u16 vid)
+{
+	struct dsa_switchdev_event_work *switchdev_work;
+
+	switchdev_work = kzalloc_obj(*switchdev_work, GFP_ATOMIC);
+	if (!switchdev_work)
+		return -ENOMEM;
+
+	INIT_WORK(&switchdev_work->work, dsa_user_bridge_host_fdb_release_work);
+	switchdev_work->dev = dev;
+	ether_addr_copy(switchdev_work->addr, addr);
+	switchdev_work->vid = vid;
+
+	dsa_schedule_work(&switchdev_work->work);
+
+	return 0;
+}
+
+void dsa_user_bridge_host_fdb_purge(struct dsa_port *dp,
+				    const struct dsa_bridge *bridge)
+{
+	struct dsa_user_priv *p = netdev_priv(dp->user);
+	struct dsa_mac_addr *a, *tmp;
+	int err;
+
+	list_for_each_entry_safe(a, tmp, &p->bridge_host_fdbs, list) {
+		err = __dsa_port_bridge_host_fdb_del(dp, bridge, a->addr,
+						     a->vid);
+		if (err)
+			dev_err(dp->ds->dev,
+				"port %d failed to delete %pM vid %d from fdb: %d\n",
+				dp->index, a->addr, a->vid, err);
+		list_del(&a->list);
+		kfree(a);
+	}
+}
+
 static void dsa_user_switchdev_event_work(struct work_struct *work)
 {
 	struct dsa_switchdev_event_work *switchdev_work =
@@ -3653,6 +3837,12 @@ static void dsa_user_switchdev_event_work(struct work_struct *work)
 	dp = dsa_user_to_port(dev);
 	ds = dp->ds;
 
+	if (switchdev_work->sync) {
+		dsa_user_bridge_host_fdb_sync(switchdev_work);
+		kfree(switchdev_work);
+		return;
+	}
+
 	switch (switchdev_work->event) {
 	case SWITCHDEV_FDB_ADD_TO_DEVICE:
 		if (switchdev_work->host_addr)
@@ -3689,22 +3879,6 @@ static void dsa_user_switchdev_event_work(struct work_struct *work)
 	kfree(switchdev_work);
 }
 
-static bool dsa_foreign_dev_check(const struct net_device *dev,
-				  const struct net_device *foreign_dev)
-{
-	const struct dsa_port *dp = dsa_user_to_port(dev);
-	struct dsa_switch_tree *dst = dp->ds->dst;
-
-	if (netif_is_bridge_master(foreign_dev))
-		return !dsa_tree_offloads_bridge_dev(dst, foreign_dev);
-
-	if (netif_is_bridge_port(foreign_dev))
-		return !dsa_tree_offloads_bridge_port(dst, foreign_dev);
-
-	/* Everything else is foreign */
-	return true;
-}
-
 static int dsa_user_fdb_event(struct net_device *dev,
 			      struct net_device *orig_dev,
 			      unsigned long event, const void *ctx,
@@ -3723,7 +3897,9 @@ static int dsa_user_fdb_event(struct net_device *dev,
 
 	if (switchdev_fdb_is_dynamically_learned(fdb_info)) {
 		if (dsa_port_offloads_bridge_port(dp, orig_dev))
-			return 0;
+			return event == SWITCHDEV_FDB_ADD_TO_DEVICE ?
+			       dsa_user_bridge_host_fdb_release(dev, fdb_info->addr,
+								fdb_info->vid) : 0;
 
 		/* FDB entries learned by the software bridge or by foreign
 		 * bridge ports should be installed as host addresses only if
@@ -3768,9 +3944,15 @@ static int dsa_user_fdb_event(struct net_device *dev,
 	ether_addr_copy(switchdev_work->addr, fdb_info->addr);
 	switchdev_work->vid = fdb_info->vid;
 	switchdev_work->host_addr = host_addr;
+	switchdev_work->sync = host_addr && !fdb_info->is_local;
+	switchdev_work->replay = !!ctx;
 
 	dsa_schedule_work(&switchdev_work->work);
 
+	if (!host_addr && event == SWITCHDEV_FDB_ADD_TO_DEVICE)
+		return dsa_user_bridge_host_fdb_release(dev, fdb_info->addr,
+							fdb_info->vid);
+
 	return 0;
 }
 
diff --git a/net/dsa/user.h b/net/dsa/user.h
index 016884bead3cd..dd38104162da4 100644
--- a/net/dsa/user.h
+++ b/net/dsa/user.h
@@ -33,6 +33,12 @@ struct dsa_user_priv {
 
 	/* TC context */
 	struct list_head	mall_tc_list;
+
+	/* Host addresses of bridge FDB entries held through this port,
+	 * changed from the DSA workqueue, or with the port out of the bridge
+	 * and the workqueue flushed
+	 */
+	struct list_head	bridge_host_fdbs;
 };
 
 void dsa_user_mii_bus_init(struct dsa_switch *ds);
@@ -47,6 +53,8 @@ void dsa_user_host_uc_uninstall(struct net_device *dev);
 void dsa_user_sync_ha(struct net_device *dev);
 void dsa_user_unsync_ha(struct net_device *dev);
 void dsa_user_setup_tagger(struct net_device *user);
+void dsa_user_bridge_host_fdb_purge(struct dsa_port *dp,
+				    const struct dsa_bridge *bridge);
 int dsa_user_change_mtu(struct net_device *dev, int new_mtu);
 int dsa_user_change_conduit(struct net_device *dev, struct net_device *conduit,
 			    struct netlink_ext_ack *extack);
-- 
2.53.0

Re: [PATCH net-next 0/2] net: dsa: fix host fdb refcount when an entry roams

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net-next 0/2] net: dsa: fix host fdb refcount when an entry roams

[Julius Bairaktaris]

> - How the issue was discovered, e.g. hit in production, hit during
>   development, syzbot report, manual code inspection, LLM or static
>   analysis tool scan.

Hit during development on a Xiaomi AX3600 (IPQ8074) running OpenWrt's
qca-ppe DSA switch driver. A station roaming between a DSA user port
and a Wi-Fi AP port of the same bridge logged

  qca-ppe 3a000000.ppe: port 5 failed to delete 02:5a:0b:a2:1a:46 vid 0 from fdb: -2

3-6 times per address when the two ports received on different CPUs,
and not at all when they shared one CPU (4 runs each). The dsa_loop
harness described in the cover letter reproduces it on net-next.

Julius