Thread (4 messages) flat view 4 messages, 3 authors, 18d ago

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

From: bot+bpf-ci@kernel.org
Date: 2026-08-18 12:59:59
Also in: bpf, linux-kselftest, lkml

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.
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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?
quoted hunk ↗ jump to hunk
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help