Project

General

Profile

Actions

Bug #482

open

tt: don't queue local events for unhashed entries

Added by Sven Eckelmann 15 days ago. Updated 11 days ago.

Status:
In Progress
Priority:
Normal
Target version:
-
Start date:
10/02/2026
Due date:
% Done:

0%

Estimated time:

Description

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

> [Severity: High]
> Does this close only one ordering of the race with the re-add path?
> 
> The add side still changes the flags and queues the ADD in two separate
> steps. batadv_tt_local_add_existing() clears PENDING inside a flags_lock
> scoped_guard and returns. batadv_tt_local_add() then queues the ADD after
> that lock is dropped:
> 
>     if (added)
>         batadv_tt_local_event(bat_priv, tt_local, BATADV_NO_FLAGS);
>     ...
>     if (batadv_tt_local_update_flags(bat_priv, tt_local, iif_is_wifi, mark))
>         batadv_tt_local_event(bat_priv, tt_local, BATADV_NO_FLAGS);
> 
> batadv_tt_local_set_pending() can run in that window from the RX path:
> 
> batadv_tt_global_add()->batadv_tt_global_purge_local()->
>   batadv_tt_local_remove()->batadv_tt_local_mark_removed()->
>     batadv_tt_local_set_pending()
> 
> Here is one interleaving, starting with DEL#1 already queued and PENDING
> set:
> 
>   CPU1 (TX)                         CPU0 (RX)
>   batadv_tt_local_add()
>     batadv_tt_local_add_existing()
>       flags &= ~PENDING;
>                                     batadv_tt_local_mark_removed()
>                                       batadv_tt_local_set_pending()
>                                         DEL|ROAM merged into DEL#1
>                                         flags |= PENDING;
>     batadv_tt_local_event(NO_FLAGS)
>       ADD cancels DEL#1
> 
> There is a similar case with no earlier DEL. batadv_tt_local_update_flags()
> returns true on CPU1, CPU0 queues a new DEL and sets PENDING, and then the
> ADD from CPU1 cancels that DEL through the DEL+ADD case in
> __batadv_tt_local_event().
> 
> Either way, the entry would be left PENDING with no DEL queued. At the
> next commit, batadv_tt_local_purge_pending_clients() would remove it
> locally, but neighbours would never receive a DEL.
> 
> Wouldn't that give the same CRC mismatch and full table request that the
> commit message describes? Would the ADD also need to be queued inside the
> flags_lock section that clears PENDING or updates the flags?

Related issues 2 (2 open — 0 closed)

Related to batman-adv - Bug #478: tt: prevent __batadv_tt_local_event() from queuing del for already unhashed local entryIn Progressbatman-adv developers10/02/2026

Actions
Related to batman-adv - Bug #475: tt: Fix current event queuing guarentees for batadv_tt_local_remove_now()In Progressbatman-adv developers10/02/2026

Actions
Actions #1

Updated by Sven Eckelmann 15 days ago

  • Related to Bug #478: tt: prevent __batadv_tt_local_event() from queuing del for already unhashed local entry added
Actions #2

Updated by Sven Eckelmann 15 days ago ยท Edited

From my answer:

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

Something like:

diff --git c/net/batman-adv/translation-table.c w/net/batman-adv/translation-table.c
index e46040fd..14e095d5 100644
--- c/net/batman-adv/translation-table.c
+++ w/net/batman-adv/translation-table.c
@@ -925,55 +925,6 @@ static bool batadv_tt_iif_is_wifi(struct net *net, int ifindex)
     return batadv_is_wifi(wifi_flags);
 }

