BPF List
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Jiri Olsa" <jolsa@kernel.org>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCHv2 bpf-next 2/3] selftests/bpf: Add tracing_multi link info tests
Date: Tue, 23 Jun 2026 14:39:43 +0000	[thread overview]
Message-ID: <20260623143944.54C6F1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260623142417.275892-3-jolsa@kernel.org>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The patch adds selftests for `tracing_multi` link info (`BPF_LINK_TYPE_TRACING_MULTI`), but the required kernel-side implementation is entirely missing.
- [Low] The patch references `info.tracing_multi` inside `struct bpf_link_info`, but this field does not exist in the UAPI headers.
--

commit 23bb30d6d6d285433f08e70bf1a166ecfd023a7d
Author: Jiri Olsa <jolsa@kernel.org>

selftests/bpf: Add tracing_multi link info tests
    
Adding tracing_multi link info tests that follow the kprobe_multi
and uprobe_multi tests logic.

> diff --git a/tools/testing/selftests/bpf/prog_tests/fill_link_info.c b/tools/testing/selftests/bpf/prog_tests/fill_link_info.c
> index f589eefbf9fbd..4d021e9b0129c 100644
> --- a/tools/testing/selftests/bpf/prog_tests/fill_link_info.c
> +++ b/tools/testing/selftests/bpf/prog_tests/fill_link_info.c
[ ... ]
> +static int verify_tracing_multi_link_info(int fd, const struct bpf_program *prog,
> +					  const struct tmulti_target *targets,
> +					  __u32 obj_id, bool has_cookies)
> +{
[ ... ]
> +	err = bpf_link_get_info_by_fd(fd, &info, &len);
> +	if (!ASSERT_OK(err, "bpf_link_get_info_by_fd"))
> +		return -1;
> +
> +	if (!ASSERT_EQ(info.type, BPF_LINK_TYPE_TRACING_MULTI, "info.type"))
> +		return -1;
> +
> +	ASSERT_EQ(info.tracing_multi.attach_type, attach_type, "info.tracing_multi.attach_type");

[Severity: Low]
Does the tracing_multi field exist in struct bpf_link_info within the UAPI
headers?

It doesn't seem to be present in include/uapi/linux/bpf.h yet, which would
cause a build failure here when accessing info.tracing_multi.attach_type.

> +	ASSERT_EQ(info.tracing_multi.count, TRACING_MULTI_CNT, "info.tracing_multi.count");

[Severity: Medium]
Is the kernel-side implementation for filling this link info missing?

Without a .fill_link_info callback defined in bpf_tracing_multi_link_lops in
kernel/trace/bpf_trace.c, bpf_link_get_info_by_fd() will leave the
tracing_multi struct zero-initialized, causing this assertion to fail.

[ ... ]
> +static void verify_tracing_multi_invalid_user_buffer(int fd, const struct tmulti_target *targets)
> +{
> +	__u32 ids[TRACING_MULTI_CNT] = {};
> +	struct bpf_link_info info;
> +	__u32 len = sizeof(info);
> +	int err, i;
> +
> +	/* Wrong info setup (ids != NULL and cnt == 0) -> EINVAL */
> +	memset(&info, 0, sizeof(info));
> +	info.tracing_multi.ids = ptr_to_u64(ids);
> +	err = bpf_link_get_info_by_fd(fd, &info, &len);
> +	ASSERT_EQ(err, -EINVAL, "tracing_multi.invalid_count");

[Severity: Medium]
Will this return the expected -EINVAL error?

Since the kernel doesn't appear to implement .fill_link_info for this link
type yet, the validation that would return -EINVAL won't execute, causing
this assertion to fail.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623142417.275892-3-jolsa@kernel.org?part=1

  reply	other threads:[~2026-06-23 14:39 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-23 14:24 [PATCHv2 bpf-next 0/3] bpf: tracing_multi link info support Jiri Olsa
2026-06-23 14:24 ` [PATCHv2 bpf-next 1/3] bpf: Add " Jiri Olsa
2026-06-23 15:32   ` Leon Hwang
2026-06-23 21:01   ` Andrii Nakryiko
2026-06-24  9:36     ` Jiri Olsa
2026-06-23 14:24 ` [PATCHv2 bpf-next 2/3] selftests/bpf: Add tracing_multi link info tests Jiri Olsa
2026-06-23 14:39   ` sashiko-bot [this message]
2026-06-23 14:24 ` [PATCHv2 bpf-next 3/3] bpftool: Add tracing_multi link info output Jiri Olsa
2026-06-23 14:38   ` sashiko-bot
2026-06-23 21:11   ` Andrii Nakryiko
2026-06-24 11:32     ` Jiri Olsa
2026-06-23 20:51 ` [PATCHv2 bpf-next 0/3] bpf: tracing_multi link info support Andrii Nakryiko
2026-06-24  9:29   ` Jiri Olsa

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=20260623143944.54C6F1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=jolsa@kernel.org \
    --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