On Wed, Mar 30, 2022 at 12:44 AM Roberto Sassu <roberto.sassu@xxxxxxxxxx> wrote: > > > From: Andrii Nakryiko [mailto:andrii.nakryiko@xxxxxxxxx] > > Sent: Wednesday, March 30, 2022 1:52 AM > > On Mon, Mar 28, 2022 at 10:52 AM Roberto Sassu > > <roberto.sassu@xxxxxxxxxx> 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 <roberto.sassu@xxxxxxxxxx> > > > --- > > > 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. > > > > [...]
