Message ID | 20190523105426.3938-3-quentin.monnet@netronome.com |
---|---|
State | Changes Requested |
Delegated to: | BPF Maintainers |
Headers | show |
Series | tools: bpftool: add an option for debug output from libbpf and verifier | expand |
On 5/23/19 3:54 AM, Quentin Monnet wrote: > libbpf was recently made aware of the log_level attribute for programs, > used to specify the level of information expected to be dumped by the > verifier. > > Create an API function to pass additional attributes when loading a > bpf_object, so we can set this log_level value in programs when loading > them, and so that so that applications relying on libbpf but not calling "so that so that" => "so that" > bpf_prog_load_xattr() can also use that feature. Do not fully understand the above statement. From the code below, I did not see how the non-zero log_level can be set for bpf_program without bpf_prog_load_xattr(). Maybe I miss something? > > v2: > - We are in a new cycle, bump libbpf extraversion number. > > Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com> > Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com> > --- > tools/lib/bpf/Makefile | 2 +- > tools/lib/bpf/libbpf.c | 20 +++++++++++++++++--- > tools/lib/bpf/libbpf.h | 6 ++++++ > tools/lib/bpf/libbpf.map | 5 +++++ > 4 files changed, 29 insertions(+), 4 deletions(-) > > diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile > index a2aceadf68db..9312066a1ae3 100644 > --- a/tools/lib/bpf/Makefile > +++ b/tools/lib/bpf/Makefile > @@ -3,7 +3,7 @@ > > BPF_VERSION = 0 > BPF_PATCHLEVEL = 0 > -BPF_EXTRAVERSION = 3 > +BPF_EXTRAVERSION = 4 > > MAKEFLAGS += --no-print-directory > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 197b574406b3..1c6fb7a3201e 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -2222,7 +2222,7 @@ static bool bpf_program__is_function_storage(struct bpf_program *prog, > } > > static int > -bpf_object__load_progs(struct bpf_object *obj) > +bpf_object__load_progs(struct bpf_object *obj, int log_level) > { > size_t i; > int err; > @@ -2230,6 +2230,7 @@ bpf_object__load_progs(struct bpf_object *obj) > for (i = 0; i < obj->nr_programs; i++) { > if (bpf_program__is_function_storage(&obj->programs[i], obj)) > continue; > + obj->programs[i].log_level = log_level; > err = bpf_program__load(&obj->programs[i], > obj->license, > obj->kern_version); > @@ -2381,10 +2382,14 @@ int bpf_object__unload(struct bpf_object *obj) > return 0; > } > > -int bpf_object__load(struct bpf_object *obj) > +int bpf_object__load_xattr(struct bpf_object_load_attr *attr) > { > + struct bpf_object *obj; > int err; > > + if (!attr) > + return -EINVAL; > + obj = attr->obj; > if (!obj) > return -EINVAL; > > @@ -2397,7 +2402,7 @@ int bpf_object__load(struct bpf_object *obj) > > CHECK_ERR(bpf_object__create_maps(obj), err, out); > CHECK_ERR(bpf_object__relocate(obj), err, out); > - CHECK_ERR(bpf_object__load_progs(obj), err, out); > + CHECK_ERR(bpf_object__load_progs(obj, attr->log_level), err, out); > > return 0; > out: > @@ -2406,6 +2411,15 @@ int bpf_object__load(struct bpf_object *obj) > return err; > } > > +int bpf_object__load(struct bpf_object *obj) > +{ > + struct bpf_object_load_attr attr = { > + .obj = obj, > + }; > + > + return bpf_object__load_xattr(&attr); > +} > + > static int check_path(const char *path) > { > char *cp, errmsg[STRERR_BUFSIZE]; > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h > index c5ff00515ce7..e1c748db44f6 100644 > --- a/tools/lib/bpf/libbpf.h > +++ b/tools/lib/bpf/libbpf.h > @@ -89,8 +89,14 @@ LIBBPF_API int bpf_object__unpin_programs(struct bpf_object *obj, > LIBBPF_API int bpf_object__pin(struct bpf_object *object, const char *path); > LIBBPF_API void bpf_object__close(struct bpf_object *object); > > +struct bpf_object_load_attr { > + struct bpf_object *obj; > + int log_level; > +}; > + > /* Load/unload object into/from kernel */ > LIBBPF_API int bpf_object__load(struct bpf_object *obj); > +LIBBPF_API int bpf_object__load_xattr(struct bpf_object_load_attr *attr); > LIBBPF_API int bpf_object__unload(struct bpf_object *obj); > LIBBPF_API const char *bpf_object__name(struct bpf_object *obj); > LIBBPF_API unsigned int bpf_object__kversion(struct bpf_object *obj); > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map > index 673001787cba..6ce61fa0baf3 100644 > --- a/tools/lib/bpf/libbpf.map > +++ b/tools/lib/bpf/libbpf.map > @@ -164,3 +164,8 @@ LIBBPF_0.0.3 { > bpf_map_freeze; > btf__finalize_data; > } LIBBPF_0.0.2; > + > +LIBBPF_0.0.4 { > + global: > + bpf_object__load_xattr; > +} LIBBPF_0.0.3; >
On 5/23/19 9:19 AM, Yonghong Song wrote: > > > On 5/23/19 3:54 AM, Quentin Monnet wrote: >> libbpf was recently made aware of the log_level attribute for programs, >> used to specify the level of information expected to be dumped by the >> verifier. >> >> Create an API function to pass additional attributes when loading a >> bpf_object, so we can set this log_level value in programs when loading >> them, and so that so that applications relying on libbpf but not calling > "so that so that" => "so that" >> bpf_prog_load_xattr() can also use that feature. > > Do not fully understand the above statement. From the code below, > I did not see how the non-zero log_level can be set for bpf_program > without bpf_prog_load_xattr(). Maybe I miss something? Looks like next patch uses it when -d is specified. Probably commit message can be made more clear. > >> >> v2: >> - We are in a new cycle, bump libbpf extraversion number. >> >> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com> >> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com> >> --- >> tools/lib/bpf/Makefile | 2 +- >> tools/lib/bpf/libbpf.c | 20 +++++++++++++++++--- >> tools/lib/bpf/libbpf.h | 6 ++++++ >> tools/lib/bpf/libbpf.map | 5 +++++ >> 4 files changed, 29 insertions(+), 4 deletions(-) >> >> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile >> index a2aceadf68db..9312066a1ae3 100644 >> --- a/tools/lib/bpf/Makefile >> +++ b/tools/lib/bpf/Makefile >> @@ -3,7 +3,7 @@ >> BPF_VERSION = 0 >> BPF_PATCHLEVEL = 0 >> -BPF_EXTRAVERSION = 3 >> +BPF_EXTRAVERSION = 4 >> MAKEFLAGS += --no-print-directory >> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c >> index 197b574406b3..1c6fb7a3201e 100644 >> --- a/tools/lib/bpf/libbpf.c >> +++ b/tools/lib/bpf/libbpf.c >> @@ -2222,7 +2222,7 @@ static bool >> bpf_program__is_function_storage(struct bpf_program *prog, >> } >> static int >> -bpf_object__load_progs(struct bpf_object *obj) >> +bpf_object__load_progs(struct bpf_object *obj, int log_level) >> { >> size_t i; >> int err; >> @@ -2230,6 +2230,7 @@ bpf_object__load_progs(struct bpf_object *obj) >> for (i = 0; i < obj->nr_programs; i++) { >> if (bpf_program__is_function_storage(&obj->programs[i], obj)) >> continue; >> + obj->programs[i].log_level = log_level; >> err = bpf_program__load(&obj->programs[i], >> obj->license, >> obj->kern_version); >> @@ -2381,10 +2382,14 @@ int bpf_object__unload(struct bpf_object *obj) >> return 0; >> } >> -int bpf_object__load(struct bpf_object *obj) >> +int bpf_object__load_xattr(struct bpf_object_load_attr *attr) >> { >> + struct bpf_object *obj; >> int err; >> + if (!attr) >> + return -EINVAL; >> + obj = attr->obj; >> if (!obj) >> return -EINVAL; >> @@ -2397,7 +2402,7 @@ int bpf_object__load(struct bpf_object *obj) >> CHECK_ERR(bpf_object__create_maps(obj), err, out); >> CHECK_ERR(bpf_object__relocate(obj), err, out); >> - CHECK_ERR(bpf_object__load_progs(obj), err, out); >> + CHECK_ERR(bpf_object__load_progs(obj, attr->log_level), err, out); >> return 0; >> out: >> @@ -2406,6 +2411,15 @@ int bpf_object__load(struct bpf_object *obj) >> return err; >> } >> +int bpf_object__load(struct bpf_object *obj) >> +{ >> + struct bpf_object_load_attr attr = { >> + .obj = obj, >> + }; >> + >> + return bpf_object__load_xattr(&attr); >> +} >> + >> static int check_path(const char *path) >> { >> char *cp, errmsg[STRERR_BUFSIZE]; >> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h >> index c5ff00515ce7..e1c748db44f6 100644 >> --- a/tools/lib/bpf/libbpf.h >> +++ b/tools/lib/bpf/libbpf.h >> @@ -89,8 +89,14 @@ LIBBPF_API int bpf_object__unpin_programs(struct >> bpf_object *obj, >> LIBBPF_API int bpf_object__pin(struct bpf_object *object, const char >> *path); >> LIBBPF_API void bpf_object__close(struct bpf_object *object); >> +struct bpf_object_load_attr { >> + struct bpf_object *obj; >> + int log_level; >> +}; >> + >> /* Load/unload object into/from kernel */ >> LIBBPF_API int bpf_object__load(struct bpf_object *obj); >> +LIBBPF_API int bpf_object__load_xattr(struct bpf_object_load_attr >> *attr); >> LIBBPF_API int bpf_object__unload(struct bpf_object *obj); >> LIBBPF_API const char *bpf_object__name(struct bpf_object *obj); >> LIBBPF_API unsigned int bpf_object__kversion(struct bpf_object *obj); >> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map >> index 673001787cba..6ce61fa0baf3 100644 >> --- a/tools/lib/bpf/libbpf.map >> +++ b/tools/lib/bpf/libbpf.map >> @@ -164,3 +164,8 @@ LIBBPF_0.0.3 { >> bpf_map_freeze; >> btf__finalize_data; >> } LIBBPF_0.0.2; >> + >> +LIBBPF_0.0.4 { >> + global: >> + bpf_object__load_xattr; >> +} LIBBPF_0.0.3; >>
2019-05-23 16:29 UTC+0000 ~ Yonghong Song <yhs@fb.com> > > > On 5/23/19 9:19 AM, Yonghong Song wrote: >> >> >> On 5/23/19 3:54 AM, Quentin Monnet wrote: >>> libbpf was recently made aware of the log_level attribute for programs, >>> used to specify the level of information expected to be dumped by the >>> verifier. >>> >>> Create an API function to pass additional attributes when loading a >>> bpf_object, so we can set this log_level value in programs when loading >>> them, and so that so that applications relying on libbpf but not calling >> "so that so that" => "so that" Oh, thanks! >>> bpf_prog_load_xattr() can also use that feature. >> >> Do not fully understand the above statement. From the code below, >> I did not see how the non-zero log_level can be set for bpf_program >> without bpf_prog_load_xattr(). Maybe I miss something? bpf_prog_load_xattr() already had support for passing a log_level, it was added by Alexei when he made libbpf aware of the different log levels (commit da11b417583e). But bpftool does not rely on bpf_prog_load_xattr(), it loads programs with bpf_object__load(), that offered no way pass a log_level parameter. Does that help? > > Looks like next patch uses it when -d is specified. It uses the new bpf_object__load_xattr() function introduced in this patch indeed. > Probably commit message can be made more clear. Yeah, probably. I'll improve it and submit a v3. Thanks! Quentin
diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile index a2aceadf68db..9312066a1ae3 100644 --- a/tools/lib/bpf/Makefile +++ b/tools/lib/bpf/Makefile @@ -3,7 +3,7 @@ BPF_VERSION = 0 BPF_PATCHLEVEL = 0 -BPF_EXTRAVERSION = 3 +BPF_EXTRAVERSION = 4 MAKEFLAGS += --no-print-directory diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 197b574406b3..1c6fb7a3201e 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -2222,7 +2222,7 @@ static bool bpf_program__is_function_storage(struct bpf_program *prog, } static int -bpf_object__load_progs(struct bpf_object *obj) +bpf_object__load_progs(struct bpf_object *obj, int log_level) { size_t i; int err; @@ -2230,6 +2230,7 @@ bpf_object__load_progs(struct bpf_object *obj) for (i = 0; i < obj->nr_programs; i++) { if (bpf_program__is_function_storage(&obj->programs[i], obj)) continue; + obj->programs[i].log_level = log_level; err = bpf_program__load(&obj->programs[i], obj->license, obj->kern_version); @@ -2381,10 +2382,14 @@ int bpf_object__unload(struct bpf_object *obj) return 0; } -int bpf_object__load(struct bpf_object *obj) +int bpf_object__load_xattr(struct bpf_object_load_attr *attr) { + struct bpf_object *obj; int err; + if (!attr) + return -EINVAL; + obj = attr->obj; if (!obj) return -EINVAL; @@ -2397,7 +2402,7 @@ int bpf_object__load(struct bpf_object *obj) CHECK_ERR(bpf_object__create_maps(obj), err, out); CHECK_ERR(bpf_object__relocate(obj), err, out); - CHECK_ERR(bpf_object__load_progs(obj), err, out); + CHECK_ERR(bpf_object__load_progs(obj, attr->log_level), err, out); return 0; out: @@ -2406,6 +2411,15 @@ int bpf_object__load(struct bpf_object *obj) return err; } +int bpf_object__load(struct bpf_object *obj) +{ + struct bpf_object_load_attr attr = { + .obj = obj, + }; + + return bpf_object__load_xattr(&attr); +} + static int check_path(const char *path) { char *cp, errmsg[STRERR_BUFSIZE]; diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h index c5ff00515ce7..e1c748db44f6 100644 --- a/tools/lib/bpf/libbpf.h +++ b/tools/lib/bpf/libbpf.h @@ -89,8 +89,14 @@ LIBBPF_API int bpf_object__unpin_programs(struct bpf_object *obj, LIBBPF_API int bpf_object__pin(struct bpf_object *object, const char *path); LIBBPF_API void bpf_object__close(struct bpf_object *object); +struct bpf_object_load_attr { + struct bpf_object *obj; + int log_level; +}; + /* Load/unload object into/from kernel */ LIBBPF_API int bpf_object__load(struct bpf_object *obj); +LIBBPF_API int bpf_object__load_xattr(struct bpf_object_load_attr *attr); LIBBPF_API int bpf_object__unload(struct bpf_object *obj); LIBBPF_API const char *bpf_object__name(struct bpf_object *obj); LIBBPF_API unsigned int bpf_object__kversion(struct bpf_object *obj); diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map index 673001787cba..6ce61fa0baf3 100644 --- a/tools/lib/bpf/libbpf.map +++ b/tools/lib/bpf/libbpf.map @@ -164,3 +164,8 @@ LIBBPF_0.0.3 { bpf_map_freeze; btf__finalize_data; } LIBBPF_0.0.2; + +LIBBPF_0.0.4 { + global: + bpf_object__load_xattr; +} LIBBPF_0.0.3;