bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 bpf-next 0/6] Add BTF_KIND_FLOAT support
@ 2021-02-19  2:25 Ilya Leoshkevich
  2021-02-19  2:25 ` [PATCH v2 bpf-next 1/6] bpf: Add BTF_KIND_FLOAT to uapi Ilya Leoshkevich
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Ilya Leoshkevich @ 2021-02-19  2:25 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Arnaldo Carvalho de Melo
  Cc: John Fastabend, bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich

Some BPF programs compiled on s390 fail to load, because s390
arch-specific linux headers contain float and double types.
    
Introduce support for such types by representing them using the new
BTF_KIND_FLOAT. This series deals with libbpf, bpftool, in-kernel BTF
parser as well as selftests and documentation.

There are also pahole and LLVM parts:

* https://github.com/iii-i/dwarves/commit/btf-kind-float-v2
* https://reviews.llvm.org/D83289

but they should go in after the libbpf part is integrated.

---

v0: https://lore.kernel.org/bpf/20210210030317.78820-1-iii@linux.ibm.com/
v0 -> v1: Per Andrii's suggestion, remove the unnecessary trailing u32.

v1: https://lore.kernel.org/bpf/20210216011216.3168-1-iii@linux.ibm.com/
v1 -> v2: John noticed that sanitization corrupts BTF, because new and
          old sizes don't match. Per Yonghong's suggestion, use a
          modifier type (which has the same size as the float type) as
          a replacement.
          Per Yonghong's suggestion, add size and alignment checks to
          the kernel BTF parser.

Based on Alexei's feedback [1] I'm proceeding with the BTF_KIND_FLOAT
approach.

[1] https://lore.kernel.org/bpf/CAADnVQKWPODWZ2RSJ5FJhfYpxkuV0cvSAL1O+FSr9oP1ercoBg@mail.gmail.com/

Ilya Leoshkevich (6):
  bpf: Add BTF_KIND_FLOAT to uapi
  libbpf: Add BTF_KIND_FLOAT support
  tools/bpftool: Add BTF_KIND_FLOAT support
  bpf: Add BTF_KIND_FLOAT support
  selftest/bpf: Add BTF_KIND_FLOAT tests
  bpf: Document BTF_KIND_FLOAT in btf.rst

 Documentation/bpf/btf.rst                    |  17 ++-
 include/uapi/linux/btf.h                     |   5 +-
 kernel/bpf/btf.c                             |  76 +++++++++++-
 tools/bpf/bpftool/btf.c                      |   8 ++
 tools/bpf/bpftool/btf_dumper.c               |   1 +
 tools/include/uapi/linux/btf.h               |   5 +-
 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 +
 tools/testing/selftests/bpf/btf_helpers.c    |   4 +
 tools/testing/selftests/bpf/prog_tests/btf.c | 122 +++++++++++++++++++
 tools/testing/selftests/bpf/test_btf.h       |   3 +
 15 files changed, 340 insertions(+), 9 deletions(-)

-- 
2.29.2


^ permalink raw reply	[flat|nested] 18+ messages in thread
* [PATCH v2 bpf-next 1/6] bpf: Add BTF_KIND_FLOAT to uapi
  2021-02-19  2:25 [PATCH v2 bpf-next 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
@ 2021-02-19  2:25 ` Ilya Leoshkevich
  2021-02-19  2:25 ` [PATCH v2 bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support Ilya Leoshkevich
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Ilya Leoshkevich @ 2021-02-19  2:25 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Arnaldo Carvalho de Melo
  Cc: John Fastabend, bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich

Add a new kind value and expand the kind bitfield.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 include/uapi/linux/btf.h       | 5 +++--
 tools/include/uapi/linux/btf.h | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
index 5a667107ad2c..d27b1708efe9 100644
--- a/include/uapi/linux/btf.h
+++ b/include/uapi/linux/btf.h
@@ -52,7 +52,7 @@ struct btf_type {
 	};
 };
 
-#define BTF_INFO_KIND(info)	(((info) >> 24) & 0x0f)
+#define BTF_INFO_KIND(info)	(((info) >> 24) & 0x1f)
 #define BTF_INFO_VLEN(info)	((info) & 0xffff)
 #define BTF_INFO_KFLAG(info)	((info) >> 31)
 
@@ -72,7 +72,8 @@ struct btf_type {
 #define BTF_KIND_FUNC_PROTO	13	/* Function Proto	*/
 #define BTF_KIND_VAR		14	/* Variable	*/
 #define BTF_KIND_DATASEC	15	/* Section	*/
-#define BTF_KIND_MAX		BTF_KIND_DATASEC
+#define BTF_KIND_FLOAT		16	/* Floating point	*/
+#define BTF_KIND_MAX		BTF_KIND_FLOAT
 #define NR_BTF_KINDS		(BTF_KIND_MAX + 1)
 
 /* For some specific BTF_KIND, "struct btf_type" is immediately
diff --git a/tools/include/uapi/linux/btf.h b/tools/include/uapi/linux/btf.h
index 5a667107ad2c..d27b1708efe9 100644
--- a/tools/include/uapi/linux/btf.h
+++ b/tools/include/uapi/linux/btf.h
@@ -52,7 +52,7 @@ struct btf_type {
 	};
 };
 
-#define BTF_INFO_KIND(info)	(((info) >> 24) & 0x0f)
+#define BTF_INFO_KIND(info)	(((info) >> 24) & 0x1f)
 #define BTF_INFO_VLEN(info)	((info) & 0xffff)
 #define BTF_INFO_KFLAG(info)	((info) >> 31)
 
@@ -72,7 +72,8 @@ struct btf_type {
 #define BTF_KIND_FUNC_PROTO	13	/* Function Proto	*/
 #define BTF_KIND_VAR		14	/* Variable	*/
 #define BTF_KIND_DATASEC	15	/* Section	*/
-#define BTF_KIND_MAX		BTF_KIND_DATASEC
+#define BTF_KIND_FLOAT		16	/* Floating point	*/
+#define BTF_KIND_MAX		BTF_KIND_FLOAT
 #define NR_BTF_KINDS		(BTF_KIND_MAX + 1)
 
 /* For some specific BTF_KIND, "struct btf_type" is immediately
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH v2 bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support
  2021-02-19  2:25 [PATCH v2 bpf-next 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
  2021-02-19  2:25 ` [PATCH v2 bpf-next 1/6] bpf: Add BTF_KIND_FLOAT to uapi Ilya Leoshkevich
