On Fri, May 13, 2022 at 8:13 PM Yonghong Song <yhs@xxxxxx> wrote:
>
> When old kernel does not support enum64 but user space btf
> contains non-zero enum kflag or enum64, libbpf needs to
> do proper sanitization so modified btf can be accepted
> by the kernel.
>
> Sanitization for enum kflag can be achieved by clearing
> the kflag bit. For enum64, the type is replaced with an
> union of integer member types and the integer member size
> must be smaller than enum64 size. If such an integer
> type cannot be found, a new type is created and used
> for union members.
>
> Signed-off-by: Yonghong Song <yhs@xxxxxx>
> ---
> tools/lib/bpf/btf.h | 3 +-
> tools/lib/bpf/libbpf.c | 53 +++++++++++++++++++++++++++++++--
> tools/lib/bpf/libbpf_internal.h | 2 ++
> 3 files changed, 55 insertions(+), 3 deletions(-)
>
> diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
> index 7da6970b8c9f..d4fe1300ed33 100644
> --- a/tools/lib/bpf/btf.h
> +++ b/tools/lib/bpf/btf.h
> @@ -395,9 +395,10 @@ btf_dump__dump_type_data(struct btf_dump *d, __u32 id,
> #ifndef BTF_KIND_FLOAT
> #define BTF_KIND_FLOAT 16 /* Floating point */
> #endif
> -/* The kernel header switched to enums, so these two were never #defined */
> +/* The kernel header switched to enums, so the following were never #defined */
> #define BTF_KIND_DECL_TAG 17 /* Decl Tag */
> #define BTF_KIND_TYPE_TAG 18 /* Type Tag */
> +#define BTF_KIND_ENUM64 19 /* Enum for up-to 64bit values */
>
> static inline __u16 btf_kind(const struct btf_type *t)
> {
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 4867a930628b..f54e70b9953d 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2114,6 +2114,7 @@ static const char *__btf_kind_str(__u16 kind)
> case BTF_KIND_FLOAT: return "float";
> case BTF_KIND_DECL_TAG: return "decl_tag";
> case BTF_KIND_TYPE_TAG: return "type_tag";
> + case BTF_KIND_ENUM64: return "enum64";
> default: return "unknown";
> }
> }
> @@ -2642,9 +2643,10 @@ static bool btf_needs_sanitization(struct bpf_object *obj)
> bool has_func = kernel_supports(obj, FEAT_BTF_FUNC);
> 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);
>
> return !has_func || !has_datasec || !has_func_global || !has_float ||
> - !has_decl_tag || !has_type_tag;
> + !has_decl_tag || !has_type_tag || !has_enum64;
> }
>
> static void bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
> @@ -2655,9 +2657,25 @@ static void bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
> bool has_func = kernel_supports(obj, FEAT_BTF_FUNC);
> 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);
> + int min_int_size = 32, min_enum64_size = 32, min_int_tid = 0;
> struct btf_type *t;
> int i, j, vlen;
>
> + if (!has_enum64) {
> + for (i = 1; i < btf__type_cnt(btf); i++) {
> + t = (struct btf_type *)btf__type_by_id(btf, i);
> + if (btf_is_int(t) && t->size < min_int_size) {
> + min_int_size = t->size;
> + min_int_tid = i;
> + } else if (btf_is_enum64(t) && t->size < min_enum64_size) {
> + min_enum64_size = t->size;
> + }
> + }
> + if (min_int_size > min_enum64_size)
> + min_int_tid = btf__add_int(btf, "char", 1, BTF_INT_SIGNED);
> + }
> +
we do this search even if bpf_object's BTF doesn't have enum64, which
seems overly pessimistic. How about we just lazily (but
unconditionally) add new BTF_KIND_INT on first encountered enum64 and
remember it's id (see below)
> for (i = 1; i < btf__type_cnt(btf); i++) {
> t = (struct btf_type *)btf__type_by_id(btf, i);
>
> @@ -2717,7 +2735,20 @@ static void bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
> /* replace TYPE_TAG with a CONST */
> t->name_off = 0;
> t->info = BTF_INFO_ENC(BTF_KIND_CONST, 0, 0);
> - }
> + } else if (!has_enum64 && btf_is_enum(t)) {
> + /* clear the kflag */
> + t->info = btf_type_info(btf_kind(t), btf_vlen(t), false);
> + } else if (!has_enum64 && btf_is_enum64(t)) {
> + /* replace ENUM64 with a union */
> + struct btf_member *m = btf_members(t);
> +
so here we just
if (enum64_placeholder_id == 0) {
enum64_placeholder_id = btf__add_int(btf, "enum64_placeholder", t->size, 0);
if (enum64_placeholder_id < 0) /* pr_warn and exit with error */
}
and then just use enum64_placeholder_id for each field type?
It seems much simpler than trying to find matching int (especially
given potentially non-8-byte size), so it seems better to just add our
own type.
Please make sure to re-initialize t and m after that because
btf__add_int() invalidates underlying memory, so you need to re-fetch
btf__type_by_id().
> + vlen = btf_vlen(t);
> + t->info = BTF_INFO_ENC(BTF_KIND_UNION, 0, vlen);
> + for (j = 0; j < vlen; j++, m++) {
> + m->type = min_int_tid;
> + m->offset = 0;
> + }
> + }
> }
> }
>
> @@ -3563,6 +3594,10 @@ static enum kcfg_type find_kcfg_type(const struct btf *btf, int id,
> if (strcmp(name, "libbpf_tristate"))
> return KCFG_UNKNOWN;
> return KCFG_TRISTATE;
> + case BTF_KIND_ENUM64:
> + if (strcmp(name, "libbpf_tristate"))
> + return KCFG_UNKNOWN;
> + return KCFG_TRISTATE;
> case BTF_KIND_ARRAY:
> if (btf_array(t)->nelems == 0)
> return KCFG_UNKNOWN;
> @@ -4746,6 +4781,17 @@ static int probe_kern_bpf_cookie(void)
> return probe_fd(ret);
> }
>
> +static int probe_kern_btf_enum64(void)
> +{
> + static const char strs[] = "\0enum64";
> + __u32 types[] = {
> + BTF_TYPE_ENC(1, BTF_INFO_ENC(BTF_KIND_ENUM64, 0, 0), 8),
> + };
> +
> + return probe_fd(libbpf__load_raw_btf((char *)types, sizeof(types),
> + strs, sizeof(strs)));
> +}
> +
> enum kern_feature_result {
> FEAT_UNKNOWN = 0,
> FEAT_SUPPORTED = 1,
> @@ -4811,6 +4857,9 @@ static struct kern_feature_desc {
> [FEAT_BPF_COOKIE] = {
> "BPF cookie support", probe_kern_bpf_cookie,
> },
> + [FEAT_BTF_ENUM64] = {
> + "BTF_KIND_ENUM64 support", probe_kern_btf_enum64,
> + },
> };
>
> bool kernel_supports(const struct bpf_object *obj, enum kern_feature_id feat_id)
> diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
> index 4abdbe2fea9d..10c16acfa8ae 100644
> --- a/tools/lib/bpf/libbpf_internal.h
> +++ b/tools/lib/bpf/libbpf_internal.h
> @@ -351,6 +351,8 @@ enum kern_feature_id {
> FEAT_MEMCG_ACCOUNT,
> /* BPF cookie (bpf_get_attach_cookie() BPF helper) support */
> FEAT_BPF_COOKIE,
> + /* BTF_KIND_ENUM64 support and BTF_KIND_ENUM kflag support */
> + FEAT_BTF_ENUM64,
> __FEAT_CNT,
> };
>
> --
> 2.30.2
>
