Re: [PATCH net 6/9] batman-adv: fix TX priority extraction for BATADV_FORW_MCAST

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

[...]

quoted

@@ -338,6 +340,9 @@ static netdev_tx_t batadv_interface_tx(struct sk_buff *skb,
 
 send:
 		if (do_bcast && !is_broadcast_ether_addr(ethhdr->h_dest)) {
+			/* WARNING batadv_mcast_forw_mode might add more headers
+			 * in front of the skb. and might even reallocate the skb
+			 */
 			forw_mode = batadv_mcast_forw_mode(bat_priv, skb, vid,
 							   &mcast_is_routable);

Discovered by: opus-5 · Missed by: gpt-5-6-sol, sashiko-gemini · Confirmed by: gpt-5-6-sol
This isn't a bug introduced by this patch, but would it make sense to put
this warning into the kernel-doc of the callee instead of (or in addition
to) the call site?

Today only batadv_mcast_forw_push() carries the contract in
net/batman-adv/multicast_forw.c:

  * Warning: This function may reallocate the skb data buffer via
  * batadv_mcast_forw_expand_head()/... Any pointer into the skb data (e.g.
  * obtained from skb->data or eth_hdr()) before this call must be
  * considered invalid afterwards and has to be reacquired.

The two callers 

What are the two callers? I only find batadv_mcast_forw_mode_by_count()

in net/batman-adv/multicast.c do not mention it.
batadv_mcast_forw_mode_by_count() only says:

  * For a multicast packet with multiple destination originators, checks which
  * mode to use. For BATADV_FORW_MCAST it also encapsulates the packet with a
  * complete batman-adv multicast header.

and the exported entry point batadv_mcast_forw_mode() (declared in
net/batman-adv/multicast.h) documents only:

  * Return: The forwarding mode as enum batadv_forw_mode.

Since batadv_mcast_forw_mode() is the exported symbol, wouldn't a future
caller reading its kernel-doc have no way to know that skb->data was pushed
or that the buffer may have been reallocated?

Will not modify this patch for this request. Linus (or anyone else) can deal 
with it: https://www.open-mesh.org/issues/466

Related to that: batadv_interface_tx() does not re-acquire ethhdr after
batadv_mcast_forw_mode() returns, even though the new comment warns about
reallocation. The only later use, is_broadcast_ether_addr(ethhdr->h_dest),
is evaluated before the call, 

The later use is before? This is crazy

so nothing is dereferenced stale right now.
Is it worth re-acquiring it anyway, or noting in the comment that ethhdr
must not be used past this point?

Will not modify the patch for this.

quoted

 			switch (forw_mode) {

[ ... ]

quoted

@@ -355,8 +360,6 @@ static netdev_tx_t batadv_interface_tx(struct sk_buff *skb,
 		}
 	}
 
-	batadv_skb_set_priority(skb, 0);
-
 	/* ethernet packet should be broadcasted */
 	if (do_bcast) {
 		primary_if = batadv_primary_if_get_selected(bat_priv);

Discovered by: opus-5 · Missed by: gpt-5-6-sol, sashiko-gemini · Confirmed by: gpt-5-6-sol
This is a pre-existing issue and not caused by this patch, but while the
priority handling is being looked at: should the multicast forwarding path
also set skb->priority?

This is a feature (or maybe minor bugfix) request. Will definitely not deal 
with it here. Documented it at https://www.open-mesh.org/issues/467

Regards,
	Sven