Re: [PATCH net-next 10/15] batman-adv: tt: extract marking of a removed local entry

[Sven Eckelmann <sven@narfation.org>]

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

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

[...]
> Discovered by: opus-5, old-prompts · Missed by: gpt-5-6-sol, sashiko-gemini
> 
> Two comments in batadv_tt_local_remove() look like they no longer match the
> code after the move.
> 
> First, the untouched comment further down in batadv_tt_local_remove() still
> refers to a check "above":
> 
> net/batman-adv/translation-table.c:batadv_tt_local_remove() {
> 	...
> 	/* 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);
> 	...
> }
> 
> After this patch there is no BATADV_TT_CLIENT_NEW test left anywhere above
> that point in this function; the only one is now inside the new helper:
>
> net/batman-adv/translation-table.c:batadv_tt_local_mark_removed() {
> 	...
> 		if (!(common->flags & BATADV_TT_CLIENT_NEW)) {
> 			common->flags |= BATADV_TT_CLIENT_PENDING;
> 			pending = true;
> 		}
> 	...
> }
> 
> Should the wording be updated in this patch to name the helper, so the
> size-accounting race stays traceable from the function it is documented in?

See patch 11 "batman-adv: tt: extract immediate purge of a local entry"


> Second, the retained comment above the "if (roaming)" line still says the
> node "has to mark the local entry as roamed", but the code it now precedes
> only assembles the announcement flag word; the actual
> common->flags |= BATADV_TT_CLIENT_ROAM happens in the
> batadv_tt_local_mark_removed() call two lines below.  Would it read better
> moved or reworded so it describes the flag assembly?
> 
> No functional change is apparent here: flags is fully computed before the
> call, *curr_flags is written unconditionally inside the guard so curr_flags
> is always initialized for the caller, and the helper is only reached after
> the !tt_local_entry early goto.

See https://patchwork.open-mesh.org/project/b.a.t.m.a.n./patch/20260908-sashiko-kdoc-v1-2-535b498f2519@narfation.org/


Regards,
	Sven

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