On 5/17/22 4:25 PM, Andrii Nakryiko wrote:
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.
Besides this implementation, I had another idea to just add one int type unconditionally for enum64->union member type. To avoid adding this type unconditionally, I added the search part. Let me try the approach to add the new int type in the fly when needed.
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; + } + } } }
[...]
