Open-Mesh » batman-adv

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

> Is the bucket list_lock enough to serialize this against
> batadv_tt_local_commit_changes_nolock()?
> 
> The lock only orders this code against the
> batadv_tt_local_purge_pending_clients() pass over this one bucket. The
> later phases of the commit take no bucket lock:
> batadv_tt_local_update_crc() walks the hash under RCU, and
> batadv_tt_tvlv_container_update() drains the change list. The only thing
> holding those phases together is tt.commit_lock, and
> batadv_tt_local_remove() never takes it.
> 
> That seems to leave two windows with the result the commit message says
> it avoids.
> 
> In the first window the DEL is lost:
> 
> CPU0                              CPU1
>                                   batadv_tt_local_commit_changes_nolock()
>                                     ..._purge_pending_clients()
>                                     batadv_tt_local_update_crc()
>                                     batadv_tt_tvlv_container_update()
>                                       /* snapshot local_changes == K */
> batadv_tt_local_mark_removed()
>   spin_lock_bh(list_lock);
>   flags |= ..._PENDING;
>   batadv_tt_local_event();
>   /* local_changes == K + 1 */
>   spin_unlock_bh(list_lock);
>                                       spin_lock_bh(&changes_list_lock);
>                                       /* copy K, free all K + 1 */
> 
> This is the batadv_tt_tvlv_container_update() code involved:
> 
>     tt_diff_entries_num = READ_ONCE(bat_priv->tt.local_changes);
>     ...
>     list_for_each_entry_safe(entry, safe, &bat_priv->tt.changes_list,
>                  list) {
>         if (tt_diff_entries_count < tt_diff_entries_num) {
>             memcpy(tt_change + tt_diff_entries_count, ...);
>             tt_diff_entries_count++;
>         }
>         list_del(&entry->list);
>         kmem_cache_free(batadv_tt_change_cache, entry);
>     }
> 
> E stays PENDING with no DEL queued. The next commit's purge removes E
> from the table and the CRC, and no DEL is ever sent. The gap between the
> snapshot and the drain also includes batadv_tt_prepare_tvlv_local_data().
> 
> In the second window the DEL is announced with a CRC that still counts
> the client. batadv_tt_local_mark_removed() runs after the purge has
> scanned this bucket but before the snapshot, so the DEL goes out in TTVN
> n+1. batadv_tt_local_crc() only skips NEW entries, which means the n+1
> CRC still includes the PENDING entry. Neighbours that apply the DEL then
> see a mismatch.
> 
> The commit message says:
> 
>   Move the event into batadv_tt_local_mark_removed() and hold the bucket
>   list_lock of the entry around both the flag change and the DEL event to
>   avoid this scenario.
> 
> That closes the exact interleaving in the diagram, but other remove vs
> commit interleavings still reach the same result.
> 
> Does closing this need commit-level serialization instead, for example
> taking tt.commit_lock around the marking and queueing?