[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEf4BzYsGnhmnhkHdUPN8yBfbv57R9h4N2R8RcqdjhmHWvJVkg@mail.gmail.com>
Date: Thu, 20 Feb 2025 17:07:24 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Eduard Zingerman <eddyz87@...il.com>
Cc: Tao Chen <chen.dylane@...il.com>, ast@...nel.org, daniel@...earbox.net,
andrii@...nel.org, haoluo@...gle.com, jolsa@...nel.org, qmo@...nel.org,
bpf@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND bpf-next v7 0/4] Add prog_kfunc feature probe
On Tue, Feb 18, 2025 at 2:51 PM Eduard Zingerman <eddyz87@...il.com> wrote:
>
> 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@...nel.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@...il.com>
Shouldn't we fix the issue with those bpf_xdp_metadata_* kfuncs? Do
you have details on what different string verifier reports?
>
> --- 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 ---
>
Powered by blists - more mailing lists