On Fri, Sep 27, 2024 at 6:28 AM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Sat, Sep 21, 2024 at 3:17 AM JP Kobryn <inwardvessel@xxxxxxxxx> wrote:
> >
> > Arrays in custom data sections can be resized via bpf_map__set_value().
> > While working with these types of arrays in some sched_ext programs, there
> > was some feedback that the manual operations involved could use helpers.
> > The macros in the potential patch are intended to make resizing bpf arrays
> > easier.
> >
> > To illustrate, declaring an array that will be resized looks like this:
> > __u32 my_map[1] SEC(".data.my_map");
> >
> > Instead, using a macro to help with the declaration:
> > __u32 BPF_RESIZABLE_ARRAY(data, my_map);
>
> I don't like hiding things in a macro.
> SEC() isn't great, but that's what we got and users
> used to it.
>
I agree, macros are sometimes a necessary evil, but they do obscure
things and shouldn't be proliferated unnecessarily.
JP, instead of these patches, I'd suggest adding a new file under
Documentation/bpf/libbpf to describe common libbpf coding patterns,
and this resizable array use case would be a perfect starter for this.
> > To allow access to the post-resized array in the bpf program, this helper
> > can be used which maintains verifier safety:
> > u32 *val = (u32 *)ARRAY_ELEM_PTR(my_map, ctx->cpu, nr_cpus);
>
> I don't like this one either.
> We have bpf_cmp_likely/unlikely that can be used
> to guard array access against the limit.
>
> > Meanwhile in the userspace program, instead of doing:
> > size_t sz = bpf_map__set_value_size(skel->maps.data_my_map, sizeof(skel->data_my_map->my_map[0]) * nr_cpus);
> > skel->data_my_map = bpf_map__initial_value(skel->maps.data_my_map, &sz);
> >
> > The resizing macro can be used:
> > BPF_RESIZE_ARRAY(data, my_map, nr_cpus);
>
> Open code of libbpf api is much more readable. Macros are not.
>
>
>
> >
> > Signed-off-by: JP Kobryn <inwardvessel@xxxxxxxxx>
> > ---
> > include/uapi/linux/bpf.h | 23 ++++++++++++++++++
> > tools/lib/bpf/bpf_helpers.h | 48 +++++++++++++++++++++++++++++++++++++
> > 2 files changed, 71 insertions(+)
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index e05b39e39c3f..92e93c9fc056 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -7513,4 +7513,27 @@ struct bpf_iter_num {
> > __u64 __opaque[1];
> > } __attribute__((aligned(8)));
> >
> > +/*
> > + * BPF_RESIZE_ARRAY - Convenience macro for resizing a BPF array
> > + * @elfsec: the data section of the BPF program in which to the array exists
> > + * @arr: the name of the array
> > + * @n: the desired array element count
> > + *
> > + * For BPF arrays declared with RESIZABLE_ARRAY(), this macro performs two
> > + * operations. It resizes the map which corresponds to the custom data
> > + * section that contains the target array. As a side effect, the BTF info for
> > + * the array is adjusted so that the array length is sized to cover the new
> > + * data section size. The second operation is reassigning the skeleton pointer
> > + * for that custom data section so that it points to the newly memory mapped
> > + * region.
> > + */
> > +#define BPF_RESIZE_ARRAY(elfsec, arr, n) \
> > + do { \
> > + size_t __sz; \
> > + bpf_map__set_value_size(skel->maps.elfsec##_##arr, \
> > + sizeof(skel->elfsec##_##arr->arr[0]) * (n)); \
> > + skel->elfsec##_##arr = \
> > + bpf_map__initial_value(skel->maps.elfsec##_##arr, &__sz); \
> > + } while (0)
> > +
> > #endif /* _UAPI__LINUX_BPF_H__ */
> > diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> > index 305c62817dd3..b0d496b0f0d6 100644
> > --- a/tools/lib/bpf/bpf_helpers.h
> > +++ b/tools/lib/bpf/bpf_helpers.h
> > @@ -420,4 +420,52 @@ extern void bpf_iter_num_destroy(struct bpf_iter_num *it) __weak __ksym;
> > )
> > #endif /* bpf_repeat */
> >
> > +/**
> > + * RESIZABLE_ARRAY - Generates annotations for an array that may be resized
> > + * @elfsec: the data section of the BPF program in which to place the array
> > + * @arr: the name of the array
> > + *
> > + * libbpf has an API for setting map value sizes. Since data sections (i.e.
> > + * bss, data, rodata) themselves are maps, a data section can be resized. If
> > + * a data section has an array as its last element, the BTF info for that
> > + * array will be adjusted so that length of the array is extended to meet the
> > + * new length of the data section. This macro annotates an array to have an
> > + * element count of one with the assumption that this array can be resized
> > + * within the userspace program. It also annotates the section specifier so
> > + * this array exists in a custom sub data section which can be resized
> > + * independently.
> > + *
> > + * See BPF_RESIZE_ARRAY() for the userspace convenience macro for resizing an
> > + * array declared with BPF_RESIZABLE_ARRAY().
> > + */
> > +#define BPF_RESIZABLE_ARRAY(elfsec, arr) arr[1] SEC("."#elfsec"."#arr)
> > +
> > +/*
> > + * BPF_ARRAY_ELEM_PTR - Obtain the verified pointer to an array element
> > + * @arr: array to index into
> > + * @i: array index
> > + * @n: number of elements in array
> > + *
> > + * Similar to MEMBER_VPTR() but is intended for use with arrays where the
> > + * element count needs to be explicit.
> > + * It can be used in cases where a global array is defined with an initial
> > + * size but is intended to be be resized before loading the BPF program.
> > + * Without this version of the macro, MEMBER_VPTR() will use the compile time
> > + * size of the array to compute the max, which will result in rejection by
> > + * the verifier.
> > + */
> > +#define BPF_ARRAY_ELEM_PTR(arr, i, n) (typeof(arr[i]) *)({ \
> > + u64 __base = (u64)arr; \
> > + u64 __addr = (u64)&(arr[i]) - __base; \
> > + asm volatile ( \
> > + "if %0 <= %[max] goto +2\n" \
> > + "%0 = 0\n" \
> > + "goto +1\n" \
> > + "%0 += %1\n" \
> > + : "+r"(__addr) \
> > + : "r"(__base), \
> > + [max]"r"(sizeof(arr[0]) * ((n) - 1))); \
> > + __addr; \
> > +})
> > +
> > #endif
> > --
> > 2.46.0
> >
