[PATCH RFC batadv v3 0/6] batman-adv: tt: atomic sashiko fixes

[PATCH RFC batadv v3 0/6] batman-adv: tt: atomic sashiko fixes

[Sven Eckelmann]

There were a couple of sashiko reports regarding the
batadv-next-pullrequest-20260728 PR. I went through the TT code and tried
to handle them and at the same time already predict what else sashiko might
report as next. I needed to change the batadv_tt_local_remove() hash
interaction as preparation for the related NEW flag (and counter) handling.

And the netdev maintainers rejected this approach (for now). I had to
re-add the atomic_t code parts back in this patchset. But since the netdev
maintainers didn't like the atomic_t implementation, everything was changed
to atomic_t with the help of scoped_guard().

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
Changes in v3:
- switched to spinlock_bh due to rejection atomic_t by netdev (and
  potentially easier code)
- switch back to RFC
- Link to v2: https://patch.msgid.link/20260804-tt-fixes-v2-0-487c48fc7fc4@narfation.org

Changes in v2:
- rebase on main
- add "batman-adv: tt: use atomic flag modifications" and "batman-adv: tt:
  simplify NEW flag transition code" because they are reverted on main
- add patch to extract the roam "add" code in a separate function
- make the "simplify NEW flag transition code" patch independent of atomic_t code
- Link to v1: https://patch.msgid.link/20260730-tt-fixes-v1-0-64e9e525d555@narfation.org

---
Sven Eckelmann (6):
      batman-adv: tt: remove only the entry which was looked up from the hash
      batman-adv: tt: extract code handling a roam on add
      batman-adv: tt: simplify NEW flag transition code
      batman-adv: tt: use atomic flag modifications
      batman-adv: tt: decrement count for committed client on local_remove
      batman-adv: tt: don't uncount never committed clients on pending purge

 net/batman-adv/translation-table.c | 693 +++++++++++++++++++++++--------------
 net/batman-adv/types.h             |   8 +-
 2 files changed, 448 insertions(+), 253 deletions(-)
---
base-commit: 46e983472304d8875436a23ccb996ea18856081a
change-id: 20260730-tt-fixes-fca2b722ebbd

Best regards,
--  
Sven Eckelmann <sven@narfation.org>

[PATCH RFC batadv v3 1/6] batman-adv: tt: remove only the entry which was looked up from the hash

[Sven Eckelmann]

batadv_hash_remove() searches a bucket with a compare callback and unlinks
the first matching entry. batadv_compare_tt() matches any entry for the
same MAC address and VLAN, not the object which was passed in, and the
callers in batadv_tt_local_remove() and batadv_tt_global_free() are not
serialized against the rest of the translation table in any way.

So when the looked up entry was already unlinked by another context and a
new entry for the same client was added in the meantime, these two
functions unlink that new entry instead.

Add batadv_compare_tt_entry(), which matches the very object which is
searched for, and use it for both removals. Nothing is unlinked when the
entry is gone already, batadv_hash_remove() then simply returns NULL and
only the reference of the calling context is dropped.

As a side effect the returned hlist_node can no longer belong to a
different object, so both functions can operate on the entry they were
given.

Fixes: af912d77181f ("batman-adv: protect tt_local_entry from concurrent delete events")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/translation-table.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 4f47c97b..af40ff81 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -136,6 +136,26 @@ static bool batadv_compare_tt(const struct hlist_node *node, const void *data2)
 	return (tt1->vid == tt2->vid) && batadv_compare_eth(data1, data2);
 }
 
+/**
+ * batadv_compare_tt_entry() - check if a hash node is a specific TT entry
+ * @node: the list element pointer of the TT entry stored in the bucket
+ * @data2: pointer to the tt_common_entry which is looked for
+ *
+ * Unlike batadv_compare_tt(), this only matches the very object which is
+ * passed as @data2 and not just any entry for the same TT client. It is meant
+ * for batadv_hash_remove() callers which must not unlink an entry they did not
+ * look up themselves.
+ *
+ * Return: true if @node belongs to @data2, false otherwise
+ */
+static bool batadv_compare_tt_entry(const struct hlist_node *node,
+				    const void *data2)
+{
+	const struct batadv_tt_common_entry *tt = data2;
+
+	return node == &tt->hash_entry;
+}
+
 /**
  * batadv_choose_tt() - return the index of the tt entry in the hash table
  * @data: pointer to the tt_common_entry object to map
@@ -628,7 +648,6 @@ static void batadv_tt_global_free(struct batadv_priv *bat_priv,
 				  struct batadv_tt_global_entry *tt_global,
 				  const char *message)
 {
-	struct batadv_tt_global_entry *tt_removed_entry;
 	struct hlist_node *tt_removed_node;
 
 	batadv_dbg(BATADV_DBG_TT, bat_priv,
@@ -636,18 +655,16 @@ static void batadv_tt_global_free(struct batadv_priv *bat_priv,
 		   tt_global->common.addr,
 		   batadv_print_vid(tt_global->common.vid), message);
 
+	/* remove exactly this object when still present in hash */
 	tt_removed_node = batadv_hash_remove(bat_priv->tt.global_hash,
-					     batadv_compare_tt,
+					     batadv_compare_tt_entry,
 					     batadv_choose_tt,
 					     &tt_global->common);
 	if (!tt_removed_node)
 		return;
 
 	/* drop reference of remove hash entry */
-	tt_removed_entry = hlist_entry(tt_removed_node,
-				       struct batadv_tt_global_entry,
-				       common.hash_entry);
-	batadv_tt_global_entry_put(tt_removed_entry);
+	batadv_tt_global_entry_put(tt_global);
 }
 
 /**
@@ -1345,7 +1362,6 @@ u16 batadv_tt_local_remove(struct batadv_priv *bat_priv, const u8 *addr,
 			   unsigned short vid, const char *message,
 			   bool roaming)
 {
-	struct batadv_tt_local_entry *tt_removed_entry;
 	struct batadv_tt_local_entry *tt_local_entry;
 	struct hlist_node *tt_removed_node;
 	u16 curr_flags = BATADV_NO_FLAGS;
@@ -1378,18 +1394,16 @@ u16 batadv_tt_local_remove(struct batadv_priv *bat_priv, const u8 *addr,
 	 */
 	batadv_tt_local_event(bat_priv, tt_local_entry, BATADV_TT_CLIENT_DEL);
 
+	/* remove exactly this object when still present in hash */
 	tt_removed_node = batadv_hash_remove(bat_priv->tt.local_hash,
-					     batadv_compare_tt,
+					     batadv_compare_tt_entry,
 					     batadv_choose_tt,
 					     &tt_local_entry->common);
 	if (!tt_removed_node)
 		goto out;
 
 	/* drop reference of remove hash entry */
-	tt_removed_entry = hlist_entry(tt_removed_node,
-				       struct batadv_tt_local_entry,
-				       common.hash_entry);
-	batadv_tt_local_entry_put(tt_removed_entry);
+	batadv_tt_local_entry_put(tt_local_entry);
 
 out:
 	batadv_tt_local_entry_put(tt_local_entry);

-- 
2.47.3

[PATCH RFC batadv v3 2/6] batman-adv: tt: extract code handling a roam on add

[Sven Eckelmann]

The code for handling the roaming of clients is mostly self contained but
embedded in the already too large batadv_tt_local_add function. The
maintainability can be improved by keeping it in an separate function.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/translation-table.c | 71 ++++++++++++++++++++++++--------------
 1 file changed, 45 insertions(+), 26 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index af40ff81..2a1d8963 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -667,6 +667,50 @@ static void batadv_tt_global_free(struct batadv_priv *bat_priv,
 	batadv_tt_global_entry_put(tt_global);
 }
 
