Re: [PATCH bpf-next v4 6/6] selftests/bpf: tc_tunnel validate decap GSO state

[sashiko-bot@kernel.org]

Sashiko AI review found 3 potential issue(s):
- [Medium] The test's definition of SKB_GSO_UDP_TUNNEL_MASK incorrectly includes SKB_GSO_TUNNEL_REMCSUM, causing a semantic mismatch with the kernel's bpf_skb_net_shrink() implementation and false failures on REMCSUM packets.
- [Medium] The assertions for IPXIP4/IPXIP6 GSO flag clearance and encapsulation state are unreachable dead code because the test never sets the BPF_F_ADJ_ROOM_DECAP_IPXIP4 or BPF_F_ADJ_ROOM_DECAP_IPXIP6 flags.
- [Low] Invalid pointer arithmetic (`kskb->head + kskb->end`) causes a compilation failure on 32-bit architectures.
--

commit 8a46b3df0c44611594d9db9e0db3c3d027d83a7d
Author: Nick Hudson <nhudson@akamai.com>

selftests/bpf: tc_tunnel validate decap GSO state

Require BPF_F_ADJ_ROOM_DECAP_L4_UDP and BPF_F_ADJ_ROOM_DECAP_L4_GRE enum
values at runtime using CO-RE enum existence checks so missing kernel
support fails fast instead of silently proceeding.

After bpf_skb_adjust_room() decapsulation, inspect skb_shared_info and
sk_buff state for GSO packets and assert that the expected tunnel GSO
bits are cleared and encapsulation matches the remaining tunnel state.

> diff --git a/tools/testing/selftests/bpf/progs/test_tc_tunnel.c b/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
> --- a/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
> +++ b/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
> @@ -37,6 +38,23 @@ struct vxlanhdr___local {
>  
>  #define	EXTPROTO_VXLAN	0x1
>  
> +#define SKB_GSO_UDP_TUNNEL_MASK	(SKB_GSO_UDP_TUNNEL |			\
> +				 SKB_GSO_UDP_TUNNEL_CSUM |		\
> +				 SKB_GSO_TUNNEL_REMCSUM)

Does this mask incorrectly include SKB_GSO_TUNNEL_REMCSUM?

The kernel's bpf_skb_net_shrink() implementation intentionally does not clear
SKB_GSO_TUNNEL_REMCSUM when processing BPF_F_ADJ_ROOM_DECAP_L4_UDP. If a
packet with REMCSUM is processed, won't this cause a spurious test failure
when the test later checks (shinfo->gso_type & SKB_GSO_UDP_TUNNEL_MASK)?

[ ... ]
> @@ -655,6 +684,35 @@ static int decap_internal(struct __sk_buff *skb, int off, int len, char proto)
>  	if (bpf_skb_adjust_room(skb, -olen, BPF_ADJ_ROOM_MAC, flags))
>  		return TC_ACT_SHOT;
>  
> +	kskb = bpf_cast_to_kern_ctx(skb);
> +	shinfo = bpf_core_cast(kskb->head + kskb->end, struct skb_shared_info);

Will this cause a compilation failure on 32-bit architectures?

On 32-bit architectures without NET_SKBUFF_DATA_USES_OFFSET, sk_buff::end
is defined as a pointer (unsigned char *) rather than an offset. Adding two
pointers is an invalid operation in C and will result in a build error.

> +	if (!shinfo->gso_size)
> +		return TC_ACT_OK;
> +
> +	if ((flags & BPF_F_ADJ_ROOM_DECAP_L4_UDP) &&
> +	    (shinfo->gso_type & SKB_GSO_UDP_TUNNEL_MASK))
> +		return TC_ACT_SHOT;
> +
> +	if ((flags & BPF_F_ADJ_ROOM_DECAP_L4_GRE) &&
> +	    (shinfo->gso_type & (SKB_GSO_GRE | SKB_GSO_GRE_CSUM)))
> +		return TC_ACT_SHOT;
> +
> +	if ((flags & BPF_F_ADJ_ROOM_DECAP_IPXIP4) &&
> +	    (shinfo->gso_type & SKB_GSO_IPXIP4))
> +		return TC_ACT_SHOT;
> +
> +	if ((flags & BPF_F_ADJ_ROOM_DECAP_IPXIP6) &&
> +	    (shinfo->gso_type & SKB_GSO_IPXIP6))
> +		return TC_ACT_SHOT;

Are these conditions unreachable?

Looking earlier in decap_internal(), for IPPROTO_IPIP and IPPROTO_IPV6, only
BPF_F_ADJ_ROOM_DECAP_L3_IPV4 and BPF_F_ADJ_ROOM_DECAP_L3_IPV6 are set on the
flags variable.

Because BPF_F_ADJ_ROOM_DECAP_IPXIP4 and BPF_F_ADJ_ROOM_DECAP_IPXIP6 are
never added to flags, this IPXIP GSO validation block and the subsequent
encapsulation validation appear to be bypassed entirely.

> +	if (flags & (BPF_F_ADJ_ROOM_DECAP_L4_MASK |
> +		     BPF_F_ADJ_ROOM_DECAP_IPXIP_MASK)) {
> +		if ((shinfo->gso_type & SKB_GSO_TUNNEL_MASK) && !kskb->encapsulation)
> +			return TC_ACT_SHOT;
> +		if (!(shinfo->gso_type & SKB_GSO_TUNNEL_MASK) && kskb->encapsulation)
> +			return TC_ACT_SHOT;
> +	}
> +
>  	return TC_ACT_OK;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260416075514.927101-1-nhudson@akamai.com?part=6