[PATCH net-next 00/15] pull request for net-next: batman-adv 2026-08-31

[PATCH net-next 00/15] pull request for net-next: batman-adv 2026-08-31

[Simon Wunderlich]

Dear net maintainers,

here is 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 1b78070aaef63512688aebfbc82365ef9d6660f1:

  Merge tag 'net-7.3-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net (2026-08-27 13:53:43 -0700)

are available in the Git repository at:

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

for you to fetch changes up to 799152bcf6cbe4393eb03690925a382cefae303c:

  batman-adv: tt: decrement count for committed client on local_remove (2026-08-30 20:05:57 +0200)

----------------------------------------------------------------
This cleanup patchset includes the following patches, all by Sven Eckelmann:

- dat: fix printing of unknown 4addr subtype

- drop direction in _batadv_is_ap_isolated kernel-doc

- bat_v: fix bonding candidate selection

- clarify cut-off in batadv_v_neigh_is_sob kernel-doc

- use more descriptive var names for is_similar_or_better

- ensure u16 aligned mac addresses in structs and arrays (2 patches)

- translation table (tt) fixes/cleanups, 8 patches:

  - remove only the entry which was looked up from the hash
  - extract code handling a roam on add
  - simplify NEW flag transition code
  - drop unnecessary cleanup goto in helpers
  - use protected flag modifications
  - transition NEW local entries only under lock
  - don't uncount never committed clients on pending purge
  - decrement count for committed client on local_remove

----------------------------------------------------------------
Sven Eckelmann (15):
      batman-adv: dat: fix printing of unknown 4addr subtype
      batman-adv: drop direction in _batadv_is_ap_isolated kernel-doc
      batman-adv: bat_v: fix bonding candidate selection
      batman-adv: clarify cut-off in batadv_v_neigh_is_sob kernel-doc
      batman-adv: use more descriptive var names for is_similar_or_better
      batman-adv: ensure u16 aligned mac address arrays on stack
      batman-adv: ensure u16 aligned mac address in structs
      batman-adv: tt: remove only the entry which was looked up from the hash
      batman-adv: tt: extract code handling a roam on add
      batman-adv: tt: simplify NEW flag transition code
      batman-adv: tt: drop unnecessary cleanup goto in helpers
      batman-adv: tt: use protected flag modifications
      batman-adv: tt: transition NEW local entries only under lock
      batman-adv: tt: don't uncount never committed clients on pending purge
      batman-adv: tt: decrement count for committed client on local_remove

 net/batman-adv/bat_iv_ogm.c            |  25 +-
 net/batman-adv/bat_v.c                 |  42 +--
 net/batman-adv/bridge_loop_avoidance.c |   6 +-
 net/batman-adv/distributed-arp-table.c |  10 +-
 net/batman-adv/mesh-interface.c        |  12 +-
 net/batman-adv/multicast.c             |   6 +-
 net/batman-adv/translation-table.c     | 503 ++++++++++++++++++++++-----------
 net/batman-adv/types.h                 |  49 ++--
 8 files changed, 425 insertions(+), 228 deletions(-)

[PATCH net-next 01/15] batman-adv: dat: fix printing of unknown 4addr subtype

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

If an unknown subtype of an unicast 4addr packets was detected in
batadv_dbg_arp(), it is printing the (known) packet_type instead of the
subtype. This must be switched to the subtype to have any kind of use for
the reader of the debug message.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/distributed-arp-table.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c
index 011cfdc47fab4..270f8d6e29545 100644
--- a/net/batman-adv/distributed-arp-table.c
+++ b/net/batman-adv/distributed-arp-table.c
@@ -548,7 +548,7 @@ static void batadv_dbg_arp(struct batadv_priv *bat_priv, struct sk_buff *skb,
 			break;
 		default:
 			batadv_dbg(BATADV_DBG_DAT, bat_priv, "* type: Unknown (%u)!\n",
-				   unicast_4addr_packet->u.packet_type);
+				   unicast_4addr_packet->subtype);
 		}
 		break;
 	case BATADV_BCAST:
-- 
2.47.3

Re: [PATCH net-next 01/15] batman-adv: dat: fix printing of unknown 4addr subtype

[Sven Eckelmann]

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

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


> --- Patch [1]: [PATCH net-next 01/15] batman-adv: dat: fix printing of unknown 4addr subtype ---
> commit 974c511b32b67ede89d952e4508493380b181449
> Author: Sven Eckelmann <sven@narfation.org>
> 
> batman-adv: dat: fix printing of unknown 4addr subtype
> 
> The default arm of the subtype switch in batadv_dbg_arp() was printing
> unicast_4addr_packet->u.packet_type instead of the subtype that was not
> recognized. This changes the debug message to print the subtype value.
> 
> > diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c
> > index 011cfdc47fab..270f8d6e2954 100644
> > --- a/net/batman-adv/distributed-arp-table.c
> > +++ b/net/batman-adv/distributed-arp-table.c
> > @@ -548,7 +548,7 @@ static void batadv_dbg_arp(struct batadv_priv *bat_priv, struct sk_buff *skb,
> >  			break;
> >  		default:
> >  			batadv_dbg(BATADV_DBG_DAT, bat_priv, "* type: Unknown (%u)!\n",
> > -				   unicast_4addr_packet->u.packet_type);
> > +				   unicast_4addr_packet->subtype);
> 
> Discovered by: gpt-5-6-sol, opus-5 · Missed by: sashiko-gemini
> Should this carry a Fixes: tag?
> 
> The change corrects a real defect in batadv_dbg_arp(): the default arm of
> the switch (unicast_4addr_packet->subtype) block was printing the outer
> packet_type, which on that path is always BATADV_UNICAST_4ADDR, so the
> message never showed the unknown value.
[...]
> Would adding
> 
>   Fixes: 5c3a0e553593 ("batman-adv: Distributed ARP Table - add ARP parsing functions")
> 
> be appropriate here, so netdev and stable tooling can date the affected
> history? The code change itself looks correct; only the missing tag stands
> out.

I am not allowed to add Fixes: lines for patches targeting net-next. See
https://lore.kernel.org/all/7d73cd74-b040-4a3d-9d78-4ad8fc0a01f4@redhat.com/

Regards,
Sven

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

[PATCH net-next 02/15] batman-adv: drop direction in _batadv_is_ap_isolated kernel-doc

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

The _batadv_is_ap_isolated() is called by

* batadv_transtable_search()
  - tt_local_entry is the source
  - tt_global_entry is the destination
* batadv_is_ap_isolated
  - tt_local_entry is the destination
  - tt_global_entry is the source

It is therefore incorrect to document the tt_local_entry as sending client
and tt_global_entry as the destination client.

