All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/6] Add BTF_KIND_FLOAT support
@ 2021-02-16  1:12 Ilya Leoshkevich
  2021-02-16  1:12 ` [PATCH bpf-next 1/6] bpf: Add BTF_KIND_FLOAT to uapi Ilya Leoshkevich
                   ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: Ilya Leoshkevich @ 2021-02-16  1:12 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Arnaldo Carvalho de Melo
  Cc: 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.

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                             | 66 ++++++++++++++-
 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                       | 29 ++++++-
 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 | 84 ++++++++++++++++++++
 tools/testing/selftests/bpf/test_btf.h       |  3 +
 15 files changed, 276 insertions(+), 9 deletions(-)

-- 
2.29.2


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH bpf-next 1/6] bpf: Add BTF_KIND_FLOAT to uapi
  2021-02-16  1:12 [PATCH bpf-next 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
@ 2021-02-16  1:12 ` Ilya Leoshkevich
  2021-02-16  1:12 ` [PATCH bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support Ilya Leoshkevich
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Ilya Leoshkevich @ 2021-02-16  1:12 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Arnaldo Carvalho de Melo
  Cc: 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] 19+ messages in thread

* [PATCH bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support
  2021-02-16  1:12 [PATCH bpf-next 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
  2021-02-16  1:12 ` [PATCH bpf-next 1/6] bpf: Add BTF_KIND_FLOAT to uapi Ilya Leoshkevich
@ 2021-02-16  1:12 ` Ilya Leoshkevich
  2021-02-17 20:58   ` John Fastabend
                     ` (2 more replies)
  2021-02-16  1:12 ` [PATCH bpf-next 3/6] tools/bpftool: " Ilya Leoshkevich
                   ` (3 subsequent siblings)
  5 siblings, 3 replies; 19+ messages in thread
From: Ilya Leoshkevich @ 2021-02-16  1:12 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Arnaldo Carvalho de Melo
  Cc: 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 equally-sized BTF_KIND_INTs 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          | 29 +++++++++++++++++++++-
 tools/lib/bpf/libbpf.map        |  5 ++++
 tools/lib/bpf/libbpf_internal.h |  2 ++
 6 files changed, 91 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..703988d68e73 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,15 +2387,17 @@ 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;
@@ -2445,6 +2450,9 @@ 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 INT */
+			t->info = BTF_INFO_ENC(BTF_KIND_FLOAT, 0, 0);
 		}
 	}
 }
@@ -3882,6 +3890,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 +4081,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 +4963,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 +5147,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] 19+ messages in thread

* [PATCH bpf-next 3/6] tools/bpftool: Add BTF_KIND_FLOAT support
  2021-02-16  1:12 [PATCH bpf-next 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
  2021-02-16  1:12 ` [PATCH bpf-next 1/6] bpf: Add BTF_KIND_FLOAT to uapi Ilya Leoshkevich
  2021-02-16  1:12 ` [PATCH bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support Ilya Leoshkevich
@ 2021-02-16  1:12 ` Ilya Leoshkevich
  2021-02-16  1:12 ` [PATCH bpf-next 4/6] bpf: " Ilya Leoshkevich
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Ilya Leoshkevich @ 2021-02-16  1:12 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Arnaldo Carvalho de Melo
  Cc: 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] 19+ messages in thread

* [PATCH bpf-next 4/6] bpf: Add BTF_KIND_FLOAT support
  2021-02-16  1:12 [PATCH bpf-next 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
                   ` (2 preceding siblings ...)
  2021-02-16  1:12 ` [PATCH bpf-next 3/6] tools/bpftool: " Ilya Leoshkevich
@ 2021-02-16  1:12 ` Ilya Leoshkevich
  2021-02-18  7:13   ` Yonghong Song
  2021-02-16  1:12 ` [PATCH bpf-next 5/6] selftest/bpf: Add BTF_KIND_FLOAT tests Ilya Leoshkevich
  2021-02-16  1:12 ` [PATCH bpf-next 6/6] bpf: Document BTF_KIND_FLOAT in btf.rst Ilya Leoshkevich
  5 siblings, 1 reply; 19+ messages in thread
From: Ilya Leoshkevich @ 2021-02-16  1:12 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Arnaldo Carvalho de Melo
  Cc: 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 | 66 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 64 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 2efeb5f4b343..6c73e5484409 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,64 @@ 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;
+	}
+
+	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 end_offset_bytes;
+	u64 end_offset_bits;
+	u64 offset_bits;
+	u64 size_bits;
+
+	size_bits = member_type->size * BITS_PER_BYTE;
+	offset_bits = member->offset;
+	end_offset_bits = offset_bits + size_bits;
+	end_offset_bytes = BITS_ROUNDUP_BYTES(end_offset_bits);
+
+	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 +3869,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] 19+ messages in thread

* [PATCH bpf-next 5/6] selftest/bpf: Add BTF_KIND_FLOAT tests
  2021-02-16  1:12 [PATCH bpf-next 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
                   ` (3 preceding siblings ...)
  2021-02-16  1:12 ` [PATCH bpf-next 4/6] bpf: " Ilya Leoshkevich
@ 2021-02-16  1:12 ` Ilya Leoshkevich
  2021-02-16  1:12 ` [PATCH bpf-next 6/6] bpf: Document BTF_KIND_FLOAT in btf.rst Ilya Leoshkevich
  5 siblings, 0 replies; 19+ messages in thread
From: Ilya Leoshkevich @ 2021-02-16  1:12 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Arnaldo Carvalho de Melo
  Cc: 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 | 84 ++++++++++++++++++++
 tools/testing/selftests/bpf/test_btf.h       |  3 +
 3 files changed, 91 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..a15a826e73b2 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf.c
@@ -3531,6 +3531,89 @@ 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, 30),			/* [6] */
+		BTF_MEMBER_ENC(NAME_TBD, 2, 0),
+		BTF_MEMBER_ENC(NAME_TBD, 3, 16),
+		BTF_MEMBER_ENC(NAME_TBD, 4, 48),
+		BTF_MEMBER_ENC(NAME_TBD, 5, 112),
+		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 = 30,
+	.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, 4),			/* [3] */
+		BTF_MEMBER_ENC(NAME_TBD, 2, 1),
+		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",
+},
+
 }; /* struct btf_raw_test raw_tests[] */
 
 static const char *get_next_str(const char *start, const char *end)
@@ -6632,6 +6715,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] 19+ messages in thread

* [PATCH bpf-next 6/6] bpf: Document BTF_KIND_FLOAT in btf.rst
  2021-02-16  1:12 [PATCH bpf-next 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
                   ` (4 preceding siblings ...)
  2021-02-16  1:12 ` [PATCH bpf-next 5/6] selftest/bpf: Add BTF_KIND_FLOAT tests Ilya Leoshkevich
@ 2021-02-16  1:12 ` Ilya Leoshkevich
  5 siblings, 0 replies; 19+ messages in thread