@ 2021-02-19  2:25 ` Ilya Leoshkevich
  2021-02-19  4:22   ` Yonghong Song
  2021-02-19  2:25 ` [PATCH v2 bpf-next 3/6] tools/bpftool: " Ilya Leoshkevich
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Ilya Leoshkevich @ 2021-02-19  2:25 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Arnaldo Carvalho de Melo
  Cc: John Fastabend, bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich

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(-)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index d9c10830d749..07a30e98c3de 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -291,6 +291,7 @@ static int btf_type_size(const struct btf_type *t)
 	case BTF_KIND_PTR:
 	case BTF_KIND_TYPEDEF:
 	case BTF_KIND_FUNC:
+	case BTF_KIND_FLOAT:
 		return base_size;
 	case BTF_KIND_INT:
 		return base_size + sizeof(__u32);
@@ -338,6 +339,7 @@ static int btf_bswap_type_rest(struct btf_type *t)
 	case BTF_KIND_PTR:
 	case BTF_KIND_TYPEDEF:
 	case BTF_KIND_FUNC:
+	case BTF_KIND_FLOAT:
 		return 0;
 	case BTF_KIND_INT:
 		*(__u32 *)(t + 1) = bswap_32(*(__u32 *)(t + 1));
@@ -578,6 +580,7 @@ __s64 btf__resolve_size(const struct btf *btf, __u32 type_id)
 		case BTF_KIND_UNION:
 		case BTF_KIND_ENUM:
 		case BTF_KIND_DATASEC:
+		case BTF_KIND_FLOAT:
 			size = t->size;
 			goto done;
 		case BTF_KIND_PTR:
@@ -621,6 +624,7 @@ int btf__align_of(const struct btf *btf, __u32 id)
 	switch (kind) {
 	case BTF_KIND_INT:
 	case BTF_KIND_ENUM:
+	case BTF_KIND_FLOAT:
 		return min(btf_ptr_sz(btf), (size_t)t->size);
 	case BTF_KIND_PTR:
 		return btf_ptr_sz(btf);
@@ -2373,6 +2377,42 @@ int btf__add_datasec(struct btf *btf, const char *name, __u32 byte_sz)
 	return btf_commit_type(btf, sz);
 }
 
+/*
+ * Append new BTF_KIND_FLOAT type with:
+ *   - *name* - non-empty, non-NULL type name;
+ *   - *sz* - size of the type, in bytes;
+ * Returns:
+ *   - >0, type ID of newly added BTF type;
+ *   - <0, on error.
+ */
+int btf__add_float(struct btf *btf, const char *name, size_t byte_sz)
+{
+	struct btf_type *t;
+	int sz, name_off;
+
+	/* non-empty name */
+	if (!name || !name[0])
+		return -EINVAL;
+
+	if (btf_ensure_modifiable(btf))
+		return -ENOMEM;
+
+	sz = sizeof(struct btf_type);
+	t = btf_add_type_mem(btf, sz);
+	if (!t)
+		return -ENOMEM;
+
+	name_off = btf__add_str(btf, name);
+	if (name_off < 0)
+		return name_off;
+
+	t->name_off = name_off;
+	t->info = btf_type_info(BTF_KIND_FLOAT, 0, 0);
+	t->size = byte_sz;
+
+	return btf_commit_type(btf, sz);
+}
+
 /*
  * Append new data section variable information entry for current DATASEC type:
  *   - *var_type_id* - type ID, describing type of the variable;
@@ -3626,6 +3666,7 @@ static int btf_dedup_prep(struct btf_dedup *d)
 		case BTF_KIND_FWD:
 		case BTF_KIND_TYPEDEF:
 		case BTF_KIND_FUNC:
+		case BTF_KIND_FLOAT:
 			h = btf_hash_common(t);
 			break;
 		case BTF_KIND_INT:
@@ -3722,6 +3763,7 @@ static int btf_dedup_prim_type(struct btf_dedup *d, __u32 type_id)
 		break;
 
 	case BTF_KIND_FWD:
+	case BTF_KIND_FLOAT:
 		h = btf_hash_common(t);
 		for_each_dedup_cand(d, hash_entry, h) {
 			cand_id = (__u32)(long)hash_entry->value;
@@ -3983,6 +4025,7 @@ static int btf_dedup_is_equiv(struct btf_dedup *d, __u32 cand_id,
 			return btf_compat_enum(cand_type, canon_type);
 
 	case BTF_KIND_FWD:
+	case BTF_KIND_FLOAT:
 		return btf_equal_common(cand_type, canon_type);
 
 	case BTF_KIND_CONST:
@@ -4479,6 +4522,7 @@ static int btf_dedup_remap_type(struct btf_dedup *d, __u32 type_id)
 	switch (btf_kind(t)) {
 	case BTF_KIND_INT:
 	case BTF_KIND_ENUM:
+	case BTF_KIND_FLOAT:
 		break;
 
 	case BTF_KIND_FWD:
diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index 1237bcd1dd17..c3b11bcebeda 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -132,6 +132,9 @@ LIBBPF_API int btf__add_datasec(struct btf *btf, const char *name, __u32 byte_sz
 LIBBPF_API int btf__add_datasec_var_info(struct btf *btf, int var_type_id,
 					 __u32 offset, __u32 byte_sz);
 
+/* float construction APIs */
+LIBBPF_API int btf__add_float(struct btf *btf, const char *name, size_t byte_sz);
+
 struct btf_dedup_opts {
 	unsigned int dedup_table_size;
 	bool dont_resolve_fwds;
@@ -294,6 +297,11 @@ static inline bool btf_is_datasec(const struct btf_type *t)
 	return btf_kind(t) == BTF_KIND_DATASEC;
 }
 
+static inline bool btf_is_float(const struct btf_type *t)
+{
+	return btf_kind(t) == BTF_KIND_FLOAT;
+}
+
 static inline __u8 btf_int_encoding(const struct btf_type *t)
 {
 	return BTF_INT_ENCODING(*(__u32 *)(t + 1));
diff --git a/tools/lib/bpf/btf_dump.c b/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(struct btf_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(struct btf_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 void btf_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 void btf_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(const struct 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 bool btf_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 void bpf_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);
+			t->type = t_array;
 		}
 	}
 }
@@ -3882,6 +3906,18 @@ static int probe_kern_btf_datasec(void)
 					     strs, sizeof(strs)));
 }
 
+static int probe_kern_btf_float(void)
+{
+	static const char strs[] = "\0float";
+	__u32 types[] = {
+		/* float */
+		BTF_TYPE_FLOAT_ENC(1, 4),
+	};
+
+	return probe_fd(libbpf__load_raw_btf((char *)types, sizeof(types),
+					     strs, sizeof(strs)));
+}
+
 static int probe_kern_array_mmap(void)
 {
 	struct bpf_create_map_attr attr = {
@@ -4061,6 +4097,9 @@ static struct kern_feature_desc {
 	[FEAT_MODULE_BTF] = {
 		"module BTF support", probe_module_btf,
 	},
+	[FEAT_BTF_FLOAT] = {
+		"BTF_KIND_FLOAT support", probe_kern_btf_float,
+	},
 };
 
 static bool kernel_supports(enum kern_feature_id feat_id)
@@ -4940,6 +4979,8 @@ static int bpf_core_fields_are_compat(const struct btf *local_btf,
 		local_id = btf_array(local_type)->type;
 		targ_id = btf_array(targ_type)->type;
 		goto recur;
+	case BTF_KIND_FLOAT:
+		return local_type->size == targ_type->size;
 	default:
 		pr_warn("unexpected kind %d relocated, local [%d], target [%d]\n",
 			btf_kind(local_type), local_id, targ_id);
@@ -5122,6 +5163,8 @@ static int bpf_core_types_are_compat(const struct btf *local_btf, __u32 local_id
 		skip_mods_and_typedefs(targ_btf, targ_type->type, &targ_id);
 		goto recur;
 	}
+	case BTF_KIND_FLOAT:
+		return local_type->size == targ_type->size;
 	default:
 		pr_warn("unexpected kind %s relocated, local [%d], target [%d]\n",
 			btf_kind_str(local_type), local_id, targ_id);
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 1c0fd2dd233a..ec898f464ab9 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -350,3 +350,8 @@ LIBBPF_0.3.0 {
 		xsk_setup_xdp_prog;
 		xsk_socket__update_xskmap;
 } LIBBPF_0.2.0;
+
+LIBBPF_0.4.0 {
+	global:
+		btf__add_float;
+} LIBBPF_0.3.0;
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 969d0ac592ba..343f6eb05637 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -31,6 +31,8 @@
 #define BTF_MEMBER_ENC(name, type, bits_offset) (name), (type), (bits_offset)
 #define BTF_PARAM_ENC(name, type) (name), (type)
 #define BTF_VAR_SECINFO_ENC(type, offset, size) (type), (offset), (size)
+#define BTF_TYPE_FLOAT_ENC(name, sz) \
+	BTF_TYPE_ENC(name, BTF_INFO_ENC(BTF_KIND_FLOAT, 0, 0), sz)
 
 #ifndef likely
 #define likely(x) __builtin_expect(!!(x), 1)
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH v2 bpf-next 3/6] tools/bpftool: Add BTF_KIND_FLOAT support
  2021-02-19  2:25 [PATCH v2 bpf-next 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
  2021-02-19  2:25 ` [PATCH v2 bpf-next 1/6] bpf: Add BTF_KIND_FLOAT to uapi Ilya Leoshkevich
  2021-02-19  2:25 ` [PATCH v2 bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support Ilya Leoshkevich
@ 2021-02-19  2:25 ` Ilya Leoshkevich
  2021-02-19  2:25 ` [PATCH v2 bpf-next 4/6] bpf: " Ilya Leoshkevich
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Ilya Leoshkevich @ 2021-02-19  2:25 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Arnaldo Carvalho de Melo
  Cc: John Fastabend, bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich

Only dumping support needs to be adjusted, the code structure follows
that of BTF_KIND_INT.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tools/bpf/bpftool/btf.c        | 8 ++++++++
 tools/bpf/bpftool/btf_dumper.c | 1 +
 2 files changed, 9 insertions(+)

diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
index fe9e7b3a4b50..985610c3f193 100644
--- a/tools/bpf/bpftool/btf.c
+++ b/tools/bpf/bpftool/btf.c
@@ -36,6 +36,7 @@ static const char * const btf_kind_str[NR_BTF_KINDS] = {
 	[BTF_KIND_FUNC_PROTO]	= "FUNC_PROTO",
 	[BTF_KIND_VAR]		= "VAR",
 	[BTF_KIND_DATASEC]	= "DATASEC",
+	[BTF_KIND_FLOAT]	= "FLOAT",
 };
 
 struct btf_attach_table {
@@ -327,6 +328,13 @@ static int dump_btf_type(const struct btf *btf, __u32 id,
 			jsonw_end_array(w);
 		break;
 	}
+	case BTF_KIND_FLOAT: {
+		if (json_output)
+			jsonw_uint_field(w, "size", t->size);
+		else
+			printf(" size=%u", t->size);
+		break;
+	}
 	default:
 		break;
 	}
diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c
index 0e9310727281..7ca54d046362 100644
--- a/tools/bpf/bpftool/btf_dumper.c
+++ b/tools/bpf/bpftool/btf_dumper.c
@@ -596,6 +596,7 @@ static int __btf_dumper_type_only(const struct btf *btf, __u32 type_id,
 	switch (BTF_INFO_KIND(t->info)) {
 	case BTF_KIND_INT:
 	case BTF_KIND_TYPEDEF:
+	case BTF_KIND_FLOAT:
 		BTF_PRINT_ARG("%s ", btf__name_by_offset(btf, t->name_off));
 		break;
 	case BTF_KIND_STRUCT:
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH v2 bpf-next 4/6] bpf: Add BTF_KIND_FLOAT support
  2021-02-19  2:25 [PATCH v2 bpf-next 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
                   ` (2 preceding siblings ...)
  2021-02-19  2:25 ` [PATCH v2 bpf-next 3/6] tools/bpftool: " Ilya Leoshkevich
@ 2021-02-19  2:25 ` Ilya Leoshkevich
  2021-02-19  4:04   ` kernel test robot
  2021-02-19  4:13   ` kernel test robot
  2021-02-19  2:25 ` [PATCH v2 bpf-next 5/6] selftest/bpf: Add BTF_KIND_FLOAT tests Ilya Leoshkevich
  2021-02-19  2:25 ` [PATCH v2 bpf-next 6/6] bpf: Document BTF_KIND_FLOAT in btf.rst Ilya Leoshkevich
  5 siblings, 2 replies; 18+ messages in thread
