[PATCH bpf-next v1] veristat: load struct_ops programs only once

[PATCH bpf-next v1] veristat: load struct_ops programs only once

[Eduard Zingerman]

libbpf automatically adjusts autoload for struct_ops programs,
see libbpf.c:bpf_object_adjust_struct_ops_autoload.

For example, if there is a map:

    SEC(".struct_ops.link")
    struct sched_ext_ops ops = {
    	.enqueue = foo,
        .tick = bar,
    };

Both 'foo' and 'bar' would be loaded if 'ops' autocreate is true,
both 'foo' and 'bar' would be skipped if 'ops' autocreate is false.

This means that when veristat processes object file with 'ops',
it would load 4 programs in total: two programs per each
'process_prog' call.

The adjustment occurs at object load time, and libbpf remembers
association between 'ops' and 'foo'/'bar' at object open time.
The only way to persuade libbpf to load one of two is to adjust map
initial value, such that only one program is referenced.
This patch does exactly that, significantly reducing time to process
object files with big number of struct_ops programs.

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 tools/testing/selftests/bpf/veristat.c | 39 ++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
index b47836ee7d4d..23b64faa54ab 100644
--- a/tools/testing/selftests/bpf/veristat.c
+++ b/tools/testing/selftests/bpf/veristat.c
@@ -1103,6 +1103,42 @@ static int guess_prog_type_by_ctx_name(const char *ctx_name,
 	return -ESRCH;
 }
 
