[PATCH net-next 0/9] pull request for net-next: batman-adv 2026-09-30

[PATCH net-next 0/9] pull request for net-next: batman-adv 2026-09-30

[Simon Wunderlich]

Dear net maintainers,

here is another feature/cleanup pull request of batman-adv to go into net-next.

Please pull or let me know of any problem!

Thank you,
      Simon

The following changes since commit 6d7f71ae4cdcbb22f541638057299fe26c451e65:

  batman-adv: correct batadv_hash_remove kdoc return type (2026-09-06 20:05:31 +0200)

are available in the Git repository at:

  https://git.open-mesh.org/batadv.git tags/batadv-next-pullrequest-20260930

for you to fetch changes up to d1807b20d548171d40c5d21a7cf22830ea8f348e:

  batman-adv: use assign_bit() where applicable (2026-09-27 19:03:34 +0200)

----------------------------------------------------------------
This feature/cleanup patchset includes the following patches:

 - bla: avoid double free after failed backbone_hash alloc,
   by Sven Eckelmann

 - tt: update kerneldoc, by Sven Eckelmann (3 patches)

 - tt: fix local DEL handling, by Sven Eckelmann (3 patches)

 - tt: reject VLAN/TT entries before reaching size limit,
   by Sven Eckelmann

 - use assign_bit() where applicable, by Peng Fan

----------------------------------------------------------------
Peng Fan (1):
      batman-adv: use assign_bit() where applicable

Sven Eckelmann (8):
      batman-adv: bla: avoid double free after failed backbone_hash alloc
      batman-adv: tt: clarify kernel-doc for batadv_tt_global_purge_local
      batman-adv: tt: clarify responsibility for roam flag during removal
      batman-adv: tt: soften kernel-doc for batadv_tt_local_remove_now()
      batman-adv: tt: only queue local del event after successful unlink
      batman-adv: tt: queue local DEL event under bucket lock
      batman-adv: tt: queue local DEL event before marking entry as pending
      batman-adv: tt: reject VLAN/TT entries before reaching size limit

 net/batman-adv/bridge_loop_avoidance.c |   1 +
 net/batman-adv/distributed-arp-table.c |   6 +-
 net/batman-adv/main.c                  |   1 +
 net/batman-adv/mesh-interface.c        |   8 +
 net/batman-adv/translation-table.c     | 359 ++++++++++++++++++++++++---------
 net/batman-adv/translation-table.h     |   3 +
 net/batman-adv/types.h                 |  19 ++
 7 files changed, 299 insertions(+), 98 deletions(-)

[PATCH net-next 1/9] batman-adv: bla: avoid double free after failed backbone_hash alloc

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

When batadv_bla_init() fails to initialize the backbone_hash, it is freeing
the (previously) allocated claim_hash. The initialization function will
then return an error and the net_device initialization will stop. The
destructor will be called instead and (indirectly via batadv_mesh_free() ->
batadv_bla_free()) will try to free the bat_priv->bla.claim_hash again.

The pointer must therefore be set to NULL after freeing it in the
initialization error path.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260831135117.574836-1-sw%40simonwunderlich.de?part=6
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/bridge_loop_avoidance.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index ad6ab4a50658f..a96f269da8d37 100644
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -1625,6 +1625,7 @@ int batadv_bla_init(struct batadv_priv *bat_priv)
 	bat_priv->bla.backbone_hash = batadv_hash_new(32);
 	if (!bat_priv->bla.backbone_hash) {
 		batadv_hash_destroy(bat_priv->bla.claim_hash);
+		bat_priv->bla.claim_hash = NULL;
 		return -ENOMEM;
 	}
 
-- 
2.47.3

Re: [PATCH net-next 1/9] batman-adv: bla: avoid double free after failed backbone_hash alloc

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit message describes a double free that cannot happen.

--

batman-adv: bla: avoid double free after failed backbone_hash alloc

This commit sets bat_priv->bla.claim_hash to NULL after
batadv_hash_destroy() in the batadv_bla_init() error path taken when the
backbone_hash allocation fails. The commit message says that the
destructor later frees claim_hash a second time via batadv_mesh_free()
-> batadv_bla_free().

> When batadv_bla_init() fails to initialize the backbone_hash, it is freeing
> the (previously) allocated claim_hash. The initialization function will
> then return an error and the net_device initialization will stop. The
> destructor will be called instead and (indirectly via batadv_mesh_free() ->
> batadv_bla_free()) will try to free the bat_priv->bla.claim_hash again.

[Severity: Medium]

Can this sequence actually happen? batadv_bla_init() has only one
caller, batadv_mesh_init(), and when it fails that function jumps past
batadv_bla_free():

net/batman-adv/main.c:batadv_mesh_init() {
    ...
	ret = batadv_bla_init(bat_priv);
	if (ret < 0) {
		WRITE_ONCE(bat_priv->mesh_state, BATADV_MESH_DEACTIVATING);
		goto err_bla;
	}
    ...
err_dat:
	batadv_bla_free(bat_priv);
err_bla:
	batadv_v_mesh_free(bat_priv);
    ...
}

batadv_mesh_init() is only reached from batadv_meshif_init_late(), the
.ndo_init callback, which returns the error. In register_netdevice(), an
ndo_init failure skips the priv_destructor call:

net/core/dev.c:register_netdevice() {
    ...
	if (dev->netdev_ops->ndo_init) {
		ret = dev->netdev_ops->ndo_init(dev);
		if (ret) {
			if (ret > 0)
				ret = -EIO;
			goto err_free_name;
		}
	}
    ...
err_uninit:
	if (dev->netdev_ops->ndo_uninit)
		dev->netdev_ops->ndo_uninit(dev);
	if (dev->priv_destructor)
		dev->priv_destructor(dev);
err_free_name:
	netdev_name_node_free(dev->name_node);
	goto out;
}

After that, free_netdev() sees reg_state == NETREG_UNINITIALIZED and
only calls kvfree(dev). So batadv_meshif_free() -> batadv_mesh_free() ->
batadv_bla_free() does not seem to run on this path, and nothing reads
or frees the stale claim_hash pointer again.

Also, if the destructor did run here, wouldn't batadv_bla_free() call
disable_delayed_work_sync(&bat_priv->bla.work) on work that was never
set up? INIT_DELAYED_WORK() only runs after both hashes are allocated.
The patch doesn't handle that case, so the description and the fix don't
seem to match.

This makes the change look like harmless hardening rather than a
double-free fix. Could the commit message describe it that way, or show
the path where the second batadv_hash_destroy() actually happens? As
written, stable or CVE tooling may treat it as a memory-safety fix.

> diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
> index ad6ab4a50658f..a96f269da8d37 100644
> --- a/net/batman-adv/bridge_loop_avoidance.c
> +++ b/net/batman-adv/bridge_loop_avoidance.c
> @@ -1625,6 +1625,7 @@ int batadv_bla_init(struct batadv_priv *bat_priv)
>  	bat_priv->bla.backbone_hash = batadv_hash_new(32);
>  	if (!bat_priv->bla.backbone_hash) {
>  		batadv_hash_destroy(bat_priv->bla.claim_hash);
> +		bat_priv->bla.claim_hash = NULL;
>  		return -ENOMEM;
>  	}
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930094558.3723766-2-sw%40simonwunderlich.de

Re: [PATCH net-next 1/9] batman-adv: bla: avoid double free after failed backbone_hash alloc

[Sven Eckelmann]

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

On Thursday, 1 October 2026 12:10:26 CEST netdev-bot+sashiko@kernel.org wrote:
[...]
> [Severity: Medium]
> 
> Can this sequence actually happen? batadv_bla_init() has only one
> caller, batadv_mesh_init(), and when it fails that function jumps past
> batadv_bla_free():

Even when trying to get sashiko to stop complaining about something, the other 
Sashiko instance will just complain about the opposite:

https://sashiko.dev/#/patchset/20260831135117.574836-1-sw%40simonwunderlich.de?part=6

Regards,
	Sven

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

Re: [PATCH net-next 1/9] batman-adv: bla: avoid double free after failed backbone_hash alloc

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Sven Eckelmann <sven@narfation.org>:

On Wed, 30 Sep 2026 11:45:50 +0200 you wrote:
> From: Sven Eckelmann <sven@narfation.org>
> 
> When batadv_bla_init() fails to initialize the backbone_hash, it is freeing
> the (previously) allocated claim_hash. The initialization function will
> then return an error and the net_device initialization will stop. The
> destructor will be called instead and (indirectly via batadv_mesh_free() ->
> batadv_bla_free()) will try to free the bat_priv->bla.claim_hash again.
> 
> [...]

Here is the summary with links:
  - [net-next,1/9] batman-adv: bla: avoid double free after failed backbone_hash alloc
    https://git.kernel.org/netdev/net-next/c/4c0a6a5b7715
  - [net-next,2/9] batman-adv: tt: clarify kernel-doc for batadv_tt_global_purge_local
    https://git.kernel.org/netdev/net-next/c/f11712fc3d5f
  - [net-next,3/9] batman-adv: tt: clarify responsibility for roam flag during removal
    https://git.kernel.org/netdev/net-next/c/08967c26cc8d
  - [net-next,4/9] batman-adv: tt: soften kernel-doc for batadv_tt_local_remove_now()
    https://git.kernel.org/netdev/net-next/c/be66c5ac5485
  - [net-next,5/9] batman-adv: tt: only queue local del event after successful unlink
    https://git.kernel.org/netdev/net-next/c/597c0f2f69c0
  - [net-next,6/9] batman-adv: tt: queue local DEL event under bucket lock
    https://git.kernel.org/netdev/net-next/c/f95b9f28a9df
  - [net-next,7/9] batman-adv: tt: queue local DEL event before marking entry as pending
    https://git.kernel.org/netdev/net-next/c/a6746cb19076
  - [net-next,8/9] batman-adv: tt: reject VLAN/TT entries before reaching size limit
    https://git.kernel.org/netdev/net-next/c/85fb027f611b
  - [net-next,9/9] batman-adv: use assign_bit() where applicable
    https://git.kernel.org/netdev/net-next/c/d1807b20d548

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

[PATCH net-next 2/9] batman-adv: tt: clarify kernel-doc for batadv_tt_global_purge_local

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

