Re: [PATCH v6 6/6] selftests/bpf: tc_tunnel - validate decap GSO and encapsulation state

Re: [PATCH v6 6/6] selftests/bpf: tc_tunnel - validate decap GSO and encapsulation state

[Hudson, Nick]

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



> On May 5, 2026, at 5:14 AM, sashiko-bot@kernel.org wrote:
> 
> !-------------------------------------------------------------------|
>  This Message Is From an External Sender
>  This message came from outside your organization.
> |-------------------------------------------------------------------!
> 
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Pointer arithmetic `kskb->head + kskb->end` causes build failures and runtime logic errors on 32-bit kernels.
> - [Medium] Missing kernel feature causes test to fail instead of skip, breaking CI on older kernels.
> --
> 
> commit f73fb4725baa7b601a1b82c32e5095c6b4294f5c
> Author: Nick Hudson <nhudson@akamai.com>
> 
> selftests/bpf: tc_tunnel - validate decap GSO and encapsulation state
> 
> This commit enhances the tc_tunnel selftest to validate post-decap tunnel
> state for both GSO and non-GSO packets, and checks IPXIP decap for IPIP/SIT
> paths.
> 
>> diff --git a/tools/testing/selftests/bpf/progs/test_tc_tunnel.c b/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
>> index 7376df405a6b..853bca962910 100644
>> --- a/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
>> +++ b/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
> [ ... ]
>> @@ -621,6 +645,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;
> 
> If the kernel is missing support for these new decapsulation flags, will
> returning TC_ACT_SHOT cause the test harness to time out and fail the test?
> 
> Since selftests often run on older kernels, should missing features gracefully
> skip the test instead of failing it to prevent false positives and CI
> breakages on stable or LTS branches?

The current TC_ACT_SHOT fallback prevents silent passes, but it does cause affected subtests to fail on kernels without the new decap flags. We should move feature gating to prog_tests and skip unsupported subtests there, so older kernels report SKIP instead of FAIL while preserving strict checks when the feature exists.

> 
> [ ... ]
>> @@ -655,6 +688,40 @@ 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 arithmetic work correctly on 32-bit kernels?
> 
> On 64-bit architectures, sk_buff_data_t is an integer offset, so
> kskb->head + kskb->end works. However, on 32-bit platforms, sk_buff_data_t is
> a pointer (unsigned char *), making the addition of two pointers invalid C and
> causing build failures.
> 
> Furthermore, even if cross-compiled with a 64-bit vmlinux.h, adding
> kskb->head to an absolute kskb->end pointer would yield an invalid address at
> runtime.

Martin previous replied

"Other tests have been using it to get shinfo already. I would leave it as is.
If it really would be needed to run and compile on 32-bit in the future,
it could use bpf_core_field_size(kskb->end) and then do things differently.”

To this.



> 
> -- 
> Sashiko AI review · https://urldefense.com/v3/__https://sashiko.dev/*/patchset/20260504101759.3319427-1-nhudson@akamai.com?part=6__;Iw!!GjvTz_vk!VFiErP1-AQql4LyaLTqxjmLQFQ7MMBQ5-R9rsHjqekoHUFNBl0WsEAX6ynWPASieLSi7E1DjAA-Rf64aLeCR$


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3066 bytes --]

Re: [PATCH v6 6/6] selftests/bpf: tc_tunnel - validate decap GSO and encapsulation state

[Martin KaFai Lau]

On Tue, May 19, 2026 at 10:52:12AM +0000, Hudson, Nick wrote:
> 
> 
> > On May 5, 2026, at 5:14 AM, sashiko-bot@kernel.org wrote:
> > 
> > !-------------------------------------------------------------------|
> >  This Message Is From an External Sender
> >  This message came from outside your organization.
> > |-------------------------------------------------------------------!
> > 
> > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > - [High] Pointer arithmetic `kskb->head + kskb->end` causes build failures and runtime logic errors on 32-bit kernels.
> > - [Medium] Missing kernel feature causes test to fail instead of skip, breaking CI on older kernels.
> > --
> > 
> > commit f73fb4725baa7b601a1b82c32e5095c6b4294f5c
> > Author: Nick Hudson <nhudson@akamai.com>
> > 
> > selftests/bpf: tc_tunnel - validate decap GSO and encapsulation state
> > 
> > This commit enhances the tc_tunnel selftest to validate post-decap tunnel
> > state for both GSO and non-GSO packets, and checks IPXIP decap for IPIP/SIT
> > paths.
> > 
> >> diff --git a/tools/testing/selftests/bpf/progs/test_tc_tunnel.c b/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
> >> index 7376df405a6b..853bca962910 100644
> >> --- a/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
> >> +++ b/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
> > [ ... ]
> >> @@ -621,6 +645,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;
> > 
> > If the kernel is missing support for these new decapsulation flags, will
> > returning TC_ACT_SHOT cause the test harness to time out and fail the test?
> > 
> > Since selftests often run on older kernels, should missing features gracefully
> > skip the test instead of failing it to prevent false positives and CI
> > breakages on stable or LTS branches?
> 
> The current TC_ACT_SHOT fallback prevents silent passes, but it does cause affected subtests to fail on kernels without the new decap flags. We should move feature gating to prog_tests and skip unsupported subtests there, so older kernels report SKIP instead of FAIL while preserving strict checks when the feature exists.

imo, it is fine to stay as-is and fail early instead of skip.
The new test added to the -next is targeting the new feature
of the upstream kernel. The new test is not supposed to be backported
alone without backporting the kernel changes.