+/* Make sure only target program is referenced from struct_ops map,
+ * otherwise libbpf would automatically set autocreate for all
+ * referenced programs.
+ * See libbpf.c:bpf_object_adjust_struct_ops_autoload.
+ */
+static void mask_unrelated_struct_ops_progs(struct bpf_object *obj,
+					    struct bpf_map *map,
+					    struct bpf_program *prog)
+{
+	struct btf *btf = bpf_object__btf(obj);
+	const struct btf_type *t, *mt;
+	struct btf_member *m;
+	int i, ptr_sz, moff;
+	size_t data_sz;
+	void *data;
+
+	t = btf__type_by_id(btf, bpf_map__btf_value_type_id(map));
+	if (!btf_is_struct(t))
+		return;
+
+	data = bpf_map__initial_value(map, &data_sz);
+	ptr_sz = min(btf__pointer_size(btf), sizeof(void *));
+	for (i = 0; i < btf_vlen(t); i++) {
+		m = &btf_members(t)[i];
+		mt = btf__type_by_id(btf, m->type);
+		if (!btf_is_ptr(mt))
+			continue;
+		moff = m->offset / 8;
+		if (moff + ptr_sz > data_sz)
+			continue;
+		if (memcmp(data + moff, &prog, ptr_sz) == 0)
+			continue;
+		memset(data + moff, 0, ptr_sz);
+	}
+}
+
 static void fixup_obj(struct bpf_object *obj, struct bpf_program *prog, const char *filename)
 {
 	struct bpf_map *map;
@@ -1118,6 +1154,9 @@ static void fixup_obj(struct bpf_object *obj, struct bpf_program *prog, const ch
 		case BPF_MAP_TYPE_INODE_STORAGE:
 		case BPF_MAP_TYPE_CGROUP_STORAGE:
 			break;
+		case BPF_MAP_TYPE_STRUCT_OPS:
+			mask_unrelated_struct_ops_progs(obj, map, prog);
+			break;
 		default:
 			if (bpf_map__max_entries(map) == 0)
 				bpf_map__set_max_entries(map, 1);
-- 
2.47.1

Re: [PATCH bpf-next v1] veristat: load struct_ops programs only once

[Andrii Nakryiko]

On Wed, Jan 15, 2025 at 2:39 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> libbpf automatically adjusts autoload for struct_ops programs,
> see libbpf.c:bpf_object_adjust_struct_ops_autoload.
>
> For example, if there is a map:
>
>     SEC(".struct_ops.link")
>     struct sched_ext_ops ops = {
>         .enqueue = foo,
>         .tick = bar,
>     };
>
> Both 'foo' and 'bar' would be loaded if 'ops' autocreate is true,
> both 'foo' and 'bar' would be skipped if 'ops' autocreate is false.
>
> This means that when veristat processes object file with 'ops',
> it would load 4 programs in total: two programs per each
> 'process_prog' call.
>
> The adjustment occurs at object load time, and libbpf remembers
> association between 'ops' and 'foo'/'bar' at object open time.
> The only way to persuade libbpf to load one of two is to adjust map
> initial value, such that only one program is referenced.
> This patch does exactly that, significantly reducing time to process
> object files with big number of struct_ops programs.
>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
>  tools/testing/selftests/bpf/veristat.c | 39 ++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
> index b47836ee7d4d..23b64faa54ab 100644
> --- a/tools/testing/selftests/bpf/veristat.c
> +++ b/tools/testing/selftests/bpf/veristat.c
> @@ -1103,6 +1103,42 @@ static int guess_prog_type_by_ctx_name(const char *ctx_name,
>         return -ESRCH;
>  }
>
> +/* Make sure only target program is referenced from struct_ops map,
> + * otherwise libbpf would automatically set autocreate for all
> + * referenced programs.
> + * See libbpf.c:bpf_object_adjust_struct_ops_autoload.
> + */
> +static void mask_unrelated_struct_ops_progs(struct bpf_object *obj,
> +                                           struct bpf_map *map,
> +                                           struct bpf_program *prog)
> +{
> +       struct btf *btf = bpf_object__btf(obj);
> +       const struct btf_type *t, *mt;
> +       struct btf_member *m;
> +       int i, ptr_sz, moff;
> +       size_t data_sz;
> +       void *data;
> +
> +       t = btf__type_by_id(btf, bpf_map__btf_value_type_id(map));
> +       if (!btf_is_struct(t))
> +               return;
> +
> +       data = bpf_map__initial_value(map, &data_sz);
> +       ptr_sz = min(btf__pointer_size(btf), sizeof(void *));

btf__pointer_size() for .bpf.o should always be 8, so this min is
pointless, I think. I can simplify to just ptr_sz = sizeof(void *)
while applying, if you agree. Let me know.

Other than that looks good.

> +       for (i = 0; i < btf_vlen(t); i++) {
> +               m = &btf_members(t)[i];
> +               mt = btf__type_by_id(btf, m->type);
> +               if (!btf_is_ptr(mt))
> +                       continue;
> +               moff = m->offset / 8;
> +               if (moff + ptr_sz > data_sz)
> +                       continue;
> +               if (memcmp(data + moff, &prog, ptr_sz) == 0)
> +                       continue;
> +               memset(data + moff, 0, ptr_sz);
> +       }
> +}
> +
>  static void fixup_obj(struct bpf_object *obj, struct bpf_program *prog, const char *filename)
>  {
>         struct bpf_map *map;
> @@ -1118,6 +1154,9 @@ static void fixup_obj(struct bpf_object *obj, struct bpf_program *prog, const ch
>                 case BPF_MAP_TYPE_INODE_STORAGE:
>                 case BPF_MAP_TYPE_CGROUP_STORAGE:
>                         break;
> +               case BPF_MAP_TYPE_STRUCT_OPS:
> +                       mask_unrelated_struct_ops_progs(obj, map, prog);
> +                       break;
>                 default:
>                         if (bpf_map__max_entries(map) == 0)
>                                 bpf_map__set_max_entries(map, 1);
> --
> 2.47.1
>

Re: [PATCH bpf-next v1] veristat: load struct_ops programs only once

[Eduard Zingerman]

On Thu, 2025-01-16 at 15:15 -0800, Andrii Nakryiko wrote:

[...]

> > +/* Make sure only target program is referenced from struct_ops map,
> > + * otherwise libbpf would automatically set autocreate for all
> > + * referenced programs.
> > + * See libbpf.c:bpf_object_adjust_struct_ops_autoload.
> > + */
> > +static void mask_unrelated_struct_ops_progs(struct bpf_object *obj,
> > +                                           struct bpf_map *map,
> > +                                           struct bpf_program *prog)
> > +{
> > +       struct btf *btf = bpf_object__btf(obj);
> > +       const struct btf_type *t, *mt;
> > +       struct btf_member *m;
> > +       int i, ptr_sz, moff;
> > +       size_t data_sz;
> > +       void *data;
> > +
> > +       t = btf__type_by_id(btf, bpf_map__btf_value_type_id(map));
> > +       if (!btf_is_struct(t))
> > +               return;
> > +
> > +       data = bpf_map__initial_value(map, &data_sz);
> > +       ptr_sz = min(btf__pointer_size(btf), sizeof(void *));
> 
> btf__pointer_size() for .bpf.o should always be 8, so this min is
> pointless, I think. I can simplify to just ptr_sz = sizeof(void *)
> while applying, if you agree. Let me know.

I wanted to be pedantic, but am ok if you prefer to switch it to
'ptr_sz = sizeof(void *)'. Can send v2, or if you change it when
applying, that would be great.

> 
> Other than that looks good.
> 
> > +       for (i = 0; i < btf_vlen(t); i++) {
> > +               m = &btf_members(t)[i];
> > +               mt = btf__type_by_id(btf, m->type);
> > +               if (!btf_is_ptr(mt))
> > +                       continue;
> > +               moff = m->offset / 8;
> > +               if (moff + ptr_sz > data_sz)
> > +                       continue;
> > +               if (memcmp(data + moff, &prog, ptr_sz) == 0)
> > +                       continue;
> > +               memset(data + moff, 0, ptr_sz);
> > +       }
> > +}
> > +

[...]

Re: [PATCH bpf-next v1] veristat: load struct_ops programs only once

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Wed, 15 Jan 2025 14:38:35 -0800 you wrote:
> libbpf automatically adjusts autoload for struct_ops programs,
> see libbpf.c:bpf_object_adjust_struct_ops_autoload.
> 
> For example, if there is a map:
> 
>     SEC(".struct_ops.link")
>     struct sched_ext_ops ops = {
>     	.enqueue = foo,
>         .tick = bar,
>     };
> 
> [...]

Here is the summary with links:
  - [bpf-next,v1] veristat: load struct_ops programs only once
    https://git.kernel.org/bpf/bpf-next/c/7c311b7cb3c7

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html