Re: [PATCH bpf-next 07/10] bpftool: add `gen bpfo` command to perform BPF static linking
flat view
From: Quentin Monnet <hidden>
Date: 2021-03-11 11:32:20
Also in:
bpf
2021-03-09 20:04 UTC-0800 ~ Andrii Nakryiko [off-list ref]
quoted hunk ↗ jump to hunk
Add `bpftool gen bpfo <output-file> <input_file>...` command to statically link multiple BPF object files into a single output BPF object file. Similarly to existing '*.o' convention, bpftool is establishing a '*.bpfo' convention for statically-linked BPF object files. Both .o and .bpfo suffixes will be stripped out during BPF skeleton generation to infer BPF object name. Signed-off-by: Andrii Nakryiko <andrii@kernel.org> --- tools/bpf/bpftool/gen.c | 46 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-)diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c index 4033c46d83e7..8b1ed6c0a62f 100644 --- a/tools/bpf/bpftool/gen.c +++ b/tools/bpf/bpftool/gen.c +static int do_bpfo(int argc, char **argv)
quoted hunk ↗ jump to hunk
+{ + struct bpf_linker *linker; + const char *output_file, *file; + int err; + + if (!REQ_ARGS(2)) { + usage(); + return -1; + } + + output_file = GET_ARG(); + + linker = bpf_linker__new(output_file, NULL); + if (!linker) { + p_err("failed to create BPF linker instance"); + return -1; + } + + while (argc) { + file = GET_ARG(); + + err = bpf_linker__add_file(linker, file); + if (err) { + p_err("failed to link '%s': %d", file, err);
I think you mentioned before that your preference was for having just the error code instead of using strerror(), but I think it would be more user-friendly for the majority of users who don't know the error codes if we had something more verbose? How about having both strerror() output and the error code?
quoted hunk ↗ jump to hunk
+ goto err_out; + } + } + + err = bpf_linker__finalize(linker); + if (err) { + p_err("failed to finalize ELF file: %d", err); + goto err_out; + } + + return 0; +err_out: + bpf_linker__free(linker); + return -1;
Should you call bpf_linker__free() even on success? I see that bpf_linker__finalize() frees some of the resources, but it seems that bpf_linker__free() does a more thorough job?
quoted hunk ↗ jump to hunk
+} + static int do_help(int argc, char **argv) { if (json_output) {@@ -611,6 +654,7 @@ static int do_help(int argc, char **argv) static const struct cmd cmds[] = { { "skeleton", do_skeleton }, + { "bpfo", do_bpfo }, { "help", do_help }, { 0 } };
Please update the usage help message, man page, and bash completion, thanks. Especially because what "bpftool gen bpfo" does is not intuitive (but I don't have a better name suggestion at the moment). Great work! Quentin