On Mon, Mar 4, 2024 at 2:52 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> Automatically select which struct_ops programs to load depending on
> which struct_ops maps are selected for automatic creation.
> E.g. for the BPF code below:
>
> SEC("struct_ops/test_1") int BPF_PROG(foo) { ... }
> SEC("struct_ops/test_2") int BPF_PROG(bar) { ... }
>
> SEC(".struct_ops.link")
> struct test_ops___v1 A = {
> .foo = (void *)foo
> };
>
> SEC(".struct_ops.link")
> struct test_ops___v2 B = {
> .foo = (void *)foo,
> .bar = (void *)bar,
> };
>
> And the following libbpf API calls:
>
> bpf_map__set_autocreate(skel->maps.A, true);
> bpf_map__set_autocreate(skel->maps.B, false);
>
> The autoload would be enabled for program 'foo' and disabled for
> program 'bar'.
>
> During load, for each struct_ops program P, referenced from some
> struct_ops map M:
> - set P.autoload = true if M.autocreate is true for some M;
> - set P.autoload = false if M.autocreate is false for all M;
> - don't change P.autoload, if P is not referenced from any map.
>
> Do this after bpf_object__init_kern_struct_ops_maps()
> to make sure that shadow vars assignment is done.
>
> Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
> ---
> tools/lib/bpf/libbpf.c | 48 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 25c452c20d7d..ce39ae34fec0 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1031,6 +1031,53 @@ static bool is_valid_st_ops_program(struct bpf_object *obj,
> return false;
> }
>
> +/* For each struct_ops program P, referenced from some struct_ops map M,
> + * enable P.autoload if there are Ms for which M.autocreate is true,
> + * disable P.autoload if for all Ms M.autocreate is false.
> + * Don't change P.autoload for programs that are not referenced from any maps.
> + */
> +static int bpf_object__adjust_struct_ops_autoload(struct bpf_object *obj)
nit: we try not to use double underscore separate for non-public
internal helper functions (see bpf_object_init_prog_arrays and
bpf_object_prepare_struct_ops)
> +{
> + struct bpf_program *prog;
> + struct bpf_map *map;
> + int i, j, k, vlen;
> + struct {
> + __u8 any_map_autocreate:1;
> + __u8 used_in_struct_ops_map:1;
> + } *refs;
bit masks just for this to save a few bytes?...
but generally, how about we keep all this much dumber:
for (each program) {
int use_cnt = 0;
int should_load = false;
if (!is_struct_ops_program)
continue;
for (each map) {
if (!is_struct_ops_map)
continue;
for (each prog entry in map->st_ops->progs[]) {
if (prog != slot_prog)
continue;
use_cnt++;
if (map->autocreate)
should_load = true;
}
}
if (use_cnt)
prog->autocreate = should_load;
}
Sure it does a few more iterations, but we avoid unnecessary
allocation/cleanup and keep things literally working how you describe
in your comment.
WDYT?
> +
> + refs = calloc(obj->nr_programs, sizeof(refs[0]));
> + if (!refs)
> + return -ENOMEM;
> +
> + for (i = 0; i < obj->nr_maps; i++) {
> + map = &obj->maps[i];
> + if (!bpf_map__is_struct_ops(map))
> + continue;
> +
> + vlen = btf_vlen(map->st_ops->type);
> + for (j = 0; j < vlen; ++j) {
nit: stay consistent with ++j vs j++?
> + prog = map->st_ops->progs[j];
> + if (!prog)
> + continue;
> +
> + k = prog - obj->programs;
> + if (k < 0 || k > obj->nr_programs)
>= ? But a clever trick, I like it!
> + continue;
> +
> + refs[k].used_in_struct_ops_map = true;
> + refs[k].any_map_autocreate |= map->autocreate;
> + }
> + }
> +
> + for (i = 0; i < obj->nr_programs; ++i)
> + if (refs[i].used_in_struct_ops_map)
> + obj->programs[i].autoload = refs[i].any_map_autocreate;
> +
> + free(refs);
> + return 0;
> +}
> +
> /* Init the map's fields that depend on kern_btf */
> static int bpf_map__init_kern_struct_ops(struct bpf_map *map)
> {
> @@ -8163,6 +8210,7 @@ static int bpf_object_load(struct bpf_object *obj, int extra_log_level, const ch
> err = err ? : bpf_object__resolve_externs(obj, obj->kconfig);
> err = err ? : bpf_object__sanitize_maps(obj);
> err = err ? : bpf_object__init_kern_struct_ops_maps(obj);
> + err = err ? : bpf_object__adjust_struct_ops_autoload(obj);
> err = err ? : bpf_object__relocate(obj, obj->btf_custom_path ? : target_btf_path);
> err = err ? : bpf_object__sanitize_and_load_btf(obj);
> err = err ? : bpf_object__create_maps(obj);
> --
> 2.43.0
>