From: Ilya Leoshkevich @ 2021-02-19  2:25 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Arnaldo Carvalho de Melo
  Cc: John Fastabend, bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich

On the kernel side, introduce a new btf_kind_operations. It is
similar to that of BTF_KIND_INT, however, it does not need to
handle encodings and bit offsets. Do not implement printing, since
the kernel does not know how to format floating-point values.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 kernel/bpf/btf.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 74 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 2efeb5f4b343..0e7555077570 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -173,7 +173,7 @@
 #define BITS_ROUNDUP_BYTES(bits) \
 	(BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits))
 
-#define BTF_INFO_MASK 0x8f00ffff
+#define BTF_INFO_MASK 0x9f00ffff
 #define BTF_INT_MASK 0x0fffffff
 #define BTF_TYPE_ID_VALID(type_id) ((type_id) <= BTF_MAX_TYPE)
 #define BTF_STR_OFFSET_VALID(name_off) ((name_off) <= BTF_MAX_NAME_OFFSET)
@@ -280,6 +280,7 @@ static const char * const btf_kind_str[NR_BTF_KINDS] = {
 	[BTF_KIND_FUNC_PROTO]	= "FUNC_PROTO",
 	[BTF_KIND_VAR]		= "VAR",
 	[BTF_KIND_DATASEC]	= "DATASEC",
+	[BTF_KIND_FLOAT]	= "FLOAT",
 };
 
 static const char *btf_type_str(const struct btf_type *t)
@@ -574,6 +575,7 @@ static bool btf_type_has_size(const struct btf_type *t)
 	case BTF_KIND_UNION:
 	case BTF_KIND_ENUM:
 	case BTF_KIND_DATASEC:
+	case BTF_KIND_FLOAT:
 		return true;
 	}
 
@@ -1704,6 +1706,7 @@ __btf_resolve_size(const struct btf *btf, const struct btf_type *type,
 		case BTF_KIND_STRUCT:
 		case BTF_KIND_UNION:
 		case BTF_KIND_ENUM:
+		case BTF_KIND_FLOAT:
 			size = type->size;
 			goto resolved;
 
@@ -1849,7 +1852,7 @@ static int btf_df_check_kflag_member(struct btf_verifier_env *env,
 	return -EINVAL;
 }
 
-/* Used for ptr, array and struct/union type members.
+/* Used for ptr, array struct/union and float type members.
  * int, enum and modifier types have their specific callback functions.
  */
 static int btf_generic_check_kflag_member(struct btf_verifier_env *env,
@@ -3675,6 +3678,74 @@ static const struct btf_kind_operations datasec_ops = {
 	.show			= btf_datasec_show,
 };
 
+static s32 btf_float_check_meta(struct btf_verifier_env *env,
+				const struct btf_type *t,
+				u32 meta_left)
+{
+	if (btf_type_vlen(t)) {
+		btf_verifier_log_type(env, t, "vlen != 0");
+		return -EINVAL;
+	}
+
+	if (btf_type_kflag(t)) {
+		btf_verifier_log_type(env, t, "Invalid btf_info kind_flag");
+		return -EINVAL;
+	}
+
+	if (t->size != 2 && t->size % 4) {
+		btf_verifier_log_type(env, t, "Invalid type_size");
+		return -EINVAL;
+	}
+
+	btf_verifier_log_type(env, t, NULL);
+
+	return 0;
+}
+
+static int btf_float_check_member(struct btf_verifier_env *env,
+				  const struct btf_type *struct_type,
+				  const struct btf_member *member,
+				  const struct btf_type *member_type)
+{
+	u64 start_offset_bytes;
+	u64 end_offset_bytes;
+	u64 align_bytes;
+	u64 align_bits;
+
+	align_bytes = min_t(u64, sizeof(void *), member_type->size);
+	align_bits = align_bytes * BITS_PER_BYTE;
+	if (member->offset % align_bits) {
+		btf_verifier_log_member(env, struct_type, member,
+					"Member is not properly aligned");
+		return -EINVAL;
+	}
+
+	start_offset_bytes = member->offset / BITS_PER_BYTE;
+	end_offset_bytes = start_offset_bytes + member_type->size;
+	if (end_offset_bytes > struct_type->size) {
+		btf_verifier_log_member(env, struct_type, member,
+					"Member exceeds struct_size");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void btf_float_log(struct btf_verifier_env *env,
+			  const struct btf_type *t)
+{
+	btf_verifier_log(env, "size=%u", t->size);
+}
+
+static const struct btf_kind_operations float_ops = {
+	.check_meta = btf_float_check_meta,
+	.resolve = btf_df_resolve,
+	.check_member = btf_float_check_member,
+	.check_kflag_member = btf_generic_check_kflag_member,
+	.log_details = btf_float_log,
+	.show = btf_df_show,
+};
+
 static int btf_func_proto_check(struct btf_verifier_env *env,
 				const struct btf_type *t)
 {
@@ -3808,6 +3879,7 @@ static const struct btf_kind_operations * const kind_ops[NR_BTF_KINDS] = {
 	[BTF_KIND_FUNC_PROTO] = &func_proto_ops,
 	[BTF_KIND_VAR] = &var_ops,
 	[BTF_KIND_DATASEC] = &datasec_ops,
+	[BTF_KIND_FLOAT] = &float_ops,
 };
 
 static s32 btf_check_meta(struct btf_verifier_env *env,
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH v2 bpf-next 5/6] selftest/bpf: Add BTF_KIND_FLOAT tests
  2021-02-19  2:25 [PATCH v2 bpf-next 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
                   ` (3 preceding siblings ...)
  2021-02-19  2:25 ` [PATCH v2 bpf-next 4/6] bpf: " Ilya Leoshkevich
@ 2021-02-19  2:25 ` Ilya Leoshkevich
  2021-02-19  5:38   ` Yonghong Song
  2021-02-19  5:44   ` Yonghong Song
  2021-02-19  2:25 ` [PATCH v2 bpf-next 6/6] bpf: Document BTF_KIND_FLOAT in btf.rst Ilya Leoshkevich
  5 siblings, 2 replies; 18+ messages in thread
From: Ilya Leoshkevich @ 2021-02-19  2:25 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Arnaldo Carvalho de Melo
  Cc: John Fastabend, bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich

Test the good variants as well as the potential malformed ones.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tools/testing/selftests/bpf/btf_helpers.c    |   4 +
 tools/testing/selftests/bpf/prog_tests/btf.c | 122 +++++++++++++++++++
 tools/testing/selftests/bpf/test_btf.h       |   3 +
 3 files changed, 129 insertions(+)

diff --git a/tools/testing/selftests/bpf/btf_helpers.c b/tools/testing/selftests/bpf/btf_helpers.c
index 48f90490f922..b692e6ead9b5 100644
--- a/tools/testing/selftests/bpf/btf_helpers.c
+++ b/tools/testing/selftests/bpf/btf_helpers.c
@@ -23,6 +23,7 @@ static const char * const btf_kind_str_mapping[] = {
 	[BTF_KIND_FUNC_PROTO]	= "FUNC_PROTO",
 	[BTF_KIND_VAR]		= "VAR",
 	[BTF_KIND_DATASEC]	= "DATASEC",
+	[BTF_KIND_FLOAT]	= "FLOAT",
 };
 
 static const char *btf_kind_str(__u16 kind)
@@ -173,6 +174,9 @@ int fprintf_btf_type_raw(FILE *out, const struct btf *btf, __u32 id)
 		}
 		break;
 	}
+	case BTF_KIND_FLOAT:
+		fprintf(out, " size=%u", t->size);
+		break;
 	default:
 		break;
 	}
diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
index 6a7ee7420701..4be14d853cc3 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf.c
@@ -3531,6 +3531,127 @@ static struct btf_raw_test raw_tests[] = {
 	.max_entries = 1,
 },
 
+{
+	.descr = "float test #1, well-formed",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),	/* [1] */
+		BTF_TYPE_FLOAT_ENC(1, 2),			/* [2] */
+		BTF_TYPE_FLOAT_ENC(10, 4),			/* [3] */
+		BTF_TYPE_FLOAT_ENC(16, 8),			/* [4] */
+		BTF_TYPE_FLOAT_ENC(23, 16),			/* [5] */
+		BTF_STRUCT_ENC(35, 4, 32),			/* [6] */
+		BTF_MEMBER_ENC(NAME_TBD, 2, 0),
+		BTF_MEMBER_ENC(NAME_TBD, 3, 32),
+		BTF_MEMBER_ENC(NAME_TBD, 4, 64),
+		BTF_MEMBER_ENC(NAME_TBD, 5, 128),
+		BTF_END_RAW,
+	},
+	BTF_STR_SEC("\0_Float16\0float\0double\0long_double\0floats"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "float_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = 32,
+	.key_type_id = 1,
+	.value_type_id = 6,
+	.max_entries = 1,
+},
+{
+	.descr = "float test #2, invalid vlen",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),	/* [1] */
+		BTF_TYPE_ENC(1, BTF_INFO_ENC(BTF_KIND_FLOAT, 0, 1), 4),
+								/* [2] */
+		BTF_END_RAW,
+	},
+	BTF_STR_SEC("\0float"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "float_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = 4,
+	.key_type_id = 1,
+	.value_type_id = 2,
+	.max_entries = 1,
+	.btf_load_err = true,
+	.err_str = "vlen != 0",
+},
+{
+	.descr = "float test #3, invalid kind_flag",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),	/* [1] */
+		BTF_TYPE_ENC(1, BTF_INFO_ENC(BTF_KIND_FLOAT, 1, 0), 4),
+								/* [2] */
+		BTF_END_RAW,
+	},
+	BTF_STR_SEC("\0float"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "float_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = 4,
+	.key_type_id = 1,
+	.value_type_id = 2,
+	.max_entries = 1,
+	.btf_load_err = true,
+	.err_str = "Invalid btf_info kind_flag",
+},
+{
+	.descr = "float test #4, member does not fit",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),	/* [1] */
+		BTF_TYPE_FLOAT_ENC(1, 4),			/* [2] */
+		BTF_STRUCT_ENC(7, 1, 2),			/* [3] */
+		BTF_MEMBER_ENC(NAME_TBD, 2, 0),
+		BTF_END_RAW,
+	},
+	BTF_STR_SEC("\0float\0floats"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "float_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = 4,
+	.key_type_id = 1,
+	.value_type_id = 3,
+	.max_entries = 1,
+	.btf_load_err = true,
+	.err_str = "Member exceeds struct_size",
+},
+{
+	.descr = "float test #5, member is not properly aligned",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),	/* [1] */
+		BTF_TYPE_FLOAT_ENC(1, 4),			/* [2] */
+		BTF_STRUCT_ENC(7, 1, 8),			/* [3] */
+		BTF_MEMBER_ENC(NAME_TBD, 2, 8),
+		BTF_END_RAW,
+	},
+	BTF_STR_SEC("\0float\0floats"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "float_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = 4,
+	.key_type_id = 1,
+	.value_type_id = 3,
+	.max_entries = 1,
+	.btf_load_err = true,
+	.err_str = "Member is not properly aligned",
+},
+{
+	.descr = "float test #6, invalid size",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),	/* [1] */
+		BTF_TYPE_FLOAT_ENC(1, 6),			/* [2] */
+		BTF_END_RAW,
+	},
+	BTF_STR_SEC("\0float"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "float_type_check_btf",
+	.key_size = sizeof(int),
+	.value_size = 6,
+	.key_type_id = 1,
+	.value_type_id = 2,
+	.max_entries = 1,
+	.btf_load_err = true,
+	.err_str = "Invalid type_size",
+},
+
 }; /* struct btf_raw_test raw_tests[] */
 
 static const char *get_next_str(const char *start, const char *end)
@@ -6632,6 +6753,7 @@ static int btf_type_size(const struct btf_type *t)
 	case BTF_KIND_FUNC:
 		return base_size;
 	case BTF_KIND_INT:
+	case BTF_KIND_FLOAT:
 		return base_size + sizeof(__u32);
 	case BTF_KIND_ENUM:
 		return base_size + vlen * sizeof(struct btf_enum);
diff --git a/tools/testing/selftests/bpf/test_btf.h b/tools/testing/selftests/bpf/test_btf.h
index 2023725f1962..e2394eea4b7f 100644
--- a/tools/testing/selftests/bpf/test_btf.h
+++ b/tools/testing/selftests/bpf/test_btf.h
@@ -66,4 +66,7 @@
 #define BTF_FUNC_ENC(name, func_proto) \
 	BTF_TYPE_ENC(name, BTF_INFO_ENC(BTF_KIND_FUNC, 0, 0), func_proto)
 