From: Ilya Leoshkevich @ 2021-02-16  1:12 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Arnaldo Carvalho de Melo
  Cc: 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] 19+ messages in thread

* RE: [PATCH bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support
  2021-02-16  1:12 ` [PATCH bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support Ilya Leoshkevich
@ 2021-02-17 20:58   ` John Fastabend
  2021-02-17 21:12     ` John Fastabend
  2021-02-18  6:58   ` Yonghong Song
  2021-02-18  7:16   ` Yonghong Song
  2 siblings, 1 reply; 19+ messages in thread
From: John Fastabend @ 2021-02-17 20:58 UTC (permalink / raw)
  To: Ilya Leoshkevich, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Yonghong Song, Arnaldo Carvalho de Melo
  Cc: bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich

Ilya Leoshkevich wrote:
> The logic follows that of BTF_KIND_INT most of the time. Sanitization
> replaces BTF_KIND_FLOATs with equally-sized BTF_KIND_INTs on older
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^
Does this match the code though?

> kernels.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---

[...]


> @@ -2445,6 +2450,9 @@ 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 INT */
> +			t->info = BTF_INFO_ENC(BTF_KIND_FLOAT, 0, 0);

Do we also need to encode the vlen here?

#define BTF_INFO_ENC(kind, kind_flag, vlen) \
	((!!(kind_flag) << 31) | ((kind) << 24) | ((vlen) & BTF_MAX_VLEN))

>  		}
>  	}
>  }
> @@ -3882,6 +3890,18 @@ static int probe_kern_btf_datasec(void)
>  					     strs, sizeof(strs)));
>  }

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support
  2021-02-17 20:58   ` John Fastabend
@ 2021-02-17 21:12     ` John Fastabend
  2021-02-17 21:28       ` Ilya Leoshkevich
  0 siblings, 1 reply; 19+ messages in thread
From: John Fastabend @ 2021-02-17 21:12 UTC (permalink / raw)
  To: John Fastabend, Ilya Leoshkevich, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Yonghong Song,
	Arnaldo Carvalho de Melo
  Cc: bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich

John Fastabend wrote:
> Ilya Leoshkevich wrote:
> > The logic follows that of BTF_KIND_INT most of the time. Sanitization
> > replaces BTF_KIND_FLOATs with equally-sized BTF_KIND_INTs on older
>                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Does this match the code though?
> 
> > kernels.
> > 
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> 
> [...]
> 
> 
> > @@ -2445,6 +2450,9 @@ 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 INT */
> > +			t->info = BTF_INFO_ENC(BTF_KIND_FLOAT, 0, 0);
> 
> Do we also need to encode the vlen here?

Sorry typo on my side, 't->size = ?' is what I was trying to point out.
Looks like its set in the other case where we replace VAR with INT.

> 
> #define BTF_INFO_ENC(kind, kind_flag, vlen) \
> 	((!!(kind_flag) << 31) | ((kind) << 24) | ((vlen) & BTF_MAX_VLEN))
> 
> >  		}
> >  	}
> >  }
> > @@ -3882,6 +3890,18 @@ static int probe_kern_btf_datasec(void)
> >  					     strs, sizeof(strs)));
> >  }



