* [PATCH v5 6/6] selftests/bpf: tc_tunnel validate decap GSO state [not found] <20260420104051.1528843-1-nhudson@akamai.com> @ 2026-04-20 10:40 ` Nick Hudson 2026-04-20 11:19 ` bot+bpf-ci 0 siblings, 1 reply; 3+ messages in thread From: Nick Hudson @ 2026-04-20 10:40 UTC (permalink / raw) To: bpf, netdev, Willem de Bruijn, Martin KaFai Lau Cc: Nick Hudson, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, Shuah Khan, linux-kselftest, linux-kernel 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. Signed-off-by: Nick Hudson <nhudson@akamai.com> --- .../selftests/bpf/progs/test_tc_tunnel.c | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) 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 @@ -6,6 +6,7 @@ #include <bpf/bpf_helpers.h> #include <bpf/bpf_endian.h> +#include <bpf/bpf_core_read.h> #include "bpf_tracing_net.h" #include "bpf_compiler.h" @@ -37,6 +38,22 @@ struct vxlanhdr___local { #define EXTPROTO_VXLAN 0x1 +#define SKB_GSO_UDP_TUNNEL_MASK (SKB_GSO_UDP_TUNNEL | \ + SKB_GSO_UDP_TUNNEL_CSUM) + +#define SKB_GSO_TUNNEL_MASK (SKB_GSO_UDP_TUNNEL_MASK | \ + SKB_GSO_GRE | \ + SKB_GSO_GRE_CSUM | \ + SKB_GSO_IPXIP4 | \ + SKB_GSO_IPXIP6 | \ + SKB_GSO_ESP) + +#define BPF_F_ADJ_ROOM_DECAP_L4_MASK (BPF_F_ADJ_ROOM_DECAP_L4_UDP | \ + BPF_F_ADJ_ROOM_DECAP_L4_GRE) + +#define BPF_F_ADJ_ROOM_DECAP_IPXIP_MASK (BPF_F_ADJ_ROOM_DECAP_IPXIP4 | \ + BPF_F_ADJ_ROOM_DECAP_IPXIP6) + #define VXLAN_FLAGS bpf_htonl(1<<27) #define VNI_ID 1 #define VXLAN_VNI bpf_htonl(VNI_ID << 8) @@ -592,6 +609,8 @@ int __encap_ip6vxlan_eth(struct __sk_buff *skb) static int decap_internal(struct __sk_buff *skb, int off, int len, char proto) { __u64 flags = BPF_F_ADJ_ROOM_FIXED_GSO; + struct sk_buff *kskb; + struct skb_shared_info *shinfo; struct ipv6_opt_hdr ip6_opt_hdr; struct gre_hdr greh; struct udphdr udph; @@ -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; + if (bpf_skb_load_bytes(skb, off + len, &greh, sizeof(greh)) < 0) return TC_ACT_OK; switch (bpf_ntohs(greh.protocol)) { @@ -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; if (bpf_skb_load_bytes(skb, off + len, &udph, sizeof(udph)) < 0) return TC_ACT_OK; switch (bpf_ntohs(udph.dest)) { @@ -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); + 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; + + 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; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v5 6/6] selftests/bpf: tc_tunnel validate decap GSO state 2026-04-20 10:40 ` [PATCH v5 6/6] selftests/bpf: tc_tunnel validate decap GSO state Nick Hudson @ 2026-04-20 11:19 ` bot+bpf-ci 2026-04-27 22:36 ` Martin KaFai Lau 0 siblings, 1 reply; 3+ messages in thread From: bot+bpf-ci @ 2026-04-20 11:19 UTC (permalink / raw) To: nhudson, bpf, netdev, willemb, martin.lau Cc: nhudson, ast, daniel, andrii, eddyz87, memxor, shuah, linux-kselftest, linux-kernel, ast, andrii, daniel, martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai [-- 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v5 6/6] selftests/bpf: tc_tunnel validate decap GSO state 2026-04-20 11:19 ` bot+bpf-ci @ 2026-04-27 22:36 ` Martin KaFai Lau 0 siblings, 0 replies; 3+ messages in thread From: Martin KaFai Lau @ 2026-04-27 22:36 UTC (permalink / raw) To: nhudson Cc: bot+bpf-ci, bpf, netdev, willemb, ast, daniel, andrii, eddyz87, memxor, shuah, linux-kselftest, linux-kernel, martin.lau, yonghong.song, clm, ihor.solodrai On Mon, Apr 20, 2026 at 11:19:30AM +0000, bot+bpf-ci@kernel.org wrote: > > + 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/ This comment has been mentioned again. It seems legit. Please don't ignore the review comment. Otherwise, it will be assumed that the solutions are being worked on and patchwork will eventually archive the patches after some time. Also, there is a 'SYS(fail_close_ns_client, "ethtool -K veth1 tso off");' in prog_tests/test_tc_tunnel.c. Is the shinfo->gso_type ever set? What will break if tso is kept on? ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-04-27 22:36 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260420104051.1528843-1-nhudson@akamai.com>
2026-04-20 10:40 ` [PATCH v5 6/6] selftests/bpf: tc_tunnel validate decap GSO state Nick Hudson
2026-04-20 11:19 ` bot+bpf-ci
2026-04-27 22:36 ` Martin KaFai Lau
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox