On Wed, Mar 30, 2022 at 12:44 AM Roberto Sassu <[email protected]> wrote:
>
> > From: Andrii Nakryiko [mailto:[email protected]]
> > Sent: Wednesday, March 30, 2022 1:52 AM
> > On Mon, Mar 28, 2022 at 10:52 AM Roberto Sassu
> > <[email protected]> wrote:
> > >
> > > The first part of the preload code generation consists in generating the
> > > static variables to be used by the code itself: the links and maps to be
> > > pinned, and the skeleton. Generation of the preload variables and
> > methods
> > > is enabled with the option -P added to 'bpftool gen skeleton'.
> > >
> > > The existing variables maps_link and progs_links in bpf_preload_kern.c
> > have
> > > been renamed respectively to dump_bpf_map_link and
> > dump_bpf_prog_link, to
> > > match the name of the variables in the main structure of the light
> > > skeleton.
> > >
> > > Signed-off-by: Roberto Sassu <[email protected]>
> > > ---
> > > kernel/bpf/preload/bpf_preload_kern.c | 35 +-
> > > kernel/bpf/preload/iterators/Makefile | 2 +-
> > > .../bpf/preload/iterators/iterators.lskel.h | 378 +++++++++---------
> > > .../bpf/bpftool/Documentation/bpftool-gen.rst | 5 +
> > > tools/bpf/bpftool/bash-completion/bpftool | 2 +-
> > > tools/bpf/bpftool/gen.c | 27 ++
> > > tools/bpf/bpftool/main.c | 7 +-
> > > tools/bpf/bpftool/main.h | 1 +
> > > 8 files changed, 254 insertions(+), 203 deletions(-)
> > >
> >
> > [...]
> >
> > > +__attribute__((unused)) static void
> > > +iterators_bpf__assert(struct iterators_bpf *s)
> > > +{
> > > +#ifdef __cplusplus
> > > +#define _Static_assert static_assert
> > > +#endif
> > > +#ifdef __cplusplus
> > > +#undef _Static_assert
> > > +#endif
> > > +}
> > > +
> > > +static struct bpf_link *dump_bpf_map_link;
> > > +static struct bpf_link *dump_bpf_prog_link;
> > > +static struct iterators_bpf *skel;
> >
> > I don't understand what is this and what for? You are making an
> > assumption that light skeleton can be instantiated just once, why? And
> > adding extra bpftool option to light skeleton codegen just to save a
> > bit of typing at the place where light skeleton is actually
> > instantiated and used doesn't seems like a right approach.
>
> True, iterator_bpf is simple. Writing the preloading code
> for it is simple. But, what if you wanted to preload an LSM
> with 10 hooks or more?
I suppose you'd write a straightforward code to do pinning ten times
for ten different programs to ten different paths. But with this you
don't have to establish a random set of conventions that might not
apply in all the situations to anyone that would try to use this
feature.
Worst case, light skeleton can be extended to provide a way to iterate
all programs programmatically.
>
> Ok, regarding where the preloading code should be, I will
> try to move the generated code to the kernel module instead
> of the light skeleton.
>
> Thanks
>
> Roberto
>
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Li Peng, Zhong Ronghua
>
> > Further, even if this is the way to go, please split out bpftool
> > changes from kernel changes. There is nothing requiring them to be
> > coupled together.
> >
> > [...]