^ permalink raw reply	[flat|nested] 19+ messages in thread

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

On Wed, 2021-02-17 at 13:12 -0800, John Fastabend wrote:
> John Fastabend wrote:
> > Ilya Leoshkevich wrote:
> > > The logic follows that of BTF_KIND_INT most of the time.
> > > Sanitization
> > > replaces BTF_KIND_FLOATs with equally-sized BTF_KIND_INTs on
> > > older
> >                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > Does this match the code though?
> > 
> > > kernels.
> > > 
> > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > > ---
> > 
> > [...]
> > 
> > 
> > > @@ -2445,6 +2450,9 @@ 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 INT */
> > > +                       t->info = BTF_INFO_ENC(BTF_KIND_FLOAT, 0,
> > > 0);
> > 
> > Do we also need to encode the vlen here?
> 
> Sorry typo on my side, 't->size = ?' is what I was trying to point
> out.
> Looks like its set in the other case where we replace VAR with INT.

The idea is to have the size of the INT equal to the size of the FLOAT
that it replaces. I guess we can't do the same for VARs, because they
don't have the size field, and if we don't have DATASECs, then we can't
find the size of a VAR at all.


^ permalink raw reply	[flat|nested] 19+ messages in thread

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

Ilya Leoshkevich wrote:
> On Wed, 2021-02-17 at 13:12 -0800, John Fastabend wrote:
> > John Fastabend wrote:
> > > Ilya Leoshkevich wrote:
> > > > The logic follows that of BTF_KIND_INT most of the time.
> > > > Sanitization
> > > > replaces BTF_KIND_FLOATs with equally-sized BTF_KIND_INTs on
> > > > older
> > >                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > Does this match the code though?
> > > 
> > > > kernels.
> > > > 
> > > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > > > ---
> > > 
> > > [...]
> > > 
> > > 
> > > > @@ -2445,6 +2450,9 @@ 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 INT */
> > > > +                       t->info = BTF_INFO_ENC(BTF_KIND_FLOAT, 0,
> > > > 0);
> > > 
> > > Do we also need to encode the vlen here?
> > 
> > Sorry typo on my side, 't->size = ?' is what I was trying to point
> > out.
> > Looks like its set in the other case where we replace VAR with INT.
> 
> The idea is to have the size of the INT equal to the size of the FLOAT
> that it replaces. I guess we can't do the same for VARs, because they
> don't have the size field, and if we don't have DATASECs, then we can't
> find the size of a VAR at all.
> 

Right, but KINT_INT has some extra constraints that don't appear to be in
place for KIND_FLOAT. For example meta_check includes max size check. We
should check these when libbpf does conversion as well? Otherwise kernel
is going to give us an error that will be a bit hard to understand.

Also what I am I missing here. I use the writers to build a float,

 btf__add_float(btf, "new_float", 8);

This will create the btf_type struct approximately like this,

 btf_type t {
   .name = name_off; // points at my name
   .info = btf_type_info(BTF_KIND_FLOAT, 0, 0);
   .size = 8
 };

