Thread (45 messages) 45 messages, 4 authors, 2022-02-04

Re: [PATCH bpf-next v5 2/9] bpftool: Add gen min_core_btf command

flat view

From: Andrii Nakryiko <hidden>
Date: 2022-02-02 17:58:31
Also in: bpf

On Fri, Jan 28, 2022 at 2:33 PM Mauricio Vásquez [off-list ref] wrote:
quoted hunk ↗ jump to hunk
This command is implemented under the "gen" command in bpftool and the
syntax is the following:

$ bpftool gen min_core_btf INPUT OUTPUT OBJECT(S)

INPUT can be either a single BTF file or a folder containing BTF files,
when it's a folder, a BTF file is generated for each BTF file contained
in this folder. OUTPUT is the file (or folder) where generated files are
stored and OBJECT(S) is the list of bpf objects we want to generate the
BTF file(s) for (each generated BTF file contains all the types needed
by all the objects).

Signed-off-by: Mauricio Vásquez <redacted>
Signed-off-by: Rafael David Tinoco <redacted>
Signed-off-by: Lorenzo Fontana <redacted>
Signed-off-by: Leonardo Di Donato <redacted>
---
 tools/bpf/bpftool/bash-completion/bpftool |   6 +-
 tools/bpf/bpftool/gen.c                   | 112 +++++++++++++++++++++-
 2 files changed, 114 insertions(+), 4 deletions(-)
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 493753a4962e..958e1fd71b5c 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -1003,9 +1003,13 @@ _bpftool()
                             ;;
                     esac
                     ;;
+                min_core_btf)
+                    _filedir
+                    return 0
+                    ;;
                 *)
                     [[ $prev == $object ]] && \
-                        COMPREPLY=( $( compgen -W 'object skeleton help' -- "$cur" ) )
+                        COMPREPLY=( $( compgen -W 'object skeleton help min_core_btf' -- "$cur" ) )
                     ;;
             esac
             ;;
diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 8f78c27d41f0..7db31b0f265f 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -5,6 +5,7 @@
 #define _GNU_SOURCE
 #endif
 #include <ctype.h>
+#include <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <linux/err.h>
@@ -1084,6 +1085,7 @@ static int do_help(int argc, char **argv)
        fprintf(stderr,
                "Usage: %1$s %2$s object OUTPUT_FILE INPUT_FILE [INPUT_FILE...]\n"
                "       %1$s %2$s skeleton FILE [name OBJECT_NAME]\n"
+               "       %1$s %2$s min_core_btf INPUT OUTPUT OBJECT(S)\n"
OBJECTS(S) should be OBJECT... for this "CLI notation", no?
quoted hunk ↗ jump to hunk
                "       %1$s %2$s help\n"
                "\n"
                "       " HELP_SPEC_OPTIONS " |\n"
@@ -1094,10 +1096,114 @@ static int do_help(int argc, char **argv)
        return 0;
 }