Reported-by: Sashiko <sashiko-bot@kernel.org>
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 68f79853032ed..88aeefa97db6e 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -2437,8 +2437,8 @@ static void batadv_tt_global_table_free(struct batadv_priv *bat_priv)
 /**
  * _batadv_is_ap_isolated() - check whether two clients are AP-isolated from
  *  each other
- * @tt_local_entry: local TT entry of the sending client
- * @tt_global_entry: global TT entry of the destination client
+ * @tt_local_entry: local TT entry of one local client
+ * @tt_global_entry: global TT entry of the remote client
  *
  * Return: true if traffic between the two clients should be dropped because
  *  either both are WiFi clients or both carry the ISOLATION flag; false
-- 
2.47.3

[PATCH net-next 03/15] batman-adv: bat_v: fix bonding candidate selection

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

batadv_v_neigh_is_sob() implements the metric (throughput) similarity check
as:

  neigh2 (the reference) has a better throughput than 3/4 of the neigh1
  (candidate) throughput

But this results in a selection of bonding candidates in
batadv_find_router() which have a really low throughput. But the goal must
be to select candidates which have a good throughput.

The neigh1/neigh2 operands must be swapped in the comparison to use the
reference as such and select only neighbor+interface combinations with good
throughput.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/bat_v.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/batman-adv/bat_v.c b/net/batman-adv/bat_v.c
index 0c27447cf6889..2d3045733db1d 100644
--- a/net/batman-adv/bat_v.c
+++ b/net/batman-adv/bat_v.c
@@ -519,8 +519,8 @@ static int batadv_v_neigh_cmp(struct batadv_neigh_node *neigh1,
  * @neigh2: second neighbour to compare
  * @if_outgoing2: outgoing interface to use for @neigh2
  *
- * Return: true if the throughput of @neigh2 is at least 3/4 of the
- *  @neigh1 throughput
+ * Return: true if the throughput of @neigh1 is at least 3/4 of the
+ *  @neigh2 throughput
  */
 static bool batadv_v_neigh_is_sob(struct batadv_neigh_node *neigh1,
 				  struct batadv_hard_iface *if_outgoing1,
@@ -540,10 +540,10 @@ static bool batadv_v_neigh_is_sob(struct batadv_neigh_node *neigh1,
 	if (!ifinfo2)
 		goto err_ifinfo2;
 
-	threshold = ifinfo1->bat_v.throughput / 4;
-	threshold = ifinfo1->bat_v.throughput - threshold;
+	threshold = ifinfo2->bat_v.throughput / 4;
+	threshold = ifinfo2->bat_v.throughput - threshold;
 
-	ret = ifinfo2->bat_v.throughput > threshold;
+	ret = ifinfo1->bat_v.throughput > threshold;
 
 	batadv_neigh_ifinfo_put(ifinfo2);
 err_ifinfo2:
-- 
2.47.3

Re: [PATCH net-next 03/15] batman-adv: bat_v: fix bonding candidate selection

[Sven Eckelmann]

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

On Tuesday, 1 September 2026 15:59:58 CEST sashiko-bot@kernel.org wrote:
> [Severity: High]
> This isn't a bug introduced by this patch, but does the throughput comparison
> in batadv_v_neigh_cmp() overflow?
> 
> In batadv_v_neigh_cmp(), we have:
> 
>     ret = ifinfo1->bat_v.throughput - ifinfo2->bat_v.throughput;
> 
> If an interface is provisioned with a throughput override via netlink that
> exceeds the signed 32-bit maximum (2,147,483,647), subtracting a low or zero
> throughput interface from it causes this unsigned subtraction to overflow.
> 
> The result is implicitly cast to a signed 32-bit integer, turning it negative.
> 
> Will returning a negative value here incorrectly signal to callers that the
> optimal high-throughput path is worse, causing the mesh routing to prefer
> suboptimal or zero-throughput paths instead?

Out-of-scope for this patch. It isn't even touched in this patch.

Document it for interested contributors in 
https://www.open-mesh.org/issues/470

Regards,
	Sven

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

Re: [PATCH net-next 03/15] batman-adv: bat_v: fix bonding candidate selection

[Sven Eckelmann]

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

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

> > diff --git a/net/batman-adv/bat_v.c b/net/batman-adv/bat_v.c
> > index 0c27447cf688..2d3045733db1 100644
> > --- a/net/batman-adv/bat_v.c
> > +++ b/net/batman-adv/bat_v.c
> > @@ -519,8 +519,8 @@ static int batadv_v_neigh_cmp(struct batadv_neigh_node *neigh1,
> >   * @neigh2: second neighbour to compare
> >   * @if_outgoing2: outgoing interface to use for @neigh2
> >   *
> > - * Return: true if the throughput of @neigh2 is at least 3/4 of the
> > - *  @neigh1 throughput
> > + * Return: true if the throughput of @neigh1 is at least 3/4 of the
> > + *  @neigh2 throughput
> >   */
> 
> Discovered by: gpt-5-6-sol, opus-5 · Missed by: sashiko-gemini
> This isn't an issue introduced by this patch, the "at least" wording was
> already there before the swap, but does the rewritten kernel-doc for
> batadv_v_neigh_is_sob() still describe the boundary case incorrectly?
> 
> The implementation uses a strict comparison:
> 
> net/batman-adv/bat_v.c:batadv_v_neigh_is_sob() {
> 	threshold = ifinfo2->bat_v.throughput / 4;
> 	threshold = ifinfo2->bat_v.throughput - threshold;
> 
> 	ret = ifinfo1->bat_v.throughput > threshold;
> }
> 
> So a candidate whose throughput is exactly tp2 - tp2/4 returns false, as
> does the tp1 == tp2 == 0 case, while "at least 3/4" reads as inclusive.

Out-of-scope for this patch and adjusted in a later patch.

> 
> The remaining inclusive wording is the pre-existing ops-table description
> in net/batman-adv/types.h, which still documents
> batadv_algo_neigh_ops::is_similar_or_better as "check if neigh1 is equally
> similar or better than neigh2"; that text is untouched by this series.
> Should it be reworded too?

Out-of-scope for this patch.

> 
> > @@ -540,10 +540,10 @@ static bool batadv_v_neigh_is_sob(struct batadv_neigh_node *neigh1,
> >  	if (!ifinfo2)
> >  		goto err_ifinfo2;
> >  
> > -	threshold = ifinfo1->bat_v.throughput / 4;
> > -	threshold = ifinfo1->bat_v.throughput - threshold;
> > +	threshold = ifinfo2->bat_v.throughput / 4;
> > +	threshold = ifinfo2->bat_v.throughput - threshold;
> >  
> > -	ret = ifinfo2->bat_v.throughput > threshold;
> > +	ret = ifinfo1->bat_v.throughput > threshold;
> 
> Discovered by: gpt-5-6-sol, opus-5 · Missed by: sashiko-gemini
> Should this carry a Fixes: tag and a stable indication?
[...]
> That means every released kernel containing 9786906022eb keeps accepting
> bonding candidates with arbitrarily low throughput whenever
> bat_priv->bonding is enabled.  Without a Fixes: tag the change is not
> visible to stable/AUTOSEL tooling.


I am not allowed to add Fixes: lines for patches targeting net-next. See
https://lore.kernel.org/all/7d73cd74-b040-4a3d-9d78-4ad8fc0a01f4@redhat.com/

Regards,
Sven

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

[PATCH net-next 04/15] batman-adv: clarify cut-off in batadv_v_neigh_is_sob kernel-doc

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

The neigh1 throughput must be more than 3/4 of the neigh2 throughput. Just
3/4 is not enough to return true.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/bat_v.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/batman-adv/bat_v.c b/net/batman-adv/bat_v.c
index 2d3045733db1d..0d0a226c3fd0c 100644
--- a/net/batman-adv/bat_v.c
+++ b/net/batman-adv/bat_v.c
@@ -519,7 +519,7 @@ static int batadv_v_neigh_cmp(struct batadv_neigh_node *neigh1,
  * @neigh2: second neighbour to compare
  * @if_outgoing2: outgoing interface to use for @neigh2
  *
- * Return: true if the throughput of @neigh1 is at least 3/4 of the
+ * Return: true if the throughput of @neigh1 is more than 3/4 of the
  *  @neigh2 throughput
  */
 static bool batadv_v_neigh_is_sob(struct batadv_neigh_node *neigh1,
-- 
2.47.3

[PATCH net-next 05/15] batman-adv: use more descriptive var names for is_similar_or_better

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

neigh1 and neigh2 is not really helpful when looking at a code which needs
to judge the metric similarity between a candidate and a reference. This
already caused an implementation error in the B.A.T.M.A.N. V bonding code.
Simply using "reference" and "candidate" for the neighbors is a lot more
descriptive and makes it easier to understand the code.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/bat_iv_ogm.c | 25 +++++++++++-----------
 net/batman-adv/bat_v.c      | 42 ++++++++++++++++++-------------------
 net/batman-adv/types.h      | 12 +++++------
 3 files changed, 40 insertions(+), 39 deletions(-)

diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 53fbdbbe8f4f7..3fe09b4c30463 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -2424,25 +2424,26 @@ static int batadv_iv_ogm_neigh_cmp(struct batadv_neigh_node *neigh1,
 /**
  * batadv_iv_ogm_neigh_is_sob() - check if neigh1 is similarly good or better
  *  than neigh2 from the metric prospective
- * @neigh1: the first neighbor object of the comparison
- * @if_outgoing1: outgoing interface for the first neighbor
- * @neigh2: the second neighbor object of the comparison
- * @if_outgoing2: outgoing interface for the second neighbor
+ * @candidate: the first neighbor object of the comparison
+ * @if_outgoing_cand: outgoing interface for the @candidate neighbor
+ * @reference: the second neighbor object of the comparison
+ * @if_outgoing_ref: outgoing interface for the @reference neighbor
  *
- * Return: true if the metric via neigh1 is equally good or better than
- * the metric via neigh2, false otherwise.
+ * Return: true if the metric via @candidate is equally good or better than
+ * the metric via @reference, false otherwise.
  */
 static bool
-batadv_iv_ogm_neigh_is_sob(struct batadv_neigh_node *neigh1,
-			   struct batadv_hard_iface *if_outgoing1,
-			   struct batadv_neigh_node *neigh2,
-			   struct batadv_hard_iface *if_outgoing2)
+batadv_iv_ogm_neigh_is_sob(struct batadv_neigh_node *candidate,
+			   struct batadv_hard_iface *if_outgoing_cand,
+			   struct batadv_neigh_node *reference,
+			   struct batadv_hard_iface *if_outgoing_ref)
 {
 	bool ret;
 	int diff;
 
-	ret = batadv_iv_ogm_neigh_diff(neigh1, if_outgoing1, neigh2,
-				       if_outgoing2, &diff);
+	ret = batadv_iv_ogm_neigh_diff(candidate, if_outgoing_cand,
+				       reference, if_outgoing_ref,
+				       &diff);
 	if (!ret)
 		return false;
 
diff --git a/net/batman-adv/bat_v.c b/net/batman-adv/bat_v.c
index 0d0a226c3fd0c..596889b3522d0 100644
--- a/net/batman-adv/bat_v.c
+++ b/net/batman-adv/bat_v.c
@@ -514,40 +514,40 @@ static int batadv_v_neigh_cmp(struct batadv_neigh_node *neigh1,
 /**
  * batadv_v_neigh_is_sob() - check whether two B.A.T.M.A.N. V neighbours have
  *  a similar or better throughput
- * @neigh1: first neighbour to compare
- * @if_outgoing1: outgoing interface to use for @neigh1
- * @neigh2: second neighbour to compare
- * @if_outgoing2: outgoing interface to use for @neigh2
+ * @candidate: the first neighbor object of the comparison
+ * @if_outgoing_cand: outgoing interface for the @candidate neighbor
+ * @reference: the second neighbor object of the comparison
+ * @if_outgoing_ref: outgoing interface for the @reference neighbor
  *
- * Return: true if the throughput of @neigh1 is more than 3/4 of the
- *  @neigh2 throughput
+ * Return: true if the throughput of @candidate is more than 3/4 of the
+ *  @reference throughput
  */
-static bool batadv_v_neigh_is_sob(struct batadv_neigh_node *neigh1,
-				  struct batadv_hard_iface *if_outgoing1,
-				  struct batadv_neigh_node *neigh2,
-				  struct batadv_hard_iface *if_outgoing2)
+static bool batadv_v_neigh_is_sob(struct batadv_neigh_node *candidate,
+				  struct batadv_hard_iface *if_outgoing_cand,
+				  struct batadv_neigh_node *reference,
+				  struct batadv_hard_iface *if_outgoing_ref)
 {
-	struct batadv_neigh_ifinfo *ifinfo1;
-	struct batadv_neigh_ifinfo *ifinfo2;
+	struct batadv_neigh_ifinfo *ifinfo_cand;
+	struct batadv_neigh_ifinfo *ifinfo_ref;
 	bool ret = false;
 	u32 threshold;
 
-	ifinfo1 = batadv_neigh_ifinfo_get(neigh1, if_outgoing1);
-	if (!ifinfo1)
+	ifinfo_cand = batadv_neigh_ifinfo_get(candidate, if_outgoing_cand);
+	if (!ifinfo_cand)
 		goto err_ifinfo1;
 
-	ifinfo2 = batadv_neigh_ifinfo_get(neigh2, if_outgoing2);
-	if (!ifinfo2)
+	ifinfo_ref = batadv_neigh_ifinfo_get(reference, if_outgoing_ref);
+	if (!ifinfo_ref)
 		goto err_ifinfo2;
 
-	threshold = ifinfo2->bat_v.throughput / 4;
-	threshold = ifinfo2->bat_v.throughput - threshold;
+	threshold = ifinfo_ref->bat_v.throughput / 4;
+	threshold = ifinfo_ref->bat_v.throughput - threshold;
 
-	ret = ifinfo1->bat_v.throughput > threshold;
+	ret = ifinfo_cand->bat_v.throughput > threshold;
 
-	batadv_neigh_ifinfo_put(ifinfo2);
+	batadv_neigh_ifinfo_put(ifinfo_ref);
 err_ifinfo2:
-	batadv_neigh_ifinfo_put(ifinfo1);
+	batadv_neigh_ifinfo_put(ifinfo_cand);
 err_ifinfo1:
 	return ret;
 }
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 9bdc5a3e799e2..99667f632d4d9 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -2087,14 +2087,14 @@ struct batadv_algo_neigh_ops {
 		   struct batadv_hard_iface *if_outgoing2);
 
 	/**
-	 * @is_similar_or_better: check if neigh1 is equally similar or better
-	 *  than neigh2 for their respective outgoing interface from the metric
+	 * @is_similar_or_better: check if @candidate is equally similar or better
+	 *  than @reference for their respective outgoing interface from the metric
 	 *  prospective
 	 */
-	bool (*is_similar_or_better)(struct batadv_neigh_node *neigh1,
-				     struct batadv_hard_iface *if_outgoing1,
-				     struct batadv_neigh_node *neigh2,
-				     struct batadv_hard_iface *if_outgoing2);
+	bool (*is_similar_or_better)(struct batadv_neigh_node *candidate,
+				     struct batadv_hard_iface *if_outgoing_cand,
+				     struct batadv_neigh_node *reference,
+				     struct batadv_hard_iface *if_outgoing_ref);
 
 	/** @dump: dump neighbors to a netlink socket (optional) */
 	void (*dump)(struct sk_buff *msg, struct netlink_callback *cb,
-- 
2.47.3

Re: [PATCH net-next 05/15] batman-adv: use more descriptive var names for is_similar_or_better

[Sven Eckelmann]

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

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

> > diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
> > index 53fbdbbe8f4f..3fe09b4c3046 100644
> > --- a/net/batman-adv/bat_iv_ogm.c
> > +++ b/net/batman-adv/bat_iv_ogm.c
> > @@ -2424,25 +2424,26 @@ static int batadv_iv_ogm_neigh_cmp(struct batadv_neigh_node *neigh1,
> >  /**
> >   * batadv_iv_ogm_neigh_is_sob() - check if neigh1 is similarly good or better
> >   *  than neigh2 from the metric prospective
>                 ^^^^^^          ^^^^^^
> 
> Discovered by: gpt-5-6-sol, opus-5 · Missed by: sashiko-gemini
> This isn't a bug, but was the summary line of the kernel-doc for
> batadv_iv_ogm_neigh_is_sob() meant to be converted too?

Good point, prepared a follow up patch.
https://patch.msgid.link/20260901-bat_iv-sob-kdoc-v1-1-54be2f58b715@narfation.org

Regards,
	Sven

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

[PATCH net-next 06/15] batman-adv: ensure u16 aligned mac address arrays on stack

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

An u8 array on the stack has a natural alignment of only 1 byte. An
u8[ETH_ALEN] array on the stack must therefore be assumed to only be 1 byte
aligned. But many etherdevices.h functions are requiring a 2 byte (u16)
alignment.

Annote all u8[ETH_ALEN] arrays on the stack as __aligned(2) to make sure
that the compiler takes care of aligning it correctly.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260821094813.201800-1-sw%40simonwunderlich.de?part=5
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/bridge_loop_avoidance.c |  6 ++++--
 net/batman-adv/distributed-arp-table.c |  8 ++++----
 net/batman-adv/mesh-interface.c        | 12 ++++++++----
 net/batman-adv/multicast.c             |  6 +++---
 4 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index f635da4b8ca65..ad6ab4a50658f 100644
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -663,7 +663,7 @@ static void batadv_bla_send_request(struct batadv_bla_backbone_gw *backbone_gw)
 static void batadv_bla_send_announce(struct batadv_priv *bat_priv,
 				     struct batadv_bla_backbone_gw *backbone_gw)
 {
-	u8 mac[ETH_ALEN];
+	u8 mac[ETH_ALEN] __aligned(2);
 	__be16 crc;
 
 	memcpy(mac, batadv_announce_mac, 4);
@@ -1582,7 +1582,9 @@ static struct lock_class_key batadv_backbone_hash_lock_class_key;
  */
 int batadv_bla_init(struct batadv_priv *bat_priv)
 {
-	u8 claim_dest[ETH_ALEN] = {0xff, 0x43, 0x05, 0x00, 0x00, 0x00};
+	static const u8 claim_dest[ETH_ALEN] __aligned(2) = {
+		0xff, 0x43, 0x05, 0x00, 0x00, 0x00
+	};
 	struct batadv_hard_iface *primary_if;
 	unsigned long entrytime;
 	u16 crc;
diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c
index 270f8d6e29545..a4be5b2a87d7a 100644
--- a/net/batman-adv/distributed-arp-table.c
+++ b/net/batman-adv/distributed-arp-table.c
@@ -940,7 +940,7 @@ batadv_dat_cache_dump_entry(struct sk_buff *msg, u32 portid,
 			    struct netlink_callback *cb,
 			    struct batadv_dat_entry *dat_entry)
 {
-	u8 mac[ETH_ALEN];
+	u8 mac[ETH_ALEN] __aligned(2);
 	u64 u64_mac;
 	int msecs;
 	void *hdr;
@@ -1236,10 +1236,10 @@ bool batadv_dat_snoop_outgoing_arp_request(struct batadv_priv *bat_priv,
 {
 	struct net_device *mesh_iface = bat_priv->mesh_iface;
 	struct batadv_dat_entry *dat_entry = NULL;
+	u8 mac[ETH_ALEN] __aligned(2);
 	struct sk_buff *skb_new;
 	unsigned short vid;
 	bool ret = false;
-	u8 mac[ETH_ALEN];
 	int hdr_size = 0;
 	__be32 ip_dst;
 	__be32 ip_src;
@@ -1344,10 +1344,10 @@ bool batadv_dat_snoop_incoming_arp_request(struct batadv_priv *bat_priv,
 					   struct sk_buff *skb, int hdr_size)
 {
 	struct batadv_dat_entry *dat_entry = NULL;
+	u8 mac[ETH_ALEN] __aligned(2);
 	struct sk_buff *skb_new;
 	unsigned short vid;
 	bool ret = false;
-	u8 mac[ETH_ALEN];
 	__be32 ip_src;
 	__be32 ip_dst;
 	u64 u64_mac;
@@ -1474,9 +1474,9 @@ bool batadv_dat_snoop_incoming_arp_reply(struct batadv_priv *bat_priv,
 					 struct sk_buff *skb, int hdr_size)
 {
 	struct batadv_dat_entry *dat_entry = NULL;
+	u8 mac[ETH_ALEN] __aligned(2);
 	bool dropped = false;
 	unsigned short vid;
-	u8 mac[ETH_ALEN];
 	__be32 ip_src;
 	__be32 ip_dst;
 	u64 u64_mac;
diff --git a/net/batman-adv/mesh-interface.c b/net/batman-adv/mesh-interface.c
index e202088cf1fd4..63af21954cf90 100644
--- a/net/batman-adv/mesh-interface.c
+++ b/net/batman-adv/mesh-interface.c
@@ -141,9 +141,9 @@ static struct net_device_stats *batadv_interface_stats(struct net_device *dev)
 static int batadv_interface_set_mac_addr(struct net_device *dev, void *p)
 {
 	struct batadv_priv *bat_priv = netdev_priv(dev);
+	u8 old_addr[ETH_ALEN] __aligned(2);
 	struct batadv_meshif_vlan *vlan;
 	struct sockaddr *addr = p;
-	u8 old_addr[ETH_ALEN];
 
 	if (!is_valid_ether_addr(addr->sa_data))
 		return -EADDRNOTAVAIL;
@@ -213,13 +213,18 @@ static void batadv_interface_set_rx_mode(struct net_device *dev)
 static netdev_tx_t batadv_interface_tx(struct sk_buff *skb,
 				       struct net_device *mesh_iface)
 {
-	static const u8 ectp_addr[ETH_ALEN] = {0xCF, 0x00, 0x00, 0x00, 0x00, 0x00};
-	static const u8 stp_addr[ETH_ALEN] = {0x01, 0x80, 0xC2, 0x00, 0x00, 0x00};
 	struct batadv_priv *bat_priv = netdev_priv(mesh_iface);
 	enum batadv_dhcp_recipient dhcp_rcp = BATADV_DHCP_NO;
 	enum batadv_forw_mode forw_mode = BATADV_FORW_BCAST;
+	static const u8 ectp_addr[ETH_ALEN] __aligned(2) = {
+		0xCF, 0x00, 0x00, 0x00, 0x00, 0x00
+	};
+	static const u8 stp_addr[ETH_ALEN] __aligned(2) = {
+		0x01, 0x80, 0xC2, 0x00, 0x00, 0x00
+	};
 	struct batadv_hard_iface *primary_if = NULL;
 	struct batadv_bcast_packet *bcast_packet;
+	u8 chaddr[ETH_ALEN] __aligned(2);
 	int network_offset = ETH_HLEN;
 	unsigned int header_len = 0;
 	unsigned long brd_delay = 0;
@@ -229,7 +234,6 @@ static netdev_tx_t batadv_interface_tx(struct sk_buff *skb,
 	struct ethhdr *ethhdr;
 	bool do_bcast = false;
 	u8 *dst_hint = NULL;
-	u8 chaddr[ETH_ALEN];
 	unsigned short vid;
 	bool client_added;
 	__be16 proto;
diff --git a/net/batman-adv/multicast.c b/net/batman-adv/multicast.c
index 82fd527b61c02..22dd28d5e1abc 100644
--- a/net/batman-adv/multicast.c
+++ b/net/batman-adv/multicast.c
@@ -368,9 +368,9 @@ batadv_mcast_mla_meshif_get_ipv4(struct net_device *dev,
 				 struct hlist_head *mcast_list,
 				 struct batadv_mcast_mla_flags *flags)
 {
+	u8 mcast_addr[ETH_ALEN] __aligned(2);
 	struct batadv_hw_addr *new;
 	struct in_device *in_dev;
-	u8 mcast_addr[ETH_ALEN];
 	struct ip_mc_list *pmc;
 	int ret = 0;
 
@@ -435,9 +435,9 @@ batadv_mcast_mla_meshif_get_ipv6(struct net_device *dev,
 				 struct hlist_head *mcast_list,
 				 struct batadv_mcast_mla_flags *flags)
 {
+	u8 mcast_addr[ETH_ALEN] __aligned(2);
 	struct batadv_hw_addr *new;
 	struct inet6_dev *in6_dev;
-	u8 mcast_addr[ETH_ALEN];
 	struct ifmcaddr6 *pmc6;
 	int ret = 0;
 
@@ -587,10 +587,10 @@ static int batadv_mcast_mla_bridge_get(struct net_device *dev,
 				       struct batadv_mcast_mla_flags *flags)
 {
 	struct list_head bridge_mcast_list = LIST_HEAD_INIT(bridge_mcast_list);
+	u8 mcast_addr[ETH_ALEN] __aligned(2);
 	u8 tvlv_flags = flags->tvlv_flags;
 	struct br_ip_list *br_ip_entry;
 	struct batadv_hw_addr *new;
-	u8 mcast_addr[ETH_ALEN];
 	struct br_ip_list *tmp;
 	int ret;
 
-- 
2.47.3

Re: [PATCH net-next 06/15] batman-adv: ensure u16 aligned mac address arrays on stack

[Sven Eckelmann]

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

On Tuesday, 1 September 2026 15:59:59 CEST sashiko-bot@kernel.org wrote:
[...]
> If memory allocation fails for bat_priv->bla.backbone_hash, the previously
> allocated claim_hash is destroyed but the pointer is left dangling:
> 
> net/batman-adv/bridge_loop_avoidance.c:batadv_bla_init() {
>         ...
>         bat_priv->bla.backbone_hash = batadv_hash_new(32);
>         if (!bat_priv->bla.backbone_hash) {
>                 batadv_hash_destroy(bat_priv->bla.claim_hash);
>                 return -ENOMEM;
>         }
>         ...
> }
> 
> This failure propagates up to batadv_mesh_init() and register_netdevice(),
> which rolls back by calling the device's priv_destructor 
> (batadv_meshif_free()).
> 
> This destructor calls batadv_mesh_free() which unconditionally calls
> batadv_bla_free(). Could batadv_bla_free() then use the dangling non-NULL
> claim_hash pointer:
> 
> net/batman-adv/bridge_loop_avoidance.c:batadv_bla_free() {
>         ...
>         if (bat_priv->bla.claim_hash) {
>                 batadv_bla_purge_claims(bat_priv, primary_if, 1);
>                 batadv_hash_destroy(bat_priv->bla.claim_hash);
>                 bat_priv->bla.claim_hash = NULL;
>         }
>         ...
> }
> 
> leading to a use-after-free in batadv_bla_purge_claims() and a double free
> via batadv_hash_destroy()?
> 
> Would it be safer to set bat_priv->bla.claim_hash = NULL immediately after
> destroying it in batadv_bla_init()?
> 
> 

Completely out-of-scope for this patch. Submitted patch for review to 
https://patch.msgid.link/20260901-claim-avoid-err-double-free-v1-1-6d6f081f5151@narfation.org

Regards,
	Sven

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

[PATCH net-next 07/15] batman-adv: ensure u16 aligned mac address in structs

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

An u8 array in structures have a natural alignment of only 1 byte. An
u8[ETH_ALEN] array must therefore be assumed to only be 1 byte aligned. But
many etherdevices.h functions are requiring a 2 byte (u16) alignment.

All these addresses were either on the start of a struct or after a struct
member which required at least an alignment of 2 byte. Still, annote all
u8[ETH_ALEN] arrays on the stack as __aligned(2) to make sure that this
assumption isn't broken by other modifications inside the structs.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/types.h | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 99667f632d4d9..c42a4aa8f41a3 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -410,7 +410,7 @@ struct batadv_orig_bat_iv {
  */
 struct batadv_orig_node {
 	/** @orig: originator ethernet address */
-	u8 orig[ETH_ALEN];
+	u8 orig[ETH_ALEN] __aligned(2);
 
 	/** @ifinfo_list: list for routers per outgoing interface */
 	struct hlist_head ifinfo_list;
@@ -634,12 +634,12 @@ struct batadv_hardif_neigh_node {
 	struct hlist_node list;
 
 	/** @addr: the MAC address of the neighboring interface */
-	u8 addr[ETH_ALEN];
+	u8 addr[ETH_ALEN] __aligned(2);
 
 	/**
 	 * @orig: the address of the originator this neighbor node belongs to
 	 */
-	u8 orig[ETH_ALEN];
+	u8 orig[ETH_ALEN] __aligned(2);
 
 	/** @if_incoming: pointer to incoming hard-interface */
 	struct batadv_hard_iface *if_incoming;
@@ -677,7 +677,7 @@ struct batadv_neigh_node {
 #endif
 
 	/** @addr: the MAC address of the neighboring interface */
-	u8 addr[ETH_ALEN];
+	u8 addr[ETH_ALEN] __aligned(2);
 
 	/** @ifinfo_list: list for routing metrics per outgoing interface */
 	struct hlist_head ifinfo_list;
@@ -777,7 +777,7 @@ struct batadv_neigh_ifinfo {
  */
 struct batadv_bcast_duplist_entry {
 	/** @orig: mac address of orig node originating the broadcast */
-	u8 orig[ETH_ALEN];
+	u8 orig[ETH_ALEN] __aligned(2);
 
 	/** @crc: crc32 checksum of broadcast payload */
 	u32 crc;
@@ -1085,7 +1085,7 @@ struct batadv_priv_bla {
 	struct batadv_hashtable *backbone_hash;
 
 	/** @loopdetect_addr: MAC address used for own loopdetection frames */
-	u8 loopdetect_addr[ETH_ALEN];
+	u8 loopdetect_addr[ETH_ALEN] __aligned(2);
 
 	/**
 	 * @loopdetect_lasttime: time when the loopdetection frames were sent
@@ -1352,7 +1352,7 @@ struct batadv_tp_vars_common {
 	struct batadv_priv *bat_priv;
 
 	/** @other_end: mac address of remote */
-	u8 other_end[ETH_ALEN];
+	u8 other_end[ETH_ALEN] __aligned(2);
 
 	/** @session: TP session identifier */
 	u8 session[2];
@@ -1765,7 +1765,7 @@ struct batadv_bla_backbone_gw {
 	 * @orig: originator address of backbone node (mac address of primary
 	 *  iface)
 	 */
-	u8 orig[ETH_ALEN];
+	u8 orig[ETH_ALEN] __aligned(2);
 
 	/** @vid: vlan id this gateway was detected on */
 	unsigned short vid;
@@ -1810,7 +1810,7 @@ struct batadv_bla_backbone_gw {
  */
 struct batadv_bla_claim {
 	/** @addr: mac address of claimed non-mesh client */
-	u8 addr[ETH_ALEN];
+	u8 addr[ETH_ALEN] __aligned(2);
 
 	/** @vid: vlan id this client was detected on */
 	unsigned short vid;
@@ -1840,7 +1840,7 @@ struct batadv_bla_claim {
  */
 struct batadv_tt_common_entry {
 	/** @addr: mac address of non-mesh client */
-	u8 addr[ETH_ALEN];
+	u8 addr[ETH_ALEN] __aligned(2);
 
 	/** @vid: VLAN identifier */
 	unsigned short vid;
@@ -1942,7 +1942,7 @@ struct batadv_tt_req_node {
 	/**
 	 * @addr: mac address of the originator this request was sent to
 	 */
-	u8 addr[ETH_ALEN];
+	u8 addr[ETH_ALEN] __aligned(2);
 
 	/** @issued_at: timestamp used for purging stale tt requests */
 	unsigned long issued_at;
@@ -1959,7 +1959,7 @@ struct batadv_tt_req_node {
  */
 struct batadv_tt_roam_node {
 	/** @addr: mac address of the client in the roaming phase */
-	u8 addr[ETH_ALEN];
+	u8 addr[ETH_ALEN] __aligned(2);
 
 	/** @vid: VLAN identifier */
 	u16 vid;
@@ -2204,7 +2204,7 @@ struct batadv_hw_addr {
 	struct hlist_node list;
 
 	/** @addr: the MAC address of this list entry */
-	unsigned char addr[ETH_ALEN];
+	unsigned char addr[ETH_ALEN] __aligned(2);
 };
 
 /**
-- 
2.47.3

Re: [PATCH net-next 07/15] batman-adv: ensure u16 aligned mac address in structs

[Sven Eckelmann]

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

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

> > An u8 array in structures have a natural alignment of only 1 byte. An
> > u8[ETH_ALEN] array must therefore be assumed to only be 1 byte aligned. But
> > many etherdevices.h functions are requiring a 2 byte (u16) alignment.
> >
> > All these addresses were either on the start of a struct or after a struct
> > member which required at least an alignment of 2 byte. Still, annote all
> > u8[ETH_ALEN] arrays on the stack as __aligned(2) to make sure that this
> > assumption isn't broken by other modifications inside the structs.
> 
> Discovered by: gpt-5-6-sol, opus-5 · Missed by: sashiko-gemini
> Should "arrays on the stack" read "arrays in structs" here?

Yes, copy+paste error. Minor for this PR

> > diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
> > index 99667f632d4d..c42a4aa8f41a 100644
> > --- a/net/batman-adv/types.h
> > +++ b/net/batman-adv/types.h
> 
> [ ... ]
> 
> > @@ -634,12 +634,12 @@ struct batadv_hardif_neigh_node {
> >  	struct hlist_node list;
> >  
> >  	/** @addr: the MAC address of the neighboring interface */
> > -	u8 addr[ETH_ALEN];
> > +	u8 addr[ETH_ALEN] __aligned(2);
> >  
> >  	/**
> >  	 * @orig: the address of the originator this neighbor node belongs to
> >  	 */
> > -	u8 orig[ETH_ALEN];
> > +	u8 orig[ETH_ALEN] __aligned(2);
> 
> Discovered by: gpt-5-6-sol, opus-5 · Missed by: sashiko-gemini
> Is the claim "All these addresses were either on the start of a struct or
> after a struct member which required at least an alignment of 2 byte"
> accurate for batadv_hardif_neigh_node.orig?
> 
> That member directly follows u8 addr[ETH_ALEN], which only requires 1 byte
> alignment; orig ends up on an even offset because ETH_ALEN happens to be
> even, not because the preceding member forces 2 byte alignment.  Would it
> be worth rewording that sentence?

But the struct member required indirectly an aligned of 2 bytes

Regards,
	Sven

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

[PATCH net-next 08/15] batman-adv: tt: remove only the entry which was looked up from the hash

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

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

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

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

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

Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/translation-table.c | 38 ++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 12 deletions(-)

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

Re: [PATCH net-next 08/15] batman-adv: tt: remove only the entry which was looked up from the hash

[Sven Eckelmann]

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

On Tuesday, 1 September 2026 16:00:00 CEST sashiko-bot@kernel.org wrote:
[...]
> [Severity: High]
> This isn't a bug introduced by this patch, but doesn't this exact same
> vulnerability still exist in the Bridge Loop Avoidance (BLA) subsystem's
> batadv_bla_del_claim() in net/batman-adv/bridge_loop_avoidance.c?


Out of scope. And it is missing the completely different promises by this 
function. It would make the code a little bit cleaner but it is definitely 
nothing missing it this patch. A ticket for a potential improvement can be 
found in https://www.open-mesh.org/issues/471.

Regards,
	Sven

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

Re: [PATCH net-next 08/15] batman-adv: tt: remove only the entry which was looked up from the hash

[Sven Eckelmann]

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

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

[...]
> > So when the looked up entry was already unlinked by another context and a
> > new entry for the same client was added in the meantime, these two
> > functions unlink that new entry instead.
> 
> Discovered by: gpt-5-6-sol, opus-5 · Missed by: sashiko-gemini
> Should this carry a Fixes: tag?  The two converted call sites had this
> defect since the refcount handling around batadv_hash_remove() was
> introduced, and the sibling fixes for the very same call sites did use
> Fixes: tags:
> 
>   ef72706a0543 ("batman-adv: protect tt_local_entry from concurrent delete
>   events") is the Fixes: target of 3d65b9accab4 for the local path
> 
>   7683fdc1e886 ("batman-adv: protect the local and the global trans-tables
>   with rcu") is the Fixes: target of f131a56880d1 for the global path
> 
> Both of those commits already noted that batadv_hash_remove() may delete
> "an entry ... which is not the same object as the needle" and only repaired
> the reference accounting, which is the part this patch completes.  Without
> a trailer, stable tooling has no hint that this is a functional fix rather
> than part of the surrounding cleanup.

I am not allowed to add Fixes: lines for patches targeting net-next. See
https://lore.kernel.org/all/7d73cd74-b040-4a3d-9d78-4ad8fc0a01f4@redhat.com/

> 
> > diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> > index 88aeefa97db6..66456a2d45e2 100644
> > --- a/net/batman-adv/translation-table.c
> > +++ b/net/batman-adv/translation-table.c
> > @@ -95,6 +95,26 @@ static bool batadv_compare_tt(const struct hlist_node *node, const void *data2)
> >  	return (tt1->vid == tt2->vid) && batadv_compare_eth(data1, data2);
> >  }
> >  
> > +/**
> > + * batadv_compare_tt_entry() - check if a hash node is a specific TT entry
> > + * @node: the list element pointer of the TT entry stored in the bucket
> > + * @data2: pointer to the tt_common_entry which is looked for
> > + *
> > + * Unlike batadv_compare_tt(), this only matches the very object which is
> > + * passed as @data2 and not just any entry for the same TT client. It is meant
> > + * for batadv_hash_remove() callers which must not unlink an entry they did not
> > + * look up themselves.
> > + *
> > + * Return: true if @node belongs to @data2, false otherwise
> > + */
> > +static bool batadv_compare_tt_entry(const struct hlist_node *node,
> > +				    const void *data2)
> > +{
> > +	const struct batadv_tt_common_entry *tt = data2;
> > +
> > +	return node == &tt->hash_entry;
> > +}
> > +
> 
> Discovered by: gpt-5-6-sol, opus-5 · Missed by: sashiko-gemini
> This isn't a bug introduced by this patch, but was the identical pattern in
> batadv_bla_del_claim() (net/batman-adv/bridge_loop_avoidance.c) considered
> for the same treatment?  It still looks a claim up by key via
> batadv_claim_hash_find() and then removes by key:

Out of scope. And it is missing the completely different promises by this 
function. It would make the code a little bit cleaner but it is definitely 
nothing missing it this patch. A ticket for a potential improvement can be 
found in https://www.open-mesh.org/issues/471.

[...]
> Discovered by: gpt-5-6-sol · Missed by: opus-5, sashiko-gemini · Confirmed by: opus-5
> This is a pre-existing issue, but should the kernel-doc of
> batadv_hash_remove() in net/batman-adv/hash.h be corrected while these
> callers are being touched?  It says:
> 
>  * Return: returns pointer to data on success, so you can remove the used
>  * structure yourself, or NULL on error
> 
> while the implementation returns the matched hash node:
> 
> 		data_save = node;
> 		hlist_del_rcu(node);
> 
> This patch drops the hlist_entry() conversions that used to make the
> node-ness of the return value obvious at both tt call sites, so the only
> remaining hint is the tt_removed_node variable name.

Out-of-scope for this patch. Follow up patch was submitted
https://patch.msgid.link/20260901-kdoc-hash_del-return-type-v1-1-07b9c26e8df3@narfation.org

> 
> Discovered by: gpt-5-6-sol · Missed by: opus-5, sashiko-gemini · Confirmed by: opus-5
> This isn't a bug introduced by this patch either, but is the caller side of
> batadv_tt_global_free() still racy in a way the identity compare cannot
> help with?  batadv_tt_global_del() tests the orig_list emptiness after
> batadv_tt_global_del_orig_node() has already dropped
> tt_global_entry->list_lock:
> 
> 		batadv_tt_global_del_orig_node(bat_priv, tt_global_entry,
> 					       orig_node, message);
> 
> 		if (hlist_empty(&tt_global_entry->orig_list))
> 			batadv_tt_global_free(bat_priv, tt_global_entry,
> 					      message);
> 
> Meanwhile batadv_tt_global_orig_entry_add() can add an originator for the
> very same object under that lock:
> 
> 	spin_lock_bh(&tt_global->list_lock);
> 	...
> 	hlist_add_head_rcu(&orig_entry->list, &tt_global->orig_list);
> 	atomic_inc(&tt_global->orig_list_count);
> 
> Since the emptiness decision and the hash removal are not atomic, can a
> global entry that has just regained a valid originator still be unlinked?
> The new comparator matches here because it is the same object.  The
> hlist_empty() read also happens outside the lock that the orig_list is
> documented to be protected by.

Outside the scope of this patch. For interested contributors
https://www.open-mesh.org/issues/473


[...]
> Discovered by: gpt-5-6-sol, opus-5 · Missed by: sashiko-gemini
> With the identity compare, batadv_hash_remove() can now fail while a
> different, live local entry for the same MAC and VLAN is still linked in
> bat_priv->tt.local_hash.  The BATADV_TT_CLIENT_DEL change event is queued
> before the removal and is not undone on that new failure path.
> 
> batadv_tt_local_event() coalesces purely by MAC and VLAN:
[...]
> Can the stale DEL then cancel the pending ADD of the surviving entry?
> 
> CPU A: batadv_tt_local_remove() (or the purge work) unlinks entry E
> CPU B: batadv_tt_local_add() inserts E' for the same addr/vid; the
>        batadv_hash_add() happens before its ADD event is queued at
>        the add_event label
> CPU A: having looked E up earlier via batadv_tt_local_hash_find(), with no
>        lock spanning lookup, event and removal, queues DEL and then finds
>        no node equal to &E->common.hash_entry
> 
> Either ordering ends with both the ADD and the DEL removed from
> bat_priv->tt.changes_list, so E' stays in the local hash with
> BATADV_TT_CLIENT_NEW set and no queued change.
[...]
> Would it make sense to queue the event only after a successful unlink, or to
> re-queue the ADD when the removal fails?  The ordering is still the same at
> the end of the series, in "batman-adv: tt: decrement count for committed
> client on local_remove".

This is not the only way something like this could happen. But ok, submitted a 
patch

https://patch.msgid.link/20260901-tt-del-event-queue-late-v1-1-fc28ca54c342@narfation.org

Regards,
	Sven

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

[PATCH net-next 09/15] batman-adv: tt: extract code handling a roam on add

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

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

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

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

Re: [PATCH net-next 09/15] batman-adv: tt: extract code handling a roam on add

[Sven Eckelmann]

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

On Tuesday, 1 September 2026 16:00:01 CEST sashiko-bot@kernel.org wrote:
[...]
> [Severity: High]
> This is a pre-existing issue, but I'm wondering if the handling of pending
> and roamed clients in batadv_tt_local_add() might have a logical flaw that
> persists here.
> 
> When batadv_tt_local_remove() is called with roaming=true for a fully
> synced client, it sets both BATADV_TT_CLIENT_ROAM and
> BATADV_TT_CLIENT_PENDING.
> 
> If the client roams back before the entry is purged, calling
> batadv_tt_local_add(), it hits this earlier code path:
> 
>     if (tt_local->common.flags & BATADV_TT_CLIENT_PENDING) {
>         ...
>         tt_local->common.flags &= ~BATADV_TT_CLIENT_PENDING;
>         spin_unlock_bh(&tt_local->common.flags_lock);
> 
>         goto add_event;
>     }
> 
>     if (tt_local->common.flags & BATADV_TT_CLIENT_ROAM) {
>         ...
>         tt_local->common.flags &= ~BATADV_TT_CLIENT_ROAM;
>         roamed_back = true;
>     }
> 
> Because of the goto add_event, it bypasses the BATADV_TT_CLIENT_ROAM
> check entirely, skipping roamed_back = true. It then proceeds from
> add_event into check_roaming, where batadv_tt_local_add_roam() is
> called with roamed_back == false.
> 
> Can this result in the local TT entry remaining incorrectly marked with
> BATADV_TT_CLIENT_ROAM, causing packets destined for the returned client to
> be misrouted or dropped?
> 
> Does this also cause the global roaming state to be incorrectly preserved
> instead of being properly canceled?
> 
> 

Out of scope for this patch. Documented it for interested contributors in 
https://www.open-mesh.org/issues/472

Regards,
	Sven

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

Re: [PATCH net-next 09/15] batman-adv: tt: extract code handling a roam on add

[Sven Eckelmann]

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

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


> Discovered by: gpt-5-6-sol, opus-5 · Missed by: sashiko-gemini
> This isn't a bug introduced by this patch, since it is pure code motion, but
> now that the sequence sits in its own helper it becomes more visible: is the
> check-then-act on tt_global->common.flags in batadv_tt_local_add_roam()
> racy?
> 
> The test and the update are separated by the ROAM_ADV loop, and no lock is
> held around either:
> 
> 	if (tt_global->common.flags & BATADV_TT_CLIENT_ROAM)
> 		return;
> 	...
> 	hlist_for_each_entry_rcu(orig_entry, head, list) {
> 		batadv_send_roam_adv(...);
> 	}
> 	...
> 		tt_global->common.flags |= BATADV_TT_CLIENT_ROAM;
> 
> The rcu_read_lock() here only covers the orig_list walk.
> 
> Meanwhile batadv_tt_global_add() does a non-atomic read-modify-write on the
> same field from TVLV processing:
> 
> 	common->flags |= flags & (~BATADV_TT_SYNC_MASK);
> 
> and batadv_tt_global_del_roaming() also sets BATADV_TT_CLIENT_ROAM. There is
> no common lock between those paths at this commit.
[...]

Please look at the next patches to see how locks are introduced.

Regards,
	Sven

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

[PATCH net-next 10/15] batman-adv: tt: simplify NEW flag transition code

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

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

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

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

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

Re: [PATCH net-next 10/15] batman-adv: tt: simplify NEW flag transition code

[Sven Eckelmann]

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

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

[...]
> Discovered by: opus-5, sonnet-5 · Missed by: gpt-5-6-sol, sashiko-gemini
> 
> This isn't a bug introduced by this patch, since the removed
> batadv_tt_local_set_flags() did the same unlocked read-modify-write under
> rcu_read_lock(), but it may be worth noting for the rewritten loop in
> batadv_tt_local_transition_new().
> 
> The loop only holds rcu_read_lock(), so it takes neither the hash bucket
> list_lock nor any per-entry lock while doing:
> 
> 	tt_common_entry->flags &= ~BATADV_TT_CLIENT_NEW;
> 
> Can this store be lost against a concurrent RMW on the same u16 from
> batadv_tt_local_add() or batadv_tt_local_set_pending() on another CPU, for
> example one that is OR-ing BATADV_TT_CLIENT_PENDING or
> BATADV_TT_CLIENT_ROAM into the same word?
[...]

Out-of-scope for this patch. And handled in the patch
"batman-adv: tt: transition NEW local entries only under lock"

Regards,
	Sven

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

[PATCH net-next 11/15] batman-adv: tt: drop unnecessary cleanup goto in helpers

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

To use scoped_guard(), it is not allowed to use goto in the same routine.
These goto's should only be used for cleanups but are not necessary of
minimal helpers like batadv_is_my_client(),
batadv_tt_global_client_is_roaming() and
batadv_tt_local_client_is_roaming(). Removing the goto's is actually making
these functions more readable.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/translation-table.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 2f7951e4c3203..fbe75e184153f 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -3544,19 +3544,18 @@ bool batadv_is_my_client(struct batadv_priv *bat_priv, const u8 *addr,
 			 unsigned short vid)
 {
 	struct batadv_tt_local_entry *tt_local_entry;
-	bool ret = false;
+	bool ret;
 
 	tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr, vid);
 	if (!tt_local_entry)
-		goto out;
+		return false;
+
 	/* Check if the client has been logically deleted (but is kept for
 	 * consistency purpose)
 	 */
-	if ((tt_local_entry->common.flags & BATADV_TT_CLIENT_PENDING) ||
-	    (tt_local_entry->common.flags & BATADV_TT_CLIENT_ROAM))
-		goto out;
-	ret = true;
-out:
+	ret = !((tt_local_entry->common.flags & BATADV_TT_CLIENT_PENDING) ||
+		(tt_local_entry->common.flags & BATADV_TT_CLIENT_ROAM));
+
 	batadv_tt_local_entry_put(tt_local_entry);
 	return ret;
 }
@@ -4082,15 +4081,15 @@ bool batadv_tt_global_client_is_roaming(struct batadv_priv *bat_priv,
 					u8 *addr, unsigned short vid)
 {
 	struct batadv_tt_global_entry *tt_global_entry;
-	bool ret = false;
+	bool ret;
 
 	tt_global_entry = batadv_tt_global_hash_find(bat_priv, addr, vid);
 	if (!tt_global_entry)
-		goto out;
+		return false;
 
 	ret = tt_global_entry->common.flags & BATADV_TT_CLIENT_ROAM;
 	batadv_tt_global_entry_put(tt_global_entry);
-out:
+
 	return ret;
 }
 
@@ -4108,15 +4107,15 @@ bool batadv_tt_local_client_is_roaming(struct batadv_priv *bat_priv,
 				       u8 *addr, unsigned short vid)
 {
 	struct batadv_tt_local_entry *tt_local_entry;
-	bool ret = false;
+	bool ret;
 
 	tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr, vid);
 	if (!tt_local_entry)
-		goto out;
+		return false;
 
 	ret = tt_local_entry->common.flags & BATADV_TT_CLIENT_ROAM;
 	batadv_tt_local_entry_put(tt_local_entry);
-out:
+
 	return ret;
 }
 
-- 
2.47.3

[PATCH net-next 12/15] batman-adv: tt: use protected flag modifications

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

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

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

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

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

Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/translation-table.c | 347 ++++++++++++++++++++---------
 net/batman-adv/types.h             |  11 +-
 2 files changed, 253 insertions(+), 105 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index fbe75e184153f..163f909623069 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -13,6 +13,7 @@
 #include <linux/build_bug.h>
 #include <linux/byteorder/generic.h>
 #include <linux/cache.h>
+#include <linux/cleanup.h>
 #include <linux/compiler.h>
 #include <linux/container_of.h>
 #include <linux/crc32.h>
@@ -433,6 +434,22 @@ batadv_tt_orig_list_entry_put(struct batadv_tt_orig_list_entry *orig_entry)
 	kref_put(&orig_entry->refcount, batadv_tt_orig_list_entry_release);
 }
 
+/**
+ * batadv_tt_flags_get() - get a snapshot of the flags of a TT entry
+ * @common: tt local & tt global common data
+ *
+ * Return: the flags of the TT entry as observed under its flags_lock.
+ */
+static u16 batadv_tt_flags_get(struct batadv_tt_common_entry *common)
+{
+	u16 flags;
+
+	scoped_guard(spinlock_bh, &common->flags_lock)
+		flags = common->flags;
+
+	return flags;
+}
+
 /**
  * batadv_tt_local_event() - store a local TT event (ADD/DEL)
  * @bat_priv: the bat priv with all the mesh interface information
@@ -445,23 +462,25 @@ static void batadv_tt_local_event(struct batadv_priv *bat_priv,
 {
 	struct batadv_tt_common_entry *common = &tt_local_entry->common;
 	struct batadv_tt_change_node *tt_change_node;
-	u8 flags = common->flags | event_flags;
 	struct batadv_tt_change_node *entry;
 	struct batadv_tt_change_node *safe;
 	bool del_op_requested;
 	bool del_op_entry;
 	size_t changes;
+	u8 flags;
 
 	tt_change_node = kmem_cache_alloc(batadv_tt_change_cache, GFP_ATOMIC);
 	if (!tt_change_node)
 		return;
 
-	tt_change_node->change.flags = flags;
 	memset(tt_change_node->change.reserved, 0,
 	       sizeof(tt_change_node->change.reserved));
 	ether_addr_copy(tt_change_node->change.addr, common->addr);
 	tt_change_node->change.vid = htons(common->vid);
 
+	flags = batadv_tt_flags_get(common) | event_flags;
+
+	tt_change_node->change.flags = flags;
 	del_op_requested = flags & BATADV_TT_CLIENT_DEL;
 
 	/* check for ADD+DEL, DEL+ADD, ADD+ADD or DEL+DEL events */
@@ -633,8 +652,23 @@ static void batadv_tt_local_add_roam(struct batadv_priv *bat_priv,
 	/* Check whether it is a roaming, but don't do anything if the roaming
 	 * process has already been handled
 	 */
-	if (tt_global->common.flags & BATADV_TT_CLIENT_ROAM)
-		return;
+	scoped_guard(spinlock_bh, &tt_global->common.flags_lock) {
+		if (tt_global->common.flags & BATADV_TT_CLIENT_ROAM)
+			return;
+
+		if (!roamed_back) {
+			/* The global entry has to be marked as ROAMING and has to be
+			 * kept for consistency purpose.
+			 *
+			 * batadv_tt_global_to_purge() evaluates roam_at as soon as it
+			 * observes BATADV_TT_CLIENT_ROAM, so the timeout has to be
+			 * stamped before the flag is published. Otherwise the entry can
+			 * be deleted right away as "Roaming timeout".
+			 */
+			tt_global->roam_at = jiffies;
+			tt_global->common.flags |= BATADV_TT_CLIENT_ROAM;
+		}
+	}
 
 	/* These node are probably going to update their tt table */
 	head = &tt_global->orig_list;
@@ -646,16 +680,8 @@ static void batadv_tt_local_add_roam(struct batadv_priv *bat_priv,
 	}
 	rcu_read_unlock();
 
-	if (roamed_back) {
-		batadv_tt_global_free(bat_priv, tt_global,
-				      "Roaming canceled");
-	} else {
-		/* The global entry has to be marked as ROAMING and
-		 * has to be kept for consistency purpose
-		 */
-		tt_global->common.flags |= BATADV_TT_CLIENT_ROAM;
-		tt_global->roam_at = jiffies;
-	}
+	if (roamed_back)
+		batadv_tt_global_free(bat_priv, tt_global, "Roaming canceled");
 }
 
 /**
@@ -688,6 +714,7 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
 	int hash_added;
 	int table_size;
 	u32 match_mark;
+	bool modified;
 
 	if (ifindex != BATADV_NULL_IFINDEX)
 		in_dev = dev_get_by_index(net, ifindex);
@@ -705,6 +732,8 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
 
 	if (tt_local) {
 		tt_local->last_seen = jiffies;
+
+		spin_lock_bh(&tt_local->common.flags_lock);
 		if (tt_local->common.flags & BATADV_TT_CLIENT_PENDING) {
 			batadv_dbg(BATADV_DBG_TT, bat_priv,
 				   "Re-adding pending client %pM (vid: %d)\n",
@@ -715,6 +744,8 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
 			 * flag can be reset like it was never enqueued
 			 */
 			tt_local->common.flags &= ~BATADV_TT_CLIENT_PENDING;
+			spin_unlock_bh(&tt_local->common.flags_lock);
+
 			goto add_event;
 		}
 
@@ -730,6 +761,8 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
 			tt_local->common.flags &= ~BATADV_TT_CLIENT_ROAM;
 			roamed_back = true;
 		}
+		spin_unlock_bh(&tt_local->common.flags_lock);
+
 		goto check_roaming;
 	}
 
@@ -765,18 +798,21 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
 		   (u8)atomic_read(&bat_priv->tt.vn));
 
 	ether_addr_copy(tt_local->common.addr, addr);
+	tt_local->common.vid = vid;
+	kref_init(&tt_local->common.refcount);
+	tt_local->last_seen = jiffies;
+	tt_local->common.added_at = tt_local->last_seen;
+	tt_local->vlan = vlan;
+	spin_lock_init(&tt_local->common.flags_lock);
+
+	spin_lock_bh(&tt_local->common.flags_lock);
 	/* The local entry has to be marked as NEW to avoid to send it in
 	 * a full table response going out before the next ttvn increment
 	 * (consistency check)
 	 */
 	tt_local->common.flags = BATADV_TT_CLIENT_NEW;
-	tt_local->common.vid = vid;
 	if (iif_is_wifi)
 		tt_local->common.flags |= BATADV_TT_CLIENT_WIFI;
-	kref_init(&tt_local->common.refcount);
-	tt_local->last_seen = jiffies;
-	tt_local->common.added_at = tt_local->last_seen;
-	tt_local->vlan = vlan;
 
 	/* the batman interface mac and multicast addresses should never be
 	 * purged
@@ -784,6 +820,7 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
 	if (batadv_compare_eth(addr, mesh_iface->dev_addr) ||
 	    is_multicast_ether_addr(addr))
 		tt_local->common.flags |= BATADV_TT_CLIENT_NOPURGE;
+	spin_unlock_bh(&tt_local->common.flags_lock);
 
 	kref_get(&tt_local->common.refcount);
 	hash_added = batadv_hash_add(bat_priv->tt.local_hash, batadv_compare_tt,
@@ -802,6 +839,7 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
 check_roaming:
 	batadv_tt_local_add_roam(bat_priv, tt_global, roamed_back);
 
+	spin_lock_bh(&tt_local->common.flags_lock);
 	/* store the current remote flags before altering them. This helps
 	 * understanding is flags are changing or not
 	 */
@@ -823,10 +861,13 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
 	else
 		tt_local->common.flags &= ~BATADV_TT_CLIENT_ISOLA;
 
+	modified = remote_flags ^ (tt_local->common.flags & BATADV_TT_REMOTE_MASK);
+	spin_unlock_bh(&tt_local->common.flags_lock);
+
 	/* if any "dynamic" flag has been modified, resend an ADD event for this
 	 * entry so that all the nodes can get the new flags
 	 */
-	if (remote_flags ^ (tt_local->common.flags & BATADV_TT_REMOTE_MASK))
+	if (modified)
 		batadv_tt_local_event(bat_priv, tt_local, BATADV_NO_FLAGS);
 
 	ret = true;
@@ -1151,6 +1192,7 @@ batadv_tt_local_dump_entry(struct sk_buff *msg, u32 portid,
 	struct batadv_meshif_vlan *vlan;
 	unsigned int last_seen_msecs;
 	void *hdr;
+	u16 flags;
 	u32 crc;
 
 	local = container_of(common, struct batadv_tt_local_entry, common);
@@ -1172,13 +1214,15 @@ batadv_tt_local_dump_entry(struct sk_buff *msg, u32 portid,
 
 	genl_dump_check_consistent(cb, hdr);
 
+	flags = batadv_tt_flags_get(common);
+
 	if (nla_put(msg, BATADV_ATTR_TT_ADDRESS, ETH_ALEN, common->addr) ||
 	    nla_put_u32(msg, BATADV_ATTR_TT_CRC32, crc) ||
 	    nla_put_u16(msg, BATADV_ATTR_TT_VID, common->vid) ||
-	    nla_put_u32(msg, BATADV_ATTR_TT_FLAGS, common->flags))
+	    nla_put_u32(msg, BATADV_ATTR_TT_FLAGS, flags))
 		goto nla_put_failure;
 
-	if (!(common->flags & BATADV_TT_CLIENT_NOPURGE) &&
+	if (!(flags & BATADV_TT_CLIENT_NOPURGE) &&
 	    nla_put_u32(msg, BATADV_ATTR_LAST_SEEN_MSECS, last_seen_msecs))
 		goto nla_put_failure;
 
@@ -1285,29 +1329,23 @@ int batadv_tt_local_dump(struct sk_buff *msg, struct netlink_callback *cb)
 }
 
 /**
- * batadv_tt_local_set_pending() - mark a local TT entry as pending removal
+ * batadv_tt_local_set_pending_event() - trigger events for TT pending removal
  * @bat_priv: the bat priv with all the mesh interface information
  * @tt_local_entry: local TT entry to mark
  * @flags: TT change flags to announce together with the pending removal
  * @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.
+ * Schedule the TT change announcement for 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.
  */
 static void
-batadv_tt_local_set_pending(struct batadv_priv *bat_priv,
-			    struct batadv_tt_local_entry *tt_local_entry,
-			    u16 flags, const char *message)
+batadv_tt_local_set_pending_event(struct batadv_priv *bat_priv,
+				  struct batadv_tt_local_entry *tt_local_entry,
+				  u16 flags, const char *message)
 {
 	batadv_tt_local_event(bat_priv, tt_local_entry, flags);
 
-	/* The local client has to be marked as "pending to be removed" but has
-	 * to be kept in the table in order to send it in a full table
-	 * response issued before the net ttvn increment (consistency check)
-	 */
-	tt_local_entry->common.flags |= BATADV_TT_CLIENT_PENDING;
-
 	batadv_dbg(BATADV_DBG_TT, bat_priv,
 		   "Local tt entry (%pM, vid: %d) pending to be removed: %s\n",
 		   tt_local_entry->common.addr,
@@ -1331,12 +1369,14 @@ u16 batadv_tt_local_remove(struct batadv_priv *bat_priv, const u8 *addr,
 	struct batadv_tt_local_entry *tt_local_entry;
 	struct hlist_node *tt_removed_node;
 	u16 curr_flags = BATADV_NO_FLAGS;
+	bool pending = false;
 	u16 flags;
 
 	tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr, vid);
 	if (!tt_local_entry)
 		goto out;
 
+	spin_lock_bh(&tt_local_entry->common.flags_lock);
 	curr_flags = tt_local_entry->common.flags;
 
 	flags = BATADV_TT_CLIENT_DEL;
@@ -1351,10 +1391,17 @@ u16 batadv_tt_local_remove(struct batadv_priv *bat_priv, const u8 *addr,
 	}
 
 	if (!(tt_local_entry->common.flags & BATADV_TT_CLIENT_NEW)) {
-		batadv_tt_local_set_pending(bat_priv, tt_local_entry, flags,
-					    message);
+		tt_local_entry->common.flags |= BATADV_TT_CLIENT_PENDING;
+		pending = true;
+	}
+	spin_unlock_bh(&tt_local_entry->common.flags_lock);
+
+	if (pending) {
+		batadv_tt_local_set_pending_event(bat_priv, tt_local_entry, flags,
+						  message);
 		goto out;
 	}
+
 	/* if this client has been added right now, it is possible to
 	 * immediately purge it
 	 */
@@ -1394,21 +1441,37 @@ static void batadv_tt_local_purge_list(struct batadv_priv *bat_priv,
 
 	hlist_for_each_entry_safe(tt_common_entry, node_tmp, head,
 				  hash_entry) {
+		bool cont = false;
+
 		tt_local_entry = container_of(tt_common_entry,
 					      struct batadv_tt_local_entry,
 					      common);
-		if (tt_local_entry->common.flags & BATADV_TT_CLIENT_NOPURGE)
-			continue;
 
-		/* entry already marked for deletion */
-		if (tt_local_entry->common.flags & BATADV_TT_CLIENT_PENDING)
-			continue;
+		scoped_guard(spinlock_bh, &tt_local_entry->common.flags_lock) {
+			if (tt_local_entry->common.flags & BATADV_TT_CLIENT_NOPURGE) {
+				cont = true;
+				break;
+			}
+
+			/* entry already marked for deletion */
+			if (tt_local_entry->common.flags & BATADV_TT_CLIENT_PENDING) {
+				cont = true;
+				break;
+			}
+
+			if (!batadv_has_timed_out(tt_local_entry->last_seen, timeout)) {
+				cont = true;
+				break;
+			}
+
+			tt_local_entry->common.flags |= BATADV_TT_CLIENT_PENDING;
+		}
 
-		if (!batadv_has_timed_out(tt_local_entry->last_seen, timeout))
+		if (cont)
 			continue;
 
-		batadv_tt_local_set_pending(bat_priv, tt_local_entry,
-					    BATADV_TT_CLIENT_DEL, "timed out");
+		batadv_tt_local_set_pending_event(bat_priv, tt_local_entry,
+						  BATADV_TT_CLIENT_DEL, "timed out");
 	}
 }
 
@@ -1614,8 +1677,10 @@ batadv_tt_global_sync_flags(struct batadv_tt_global_entry *tt_global)
 		flags |= orig_entry->flags;
 	rcu_read_unlock();
 
-	flags |= tt_global->common.flags & (~BATADV_TT_SYNC_MASK);
-	tt_global->common.flags = flags;
+	scoped_guard(spinlock_bh, &tt_global->common.flags_lock) {
+		flags |= tt_global->common.flags & (~BATADV_TT_SYNC_MASK);
+		tt_global->common.flags = flags;
+	}
 }
 
 /**
@@ -1697,8 +1762,10 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv,
 	struct batadv_tt_local_entry *tt_local_entry;
 	struct batadv_tt_common_entry *common;
 	bool ret = false;
+	u16 global_flags;
 	u16 local_flags;
 	int hash_added;
+	bool delete;
 
 	/* ignore global entries from backbone nodes */
 	if (batadv_bla_is_backbone_gw_orig(bat_priv, orig_node->orig, vid))
@@ -1711,9 +1778,11 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv,
 	 * for a roaming advertisement instead of manually messing up the global
 	 * table
 	 */
-	if ((flags & BATADV_TT_CLIENT_TEMP) && tt_local_entry &&
-	    !(tt_local_entry->common.flags & BATADV_TT_CLIENT_NEW))
-		goto out;
+	if ((flags & BATADV_TT_CLIENT_TEMP) && tt_local_entry) {
+		local_flags = batadv_tt_flags_get(&tt_local_entry->common);
+		if (!(local_flags & BATADV_TT_CLIENT_NEW))
+			goto out;
+	}
 
 	if (!tt_global_entry) {
 		tt_global_entry = kmem_cache_zalloc(batadv_tg_cache,
@@ -1724,9 +1793,13 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv,
 		common = &tt_global_entry->common;
 		ether_addr_copy(common->addr, tt_addr);
 		common->vid = vid;
+		spin_lock_init(&common->flags_lock);
 
-		if (!is_multicast_ether_addr(common->addr))
+		if (!is_multicast_ether_addr(common->addr)) {
+			spin_lock_bh(&common->flags_lock);
 			common->flags = flags & (~BATADV_TT_SYNC_MASK);
+			spin_unlock_bh(&common->flags_lock);
+		}
 
 		tt_global_entry->roam_at = 0;
 		/* node must store current time in case of roaming. This is
@@ -1766,8 +1839,10 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv,
 		 *    originator list and add the new one orig_entry
 		 */
 		if (flags & BATADV_TT_CLIENT_TEMP) {
-			if (!(common->flags & BATADV_TT_CLIENT_TEMP))
+			global_flags = batadv_tt_flags_get(common);
+			if (!(global_flags & BATADV_TT_CLIENT_TEMP))
 				goto out;
+
 			if (batadv_tt_global_entry_has_orig(tt_global_entry,
 							    orig_node, NULL))
 				goto out_remove;
@@ -1775,6 +1850,9 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv,
 			goto add_orig_entry;
 		}
 
+		delete = false;
+
+		spin_lock_bh(&common->flags_lock);
 		/* if the client was temporary added before receiving the first
 		 * OGM announcing it, we have to clear the TEMP flag. Also,
 		 * remove the previous temporary orig node and re-add it
@@ -1782,7 +1860,7 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv,
 		 * is a non-temporary entry is preferred.
 		 */
 		if (common->flags & BATADV_TT_CLIENT_TEMP) {
-			batadv_tt_global_del_orig_list(tt_global_entry);
+			delete = true;
 			common->flags &= ~BATADV_TT_CLIENT_TEMP;
 		}
 
@@ -1801,10 +1879,14 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv,
 		 * new one.
 		 */
 		if (common->flags & BATADV_TT_CLIENT_ROAM) {
-			batadv_tt_global_del_orig_list(tt_global_entry);
-			common->flags &= ~BATADV_TT_CLIENT_ROAM;
+			delete = true;
 			tt_global_entry->roam_at = 0;
+			common->flags &= ~BATADV_TT_CLIENT_ROAM;
 		}
+		spin_unlock_bh(&common->flags_lock);
+
+		if (delete)
+			batadv_tt_global_del_orig_list(tt_global_entry);
 	}
 add_orig_entry:
 	/* add the new orig_entry (if needed) or update it */
@@ -1828,6 +1910,8 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv,
 	local_flags = batadv_tt_local_remove(bat_priv, tt_addr, vid,
 					     "global tt received",
 					     flags & BATADV_TT_CLIENT_ROAM);
+
+	spin_lock_bh(&tt_global_entry->common.flags_lock);
 	tt_global_entry->common.flags |= local_flags & BATADV_TT_CLIENT_WIFI;
 
 	if (!(flags & BATADV_TT_CLIENT_ROAM))
@@ -1835,6 +1919,7 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv,
 		 * roaming state anymore.
 		 */
 		tt_global_entry->common.flags &= ~BATADV_TT_CLIENT_ROAM;
+	spin_unlock_bh(&tt_global_entry->common.flags_lock);
 
 out:
 	batadv_tt_global_entry_put(tt_global_entry);
@@ -1904,9 +1989,9 @@ batadv_tt_global_dump_subentry(struct sk_buff *msg, u32 portid, u32 seq,
 			       struct batadv_tt_orig_list_entry *orig,
 			       bool best)
 {
-	u16 flags = (common->flags & (~BATADV_TT_SYNC_MASK)) | orig->flags;
 	struct batadv_orig_node_vlan *vlan;
 	u8 last_ttvn;
+	u16 flags;
 	void *hdr;
 	u32 crc;
 
@@ -1925,6 +2010,7 @@ batadv_tt_global_dump_subentry(struct sk_buff *msg, u32 portid, u32 seq,
 	if (!hdr)
 		return -ENOBUFS;
 
+	flags = (batadv_tt_flags_get(common) & (~BATADV_TT_SYNC_MASK)) | orig->flags;
 	last_ttvn = READ_ONCE(orig->orig_node->last_ttvn);
 
 	if (nla_put(msg, BATADV_ATTR_TT_ADDRESS, ETH_ALEN, common->addr) ||
@@ -2212,9 +2298,12 @@ batadv_tt_global_del_roaming(struct batadv_priv *bat_priv,
 	rcu_read_unlock();
 
 	if (last_entry) {
-		/* its the last one, mark for roaming. */
-		tt_global_entry->common.flags |= BATADV_TT_CLIENT_ROAM;
-		tt_global_entry->roam_at = jiffies;
+		scoped_guard(spinlock_bh, &tt_global_entry->common.flags_lock) {
+			/* its the last one, mark for roaming. */
+			tt_global_entry->roam_at = jiffies;
+
+			tt_global_entry->common.flags |= BATADV_TT_CLIENT_ROAM;
+		}
 	} else {
 		/* there is another entry, we can simply delete this
 		 * one and can still use the other one.
@@ -2363,16 +2452,18 @@ static bool batadv_tt_global_to_purge(struct batadv_tt_global_entry *tt_global,
 	unsigned long temp_timeout = BATADV_TT_CLIENT_TEMP_TIMEOUT;
 	bool purge = false;
 
-	if ((tt_global->common.flags & BATADV_TT_CLIENT_ROAM) &&
-	    batadv_has_timed_out(tt_global->roam_at, roam_timeout)) {
-		purge = true;
-		*msg = "Roaming timeout\n";
-	}
+	scoped_guard(spinlock_bh, &tt_global->common.flags_lock) {
+		if ((tt_global->common.flags & BATADV_TT_CLIENT_ROAM) &&
+		    batadv_has_timed_out(tt_global->roam_at, roam_timeout)) {
+			purge = true;
+			*msg = "Roaming timeout\n";
+		}
 
-	if ((tt_global->common.flags & BATADV_TT_CLIENT_TEMP) &&
-	    batadv_has_timed_out(tt_global->common.added_at, temp_timeout)) {
-		purge = true;
-		*msg = "Temporary client timeout\n";
+		if ((tt_global->common.flags & BATADV_TT_CLIENT_TEMP) &&
+		    batadv_has_timed_out(tt_global->common.added_at, temp_timeout)) {
+			purge = true;
+			*msg = "Temporary client timeout\n";
+		}
 	}
 
 	return purge;
@@ -2481,13 +2572,19 @@ static bool
 _batadv_is_ap_isolated(struct batadv_tt_local_entry *tt_local_entry,
 		       struct batadv_tt_global_entry *tt_global_entry)
 {
-	if (tt_local_entry->common.flags & BATADV_TT_CLIENT_WIFI &&
-	    tt_global_entry->common.flags & BATADV_TT_CLIENT_WIFI)
+	u16 global_flags;
+	u16 local_flags;
+
+	global_flags = batadv_tt_flags_get(&tt_global_entry->common);
+	local_flags = batadv_tt_flags_get(&tt_local_entry->common);
+
+	if (local_flags & BATADV_TT_CLIENT_WIFI &&
+	    global_flags & BATADV_TT_CLIENT_WIFI)
 		return true;
 
 	/* check if the two clients are marked as isolated */
-	if (tt_local_entry->common.flags & BATADV_TT_CLIENT_ISOLA &&
-	    tt_global_entry->common.flags & BATADV_TT_CLIENT_ISOLA)
+	if (local_flags & BATADV_TT_CLIENT_ISOLA &&
+	    global_flags & BATADV_TT_CLIENT_ISOLA)
 		return true;
 
 	return false;
@@ -2516,11 +2613,15 @@ struct batadv_orig_node *batadv_transtable_search(struct batadv_priv *bat_priv,
 	struct batadv_tt_local_entry *tt_local_entry = NULL;
 	struct batadv_tt_orig_list_entry *best_entry;
 	struct batadv_orig_node *orig_node = NULL;
+	u16 flags;
 
 	if (src && batadv_vlan_ap_isola_get(bat_priv, vid)) {
 		tt_local_entry = batadv_tt_local_hash_find(bat_priv, src, vid);
-		if (!tt_local_entry ||
-		    (tt_local_entry->common.flags & BATADV_TT_CLIENT_PENDING))
+		if (!tt_local_entry)
+			goto out;
+
+		flags = batadv_tt_flags_get(&tt_local_entry->common);
+		if (flags & BATADV_TT_CLIENT_PENDING)
 			goto out;
 	}
 
@@ -2595,6 +2696,8 @@ static u32 batadv_tt_global_crc(struct batadv_priv *bat_priv,
 
 		rcu_read_lock();
 		hlist_for_each_entry_rcu(tt_common, head, hash_entry) {
+			u16 tt_flags;
+
 			tt_global = container_of(tt_common,
 						 struct batadv_tt_global_entry,
 						 common);
@@ -2604,18 +2707,21 @@ static u32 batadv_tt_global_crc(struct batadv_priv *bat_priv,
 			if (tt_common->vid != vid)
 				continue;
 
+			tt_flags = batadv_tt_flags_get(tt_common);
+
 			/* Roaming clients are in the global table for
 			 * consistency only. They don't have to be
 			 * taken into account while computing the
 			 * global crc
 			 */
-			if (tt_common->flags & BATADV_TT_CLIENT_ROAM)
+			if (tt_flags & BATADV_TT_CLIENT_ROAM)
 				continue;
+
 			/* Temporary clients have not been announced yet, so
 			 * they have to be skipped while computing the global
 			 * crc
 			 */
-			if (tt_common->flags & BATADV_TT_CLIENT_TEMP)
+			if (tt_flags & BATADV_TT_CLIENT_TEMP)
 				continue;
 
 			/* find out if this global entry is announced by this
@@ -2675,18 +2781,24 @@ static u32 batadv_tt_local_crc(struct batadv_priv *bat_priv,
 
 		rcu_read_lock();
 		hlist_for_each_entry_rcu(tt_common, head, hash_entry) {
+			u16 tt_flags;
+
 			/* compute the CRC only for entries belonging to the
 			 * VLAN identified by vid
 			 */
 			if (tt_common->vid != vid)
 				continue;
 
+			tt_flags = batadv_tt_flags_get(tt_common);
+
 			/* not yet committed clients have not to be taken into
 			 * account while computing the CRC
 			 */
-			if (tt_common->flags & BATADV_TT_CLIENT_NEW)
+			if (tt_flags & BATADV_TT_CLIENT_NEW)
 				continue;
 
+			flags = tt_flags & BATADV_TT_SYNC_MASK;
+
 			/* use network order to read the VID: this ensures that
 			 * every node reads the bytes in the same order.
 			 */
@@ -2696,7 +2808,6 @@ static u32 batadv_tt_local_crc(struct batadv_priv *bat_priv,
 			/* compute the CRC on flags that have to be kept in sync
 			 * among nodes
 			 */
-			flags = tt_common->flags & BATADV_TT_SYNC_MASK;
 			crc_tmp = crc32c(crc_tmp, &flags, sizeof(flags));
 
 			crc ^= crc32c(crc_tmp, tt_common->addr, ETH_ALEN);
@@ -2854,17 +2965,19 @@ batadv_tt_req_node_new(struct batadv_priv *bat_priv,
  *
  * Return: true if the entry is a valid, false otherwise.
  */
-static bool batadv_tt_local_valid(const void *entry_ptr,
+static bool batadv_tt_local_valid(void *entry_ptr,
 				  const void *data_ptr,
 				  u8 *flags)
 {
-	const struct batadv_tt_common_entry *tt_common_entry = entry_ptr;
+	struct batadv_tt_common_entry *tt_common_entry = entry_ptr;
+	u16 tt_flags;
 
-	if (tt_common_entry->flags & BATADV_TT_CLIENT_NEW)
+	tt_flags = batadv_tt_flags_get(tt_common_entry);
+	if (tt_flags & BATADV_TT_CLIENT_NEW)
 		return false;
 
 	if (flags)
-		*flags = tt_common_entry->flags;
+		*flags = tt_flags;
 
 	return true;
 }
@@ -2881,16 +2994,18 @@ static bool batadv_tt_local_valid(const void *entry_ptr,
  *
  * Return: true if the entry is a valid, false otherwise.
  */
-static bool batadv_tt_global_valid(const void *entry_ptr,
+static bool batadv_tt_global_valid(void *entry_ptr,
 				   const void *data_ptr,
 				   u8 *flags)
 {
-	const struct batadv_tt_common_entry *tt_common_entry = entry_ptr;
-	const struct batadv_tt_global_entry *tt_global_entry;
+	struct batadv_tt_common_entry *tt_common_entry = entry_ptr;
 	const struct batadv_orig_node *orig_node = data_ptr;
+	struct batadv_tt_global_entry *tt_global_entry;
+	u16 tt_flags;
 
-	if (tt_common_entry->flags & BATADV_TT_CLIENT_ROAM ||
-	    tt_common_entry->flags & BATADV_TT_CLIENT_TEMP)
+	tt_flags = batadv_tt_flags_get(tt_common_entry);
+	if (tt_flags & BATADV_TT_CLIENT_ROAM ||
+	    tt_flags & BATADV_TT_CLIENT_TEMP)
 		return false;
 
 	tt_global_entry = container_of(tt_common_entry,
@@ -2919,7 +3034,7 @@ static bool batadv_tt_global_valid(const void *entry_ptr,
 static u16 batadv_tt_tvlv_generate(struct batadv_priv *bat_priv,
 				   struct batadv_hashtable *hash,
 				   void *tvlv_buff, u16 tt_len,
-				   bool (*valid_cb)(const void *,
+				   bool (*valid_cb)(void *,
 						    const void *,
 						    u8 *flags),
 				   void *cb_data)
@@ -3544,17 +3659,20 @@ bool batadv_is_my_client(struct batadv_priv *bat_priv, const u8 *addr,
 			 unsigned short vid)
 {
 	struct batadv_tt_local_entry *tt_local_entry;
+	u16 tt_flags;
 	bool ret;
 
 	tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr, vid);
 	if (!tt_local_entry)
 		return false;
 
+	tt_flags = batadv_tt_flags_get(&tt_local_entry->common);
+
 	/* Check if the client has been logically deleted (but is kept for
 	 * consistency purpose)
 	 */
-	ret = !((tt_local_entry->common.flags & BATADV_TT_CLIENT_PENDING) ||
-		(tt_local_entry->common.flags & BATADV_TT_CLIENT_ROAM));
+	ret = !((tt_flags & BATADV_TT_CLIENT_PENDING) ||
+		(tt_flags & BATADV_TT_CLIENT_ROAM));
 
 	batadv_tt_local_entry_put(tt_local_entry);
 	return ret;
@@ -3834,10 +3952,19 @@ static void batadv_tt_local_transition_new(struct batadv_priv *bat_priv)
 		rcu_read_lock();
 		hlist_for_each_entry_rcu(tt_common_entry,
 					 head, hash_entry) {
-			if (!(tt_common_entry->flags & BATADV_TT_CLIENT_NEW))
-				continue;
+			bool cont = false;
+
+			scoped_guard(spinlock_bh, &tt_common_entry->flags_lock) {
+				if (!(tt_common_entry->flags & BATADV_TT_CLIENT_NEW)) {
+					cont = true;
+					break;
+				}
 
-			tt_common_entry->flags &= ~BATADV_TT_CLIENT_NEW;
+				tt_common_entry->flags &= ~BATADV_TT_CLIENT_NEW;
+			}
+
+			if (cont)
+				continue;
 
 			batadv_tt_local_size_inc(bat_priv,
 						 tt_common_entry->vid);
@@ -3874,16 +4001,26 @@ static void batadv_tt_local_purge_pending_clients(struct batadv_priv *bat_priv)
 		spin_lock_bh(list_lock);
 		hlist_for_each_entry_safe(tt_common, node_tmp, head,
 					  hash_entry) {
-			if (!(tt_common->flags & BATADV_TT_CLIENT_PENDING))
-				continue;
+			bool cont = false;
 
-			batadv_dbg(BATADV_DBG_TT, bat_priv,
-				   "Deleting local tt entry (%pM, vid: %d): pending\n",
-				   tt_common->addr,
-				   batadv_print_vid(tt_common->vid));
+			scoped_guard(spinlock_bh, &tt_common->flags_lock) {
+				if (!(tt_common->flags & BATADV_TT_CLIENT_PENDING)) {
+					cont = true;
+					break;
+				}
+
+				batadv_dbg(BATADV_DBG_TT, bat_priv,
+					   "Deleting local tt entry (%pM, vid: %d): pending\n",
+					   tt_common->addr,
+					   batadv_print_vid(tt_common->vid));
+
+				batadv_tt_local_size_dec(bat_priv, tt_common->vid);
+				hlist_del_rcu(&tt_common->hash_entry);
+			}
+
+			if (cont)
+				continue;
 
-			batadv_tt_local_size_dec(bat_priv, tt_common->vid);
-			hlist_del_rcu(&tt_common->hash_entry);
 			tt_local = container_of(tt_common,
 						struct batadv_tt_local_entry,
 						common);
@@ -4087,7 +4224,8 @@ bool batadv_tt_global_client_is_roaming(struct batadv_priv *bat_priv,
 	if (!tt_global_entry)
 		return false;
 
-	ret = tt_global_entry->common.flags & BATADV_TT_CLIENT_ROAM;
+	ret = batadv_tt_flags_get(&tt_global_entry->common) & BATADV_TT_CLIENT_ROAM;
+
 	batadv_tt_global_entry_put(tt_global_entry);
 
 	return ret;
@@ -4113,7 +4251,8 @@ bool batadv_tt_local_client_is_roaming(struct batadv_priv *bat_priv,
 	if (!tt_local_entry)
 		return false;
 
-	ret = tt_local_entry->common.flags & BATADV_TT_CLIENT_ROAM;
+	ret = batadv_tt_flags_get(&tt_local_entry->common) & BATADV_TT_CLIENT_ROAM;
+
 	batadv_tt_local_entry_put(tt_local_entry);
 
 	return ret;
@@ -4424,7 +4563,7 @@ bool batadv_tt_global_is_isolated(struct batadv_priv *bat_priv,
 	if (!tt)
 		return false;
 
-	ret = tt->common.flags & BATADV_TT_CLIENT_ISOLA;
+	ret = batadv_tt_flags_get(&tt->common) & BATADV_TT_CLIENT_ISOLA;
 
 	batadv_tt_global_entry_put(tt);
 
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index c42a4aa8f41a3..67872927cfb50 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -1851,12 +1851,21 @@ struct batadv_tt_common_entry {
 	 */
 	struct hlist_node hash_entry;
 
-	/** @flags: various state handling flags (see batadv_tt_client_flags) */
+	/**
+	 * @flags: various state handling flags (see batadv_tt_client_flags),
+	 * protected by @flags_lock
+	 */
 	u16 flags;
 
 	/** @added_at: timestamp used for purging stale tt common entries */
 	unsigned long added_at;
 
+	/**
+	 * @flags_lock: protect modifications of @flags and
+	 *  @batadv_tt_global_entry.roam_at
+	 */
+	spinlock_t flags_lock;
+
 	/** @refcount: number of contexts the object is used */
 	struct kref refcount;
 
-- 
2.47.3

Re: [PATCH net-next 12/15] batman-adv: tt: use protected flag modifications

[Sven Eckelmann]

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

On Tuesday, 1 September 2026 16:00:02 CEST sashiko-bot@kernel.org wrote:
[...]
> > @@ -1351,10 +1391,17 @@ u16 batadv_tt_local_remove(struct batadv_priv *bat_priv, const u8 *addr,
> >  	}
> >  
> >  	if (!(tt_local_entry->common.flags & BATADV_TT_CLIENT_NEW)) {
> > -		batadv_tt_local_set_pending(bat_priv, tt_local_entry, flags,
> > -					    message);
> > +		tt_local_entry->common.flags |= BATADV_TT_CLIENT_PENDING;
> > +		pending = true;
> > +	}
> > +	spin_unlock_bh(&tt_local_entry->common.flags_lock);
> > +
> > +	if (pending) {
> > +		batadv_tt_local_set_pending_event(bat_priv, tt_local_entry, flags,
> > +						  message);
> 
> [Severity: High]
> Does detaching the DEL event emission from the flag update create a
> split-brain routing state?
> 
> By setting the BATADV_TT_CLIENT_PENDING flag under the flags_lock but
> delaying the emission of the DEL event (via
> batadv_tt_local_set_pending_event) until after the lock is released,
> there is a window where concurrent operations can interleave.
> 
> If a concurrent batadv_tt_local_add() executes within this window, it
> will clear the PENDING flag and queue an ADD event:
> 
> CPU1: batadv_tt_local_remove()
>     tt_local_entry->common.flags |= BATADV_TT_CLIENT_PENDING;
>     spin_unlock_bh(&tt_local_entry->common.flags_lock);
>     // Preempted
> 
> CPU2: batadv_tt_local_add()
>     tt_local->common.flags &= ~BATADV_TT_CLIENT_PENDING;
>     spin_unlock_bh(&tt_local->common.flags_lock);
>     batadv_tt_local_event(bat_priv, tt_local, BATADV_NO_FLAGS); // Queues ADD
> 
> If the OGM worker runs now, it broadcasts the ADD.
> 
> CPU1: batadv_tt_local_remove() (resumes)
>     batadv_tt_local_set_pending_event(..., BATADV_TT_CLIENT_DEL, ...);
> 
> The next OGM broadcasts the DEL event. The local node considers the MAC
> active (because PENDING was cleared), but the rest of the mesh drops it
> due to the DEL event, leading to persistent split-brain routing.

This is not how TT works. The events are a best effort in informing other 
originators. But it is not the source of all truth.

And this is not changed by this patch.

> > @@ -1394,21 +1441,37 @@ static void batadv_tt_local_purge_list(struct batadv_priv *bat_priv,
[...]
> > -		batadv_tt_local_set_pending(bat_priv, tt_local_entry,
> > -					    BATADV_TT_CLIENT_DEL, "timed out");
> > +		batadv_tt_local_set_pending_event(bat_priv, tt_local_entry,
> > +						  BATADV_TT_CLIENT_DEL, "timed out");
> 
> [Severity: High]
> Can this sequence similarly cause a split-brain state?
> 
> Like in batadv_tt_local_remove(), this delays the DEL event emission
> until after the lock is dropped. A concurrent batadv_tt_local_add()
> could interleave during this window, resulting in the local node
> keeping the entry active while the rest of the mesh removes it.

This is not how TT works. The events are a best effort in informing other 
originators. But it is not the source of all truth.

And this is not changed by this patch.

Regards,
	Sven

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

Re: [PATCH net-next 12/15] batman-adv: tt: use protected flag modifications

[Sven Eckelmann]

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

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


> > diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> > index fbe75e184153..163f90962306 100644
> > --- a/net/batman-adv/translation-table.c
> > +++ b/net/batman-adv/translation-table.c
> 
> [ ... ]
> 
> > @@ -633,8 +652,23 @@ static void batadv_tt_local_add_roam(struct batadv_priv *bat_priv,
> >  	/* Check whether it is a roaming, but don't do anything if the roaming
> >  	 * process has already been handled
> >  	 */
> > -	if (tt_global->common.flags & BATADV_TT_CLIENT_ROAM)
> > -		return;
> > +	scoped_guard(spinlock_bh, &tt_global->common.flags_lock) {
> > +		if (tt_global->common.flags & BATADV_TT_CLIENT_ROAM)
> > +			return;
> > +
> > +		if (!roamed_back) {
> > +			/* The global entry has to be marked as ROAMING and has to be
> > +			 * kept for consistency purpose.
> > +			 *
> > +			 * batadv_tt_global_to_purge() evaluates roam_at as soon as it
> > +			 * observes BATADV_TT_CLIENT_ROAM, so the timeout has to be
> > +			 * stamped before the flag is published. Otherwise the entry can
> > +			 * be deleted right away as "Roaming timeout".
> > +			 */
> > +			tt_global->roam_at = jiffies;
> > +			tt_global->common.flags |= BATADV_TT_CLIENT_ROAM;
> > +		}
> > +	}
> 
> Discovered by: gpt-5-6-sol, opus-5 · Missed by: sashiko-gemini
> Should the changelog mention this ordering fix, and should the patch carry
> a Fixes: tag?

I am not allowed to add Fixes: lines for patches targeting net-next. And this 
is not suitable for net or stable backports. See
https://lore.kernel.org/all/7d73cd74-b040-4a3d-9d78-4ad8fc0a01f4@redhat.com/


> 
> The commit message describes the change as lost-update hardening with a
> self-correcting symptom:
> 
>     This problem can usually be fixed at a later point when the flags are
>     tried to be adjusted again.
> 
>     To reduce the time the wrong flags are used, [...]
> 
> But the new comment above documents something stronger: the store of
> tt_global->roam_at is now moved ahead of publishing
> BATADV_TT_CLIENT_ROAM, and batadv_tt_global_to_purge() is brought under
> the same flags_lock. The same reordering is applied a second time in
> batadv_tt_global_del_roaming().
[...]

The ordering is not too relevant because they are now both under the same 
lock. Which is more important because roam_at is tied to the flag 
BATADV_TT_CLIENT_ROAM. So I dont' seem why they should now be splitted or 
handled separately.


> > @@ -1285,29 +1329,23 @@ int batadv_tt_local_dump(struct sk_buff *msg, struct netlink_callback *cb)
> >  }
> >  
> >  /**
> > - * batadv_tt_local_set_pending() - mark a local TT entry as pending removal
> > + * batadv_tt_local_set_pending_event() - trigger events for TT pending removal
> >   * @bat_priv: the bat priv with all the mesh interface information
> >   * @tt_local_entry: local TT entry to mark
> >   * @flags: TT change flags to announce together with the pending removal
> >   * @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.
> > + * Schedule the TT change announcement for 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.
> >   */
> 
> Discovered by: gpt-5-6-sol, opus-5 · Missed by: sashiko-gemini
> The kernel-doc of batadv_tt_local_set_pending_event() still says
> "@tt_local_entry: local TT entry to mark" and still claims the entry "is
> kept in the local table until the next TTVN increment", but the body only
> calls batadv_tt_local_event() and batadv_dbg() now - the
> BATADV_TT_CLIENT_PENDING store moved into both callers.

https://patch.msgid.link/20260901-tt-pending-event-kdoc-v1-1-0454b0799d48@narfation.org

> 
> > @@ -1331,12 +1369,14 @@ u16 batadv_tt_local_remove(struct batadv_priv *bat_priv, const u8 *addr,
> >  	struct batadv_tt_local_entry *tt_local_entry;
> >  	struct hlist_node *tt_removed_node;
> >  	u16 curr_flags = BATADV_NO_FLAGS;
> > +	bool pending = false;
> >  	u16 flags;
> >  
> >  	tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr, vid);
> >  	if (!tt_local_entry)
> >  		goto out;
> >  
> > +	spin_lock_bh(&tt_local_entry->common.flags_lock);
> >  	curr_flags = tt_local_entry->common.flags;
> >  
> >  	flags = BATADV_TT_CLIENT_DEL;
> > @@ -1351,10 +1391,17 @@ u16 batadv_tt_local_remove(struct batadv_priv *bat_priv, const u8 *addr,
> >  	}
> >  
> >  	if (!(tt_local_entry->common.flags & BATADV_TT_CLIENT_NEW)) {
> > -		batadv_tt_local_set_pending(bat_priv, tt_local_entry, flags,
> > -					    message);
> > +		tt_local_entry->common.flags |= BATADV_TT_CLIENT_PENDING;
> > +		pending = true;
> > +	}
> > +	spin_unlock_bh(&tt_local_entry->common.flags_lock);
> > +
> > +	if (pending) {
> > +		batadv_tt_local_set_pending_event(bat_priv, tt_local_entry, flags,
> > +						  message);
> >  		goto out;
> >  	}
> 
> Does this invert the publication order that the old
> batadv_tt_local_set_pending() guaranteed?
> 
> Previously the DEL change event was queued first and
> BATADV_TT_CLIENT_PENDING was set afterwards. Now the flag becomes
> visible before batadv_tt_local_event() runs, and
> batadv_tt_local_remove() holds neither the hash bucket list_lock nor
> bat_priv->tt.commit_lock:
> 
> CPU0                                    CPU1
> batadv_tt_local_remove()
>   spin_lock_bh(&...flags_lock);
>   flags |= BATADV_TT_CLIENT_PENDING;
>   spin_unlock_bh(&...flags_lock);
>                                         batadv_tt_local_commit_changes()
>                                           batadv_tt_local_purge_pending_clients()
>                                             hlist_del_rcu(&tt_common->hash_entry);
>                                           batadv_tt_local_update_crc()
>                                           atomic_inc(&bat_priv->tt.vn);
>   batadv_tt_local_set_pending_event()
>     batadv_tt_local_event()   /* DEL queued only now */
> 
> Can the client then vanish from the local table and from the CRC of TTVN
> N while no DEL change was announced for TTVN N, leaving the neighbours to
> recover via a full table TT request after their CRC check fails?
> 
> The sibling caller batadv_tt_local_purge_list() does not have this
> problem because batadv_tt_local_purge() holds the bucket list_lock around
> it, but batadv_tt_local_remove() is also reachable from received TT TVLV
> and roaming data via batadv_tt_global_add(), so both sides can run
> concurrently.
> 
> Would it help to set BATADV_TT_CLIENT_PENDING after
> batadv_tt_local_event() has queued the change, or to publish the flag and
> the event under a common lock?

The old code didn't guarantee anything and wasn't really ensuring the order 
other callers see it. And it is unfortunately not too trivial as moving the 
position because it is not known earlier if the entry is pending or not. 
Something more like this would then be required:


diff --git i/net/batman-adv/translation-table.c w/net/batman-adv/translation-table.c
index 1fcbe417..b008c8b1 100644
--- i/net/batman-adv/translation-table.c
+++ w/net/batman-adv/translation-table.c
@@ -1498,7 +1498,9 @@ batadv_tt_local_set_pending_event(struct batadv_priv *bat_priv,
 
 /**
  * batadv_tt_local_mark_removed() - mark a local entry as removed
+ * @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
  *
@@ -1506,23 +1508,46 @@ batadv_tt_local_set_pending_event(struct batadv_priv *bat_priv,
  * 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;
 
-	scoped_guard(spinlock_bh, &common->flags_lock) {
-		*curr_flags = common->flags;
+	flags = BATADV_TT_CLIENT_DEL;
+	/* if this global entry addition is due to a roaming, the node has to
+	 * mark the local entry as "roamed" in order to correctly reroute
+	 * packets later
+	 */
+	if (roaming)
+		flags |= BATADV_TT_CLIENT_ROAM;
 
-		/* mark the local client as ROAMed */
-		if (roaming)
-			common->flags |= BATADV_TT_CLIENT_ROAM;
+	i = batadv_choose_tt(tt_local_entry, hash->size);
+	list_lock = &hash->list_locks[i];
 
-		if (!(common->flags & BATADV_TT_CLIENT_NEW)) {
-			common->flags |= BATADV_TT_CLIENT_PENDING;
-			pending = true;
+	scoped_guard(spinlock_bh, list_lock) {
+		scoped_guard(spinlock_bh, &common->flags_lock) {
+			*curr_flags = common->flags;
+
+			/* mark the local client as ROAMed */
+			if (roaming)
+				common->flags |= BATADV_TT_CLIENT_ROAM;
+
+			if (!(common->flags & BATADV_TT_CLIENT_NEW)) {
+				common->flags |= BATADV_TT_CLIENT_PENDING;
+				pending = true;
+			}
 		}
+
+		if (pending)
+			batadv_tt_local_set_pending_event(bat_priv,
+							  tt_local_entry,
+							  flags, message);
 	}
 
 	return pending;
@@ -1580,24 +1605,13 @@ 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;
 
-	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))
-		batadv_tt_local_set_pending_event(bat_priv, tt_local_entry,
-						  flags, message);
-	else
+	if (!batadv_tt_local_mark_removed(bat_priv,tt_local_entry,
+					  message, roaming, &curr_flags))
 		/* if this client has been added right now, it is possible to
 		 * immediately purge it
 		 */

[...]
> Discovered by: gpt-5-6-sol · Missed by: opus-5, sashiko-gemini · Confirmed by: opus-5
> Can deferring batadv_tt_global_del_orig_list() until after flags_lock is
> dropped lose an originator entry added in between?
> 
> The old code purged the originator list first and cleared
> BATADV_TT_CLIENT_TEMP / BATADV_TT_CLIENT_ROAM afterwards, so a second
> context still saw the flag and did its own purge. Now the cleared flag is
> published before the list is wiped:
> 
> CPU0 batadv_tt_global_add()             CPU1 batadv_tt_global_add()
>   spin_lock_bh(&common->flags_lock);
>   delete = true;
>   common->flags &= ~BATADV_TT_CLIENT_TEMP;
>   spin_unlock_bh(&common->flags_lock);
>                                         spin_lock_bh(&common->flags_lock);
>                                         /* TEMP already clear, so
>                                          * delete stays false
>                                          */
>                                         spin_unlock_bh(&common->flags_lock);
>                                         batadv_tt_global_orig_entry_add()
>   batadv_tt_global_del_orig_list()
>     /* drops every orig entry,
>      * including CPU1's fresh one
>      */
> 
> batadv_tt_global_del_orig_list() takes tt_global_entry->list_lock and
> removes all entries:
> 
> 	spin_lock_bh(&tt_global_entry->list_lock);
> 	head = &tt_global_entry->orig_list;
> 	hlist_for_each_entry_safe(orig_entry, safe, head, list)
> 		_batadv_tt_global_del_orig_entry(tt_global_entry, orig_entry);
> 
> Both callers return true in that case, so does the route via CPU1's
> originator silently disappear until the next global CRC mismatch triggers
> a fresh TT request? batadv_tt_global_add() is reached from received TT
> TVLV data (serialized only per originator by orig_node->tt_lock) and from
> batadv_tt_add_temporary_global_entry(), so two CPUs can process
> announcements for the same client from different originators at the same
> time.

I really wait until you tell me I couldn't solve world hunger and all 
conflicts on the world with this patch.
https://www.open-mesh.org/issues/474

> 
> One more note, on the sibling patch "batman-adv: clarify cut-off in
> batadv_v_neigh_is_sob kernel-doc" in this series:
> 
> Discovered by: gpt-5-6-sol · Missed by: opus-5, sashiko-gemini · Confirmed by: opus-5
> The kernel-doc of batadv_v_neigh_is_sob() in net/batman-adv/bat_v.c says
> the function returns true if the candidate throughput is "more than 3/4 of
> the @reference throughput", while the body computes:
> 
> 	threshold = ifinfo_ref->bat_v.throughput / 4;
> 	threshold = ifinfo_ref->bat_v.throughput - threshold;
> 
> 	ret = ifinfo_cand->bat_v.throughput > threshold;
> 
> With integer division that is ceil(3 * ref / 4) and a strict comparison,
> so for a reference throughput of 5 the threshold becomes 4 and a
> candidate of 4 is rejected although 4 is more than 3/4 * 5. Should the doc
> say the cut-off is the reference minus reference / 4, exclusive?

Out-of-scope. 

Regards,
	Sven

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

[PATCH net-next 13/15] batman-adv: tt: transition NEW local entries only under lock

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

The batadv_tt_local_size_inc() must never be called for an entry which was
already removed from the list. Otherwise the removal from the hash cannot
correctly determine if the batadv_tt_local_size_dec() needs to be called or
not.

This assumption is broken by the use of rcu_read_lock() in
batadv_tt_local_transition_new() because it might still see entries in the
list which were already removed by a different context from the list. If it
then increments the size counter, nothing will reduce the counter again.
Simply because the removal (responsible for the decrement) already
happened.

Over the whole time, the actual hash list spinlock must be held when
transitioning NEW local entries to avoid list manipulations.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/translation-table.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 163f909623069..a870d9a97e329 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -333,6 +333,10 @@ static void batadv_tt_local_size_mod(struct batadv_priv *bat_priv,
  *  given vid
  * @bat_priv: the bat priv with all the mesh interface information
  * @vid: the VLAN identifier
+ *
+ * It must only be called when removing the NEW flag of a
+ * batadv_tt_local_entry while it is still part of the bat_priv->tt.local_hash.
+ * It must therefore be checked under the specific list_locks[i].
  */
 static void batadv_tt_local_size_inc(struct batadv_priv *bat_priv,
 				     unsigned short vid)
@@ -3938,6 +3942,7 @@ void batadv_tt_free(struct batadv_priv *bat_priv)
  */
 static void batadv_tt_local_transition_new(struct batadv_priv *bat_priv)
 {
+	spinlock_t *list_lock; /* protects write access to the hash lists */
 	struct batadv_hashtable *hash = bat_priv->tt.local_hash;
 	struct batadv_tt_common_entry *tt_common_entry;
 	struct hlist_head *head;
@@ -3948,10 +3953,10 @@ static void batadv_tt_local_transition_new(struct batadv_priv *bat_priv)
 
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
+		list_lock = &hash->list_locks[i];
 
-		rcu_read_lock();
-		hlist_for_each_entry_rcu(tt_common_entry,
-					 head, hash_entry) {
+		spin_lock_bh(list_lock);
+		hlist_for_each_entry(tt_common_entry, head, hash_entry) {
 			bool cont = false;
 
 			scoped_guard(spinlock_bh, &tt_common_entry->flags_lock) {
@@ -3969,7 +3974,7 @@ static void batadv_tt_local_transition_new(struct batadv_priv *bat_priv)
 			batadv_tt_local_size_inc(bat_priv,
 						 tt_common_entry->vid);
 		}
-		rcu_read_unlock();
+		spin_unlock_bh(list_lock);
 	}
 }
 
-- 
2.47.3

Re: [PATCH net-next 13/15] batman-adv: tt: transition NEW local entries only under lock

[Sven Eckelmann]

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

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


[...]
> Should a Fixes: tag naming that commit be added?
> 
> Note that this patch depends on the tt_common_entry->flags_lock
> infrastructure added earlier in the same series, so a direct stable
> backport would need adaptation and the tag would mostly serve
> identification.

I am not allowed to add Fixes: lines for patches targeting net-next. And this 
is not suitable for net or stable backports. See
https://lore.kernel.org/all/7d73cd74-b040-4a3d-9d78-4ad8fc0a01f4@redhat.com/


> 
> > diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> > index 163f90962306..a870d9a97e32 100644
> > --- a/net/batman-adv/translation-table.c
> > +++ b/net/batman-adv/translation-table.c
> > @@ -333,6 +333,10 @@ static void batadv_tt_local_size_mod(struct batadv_priv *bat_priv,
> >   *  given vid
> >   * @bat_priv: the bat priv with all the mesh interface information
> >   * @vid: the VLAN identifier
> > + *
> > + * It must only be called when removing the NEW flag of a
> > + * batadv_tt_local_entry while it is still part of the bat_priv->tt.local_hash.
> > + * It must therefore be checked under the specific list_locks[i].
> >   */
> >  static void batadv_tt_local_size_inc(struct batadv_priv *bat_priv,
> >  				     unsigned short vid)
> 
> Discovered by: gpt-5-6-sol, opus-5 · Missed by: sashiko-gemini
> This is a pre-existing issue and not introduced by this patch, but does
> the new invariant ("It must therefore be checked under the specific
> list_locks[i]") hold on the removal side as well?
> 
> batadv_tt_local_remove() decides that no batadv_tt_local_size_dec() is
> owed while holding only flags_lock, then drops it before unlinking:
[...]
> Can vlan->tt.num_entries stay inflated by one in that case, the same
> "nothing will reduce the counter again" failure mode, reached through
> the check side instead of the walk side?


Out-of-scope for this specific patch. See patch
"batman-adv: tt: decrement count for committed client on local_remove"

Regards,
	Sven

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

[PATCH net-next 14/15] batman-adv: tt: don't uncount never committed clients on pending purge

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

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

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

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, 10 insertions(+), 1 deletion(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index a870d9a97e329..794692170b611 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -349,6 +349,10 @@ static void batadv_tt_local_size_inc(struct batadv_priv *bat_priv,
  *  given vid
  * @bat_priv: the bat priv with all the mesh interface information
  * @vid: the VLAN identifier
+ *
+ * It must only be called after a batadv_tt_local_entry without NEW flag
+ * was removed from bat_priv->tt.local_hash (under the specific
+ * list_locks[i]).
  */
 static void batadv_tt_local_size_dec(struct batadv_priv *bat_priv,
 				     unsigned short vid)
@@ -4019,8 +4023,13 @@ static void batadv_tt_local_purge_pending_clients(struct batadv_priv *bat_priv)
 					   tt_common->addr,
 					   batadv_print_vid(tt_common->vid));
 
-				batadv_tt_local_size_dec(bat_priv, tt_common->vid);
 				hlist_del_rcu(&tt_common->hash_entry);
+
+				/* An entry which still carries BATADV_TT_CLIENT_NEW was
+				 * never counted and must not be uncounted here.
+				 */
+				if (!(tt_common->flags & BATADV_TT_CLIENT_NEW))
+					batadv_tt_local_size_dec(bat_priv, tt_common->vid);
 			}
 
 			if (cont)
-- 
2.47.3

Re: [PATCH net-next 14/15] batman-adv: tt: don't uncount never committed clients on pending purge

[Sven Eckelmann]

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

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

> > diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> > index a870d9a97e32..794692170b61 100644
> > --- a/net/batman-adv/translation-table.c
> > +++ b/net/batman-adv/translation-table.c
> 
> [ ... ]
> 
> > @@ -4019,8 +4023,13 @@ static void batadv_tt_local_purge_pending_clients(struct batadv_priv *bat_priv)
> >  					   tt_common->addr,
> >  					   batadv_print_vid(tt_common->vid));
> >  
> > -				batadv_tt_local_size_dec(bat_priv, tt_common->vid);
> >  				hlist_del_rcu(&tt_common->hash_entry);
> > +
> > +				/* An entry which still carries BATADV_TT_CLIENT_NEW was
> > +				 * never counted and must not be uncounted here.
> > +				 */
> > +				if (!(tt_common->flags & BATADV_TT_CLIENT_NEW))
> > +					batadv_tt_local_size_dec(bat_priv, tt_common->vid);
> >  			}
> 
> Discovered by: gpt-5-6-sol, opus-5, sonnet-5 · Missed by: sashiko-gemini
> Should this carry a Fixes: tag and a Cc: stable hint?


I am not allowed to add Fixes: lines for patches targeting net-next. And this 
is not suitable for net or stable backports. See
https://lore.kernel.org/all/7d73cd74-b040-4a3d-9d78-4ad8fc0a01f4@redhat.com/

Regards,
	Sven

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

[PATCH net-next 15/15] batman-adv: tt: decrement count for committed client on local_remove

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

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

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

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 794692170b611..44a8cf89382c6 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -352,7 +352,8 @@ static void batadv_tt_local_size_inc(struct batadv_priv *bat_priv,
  *
  * It must only be called after a batadv_tt_local_entry without NEW flag
  * was removed from bat_priv->tt.local_hash (under the specific
- * list_locks[i]).
+ * list_locks[i]) or when it is ensured that it is not accessible
+ * in this list.
  */
 static void batadv_tt_local_size_dec(struct batadv_priv *bat_priv,
 				     unsigned short vid)
@@ -1423,6 +1424,13 @@ u16 batadv_tt_local_remove(struct batadv_priv *bat_priv, const u8 *addr,
 	if (!tt_removed_node)
 		goto out;
 
+	/* batadv_tt_local_transition_new() may have committed the entry and
+	 * thus counted it in the local table size since the BATADV_TT_CLIENT_NEW
+	 * check above.
+	 */
+	if (!(batadv_tt_flags_get(&tt_local_entry->common) & BATADV_TT_CLIENT_NEW))
+		batadv_tt_local_size_dec(bat_priv, tt_local_entry->common.vid);
+
 	/* drop reference of remove hash entry */
 	batadv_tt_local_entry_put(tt_local_entry);
 
-- 
2.47.3

Re: [PATCH net-next 15/15] batman-adv: tt: decrement count for committed client on local_remove

[Sven Eckelmann]

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

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


> > diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> > index 794692170b61..44a8cf89382c 100644
> > --- a/net/batman-adv/translation-table.c
> > +++ b/net/batman-adv/translation-table.c
> 
> [ ... ]
> 
> > @@ -1423,6 +1424,13 @@ u16 batadv_tt_local_remove(struct batadv_priv *bat_priv, const u8 *addr,
> >  	if (!tt_removed_node)
> >  		goto out;
> >  
> > +	/* batadv_tt_local_transition_new() may have committed the entry and
> > +	 * thus counted it in the local table size since the BATADV_TT_CLIENT_NEW
> > +	 * check above.
> > +	 */
> > +	if (!(batadv_tt_flags_get(&tt_local_entry->common) & BATADV_TT_CLIENT_NEW))
> > +		batadv_tt_local_size_dec(bat_priv, tt_local_entry->common.vid);
> > +
> 
> Discovered by: gpt-5-6-sol, opus-5 · Missed by: sashiko-gemini
> Should this change carry a Fixes: tag?


I am not allowed to add Fixes: lines for patches targeting net-next. And this 
is not suitable for net or stable backports. See
https://lore.kernel.org/all/7d73cd74-b040-4a3d-9d78-4ad8fc0a01f4@redhat.com/


Regards,
Sven

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