But if I create an int_type with btf__add_int(btf, "net_int", 8); I will
get a btf_type + __u32. When we do the conversion how do we skip the 
extra u32 setup?

 *(__u32 *)(t + 1) = (encoding << 24) | (byte_sz * 8);

Should we set this up on the conversion as well? Otherwise later steps
might try to read the __u32 piece to find some arbitrary memory?

.John

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support
  2021-02-16  1:12 ` [PATCH bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support Ilya Leoshkevich
  2021-02-17 20:58   ` John Fastabend
@ 2021-02-18  6:58   ` Yonghong Song
  2021-02-18 13:41     ` Ilya Leoshkevich
  2021-02-18  7:16   ` Yonghong Song
  2 siblings, 1 reply; 19+ messages in thread
From: Yonghong Song @ 2021-02-18  6:58 UTC (permalink / raw)
  To: Ilya Leoshkevich, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Arnaldo Carvalho de Melo
  Cc: bpf, Heiko Carstens, Vasily Gorbik



On 2/15/21 5:12 PM, Ilya Leoshkevich wrote:
> The logic follows that of BTF_KIND_INT most of the time. Sanitization
> replaces BTF_KIND_FLOATs with equally-sized BTF_KIND_INTs 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          | 29 +++++++++++++++++++++-
>   tools/lib/bpf/libbpf.map        |  5 ++++
>   tools/lib/bpf/libbpf_internal.h |  2 ++
>   6 files changed, 91 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;

Do we want to ensure byte_sz to be 2/4/8/16?
Currently, the int type supports 1/2/4/8/16.

In LLVM, the following are supported float types:

   case BuiltinType::Half:
   case BuiltinType::Float:
   case BuiltinType::LongDouble:
   case BuiltinType::Float16:
   case BuiltinType::BFloat16:
   case BuiltinType::Float128:
   case BuiltinType::Double:


> +
> +	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);
> +}
> +
[...]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH bpf-next 4/6] bpf: Add BTF_KIND_FLOAT support
  2021-02-16  1:12 ` [PATCH bpf-next 4/6] bpf: " Ilya Leoshkevich
@ 2021-02-18  7:13   ` Yonghong Song
  0 siblings, 0 replies; 19+ messages in thread
From: Yonghong Song @ 2021-02-18  7:13 UTC (permalink / raw)
  To: Ilya Leoshkevich, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Arnaldo Carvalho de Melo
  Cc: bpf, Heiko Carstens, Vasily Gorbik



On 2/15/21 5:12 PM, Ilya Leoshkevich wrote:
> 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 | 66 ++++++++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 64 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 2efeb5f4b343..6c73e5484409 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,64 @@ 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;
> +	}

I think we should enforce proper float size as well (2/4/8/16).

> +
> +	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 end_offset_bytes;
> +	u64 end_offset_bits;
> +	u64 offset_bits;
> +	u64 size_bits;
> +
> +	size_bits = member_type->size * BITS_PER_BYTE;
> +	offset_bits = member->offset;

float type cannot be used for bitfield, so offset here must of
a multiple of allowed alignment, i.e., min(ptr_size, member_type->size).
We should enforce here.

> +	end_offset_bits = offset_bits + size_bits;
> +	end_offset_bytes = BITS_ROUNDUP_BYTES(end_offset_bits);

There is no need to do BITS_ROUNDUP_BYTES if we enforce member->offset
as in the above.

> +
> +	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 +3869,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,
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support
  2021-02-16  1:12 ` [PATCH bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support Ilya Leoshkevich
  2021-02-17 20:58   ` John Fastabend
  2021-02-18  6:58   ` Yonghong Song
@ 2021-02-18  7:16   ` Yonghong Song
  2021-02-18 13:34     ` Ilya Leoshkevich
  2 siblings, 1 reply; 19+ messages in thread
From: Yonghong Song @ 2021-02-18  7:16 UTC (permalink / raw)
  To: Ilya Leoshkevich, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Arnaldo Carvalho de Melo
  Cc: bpf, Heiko Carstens, Vasily Gorbik



On 2/15/21 5:12 PM, Ilya Leoshkevich wrote:
> The logic follows that of BTF_KIND_INT most of the time. Sanitization
> replaces BTF_KIND_FLOATs with equally-sized BTF_KIND_INTs 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          | 29 +++++++++++++++++++++-
>   tools/lib/bpf/libbpf.map        |  5 ++++
>   tools/lib/bpf/libbpf_internal.h |  2 ++
>   6 files changed, 91 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);
[...]
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index d43cc3f29dae..703988d68e73 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,15 +2387,17 @@ 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;
> @@ -2445,6 +2450,9 @@ 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 INT */
> +			t->info = BTF_INFO_ENC(BTF_KIND_FLOAT, 0, 0);

