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