public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Eduard Zingerman <eddyz87@gmail.com>
To: Tao Chen <chen.dylane@gmail.com>,
	ast@kernel.org, daniel@iogearbox.net,  andrii@kernel.org,
	haoluo@google.com, jolsa@kernel.org, qmo@kernel.org
Cc: bpf@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH RESEND bpf-next v7 0/4] Add prog_kfunc feature probe
Date: Tue, 18 Feb 2025 14:51:36 -0800	[thread overview]
Message-ID: <598a7d089936b18472937679d4131286f102cb18.camel@gmail.com> (raw)
In-Reply-To: <2b025df3-144b-4909-a2b4-66356540f71c@gmail.com>

On Mon, 2025-02-17 at 13:21 +0800, Tao Chen wrote:
> 在 2025/2/12 23:39, Tao Chen 写道:
> > More and more kfunc functions are being added to the kernel.
> > Different prog types have different restrictions when using kfunc.
> > Therefore, prog_kfunc probe is added to check whether it is supported,
> > and the use of this api will be added to bpftool later.
> > 
> > Change list:
> > - v6 -> v7:
> >    - wrap err with libbpf_err
> >    - comments fix
> >    - handle btf_fd < 0 as vmlinux
> >    - patchset Reviewed-by: Jiri Olsa <jolsa@kernel.org>
> > - v6
> >    https://lore.kernel.org/bpf/20250211111859.6029-1-chen.dylane@gmail.com
> > 
> > - v5 -> v6:
> >    - remove fd_array_cnt
> >    - test case clean code
> > - v5
> >    https://lore.kernel.org/bpf/20250210055945.27192-1-chen.dylane@gmail.com
> > 
> > - v4 -> v5:
> >    - use fd_array on stack
> >    - declare the scope of use of btf_fd
> > - v4
> >    https://lore.kernel.org/bpf/20250206051557.27913-1-chen.dylane@gmail.com/
> > 
> > - v3 -> v4:
> >    - add fd_array init for kfunc in mod btf
> >    - add test case for kfunc in mod btf
> >    - refactor common part as prog load type check for
> >      libbpf_probe_bpf_{helper,kfunc}
> > - v3
> >    https://lore.kernel.org/bpf/20250124144411.13468-1-chen.dylane@gmail.com
> > 
> > - v2 -> v3:
> >    - rename parameter off with btf_fd
> >    - extract the common part for libbpf_probe_bpf_{helper,kfunc}
> > - v2
> >    https://lore.kernel.org/bpf/20250123170555.291896-1-chen.dylane@gmail.com
> > 
> > - v1 -> v2:
> >    - check unsupported prog type like probe_bpf_helper
> >    - add off parameter for module btf
> >    - check verifier info when kfunc id invalid
> > - v1
> >    https://lore.kernel.org/bpf/20250122171359.232791-1-chen.dylane@gmail.com
> > 
> > Tao Chen (4):
> >    libbpf: Extract prog load type check from libbpf_probe_bpf_helper
> >    libbpf: Init fd_array when prog probe load
> >    libbpf: Add libbpf_probe_bpf_kfunc API
> >    selftests/bpf: Add libbpf_probe_bpf_kfunc API selftests
> > 
> >   tools/lib/bpf/libbpf.h                        |  19 ++-
> >   tools/lib/bpf/libbpf.map                      |   1 +
> >   tools/lib/bpf/libbpf_probes.c                 |  86 +++++++++++---
> >   .../selftests/bpf/prog_tests/libbpf_probes.c  | 111 ++++++++++++++++++
> >   4 files changed, 201 insertions(+), 16 deletions(-)
> > 
> 
> Ping...
> 
> Hi Andrii, Eduard,
> 
> I've revised the previous suggestions. Please review it again. Thanks.
> 

I tried the test enumerating all kfuncs in BTF and doing
libbpf_probe_bpf_kfunc for BPF_PROG_TYPE_{KPROBE,XDP}.
(Source code at the end of the email).

