Re: [PATCH v5 6/6] selftests/bpf: tc_tunnel validate decap GSO state

bot+bpf-ci
2026-04-20T11:19:30Z

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

> diff --git a/tools/testing/selftests/bpf/progs/test_tc_tunnel.c b/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
> index 7376df405a6b..511022630bb1 100644
> --- a/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
> +++ b/tools/testing/selftests/bpf/progs/test_tc_tunnel.c

[ ... ]

> @@ -621,6 +640,11 @@ static int decap_internal(struct __sk_buff *skb, int off, int len, char proto)
> break;
> case IPPROTO_GRE:
> olen += sizeof(struct gre_hdr);
> + if (!bpf_core_enum_value_exists(enum bpf_adj_room_flags,
> + BPF_F_ADJ_ROOM_DECAP_L4_GRE))
> + return TC_ACT_SHOT;
> + flags |= BPF_F_ADJ_ROOM_DECAP_L4_GRE;
> +

[ ... ]

> @@ -634,6 +658,10 @@ static int decap_internal(struct __sk_buff *skb, int off, int len, char proto)
> break;
> case IPPROTO_UDP:
> olen += sizeof(struct udphdr);
> + if (!bpf_core_enum_value_exists(enum bpf_adj_room_flags,
> + BPF_F_ADJ_ROOM_DECAP_L4_UDP))
> + return TC_ACT_SHOT;
> + flags |= BPF_F_ADJ_ROOM_DECAP_L4_UDP;

[ ... ]

> @@ -655,6 +683,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);

Could the pointer arithmetic here cause compilation issues 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 invalid in C and would fail to compile.

Martin KaFai Lau noted that other tests use this pattern and suggested it
could be addressed with bpf_core_field_size(kskb->end) if 32-bit support
becomes necessary in the future.

Ref: https://lore.kernel.org/bpf/20260416124324.B0CE6C2BCAF@smtp.kernel.org/

> + 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 IPXIP4 and IPXIP6 checks reachable?

Looking earlier in decap_internal(), for IPPROTO_IPIP and IPPROTO_IPV6
cases, the function only sets BPF_F_ADJ_ROOM_DECAP_L3_IPV4 and
BPF_F_ADJ_ROOM_DECAP_L3_IPV6 on the flags variable (around the protocol
switch statement).

Because BPF_F_ADJ_ROOM_DECAP_IPXIP4 and BPF_F_ADJ_ROOM_DECAP_IPXIP6 are
never added to flags, wouldn't these validation checks always be skipped?

Ref: https://lore.kernel.org/bpf/20260416124324.B0CE6C2BCAF@smtp.kernel.org/

> +
> + 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;
> }

---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/24662733093