-/**
- * batadv_tt_local_add_existing() - refresh an already known local TT entry
- * @bat_priv: the bat priv with all the mesh interface information
- * @tt_local: the local TT entry which was found in the local table
- * @roamed_back: set to true when the client returned to its original location
- *
- * Return: true when the client has to be announced to the mesh again, false
- * otherwise.
- */
-static bool batadv_tt_local_add_existing(struct batadv_priv *bat_priv,
-                     struct batadv_tt_local_entry *tt_local,
-                     bool *roamed_back)
-{
-    struct batadv_tt_common_entry *common = &tt_local->common;
-
-    tt_local->last_seen = jiffies;
-
-    scoped_guard(spinlock_bh, &common->flags_lock) {
-        if (common->flags & BATADV_TT_CLIENT_PENDING) {
-            batadv_dbg(BATADV_DBG_TT, bat_priv,
-                   "Re-adding pending client %pM (vid: %d)\n",
-                   common->addr, batadv_print_vid(common->vid));
-            /* whatever the reason why the PENDING flag was set,
-             * this is a client which was enqueued to be removed in
-             * this orig_interval. Since it popped up again, the
-             * flag can be reset like it was never enqueued
-             */
-            common->flags &= ~BATADV_TT_CLIENT_PENDING;
-
-            return true;
-        }
-
-        if (common->flags & BATADV_TT_CLIENT_ROAM) {
-            batadv_dbg(BATADV_DBG_TT, bat_priv,
-                   "Roaming client %pM (vid: %d) came back to its original location\n",
-                   common->addr, batadv_print_vid(common->vid));
-            /* the ROAM flag is set because this client roamed away
-             * and the node got a roaming_advertisement message. Now
-             * that the client popped up again at its original
-             * location such flag can be unset
-             */
-            common->flags &= ~BATADV_TT_CLIENT_ROAM;
-            *roamed_back = true;
-        }
-    }
-
-    return false;
-}
-
 /**
  * batadv_tt_local_create() - allocate and initialize a local TT entry
  * @mesh_iface: netdev struct of the mesh interface
@@ -1046,27 +997,72 @@ batadv_tt_local_create(struct net_device *mesh_iface, const u8 *addr,
 }

 /**
- * batadv_tt_local_update_flags() - update the dynamic flags of a local entry
+ * batadv_tt_local_refresh() - refresh a local TT entry of an active client
  * @bat_priv: the bat priv with all the mesh interface information
- * @tt_local: the local TT entry to update
+ * @tt_local: the local TT entry to refresh
  * @iif_is_wifi: whether the client is connected via a wifi interface
  * @mark: the value contained in the skb->mark field of the received packet (if
  *  any)
+ * @announce: whether an ADD event has to be queued even when no announced
+ *  flag was modified
+ * @roamed_back: set to true when the client returned to its original location
  *
- * Return: true if a flag announced to the other nodes was modified, false
- * otherwise.
+ * TODO
+ *
+ * Return: true if the entry was refreshed, false if it is no longer part of
+ * the local table.
  */
 static bool