+#define BTF_TYPE_FLOAT_ENC(name, sz) \
+	BTF_TYPE_ENC(name, BTF_INFO_ENC(BTF_KIND_FLOAT, 0, 0), sz)
+
 #endif /* _TEST_BTF_H */
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH v2 bpf-next 6/6] bpf: Document BTF_KIND_FLOAT in btf.rst
  2021-02-19  2:25 [PATCH v2 bpf-next 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
                   ` (4 preceding siblings ...)
  2021-02-19  2:25 ` [PATCH v2 bpf-next 5/6] selftest/bpf: Add BTF_KIND_FLOAT tests Ilya Leoshkevich
@ 2021-02-19  2:25 ` Ilya Leoshkevich
  2021-02-19  5:41   ` Yonghong Song
  5 siblings, 1 reply; 18+ messages in thread
From: Ilya Leoshkevich @ 2021-02-19  2:25 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Arnaldo Carvalho de Melo
  Cc: John Fastabend, bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich

Also document the expansion of the kind bitfield.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 Documentation/bpf/btf.rst | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/bpf/btf.rst b/Documentation/bpf/btf.rst
index 44dc789de2b4..4f25c992d442 100644
--- a/Documentation/bpf/btf.rst
+++ b/Documentation/bpf/btf.rst
@@ -84,6 +84,7 @@ sequentially and type id is assigned to each recognized type starting from id
     #define BTF_KIND_FUNC_PROTO     13      /* Function Proto       */
     #define BTF_KIND_VAR            14      /* Variable     */
     #define BTF_KIND_DATASEC        15      /* Section      */
+    #define BTF_KIND_FLOAT          16      /* Floating point       */
 
 Note that the type section encodes debug info, not just pure types.
 ``BTF_KIND_FUNC`` is not a type, and it represents a defined subprogram.
@@ -95,8 +96,8 @@ Each type contains the following common data::
         /* "info" bits arrangement
          * bits  0-15: vlen (e.g. # of struct's members)
          * bits 16-23: unused
-         * bits 24-27: kind (e.g. int, ptr, array...etc)
-         * bits 28-30: unused
+         * bits 24-28: kind (e.g. int, ptr, array...etc)
+         * bits 29-30: unused
          * bit     31: kind_flag, currently used by
          *             struct, union and fwd
          */
@@ -452,6 +453,18 @@ map definition.
   * ``offset``: the in-section offset of the variable
   * ``size``: the size of the variable in bytes
 
+2.2.16 BTF_KIND_FLOAT
+~~~~~~~~~~~~~~~~~~~~~
+
+``struct btf_type`` encoding requirement:
+ * ``name_off``: any valid offset
+ * ``info.kind_flag``: 0
+ * ``info.kind``: BTF_KIND_FLOAT
+ * ``info.vlen``: 0
+ * ``size``: the size of the float type in bytes.
+
+No additional type data follow ``btf_type``.
+
 3. BTF Kernel API
 *****************
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 bpf-next 4/6] bpf: Add BTF_KIND_FLOAT support
  2021-02-19  2:25 ` [PATCH v2 bpf-next 4/6] bpf: " Ilya Leoshkevich
@ 2021-02-19  4:04   ` kernel test robot
  2021-02-19  4:13   ` kernel test robot
  1 sibling, 0 replies; 18+ messages in thread
From: kernel test robot @ 2021-02-19  4:04 UTC (permalink / raw)
  To: Ilya Leoshkevich, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Yonghong Song, Arnaldo Carvalho de Melo
  Cc: kbuild-all, John Fastabend, bpf, Heiko Carstens, Vasily Gorbik,
	Ilya Leoshkevich

[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]

Hi Ilya,

I love your patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Ilya-Leoshkevich/Add-BTF_KIND_FLOAT-support/20210219-103132
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: m68k-bvme6000_defconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/2ce77d2bc322bcb14148c06a96992133b39fdf1c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Ilya-Leoshkevich/Add-BTF_KIND_FLOAT-support/20210219-103132
        git checkout 2ce77d2bc322bcb14148c06a96992133b39fdf1c
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=m68k 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   m68k-linux-ld: kernel/bpf/btf.o: in function `btf_float_check_member':
>> btf.c:(.text+0xf98): undefined reference to `__umoddi3'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 13406 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 bpf-next 4/6] bpf: Add BTF_KIND_FLOAT support
  2021-02-19  2:25 ` [PATCH v2 bpf-next 4/6] bpf: " Ilya Leoshkevich
  2021-02-19  4:04   ` kernel test robot
@ 2021-02-19  4:13   ` kernel test robot
  1 sibling, 0 replies; 18+ messages in thread
From: kernel test robot @ 2021-02-19  4:13 UTC (permalink / raw)
  To: Ilya Leoshkevich, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Yonghong Song, Arnaldo Carvalho de Melo
  Cc: kbuild-all, John Fastabend, bpf, Heiko Carstens, Vasily Gorbik,
	Ilya Leoshkevich