The set of kfuncs returned for XDP looks correct.
The set of kfuncs returned for KPROBE contains a few incorrect entries:
- bpf_xdp_metadata_rx_hash
- bpf_xdp_metadata_rx_timestamp
- bpf_xdp_metadata_rx_vlan_tag

This is because of a different string reported by verifier for these
three functions.

Ideally, I'd write some script looking for
register_btf_kfunc_id_set(BPF_PROG_TYPE_***, kfunc_set)
calls in the kernel source code and extracting the prog type /
functions in the set, and comparing results of this script with
output of the test below for all program types.
But up to you if you'd like to do such rigorous verification or not.

Otherwise patch-set looks good to me, for all patch-set:

Reviewed-by: Eduard Zingerman <eddyz87@gmail.com>

--- 8< -----------------------------------------------------

static const struct {
	const char *name;
	int code;
} program_types[] = {
#define _T(n) { #n, BPF_PROG_TYPE_ ## n }
	_T(KPROBE),
	_T(XDP),
#undef _T
};

void test_libbpf_probe_kfuncs_many(void)
{
	int i, kfunc_id, ret, id;
	const struct btf_type *t;
	struct btf *btf = NULL;
	const char *kfunc;
	const char *tag;

	btf = btf__parse("/sys/kernel/btf/vmlinux", NULL);
	if (!ASSERT_OK_PTR(btf, "btf_parse"))
		return;

	for (id = 0; id < btf__type_cnt(btf); ++id) {
		t = btf__type_by_id(btf, id);
		if (!btf_is_decl_tag(t))
			continue;
		tag = btf__name_by_offset(btf, t->name_off);
		if (strcmp(tag, "bpf_kfunc") != 0)
			continue;
		kfunc_id = t->type;
		t = btf__type_by_id(btf, kfunc_id);
		if (!btf_is_func(t))
			continue;
		kfunc = btf__name_by_offset(btf, t->name_off);
		printf("[%-6d] %-42s ", kfunc_id, kfunc);
		for (i = 0; i < ARRAY_SIZE(program_types); ++i) {
			ret = libbpf_probe_bpf_kfunc(program_types[i].code, kfunc_id, -1, NULL);
			if (ret < 0)
				printf("%-8d  ", ret);
			else if (ret == 0)
				printf("%8s  ", "");
			else
				printf("%8s  ", program_types[i].name);
		}
		printf("\n");
	}
	btf__free(btf);
}

----------------------------------------------------- >8 ---


  reply	other threads:[~2025-02-18 22:51 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-12 15:39 [PATCH RESEND bpf-next v7 0/4] Add prog_kfunc feature probe Tao Chen
2025-02-12 15:39 ` [PATCH bpf-next v7 1/4] libbpf: Extract prog load type check from libbpf_probe_bpf_helper Tao Chen
2025-02-12 15:39 ` [PATCH bpf-next v7 2/4] libbpf: Init fd_array when prog probe load Tao Chen
2025-02-12 15:39 ` [PATCH bpf-next v7 3/4] libbpf: Add libbpf_probe_bpf_kfunc API Tao Chen
2025-02-12 15:39 ` [PATCH bpf-next v7 4/4] selftests/bpf: Add libbpf_probe_bpf_kfunc API selftests Tao Chen
2025-02-17  5:21 ` [PATCH RESEND bpf-next v7 0/4] Add prog_kfunc feature probe Tao Chen
2025-02-18 22:51   ` Eduard Zingerman [this message]
2025-02-20 18:09     ` Tao Chen
2025-02-20 18:43       ` Eduard Zingerman
2025-02-21 10:03         ` Tao Chen
2025-02-21  1:07     ` Andrii Nakryiko
2025-02-21  1:11       ` Eduard Zingerman
2025-02-21 10:03         ` Tao Chen

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=598a7d089936b18472937679d4131286f102cb18.camel@gmail.com \
    --to=eddyz87@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=chen.dylane@gmail.com \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=qmo@kernel.org \
    /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