On Mon, Mar 4, 2024 at 2:52 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> Optional struct_ops maps are defined using question mark at the start
> of the section name, e.g.:
>
> SEC("?.struct_ops")
> struct test_ops optional_map = { ... };
>
> This commit teaches libbpf to detect if kernel allows '?' prefix
> in datasec names, and if it doesn't then to rewrite such names
> by removing '?' prefix and adding ".optional" suffix.
> For example:
>
> DATASEC ?.struct_ops -> DATASEC .struct_ops.optional
>
> Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
> ---
> tools/lib/bpf/features.c | 22 ++++++++++++++++++++++
> tools/lib/bpf/libbpf.c | 30 +++++++++++++++++++++++++++++-
> tools/lib/bpf/libbpf_internal.h | 2 ++
> 3 files changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/features.c b/tools/lib/bpf/features.c
> index 6b0738ad7063..4e783cc7fc4b 100644
> --- a/tools/lib/bpf/features.c
> +++ b/tools/lib/bpf/features.c
> @@ -147,6 +147,25 @@ static int probe_kern_btf_datasec(int token_fd)
> strs, sizeof(strs), token_fd));
> }
>
> +static int probe_kern_btf_qmark_datasec(int token_fd)
> +{
> + static const char strs[] = "\0x\0?.data";
> + /* static int a; */
> + __u32 types[] = {
> + /* int */
> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
> + /* VAR x */ /* [2] */
> + BTF_TYPE_ENC(1, BTF_INFO_ENC(BTF_KIND_VAR, 0, 0), 1),
> + BTF_VAR_STATIC,
> + /* DATASEC ?.data */ /* [3] */
> + BTF_TYPE_ENC(3, BTF_INFO_ENC(BTF_KIND_DATASEC, 0, 1), 4),
> + BTF_VAR_SECINFO_ENC(2, 0, 4),
> + };
> +
> + return probe_fd(libbpf__load_raw_btf((char *)types, sizeof(types),
> + strs, sizeof(strs), token_fd));
> +}
> +
> static int probe_kern_btf_float(int token_fd)
> {
> static const char strs[] = "\0float";
> @@ -534,6 +553,9 @@ static struct kern_feature_desc {
> [FEAT_ARG_CTX_TAG] = {
> "kernel-side __arg_ctx tag", probe_kern_arg_ctx_tag,
> },
> + [FEAT_BTF_QMARK_DATASEC] = {
> + "BTF DATASEC names starting from '?'", probe_kern_btf_qmark_datasec,
> + },
> };
>
> bool feat_supported(struct kern_feature_cache *cache, enum kern_feature_id feat_id)
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index c0212244bdf7..cf60291db8fd 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2874,6 +2874,11 @@ static bool section_have_execinstr(struct bpf_object *obj, int idx)
> return sh->sh_flags & SHF_EXECINSTR;
> }
>
> +static bool starts_with_qmark(const char *s)
> +{
> + return s && s[0] == '?';
> +}
> +
> static bool btf_needs_sanitization(struct bpf_object *obj)
> {
> bool has_func_global = kernel_supports(obj, FEAT_BTF_GLOBAL_FUNC);
> @@ -2883,9 +2888,10 @@ static bool btf_needs_sanitization(struct bpf_object *obj)
> bool has_decl_tag = kernel_supports(obj, FEAT_BTF_DECL_TAG);
> bool has_type_tag = kernel_supports(obj, FEAT_BTF_TYPE_TAG);
> bool has_enum64 = kernel_supports(obj, FEAT_BTF_ENUM64);
> + bool has_qmark_datasec = kernel_supports(obj, FEAT_BTF_QMARK_DATASEC);
>
> return !has_func || !has_datasec || !has_func_global || !has_float ||
> - !has_decl_tag || !has_type_tag || !has_enum64;
> + !has_decl_tag || !has_type_tag || !has_enum64 || !has_qmark_datasec;
> }
>
> static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
> @@ -2897,6 +2903,7 @@ static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
> bool has_decl_tag = kernel_supports(obj, FEAT_BTF_DECL_TAG);
> bool has_type_tag = kernel_supports(obj, FEAT_BTF_TYPE_TAG);
> bool has_enum64 = kernel_supports(obj, FEAT_BTF_ENUM64);
> + bool has_qmark_datasec = kernel_supports(obj, FEAT_BTF_QMARK_DATASEC);
> int enum64_placeholder_id = 0;
> struct btf_type *t;
> int i, j, vlen;
> @@ -2922,6 +2929,8 @@ static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
> char *name;
>
> name = (char *)btf__name_by_offset(btf, t->name_off);
> + if (*name == '?')
> + *name++ = '_';
> while (*name) {
> if (*name == '.')
let's just extend this to `if (*name == '.' || *name == '?')` ?
> *name = '_';
> @@ -2938,6 +2947,25 @@ static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
> vt = (void *)btf__type_by_id(btf, v->type);
> m->name_off = vt->name_off;
> }
> + } else if (!has_qmark_datasec && btf_is_datasec(t) &&
> + starts_with_qmark(btf__name_by_offset(btf, t->name_off))) {
> + /* remove '?' prefix and add '.optional' suffix for
> + * DATASEC names staring from '?':
> + *
> + * DATASEC ?.foo -> DATASEC .foo.optional
> + */
> + const char *name;
> + char buf[256];
> + int str;
> +
> + name = btf__name_by_offset(btf, t->name_off);
> + snprintf(buf, sizeof(buf), "%s.optional", &name[1] /* skip '?' */);
> + str = btf__add_str(btf, buf);
> + if (str < 0)
> + return str;
> +
> + t = (struct btf_type *)btf__type_by_id(btf, i);
> + t->name_off = str;
let's keep it simpler, just do in-place name sanitization like we did
for !has_datasec case? It's fine if "?.struct_ops" becomes
"_.struct_ops", kernel doesn't care and doesn't assign any special
meaning to DATASEC names
> } else if (!has_func && btf_is_func_proto(t)) {
> /* replace FUNC_PROTO with ENUM */
> vlen = btf_vlen(t);
> diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
> index ad936ac5e639..864b36177424 100644
> --- a/tools/lib/bpf/libbpf_internal.h
> +++ b/tools/lib/bpf/libbpf_internal.h
> @@ -374,6 +374,8 @@ enum kern_feature_id {
> FEAT_UPROBE_MULTI_LINK,
> /* Kernel supports arg:ctx tag (__arg_ctx) for global subprogs natively */
> FEAT_ARG_CTX_TAG,
> + /* Kernel supports '?' at the front of datasec names */
> + FEAT_BTF_QMARK_DATASEC,
> __FEAT_CNT,
> };
>
> --
> 2.43.0
>