+/**
+ * batadv_tt_local_add_roam() - handle roamed clients during batadv_tt_local_add()
+ * @bat_priv: the bat priv with all the mesh interface information
+ * @tt_global: the global TT entry
+ * @roamed_back: whether @tt_global roamed back
+ */
+static void batadv_tt_local_add_roam(struct batadv_priv *bat_priv,
+				     struct batadv_tt_global_entry *tt_global,
+				     bool roamed_back)
+{
+	struct batadv_tt_orig_list_entry *orig_entry;
+	struct hlist_head *head;
+
+	if (!tt_global)
+		return;
+
+	/* Check whether it is a roaming, but don't do anything if the roaming
+	 * process has already been handled
+	 */
+	if (tt_global->common.flags & BATADV_TT_CLIENT_ROAM)
+		return;
+
+	/* These node are probably going to update their tt table */
+	head = &tt_global->orig_list;
+	rcu_read_lock();
+	hlist_for_each_entry_rcu(orig_entry, head, list) {
+		batadv_send_roam_adv(bat_priv, tt_global->common.addr,
+				     tt_global->common.vid,
+				     orig_entry->orig_node);
+	}
+	rcu_read_unlock();
+
+	if (roamed_back) {
+		batadv_tt_global_free(bat_priv, tt_global,
+				      "Roaming canceled");
+	} else {
+		/* The global entry has to be marked as ROAMING and
+		 * has to be kept for consistency purpose
+		 */
+		tt_global->common.flags |= BATADV_TT_CLIENT_ROAM;
+		tt_global->roam_at = jiffies;
+	}
+}
+
 /**
  * batadv_tt_local_add() - add a new client to the local table or update an
  *  existing client
@@ -685,14 +729,12 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
 {
 	struct batadv_priv *bat_priv = netdev_priv(mesh_iface);
 	struct batadv_tt_global_entry *tt_global = NULL;
-	struct batadv_tt_orig_list_entry *orig_entry;
 	struct batadv_tt_local_entry *tt_local;
 	struct net *net = dev_net(mesh_iface);
 	struct net_device *in_dev = NULL;
 	struct batadv_meshif_vlan *vlan;
 	bool roamed_back = false;
 	bool iif_is_wifi = false;
-	struct hlist_head *head;
 	int packet_size_max;
 	bool ret = false;
 	u8 remote_flags;
@@ -811,30 +853,7 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
 	batadv_tt_local_event(bat_priv, tt_local, BATADV_NO_FLAGS);
 
 check_roaming:
-	/* Check whether it is a roaming, but don't do anything if the roaming
-	 * process has already been handled
-	 */
-	if (tt_global && !(tt_global->common.flags & BATADV_TT_CLIENT_ROAM)) {
-		/* These node are probably going to update their tt table */
-		head = &tt_global->orig_list;
-		rcu_read_lock();
-		hlist_for_each_entry_rcu(orig_entry, head, list) {
-			batadv_send_roam_adv(bat_priv, tt_global->common.addr,
-					     tt_global->common.vid,
-					     orig_entry->orig_node);
-		}
-		rcu_read_unlock();
-		if (roamed_back) {
-			batadv_tt_global_free(bat_priv, tt_global,
-					      "Roaming canceled");
-		} else {
-			/* The global entry has to be marked as ROAMING and
-			 * has to be kept for consistency purpose
-			 */
-			tt_global->common.flags |= BATADV_TT_CLIENT_ROAM;
-			tt_global->roam_at = jiffies;
-		}
-	}
+	batadv_tt_local_add_roam(bat_priv, tt_global, roamed_back);
 
 	/* store the current remote flags before altering them. This helps
 	 * understanding is flags are changing or not

-- 
2.47.3

[PATCH RFC batadv v3 3/6] batman-adv: tt: simplify NEW flag transition code

[Sven Eckelmann]

The function batadv_tt_local_set_flags() implements support for various
flags and to set/unset them. It is also possible to decide whether this
change should increase the TT size or not.

But in reality, this function is only used to transition tt local entries
from the NEW state and count these entries. Remove the rest of the code to
simplify the function.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/translation-table.c | 26 +++++++-------------------
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 2a1d8963..5f0f7fb6 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -3868,15 +3868,11 @@ void batadv_tt_free(struct batadv_priv *bat_priv)
 }
 
 /**
- * batadv_tt_local_set_flags() - set or unset the specified flags on the local
- *  table and possibly count them in the TT size
+ * batadv_tt_local_transition_new() - unset the NEW flag on the local
+ *  table and count them in the TT size
  * @bat_priv: the bat priv with all the mesh interface information
- * @flags: the flag to switch
- * @enable: whether to set or unset the flag
- * @count: whether to increase the TT size by the number of changed entries
  */
