Re: [PATCH net-next 02/10] batman-adv: tvlv: handle negative tvlv processing return codes

[Sven Eckelmann <sven@narfation.org>]

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

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

> --- Patch [2]: [PATCH net-next 02/10] batman-adv: tvlv: handle negative tvlv 
processing return codes ---
[...]
> Discovered by: opus-5 · Missed by: sashiko-gemini
> Does batadv_recv_mcast_packet() really match the new "NET_RX_SUCCESS if the
> skb was locally received" wording?
> 
> In net/batman-adv/routing.c, ret is assigned exactly once, from
> batadv_tvlv_containers_process(), and the local delivery is gated by an
> additional length test:
> 
> 	ret = batadv_tvlv_containers_process(bat_priv, BATADV_MCAST, NULL, 
skb,
> 					     tvlv_buff, 
tvlv_buff_len);
> 	...
> 	hdr_size += tvlv_buff_len;
> 
> 	if (ret == NET_RX_SUCCESS && (skb->len - hdr_size >= ETH_HLEN)) {
> 		...
> 		batadv_interface_rx(bat_priv->mesh_iface, skb, hdr_size, 
NULL);
> 		/* skb was consumed */
> 		skb = NULL;
> 	}
> 
> free_skb:
> 	kfree_skb(skb);
> 
> 	return ret;

Yes, there are additional checks down the path. But it was still considered in 
the parsing of the TVLV as code which should be received locally. I am not a 
person which writes documentation all day and I have no idea how to convey all 
the details without repeating basically the whole code. At some point, we have 
to use something which is just good enough or somebody has to be found which 
is better than me in writing of documentation.

I will not submit any patch for this.

[..]
> Since the stated goal here is to make these return values reliable for
> future callers, should the wording be something like "NET_RX_SUCCESS if the
> skb was processed successfully"

No, this is not what is the (unfortunate) meaning of NET_RX_SUCCESS and 
NET_RX_DROP here.

Regards,
	Sven

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