[-- Attachment #1: Type: text/plain, Size: 1504 bytes --]

Hi Ilya,

I love your patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Ilya-Leoshkevich/Add-BTF_KIND_FLOAT-support/20210219-103132
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: nds32-randconfig-r004-20210216 (attached as .config)
compiler: nds32le-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/2ce77d2bc322bcb14148c06a96992133b39fdf1c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Ilya-Leoshkevich/Add-BTF_KIND_FLOAT-support/20210219-103132
        git checkout 2ce77d2bc322bcb14148c06a96992133b39fdf1c
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=nds32 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   nds32le-linux-ld: kernel/bpf/btf.o: in function `btf_float_check_member':
   btf.c:(.text+0x25a4): undefined reference to `__umoddi3'
>> nds32le-linux-ld: btf.c:(.text+0x25a8): undefined reference to `__umoddi3'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31410 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support
  2021-02-19  2:25 ` [PATCH v2 bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support Ilya Leoshkevich
@ 2021-02-19  4:22   ` Yonghong Song
  2021-02-19 23:01     ` Ilya Leoshkevich
  0 siblings, 1 reply; 18+ messages in thread
From: Yonghong Song @ 2021-02-19  4:22 UTC (permalink / raw)
  To: Ilya Leoshkevich, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Arnaldo Carvalho de Melo
  Cc: John Fastabend, bpf, Heiko Carstens, Vasily Gorbik



On 2/18/21 6:25 PM, Ilya Leoshkevich wrote:
> 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(-)
> 
[...]
> diff --git a/tools/lib/bpf/btf_dump.c b/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(struct btf_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(struct btf_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 void btf_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 void btf_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(const struct 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 bool btf_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 void bpf_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.

> +			t->type = t_array;
>   		}
>   	}
>   }
[...]

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 bpf-next 5/6] selftest/bpf: Add BTF_KIND_FLOAT tests
  2021-02-19  2:25 ` [PATCH v2 bpf-next 5/6] selftest/bpf: Add BTF_KIND_FLOAT tests Ilya Leoshkevich
@ 2021-02-19  5:38   ` Yonghong Song
  2021-02-19  5:44   ` Yonghong Song
  1 sibling, 0 replies; 18+ messages in thread
From: Yonghong Song @ 2021-02-19  5:38 UTC (permalink / raw)
  To: Ilya Leoshkevich, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Arnaldo Carvalho de Melo
  Cc: John Fastabend, bpf, Heiko Carstens, Vasily Gorbik



On 2/18/21 6:25 PM, Ilya Leoshkevich wrote:
> Test the good variants as well as the potential malformed ones.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   tools/testing/selftests/bpf/btf_helpers.c    |   4 +
>   tools/testing/selftests/bpf/prog_tests/btf.c | 122 +++++++++++++++++++
>   tools/testing/selftests/bpf/test_btf.h       |   3 +
>   3 files changed, 129 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/btf_helpers.c b/tools/testing/selftests/bpf/btf_helpers.c
> index 48f90490f922..b692e6ead9b5 100644
> --- a/tools/testing/selftests/bpf/btf_helpers.c
> +++ b/tools/testing/selftests/bpf/btf_helpers.c
> @@ -23,6 +23,7 @@ static const char * const btf_kind_str_mapping[] = {
>   	[BTF_KIND_FUNC_PROTO]	= "FUNC_PROTO",
>   	[BTF_KIND_VAR]		= "VAR",
>   	[BTF_KIND_DATASEC]	= "DATASEC",
> +	[BTF_KIND_FLOAT]	= "FLOAT",
>   };
>   
>   static const char *btf_kind_str(__u16 kind)
> @@ -173,6 +174,9 @@ int fprintf_btf_type_raw(FILE *out, const struct btf *btf, __u32 id)
>   		}
>   		break;
>   	}
> +	case BTF_KIND_FLOAT:
> +		fprintf(out, " size=%u", t->size);
> +		break;
>   	default:
>   		break;
>   	}
> diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
> index 6a7ee7420701..4be14d853cc3 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf.c
> @@ -3531,6 +3531,127 @@ static struct btf_raw_test raw_tests[] = {
>   	.max_entries = 1,
>   },
>   
> +{
> +	.descr = "float test #1, well-formed",
> +	.raw_types = {
> +		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),	/* [1] */
> +		BTF_TYPE_FLOAT_ENC(1, 2),			/* [2] */
> +		BTF_TYPE_FLOAT_ENC(10, 4),			/* [3] */
> +		BTF_TYPE_FLOAT_ENC(16, 8),			/* [4] */
> +		BTF_TYPE_FLOAT_ENC(23, 16),			/* [5] */
> +		BTF_STRUCT_ENC(35, 4, 32),			/* [6] */
> +		BTF_MEMBER_ENC(NAME_TBD, 2, 0),
> +		BTF_MEMBER_ENC(NAME_TBD, 3, 32),
> +		BTF_MEMBER_ENC(NAME_TBD, 4, 64),
> +		BTF_MEMBER_ENC(NAME_TBD, 5, 128),
> +		BTF_END_RAW,
> +	},
> +	BTF_STR_SEC("\0_Float16\0float\0double\0long_double\0floats"),
> +	.map_type = BPF_MAP_TYPE_ARRAY,
> +	.map_name = "float_type_check_btf",
> +	.key_size = sizeof(int),
> +	.value_size = 32,
> +	.key_type_id = 1,
> +	.value_type_id = 6,
> +	.max_entries = 1,
> +},
> +{
> +	.descr = "float test #2, invalid vlen",
> +	.raw_types = {
> +		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),	/* [1] */
> +		BTF_TYPE_ENC(1, BTF_INFO_ENC(BTF_KIND_FLOAT, 0, 1), 4),
> +								/* [2] */
> +		BTF_END_RAW,
> +	},
> +	BTF_STR_SEC("\0float"),
> +	.map_type = BPF_MAP_TYPE_ARRAY,
> +	.map_name = "float_type_check_btf",
> +	.key_size = sizeof(int),
> +	.value_size = 4,
> +	.key_type_id = 1,
> +	.value_type_id = 2,
> +	.max_entries = 1,
> +	.btf_load_err = true,
> +	.err_str = "vlen != 0",
> +},
> +{
> +	.descr = "float test #3, invalid kind_flag",
> +	.raw_types = {
> +		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),	/* [1] */
> +		BTF_TYPE_ENC(1, BTF_INFO_ENC(BTF_KIND_FLOAT, 1, 0), 4),
> +								/* [2] */
> +		BTF_END_RAW,
> +	},
> +	BTF_STR_SEC("\0float"),
> +	.map_type = BPF_MAP_TYPE_ARRAY,
> +	.map_name = "float_type_check_btf",
> +	.key_size = sizeof(int),
> +	.value_size = 4,
> +	.key_type_id = 1,
> +	.value_type_id = 2,
> +	.max_entries = 1,
> +	.btf_load_err = true,
> +	.err_str = "Invalid btf_info kind_flag",
> +},
> +{
> +	.descr = "float test #4, member does not fit",
> +	.raw_types = {
> +		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),	/* [1] */
> +		BTF_TYPE_FLOAT_ENC(1, 4),			/* [2] */
> +		BTF_STRUCT_ENC(7, 1, 2),			/* [3] */
> +		BTF_MEMBER_ENC(NAME_TBD, 2, 0),
> +		BTF_END_RAW,
> +	},
> +	BTF_STR_SEC("\0float\0floats"),
> +	.map_type = BPF_MAP_TYPE_ARRAY,
> +	.map_name = "float_type_check_btf",
> +	.key_size = sizeof(int),
> +	.value_size = 4,
> +	.key_type_id = 1,
> +	.value_type_id = 3,
> +	.max_entries = 1,
> +	.btf_load_err = true,
> +	.err_str = "Member exceeds struct_size",
> +},
> +{
> +	.descr = "float test #5, member is not properly aligned",
> +	.raw_types = {
> +		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),	/* [1] */
> +		BTF_TYPE_FLOAT_ENC(1, 4),			/* [2] */
> +		BTF_STRUCT_ENC(7, 1, 8),			/* [3] */
> +		BTF_MEMBER_ENC(NAME_TBD, 2, 8),
> +		BTF_END_RAW,
> +	},
> +	BTF_STR_SEC("\0float\0floats"),
> +	.map_type = BPF_MAP_TYPE_ARRAY,
> +	.map_name = "float_type_check_btf",
> +	.key_size = sizeof(int),
> +	.value_size = 4,
> +	.key_type_id = 1,
> +	.value_type_id = 3,
> +	.max_entries = 1,
> +	.btf_load_err = true,
> +	.err_str = "Member is not properly aligned",
> +},
> +{
> +	.descr = "float test #6, invalid size",
> +	.raw_types = {
> +		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),	/* [1] */
> +		BTF_TYPE_FLOAT_ENC(1, 6),			/* [2] */
> +		BTF_END_RAW,
> +	},
> +	BTF_STR_SEC("\0float"),
> +	.map_type = BPF_MAP_TYPE_ARRAY,
> +	.map_name = "float_type_check_btf",
> +	.key_size = sizeof(int),
> +	.value_size = 6,
> +	.key_type_id = 1,
> +	.value_type_id = 2,
> +	.max_entries = 1,
> +	.btf_load_err = true,
> +	.err_str = "Invalid type_size",
> +},
> +
>   }; /* struct btf_raw_test raw_tests[] */
>   
>   static const char *get_next_str(const char *start, const char *end)
> @@ -6632,6 +6753,7 @@ static int btf_type_size(const struct btf_type *t)
>   	case BTF_KIND_FUNC:
>   		return base_size;
>   	case BTF_KIND_INT:
> +	case BTF_KIND_FLOAT:
>   		return base_size + sizeof(__u32);

This is not correct.

>   	case BTF_KIND_ENUM:
>   		return base_size + vlen * sizeof(struct btf_enum);
> diff --git a/tools/testing/selftests/bpf/test_btf.h b/tools/testing/selftests/bpf/test_btf.h
> index 2023725f1962..e2394eea4b7f 100644
> --- a/tools/testing/selftests/bpf/test_btf.h
> +++ b/tools/testing/selftests/bpf/test_btf.h
> @@ -66,4 +66,7 @@
>   #define BTF_FUNC_ENC(name, func_proto) \
>   	BTF_TYPE_ENC(name, BTF_INFO_ENC(BTF_KIND_FUNC, 0, 0), func_proto)
>   
> +#define BTF_TYPE_FLOAT_ENC(name, sz) \
> +	BTF_TYPE_ENC(name, BTF_INFO_ENC(BTF_KIND_FLOAT, 0, 0), sz)
> +
>   #endif /* _TEST_BTF_H */
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 bpf-next 6/6] bpf: Document BTF_KIND_FLOAT in btf.rst
  2021-02-19  2:25 ` [PATCH v2 bpf-next 6/6] bpf: Document BTF_KIND_FLOAT in btf.rst Ilya Leoshkevich
@ 2021-02-19  5:41   ` Yonghong Song
  2021-02-19 13:00     ` Ilya Leoshkevich
  0 siblings, 1 reply; 18+ messages in thread
From: Yonghong Song @ 2021-02-19  5:41 UTC (permalink / raw)
  To: Ilya Leoshkevich, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Arnaldo Carvalho de Melo
  Cc: John Fastabend, bpf, Heiko Carstens, Vasily Gorbik



On 2/18/21 6:25 PM, Ilya Leoshkevich wrote:
> Also document the expansion of the kind bitfield.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   Documentation/bpf/btf.rst | 17 +++++++++++++++--
>   1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/bpf/btf.rst b/Documentation/bpf/btf.rst
> index 44dc789de2b4..4f25c992d442 100644
> --- a/Documentation/bpf/btf.rst
> +++ b/Documentation/bpf/btf.rst
> @@ -84,6 +84,7 @@ sequentially and type id is assigned to each recognized type starting from id
>       #define BTF_KIND_FUNC_PROTO     13      /* Function Proto       */
>       #define BTF_KIND_VAR            14      /* Variable     */
>       #define BTF_KIND_DATASEC        15      /* Section      */
> +    #define BTF_KIND_FLOAT          16      /* Floating point       */
>   
>   Note that the type section encodes debug info, not just pure types.
>   ``BTF_KIND_FUNC`` is not a type, and it represents a defined subprogram.
> @@ -95,8 +96,8 @@ Each type contains the following common data::
>           /* "info" bits arrangement
>            * bits  0-15: vlen (e.g. # of struct's members)
>            * bits 16-23: unused
> -         * bits 24-27: kind (e.g. int, ptr, array...etc)
> -         * bits 28-30: unused
> +         * bits 24-28: kind (e.g. int, ptr, array...etc)
> +         * bits 29-30: unused
>            * bit     31: kind_flag, currently used by
>            *             struct, union and fwd
>            */
> @@ -452,6 +453,18 @@ map definition.
>     * ``offset``: the in-section offset of the variable
>     * ``size``: the size of the variable in bytes
>   
> +2.2.16 BTF_KIND_FLOAT
> +~~~~~~~~~~~~~~~~~~~~~
> +
> +``struct btf_type`` encoding requirement:
> + * ``name_off``: any valid offset
> + * ``info.kind_flag``: 0
> + * ``info.kind``: BTF_KIND_FLOAT
> + * ``info.vlen``: 0
> + * ``size``: the size of the float type in bytes.

I would be good to specify the allowed size in bytes 2, multiple of 4.
currently we do not have a maximum value, maybe 128. have a float type
something like 2^10 seems strange.

> +
> +No additional type data follow ``btf_type``.
> +
>   3. BTF Kernel API
>   *****************
>   
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 bpf-next 5/6] selftest/bpf: Add BTF_KIND_FLOAT tests
  2021-02-19  2:25 ` [PATCH v2 bpf-next 5/6] selftest/bpf: Add BTF_KIND_FLOAT tests Ilya Leoshkevich
  2021-02-19  5:38   ` Yonghong Song
