BPF List
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Jakub Sitnicki" <jakub@cloudflare.com>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH net-next 12/14] selftests/bpf: Test skb_ext read from seg6local End.BPF hook
Date: Sat, 15 Aug 2026 08:14:56 +0000	[thread overview]
Message-ID: <20260815081456.926C91F00A3D@smtp.kernel.org> (raw)
In-Reply-To: <20260814-bpf-meta-inside-skb-ext-v1-12-767edd862656@cloudflare.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The assertion `ASSERT_GE(tc_hook.ifindex, 0)` incorrectly accepts 0 as a valid interface index, masking `if_nametoindex()` failures.
--

--- Patch [12]: [PATCH net-next 12/14] selftests/bpf: Test skb_ext read from seg6local End.BPF hook ---
commit b36d673a00ed1b242e3b7537fce3622d564183f9
Author: Jakub Sitnicki <jakub@cloudflare.com>

selftests/bpf: Test skb_ext read from seg6local End.BPF hook

This patch adds a test for reading skb_ext from an lwt_seg6local context.
It sets up a three-namespace topology to verify that an skb_ext written
at TC egress can be successfully read back by a seg6local End.BPF hook
running in a different namespace.

> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c
> index 7a7537ef80e75..5f9247941aa01 100644
> --- a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c
[ ... ]
> +	/* NS1: write skb_ext at TC egress on veth1, then send.
> +	 * TC egress fires after seg6 encap, on the encapped packet.
> +	 */
> +	nstoken = open_netns("seg6_1");
> +	if (!ASSERT_OK_PTR(nstoken, "open seg6_1"))
> +		goto cleanup;
> +
> +	tc_hook.ifindex = if_nametoindex("veth1");
> +	if (!ASSERT_GE(tc_hook.ifindex, 0, "ifindex veth1"))

[Severity: Medium]
Does this assertion properly detect an interface lookup failure? 

When if_nametoindex() fails to find the network interface, it returns 0. 
Using ASSERT_GE(..., 0) allows a return value of 0 to pass the check. If the 
interface is missing, the test will proceed and likely fail later in 
bpf_tc_hook_create(), which could obfuscate the true failure point.

Would it be better to ensure the index is strictly greater than 0 here?

> +		goto cleanup;
> +
> +	ret = bpf_tc_hook_create(&tc_hook);
> +	if (!ASSERT_OK(ret, "bpf_tc_hook_create"))
> +		goto cleanup;
> +
> +	tc_opts.prog_fd = bpf_program__fd(skel->progs.tc_skb_ext_write);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814-bpf-meta-inside-skb-ext-v1-0-767edd862656@cloudflare.com?part=12

  reply	other threads:[~2026-08-15  8:14 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-14  8:14 [PATCH net-next 00/14] skb extension for BPF metadata Jakub Sitnicki
2026-08-14  8:14 ` [PATCH net-next 01/14] bpf: Introduce per-packet metadata storage for BPF programs Jakub Sitnicki
2026-08-15  8:14   ` sashiko-bot
2026-08-14  8:14 ` [PATCH net-next 02/14] bpf: Allow access to bpf_sock_ops_kern->skb Jakub Sitnicki
2026-08-14  8:14 ` [PATCH net-next 03/14] bpf: Make BPF skb extension survive packet scrubbing Jakub Sitnicki
2026-08-15  8:14   ` sashiko-bot
2026-08-14  8:14 ` [PATCH net-next 04/14] selftests/bpf: Add tests for bpf_dynptr_from_skb_ext Jakub Sitnicki
2026-08-15  8:14   ` sashiko-bot
2026-08-14  8:14 ` [PATCH net-next 05/14] selftests/bpf: Test skb_ext on cloned skbs Jakub Sitnicki
2026-08-15  8:14   ` sashiko-bot
2026-08-14  8:14 ` [PATCH net-next 06/14] selftests/bpf: Test skb_ext survival across veth and GRE Jakub Sitnicki
2026-08-15  8:14   ` sashiko-bot
2026-08-14  8:14 ` [PATCH net-next 07/14] selftests/bpf: Test skb_ext read from cgroup_skb and sk_filter hooks Jakub Sitnicki
2026-08-15  8:14   ` sashiko-bot
2026-08-14  8:14 ` [PATCH net-next 08/14] selftests/bpf: Test skb_ext read from sock_ops and LSM hooks Jakub Sitnicki
2026-08-15  8:14   ` sashiko-bot
2026-08-14  8:14 ` [PATCH net-next 09/14] selftests/bpf: Test skb_ext read from kfree_skb tracepoint Jakub Sitnicki
2026-08-15  8:14   ` sashiko-bot
2026-08-14  8:14 ` [PATCH net-next 10/14] selftests/bpf: Test skb_ext read from netfilter hook Jakub Sitnicki
2026-08-14  8:14 ` [PATCH net-next 11/14] selftests/bpf: Test skb_ext from LWT in, out, and xmit hooks Jakub Sitnicki
2026-08-15  8:14   ` sashiko-bot
2026-08-14  8:14 ` [PATCH net-next 12/14] selftests/bpf: Test skb_ext read from seg6local End.BPF hook Jakub Sitnicki
2026-08-15  8:14   ` sashiko-bot [this message]
2026-08-14  8:14 ` [PATCH net-next 13/14] selftests/bpf: Test skb_ext read from sk_skb stream verdict hook Jakub Sitnicki
2026-08-15  8:14   ` sashiko-bot
2026-08-14  8:14 ` [PATCH net-next 14/14] selftests/bpf: Use non-trivial test payload in xdp_context tests Jakub Sitnicki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260815081456.926C91F00A3D@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=jakub@cloudflare.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox