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

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

[Nick Hudson]

This follow-up tightens the selftest to guard against false passes in
GSO decap validation.

It validates the expected post-decap state for both GSO and non-GSO
packets: expected tunnel gso_type bits must be cleared and
skb->encapsulation must match the remaining tunnel state.

It also adds explicit assertions that the decap validation path was
executed, including the large-send phase for GSO-marked subtests, and
asserts that the large-send marker is reset after the send.

This keeps the tc_tunnel decap checks meaningful and avoids false-pass
regressions without over-constraining behavior across tunnel modes.

Signed-off-by: Nick Hudson <nhudson@akamai.com>
---
 .../selftests/bpf/prog_tests/test_tc_tunnel.c | 33 +++++++++++++++----
 .../selftests/bpf/progs/test_tc_tunnel.c      | 19 +++++++++++
 2 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c b/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c
index 67ba27d69347..08d7d90f7772 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c
@@ -206,7 +206,7 @@ static void disconnect_client_from_server(struct subtest_cfg *cfg,
 	free(conn);
 }
 
-static int send_and_test_data(struct subtest_cfg *cfg)
+static int send_and_test_data(struct subtest_cfg *cfg, struct test_tc_tunnel *skel)
 {
 	struct connection *conn;
 	int err, res = -1;
@@ -215,6 +215,7 @@ static int send_and_test_data(struct subtest_cfg *cfg)
 	if (!ASSERT_OK_PTR(conn, "connect to server"))
 		return -1;
 
+	skel->bss->decap_expect_large_send = 0;
 	err = send(conn->client_fd, tx_buffer, DEFAULT_TEST_DATA_SIZE, 0);
 	if (!ASSERT_EQ(err, DEFAULT_TEST_DATA_SIZE, "send data from client"))
 		goto end;
@@ -226,14 +227,17 @@ static int send_and_test_data(struct subtest_cfg *cfg)
 		goto end;
 	}
 
+	skel->bss->decap_expect_large_send = 1;
 	err = send(conn->client_fd, tx_buffer, GSO_TEST_DATA_SIZE, 0);
 	if (!ASSERT_EQ(err, GSO_TEST_DATA_SIZE, "send (large) data from client"))
 		goto end;
 	if (check_server_rx_data(cfg, conn, DEFAULT_TEST_DATA_SIZE))
 		goto end;
+	skel->bss->decap_expect_large_send = 0;
 
 	res = 0;
 end:
+	skel->bss->decap_expect_large_send = 0;
 	disconnect_client_from_server(cfg, conn);
 	return res;
 }
@@ -374,10 +378,15 @@ static int configure_ebpf_decapsulation(struct subtest_cfg *cfg)
 	return ret;
 }
 
-static void run_test(struct subtest_cfg *cfg)
+static void run_test(struct subtest_cfg *cfg, struct test_tc_tunnel *skel)
 {
 	struct nstoken *nstoken;
 
+	skel->bss->decap_validation_seen = 0;
+	skel->bss->decap_gso_validation_seen = 0;
+	skel->bss->decap_large_send_validation_seen = 0;
+	skel->bss->decap_expect_large_send = 0;
+
 	if (!ASSERT_OK(run_server(cfg), "run server"))
 		return;
 
@@ -386,7 +395,7 @@ static void run_test(struct subtest_cfg *cfg)
 		goto fail;
 
 	/* Basic communication must work */
-	if (!ASSERT_OK(send_and_test_data(cfg), "connect without any encap"))
+	if (!ASSERT_OK(send_and_test_data(cfg, skel), "connect without any encap"))
 		goto fail;
 
 	/* Attach encapsulation program to client */
@@ -398,7 +407,7 @@ static void run_test(struct subtest_cfg *cfg)
 		if (!ASSERT_OK(configure_kernel_decapsulation(cfg),
 					"configure kernel decapsulation"))
 			goto fail;
-		if (!ASSERT_OK(send_and_test_data(cfg),
+		if (!ASSERT_OK(send_and_test_data(cfg, skel),
 			       "connect with encap prog and kern decap"))
 			goto fail;
 	}
@@ -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");
+	}
 
 fail:
 	close_netns(nstoken);
@@ -438,6 +458,7 @@ static int setup(void)
 	SYS(fail_close_ns_client, "ip link add %s type veth peer name %s",
 	    "veth1 mtu 1500 netns " CLIENT_NS " address " MAC_ADDR_VETH1,
 	    "veth2 mtu 1500 netns " SERVER_NS " address " MAC_ADDR_VETH2);
+	SYS(fail_close_ns_client, "ethtool -K veth1 tso off");
 	SYS(fail_close_ns_client, "ip link set veth1 up");
 	nstoken_server = open_netns(SERVER_NS);
 	if (!ASSERT_OK_PTR(nstoken_server, "open server ns"))
@@ -701,7 +722,7 @@ void test_tc_tunnel(void)
 		if (ret < 0 || !test__start_subtest(cfg->name))
 			continue;
 		if (subtest_setup(skel, cfg) == 0)
-			run_test(cfg);
+			run_test(cfg, skel);
 		subtest_cleanup(cfg);
 	}
 	cleanup();