@ 2021-02-19  5:44   ` Yonghong Song
  1 sibling, 0 replies; 18+ messages in thread
From: Yonghong Song @ 2021-02-19  5:44 UTC (permalink / raw)
  To: Ilya Leoshkevich, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Arnaldo Carvalho de Melo
  Cc: John Fastabend, bpf, Heiko Carstens, Vasily Gorbik



On 2/18/21 6:25 PM, Ilya Leoshkevich wrote:
> Test the good variants as well as the potential malformed ones.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   tools/testing/selftests/bpf/btf_helpers.c    |   4 +
>   tools/testing/selftests/bpf/prog_tests/btf.c | 122 +++++++++++++++++++
>   tools/testing/selftests/bpf/test_btf.h       |   3 +
>   3 files changed, 129 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/btf_helpers.c b/tools/testing/selftests/bpf/btf_helpers.c
> index 48f90490f922..b692e6ead9b5 100644
> --- a/tools/testing/selftests/bpf/btf_helpers.c
> +++ b/tools/testing/selftests/bpf/btf_helpers.c
> @@ -23,6 +23,7 @@ static const char * const btf_kind_str_mapping[] = {
>   	[BTF_KIND_FUNC_PROTO]	= "FUNC_PROTO",
>   	[BTF_KIND_VAR]		= "VAR",
>   	[BTF_KIND_DATASEC]	= "DATASEC",
> +	[BTF_KIND_FLOAT]	= "FLOAT",
>   };
>   
>   static const char *btf_kind_str(__u16 kind)
> @@ -173,6 +174,9 @@ int fprintf_btf_type_raw(FILE *out, const struct btf *btf, __u32 id)
>   		}
>   		break;
>   	}
> +	case BTF_KIND_FLOAT:
> +		fprintf(out, " size=%u", t->size);
> +		break;
>   	default:
>   		break;
>   	}
> diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
> index 6a7ee7420701..4be14d853cc3 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf.c
> @@ -3531,6 +3531,127 @@ static struct btf_raw_test raw_tests[] = {
>   	.max_entries = 1,
>   },
>   
> +{
> +	.descr = "float test #1, well-formed",
> +	.raw_types = {
> +		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),	/* [1] */
> +		BTF_TYPE_FLOAT_ENC(1, 2),			/* [2] */
> +		BTF_TYPE_FLOAT_ENC(10, 4),			/* [3] */
> +		BTF_TYPE_FLOAT_ENC(16, 8),			/* [4] */
> +		BTF_TYPE_FLOAT_ENC(23, 16),			/* [5] */
> +		BTF_STRUCT_ENC(35, 4, 32),			/* [6] */

The float and struct names can also use NAME_TBD to avoid potential
miss counting, I think. This also make it consistent with using NAME_TBD 
below.

> +		BTF_MEMBER_ENC(NAME_TBD, 2, 0),
> +		BTF_MEMBER_ENC(NAME_TBD, 3, 32),
> +		BTF_MEMBER_ENC(NAME_TBD, 4, 64),
> +		BTF_MEMBER_ENC(NAME_TBD, 5, 128),
> +		BTF_END_RAW,
> +	},
> +	BTF_STR_SEC("\0_Float16\0float\0double\0long_double\0floats"),
> +	.map_type = BPF_MAP_TYPE_ARRAY,
> +	.map_name = "float_type_check_btf",
> +	.key_size = sizeof(int),
> +	.value_size = 32,
> +	.key_type_id = 1,
> +	.value_type_id = 6,
> +	.max_entries = 1,
> +},
> +{
[...]

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 bpf-next 6/6] bpf: Document BTF_KIND_FLOAT in btf.rst
  2021-02-19  5:41   ` Yonghong Song
@ 2021-02-19 13:00     ` Ilya Leoshkevich
  2021-02-19 15:26       ` Yonghong Song
  0 siblings, 1 reply; 18+ messages in thread
From: Ilya Leoshkevich @ 2021-02-19 13:00 UTC (permalink / raw)
  To: Yonghong Song, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Arnaldo Carvalho de Melo
  Cc: John Fastabend, bpf, Heiko Carstens, Vasily Gorbik

On Thu, 2021-02-18 at 21:41 -0800, Yonghong Song wrote:
> 
> 
> On 2/18/21 6:25 PM, Ilya Leoshkevich wrote:
> > Also document the expansion of the kind bitfield.
> > 
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >   Documentation/bpf/btf.rst | 17 +++++++++++++++--
> >   1 file changed, 15 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/bpf/btf.rst b/Documentation/bpf/btf.rst
> > index 44dc789de2b4..4f25c992d442 100644
> > --- a/Documentation/bpf/btf.rst
> > +++ b/Documentation/bpf/btf.rst
> > @@ -84,6 +84,7 @@ sequentially and type id is assigned to each
> > recognized type starting from id
> >       #define BTF_KIND_FUNC_PROTO     13      /* Function
> > Proto       */
> >       #define BTF_KIND_VAR            14      /* Variable     */
> >       #define BTF_KIND_DATASEC        15      /* Section      */
> > +    #define BTF_KIND_FLOAT          16      /* Floating
> > point       */
> >   
> >   Note that the type section encodes debug info, not just pure
> > types.
> >   ``BTF_KIND_FUNC`` is not a type, and it represents a defined
> > subprogram.
> > @@ -95,8 +96,8 @@ Each type contains the following common data::
> >           /* "info" bits arrangement
> >            * bits  0-15: vlen (e.g. # of struct's members)
> >            * bits 16-23: unused
> > -         * bits 24-27: kind (e.g. int, ptr, array...etc)
> > -         * bits 28-30: unused
> > +         * bits 24-28: kind (e.g. int, ptr, array...etc)
> > +         * bits 29-30: unused
> >            * bit     31: kind_flag, currently used by
> >            *             struct, union and fwd
> >            */
> > @@ -452,6 +453,18 @@ map definition.
> >     * ``offset``: the in-section offset of the variable
> >     * ``size``: the size of the variable in bytes
> >   
> > +2.2.16 BTF_KIND_FLOAT
> > +~~~~~~~~~~~~~~~~~~~~~
> > +
> > +``struct btf_type`` encoding requirement:
> > + * ``name_off``: any valid offset
> > + * ``info.kind_flag``: 0
> > + * ``info.kind``: BTF_KIND_FLOAT
> > + * ``info.vlen``: 0
> > + * ``size``: the size of the float type in bytes.
> 
> I would be good to specify the allowed size in bytes 2, multiple of
> 4.
> currently we do not have a maximum value, maybe 128. have a float
> type
> something like 2^10 seems strange.

I tried to write this all down and realized it's simpler to enumerate
the allowed values: 2, 4, 8, 12 and 16. I don't think there are 32-byte
floats on any of the architectures supported by the kernel.


