[PATCH net 00/10] pull request: batman-adv 2026-10-26

[PATCH net 00/10] pull request: batman-adv 2026-10-26

[Simon Wunderlich]

Dear net maintainers,

here are some bugfixes for batman-adv which we would like to have integrated into net.

Please pull or let me know of any problem!

Thank you,
      Simon

The following changes since commit d1807b20d548171d40c5d21a7cf22830ea8f348e:

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

are available in the Git repository at:

  https://git.open-mesh.org/batadv.git tags/batadv-net-pullrequest-20261026

for you to fetch changes up to 6e7c45ed237c5cfe10ee121f8c0c8909e7d6e0f8:

  batman-adv: tt: limit local table to single TVLV response size (2026-10-25 08:45:11 +0100)

----------------------------------------------------------------
Here are a few batman-adv bugfixes:

- avoid WARN_ON for orig_ifinfo allocation failure, by Deepanshu Kartikey

- close OGM aggregation before transmission, by Chengfeng Ye

The remaining bufixes are authored by Sven Eckelmann:

- tpmeter: fix u32 truncation of reported bytes

- dat: initialize orig_node of unfilled candidate slots

- fix skb leak when hard_iface is being released

- stop originator purge worker before freeing tt

- tt: fix flag and event handling (3 patches)

- tt: limit local table to single TVLV response size

----------------------------------------------------------------
Chengfeng Ye (1):
      batman-adv: Close OGM aggregation before transmission

Deepanshu Kartikey (1):
      batman-adv: avoid WARN_ON for orig_ifinfo allocation failure

Sven Eckelmann (8):
      batman-adv: tpmeter: fix u32 truncation of reported bytes
      batman-adv: dat: initialize orig_node of unfilled candidate slots
      batman-adv: fix skb leak when hard_iface is being released
      batman-adv: stop originator purge worker before freeing TT
      batman-adv: tt: only transmit local ROAM flag together with DEL
      batman-adv: tt: only update flags for TT events
      batman-adv: tt: don't drop local events queued during TVLV update
      batman-adv: tt: limit local table to single TVLV response size

 net/batman-adv/bat_iv_ogm.c            |   7 +-
 net/batman-adv/distributed-arp-table.c |   1 +
 net/batman-adv/main.c                  |  12 ++-
 net/batman-adv/tp_meter.c              |   2 +-
 net/batman-adv/translation-table.c     | 138 +++++++++++++++++++++------------
 5 files changed, 104 insertions(+), 56 deletions(-)

[PATCH net 01/10] batman-adv: avoid WARN_ON for orig_ifinfo allocation failure

[Simon Wunderlich]

From: Deepanshu Kartikey <kartikey406@gmail.com>

batadv_orig_ifinfo_new() returns NULL only when its GFP_ATOMIC
allocation fails, which is a normal runtime condition and not a bug.
batadv_v_ogm_forward() already handles this the same way, simply
checking for NULL without a WARN_ON.

Do the same in batadv_iv_ogm_update_seqnos() to avoid a spurious
WARN_ON splat when the allocation fails (e.g. under fault injection).

Fixes: 7351a4822d42 ("batman-adv: split out router from orig_node")
Reported-by: syzbot+a079f084653164a652d1@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=a079f084653164a652d1
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/bat_iv_ogm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 89b499f7a37dd..158d2b8f04036 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -1410,7 +1410,7 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr,
 		return BATADV_NO_DUP;
 
 	orig_ifinfo = batadv_orig_ifinfo_new(orig_node, if_outgoing);
-	if (WARN_ON(!orig_ifinfo)) {
+	if (!orig_ifinfo) {
 		batadv_orig_node_put(orig_node);
 		return 0;
 	}
-- 
2.47.3

Re: [PATCH net 01/10] batman-adv: avoid WARN_ON for orig_ifinfo allocation failure

[netdev-bot+sinfo]

Hi!

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

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

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

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

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

[PATCH net 02/10] batman-adv: Close OGM aggregation before transmission

[Simon Wunderlich]

From: Chengfeng Ye <nicoyip.dev@gmail.com>

The OGM send worker leaves its forwarding packet on forw_bat_list until
after batadv_iv_ogm_emit() returns. Aggregation holds forw_bat_list_lock,
but emission reads and clones the packet without that lock.

CPU 0 can therefore start emitting a queued packet while CPU 1 takes the
list lock, finds the same packet and appends another OGM. The sender can
observe the new packet length before the corresponding direct-link flag
is set, or clone the skb while its length and payload are being updated.
This can transmit an OGM with incorrect flags or inconsistent data.

KCSAN reported:

  BUG: KCSAN: data-race in batadv_iv_ogm_queue_add / batadv_iv_send_outstanding_bat_ogm_packet
  write to 0xffff888100fedcf8 of 2 bytes by interrupt on cpu 1:
  read to 0xffff888100fedcf8 of 2 bytes by task 70 on cpu 2:
  value changed: 0x00c0 -> 0x00d8

Mark the aggregate as full under forw_bat_list_lock before emission. This
waits for any ongoing append and prevents further aggregation. Emission
uses packet_len to walk the OGMs, so all queued packets are still sent.
Keep the packet on the list so interface purging can find the worker,
wait for it to finish and free the packet.

Fixes: 9b4aec647a92 ("batman-adv: fix rare race conditions on interface removal")
Cc: stable@vger.kernel.org # 4.10.x: 434becf57bdc ("batman-adv: Limit number of aggregated packets directly")
Cc: stable@vger.kernel.org # 4.10.x
Assisted-by: GPT-6-Astra
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/bat_iv_ogm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 158d2b8f04036..85e99506a53f5 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -1909,6 +1909,11 @@ static void batadv_iv_send_outstanding_bat_ogm_packet(struct work_struct *work)
 		goto out;
 	}
 
+	/* Mark aggregate as full before forcing emit. */
+	spin_lock_bh(&bat_priv->forw_bat_list_lock);
+	forw_packet->num_packets = BATADV_MAX_AGGREGATION_PACKETS;
+	spin_unlock_bh(&bat_priv->forw_bat_list_lock);
+
 	batadv_iv_ogm_emit(forw_packet);
 
 	/* we have to have at least one packet in the queue to determine the
-- 
2.47.3

Re: [PATCH net 02/10] batman-adv: Close OGM aggregation before transmission

[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 patch gives `struct batadv_forw_packet::num_packets` a second
  meaning without updating its kernel-doc.

--

batman-adv: Close OGM aggregation before transmission

This patch makes the OGM send worker mark the forwarding packet as a full
aggregate under forw_bat_list_lock before it calls batadv_iv_ogm_emit().
After that, no other context can append OGMs while the packet is being
emitted and cloned. The packet stays on forw_bat_list so interface purging
can still find it.

> diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
> index 158d2b8f04036..85e99506a53f5 100644
> --- a/net/batman-adv/bat_iv_ogm.c
> +++ b/net/batman-adv/bat_iv_ogm.c
> @@ -1909,6 +1909,11 @@ static void batadv_iv_send_outstanding_bat_ogm_packet(struct work_struct *work)
>  		goto out;
>  	}
>  
> +	/* Mark aggregate as full before forcing emit. */
> +	spin_lock_bh(&bat_priv->forw_bat_list_lock);
> +	forw_packet->num_packets = BATADV_MAX_AGGREGATION_PACKETS;
> +	spin_unlock_bh(&bat_priv->forw_bat_list_lock);

