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

Sven Eckelmann <[email protected]>
Newsgroups org.open-mesh.lists.batman,org.kernel.vger.netdev
Message-ID <13100789.O9o76ZdvQC@sven-desktop>
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
signature.asc (application/pgp-signature, 228 B)
-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQS81G/PswftH/OW8cVND3cr0xT1ywUCanSp7AAKCRBND3cr0xT1
y+3cAP9Bgos/68o4rtE36Mp1RREe0rZHjuc9O9x2dvk1ChHLRAEAi8ofm3b8zmuj
wFad719lpRvDouBXIwMqgQDf4xtGuAQ=
=4etA
-----END PGP SIGNATURE-----
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.