Re: [PATCH v2 bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support
From: Ilya Leoshkevich <iii@linux.ibm.com>
Date: 2021-02-19 23:44:41
On Fri, 2021-02-19 at 15:35 -0800, Yonghong Song wrote:
On 2/19/21 3:01 PM, Ilya Leoshkevich wrote:quoted
On Thu, 2021-02-18 at 20:22 -0800, Yonghong Song wrote:quoted
On 2/18/21 6:25 PM, Ilya Leoshkevich wrote:quoted
The logic follows that of BTF_KIND_INT most of the time. Sanitization replaces BTF_KIND_FLOATs with BTF_KIND_TYPEDEFs pointing to equally-sized BTF_KIND_ARRAYs on older kernels. Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- tools/lib/bpf/btf.c | 44 ++++++++++++++++++++++++++++++++ tools/lib/bpf/btf.h | 8 ++++++ tools/lib/bpf/btf_dump.c | 4 +++ tools/lib/bpf/libbpf.c | 45 ++++++++++++++++++++++++++++++++- tools/lib/bpf/libbpf.map | 5 ++++ tools/lib/bpf/libbpf_internal.h | 2 ++ 6 files changed, 107 insertions(+), 1 deletion(-)[...]quoted
diff --git a/tools/lib/bpf/btf_dump.cb/tools/lib/bpf/btf_dump.c index 2f9d685bd522..5e957fcceee6 100644--- a/tools/lib/bpf/btf_dump.c +++ b/tools/lib/bpf/btf_dump.c@@ -279,6 +279,7 @@ static int btf_dump_mark_referenced(structbtf_dump *d) case BTF_KIND_INT: case BTF_KIND_ENUM: case BTF_KIND_FWD: + case BTF_KIND_FLOAT: break; case BTF_KIND_VOLATILE:@@ -453,6 +454,7 @@ static int btf_dump_order_type(structbtf_dump *d, __u32 id, bool through_ptr) switch (btf_kind(t)) { case BTF_KIND_INT: + case BTF_KIND_FLOAT: tstate->order_state = ORDERED; return 0;@@ -1133,6 +1135,7 @@ static voidbtf_dump_emit_type_decl(struct btf_dump *d, __u32 id, case BTF_KIND_STRUCT: case BTF_KIND_UNION: case BTF_KIND_TYPEDEF: + case BTF_KIND_FLOAT: goto done; default: pr_warn("unexpected type in decl chain, kind:%u, id:[%u]\n",@@ -1247,6 +1250,7 @@ static voidbtf_dump_emit_type_chain(struct btf_dump *d, switch (kind) { case BTF_KIND_INT: + case BTF_KIND_FLOAT: btf_dump_emit_mods(d, decls); name = btf_name_of(d, t->name_off); btf_dump_printf(d, "%s", name);diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index d43cc3f29dae..3b170066d613 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c@@ -178,6 +178,8 @@ enum kern_feature_id {FEAT_PROG_BIND_MAP, /* Kernel support for module BTFs */ FEAT_MODULE_BTF, + /* BTF_KIND_FLOAT support */ + FEAT_BTF_FLOAT, __FEAT_CNT, };@@ -1935,6 +1937,7 @@ static const char *btf_kind_str(conststruct btf_type *t) case BTF_KIND_FUNC_PROTO: return "func_proto"; case BTF_KIND_VAR: return "var"; case BTF_KIND_DATASEC: return "datasec"; + case BTF_KIND_FLOAT: return "float"; default: return "unknown"; } }@@ -2384,18 +2387,22 @@ static boolbtf_needs_sanitization(struct bpf_object *obj) { bool has_func_global = kernel_supports(FEAT_BTF_GLOBAL_FUNC); bool has_datasec = kernel_supports(FEAT_BTF_DATASEC); + bool has_float = kernel_supports(FEAT_BTF_FLOAT); bool has_func = kernel_supports(FEAT_BTF_FUNC); - return !has_func || !has_datasec || !has_func_global; + return !has_func || !has_datasec || !has_func_global || !has_float; } static void bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf) { bool has_func_global = kernel_supports(FEAT_BTF_GLOBAL_FUNC); bool has_datasec = kernel_supports(FEAT_BTF_DATASEC); + bool has_float = kernel_supports(FEAT_BTF_FLOAT); bool has_func = kernel_supports(FEAT_BTF_FUNC); struct btf_type *t; int i, j, vlen; + int t_u32 = 0; + int t_u8 = 0; for (i = 1; i <= btf__get_nr_types(btf); i++) { t = (struct btf_type *)btf__type_by_id(btf, i);@@ -2445,6 +2452,23 @@ static voidbpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf) } else if (!has_func_global && btf_is_func(t)) { /* replace BTF_FUNC_GLOBAL with BTF_FUNC_STATIC */ t->info = BTF_INFO_ENC(BTF_KIND_FUNC, 0, 0); + } else if (!has_float && btf_is_float(t)) { + /* replace FLOAT with TYPEDEF(u8[]) */ + int t_array; + __u32 size; + + size = t->size; + if (!t_u8) + t_u8 = btf__add_int(btf, "u8", 1, 0); + if (!t_u32) + t_u32 = btf__add_int(btf, "u32", 4, 0); + t_array = btf__add_array(btf, t_u32, t_u8, size); + + /* adding new types may have invalidated t */ + t = (struct btf_type *)btf__type_by_id(btf, i); + + t->info = BTF_INFO_ENC(BTF_KIND_TYPEDEF, 0, 0);This won't work. The typedef must have a valid name. Otherwise, kernel will reject it. A const char array should be okay here.Wouldn't it reuse the old t->name? At least in my testing with v5.7 kernel this was the case, and BTF wasn't rejected. And the original BTF_KIND_FLOAT always has a valid name, this is enforced in libbpf and in the verifier. For example: [1] PTR '(anon)' type_id=2 [2] STRUCT 'foo' size=4 vlen=1 'bar' type_id=3 bits_offset=0 [3] FLOAT 'float' size=4 [4] FUNC_PROTO '(anon)' ret_type_id=5 vlen=1 'f' type_id=1 [5] PTR '(anon)' type_id=0 [6] FUNC 'func' type_id=4 linkage=global becomes [1] PTR '(anon)' type_id=2 [2] STRUCT 'foo' size=4 vlen=1 'bar' type_id=3 bits_offset=0 [3] TYPEDEF 'float' type_id=9 [4] FUNC_PROTO '(anon)' ret_type_id=5 vlen=1 'f' type_id=1 [5] PTR '(anon)' type_id=0 [6] FUNC 'func' type_id=4 linkage=global [7] INT 'u8' size=1 bits_offset=0 nr_bits=8 encoding=(none) [8] INT 'u32' size=4 bits_offset=0 nr_bits=32 encoding=(none) [9] ARRAY '(anon)' type_id=7 index_type_id=8 nr_elems=4good point, the name is indeed there. I originally also thought about using "typedef" but worried whether new typedef may polute the existing type names. Could you try to dump the modified BTF to a C file and compile to see whether typedef mechanism works or not? I tried the following code and compilation failed. -bash-4.4$ cat t.c typedef char * float; -bash-4.4$ gcc -c t.c t.c:1:16: error: expected identifier or ‘(’ before ‘float’ typedef char * float; ^ -bash-4.4$ Changing to a different name may interfere with existing types. It may work with the kernel today because we didn't do strict type legality checking, but it would be still be good if we can avoid it.
Yeah, the following generated C code does not compile:
typedef u8 float[4];
struct foo {
float bar;
};
I'll go with const. Thanks for the hint!