| Message ID | 20210423002646.35043-10-alexei.starovoitov@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: syscall program, FD array, loader program, light skeleton. | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | fail | Series longer than 15 patches |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 6 maintainers not CCed: yhs@fb.com kpsingh@kernel.org kafai@fb.com ast@kernel.org john.fastabend@gmail.com songliubraving@fb.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: line length of 83 exceeds 80 columns WARNING: line length of 89 exceeds 80 columns |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On Thu, Apr 22, 2021 at 5:27 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> 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; > +} > + [...] > @@ -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 > + } > + } > + > 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. > 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 >
On Mon, Apr 26, 2021 at 10:14:45AM -0700, Andrii Nakryiko wrote: > On Thu, Apr 22, 2021 at 5:27 PM Alexei Starovoitov > <alexei.starovoitov@gmail.com> 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. It will fail on all kernels. That probe_fd was a left over of earlier detection approach where it would proceed to load all the way, but then I switched to: > > + return errno == EPROTO; since such style of probing is much cheaper for the kernel and user space. But point taken. Will open code it. > > +} > > + > > [...] > > > @@ -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 > > > + } > > + } > > + > > 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. hmm. there is free on success below. > 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. hmm. that sounds worse. why add another 8 byte to bpf_object that won't be used until this last step of bpf_object__load_progs. And only for the duration of this loading. It's cheaper to have this alloc here with two free()s below. > > > 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 > > --
On Mon, Apr 26, 2021 at 7:53 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Mon, Apr 26, 2021 at 10:14:45AM -0700, Andrii Nakryiko wrote: > > On Thu, Apr 22, 2021 at 5:27 PM Alexei Starovoitov > > <alexei.starovoitov@gmail.com> 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(-) > > > > > [...] > > > 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. > > hmm. there is free on success below. right, my bad, I somehow understood as if it was only for error case > > > 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. > > hmm. that sounds worse. > why add another 8 byte to bpf_object that won't be used > until this last step of bpf_object__load_progs. > And only for the duration of this loading. > It's cheaper to have this alloc here with two free()s below. So if you care about extra 8 bytes, then it's even more efficient to have just one obj->fd_array rather than N prog->fd_array, no? And it's also not very clean that prog->fd_array will have a dangling pointer to deallocated memory after bpf_object__load_progs(). But that brings the entire question of why use fd_array at all here? Commit description doesn't explain why libbpf has to use fd_array and why it should be preferred. What are the advantages justifying added complexity and extra memory allocation/clean up? It also reduces test coverage of the "old ways" that offer the same capabilities. I think this should be part of the commit description, if we agree that fd_array has to be used outside of the auto-generated loader program. > > > > > > 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 > > > > > --
On Tue, Apr 27, 2021 at 09:36:54AM -0700, Andrii Nakryiko wrote: > On Mon, Apr 26, 2021 at 7:53 PM Alexei Starovoitov > <alexei.starovoitov@gmail.com> wrote: > > > > On Mon, Apr 26, 2021 at 10:14:45AM -0700, Andrii Nakryiko wrote: > > > On Thu, Apr 22, 2021 at 5:27 PM Alexei Starovoitov > > > <alexei.starovoitov@gmail.com> 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(-) > > > > > > > > > [...] > > > > > 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. > > > > hmm. there is free on success below. > > right, my bad, I somehow understood as if it was only for error case > > > > > > 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. > > > > hmm. that sounds worse. > > why add another 8 byte to bpf_object that won't be used > > until this last step of bpf_object__load_progs. > > And only for the duration of this loading. > > It's cheaper to have this alloc here with two free()s below. > > So if you care about extra 8 bytes, then it's even more efficient to > have just one obj->fd_array rather than N prog->fd_array, no? I think it's layer breaking when bpf_program__load()->load_program() has to reach out to prog->obj to do its work. The layers are already a mess due to: &prog->obj->maps[prog->obj->rodata_map_idx] I wanted to avoid making it uglier. > And it's > also not very clean that prog->fd_array will have a dangling pointer > to deallocated memory after bpf_object__load_progs(). prog->reloc_desc is free and zeroed after __relocate. prog->insns are freed and _not_ zereod after __load_progs. so prog->fd_array won't be the first such pointer. I can add zeroing, of course. > > But that brings the entire question of why use fd_array at all here? > Commit description doesn't explain why libbpf has to use fd_array and > why it should be preferred. What are the advantages justifying added > complexity and extra memory allocation/clean up? It also reduces test > coverage of the "old ways" that offer the same capabilities. I think > this should be part of the commit description, if we agree that > fd_array has to be used outside of the auto-generated loader program. I can add a knob to it to use it during loader gen for the loader gen and for the runner of the loader prog. I think it will add more complexity. The bpf CI runs on older kernels, so the test coverage of "old ways" is not reduced regardless. From the kernel pov BPF_PSEUDO_MAP_FD vs BPF_PSEUDO_MAP_IDX there is no advantage. From the libbpf side patch 9 looked trivial enough _not_ do it conditionally, but whatever. I don't mind more 'if'-s.
On Tue, Apr 27, 2021 at 6:32 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Tue, Apr 27, 2021 at 09:36:54AM -0700, Andrii Nakryiko wrote: > > On Mon, Apr 26, 2021 at 7:53 PM Alexei Starovoitov > > <alexei.starovoitov@gmail.com> wrote: > > > > > > On Mon, Apr 26, 2021 at 10:14:45AM -0700, Andrii Nakryiko wrote: > > > > On Thu, Apr 22, 2021 at 5:27 PM Alexei Starovoitov > > > > <alexei.starovoitov@gmail.com> 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(-) > > > > > > > > > > > > > [...] > > > > > > > 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. > > > > > > hmm. there is free on success below. > > > > right, my bad, I somehow understood as if it was only for error case > > > > > > > > > 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. > > > > > > hmm. that sounds worse. > > > why add another 8 byte to bpf_object that won't be used > > > until this last step of bpf_object__load_progs. > > > And only for the duration of this loading. > > > It's cheaper to have this alloc here with two free()s below. > > > > So if you care about extra 8 bytes, then it's even more efficient to > > have just one obj->fd_array rather than N prog->fd_array, no? > > I think it's layer breaking when bpf_program__load()->load_program() > has to reach out to prog->obj to do its work. > The layers are already a mess due to: > &prog->obj->maps[prog->obj->rodata_map_idx] > I wanted to avoid making it uglier. I don't think it's breaking any layer. bpf_program is not an independent entity from libbpf's point of view, it always belongs to bpf_object. And there are bpf_object-scoped properties, shared across all progs, like BTF, global variables, maps, license, etc. It's another thing that bpf_program__load() just shouldn't be a public API, and we are going to address that in libbpf 1.0. > > > And it's > > also not very clean that prog->fd_array will have a dangling pointer > > to deallocated memory after bpf_object__load_progs(). > > prog->reloc_desc is free and zeroed after __relocate. > prog->insns are freed and _not_ zereod after __load_progs. > so prog->fd_array won't be the first such pointer. > I can add zeroing, of course. cool, it would be great to fix prog->insns to be zeroed out as well > > > > > But that brings the entire question of why use fd_array at all here? > > Commit description doesn't explain why libbpf has to use fd_array and > > why it should be preferred. What are the advantages justifying added > > complexity and extra memory allocation/clean up? It also reduces test > > coverage of the "old ways" that offer the same capabilities. I think > > this should be part of the commit description, if we agree that > > fd_array has to be used outside of the auto-generated loader program. > > I can add a knob to it to use it during loader gen for the loader gen > and for the runner of the loader prog. So that's why I'm saying a better commit description is necessary. I lost track, again, that those patched instructions with embedded map_idx are assumed by prog loader program and then only fd_array is modified in runtime by BPF loader program. Please, don't skim on commit description, there are many moving pieces that are obvious only in hindsight. Getting back to code, given it's necessary for gen_loader only, I'd switch out all those `kernel_supports(FEAT_FD_IDX)` checks with `obj->gen_loader` and leave the current behavior as is. And we also won't need to do FEAT_FD_IDX feature probing and extra memory allocation at all. And bpf_load_and_run() uses fd_array unconditionally without feature probing anyways. > I think it will add more complexity. > The bpf CI runs on older kernels, so the test coverage of "old ways" > is not reduced regardless. I'm the only one who checks that, and we keep shrinking the set of tests that run on older kernels because we update existing ones with dependencies on newer kernel features. So coverage is shrinking, but basic stuff is still tested, of course. > From the kernel pov BPF_PSEUDO_MAP_FD vs BPF_PSEUDO_MAP_IDX there is > no advantage. > From the libbpf side patch 9 looked trivial enough _not_ do it conditionally, > but whatever. I don't mind more 'if'-s. I do mind unnecessary ifs, that's not what I was proposing.
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c index bba48ff4c5c0..b96a3aba6fcc 100644 --- a/tools/lib/bpf/bpf.c +++ b/tools/lib/bpf/bpf.c @@ -252,6 +252,7 @@ int libbpf__bpf_prog_load(const struct bpf_prog_load_params *load_attr) attr.prog_btf_fd = load_attr->prog_btf_fd; attr.prog_flags = load_attr->prog_flags; + attr.fd_array = ptr_to_u64(load_attr->fd_array); attr.func_info_rec_size = load_attr->func_info_rec_size; attr.func_info_cnt = load_attr->func_info_cnt; diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 254a0c9aa6cf..17cfc5b66111 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -176,6 +176,8 @@ enum kern_feature_id { FEAT_MODULE_BTF, /* BTF_KIND_FLOAT support */ FEAT_BTF_FLOAT, + /* Kernel support for FD_IDX */ + FEAT_FD_IDX, __FEAT_CNT, }; @@ -288,6 +290,7 @@ struct bpf_program { __u32 line_info_rec_size; __u32 line_info_cnt; __u32 prog_flags; + int *fd_array; }; struct bpf_struct_ops { @@ -4181,6 +4184,24 @@ static int probe_module_btf(void) return !err; } +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)); + return errno == EPROTO; +} + enum kern_feature_result { FEAT_UNKNOWN = 0, FEAT_SUPPORTED = 1, @@ -4231,6 +4252,9 @@ static struct kern_feature_desc { [FEAT_BTF_FLOAT] = { "BTF_KIND_FLOAT support", probe_kern_btf_float, }, + [FEAT_FD_IDX] = { + "prog_load with fd_idx", probe_kern_fd_idx, + }, }; static bool kernel_supports(enum kern_feature_id feat_id) @@ -6309,19 +6333,34 @@ bpf_object__relocate_data(struct bpf_object *obj, struct bpf_program *prog) switch (relo->type) { case RELO_LD64: - insn[0].src_reg = BPF_PSEUDO_MAP_FD; - insn[0].imm = obj->maps[relo->map_idx].fd; + if (kernel_supports(FEAT_FD_IDX)) { + insn[0].src_reg = BPF_PSEUDO_MAP_IDX; + insn[0].imm = relo->map_idx; + } else { + insn[0].src_reg = BPF_PSEUDO_MAP_FD; + insn[0].imm = obj->maps[relo->map_idx].fd; + } break; case RELO_DATA: - insn[0].src_reg = BPF_PSEUDO_MAP_VALUE; insn[1].imm = insn[0].imm + relo->sym_off; - insn[0].imm = obj->maps[relo->map_idx].fd; + if (kernel_supports(FEAT_FD_IDX)) { + insn[0].src_reg = BPF_PSEUDO_MAP_IDX_VALUE; + insn[0].imm = relo->map_idx; + } else { + insn[0].src_reg = BPF_PSEUDO_MAP_VALUE; + insn[0].imm = obj->maps[relo->map_idx].fd; + } break; case RELO_EXTERN_VAR: ext = &obj->externs[relo->sym_off]; if (ext->type == EXT_KCFG) { - insn[0].src_reg = BPF_PSEUDO_MAP_VALUE; - insn[0].imm = obj->maps[obj->kconfig_map_idx].fd; + if (kernel_supports(FEAT_FD_IDX)) { + insn[0].src_reg = BPF_PSEUDO_MAP_IDX_VALUE; + insn[0].imm = obj->kconfig_map_idx; + } else { + insn[0].src_reg = BPF_PSEUDO_MAP_VALUE; + insn[0].imm = obj->maps[obj->kconfig_map_idx].fd; + } insn[1].imm = ext->kcfg.data_off; } else /* EXT_KSYM */ { if (ext->ksym.type_id) { /* typed ksyms */ @@ -7047,6 +7086,7 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt, load_attr.attach_btf_id = prog->attach_btf_id; load_attr.kern_version = kern_version; load_attr.prog_ifindex = prog->prog_ifindex; + load_attr.fd_array = prog->fd_array; /* specify func_info/line_info only if kernel supports them */ btf_fd = bpf_object__btf_fd(prog->obj); @@ -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; + } + } + 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; 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);