You can replace float with a "pointer to void" type.

>   		}
>   	}
>   }
> @@ -3882,6 +3890,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 = {
[...]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support
  2021-02-18  7:16   ` Yonghong Song
@ 2021-02-18 13:34     ` Ilya Leoshkevich
  2021-02-18 17:29       ` Yonghong Song
  0 siblings, 1 reply; 19+ messages in thread
From: Ilya Leoshkevich @ 2021-02-18 13:34 UTC (permalink / raw)
  To: Yonghong Song, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Arnaldo Carvalho de Melo
  Cc: bpf, Heiko Carstens, Vasily Gorbik

On Wed, 2021-02-17 at 23:16 -0800, Yonghong Song wrote:
> On 2/15/21 5:12 PM, Ilya Leoshkevich wrote:
> > The logic follows that of BTF_KIND_INT most of the time.
> > Sanitization
> > replaces BTF_KIND_FLOATs with equally-sized BTF_KIND_INTs 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          | 29 +++++++++++++++++++++-
> >   tools/lib/bpf/libbpf.map        |  5 ++++
> >   tools/lib/bpf/libbpf_internal.h |  2 ++
> >   6 files changed, 91 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

[...]

> > @@ -2445,6 +2450,9 @@ 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 INT */
> > +                       t->info = BTF_INFO_ENC(BTF_KIND_FLOAT, 0,
> > 0);
> 
> You can replace float with a "pointer to void" type.

Wouldn't this cause problems with 32-bit floats on 64-bit machines?

[...]


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support
  2021-02-18  6:58   ` Yonghong Song
@ 2021-02-18 13:41     ` Ilya Leoshkevich
  2021-02-18 17:39       ` Yonghong Song
  0 siblings, 1 reply; 19+ messages in thread
From: Ilya Leoshkevich @ 2021-02-18 13:41 UTC (permalink / raw)
  To: Yonghong Song, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Arnaldo Carvalho de Melo
  Cc: bpf, Heiko Carstens, Vasily Gorbik

On Wed, 2021-02-17 at 22:58 -0800, Yonghong Song wrote:
> On 2/15/21 5:12 PM, Ilya Leoshkevich wrote:
> > The logic follows that of BTF_KIND_INT most of the time.
> > Sanitization
> > replaces BTF_KIND_FLOATs with equally-sized BTF_KIND_INTs 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          | 29 +++++++++++++++++++++-
> >   tools/lib/bpf/libbpf.map        |  5 ++++
> >   tools/lib/bpf/libbpf_internal.h |  2 ++
> >   6 files changed, 91 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

[...]

> > @@ -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;
> 
> Do we want to ensure byte_sz to be 2/4/8/16?
> Currently, the int type supports 1/2/4/8/16.
> 
> In LLVM, the following are supported float types:
> 
>    case BuiltinType::Half:
>    case BuiltinType::Float:
>    case BuiltinType::LongDouble:
>    case BuiltinType::Float16:
>    case BuiltinType::BFloat16:
>    case BuiltinType::Float128:
>    case BuiltinType::Double:

There can be 80-bit floats on x86:

#include <stdio.h>
int main() { printf("%zu\n", sizeof(long double)); }

prints 12 when compiled with -m32 (not 10 due to alignment I assume).

I guess this now completely kills the idea with sanitizing FLOATs to
equally-sized INTs...

[...]


^ permalink raw reply	[flat|nested] 19+ messages in thread

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

On Wed, 2021-02-17 at 17:26 -0800, John Fastabend wrote:
> Ilya Leoshkevich wrote:
> > On Wed, 2021-02-17 at 13:12 -0800, John Fastabend wrote:
> > > John Fastabend wrote:
> > > > Ilya Leoshkevich wrote:
> > > > > The logic follows that of BTF_KIND_INT most of the time.
> > > > > Sanitization
> > > > > replaces BTF_KIND_FLOATs with equally-sized BTF_KIND_INTs on
> > > > > older
> > > >                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > > Does this match the code though?
> > > > 
> > > > > kernels.
> > > > > 
> > > > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > > > > ---
> > > > 
> > > > [...]
> > > > 
> > > > 
> > > > > @@ -2445,6 +2450,9 @@ 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 INT */
> > > > > +                       t->info =
> > > > > BTF_INFO_ENC(BTF_KIND_FLOAT, 0,
> > > > > 0);
> > > > 
> > > > Do we also need to encode the vlen here?
> > > 
> > > Sorry typo on my side, 't->size = ?' is what I was trying to
> > > point
> > > out.
> > > Looks like its set in the other case where we replace VAR with
> > > INT.
> > 
> > The idea is to have the size of the INT equal to the size of the
> > FLOAT
> > that it replaces. I guess we can't do the same for VARs, because
> > they
> > don't have the size field, and if we don't have DATASECs, then we
> > can't
> > find the size of a VAR at all.
> > 
> 
> Right, but KINT_INT has some extra constraints that don't appear to
> be in
> place for KIND_FLOAT. For example meta_check includes max size check.
> We
> should check these when libbpf does conversion as well? Otherwise
> kernel
> is going to give us an error that will be a bit hard to understand.

Yeah, apparently floats can have non-power-of-2 sizes, which kills the
idea with such a replacement. Maybe we should do exactly the same thing
as we do for VARs after all.

> Also what I am I missing here. I use the writers to build a float,
> 
>  btf__add_float(btf, "new_float", 8);
> 
> This will create the btf_type struct approximately like this,
> 
>  btf_type t {
>    .name = name_off; // points at my name
>    .info = btf_type_info(BTF_KIND_FLOAT, 0, 0);
>    .size = 8
>  };
> 
> But if I create an int_type with btf__add_int(btf, "net_int", 8); I
> will
> get a btf_type + __u32. When we do the conversion how do we skip the 
> extra u32 setup?
> 
>  *(__u32 *)(t + 1) = (encoding << 24) | (byte_sz * 8);
> 
> Should we set this up on the conversion as well? Otherwise later
> steps
> might try to read the __u32 piece to find some arbitrary memory?

Ah, you are absolutely right. I was hoping that e.g. btf_get_raw_data()
would clean that up, but turns out it doesn't do that. Seems like I'll
have to implement this myself.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support
  2021-02-18 13:34     ` Ilya Leoshkevich
@ 2021-02-18 17:29       ` Yonghong Song
  0 siblings, 0 replies; 19+ messages in thread
From: Yonghong Song @ 2021-02-18 17:29 UTC (permalink / raw)
  To: Ilya Leoshkevich, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Arnaldo Carvalho de Melo
  Cc: bpf, Heiko Carstens, Vasily Gorbik



On 2/18/21 5:34 AM, Ilya Leoshkevich wrote:
> On Wed, 2021-02-17 at 23:16 -0800, Yonghong Song wrote:
>> On 2/15/21 5:12 PM, Ilya Leoshkevich wrote:
>>> The logic follows that of BTF_KIND_INT most of the time.
>>> Sanitization
>>> replaces BTF_KIND_FLOATs with equally-sized BTF_KIND_INTs 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          | 29 +++++++++++++++++++++-
>>>    tools/lib/bpf/libbpf.map        |  5 ++++
>>>    tools/lib/bpf/libbpf_internal.h |  2 ++
>>>    6 files changed, 91 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
> 
> [...]
> 
>>> @@ -2445,6 +2450,9 @@ 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 INT */
>>> +                       t->info = BTF_INFO_ENC(BTF_KIND_FLOAT, 0,
>>> 0);
>>
>> You can replace float with a "pointer to void" type.
> 
> Wouldn't this cause problems with 32-bit floats on 64-bit machines?

Oh, yes. You are right. I am just thinking of standalone float type, but
obviously it could be struct/union member which makes it very important
to keep the sanatized type having the same size.

So replacing float with "point to void" won't work as you suggested.
Looks like INT is the best candidate to replace, another is CHAR array.
They do not match total size though. Maybe one of modifier type will
be a good choice. For example, you can replace float with a
BTF_KIND_CONST type and the base type for BTF_KIND_CONST type
is an int type with the same size of float and you have to
create that int type somewhere. In power-of-2 cases, it is possible
this type already exists.

> 
> [...]
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support
  2021-02-18 13:41     ` Ilya Leoshkevich
@ 2021-02-18 17:39       ` Yonghong Song
  0 siblings, 0 replies; 19+ messages in thread
From: Yonghong Song @ 2021-02-18 17:39 UTC (permalink / raw)
  To: Ilya Leoshkevich, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Arnaldo Carvalho de Melo
  Cc: bpf, Heiko Carstens, Vasily Gorbik



On 2/18/21 5:41 AM, Ilya Leoshkevich wrote:
> On Wed, 2021-02-17 at 22:58 -0800, Yonghong Song wrote:
>> On 2/15/21 5:12 PM, Ilya Leoshkevich wrote:
>>> The logic follows that of BTF_KIND_INT most of the time.
>>> Sanitization
>>> replaces BTF_KIND_FLOATs with equally-sized BTF_KIND_INTs 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          | 29 +++++++++++++++++++++-
>>>    tools/lib/bpf/libbpf.map        |  5 ++++
>>>    tools/lib/bpf/libbpf_internal.h |  2 ++
>>>    6 files changed, 91 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
> 
> [...]
> 
>>> @@ -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;
>>
>> Do we want to ensure byte_sz to be 2/4/8/16?
>> Currently, the int type supports 1/2/4/8/16.
>>
>> In LLVM, the following are supported float types:
>>
>>     case BuiltinType::Half:
>>     case BuiltinType::Float:
>>     case BuiltinType::LongDouble:
>>     case BuiltinType::Float16:
>>     case BuiltinType::BFloat16:
>>     case BuiltinType::Float128:
>>     case BuiltinType::Double:
> 
> There can be 80-bit floats on x86:
> 
> #include <stdio.h>
> int main() { printf("%zu\n", sizeof(long double)); }
> 
> prints 12 when compiled with -m32 (not 10 due to alignment I assume).

Wow, I do not know we have long double float size = 12 for m32.
In this case, maybe we can enforce float size of 2 and multiple of 4?
I still like some enforcement, arbitrary float size seems wrong to me.

> 
> I guess this now completely kills the idea with sanitizing FLOATs to
> equally-sized INTs...

My previous email suggests to use "const int" for sanitization.
In kernel, we did not enforce the "int" type size as long as it
is equal to or greater than the one represented by bits.

         if (BITS_ROUNDUP_BYTES(nr_bits) > t->size) {
                 btf_verifier_log_type(env, t, "nr_bits exceeds type_size");
                 return -EINVAL;
         }

But in libbpf, we do want to the int size to be power-of-2.

         /* byte_sz must be power of 2 */
         if (!byte_sz || (byte_sz & (byte_sz - 1)) || byte_sz > 16)
                 return -EINVAL;

When people dump in-kernel sanitized btf and they may see
an int with size 12, which will be weird.

So maybe "const char_array" is the best choice here.

> 
> [...]
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2021-02-18 18:49 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-16  1:12 [PATCH bpf-next 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
2021-02-16  1:12 ` [PATCH bpf-next 1/6] bpf: Add BTF_KIND_FLOAT to uapi Ilya Leoshkevich
2021-02-16  1:12 ` [PATCH bpf-next 2/6] libbpf: Add BTF_KIND_FLOAT support Ilya Leoshkevich
2021-02-17 20:58   ` John Fastabend
2021-02-17 21:12     ` John Fastabend
2021-02-17 21:28       ` Ilya Leoshkevich
2021-02-18  1:26         ` John Fastabend
2021-02-18 13:57           ` Ilya Leoshkevich
2021-02-18  6:58   ` Yonghong Song
2021-02-18 13:41     ` Ilya Leoshkevich
2021-02-18 17:39       ` Yonghong Song
2021-02-18  7:16   ` Yonghong Song
2021-02-18 13:34     ` Ilya Leoshkevich
2021-02-18 17:29       ` Yonghong Song
2021-02-16  1:12 ` [PATCH bpf-next 3/6] tools/bpftool: " Ilya Leoshkevich
2021-02-16  1:12 ` [PATCH bpf-next 4/6] bpf: " Ilya Leoshkevich
2021-02-18  7:13   ` Yonghong Song
2021-02-16  1:12 ` [PATCH bpf-next 5/6] selftest/bpf: Add BTF_KIND_FLOAT tests Ilya Leoshkevich
2021-02-16  1:12 ` [PATCH bpf-next 6/6] bpf: Document BTF_KIND_FLOAT in btf.rst Ilya Leoshkevich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.