-batadv_tt_local_update_flags(struct batadv_priv *bat_priv,
-                 struct batadv_tt_local_entry *tt_local,
-                 bool iif_is_wifi, u32 mark)
+batadv_tt_local_refresh(struct batadv_priv *bat_priv,
+            struct batadv_tt_local_entry *tt_local,
+            bool iif_is_wifi, u32 mark, bool announce,
+            bool *roamed_back)
 {
+    spinlock_t *list_lock; /* protects write access to the hash lists */
     struct batadv_tt_common_entry *common = &tt_local->common;
+    struct batadv_hashtable *hash = bat_priv->tt.local_hash;
     u8 remote_flags;
     u32 match_mark;
-    bool modified;
+    u32 i;
+
+    i = batadv_choose_tt(common, hash->size);
+    list_lock = &hash->list_locks[i];
+
+    tt_local->last_seen = jiffies;
+
+    spin_lock_bh(list_lock);
+
+    /* the entry was removed from the local table after it was looked up */
+    if (hlist_unhashed(&common->hash_entry)) {
+        spin_unlock_bh(list_lock);
+        return false;
+    }

     scoped_guard(spinlock_bh, &common->flags_lock) {
+        if (common->flags & BATADV_TT_CLIENT_PENDING) {
+            batadv_dbg(BATADV_DBG_TT, bat_priv,
+                   "Re-adding pending client %pM (vid: %d)\n",
+                   common->addr, batadv_print_vid(common->vid));
+            /* whatever the reason why the PENDING flag was set,
+             * this is a client which was enqueued to be removed in
+             * this orig_interval. Since it popped up again, the
+             * flag can be reset like it was never enqueued
+             */
+            common->flags &= ~BATADV_TT_CLIENT_PENDING;
+            announce = true;
+        } else if (common->flags & BATADV_TT_CLIENT_ROAM) {
+            batadv_dbg(BATADV_DBG_TT, bat_priv,
+                   "Roaming client %pM (vid: %d) came back to its original location\n",
+                   common->addr, batadv_print_vid(common->vid));
+            /* the ROAM flag is set because this client roamed away
+             * and the node got a roaming_advertisement message. Now
+             * that the client popped up again at its original
+             * location such flag can be unset
+             */
+            common->flags &= ~BATADV_TT_CLIENT_ROAM;
+            *roamed_back = true;
+        }
+
         /* store the current remote flags before altering them. This
          * helps understanding is flags are changing or not
          */
@@ -1088,10 +1084,17 @@ batadv_tt_local_update_flags(struct batadv_priv *bat_priv,
         else
             common->flags &= ~BATADV_TT_CLIENT_ISOLA;

-        modified = remote_flags ^ (common->flags & BATADV_TT_REMOTE_MASK);
+        if (remote_flags ^ (common->flags & BATADV_TT_REMOTE_MASK))
+            announce = true;
+
+        if (announce)
+            __batadv_tt_local_event(bat_priv, common,
+                        common->flags);
     }

-    return modified;
+    spin_unlock_bh(list_lock);
+
+    return true;
 }

 /**
@@ -1114,7 +1117,6 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
     struct batadv_tt_global_entry *tt_global = NULL;
     struct batadv_tt_local_entry *tt_local;
     bool roamed_back = false;
-    bool added = false;
     bool iif_is_wifi;
     bool ret = false;
     int hash_added;
@@ -1126,10 +1128,15 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
     if (!is_multicast_ether_addr(addr))
         tt_global = batadv_tt_global_hash_find(bat_priv, addr, vid);

-    if (tt_local) {
-        added = batadv_tt_local_add_existing(bat_priv, tt_local,
-                             &roamed_back);
-    } else {
+    if (tt_local &&
+        !batadv_tt_local_refresh(bat_priv, tt_local, iif_is_wifi, mark,
+                     false, &roamed_back)) {
+        /* stale entry which has to be replaced by a new one */
+        batadv_tt_local_entry_put(tt_local);
+        tt_local = NULL;
+    }
+
+    if (!tt_local) {
         tt_local = batadv_tt_local_create(mesh_iface, addr, vid, iif_is_wifi);
         if (!tt_local)
             goto out;
@@ -1145,21 +1152,12 @@ bool batadv_tt_local_add(struct net_device *mesh_iface, const u8 *addr,
             goto out;
         }

-        added = true;
+        batadv_tt_local_refresh(bat_priv, tt_local, iif_is_wifi, mark,
+                    true, &roamed_back);
     }

-    /* announce the (re-)added client to the mesh */
-    if (added)
-        batadv_tt_local_event(bat_priv, tt_local, BATADV_NO_FLAGS);
-
     batadv_tt_local_add_roam(bat_priv, tt_global, roamed_back);

-    /* if any "dynamic" flag has been modified, resend an ADD event for this
-     * entry so that all the nodes can get the new flags
-     */
-    if (batadv_tt_local_update_flags(bat_priv, tt_local, iif_is_wifi, mark))
-        batadv_tt_local_event(bat_priv, tt_local, BATADV_NO_FLAGS);
-
     ret = true;
 out:
     batadv_tt_local_entry_put(tt_local);
@@ -1624,14 +1622,7 @@ int batadv_tt_local_dump(struct sk_buff *msg, struct netlink_callback *cb)
  * @flags: TT change flags to announce together with the pending removal
  * @message: debug message describing the reason for the change
  *