+/* Create BTF file for a set of BPF objects */
+static int btfgen(const char *src_btf, const char *dst_btf, const char *objspaths[])
+{
+       return -EOPNOTSUPP;
+}
+
+static int do_min_core_btf(int argc, char **argv)
+{
+       char src_btf_path[PATH_MAX], dst_btf_path[PATH_MAX];
+       bool input_is_file, output_is_file = true;
+       const char *input, *output;
+       const char **objs = NULL;
+       struct dirent *dir;
+       struct stat st;
+       DIR *d = NULL;
+       int i, err;
+
+       if (!REQ_ARGS(3)) {
+               usage();
+               return -1;
+       }
+
+       input = GET_ARG();
+       if (stat(input, &st) < 0) {
+               p_err("failed to stat %s: %s", input, strerror(errno));
+               return -errno;
+       }
+
+       if ((st.st_mode & S_IFMT) != S_IFDIR && (st.st_mode & S_IFMT) != S_IFREG) {
+               p_err("file type not valid: %s", input);
+               return -EINVAL;
+       }
+
+       input_is_file = (st.st_mode & S_IFMT) == S_IFREG;
move before if and use input_is_file in the if itself instead of
duplicating all the S_IFREG flags?
quoted hunk ↗ jump to hunk
+
+       output = GET_ARG();
+       if (stat(output, &st) == 0 && (st.st_mode & S_IFMT) == S_IFDIR)
+               output_is_file = false;
if stat() succeeds but it's neither directory or file, should be an
error, right?
+
+       objs = (const char **) malloc((argc + 1) * sizeof(*objs));
calloc() seems to be better suited for this (and zero-intialization is
nice for safety and to avoid objs[argc] = NULL after the loop below)
quoted hunk ↗ jump to hunk
+       if (!objs) {
+               p_err("failed to allocate array for object names");
+               return -ENOMEM;
+       }
+
+       i = 0;
+       while (argc > 0)
+               objs[i++] = GET_ARG();
for (i = 0; i < argc; i++) ?
quoted hunk ↗ jump to hunk
+
+       objs[i] = NULL;
+
+       /* single BTF file */
+       if (input_is_file) {
+               p_info("Processing source BTF file: %s", input);
+
+               if (output_is_file) {
+                       err = btfgen(input, output, objs);
+                       goto out;
+               }
+               snprintf(dst_btf_path, sizeof(dst_btf_path), "%s/%s", output,
+                        basename(input));
+               err = btfgen(input, dst_btf_path, objs);
+               goto out;
+       }
+
+       if (output_is_file) {
+               p_err("can't have just one file as output");
+               err = -EINVAL;
+               goto out;
+       }
+
+       /* directory with BTF files */
+       d = opendir(input);
+       if (!d) {
+               p_err("error opening input dir: %s", strerror(errno));
+               err = -errno;
+               goto out;
+       }
+
+       while ((dir = readdir(d)) != NULL) {
+               if (dir->d_type != DT_REG)
+                       continue;
+
+               if (strncmp(dir->d_name + strlen(dir->d_name) - 4, ".btf", 4))
+                       continue;
this whole handling of input directory feels a bit icky, tbh... maybe
we should require explicit listing of input files always. In CLI
invocation those could be separated by "keywords", something like
this:

bpftool gen min_core_btf <output> inputs <file1> <file2> .... objects
<obj1> <obj2> ...

a bit of a downside is that you can't have a file named "inputs" or
"objects", but that seems extremely unlikely? Quentin, any opinion as
well?

I'm mainly off put by a bit random ".btf" naming convention, the
DT_REG skipping, etc.

Another cleaner alternative from POV of bpftool (but might be less
convenient for users) is to use @file convention to specify a file
that contains a list of files. So

bpftool gen min_core_btf <output> @btf_filelist.txt @obj_filelist.txt

would take lists of inputs and outputs from respective files?


But actually, let's take a step back again. Why should there be
multiple inputs and outputs? I can see why multiple objects are
mandatory (you have an application that has multiple BPF objects used
internally). But processing single vmlinux BTF at a time seems
absolutely fine. I don't buy that CO-RE relo processing is that slow
to require optimized batch processing.

I might have asked this before, sorry, but the duration between each
iteration of btfgen is pretty long and I'm losing the context.
+
+               snprintf(src_btf_path, sizeof(src_btf_path), "%s%s", input, dir->d_name);
+               snprintf(dst_btf_path, sizeof(dst_btf_path), "%s%s", output, dir->d_name);
+
+               p_info("Processing source BTF file: %s", src_btf_path);
+
+               err = btfgen(src_btf_path, dst_btf_path, objs);
+               if (err)
+                       goto out;
+       }
+
+out:
+       free(objs);
+       if (d)
+               closedir(d);
+       return err;
+}
+
 static const struct cmd cmds[] = {
-       { "object",     do_object },
-       { "skeleton",   do_skeleton },
-       { "help",       do_help },
+       { "object",             do_object },
+       { "skeleton",           do_skeleton },
+       { "min_core_btf",       do_min_core_btf},
+       { "help",               do_help },
        { 0 }
 };

--
2.25.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help