On 2/5/19 11:48 AM, Andrii Nakryiko wrote: > This change splits out previous btf__new functionality of constructing > struct btf and loading it into kernel into two: > - btf__new() just creates and initializes struct btf > - btf__load() attempts to load existing struct btf into kernel > > btf__free will still close BTF fd, if it was ever loaded successfully > into kernel. > > This change allows users of libbpf to manipulate BTF using its API, > without the need to unnecessarily load it into kernel. > > One of the intended use cases is pahole using libbpf to do DWARF to BTF > conversion and deduplication using libbpf, while handling ELF sections > overwrites and other concerns on its own. > > Signed-off-by: Andrii Nakryiko <andr...@fb.com> > Acked-by: Song Liu <songliubrav...@fb.com> > --- > tools/lib/bpf/btf.c | 53 ++++++++++++++++++++++------------------ > tools/lib/bpf/btf.h | 1 + > tools/lib/bpf/libbpf.c | 2 +- > tools/lib/bpf/libbpf.map | 1 + > 4 files changed, 32 insertions(+), 25 deletions(-) > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c > index 4949f8840bda..065d51fa63e5 100644 > --- a/tools/lib/bpf/btf.c > +++ b/tools/lib/bpf/btf.c > @@ -366,8 +366,6 @@ void btf__free(struct btf *btf) > > struct btf *btf__new(__u8 *data, __u32 size) > { > - __u32 log_buf_size = 0; > - char *log_buf = NULL; > struct btf *btf; > int err; > > @@ -377,15 +375,6 @@ struct btf *btf__new(__u8 *data, __u32 size) > > btf->fd = -1; > > - log_buf = malloc(BPF_LOG_BUF_SIZE); > - if (!log_buf) { > - err = -ENOMEM; > - goto done; > - } > - > - *log_buf = 0; > - log_buf_size = BPF_LOG_BUF_SIZE; > - > btf->data = malloc(size); > if (!btf->data) { > err = -ENOMEM; > @@ -395,17 +384,6 @@ struct btf *btf__new(__u8 *data, __u32 size) > memcpy(btf->data, data, size); > btf->data_size = size; > > - btf->fd = bpf_load_btf(btf->data, btf->data_size, > - log_buf, log_buf_size, false); > - > - if (btf->fd == -1) { > - err = -errno; > - pr_warning("Error loading BTF: %s(%d)\n", strerror(errno), > errno); > - if (log_buf && *log_buf) > - pr_warning("%s\n", log_buf); > - goto done; > - } > - > err = btf_parse_hdr(btf); > if (err) > goto done; > @@ -417,8 +395,6 @@ struct btf *btf__new(__u8 *data, __u32 size) > err = btf_parse_type_sec(btf); > > done: > - free(log_buf); > - > if (err) { > btf__free(btf); > return ERR_PTR(err); > @@ -427,6 +403,35 @@ struct btf *btf__new(__u8 *data, __u32 size) > return btf; > } > > +int btf__load(struct btf* btf) { > + __u32 log_buf_size = BPF_LOG_BUF_SIZE; > + char *log_buf = NULL; > + > + if (btf->fd >= 0) { > + return -EEXIST; > + } > + > + log_buf = malloc(log_buf_size); > + if (!log_buf) > + return -ENOMEM; > + > + *log_buf = 0; > + > + btf->fd = bpf_load_btf(btf->data, btf->data_size, > + log_buf, log_buf_size, false); > + if (btf->fd < 0) { > + btf->fd = -errno;
Why you set btf->fd = -errno? Do you have any intended use for it later. If not, I would still prefer the existing value -1. This will be consistent with all other fd field convention in libbpf. > + pr_warning("Error loading BTF: %s(%d)\n", strerror(errno), > errno); > + if (*log_buf) > + pr_warning("%s\n", log_buf); > + goto done; > + } > + > +done: > + free(log_buf); > + return btf->fd; > +} > + > int btf__fd(const struct btf *btf) > { > return btf->fd; > diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h > index 25a9d2db035d..e8410887f93a 100644 > --- a/tools/lib/bpf/btf.h > +++ b/tools/lib/bpf/btf.h > @@ -57,6 +57,7 @@ struct btf_ext_header { > > LIBBPF_API void btf__free(struct btf *btf); > LIBBPF_API struct btf *btf__new(__u8 *data, __u32 size); > +LIBBPF_API int btf__load(struct btf* btf); > LIBBPF_API __s32 btf__find_by_name(const struct btf *btf, > const char *type_name); > LIBBPF_API __u32 btf__get_nr_types(const struct btf *btf); > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 47969aa0faf8..75b82c1cfc72 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -835,7 +835,7 @@ static int bpf_object__elf_collect(struct bpf_object > *obj, int flags) > obj->efile.maps_shndx = idx; > else if (strcmp(name, BTF_ELF_SEC) == 0) { > obj->btf = btf__new(data->d_buf, data->d_size); > - if (IS_ERR(obj->btf)) { > + if (IS_ERR(obj->btf) || btf__load(obj->btf) < 0) { > pr_warning("Error loading ELF section %s: %ld. > Ignored and continue.\n", > BTF_ELF_SEC, PTR_ERR(obj->btf)); > obj->btf = NULL; > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map > index 89c1149e32ee..ffa1fe044f6a 100644 > --- a/tools/lib/bpf/libbpf.map > +++ b/tools/lib/bpf/libbpf.map > @@ -134,6 +134,7 @@ LIBBPF_0.0.2 { > bpf_object__find_map_fd_by_name; > bpf_get_link_xdp_id; > btf__dedup; > + btf__load; Maybe put btf__load after btf__get_strings based on alphabetical order. With the above changes, > btf__get_map_kv_tids; > btf__get_nr_types; > btf__get_strings; >