On Tue, Feb 27, 2024 at 10:45:50PM +0200, Eduard Zingerman wrote:
> Enforce the following existing limitation on struct_ops programs based
> on kernel BTF id instead of program-local BTF id:
>
> struct_ops BPF prog can be re-used between multiple .struct_ops &
> .struct_ops.link as long as it's the same struct_ops struct
> definition and the same function pointer field
Am I correct in understanding the code that the prog also has to be at the same
offset in the new type? So if we have for example:
SEC("struct_ops/test")
int BPF_PROG(foo) { ... }
struct some_ops___v1 {
int (*test)(void);
};
struct some_ops___v2 {
int (*init)(void);
int (*test)(void);
};
Then this wouldn't work? If so, would it be possible for libbpf to do something
like invisibly duplicate the prog and create a separate one for each struct_ops
map where it's encountered? It feels like a rather awkward restriction to
impose given that the idea behind the feature is to enable loading one of
multiple possible definitions of a struct_ops type.
>
> This allows reusing same BPF program for versioned struct_ops map
> definitions, e.g.:
>
> SEC("struct_ops/test")
> int BPF_PROG(foo) { ... }
>
> struct some_ops___v1 { int (*test)(void); };
> struct some_ops___v2 { int (*test)(void); };
>
> SEC(".struct_ops.link") struct some_ops___v1 a = { .test = foo }
> SEC(".struct_ops.link") struct some_ops___v2 b = { .test = foo }
>
> Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
> ---
> tools/lib/bpf/libbpf.c | 44 ++++++++++++++++++++----------------------
> 1 file changed, 21 insertions(+), 23 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index abe663927013..c239b75d5816 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1134,8 +1134,27 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map)
>
> if (mod_btf)
> prog->attach_btf_obj_fd = mod_btf->fd;
> - prog->attach_btf_id = kern_type_id;
> - prog->expected_attach_type = kern_member_idx;
> +
> + /* if we haven't yet processed this BPF program, record proper
> + * attach_btf_id and member_idx
> + */
> + if (!prog->attach_btf_id) {
> + prog->attach_btf_id = kern_type_id;
> + prog->expected_attach_type = kern_member_idx;
> + }
> +
> + /* struct_ops BPF prog can be re-used between multiple
> + * .struct_ops & .struct_ops.link as long as it's the
> + * same struct_ops struct definition and the same
> + * function pointer field
> + */
> + if (prog->attach_btf_id != kern_type_id ||
> + prog->expected_attach_type != kern_member_idx) {
> + pr_warn("struct_ops reloc %s: cannot use prog %s in sec %s with type %u attach_btf_id %u expected_attach_type %u for func ptr %s\n",
> + map->name, prog->name, prog->sec_name, prog->type,
> + prog->attach_btf_id, prog->expected_attach_type, mname);
> + return -EINVAL;
> + }
>
> st_ops->kern_func_off[i] = kern_data_off + kern_moff;
>
> @@ -9409,27 +9428,6 @@ static int bpf_object__collect_st_ops_relos(struct bpf_object *obj,
> return -EINVAL;
> }
>
> - /* if we haven't yet processed this BPF program, record proper
> - * attach_btf_id and member_idx
> - */
> - if (!prog->attach_btf_id) {
> - prog->attach_btf_id = st_ops->type_id;
> - prog->expected_attach_type = member_idx;
> - }
> -
> - /* struct_ops BPF prog can be re-used between multiple
> - * .struct_ops & .struct_ops.link as long as it's the
> - * same struct_ops struct definition and the same
> - * function pointer field
> - */
> - if (prog->attach_btf_id != st_ops->type_id ||
> - prog->expected_attach_type != member_idx) {
> - pr_warn("struct_ops reloc %s: cannot use prog %s in sec %s with type %u attach_btf_id %u expected_attach_type %u for func ptr %s\n",
> - map->name, prog->name, prog->sec_name, prog->type,
> - prog->attach_btf_id, prog->expected_attach_type, name);
> - return -EINVAL;
> - }
> -
> st_ops->progs[member_idx] = prog;
> }
>
> --
> 2.43.0
>
Attachment:
signature.asc
Description: PGP signature