The new kernel-doc didn't mention the "non-multicast clients" limitation.
It was also not clear that the roaming flag gets cleared when the
announcement didn't contain the roaming flag.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907081824.2474040-1-sw%40simonwunderlich.de?part=9
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/translation-table.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 12ea557cb8033..6d8fa6d28fee8 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -1870,9 +1870,10 @@ batadv_tt_global_orig_entry_add(struct batadv_tt_global_entry *tt_global,
  * @tt_global_entry: the global TT entry of the announced client
  * @flags: TT flags announced for this non-mesh client
  *
- * A client which is announced by another originator is no longer a local
- * client. Remove it from the local table and take over the WIFI flag it was
- * tracked with.
+ * A non-multicast client which is announced by another originator is no longer
+ * a local client. Remove it from the local table and (for the global entry)
+ * take over the WIFI flag it was tracked with. Reset the roaming flag in case
+ * the announcement didn't contain the roaming flag.
  */
 static void
 batadv_tt_global_purge_local(struct batadv_priv *bat_priv,
-- 
2.47.3

[PATCH net-next 3/9] batman-adv: tt: clarify responsibility for roam flag during removal

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

If an local client gets removed due to an roam, then the local entry must
be marked as "roamed" to handle the rerouting correctly. But this is not
done by batadv_tt_local_remove() directly but by the new helper
batadv_tt_local_mark_removed(). The former will only create similar flags
for the roaming event.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907081824.2474040-1-sw%40simonwunderlich.de?part=10
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/translation-table.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 6d8fa6d28fee8..78f032c9ad65c 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -1461,7 +1461,10 @@ batadv_tt_local_mark_removed(struct batadv_tt_local_entry *tt_local_entry,
 	scoped_guard(spinlock_bh, &common->flags_lock) {
 		*curr_flags = common->flags;
 
-		/* mark the local client as ROAMed */
+		/* 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)
 			common->flags |= BATADV_TT_CLIENT_ROAM;
 
@@ -1532,22 +1535,22 @@ u16 batadv_tt_local_remove(struct batadv_priv *bat_priv, const u8 *addr,
 	if (!tt_local_entry)
 		return BATADV_NO_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;
+	if (batadv_tt_local_mark_removed(tt_local_entry, roaming, &curr_flags)) {
+		/* queue (roamed) del event which was prepared by
+		 * batadv_tt_local_mark_removed()
+		 */
+		flags = BATADV_TT_CLIENT_DEL;
+		if (roaming)
+			flags |= BATADV_TT_CLIENT_ROAM;
 
-	if (batadv_tt_local_mark_removed(tt_local_entry, roaming, &curr_flags))
 		batadv_tt_local_set_pending_event(bat_priv, tt_local_entry,
 						  flags, message);
-	else
+	} else {
 		/* if this client has been added right now, it is possible to
 		 * immediately purge it
 		 */
 		batadv_tt_local_remove_now(bat_priv, tt_local_entry);
+	}
 
 	batadv_tt_local_entry_put(tt_local_entry);
 
-- 
2.47.3

Re: [PATCH net-next 3/9] batman-adv: tt: clarify responsibility for roam flag during removal

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The comments rearranged by this patch don't fully match their
  functions.

--

batman-adv: tt: clarify responsibility for roam flag during removal

The comment about marking a local entry as "roamed" moves out of
batadv_tt_local_remove() and into batadv_tt_local_mark_removed(). That
helper is where BATADV_TT_CLIENT_ROAM is actually set on the entry.
Building the DEL/ROAM event flags in batadv_tt_local_remove() now happens
only inside the branch that queues the pending event.

Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907081824.2474040-1-sw%40simonwunderlich.de?part=10

> diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> index 6d8fa6d28fee8..78f032c9ad65c 100644
> --- a/net/batman-adv/translation-table.c
> +++ b/net/batman-adv/translation-table.c
> @@ -1461,7 +1461,10 @@ batadv_tt_local_mark_removed(struct batadv_tt_local_entry *tt_local_entry,
>  	scoped_guard(spinlock_bh, &common->flags_lock) {
>  		*curr_flags = common->flags;
>  
> -		/* mark the local client as ROAMed */
> +		/* 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)
>  			common->flags |= BATADV_TT_CLIENT_ROAM;
>  
> @@ -1532,22 +1535,22 @@ u16 batadv_tt_local_remove(struct batadv_priv *bat_priv, const u8 *addr,
>  	if (!tt_local_entry)
>  		return BATADV_NO_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;
> +	if (batadv_tt_local_mark_removed(tt_local_entry, roaming, &curr_flags)) {
> +		/* queue (roamed) del event which was prepared by
> +		 * batadv_tt_local_mark_removed()
> +		 */
> +		flags = BATADV_TT_CLIENT_DEL;
> +		if (roaming)
> +			flags |= BATADV_TT_CLIENT_ROAM;

[Severity: Low]

This isn't a bug, but is the "prepared by batadv_tt_local_mark_removed()"
wording accurate at this commit?

In this commit, batadv_tt_local_mark_removed() doesn't build any event. It
only saves *curr_flags and sets BATADV_TT_CLIENT_ROAM and
BATADV_TT_CLIENT_PENDING on common->flags. The DEL and ROAM event flags are
built right after this comment and then passed to
batadv_tt_local_set_pending_event().

Two later patches in this series move the DEL event queuing into
batadv_tt_local_mark_removed() and drop this comment. They are "batman-adv:
tt: queue local DEL event under bucket lock" and "batman-adv: tt: queue
local DEL event before marking entry as pending". So the mismatch only
exists at this commit.

The other two points are small wording nits. Both come from code that was
already there before this patch.

The comment moved into batadv_tt_local_mark_removed() starts with "if this
global entry addition is due to a roaming". The helper's kernel-doc
describes @roaming as "true if the deletion is due to a roaming event".

The only caller passing roaming=true is batadv_tt_global_add(), so the
wording is correct in context. Would it read more naturally in terms of
the deletion, to match the kernel-doc?

The ROAM bit added to flags here is already part of the emitted change.
batadv_tt_local_event() does:

	flags = batadv_tt_flags_get(common) | event_flags;

and batadv_tt_local_mark_removed() has already set BATADV_TT_CLIENT_ROAM
on common->flags. The commit message already says this ("The former will
only create similar flags for the roaming event"), so this is just a note.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930094558.3723766-2-sw%40simonwunderlich.de

Re: [PATCH net-next 3/9] batman-adv: tt: clarify responsibility for roam flag during removal

[Sven Eckelmann]

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

On Thursday, 1 October 2026 12:10:27 CEST netdev-bot+sashiko@kernel.org wrote:
[...]
> [Severity: Low]
> 
> This isn't a bug, but is the "prepared by batadv_tt_local_mark_removed()"
> wording accurate at this commit?
> 
> In this commit, batadv_tt_local_mark_removed() doesn't build any event. It
> only saves *curr_flags and sets BATADV_TT_CLIENT_ROAM and
> BATADV_TT_CLIENT_PENDING on common->flags. The DEL and ROAM event flags are
> built right after this comment and then passed to
> batadv_tt_local_set_pending_event().

This is just an intermediate move

> The other two points are small wording nits. Both come from code that was
> already there before this patch.
> 
> The comment moved into batadv_tt_local_mark_removed() starts with "if this
> global entry addition is due to a roaming". The helper's kernel-doc
> describes @roaming as "true if the deletion is due to a roaming event".
> 
> The only caller passing roaming=true is batadv_tt_global_add(), so the
> wording is correct in context. Would it read more naturally in terms of
> the deletion, to match the kernel-doc?

No, it would be wrong. The global one is added and the local one is removed.

Regards,
	Sven

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

[PATCH net-next 4/9] batman-adv: tt: soften kernel-doc for batadv_tt_local_remove_now()

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

The "never announced" statement is too harsh for an entry which could have
been actually be announced by a different context while the
batadv_tt_local_remove_now() is in the process of being called. "not yet"
might be more appropriate.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907081824.2474040-1-sw%40simonwunderlich.de?part=11
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/translation-table.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 78f032c9ad65c..354d9416c1b75 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -1478,13 +1478,16 @@ batadv_tt_local_mark_removed(struct batadv_tt_local_entry *tt_local_entry,
 }
 
 /**
- * batadv_tt_local_remove_now() - purge a local entry which was never announced
+ * batadv_tt_local_remove_now() - purge a local entry which was not (yet) announced
  * @bat_priv: the bat priv with all the mesh interface information
  * @tt_local_entry: local TT entry to purge
  *
- * A client which was added right after the last ttvn increment was never sent
- * to the other nodes. It can therefore be dropped from the local table without
- * waiting for the next ttvn increment.
+ * A client which was added right after the last ttvn increment was not (yet)
+ * sent to the other nodes. It can therefore be dropped from the local table
+ * without waiting for the next ttvn increment.
+ *
+ * If it was still announced by a parallel context before it was removed from
+ * the hash, then the local TT size adjustment will be handled automatically.
  */
 static void
 batadv_tt_local_remove_now(struct batadv_priv *bat_priv,
-- 
2.47.3

[PATCH net-next 5/9] batman-adv: tt: only queue local del event after successful unlink

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

When batadv_tt_local_remove_now() finds another local TT entry with the
same MAC + VLAN, it will not try to remove it. A delete event must not be
queued because it would otherwise cause the queued ADD event for the other
TT entry to be dropped.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831135117.574836-1-sw%40simonwunderlich.de?part=8
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/translation-table.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 354d9416c1b75..c904d67791f8f 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -1496,8 +1496,6 @@ batadv_tt_local_remove_now(struct batadv_priv *bat_priv,
 	struct batadv_tt_common_entry *common = &tt_local_entry->common;
 	struct hlist_node *tt_removed_node;
 
-	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_entry,
@@ -1505,6 +1503,8 @@ batadv_tt_local_remove_now(struct batadv_priv *bat_priv,
 	if (!tt_removed_node)
 		return;
 
+	batadv_tt_local_event(bat_priv, tt_local_entry, BATADV_TT_CLIENT_DEL);
+
 	/* 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 in batadv_tt_local_mark_removed().
-- 
2.47.3

Re: [PATCH net-next 5/9] batman-adv: tt: only queue local del event after successful unlink

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix is incomplete, and the reordering opens a new window.
- [Low] The commit fixes a TT announcement bug but has no Fixes: tag.

--

batman-adv: tt: only queue local del event after successful unlink

This commit moves the BATADV_TT_CLIENT_DEL event in
batadv_tt_local_remove_now() so it runs after batadv_hash_remove(). A
delete event is now only queued when this exact entry was unlinked. The
goal is to stop cancelling the pending add event of another local entry
with the same MAC and VLAN.

Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831135117.574836-1-sw%40simonwunderlich.de?part=8

> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831135117.574836-1-sw%40simonwunderlich.de?part=8
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>

[Severity: Low]

This isn't a runtime bug, but should this have a Fixes: tag?

The problem seems to come from bbfb0d2cf035 ("batman-adv: tt: remove only
the entry which was looked up from the hash"). That commit changed the
unlink in batadv_tt_local_remove() from batadv_compare_tt to
batadv_compare_tt_entry. It kept the unconditional delete event in front
of the unlink.

931de98d3960 ("batman-adv: tt: extract immediate purge of a local entry")
only moved that code into batadv_tt_local_remove_now().

Other fixes in the same series have Fixes: tags, for example "batman-adv:
tt: queue local DEL event under bucket lock". Perhaps something like:

Fixes: bbfb0d2cf035 ("batman-adv: tt: remove only the entry which was looked up from the hash")

> diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> index 354d9416c1b75..c904d67791f8f 100644
> --- a/net/batman-adv/translation-table.c
> +++ b/net/batman-adv/translation-table.c
> @@ -1496,8 +1496,6 @@ batadv_tt_local_remove_now(struct batadv_priv *bat_priv,
>  	struct batadv_tt_common_entry *common = &tt_local_entry->common;
>  	struct hlist_node *tt_removed_node;
>  
> -	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_entry,
> @@ -1505,6 +1503,8 @@ batadv_tt_local_remove_now(struct batadv_priv *bat_priv,
>  	if (!tt_removed_node)
>  		return;
>  
> +	batadv_tt_local_event(bat_priv, tt_local_entry, BATADV_TT_CLIENT_DEL);
> +

[Severity: High]

Does this reordering open a new window in which a replacement entry's add
event gets dropped?

batadv_hash_remove() takes and releases the bucket list_lock before it
returns. batadv_tt_local_event() only takes tt.changes_list_lock after
that. Nothing seems to serialize this gap against batadv_tt_local_add()
called from batadv_interface_tx() for the same MAC and VID.

The remove side can be reached from a remote TT announcement:

batadv_tt_global_add()
  batadv_tt_global_purge_local()
    batadv_tt_local_remove()
      batadv_tt_local_remove_now()

Consider entry E, which is still NEW and has a queued add event:

CPU0 (remove path)
batadv_hash_remove(E)
  /* E unlinked, its add event still queued */

CPU1 (batadv_tt_local_add())
batadv_tt_local_hash_find()      /* misses E */
batadv_tt_local_create()         /* new entry E' */
batadv_hash_add(E')
batadv_tt_local_event(E', BATADV_NO_FLAGS)
  /* same addr/vid as E's pending add, so only flags are updated */

CPU0
batadv_tt_local_event(E, BATADV_TT_CLIENT_DEL)
  /* matches the remaining add by addr/vid only */
  if (del_op_requested != del_op_entry) {
      list_del(&entry->list);
      kmem_cache_free(batadv_tt_change_cache, entry);
      changes--;

E' is then in tt.local_hash with BATADV_TT_CLIENT_NEW set and no add
event queued. The commit message says this should be prevented.

Later packets from the client do not queue it again, because
batadv_tt_local_add_existing() returns false for a NEW entry that is not
PENDING.

If local_changes drops to 0, batadv_tt_local_commit_changes_nolock()
returns early and E' stays unannounced. A later unrelated commit would
move E' into the CRC through batadv_tt_local_transition_new() without an
add diff. Neighbours would then see a CRC mismatch and fall back to full
table requests.

Before this patch the delete event was queued while E was still in the
hash. An add that ran after the unlink would then get a fresh add event
that nothing cancels.

The later patches in the series don't seem to change this part of
batadv_tt_local_remove_now().

Could the delete event instead be queued inside the same bucket list_lock
critical section as the unlink, and only when this exact object was
found? batadv_tt_local_set_pending() in "batman-adv: tt: queue local DEL
event under bucket lock" already takes list_lock and then
changes_list_lock in that order.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930094558.3723766-2-sw%40simonwunderlich.de

Re: [PATCH net-next 5/9] batman-adv: tt: only queue local del event after successful unlink

[Sven Eckelmann]

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

On Thursday, 1 October 2026 12:10:28 CEST netdev-bot+sashiko@kernel.org wrote:
[...]
> [Severity: Low]
> 
> This isn't a runtime bug, but should this have a Fixes: tag?
> 
> The problem seems to come from bbfb0d2cf035 ("batman-adv: tt: remove only
> the entry which was looked up from the hash"). That commit changed the
> unlink in batadv_tt_local_remove() from batadv_compare_tt to
> batadv_compare_tt_entry. It kept the unconditional delete event in front
> of the unlink.

No, this is not causing the "problem" and was there since a very long time. 
And the change is not suitable for stable backports. And since it is submitted 
for net-next, I am not allowed to add Fixes: or Cc: stable@ tags.

[...] 
> [Severity: High]
> 
> Does this reordering open a new window in which a replacement entry's add
> event gets dropped?
[...]
> 
> Could the delete event instead be queued inside the same bucket list_lock
> critical section as the unlink, and only when this exact object was
> found? batadv_tt_local_set_pending() in "batman-adv: tt: queue local DEL
> event under bucket lock" already takes list_lock and then
> changes_list_lock in that order.
> 
> [ ... ]
> 
> 

Noted in https://www.open-mesh.org/issues/475 as potential further work to 
harden the code path further.

Regards,
	Sven

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

Re: [PATCH net-next 5/9] batman-adv: tt: only queue local del event after successful unlink

[Sven Eckelmann]

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

On Thursday, 1 October 2026 11:55:18 CEST sashiko-bot@kernel.org wrote:
[...]
> > diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> > index 354d9416c1b75..c904d67791f8f 100644
> > --- a/net/batman-adv/translation-table.c
> > +++ b/net/batman-adv/translation-table.c
> [ ... ]
> > @@ -1496,8 +1496,6 @@ batadv_tt_local_remove_now(struct batadv_priv *bat_priv,
> >  	struct batadv_tt_common_entry *common = &tt_local_entry->common;
> >  	struct hlist_node *tt_removed_node;
> >  
> > -	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_entry,
> > @@ -1505,6 +1503,8 @@ batadv_tt_local_remove_now(struct batadv_priv *bat_priv,
> >  	if (!tt_removed_node)
> >  		return;
> >  
> > +	batadv_tt_local_event(bat_priv, tt_local_entry, BATADV_TT_CLIENT_DEL);
> > +
> 
> [Severity: High]
> Does moving this event queueing after the hash removal create a lockless
> window where a concurrent ADD can be incorrectly cancelled by the delayed DEL
> event?

Previously, you're partner instance complained about the opposite order:
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831135117.574836-1-sw%40simonwunderlich.de?part=8

Will postpone further changes (and not modify this PR) until a better way for 
consistency guarantee between announcements and hash states is implemented:
https://www.open-mesh.org/issues/475

Regards,
	Sven

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

[PATCH net-next 6/9] batman-adv: tt: queue local DEL event under bucket lock

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

batadv_tt_local_remove() sets BATADV_TT_CLIENT_PENDING on an already
announced local entry and only afterwards queues the DEL change event. It
holds neither the hash bucket list_lock nor bat_priv->tt.commit_lock.

It can therefore be potentially interrupted in the middle:

  CPU0                          CPU1
  batadv_tt_local_remove()
    flags |= ..._PENDING;
                                batadv_tt_local_commit_changes()
                                  ..._purge_pending_clients()
                                    hlist_del_rcu(&...->hash_entry);
                                  batadv_tt_local_update_crc()
                                  atomic_inc(&bat_priv->tt.vn);
    batadv_tt_local_event()
    /* DEL queued only now */

The client then disappears from the local table and from the CRC of the new
TTVN after batadv_tt_local_commit_changes() without a DEL change being
announced for it. Neighbours receiving the new CRC without previously
seeing the DEL will try to recover via a full table request.

Move the event into batadv_tt_local_mark_removed() and hold the bucket
list_lock of the entry around both the flag change and the DEL event to
avoid this scenario.

Fixes: 976b159b3c12 ("batman-adv: tt: use protected flag modifications")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831135117.574836-1-sw%40simonwunderlich.de?part=12
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/translation-table.c | 64 ++++++++++++++++++++----------
 1 file changed, 44 insertions(+), 20 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index c904d67791f8f..c229c51cafa72 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -1427,13 +1427,20 @@ int batadv_tt_local_dump(struct sk_buff *msg, struct netlink_callback *cb)
  * @message: debug message describing the reason for the change
  *
  * Schedule the TT change announcement for the entry. The caller must already
- * have added BATADV_TT_CLIENT_PENDING to the @tt_local_entry
+ * have added BATADV_TT_CLIENT_PENDING to the @tt_local_entry and must hold the
+ * hash bucket list_lock of @tt_local_entry since setting the flag.
  */
 static void
 batadv_tt_local_set_pending_event(struct batadv_priv *bat_priv,
 				  struct batadv_tt_local_entry *tt_local_entry,
 				  u16 flags, const char *message)
 {
+	struct batadv_hashtable *hash = bat_priv->tt.local_hash;
+	u32 i;
+
+	i = batadv_choose_tt(&tt_local_entry->common, hash->size);
+	lockdep_assert_held(&hash->list_locks[i]);
+
 	batadv_tt_local_event(bat_priv, tt_local_entry, flags);
 
 	batadv_dbg(BATADV_DBG_TT, bat_priv,
@@ -1443,20 +1450,37 @@ batadv_tt_local_set_pending_event(struct batadv_priv *bat_priv,
 }
 
 /**
- * batadv_tt_local_mark_removed() - mark a local entry as removed
+ * batadv_tt_local_mark_removed() - mark a local entry as removed and queue DEL
+ * @bat_priv: the bat priv with all the mesh interface information
  * @tt_local_entry: local TT entry to mark
+ * @message: message to append to the log on deletion
  * @roaming: true if the deletion is due to a roaming event
  * @curr_flags: pointer to store the flags of the entry before it was marked
  *
+ * An already announced entry is marked as BATADV_TT_CLIENT_PENDING and the
+ * (roamed) DEL change is queued. Both happen under the hash bucket list_lock
+ * of the entry to prevent concurrent batadv_tt_local_purge_pending_clients()
+ * from removing the entry.
+ *
  * Return: true if the entry has to be kept in the local table until the next
  * ttvn increment, false if it can be purged immediately.
  */
 static bool
-batadv_tt_local_mark_removed(struct batadv_tt_local_entry *tt_local_entry,
-			     bool roaming, u16 *curr_flags)
+batadv_tt_local_mark_removed(struct batadv_priv *bat_priv,
+			     struct batadv_tt_local_entry *tt_local_entry,
+			     const char *message, bool roaming, u16 *curr_flags)
 {
+	spinlock_t *list_lock; /* protects write access to the hash lists */
 	struct batadv_tt_common_entry *common = &tt_local_entry->common;
+	struct batadv_hashtable *hash = bat_priv->tt.local_hash;
 	bool pending = false;
+	u16 flags;
+	u32 i;
+
+	i = batadv_choose_tt(common, hash->size);
+	list_lock = &hash->list_locks[i];
+
+	spin_lock_bh(list_lock);
 
 	scoped_guard(spinlock_bh, &common->flags_lock) {
 		*curr_flags = common->flags;
@@ -1474,6 +1498,17 @@ batadv_tt_local_mark_removed(struct batadv_tt_local_entry *tt_local_entry,
 		}
 	}
 
+	if (pending) {
+		flags = BATADV_TT_CLIENT_DEL;
+		if (roaming)
+			flags |= BATADV_TT_CLIENT_ROAM;
+
+		batadv_tt_local_set_pending_event(bat_priv, tt_local_entry,
+						  flags, message);
+	}
+
+	spin_unlock_bh(list_lock);
+
 	return pending;
 }
 
@@ -1532,28 +1567,17 @@ u16 batadv_tt_local_remove(struct batadv_priv *bat_priv, const u8 *addr,
 {
 	struct batadv_tt_local_entry *tt_local_entry;
 	u16 curr_flags;
-	u16 flags;
 
 	tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr, vid);
 	if (!tt_local_entry)
 		return BATADV_NO_FLAGS;
 
-	if (batadv_tt_local_mark_removed(tt_local_entry, roaming, &curr_flags)) {
-		/* queue (roamed) del event which was prepared by
-		 * batadv_tt_local_mark_removed()
-		 */
-		flags = BATADV_TT_CLIENT_DEL;
-		if (roaming)
-			flags |= BATADV_TT_CLIENT_ROAM;
-
-		batadv_tt_local_set_pending_event(bat_priv, tt_local_entry,
-						  flags, message);
-	} else {
-		/* if this client has been added right now, it is possible to
-		 * immediately purge it
-		 */
+	/* if this client has been added right now, it is possible to
+	 * immediately purge it
+	 */
+	if (!batadv_tt_local_mark_removed(bat_priv, tt_local_entry, message,
+					  roaming, &curr_flags))
 		batadv_tt_local_remove_now(bat_priv, tt_local_entry);
-	}
 
 	batadv_tt_local_entry_put(tt_local_entry);
 
-- 
2.47.3

Re: [PATCH net-next 6/9] batman-adv: tt: queue local DEL event under bucket lock

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 (4) · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Holding the bucket list_lock is not enough to serialize
  batadv_tt_local_remove() against…

Pre-existing issues:
- [Medium] batadv_tt_local_mark_removed() (net/batman-adv/translation-
  table.c) acts on an entry that may no longer be in the local hash.
- [Medium] The add path undoes a pending removal without the bucket lock.
- [Medium] If the change-node allocation fails, the entry is still
  treated as pending but its DEL is never queued.
- [Medium] The immediate-purge branch of batadv_tt_local_remove() (the
  entry is still NEW) is not atomic with respect to…

--

batman-adv: tt: queue local DEL event under bucket lock

This moves queueing of the DEL change event into
batadv_tt_local_mark_removed(). The hash bucket list_lock is now held
across setting BATADV_TT_CLIENT_PENDING and queueing the DEL, so that
batadv_tt_local_purge_pending_clients() cannot remove the entry between
the two steps. batadv_tt_local_set_pending_event() now asserts that the
bucket lock is held.

> diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> index c904d67791f8f..c229c51cafa72 100644
> --- a/net/batman-adv/translation-table.c
> +++ b/net/batman-adv/translation-table.c
> @@ -1427,13 +1427,20 @@ int batadv_tt_local_dump(struct sk_buff *msg, struct netlink_callback *cb)

[ ... ]

>  static void
>  batadv_tt_local_set_pending_event(struct batadv_priv *bat_priv,
>  				  struct batadv_tt_local_entry *tt_local_entry,
>  				  u16 flags, const char *message)
>  {
> +	struct batadv_hashtable *hash = bat_priv->tt.local_hash;
> +	u32 i;
> +
> +	i = batadv_choose_tt(&tt_local_entry->common, hash->size);
> +	lockdep_assert_held(&hash->list_locks[i]);

[Severity: Medium]
This is a pre-existing issue, but the code that clears
BATADV_TT_CLIENT_PENDING doesn't follow the rule asserted here. Is the
add path safe against batadv_tt_local_purge_pending_clients()?

batadv_tt_local_add_existing() clears the flag while holding only
flags_lock:

	scoped_guard(spinlock_bh, &common->flags_lock) {
		if (common->flags & BATADV_TT_CLIENT_PENDING) {
			...
			common->flags &= ~BATADV_TT_CLIENT_PENDING;

			return true;
		}

batadv_tt_local_add() then queues the ADD through batadv_tt_local_event()
without the bucket list_lock.

Say batadv_tt_local_purge_pending_clients() unlinks a PENDING entry after
the lockless hash_find() in batadv_tt_local_add() but before
batadv_tt_local_add_existing() runs. The flag is then cleared on an entry
that is no longer hashed, and an ADD is queued for it:

- If the ADD is queued before batadv_tt_tvlv_container_update() drains
  the list, it cancels the entry's DEL in batadv_tt_local_event(). The
  client leaves the table and the CRC with no DEL announced.
- If it is queued after the drain, the next TTVN announces an ADD for a
  client that isn't in the local table.

Either way neighbours see a CRC mismatch. The active client is also
missing from the local table until another frame recreates it.

The later commit "batman-adv: tt: queue local DEL event before marking
entry as pending" reorders DEL and PENDING. It doesn't add list_lock or
an unlink check to batadv_tt_local_add_existing().

> +
>  	batadv_tt_local_event(bat_priv, tt_local_entry, flags);

[Severity: Medium]
This isn't a bug introduced by this patch, but what happens if the
change node allocation fails here? batadv_tt_local_event() returns
silently:

	tt_change_node = kmem_cache_alloc(batadv_tt_change_cache, GFP_ATOMIC);
	if (!tt_change_node)
		return;

By then batadv_tt_local_mark_removed() has already set
BATADV_TT_CLIENT_PENDING and will return true, so
batadv_tt_local_remove() skips the immediate removal. No DEL is queued
and local_changes isn't incremented. The void return leaves no way to
undo PENDING.

If another change makes local_changes non-zero, the next commit's
batadv_tt_local_purge_pending_clients() drops the entry from the table
and the CRC. No DEL is ever announced.

If there are no other changes, batadv_tt_local_commit_changes_nolock()
returns early:

	if (READ_ONCE(bat_priv->tt.local_changes) == 0) {
		...
		return;
	}

batadv_tt_local_purge_list() also skips entries that are already
PENDING. The removal is never retried, and neighbours keep stale client
information.

The same order is still there at the end of the series.
batadv_tt_local_set_pending() calls __batadv_tt_local_event() and then
sets PENDING unconditionally.

>  
>  	batadv_dbg(BATADV_DBG_TT, bat_priv,

[ ... ]

> @@ -1443,20 +1450,37 @@ batadv_tt_local_set_pending_event(struct batadv_priv *bat_priv,

[ ... ]

>  static bool
> -batadv_tt_local_mark_removed(struct batadv_tt_local_entry *tt_local_entry,
> -			     bool roaming, u16 *curr_flags)
> +batadv_tt_local_mark_removed(struct batadv_priv *bat_priv,
> +			     struct batadv_tt_local_entry *tt_local_entry,
> +			     const char *message, bool roaming, u16 *curr_flags)
>  {
> +	spinlock_t *list_lock; /* protects write access to the hash lists */
>  	struct batadv_tt_common_entry *common = &tt_local_entry->common;
> +	struct batadv_hashtable *hash = bat_priv->tt.local_hash;
>  	bool pending = false;
> +	u16 flags;
> +	u32 i;
> +
> +	i = batadv_choose_tt(common, hash->size);
> +	list_lock = &hash->list_locks[i];
> +
> +	spin_lock_bh(list_lock);

[Severity: Medium]
This is a pre-existing issue, but now that the bucket lock is taken here,
should this also check that tt_local_entry is still hashed?

batadv_tt_local_remove() gets the entry from the lockless
batadv_tt_local_hash_find(). Nothing checks the entry again once
list_lock is held. Consider:

1. Entry E is already PENDING with a DEL queued, for example from
   batadv_tt_local_purge_list().
2. CPU0: batadv_tt_local_remove() finds E and takes a reference.
3. CPU1: a commit runs batadv_tt_local_purge_pending_clients(), which
   calls hlist_del_rcu() on E. The first DEL is sent.
4. batadv_tt_local_add() creates E' for the same addr/vid and queues its
   ADD.
5. CPU0 takes list_lock and sees that E isn't NEW. It sets PENDING again
   and queues a second DEL for E.

batadv_tt_local_event() only matches on addr and vid:

		del_op_entry = entry->change.flags & BATADV_TT_CLIENT_DEL;
		if (del_op_requested != del_op_entry) {
			...
			list_del(&entry->list);
			kmem_cache_free(batadv_tt_change_cache, entry);
			changes--;

So the stale DEL cancels the ADD for E'. E' stays in the local hash and
counts toward the CRC after the next batadv_tt_local_transition_new(),
but it is never announced. Neighbours then see a CRC mismatch and need a
full table request.

batadv_tt_local_remove_now() already guards against this. It queues its
DEL only after batadv_hash_remove() confirms that this exact object was
unlinked. That check came from the earlier series commit "batman-adv: tt:
only queue local del event after successful unlink".

Would a hlist_unhashed() check under list_lock be the equivalent here?
The same gap is still present at the end of the series in
batadv_tt_local_set_pending().

>  
>  	scoped_guard(spinlock_bh, &common->flags_lock) {
>  		*curr_flags = common->flags;
> @@ -1474,6 +1498,17 @@ batadv_tt_local_mark_removed(struct batadv_tt_local_entry *tt_local_entry,
>  		}
>  	}
>  
> +	if (pending) {
> +		flags = BATADV_TT_CLIENT_DEL;
> +		if (roaming)
> +			flags |= BATADV_TT_CLIENT_ROAM;
> +
> +		batadv_tt_local_set_pending_event(bat_priv, tt_local_entry,
> +						  flags, message);
> +	}
> +
> +	spin_unlock_bh(list_lock);

[Severity: High]
Is the bucket list_lock enough to serialize this against
batadv_tt_local_commit_changes_nolock()?

The lock only orders this code against the
batadv_tt_local_purge_pending_clients() pass over this one bucket. The
later phases of the commit take no bucket lock:
batadv_tt_local_update_crc() walks the hash under RCU, and
batadv_tt_tvlv_container_update() drains the change list. The only thing
holding those phases together is tt.commit_lock, and
batadv_tt_local_remove() never takes it.

That seems to leave two windows with the result the commit message says
it avoids.

In the first window the DEL is lost:

CPU0                              CPU1
                                  batadv_tt_local_commit_changes_nolock()
                                    ..._purge_pending_clients()
                                    batadv_tt_local_update_crc()
                                    batadv_tt_tvlv_container_update()
                                      /* snapshot local_changes == K */
batadv_tt_local_mark_removed()
  spin_lock_bh(list_lock);
  flags |= ..._PENDING;
  batadv_tt_local_event();
  /* local_changes == K + 1 */
  spin_unlock_bh(list_lock);
                                      spin_lock_bh(&changes_list_lock);
                                      /* copy K, free all K + 1 */

This is the batadv_tt_tvlv_container_update() code involved:

	tt_diff_entries_num = READ_ONCE(bat_priv->tt.local_changes);
	...
	list_for_each_entry_safe(entry, safe, &bat_priv->tt.changes_list,
				 list) {
		if (tt_diff_entries_count < tt_diff_entries_num) {
			memcpy(tt_change + tt_diff_entries_count, ...);
			tt_diff_entries_count++;
		}
		list_del(&entry->list);
		kmem_cache_free(batadv_tt_change_cache, entry);
	}

E stays PENDING with no DEL queued. The next commit's purge removes E
from the table and the CRC, and no DEL is ever sent. The gap between the
snapshot and the drain also includes batadv_tt_prepare_tvlv_local_data().

In the second window the DEL is announced with a CRC that still counts
the client. batadv_tt_local_mark_removed() runs after the purge has
scanned this bucket but before the snapshot, so the DEL goes out in TTVN
n+1. batadv_tt_local_crc() only skips NEW entries, which means the n+1
CRC still includes the PENDING entry. Neighbours that apply the DEL then
see a mismatch.

The commit message says:

  Move the event into batadv_tt_local_mark_removed() and hold the bucket
  list_lock of the entry around both the flag change and the DEL event to
  avoid this scenario.

That closes the exact interleaving in the diagram, but other remove vs
commit interleavings still reach the same result.

Does closing this need commit-level serialization instead, for example
taking tt.commit_lock around the marking and queueing? Nothing later in
the series changes batadv_tt_tvlv_container_update(),
batadv_tt_local_update_crc() or the locking in batadv_tt_local_remove().

> +
>  	return pending;
>  }
>  
> @@ -1532,28 +1567,17 @@ u16 batadv_tt_local_remove(struct batadv_priv *bat_priv, const u8 *addr,

[ ... ]

>  	tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr, vid);
>  	if (!tt_local_entry)
>  		return BATADV_NO_FLAGS;
>  

[ ... ]

> +	/* if this client has been added right now, it is possible to
> +	 * immediately purge it
> +	 */
> +	if (!batadv_tt_local_mark_removed(bat_priv, tt_local_entry, message,
> +					  roaming, &curr_flags))
>  		batadv_tt_local_remove_now(bat_priv, tt_local_entry);

[Severity: Medium]
This isn't a bug introduced by this patch, but the immediate purge
branch has a similar ordering problem.

batadv_tt_local_mark_removed() checks BATADV_TT_CLIENT_NEW under
list_lock, but drops the lock before batadv_tt_local_remove_now() acts
on the result. batadv_tt_local_remove_now() then unlinks the entry first
and queues the DEL afterwards:

	tt_removed_node = batadv_hash_remove(bat_priv->tt.local_hash,
					     batadv_compare_tt_entry,
					     batadv_choose_tt, common);
	if (!tt_removed_node)
		return;

	batadv_tt_local_event(bat_priv, tt_local_entry, BATADV_TT_CLIENT_DEL);

If CPU0 stalls between those two calls while CPU1 commits, can this
happen?

CPU0                              CPU1
batadv_tt_local_remove_now()
  batadv_hash_remove()
                                  batadv_tt_local_commit_changes_nolock()
                                    batadv_tt_local_transition_new()
                                    batadv_tt_local_update_crc()
                                    /* CRC for n + 1 excludes E */
                                    batadv_tt_tvlv_container_update()
                                    /* queued ADD for E sent in n + 1 */
  batadv_tt_local_event(..., DEL)
  /* DEL only sent in n + 2 */

Neighbours apply the ADD. Their CRC for us then includes E while ours
doesn't, so they request the full table. The comment above
batadv_tt_local_remove_now() mentions a parallel context announcing the
entry, but only the size accounting handles that case.

The DEL-after-unlink order comes from the earlier series commit
"batman-adv: tt: only queue local del event after successful unlink".
The baseline queued the DEL before the unlink and had a mirror race with
the same CRC mismatch.

As with the PENDING branch, holding list_lock around both steps wouldn't
close this. batadv_tt_local_update_crc() and
batadv_tt_tvlv_container_update() never take list_lock.

>  
>  	batadv_tt_local_entry_put(tt_local_entry);
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930094558.3723766-2-sw%40simonwunderlich.de

Re: [PATCH net-next 6/9] batman-adv: tt: queue local DEL event under bucket lock

[Sven Eckelmann]

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

On Thursday, 1 October 2026 12:10:29 CEST netdev-bot+sashiko@kernel.org wrote:
[...]
> > diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> > index c904d67791f8f..c229c51cafa72 100644
> > --- a/net/batman-adv/translation-table.c
> > +++ b/net/batman-adv/translation-table.c
> > @@ -1427,13 +1427,20 @@ int batadv_tt_local_dump(struct sk_buff *msg, struct netlink_callback *cb)
> 
> [ ... ]
> 
> >  static void
> >  batadv_tt_local_set_pending_event(struct batadv_priv *bat_priv,
> >  				  struct batadv_tt_local_entry *tt_local_entry,
> >  				  u16 flags, const char *message)
> >  {
> > +	struct batadv_hashtable *hash = bat_priv->tt.local_hash;
> > +	u32 i;
> > +
> > +	i = batadv_choose_tt(&tt_local_entry->common, hash->size);
> > +	lockdep_assert_held(&hash->list_locks[i]);
> 
> [Severity: Medium]
> This is a pre-existing issue, but the code that clears
> BATADV_TT_CLIENT_PENDING doesn't follow the rule asserted here. Is the
> add path safe against batadv_tt_local_purge_pending_clients()?

You are inferring here something which is not written down like this. You are 
just assuming that another code path unrelated to this one must have the same 
rules. This is only partially correct because this is not yet the goal of this 
commit but might be something which could/should be the case at some point.

Overall: Outside the scope of this patch. Added a ticket about this for the 
actual maintainer of this code. Maybe he wakes up at some point. 
https://www.open-mesh.org/issues/476


> [Severity: Medium]
> This isn't a bug introduced by this patch, but what happens if the
> change node allocation fails here? batadv_tt_local_event() returns
> silently:
> 
> 	tt_change_node = kmem_cache_alloc(batadv_tt_change_cache, GFP_ATOMIC);
> 	if (!tt_change_node)
> 		return;
> 
> By then batadv_tt_local_mark_removed() has already set
> BATADV_TT_CLIENT_PENDING and will return true, so
> batadv_tt_local_remove() skips the immediate removal. No DEL is queued
> and local_changes isn't incremented. The void return leaves no way to
> undo PENDING.

And it also shouldn't undo the pending. Instead, the correct approach is to 
make sure to announce a changed CRC.

> If another change makes local_changes non-zero, the next commit's
> batadv_tt_local_purge_pending_clients() drops the entry from the table
> and the CRC. No DEL is ever announced.

The not announced DEL seems to be OK for me here. Of course, in an ideal 
world, we would have infinite RAM and never have allocation errors. But 
falling back to a full sync is a viable option.

The only thing I don't like is the delayed recalculation of the CRC on an 
allocation error. And this is caused by the precheck in 
`batadv_tt_local_commit_changes_nolock` which depends on `local_changes` 
instead of a flag which announces any potential change (even when the 
allocation for the changes announcement list failed).

Overall: outside the scope of this patch: https://www.open-mesh.org/issues/477

[...]
> [Severity: Medium]
> This is a pre-existing issue, but now that the bucket lock is taken here,
> should this also check that tt_local_entry is still hashed?
[...]
> batadv_tt_local_remove_now() already guards against this. It queues its
> DEL only after batadv_hash_remove() confirms that this exact object was
> unlinked. That check came from the earlier series commit "batman-adv: tt:
> only queue local del event after successful unlink".
> 
> Would a hlist_unhashed() check under list_lock be the equivalent here?

Ok, list_lock is already held, so it could be a worthwhile addition. 
But outside the scope of this patch: https://www.open-mesh.org/issues/478

[...]
> [Severity: High]
> Is the bucket list_lock enough to serialize this against
> batadv_tt_local_commit_changes_nolock()?
> 
> The lock only orders this code against the
> batadv_tt_local_purge_pending_clients() pass over this one bucket. The
> later phases of the commit take no bucket lock:
> batadv_tt_local_update_crc() walks the hash under RCU, and
> batadv_tt_tvlv_container_update() drains the change list. The only thing
> holding those phases together is tt.commit_lock, and
> batadv_tt_local_remove() never takes it.
> 
> That seems to leave two windows with the result the commit message says
> it avoids.

No, it doesn't say that. It never mentions batadv_tt_tvlv_container_update. 
This is outside the scope of this patch: https://www.open-mesh.org/issues/479

> [Severity: Medium]
> This isn't a bug introduced by this patch, but the immediate purge
> branch has a similar ordering problem.

Outside the scope of this patch:

* https://www.open-mesh.org/issues/475
* https://www.open-mesh.org/issues/480

Regards,
	Sven

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

[PATCH net-next 7/9] batman-adv: tt: queue local DEL event before marking entry as pending

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

batadv_tt_local_set_pending() originally queued the DEL change event and
only afterwards set BATADV_TT_CLIENT_PENDING on the local entry. Commit
976b159b3c12 ("batman-adv: tt: use protected flag modifications") inverted
this order for batadv_tt_local_remove() and batadv_tt_local_purge_list().

The hash bucket list_lock held around both steps does not help against
batadv_tt_local_add_existing() because it is not holding it.

  CPU0                          CPU1
  flags |= ..._PENDING;
                                batadv_tt_local_add_existing()
                                  flags &= ~..._PENDING;
                                batadv_tt_local_event(ADD)
  batadv_tt_local_event(DEL)

If the ADD was already announced by a commit in between, the DEL is sent in
the next TTVN although the entry is no longer pending (after
batadv_tt_local_add_existing()) and the local entry was never purged. The
incorrect DEL will corrupt the CRC on neighbor nodes. A full table sync
request is therefore issued to resolve this problem.

When the DEL event is queued before the flag is set, a
batadv_tt_local_add_existing() which clears the flag afterwards queues its
ADD behind the DEL, and both cancel each other out.

But the check whether an entry has to be removed must not be separated (by
using two different critial flags_lock sections) from setting the flag (in
batadv_tt_local_event) either. Otherwise batadv_tt_local_add_existing()
could refresh the entry between both steps without queuing an ADD, and an
active client would be announced as removed and purged.

Fixes: 976b159b3c12 ("batman-adv: tt: use protected flag modifications")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831135117.574836-1-sw%40simonwunderlich.de?part=12
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/translation-table.c | 102 ++++++++++++++++-------------
 1 file changed, 55 insertions(+), 47 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index c229c51cafa72..481dc6afaaba1 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -461,23 +461,21 @@ static u16 batadv_tt_flags_get(struct batadv_tt_common_entry *common)
 }
 
 /**
- * batadv_tt_local_event() - store a local TT event (ADD/DEL)
+ * __batadv_tt_local_event() - store a local TT event (ADD/DEL) with given flags
  * @bat_priv: the bat priv with all the mesh interface information
- * @tt_local_entry: the TT entry involved in the event
- * @event_flags: flags to store in the event structure
+ * @common: the TT entry involved in the event
+ * @flags: flags of the TT entry combined with the event flags
  */
-static void batadv_tt_local_event(struct batadv_priv *bat_priv,
-				  struct batadv_tt_local_entry *tt_local_entry,
-				  u8 event_flags)
+static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
+				    const struct batadv_tt_common_entry *common,
+				    u8 flags)
 {
-	struct batadv_tt_common_entry *common = &tt_local_entry->common;
 	struct batadv_tt_change_node *tt_change_node;
 	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)
@@ -488,8 +486,6 @@ static void batadv_tt_local_event(struct batadv_priv *bat_priv,
 	ether_addr_copy(tt_change_node->change.addr, common->addr);
 	tt_change_node->change.vid = htons(common->vid);
 
-	flags = batadv_tt_flags_get(common) | event_flags;
-
 	tt_change_node->change.flags = flags;
 	del_op_requested = flags & BATADV_TT_CLIENT_DEL;
 
@@ -537,6 +533,23 @@ static void batadv_tt_local_event(struct batadv_priv *bat_priv,
 	spin_unlock_bh(&bat_priv->tt.changes_list_lock);
 }
 
+/**
+ * batadv_tt_local_event() - store a local TT event (ADD/DEL)
+ * @bat_priv: the bat priv with all the mesh interface information
+ * @tt_local_entry: the TT entry involved in the event
+ * @event_flags: flags to store in the event structure
+ */
+static void batadv_tt_local_event(struct batadv_priv *bat_priv,
+				  struct batadv_tt_local_entry *tt_local_entry,
+				  u8 event_flags)
+{
+	struct batadv_tt_common_entry *common = &tt_local_entry->common;
+	u8 flags;
+
+	flags = batadv_tt_flags_get(common) | event_flags;
+	__batadv_tt_local_event(bat_priv, common, flags);
+}
+
 /**
  * batadv_tt_len() - compute length in bytes of given number of tt changes
  * @changes_num: number of tt changes
@@ -1420,28 +1433,37 @@ int batadv_tt_local_dump(struct sk_buff *msg, struct netlink_callback *cb)
 }
 
 /**
- * batadv_tt_local_set_pending_event() - trigger events for TT pending removal
+ * batadv_tt_local_set_pending() - mark local TT entry as pending removal
  * @bat_priv: the bat priv with all the mesh interface information
- * @tt_local_entry: local TT entry which was marked as BATADV_TT_CLIENT_PENDING
+ * @tt_local_entry: local TT entry to mark as BATADV_TT_CLIENT_PENDING
  * @flags: TT change flags to announce together with the pending removal
  * @message: debug message describing the reason for the change
  *
- * Schedule the TT change announcement for the entry. The caller must already
- * have added BATADV_TT_CLIENT_PENDING to the @tt_local_entry and must hold the
- * hash bucket list_lock of @tt_local_entry since setting the flag.
+ * Schedule the TT change announcement and set BATADV_TT_CLIENT_PENDING on the
+ * entry. The entry is kept in the local table until the next TTVN increment
+ * so that a consistency-check response can still be answered.
+ *
+ * Next to the flags_lock of the entry, the caller must hold the hash bucket
+ * list_lock of @tt_local_entry. Otherwise
+ * batadv_tt_local_purge_pending_clients() could remove the entry before its
+ * change was queued.
  */
 static void
-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_set_pending(struct batadv_priv *bat_priv,
+			    struct batadv_tt_local_entry *tt_local_entry,
+			    u16 flags, const char *message)
+	__must_hold(&tt_local_entry->common.flags_lock)
 {
+	struct batadv_tt_common_entry *common = &tt_local_entry->common;
 	struct batadv_hashtable *hash = bat_priv->tt.local_hash;
 	u32 i;
 
-	i = batadv_choose_tt(&tt_local_entry->common, hash->size);
+	i = batadv_choose_tt(common, hash->size);
 	lockdep_assert_held(&hash->list_locks[i]);
+	lockdep_assert_held(&common->flags_lock);
 
-	batadv_tt_local_event(bat_priv, tt_local_entry, flags);
+	__batadv_tt_local_event(bat_priv, common, common->flags | flags);
+	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",
@@ -1460,7 +1482,8 @@ batadv_tt_local_set_pending_event(struct batadv_priv *bat_priv,
  * An already announced entry is marked as BATADV_TT_CLIENT_PENDING and the
  * (roamed) DEL change is queued. Both happen under the hash bucket list_lock
  * of the entry to prevent concurrent batadv_tt_local_purge_pending_clients()
- * from removing the entry.
+ * from removing the entry and batadv_tt_local_transition_new() from clearing
+ * BATADV_TT_CLIENT_NEW after it was checked.
  *
  * Return: true if the entry has to be kept in the local table until the next
  * ttvn increment, false if it can be purged immediately.
@@ -1492,19 +1515,16 @@ batadv_tt_local_mark_removed(struct batadv_priv *bat_priv,
 		if (roaming)
 			common->flags |= BATADV_TT_CLIENT_ROAM;
 
-		if (!(common->flags & BATADV_TT_CLIENT_NEW)) {
-			common->flags |= BATADV_TT_CLIENT_PENDING;
-			pending = true;
-		}
-	}
+		if (common->flags & BATADV_TT_CLIENT_NEW)
+			break;
 
-	if (pending) {
 		flags = BATADV_TT_CLIENT_DEL;
 		if (roaming)
 			flags |= BATADV_TT_CLIENT_ROAM;
 
-		batadv_tt_local_set_pending_event(bat_priv, tt_local_entry,
-						  flags, message);
+		batadv_tt_local_set_pending(bat_priv, tt_local_entry, flags,
+					    message);
+		pending = true;
 	}
 
 	spin_unlock_bh(list_lock);
@@ -1601,37 +1621,25 @@ 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);
 
 		scoped_guard(spinlock_bh, &tt_local_entry->common.flags_lock) {
-			if (tt_local_entry->common.flags & BATADV_TT_CLIENT_NOPURGE) {
-				cont = true;
+			if (tt_local_entry->common.flags & BATADV_TT_CLIENT_NOPURGE)
 				break;
-			}
 
 			/* entry already marked for deletion */
-			if (tt_local_entry->common.flags & BATADV_TT_CLIENT_PENDING) {
-				cont = true;
+			if (tt_local_entry->common.flags & BATADV_TT_CLIENT_PENDING)
 				break;
-			}
 
-			if (!batadv_has_timed_out(tt_local_entry->last_seen, timeout)) {
-				cont = true;
+			if (!batadv_has_timed_out(tt_local_entry->last_seen, timeout))
 				break;
-			}
 
-			tt_local_entry->common.flags |= BATADV_TT_CLIENT_PENDING;
+			batadv_tt_local_set_pending(bat_priv, tt_local_entry,
+						    BATADV_TT_CLIENT_DEL,
+						    "timed out");
 		}
-
-		if (cont)
-			continue;
-
-		batadv_tt_local_set_pending_event(bat_priv, tt_local_entry,
-						  BATADV_TT_CLIENT_DEL, "timed out");
 	}
 }
 
-- 
2.47.3

Re: [PATCH net-next 7/9] batman-adv: tt: queue local DEL event before marking entry as pending

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix covers only one ordering of the set_pending vs. re-add
  race.
- [Low] Stale locking rationale in the new kernel-doc.

--

batman-adv: tt: queue local DEL event before marking entry as pending

This patch restores the original order in batadv_tt_local_set_pending(),
so the DEL change is queued before BATADV_TT_CLIENT_PENDING is set. It
also moves the NEW check, the event queuing and the PENDING update into a
single flags_lock critical section. The goal is to stop a concurrent
batadv_tt_local_add_existing() from leaving the PENDING state and the
queued changes out of sync, which corrupts the TT CRC on neighbour nodes.

> diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> index c229c51cafa72..481dc6afaaba1 100644
> --- a/net/batman-adv/translation-table.c
> +++ b/net/batman-adv/translation-table.c

[ ... ]

> @@ -1420,28 +1433,37 @@ int batadv_tt_local_dump(struct sk_buff *msg, struct netlink_callback *cb)
>  }
>  
>  /**
> - * batadv_tt_local_set_pending_event() - trigger events for TT pending removal
> + * batadv_tt_local_set_pending() - mark local TT entry as pending removal
>   * @bat_priv: the bat priv with all the mesh interface information
> - * @tt_local_entry: local TT entry which was marked as BATADV_TT_CLIENT_PENDING
> + * @tt_local_entry: local TT entry to mark as BATADV_TT_CLIENT_PENDING
>   * @flags: TT change flags to announce together with the pending removal
>   * @message: debug message describing the reason for the change
>   *
> - * Schedule the TT change announcement for the entry. The caller must already
> - * have added BATADV_TT_CLIENT_PENDING to the @tt_local_entry and must hold the
> - * hash bucket list_lock of @tt_local_entry since setting the flag.
> + * Schedule the TT change announcement and set BATADV_TT_CLIENT_PENDING on the
> + * entry. The entry is kept in the local table until the next TTVN increment
> + * so that a consistency-check response can still be answered.
> + *
> + * Next to the flags_lock of the entry, the caller must hold the hash bucket
> + * list_lock of @tt_local_entry. Otherwise
> + * batadv_tt_local_purge_pending_clients() could remove the entry before its
> + * change was queued.

[Severity: Low]
Is this rationale still accurate after the reordering?

With this patch, batadv_tt_local_set_pending() queues the event and then
sets BATADV_TT_CLIENT_PENDING, and both happen inside one
common->flags_lock section:

    __batadv_tt_local_event(bat_priv, common, common->flags | flags);
    common->flags |= BATADV_TT_CLIENT_PENDING;

batadv_tt_local_purge_pending_clients() only acts on PENDING after
reading it under the same flags_lock:

    scoped_guard(spinlock_bh, &tt_common->flags_lock) {
        if (!(tt_common->flags & BATADV_TT_CLIENT_PENDING)) {

So it looks like purge_pending_clients() cannot see PENDING before the
change is queued, whether or not list_lock is held.

The updated batadv_tt_local_mark_removed() kernel-doc has the same
problem. It still gives the old order ("marked as
BATADV_TT_CLIENT_PENDING and the (roamed) DEL change is queued"). It also
adds a claim that list_lock prevents batadv_tt_local_transition_new() from
clearing BATADV_TT_CLIENT_NEW after it was checked. As far as I can tell,
that protection comes from the NEW check and set_pending() sharing one
flags_lock scoped_guard. The commit message says the same thing.

Could both comments name the single flags_lock section, together with the
queue-then-mark order, as the invariant? A later change could otherwise
move __batadv_tt_local_event() out of the flags_lock section while keeping
list_lock, and bring back the ADD/DEL misordering this patch fixes.

>   */
>  static void
> -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_set_pending(struct batadv_priv *bat_priv,
> +			    struct batadv_tt_local_entry *tt_local_entry,
> +			    u16 flags, const char *message)
> +	__must_hold(&tt_local_entry->common.flags_lock)
>  {
> +	struct batadv_tt_common_entry *common = &tt_local_entry->common;
>  	struct batadv_hashtable *hash = bat_priv->tt.local_hash;
>  	u32 i;
>  
> -	i = batadv_choose_tt(&tt_local_entry->common, hash->size);
> +	i = batadv_choose_tt(common, hash->size);
>  	lockdep_assert_held(&hash->list_locks[i]);
> +	lockdep_assert_held(&common->flags_lock);
>  
> -	batadv_tt_local_event(bat_priv, tt_local_entry, flags);
> +	__batadv_tt_local_event(bat_priv, common, common->flags | flags);
> +	common->flags |= BATADV_TT_CLIENT_PENDING;

[Severity: High]
Does this close only one ordering of the race with the re-add path?

The add side still changes the flags and queues the ADD in two separate
steps. batadv_tt_local_add_existing() clears PENDING inside a flags_lock
scoped_guard and returns. batadv_tt_local_add() then queues the ADD after
that lock is dropped:

    if (added)
        batadv_tt_local_event(bat_priv, tt_local, BATADV_NO_FLAGS);
    ...
    if (batadv_tt_local_update_flags(bat_priv, tt_local, iif_is_wifi, mark))
        batadv_tt_local_event(bat_priv, tt_local, BATADV_NO_FLAGS);

batadv_tt_local_set_pending() can run in that window from the RX path:

batadv_tt_global_add()->batadv_tt_global_purge_local()->
  batadv_tt_local_remove()->batadv_tt_local_mark_removed()->
    batadv_tt_local_set_pending()

Here is one interleaving, starting with DEL#1 already queued and PENDING
set:

  CPU1 (TX)                         CPU0 (RX)
  batadv_tt_local_add()
    batadv_tt_local_add_existing()
      flags &= ~PENDING;
                                    batadv_tt_local_mark_removed()
                                      batadv_tt_local_set_pending()
                                        DEL|ROAM merged into DEL#1
                                        flags |= PENDING;
    batadv_tt_local_event(NO_FLAGS)
      ADD cancels DEL#1

There is a similar case with no earlier DEL. batadv_tt_local_update_flags()
returns true on CPU1, CPU0 queues a new DEL and sets PENDING, and then the
ADD from CPU1 cancels that DEL through the DEL+ADD case in
__batadv_tt_local_event().

Either way, the entry would be left PENDING with no DEL queued. At the
next commit, batadv_tt_local_purge_pending_clients() would remove it
locally, but neighbours would never receive a DEL.

Wouldn't that give the same CRC mismatch and full table request that the
commit message describes? Would the ADD also need to be queued inside the
flags_lock section that clears PENDING or updates the flags?

>  
>  	batadv_dbg(BATADV_DBG_TT, bat_priv,
>  		   "Local tt entry (%pM, vid: %d) pending to be removed: %s\n",
> @@ -1460,7 +1482,8 @@ batadv_tt_local_set_pending_event(struct batadv_priv *bat_priv,
>   * An already announced entry is marked as BATADV_TT_CLIENT_PENDING and the
>   * (roamed) DEL change is queued. Both happen under the hash bucket list_lock
>   * of the entry to prevent concurrent batadv_tt_local_purge_pending_clients()
> - * from removing the entry.
> + * from removing the entry and batadv_tt_local_transition_new() from clearing
> + * BATADV_TT_CLIENT_NEW after it was checked.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930094558.3723766-2-sw%40simonwunderlich.de

Re: [PATCH net-next 7/9] batman-adv: tt: queue local DEL event before marking entry as pending

[Sven Eckelmann]

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

On Thursday, 1 October 2026 12:10:30 CEST netdev-bot+sashiko@kernel.org wrote:
[...]
> > diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> > index c229c51cafa72..481dc6afaaba1 100644
> > --- a/net/batman-adv/translation-table.c
> > +++ b/net/batman-adv/translation-table.c
> 
> [ ... ]
> 
> > @@ -1420,28 +1433,37 @@ int batadv_tt_local_dump(struct sk_buff *msg, struct netlink_callback *cb)
> >  }
> >  
> >  /**
> > - * batadv_tt_local_set_pending_event() - trigger events for TT pending removal
> > + * batadv_tt_local_set_pending() - mark local TT entry as pending removal
> >   * @bat_priv: the bat priv with all the mesh interface information
> > - * @tt_local_entry: local TT entry which was marked as BATADV_TT_CLIENT_PENDING
> > + * @tt_local_entry: local TT entry to mark as BATADV_TT_CLIENT_PENDING
> >   * @flags: TT change flags to announce together with the pending removal
> >   * @message: debug message describing the reason for the change
> >   *
> > - * Schedule the TT change announcement for the entry. The caller must already
> > - * have added BATADV_TT_CLIENT_PENDING to the @tt_local_entry and must hold the
> > - * hash bucket list_lock of @tt_local_entry since setting the flag.
> > + * Schedule the TT change announcement and set BATADV_TT_CLIENT_PENDING on the
> > + * entry. The entry is kept in the local table until the next TTVN increment
> > + * so that a consistency-check response can still be answered.
> > + *
> > + * Next to the flags_lock of the entry, the caller must hold the hash bucket
> > + * list_lock of @tt_local_entry. Otherwise
> > + * batadv_tt_local_purge_pending_clients() could remove the entry before its
> > + * change was queued.
> 
> [Severity: Low]
> Is this rationale still accurate after the reordering?
> 
> With this patch, batadv_tt_local_set_pending() queues the event and then
> sets BATADV_TT_CLIENT_PENDING, and both happen inside one
> common->flags_lock section:
> 
>     __batadv_tt_local_event(bat_priv, common, common->flags | flags);
>     common->flags |= BATADV_TT_CLIENT_PENDING;
> 
> batadv_tt_local_purge_pending_clients() only acts on PENDING after
> reading it under the same flags_lock:
> 
>     scoped_guard(spinlock_bh, &tt_common->flags_lock) {
>         if (!(tt_common->flags & BATADV_TT_CLIENT_PENDING)) {
> 
> So it looks like purge_pending_clients() cannot see PENDING before the
> change is queued, whether or not list_lock is held.

The list_lock comes from the previous statement and only the flags_lock part 
was added. The list_lock one comes from the parallel 
batadv_tt_local_commit_changes() -> batadv_tt_local_purge_pending_clients(). 
And it was previously better to have the list lock because 
batadv_tt_local_event and batadv_tt_local_purge_pending_clients only held it 
for a short period of time. But the list_lock was held over the whole time in 
batadv_tt_local_purge_pending_clients() and now still held for 
batadv_tt_local_mark_removed(). A request which actually came from Sashiko.

I consider removing/rewriting this statement at a later point when simplifying 
the locking situation with batadv_tt_local_set_pending() and 
batadv_tt_local_mark_removed(). But it needs a further review of 
batadv_tt_local_commit_changes() to make sure that it doesn't reintroduce the 
previous problems.

But since this needs further checking, it is outside the scope of this patch. 
And maybe it doesn't even work out (see below)


> [Severity: High]
> Does this close only one ordering of the race with the re-add path?

Yes, which is exactly the scope of the patch.

> 
> The add side still changes the flags and queues the ADD in two separate
> steps. batadv_tt_local_add_existing() clears PENDING inside a flags_lock
> scoped_guard and returns. batadv_tt_local_add() then queues the ADD after
> that lock is dropped:
> 
>     if (added)
>         batadv_tt_local_event(bat_priv, tt_local, BATADV_NO_FLAGS);
>     ...
>     if (batadv_tt_local_update_flags(bat_priv, tt_local, iif_is_wifi, mark))
>         batadv_tt_local_event(bat_priv, tt_local, BATADV_NO_FLAGS);

This part is outside the scope of this patch:
https://www.open-mesh.org/issues/481

It would be really invasive because it is not enough to just have a single 
region with list_lock held which then checks for batadv_tt_local_add if 
"common->flags & BATADV_TT_CLIENT_PENDING" is not set before running 
__batadv_tt_local_event() but also needs to hold the list_lock to avoid that 
batadv_tt_local_remove_now() queues a DEL when a new entry was already added. 
Or to avoid that batadv_tt_local_add() added the newly allocated entry which 
was then removed again by something like batadv_tt_local_remove_now() before 
it checks for "!common->flags & BATADV_TT_CLIENT_PENDING" and queues the 
event.

Something like:

diff --git c/net/batman-adv/translation-table.c w/net/batman-adv/translation-table.c
index e46040fd..14e095d5 100644
--- c/net/batman-adv/translation-table.c
+++ w/net/batman-adv/translation-table.c
@@ -925,55 +925,6 @@ static bool batadv_tt_iif_is_wifi(struct net *net, int ifindex)
 	return batadv_is_wifi(wifi_flags);
 }
 
-/**
- * batadv_tt_local_add_existing() - refresh an already known local TT entry
- * @bat_priv: the bat priv with all the mesh interface information
- * @tt_local: the local TT entry which was found in the local table
- * @roamed_back: set to true when the client returned to its original location
- *
- * Return: true when the client has to be announced to the mesh again, false
- * otherwise.
- */
-static bool batadv_tt_local_add_existing(struct batadv_priv *bat_priv,
-					 struct batadv_tt_local_entry *tt_local,
-					 bool *roamed_back)
-{
-	struct batadv_tt_common_entry *common = &tt_local->common;
-
-	tt_local->last_seen = jiffies;
-
-	scoped_guard(spinlock_bh, &common->flags_lock) {
-		if (common->flags & BATADV_TT_CLIENT_PENDING) {
-			batadv_dbg(BATADV_DBG_TT, bat_priv,
-				   "Re-adding pending client %pM (vid: %d)\n",
-				   common->addr, batadv_print_vid(common->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
-			 */
-			common->flags &= ~BATADV_TT_CLIENT_PENDING;
-
-			return true;
-		}
-
-		if (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",
-				   common->addr, batadv_print_vid(common->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
-			 */
-			common->flags &= ~BATADV_TT_CLIENT_ROAM;
-			*roamed_back = true;
-		}
-	}
-
-	return false;
-}
-
 /**
  * batadv_tt_local_create() - allocate and initialize a local TT entry
  * @mesh_iface: netdev struct of the mesh interface
@@ -1046,27 +997,72 @@ batadv_tt_local_create(struct net_device *mesh_iface, const u8 *addr,
 }
 
 /**
- * batadv_tt_local_update_flags() - update the dynamic flags of a local entry
+ * batadv_tt_local_refresh() - refresh a local TT entry of an active client
  * @bat_priv: the bat priv with all the mesh interface information
- * @tt_local: the local TT entry to update
+ * @tt_local: the local TT entry to refresh
  * @iif_is_wifi: whether the client is connected via a wifi interface
  * @mark: the value contained in the skb->mark field of the received packet (if
  *  any)
+ * @announce: whether an ADD event has to be queued even when no announced
+ *  flag was modified
+ * @roamed_back: set to true when the client returned to its original location
  *
- * Return: true if a flag announced to the other nodes was modified, false
- * otherwise.
+ * TODO
+ *
+ * Return: true if the entry was refreshed, false if it is no longer part of
+ * the local table.
  */
 static bool
-batadv_tt_local_update_flags(struct batadv_priv *bat_priv,
-			     struct batadv_tt_local_entry *tt_local,
-			     bool iif_is_wifi, u32 mark)
+batadv_tt_local_refresh(struct batadv_priv *bat_priv,
+			struct batadv_tt_local_entry *tt_local,
+			bool iif_is_wifi, u32 mark, bool announce,
+			bool *roamed_back)
 {
+	spinlock_t *list_lock; /* protects write access to the hash lists */
 	struct batadv_tt_common_entry *common = &tt_local->common;
+	struct batadv_hashtable *hash = bat_priv->tt.local_hash;
 	u8 remote_flags;
 	u32 match_mark;
-	bool modified;
+	u32 i;
+
+	i = batadv_choose_tt(common, hash->size);
+	list_lock = &hash->list_locks[i];
+
+	tt_local->last_seen = jiffies;
+
+	spin_lock_bh(list_lock);
+
+	/* the entry was removed from the local table after it was looked up */
+	if (hlist_unhashed(&common->hash_entry)) {
+		spin_unlock_bh(list_lock);
+		return false;
+	}
 
 	scoped_guard(spinlock_bh, &common->flags_lock) {
+		if (common->flags & BATADV_TT_CLIENT_PENDING) {
+			batadv_dbg(BATADV_DBG_TT, bat_priv,
+				   "Re-adding pending client %pM (vid: %d)\n",
+				   common->addr, batadv_print_vid(common->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
+			 */
+			common->flags &= ~BATADV_TT_CLIENT_PENDING;
+			announce = true;
+		} else if (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",
+				   common->addr, batadv_print_vid(common->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
+			 */
+			common->flags &= ~BATADV_TT_CLIENT_ROAM;
+			*roamed_back = true;
+		}
+
 		/* store the current remote flags before altering them. This
 		 * helps understanding is flags are changing or not
 		 */
@@ -1088,10 +1084,17 @@ batadv_tt_local_update_flags(struct batadv_priv *bat_priv,
 		else
 			common->flags &= ~BATADV_TT_CLIENT_ISOLA;
 
-		modified = remote_flags ^ (common->flags & BATADV_TT_REMOTE_MASK);
+		if (remote_flags ^ (common->flags & BATADV_TT_REMOTE_MASK))
+			announce = true;
+
+		if (announce)
+			__batadv_tt_local_event(bat_priv, common,
+						common->flags);
 	}
 
-	return modified;
+	spin_unlock_bh(list_lock);
+
+	return true;
 }
 
 /**
@@ -1114,7 +1117,6 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
 	struct batadv_tt_global_entry *tt_global = NULL;
 	struct batadv_tt_local_entry *tt_local;
 	bool roamed_back = false;
-	bool added = false;
 	bool iif_is_wifi;
 	bool ret = false;
 	int hash_added;
@@ -1126,10 +1128,15 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
 	if (!is_multicast_ether_addr(addr))
 		tt_global = batadv_tt_global_hash_find(bat_priv, addr, vid);
 
-	if (tt_local) {
-		added = batadv_tt_local_add_existing(bat_priv, tt_local,
-						     &roamed_back);
-	} else {
+	if (tt_local &&
+	    !batadv_tt_local_refresh(bat_priv, tt_local, iif_is_wifi, mark,
+				     false, &roamed_back)) {
+		/* stale entry which has to be replaced by a new one */
+		batadv_tt_local_entry_put(tt_local);
+		tt_local = NULL;
+	}
+
+	if (!tt_local) {
 		tt_local = batadv_tt_local_create(mesh_iface, addr, vid, iif_is_wifi);
 		if (!tt_local)
 			goto out;
@@ -1145,21 +1152,12 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
 			goto out;
 		}
 
-		added = true;
+		batadv_tt_local_refresh(bat_priv, tt_local, iif_is_wifi, mark,
+					true, &roamed_back);
 	}
 
-	/* announce the (re-)added client to the mesh */
-	if (added)
-		batadv_tt_local_event(bat_priv, tt_local, BATADV_NO_FLAGS);
-
 	batadv_tt_local_add_roam(bat_priv, tt_global, roamed_back);
 
-	/* 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 (batadv_tt_local_update_flags(bat_priv, tt_local, iif_is_wifi, mark))
-		batadv_tt_local_event(bat_priv, tt_local, BATADV_NO_FLAGS);
-
 	ret = true;
 out:
 	batadv_tt_local_entry_put(tt_local);
@@ -1624,14 +1622,7 @@ int batadv_tt_local_dump(struct sk_buff *msg, struct netlink_callback *cb)
  * @flags: TT change flags to announce together with the pending removal
  * @message: debug message describing the reason for the change
  *
- * Schedule the TT change announcement and set BATADV_TT_CLIENT_PENDING on the
- * entry. The entry is kept in the local table until the next TTVN increment
- * so that a consistency-check response can still be answered.
- *
- * Next to the flags_lock of the entry, the caller must hold the hash bucket
- * list_lock of @tt_local_entry. Otherwise
- * batadv_tt_local_purge_pending_clients() could remove the entry before its
- * change was queued.
+ * TODO
  */
 static void
 batadv_tt_local_set_pending(struct batadv_priv *bat_priv,
@@ -1664,14 +1655,9 @@ batadv_tt_local_set_pending(struct batadv_priv *bat_priv,
  * @roaming: true if the deletion is due to a roaming event
  * @curr_flags: pointer to store the flags of the entry before it was marked
  *
- * An already announced entry is marked as BATADV_TT_CLIENT_PENDING and the
- * (roamed) DEL change is queued. Both happen under the hash bucket list_lock
- * of the entry to prevent concurrent batadv_tt_local_purge_pending_clients()
- * from removing the entry and batadv_tt_local_transition_new() from clearing
- * BATADV_TT_CLIENT_NEW after it was checked.
+ * TODO
  *
- * Return: true if the entry has to be kept in the local table until the next
- * ttvn increment, false if it can be purged immediately.
+ * Return: false if the entry has to be purged immediately, true otherwise.
  */
 static bool
 batadv_tt_local_mark_removed(struct batadv_priv *bat_priv,
@@ -1690,6 +1676,12 @@ batadv_tt_local_mark_removed(struct batadv_priv *bat_priv,
 
 	spin_lock_bh(list_lock);
 
+	if (hlist_unhashed(&common->hash_entry)) {
+		*curr_flags = BATADV_NO_FLAGS;
+		spin_unlock_bh(list_lock);
+		return true;
+	}
+
 	scoped_guard(spinlock_bh, &common->flags_lock) {
 		*curr_flags = common->flags;
 
@@ -1733,15 +1725,24 @@ static void
 batadv_tt_local_remove_now(struct batadv_priv *bat_priv,
 			   struct batadv_tt_local_entry *tt_local_entry)
 {
+	spinlock_t *list_lock; /* protects write access to the hash lists */
 	struct batadv_tt_common_entry *common = &tt_local_entry->common;
-	struct hlist_node *tt_removed_node;
+	struct batadv_hashtable *hash = bat_priv->tt.local_hash;
+	u32 i;
+
+	i = batadv_choose_tt(common, hash->size);
+	list_lock = &hash->list_locks[i];
+
+	spin_lock_bh(list_lock);
 
 	/* remove exactly this object when still present in hash */
-	tt_removed_node = batadv_hash_remove(bat_priv->tt.local_hash,
-					     batadv_compare_tt_entry,
-					     batadv_choose_tt, common);
-	if (!tt_removed_node)
+	if (hlist_unhashed(&common->hash_entry)) {
+		spin_unlock_bh(list_lock);
 		return;
+	}
+
+	hlist_del_init_rcu(&common->hash_entry);
+	atomic_inc(&hash->generation);
 
 	batadv_tt_local_event(bat_priv, tt_local_entry, BATADV_TT_CLIENT_DEL);
 
@@ -1752,6 +1753,8 @@ batadv_tt_local_remove_now(struct batadv_priv *bat_priv,
 	if (!(batadv_tt_flags_get(common) & BATADV_TT_CLIENT_NEW))
 		batadv_tt_local_size_dec(bat_priv, common->vid);
 
+	spin_unlock_bh(list_lock);
+
 	/* drop reference of remove hash entry */
 	batadv_tt_local_entry_put(tt_local_entry);
 }
@@ -1881,7 +1884,7 @@ static void batadv_tt_local_table_free(struct batadv_priv *bat_priv)
 		spin_lock_bh(list_lock);
 		hlist_for_each_entry_safe(tt_common_entry, node_tmp,
 					  head, hash_entry) {
-			hlist_del_rcu(&tt_common_entry->hash_entry);
+			hlist_del_init_rcu(&tt_common_entry->hash_entry);
 			tt_local = container_of(tt_common_entry,
 						struct batadv_tt_local_entry,
 						common);
@@ -4434,7 +4437,7 @@ static void batadv_tt_local_purge_pending_clients(struct batadv_priv *bat_priv)
 					   tt_common->addr,
 					   batadv_print_vid(tt_common->vid));
 
-				hlist_del_rcu(&tt_common->hash_entry);
+				hlist_del_init_rcu(&tt_common->hash_entry);
 
 				/* An entry which still carries BATADV_TT_CLIENT_NEW was
 				 * never counted and must not be uncounted here.





And it could be a good idea to have a fast-path because tt_local_add will 
otherwise choke easily on the shared list lock:


diff --git c/net/batman-adv/translation-table.c w/net/batman-adv/translation-table.c
index 14e095d5..2ab66601 100644
--- c/net/batman-adv/translation-table.c
+++ w/net/batman-adv/translation-table.c
@@ -996,6 +996,48 @@ batadv_tt_local_create(struct net_device *mesh_iface, const u8 *addr,
 	return tt_local;
 }
 
+#define BATADV_TT_LOCAL_DYNAMIC_MASK \
+	(BATADV_TT_CLIENT_WIFI | BATADV_TT_CLIENT_ISOLA)
+
+static u16 batadv_tt_local_dynamic_flags(struct batadv_priv *bat_priv,
+					 bool iif_is_wifi, u32 mark)
+{
+	u16 flags = 0;
+	u32 match_mark;
+
+	if (iif_is_wifi)
+		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)
+		flags |= BATADV_TT_CLIENT_ISOLA;
+
+	return flags;
+}
+
+static bool
+batadv_tt_local_refresh_needed(struct batadv_tt_local_entry *tt_local,
+			       u16 dynamic_flags)
+{
+	struct batadv_tt_common_entry *common = &tt_local->common;
+	u16 flags;
+
+	flags = batadv_tt_flags_get(common);
+
+	if (flags & (BATADV_TT_CLIENT_PENDING | BATADV_TT_CLIENT_ROAM))
+		return true;
+
+	if ((flags & BATADV_TT_LOCAL_DYNAMIC_MASK) != dynamic_flags)
+		return true;
+
+	return hlist_unhashed_lockless(&common->hash_entry);
+}
+
 /**
  * batadv_tt_local_refresh() - refresh a local TT entry of an active client
  * @bat_priv: the bat priv with all the mesh interface information
@@ -1021,15 +1063,23 @@ batadv_tt_local_refresh(struct batadv_priv *bat_priv,
 	spinlock_t *list_lock; /* protects write access to the hash lists */
 	struct batadv_tt_common_entry *common = &tt_local->common;
 	struct batadv_hashtable *hash = bat_priv->tt.local_hash;
+	u16 dynamic_flags;
 	u8 remote_flags;
-	u32 match_mark;
 	u32 i;
 
+	dynamic_flags = batadv_tt_local_dynamic_flags(bat_priv, iif_is_wifi,
+						      mark);
+
+	tt_local->last_seen = jiffies;
+
+	/* fast path: nothing to modify or to announce */
+	if (!announce &&
+	    !batadv_tt_local_refresh_needed(tt_local, dynamic_flags))
+		return true;
+
 	i = batadv_choose_tt(common, hash->size);
 	list_lock = &hash->list_locks[i];
 
-	tt_local->last_seen = jiffies;
-
 	spin_lock_bh(list_lock);
 
 	/* the entry was removed from the local table after it was looked up */
@@ -1068,21 +1118,8 @@ batadv_tt_local_refresh(struct batadv_priv *bat_priv,
 		 */
 		remote_flags = common->flags & BATADV_TT_REMOTE_MASK;
 
-		if (iif_is_wifi)
-			common->flags |= BATADV_TT_CLIENT_WIFI;
-		else
-			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)
-			common->flags |= BATADV_TT_CLIENT_ISOLA;
-		else
-			common->flags &= ~BATADV_TT_CLIENT_ISOLA;
+		common->flags &= ~BATADV_TT_LOCAL_DYNAMIC_MASK;
+		common->flags |= dynamic_flags;
 
 		if (remote_flags ^ (common->flags & BATADV_TT_REMOTE_MASK))
 			announce = true;




But this doesn't deal yet with the DEL queued in case 
batadv_tt_local_remove_now is called (by parallel context) in the middle of 
the batadv_hash_add and batadv_tt_local_refresh of batadv_tt_local_add().
This would require something like:


diff --git c/net/batman-adv/translation-table.c w/net/batman-adv/translation-table.c
index ff87bd30..5926a878 100644
--- c/net/batman-adv/translation-table.c
+++ w/net/batman-adv/translation-table.c
@@ -671,14 +671,22 @@ static u16 batadv_tt_flags_get(struct batadv_tt_common_entry *common)
 }
 
 /**
- * __batadv_tt_local_event() - store a local TT event (ADD/DEL) with given flags
+ * batadv_tt_local_event() - store a local TT event (ADD/DEL) with given flags
  * @bat_priv: the bat priv with all the mesh interface information
  * @common: the TT entry involved in the event
  * @flags: flags of the TT entry combined with the event flags
+ * @cancel_add: whether a DEL may cancel a queued ADD because the entry was
+ *  never announced to the other nodes
+ *
+ * Only a single change per client is queued. It describes the latest state of
+ * the client and a new event therefore replaces an already queued one. The
+ * only exception is a DEL for an entry which was never announced. It cancels
+ * the ADD of this entry because the other nodes don't need to know anything
+ * about it.
  */
-static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
-				    const struct batadv_tt_common_entry *common,
-				    u8 flags)
+static void batadv_tt_local_event(struct batadv_priv *bat_priv,
+				  const struct batadv_tt_common_entry *common,
+				  u8 flags, bool cancel_add)
 {
 	struct batadv_tt_change_node *tt_change_node;
 	struct batadv_tt_change_node *entry;
@@ -711,21 +719,19 @@ static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
 			continue;
 
 		del_op_entry = entry->change.flags & BATADV_TT_CLIENT_DEL;
-		if (del_op_requested != del_op_entry) {
-			/* DEL+ADD in the same orig interval have no effect and
-			 * can be removed to avoid silly behaviour on the
-			 * receiver side. The  other way around (ADD+DEL) can
-			 * happen in case of roaming of  a client still in the
-			 * NEW state. Roaming of NEW clients is now possible due
-			 * to automatically recognition of "temporary" clients
+		if (cancel_add && del_op_requested && !del_op_entry) {
+			/* ADD+DEL of a client which was never announced (e.g.
+			 * roaming of a client still in the NEW state) have no
+			 * effect and can be removed
 			 */
 			list_del(&entry->list);
 			kmem_cache_free(batadv_tt_change_cache, entry);
 			changes--;
 		} else {
-			/* this is a second add or del in the same originator
-			 * interval. It could mean that flags have been changed
-			 * (e.g. double add): update them
+			/* the other nodes may know the client with different
+			 * flags. A DEL+ADD must therefore still announce the
+			 * current flags and an ADD+DEL must still remove the
+			 * client. ADD+ADD and DEL+DEL only update the flags
 			 */
 			entry->change.flags = flags;
 		}
@@ -743,23 +749,6 @@ static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
 	spin_unlock_bh(&bat_priv->tt.changes_list_lock);
 }
 
-/**
- * batadv_tt_local_event() - store a local TT event (ADD/DEL)
- * @bat_priv: the bat priv with all the mesh interface information
- * @tt_local_entry: the TT entry involved in the event
- * @event_flags: flags to store in the event structure
- */
-static void batadv_tt_local_event(struct batadv_priv *bat_priv,
-				  struct batadv_tt_local_entry *tt_local_entry,
-				  u8 event_flags)
-{
-	struct batadv_tt_common_entry *common = &tt_local_entry->common;
-	u8 flags;
-
-	flags = batadv_tt_flags_get(common) | event_flags;
-	__batadv_tt_local_event(bat_priv, common, flags);
-}
-
 /**
  * batadv_tt_entries() - compute the number of entries fitting in tt_len bytes
  * @tt_len: available space
@@ -1099,6 +1088,7 @@ batadv_tt_local_refresh(struct batadv_priv *bat_priv,
 	struct batadv_tt_common_entry *common = &tt_local->common;
 	struct batadv_hashtable *hash = bat_priv->tt.local_hash;
 	u16 dynamic_flags;
+	u16 event_flags;
 	u8 remote_flags;
 	u32 i;
 
@@ -1159,9 +1149,14 @@ batadv_tt_local_refresh(struct batadv_priv *bat_priv,
 		if (remote_flags ^ (common->flags & BATADV_TT_REMOTE_MASK))
 			announce = true;
 
-		if (announce)
-			__batadv_tt_local_event(bat_priv, common,
-						common->flags);
+		/* BATADV_TT_CLIENT_ROAM can still be set when a client which
+		 * roamed away was re-added. It must not be announced with an
+		 * ADD because the other nodes would then handle it as roaming
+		 */
+		if (announce) {
+			event_flags = common->flags & ~BATADV_TT_CLIENT_ROAM;
+			batadv_tt_local_event(bat_priv, common, event_flags, false);
+		}
 	}
 
 	spin_unlock_bh(list_lock);
@@ -1710,7 +1705,7 @@ batadv_tt_local_set_pending(struct batadv_priv *bat_priv,
 	lockdep_assert_held(&hash->list_locks[i]);
 	lockdep_assert_held(&common->flags_lock);
 
-	__batadv_tt_local_event(bat_priv, common, common->flags | flags);
+	batadv_tt_local_event(bat_priv, common, common->flags | flags, false);
 	common->flags |= BATADV_TT_CLIENT_PENDING;
 
 	batadv_dbg(BATADV_DBG_TT, bat_priv,
@@ -1800,6 +1795,7 @@ batadv_tt_local_remove_now(struct batadv_priv *bat_priv,
 	spinlock_t *list_lock; /* protects write access to the hash lists */
 	struct batadv_tt_common_entry *common = &tt_local_entry->common;
 	struct batadv_hashtable *hash = bat_priv->tt.local_hash;
+	u16 flags;
 	u32 i;
 
 	i = batadv_choose_tt(common, hash->size);
@@ -1816,13 +1812,18 @@ batadv_tt_local_remove_now(struct batadv_priv *bat_priv,
 	hlist_del_init_rcu(&common->hash_entry);
 	atomic_inc(&hash->generation);
 
-	batadv_tt_local_event(bat_priv, tt_local_entry, BATADV_TT_CLIENT_DEL);
-
-	/* 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 in batadv_tt_local_mark_removed().
+	/* batadv_tt_local_transition_new() clears BATADV_TT_CLIENT_NEW under
+	 * the list_lock. The flags read here therefore show whether the entry
+	 * was committed (and announced) since the BATADV_TT_CLIENT_NEW check
+	 * in batadv_tt_local_mark_removed(). Only the ADD of an entry which
+	 * was never announced can be cancelled by its DEL.
 	 */
-	if (!(batadv_tt_flags_get(common) & BATADV_TT_CLIENT_NEW))
+	flags = batadv_tt_flags_get(common);
+	batadv_tt_local_event(bat_priv, common, flags | BATADV_TT_CLIENT_DEL,
+			      flags & BATADV_TT_CLIENT_NEW);
+
+	/* a committed entry was counted in the local table size */
+	if (!(flags & BATADV_TT_CLIENT_NEW))
 		batadv_tt_local_size_dec(bat_priv, common->vid);
 
 	spin_unlock_bh(list_lock);

Regards,
	Sven

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

[PATCH net-next 8/9] batman-adv: tt: reject VLAN/TT entries before reaching size limit

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

The translation table is announcing its current state via the TT TVLV in
each OGM(2). If another originator detects a desync with its own copy of
this state information, it will request a full table sync. Each originator
must therefore be able to send a reply with its full table without hitting
the size limit of this response.

The translation table code already tries to limit the amount of local TT
entries. But the code itself fails to achieve this task because:

* the size check uses shared, unlocked information
* the size check isn't done for VLANs

When enough VLANs + TT entries are created in parallel, then the TT will
need more room then allowed for a full translation table reply. Other
originators will then send regularly requests for a table sync but never
get a reply.

To avoid this problem for new VLAN and local TT entries, each new entry
must reserve "room" before it is actually allocated. Only when enough
"room" is available, this reservation is granted. This makes it possible to
keep the reservation handling locked and therefore safe for multiple
parallel contexts.

For now, over-reservation caused by a reduction of the MTU is not taken
into account.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/main.c              |   1 +
 net/batman-adv/mesh-interface.c    |   8 ++
 net/batman-adv/translation-table.c | 194 ++++++++++++++++++++++++-----
 net/batman-adv/translation-table.h |   3 +
 net/batman-adv/types.h             |  19 +++
 5 files changed, 194 insertions(+), 31 deletions(-)

diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index d89d44706269b..6c54a8d4be3ec 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -198,6 +198,7 @@ int batadv_mesh_init(struct net_device *mesh_iface)
 	spin_lock_init(&bat_priv->tt.roam_list_lock);
 	spin_lock_init(&bat_priv->tt.last_changeset_lock);
 	spin_lock_init(&bat_priv->tt.commit_lock);
+	spin_lock_init(&bat_priv->tt.reserve_lock);
 	spin_lock_init(&bat_priv->gw.list_lock);
 #ifdef CONFIG_BATMAN_ADV_MCAST
 	spin_lock_init(&bat_priv->mcast.mla_lock);
diff --git a/net/batman-adv/mesh-interface.c b/net/batman-adv/mesh-interface.c
index 63af21954cf90..1ce707aaf48a0 100644
--- a/net/batman-adv/mesh-interface.c
+++ b/net/batman-adv/mesh-interface.c
@@ -561,6 +561,8 @@ void batadv_meshif_vlan_release(struct kref *ref)
 	hlist_del_rcu(&vlan->list);
 	spin_unlock_bh(&vlan->bat_priv->meshif_vlan_list_lock);
 
+	batadv_tt_local_unreserve_vlan(vlan->bat_priv);
+
 	kfree_rcu(vlan, rcu);
 }
 
@@ -614,9 +616,15 @@ int batadv_meshif_create_vlan(struct batadv_priv *bat_priv, unsigned short vid)
 		return -EEXIST;
 	}
 
+	if (!batadv_tt_local_reserve_vlan(bat_priv, vid)) {
+		spin_unlock_bh(&bat_priv->meshif_vlan_list_lock);
+		return -EMSGSIZE;
+	}
+
 	vlan = kzalloc_obj(*vlan, GFP_ATOMIC);
 	if (!vlan) {
 		spin_unlock_bh(&bat_priv->meshif_vlan_list_lock);
+		batadv_tt_local_unreserve_vlan(bat_priv);
 		return -ENOMEM;
 	}
 
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 481dc6afaaba1..3349376a9087d 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -234,6 +234,162 @@ batadv_tt_global_hash_find(struct batadv_priv *bat_priv, const u8 *addr,
 	return tt_global_entry;
 }
 
+/**
+ * batadv_tt_len() - compute length in bytes of given number of tt changes
+ * @changes_num: number of tt changes
+ *
+ * Return: computed length in bytes.
+ */
+static int batadv_tt_len(int changes_num)
+{
+	return changes_num * sizeof(struct batadv_tvlv_tt_change);
+}
+
+/**
+ * batadv_tt_local_transmit_size() - calculate the size of a full table response
+ *  for a given number of VLANs and local TT entries
+ * @num_vlan: number of announced VLANs
+ * @num_entries: number of announced local TT entries
+ *
+ * Return: local translation table size in bytes.
+ */
+static int batadv_tt_local_transmit_size(u16 num_vlan, u16 num_entries)
+{
+	int hdr_size;
+
+	/* header size of tvlv encapsulated tt response payload */
+	hdr_size = sizeof(struct batadv_unicast_tvlv_packet);
+	hdr_size += sizeof(struct batadv_tvlv_hdr);
+	hdr_size += sizeof(struct batadv_tvlv_tt_data);
+	hdr_size += num_vlan * sizeof(struct batadv_tvlv_tt_vlan_data);
+
+	return hdr_size + batadv_tt_len(num_entries);
+}
+
+/**
+ * batadv_tt_local_reserve() - reserve room in the transmittable local table
+ * @bat_priv: the bat priv with all the mesh interface information
+ * @num_vlan: number of VLANs to reserve
+ * @num_entries: number of local TT entries to reserve
+ * @table_size: stores the resulting worst case table size in bytes
+ *
+ * Add the requested number of VLANs and local TT entries to the reservation
+ * counters and check whether the local translation table would then still fit
+ * in a single full table response. The reservation is dropped again when it
+ * would not.
+ *
+ * The reservation has to happen before the related object is allocated. This
+ * way two parallel allocations cannot both observe enough room for themselves
+ * and end up with a local table which can no longer be transmitted.
+ *
+ * A granted reservation must be returned via batadv_tt_local_unreserve() when
+ * the related object is released or was never created.
+ *
+ * Return: true when the reservation was granted, false otherwise.
+ */
+static bool batadv_tt_local_reserve(struct batadv_priv *bat_priv, u16 num_vlan,
+				    u16 num_entries, int *table_size)
+{
+	int packet_size_max = READ_ONCE(bat_priv->packet_size_max);
+
+	scoped_guard(spinlock_bh, &bat_priv->tt.reserve_lock) {
+		bat_priv->tt.reserved_vlans += num_vlan;
+		bat_priv->tt.reserved_entries += num_entries;
+
+		*table_size = batadv_tt_local_transmit_size(bat_priv->tt.reserved_vlans,
+							    bat_priv->tt.reserved_entries);
+		if (*table_size <= packet_size_max)
+			return true;
+
+		bat_priv->tt.reserved_vlans -= num_vlan;
+		bat_priv->tt.reserved_entries -= num_entries;
+	}
+
+	return false;
+}
+
+/**
+ * batadv_tt_local_unreserve() - return room in the transmittable local table
+ * @bat_priv: the bat priv with all the mesh interface information
+ * @num_vlan: number of VLANs to return
+ * @num_entries: number of local TT entries to return
+ */
+static void batadv_tt_local_unreserve(struct batadv_priv *bat_priv,
+				      u16 num_vlan, u16 num_entries)
+{
+	scoped_guard(spinlock_bh, &bat_priv->tt.reserve_lock) {
+		bat_priv->tt.reserved_vlans -= num_vlan;
+		bat_priv->tt.reserved_entries -= num_entries;
+	}
+}
+
+/**
+ * batadv_tt_local_reserve_entry() - reserve room for a new local TT entry
+ * @bat_priv: the bat priv with all the mesh interface information
+ * @addr: the mac address of the client to add
+ *
+ * Return: true when the reservation was granted, false otherwise.
+ */
+static bool batadv_tt_local_reserve_entry(struct batadv_priv *bat_priv,
+					  const u8 *addr)
+{
+	int table_size;
+
+	if (batadv_tt_local_reserve(bat_priv, 0, 1, &table_size))
+		return true;
+
+	net_ratelimited_function(batadv_info, bat_priv->mesh_iface,
+				 "Local translation table size (%i) exceeds maximum packet size (%i); Ignoring new local tt entry: %pM\n",
+				 table_size,
+				 READ_ONCE(bat_priv->packet_size_max), addr);
+
+	return false;
+}
+
+/**
+ * batadv_tt_local_unreserve_entry() - return the room of a local TT entry
+ * @bat_priv: the bat priv with all the mesh interface information
+ */
+static void batadv_tt_local_unreserve_entry(struct batadv_priv *bat_priv)
+{
+	batadv_tt_local_unreserve(bat_priv, 0, 1);
+}
+
+/**
+ * batadv_tt_local_reserve_vlan() - reserve room for a new VLAN
+ * @bat_priv: the bat priv with all the mesh interface information
+ * @vid: the VLAN identifier
+ *
+ * Each VLAN adds a per VLAN header to the full table response and therefore
+ * has to be reserved before batadv_meshif_create_vlan() allocates it.
+ *
+ * Return: true when the reservation was granted, false otherwise.
+ */
+bool batadv_tt_local_reserve_vlan(struct batadv_priv *bat_priv,
+				  unsigned short vid)
+{
+	int table_size;
+
+	if (batadv_tt_local_reserve(bat_priv, 1, 0, &table_size))
+		return true;
+
+	net_ratelimited_function(batadv_info, bat_priv->mesh_iface,
+				 "Local translation table size (%i) exceeds maximum packet size (%i); Ignoring new VLAN: %d\n",
+				 table_size, READ_ONCE(bat_priv->packet_size_max),
+				 batadv_print_vid(vid));
+
+	return false;
+}
+
+/**
+ * batadv_tt_local_unreserve_vlan() - return the room of a VLAN
+ * @bat_priv: the bat priv with all the mesh interface information
+ */
+void batadv_tt_local_unreserve_vlan(struct batadv_priv *bat_priv)
+{
+	batadv_tt_local_unreserve(bat_priv, 1, 0);
+}
+
 /**
  * batadv_tt_local_entry_release() - release tt_local_entry from lists and queue
  *  for free after rcu grace period
@@ -246,6 +402,7 @@ static void batadv_tt_local_entry_release(struct kref *ref)
 	tt_local_entry = container_of(ref, struct batadv_tt_local_entry,
 				      common.refcount);
 
+	batadv_tt_local_unreserve_entry(tt_local_entry->vlan->bat_priv);
 	batadv_meshif_vlan_put(tt_local_entry->vlan);
 
 	kfree_rcu(tt_local_entry, common.rcu);
@@ -550,17 +707,6 @@ static void batadv_tt_local_event(struct batadv_priv *bat_priv,
 	__batadv_tt_local_event(bat_priv, common, flags);
 }
 
-/**
- * batadv_tt_len() - compute length in bytes of given number of tt changes
- * @changes_num: number of tt changes
- *
- * Return: computed length in bytes.
- */
-static int batadv_tt_len(int changes_num)
-{
-	return changes_num * sizeof(struct batadv_tvlv_tt_change);
-}
-
 /**
  * batadv_tt_entries() - compute the number of entries fitting in tt_len bytes
  * @tt_len: available space
@@ -584,7 +730,6 @@ static int batadv_tt_local_table_transmit_size(struct batadv_priv *bat_priv)
 	struct batadv_meshif_vlan *vlan;
 	u16 tt_local_entries = 0;
 	u16 num_vlan = 0;
-	int hdr_size;
 
 	rcu_read_lock();
 	hlist_for_each_entry_rcu(vlan, &bat_priv->meshif_vlan_list, list) {
@@ -593,13 +738,7 @@ static int batadv_tt_local_table_transmit_size(struct batadv_priv *bat_priv)
 	}
 	rcu_read_unlock();
 
-	/* header size of tvlv encapsulated tt response payload */
-	hdr_size = sizeof(struct batadv_unicast_tvlv_packet);
-	hdr_size += sizeof(struct batadv_tvlv_hdr);
-	hdr_size += sizeof(struct batadv_tvlv_tt_data);
-	hdr_size += num_vlan * sizeof(struct batadv_tvlv_tt_vlan_data);
-
-	return hdr_size + batadv_tt_len(tt_local_entries);
+	return batadv_tt_local_transmit_size(num_vlan, tt_local_entries);
 }
 
 /**
@@ -803,23 +942,15 @@ batadv_tt_local_create(struct net_device *mesh_iface, const u8 *addr,
 	struct batadv_priv *bat_priv = netdev_priv(mesh_iface);
 	struct batadv_tt_local_entry *tt_local;
 	struct batadv_meshif_vlan *vlan;
-	int packet_size_max;
-	int table_size;
 
-	/* Ignore the client if we cannot send it in a full table response. */
-	table_size = batadv_tt_local_table_transmit_size(bat_priv);
-	table_size += batadv_tt_len(1);
-	packet_size_max = READ_ONCE(bat_priv->packet_size_max);
-	if (table_size > packet_size_max) {
-		net_ratelimited_function(batadv_info, mesh_iface,
-					 "Local translation table size (%i) exceeds maximum packet size (%i); Ignoring new local tt entry: %pM\n",
-					 table_size, packet_size_max, addr);
+	if (!batadv_tt_local_reserve_entry(bat_priv, addr))
 		return NULL;
-	}
 
 	tt_local = kmem_cache_alloc(batadv_tl_cache, GFP_ATOMIC);
-	if (!tt_local)
+	if (!tt_local) {
+		batadv_tt_local_unreserve_entry(bat_priv);
 		return NULL;
+	}
 
 	/* increase the refcounter of the related vlan */
 	vlan = batadv_meshif_vlan_get(bat_priv, vid);
@@ -828,6 +959,7 @@ batadv_tt_local_create(struct net_device *mesh_iface, const u8 *addr,
 					 "adding TT local entry %pM to non-existent VLAN %d\n",
 					 addr, batadv_print_vid(vid));
 		kmem_cache_free(batadv_tl_cache, tt_local);
+		batadv_tt_local_unreserve_entry(bat_priv);
 		return NULL;
 	}
 
diff --git a/net/batman-adv/translation-table.h b/net/batman-adv/translation-table.h
index 618d9dbca5eac..ca01d20ee2d7f 100644
--- a/net/batman-adv/translation-table.h
+++ b/net/batman-adv/translation-table.h
@@ -16,6 +16,9 @@
 #include <linux/types.h>
 
 int batadv_tt_init(struct batadv_priv *bat_priv);
+bool batadv_tt_local_reserve_vlan(struct batadv_priv *bat_priv,
+				  unsigned short vid);
+void batadv_tt_local_unreserve_vlan(struct batadv_priv *bat_priv);
 bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
 			 unsigned short vid, int ifindex, u32 mark);
 u16 batadv_tt_local_remove(struct batadv_priv *bat_priv,
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 67872927cfb50..c19caa84e6920 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -1055,6 +1055,25 @@ struct batadv_priv_tt {
 	 */
 	spinlock_t commit_lock;
 
+	/**
+	 * @reserved_entries: number of local TT entries which are currently
+	 *  allocated or about to be allocated. Together with @reserved_vlans it
+	 *  describes the worst case size of a full table response.
+	 */
+	u16 reserved_entries;
+
+	/**
+	 * @reserved_vlans: number of mesh interface VLANs which are currently
+	 *  allocated or about to be allocated. Together with @reserved_entries
+	 *  it describes the worst case size of a full table response.
+	 */
+	u16 reserved_vlans;
+
+	/**
+	 * @reserve_lock: lock protecting @reserved_entries & @reserved_vlans
+	 */
+	spinlock_t reserve_lock;
+
 	/** @work: work queue callback item for translation table purging */
 	struct delayed_work work;
 };
-- 
2.47.3

Re: [PATCH net-next 8/9] batman-adv: tt: reject VLAN/TT entries before reaching size limit

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] batadv_tt_local_reserve() (net/batman-adv/translation-
  table.c:290) grows the new u16 counters bat_priv->tt.reserved_entries…
- [Low] The commit fixes a user-visible protocol failure but has no
  Fixes: tag and no stable Cc.

--

batman-adv: tt: reject VLAN/TT entries before reaching size limit

This commit replaces the unlocked size check in batadv_tt_local_create()
with a spinlock-protected reservation of room for VLANs and local TT
entries. The goal is that the local translation table always fits in a
full table response. VLAN creation in batadv_meshif_create_vlan() now also
has to reserve room, and it fails with -EMSGSIZE when no room is left.

> When enough VLANs + TT entries are created in parallel, then the TT will
> need more room then allowed for a full translation table reply. Other
> originators will then send regularly requests for a table sync but never
> get a reply.

[ ... ]

> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>

[Severity: Low]
Should this have a Fixes: tag, and maybe a Cc to stable?

The racy check replaced in batadv_tt_local_create() only moved there with
the recent split of the local entry allocation. The check itself goes back
to when batman-adv first limited the local TT size and made TT VLAN-aware.

Other fixes in this series carry Fixes: tags, for example
"batman-adv: tt: queue local DEL event under bucket lock".

A backport of this patch would also depend on the batadv_tt_local_create()
split and on scoped_guard(). Could the commit message mention that as well?

> diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> index 481dc6afaaba1..3349376a9087d 100644
> --- a/net/batman-adv/translation-table.c
> +++ b/net/batman-adv/translation-table.c
> @@ -234,6 +234,162 @@ batadv_tt_global_hash_find(struct batadv_priv *bat_priv, const u8 *addr,

[ ... ]

> +static bool batadv_tt_local_reserve(struct batadv_priv *bat_priv, u16 num_vlan,
> +				    u16 num_entries, int *table_size)
> +{
> +	int packet_size_max = READ_ONCE(bat_priv->packet_size_max);
> +
> +	scoped_guard(spinlock_bh, &bat_priv->tt.reserve_lock) {
> +		bat_priv->tt.reserved_vlans += num_vlan;
> +		bat_priv->tt.reserved_entries += num_entries;
> +
> +		*table_size = batadv_tt_local_transmit_size(bat_priv->tt.reserved_vlans,
> +							    bat_priv->tt.reserved_entries);
> +		if (*table_size <= packet_size_max)
> +			return true;

[Severity: Medium]
Can bat_priv->tt.reserved_entries wrap here?

The new counters in struct batadv_priv_tt are u16. The only bound on them
is packet_size_max, and nothing caps packet_size_max at what a single TT
TVLV can hold. When fragmentation is disabled, batadv_hardif_min_mtu()
skips the clamp:

net/batman-adv/hard-interface.c:batadv_hardif_min_mtu() {
	int min_mtu = INT_MAX;
	...
	if (READ_ONCE(bat_priv->fragmentation) == 0)
		goto out;
	...
out:
	...
	WRITE_ONCE(bat_priv->packet_size_max, min_mtu);
	...
}

In that case packet_size_max is INT_MAX when no hard interface is active.
Otherwise it is the raw MTU of the hard interface, which can be above
65535 (dummy has an unlimited max_mtu, for example).

Untrusted source MACs can create local entries, because
batadv_interface_tx() calls batadv_tt_local_add() for every non-multicast
source address:

batadv_interface_tx()
  batadv_tt_local_add()
    batadv_tt_local_create()
      batadv_tt_local_reserve_entry()
        batadv_tt_local_reserve()

If packet_size_max is above roughly 65535, the table can still grow past
what batadv_tt_prepare_tvlv_local_data() can build, because tvlv_len is a
u16:

	if (check_add_overflow(*tt_len, change_offset, &tvlv_len)) {
		tvlv_len = 0;
		*tt_len = 0;
		goto out;
	}

batadv_send_my_tt_response() then never sends the full table. Isn't that
the same "never get a reply" condition the commit message describes?

If packet_size_max is at least batadv_tt_local_transmit_size(v, 65535),
about 786KB (INT_MAX qualifies), the 65536th reservation wraps
reserved_entries to 0. From then on the counter holds the real count
modulo 65536, so the check keeps granting entries. If packet_size_max
later drops, the check also admits new entries that should be refused.

batadv_tt_local_table_transmit_size(), which
batadv_tt_local_resize_to_mtu() uses, also sums into a u16 and can wrap
the same way.

Would capping the limit at the u16 capacity of the TVLV, and saturating
or widening the counters, make the guarantee in the commit message hold?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930094558.3723766-2-sw%40simonwunderlich.de

Re: [PATCH net-next 8/9] batman-adv: tt: reject VLAN/TT entries before reaching size limit

[Sven Eckelmann]

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

On Thursday, 1 October 2026 12:10:31 CEST netdev-bot+sashiko@kernel.org wrote:
[...]
> > Signed-off-by: Sven Eckelmann <sven@narfation.org>
> > Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
> 
> [Severity: Low]
> Should this have a Fixes: tag, and maybe a Cc to stable?

No, the change is not suitable for stable backports. And since it is submitted 
for net-next, I am not allowed to add Fixes: (for old kernels) or Cc: stable@ tags.

[...]
> > diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> > index 481dc6afaaba1..3349376a9087d 100644
> > --- a/net/batman-adv/translation-table.c
> > +++ b/net/batman-adv/translation-table.c
> > @@ -234,6 +234,162 @@ batadv_tt_global_hash_find(struct batadv_priv *bat_priv, const u8 *addr,
> 
> [ ... ]
> 
> > +static bool batadv_tt_local_reserve(struct batadv_priv *bat_priv, u16 num_vlan,
> > +				    u16 num_entries, int *table_size)
> > +{
> > +	int packet_size_max = READ_ONCE(bat_priv->packet_size_max);
> > +
> > +	scoped_guard(spinlock_bh, &bat_priv->tt.reserve_lock) {
> > +		bat_priv->tt.reserved_vlans += num_vlan;
> > +		bat_priv->tt.reserved_entries += num_entries;
> > +
> > +		*table_size = batadv_tt_local_transmit_size(bat_priv->tt.reserved_vlans,
> > +							    bat_priv->tt.reserved_entries);
> > +		if (*table_size <= packet_size_max)
> > +			return true;
> 
> [Severity: Medium]
> Can bat_priv->tt.reserved_entries wrap here?
>
> The new counters in struct batadv_priv_tt are u16. The only bound on them
> is packet_size_max, and nothing caps packet_size_max at what a single TT
> TVLV can hold. When fragmentation is disabled, batadv_hardif_min_mtu()
> skips the clamp:

This is actually a good point. I didn't think about the case of an extremely 
large MTU and used a 9000/64K MTU in my head as reason for using only u16. I 
still consider it as a corner case and will submit a follow up patch for it.


> If packet_size_max is above roughly 65535, the table can still grow past
> what batadv_tt_prepare_tvlv_local_data() can build, because tvlv_len is a
> u16:
> 
> 	if (check_add_overflow(*tt_len, change_offset, &tvlv_len)) {
> 		tvlv_len = 0;
> 		*tt_len = 0;
> 		goto out;
> 	}

Correct, so widening over u16 doesn't make any sense. It seems to be better to 
just limit the size of the packet against which it is checked:

Will be submitted later to netdev as follow up check because it is not a 
regression but just a limitation in the supported packet_size_max limits (for 
non-standard setups).

https://lore.kernel.org/r/20261003-tvlv-limited-tt-response-reservation-v1-1-82fbca929cd9@narfation.org

Regards,
	Sven

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

[PATCH net-next 9/9] batman-adv: use assign_bit() where applicable

[Simon Wunderlich]

From: Peng Fan <peng.fan@nxp.com>

Convert open-coded if/else with set_bit/clear_bit the assign_bit API.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/distributed-arp-table.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c
index a4be5b2a87d7a..74050bffedbce 100644
--- a/net/batman-adv/distributed-arp-table.c
+++ b/net/batman-adv/distributed-arp-table.c
@@ -863,10 +863,8 @@ static void batadv_dat_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv,
 					   u8 flags,
 					   void *tvlv_value, u16 tvlv_value_len)
 {
-	if (flags & BATADV_TVLV_HANDLER_OGM_CIFNOTFND)
-		clear_bit(BATADV_ORIG_CAPA_HAS_DAT, &orig->capabilities);
-	else
-		set_bit(BATADV_ORIG_CAPA_HAS_DAT, &orig->capabilities);
+	assign_bit(BATADV_ORIG_CAPA_HAS_DAT, &orig->capabilities,
+		   !(flags & BATADV_TVLV_HANDLER_OGM_CIFNOTFND));
 }
 
 /**
-- 
2.47.3