- * Schedule the TT change announcement and set BATADV_TT_CLIENT_PENDING on the
- * entry. The entry is kept in the local table until the next TTVN increment
- * so that a consistency-check response can still be answered.
- *
- * Next to the flags_lock of the entry, the caller must hold the hash bucket
- * list_lock of @tt_local_entry. Otherwise
- * batadv_tt_local_purge_pending_clients() could remove the entry before its
- * change was queued.
+ * TODO
  */
 static void
 batadv_tt_local_set_pending(struct batadv_priv *bat_priv,
@@ -1664,14 +1655,9 @@ batadv_tt_local_set_pending(struct batadv_priv *bat_priv,
  * @roaming: true if the deletion is due to a roaming event
  * @curr_flags: pointer to store the flags of the entry before it was marked
  *
- * An already announced entry is marked as BATADV_TT_CLIENT_PENDING and the
- * (roamed) DEL change is queued. Both happen under the hash bucket list_lock
- * of the entry to prevent concurrent batadv_tt_local_purge_pending_clients()
- * from removing the entry and batadv_tt_local_transition_new() from clearing
- * BATADV_TT_CLIENT_NEW after it was checked.
+ * TODO
  *
- * Return: true if the entry has to be kept in the local table until the next
- * ttvn increment, false if it can be purged immediately.
+ * Return: false if the entry has to be purged immediately, true otherwise.
  */
 static bool
 batadv_tt_local_mark_removed(struct batadv_priv *bat_priv,
@@ -1690,6 +1676,12 @@ batadv_tt_local_mark_removed(struct batadv_priv *bat_priv,

     spin_lock_bh(list_lock);

+    if (hlist_unhashed(&common->hash_entry)) {
+        *curr_flags = BATADV_NO_FLAGS;
+        spin_unlock_bh(list_lock);
+        return true;
+    }
+
     scoped_guard(spinlock_bh, &common->flags_lock) {
         *curr_flags = common->flags;

@@ -1733,15 +1725,24 @@ static void
 batadv_tt_local_remove_now(struct batadv_priv *bat_priv,
                struct batadv_tt_local_entry *tt_local_entry)
 {
+    spinlock_t *list_lock; /* protects write access to the hash lists */
     struct batadv_tt_common_entry *common = &tt_local_entry->common;
-    struct hlist_node *tt_removed_node;
+    struct batadv_hashtable *hash = bat_priv->tt.local_hash;
+    u32 i;
+
+    i = batadv_choose_tt(common, hash->size);
+    list_lock = &hash->list_locks[i];
+
+    spin_lock_bh(list_lock);

     /* remove exactly this object when still present in hash */
-    tt_removed_node = batadv_hash_remove(bat_priv->tt.local_hash,
-                         batadv_compare_tt_entry,
-                         batadv_choose_tt, common);
-    if (!tt_removed_node)
+    if (hlist_unhashed(&common->hash_entry)) {
+        spin_unlock_bh(list_lock);
         return;
+    }
+
+    hlist_del_init_rcu(&common->hash_entry);
+    atomic_inc(&hash->generation);

     batadv_tt_local_event(bat_priv, tt_local_entry, BATADV_TT_CLIENT_DEL);

@@ -1752,6 +1753,8 @@ batadv_tt_local_remove_now(struct batadv_priv *bat_priv,
     if (!(batadv_tt_flags_get(common) & BATADV_TT_CLIENT_NEW))
         batadv_tt_local_size_dec(bat_priv, common->vid);

+    spin_unlock_bh(list_lock);
+
     /* drop reference of remove hash entry */
     batadv_tt_local_entry_put(tt_local_entry);
 }
@@ -1881,7 +1884,7 @@ static void batadv_tt_local_table_free(struct batadv_priv *bat_priv)
         spin_lock_bh(list_lock);
         hlist_for_each_entry_safe(tt_common_entry, node_tmp,
                       head, hash_entry) {
-            hlist_del_rcu(&tt_common_entry->hash_entry);
+            hlist_del_init_rcu(&tt_common_entry->hash_entry);
             tt_local = container_of(tt_common_entry,
                         struct batadv_tt_local_entry,
                         common);
@@ -4434,7 +4437,7 @@ static void batadv_tt_local_purge_pending_clients(struct batadv_priv *bat_priv)
                        tt_common->addr,
                        batadv_print_vid(tt_common->vid));

-                hlist_del_rcu(&tt_common->hash_entry);
+                hlist_del_init_rcu(&tt_common->hash_entry);

                 /* An entry which still carries BATADV_TT_CLIENT_NEW was
                  * never counted and must not be uncounted here.

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

diff --git c/net/batman-adv/translation-table.c w/net/batman-adv/translation-table.c
index 14e095d5..2ab66601 100644
--- c/net/batman-adv/translation-table.c
+++ w/net/batman-adv/translation-table.c
@@ -996,6 +996,48 @@ batadv_tt_local_create(struct net_device *mesh_iface, const u8 *addr,
     return tt_local;
 }

+#define BATADV_TT_LOCAL_DYNAMIC_MASK \
+    (BATADV_TT_CLIENT_WIFI | BATADV_TT_CLIENT_ISOLA)
+
+static u16 batadv_tt_local_dynamic_flags(struct batadv_priv *bat_priv,
+                     bool iif_is_wifi, u32 mark)
+{
+    u16 flags = 0;
+    u32 match_mark;
+
+    if (iif_is_wifi)
+        flags |= BATADV_TT_CLIENT_WIFI;
+
+    /* check the mark in the skb: if it's equal to the configured
+     * isolation_mark, it means the packet is coming from an
+     * isolated non-mesh client
+     */
+    match_mark = (mark & bat_priv->isolation_mark_mask);
+    if (bat_priv->isolation_mark_mask &&
+        match_mark == bat_priv->isolation_mark)
+        flags |= BATADV_TT_CLIENT_ISOLA;
+
+    return flags;
+}
+
+static bool
+batadv_tt_local_refresh_needed(struct batadv_tt_local_entry *tt_local,
+                   u16 dynamic_flags)
+{
+    struct batadv_tt_common_entry *common = &tt_local->common;
+    u16 flags;
+
+    flags = batadv_tt_flags_get(common);
+
+    if (flags & (BATADV_TT_CLIENT_PENDING | BATADV_TT_CLIENT_ROAM))
+        return true;
+
+    if ((flags & BATADV_TT_LOCAL_DYNAMIC_MASK) != dynamic_flags)
+        return true;
+
+    return hlist_unhashed_lockless(&common->hash_entry);
+}
+
 /**
  * batadv_tt_local_refresh() - refresh a local TT entry of an active client
  * @bat_priv: the bat priv with all the mesh interface information
@@ -1021,15 +1063,23 @@ batadv_tt_local_refresh(struct batadv_priv *bat_priv,
     spinlock_t *list_lock; /* protects write access to the hash lists */
     struct batadv_tt_common_entry *common = &tt_local->common;
     struct batadv_hashtable *hash = bat_priv->tt.local_hash;
+    u16 dynamic_flags;
     u8 remote_flags;
-    u32 match_mark;
     u32 i;

+    dynamic_flags = batadv_tt_local_dynamic_flags(bat_priv, iif_is_wifi,
+                              mark);
+
+    tt_local->last_seen = jiffies;
+
+    /* fast path: nothing to modify or to announce */
+    if (!announce &&
+        !batadv_tt_local_refresh_needed(tt_local, dynamic_flags))
+        return true;
+
     i = batadv_choose_tt(common, hash->size);
     list_lock = &hash->list_locks[i];

-    tt_local->last_seen = jiffies;
-
     spin_lock_bh(list_lock);

     /* the entry was removed from the local table after it was looked up */
@@ -1068,21 +1118,8 @@ batadv_tt_local_refresh(struct batadv_priv *bat_priv,
          */
         remote_flags = common->flags & BATADV_TT_REMOTE_MASK;

-        if (iif_is_wifi)
-            common->flags |= BATADV_TT_CLIENT_WIFI;
-        else
-            common->flags &= ~BATADV_TT_CLIENT_WIFI;
-
-        /* check the mark in the skb: if it's equal to the configured
-         * isolation_mark, it means the packet is coming from an
-         * isolated non-mesh client
-         */
-        match_mark = (mark & bat_priv->isolation_mark_mask);
-        if (bat_priv->isolation_mark_mask &&
-            match_mark == bat_priv->isolation_mark)
-            common->flags |= BATADV_TT_CLIENT_ISOLA;
-        else
-            common->flags &= ~BATADV_TT_CLIENT_ISOLA;
+        common->flags &= ~BATADV_TT_LOCAL_DYNAMIC_MASK;
+        common->flags |= dynamic_flags;

         if (remote_flags ^ (common->flags & BATADV_TT_REMOTE_MASK))
             announce = true;

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

diff --git c/net/batman-adv/translation-table.c w/net/batman-adv/translation-table.c
index ff87bd30..5926a878 100644
--- c/net/batman-adv/translation-table.c
+++ w/net/batman-adv/translation-table.c
@@ -671,14 +671,22 @@ static u16 batadv_tt_flags_get(struct batadv_tt_common_entry *common)
 }

 /**
- * __batadv_tt_local_event() - store a local TT event (ADD/DEL) with given flags
+ * batadv_tt_local_event() - store a local TT event (ADD/DEL) with given flags
  * @bat_priv: the bat priv with all the mesh interface information
  * @common: the TT entry involved in the event
  * @flags: flags of the TT entry combined with the event flags
+ * @cancel_add: whether a DEL may cancel a queued ADD because the entry was
+ *  never announced to the other nodes
+ *
+ * Only a single change per client is queued. It describes the latest state of
+ * the client and a new event therefore replaces an already queued one. The
+ * only exception is a DEL for an entry which was never announced. It cancels
+ * the ADD of this entry because the other nodes don't need to know anything
+ * about it.
  */
-static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
-                    const struct batadv_tt_common_entry *common,
-                    u8 flags)
+static void batadv_tt_local_event(struct batadv_priv *bat_priv,
+                  const struct batadv_tt_common_entry *common,
+                  u8 flags, bool cancel_add)
 {
     struct batadv_tt_change_node *tt_change_node;
     struct batadv_tt_change_node *entry;
@@ -711,21 +719,19 @@ static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
             continue;

         del_op_entry = entry->change.flags & BATADV_TT_CLIENT_DEL;
-        if (del_op_requested != del_op_entry) {
-            /* DEL+ADD in the same orig interval have no effect and
-             * can be removed to avoid silly behaviour on the
-             * receiver side. The  other way around (ADD+DEL) can
-             * happen in case of roaming of  a client still in the
-             * NEW state. Roaming of NEW clients is now possible due
-             * to automatically recognition of "temporary" clients
+        if (cancel_add && del_op_requested && !del_op_entry) {
+            /* ADD+DEL of a client which was never announced (e.g.
+             * roaming of a client still in the NEW state) have no
+             * effect and can be removed
              */
             list_del(&entry->list);
             kmem_cache_free(batadv_tt_change_cache, entry);
             changes--;
         } else {
-            /* this is a second add or del in the same originator
-             * interval. It could mean that flags have been changed
-             * (e.g. double add): update them
+            /* the other nodes may know the client with different
+             * flags. A DEL+ADD must therefore still announce the
+             * current flags and an ADD+DEL must still remove the
+             * client. ADD+ADD and DEL+DEL only update the flags
              */
             entry->change.flags = flags;
         }
@@ -743,23 +749,6 @@ static void __batadv_tt_local_event(struct batadv_priv *bat_priv,
     spin_unlock_bh(&bat_priv->tt.changes_list_lock);
 }

-/**
- * batadv_tt_local_event() - store a local TT event (ADD/DEL)
- * @bat_priv: the bat priv with all the mesh interface information
- * @tt_local_entry: the TT entry involved in the event
- * @event_flags: flags to store in the event structure
- */
-static void batadv_tt_local_event(struct batadv_priv *bat_priv,
-                  struct batadv_tt_local_entry *tt_local_entry,
-                  u8 event_flags)
-{
-    struct batadv_tt_common_entry *common = &tt_local_entry->common;
-    u8 flags;
-
-    flags = batadv_tt_flags_get(common) | event_flags;
-    __batadv_tt_local_event(bat_priv, common, flags);
-}
-
 /**
  * batadv_tt_entries() - compute the number of entries fitting in tt_len bytes
  * @tt_len: available space
@@ -1099,6 +1088,7 @@ batadv_tt_local_refresh(struct batadv_priv *bat_priv,
     struct batadv_tt_common_entry *common = &tt_local->common;
     struct batadv_hashtable *hash = bat_priv->tt.local_hash;
     u16 dynamic_flags;
+    u16 event_flags;
     u8 remote_flags;
     u32 i;

@@ -1159,9 +1149,14 @@ batadv_tt_local_refresh(struct batadv_priv *bat_priv,
         if (remote_flags ^ (common->flags & BATADV_TT_REMOTE_MASK))
             announce = true;

-        if (announce)
-            __batadv_tt_local_event(bat_priv, common,
-                        common->flags);
+        /* BATADV_TT_CLIENT_ROAM can still be set when a client which
+         * roamed away was re-added. It must not be announced with an
+         * ADD because the other nodes would then handle it as roaming
+         */
+        if (announce) {
+            event_flags = common->flags & ~BATADV_TT_CLIENT_ROAM;
+            batadv_tt_local_event(bat_priv, common, event_flags, false);
+        }
     }

     spin_unlock_bh(list_lock);
@@ -1710,7 +1705,7 @@ batadv_tt_local_set_pending(struct batadv_priv *bat_priv,
     lockdep_assert_held(&hash->list_locks[i]);
     lockdep_assert_held(&common->flags_lock);

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

     batadv_dbg(BATADV_DBG_TT, bat_priv,
@@ -1800,6 +1795,7 @@ batadv_tt_local_remove_now(struct batadv_priv *bat_priv,
     spinlock_t *list_lock; /* protects write access to the hash lists */
     struct batadv_tt_common_entry *common = &tt_local_entry->common;
     struct batadv_hashtable *hash = bat_priv->tt.local_hash;
+    u16 flags;
     u32 i;

     i = batadv_choose_tt(common, hash->size);
@@ -1816,13 +1812,18 @@ batadv_tt_local_remove_now(struct batadv_priv *bat_priv,
     hlist_del_init_rcu(&common->hash_entry);
     atomic_inc(&hash->generation);

-    batadv_tt_local_event(bat_priv, tt_local_entry, BATADV_TT_CLIENT_DEL);
-
-    /* batadv_tt_local_transition_new() may have committed the entry and
-     * thus counted it in the local table size since the
-     * BATADV_TT_CLIENT_NEW check in batadv_tt_local_mark_removed().
+    /* batadv_tt_local_transition_new() clears BATADV_TT_CLIENT_NEW under
+     * the list_lock. The flags read here therefore show whether the entry
+     * was committed (and announced) since the BATADV_TT_CLIENT_NEW check
+     * in batadv_tt_local_mark_removed(). Only the ADD of an entry which
+     * was never announced can be cancelled by its DEL.
      */
-    if (!(batadv_tt_flags_get(common) & BATADV_TT_CLIENT_NEW))
+    flags = batadv_tt_flags_get(common);
+    batadv_tt_local_event(bat_priv, common, flags | BATADV_TT_CLIENT_DEL,
+                  flags & BATADV_TT_CLIENT_NEW);
+
+    /* a committed entry was counted in the local table size */
+    if (!(flags & BATADV_TT_CLIENT_NEW))
         batadv_tt_local_size_dec(bat_priv, common->vid);

     spin_unlock_bh(list_lock);
Actions #3

Updated by Sven Eckelmann 15 days ago

  • Related to Bug #475: tt: Fix current event queuing guarentees for batadv_tt_local_remove_now() added
Actions

Also available in: Atom PDF