All of lore.kernel.org
 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 10/15] selftests/bpf: Test passing metadata to TCP egress
Date: Tue, 29 Sep 2026 11:40:52 +0000	[thread overview]
Message-ID: <20260929114052.A01D71F00899@smtp.kernel.org> (raw)
In-Reply-To: <20260928-bpf-meta-gated-tracepoints-v1-10-844dbf3e1edf@cloudflare.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: test_skb_trace__tcp_retrans flakiness due to premature map assertion
--

commit 2cb521395baafb017de2859f1a86b8fa86c112a8
Author: Jakub Sitnicki <jakub@cloudflare.com>

selftests/bpf: Test passing metadata to TCP egress
    
Tag the skb at cgroup/skb egress and read the stashed metadata back at TC
egress: for a regular TCP transmission, and for a retransmission, where
the first transmission is dropped at TC and the write-queue original,
tagged via bpf_skb_fclone_orig, passes the metadata to the retransmitted
fast clone via the copy tracepoint.

> 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 890a881a6a1f8..ac5c0f067b3f1 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
[ ... ]
> +static void test_skb_trace__tcp_retrans(struct test_xdp_meta *skel)
> +{
[ ... ]
> +	/* First transmission is dropped at TC; recv unblocks on retransmit */
> +	ret = recv(conn_fd, buf, sizeof(buf), 0);
> +	ASSERT_EQ(ret, TEST_PAYLOAD_LEN, "recv");
> +
> +	/* Poll every 10 msec up to 1 sec */
> +	for (i = 0; i < 100 && !skel->bss->test_pass; i++)
> +		usleep(10000);
> +	ASSERT_TRUE(skel->bss->test_pass, "test_pass");
> +
> +	/* Skb is freed after RX delivery; wait for free tp */
> +	for (i = 0; i < 100 && !skel->bss->free_count; i++)
> +		usleep(10000);
> +	ASSERT_GE(skel->bss->free_count, 1, "free_count");
> +
> +	assert_meta_stash_empty(skel->maps.meta_map);

[Severity: Medium]
Will this polling loop terminate prematurely?

Since the first transmission is dropped at TC egress, it gets freed and
increments free_count to 1 early. This causes the loop to exit immediately
because !skel->bss->free_count is already false.

Could this cause assert_meta_stash_empty() to fail intermittently because the
retransmitted clone and original write-queue SKB haven't been asynchronously
freed yet?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928-bpf-meta-gated-tracepoints-v1-0-844dbf3e1edf@cloudflare.com?part=10

  reply	other threads:[~2026-09-29 11:40 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-28 11:40 [PATCH net-next 00/15] Gated tracepoints for skb lifecycle (free+clone) Jakub Sitnicki
2026-09-28 11:40 ` [PATCH net-next 01/15] bpf: Add gated tracepoints for skb life-cycle Jakub Sitnicki
2026-09-29 11:40   ` sashiko-bot
2026-09-28 11:40 ` [PATCH net-next 02/15] bpf: Add bpf_skb_fclone_orig() kfunc Jakub Sitnicki
2026-09-29 11:40   ` sashiko-bot
2026-09-28 11:40 ` [PATCH net-next 03/15] bpf: Accept trusted pointers to kern ctx type for kfunc ctx args Jakub Sitnicki
2026-09-29 11:40   ` sashiko-bot
2026-09-28 11:40 ` [PATCH net-next 04/15] bpf: Allow access to bpf_sock_ops_kern->skb Jakub Sitnicki
2026-09-28 11:40 ` [PATCH net-next 05/15] selftests/bpf: Basic tests for metadata passing with gated skb tracepoints Jakub Sitnicki
2026-09-29 11:40   ` sashiko-bot
2026-09-28 11:40 ` [PATCH net-next 06/15] selftests/bpf: Test passing metadata across veth and GRE Jakub Sitnicki
2026-09-29 11:40   ` sashiko-bot
2026-09-28 11:40 ` [PATCH net-next 07/15] selftests/bpf: Test metadata passing to cgroup_skb and sk_filter hooks Jakub Sitnicki
2026-09-29 11:40   ` sashiko-bot
2026-09-28 11:40 ` [PATCH net-next 08/15] selftests/bpf: Test passing metadata to sock_ops and LSM hooks Jakub Sitnicki
2026-09-29 11:40   ` sashiko-bot
2026-09-28 11:40 ` [PATCH net-next 09/15] selftests/bpf: Test passing metadata to kfree_skb tracepoint Jakub Sitnicki
2026-09-28 11:40 ` [PATCH net-next 10/15] selftests/bpf: Test passing metadata to TCP egress Jakub Sitnicki
2026-09-29 11:40   ` sashiko-bot [this message]
2026-09-28 11:40 ` [PATCH net-next 11/15] selftests/bpf: Test passing metadata to netfilter hook Jakub Sitnicki
2026-09-28 11:40 ` [PATCH net-next 12/15] selftests/bpf: Test passing metadata to/from LWT hooks Jakub Sitnicki
2026-09-29 11:40   ` sashiko-bot
2026-09-28 11:40 ` [PATCH net-next 13/15] selftests/bpf: Test passing metadata to seg6local End.BPF hook Jakub Sitnicki
2026-09-28 11:40 ` [PATCH net-next 14/15] selftests/bpf: Test passing metadata to sk_skb stream verdict hook Jakub Sitnicki
2026-09-28 11:40 ` [PATCH net-next 15/15] selftests/bpf: Use non-trivial test payload in xdp_context tests Jakub Sitnicki
2026-10-02  1:45 ` [PATCH net-next 00/15] Gated tracepoints for skb lifecycle (free+clone) Jakub Kicinski
2026-10-02 12:47   ` 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=20260929114052.A01D71F00899@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.