diff --git a/tools/testing/selftests/bpf/progs/test_tc_tunnel.c b/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
index 853bca962910..e9bd1c9781f7 100644
--- a/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
+++ b/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
@@ -16,6 +16,11 @@ static const int cfg_port = 8000;
 
 static const int cfg_udp_src = 20000;
 
+__u64 decap_validation_seen;
+__u64 decap_gso_validation_seen;
+__u64 decap_large_send_validation_seen;
+__u32 decap_expect_large_send;
+
 #define ETH_P_MPLS_UC	0x8847
 #define ETH_P_TEB	0x6558
 
@@ -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++;
+
 		if ((flags & BPF_F_ADJ_ROOM_DECAP_L4_UDP) &&
 		    (shinfo->gso_type & SKB_GSO_UDP_TUNNEL_MASK))
 			return TC_ACT_SHOT;
-- 
2.34.1

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

[sashiko-bot]

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

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

[bot+bpf-ci]

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

> This follow-up tightens the selftest to guard against false passes in
> GSO decap validation.
>
> It validates the expected post-decap state for both GSO and non-GSO
> packets: expected tunnel gso_type bits must be cleared and
> skb->encapsulation must match the remaining tunnel state.

Does this paragraph accurately describe what the patch adds?

The gso_type and skb->encapsulation validation it mentions already exists
in the base revision. Comparing HEAD^ with HEAD for
progs/test_tc_tunnel.c shows the added lines are only the four .bss
globals and three counter increments. The checks described appear only
as context lines:

  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_MASK |
               BPF_F_ADJ_ROOM_DECAP_IPXIP_MASK)) {
          if ((shinfo->gso_type & SKB_GSO_TUNNEL_MASK) &&
              !kskb->encapsulation)
                  return TC_ACT_SHOT;

All of this validation is pre-existing. The series is a single commit
(530113d..1dc5f339), so this code is already upstream before this patch.

> It also adds explicit assertions that the decap validation path was
> executed, including the large-send phase for GSO-marked subtests, and
> asserts that the large-send marker is reset after the send.

The large-send marker reset assertion mentioned here checks a variable
that only userspace writes. decap_expect_large_send is read-only in the
BPF program, and send_and_test_data() sets it to 0 on both the success
path and the `end:` path before returning. Can that assertion ever fail?

Also, the commit message doesn't mention the 'ethtool -K veth1 tso off'
change in setup() even though it alters what traffic the test generates.

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c b/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c
> index 67ba27d69347..08d7d90f7772 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c

[ ... ]

> @@ -374,10 +378,15 @@ static int configure_ebpf_decapsulation(struct subtest_cfg *cfg)
>  	return ret;
>  }
>
> -static void run_test(struct subtest_cfg *cfg)
> +static void run_test(struct subtest_cfg *cfg, struct test_tc_tunnel *skel)
>  {
>  	struct nstoken *nstoken;
>
> +	skel->bss->decap_validation_seen = 0;
> +	skel->bss->decap_gso_validation_seen = 0;
> +	skel->bss->decap_large_send_validation_seen = 0;
> +	skel->bss->decap_expect_large_send = 0;
> +
>  	if (!ASSERT_OK(run_server(cfg), "run server"))
>  		return;
>
> @@ -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");
> +	}

Is the decap_expect_large_send assertion meaningful here?

decap_expect_large_send is written only by user space; the BPF program
only reads it. send_and_test_data() clears it on every exit path:

  skel->bss->decap_expect_large_send = 0;   /* line 236, success */

  res = 0;
  end:
  skel->bss->decap_expect_large_send = 0;   /* line 240, all paths */

Every return from send_and_test_data() passes through the `end:` label,
so by the time run_test() reads the value it is unconditionally 0.

A broken kernel, a broken BPF program and a correct one all produce the
same result. Can this check ever fail?

> diff --git a/tools/testing/selftests/bpf/progs/test_tc_tunnel.c b/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
> index 853bca962910..e9bd1c9781f7 100644
> --- a/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
> +++ b/tools/testing/selftests/bpf/progs/test_tc_tunnel.c

[ ... ]

This looks like a bug fix for false-pass gaps in the selftest. Should
this include:

  Fixes: adb771973026 ("selftests/bpf: tc_tunnel - validate decap GSO and encapsulation state")


---
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/32136539422