* [PATCH bpf-next] libbpf: Extend linker API to support in-memory ELF files
@ 2024-11-20 17:02 Alastair Robertson
2024-11-21 23:19 ` Andrii Nakryiko
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Alastair Robertson @ 2024-11-20 17:02 UTC (permalink / raw)
To: bpf, andrii; +Cc: Alastair Robertson
The new_fd, add_fd and finalize_fd functions correspond to the original
new, add_file and finalize functions, but accept an FD instead of a file
name. This gives API consumers the option of using anonymous
files/memfds to avoid writing ELFs to disk.
This new API will be useful for performing linking as part of
bpftrace's JIT compilation.
The add_buf function is a convenience wrapper that does the work of
creating a memfd for the caller.
Signed-off-by: Alastair Robertson <ajor@meta.com>
---
tools/lib/bpf/libbpf.h | 9 ++
tools/lib/bpf/libbpf.map | 4 +
tools/lib/bpf/linker.c | 229 +++++++++++++++++++++++++++++----------
3 files changed, 185 insertions(+), 57 deletions(-)
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index b2ce3a72b11d..aae8f954c4fc 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -1796,10 +1796,19 @@ struct bpf_linker_file_opts {
struct bpf_linker;
LIBBPF_API struct bpf_linker *bpf_linker__new(const char *filename, struct bpf_linker_opts *opts);
+LIBBPF_API struct bpf_linker *bpf_linker__new_fd(const char *name, int fd,
+ struct bpf_linker_opts *opts);
LIBBPF_API int bpf_linker__add_file(struct bpf_linker *linker,
const char *filename,
const struct bpf_linker_file_opts *opts);
+LIBBPF_API int bpf_linker__add_fd(struct bpf_linker *linker,
+ const char *name, int fd,
+ const struct bpf_linker_file_opts *opts);
+LIBBPF_API int bpf_linker__add_buf(struct bpf_linker *linker, const char *name,
+ void *buffer, int buffer_sz,
+ const struct bpf_linker_file_opts *opts);
LIBBPF_API int bpf_linker__finalize(struct bpf_linker *linker);
+LIBBPF_API int bpf_linker__finalize_fd(struct bpf_linker *linker);
LIBBPF_API void bpf_linker__free(struct bpf_linker *linker);
/*
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 54b6f312cfa8..e767f34c1d08 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -432,4 +432,8 @@ LIBBPF_1.5.0 {
} LIBBPF_1.4.0;
LIBBPF_1.6.0 {
+ bpf_linker__new_fd;
+ bpf_linker__add_fd;
+ bpf_linker__add_buf;
+ bpf_linker__finalize_fd;
} LIBBPF_1.5.0;
diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
index cf71d149fe26..6571ed8b858f 100644
--- a/tools/lib/bpf/linker.c
+++ b/tools/lib/bpf/linker.c
@@ -4,6 +4,8 @@
*
* Copyright (c) 2021 Facebook
*/
+#define _GNU_SOURCE
+
#include <stdbool.h>
#include <stddef.h>
#include <stdio.h>
@@ -16,6 +18,7 @@
#include <elf.h>
#include <libelf.h>
#include <fcntl.h>
+#include <sys/mman.h>
#include "libbpf.h"
#include "btf.h"
#include "libbpf_internal.h"
@@ -157,9 +160,9 @@ struct bpf_linker {
#define pr_warn_elf(fmt, ...) \
libbpf_print(LIBBPF_WARN, "libbpf: " fmt ": %s\n", ##__VA_ARGS__, elf_errmsg(-1))
-static int init_output_elf(struct bpf_linker *linker, const char *file);
+static int init_output_elf(struct bpf_linker *linker);
-static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
+static int linker_load_obj_file(struct bpf_linker *linker,
const struct bpf_linker_file_opts *opts,
struct src_obj *obj);
static int linker_sanity_check_elf(struct src_obj *obj);
@@ -233,9 +236,56 @@ struct bpf_linker *bpf_linker__new(const char *filename, struct bpf_linker_opts
if (!linker)
return errno = ENOMEM, NULL;
- linker->fd = -1;
+ linker->filename = strdup(filename);
+ if (!linker->filename)
+ return errno = ENOMEM, NULL;
+
+ linker->fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC, 0644);
+ if (linker->fd < 0) {
+ err = -errno;
+ pr_warn("failed to create '%s': %d\n", filename, err);
+ goto err_out;
+ }
+
+ err = init_output_elf(linker);
+ if (err)
+ goto err_out;
+
+ return linker;
+
+err_out:
+ bpf_linker__free(linker);
+ return errno = -err, NULL;
+}
+
+struct bpf_linker *bpf_linker__new_fd(const char *name, int fd,
+ struct bpf_linker_opts *opts)
+{
+ struct bpf_linker *linker;
+ int err;
+
+ if (fd < 0)
+ return errno = EINVAL, NULL;
+
+ if (!OPTS_VALID(opts, bpf_linker_opts))
+ return errno = EINVAL, NULL;
+
+ if (elf_version(EV_CURRENT) == EV_NONE) {
+ pr_warn_elf("libelf initialization failed");
+ return errno = EINVAL, NULL;
+ }
+
+ linker = calloc(1, sizeof(*linker));
+ if (!linker)
+ return errno = ENOMEM, NULL;
+
+ linker->filename = strdup(name);
+ if (!linker->filename)
+ return errno = ENOMEM, NULL;
+
+ linker->fd = fd;
- err = init_output_elf(linker, filename);
+ err = init_output_elf(linker);
if (err)
goto err_out;
@@ -294,23 +344,12 @@ static Elf64_Sym *add_new_sym(struct bpf_linker *linker, size_t *sym_idx)
return sym;
}
-static int init_output_elf(struct bpf_linker *linker, const char *file)
+static int init_output_elf(struct bpf_linker *linker)
{
int err, str_off;
Elf64_Sym *init_sym;
struct dst_sec *sec;
- linker->filename = strdup(file);
- if (!linker->filename)
- return -ENOMEM;
-
- linker->fd = open(file, O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC, 0644);
- if (linker->fd < 0) {
- err = -errno;
- pr_warn("failed to create '%s': %s\n", file, errstr(err));
- return err;
- }
-
linker->elf = elf_begin(linker->fd, ELF_C_WRITE, NULL);
if (!linker->elf) {
pr_warn_elf("failed to create ELF object");
@@ -436,10 +475,9 @@ static int init_output_elf(struct bpf_linker *linker, const char *file)
return 0;
}
-int bpf_linker__add_file(struct bpf_linker *linker, const char *filename,
- const struct bpf_linker_file_opts *opts)
+static int linker_add_common(struct bpf_linker *linker, struct src_obj *obj,
+ const struct bpf_linker_file_opts *opts)
{
- struct src_obj obj = {};
int err = 0;
if (!OPTS_VALID(opts, bpf_linker_file_opts))
@@ -448,25 +486,91 @@ int bpf_linker__add_file(struct bpf_linker *linker, const char *filename,
if (!linker->elf)
return libbpf_err(-EINVAL);
- err = err ?: linker_load_obj_file(linker, filename, opts, &obj);
- err = err ?: linker_append_sec_data(linker, &obj);
- err = err ?: linker_append_elf_syms(linker, &obj);
- err = err ?: linker_append_elf_relos(linker, &obj);
- err = err ?: linker_append_btf(linker, &obj);
- err = err ?: linker_append_btf_ext(linker, &obj);
+ err = err ?: linker_load_obj_file(linker, opts, obj);
+ err = err ?: linker_append_sec_data(linker, obj);
+ err = err ?: linker_append_elf_syms(linker, obj);
+ err = err ?: linker_append_elf_relos(linker, obj);
+ err = err ?: linker_append_btf(linker, obj);
+ err = err ?: linker_append_btf_ext(linker, obj);
/* free up src_obj resources */
- free(obj.btf_type_map);
- btf__free(obj.btf);
- btf_ext__free(obj.btf_ext);
- free(obj.secs);
- free(obj.sym_map);
- if (obj.elf)
- elf_end(obj.elf);
+ free(obj->btf_type_map);
+ btf__free(obj->btf);
+ btf_ext__free(obj->btf_ext);
+ free(obj->secs);
+ free(obj->sym_map);
+ if (obj->elf)
+ elf_end(obj->elf);
+ /* leave obj->fd for the caller to clean up if appropriate */
+
+ return libbpf_err(err);
+}
+
+int bpf_linker__add_file(struct bpf_linker *linker, const char *filename,
+ const struct bpf_linker_file_opts *opts)
+{
+ struct src_obj obj = {};
+ int ret;
+
+ obj.filename = filename;
+ obj.fd = open(filename, O_RDONLY | O_CLOEXEC);
+ if (obj.fd < 0) {
+ pr_warn("failed to open file '%s': %s\n", filename, errstr(errno));
+ return -errno;
+ }
+
+ ret = linker_add_common(linker, &obj, opts);
+
if (obj.fd >= 0)
close(obj.fd);
- return libbpf_err(err);
+ return ret;
+}
+
+int bpf_linker__add_fd(struct bpf_linker *linker, const char *name, int fd,
+ const struct bpf_linker_file_opts *opts)
+{
+ struct src_obj obj = {};
+
+ if (fd < 0)
+ return libbpf_err(-EINVAL);
+
+ obj.filename = name;
+ obj.fd = fd;
+
+ return linker_add_common(linker, &obj, opts);
+}
+
+int bpf_linker__add_buf(struct bpf_linker *linker, const char *name,
+ void *buffer, int buffer_sz,
+ const struct bpf_linker_file_opts *opts)
+{
+ struct src_obj obj = {};
+ int written, ret;
+
+ obj.filename = name;
+ obj.fd = memfd_create(name, 0);
+ if (obj.fd < 0) {
+ pr_warn("failed to create memfd '%s': %s\n", name, errstr(errno));
+ return -errno;
+ }
+
+ written = 0;
+ while (written < buffer_sz) {
+ ret = write(obj.fd, buffer, buffer_sz);
+ if (ret < 0) {
+ pr_warn("failed to write '%s': %s\n", name, errstr(errno));
+ return -errno;
+ }
+ written += ret;
+ }
+
+ ret = linker_add_common(linker, &obj, opts);
+
+ if (obj.fd >= 0)
+ close(obj.fd);
+
+ return ret;
}
static bool is_dwarf_sec_name(const char *name)
@@ -534,7 +638,7 @@ static struct src_sec *add_src_sec(struct src_obj *obj, const char *sec_name)
return sec;
}
-static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
+static int linker_load_obj_file(struct bpf_linker *linker,
const struct bpf_linker_file_opts *opts,
struct src_obj *obj)
{
@@ -554,20 +658,12 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
#error "Unknown __BYTE_ORDER__"
#endif
- pr_debug("linker: adding object file '%s'...\n", filename);
-
- obj->filename = filename;
+ pr_debug("linker: adding object file '%s'...\n", obj->filename);
- obj->fd = open(filename, O_RDONLY | O_CLOEXEC);
- if (obj->fd < 0) {
- err = -errno;
- pr_warn("failed to open file '%s': %s\n", filename, errstr(err));
- return err;
- }
obj->elf = elf_begin(obj->fd, ELF_C_READ_MMAP, NULL);
if (!obj->elf) {
err = -errno;
- pr_warn_elf("failed to parse ELF file '%s'", filename);
+ pr_warn_elf("failed to parse ELF file '%s'", obj->filename);
return err;
}
@@ -575,7 +671,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
ehdr = elf64_getehdr(obj->elf);
if (!ehdr) {
err = -errno;
- pr_warn_elf("failed to get ELF header for %s", filename);
+ pr_warn_elf("failed to get ELF header for %s", obj->filename);
return err;
}
@@ -583,7 +679,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
obj_byteorder = ehdr->e_ident[EI_DATA];
if (obj_byteorder != ELFDATA2LSB && obj_byteorder != ELFDATA2MSB) {
err = -EOPNOTSUPP;
- pr_warn("unknown byte order of ELF file %s\n", filename);
+ pr_warn("unknown byte order of ELF file %s\n", obj->filename);
return err;
}
if (link_byteorder == ELFDATANONE) {
@@ -593,7 +689,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
obj_byteorder == ELFDATA2MSB ? "big" : "little");
} else if (link_byteorder != obj_byteorder) {
err = -EOPNOTSUPP;
- pr_warn("byte order mismatch with ELF file %s\n", filename);
+ pr_warn("byte order mismatch with ELF file %s\n", obj->filename);
return err;
}
@@ -601,13 +697,13 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
|| ehdr->e_machine != EM_BPF
|| ehdr->e_ident[EI_CLASS] != ELFCLASS64) {
err = -EOPNOTSUPP;
- pr_warn_elf("unsupported kind of ELF file %s", filename);
+ pr_warn_elf("unsupported kind of ELF file %s", obj->filename);
return err;
}
if (elf_getshdrstrndx(obj->elf, &obj->shstrs_sec_idx)) {
err = -errno;
- pr_warn_elf("failed to get SHSTRTAB section index for %s", filename);
+ pr_warn_elf("failed to get SHSTRTAB section index for %s", obj->filename);
return err;
}
@@ -620,7 +716,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
if (!shdr) {
err = -errno;
pr_warn_elf("failed to get section #%zu header for %s",
- sec_idx, filename);
+ sec_idx, obj->filename);
return err;
}
@@ -628,7 +724,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
if (!sec_name) {
err = -errno;
pr_warn_elf("failed to get section #%zu name for %s",
- sec_idx, filename);
+ sec_idx, obj->filename);
return err;
}
@@ -636,7 +732,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
if (!data) {
err = -errno;
pr_warn_elf("failed to get section #%zu (%s) data from %s",
- sec_idx, sec_name, filename);
+ sec_idx, sec_name, obj->filename);
return err;
}
@@ -672,7 +768,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
err = libbpf_get_error(obj->btf);
if (err) {
pr_warn("failed to parse .BTF from %s: %s\n",
- filename, errstr(err));
+ obj->filename, errstr(err));
return err;
}
sec->skipped = true;
@@ -683,7 +779,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
err = libbpf_get_error(obj->btf_ext);
if (err) {
pr_warn("failed to parse .BTF.ext from '%s': %s\n",
- filename, errstr(err));
+ obj->filename, errstr(err));
return err;
}
sec->skipped = true;
@@ -700,7 +796,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
break;
default:
pr_warn("unrecognized section #%zu (%s) in %s\n",
- sec_idx, sec_name, filename);
+ sec_idx, sec_name, obj->filename);
err = -EINVAL;
return err;
}
@@ -2634,7 +2730,7 @@ static int linker_append_btf_ext(struct bpf_linker *linker, struct src_obj *obj)
return 0;
}
-int bpf_linker__finalize(struct bpf_linker *linker)
+int linker_finalize_common(struct bpf_linker *linker)
{
struct dst_sec *sec;
size_t strs_sz;
@@ -2693,9 +2789,28 @@ int bpf_linker__finalize(struct bpf_linker *linker)
}
elf_end(linker->elf);
+ linker->elf = NULL;
+
+ /* leave linker->fd for the caller to close if appropriate */
+
+ return 0;
+}
+
+int bpf_linker__finalize(struct bpf_linker *linker)
+{
+ linker_finalize_common(linker);
+
close(linker->fd);
+ linker->fd = -1;
- linker->elf = NULL;
+ return 0;
+}
+
+int bpf_linker__finalize_fd(struct bpf_linker *linker)
+{
+ linker_finalize_common(linker);
+
+ /* linker->fd was opened by the caller, so do not close it here */
linker->fd = -1;
return 0;
--
2.43.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] libbpf: Extend linker API to support in-memory ELF files
2024-11-20 17:02 [PATCH bpf-next] libbpf: Extend linker API to support in-memory ELF files Alastair Robertson
@ 2024-11-21 23:19 ` Andrii Nakryiko
2024-11-22 1:46 ` kernel test robot
2024-11-22 2:17 ` kernel test robot
2 siblings, 0 replies; 4+ messages in thread
From: Andrii Nakryiko @ 2024-11-21 23:19 UTC (permalink / raw)
To: Alastair Robertson; +Cc: bpf, andrii
On Wed, Nov 20, 2024 at 9:02 AM Alastair Robertson <ajor@meta.com> wrote:
>
> The new_fd, add_fd and finalize_fd functions correspond to the original
> new, add_file and finalize functions, but accept an FD instead of a file
> name. This gives API consumers the option of using anonymous
> files/memfds to avoid writing ELFs to disk.
>
> This new API will be useful for performing linking as part of
> bpftrace's JIT compilation.
>
> The add_buf function is a convenience wrapper that does the work of
> creating a memfd for the caller.
>
> Signed-off-by: Alastair Robertson <ajor@meta.com>
> ---
> tools/lib/bpf/libbpf.h | 9 ++
> tools/lib/bpf/libbpf.map | 4 +
> tools/lib/bpf/linker.c | 229 +++++++++++++++++++++++++++++----------
> 3 files changed, 185 insertions(+), 57 deletions(-)
>
Hey Alastair,
I think adding these new APIs makes sense, but I'll nitpick a bit below.
But overall it would be good to extract preparatory stuff like change
to obj->filename everywhere and stuff like this that's pretty
widespread, but very distracting. I'll point it out as I go through
the patch below.
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index b2ce3a72b11d..aae8f954c4fc 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -1796,10 +1796,19 @@ struct bpf_linker_file_opts {
> struct bpf_linker;
>
> LIBBPF_API struct bpf_linker *bpf_linker__new(const char *filename, struct bpf_linker_opts *opts);
> +LIBBPF_API struct bpf_linker *bpf_linker__new_fd(const char *name, int fd,
> + struct bpf_linker_opts *opts);
API nitpick, given `int fd` is the main thing, let's put it as a first argument.
As for the "name" parameter, it's only going to be used for logging
and error reporting, right? I'm more inclined to add it into
bpf_linker_opts as some sort of name override field. It will keep the
API clean, we can always have some default "fd=%d" name, unless the
user provides a more meaningful (for them) name. And for
filename-based APIs we can still use that name as an override (if it's
provided).
> LIBBPF_API int bpf_linker__add_file(struct bpf_linker *linker,
> const char *filename,
> const struct bpf_linker_file_opts *opts);
> +LIBBPF_API int bpf_linker__add_fd(struct bpf_linker *linker,
> + const char *name, int fd,
same nit about fd being first param, and question if we need "name" at all
> + const struct bpf_linker_file_opts *opts);
> +LIBBPF_API int bpf_linker__add_buf(struct bpf_linker *linker, const char *name,
> + void *buffer, int buffer_sz,
> + const struct bpf_linker_file_opts *opts);
ditto, buffer + buffer_sz should come right after linker (and let's
shorten to "buf" and "buf_sz"?)
> LIBBPF_API int bpf_linker__finalize(struct bpf_linker *linker);
> +LIBBPF_API int bpf_linker__finalize_fd(struct bpf_linker *linker);
this I don't think we need, let's see below
> LIBBPF_API void bpf_linker__free(struct bpf_linker *linker);
>
> /*
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 54b6f312cfa8..e767f34c1d08 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -432,4 +432,8 @@ LIBBPF_1.5.0 {
> } LIBBPF_1.4.0;
>
> LIBBPF_1.6.0 {
> + bpf_linker__new_fd;
> + bpf_linker__add_fd;
> + bpf_linker__add_buf;
> + bpf_linker__finalize_fd;
this should be sorted
> } LIBBPF_1.5.0;
> diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
> index cf71d149fe26..6571ed8b858f 100644
> --- a/tools/lib/bpf/linker.c
> +++ b/tools/lib/bpf/linker.c
> @@ -4,6 +4,8 @@
> *
> * Copyright (c) 2021 Facebook
> */
> +#define _GNU_SOURCE
please add #ifndef guard around, we have/had issues with _GNU_SOURCE
being re-defined
> +
> #include <stdbool.h>
> #include <stddef.h>
> #include <stdio.h>
> @@ -16,6 +18,7 @@
> #include <elf.h>
> #include <libelf.h>
> #include <fcntl.h>
> +#include <sys/mman.h>
> #include "libbpf.h"
> #include "btf.h"
> #include "libbpf_internal.h"
> @@ -157,9 +160,9 @@ struct bpf_linker {
> #define pr_warn_elf(fmt, ...) \
> libbpf_print(LIBBPF_WARN, "libbpf: " fmt ": %s\n", ##__VA_ARGS__, elf_errmsg(-1))
>
> -static int init_output_elf(struct bpf_linker *linker, const char *file);
> +static int init_output_elf(struct bpf_linker *linker);
>
> -static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
> +static int linker_load_obj_file(struct bpf_linker *linker,
> const struct bpf_linker_file_opts *opts,
> struct src_obj *obj);
> static int linker_sanity_check_elf(struct src_obj *obj);
> @@ -233,9 +236,56 @@ struct bpf_linker *bpf_linker__new(const char *filename, struct bpf_linker_opts
> if (!linker)
> return errno = ENOMEM, NULL;
>
> - linker->fd = -1;
> + linker->filename = strdup(filename);
> + if (!linker->filename)
> + return errno = ENOMEM, NULL;
> +
> + linker->fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC, 0644);
> + if (linker->fd < 0) {
> + err = -errno;
> + pr_warn("failed to create '%s': %d\n", filename, err);
> + goto err_out;
> + }
> +
> + err = init_output_elf(linker);
> + if (err)
> + goto err_out;
> +
> + return linker;
> +
> +err_out:
> + bpf_linker__free(linker);
> + return errno = -err, NULL;
> +}
> +
> +struct bpf_linker *bpf_linker__new_fd(const char *name, int fd,
> + struct bpf_linker_opts *opts)
> +{
> + struct bpf_linker *linker;
> + int err;
> +
> + if (fd < 0)
> + return errno = EINVAL, NULL;
> +
> + if (!OPTS_VALID(opts, bpf_linker_opts))
> + return errno = EINVAL, NULL;
> +
> + if (elf_version(EV_CURRENT) == EV_NONE) {
> + pr_warn_elf("libelf initialization failed");
> + return errno = EINVAL, NULL;
> + }
> +
> + linker = calloc(1, sizeof(*linker));
> + if (!linker)
> + return errno = ENOMEM, NULL;
> +
> + linker->filename = strdup(name);
> + if (!linker->filename)
> + return errno = ENOMEM, NULL;
> +
> + linker->fd = fd;
>
> - err = init_output_elf(linker, filename);
> + err = init_output_elf(linker);
> if (err)
> goto err_out;
>
> @@ -294,23 +344,12 @@ static Elf64_Sym *add_new_sym(struct bpf_linker *linker, size_t *sym_idx)
> return sym;
> }
>
> -static int init_output_elf(struct bpf_linker *linker, const char *file)
> +static int init_output_elf(struct bpf_linker *linker)
> {
> int err, str_off;
> Elf64_Sym *init_sym;
> struct dst_sec *sec;
>
> - linker->filename = strdup(file);
> - if (!linker->filename)
> - return -ENOMEM;
> -
> - linker->fd = open(file, O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC, 0644);
> - if (linker->fd < 0) {
> - err = -errno;
> - pr_warn("failed to create '%s': %s\n", file, errstr(err));
> - return err;
> - }
> -
> linker->elf = elf_begin(linker->fd, ELF_C_WRITE, NULL);
> if (!linker->elf) {
> pr_warn_elf("failed to create ELF object");
> @@ -436,10 +475,9 @@ static int init_output_elf(struct bpf_linker *linker, const char *file)
> return 0;
> }
>
> -int bpf_linker__add_file(struct bpf_linker *linker, const char *filename,
> - const struct bpf_linker_file_opts *opts)
> +static int linker_add_common(struct bpf_linker *linker, struct src_obj *obj,
> + const struct bpf_linker_file_opts *opts)
it seems like bpf_linker__add_file() should be implemented on top of
bpf_linker__add_fd() + clean up on error, do we need
"linker_add_common"?
> {
> - struct src_obj obj = {};
> int err = 0;
>
> if (!OPTS_VALID(opts, bpf_linker_file_opts))
> @@ -448,25 +486,91 @@ int bpf_linker__add_file(struct bpf_linker *linker, const char *filename,
> if (!linker->elf)
> return libbpf_err(-EINVAL);
>
> - err = err ?: linker_load_obj_file(linker, filename, opts, &obj);
> - err = err ?: linker_append_sec_data(linker, &obj);
> - err = err ?: linker_append_elf_syms(linker, &obj);
> - err = err ?: linker_append_elf_relos(linker, &obj);
> - err = err ?: linker_append_btf(linker, &obj);
> - err = err ?: linker_append_btf_ext(linker, &obj);
> + err = err ?: linker_load_obj_file(linker, opts, obj);
> + err = err ?: linker_append_sec_data(linker, obj);
> + err = err ?: linker_append_elf_syms(linker, obj);
> + err = err ?: linker_append_elf_relos(linker, obj);
> + err = err ?: linker_append_btf(linker, obj);
> + err = err ?: linker_append_btf_ext(linker, obj);
>
> /* free up src_obj resources */
> - free(obj.btf_type_map);
> - btf__free(obj.btf);
> - btf_ext__free(obj.btf_ext);
> - free(obj.secs);
> - free(obj.sym_map);
> - if (obj.elf)
> - elf_end(obj.elf);
> + free(obj->btf_type_map);
> + btf__free(obj->btf);
> + btf_ext__free(obj->btf_ext);
> + free(obj->secs);
> + free(obj->sym_map);
> + if (obj->elf)
> + elf_end(obj->elf);
> + /* leave obj->fd for the caller to clean up if appropriate */
> +
> + return libbpf_err(err);
> +}
> +
> +int bpf_linker__add_file(struct bpf_linker *linker, const char *filename,
> + const struct bpf_linker_file_opts *opts)
> +{
> + struct src_obj obj = {};
> + int ret;
> +
> + obj.filename = filename;
> + obj.fd = open(filename, O_RDONLY | O_CLOEXEC);
> + if (obj.fd < 0) {
> + pr_warn("failed to open file '%s': %s\n", filename, errstr(errno));
> + return -errno;
> + }
> +
> + ret = linker_add_common(linker, &obj, opts);
this can be just bpf_linker__add_fd() call, no?
> +
> if (obj.fd >= 0)
> close(obj.fd);
>
> - return libbpf_err(err);
> + return ret;
> +}
> +
> +int bpf_linker__add_fd(struct bpf_linker *linker, const char *name, int fd,
> + const struct bpf_linker_file_opts *opts)
> +{
> + struct src_obj obj = {};
> +
> + if (fd < 0)
> + return libbpf_err(-EINVAL);
> +
> + obj.filename = name;
> + obj.fd = fd;
> +
> + return linker_add_common(linker, &obj, opts);
> +}
> +
> +int bpf_linker__add_buf(struct bpf_linker *linker, const char *name,
> + void *buffer, int buffer_sz,
> + const struct bpf_linker_file_opts *opts)
> +{
> + struct src_obj obj = {};
> + int written, ret;
> +
> + obj.filename = name;
> + obj.fd = memfd_create(name, 0);
> + if (obj.fd < 0) {
> + pr_warn("failed to create memfd '%s': %s\n", name, errstr(errno));
> + return -errno;
> + }
> +
> + written = 0;
> + while (written < buffer_sz) {
> + ret = write(obj.fd, buffer, buffer_sz);
> + if (ret < 0) {
> + pr_warn("failed to write '%s': %s\n", name, errstr(errno));
> + return -errno;
> + }
> + written += ret;
> + }
> +
> + ret = linker_add_common(linker, &obj, opts);
ditto, bpf_linker__add_fd() should be fine
> +
> + if (obj.fd >= 0)
> + close(obj.fd);
> +
> + return ret;
> }
>
> static bool is_dwarf_sec_name(const char *name)
> @@ -534,7 +638,7 @@ static struct src_sec *add_src_sec(struct src_obj *obj, const char *sec_name)
> return sec;
> }
>
> -static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
> +static int linker_load_obj_file(struct bpf_linker *linker,
> const struct bpf_linker_file_opts *opts,
> struct src_obj *obj)
> {
> @@ -554,20 +658,12 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
> #error "Unknown __BYTE_ORDER__"
> #endif
>
> - pr_debug("linker: adding object file '%s'...\n", filename);
> -
> - obj->filename = filename;
> + pr_debug("linker: adding object file '%s'...\n", obj->filename);
>
so it would be nice to refactor this obj->filename as an initial patch
(and whatever other "cosmetic" changes necessary to add new APIs),
that way we can have more focused patch for new APIs
> - obj->fd = open(filename, O_RDONLY | O_CLOEXEC);
> - if (obj->fd < 0) {
> - err = -errno;
> - pr_warn("failed to open file '%s': %s\n", filename, errstr(err));
> - return err;
> - }
> obj->elf = elf_begin(obj->fd, ELF_C_READ_MMAP, NULL);
> if (!obj->elf) {
> err = -errno;
> - pr_warn_elf("failed to parse ELF file '%s'", filename);
> + pr_warn_elf("failed to parse ELF file '%s'", obj->filename);
> return err;
> }
>
> @@ -575,7 +671,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
> ehdr = elf64_getehdr(obj->elf);
> if (!ehdr) {
> err = -errno;
> - pr_warn_elf("failed to get ELF header for %s", filename);
> + pr_warn_elf("failed to get ELF header for %s", obj->filename);
> return err;
> }
>
> @@ -583,7 +679,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
> obj_byteorder = ehdr->e_ident[EI_DATA];
> if (obj_byteorder != ELFDATA2LSB && obj_byteorder != ELFDATA2MSB) {
> err = -EOPNOTSUPP;
> - pr_warn("unknown byte order of ELF file %s\n", filename);
> + pr_warn("unknown byte order of ELF file %s\n", obj->filename);
> return err;
> }
> if (link_byteorder == ELFDATANONE) {
> @@ -593,7 +689,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
> obj_byteorder == ELFDATA2MSB ? "big" : "little");
> } else if (link_byteorder != obj_byteorder) {
> err = -EOPNOTSUPP;
> - pr_warn("byte order mismatch with ELF file %s\n", filename);
> + pr_warn("byte order mismatch with ELF file %s\n", obj->filename);
> return err;
> }
>
> @@ -601,13 +697,13 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
> || ehdr->e_machine != EM_BPF
> || ehdr->e_ident[EI_CLASS] != ELFCLASS64) {
> err = -EOPNOTSUPP;
> - pr_warn_elf("unsupported kind of ELF file %s", filename);
> + pr_warn_elf("unsupported kind of ELF file %s", obj->filename);
> return err;
> }
>
> if (elf_getshdrstrndx(obj->elf, &obj->shstrs_sec_idx)) {
> err = -errno;
> - pr_warn_elf("failed to get SHSTRTAB section index for %s", filename);
> + pr_warn_elf("failed to get SHSTRTAB section index for %s", obj->filename);
> return err;
> }
>
> @@ -620,7 +716,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
> if (!shdr) {
> err = -errno;
> pr_warn_elf("failed to get section #%zu header for %s",
> - sec_idx, filename);
> + sec_idx, obj->filename);
> return err;
> }
>
> @@ -628,7 +724,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
> if (!sec_name) {
> err = -errno;
> pr_warn_elf("failed to get section #%zu name for %s",
> - sec_idx, filename);
> + sec_idx, obj->filename);
> return err;
> }
>
> @@ -636,7 +732,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
> if (!data) {
> err = -errno;
> pr_warn_elf("failed to get section #%zu (%s) data from %s",
> - sec_idx, sec_name, filename);
> + sec_idx, sec_name, obj->filename);
> return err;
> }
>
> @@ -672,7 +768,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
> err = libbpf_get_error(obj->btf);
> if (err) {
> pr_warn("failed to parse .BTF from %s: %s\n",
> - filename, errstr(err));
> + obj->filename, errstr(err));
> return err;
> }
> sec->skipped = true;
> @@ -683,7 +779,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
> err = libbpf_get_error(obj->btf_ext);
> if (err) {
> pr_warn("failed to parse .BTF.ext from '%s': %s\n",
> - filename, errstr(err));
> + obj->filename, errstr(err));
> return err;
> }
> sec->skipped = true;
> @@ -700,7 +796,7 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
> break;
> default:
> pr_warn("unrecognized section #%zu (%s) in %s\n",
> - sec_idx, sec_name, filename);
> + sec_idx, sec_name, obj->filename);
> err = -EINVAL;
> return err;
> }
as I mentioned all of the above logging changes don't have to be in this patch
> @@ -2634,7 +2730,7 @@ static int linker_append_btf_ext(struct bpf_linker *linker, struct src_obj *obj)
> return 0;
> }
>
> -int bpf_linker__finalize(struct bpf_linker *linker)
> +int linker_finalize_common(struct bpf_linker *linker)
> {
> struct dst_sec *sec;
> size_t strs_sz;
> @@ -2693,9 +2789,28 @@ int bpf_linker__finalize(struct bpf_linker *linker)
> }
>
> elf_end(linker->elf);
> + linker->elf = NULL;
> +
> + /* leave linker->fd for the caller to close if appropriate */
> +
> + return 0;
> +}
> +
> +int bpf_linker__finalize(struct bpf_linker *linker)
> +{
> + linker_finalize_common(linker);
> +
> close(linker->fd);
> + linker->fd = -1;
>
> - linker->elf = NULL;
> + return 0;
> +}
> +
> +int bpf_linker__finalize_fd(struct bpf_linker *linker)
> +{
so as I mentioned, I don't think we need an extra finalize variant. We
can just remember in linker struct whether we own fd or it was just
passed to use, and then existing bpf_linker__finalize() will just
check that and skip close, if necessary
This will also make it harder to mismatch new and finalize APIs.
pw-bot: cr
> + linker_finalize_common(linker);
> +
> + /* linker->fd was opened by the caller, so do not close it here */
> linker->fd = -1;
>
> return 0;
> --
> 2.43.5
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] libbpf: Extend linker API to support in-memory ELF files
2024-11-20 17:02 [PATCH bpf-next] libbpf: Extend linker API to support in-memory ELF files Alastair Robertson
2024-11-21 23:19 ` Andrii Nakryiko
@ 2024-11-22 1:46 ` kernel test robot
2024-11-22 2:17 ` kernel test robot
2 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2024-11-22 1:46 UTC (permalink / raw)
To: Alastair Robertson, bpf, andrii; +Cc: oe-kbuild-all, Alastair Robertson
Hi Alastair,
kernel test robot noticed the following build errors:
[auto build test ERROR on bpf-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/Alastair-Robertson/libbpf-Extend-linker-API-to-support-in-memory-ELF-files/20241121-152231
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/r/20241120170206.2592931-1-ajor%40meta.com
patch subject: [PATCH bpf-next] libbpf: Extend linker API to support in-memory ELF files
config: sparc-randconfig-002-20241122 (https://download.01.org/0day-ci/archive/20241122/202411220938.1TZjtSM3-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241122/202411220938.1TZjtSM3-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411220938.1TZjtSM3-lkp@intel.com/
All errors (new ones prefixed by >>):
>> linker.c:2733:5: error: no previous prototype for 'linker_finalize_common' [-Werror=missing-prototypes]
2733 | int linker_finalize_common(struct bpf_linker *linker)
| ^~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[6]: *** [tools/build/Makefile.build:105: tools/bpf/resolve_btfids/libbpf/staticobjs/linker.o] Error 1
make[6]: *** Waiting for unfinished jobs....
make[5]: *** [Makefile:165: tools/bpf/resolve_btfids/libbpf/staticobjs/libbpf-in.o] Error 2
make[4]: *** [Makefile:63: tools/bpf/resolve_btfids//libbpf/libbpf.a] Error 2
make[3]: *** [Makefile:76: bpf/resolve_btfids] Error 2 shuffle=1440844219
make[2]: *** [Makefile:1370: tools/bpf/resolve_btfids] Error 2 shuffle=1440844219
<stdin>:1519:2: warning: #warning syscall clone3 not implemented [-Wcpp]
make[2]: Target 'prepare' not remade because of errors.
make[1]: *** [Makefile:224: __sub-make] Error 2 shuffle=1440844219
make[1]: Target 'prepare' not remade because of errors.
make: *** [Makefile:224: __sub-make] Error 2 shuffle=1440844219
make: Target 'prepare' not remade because of errors.
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] libbpf: Extend linker API to support in-memory ELF files
2024-11-20 17:02 [PATCH bpf-next] libbpf: Extend linker API to support in-memory ELF files Alastair Robertson
2024-11-21 23:19 ` Andrii Nakryiko
2024-11-22 1:46 ` kernel test robot
@ 2024-11-22 2:17 ` kernel test robot
2 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2024-11-22 2:17 UTC (permalink / raw)
To: Alastair Robertson, bpf, andrii; +Cc: llvm, oe-kbuild-all, Alastair Robertson
Hi Alastair,
kernel test robot noticed the following build errors:
[auto build test ERROR on bpf-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/Alastair-Robertson/libbpf-Extend-linker-API-to-support-in-memory-ELF-files/20241121-152231
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/r/20241120170206.2592931-1-ajor%40meta.com
patch subject: [PATCH bpf-next] libbpf: Extend linker API to support in-memory ELF files
config: i386-buildonly-randconfig-001-20241122 (https://download.01.org/0day-ci/archive/20241122/202411221039.4cgeHanS-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241122/202411221039.4cgeHanS-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411221039.4cgeHanS-lkp@intel.com/
All errors (new ones prefixed by >>):
>> linker.c:2733:5: error: no previous prototype for function 'linker_finalize_common' [-Werror,-Wmissing-prototypes]
2733 | int linker_finalize_common(struct bpf_linker *linker)
| ^
linker.c:2733:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
2733 | int linker_finalize_common(struct bpf_linker *linker)
| ^
| static
1 error generated.
make[6]: *** [tools/build/Makefile.build:105: tools/bpf/resolve_btfids/libbpf/staticobjs/linker.o] Error 1
make[6]: *** Waiting for unfinished jobs....
make[5]: *** [Makefile:165: tools/bpf/resolve_btfids/libbpf/staticobjs/libbpf-in.o] Error 2
make[4]: *** [Makefile:63: tools/bpf/resolve_btfids//libbpf/libbpf.a] Error 2
make[3]: *** [Makefile:76: bpf/resolve_btfids] Error 2 shuffle=3701184676
make[2]: *** [Makefile:1370: tools/bpf/resolve_btfids] Error 2 shuffle=3701184676
In file included from arch/x86/kernel/asm-offsets.c:14:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:21:
In file included from include/linux/mm.h:2213:
include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
518 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
1 warning generated.
make[2]: Target 'prepare' not remade because of errors.
make[1]: *** [Makefile:224: __sub-make] Error 2 shuffle=3701184676
make[1]: Target 'prepare' not remade because of errors.
make: *** [Makefile:224: __sub-make] Error 2 shuffle=3701184676
make: Target 'prepare' not remade because of errors.
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-11-22 2:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-20 17:02 [PATCH bpf-next] libbpf: Extend linker API to support in-memory ELF files Alastair Robertson
2024-11-21 23:19 ` Andrii Nakryiko
2024-11-22 1:46 ` kernel test robot
2024-11-22 2:17 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox