Re: [PATCH net 3/3] selftests/bpf: Add test for redirect from qdisc qevent block

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The test is flaky due to a race condition with asynchronous ARP resolution.
- [Low] Semantically incorrect use of the tcx-specific constant `TCX_REDIRECT` in a legacy `cls_bpf` program.
--

commit fc8f69a4140a0efbcb7d3933eb46154533713ffc
Author: Daniel Borkmann <daniel@iogearbox.net>

selftests/bpf: Add test for redirect from qdisc qevent block

This commit adds a regression test for a NULL current->bpf_net_context
dereference. It verifies the behavior when a BPF classifier attached to
a qdisc qevent block asks for a redirect.

> diff --git a/tools/testing/selftests/bpf/prog_tests/tc_qevent.c b/tools/testing/selftests/bpf/prog_tests/tc_qevent.c
> new file mode 100644
> index 0000000000000..67e1d17567abd
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/tc_qevent.c
> @@ -0,0 +1,113 @@
[ ... ]
> +static void blast_udp(void)
> +{
[ ... ]
> +	/*
> +	 * Push far more than the RED queue can hold. Once qavg crosses qth_min
> +	 * every further packet hits the congestion_drop / early_drop qevent.
> +	 */
> +	for (i = 0; i < 50000; i++)
> +		sendto(fd, buf, sizeof(buf), MSG_DONTWAIT,
> +		       (struct sockaddr *)&dst, sizeof(dst));
> +
> +	close(fd);
> +}
[ ... ]
> +static void run_qevent_redirect(struct bpf_program *prog, __u64 *counter)
> +{
[ ... ]
> +	if (system("tc filter add block 10 bpf da object-pinned "
> +		   PIN_PATH " 2>/dev/null")) {
> +		test__skip();
> +		goto close_ns;
> +	}
> +
> +	blast_udp();
> +	ASSERT_GT(*counter, 0, "qevent classifier ran");

[Severity: Medium]
Since static ARP entries are not configured, could this lead to a flaky test
due to a race condition with asynchronous ARP resolution?

When blast_udp() loops through sendto(), the first packets might trigger an
ARP request while the rest get dropped or queued by the neighbor layer before
reaching the RED qdisc.

Because ASSERT_GT() is checked immediately after blast_udp(), is it possible
the check evaluates before the network stack finishes processing the ARP
reply, leaving the qdisc with no packets and causing the test to fail?

> diff --git a/tools/testing/selftests/bpf/progs/test_tc_qevent.c b/tools/testing/selftests/bpf/progs/test_tc_qevent.c
> new file mode 100644
> index 0000000000000..1529c111f4aa4
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_tc_qevent.c
> @@ -0,0 +1,23 @@
[ ... ]
> +SEC("tc")
> +int qevent_redirect_verdict(struct __sk_buff *skb)
> +{
> +	__sync_fetch_and_add(&verdict_calls, 1);
> +	return TCX_REDIRECT;
> +}

[Severity: Low]
Is it correct to return the TCX_REDIRECT constant here?

Since this program is attached using the legacy cls_bpf via tc filter add bpf
da in run_qevent_redirect(), shouldn't it return standard TC_ACT_* return
codes like TC_ACT_REDIRECT?

They both evaluate to 7 so it functions correctly, but mixing the newer TCX
framework constants in a legacy program might cause API confusion.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630123331.186840-1-daniel@iogearbox.net?part=3