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

[Sven Eckelmann <sven@narfation.org>]

[-- 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 --]