On Tue, 23 Nov 2021, Andrii Nakryiko wrote: > Load global data maps lazily, if kernel is too old to support global > data. Make sure that programs are still correct by detecting if any of > the to-be-loaded programs have relocation against any of such maps. > > This allows to solve the issue ([0]) with bpf_printk() and Clang > generating unnecessary and unreferenced .rodata.strX.Y sections, but it > also goes further along the CO-RE lines, allowing to have a BPF object > in which some code can work on very old kernels and relies only on BPF > maps explicitly, while other BPF programs might enjoy global variable > support. If such programs are correctly set to not load at runtime on > old kernels, bpf_object will load and function correctly now. > > [0] https://lore.kernel.org/bpf/CAK-59YFPU3qO+_pXWOH+c1LSA=8WA1yabJZfREjOEXNHAqgXNg@xxxxxxxxxxxxxx/ > > Fixes: aed659170a31 ("libbpf: Support multiple .rodata.* and .data.* BPF maps") > Acked-by: Song Liu <songliubraving@xxxxxx> > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx> Thanks for fixing this! I ran into it today on a 4.14 kernel and verified that with the patch applied to latest libbpf, BPF objects with legacy maps loaded and ran where previously loading failed. Tested-by: Alan Maguire <alan.maguire@xxxxxxxxxx> > --- > tools/lib/bpf/libbpf.c | 34 ++++++++++++++++++++++++++++++---- > 1 file changed, 30 insertions(+), 4 deletions(-) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index af405c38aadc..27695bf31250 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -5006,6 +5006,24 @@ bpf_object__create_maps(struct bpf_object *obj) > for (i = 0; i < obj->nr_maps; i++) { > map = &obj->maps[i]; > > + /* To support old kernels, we skip creating global data maps > + * (.rodata, .data, .kconfig, etc); later on, during program > + * loading, if we detect that at least one of the to-be-loaded > + * programs is referencing any global data map, we'll error > + * out with program name and relocation index logged. > + * This approach allows to accommodate Clang emitting > + * unnecessary .rodata.str1.1 sections for string literals, > + * but also it allows to have CO-RE applications that use > + * global variables in some of BPF programs, but not others. > + * If those global variable-using programs are not loaded at > + * runtime due to bpf_program__set_autoload(prog, false), > + * bpf_object loading will succeed just fine even on old > + * kernels. > + */ > + if (bpf_map__is_internal(map) && > + !kernel_supports(obj, FEAT_GLOBAL_DATA)) > + continue; > + > retried = false; > retry: > if (map->pin_path) { > @@ -5605,6 +5623,14 @@ bpf_object__relocate_data(struct bpf_object *obj, struct bpf_program *prog) > insn[0].src_reg = BPF_PSEUDO_MAP_IDX_VALUE; > insn[0].imm = relo->map_idx; > } else { > + const struct bpf_map *map = &obj->maps[relo->map_idx]; > + > + if (bpf_map__is_internal(map) && > + !kernel_supports(obj, FEAT_GLOBAL_DATA)) { > + pr_warn("prog '%s': relo #%d: kernel doesn't support global data\n", > + prog->name, i); > + return -ENOTSUP; > + } > insn[0].src_reg = BPF_PSEUDO_MAP_VALUE; > insn[0].imm = obj->maps[relo->map_idx].fd; > } > @@ -6139,6 +6165,8 @@ bpf_object__relocate(struct bpf_object *obj, const char *targ_btf_path) > */ > if (prog_is_subprog(obj, prog)) > continue; > + if (!prog->load) > + continue; > > err = bpf_object__relocate_calls(obj, prog); > if (err) { > @@ -6152,6 +6180,8 @@ bpf_object__relocate(struct bpf_object *obj, const char *targ_btf_path) > prog = &obj->programs[i]; > if (prog_is_subprog(obj, prog)) > continue; > + if (!prog->load) > + continue; > err = bpf_object__relocate_data(obj, prog); > if (err) { > pr_warn("prog '%s': failed to relocate data references: %d\n", > @@ -6939,10 +6969,6 @@ static int bpf_object__sanitize_maps(struct bpf_object *obj) > bpf_object__for_each_map(m, obj) { > if (!bpf_map__is_internal(m)) > continue; > - if (!kernel_supports(obj, FEAT_GLOBAL_DATA)) { > - pr_warn("kernel doesn't support global data\n"); > - return -ENOTSUP; > - } > if (!kernel_supports(obj, FEAT_ARRAY_MMAP)) > m->def.map_flags ^= BPF_F_MMAPABLE; > } > -- > 2.30.2 > >
