Thread (24 messages) 24 messages, 3 authors, 2022-01-21

Re: [PATCH bpf-next v4 3/8] bpftool: Add gen btf command

flat view

From: Quentin Monnet <hidden>
Date: 2022-01-12 18:10:35
Also in: bpf

2022-01-12 09:27 UTC-0500 ~ Mauricio Vásquez [off-list ref]
This command is implemented under the "gen" command in bpftool and the
syntax is the following:

$ bpftool gen btf INPUT OUTPUT OBJECT(S)
Thanks a lot for this work!

Please update the relevant manual page under Documentation, to let users
know how to use the feature. You may also consider adding an example at
the end of that document.

The bash completion script should also be updated with the new "btf"
subcommand for "gen". Given that all the arguments are directories and
files, it should not be hard.

Have you considered adding tests for the feature? There are a few
bpftool-related selftests under tools/testing/selftests/bpf/.
quoted hunk ↗ jump to hunk
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/gen.c | 117 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 117 insertions(+)
diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 43e3f8700ecc..cdeb1047d79d 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 btf INPUT OUTPUT OBJECT(S)\n"
 		"       %1$s %2$s help\n"
 		"\n"
 		"       " HELP_SPEC_OPTIONS " |\n"
@@ -1094,9 +1096,124 @@ 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 is_file(const char *path)
+{
+	struct stat st;
+
+	if (stat(path, &st) < 0)
+		return -1;
+
+	switch (st.st_mode & S_IFMT) {
+	case S_IFDIR:
+		return 0;
+	case S_IFREG:
+		return 1;
+	default:
+		return -1;
+	}
+}
+
+static int do_gen_btf(int argc, char **argv)
+{
+	char src_btf_path[PATH_MAX], dst_btf_path[PATH_MAX];
+	bool input_is_file, output_is_file = false;
+	const char *input, *output;
+	const char **objs = NULL;
+	struct dirent *dir;
+	DIR *d = NULL;
+	int i, err;
+
+	if (!REQ_ARGS(3)) {
+		usage();
+		return -1;
+	}
+
+	input = GET_ARG();
+	err = is_file(input);
+	if (err < 0) {
+		p_err("failed to stat %s: %s", input, strerror(errno));
+		return err;
+	}
+	input_is_file = err;
+
+	output = GET_ARG();
+	err = is_file(output);
+	if (err != 0)
+		output_is_file = true;
Why not return if err < 0? This will set output_is_file to true and will
fail later, I think?
quoted hunk ↗ jump to hunk
+
+	objs = (const char **) malloc((argc + 1) * sizeof(*objs));
+	if (!objs)
+		return -ENOMEM;
Let's p_err() a message.
quoted hunk ↗ jump to hunk
+
+	i = 0;
+	while (argc > 0)
+		objs[i++] = GET_ARG();
+
+	objs[i] = NULL;
+
+	/* single BTF file */
+	if (input_is_file) {
+		printf("SBTF: %s\n", input);
We can use "p_info()" instead of "printf()". In particular, this avoids
printing the message when the JSON output is required.
quoted hunk ↗ jump to hunk
+
+		if (output_is_file) {
+			err = btfgen(input, output, objs);
+			goto out;
+		}
+		snprintf(dst_btf_path, sizeof(dst_btf_path), "%s/%s", output,
+			 basename(input));
Am I right that the output file should be just a file name, and not a
path, and that it will be created under the same directory as the input
file? And that providing a relative or absolute path instead will cause
issues here? If so, please document it. It would be nice to be able to
support paths too, but I'm not sure how much work that represents.
quoted hunk ↗ jump to hunk
+		err = btfgen(input, dst_btf_path, objs);
+		goto out;
+	}
+
+	if (output_is_file) {
+		p_err("can't have just one file as output");
See comment above, this message is misleading if stat() returned with an
error.
quoted hunk ↗ jump to hunk
+		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;
+
+		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);
+
+		printf("SBTF: %s\n", src_btf_path);
+
+		err = btfgen(src_btf_path, dst_btf_path, objs);
+		if (err)
+			goto out;
+	}
+
+out:
+	if (!err)
+		printf("STAT: done!\n");
p_info()
+	free(objs);
+	closedir(d);
If input is a single file, "d" will not be a valid directory stream
descriptor, and I expect closedir() to fail and set errno even if
everything else went fine. The return code does not matter, but "errno
!= 0" may cause bpftool's batch mode to error out. Can you please run
closedir() only "if (d)"?
quoted hunk ↗ jump to hunk
+	return err;
+}
+
 static const struct cmd cmds[] = {
 	{ "object",	do_object },
 	{ "skeleton",	do_skeleton },
+	{ "btf",	do_gen_btf},
 	{ "help",	do_help },
 	{ 0 }
 };
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help