[Severity: Low]
This isn't a bug, but should the kernel-doc for num_packets be updated
to match this new use?

In net/batman-adv/types.h the field is still documented as a plain
counter:

	/** @num_packets: counter for aggregated OGMv1 packets */
	u8 num_packets;

After this change, the worker in
batadv_iv_send_outstanding_bat_ogm_packet() replaces the real count with
BATADV_MAX_AGGREGATION_PACKETS, whatever the number of queued OGMs is.
For example, an aggregate holding 3 OGMs reports 32 from then on. The
field is now both a counter and a "closed for aggregation" marker, and
only the comment at this call site says so.

As far as I can tell, nothing reads the inflated value today:

- batadv_iv_ogm_can_aggregate() rejects packet_num >=
  BATADV_MAX_AGGREGATION_PACKETS under the same lock.
- batadv_iv_ogm_aggregate() is only reached after that check passes, so
  set_bit(num_packets, direct_link_flags) cannot index past the bitmap.
- batadv_iv_ogm_send_to_if() walks the OGMs using packet_len and a local
  counter.

A short note in the struct documentation would help later readers who
rely on the field description.

>  	batadv_iv_ogm_emit(forw_packet);
>  
>  	/* we have to have at least one packet in the queue to determine the

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

Re: [PATCH net 02/10] batman-adv: Close OGM aggregation before transmission

[Sven Eckelmann]

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

On Tuesday, 27 October 2026 09:55:21 CET netdev-bot+sashiko@kernel.org wrote:
> batman-adv: Close OGM aggregation before transmission
> 
> This patch makes the OGM send worker mark the forwarding packet as a full
> aggregate under forw_bat_list_lock before it calls batadv_iv_ogm_emit().
> After that, no other context can append OGMs while the packet is being
> emitted and cloned. The packet stays on forw_bat_list so interface purging
> can still find it.
> 
> > diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
> > index 158d2b8f04036..85e99506a53f5 100644
> > --- a/net/batman-adv/bat_iv_ogm.c
> > +++ b/net/batman-adv/bat_iv_ogm.c
> > @@ -1909,6 +1909,11 @@ static void batadv_iv_send_outstanding_bat_ogm_packet(struct work_struct *work)
> >  		goto out;
> >  	}
> >  
> > +	/* Mark aggregate as full before forcing emit. */
> > +	spin_lock_bh(&bat_priv->forw_bat_list_lock);
> > +	forw_packet->num_packets = BATADV_MAX_AGGREGATION_PACKETS;
> > +	spin_unlock_bh(&bat_priv->forw_bat_list_lock);
> 
> [Severity: Low]
> This isn't a bug, but should the kernel-doc for num_packets be updated
> to match this new use?

https://patchwork.open-mesh.org/project/b.a.t.m.a.n./patch/20261027-kdoc-num_packets-v1-1-1fa6d584b7b3@narfation.org/

Regards,
	Sven

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

[PATCH net 03/10] batman-adv: tpmeter: fix u32 truncation of reported bytes

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

When more than 4 GB are transferred by the throughput meter,
batadv_tp_batctl_notify() will cut of the upper 32 bit of the u64 counter
and thus under-report the actual transferred bytes.

The data type of the temporary storage variable must therefore still
continue to use u64.

Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/bug/linux-08bb3be4-6c6a-452d-b4e4-606940c6cfa5
Fixes: 33a3bb4a3345 ("batman-adv: throughput meter implementation")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/tp_meter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/batman-adv/tp_meter.c b/net/batman-adv/tp_meter.c
index 022eb13e8ec47..56feb02d2b7ef 100644
--- a/net/batman-adv/tp_meter.c
+++ b/net/batman-adv/tp_meter.c
@@ -221,7 +221,7 @@ static void batadv_tp_batctl_notify(enum batadv_tp_meter_reason reason,
 				    unsigned long start_time, u64 total_sent,
 				    u32 cookie)
 {
-	u32 total_bytes;
+	u64 total_bytes;
 	u32 test_time;
 	u8 result;
 
-- 
2.47.3

[PATCH net 04/10] batman-adv: dat: initialize orig_node of unfilled candidate slots

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

batadv_dat_select_candidates() allocates the candidate array without
zeroing it. batadv_choose_next_candidate() marks each slot as
BATADV_DAT_CANDIDATE_NOT_FOUND but only sets orig_node when an eligible
originator was found. In the following rounds,
batadv_is_orig_node_eligible() compares res[j].orig_node of all previous
slots against the evaluated originator. For slots without candidates, it
reads uninitialized memory.

The orig_node has to be cleared together with the
BATADV_DAT_CANDIDATE_NOT_FOUND type.

Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/bug/linux-b25d9aa4-10bb-427c-a145-ccaadc2a1bb9
Fixes: 785ea1144182 ("batman-adv: Distributed ARP Table - create DHT helper functions")
Acked-by: Linus Lüssing <linus.luessing@c0d3.blue>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/distributed-arp-table.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c
index 74050bffedbce..0260ed7e828b2 100644
--- a/net/batman-adv/distributed-arp-table.c
+++ b/net/batman-adv/distributed-arp-table.c
@@ -653,6 +653,7 @@ static void batadv_choose_next_candidate(struct batadv_priv *bat_priv,
 	 * NOT_FOUND
 	 */
 	cands[select].type = BATADV_DAT_CANDIDATE_NOT_FOUND;
+	cands[select].orig_node = NULL;
 
 	/* iterate over the originator list and find the node with the closest
 	 * dat_address which has not been selected yet
-- 
2.47.3

[PATCH net 05/10] batman-adv: fix skb leak when hard_iface is being released

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

batadv_batman_skb_recv() owns the skb it is given. When the hard_iface
refcount already dropped to zero, the function jumped to err_out, which
only returned NET_RX_DROP and skipped the kfree_skb() under err_free.

To avoid a leak, free the skb before returning in this case.

Cc: stable@vger.kernel.org
Fixes: 4fe56e60ac1b ("batman-adv: Check hard_iface refcnt when receiving skb")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/main.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index 6c54a8d4be3ec..ced76942f33c5 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -484,8 +484,10 @@ int batadv_batman_skb_recv(struct sk_buff *skb, struct net_device *dev,
 	 * shut down otherwise the packet may trigger de-reference errors
 	 * further down in the receive path.
 	 */
-	if (!kref_get_unless_zero(&hard_iface->refcount))
-		goto err_out;
+	if (!kref_get_unless_zero(&hard_iface->refcount)) {
+		kfree_skb(skb);
+		return NET_RX_DROP;
+	}
 
 	skb = skb_share_check(skb, GFP_ATOMIC);
 
@@ -541,7 +543,6 @@ int batadv_batman_skb_recv(struct sk_buff *skb, struct net_device *dev,
 	kfree_skb(skb);
 err_put:
 	batadv_hardif_put(hard_iface);
-err_out:
 	return NET_RX_DROP;
 }
 
-- 
2.47.3

[PATCH net 06/10] batman-adv: stop originator purge worker before freeing TT

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

batadv_mesh_free() releases the translation tables via batadv_tt_free()
before batadv_originator_free() stops orig_work. The purge worker can
therefore still run while the TT is torn down. When an originator times
out, batadv_purge_orig_ref() calls batadv_tt_global_del_orig(), which
loads tt.global_hash and walks all buckets while taking their locks:

  CPU0                          CPU1
  batadv_tt_free()              batadv_purge_orig()
                                  batadv_tt_global_del_orig()
                                    hash = ...tt.global_hash;
    batadv_tt_global_table_free()
      batadv_hash_destroy(hash);
                                    spin_lock_bh(&hash->list_locks[i]);

Disable orig_work at the beginning of batadv_mesh_free(), directly after
the mesh was marked as deactivating. The second
disable_delayed_work_sync() in batadv_originator_free() is harmless because
it is free'd anyway.

Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260708091821.314516-1-sw%40simonwunderlich.de?part=3
Fixes: 9d31b3ce8168 ("batman-adv: fix potential TT client + orig-node memory leak")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/main.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index ced76942f33c5..3b9fa861be7be 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -295,6 +295,11 @@ void batadv_mesh_free(struct net_device *mesh_iface)
 
 	WRITE_ONCE(bat_priv->mesh_state, BATADV_MESH_DEACTIVATING);
 
+	/* the originator purge worker accesses (among others) the TT tables
+	 * and must therefore be stopped before any of them are freed
+	 */
+	disable_delayed_work_sync(&bat_priv->orig_work);
+
 	batadv_purge_outstanding_packets(bat_priv, NULL);
 	batadv_tp_stop_all(bat_priv);
 
-- 
2.47.3

[PATCH net 07/10] batman-adv: tt: only transmit local ROAM flag together with DEL

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

BATADV_TT_CLIENT_ROAM is part of BATADV_TT_REMOTE_MASK but on the wire it
only has a defined meaning together with BATADV_TT_CLIENT_DEL: the
originator no longer serves the client because it roamed to another node.
Such a DEL is only relevant for events (non-full table requests).

An ADD carrying ROAM is processed (by accident) by the receiver like a
ROAMING_ADV:

* a newly created global entry is marked as roaming and is excluded from
  the global CRC. but the sender includes it in its local CRC
* batadv_tt_global_purge_local() removes a local entry of the receiver

But such a behavior doesn't make sense for non-ROAM_ADV because a new TTVN
is meant as announced commit of the local TT table of a node and should
therefore not be used not be used to create a temporary (roam) global entry
in a node.

At the moment, such an ADD with the BATADV_TT_CLIENT_ROAM flag set can
happen due to two mechanisms:

batadv_tt_local_valid() copies all flags of the local entry into a full
table response. It is not synchronized with batadv_tt_local_mark_removed().
An entry which was marked as ROAM|PENDING by a ROAMING_ADV is still part of
the current TTVN and is sent with the ROAM flag set. When the requester is
the node which sent the ROAMING_ADV, it then marks its own (new) local
client as roamed back to the old originator and drops it when it is still
NEW. Other receivers will detect a CRC mismatch and request the full table
again.

The same can happen for an ADD change which is queued for an entry which
(still) has the ROAM flag set. For example when only the dynamic WIFI or
ISOLA flags were updated.

It is not triggered by batadv_send_other_tt_response() because
batadv_tt_global_valid() is already taking care of filtering out
BATADV_TT_CLIENT_ROAM entries.

For the two problematic announcement mechanism, only send the ROAM flag of
local entries together with a DEL.

Cc: stable@vger.kernel.org
Fixes: 27b37ebfa2d2 ("batman-adv: correctly pass the client flag on tt_response")
Fixes: 068ee6e204e1 ("batman-adv: roaming handling mechanism redesign")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/translation-table.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 3349376a9087d..39b61426f1680 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -643,9 +643,17 @@ 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);
 
-	tt_change_node->change.flags = flags;
 	del_op_requested = flags & BATADV_TT_CLIENT_DEL;
 
+	/* the ROAM flag is only meaningful for the receiver in combination
+	 * with a DEL. An ADD with ROAM would be interpreted as a roaming
+	 * advertisement
+	 */
+	if (!del_op_requested)
+		flags &= ~BATADV_TT_CLIENT_ROAM;
+
+	tt_change_node->change.flags = flags;
+
 	/* check for ADD+DEL, DEL+ADD, ADD+ADD or DEL+DEL events */
 	spin_lock_bh(&bat_priv->tt.changes_list_lock);
 	changes = READ_ONCE(bat_priv->tt.local_changes);
@@ -3342,8 +3350,12 @@ static bool batadv_tt_local_valid(void *entry_ptr,
 	if (tt_flags & BATADV_TT_CLIENT_NEW)
 		return false;
 
+	/* the entry is (still) announced. A ROAM flag of an entry which is
+	 * pending to be removed must not be transmitted. Otherwise, the
+	 * receiver would handle it like a roaming advertisement
+	 */
 	if (flags)
-		*flags = tt_flags;
+		*flags = tt_flags & ~BATADV_TT_CLIENT_ROAM;
 
 	return true;
 }
-- 
2.47.3

Re: [PATCH net 07/10] batman-adv: tt: only transmit local ROAM flag together with DEL

[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 fix leaves the requester-side client drop in place even
  though the commit message lists it as a symptom, and the message…

--

batman-adv: tt: only transmit local ROAM flag together with DEL

This patch stops local TT entries from sending BATADV_TT_CLIENT_ROAM
unless it comes with BATADV_TT_CLIENT_DEL. The ROAM bit is now cleared
for non-DEL events in __batadv_tt_local_event() and for full table
responses in batadv_tt_local_valid(). The goal is to stop receivers from
treating a plain ADD as a roaming advertisement.

> diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> index 3349376a9087d..39b61426f1680 100644
> --- a/net/batman-adv/translation-table.c
> +++ b/net/batman-adv/translation-table.c

[ ... ]

> @@ -3342,8 +3350,12 @@ static bool batadv_tt_local_valid(void *entry_ptr,
>  	if (tt_flags & BATADV_TT_CLIENT_NEW)
>  		return false;
>  
> +	/* the entry is (still) announced. A ROAM flag of an entry which is
> +	 * pending to be removed must not be transmitted. Otherwise, the
> +	 * receiver would handle it like a roaming advertisement
> +	 */
>  	if (flags)
> -		*flags = tt_flags;
> +		*flags = tt_flags & ~BATADV_TT_CLIENT_ROAM;
>  
>  	return true;
>  }

[Severity: Medium]

Does this fix the requester-side symptom described in the commit
message? The commit message says:

  When the requester is the node which sent the ROAMING_ADV, it then
  marks its own (new) local client as roamed back to the old originator
  and drops it when it is still NEW.

After this change, batadv_tt_local_valid() still reports an entry that
batadv_tt_local_mark_removed() marked ROAM|PENDING as valid. It only
clears the ROAM bit. So the entry still goes out as a plain ADD in the
full table response built by batadv_send_my_tt_response().

Take node B, which sent the ROAMING_ADV for client C to the old
originator A. B gets A's full table response before A's next TT commit:

batadv_tt_fill_gtable()
  batadv_tt_global_del_orig(bat_priv, orig_node, -1, "Received full table")
    /* removes B's ROAM-marked global entry for C if A was the only orig */
  _batadv_tt_update_changes()
    batadv_tt_global_add(..., flags without ROAM, ...)
      out_remove:
      batadv_tt_global_purge_local(bat_priv, tt_global_entry, flags)
        batadv_tt_local_remove(..., flags & BATADV_TT_CLIENT_ROAM) /* false */
          batadv_tt_local_mark_removed()
              if (common->flags & BATADV_TT_CLIENT_NEW)
                  break;
          batadv_tt_local_remove_now()

The NEW check in batadv_tt_local_mark_removed() does not depend on
roaming. B's newly roamed-in local entry for C is therefore still
removed right away by batadv_tt_local_remove_now(). If the entry was
already committed, a DEL is queued instead. B's global table then
points C at A.

On A, the local entry for C is ROAM|PENDING, so batadv_is_my_client()
returns false there as well. Would traffic for C then be disrupted
until C sends another frame through B and batadv_tt_local_add()
recreates the entry?

The first bullet list in the commit message also has a related
statement:

  * batadv_tt_global_purge_local() removes a local entry of the receiver

It is listed there as an effect of an ADD carrying ROAM. However,
batadv_tt_global_add() reaches out_remove and calls
batadv_tt_global_purge_local() on every successful non-multicast global
add, whether or not ROAM is set. ROAM only selects the roaming argument
passed to batadv_tt_local_remove().

Is more needed to keep B from dropping its local client in this case?
If not, should the commit message say that this patch only fixes the
ROAM marking of the global entry and the resulting CRC mismatch?

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

Re: [PATCH net 07/10] batman-adv: tt: only transmit local ROAM flag together with DEL

[Sven Eckelmann]

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

On Tuesday, 27 October 2026 09:55:22 CET netdev-bot+sashiko@kernel.org wrote:
> batman-adv: tt: only transmit local ROAM flag together with DEL
> 
> This patch stops local TT entries from sending BATADV_TT_CLIENT_ROAM
> unless it comes with BATADV_TT_CLIENT_DEL. The ROAM bit is now cleared
> for non-DEL events in __batadv_tt_local_event() and for full table
> responses in batadv_tt_local_valid(). The goal is to stop receivers from
> treating a plain ADD as a roaming advertisement.
[...]
> 
> [Severity: Medium]
> 
> Does this fix the requester-side symptom described in the commit
> message?
>
> The commit message says:
> 
>   When the requester is the node which sent the ROAMING_ADV, it then
>   marks its own (new) local client as roamed back to the old originator
>   and drops it when it is still NEW.


No, this is not the symptom. It is just the way the roaming works.

The relevant part is "But such a behavior doesn't make sense for non-ROAM_ADV 
because a new TTVN [...] should therefore be used to create a temporary (roam) 
global entry in a node." It is there to announce the "committed" local entries 
which everyone should use in their global table for this originator. And ADD 
is therefore required - but not the roaming flag because it would be 
interpreted as roaming announcement. See also 
https://www.open-mesh.org/projects/batman-adv/wiki/Client-roaming#Translation-table-consistency

There are also pending changes for TT which avoid this odd removal of the 
ROAMING and flash it out more in the receiver side [1,2,3].

Regards,
	Sven

[1] https://patchwork.open-mesh.org/project/b.a.t.m.a.n./patch/20261008-tt-avoid-unhashed-event-v2-2-7c4eed81606d@narfation.org/
[2] https://patchwork.open-mesh.org/project/b.a.t.m.a.n./patch/20261008-tt-avoid-unhashed-event-v2-3-7c4eed81606d@narfation.org/
[3] https://patchwork.open-mesh.org/project/b.a.t.m.a.n./patch/20261008-tt-avoid-unhashed-event-v2-4-7c4eed81606d@narfation.org/

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

[PATCH net 08/10] batman-adv: tt: only update flags for TT events

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

In the first implementation of the translation table events, it was only
possible to announce:

* ADD: new locally detected client
* DEL: disappeared local client

If an ADD event was queued and then a DEL event was queued in the same
timeslot, it is valid to destroy both events because nothing actually
changed for remode nodes

But with commit e0eb5f8df352 ("batman-adv: improve the TT component to
support runtime flag changes") and subsequent changes, it could also happen
that there are extra ADD events:

* ADD: _NEW_ locally detected client
* ADD: already existing client with new flags
* DEL: disappeared local client

The first and the last type of event could in theory still be removed
together from the (to be announced) events queue. But if the "update flags"
ADD is dropped together with the DEL, it would still leave global
translation table of a remote node in the incorrect state - believing that
the client didn't disappear.

If a detected client was previously announced, a DEL event must only
overwrite the "update flags" ADD but the DEL itself must still continue to
exist in the (to be anounced) events queue. For now, adjustment of the
announced flags is used for everything instead of dropping already queued
up events completely.

Similarly, a DEL followed by an ADD caused both entries to be deleted. But
the new ADD could announce flags changes which were not yet present for the
client before it was marked for DELetion. The new ADD must therefore only
overwrite the DEL - but still be kept.

Cc: stable@vger.kernel.org
Fixes: cea194d90b11 ("batman-adv: improved client announcement mechanism")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/translation-table.c | 29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 39b61426f1680..24054077cf3db 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -622,6 +622,9 @@ static u16 batadv_tt_flags_get(struct batadv_tt_common_entry *common)
  * @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
+ *
+ * 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.
  */
 static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
 				    const struct batadv_tt_common_entry *common,
@@ -631,7 +634,6 @@ static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
 	struct batadv_tt_change_node *entry;
 	struct batadv_tt_change_node *safe;
 	bool del_op_requested;
-	bool del_op_entry;
 	size_t changes;
 
 	tt_change_node = kmem_cache_alloc(batadv_tt_change_cache, GFP_ATOMIC);
@@ -665,25 +667,12 @@ static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
 		if (entry->change.vid != tt_change_node->change.vid)
 			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
-			 */
-			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
-			 */
-			entry->change.flags = flags;
-		}
+		/* 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;
 
 		kmem_cache_free(batadv_tt_change_cache, tt_change_node);
 		goto update_changes;
-- 
2.47.3

[PATCH net 09/10] batman-adv: tt: don't drop local events queued during TVLV update

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

batadv_tt_tvlv_container_update() read the number of local changes without
holding changes_list_lock. batadv_tt_local_event() is not serialized by
the commit_lock and can queue new events before the changes_list is
flushed:

  CPU0                          CPU1
  batadv_tt_tvlv_container_update()
    num = ...local_changes;
                                batadv_tt_local_event()
                                  list_add_tail(...);
                                  ...local_changes++;
    spin_lock_bh(&...changes_list_lock);
    /* only num events copied but all freed */

The additional events were freed without ever being sent. Other nodes will
only notice this via a CRC mismatch and must request the full table.

The number of changes and the changes_list must therefore be taken
atomically. Events queued afterwards then stay in the tt.changes_list for
the next TT round.

The changes can no longer be kept for the next round when the TVLV buffer
allocation fails. But they would be sent with a skipped TTVN anyway, so the
other nodes have to request the full table in this case either way.

This does not make the TTVN and CRC decisions in
batadv_tt_local_commit_changes_nolock() consistent with the announced
changeset. Events which are queued between these decisions and the
snapshot can still be announced under a TTVN or CRC which doesn't cover
them.

Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/bug/linux-fc286ddd-5bed-4683-b842-ee7a307eeb19
Fixes: a73105b8d4c7 ("batman-adv: improved client announcement mechanism")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/translation-table.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 24054077cf3db..6774484f1c573 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -1327,11 +1327,18 @@ static void batadv_tt_tvlv_container_update(struct batadv_priv *bat_priv)
 	int tt_diff_entries_num = 0;
 	bool drop_changes = false;
 	size_t tt_extra_len = 0;
+	LIST_HEAD(changes_list);
 	int tt_change_len = 0;
 	int tt_diff_len;
 	u16 tvlv_len;
 
+	/* atomically get the list of changes */
+	spin_lock_bh(&bat_priv->tt.changes_list_lock);
+	list_splice_init(&bat_priv->tt.changes_list, &changes_list);
 	tt_diff_entries_num = READ_ONCE(bat_priv->tt.local_changes);
+	WRITE_ONCE(bat_priv->tt.local_changes, 0);
+	spin_unlock_bh(&bat_priv->tt.changes_list_lock);
+
 	tt_diff_len = batadv_tt_len(tt_diff_entries_num);
 
 	/* if we have too many changes for one packet don't send any
@@ -1349,19 +1356,23 @@ static void batadv_tt_tvlv_container_update(struct batadv_priv *bat_priv)
 
 	tvlv_len = batadv_tt_prepare_tvlv_local_data(bat_priv, &tt_data,
 						     &tt_change, &tt_diff_len);
-	if (!tvlv_len)
+	if (!tvlv_len) {
+		/* the TTVN of the changes is lost and the other nodes will
+		 * have to request the full table anyway
+		 */
+		list_for_each_entry_safe(entry, safe, &changes_list, list) {
+			list_del(&entry->list);
+			kmem_cache_free(batadv_tt_change_cache, entry);
+		}
 		return;
+	}
 
 	tt_data->flags = BATADV_TT_OGM_DIFF;
 
 	if (!drop_changes && tt_diff_len == 0)
 		goto container_register;
 
-	spin_lock_bh(&bat_priv->tt.changes_list_lock);
-	WRITE_ONCE(bat_priv->tt.local_changes, 0);
-
-	list_for_each_entry_safe(entry, safe, &bat_priv->tt.changes_list,
-				 list) {
+	list_for_each_entry_safe(entry, safe, &changes_list, list) {
 		if (tt_diff_entries_count < tt_diff_entries_num) {
 			memcpy(tt_change + tt_diff_entries_count,
 			       &entry->change,
@@ -1371,7 +1382,6 @@ static void batadv_tt_tvlv_container_update(struct batadv_priv *bat_priv)
 		list_del(&entry->list);
 		kmem_cache_free(batadv_tt_change_cache, entry);
 	}
-	spin_unlock_bh(&bat_priv->tt.changes_list_lock);
 
 	tt_extra_len = batadv_tt_len(tt_diff_entries_num -
 				     tt_diff_entries_count);
-- 
2.47.3

Re: [PATCH net 09/10] batman-adv: tt: don't drop local events queued during TVLV update

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 (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] The new `!tvlv_len` error path in
  batadv_tt_tvlv_container_update() drops the snapshotted changes…
- [Low] The new allocation-failure path in
  batadv_tt_tvlv_container_update() (net/batman-adv/translation-table.c,…

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but the window
  it leaves open has worse consequences than the commit message says.

--

batman-adv: tt: don't drop local events queued during TVLV update

This patch makes batadv_tt_tvlv_container_update() splice the changes_list
and read/reset local_changes atomically under changes_list_lock. Events
queued after the snapshot then stay pending for the next TT round. If the
TVLV buffer allocation fails, the snapshotted changes are now freed instead
of kept.

Closes: https://sashiko.dev/#/bug/linux-fc286ddd-5bed-4683-b842-ee7a307eeb19

> diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> index 24054077cf3db..6774484f1c573 100644
> --- a/net/batman-adv/translation-table.c
> +++ b/net/batman-adv/translation-table.c
> @@ -1327,11 +1327,18 @@ static void batadv_tt_tvlv_container_update(struct batadv_priv *bat_priv)
>  	int tt_diff_entries_num = 0;
>  	bool drop_changes = false;
>  	size_t tt_extra_len = 0;
> +	LIST_HEAD(changes_list);
>  	int tt_change_len = 0;
>  	int tt_diff_len;
>  	u16 tvlv_len;
>
> +	/* atomically get the list of changes */
> +	spin_lock_bh(&bat_priv->tt.changes_list_lock);
> +	list_splice_init(&bat_priv->tt.changes_list, &changes_list);
>  	tt_diff_entries_num = READ_ONCE(bat_priv->tt.local_changes);
> +	WRITE_ONCE(bat_priv->tt.local_changes, 0);
> +	spin_unlock_bh(&bat_priv->tt.changes_list_lock);

[Severity: Medium]
This isn't a bug introduced by this patch, but the window left open at the
end of the commit message seems to have a worse effect than described. An
event in that window can be lost, not just announced under the wrong TTVN
or CRC.

batadv_tt_local_commit_changes_nolock() makes its decisions from a lockless
read, before this splice happens:

    if (READ_ONCE(bat_priv->tt.local_changes) == 0) {
        if (!batadv_atomic_dec_not_zero(&bat_priv->tt.ogm_append_cnt))
            batadv_tt_tvlv_container_update(bat_priv);
        return;
    }

    batadv_tt_local_transition_new(bat_priv);
    ...
    batadv_tt_local_update_crc(bat_priv);
    atomic_inc(&bat_priv->tt.vn);

__batadv_tt_local_event() only takes changes_list_lock, not commit_lock.
Consider an ADD for a new client queued through
batadv_interface_tx()->batadv_tt_local_add()->__batadv_tt_local_event()
after that check but before the splice. In the ogm_append_cnt branch, the
splice takes the event, announces it under the unchanged TTVN, and sets
local_changes to 0.

Receivers see ttvn == orig_ttvn in batadv_tt_update_orig() and ignore the
attached changes. batadv_tt_local_crc() also skips NEW entries:

    if (tt_flags & BATADV_TT_CLIENT_NEW)
        continue;

So the CRC still matches and no table request is sent.

Later rounds see local_changes == 0, so batadv_tt_local_transition_new()
never runs for this client. Could the client then stay NEW, excluded from
the CRC and from full-table replies via batadv_tt_local_valid(), until some
unrelated local TT event triggers a commit?

Would either of these close the window: taking the snapshot in
batadv_tt_local_commit_changes_nolock() before the local_changes check and
passing the list down, or not consuming the list in the non-committing
branch?

> +
>  	tt_diff_len = batadv_tt_len(tt_diff_entries_num);
>
>  	/* if we have too many changes for one packet don't send any

[ ... ]

> @@ -1349,19 +1356,23 @@ static void batadv_tt_tvlv_container_update(struct batadv_priv *bat_priv)
>
>  	tvlv_len = batadv_tt_prepare_tvlv_local_data(bat_priv, &tt_data,
>  						     &tt_change, &tt_diff_len);
> -	if (!tvlv_len)
> +	if (!tvlv_len) {
> +		/* the TTVN of the changes is lost and the other nodes will
> +		 * have to request the full table anyway
> +		 */
> +		list_for_each_entry_safe(entry, safe, &changes_list, list) {
> +			list_del(&entry->list);
> +			kmem_cache_free(batadv_tt_change_cache, entry);
> +		}
>  		return;
> +	}

[Severity: Low]
Does this statement in the commit message hold when the node is idle?

  "The changes can no longer be kept for the next round when the TVLV
  buffer allocation fails. But they would be sent with a skipped TTVN
  anyway, so the other nodes have to request the full table in this case
  either way."

When batadv_tt_prepare_tvlv_local_data() fails here, the caller
batadv_tt_local_commit_changes_nolock() has already bumped tt.vn to N,
updated the CRCs and reset ogm_append_cnt to BATADV_TT_OGM_APPEND_MAX. No
new container is registered, so OGMs keep carrying the old container (ttvn
N-1 with the old CRCs). That container is self-consistent, so peers take no
action.

local_changes is now 0, so if no further local event arrives the next
rounds take the idle branch:

    if (READ_ONCE(bat_priv->tt.local_changes) == 0) {
        if (!batadv_atomic_dec_not_zero(&bat_priv->tt.ogm_append_cnt))
            batadv_tt_tvlv_container_update(bat_priv);
        return;
    }

ttvn N is only published once ogm_append_cnt has counted down to 0, a few
OGM intervals after the failure.

Before this patch, the !tvlv_len return left the list and counter in place.
The next interval then bumped the TTVN to N+1, and peers saw the skipped
TTVN immediately.

With this change, can peers learn about the committed local changes several
OGM intervals later, with no TTVN skipped at all? Re-queueing the changes
isn't simple because __batadv_tt_local_event() expects at most one queued
change per (addr, vid). Would it be enough to force a TTVN bump or commit in
the next round instead?

[Severity: Medium]
Can this error path leave a stale bat_priv->tt.last_changeset that is later
served under the new TTVN?

Suppose round N-1 left last_changeset = D1, and new events D2 are committed
with tt.vn bumped to N. If batadv_tt_prepare_tvlv_local_data() fails here,
D2 is freed and local_changes is already 0. last_changeset still holds D1.

When ogm_append_cnt reaches 0, the idle branch calls
batadv_tt_tvlv_container_update() again with an empty snapshot.
tt_diff_len is 0, so it takes:

    if (!drop_changes && tt_diff_len == 0)
        goto container_register;

That skips the last_changeset refresh, and the container is registered with
ttvn N and the new CRCs.

A peer at orig_ttvn N-1 then sees ttvn - orig_ttvn == 1 with no changes in
batadv_tt_update_orig(). It sends a diff request, not a full table request.

In batadv_send_my_tt_response():

    if (tt_data->flags & BATADV_TT_FULL_TABLE || my_ttvn != req_ttvn ||
        !bat_priv->tt.last_changeset)
        full_table = true;
    else
        full_table = false;

my_ttvn == req_ttvn and last_changeset is non-NULL, so the reply is the old
D1 diff labelled ttvn N. The peer applies it through
batadv_tt_update_changes() and only fetches the full table after a later
CRC mismatch.

The new comment says "the other nodes will have to request the full table
anyway", but here the first request is a diff request. Should this path
free last_changeset and set it to NULL under last_changeset_lock, or force
the next round to bump the TTVN?

>
>  	tt_data->flags = BATADV_TT_OGM_DIFF;
>
>  	if (!drop_changes && tt_diff_len == 0)
>  		goto container_register;

[ ... ]

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

Re: [PATCH net 09/10] batman-adv: tt: don't drop local events queued during TVLV update

[Sven Eckelmann]

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

On Tuesday, 27 October 2026 09:55:23 CET netdev-bot+sashiko@kernel.org wrote:
[...]
> > diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> > index 24054077cf3db..6774484f1c573 100644
> > --- a/net/batman-adv/translation-table.c
> > +++ b/net/batman-adv/translation-table.c
> > @@ -1327,11 +1327,18 @@ static void batadv_tt_tvlv_container_update(struct batadv_priv *bat_priv)
> >  	int tt_diff_entries_num = 0;
> >  	bool drop_changes = false;
> >  	size_t tt_extra_len = 0;
> > +	LIST_HEAD(changes_list);
> >  	int tt_change_len = 0;
> >  	int tt_diff_len;
> >  	u16 tvlv_len;
> >
> > +	/* atomically get the list of changes */
> > +	spin_lock_bh(&bat_priv->tt.changes_list_lock);
> > +	list_splice_init(&bat_priv->tt.changes_list, &changes_list);
> >  	tt_diff_entries_num = READ_ONCE(bat_priv->tt.local_changes);
> > +	WRITE_ONCE(bat_priv->tt.local_changes, 0);
> > +	spin_unlock_bh(&bat_priv->tt.changes_list_lock);
> 
> [Severity: Medium]
> This isn't a bug introduced by this patch, but the window left open at the
> end of the commit message seems to have a worse effect than described. An
> event in that window can be lost, not just announced under the wrong TTVN
> or CRC.
> 
> batadv_tt_local_commit_changes_nolock() makes its decisions from a lockless
> read, before this splice happens:
[...]
> Would either of these close the window: taking the snapshot in
> batadv_tt_local_commit_changes_nolock() before the local_changes check and
> passing the list down, or not consuming the list in the non-committing
> branch?

Out-of-scope for this patch and handled in another pending patch

https://patchwork.open-mesh.org/project/b.a.t.m.a.n./patch/20261025-tt-tvlv-changes-check-v1-1-c12b246c0956@narfation.org/

> [ ... ]
> 
> > @@ -1349,19 +1356,23 @@ static void batadv_tt_tvlv_container_update(struct batadv_priv *bat_priv)
> >
> >  	tvlv_len = batadv_tt_prepare_tvlv_local_data(bat_priv, &tt_data,
> >  						     &tt_change, &tt_diff_len);
> > -	if (!tvlv_len)
> > +	if (!tvlv_len) {
> > +		/* the TTVN of the changes is lost and the other nodes will
> > +		 * have to request the full table anyway
> > +		 */
> > +		list_for_each_entry_safe(entry, safe, &changes_list, list) {
> > +			list_del(&entry->list);
> > +			kmem_cache_free(batadv_tt_change_cache, entry);
> > +		}
> >  		return;
> > +	}
> 
> [Severity: Low]
> Does this statement in the commit message hold when the node is idle?
> 
>   "The changes can no longer be kept for the next round when the TVLV
>   buffer allocation fails. But they would be sent with a skipped TTVN
>   anyway, so the other nodes have to request the full table in this case
>   either way."
> 
> When batadv_tt_prepare_tvlv_local_data() fails here, the caller
> batadv_tt_local_commit_changes_nolock() has already bumped tt.vn to N,
> updated the CRCs and reset ogm_append_cnt to BATADV_TT_OGM_APPEND_MAX. No
> new container is registered, so OGMs keep carrying the old container (ttvn
> N-1 with the old CRCs).
[...]
> Before this patch, the !tvlv_len return left the list and counter in place.
> The next interval then bumped the TTVN to N+1, and peers saw the skipped
> TTVN immediately.

Why would the first round fail and the second round not? Please don't start 
with kmalloc-failures - this would open a completely different can of worms 
related to containers and OGMs.

> With this change, can peers learn about the committed local changes several
> OGM intervals later, with no TTVN skipped at all? Re-queueing the changes
> isn't simple because __batadv_tt_local_event() expects at most one queued
> change per (addr, vid). Would it be enough to force a TTVN bump or commit in
> the next round instead?

What is proposed here?

        if (!tvlv_len) {
                /* the changes are lost and the other nodes have to request the
                 * full table. Announce the new TTVN in the next OGM interval
                 * and don't answer TT requests for it with the stale changeset
                 * of the previous TTVN
                 */
                list_for_each_entry_safe(entry, safe, &changes_list, list) {
                        list_del(&entry->list);
                        kmem_cache_free(batadv_tt_change_cache, entry);
                }

                spin_lock_bh(&bat_priv->tt.last_changeset_lock);
                kfree(bat_priv->tt.last_changeset);
                bat_priv->tt.last_changeset = NULL;
                bat_priv->tt.last_changeset_len = 0;
                spin_unlock_bh(&bat_priv->tt.last_changeset_lock);

                /* announce the already committed TTVN in the next OGM interval
                 * instead of waiting for BATADV_TT_OGM_APPEND_MAX idle rounds
                 */
                atomic_set(&bat_priv->tt.ogm_append_cnt, 0);
                return;
        }

https://www.open-mesh.org/issues/485
https://patchwork.open-mesh.org/project/b.a.t.m.a.n./patch/20261027-tt-tvlv-alloc-fail-v1-1-02058bcc4595@narfation.org/

> 
> [Severity: Medium]
> Can this error path leave a stale bat_priv->tt.last_changeset that is later
> served under the new TTVN?

Which it did even before the patch

[...]
> The new comment says "the other nodes will have to request the full table
> anyway", but here the first request is a diff request.

Which makes the statements untrue because of "first request"? Which is not
even mentioned in the comment.

> Should this path
> free last_changeset and set it to NULL under last_changeset_lock, or force
> the next round to bump the TTVN?

https://www.open-mesh.org/issues/485

Regards,
	Sven

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

[PATCH net 10/10] batman-adv: tt: limit local table to single TVLV response size

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

The reservation of local TT entries and VLANs only limits the local table
by (int) packet_size_max. But the full table response is sent as a single
unicast TVLV packet whose tvlv_len (covering the TVLV header and the TT
TVLV) is only 16 bit wide. The reservation must therefore be capped at the
minimum of both the packet_size_max and the maximum theoretic table
response.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930094558.3723766-2-sw%40simonwunderlich.de?part=8
Fixes: 85fb027f611b ("batman-adv: tt: reject VLAN/TT entries before reaching size limit")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/translation-table.c | 69 ++++++++++++++++++++----------
 1 file changed, 47 insertions(+), 22 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 6774484f1c573..6931ce88c3062 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -26,8 +26,10 @@
 #include <linux/jhash.h>
 #include <linux/jiffies.h>
 #include <linux/kref.h>
+#include <linux/limits.h>
 #include <linux/list.h>
 #include <linux/lockdep.h>
+#include <linux/minmax.h>
 #include <linux/net.h>
 #include <linux/netdevice.h>
 #include <linux/netlink.h>
@@ -253,7 +255,8 @@ static int batadv_tt_len(int changes_num)
  *
  * Return: local translation table size in bytes.
  */
-static int batadv_tt_local_transmit_size(u16 num_vlan, u16 num_entries)
+static int batadv_tt_local_transmit_size(unsigned int num_vlan,
+					 unsigned int num_entries)
 {
 	int hdr_size;
 
@@ -266,6 +269,25 @@ static int batadv_tt_local_transmit_size(u16 num_vlan, u16 num_entries)
 	return hdr_size + batadv_tt_len(num_entries);
 }
 
+/**
+ * batadv_tt_local_transmit_size_max() - get the maximum size of a full local table
+ *  response
+ * @bat_priv: the bat priv with all the mesh interface information
+ *
+ * The full table response is sent as a single unicast TVLV packet. Its
+ * tvlv_len, which covers the TVLV header and the TT TVLV, is only 16 bit wide.
+ * This limits the response size even when packet_size_max is larger.
+ *
+ * Return: maximum size of a full local table response in bytes
+ */
+static int batadv_tt_local_transmit_size_max(struct batadv_priv *bat_priv)
+{
+	int tvlv_size_max = sizeof(struct batadv_unicast_tvlv_packet) + U16_MAX;
+	int packet_size_max = READ_ONCE(bat_priv->packet_size_max);
+
+	return min(packet_size_max, tvlv_size_max);
+}
+
 /**
  * batadv_tt_local_reserve() - reserve room in the transmittable local table
  * @bat_priv: the bat priv with all the mesh interface information
@@ -290,22 +312,23 @@ static int batadv_tt_local_transmit_size(u16 num_vlan, u16 num_entries)
 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);
+	int size_max = batadv_tt_local_transmit_size_max(bat_priv);
+	unsigned int entries;
+	unsigned int vlans;
 
 	scoped_guard(spinlock_bh, &bat_priv->tt.reserve_lock) {
-		bat_priv->tt.reserved_vlans += num_vlan;
-		bat_priv->tt.reserved_entries += num_entries;
+		vlans = bat_priv->tt.reserved_vlans + num_vlan;
+		entries = 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;
+		*table_size = batadv_tt_local_transmit_size(vlans, entries);
+		if (*table_size > size_max)
+			return false;
 
-		bat_priv->tt.reserved_vlans -= num_vlan;
-		bat_priv->tt.reserved_entries -= num_entries;
+		bat_priv->tt.reserved_vlans = vlans;
+		bat_priv->tt.reserved_entries = entries;
 	}
 
-	return false;
+	return true;
 }
 
 /**
@@ -339,9 +362,10 @@ static bool batadv_tt_local_reserve_entry(struct batadv_priv *bat_priv,
 		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",
+				 "Local translation table size (%i) exceeds maximum table response size (%i); Ignoring new local tt entry: %pM\n",
 				 table_size,
-				 READ_ONCE(bat_priv->packet_size_max), addr);
+				 batadv_tt_local_transmit_size_max(bat_priv),
+				 addr);
 
 	return false;
 }
@@ -374,8 +398,9 @@ bool batadv_tt_local_reserve_vlan(struct batadv_priv *bat_priv,
 		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),
+				 "Local translation table size (%i) exceeds maximum table response size (%i); Ignoring new VLAN: %d\n",
+				 table_size,
+				 batadv_tt_local_transmit_size_max(bat_priv),
 				 batadv_print_vid(vid));
 
 	return false;
@@ -724,9 +749,9 @@ static u16 batadv_tt_entries(u16 tt_len)
  */
 static int batadv_tt_local_table_transmit_size(struct batadv_priv *bat_priv)
 {
+	unsigned int tt_local_entries = 0;
 	struct batadv_meshif_vlan *vlan;
-	u16 tt_local_entries = 0;
-	u16 num_vlan = 0;
+	unsigned int num_vlan = 0;
 
 	rcu_read_lock();
 	hlist_for_each_entry_rcu(vlan, &bat_priv->meshif_vlan_list, list) {
@@ -4686,16 +4711,16 @@ void batadv_tt_local_resize_to_mtu(struct net_device *mesh_iface)
 	struct batadv_priv *bat_priv = netdev_priv(mesh_iface);
 	int timeout = BATADV_TT_LOCAL_TIMEOUT / 2;
 	bool reduced = false;
-	int packet_size_max;
 	int table_size;
+	int size_max;
 
-	packet_size_max = READ_ONCE(bat_priv->packet_size_max);
+	size_max = batadv_tt_local_transmit_size_max(bat_priv);
 
 	spin_lock_bh(&bat_priv->tt.commit_lock);
 
 	while (timeout) {
 		table_size = batadv_tt_local_table_transmit_size(bat_priv);
-		if (packet_size_max >= table_size)
+		if (size_max >= table_size)
 			break;
 
 		batadv_tt_local_purge(bat_priv, timeout);
@@ -4704,8 +4729,8 @@ void batadv_tt_local_resize_to_mtu(struct net_device *mesh_iface)
 		timeout /= 2;
 		reduced = true;
 		net_ratelimited_function(batadv_info, mesh_iface,
-					 "Forced to purge local tt entries to fit new maximum fragment MTU (%i)\n",
-					 packet_size_max);
+					 "Forced to purge local tt entries to fit new maximum table response size (%i)\n",
+					 size_max);
 	}
 
 	/* commit these changes immediately, to avoid synchronization problem
-- 
2.47.3

Re: [PATCH net 10/10] batman-adv: tt: limit local table to single TVLV response size

[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 Fixes: tag points at 85fb027f611b ("batman-adv: tt: reject
  VLAN/TT entries before reaching size limit"), but that commit did not…

--

batman-adv: tt: limit local table to single TVLV response size

This patch caps the reservation of local TT entries and VLANs at the
smaller of packet_size_max and the largest full table response that fits
in one unicast TVLV packet, since tvlv_len is only 16 bits wide. A new
helper, batadv_tt_local_transmit_size_max(), computes that limit. The
helper is used in batadv_tt_local_reserve(), in the related log messages
and in batadv_tt_local_resize_to_mtu().

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

> Fixes: 85fb027f611b ("batman-adv: tt: reject VLAN/TT entries before reaching size limit")

[Severity: Low]

Does this Fixes: tag point at the commit that introduced the problem?

85fb027f611b removed a check from batadv_tt_local_create() that already
limited the table by packet_size_max only:

-	/* 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) {

That older check had no cap at the 16-bit tvlv_len either. So the missing
limit seems to predate 85fb027f611b.

Blame on the parent of 85fb027f611b points at defa87f5ce75fe
("batman-adv: tt: extract allocation of new local entries"). That commit
only moved this code, so the check is older still. The original commit
was not identified.

Kernels without 85fb027f611b can still let the local table grow past what
one unicast TVLV full table response can carry. This happens with
fragmentation disabled and a hard interface MTU above
sizeof(struct batadv_unicast_tvlv_packet) + U16_MAX. In that case,
batadv_tt_prepare_tvlv_local_data() hits its check_add_overflow() path and
returns 0, so no response is sent.

With the current tag, stable tooling would only pick this fix for trees
that contain 85fb027f611b. Could the tag name the commit that first added
the packet_size_max-only check? Or could the commit message say that
older trees are affected too?

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

Re: [PATCH net 10/10] batman-adv: tt: limit local table to single TVLV response size

[Sven Eckelmann]

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

On Tuesday, 27 October 2026 09:55:24 CET netdev-bot+sashiko@kernel.org wrote:
> [Severity: Low]
> 
> Does this Fixes: tag point at the commit that introduced the problem?
> 
> 85fb027f611b removed a check from batadv_tt_local_create() that already
> limited the table by packet_size_max only:
> 
> -       /* 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) {
> 
> That older check had no cap at the 16-bit tvlv_len either. So the missing
> limit seems to predate 85fb027f611b.
> 
> Blame on the parent of 85fb027f611b points at defa87f5ce75fe
> ("batman-adv: tt: extract allocation of new local entries"). That commit
> only moved this code, so the check is older still. The original commit
> was not identified.
> 
> Kernels without 85fb027f611b can still let the local table grow past what
> one unicast TVLV full table response can carry. This happens with
> fragmentation disabled and a hard interface MTU above
> sizeof(struct batadv_unicast_tvlv_packet) + U16_MAX. In that case,
> batadv_tt_prepare_tvlv_local_data() hits its check_add_overflow() path and
> returns 0, so no response is sent.
> 
> With the current tag, stable tooling would only pick this fix for trees
> that contain 85fb027f611b. Could the tag name the commit that first added
> the packet_size_max-only check? Or could the commit message say that
> older trees are affected too?
> 

The code only does it job partially after this commit. All of the previous 
commits would need a big revamp of the TT code - which is not backport-able 
via stable.

The really relevant part is in batadv_tt_local_reserve() and this was only 
introduced with this specific commit. And only for this commit, Sashiko 
pointed this out.

Regards,
	Sven

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