* [PATCH bpf-next v2 1/4] libbpf: Implement btf__save_raw()
2021-11-16 16:42 [PATCH bpf-next v2 0/4] libbpf: Provide APIs for BTFGen Mauricio Vásquez
@ 2021-11-16 16:42 ` Mauricio Vásquez
2021-11-17 5:25 ` Andrii Nakryiko
2021-11-16 16:42 ` [PATCH bpf-next v2 2/4] libbpf: Introduce 'btf_custom' to 'bpf_obj_open_opts' Mauricio Vásquez
` (2 subsequent siblings)
3 siblings, 1 reply; 15+ messages in thread
From: Mauricio Vásquez @ 2021-11-16 16:42 UTC (permalink / raw)
To: netdev, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Rafael David Tinoco, Lorenzo Fontana, Leonardo Di Donato
Implement helper function to save the contents of a BTF object to a
file.
Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
---
tools/lib/bpf/btf.c | 30 ++++++++++++++++++++++++++++++
tools/lib/bpf/btf.h | 2 ++
tools/lib/bpf/libbpf.map | 1 +
3 files changed, 33 insertions(+)
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index fadf089ae8fe..96a242f91832 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -1121,6 +1121,36 @@ struct btf *btf__parse_split(const char *path, struct btf *base_btf)
return libbpf_ptr(btf_parse(path, base_btf, NULL));
}
+int btf__save_raw(const struct btf *btf, const char *path)
+{
+ const void *data;
+ FILE *f = NULL;
+ __u32 data_sz;
+ int err = 0;
+
+ data = btf__raw_data(btf, &data_sz);
+ if (!data) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ f = fopen(path, "wb");
+ if (!f) {
+ err = -errno;
+ goto out;
+ }
+
+ if (fwrite(data, 1, data_sz, f) != data_sz) {
+ err = -errno;
+ goto out;
+ }
+
+out:
+ if (f)
+ fclose(f);
+ return libbpf_err(err);
+}
+
static void *btf_get_raw_data(const struct btf *btf, __u32 *size, bool swap_endian);
int btf__load_into_kernel(struct btf *btf)
diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index 5c73a5b0a044..4f8d3f303aa6 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -114,6 +114,8 @@ LIBBPF_API struct btf *btf__parse_elf_split(const char *path, struct btf *base_b
LIBBPF_API struct btf *btf__parse_raw(const char *path);
LIBBPF_API struct btf *btf__parse_raw_split(const char *path, struct btf *base_btf);
+LIBBPF_API int btf__save_raw(const struct btf *btf, const char *path);
+
LIBBPF_API struct btf *btf__load_vmlinux_btf(void);
LIBBPF_API struct btf *btf__load_module_btf(const char *module_name, struct btf *vmlinux_btf);
LIBBPF_API struct btf *libbpf_find_kernel_btf(void);
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 6a59514a48cf..c9555f8655af 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -414,4 +414,5 @@ LIBBPF_0.6.0 {
perf_buffer__new_deprecated;
perf_buffer__new_raw;
perf_buffer__new_raw_deprecated;
+ btf__save_raw;
} LIBBPF_0.5.0;
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next v2 1/4] libbpf: Implement btf__save_raw()
2021-11-16 16:42 ` [PATCH bpf-next v2 1/4] libbpf: Implement btf__save_raw() Mauricio Vásquez
@ 2021-11-17 5:25 ` Andrii Nakryiko
2021-11-19 17:25 ` Andrii Nakryiko
0 siblings, 1 reply; 15+ messages in thread
From: Andrii Nakryiko @ 2021-11-17 5:25 UTC (permalink / raw)
To: Mauricio Vásquez
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Rafael David Tinoco, Lorenzo Fontana,
Leonardo Di Donato
On Tue, Nov 16, 2021 at 8:42 AM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
>
> Implement helper function to save the contents of a BTF object to a
> file.
>
> Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> ---
> tools/lib/bpf/btf.c | 30 ++++++++++++++++++++++++++++++
> tools/lib/bpf/btf.h | 2 ++
> tools/lib/bpf/libbpf.map | 1 +
> 3 files changed, 33 insertions(+)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index fadf089ae8fe..96a242f91832 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -1121,6 +1121,36 @@ struct btf *btf__parse_split(const char *path, struct btf *base_btf)
> return libbpf_ptr(btf_parse(path, base_btf, NULL));
> }
>
> +int btf__save_raw(const struct btf *btf, const char *path)
> +{
> + const void *data;
> + FILE *f = NULL;
> + __u32 data_sz;
> + int err = 0;
> +
> + data = btf__raw_data(btf, &data_sz);
> + if (!data) {
> + err = -ENOMEM;
> + goto out;
> + }
> +
> + f = fopen(path, "wb");
> + if (!f) {
> + err = -errno;
> + goto out;
> + }
> +
> + if (fwrite(data, 1, data_sz, f) != data_sz) {
> + err = -errno;
> + goto out;
> + }
> +
> +out:
> + if (f)
> + fclose(f);
> + return libbpf_err(err);
> +}
> +
> static void *btf_get_raw_data(const struct btf *btf, __u32 *size, bool swap_endian);
>
> int btf__load_into_kernel(struct btf *btf)
> diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
> index 5c73a5b0a044..4f8d3f303aa6 100644
> --- a/tools/lib/bpf/btf.h
> +++ b/tools/lib/bpf/btf.h
> @@ -114,6 +114,8 @@ LIBBPF_API struct btf *btf__parse_elf_split(const char *path, struct btf *base_b
> LIBBPF_API struct btf *btf__parse_raw(const char *path);
> LIBBPF_API struct btf *btf__parse_raw_split(const char *path, struct btf *base_btf);
>
> +LIBBPF_API int btf__save_raw(const struct btf *btf, const char *path);
> +
> LIBBPF_API struct btf *btf__load_vmlinux_btf(void);
> LIBBPF_API struct btf *btf__load_module_btf(const char *module_name, struct btf *vmlinux_btf);
> LIBBPF_API struct btf *libbpf_find_kernel_btf(void);
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 6a59514a48cf..c9555f8655af 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -414,4 +414,5 @@ LIBBPF_0.6.0 {
> perf_buffer__new_deprecated;
> perf_buffer__new_raw;
> perf_buffer__new_raw_deprecated;
> + btf__save_raw;
this is a sorted list, please keep it so
> } LIBBPF_0.5.0;
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next v2 1/4] libbpf: Implement btf__save_raw()
2021-11-17 5:25 ` Andrii Nakryiko
@ 2021-11-19 17:25 ` Andrii Nakryiko
0 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2021-11-19 17:25 UTC (permalink / raw)
To: Mauricio Vásquez
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Rafael David Tinoco, Lorenzo Fontana,
Leonardo Di Donato
On Tue, Nov 16, 2021 at 9:25 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Tue, Nov 16, 2021 at 8:42 AM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
> >
> > Implement helper function to save the contents of a BTF object to a
> > file.
> >
> > Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> > Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> > Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> > Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> > ---
> > tools/lib/bpf/btf.c | 30 ++++++++++++++++++++++++++++++
> > tools/lib/bpf/btf.h | 2 ++
> > tools/lib/bpf/libbpf.map | 1 +
> > 3 files changed, 33 insertions(+)
> >
> > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > index fadf089ae8fe..96a242f91832 100644
> > --- a/tools/lib/bpf/btf.c
> > +++ b/tools/lib/bpf/btf.c
> > @@ -1121,6 +1121,36 @@ struct btf *btf__parse_split(const char *path, struct btf *base_btf)
> > return libbpf_ptr(btf_parse(path, base_btf, NULL));
> > }
> >
> > +int btf__save_raw(const struct btf *btf, const char *path)
> > +{
> > + const void *data;
> > + FILE *f = NULL;
> > + __u32 data_sz;
> > + int err = 0;
> > +
> > + data = btf__raw_data(btf, &data_sz);
> > + if (!data) {
> > + err = -ENOMEM;
> > + goto out;
> > + }
> > +
> > + f = fopen(path, "wb");
> > + if (!f) {
> > + err = -errno;
> > + goto out;
> > + }
> > +
> > + if (fwrite(data, 1, data_sz, f) != data_sz) {
> > + err = -errno;
> > + goto out;
> > + }
> > +
> > +out:
> > + if (f)
> > + fclose(f);
> > + return libbpf_err(err);
> > +}
> > +
> > static void *btf_get_raw_data(const struct btf *btf, __u32 *size, bool swap_endian);
> >
> > int btf__load_into_kernel(struct btf *btf)
> > diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
> > index 5c73a5b0a044..4f8d3f303aa6 100644
> > --- a/tools/lib/bpf/btf.h
> > +++ b/tools/lib/bpf/btf.h
> > @@ -114,6 +114,8 @@ LIBBPF_API struct btf *btf__parse_elf_split(const char *path, struct btf *base_b
> > LIBBPF_API struct btf *btf__parse_raw(const char *path);
> > LIBBPF_API struct btf *btf__parse_raw_split(const char *path, struct btf *base_btf);
> >
> > +LIBBPF_API int btf__save_raw(const struct btf *btf, const char *path);
Thinking about this some more, I don't feel it's necessary to have
this kind of API in libbpf. Libbpf provides raw bytes and it's not
hard to write dumping byte array to file. We don't have
btf__save_elf() and we probably shouldn't, so I don't see the need for
btf__save_raw() either. It's neither complicated nor frequently used
code to write.
> > +
> > LIBBPF_API struct btf *btf__load_vmlinux_btf(void);
> > LIBBPF_API struct btf *btf__load_module_btf(const char *module_name, struct btf *vmlinux_btf);
> > LIBBPF_API struct btf *libbpf_find_kernel_btf(void);
> > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > index 6a59514a48cf..c9555f8655af 100644
> > --- a/tools/lib/bpf/libbpf.map
> > +++ b/tools/lib/bpf/libbpf.map
> > @@ -414,4 +414,5 @@ LIBBPF_0.6.0 {
> > perf_buffer__new_deprecated;
> > perf_buffer__new_raw;
> > perf_buffer__new_raw_deprecated;
> > + btf__save_raw;
>
> this is a sorted list, please keep it so
>
> > } LIBBPF_0.5.0;
> > --
> > 2.25.1
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH bpf-next v2 2/4] libbpf: Introduce 'btf_custom' to 'bpf_obj_open_opts'
2021-11-16 16:42 [PATCH bpf-next v2 0/4] libbpf: Provide APIs for BTFGen Mauricio Vásquez
2021-11-16 16:42 ` [PATCH bpf-next v2 1/4] libbpf: Implement btf__save_raw() Mauricio Vásquez
@ 2021-11-16 16:42 ` Mauricio Vásquez
2021-11-19 17:25 ` Andrii Nakryiko
2021-11-16 16:42 ` [PATCH bpf-next v2 3/4] libbpf: Introduce 'bpf_object__prepare()' Mauricio Vásquez
2021-11-16 16:42 ` [PATCH bpf-next v2 4/4] libbpf: Expose CO-RE relocation results Mauricio Vásquez
3 siblings, 1 reply; 15+ messages in thread
From: Mauricio Vásquez @ 2021-11-16 16:42 UTC (permalink / raw)
To: netdev, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Rafael David Tinoco, Lorenzo Fontana, Leonardo Di Donato
Commit 1373ff599556 ("libbpf: Introduce 'btf_custom_path' to
'bpf_obj_open_opts'") introduced btf_custom_path which allows developers
to specify a BTF file path to be used for CO-RE relocations. This
implementation parses and releases the BTF file for each bpf object.
This commit introduces a new 'btf_custom' option to allow users to
specify directly the btf object instead of the path. This avoids
parsing/releasing the same BTF file multiple times when the application
loads multiple bpf objects.
Our specific use case is BTFGen[0], where we want to reuse the same BTF
file with multiple bpf objects. In this case passing btf_custom_path is
not only inefficient but it also complicates the implementation as we
want to save pointers of BTF types but they are invalidated after the
bpf object is closed with bpf_object__close().
[0]: https://github.com/kinvolk/btfgen/
Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
---
tools/lib/bpf/libbpf.c | 20 ++++++++++++++++----
tools/lib/bpf/libbpf.h | 9 ++++++++-
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index de7e09a6b5ec..6ca76365c6da 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -542,6 +542,8 @@ struct bpf_object {
char *btf_custom_path;
/* vmlinux BTF override for CO-RE relocations */
struct btf *btf_vmlinux_override;
+ /* true when the user provided the btf structure with the btf_custom opt */
+ bool user_provided_btf_vmlinux;
/* Lazily initialized kernel module BTFs */
struct module_btf *btf_modules;
bool btf_modules_loaded;
@@ -2886,7 +2888,7 @@ static int bpf_object__load_vmlinux_btf(struct bpf_object *obj, bool force)
int err;
/* btf_vmlinux could be loaded earlier */
- if (obj->btf_vmlinux || obj->gen_loader)
+ if (obj->btf_vmlinux || obj->btf_vmlinux_override || obj->gen_loader)
return 0;
if (!force && !obj_needs_vmlinux_btf(obj))
@@ -5474,7 +5476,7 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
if (obj->btf_ext->core_relo_info.len == 0)
return 0;
- if (targ_btf_path) {
+ if (!obj->user_provided_btf_vmlinux && targ_btf_path) {
obj->btf_vmlinux_override = btf__parse(targ_btf_path, NULL);
err = libbpf_get_error(obj->btf_vmlinux_override);
if (err) {
@@ -5543,8 +5545,10 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
out:
/* obj->btf_vmlinux and module BTFs are freed after object load */
- btf__free(obj->btf_vmlinux_override);
- obj->btf_vmlinux_override = NULL;
+ if (!obj->user_provided_btf_vmlinux) {
+ btf__free(obj->btf_vmlinux_override);
+ obj->btf_vmlinux_override = NULL;
+ }
if (!IS_ERR_OR_NULL(cand_cache)) {
hashmap__for_each_entry(cand_cache, entry, i) {
@@ -6767,6 +6771,10 @@ __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
if (!OPTS_VALID(opts, bpf_object_open_opts))
return ERR_PTR(-EINVAL);
+ /* btf_custom_path and btf_custom can't be used together */
+ if (OPTS_GET(opts, btf_custom_path, NULL) && OPTS_GET(opts, btf_custom, NULL))
+ return ERR_PTR(-EINVAL);
+
obj_name = OPTS_GET(opts, object_name, NULL);
if (obj_buf) {
if (!obj_name) {
@@ -6796,6 +6804,10 @@ __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
}
}
+ obj->btf_vmlinux_override = OPTS_GET(opts, btf_custom, NULL);
+ if (obj->btf_vmlinux_override)
+ obj->user_provided_btf_vmlinux = true;
+
kconfig = OPTS_GET(opts, kconfig, NULL);
if (kconfig) {
obj->kconfig = strdup(kconfig);
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 4ec69f224342..908ab04dc9bd 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -104,8 +104,15 @@ struct bpf_object_open_opts {
* struct_ops, etc) will need actual kernel BTF at /sys/kernel/btf/vmlinux.
*/
const char *btf_custom_path;
+ /* Pointer to the custom BTF object to be used for BPF CO-RE relocations.
+ * This custom BTF completely replaces the use of vmlinux BTF
+ * for the purpose of CO-RE relocations.
+ * NOTE: any other BPF feature (e.g., fentry/fexit programs,
+ * struct_ops, etc) will need actual kernel BTF at /sys/kernel/btf/vmlinux.
+ */
+ struct btf *btf_custom;
};
-#define bpf_object_open_opts__last_field btf_custom_path
+#define bpf_object_open_opts__last_field btf_custom
LIBBPF_API struct bpf_object *bpf_object__open(const char *path);
LIBBPF_API struct bpf_object *
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next v2 2/4] libbpf: Introduce 'btf_custom' to 'bpf_obj_open_opts'
2021-11-16 16:42 ` [PATCH bpf-next v2 2/4] libbpf: Introduce 'btf_custom' to 'bpf_obj_open_opts' Mauricio Vásquez
@ 2021-11-19 17:25 ` Andrii Nakryiko
0 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2021-11-19 17:25 UTC (permalink / raw)
To: Mauricio Vásquez
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Rafael David Tinoco, Lorenzo Fontana,
Leonardo Di Donato
On Tue, Nov 16, 2021 at 8:42 AM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
>
> Commit 1373ff599556 ("libbpf: Introduce 'btf_custom_path' to
> 'bpf_obj_open_opts'") introduced btf_custom_path which allows developers
> to specify a BTF file path to be used for CO-RE relocations. This
> implementation parses and releases the BTF file for each bpf object.
>
> This commit introduces a new 'btf_custom' option to allow users to
> specify directly the btf object instead of the path. This avoids
> parsing/releasing the same BTF file multiple times when the application
> loads multiple bpf objects.
>
> Our specific use case is BTFGen[0], where we want to reuse the same BTF
> file with multiple bpf objects. In this case passing btf_custom_path is
> not only inefficient but it also complicates the implementation as we
> want to save pointers of BTF types but they are invalidated after the
> bpf object is closed with bpf_object__close().
How much slower and harder is it in practice, though? Can you please
provide some numbers? How many objects are going to reuse the same
struct btf? Parsing raw BTF file is quite efficient, I'm curious
what's the scale where this becomes unacceptable.
>
> [0]: https://github.com/kinvolk/btfgen/
>
> Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> ---
> tools/lib/bpf/libbpf.c | 20 ++++++++++++++++----
> tools/lib/bpf/libbpf.h | 9 ++++++++-
> 2 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index de7e09a6b5ec..6ca76365c6da 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -542,6 +542,8 @@ struct bpf_object {
> char *btf_custom_path;
> /* vmlinux BTF override for CO-RE relocations */
> struct btf *btf_vmlinux_override;
> + /* true when the user provided the btf structure with the btf_custom opt */
> + bool user_provided_btf_vmlinux;
> /* Lazily initialized kernel module BTFs */
> struct module_btf *btf_modules;
> bool btf_modules_loaded;
> @@ -2886,7 +2888,7 @@ static int bpf_object__load_vmlinux_btf(struct bpf_object *obj, bool force)
> int err;
>
> /* btf_vmlinux could be loaded earlier */
> - if (obj->btf_vmlinux || obj->gen_loader)
> + if (obj->btf_vmlinux || obj->btf_vmlinux_override || obj->gen_loader)
> return 0;
>
> if (!force && !obj_needs_vmlinux_btf(obj))
> @@ -5474,7 +5476,7 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
> if (obj->btf_ext->core_relo_info.len == 0)
> return 0;
>
> - if (targ_btf_path) {
> + if (!obj->user_provided_btf_vmlinux && targ_btf_path) {
> obj->btf_vmlinux_override = btf__parse(targ_btf_path, NULL);
> err = libbpf_get_error(obj->btf_vmlinux_override);
> if (err) {
> @@ -5543,8 +5545,10 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
>
> out:
> /* obj->btf_vmlinux and module BTFs are freed after object load */
> - btf__free(obj->btf_vmlinux_override);
> - obj->btf_vmlinux_override = NULL;
> + if (!obj->user_provided_btf_vmlinux) {
> + btf__free(obj->btf_vmlinux_override);
> + obj->btf_vmlinux_override = NULL;
> + }
>
> if (!IS_ERR_OR_NULL(cand_cache)) {
> hashmap__for_each_entry(cand_cache, entry, i) {
> @@ -6767,6 +6771,10 @@ __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
> if (!OPTS_VALID(opts, bpf_object_open_opts))
> return ERR_PTR(-EINVAL);
>
> + /* btf_custom_path and btf_custom can't be used together */
> + if (OPTS_GET(opts, btf_custom_path, NULL) && OPTS_GET(opts, btf_custom, NULL))
> + return ERR_PTR(-EINVAL);
> +
> obj_name = OPTS_GET(opts, object_name, NULL);
> if (obj_buf) {
> if (!obj_name) {
> @@ -6796,6 +6804,10 @@ __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
> }
> }
>
> + obj->btf_vmlinux_override = OPTS_GET(opts, btf_custom, NULL);
> + if (obj->btf_vmlinux_override)
> + obj->user_provided_btf_vmlinux = true;
> +
> kconfig = OPTS_GET(opts, kconfig, NULL);
> if (kconfig) {
> obj->kconfig = strdup(kconfig);
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 4ec69f224342..908ab04dc9bd 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -104,8 +104,15 @@ struct bpf_object_open_opts {
> * struct_ops, etc) will need actual kernel BTF at /sys/kernel/btf/vmlinux.
> */
> const char *btf_custom_path;
> + /* Pointer to the custom BTF object to be used for BPF CO-RE relocations.
> + * This custom BTF completely replaces the use of vmlinux BTF
> + * for the purpose of CO-RE relocations.
> + * NOTE: any other BPF feature (e.g., fentry/fexit programs,
> + * struct_ops, etc) will need actual kernel BTF at /sys/kernel/btf/vmlinux.
> + */
> + struct btf *btf_custom;
> };
> -#define bpf_object_open_opts__last_field btf_custom_path
> +#define bpf_object_open_opts__last_field btf_custom
>
> LIBBPF_API struct bpf_object *bpf_object__open(const char *path);
> LIBBPF_API struct bpf_object *
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH bpf-next v2 3/4] libbpf: Introduce 'bpf_object__prepare()'
2021-11-16 16:42 [PATCH bpf-next v2 0/4] libbpf: Provide APIs for BTFGen Mauricio Vásquez
2021-11-16 16:42 ` [PATCH bpf-next v2 1/4] libbpf: Implement btf__save_raw() Mauricio Vásquez
2021-11-16 16:42 ` [PATCH bpf-next v2 2/4] libbpf: Introduce 'btf_custom' to 'bpf_obj_open_opts' Mauricio Vásquez
@ 2021-11-16 16:42 ` Mauricio Vásquez
2021-11-16 19:23 ` Mauricio Vásquez Bernal
2021-11-19 17:26 ` Andrii Nakryiko
2021-11-16 16:42 ` [PATCH bpf-next v2 4/4] libbpf: Expose CO-RE relocation results Mauricio Vásquez
3 siblings, 2 replies; 15+ messages in thread
From: Mauricio Vásquez @ 2021-11-16 16:42 UTC (permalink / raw)
To: netdev, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Rafael David Tinoco, Lorenzo Fontana, Leonardo Di Donato
BTFGen[0] requires access to the result of the CO-RE relocations without
actually loading the bpf programs. The current libbpf API doesn't allow
it because all the object preparation (subprogs, relocations: co-re,
elf, maps) happens inside bpf_object__load().
This commit introduces a new bpf_object__prepare() function to perform
all the preparation steps than an ebpf object requires, allowing users
to access the result of those preparation steps without having to load
the program. Almost all the steps that were done in bpf_object__load()
are now done in bpf_object__prepare(), except map creation and program
loading.
Map relocations require a bit more attention as maps are only created in
bpf_object__load(). For this reason bpf_object__prepare() relocates maps
using BPF_PSEUDO_MAP_IDX, if someone dumps the instructions before
loading the program they get something meaningful. Map relocations are
completed in bpf_object__load() once the maps are created and we have
their fd to use with BPF_PSEUDO_MAP_FD.
Users won’t see any visible changes if they’re using bpf_object__open()
+ bpf_object__load() because this commit keeps backwards compatibility
by calling bpf_object__prepare() in bpf_object_load() if it wasn’t
called by the user.
bpf_object__prepare_xattr() is not implemented as their counterpart
bpf_object__load_xattr() will be deprecated[1]. New options will be
added only to bpf_object_open_opts.
[0]: https://github.com/kinvolk/btfgen/
[1]: https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#libbpfh-high-level-apis
Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
---
tools/lib/bpf/libbpf.c | 130 ++++++++++++++++++++++++++++-----------
tools/lib/bpf/libbpf.h | 2 +
tools/lib/bpf/libbpf.map | 1 +
3 files changed, 98 insertions(+), 35 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 6ca76365c6da..f50f9428bb03 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -514,6 +514,7 @@ struct bpf_object {
int nr_extern;
int kconfig_map_idx;
+ bool prepared;
bool loaded;
bool has_subcalls;
bool has_rodata;
@@ -5576,34 +5577,19 @@ bpf_object__relocate_data(struct bpf_object *obj, struct bpf_program *prog)
switch (relo->type) {
case RELO_LD64:
- if (obj->gen_loader) {
- insn[0].src_reg = BPF_PSEUDO_MAP_IDX;
- insn[0].imm = relo->map_idx;
- } else {
- insn[0].src_reg = BPF_PSEUDO_MAP_FD;
- insn[0].imm = obj->maps[relo->map_idx].fd;
- }
+ insn[0].src_reg = BPF_PSEUDO_MAP_IDX;
+ insn[0].imm = relo->map_idx;
break;
case RELO_DATA:
insn[1].imm = insn[0].imm + relo->sym_off;
- if (obj->gen_loader) {
- insn[0].src_reg = BPF_PSEUDO_MAP_IDX_VALUE;
- insn[0].imm = relo->map_idx;
- } else {
- insn[0].src_reg = BPF_PSEUDO_MAP_VALUE;
- insn[0].imm = obj->maps[relo->map_idx].fd;
- }
+ insn[0].src_reg = BPF_PSEUDO_MAP_IDX_VALUE;
+ insn[0].imm = relo->map_idx;
break;
case RELO_EXTERN_VAR:
ext = &obj->externs[relo->sym_off];
if (ext->type == EXT_KCFG) {
- if (obj->gen_loader) {
- insn[0].src_reg = BPF_PSEUDO_MAP_IDX_VALUE;
- insn[0].imm = obj->kconfig_map_idx;
- } else {
- insn[0].src_reg = BPF_PSEUDO_MAP_VALUE;
- insn[0].imm = obj->maps[obj->kconfig_map_idx].fd;
- }
+ insn[0].src_reg = BPF_PSEUDO_MAP_IDX_VALUE;
+ insn[0].imm = obj->kconfig_map_idx;
insn[1].imm = ext->kcfg.data_off;
} else /* EXT_KSYM */ {
if (ext->ksym.type_id && ext->is_set) { /* typed ksyms */
@@ -6144,8 +6130,50 @@ bpf_object__relocate(struct bpf_object *obj, const char *targ_btf_path)
return err;
}
}
- if (!obj->gen_loader)
- bpf_object__free_relocs(obj);
+
+ return 0;
+}
+
+/* relocate instructions that refer to map fds */
+static int
+bpf_object__finish_relocate(struct bpf_object *obj)
+{
+ int i, j;
+
+ if (obj->gen_loader)
+ return 0;
+
+ for (i = 0; i < obj->nr_programs; i++) {
+ struct bpf_program *prog = &obj->programs[i];
+
+ if (prog_is_subprog(obj, prog))
+ continue;
+ for (j = 0; j < prog->nr_reloc; j++) {
+ struct reloc_desc *relo = &prog->reloc_desc[j];
+ struct bpf_insn *insn = &prog->insns[relo->insn_idx];
+ struct extern_desc *ext;
+
+ switch (relo->type) {
+ case RELO_LD64:
+ insn[0].src_reg = BPF_PSEUDO_MAP_FD;
+ insn[0].imm = obj->maps[relo->map_idx].fd;
+ break;
+ case RELO_DATA:
+ insn[0].src_reg = BPF_PSEUDO_MAP_VALUE;
+ insn[0].imm = obj->maps[relo->map_idx].fd;
+ break;
+ case RELO_EXTERN_VAR:
+ ext = &obj->externs[relo->sym_off];
+ if (ext->type == EXT_KCFG) {
+ insn[0].src_reg = BPF_PSEUDO_MAP_VALUE;
+ insn[0].imm = obj->maps[obj->kconfig_map_idx].fd;
+ }
+ default:
+ break;
+ }
+ }
+ }
+
return 0;
}
@@ -6706,8 +6734,8 @@ bpf_object__load_progs(struct bpf_object *obj, int log_level)
if (err)
return err;
}
- if (obj->gen_loader)
- bpf_object__free_relocs(obj);
+
+ bpf_object__free_relocs(obj);
return 0;
}
@@ -7258,6 +7286,39 @@ static int bpf_object__resolve_externs(struct bpf_object *obj,
return 0;
}
+static int __bpf_object__prepare(struct bpf_object *obj, int log_level,
+ const char *target_btf_path)
+{
+ int err;
+
+ if (obj->prepared) {
+ pr_warn("object '%s': prepare can't be attempted twice\n", obj->name);
+ return libbpf_err(-EINVAL);
+ }
+
+ if (obj->gen_loader)
+ bpf_gen__init(obj->gen_loader, log_level);
+
+ err = bpf_object__probe_loading(obj);
+ err = err ? : bpf_object__load_vmlinux_btf(obj, false);
+ err = err ? : bpf_object__resolve_externs(obj, obj->kconfig);
+ err = err ? : bpf_object__sanitize_and_load_btf(obj);
+ err = err ? : bpf_object__sanitize_maps(obj);
+ err = err ? : bpf_object__init_kern_struct_ops_maps(obj);
+ err = err ? : bpf_object__relocate(obj, obj->btf_custom_path ? : target_btf_path);
+
+ obj->prepared = true;
+
+ return err;
+}
+
+LIBBPF_API int bpf_object__prepare(struct bpf_object *obj)
+{
+ if (!obj)
+ return libbpf_err(-EINVAL);
+ return __bpf_object__prepare(obj, 0, NULL);
+}
+
int bpf_object__load_xattr(struct bpf_object_load_attr *attr)
{
struct bpf_object *obj;
@@ -7274,17 +7335,14 @@ int bpf_object__load_xattr(struct bpf_object_load_attr *attr)
return libbpf_err(-EINVAL);
}
- if (obj->gen_loader)
- bpf_gen__init(obj->gen_loader, attr->log_level);
+ if (!obj->prepared) {
+ err = __bpf_object__prepare(obj, attr->log_level, attr->target_btf_path);
+ if (err)
+ return err;
+ }
- err = bpf_object__probe_loading(obj);
- err = err ? : bpf_object__load_vmlinux_btf(obj, false);
- err = err ? : bpf_object__resolve_externs(obj, obj->kconfig);
- err = err ? : bpf_object__sanitize_and_load_btf(obj);
- err = err ? : bpf_object__sanitize_maps(obj);
- err = err ? : bpf_object__init_kern_struct_ops_maps(obj);
- err = err ? : bpf_object__create_maps(obj);
- err = err ? : bpf_object__relocate(obj, obj->btf_custom_path ? : attr->target_btf_path);
+ err = bpf_object__create_maps(obj);
+ err = err ? : bpf_object__finish_relocate(obj);
err = err ? : bpf_object__load_progs(obj, attr->log_level);
if (obj->gen_loader) {
@@ -7940,6 +7998,8 @@ void bpf_object__close(struct bpf_object *obj)
bpf_object__elf_finish(obj);
bpf_object_unload(obj);
btf__free(obj->btf);
+ if (!obj->user_provided_btf_vmlinux)
+ btf__free(obj->btf_vmlinux_override);
btf_ext__free(obj->btf_ext);
for (i = 0; i < obj->nr_maps; i++)
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 908ab04dc9bd..d206b4400a4d 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -148,6 +148,8 @@ 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);
+LIBBPF_API int bpf_object__prepare(struct bpf_object *obj);
+
struct bpf_object_load_attr {
struct bpf_object *obj;
int log_level;
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index c9555f8655af..459b41228933 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -415,4 +415,5 @@ LIBBPF_0.6.0 {
perf_buffer__new_raw;
perf_buffer__new_raw_deprecated;
btf__save_raw;
+ bpf_object__prepare;
} LIBBPF_0.5.0;
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next v2 3/4] libbpf: Introduce 'bpf_object__prepare()'
2021-11-16 16:42 ` [PATCH bpf-next v2 3/4] libbpf: Introduce 'bpf_object__prepare()' Mauricio Vásquez
@ 2021-11-16 19:23 ` Mauricio Vásquez Bernal
2021-11-17 5:35 ` Andrii Nakryiko
2021-11-19 17:26 ` Andrii Nakryiko
1 sibling, 1 reply; 15+ messages in thread
From: Mauricio Vásquez Bernal @ 2021-11-16 19:23 UTC (permalink / raw)
To: Networking, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Rafael David Tinoco, Lorenzo Fontana, Leonardo Di Donato
On Tue, Nov 16, 2021 at 11:42 AM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
>
> BTFGen[0] requires access to the result of the CO-RE relocations without
> actually loading the bpf programs. The current libbpf API doesn't allow
> it because all the object preparation (subprogs, relocations: co-re,
> elf, maps) happens inside bpf_object__load().
>
> This commit introduces a new bpf_object__prepare() function to perform
> all the preparation steps than an ebpf object requires, allowing users
> to access the result of those preparation steps without having to load
> the program. Almost all the steps that were done in bpf_object__load()
> are now done in bpf_object__prepare(), except map creation and program
> loading.
>
> Map relocations require a bit more attention as maps are only created in
> bpf_object__load(). For this reason bpf_object__prepare() relocates maps
> using BPF_PSEUDO_MAP_IDX, if someone dumps the instructions before
> loading the program they get something meaningful. Map relocations are
> completed in bpf_object__load() once the maps are created and we have
> their fd to use with BPF_PSEUDO_MAP_FD.
>
> Users won’t see any visible changes if they’re using bpf_object__open()
> + bpf_object__load() because this commit keeps backwards compatibility
> by calling bpf_object__prepare() in bpf_object_load() if it wasn’t
> called by the user.
>
> bpf_object__prepare_xattr() is not implemented as their counterpart
> bpf_object__load_xattr() will be deprecated[1]. New options will be
> added only to bpf_object_open_opts.
>
> [0]: https://github.com/kinvolk/btfgen/
> [1]: https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#libbpfh-high-level-apis
>
> Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> ---
> tools/lib/bpf/libbpf.c | 130 ++++++++++++++++++++++++++++-----------
> tools/lib/bpf/libbpf.h | 2 +
> tools/lib/bpf/libbpf.map | 1 +
> 3 files changed, 98 insertions(+), 35 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 6ca76365c6da..f50f9428bb03 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -514,6 +514,7 @@ struct bpf_object {
> int nr_extern;
> int kconfig_map_idx;
>
> + bool prepared;
> bool loaded;
> bool has_subcalls;
> bool has_rodata;
> @@ -5576,34 +5577,19 @@ bpf_object__relocate_data(struct bpf_object *obj, struct bpf_program *prog)
>
> switch (relo->type) {
> case RELO_LD64:
> - if (obj->gen_loader) {
> - insn[0].src_reg = BPF_PSEUDO_MAP_IDX;
> - insn[0].imm = relo->map_idx;
> - } else {
> - insn[0].src_reg = BPF_PSEUDO_MAP_FD;
> - insn[0].imm = obj->maps[relo->map_idx].fd;
> - }
> + insn[0].src_reg = BPF_PSEUDO_MAP_IDX;
> + insn[0].imm = relo->map_idx;
> break;
> case RELO_DATA:
> insn[1].imm = insn[0].imm + relo->sym_off;
> - if (obj->gen_loader) {
> - insn[0].src_reg = BPF_PSEUDO_MAP_IDX_VALUE;
> - insn[0].imm = relo->map_idx;
> - } else {
> - insn[0].src_reg = BPF_PSEUDO_MAP_VALUE;
> - insn[0].imm = obj->maps[relo->map_idx].fd;
> - }
> + insn[0].src_reg = BPF_PSEUDO_MAP_IDX_VALUE;
> + insn[0].imm = relo->map_idx;
> break;
> case RELO_EXTERN_VAR:
> ext = &obj->externs[relo->sym_off];
> if (ext->type == EXT_KCFG) {
> - if (obj->gen_loader) {
> - insn[0].src_reg = BPF_PSEUDO_MAP_IDX_VALUE;
> - insn[0].imm = obj->kconfig_map_idx;
> - } else {
> - insn[0].src_reg = BPF_PSEUDO_MAP_VALUE;
> - insn[0].imm = obj->maps[obj->kconfig_map_idx].fd;
> - }
> + insn[0].src_reg = BPF_PSEUDO_MAP_IDX_VALUE;
> + insn[0].imm = obj->kconfig_map_idx;
> insn[1].imm = ext->kcfg.data_off;
> } else /* EXT_KSYM */ {
> if (ext->ksym.type_id && ext->is_set) { /* typed ksyms */
> @@ -6144,8 +6130,50 @@ bpf_object__relocate(struct bpf_object *obj, const char *targ_btf_path)
> return err;
> }
> }
> - if (!obj->gen_loader)
> - bpf_object__free_relocs(obj);
> +
> + return 0;
> +}
> +
> +/* relocate instructions that refer to map fds */
> +static int
> +bpf_object__finish_relocate(struct bpf_object *obj)
> +{
> + int i, j;
> +
> + if (obj->gen_loader)
> + return 0;
> +
> + for (i = 0; i < obj->nr_programs; i++) {
> + struct bpf_program *prog = &obj->programs[i];
> +
> + if (prog_is_subprog(obj, prog))
> + continue;
> + for (j = 0; j < prog->nr_reloc; j++) {
> + struct reloc_desc *relo = &prog->reloc_desc[j];
> + struct bpf_insn *insn = &prog->insns[relo->insn_idx];
> + struct extern_desc *ext;
> +
> + switch (relo->type) {
> + case RELO_LD64:
> + insn[0].src_reg = BPF_PSEUDO_MAP_FD;
> + insn[0].imm = obj->maps[relo->map_idx].fd;
> + break;
> + case RELO_DATA:
> + insn[0].src_reg = BPF_PSEUDO_MAP_VALUE;
> + insn[0].imm = obj->maps[relo->map_idx].fd;
> + break;
> + case RELO_EXTERN_VAR:
> + ext = &obj->externs[relo->sym_off];
> + if (ext->type == EXT_KCFG) {
> + insn[0].src_reg = BPF_PSEUDO_MAP_VALUE;
> + insn[0].imm = obj->maps[obj->kconfig_map_idx].fd;
> + }
> + default:
> + break;
> + }
> + }
> + }
> +
> return 0;
> }
>
> @@ -6706,8 +6734,8 @@ bpf_object__load_progs(struct bpf_object *obj, int log_level)
> if (err)
> return err;
> }
> - if (obj->gen_loader)
> - bpf_object__free_relocs(obj);
> +
> + bpf_object__free_relocs(obj);
> return 0;
> }
>
> @@ -7258,6 +7286,39 @@ static int bpf_object__resolve_externs(struct bpf_object *obj,
> return 0;
> }
>
> +static int __bpf_object__prepare(struct bpf_object *obj, int log_level,
> + const char *target_btf_path)
> +{
> + int err;
> +
> + if (obj->prepared) {
> + pr_warn("object '%s': prepare can't be attempted twice\n", obj->name);
> + return libbpf_err(-EINVAL);
> + }
> +
> + if (obj->gen_loader)
> + bpf_gen__init(obj->gen_loader, log_level);
> +
> + err = bpf_object__probe_loading(obj);
> + err = err ? : bpf_object__load_vmlinux_btf(obj, false);
> + err = err ? : bpf_object__resolve_externs(obj, obj->kconfig);
> + err = err ? : bpf_object__sanitize_and_load_btf(obj);
> + err = err ? : bpf_object__sanitize_maps(obj);
> + err = err ? : bpf_object__init_kern_struct_ops_maps(obj);
> + err = err ? : bpf_object__relocate(obj, obj->btf_custom_path ? : target_btf_path);
> +
> + obj->prepared = true;
> +
> + return err;
> +}
> +
> +LIBBPF_API int bpf_object__prepare(struct bpf_object *obj)
> +{
> + if (!obj)
> + return libbpf_err(-EINVAL);
> + return __bpf_object__prepare(obj, 0, NULL);
> +}
> +
> int bpf_object__load_xattr(struct bpf_object_load_attr *attr)
> {
> struct bpf_object *obj;
> @@ -7274,17 +7335,14 @@ int bpf_object__load_xattr(struct bpf_object_load_attr *attr)
> return libbpf_err(-EINVAL);
> }
>
> - if (obj->gen_loader)
> - bpf_gen__init(obj->gen_loader, attr->log_level);
> + if (!obj->prepared) {
> + err = __bpf_object__prepare(obj, attr->log_level, attr->target_btf_path);
> + if (err)
> + return err;
> + }
>
> - err = bpf_object__probe_loading(obj);
After sending the patches we realized they weren't working without
root privileges in systems with unprivileged BPF disabled. This line
should not be moved to bpf_object__prepare indeed. We'll fix it in the
next iteration.
> - err = err ? : bpf_object__load_vmlinux_btf(obj, false);
> - err = err ? : bpf_object__resolve_externs(obj, obj->kconfig);
> - err = err ? : bpf_object__sanitize_and_load_btf(obj);
> - err = err ? : bpf_object__sanitize_maps(obj);
> - err = err ? : bpf_object__init_kern_struct_ops_maps(obj);
> - err = err ? : bpf_object__create_maps(obj);
> - err = err ? : bpf_object__relocate(obj, obj->btf_custom_path ? : attr->target_btf_path);
> + err = bpf_object__create_maps(obj);
> + err = err ? : bpf_object__finish_relocate(obj);
> err = err ? : bpf_object__load_progs(obj, attr->log_level);
>
> if (obj->gen_loader) {
> @@ -7940,6 +7998,8 @@ void bpf_object__close(struct bpf_object *obj)
> bpf_object__elf_finish(obj);
> bpf_object_unload(obj);
> btf__free(obj->btf);
> + if (!obj->user_provided_btf_vmlinux)
> + btf__free(obj->btf_vmlinux_override);
> btf_ext__free(obj->btf_ext);
>
> for (i = 0; i < obj->nr_maps; i++)
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 908ab04dc9bd..d206b4400a4d 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -148,6 +148,8 @@ 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);
>
> +LIBBPF_API int bpf_object__prepare(struct bpf_object *obj);
> +
> struct bpf_object_load_attr {
> struct bpf_object *obj;
> int log_level;
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index c9555f8655af..459b41228933 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -415,4 +415,5 @@ LIBBPF_0.6.0 {
> perf_buffer__new_raw;
> perf_buffer__new_raw_deprecated;
> btf__save_raw;
> + bpf_object__prepare;
> } LIBBPF_0.5.0;
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next v2 3/4] libbpf: Introduce 'bpf_object__prepare()'
2021-11-16 19:23 ` Mauricio Vásquez Bernal
@ 2021-11-17 5:35 ` Andrii Nakryiko
0 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2021-11-17 5:35 UTC (permalink / raw)
To: Mauricio Vásquez Bernal
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Rafael David Tinoco, Lorenzo Fontana,
Leonardo Di Donato
On Tue, Nov 16, 2021 at 11:24 AM Mauricio Vásquez Bernal
<mauricio@kinvolk.io> wrote:
>
> On Tue, Nov 16, 2021 at 11:42 AM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
> >
> > BTFGen[0] requires access to the result of the CO-RE relocations without
> > actually loading the bpf programs. The current libbpf API doesn't allow
> > it because all the object preparation (subprogs, relocations: co-re,
> > elf, maps) happens inside bpf_object__load().
> >
> > This commit introduces a new bpf_object__prepare() function to perform
> > all the preparation steps than an ebpf object requires, allowing users
> > to access the result of those preparation steps without having to load
> > the program. Almost all the steps that were done in bpf_object__load()
> > are now done in bpf_object__prepare(), except map creation and program
> > loading.
> >
> > Map relocations require a bit more attention as maps are only created in
> > bpf_object__load(). For this reason bpf_object__prepare() relocates maps
> > using BPF_PSEUDO_MAP_IDX, if someone dumps the instructions before
> > loading the program they get something meaningful. Map relocations are
> > completed in bpf_object__load() once the maps are created and we have
> > their fd to use with BPF_PSEUDO_MAP_FD.
> >
> > Users won’t see any visible changes if they’re using bpf_object__open()
> > + bpf_object__load() because this commit keeps backwards compatibility
> > by calling bpf_object__prepare() in bpf_object_load() if it wasn’t
> > called by the user.
> >
> > bpf_object__prepare_xattr() is not implemented as their counterpart
> > bpf_object__load_xattr() will be deprecated[1]. New options will be
> > added only to bpf_object_open_opts.
> >
> > [0]: https://github.com/kinvolk/btfgen/
> > [1]: https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#libbpfh-high-level-apis
> >
> > Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> > Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> > Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> > Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> > ---
> > tools/lib/bpf/libbpf.c | 130 ++++++++++++++++++++++++++++-----------
> > tools/lib/bpf/libbpf.h | 2 +
> > tools/lib/bpf/libbpf.map | 1 +
> > 3 files changed, 98 insertions(+), 35 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 6ca76365c6da..f50f9428bb03 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -514,6 +514,7 @@ struct bpf_object {
> > int nr_extern;
> > int kconfig_map_idx;
> >
> > + bool prepared;
> > bool loaded;
> > bool has_subcalls;
> > bool has_rodata;
> > @@ -5576,34 +5577,19 @@ bpf_object__relocate_data(struct bpf_object *obj, struct bpf_program *prog)
> >
> > switch (relo->type) {
> > case RELO_LD64:
> > - if (obj->gen_loader) {
> > - insn[0].src_reg = BPF_PSEUDO_MAP_IDX;
> > - insn[0].imm = relo->map_idx;
> > - } else {
> > - insn[0].src_reg = BPF_PSEUDO_MAP_FD;
> > - insn[0].imm = obj->maps[relo->map_idx].fd;
> > - }
> > + insn[0].src_reg = BPF_PSEUDO_MAP_IDX;
> > + insn[0].imm = relo->map_idx;
> > break;
> > case RELO_DATA:
> > insn[1].imm = insn[0].imm + relo->sym_off;
> > - if (obj->gen_loader) {
> > - insn[0].src_reg = BPF_PSEUDO_MAP_IDX_VALUE;
> > - insn[0].imm = relo->map_idx;
> > - } else {
> > - insn[0].src_reg = BPF_PSEUDO_MAP_VALUE;
> > - insn[0].imm = obj->maps[relo->map_idx].fd;
> > - }
> > + insn[0].src_reg = BPF_PSEUDO_MAP_IDX_VALUE;
> > + insn[0].imm = relo->map_idx;
> > break;
> > case RELO_EXTERN_VAR:
> > ext = &obj->externs[relo->sym_off];
> > if (ext->type == EXT_KCFG) {
> > - if (obj->gen_loader) {
> > - insn[0].src_reg = BPF_PSEUDO_MAP_IDX_VALUE;
> > - insn[0].imm = obj->kconfig_map_idx;
> > - } else {
> > - insn[0].src_reg = BPF_PSEUDO_MAP_VALUE;
> > - insn[0].imm = obj->maps[obj->kconfig_map_idx].fd;
> > - }
> > + insn[0].src_reg = BPF_PSEUDO_MAP_IDX_VALUE;
> > + insn[0].imm = obj->kconfig_map_idx;
> > insn[1].imm = ext->kcfg.data_off;
> > } else /* EXT_KSYM */ {
> > if (ext->ksym.type_id && ext->is_set) { /* typed ksyms */
> > @@ -6144,8 +6130,50 @@ bpf_object__relocate(struct bpf_object *obj, const char *targ_btf_path)
> > return err;
> > }
> > }
> > - if (!obj->gen_loader)
> > - bpf_object__free_relocs(obj);
> > +
> > + return 0;
> > +}
> > +
> > +/* relocate instructions that refer to map fds */
> > +static int
> > +bpf_object__finish_relocate(struct bpf_object *obj)
> > +{
> > + int i, j;
> > +
> > + if (obj->gen_loader)
> > + return 0;
> > +
> > + for (i = 0; i < obj->nr_programs; i++) {
> > + struct bpf_program *prog = &obj->programs[i];
> > +
> > + if (prog_is_subprog(obj, prog))
> > + continue;
> > + for (j = 0; j < prog->nr_reloc; j++) {
> > + struct reloc_desc *relo = &prog->reloc_desc[j];
> > + struct bpf_insn *insn = &prog->insns[relo->insn_idx];
> > + struct extern_desc *ext;
> > +
> > + switch (relo->type) {
> > + case RELO_LD64:
> > + insn[0].src_reg = BPF_PSEUDO_MAP_FD;
> > + insn[0].imm = obj->maps[relo->map_idx].fd;
> > + break;
> > + case RELO_DATA:
> > + insn[0].src_reg = BPF_PSEUDO_MAP_VALUE;
> > + insn[0].imm = obj->maps[relo->map_idx].fd;
> > + break;
> > + case RELO_EXTERN_VAR:
> > + ext = &obj->externs[relo->sym_off];
> > + if (ext->type == EXT_KCFG) {
> > + insn[0].src_reg = BPF_PSEUDO_MAP_VALUE;
> > + insn[0].imm = obj->maps[obj->kconfig_map_idx].fd;
> > + }
> > + default:
> > + break;
> > + }
> > + }
> > + }
> > +
> > return 0;
> > }
> >
> > @@ -6706,8 +6734,8 @@ bpf_object__load_progs(struct bpf_object *obj, int log_level)
> > if (err)
> > return err;
> > }
> > - if (obj->gen_loader)
> > - bpf_object__free_relocs(obj);
> > +
> > + bpf_object__free_relocs(obj);
> > return 0;
> > }
> >
> > @@ -7258,6 +7286,39 @@ static int bpf_object__resolve_externs(struct bpf_object *obj,
> > return 0;
> > }
> >
> > +static int __bpf_object__prepare(struct bpf_object *obj, int log_level,
> > + const char *target_btf_path)
> > +{
> > + int err;
> > +
> > + if (obj->prepared) {
> > + pr_warn("object '%s': prepare can't be attempted twice\n", obj->name);
> > + return libbpf_err(-EINVAL);
> > + }
> > +
> > + if (obj->gen_loader)
> > + bpf_gen__init(obj->gen_loader, log_level);
> > +
> > + err = bpf_object__probe_loading(obj);
> > + err = err ? : bpf_object__load_vmlinux_btf(obj, false);
> > + err = err ? : bpf_object__resolve_externs(obj, obj->kconfig);
> > + err = err ? : bpf_object__sanitize_and_load_btf(obj);
> > + err = err ? : bpf_object__sanitize_maps(obj);
> > + err = err ? : bpf_object__init_kern_struct_ops_maps(obj);
> > + err = err ? : bpf_object__relocate(obj, obj->btf_custom_path ? : target_btf_path);
> > +
> > + obj->prepared = true;
> > +
> > + return err;
> > +}
> > +
> > +LIBBPF_API int bpf_object__prepare(struct bpf_object *obj)
> > +{
> > + if (!obj)
> > + return libbpf_err(-EINVAL);
> > + return __bpf_object__prepare(obj, 0, NULL);
> > +}
> > +
> > int bpf_object__load_xattr(struct bpf_object_load_attr *attr)
> > {
> > struct bpf_object *obj;
> > @@ -7274,17 +7335,14 @@ int bpf_object__load_xattr(struct bpf_object_load_attr *attr)
> > return libbpf_err(-EINVAL);
> > }
> >
> > - if (obj->gen_loader)
> > - bpf_gen__init(obj->gen_loader, attr->log_level);
> > + if (!obj->prepared) {
> > + err = __bpf_object__prepare(obj, attr->log_level, attr->target_btf_path);
> > + if (err)
> > + return err;
> > + }
> >
> > - err = bpf_object__probe_loading(obj);
>
> After sending the patches we realized they weren't working without
> root privileges in systems with unprivileged BPF disabled. This line
> should not be moved to bpf_object__prepare indeed. We'll fix it in the
> next iteration.
It's not just probe_loading, loading BTF is also privileged. We need
to also think whether to really resolve externs and do other steps.
Non-weak externs might prevent BTFgen from working, if the intended
host kernel will have the extern, but the host on which you are
generating reduced BTF doesn't have such kernel symbol.
For BTFGen, keeping the amount of work done in preparation to a
minimum (which basically means load BTF and perform CO-RE
relocations), would probably be the simplest and also best.
Let me get back to reviewing this patch set tomorrow. It's pretty late
today, not the best time to do a thorough review.
>
>
> > - err = err ? : bpf_object__load_vmlinux_btf(obj, false);
> > - err = err ? : bpf_object__resolve_externs(obj, obj->kconfig);
> > - err = err ? : bpf_object__sanitize_and_load_btf(obj);
> > - err = err ? : bpf_object__sanitize_maps(obj);
> > - err = err ? : bpf_object__init_kern_struct_ops_maps(obj);
> > - err = err ? : bpf_object__create_maps(obj);
> > - err = err ? : bpf_object__relocate(obj, obj->btf_custom_path ? : attr->target_btf_path);
> > + err = bpf_object__create_maps(obj);
> > + err = err ? : bpf_object__finish_relocate(obj);
> > err = err ? : bpf_object__load_progs(obj, attr->log_level);
> >
> > if (obj->gen_loader) {
> > @@ -7940,6 +7998,8 @@ void bpf_object__close(struct bpf_object *obj)
> > bpf_object__elf_finish(obj);
> > bpf_object_unload(obj);
> > btf__free(obj->btf);
> > + if (!obj->user_provided_btf_vmlinux)
> > + btf__free(obj->btf_vmlinux_override);
> > btf_ext__free(obj->btf_ext);
> >
> > for (i = 0; i < obj->nr_maps; i++)
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index 908ab04dc9bd..d206b4400a4d 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -148,6 +148,8 @@ 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);
> >
> > +LIBBPF_API int bpf_object__prepare(struct bpf_object *obj);
> > +
> > struct bpf_object_load_attr {
> > struct bpf_object *obj;
> > int log_level;
> > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > index c9555f8655af..459b41228933 100644
> > --- a/tools/lib/bpf/libbpf.map
> > +++ b/tools/lib/bpf/libbpf.map
> > @@ -415,4 +415,5 @@ LIBBPF_0.6.0 {
> > perf_buffer__new_raw;
> > perf_buffer__new_raw_deprecated;
> > btf__save_raw;
> > + bpf_object__prepare;
> > } LIBBPF_0.5.0;
> > --
> > 2.25.1
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next v2 3/4] libbpf: Introduce 'bpf_object__prepare()'
2021-11-16 16:42 ` [PATCH bpf-next v2 3/4] libbpf: Introduce 'bpf_object__prepare()' Mauricio Vásquez
2021-11-16 19:23 ` Mauricio Vásquez Bernal
@ 2021-11-19 17:26 ` Andrii Nakryiko
1 sibling, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2021-11-19 17:26 UTC (permalink / raw)
To: Mauricio Vásquez
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Rafael David Tinoco, Lorenzo Fontana,
Leonardo Di Donato
On Tue, Nov 16, 2021 at 8:42 AM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
>
> BTFGen[0] requires access to the result of the CO-RE relocations without
> actually loading the bpf programs. The current libbpf API doesn't allow
> it because all the object preparation (subprogs, relocations: co-re,
> elf, maps) happens inside bpf_object__load().
>
> This commit introduces a new bpf_object__prepare() function to perform
> all the preparation steps than an ebpf object requires, allowing users
> to access the result of those preparation steps without having to load
> the program. Almost all the steps that were done in bpf_object__load()
> are now done in bpf_object__prepare(), except map creation and program
> loading.
>
> Map relocations require a bit more attention as maps are only created in
> bpf_object__load(). For this reason bpf_object__prepare() relocates maps
> using BPF_PSEUDO_MAP_IDX, if someone dumps the instructions before
> loading the program they get something meaningful. Map relocations are
> completed in bpf_object__load() once the maps are created and we have
> their fd to use with BPF_PSEUDO_MAP_FD.
>
> Users won’t see any visible changes if they’re using bpf_object__open()
> + bpf_object__load() because this commit keeps backwards compatibility
> by calling bpf_object__prepare() in bpf_object_load() if it wasn’t
> called by the user.
>
> bpf_object__prepare_xattr() is not implemented as their counterpart
> bpf_object__load_xattr() will be deprecated[1]. New options will be
> added only to bpf_object_open_opts.
>
> [0]: https://github.com/kinvolk/btfgen/
> [1]: https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0#libbpfh-high-level-apis
>
> Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> ---
Most of the comments are irrelevant after my comments on patch #4, but
still sending them out for the sake of completeness.
> tools/lib/bpf/libbpf.c | 130 ++++++++++++++++++++++++++++-----------
> tools/lib/bpf/libbpf.h | 2 +
> tools/lib/bpf/libbpf.map | 1 +
> 3 files changed, 98 insertions(+), 35 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 6ca76365c6da..f50f9428bb03 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -514,6 +514,7 @@ struct bpf_object {
> int nr_extern;
> int kconfig_map_idx;
>
> + bool prepared;
> bool loaded;
let's turn `bool loaded` into an enum to represent the stage of a
bpf_object, as there is a strict sequence of state transitions.
> bool has_subcalls;
> bool has_rodata;
[...]
> +static int __bpf_object__prepare(struct bpf_object *obj, int log_level,
> + const char *target_btf_path)
> +{
> + int err;
> +
> + if (obj->prepared) {
> + pr_warn("object '%s': prepare can't be attempted twice\n", obj->name);
> + return libbpf_err(-EINVAL);
> + }
> +
> + if (obj->gen_loader)
> + bpf_gen__init(obj->gen_loader, log_level);
> +
> + err = bpf_object__probe_loading(obj);
> + err = err ? : bpf_object__load_vmlinux_btf(obj, false);
> + err = err ? : bpf_object__resolve_externs(obj, obj->kconfig);
> + err = err ? : bpf_object__sanitize_and_load_btf(obj);
> + err = err ? : bpf_object__sanitize_maps(obj);
> + err = err ? : bpf_object__init_kern_struct_ops_maps(obj);
> + err = err ? : bpf_object__relocate(obj, obj->btf_custom_path ? : target_btf_path);
> +
I think only load_vmlinux_btf and relocations should happen in prepare
phase. resolve_externs might fail if you don't run it on target system
(non-weak extern that's missing will error out). There is no need to
load and sanitize BTFs or sanitize maps either. struct_ops can't work
on the kernel that doesn't have BTF enabled, so BTFgen won't help
there, so it's fine to move it to load phase as well.
We need to move relocations way earlier, right after load_vmlinux_btf
(and move probe_loading to load phase, of course) and perform them in
preparation phase. We can also split relocation into individual steps
and do map relocations in the load phase, so you won't have to do the
dance with map_idx (and given it's internal process, we can always
change our mind later and rework it, if necessary; but for now let's
keep things simple).
> + obj->prepared = true;
> +
> + return err;
> +}
> +
> +LIBBPF_API int bpf_object__prepare(struct bpf_object *obj)
LIBBPF_API is used only in header files
> +{
> + if (!obj)
> + return libbpf_err(-EINVAL);
if someone passes NULL pointer instead of obj, then it's a completely
inappropriate use of libbpf APIs. No need to check for that (we don't
in a lot of APIs for sure).
> + return __bpf_object__prepare(obj, 0, NULL);
> +}
> +
> int bpf_object__load_xattr(struct bpf_object_load_attr *attr)
> {
> struct bpf_object *obj;
[...]
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH bpf-next v2 4/4] libbpf: Expose CO-RE relocation results
2021-11-16 16:42 [PATCH bpf-next v2 0/4] libbpf: Provide APIs for BTFGen Mauricio Vásquez
` (2 preceding siblings ...)
2021-11-16 16:42 ` [PATCH bpf-next v2 3/4] libbpf: Introduce 'bpf_object__prepare()' Mauricio Vásquez
@ 2021-11-16 16:42 ` Mauricio Vásquez
2021-11-19 17:25 ` Andrii Nakryiko
3 siblings, 1 reply; 15+ messages in thread
From: Mauricio Vásquez @ 2021-11-16 16:42 UTC (permalink / raw)
To: netdev, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Rafael David Tinoco, Lorenzo Fontana, Leonardo Di Donato
The result of the CO-RE relocations can be useful for some use cases
like BTFGen[0]. This commit adds a new ‘record_core_relos’ option to
save the result of such relocations and a couple of functions to access
them.
[0]: https://github.com/kinvolk/btfgen/
Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
---
tools/lib/bpf/libbpf.c | 63 ++++++++++++++++++++++++++++++++++++++-
tools/lib/bpf/libbpf.h | 49 +++++++++++++++++++++++++++++-
tools/lib/bpf/libbpf.map | 2 ++
tools/lib/bpf/relo_core.c | 28 +++++++++++++++--
tools/lib/bpf/relo_core.h | 21 ++-----------
5 files changed, 140 insertions(+), 23 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index f50f9428bb03..a5da977a9f5d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -306,6 +306,10 @@ struct bpf_program {
struct reloc_desc *reloc_desc;
int nr_reloc;
+
+ struct bpf_core_relo_result *core_relos;
+ int nr_core_relos;
+
int log_level;
struct {
@@ -519,6 +523,9 @@ struct bpf_object {
bool has_subcalls;
bool has_rodata;
+ /* Record CO-RE relocations for the different programs in prog->core_relos */
+ bool record_core_relos;
+
struct bpf_gen *gen_loader;
/* Information when doing ELF related work. Only valid if efile.elf is not NULL */
@@ -614,8 +621,10 @@ static void bpf_program__exit(struct bpf_program *prog)
zfree(&prog->pin_name);
zfree(&prog->insns);
zfree(&prog->reloc_desc);
+ zfree(&prog->core_relos);
prog->nr_reloc = 0;
+ prog->nr_core_relos = 0;
prog->insns_cnt = 0;
prog->sec_idx = -1;
}
@@ -5408,6 +5417,7 @@ static int bpf_core_apply_relo(struct bpf_program *prog,
struct hashmap *cand_cache)
{
const void *type_key = u32_as_hash_key(relo->type_id);
+ struct bpf_core_relo_result *core_relo = NULL;
struct bpf_core_cand_list *cands = NULL;
const char *prog_name = prog->name;
const struct btf_type *local_type;
@@ -5459,7 +5469,18 @@ static int bpf_core_apply_relo(struct bpf_program *prog,
}
}
- return bpf_core_apply_relo_insn(prog_name, insn, insn_idx, relo, relo_idx, local_btf, cands);
+ if (prog->obj->record_core_relos) {
+ prog->core_relos = libbpf_reallocarray(prog->core_relos,
+ sizeof(*prog->core_relos),
+ prog->nr_core_relos + 1);
+ if (!prog->core_relos)
+ return -ENOMEM;
+
+ core_relo = &prog->core_relos[prog->nr_core_relos++];
+ }
+
+ return bpf_core_apply_relo_insn(prog_name, insn, insn_idx, relo, relo_idx, local_btf, cands,
+ core_relo);
}
static int
@@ -5815,6 +5836,28 @@ static int append_subprog_relos(struct bpf_program *main_prog, struct bpf_progra
return 0;
}
+static int append_subprog_core_relos(struct bpf_program *main_prog, struct bpf_program *subprog)
+{
+ int new_cnt = main_prog->nr_core_relos + subprog->nr_core_relos;
+ struct bpf_core_relo_result *relos;
+ int i;
+
+ if (main_prog == subprog)
+ return 0;
+ relos = libbpf_reallocarray(main_prog->core_relos, new_cnt, sizeof(*relos));
+ if (!relos)
+ return -ENOMEM;
+ memcpy(relos + main_prog->nr_core_relos, subprog->core_relos,
+ sizeof(*relos) * subprog->nr_core_relos);
+
+ for (i = main_prog->nr_core_relos; i < new_cnt; i++)
+ relos[i].insn_idx += subprog->sub_insn_off;
+
+ main_prog->core_relos = relos;
+ main_prog->nr_core_relos = new_cnt;
+ return 0;
+}
+
static int
bpf_object__reloc_code(struct bpf_object *obj, struct bpf_program *main_prog,
struct bpf_program *prog)
@@ -5918,6 +5961,11 @@ bpf_object__reloc_code(struct bpf_object *obj, struct bpf_program *main_prog,
err = append_subprog_relos(main_prog, subprog);
if (err)
return err;
+
+ err = append_subprog_core_relos(main_prog, subprog);
+ if (err)
+ return err;
+
err = bpf_object__reloc_code(obj, main_prog, subprog);
if (err)
return err;
@@ -6168,6 +6216,7 @@ bpf_object__finish_relocate(struct bpf_object *obj)
insn[0].src_reg = BPF_PSEUDO_MAP_VALUE;
insn[0].imm = obj->maps[obj->kconfig_map_idx].fd;
}
+ break;
default:
break;
}
@@ -6845,6 +6894,8 @@ __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
}
}
+ obj->record_core_relos = OPTS_GET(opts, record_core_relos, false);
+
err = bpf_object__elf_init(obj);
err = err ? : bpf_object__check_endianness(obj);
err = err ? : bpf_object__elf_collect(obj);
@@ -8253,6 +8304,16 @@ size_t bpf_program__insn_cnt(const struct bpf_program *prog)
return prog->insns_cnt;
}
+const struct bpf_core_relo_result *bpf_program__core_relos(struct bpf_program *prog)
+{
+ return prog->core_relos;
+}
+
+int bpf_program__core_relos_cnt(struct bpf_program *prog)
+{
+ return prog->nr_core_relos;
+}
+
int bpf_program__set_prep(struct bpf_program *prog, int nr_instances,
bpf_program_prep_t prep)
{
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index d206b4400a4d..47c8ac41f61b 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -111,8 +111,12 @@ struct bpf_object_open_opts {
* struct_ops, etc) will need actual kernel BTF at /sys/kernel/btf/vmlinux.
*/
struct btf *btf_custom;
+ /* Keep track of CO-RE relocation results. This information can be retrieved
+ * with bpf_program__core_relos() after the object is prepared.
+ */
+ bool record_core_relos;
};
-#define bpf_object_open_opts__last_field btf_custom
+#define bpf_object_open_opts__last_field record_core_relos
LIBBPF_API struct bpf_object *bpf_object__open(const char *path);
LIBBPF_API struct bpf_object *
@@ -286,6 +290,49 @@ LIBBPF_API int bpf_program__pin(struct bpf_program *prog, const char *path);
LIBBPF_API int bpf_program__unpin(struct bpf_program *prog, const char *path);
LIBBPF_API void bpf_program__unload(struct bpf_program *prog);
+/* bpf_core_relo_kind encodes which aspect of captured field/type/enum value
+ * has to be adjusted by relocations.
+ */
+enum bpf_core_relo_kind {
+ BPF_FIELD_BYTE_OFFSET = 0, /* field byte offset */
+ BPF_FIELD_BYTE_SIZE = 1, /* field size in bytes */
+ BPF_FIELD_EXISTS = 2, /* field existence in target kernel */
+ BPF_FIELD_SIGNED = 3, /* field signedness (0 - unsigned, 1 - signed) */
+ BPF_FIELD_LSHIFT_U64 = 4, /* bitfield-specific left bitshift */
+ BPF_FIELD_RSHIFT_U64 = 5, /* bitfield-specific right bitshift */
+ BPF_TYPE_ID_LOCAL = 6, /* type ID in local BPF object */
+ BPF_TYPE_ID_TARGET = 7, /* type ID in target kernel */
+ BPF_TYPE_EXISTS = 8, /* type existence in target kernel */
+ BPF_TYPE_SIZE = 9, /* type size in bytes */
+ BPF_ENUMVAL_EXISTS = 10, /* enum value existence in target kernel */
+ BPF_ENUMVAL_VALUE = 11, /* enum value integer value */
+};
+
+#define BPF_CORE_SPEC_MAX_LEN 64
+
+struct bpf_core_relo_spec {
+ const struct btf *btf;
+ __u32 root_type_id;
+ /* accessor spec */
+ int spec[BPF_CORE_SPEC_MAX_LEN];
+ int spec_len;
+};
+
+struct bpf_core_relo_result {
+ struct bpf_core_relo_spec local_spec, targ_spec;
+ int insn_idx;
+ enum bpf_core_relo_kind relo_kind;
+ /* true if libbpf wasn't able to perform the relocation */
+ bool poison;
+ /* original value in the instruction */
+ __u32 orig_val;
+ /* new value that the instruction needs to be patched up to */
+ __u32 new_val;
+};
+
+LIBBPF_API const struct bpf_core_relo_result *bpf_program__core_relos(struct bpf_program *prog);
+LIBBPF_API int bpf_program__core_relos_cnt(struct bpf_program *prog);
+
struct bpf_link;
LIBBPF_API struct bpf_link *bpf_link__open(const char *path);
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 459b41228933..4aeb5db9c4e3 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -416,4 +416,6 @@ LIBBPF_0.6.0 {
perf_buffer__new_raw_deprecated;
btf__save_raw;
bpf_object__prepare;
+ bpf_program__core_relos;
+ bpf_program__core_relos_cnt;
} LIBBPF_0.5.0;
diff --git a/tools/lib/bpf/relo_core.c b/tools/lib/bpf/relo_core.c
index b5b8956a1be8..11b04c5961a1 100644
--- a/tools/lib/bpf/relo_core.c
+++ b/tools/lib/bpf/relo_core.c
@@ -13,8 +13,6 @@
#include "str_error.h"
#include "libbpf_internal.h"
-#define BPF_CORE_SPEC_MAX_LEN 64
-
/* represents BPF CO-RE field or array element accessor */
struct bpf_core_accessor {
__u32 type_id; /* struct/union type or array element type */
@@ -1092,6 +1090,18 @@ static void bpf_core_dump_spec(int level, const struct bpf_core_spec *spec)
}
}
+static void copy_core_spec(const struct bpf_core_spec *src, struct bpf_core_relo_spec *dst)
+{
+ int i;
+
+ dst->root_type_id = src->root_type_id;
+ dst->btf = src->btf;
+ dst->spec_len = src->raw_len;
+
+ for (i = 0; i < src->raw_len; i++)
+ dst->spec[i] = src->raw_spec[i];
+}
+
/*
* CO-RE relocate single instruction.
*
@@ -1147,7 +1157,8 @@ int bpf_core_apply_relo_insn(const char *prog_name, struct bpf_insn *insn,
const struct bpf_core_relo *relo,
int relo_idx,
const struct btf *local_btf,
- struct bpf_core_cand_list *cands)
+ struct bpf_core_cand_list *cands,
+ struct bpf_core_relo_result *core_relo)
{
struct bpf_core_spec local_spec, cand_spec, targ_spec = {};
struct bpf_core_relo_res cand_res, targ_res;
@@ -1291,5 +1302,16 @@ int bpf_core_apply_relo_insn(const char *prog_name, struct bpf_insn *insn,
return -EINVAL;
}
+ if (core_relo) {
+ copy_core_spec(&local_spec, &core_relo->local_spec);
+ copy_core_spec(&targ_spec, &core_relo->targ_spec);
+
+ core_relo->insn_idx = insn_idx;
+ core_relo->poison = targ_res.poison;
+ core_relo->relo_kind = targ_spec.relo_kind;
+ core_relo->orig_val = targ_res.orig_val;
+ core_relo->new_val = targ_res.new_val;
+ }
+
return 0;
}
diff --git a/tools/lib/bpf/relo_core.h b/tools/lib/bpf/relo_core.h
index 3b9f8f18346c..89d7c4c31ccd 100644
--- a/tools/lib/bpf/relo_core.h
+++ b/tools/lib/bpf/relo_core.h
@@ -4,23 +4,7 @@
#ifndef __RELO_CORE_H
#define __RELO_CORE_H
-/* bpf_core_relo_kind encodes which aspect of captured field/type/enum value
- * has to be adjusted by relocations.
- */
-enum bpf_core_relo_kind {
- BPF_FIELD_BYTE_OFFSET = 0, /* field byte offset */
- BPF_FIELD_BYTE_SIZE = 1, /* field size in bytes */
- BPF_FIELD_EXISTS = 2, /* field existence in target kernel */
- BPF_FIELD_SIGNED = 3, /* field signedness (0 - unsigned, 1 - signed) */
- BPF_FIELD_LSHIFT_U64 = 4, /* bitfield-specific left bitshift */
- BPF_FIELD_RSHIFT_U64 = 5, /* bitfield-specific right bitshift */
- BPF_TYPE_ID_LOCAL = 6, /* type ID in local BPF object */
- BPF_TYPE_ID_TARGET = 7, /* type ID in target kernel */
- BPF_TYPE_EXISTS = 8, /* type existence in target kernel */
- BPF_TYPE_SIZE = 9, /* type size in bytes */
- BPF_ENUMVAL_EXISTS = 10, /* enum value existence in target kernel */
- BPF_ENUMVAL_VALUE = 11, /* enum value integer value */
-};
+#include "libbpf.h"
/* The minimum bpf_core_relo checked by the loader
*
@@ -92,7 +76,8 @@ int bpf_core_apply_relo_insn(const char *prog_name,
struct bpf_insn *insn, int insn_idx,
const struct bpf_core_relo *relo, int relo_idx,
const struct btf *local_btf,
- struct bpf_core_cand_list *cands);
+ struct bpf_core_cand_list *cands,
+ struct bpf_core_relo_result *core_relo);
int bpf_core_types_are_compat(const struct btf *local_btf, __u32 local_id,
const struct btf *targ_btf, __u32 targ_id);
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next v2 4/4] libbpf: Expose CO-RE relocation results
2021-11-16 16:42 ` [PATCH bpf-next v2 4/4] libbpf: Expose CO-RE relocation results Mauricio Vásquez
@ 2021-11-19 17:25 ` Andrii Nakryiko
2021-11-19 23:51 ` Alexei Starovoitov
2021-12-03 21:08 ` Mauricio Vásquez Bernal
0 siblings, 2 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2021-11-19 17:25 UTC (permalink / raw)
To: Mauricio Vásquez
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Rafael David Tinoco, Lorenzo Fontana,
Leonardo Di Donato
On Tue, Nov 16, 2021 at 8:42 AM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
>
> The result of the CO-RE relocations can be useful for some use cases
> like BTFGen[0]. This commit adds a new ‘record_core_relos’ option to
> save the result of such relocations and a couple of functions to access
> them.
>
> [0]: https://github.com/kinvolk/btfgen/
>
> Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> ---
> tools/lib/bpf/libbpf.c | 63 ++++++++++++++++++++++++++++++++++++++-
> tools/lib/bpf/libbpf.h | 49 +++++++++++++++++++++++++++++-
> tools/lib/bpf/libbpf.map | 2 ++
> tools/lib/bpf/relo_core.c | 28 +++++++++++++++--
> tools/lib/bpf/relo_core.h | 21 ++-----------
> 5 files changed, 140 insertions(+), 23 deletions(-)
>
Ok, I've meditated on this patch set long enough. I still don't like
that libbpf will be doing all this just for the sake of BTFGen's use
case.
In the end, I think giving bpftool access to internal APIs of libbpf
is more appropriate, and it seems like it's pretty easy to achieve. It
might actually clean up gen_loader parts a bit as well. So we'll need
to coordinate all this with Alexei's current work on CO-RE for kernel
as well.
But here's what I think could be done to keep libbpf internals simple.
We split bpf_core_apply_relo() into two parts: 1) calculating the
struct bpf_core_relo_res and 2) patching the instruction. If you look
at bpf_core_apply_relo, it needs prog just for prog_name (which we can
just pass everywhere for logging purposes) and to extract one specific
instruction to be patched. This instruction is passed at the very end
to bpf_core_patch_insn() after bpf_core_relo_res is calculated. So I
propose to make those two explicitly separate steps done by libbpf. So
bpf_core_apply_relo() (which we should rename to bpf_core_calc_relo()
or something like that) won't do any modification to the program
instructions. bpf_object__relocate_core() will do bpf_core_calc_relo()
first, if that's successful, it will pass the result into
bpf_core_patch_insn(). Simple and clean, unless I missed some
complication (happens all the time, but..)
At this point, we can teach bpftool to just take btf_ext (from BPF
object file), iterate over all CO-RE relos and call only
bpf_core_calc_relo() (no instruction patching). Using
bpf_core_relo_res bpftool will know types and fields that need to be
preserved and it will be able to construct minimal btf. So interface
for bpftool looks like this:
bpftool gen distill_btf (or whatever the name) <file.bpf.o>
<distilled_raw.btf>
BTFGen as a solution, then, can use bpftool to process each pair of
btf + bpf object.
Thoughts?
[...]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next v2 4/4] libbpf: Expose CO-RE relocation results
2021-11-19 17:25 ` Andrii Nakryiko
@ 2021-11-19 23:51 ` Alexei Starovoitov
2021-12-03 21:08 ` Mauricio Vásquez Bernal
1 sibling, 0 replies; 15+ messages in thread
From: Alexei Starovoitov @ 2021-11-19 23:51 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Mauricio Vásquez, Networking, bpf, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Rafael David Tinoco,
Lorenzo Fontana, Leonardo Di Donato
On Fri, Nov 19, 2021 at 09:25:03AM -0800, Andrii Nakryiko wrote:
> On Tue, Nov 16, 2021 at 8:42 AM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
> >
> > The result of the CO-RE relocations can be useful for some use cases
> > like BTFGen[0]. This commit adds a new ‘record_core_relos’ option to
> > save the result of such relocations and a couple of functions to access
> > them.
> >
> > [0]: https://github.com/kinvolk/btfgen/
> >
> > Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> > Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> > Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> > Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> > ---
> > tools/lib/bpf/libbpf.c | 63 ++++++++++++++++++++++++++++++++++++++-
> > tools/lib/bpf/libbpf.h | 49 +++++++++++++++++++++++++++++-
> > tools/lib/bpf/libbpf.map | 2 ++
> > tools/lib/bpf/relo_core.c | 28 +++++++++++++++--
> > tools/lib/bpf/relo_core.h | 21 ++-----------
> > 5 files changed, 140 insertions(+), 23 deletions(-)
> >
>
> Ok, I've meditated on this patch set long enough. I still don't like
> that libbpf will be doing all this just for the sake of BTFGen's use
> case.
>
> In the end, I think giving bpftool access to internal APIs of libbpf
> is more appropriate, and it seems like it's pretty easy to achieve. It
> might actually clean up gen_loader parts a bit as well. So we'll need
> to coordinate all this with Alexei's current work on CO-RE for kernel
> as well.
>
> But here's what I think could be done to keep libbpf internals simple.
> We split bpf_core_apply_relo() into two parts: 1) calculating the
> struct bpf_core_relo_res and 2) patching the instruction. If you look
> at bpf_core_apply_relo, it needs prog just for prog_name (which we can
> just pass everywhere for logging purposes) and to extract one specific
> instruction to be patched. This instruction is passed at the very end
> to bpf_core_patch_insn() after bpf_core_relo_res is calculated. So I
> propose to make those two explicitly separate steps done by libbpf. So
> bpf_core_apply_relo() (which we should rename to bpf_core_calc_relo()
> or something like that) won't do any modification to the program
> instructions. bpf_object__relocate_core() will do bpf_core_calc_relo()
> first, if that's successful, it will pass the result into
> bpf_core_patch_insn(). Simple and clean, unless I missed some
> complication (happens all the time, but..)
I was thinking about such split as well, but for a different reason :)
Since we've discussed future kernel flag 'check what libbpf had done'
the idea is to use bpf_core_relo_res after first step and let kernel
look at insn to see whether libbpf relocated the insn the same way
as kernel is going to do.
Also I was thinking to pass struct bpf_core_spec [3] and
struct bpf_core_relo_res [2] as two arrays into bpf_core_calc_relo() to
reduce stack size, since reduction of BPF_CORE_SPEC_MAX_LEN to 32
is not enough when all kconfig debugs are on on some architectures.
I was planning to work on that as a follow up to my set.
In the light of BTFgen I was thinking whether bpf_core_relo_res should
be part of uapi returned by the kernel, but that is probably overkill.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next v2 4/4] libbpf: Expose CO-RE relocation results
2021-11-19 17:25 ` Andrii Nakryiko
2021-11-19 23:51 ` Alexei Starovoitov
@ 2021-12-03 21:08 ` Mauricio Vásquez Bernal
2021-12-07 3:48 ` Andrii Nakryiko
1 sibling, 1 reply; 15+ messages in thread
From: Mauricio Vásquez Bernal @ 2021-12-03 21:08 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Rafael David Tinoco, Lorenzo Fontana,
Leonardo Di Donato
On Fri, Nov 19, 2021 at 12:25 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Tue, Nov 16, 2021 at 8:42 AM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
> >
> > The result of the CO-RE relocations can be useful for some use cases
> > like BTFGen[0]. This commit adds a new ‘record_core_relos’ option to
> > save the result of such relocations and a couple of functions to access
> > them.
> >
> > [0]: https://github.com/kinvolk/btfgen/
> >
> > Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> > Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> > Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> > Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> > ---
> > tools/lib/bpf/libbpf.c | 63 ++++++++++++++++++++++++++++++++++++++-
> > tools/lib/bpf/libbpf.h | 49 +++++++++++++++++++++++++++++-
> > tools/lib/bpf/libbpf.map | 2 ++
> > tools/lib/bpf/relo_core.c | 28 +++++++++++++++--
> > tools/lib/bpf/relo_core.h | 21 ++-----------
> > 5 files changed, 140 insertions(+), 23 deletions(-)
> >
>
> Ok, I've meditated on this patch set long enough. I still don't like
> that libbpf will be doing all this just for the sake of BTFGen's use
> case.
>
> In the end, I think giving bpftool access to internal APIs of libbpf
> is more appropriate, and it seems like it's pretty easy to achieve. It
> might actually clean up gen_loader parts a bit as well. So we'll need
> to coordinate all this with Alexei's current work on CO-RE for kernel
> as well.
Fine for us. I followed the CO-RE in the kernel patch and I didn't
spot any change that could complicate the BTFGen implementation.
> But here's what I think could be done to keep libbpf internals simple.
> We split bpf_core_apply_relo() into two parts: 1) calculating the
> struct bpf_core_relo_res and
For the BTFGen case we actually need "struct bpf_core_relo_res". I
suppose it's not a big deal to move its definition to a header file
that can be included by bpftool.
> 2) patching the instruction. If you look
> at bpf_core_apply_relo, it needs prog just for prog_name (which we can
> just pass everywhere for logging purposes) and to extract one specific
> instruction to be patched. This instruction is passed at the very end
> to bpf_core_patch_insn() after bpf_core_relo_res is calculated. So I
> propose to make those two explicitly separate steps done by libbpf. So
> bpf_core_apply_relo() (which we should rename to bpf_core_calc_relo()
> or something like that) won't do any modification to the program
> instructions. bpf_object__relocate_core() will do bpf_core_calc_relo()
> first, if that's successful, it will pass the result into
> bpf_core_patch_insn(). Simple and clean, unless I missed some
> complication (happens all the time, but..)
While implementing a prototype of this idea I faced the following challenges:
- bpf_core_apply_relo() requires a candidate cache. I think we can
create two helpers functions to create / destroy a candidate cache so
we don't have to worry about it's internals in bpftool.
- we need to access obj->btf_ext in bpftool. It should be fine to
create bpf_object__btf_ext() as part of the public libbpf api.
- bpf_core_apply_relo() requires the bpf program as argument. Before
Alexei's patches it was used only for logging and getting the
instruction. Now it's also used to call record_relo_core(). Getting it
from bpftool is not that easy, in order to do I had to expose
bpf_program__sec_idx() and find_prog_by_sec_insn() to bpftool. We
could find a way to avoid passing prog but I think it's important for
the logs.
- obj->btf_vmlinux_override needs to be set in order to calculate the
core relocations. It's only set in bpf_object__relocate_core() but
we're not using this function. I created and exposed a
bpf_object_set_vmlinux_override() function to bpftool.
- There are also some naming complications but we can discuss them
when I send the patch.
I'm going to polish a bit more and finish rebasing on top of "CO-RE in
the kernel" changes to then send the patch. Please let me know if you
have any big concerns regarding my points above.
> At this point, we can teach bpftool to just take btf_ext (from BPF
> object file), iterate over all CO-RE relos and call only
> bpf_core_calc_relo() (no instruction patching). Using
> bpf_core_relo_res bpftool will know types and fields that need to be
> preserved and it will be able to construct minimal btf. So interface
> for bpftool looks like this:
>
> bpftool gen distill_btf (or whatever the name) <file.bpf.o>
> <distilled_raw.btf>
>
> BTFGen as a solution, then, can use bpftool to process each pair of
> btf + bpf object.
>
> Thoughts?
I have the feeling that it could be easier to extend
bpf_object__relocate_core() to be able to calculate the core
relocations without patching the instructions (something similar to
what we did in v1). bpftool could pass two parameters to gather this
information and the normal libbpf workflow could just pass NULL to
indicate the instructions should be actually patched. I think this
could help specially with the difficulty to get the prog argument from
bpftool (we are almost implementing the same logic present on
bpf_object__relocate_core() to get sec_idx, prog and so on). Does it
make any sense to you?
Thanks!
> [...]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next v2 4/4] libbpf: Expose CO-RE relocation results
2021-12-03 21:08 ` Mauricio Vásquez Bernal
@ 2021-12-07 3:48 ` Andrii Nakryiko
0 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2021-12-07 3:48 UTC (permalink / raw)
To: Mauricio Vásquez Bernal
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Rafael David Tinoco, Lorenzo Fontana,
Leonardo Di Donato
On Fri, Dec 3, 2021 at 1:08 PM Mauricio Vásquez Bernal
<mauricio@kinvolk.io> wrote:
>
> On Fri, Nov 19, 2021 at 12:25 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Tue, Nov 16, 2021 at 8:42 AM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
> > >
> > > The result of the CO-RE relocations can be useful for some use cases
> > > like BTFGen[0]. This commit adds a new ‘record_core_relos’ option to
> > > save the result of such relocations and a couple of functions to access
> > > them.
> > >
> > > [0]: https://github.com/kinvolk/btfgen/
> > >
> > > Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> > > Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> > > Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> > > Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> > > ---
> > > tools/lib/bpf/libbpf.c | 63 ++++++++++++++++++++++++++++++++++++++-
> > > tools/lib/bpf/libbpf.h | 49 +++++++++++++++++++++++++++++-
> > > tools/lib/bpf/libbpf.map | 2 ++
> > > tools/lib/bpf/relo_core.c | 28 +++++++++++++++--
> > > tools/lib/bpf/relo_core.h | 21 ++-----------
> > > 5 files changed, 140 insertions(+), 23 deletions(-)
> > >
> >
> > Ok, I've meditated on this patch set long enough. I still don't like
> > that libbpf will be doing all this just for the sake of BTFGen's use
> > case.
> >
> > In the end, I think giving bpftool access to internal APIs of libbpf
> > is more appropriate, and it seems like it's pretty easy to achieve. It
> > might actually clean up gen_loader parts a bit as well. So we'll need
> > to coordinate all this with Alexei's current work on CO-RE for kernel
> > as well.
>
> Fine for us. I followed the CO-RE in the kernel patch and I didn't
> spot any change that could complicate the BTFGen implementation.
>
> > But here's what I think could be done to keep libbpf internals simple.
> > We split bpf_core_apply_relo() into two parts: 1) calculating the
> > struct bpf_core_relo_res and
>
> For the BTFGen case we actually need "struct bpf_core_relo_res". I
> suppose it's not a big deal to move its definition to a header file
> that can be included by bpftool.
either move it to relo_core.h (if it's ok to be used in kernel, not
sure, try it) or we can put it into libbpf_internal.h
>
> > 2) patching the instruction. If you look
> > at bpf_core_apply_relo, it needs prog just for prog_name (which we can
> > just pass everywhere for logging purposes) and to extract one specific
> > instruction to be patched. This instruction is passed at the very end
> > to bpf_core_patch_insn() after bpf_core_relo_res is calculated. So I
> > propose to make those two explicitly separate steps done by libbpf. So
> > bpf_core_apply_relo() (which we should rename to bpf_core_calc_relo()
> > or something like that) won't do any modification to the program
> > instructions. bpf_object__relocate_core() will do bpf_core_calc_relo()
> > first, if that's successful, it will pass the result into
> > bpf_core_patch_insn(). Simple and clean, unless I missed some
> > complication (happens all the time, but..)
>
> While implementing a prototype of this idea I faced the following challenges:
> - bpf_core_apply_relo() requires a candidate cache. I think we can
> create two helpers functions to create / destroy a candidate cache so
> we don't have to worry about it's internals in bpftool.
yeah, two helpers exposed through libbpf_internal.h is the lesser evil
> - we need to access obj->btf_ext in bpftool. It should be fine to
> create bpf_object__btf_ext() as part of the public libbpf api.
Yeah, bpf_object__btf_ext() sounds good, it's a natural counterpart to
btf_object__btf().
It probably makes sense to also move struct btf_ext_header into btf.h
(btf_header is part of kernel UAPI, BTF.ext header is not going to
change in a non-backwards-compatible way either). There are also
btf_ext_info_sec, bpf_func_info_min and bpf_line_info_min (and Alexei
already exposed core reloc record), which probably makes sense to make
part of btf.h, but that should be tackled separately, probably.
> - bpf_core_apply_relo() requires the bpf program as argument. Before
> Alexei's patches it was used only for logging and getting the
> instruction. Now it's also used to call record_relo_core(). Getting it
> from bpftool is not that easy, in order to do I had to expose
> bpf_program__sec_idx() and find_prog_by_sec_insn() to bpftool. We
> could find a way to avoid passing prog but I think it's important for
> the logs.
record_relo_core can be moved out of bpf_core_apply_relo(). It only
needs prog and relo. It can be called as an alternative to
bpf_core_apply_relo() if gen_loader is set.
> - obj->btf_vmlinux_override needs to be set in order to calculate the
> core relocations. It's only set in bpf_object__relocate_core() but
> we're not using this function. I created and exposed a
> bpf_object_set_vmlinux_override() function to bpftool.
ok, this one I'm confused with. btf_custom_path in
bpf_object_open_opts doesn't work?..
> - There are also some naming complications but we can discuss them
> when I send the patch.
>
> I'm going to polish a bit more and finish rebasing on top of "CO-RE in
> the kernel" changes to then send the patch. Please let me know if you
> have any big concerns regarding my points above.
>
ok
> > At this point, we can teach bpftool to just take btf_ext (from BPF
> > object file), iterate over all CO-RE relos and call only
> > bpf_core_calc_relo() (no instruction patching). Using
> > bpf_core_relo_res bpftool will know types and fields that need to be
> > preserved and it will be able to construct minimal btf. So interface
> > for bpftool looks like this:
> >
> > bpftool gen distill_btf (or whatever the name) <file.bpf.o>
> > <distilled_raw.btf>
> >
> > BTFGen as a solution, then, can use bpftool to process each pair of
> > btf + bpf object.
> >
> > Thoughts?
>
> I have the feeling that it could be easier to extend
> bpf_object__relocate_core() to be able to calculate the core
> relocations without patching the instructions (something similar to
> what we did in v1). bpftool could pass two parameters to gather this
> information and the normal libbpf workflow could just pass NULL to
> indicate the instructions should be actually patched. I think this
> could help specially with the difficulty to get the prog argument from
> bpftool (we are almost implementing the same logic present on
> bpf_object__relocate_core() to get sec_idx, prog and so on). Does it
> make any sense to you?
No, not really, see above. It might be a slightly smaller amount of
refactoring to achieve this, but will make for more complicated code,
I think. Let's try the original approach, see my suggestions above.
Seems like everything is pretty straightforward at this point.
>
> Thanks!
>
> > [...]
^ permalink raw reply [flat|nested] 15+ messages in thread