Re: [PATCH] selftests/bpf: tc_tunnel - validate decap tunnel state without false-pass gaps

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The variable `decap_gso_validation_seen` is introduced to track whether GSO metadata was present during decap validation, but it is never asserted in the test suite.
--

--- Patch [1]: [PATCH] selftests/bpf: tc_tunnel - validate decap tunnel state without false-pass gaps ---
commit f4a6b83a5918e8a74560dcb08d2827677bbdfe70
Author: Nick Hudson <nhudson@akamai.com>

selftests/bpf: tc_tunnel - validate decap tunnel state without false-pass gaps

This commit tightens the tc_tunnel selftest to guard against false passes
in GSO decap validation. It asserts that decap validation paths are executed
during large-send phases and verifies post-decap states.

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c b/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c
> --- a/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c

[ ... ]

> @@ -406,7 +415,18 @@ static void run_test(struct subtest_cfg *cfg)
>  	/* Replace kernel decapsulation with BPF decapsulation, test must pass */
>  	if (!ASSERT_OK(configure_ebpf_decapsulation(cfg), "configure ebpf decapsulation"))
>  		goto fail;
> -	ASSERT_OK(send_and_test_data(cfg), "connect with encap and decap progs");
> +	if (!ASSERT_OK(send_and_test_data(cfg, skel), "connect with encap and decap progs"))
> +		goto fail;
> +	if (!ASSERT_NEQ(skel->bss->decap_validation_seen, 0,
> +			"decap validation executed"))
> +		goto fail;
> +	if (!ASSERT_EQ(skel->bss->decap_expect_large_send, 0,
> +		       "decap large-send marker reset"))
> +		goto fail;
> +	if (cfg->test_gso) {
> +		ASSERT_NEQ(skel->bss->decap_large_send_validation_seen, 0,
> +			   "decap validation executed for large send");
> +	}

[Severity: Medium]
Is decap_gso_validation_seen missing an assertion here?

The BPF program tracks this variable to verify the presence of GSO metadata,
but it appears the userspace test runner never actually checks it. 

If a kernel bug strips the GSO metadata before reaching the BPF program,
shinfo->gso_size would be 0, bypassing the inner GSO validations. The test
might still pass because it only asserts decap_large_send_validation_seen.

> 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

[ ... ]

> @@ -690,7 +695,21 @@ static int decap_internal(struct __sk_buff *skb, int off, int len, char proto,
>  
>  	kskb = bpf_cast_to_kern_ctx(skb);
>  	shinfo = bpf_core_cast(kskb->head + kskb->end, struct skb_shared_info);
> +
> +	if (flags & (BPF_F_ADJ_ROOM_DECAP_L4_MASK |
> +		     BPF_F_ADJ_ROOM_DECAP_IPXIP_MASK))
> +		decap_validation_seen++;
> +
> +	if ((flags & (BPF_F_ADJ_ROOM_DECAP_L4_MASK |
> +		      BPF_F_ADJ_ROOM_DECAP_IPXIP_MASK)) &&
> +	    decap_expect_large_send)
> +		decap_large_send_validation_seen++;
> +
>  	if (shinfo->gso_size) {
> +		if (flags & (BPF_F_ADJ_ROOM_DECAP_L4_MASK |
> +			     BPF_F_ADJ_ROOM_DECAP_IPXIP_MASK))
> +			decap_gso_validation_seen++;
> +

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