Re: [PATCH v2 bpf-next 09/16] libbpf: Support for fd_idx
From: Andrii Nakryiko <hidden>
Date: 2021-04-26 17:15:04
Also in:
bpf
On Thu, Apr 22, 2021 at 5:27 PM Alexei Starovoitov [off-list ref] wrote:
From: Alexei Starovoitov <ast@kernel.org> Add support for FD_IDX make libbpf prefer that approach to loading programs. Signed-off-by: Alexei Starovoitov <ast@kernel.org> --- tools/lib/bpf/bpf.c | 1 + tools/lib/bpf/libbpf.c | 70 +++++++++++++++++++++++++++++---- tools/lib/bpf/libbpf_internal.h | 1 + 3 files changed, 65 insertions(+), 7 deletions(-)
[...]
+static int probe_kern_fd_idx(void)
+{
+ struct bpf_load_program_attr attr;
+ struct bpf_insn insns[] = {
+ BPF_LD_IMM64_RAW(BPF_REG_0, BPF_PSEUDO_MAP_IDX, 0),
+ BPF_EXIT_INSN(),
+ };
+
+ memset(&attr, 0, sizeof(attr));
+ attr.prog_type = BPF_PROG_TYPE_SOCKET_FILTER;
+ attr.insns = insns;
+ attr.insns_cnt = ARRAY_SIZE(insns);
+ attr.license = "GPL";
+
+ probe_fd(bpf_load_program_xattr(&attr, NULL, 0));probe_fd() calls close(fd) internally, which technically can interfere with errno, though close() shouldn't be called because syscall has to fail on correct kernels... So this should work, but I feel like open-coding that logic is better than ignoring probe_fd() result.
+ return errno == EPROTO; +} +
[...]
quoted hunk ↗ jump to hunk
@@ -7239,6 +7279,8 @@ bpf_object__load_progs(struct bpf_object *obj, int log_level) struct bpf_program *prog; size_t i; int err; + struct bpf_map *map; + int *fd_array = NULL; for (i = 0; i < obj->nr_programs; i++) { prog = &obj->programs[i];@@ -7247,6 +7289,16 @@ bpf_object__load_progs(struct bpf_object *obj, int log_level) return err; } + if (kernel_supports(FEAT_FD_IDX) && obj->nr_maps) { + fd_array = malloc(sizeof(int) * obj->nr_maps); + if (!fd_array) + return -ENOMEM; + for (i = 0; i < obj->nr_maps; i++) { + map = &obj->maps[i]; + fd_array[i] = map->fd;
nit: obj->maps[i].fd will keep it a single line
quoted hunk ↗ jump to hunk
+ } + } + for (i = 0; i < obj->nr_programs; i++) { prog = &obj->programs[i]; if (prog_is_subprog(obj, prog))@@ -7256,10 +7308,14 @@ bpf_object__load_progs(struct bpf_object *obj, int log_level) continue; } prog->log_level |= log_level; + prog->fd_array = fd_array;
you are not freeing this memory on success, as far as I can see. And given multiple programs are sharing fd_array, it's a bit problematic for prog to have fd_array. This is per-object properly, so let's add it at bpf_object level and clean it up on bpf_object__close()? And by assigning to obj->fd_array at malloc() site, you won't need to do all the error-handling free()s below.
quoted hunk ↗ jump to hunk
err = bpf_program__load(prog, obj->license, obj->kern_version); - if (err) + if (err) { + free(fd_array); return err; + } } + free(fd_array); return 0; }diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h index 6017902c687e..9114c7085f2a 100644 --- a/tools/lib/bpf/libbpf_internal.h +++ b/tools/lib/bpf/libbpf_internal.h@@ -204,6 +204,7 @@ struct bpf_prog_load_params { __u32 log_level; char *log_buf; size_t log_buf_sz; + int *fd_array; }; int libbpf__bpf_prog_load(const struct bpf_prog_load_params *load_attr); --2.30.2