-static void batadv_tt_local_set_flags(struct batadv_priv *bat_priv, u16 flags,
-				      bool enable, bool count)
+static void batadv_tt_local_transition_new(struct batadv_priv *bat_priv)
 {
 	struct batadv_hashtable *hash = bat_priv->tt.local_hash;
 	struct batadv_tt_common_entry *tt_common_entry;
@@ -3892,19 +3888,11 @@ static void batadv_tt_local_set_flags(struct batadv_priv *bat_priv, u16 flags,
 		rcu_read_lock();
 		hlist_for_each_entry_rcu(tt_common_entry,
 					 head, hash_entry) {
-			if (enable) {
-				if ((tt_common_entry->flags & flags) == flags)
-					continue;
-				tt_common_entry->flags |= flags;
-			} else {
-				if (!(tt_common_entry->flags & flags))
-					continue;
-				tt_common_entry->flags &= ~flags;
-			}
-
-			if (!count)
+			if (!(tt_common_entry->flags & BATADV_TT_CLIENT_NEW))
 				continue;
 
+			tt_common_entry->flags &= ~BATADV_TT_CLIENT_NEW;
+
 			batadv_tt_local_size_inc(bat_priv,
 						 tt_common_entry->vid);
 		}
@@ -3977,7 +3965,7 @@ static void batadv_tt_local_commit_changes_nolock(struct batadv_priv *bat_priv)
 		return;
 	}
 
-	batadv_tt_local_set_flags(bat_priv, BATADV_TT_CLIENT_NEW, false, true);
+	batadv_tt_local_transition_new(bat_priv);
 
 	batadv_tt_local_purge_pending_clients(bat_priv);
 	batadv_tt_local_update_crc(bat_priv);

-- 
2.47.3

[PATCH RFC batadv v3 4/6] batman-adv: tt: use atomic flag modifications

[Sven Eckelmann]

The flags of translation table entries are modified in various places using
RMW operations like:

* read flags + add flag + store
* read flags + remove flag + store

These were done without making sure that no other context is doing a
similar operation at the same time. If another context does modify the
flags then it could happen that a store of the flag modifications is simply
lost. This problem can usually be fixed at a later point when the flags are
tried to be adjusted again.

To reduce the time the wrong flags are used, it is better to use a TT entry
specific spinlock when accessing the u16.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/translation-table.c | 569 +++++++++++++++++++++++--------------
 net/batman-adv/types.h             |   8 +-
 2 files changed, 366 insertions(+), 211 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 5f0f7fb6..df99c67e 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -13,6 +13,7 @@
 #include <linux/build_bug.h>
 #include <linux/byteorder/generic.h>
 #include <linux/cache.h>
+#include <linux/cleanup.h>
 #include <linux/compiler.h>
 #include <linux/container_of.h>
 #include <linux/crc32.h>
@@ -498,23 +499,26 @@ static void batadv_tt_local_event(struct batadv_priv *bat_priv,
 {
 	struct batadv_tt_common_entry *common = &tt_local_entry->common;
 	struct batadv_tt_change_node *tt_change_node;
-	u8 flags = common->flags | event_flags;
 	struct batadv_tt_change_node *entry;
 	struct batadv_tt_change_node *safe;
 	bool del_op_requested;
 	bool del_op_entry;
 	size_t changes;
+	u8 flags;
 
 	tt_change_node = kmem_cache_alloc(batadv_tt_change_cache, GFP_ATOMIC);
 	if (!tt_change_node)
 		return;
 
-	tt_change_node->change.flags = flags;
 	memset(tt_change_node->change.reserved, 0,
 	       sizeof(tt_change_node->change.reserved));
 	ether_addr_copy(tt_change_node->change.addr, common->addr);
 	tt_change_node->change.vid = htons(common->vid);
 
+	scoped_guard(spinlock_bh, &common->flags_lock)
+		flags = common->flags | event_flags;
+
+	tt_change_node->change.flags = flags;
 	del_op_requested = flags & BATADV_TT_CLIENT_DEL;
 
 	/* check for ADD+DEL, DEL+ADD, ADD+ADD or DEL+DEL events */
@@ -686,8 +690,23 @@ static void batadv_tt_local_add_roam(struct batadv_priv *bat_priv,
 	/* Check whether it is a roaming, but don't do anything if the roaming
 	 * process has already been handled
 	 */
-	if (tt_global->common.flags & BATADV_TT_CLIENT_ROAM)
-		return;
+	scoped_guard(spinlock_bh, &tt_global->common.flags_lock) {
+		if (tt_global->common.flags & BATADV_TT_CLIENT_ROAM)
+			return;
+
+		if (!roamed_back) {
+			/* The global entry has to be marked as ROAMING and has to be
+			 * kept for consistency purpose.
+			 *
+			 * batadv_tt_global_to_purge() evaluates roam_at as soon as it
+			 * observes BATADV_TT_CLIENT_ROAM, so the timeout has to be
+			 * stamped before the flag is published. Otherwise the entry can
+			 * be deleted right away as "Roaming timeout".
+			 */
+			tt_global->roam_at = jiffies;
+			tt_global->common.flags &= ~BATADV_TT_CLIENT_ROAM;
+		}
+	}
 
 	/* These node are probably going to update their tt table */
 	head = &tt_global->orig_list;
@@ -699,16 +718,8 @@ static void batadv_tt_local_add_roam(struct batadv_priv *bat_priv,
 	}
 	rcu_read_unlock();
 
-	if (roamed_back) {
-		batadv_tt_global_free(bat_priv, tt_global,
-				      "Roaming canceled");
-	} else {
-		/* The global entry has to be marked as ROAMING and
-		 * has to be kept for consistency purpose
-		 */
-		tt_global->common.flags |= BATADV_TT_CLIENT_ROAM;
-		tt_global->roam_at = jiffies;
-	}
+	if (roamed_back)
+		batadv_tt_global_free(bat_priv, tt_global, "Roaming canceled");
 }
 
 /**
@@ -741,6 +752,7 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
 	int hash_added;
 	int table_size;
 	u32 match_mark;
+	bool modified;
 
 	if (ifindex != BATADV_NULL_IFINDEX)
 		in_dev = dev_get_by_index(net, ifindex);
@@ -758,30 +770,33 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
 
 	if (tt_local) {
 		tt_local->last_seen = jiffies;
-		if (tt_local->common.flags & BATADV_TT_CLIENT_PENDING) {
-			batadv_dbg(BATADV_DBG_TT, bat_priv,
-				   "Re-adding pending client %pM (vid: %d)\n",
-				   addr, batadv_print_vid(vid));
-			/* whatever the reason why the PENDING flag was set,
-			 * this is a client which was enqueued to be removed in
-			 * this orig_interval. Since it popped up again, the
-			 * flag can be reset like it was never enqueued
-			 */
-			tt_local->common.flags &= ~BATADV_TT_CLIENT_PENDING;
-			goto add_event;
-		}
 
-		if (tt_local->common.flags & BATADV_TT_CLIENT_ROAM) {
-			batadv_dbg(BATADV_DBG_TT, bat_priv,
-				   "Roaming client %pM (vid: %d) came back to its original location\n",
-				   addr, batadv_print_vid(vid));
-			/* the ROAM flag is set because this client roamed away
-			 * and the node got a roaming_advertisement message. Now
-			 * that the client popped up again at its original
-			 * location such flag can be unset
-			 */
-			tt_local->common.flags &= ~BATADV_TT_CLIENT_ROAM;
-			roamed_back = true;
+		scoped_guard(spinlock_bh, &tt_local->common.flags_lock) {
+			if (tt_local->common.flags & BATADV_TT_CLIENT_PENDING) {
+				batadv_dbg(BATADV_DBG_TT, bat_priv,
+					   "Re-adding pending client %pM (vid: %d)\n",
+					   addr, batadv_print_vid(vid));
+				/* whatever the reason why the PENDING flag was set,
+				 * this is a client which was enqueued to be removed in
+				 * this orig_interval. Since it popped up again, the
+				 * flag can be reset like it was never enqueued
+				 */
+				tt_local->common.flags &= ~BATADV_TT_CLIENT_PENDING;
+				goto add_event;
+			}
+
+			if (tt_local->common.flags & BATADV_TT_CLIENT_ROAM) {
+				batadv_dbg(BATADV_DBG_TT, bat_priv,
+					   "Roaming client %pM (vid: %d) came back to its original location\n",
+					   addr, batadv_print_vid(vid));
+				/* the ROAM flag is set because this client roamed away
+				 * and the node got a roaming_advertisement message. Now
+				 * that the client popped up again at its original
+				 * location such flag can be unset
+				 */
+				tt_local->common.flags &= ~BATADV_TT_CLIENT_ROAM;
+				roamed_back = true;
+			}
 		}
 		goto check_roaming;
 	}
@@ -818,25 +833,29 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
 		   (u8)atomic_read(&bat_priv->tt.vn));
 
 	ether_addr_copy(tt_local->common.addr, addr);
-	/* The local entry has to be marked as NEW to avoid to send it in
-	 * a full table response going out before the next ttvn increment
-	 * (consistency check)
-	 */
-	tt_local->common.flags = BATADV_TT_CLIENT_NEW;
 	tt_local->common.vid = vid;
-	if (iif_is_wifi)
-		tt_local->common.flags |= BATADV_TT_CLIENT_WIFI;
 	kref_init(&tt_local->common.refcount);
 	tt_local->last_seen = jiffies;
 	tt_local->common.added_at = tt_local->last_seen;
 	tt_local->vlan = vlan;
+	spin_lock_init(&tt_local->common.flags_lock);
 
-	/* the batman interface mac and multicast addresses should never be
-	 * purged
-	 */
-	if (batadv_compare_eth(addr, mesh_iface->dev_addr) ||
-	    is_multicast_ether_addr(addr))
-		tt_local->common.flags |= BATADV_TT_CLIENT_NOPURGE;
+	scoped_guard(spinlock_bh, &tt_local->common.flags_lock) {
+		/* The local entry has to be marked as NEW to avoid to send it in
+		 * a full table response going out before the next ttvn increment
+		 * (consistency check)
+		 */
+		tt_local->common.flags = BATADV_TT_CLIENT_NEW;
+		if (iif_is_wifi)
+			tt_local->common.flags |= BATADV_TT_CLIENT_WIFI;
+
+		/* the batman interface mac and multicast addresses should never be
+		 * purged
+		 */
+		if (batadv_compare_eth(addr, mesh_iface->dev_addr) ||
+		    is_multicast_ether_addr(addr))
+			tt_local->common.flags |= BATADV_TT_CLIENT_NOPURGE;
+	}
 
 	kref_get(&tt_local->common.refcount);
 	hash_added = batadv_hash_add(bat_priv->tt.local_hash, batadv_compare_tt,
@@ -855,31 +874,35 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
 check_roaming:
 	batadv_tt_local_add_roam(bat_priv, tt_global, roamed_back);
 
-	/* store the current remote flags before altering them. This helps
-	 * understanding is flags are changing or not
-	 */
-	remote_flags = tt_local->common.flags & BATADV_TT_REMOTE_MASK;
+	scoped_guard(spinlock_bh, &tt_local->common.flags_lock) {
+		/* store the current remote flags before altering them. This helps
+		 * understanding is flags are changing or not
+		 */
+		remote_flags = tt_local->common.flags & BATADV_TT_REMOTE_MASK;
 
-	if (iif_is_wifi)
-		tt_local->common.flags |= BATADV_TT_CLIENT_WIFI;
-	else
-		tt_local->common.flags &= ~BATADV_TT_CLIENT_WIFI;
+		if (iif_is_wifi)
+			tt_local->common.flags |= BATADV_TT_CLIENT_WIFI;
+		else
+			tt_local->common.flags &= ~BATADV_TT_CLIENT_WIFI;
 
-	/* check the mark in the skb: if it's equal to the configured
-	 * isolation_mark, it means the packet is coming from an isolated
-	 * non-mesh client
-	 */
-	match_mark = (mark & bat_priv->isolation_mark_mask);
-	if (bat_priv->isolation_mark_mask &&
-	    match_mark == bat_priv->isolation_mark)
-		tt_local->common.flags |= BATADV_TT_CLIENT_ISOLA;
-	else
-		tt_local->common.flags &= ~BATADV_TT_CLIENT_ISOLA;
+		/* check the mark in the skb: if it's equal to the configured
+		 * isolation_mark, it means the packet is coming from an isolated
+		 * non-mesh client
+		 */
+		match_mark = (mark & bat_priv->isolation_mark_mask);
+		if (bat_priv->isolation_mark_mask &&
+		    match_mark == bat_priv->isolation_mark)
+			tt_local->common.flags |= BATADV_TT_CLIENT_ISOLA;
+		else
+			tt_local->common.flags &= ~BATADV_TT_CLIENT_ISOLA;
+
+		modified = remote_flags ^ (tt_local->common.flags & BATADV_TT_REMOTE_MASK);
+	}
 
 	/* if any "dynamic" flag has been modified, resend an ADD event for this
 	 * entry so that all the nodes can get the new flags
 	 */
-	if (remote_flags ^ (tt_local->common.flags & BATADV_TT_REMOTE_MASK))
+	if (modified)
 		batadv_tt_local_event(bat_priv, tt_local, BATADV_NO_FLAGS);
 
 	ret = true;
@@ -1204,6 +1227,7 @@ batadv_tt_local_dump_entry(struct sk_buff *msg, u32 portid,
 	struct batadv_meshif_vlan *vlan;
 	unsigned int last_seen_msecs;
 	void *hdr;
+	u16 flags;
 	u32 crc;
 
 	local = container_of(common, struct batadv_tt_local_entry, common);
@@ -1225,13 +1249,16 @@ batadv_tt_local_dump_entry(struct sk_buff *msg, u32 portid,
 
 	genl_dump_check_consistent(cb, hdr);
 
+	scoped_guard(spinlock_bh, &common->flags_lock)
+		flags = common->flags;
+
 	if (nla_put(msg, BATADV_ATTR_TT_ADDRESS, ETH_ALEN, common->addr) ||
 	    nla_put_u32(msg, BATADV_ATTR_TT_CRC32, crc) ||
 	    nla_put_u16(msg, BATADV_ATTR_TT_VID, common->vid) ||
-	    nla_put_u32(msg, BATADV_ATTR_TT_FLAGS, common->flags))
+	    nla_put_u32(msg, BATADV_ATTR_TT_FLAGS, flags))
 		goto nla_put_failure;
 
-	if (!(common->flags & BATADV_TT_CLIENT_NOPURGE) &&
+	if (!(flags & BATADV_TT_CLIENT_NOPURGE) &&
 	    nla_put_u32(msg, BATADV_ATTR_LAST_SEEN_MSECS, last_seen_msecs))
 		goto nla_put_failure;
 
@@ -1338,7 +1365,7 @@ int batadv_tt_local_dump(struct sk_buff *msg, struct netlink_callback *cb)
 }
 
 /**
- * batadv_tt_local_set_pending() - mark a local TT entry as pending removal
+ * batadv_tt_local_set_pending_event() - trigger events for TT pending removal
  * @bat_priv: the bat priv with all the mesh interface information
  * @tt_local_entry: local TT entry to mark
  * @flags: TT change flags to announce together with the pending removal
@@ -1349,18 +1376,12 @@ int batadv_tt_local_dump(struct sk_buff *msg, struct netlink_callback *cb)
  * so that a consistency-check response can still be answered.
  */
 static void
-batadv_tt_local_set_pending(struct batadv_priv *bat_priv,
-			    struct batadv_tt_local_entry *tt_local_entry,
-			    u16 flags, const char *message)
+batadv_tt_local_set_pending_event(struct batadv_priv *bat_priv,
+				  struct batadv_tt_local_entry *tt_local_entry,
+				  u16 flags, const char *message)
 {
 	batadv_tt_local_event(bat_priv, tt_local_entry, flags);
 
-	/* The local client has to be marked as "pending to be removed" but has
-	 * to be kept in the table in order to send it in a full table
-	 * response issued before the net ttvn increment (consistency check)
-	 */
-	tt_local_entry->common.flags |= BATADV_TT_CLIENT_PENDING;
-
 	batadv_dbg(BATADV_DBG_TT, bat_priv,
 		   "Local tt entry (%pM, vid: %d) pending to be removed: %s\n",
 		   tt_local_entry->common.addr,
@@ -1384,30 +1405,46 @@ u16 batadv_tt_local_remove(struct batadv_priv *bat_priv, const u8 *addr,
 	struct batadv_tt_local_entry *tt_local_entry;
 	struct hlist_node *tt_removed_node;
 	u16 curr_flags = BATADV_NO_FLAGS;
+	bool pending = false;
 	u16 flags;
 
 	tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr, vid);
 	if (!tt_local_entry)
 		goto out;
 
-	curr_flags = tt_local_entry->common.flags;
+	scoped_guard(spinlock_bh, &tt_local_entry->common.flags_lock) {
+		curr_flags = tt_local_entry->common.flags;
 
-	flags = BATADV_TT_CLIENT_DEL;
-	/* if this global entry addition is due to a roaming, the node has to
-	 * mark the local entry as "roamed" in order to correctly reroute
-	 * packets later
-	 */
-	if (roaming) {
-		flags |= BATADV_TT_CLIENT_ROAM;
-		/* mark the local client as ROAMed */
-		tt_local_entry->common.flags |= BATADV_TT_CLIENT_ROAM;
+		flags = BATADV_TT_CLIENT_DEL;
+		/* if this global entry addition is due to a roaming, the node has to
+		 * mark the local entry as "roamed" in order to correctly reroute
+		 * packets later
+		 */
+		if (roaming) {
+			flags |= BATADV_TT_CLIENT_ROAM;
+			/* mark the local client as ROAMed */
+			tt_local_entry->common.flags |= BATADV_TT_CLIENT_ROAM;
+		}
+
+		/* This must stay a read-only test while the entry is still hashed:
+		 * clearing the NEW flag here would make a concurrent remover of the
+		 * same entry pick the branch below and mark a not yet committed entry
+		 * as pending. A parallel batadv_tt_local_purge_pending_clients()
+		 * then calls batadv_tt_local_size_dec() for this entry even when
+		 * it was never counted by via batadv_tt_local_set_flags()
+		 */
+		if (!(tt_local_entry->common.flags & BATADV_TT_CLIENT_NEW)) {
+			tt_local_entry->common.flags |= BATADV_TT_CLIENT_PENDING;
+			pending = true;
+		}
 	}
 
-	if (!(tt_local_entry->common.flags & BATADV_TT_CLIENT_NEW)) {
-		batadv_tt_local_set_pending(bat_priv, tt_local_entry, flags,
-					    message);
+	if (pending) {
+		batadv_tt_local_set_pending_event(bat_priv, tt_local_entry, flags,
+						  message);
 		goto out;
 	}
+
 	/* if this client has been added right now, it is possible to
 	 * immediately purge it
 	 */
@@ -1447,21 +1484,37 @@ static void batadv_tt_local_purge_list(struct batadv_priv *bat_priv,
 
 	hlist_for_each_entry_safe(tt_common_entry, node_tmp, head,
 				  hash_entry) {
+		bool cont = false;
+
 		tt_local_entry = container_of(tt_common_entry,
 					      struct batadv_tt_local_entry,
 					      common);
-		if (tt_local_entry->common.flags & BATADV_TT_CLIENT_NOPURGE)
+
+		scoped_guard(spinlock_bh, &tt_local_entry->common.flags_lock) {
+			if (tt_local_entry->common.flags & BATADV_TT_CLIENT_NOPURGE) {
+				cont = true;
+				break;
+			}
+
+			/* entry already marked for deletion */
+			if (tt_local_entry->common.flags & BATADV_TT_CLIENT_PENDING) {
+				cont = true;
+				break;
+			}
+
+			if (!batadv_has_timed_out(tt_local_entry->last_seen, timeout)) {
+				cont = true;
+				break;
+			}
+
+			tt_local_entry->common.flags |= BATADV_TT_CLIENT_PENDING;
+		}
+
+		if (cont)
 			continue;
 
-		/* entry already marked for deletion */
-		if (tt_local_entry->common.flags & BATADV_TT_CLIENT_PENDING)
-			continue;
-
-		if (!batadv_has_timed_out(tt_local_entry->last_seen, timeout))
-			continue;
-
-		batadv_tt_local_set_pending(bat_priv, tt_local_entry,
-					    BATADV_TT_CLIENT_DEL, "timed out");
+		batadv_tt_local_set_pending_event(bat_priv, tt_local_entry,
+						  BATADV_TT_CLIENT_DEL, "timed out");
 	}
 }
 
@@ -1667,8 +1720,10 @@ batadv_tt_global_sync_flags(struct batadv_tt_global_entry *tt_global)
 		flags |= orig_entry->flags;
 	rcu_read_unlock();
 
-	flags |= tt_global->common.flags & (~BATADV_TT_SYNC_MASK);
-	tt_global->common.flags = flags;
+	scoped_guard(spinlock_bh, &tt_global->common.flags_lock) {
+		flags |= tt_global->common.flags & (~BATADV_TT_SYNC_MASK);
+		tt_global->common.flags = flags;
+	}
 }
 
 /**
@@ -1752,6 +1807,7 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv,
 	bool ret = false;
 	u16 local_flags;
 	int hash_added;
+	bool delete;
 
 	/* ignore global entries from backbone nodes */
 	if (batadv_bla_is_backbone_gw_orig(bat_priv, orig_node->orig, vid))
@@ -1764,9 +1820,12 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv,
 	 * for a roaming advertisement instead of manually messing up the global
 	 * table
 	 */
-	if ((flags & BATADV_TT_CLIENT_TEMP) && tt_local_entry &&
-	    !(tt_local_entry->common.flags & BATADV_TT_CLIENT_NEW))
-		goto out;
+	if ((flags & BATADV_TT_CLIENT_TEMP) && tt_local_entry) {
+		scoped_guard(spinlock_bh, &tt_local_entry->common.flags_lock) {
+			if (!(tt_local_entry->common.flags & BATADV_TT_CLIENT_NEW))
+				goto out;
+		}
+	}
 
 	if (!tt_global_entry) {
 		tt_global_entry = kmem_cache_zalloc(batadv_tg_cache,
@@ -1777,9 +1836,12 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv,
 		common = &tt_global_entry->common;
 		ether_addr_copy(common->addr, tt_addr);
 		common->vid = vid;
+		spin_lock_init(&common->flags_lock);
 
-		if (!is_multicast_ether_addr(common->addr))
-			common->flags = flags & (~BATADV_TT_SYNC_MASK);
+		if (!is_multicast_ether_addr(common->addr)) {
+			scoped_guard(spinlock_bh, &common->flags_lock)
+				common->flags = flags & (~BATADV_TT_SYNC_MASK);
+		}
 
 		tt_global_entry->roam_at = 0;
 		/* node must store current time in case of roaming. This is
@@ -1808,6 +1870,7 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv,
 		}
 	} else {
 		common = &tt_global_entry->common;
+
 		/* If there is already a global entry, we can use this one for
 		 * our processing.
 		 * But if we are trying to add a temporary client then here are
@@ -1819,8 +1882,11 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv,
 		 *    originator list and add the new one orig_entry
 		 */
 		if (flags & BATADV_TT_CLIENT_TEMP) {
-			if (!(common->flags & BATADV_TT_CLIENT_TEMP))
-				goto out;
+			scoped_guard(spinlock_bh, &common->flags_lock) {
+				if (!(common->flags & BATADV_TT_CLIENT_TEMP))
+					goto out;
+			}
+
 			if (batadv_tt_global_entry_has_orig(tt_global_entry,
 							    orig_node, NULL))
 				goto out_remove;
@@ -1828,36 +1894,42 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv,
 			goto add_orig_entry;
 		}
 
-		/* if the client was temporary added before receiving the first
-		 * OGM announcing it, we have to clear the TEMP flag. Also,
-		 * remove the previous temporary orig node and re-add it
-		 * if required. If the orig entry changed, the new one which
-		 * is a non-temporary entry is preferred.
-		 */
-		if (common->flags & BATADV_TT_CLIENT_TEMP) {
-			batadv_tt_global_del_orig_list(tt_global_entry);
-			common->flags &= ~BATADV_TT_CLIENT_TEMP;
+		delete = false;
+		scoped_guard(spinlock_bh, &common->flags_lock) {
+			/* if the client was temporary added before receiving the first
+			 * OGM announcing it, we have to clear the TEMP flag. Also,
+			 * remove the previous temporary orig node and re-add it
+			 * if required. If the orig entry changed, the new one which
+			 * is a non-temporary entry is preferred.
+			 */
+			if (common->flags & BATADV_TT_CLIENT_TEMP) {
+				delete = true;
+				common->flags &= ~BATADV_TT_CLIENT_TEMP;
+			}
+
+			/* the change can carry possible "attribute" flags like the
+			 * TT_CLIENT_TEMP, therefore they have to be copied in the
+			 * client entry
+			 */
+			if (!is_multicast_ether_addr(common->addr))
+				common->flags |= flags & (~BATADV_TT_SYNC_MASK);
+
+			/* If there is the BATADV_TT_CLIENT_ROAM flag set, there is only
+			 * one originator left in the list and we previously received a
+			 * delete + roaming change for this originator.
+			 *
+			 * We should first delete the old originator before adding the
+			 * new one.
+			 */
+			if (common->flags & BATADV_TT_CLIENT_ROAM) {
+				delete = true;
+				common->flags &= ~BATADV_TT_CLIENT_ROAM;
+				tt_global_entry->roam_at = 0;
+			}
 		}
 
-		/* the change can carry possible "attribute" flags like the
-		 * TT_CLIENT_TEMP, therefore they have to be copied in the
-		 * client entry
-		 */
-		if (!is_multicast_ether_addr(common->addr))
-			common->flags |= flags & (~BATADV_TT_SYNC_MASK);
-
-		/* If there is the BATADV_TT_CLIENT_ROAM flag set, there is only
-		 * one originator left in the list and we previously received a
-		 * delete + roaming change for this originator.
-		 *
-		 * We should first delete the old originator before adding the
-		 * new one.
-		 */
-		if (common->flags & BATADV_TT_CLIENT_ROAM) {
+		if (delete)
 			batadv_tt_global_del_orig_list(tt_global_entry);
-			common->flags &= ~BATADV_TT_CLIENT_ROAM;
-			tt_global_entry->roam_at = 0;
-		}
 	}
 add_orig_entry:
 	/* add the new orig_entry (if needed) or update it */
@@ -1881,13 +1953,16 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv,
 	local_flags = batadv_tt_local_remove(bat_priv, tt_addr, vid,
 					     "global tt received",
 					     flags & BATADV_TT_CLIENT_ROAM);
-	tt_global_entry->common.flags |= local_flags & BATADV_TT_CLIENT_WIFI;
 
-	if (!(flags & BATADV_TT_CLIENT_ROAM))
-		/* this is a normal global add. Therefore the client is not in a
-		 * roaming state anymore.
-		 */
-		tt_global_entry->common.flags &= ~BATADV_TT_CLIENT_ROAM;
+	scoped_guard(spinlock_bh, &tt_global_entry->common.flags_lock) {
+		tt_global_entry->common.flags |= local_flags & BATADV_TT_CLIENT_WIFI;
+
+		if (!(flags & BATADV_TT_CLIENT_ROAM))
+			/* this is a normal global add. Therefore the client is not in a
+			 * roaming state anymore.
+			 */
+			tt_global_entry->common.flags &= ~BATADV_TT_CLIENT_ROAM;
+	}
 
 out:
 	batadv_tt_global_entry_put(tt_global_entry);
@@ -1957,9 +2032,9 @@ batadv_tt_global_dump_subentry(struct sk_buff *msg, u32 portid, u32 seq,
 			       struct batadv_tt_orig_list_entry *orig,
 			       bool best)
 {
-	u16 flags = (common->flags & (~BATADV_TT_SYNC_MASK)) | orig->flags;
 	struct batadv_orig_node_vlan *vlan;
 	u8 last_ttvn;
+	u16 flags;
 	void *hdr;
 	u32 crc;
 
@@ -1978,6 +2053,9 @@ batadv_tt_global_dump_subentry(struct sk_buff *msg, u32 portid, u32 seq,
 	if (!hdr)
 		return -ENOBUFS;
 
+	scoped_guard(spinlock_bh, &common->flags_lock)
+		flags = (common->flags & (~BATADV_TT_SYNC_MASK)) | orig->flags;
+
 	last_ttvn = READ_ONCE(orig->orig_node->last_ttvn);
 
 	if (nla_put(msg, BATADV_ATTR_TT_ADDRESS, ETH_ALEN, common->addr) ||
@@ -2266,7 +2344,9 @@ batadv_tt_global_del_roaming(struct batadv_priv *bat_priv,
 
 	if (last_entry) {
 		/* its the last one, mark for roaming. */
-		tt_global_entry->common.flags |= BATADV_TT_CLIENT_ROAM;
+		scoped_guard(spinlock_bh, &tt_global_entry->common.flags_lock)
+			tt_global_entry->common.flags |= BATADV_TT_CLIENT_ROAM;
+
 		tt_global_entry->roam_at = jiffies;
 	} else {
 		/* there is another entry, we can simply delete this
@@ -2416,16 +2496,18 @@ static bool batadv_tt_global_to_purge(struct batadv_tt_global_entry *tt_global,
 	unsigned long temp_timeout = BATADV_TT_CLIENT_TEMP_TIMEOUT;
 	bool purge = false;
 
-	if ((tt_global->common.flags & BATADV_TT_CLIENT_ROAM) &&
-	    batadv_has_timed_out(tt_global->roam_at, roam_timeout)) {
-		purge = true;
-		*msg = "Roaming timeout\n";
-	}
+	scoped_guard(spinlock_bh, &tt_global->common.flags_lock) {
+		if ((tt_global->common.flags & BATADV_TT_CLIENT_ROAM) &&
+		    batadv_has_timed_out(tt_global->roam_at, roam_timeout)) {
+			purge = true;
+			*msg = "Roaming timeout\n";
+		}
 
-	if ((tt_global->common.flags & BATADV_TT_CLIENT_TEMP) &&
-	    batadv_has_timed_out(tt_global->common.added_at, temp_timeout)) {
-		purge = true;
-		*msg = "Temporary client timeout\n";
+		if ((tt_global->common.flags & BATADV_TT_CLIENT_TEMP) &&
+		    batadv_has_timed_out(tt_global->common.added_at, temp_timeout)) {
+			purge = true;
+			*msg = "Temporary client timeout\n";
+		}
 	}
 
 	return purge;
@@ -2534,13 +2616,22 @@ static bool
 _batadv_is_ap_isolated(struct batadv_tt_local_entry *tt_local_entry,
 		       struct batadv_tt_global_entry *tt_global_entry)
 {
-	if (tt_local_entry->common.flags & BATADV_TT_CLIENT_WIFI &&
-	    tt_global_entry->common.flags & BATADV_TT_CLIENT_WIFI)
+	u16 global_flags;
+	u16 local_flags;
+
+	scoped_guard(spinlock_bh, &tt_global_entry->common.flags_lock)
+		global_flags = tt_global_entry->common.flags;
+
+	scoped_guard(spinlock_bh, &tt_local_entry->common.flags_lock)
+		local_flags = tt_local_entry->common.flags;
+
+	if (local_flags & BATADV_TT_CLIENT_WIFI &&
+	    global_flags & BATADV_TT_CLIENT_WIFI)
 		return true;
 
 	/* check if the two clients are marked as isolated */
-	if (tt_local_entry->common.flags & BATADV_TT_CLIENT_ISOLA &&
-	    tt_global_entry->common.flags & BATADV_TT_CLIENT_ISOLA)
+	if (local_flags & BATADV_TT_CLIENT_ISOLA &&
+	    global_flags & BATADV_TT_CLIENT_ISOLA)
 		return true;
 
 	return false;
@@ -2572,9 +2663,13 @@ struct batadv_orig_node *batadv_transtable_search(struct batadv_priv *bat_priv,
 
 	if (src && batadv_vlan_ap_isola_get(bat_priv, vid)) {
 		tt_local_entry = batadv_tt_local_hash_find(bat_priv, src, vid);
-		if (!tt_local_entry ||
-		    (tt_local_entry->common.flags & BATADV_TT_CLIENT_PENDING))
+		if (!tt_local_entry)
 			goto out;
+
+		scoped_guard(spinlock_bh, &tt_local_entry->common.flags_lock) {
+			if (tt_local_entry->common.flags & BATADV_TT_CLIENT_PENDING)
+				goto out;
+		}
 	}
 
 	tt_global_entry = batadv_tt_global_hash_find(bat_priv, addr, vid);
@@ -2648,6 +2743,8 @@ static u32 batadv_tt_global_crc(struct batadv_priv *bat_priv,
 
 		rcu_read_lock();
 		hlist_for_each_entry_rcu(tt_common, head, hash_entry) {
+			bool cont = false;
+
 			tt_global = container_of(tt_common,
 						 struct batadv_tt_global_entry,
 						 common);
@@ -2657,18 +2754,28 @@ static u32 batadv_tt_global_crc(struct batadv_priv *bat_priv,
 			if (tt_common->vid != vid)
 				continue;
 
-			/* Roaming clients are in the global table for
-			 * consistency only. They don't have to be
-			 * taken into account while computing the
-			 * global crc
-			 */
-			if (tt_common->flags & BATADV_TT_CLIENT_ROAM)
-				continue;
-			/* Temporary clients have not been announced yet, so
-			 * they have to be skipped while computing the global
-			 * crc
-			 */
-			if (tt_common->flags & BATADV_TT_CLIENT_TEMP)
+			scoped_guard(spinlock_bh, &tt_common->flags_lock) {
+				/* Roaming clients are in the global table for
+				 * consistency only. They don't have to be
+				 * taken into account while computing the
+				 * global crc
+				 */
+				if (tt_common->flags & BATADV_TT_CLIENT_ROAM) {
+					cont = true;
+					break;
+				}
+
+				/* Temporary clients have not been announced yet, so
+				 * they have to be skipped while computing the global
+				 * crc
+				 */
+				if (tt_common->flags & BATADV_TT_CLIENT_TEMP) {
+					cont = true;
+					break;
+				}
+			}
+
+			if (cont)
 				continue;
 
 			/* find out if this global entry is announced by this
@@ -2728,16 +2835,27 @@ static u32 batadv_tt_local_crc(struct batadv_priv *bat_priv,
 
 		rcu_read_lock();
 		hlist_for_each_entry_rcu(tt_common, head, hash_entry) {
+			bool cont = false;
+
 			/* compute the CRC only for entries belonging to the
 			 * VLAN identified by vid
 			 */
 			if (tt_common->vid != vid)
 				continue;
 
-			/* not yet committed clients have not to be taken into
-			 * account while computing the CRC
-			 */
-			if (tt_common->flags & BATADV_TT_CLIENT_NEW)
+			scoped_guard(spinlock_bh, &tt_common->flags_lock) {
+				/* not yet committed clients have not to be taken into
+				 * account while computing the CRC
+				 */
+				if (tt_common->flags & BATADV_TT_CLIENT_NEW) {
+					cont = true;
+					break;
+				}
+
+				flags = tt_common->flags & BATADV_TT_SYNC_MASK;
+			}
+
+			if (cont)
 				continue;
 
 			/* use network order to read the VID: this ensures that
@@ -2749,7 +2867,6 @@ static u32 batadv_tt_local_crc(struct batadv_priv *bat_priv,
 			/* compute the CRC on flags that have to be kept in sync
 			 * among nodes
 			 */
-			flags = tt_common->flags & BATADV_TT_SYNC_MASK;
 			crc_tmp = crc32c(crc_tmp, &flags, sizeof(flags));
 
 			crc ^= crc32c(crc_tmp, tt_common->addr, ETH_ALEN);
@@ -2907,17 +3024,19 @@ batadv_tt_req_node_new(struct batadv_priv *bat_priv,
  *
  * Return: true if the entry is a valid, false otherwise.
  */
-static bool batadv_tt_local_valid(const void *entry_ptr,
+static bool batadv_tt_local_valid(void *entry_ptr,
 				  const void *data_ptr,
 				  u8 *flags)
 {
-	const struct batadv_tt_common_entry *tt_common_entry = entry_ptr;
+	struct batadv_tt_common_entry *tt_common_entry = entry_ptr;
 
-	if (tt_common_entry->flags & BATADV_TT_CLIENT_NEW)
-		return false;
+	scoped_guard(spinlock_bh, &tt_common_entry->flags_lock) {
+		if (tt_common_entry->flags & BATADV_TT_CLIENT_NEW)
+			return false;
 
-	if (flags)
-		*flags = tt_common_entry->flags;
+		if (flags)
+			*flags = tt_common_entry->flags;
+	}
 
 	return true;
 }
@@ -2934,17 +3053,19 @@ static bool batadv_tt_local_valid(const void *entry_ptr,
  *
  * Return: true if the entry is a valid, false otherwise.
  */
-static bool batadv_tt_global_valid(const void *entry_ptr,
+static bool batadv_tt_global_valid(void *entry_ptr,
 				   const void *data_ptr,
 				   u8 *flags)
 {
-	const struct batadv_tt_common_entry *tt_common_entry = entry_ptr;
-	const struct batadv_tt_global_entry *tt_global_entry;
+	struct batadv_tt_common_entry *tt_common_entry = entry_ptr;
 	const struct batadv_orig_node *orig_node = data_ptr;
+	struct batadv_tt_global_entry *tt_global_entry;
 
-	if (tt_common_entry->flags & BATADV_TT_CLIENT_ROAM ||
-	    tt_common_entry->flags & BATADV_TT_CLIENT_TEMP)
-		return false;
+	scoped_guard(spinlock_bh, &tt_common_entry->flags_lock) {
+		if (tt_common_entry->flags & BATADV_TT_CLIENT_ROAM ||
+		    tt_common_entry->flags & BATADV_TT_CLIENT_TEMP)
+			return false;
+	}
 
 	tt_global_entry = container_of(tt_common_entry,
 				       struct batadv_tt_global_entry,
@@ -2972,7 +3093,7 @@ static bool batadv_tt_global_valid(const void *entry_ptr,
 static u16 batadv_tt_tvlv_generate(struct batadv_priv *bat_priv,
 				   struct batadv_hashtable *hash,
 				   void *tvlv_buff, u16 tt_len,
-				   bool (*valid_cb)(const void *,
+				   bool (*valid_cb)(void *,
 						    const void *,
 						    u8 *flags),
 				   void *cb_data)
@@ -3602,12 +3723,16 @@ bool batadv_is_my_client(struct batadv_priv *bat_priv, const u8 *addr,
 	tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr, vid);
 	if (!tt_local_entry)
 		goto out;
+
 	/* Check if the client has been logically deleted (but is kept for
 	 * consistency purpose)
 	 */
-	if ((tt_local_entry->common.flags & BATADV_TT_CLIENT_PENDING) ||
-	    (tt_local_entry->common.flags & BATADV_TT_CLIENT_ROAM))
-		goto out;
+	scoped_guard(spinlock_bh, &tt_local_entry->common.flags_lock) {
+		if ((tt_local_entry->common.flags & BATADV_TT_CLIENT_PENDING) ||
+		    (tt_local_entry->common.flags & BATADV_TT_CLIENT_ROAM))
+			goto out;
+	}
+
 	ret = true;
 out:
 	batadv_tt_local_entry_put(tt_local_entry);
@@ -3888,10 +4013,19 @@ static void batadv_tt_local_transition_new(struct batadv_priv *bat_priv)
 		rcu_read_lock();
 		hlist_for_each_entry_rcu(tt_common_entry,
 					 head, hash_entry) {
-			if (!(tt_common_entry->flags & BATADV_TT_CLIENT_NEW))
-				continue;
+			bool cont = false;
 
-			tt_common_entry->flags &= ~BATADV_TT_CLIENT_NEW;
+			scoped_guard(spinlock_bh, &tt_common_entry->flags_lock) {
+				if (!(tt_common_entry->flags & BATADV_TT_CLIENT_NEW)) {
+					cont = true;
+					break;
+				}
+
+				tt_common_entry->flags &= ~BATADV_TT_CLIENT_NEW;
+			}
+
+			if (cont)
+				continue;
 
 			batadv_tt_local_size_inc(bat_priv,
 						 tt_common_entry->vid);
@@ -3928,16 +4062,26 @@ static void batadv_tt_local_purge_pending_clients(struct batadv_priv *bat_priv)
 		spin_lock_bh(list_lock);
 		hlist_for_each_entry_safe(tt_common, node_tmp, head,
 					  hash_entry) {
-			if (!(tt_common->flags & BATADV_TT_CLIENT_PENDING))
+			bool cont = false;
+
+			scoped_guard(spinlock_bh, &tt_common->flags_lock) {
+				if (!(tt_common->flags & BATADV_TT_CLIENT_PENDING)) {
+					cont = true;
+					break;
+				}
+
+				batadv_dbg(BATADV_DBG_TT, bat_priv,
+					   "Deleting local tt entry (%pM, vid: %d): pending\n",
+					   tt_common->addr,
+					   batadv_print_vid(tt_common->vid));
+
+				batadv_tt_local_size_dec(bat_priv, tt_common->vid);
+				hlist_del_rcu(&tt_common->hash_entry);
+			}
+
+			if (cont)
 				continue;
 
-			batadv_dbg(BATADV_DBG_TT, bat_priv,
-				   "Deleting local tt entry (%pM, vid: %d): pending\n",
-				   tt_common->addr,
-				   batadv_print_vid(tt_common->vid));
-
-			batadv_tt_local_size_dec(bat_priv, tt_common->vid);
-			hlist_del_rcu(&tt_common->hash_entry);
 			tt_local = container_of(tt_common,
 						struct batadv_tt_local_entry,
 						common);
@@ -4141,7 +4285,9 @@ bool batadv_tt_global_client_is_roaming(struct batadv_priv *bat_priv,
 	if (!tt_global_entry)
 		goto out;
 
-	ret = tt_global_entry->common.flags & BATADV_TT_CLIENT_ROAM;
+	scoped_guard(spinlock_bh, &tt_global_entry->common.flags_lock)
+		ret = tt_global_entry->common.flags & BATADV_TT_CLIENT_ROAM;
+
 	batadv_tt_global_entry_put(tt_global_entry);
 out:
 	return ret;
@@ -4167,7 +4313,9 @@ bool batadv_tt_local_client_is_roaming(struct batadv_priv *bat_priv,
 	if (!tt_local_entry)
 		goto out;
 
-	ret = tt_local_entry->common.flags & BATADV_TT_CLIENT_ROAM;
+	scoped_guard(spinlock_bh, &tt_local_entry->common.flags_lock)
+		ret = tt_local_entry->common.flags & BATADV_TT_CLIENT_ROAM;
+
 	batadv_tt_local_entry_put(tt_local_entry);
 out:
 	return ret;
@@ -4478,7 +4626,8 @@ bool batadv_tt_global_is_isolated(struct batadv_priv *bat_priv,
 	if (!tt)
 		return false;
 
-	ret = tt->common.flags & BATADV_TT_CLIENT_ISOLA;
+	scoped_guard(spinlock_bh, &tt->common.flags_lock)
+		ret = tt->common.flags & BATADV_TT_CLIENT_ISOLA;
 
 	batadv_tt_global_entry_put(tt);
 
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 9bdc5a3e..142169be 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -1851,12 +1851,18 @@ struct batadv_tt_common_entry {
 	 */
 	struct hlist_node hash_entry;
 
-	/** @flags: various state handling flags (see batadv_tt_client_flags) */
+	/**
+	 * @flags: various state handling flags (see batadv_tt_client_flags),
+	 * protected by @flags_lock
+	 */
 	u16 flags;
 
 	/** @added_at: timestamp used for purging stale tt common entries */
 	unsigned long added_at;
 
+	/** @flags_lock: protect modifications of @flags */
+	spinlock_t flags_lock;
+
 	/** @refcount: number of contexts the object is used */
 	struct kref refcount;
 

-- 
2.47.3

[PATCH RFC batadv v3 5/6] batman-adv: tt: decrement count for committed client on local_remove

[Sven Eckelmann]

Local TT entries which have the BATADV_TT_CLIENT_NEW no longer set are
committed. The batadv_tt_local_size_inc() was called for them and thus the
batadv_tt_local_size_dec() has to be called also when
batadv_tt_local_remove() is called for them when the BATADV_TT_CLIENT_NEW
wasn't consumed in the remove path.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/translation-table.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index df99c67e..ce76d056 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -1458,6 +1458,18 @@ u16 batadv_tt_local_remove(struct batadv_priv *bat_priv, const u8 *addr,
 	if (!tt_removed_node)
 		goto out;
 
+	/* batadv_tt_local_transition_new() may have committed the entry and
+	 * thus counted it in the local table size since the BATADV_TT_CLIENT_NEW
+	 * check above.
+	 */
+	scoped_guard(spinlock_bh, &tt_local_entry->common.flags_lock) {
+		if (tt_local_entry->common.flags & BATADV_TT_CLIENT_NEW)
+			break;
+
+		tt_local_entry->common.flags &= ~BATADV_TT_CLIENT_NEW;
+		batadv_tt_local_size_dec(bat_priv, tt_local_entry->common.vid);
+	}
+
 	/* drop reference of remove hash entry */
 	batadv_tt_local_entry_put(tt_local_entry);
 

-- 
2.47.3

[PATCH RFC batadv v3 6/6] batman-adv: tt: don't uncount never committed clients on pending purge

[Sven Eckelmann]

batadv_tt_local_purge_pending_clients() decreases vlan->tt.num_entries for
every entry it unlinks, but an entry which still carries
BATADV_TT_CLIENT_NEW was never counted by batadv_tt_local_transition_new().

batadv_tt_local_resize_to_mtu() calls batadv_tt_local_purge() and
batadv_tt_local_purge_pending_clients() directly, without committing in
between, and it halves its timeout down towards zero. Once the timeout is
short enough, batadv_tt_local_purge_list() marks even freshly added clients
as pending. And each of them decreases a counter it never increased.

Consume the BATADV_TT_CLIENT_NEW accounting token here as well, so that the
counter is only decreased for entries which were actually counted.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/translation-table.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index ce76d056..f6c18620 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -4087,8 +4087,15 @@ static void batadv_tt_local_purge_pending_clients(struct batadv_priv *bat_priv)
 					   tt_common->addr,
 					   batadv_print_vid(tt_common->vid));
 
-				batadv_tt_local_size_dec(bat_priv, tt_common->vid);
 				hlist_del_rcu(&tt_common->hash_entry);
+
+				/* An entry which still carries BATADV_TT_CLIENT_NEW was
+				 * never counted and must not be uncounted here.
+				 */
+				if (!(tt_common->flags & BATADV_TT_CLIENT_NEW))
+					batadv_tt_local_size_dec(bat_priv, tt_common->vid);
+				else
+					tt_common->flags &= ~BATADV_TT_CLIENT_NEW;
 			}
 
 			if (cont)

-- 
2.47.3

Re: [PATCH RFC batadv v3 0/6] batman-adv: tt: atomic sashiko fixes

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 1513 bytes --]

On Wednesday, 5 August 2026 17:09:34 CEST Sven Eckelmann wrote:
> There were a couple of sashiko reports regarding the
> batadv-next-pullrequest-20260728 PR. I went through the TT code and tried
> to handle them and at the same time already predict what else sashiko might
> report as next. I needed to change the batadv_tt_local_remove() hash
> interaction as preparation for the related NEW flag (and counter) handling.
> 
> And the netdev maintainers rejected this approach (for now). I had to
> re-add the atomic_t code parts back in this patchset. But since the netdev
> maintainers didn't like the atomic_t implementation, everything was changed
> to atomic_t with the help of scoped_guard().
> 
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> ---
> Changes in v3:
> - switched to spinlock_bh due to rejection atomic_t by netdev (and
>   potentially easier code)

Mixed up some corner case flag handling while converting it back from atomic_t 
to the original simple handling. My bad but easy to fix in v4.

But it is more problematic that the cleanup.h states:

 * Lastly, given that the benefit of cleanup helpers is removal of
 * "goto", and that the "goto" statement can jump between scopes, the
 * expectation is that usage of "goto" and cleanup helpers is never
 * mixed in the same function. I.e. for a given routine, convert all
 * resources that need a "goto" cleanup to scope-based cleanup, or
 * convert none of them.

I am not 100% sure how to deal with this at the moment

Regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]