^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 bpf-next 6/6] bpf: Document BTF_KIND_FLOAT in btf.rst
  2021-02-19 13:00     ` Ilya Leoshkevich
@ 2021-02-19 15:26       ` Yonghong Song
  0 siblings, 0 replies; 18+ messages in thread
From: Yonghong Song @ 2021-02-19 15:26 UTC (permalink / raw)
  To: Ilya Leoshkevich, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Arnaldo Carvalho de Melo
  Cc: John Fastabend, bpf, Heiko Carstens, Vasily Gorbik



On 2/19/21 5:00 AM, Ilya Leoshkevich wrote:
> On Thu, 2021-02-18 at 21:41 -0800, Yonghong Song wrote:
>>
>>
>> On 2/18/21 6:25 PM, Ilya Leoshkevich wrote:
>>> Also document the expansion of the kind bitfield.
>>>
>>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>>> ---
>>>    Documentation/bpf/btf.rst | 17 +++++++++++++++--
>>>    1 file changed, 15 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/bpf/btf.rst b/Documentation/bpf/btf.rst
>>> index 44dc789de2b4..4f25c992d442 100644
>>> --- a/Documentation/bpf/btf.rst
>>> +++ b/Documentation/bpf/btf.rst
>>> @@ -84,6 +84,7 @@ sequentially and type id is assigned to each
>>> recognized type starting from id
>>>        #define BTF_KIND_FUNC_PROTO     13      /* Function
>>> Proto       */
>>>        #define BTF_KIND_VAR            14      /* Variable     */
>>>        #define BTF_KIND_DATASEC        15      /* Section      */
>>> +    #define BTF_KIND_FLOAT          16      /* Floating
>>> point       */
>>>    
>>>    Note that the type section encodes debug info, not just pure
>>> types.
>>>    ``BTF_KIND_FUNC`` is not a type, and it represents a defined
>>> subprogram.
>>> @@ -95,8 +96,8 @@ Each type contains the following common data::
>>>            /* "info" bits arrangement
>>>             * bits  0-15: vlen (e.g. # of struct's members)
>>>             * bits 16-23: unused
>>> -         * bits 24-27: kind (e.g. int, ptr, array...etc)
>>> -         * bits 28-30: unused
>>> +         * bits 24-28: kind (e.g. int, ptr, array...etc)
>>> +         * bits 29-30: unused
>>>             * bit     31: kind_flag, currently used by
>>>             *             struct, union and fwd
>>>             */
>>> @@ -452,6 +453,18 @@ map definition.
>>>      * ``offset``: the in-section offset of the variable
>>>      * ``size``: the size of the variable in bytes
>>>    
>>> +2.2.16 BTF_KIND_FLOAT
>>> +~~~~~~~~~~~~~~~~~~~~~
>>> +
>>> +``struct btf_type`` encoding requirement:
>>> + * ``name_off``: any valid offset
>>> + * ``info.kind_flag``: 0
>>> + * ``info.kind``: BTF_KIND_FLOAT
>>> + * ``info.vlen``: 0
>>> + * ``size``: the size of the float type in bytes.
>>
>> I would be good to specify the allowed size in bytes 2, multiple of
>> 4.
>> currently we do not have a maximum value, maybe 128. have a float
>> type
>> something like 2^10 seems strange.
> 
> I tried to write this all down and realized it's simpler to enumerate
> the allowed values: 2, 4, 8, 12 and 16. I don't think there are 32-byte
> floats on any of the architectures supported by the kernel.

This make senses. My above 128 means 128bits (sorry!), which is 16 
bytes, align with what you suggested.

> 

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support
  2021-02-19  4:22   ` Yonghong Song
@ 2021-02-19 23:01     ` Ilya Leoshkevich
  2021-02-19 23:35       ` Yonghong Song
  0 siblings, 1 reply; 18+ messages in thread
From: Ilya Leoshkevich @ 2021-02-19 23:01 UTC (permalink / raw)
  To: Yonghong Song, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Arnaldo Carvalho de Melo
  Cc: John Fastabend, bpf, Heiko Carstens, Vasily Gorbik

On Thu, 2021-02-18 at 20:22 -0800, Yonghong Song wrote:
> 
> 
> On 2/18/21 6:25 PM, Ilya Leoshkevich wrote:
> > 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(-)
> > 
> [...]
> > diff --git a/tools/lib/bpf/btf_dump.c b/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(struct
> > btf_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(struct btf_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 void btf_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 void btf_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(const struct
> > 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 bool btf_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 void bpf_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=4


^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support
  2021-02-19 23:01     ` Ilya Leoshkevich
@ 2021-02-19 23:35       ` Yonghong Song
  2021-02-19 23:43         ` Ilya Leoshkevich
  0 siblings, 1 reply; 18+ messages in thread
From: Yonghong Song @ 2021-02-19 23:35 UTC (permalink / raw)
  To: Ilya Leoshkevich, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Arnaldo Carvalho de Melo
  Cc: John Fastabend, bpf, Heiko Carstens, Vasily Gorbik



On 2/19/21 3:01 PM, Ilya Leoshkevich wrote:
> On Thu, 2021-02-18 at 20:22 -0800, Yonghong Song wrote:
>>
>>
>> On 2/18/21 6:25 PM, Ilya Leoshkevich wrote:
>>> 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(-)
>>>
>> [...]
>>> diff --git a/tools/lib/bpf/btf_dump.c b/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(struct
>>> btf_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(struct btf_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 void btf_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 void btf_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(const struct
>>> 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 bool btf_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 void bpf_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=4

good 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.

> 

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH v2 bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support
  2021-02-19 23:35       ` Yonghong Song
@ 2021-02-19 23:43         ` Ilya Leoshkevich
  0 siblings, 0 replies; 18+ messages in thread
From: Ilya Leoshkevich @ 2021-02-19 23:43 UTC (permalink / raw)
  To: Yonghong Song, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Arnaldo Carvalho de Melo
  Cc: John Fastabend, bpf, Heiko Carstens, Vasily Gorbik

On Fri, 2021-02-19 at 15:35 -0800, Yonghong Song wrote:
> 
> 
> On 2/19/21 3:01 PM, Ilya Leoshkevich wrote:
> > On Thu, 2021-02-18 at 20:22 -0800, Yonghong Song wrote:
> > > 
> > > 
> > > On 2/18/21 6:25 PM, Ilya Leoshkevich wrote:
> > > > 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(-)
> > > > 
> > > [...]
> > > > diff --git a/tools/lib/bpf/btf_dump.c
> > > > b/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(struct
> > > > btf_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(struct
> > > > btf_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 void
> > > > btf_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 void
> > > > btf_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(const
> > > > struct
> > > > 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 bool
> > > > btf_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 void
> > > > bpf_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=4
> 
> good 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!


^ permalink raw reply	[flat|nested] 18+ messages in thread
end of thread, other threads:[~2021-02-19 23:44 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-19  2:25 [PATCH v2 bpf-next 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
2021-02-19  2:25 ` [PATCH v2 bpf-next 1/6] bpf: Add BTF_KIND_FLOAT to uapi Ilya Leoshkevich
2021-02-19  2:25 ` [PATCH v2 bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support Ilya Leoshkevich
2021-02-19  4:22   ` Yonghong Song
2021-02-19 23:01     ` Ilya Leoshkevich
2021-02-19 23:35       ` Yonghong Song
2021-02-19 23:43         ` Ilya Leoshkevich
2021-02-19  2:25 ` [PATCH v2 bpf-next 3/6] tools/bpftool: " Ilya Leoshkevich
2021-02-19  2:25 ` [PATCH v2 bpf-next 4/6] bpf: " Ilya Leoshkevich
2021-02-19  4:04   ` kernel test robot
2021-02-19  4:13   ` kernel test robot
2021-02-19  2:25 ` [PATCH v2 bpf-next 5/6] selftest/bpf: Add BTF_KIND_FLOAT tests Ilya Leoshkevich
2021-02-19  5:38   ` Yonghong Song
2021-02-19  5:44   ` Yonghong Song
2021-02-19  2:25 ` [PATCH v2 bpf-next 6/6] bpf: Document BTF_KIND_FLOAT in btf.rst Ilya Leoshkevich
2021-02-19  5:41   ` Yonghong Song
2021-02-19 13:00     ` Ilya Leoshkevich
2021-02-19 15:26       ` Yonghong Song

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).