[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200630135538.GB3071036@krava>
Date: Tue, 30 Jun 2020 15:55:38 +0200
From: Jiri Olsa <jolsa@...hat.com>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: Jiri Olsa <jolsa@...nel.org>, Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
Martin KaFai Lau <kafai@...com>,
David Miller <davem@...hat.com>,
John Fastabend <john.fastabend@...il.com>,
Wenbo Zhang <ethercflow@...il.com>,
KP Singh <kpsingh@...omium.org>,
Andrii Nakryiko <andriin@...com>,
Brendan Gregg <bgregg@...flix.com>,
Florent Revest <revest@...omium.org>,
Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [PATCH v4 bpf-next 00/14] bpf: Add d_path helper
On Mon, Jun 29, 2020 at 06:54:30PM -0700, Andrii Nakryiko wrote:
> On Thu, Jun 25, 2020 at 4:47 PM Jiri Olsa <jolsa@...nel.org> wrote:
> >
> > hi,
> > adding d_path helper to return full path for 'path' object.
> >
> > In a preparation for that, this patchset also adds support for BTF ID
> > whitelists, because d_path can't be called from any probe due to its
> > locks usage. The whitelists allow verifier to check if the caller is
> > one of the functions from the whitelist.
> >
> > The whitelist is implemented in a generic way. This patchset introduces
> > macros that allow to define lists of BTF IDs, which are compiled in
> > the kernel image in a new .BTF.ids ELF section.
> >
> > The generic way of BTF ID lists allows us to use them in other places
> > in kernel (than just for whitelists), that could use static BTF ID
> > values compiled in and it's also implemented in this patchset.
> >
> > I originally added and used 'file_path' helper, which did the same,
> > but used 'struct file' object. Then realized that file_path is just
> > a wrapper for d_path, so we'd cover more calling sites if we add
> > d_path helper and allowed resolving BTF object within another object,
> > so we could call d_path also with file pointer, like:
> >
> > bpf_d_path(&file->f_path, buf, size);
> >
> > This feature is mainly to be able to add dpath (filepath originally)
> > function to bpftrace:
> >
> > # bpftrace -e 'kfunc:vfs_open { printf("%s\n", dpath(args->path)); }'
> >
> > v4 changes:
> > - added ID sanity checks in btf_resolve_helper_id [Andrii]
> > - resolve bpf_ctx_convert via BTF_ID [Andrii]
> > - keep bpf_access_type in btf_struct_access [Andrii]
> > - rename whitelist to se and use struct btf_id_set [Andrii]
> > - several fixes for d_path prog/verifier tests [Andrii]
> > - added union and typedefs types support [Andrii]
> > - rename btfid to resolve_btfids [Andrii]
> > - fix segfault in resolve_btfids [John]
> > - rename section from .BTF_ids .BTF.ids (following .BTF.ext example)
> > - add .BTF.ids section info into btf.rst [John]
> > - updated over letter with more details [John]
> >
> > Also available at:
> > https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> > bpf/d_path
> >
> > thanks,
> > jirka
> >
> >
> > ---
>
> Have you considered splitting this series into two? One with BTF ID
> resolution and corresponding patches. I'm pretty confident in that one
> and it seems ready (with some minor selftest changes). Then,
> separately, d_path and that sub-struct address logic. That one depends
> on the first one, but shouldn't really block BTF ID resolution from
> going in sooner.
ok, will split it and send the first part for now
jirka
Powered by blists - more mailing lists