* [PATCH batadv 1/4] batman-adv: tt: don't queue local events for unhashed entries
2026-10-07 10:29 [PATCH batadv 0/4] batman-adv: tt: shashiko 20260930 follow ups Sven Eckelmann
@ 2026-10-07 10:29 ` Sven Eckelmann
2026-10-07 10:29 ` [PATCH batadv 2/4] batman-adv: tt: only update flags for TT events Sven Eckelmann
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Sven Eckelmann @ 2026-10-07 10:29 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Sven Eckelmann, Sashiko
batadv_tt_local_add() and batadv_tt_local_remove() look up the local entry
via RCU only. batadv_tt_local_purge_pending_clients() or
batadv_tt_local_remove_now() can unhash it before the entry is modified and
an event is queued for it:
CPU0 CPU1
/* DEL#1 queued, PENDING set */
batadv_tt_local_hash_find()
batadv_tt_local_commit_changes()
..._purge_pending_clients()
hlist_del_rcu(&...->hash_entry);
/* DEL#1 announced */
batadv_tt_local_add_existing()
flags &= ~..._PENDING;
/* ADD queued for unhashed entry */
The queued event will then be announced for a client which is not part
anymore of the table (+ its CRC). A full table request in this and similar
scenarios is required to fix the inconsistency.
Only the hash bucket list_lock can ensure that an entry is still part of
the local table. Local entries are therefore unhashed with
hlist_del_init_rcu() to make hlist_unhashed() usable under this lock.
batadv_tt_local_add_existing() + batadv_tt_local_add_existing() need to be
merged together to ensure only a single locked region for
batadv_tt_local_add() instead of multiple ones.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930094558.3723766-2-sw%40simonwunderlich.de?part=7
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
net/batman-adv/translation-table.c | 206 +++++++++++++++++++++----------------
1 file changed, 115 insertions(+), 91 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index e46040fd..b545001b 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -925,55 +925,6 @@ static bool batadv_tt_iif_is_wifi(struct net *net, int ifindex)
return batadv_is_wifi(wifi_flags);
}
-/**
- * batadv_tt_local_add_existing() - refresh an already known local TT entry
- * @bat_priv: the bat priv with all the mesh interface information
- * @tt_local: the local TT entry which was found in the local table
- * @roamed_back: set to true when the client returned to its original location
- *
- * Return: true when the client has to be announced to the mesh again, false
- * otherwise.
- */
-static bool batadv_tt_local_add_existing(struct batadv_priv *bat_priv,
- struct batadv_tt_local_entry *tt_local,
- bool *roamed_back)
-{
- struct batadv_tt_common_entry *common = &tt_local->common;
-
- tt_local->last_seen = jiffies;
-
- scoped_guard(spinlock_bh, &common->flags_lock) {
- if (common->flags & BATADV_TT_CLIENT_PENDING) {
- batadv_dbg(BATADV_DBG_TT, bat_priv,
- "Re-adding pending client %pM (vid: %d)\n",
- common->addr, batadv_print_vid(common->vid));
- /* whatever the reason why the PENDING flag was set,
- * this is a client which was enqueued to be removed in
- * this orig_interval. Since it popped up again, the
- * flag can be reset like it was never enqueued
- */
- common->flags &= ~BATADV_TT_CLIENT_PENDING;
-
- return true;
- }
-
- if (common->flags & BATADV_TT_CLIENT_ROAM) {
- batadv_dbg(BATADV_DBG_TT, bat_priv,
- "Roaming client %pM (vid: %d) came back to its original location\n",
- common->addr, batadv_print_vid(common->vid));
- /* the ROAM flag is set because this client roamed away
- * and the node got a roaming_advertisement message. Now
- * that the client popped up again at its original
- * location such flag can be unset
- */
- common->flags &= ~BATADV_TT_CLIENT_ROAM;
- *roamed_back = true;
- }
- }
-
- return false;
-}
-
/**
* batadv_tt_local_create() - allocate and initialize a local TT entry
* @mesh_iface: netdev struct of the mesh interface
@@ -1046,27 +997,74 @@ batadv_tt_local_create(struct net_device *mesh_iface, const u8 *addr,
}
/**
- * batadv_tt_local_update_flags() - update the dynamic flags of a local entry
+ * batadv_tt_local_refresh() - refresh a local TT entry of an active client
* @bat_priv: the bat priv with all the mesh interface information
- * @tt_local: the local TT entry to update
+ * @tt_local: the local TT entry to refresh
* @iif_is_wifi: whether the client is connected via a wifi interface
* @mark: the value contained in the skb->mark field of the received packet (if
* any)
+ * @announce: whether an ADD event has to be queued even when no announced
+ * flag was modified
+ * @roamed_back: set to true when the client returned to its original location
*
- * Return: true if a flag announced to the other nodes was modified, false
- * otherwise.
+ * A pending removal of the entry is cancelled and its dynamic flags are
+ * updated. An ADD event is queued when @announce is set, the pending removal
+ * was cancelled or a flag announced to the other nodes was modified.
+ *
+ * Return: true if the entry was refreshed, false if it is no longer part of
+ * the local table.
*/
static bool
-batadv_tt_local_update_flags(struct batadv_priv *bat_priv,
- struct batadv_tt_local_entry *tt_local,
- bool iif_is_wifi, u32 mark)
+batadv_tt_local_refresh(struct batadv_priv *bat_priv,
+ struct batadv_tt_local_entry *tt_local,
+ bool iif_is_wifi, u32 mark, bool announce,
+ bool *roamed_back)
{
+ spinlock_t *list_lock; /* protects write access to the hash lists */
struct batadv_tt_common_entry *common = &tt_local->common;
+ struct batadv_hashtable *hash = bat_priv->tt.local_hash;
u8 remote_flags;
u32 match_mark;
- bool modified;
+ u32 i;
+
+ i = batadv_choose_tt(common, hash->size);
+ list_lock = &hash->list_locks[i];
+
+ tt_local->last_seen = jiffies;
+
+ spin_lock_bh(list_lock);
+
+ /* the entry was removed from the local table after it was looked up */
+ if (hlist_unhashed(&common->hash_entry)) {
+ spin_unlock_bh(list_lock);
+ return false;
+ }
scoped_guard(spinlock_bh, &common->flags_lock) {
+ if (common->flags & BATADV_TT_CLIENT_PENDING) {
+ batadv_dbg(BATADV_DBG_TT, bat_priv,
+ "Re-adding pending client %pM (vid: %d)\n",
+ common->addr, batadv_print_vid(common->vid));
+ /* whatever the reason why the PENDING flag was set,
+ * this is a client which was enqueued to be removed in
+ * this orig_interval. Since it popped up again, the
+ * flag can be reset like it was never enqueued
+ */
+ common->flags &= ~BATADV_TT_CLIENT_PENDING;
+ announce = true;
+ } else if (common->flags & BATADV_TT_CLIENT_ROAM) {
+ batadv_dbg(BATADV_DBG_TT, bat_priv,
+ "Roaming client %pM (vid: %d) came back to its original location\n",
+ common->addr, batadv_print_vid(common->vid));
+ /* the ROAM flag is set because this client roamed away
+ * and the node got a roaming_advertisement message. Now
+ * that the client popped up again at its original
+ * location such flag can be unset
+ */
+ common->flags &= ~BATADV_TT_CLIENT_ROAM;
+ *roamed_back = true;
+ }
+
/* store the current remote flags before altering them. This
* helps understanding is flags are changing or not
*/
@@ -1088,10 +1086,17 @@ batadv_tt_local_update_flags(struct batadv_priv *bat_priv,
else
common->flags &= ~BATADV_TT_CLIENT_ISOLA;
- modified = remote_flags ^ (common->flags & BATADV_TT_REMOTE_MASK);
+ if (remote_flags ^ (common->flags & BATADV_TT_REMOTE_MASK))
+ announce = true;
+
+ if (announce)
+ __batadv_tt_local_event(bat_priv, common,
+ common->flags);
}
- return modified;
+ spin_unlock_bh(list_lock);
+
+ return true;
}
/**
@@ -1114,7 +1119,6 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
struct batadv_tt_global_entry *tt_global = NULL;
struct batadv_tt_local_entry *tt_local;
bool roamed_back = false;
- bool added = false;
bool iif_is_wifi;
bool ret = false;
int hash_added;
@@ -1126,10 +1130,15 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
if (!is_multicast_ether_addr(addr))
tt_global = batadv_tt_global_hash_find(bat_priv, addr, vid);
- if (tt_local) {
- added = batadv_tt_local_add_existing(bat_priv, tt_local,
- &roamed_back);
- } else {
+ if (tt_local &&
+ !batadv_tt_local_refresh(bat_priv, tt_local, iif_is_wifi, mark,
+ false, &roamed_back)) {
+ /* stale entry which has to be replaced by a new one */
+ batadv_tt_local_entry_put(tt_local);
+ tt_local = NULL;
+ }
+
+ if (!tt_local) {
tt_local = batadv_tt_local_create(mesh_iface, addr, vid, iif_is_wifi);
if (!tt_local)
goto out;
@@ -1145,21 +1154,12 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
goto out;
}
- added = true;
+ batadv_tt_local_refresh(bat_priv, tt_local, iif_is_wifi, mark,
+ true, &roamed_back);
}
- /* announce the (re-)added client to the mesh */
- if (added)
- batadv_tt_local_event(bat_priv, tt_local, BATADV_NO_FLAGS);
-
batadv_tt_local_add_roam(bat_priv, tt_global, roamed_back);
- /* if any "dynamic" flag has been modified, resend an ADD event for this
- * entry so that all the nodes can get the new flags
- */
- if (batadv_tt_local_update_flags(bat_priv, tt_local, iif_is_wifi, mark))
- batadv_tt_local_event(bat_priv, tt_local, BATADV_NO_FLAGS);
-
ret = true;
out:
batadv_tt_local_entry_put(tt_local);
@@ -1628,10 +1628,14 @@ int batadv_tt_local_dump(struct sk_buff *msg, struct netlink_callback *cb)
* entry. The entry is kept in the local table until the next TTVN increment
* so that a consistency-check response can still be answered.
*
- * Next to the flags_lock of the entry, the caller must hold the hash bucket
- * list_lock of @tt_local_entry. Otherwise
- * batadv_tt_local_purge_pending_clients() could remove the entry before its
- * change was queued.
+ * The caller must hold the flags_lock of @tt_local_entry and must not have
+ * released it since it checked that the entry has to be removed. Both the
+ * queue-then-mark order and this single flags_lock section are required.
+ *
+ * The caller must also hold the hash bucket list_lock of @tt_local_entry and
+ * must have ensured that the entry is still part of the local table. A DEL for
+ * an entry which was already purged could otherwise cancel the ADD of a new
+ * entry for the same client.
*/
static void
batadv_tt_local_set_pending(struct batadv_priv *bat_priv,
@@ -1664,11 +1668,14 @@ batadv_tt_local_set_pending(struct batadv_priv *bat_priv,
* @roaming: true if the deletion is due to a roaming event
* @curr_flags: pointer to store the flags of the entry before it was marked
*
- * An already announced entry is marked as BATADV_TT_CLIENT_PENDING and the
- * (roamed) DEL change is queued. Both happen under the hash bucket list_lock
- * of the entry to prevent concurrent batadv_tt_local_purge_pending_clients()
- * from removing the entry and batadv_tt_local_transition_new() from clearing
- * BATADV_TT_CLIENT_NEW after it was checked.
++ * For an already announced entry, the (roamed) DEL change is queued and the
++ * entry is marked as BATADV_TT_CLIENT_PENDING by batadv_tt_local_set_pending().
+ *
++ * An entry which still carries BATADV_TT_CLIENT_NEW is not marked. The caller
++ * has to purge it via batadv_tt_local_remove_now().
++ *
++ * An entry which was already removed from the local table after it was looked
++ * up is ignored and BATADV_NO_FLAGS is stored in @curr_flags.
*
* Return: true if the entry has to be kept in the local table until the next
* ttvn increment, false if it can be purged immediately.
@@ -1690,6 +1697,12 @@ batadv_tt_local_mark_removed(struct batadv_priv *bat_priv,
spin_lock_bh(list_lock);
+ if (hlist_unhashed(&common->hash_entry)) {
+ *curr_flags = BATADV_NO_FLAGS;
+ spin_unlock_bh(list_lock);
+ return true;
+ }
+
scoped_guard(spinlock_bh, &common->flags_lock) {
*curr_flags = common->flags;
@@ -1733,15 +1746,24 @@ static void
batadv_tt_local_remove_now(struct batadv_priv *bat_priv,
struct batadv_tt_local_entry *tt_local_entry)
{
+ spinlock_t *list_lock; /* protects write access to the hash lists */
struct batadv_tt_common_entry *common = &tt_local_entry->common;
- struct hlist_node *tt_removed_node;
+ struct batadv_hashtable *hash = bat_priv->tt.local_hash;
+ u32 i;
+
+ i = batadv_choose_tt(common, hash->size);
+ list_lock = &hash->list_locks[i];
+
+ spin_lock_bh(list_lock);
/* remove exactly this object when still present in hash */
- tt_removed_node = batadv_hash_remove(bat_priv->tt.local_hash,
- batadv_compare_tt_entry,
- batadv_choose_tt, common);
- if (!tt_removed_node)
+ if (hlist_unhashed(&common->hash_entry)) {
+ spin_unlock_bh(list_lock);
return;
+ }
+
+ hlist_del_init_rcu(&common->hash_entry);
+ atomic_inc(&hash->generation);
batadv_tt_local_event(bat_priv, tt_local_entry, BATADV_TT_CLIENT_DEL);
@@ -1752,6 +1774,8 @@ batadv_tt_local_remove_now(struct batadv_priv *bat_priv,
if (!(batadv_tt_flags_get(common) & BATADV_TT_CLIENT_NEW))
batadv_tt_local_size_dec(bat_priv, common->vid);
+ spin_unlock_bh(list_lock);
+
/* drop reference of remove hash entry */
batadv_tt_local_entry_put(tt_local_entry);
}
@@ -1881,7 +1905,7 @@ static void batadv_tt_local_table_free(struct batadv_priv *bat_priv)
spin_lock_bh(list_lock);
hlist_for_each_entry_safe(tt_common_entry, node_tmp,
head, hash_entry) {
- hlist_del_rcu(&tt_common_entry->hash_entry);
+ hlist_del_init_rcu(&tt_common_entry->hash_entry);
tt_local = container_of(tt_common_entry,
struct batadv_tt_local_entry,
common);
@@ -4434,7 +4458,7 @@ static void batadv_tt_local_purge_pending_clients(struct batadv_priv *bat_priv)
tt_common->addr,
batadv_print_vid(tt_common->vid));
- hlist_del_rcu(&tt_common->hash_entry);
+ hlist_del_init_rcu(&tt_common->hash_entry);
/* An entry which still carries BATADV_TT_CLIENT_NEW was
* never counted and must not be uncounted here.
--
2.47.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH batadv 2/4] batman-adv: tt: only update flags for TT events
2026-10-07 10:29 [PATCH batadv 0/4] batman-adv: tt: shashiko 20260930 follow ups Sven Eckelmann
2026-10-07 10:29 ` [PATCH batadv 1/4] batman-adv: tt: don't queue local events for unhashed entries Sven Eckelmann
@ 2026-10-07 10:29 ` Sven Eckelmann
2026-10-07 10:46 ` Sven Eckelmann
[not found] ` <sashiko-outbox-162887@kernel.org>
2026-10-07 10:29 ` [PATCH batadv 3/4] batman-adv: tt: avoid list_lock for unchanged local clients Sven Eckelmann
2026-10-07 10:29 ` [PATCH batadv 4/4] batman-adv: tt: drop ADD+DEL events of never announced clients Sven Eckelmann
3 siblings, 2 replies; 7+ messages in thread
From: Sven Eckelmann @ 2026-10-07 10:29 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Sven Eckelmann
In the first implementation of the translation table events, it was only
possible to announce:
* ADD: new locally detected client
* DEL: disappeared local client
If an ADD event was queued and then a DEL event was queued in the same
timeslot, it is valid to destroy both events because nothing actually
changed for remode nodes
But with commit e0eb5f8df352 ("batman-adv: improve the TT component to
support runtime flag changes") and subsequent changes, it could also happen
that there are extra ADD events:
* ADD: _NEW_ locally detected client
* ADD: already existing client with new flags
* DEL: disappeared local client
The first and the last type of event could in theory still be removed
together from the (to be announced) events queue. But if the "update flags"
ADD is dropped together with the DEL, it would still leave global
translation table of a remote node in the incorrect state - believing that
the client didn't disappear.
If a detected client was previously announced, a DEL event must only
overwrite the "update flags" ADD but the DEL itself must still continue to
exist in the (to be anounced) events queue. For now, adjustment of the
announced flags is used for everything instead of dropping already queued
up events completely.
Similarly, a DEL followed by an ADD caused both entries to be deleted. But
the new ADD could announce flags changes which were not yet present for the
client before it was marked for DELetion. The new ADD must therefore only
overwrite the DEL - but still be kept.
A re-added client which roamed away still carries BATADV_TT_CLIENT_ROAM. It
was never announced with an ADD because the DEL always cancelled it. Since
this ADD is now sent, the ROAM flag is removed from it. Otherwise the other
nodes would handle the ADD as roaming and drop their other originators for
this client.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
net/batman-adv/translation-table.c | 44 ++++++++++++++++----------------------
1 file changed, 19 insertions(+), 25 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index b545001b..a10345a1 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -675,6 +675,9 @@ static u16 batadv_tt_flags_get(struct batadv_tt_common_entry *common)
* @bat_priv: the bat priv with all the mesh interface information
* @common: the TT entry involved in the event
* @flags: flags of the TT entry combined with the event flags
+ *
+ * Only a single change per client is queued. It describes the latest state of
+ * the client and a new event therefore replaces an already queued one.
*/
static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
const struct batadv_tt_common_entry *common,
@@ -683,8 +686,6 @@ static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
struct batadv_tt_change_node *tt_change_node;
struct batadv_tt_change_node *entry;
struct batadv_tt_change_node *safe;
- bool del_op_requested;
- bool del_op_entry;
size_t changes;
tt_change_node = kmem_cache_alloc(batadv_tt_change_cache, GFP_ATOMIC);
@@ -697,7 +698,6 @@ static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
tt_change_node->change.vid = htons(common->vid);
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 */
spin_lock_bh(&bat_priv->tt.changes_list_lock);
@@ -710,25 +710,12 @@ static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
if (entry->change.vid != tt_change_node->change.vid)
continue;
- del_op_entry = entry->change.flags & BATADV_TT_CLIENT_DEL;
- if (del_op_requested != del_op_entry) {
- /* DEL+ADD in the same orig interval have no effect and
- * can be removed to avoid silly behaviour on the
- * receiver side. The other way around (ADD+DEL) can
- * happen in case of roaming of a client still in the
- * NEW state. Roaming of NEW clients is now possible due
- * to automatically recognition of "temporary" clients
- */
- list_del(&entry->list);
- kmem_cache_free(batadv_tt_change_cache, entry);
- changes--;
- } else {
- /* this is a second add or del in the same originator
- * interval. It could mean that flags have been changed
- * (e.g. double add): update them
- */
- entry->change.flags = flags;
- }
+ /* the other nodes may know the client with different
+ * flags. A DEL+ADD must therefore still announce the
+ * current flags and an ADD+DEL must still remove the
+ * client. ADD+ADD and DEL+DEL only update the flags
+ */
+ entry->change.flags = flags;
kmem_cache_free(batadv_tt_change_cache, tt_change_node);
goto update_changes;
@@ -1023,6 +1010,7 @@ batadv_tt_local_refresh(struct batadv_priv *bat_priv,
spinlock_t *list_lock; /* protects write access to the hash lists */
struct batadv_tt_common_entry *common = &tt_local->common;
struct batadv_hashtable *hash = bat_priv->tt.local_hash;
+ u16 event_flags;
u8 remote_flags;
u32 match_mark;
u32 i;
@@ -1089,9 +1077,15 @@ batadv_tt_local_refresh(struct batadv_priv *bat_priv,
if (remote_flags ^ (common->flags & BATADV_TT_REMOTE_MASK))
announce = true;
- if (announce)
- __batadv_tt_local_event(bat_priv, common,
- common->flags);
+ /* BATADV_TT_CLIENT_ROAM can still be set (see above) when a
+ * client which roamed away was re-added. It must not be
+ * announced with an ADD because the other nodes would then
+ * handle it as still roaming.
+ */
+ if (announce) {
+ event_flags = common->flags & ~BATADV_TT_CLIENT_ROAM;
+ __batadv_tt_local_event(bat_priv, common, event_flags);
+ }
}
spin_unlock_bh(list_lock);
--
2.47.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH batadv 3/4] batman-adv: tt: avoid list_lock for unchanged local clients
2026-10-07 10:29 [PATCH batadv 0/4] batman-adv: tt: shashiko 20260930 follow ups Sven Eckelmann
2026-10-07 10:29 ` [PATCH batadv 1/4] batman-adv: tt: don't queue local events for unhashed entries Sven Eckelmann
2026-10-07 10:29 ` [PATCH batadv 2/4] batman-adv: tt: only update flags for TT events Sven Eckelmann
@ 2026-10-07 10:29 ` Sven Eckelmann
2026-10-07 10:29 ` [PATCH batadv 4/4] batman-adv: tt: drop ADD+DEL events of never announced clients Sven Eckelmann
3 siblings, 0 replies; 7+ messages in thread
From: Sven Eckelmann @ 2026-10-07 10:29 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Sven Eckelmann
batadv_tt_local_add() is called for every packet transmitted by a local
client. batadv_tt_local_refresh() takes the hash bucket list_lock for each
of them, although this lock is shared by all entries of the bucket and is
only needed when the entry has to be modified or announced.
For the common case, the entry is neither BATADV_TT_CLIENT_PENDING nor
BATADV_TT_CLIENT_ROAM, the dynamic flags (BATADV_TT_CLIENT_WIFI and
BATADV_TT_CLIENT_ISOLA) are unchanged and the entry is still hashed. Only
last_seen has to be refreshed then.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
net/batman-adv/translation-table.c | 97 +++++++++++++++++++++++++++++++-------
1 file changed, 79 insertions(+), 18 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index a10345a1..e8b0de51 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -983,6 +983,72 @@ batadv_tt_local_create(struct net_device *mesh_iface, const u8 *addr,
return tt_local;
}
+/* flags of a local TT entry which depend on the packets of the client */
+#define BATADV_TT_LOCAL_DYNAMIC_MASK \
+ (BATADV_TT_CLIENT_WIFI | BATADV_TT_CLIENT_ISOLA)
+
+/**
+ * batadv_tt_local_dynamic_flags() - get the dynamic flags of a local client
+ * @bat_priv: the bat priv with all the mesh interface information
+ * @iif_is_wifi: whether the client is connected via a wifi interface
+ * @mark: the value contained in the skb->mark field of the received packet (if
+ * any)
+ *
+ * Return: the BATADV_TT_CLIENT_WIFI and BATADV_TT_CLIENT_ISOLA flags which the
+ * local entry of the client has to carry.
+ */
+static u16 batadv_tt_local_dynamic_flags(struct batadv_priv *bat_priv,
+ bool iif_is_wifi, u32 mark)
+{
+ u16 flags = 0;
+ u32 match_mark;
+
+ if (iif_is_wifi)
+ flags |= BATADV_TT_CLIENT_WIFI;
+
+ /* check the mark in the skb: if it's equal to the configured
+ * isolation_mark, it means the packet is coming from an
+ * isolated non-mesh client
+ */
+ match_mark = (mark & bat_priv->isolation_mark_mask);
+ if (bat_priv->isolation_mark_mask &&
+ match_mark == bat_priv->isolation_mark)
+ flags |= BATADV_TT_CLIENT_ISOLA;
+
+ return flags;
+}
+
+/**
+ * batadv_tt_local_refresh_needed() - check if a local entry has to be modified
+ * @tt_local: the local TT entry to check
+ * @dynamic_flags: the dynamic flags the entry has to carry
+ *
+ * The bucket list_lock which is shared by many entries is only needed when
+ * the entry has to be modified or announced. For the common case of an
+ * already announced client with unchanged flags, it is enough to refresh the
+ * last_seen timestamp.
+ *
+ * Return: true if the slow path of batadv_tt_local_refresh() is required,
+ * false otherwise.
+ */
+static bool
+batadv_tt_local_refresh_needed(struct batadv_tt_local_entry *tt_local,
+ u16 dynamic_flags)
+{
+ struct batadv_tt_common_entry *common = &tt_local->common;
+ u16 flags;
+
+ flags = batadv_tt_flags_get(common);
+
+ if (flags & (BATADV_TT_CLIENT_PENDING | BATADV_TT_CLIENT_ROAM))
+ return true;
+
+ if ((flags & BATADV_TT_LOCAL_DYNAMIC_MASK) != dynamic_flags)
+ return true;
+
+ return hlist_unhashed_lockless(&common->hash_entry);
+}
+
/**
* batadv_tt_local_refresh() - refresh a local TT entry of an active client
* @bat_priv: the bat priv with all the mesh interface information
@@ -1010,16 +1076,24 @@ batadv_tt_local_refresh(struct batadv_priv *bat_priv,
spinlock_t *list_lock; /* protects write access to the hash lists */
struct batadv_tt_common_entry *common = &tt_local->common;
struct batadv_hashtable *hash = bat_priv->tt.local_hash;
+ u16 dynamic_flags;
u16 event_flags;
u8 remote_flags;
- u32 match_mark;
u32 i;
+ dynamic_flags = batadv_tt_local_dynamic_flags(bat_priv, iif_is_wifi,
+ mark);
+
+ tt_local->last_seen = jiffies;
+
+ /* fast path: nothing to modify or to announce */
+ if (!announce &&
+ !batadv_tt_local_refresh_needed(tt_local, dynamic_flags))
+ return true;
+
i = batadv_choose_tt(common, hash->size);
list_lock = &hash->list_locks[i];
- tt_local->last_seen = jiffies;
-
spin_lock_bh(list_lock);
/* the entry was removed from the local table after it was looked up */
@@ -1058,21 +1132,8 @@ batadv_tt_local_refresh(struct batadv_priv *bat_priv,
*/
remote_flags = common->flags & BATADV_TT_REMOTE_MASK;
- if (iif_is_wifi)
- common->flags |= BATADV_TT_CLIENT_WIFI;
- else
- common->flags &= ~BATADV_TT_CLIENT_WIFI;
-
- /* check the mark in the skb: if it's equal to the configured
- * isolation_mark, it means the packet is coming from an
- * isolated non-mesh client
- */
- match_mark = (mark & bat_priv->isolation_mark_mask);
- if (bat_priv->isolation_mark_mask &&
- match_mark == bat_priv->isolation_mark)
- common->flags |= BATADV_TT_CLIENT_ISOLA;
- else
- common->flags &= ~BATADV_TT_CLIENT_ISOLA;
+ common->flags &= ~BATADV_TT_LOCAL_DYNAMIC_MASK;
+ common->flags |= dynamic_flags;
if (remote_flags ^ (common->flags & BATADV_TT_REMOTE_MASK))
announce = true;
--
2.47.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH batadv 4/4] batman-adv: tt: drop ADD+DEL events of never announced clients
2026-10-07 10:29 [PATCH batadv 0/4] batman-adv: tt: shashiko 20260930 follow ups Sven Eckelmann
` (2 preceding siblings ...)
2026-10-07 10:29 ` [PATCH batadv 3/4] batman-adv: tt: avoid list_lock for unchanged local clients Sven Eckelmann
@ 2026-10-07 10:29 ` Sven Eckelmann
3 siblings, 0 replies; 7+ messages in thread
From: Sven Eckelmann @ 2026-10-07 10:29 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Sven Eckelmann
If a detected client was not yet announced, a DEL event can directly drop
the ADD event. The BATADV_TT_CLIENT_NEW flag must be checked to identify
clients which were or were not yet announced.
Since this would be only the case for clients which appeared and directly
disappeared, this behavior must only be triggered by
batadv_tt_local_remove_now().
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
net/batman-adv/translation-table.c | 81 +++++++++++++++++++++-----------------
1 file changed, 45 insertions(+), 36 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index e8b0de51..984a55cd 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -671,21 +671,28 @@ static u16 batadv_tt_flags_get(struct batadv_tt_common_entry *common)
}
/**
- * __batadv_tt_local_event() - store a local TT event (ADD/DEL) with given flags
+ * batadv_tt_local_event() - store a local TT event (ADD/DEL) with given flags
* @bat_priv: the bat priv with all the mesh interface information
* @common: the TT entry involved in the event
* @flags: flags of the TT entry combined with the event flags
+ * @cancel_add: whether a DEL may cancel a queued ADD because the entry was
+ * never announced to the other nodes
*
* Only a single change per client is queued. It describes the latest state of
- * the client and a new event therefore replaces an already queued one.
+ * the client and a new event therefore replaces an already queued one. The
+ * only exception is a DEL for an entry which was never announced. It cancels
+ * the ADD of this entry because the other nodes don't need to know anything
+ * about it.
*/
-static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
- const struct batadv_tt_common_entry *common,
- u8 flags)
+static void batadv_tt_local_event(struct batadv_priv *bat_priv,
+ const struct batadv_tt_common_entry *common,
+ u8 flags, bool cancel_add)
{
struct batadv_tt_change_node *tt_change_node;
struct batadv_tt_change_node *entry;
struct batadv_tt_change_node *safe;
+ bool del_op_requested;
+ bool del_op_entry;
size_t changes;
tt_change_node = kmem_cache_alloc(batadv_tt_change_cache, GFP_ATOMIC);
@@ -698,6 +705,7 @@ static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
tt_change_node->change.vid = htons(common->vid);
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 */
spin_lock_bh(&bat_priv->tt.changes_list_lock);
@@ -710,12 +718,23 @@ static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
if (entry->change.vid != tt_change_node->change.vid)
continue;
- /* the other nodes may know the client with different
- * flags. A DEL+ADD must therefore still announce the
- * current flags and an ADD+DEL must still remove the
- * client. ADD+ADD and DEL+DEL only update the flags
- */
- entry->change.flags = flags;
+ del_op_entry = entry->change.flags & BATADV_TT_CLIENT_DEL;
+ if (cancel_add && del_op_requested && !del_op_entry) {
+ /* ADD+DEL of a client which was never announced (e.g.
+ * roaming of a client still in the NEW state) have no
+ * effect and can be removed
+ */
+ list_del(&entry->list);
+ kmem_cache_free(batadv_tt_change_cache, entry);
+ changes--;
+ } else {
+ /* the other nodes may know the client with different
+ * flags. A DEL+ADD must therefore still announce the
+ * current flags and an ADD+DEL must still remove the
+ * client. ADD+ADD and DEL+DEL only update the flags
+ */
+ entry->change.flags = flags;
+ }
kmem_cache_free(batadv_tt_change_cache, tt_change_node);
goto update_changes;
@@ -730,23 +749,6 @@ static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
spin_unlock_bh(&bat_priv->tt.changes_list_lock);
}
-/**
- * batadv_tt_local_event() - store a local TT event (ADD/DEL)
- * @bat_priv: the bat priv with all the mesh interface information
- * @tt_local_entry: the TT entry involved in the event
- * @event_flags: flags to store in the event structure
- */
-static void batadv_tt_local_event(struct batadv_priv *bat_priv,
- struct batadv_tt_local_entry *tt_local_entry,
- u8 event_flags)
-{
- struct batadv_tt_common_entry *common = &tt_local_entry->common;
- u8 flags;
-
- flags = batadv_tt_flags_get(common) | event_flags;
- __batadv_tt_local_event(bat_priv, common, flags);
-}
-
/**
* batadv_tt_entries() - compute the number of entries fitting in tt_len bytes
* @tt_len: available space
@@ -1145,7 +1147,8 @@ batadv_tt_local_refresh(struct batadv_priv *bat_priv,
*/
if (announce) {
event_flags = common->flags & ~BATADV_TT_CLIENT_ROAM;
- __batadv_tt_local_event(bat_priv, common, event_flags);
+ batadv_tt_local_event(bat_priv, common, event_flags,
+ false);
}
}
@@ -1706,7 +1709,7 @@ batadv_tt_local_set_pending(struct batadv_priv *bat_priv,
lockdep_assert_held(&hash->list_locks[i]);
lockdep_assert_held(&common->flags_lock);
- __batadv_tt_local_event(bat_priv, common, common->flags | flags);
+ batadv_tt_local_event(bat_priv, common, common->flags | flags, false);
common->flags |= BATADV_TT_CLIENT_PENDING;
batadv_dbg(BATADV_DBG_TT, bat_priv,
@@ -1804,6 +1807,7 @@ batadv_tt_local_remove_now(struct batadv_priv *bat_priv,
spinlock_t *list_lock; /* protects write access to the hash lists */
struct batadv_tt_common_entry *common = &tt_local_entry->common;
struct batadv_hashtable *hash = bat_priv->tt.local_hash;
+ u16 flags;
u32 i;
i = batadv_choose_tt(common, hash->size);
@@ -1820,13 +1824,18 @@ batadv_tt_local_remove_now(struct batadv_priv *bat_priv,
hlist_del_init_rcu(&common->hash_entry);
atomic_inc(&hash->generation);
- batadv_tt_local_event(bat_priv, tt_local_entry, BATADV_TT_CLIENT_DEL);
-
- /* batadv_tt_local_transition_new() may have committed the entry and
- * thus counted it in the local table size since the
- * BATADV_TT_CLIENT_NEW check in batadv_tt_local_mark_removed().
+ /* batadv_tt_local_transition_new() clears BATADV_TT_CLIENT_NEW under
+ * the list_lock. The flags read here therefore show whether the entry
+ * was committed (and announced) since the BATADV_TT_CLIENT_NEW check
+ * in batadv_tt_local_mark_removed(). Only the ADD of an entry which
+ * was never announced can be cancelled by its DEL.
*/
- if (!(batadv_tt_flags_get(common) & BATADV_TT_CLIENT_NEW))
+ flags = batadv_tt_flags_get(common);
+ batadv_tt_local_event(bat_priv, common, flags | BATADV_TT_CLIENT_DEL,
+ flags & BATADV_TT_CLIENT_NEW);
+
+ /* a committed entry was counted in the local table size */
+ if (!(flags & BATADV_TT_CLIENT_NEW))
batadv_tt_local_size_dec(bat_priv, common->vid);
spin_unlock_bh(list_lock);
--
2.47.3
^ permalink raw reply related [flat|nested] 7+ messages in thread