bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC 0/6] Add BTF_KIND_FLOAT support
@ 2021-02-10  3:03 Ilya Leoshkevich
  2021-02-10  3:03 ` [PATCH RFC 1/6] bpf: Add BTF_KIND_FLOAT to uapi Ilya Leoshkevich
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Ilya Leoshkevich @ 2021-02-10  3:03 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-v1
* https://reviews.llvm.org/D83289

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

There is also an open question: should we go forward with
BTF_KIND_FLOAT, or should this be merely a BTF_KIND_INT encoding? The
argument for BTF_KIND_FLOAT is that it's more explicit and therefore
less prone to unintentional mixups. The argument for BTF_KIND_INT
encoding is that there would be less code and the overall integration
process would be simpler.

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                    |  19 ++-
 include/uapi/linux/btf.h                     |  10 +-
 kernel/bpf/btf.c                             | 101 ++++++++++++-
 tools/bpf/bpftool/btf.c                      |  13 ++
 tools/bpf/bpftool/btf_dumper.c               |   1 +
 tools/include/uapi/linux/btf.h               |  10 +-
 tools/lib/bpf/btf.c                          |  85 ++++++++---
 tools/lib/bpf/btf.h                          |  13 ++
 tools/lib/bpf/btf_dump.c                     |   4 +
 tools/lib/bpf/libbpf.c                       |  32 +++-
 tools/lib/bpf/libbpf.map                     |   5 +
 tools/lib/bpf/libbpf_internal.h              |   4 +
 tools/testing/selftests/bpf/btf_helpers.c    |   4 +
 tools/testing/selftests/bpf/prog_tests/btf.c | 145 +++++++++++++++++++
 tools/testing/selftests/bpf/test_btf.h       |   5 +
 15 files changed, 418 insertions(+), 33 deletions(-)

-- 
2.29.2


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

* [PATCH RFC 1/6] bpf: Add BTF_KIND_FLOAT to uapi
  2021-02-10  3:03 [PATCH RFC 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
@ 2021-02-10  3:03 ` Ilya Leoshkevich
  2021-02-11  0:19   ` Andrii Nakryiko
  2021-02-10  3:03 ` [PATCH RFC 2/6] libbpf: Add BTF_KIND_FLOAT support Ilya Leoshkevich
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Ilya Leoshkevich @ 2021-02-10  3:03 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, expand the kind bitfield, add a macro for
parsing the additional u32.

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

diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
index 5a667107ad2c..e713430cb033 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
@@ -169,4 +170,9 @@ struct btf_var_secinfo {
 	__u32	size;
 };
 
+/* BTF_KIND_FLOAT is followed by a u32 and the following
+ * is the 32 bits arrangement:
+ */
+#define BTF_FLOAT_BITS(VAL)	((VAL)  & 0x000000ff)
+
 #endif /* _UAPI__LINUX_BTF_H__ */
diff --git a/tools/include/uapi/linux/btf.h b/tools/include/uapi/linux/btf.h
index 5a667107ad2c..e713430cb033 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
@@ -169,4 +170,9 @@ struct btf_var_secinfo {
 	__u32	size;
 };
 
+/* BTF_KIND_FLOAT is followed by a u32 and the following
+ * is the 32 bits arrangement:
+ */
+#define BTF_FLOAT_BITS(VAL)	((VAL)  & 0x000000ff)
+
 #endif /* _UAPI__LINUX_BTF_H__ */
-- 
2.29.2


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

* [PATCH RFC 2/6] libbpf: Add BTF_KIND_FLOAT support
  2021-02-10  3:03 [PATCH RFC 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
  2021-02-10  3:03 ` [PATCH RFC 1/6] bpf: Add BTF_KIND_FLOAT to uapi Ilya Leoshkevich
@ 2021-02-10  3:03 ` Ilya Leoshkevich
  2021-02-11  0:15   ` Andrii Nakryiko
  2021-02-10  3:03 ` [PATCH RFC 3/6] tools/bpftool: " Ilya Leoshkevich
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Ilya Leoshkevich @ 2021-02-10  3:03 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, some functions
are even unified to work on both. 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             | 85 +++++++++++++++++++++++----------
 tools/lib/bpf/btf.h             | 13 +++++
 tools/lib/bpf/btf_dump.c        |  4 ++
 tools/lib/bpf/libbpf.c          | 32 ++++++++++++-
 tools/lib/bpf/libbpf.map        |  5 ++
 tools/lib/bpf/libbpf_internal.h |  4 ++
 6 files changed, 118 insertions(+), 25 deletions(-)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index d9c10830d749..0cc91e94f7c3 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -293,6 +293,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);
@@ -340,6 +341,7 @@ static int btf_bswap_type_rest(struct btf_type *t)
 	case BTF_KIND_FUNC:
 		return 0;
 	case BTF_KIND_INT:
+	case BTF_KIND_FLOAT:
 		*(__u32 *)(t + 1) = bswap_32(*(__u32 *)(t + 1));
 		return 0;
 	case BTF_KIND_ENUM:
@@ -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);
@@ -1707,16 +1711,8 @@ static int btf_commit_type(struct btf *btf, int data_sz)
 	return btf->start_id + btf->nr_types - 1;
 }
 
-/*
- * Append new BTF_KIND_INT type with:
- *   - *name* - non-empty, non-NULL type name;
- *   - *sz* - power-of-2 (1, 2, 4, ..) size of the type, in bytes;
- *   - encoding is a combination of BTF_INT_SIGNED, BTF_INT_CHAR, BTF_INT_BOOL.
- * Returns:
- *   - >0, type ID of newly added BTF type;
- *   - <0, on error.
- */
-int btf__add_int(struct btf *btf, const char *name, size_t byte_sz, int encoding)
+static int btf_add_int_float(struct btf *btf, int kind, const char *name,
+			     size_t byte_sz, int encoding)
 {
 	struct btf_type *t;
 	int sz, name_off;
@@ -1724,10 +1720,13 @@ int btf__add_int(struct btf *btf, const char *name, size_t byte_sz, int encoding
 	/* non-empty name */
 	if (!name || !name[0])
 		return -EINVAL;
-	/* byte_sz must be power of 2 */
-	if (!byte_sz || (byte_sz & (byte_sz - 1)) || byte_sz > 16)
+	/* BTF_KIND_INT's byte_sz must be power of 2 */
+	if (!byte_sz ||
+	    (kind == BTF_KIND_INT && (byte_sz & (byte_sz - 1))) ||
+	    byte_sz > 16)
 		return -EINVAL;
-	if (encoding & ~(BTF_INT_SIGNED | BTF_INT_CHAR | BTF_INT_BOOL))
+	if (kind == BTF_KIND_INT &&
+	    (encoding & ~(BTF_INT_SIGNED | BTF_INT_CHAR | BTF_INT_BOOL)))
 		return -EINVAL;
 
 	/* deconstruct BTF, if necessary, and invalidate raw_data */
@@ -1748,14 +1747,35 @@ int btf__add_int(struct btf *btf, const char *name, size_t byte_sz, int encoding
 		return name_off;
 
 	t->name_off = name_off;
-	t->info = btf_type_info(BTF_KIND_INT, 0, 0);
+	t->info = btf_type_info(kind, 0, 0);
 	t->size = byte_sz;
-	/* set INT info, we don't allow setting legacy bit offset/size */
-	*(__u32 *)(t + 1) = (encoding << 24) | (byte_sz * 8);
+	if (kind == BTF_KIND_INT)
+		/* set INT info, we don't allow setting legacy bit offset/size
+		 */
+		*(__u32 *)(t + 1) = (encoding << 24) | (byte_sz * 8);
+	else if (kind == BTF_KIND_FLOAT)
+		/* set FLOAT info */
+		*(__u32 *)(t + 1) = byte_sz * 8;
+	else
+		return -EINVAL;
 
 	return btf_commit_type(btf, sz);
 }
 
+/*
+ * Append new BTF_KIND_INT type with:
+ *   - *name* - non-empty, non-NULL type name;
+ *   - *sz* - power-of-2 (1, 2, 4, ..) size of the type, in bytes;
+ *   - encoding is a combination of BTF_INT_SIGNED, BTF_INT_CHAR, BTF_INT_BOOL.
+ * Returns:
+ *   - >0, type ID of newly added BTF type;
+ *   - <0, on error.
+ */
+int btf__add_int(struct btf *btf, const char *name, size_t byte_sz, int encoding)
+{
+	return btf_add_int_float(btf, BTF_KIND_INT, name, byte_sz, encoding);
+}
+
 /* it's completely legal to append BTF types with type IDs pointing forward to
  * types that haven't been appended yet, so we only make sure that id looks
  * sane, we can't guarantee that ID will always be valid
@@ -2373,6 +2393,19 @@ 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)
+{
+	return btf_add_int_float(btf, BTF_KIND_FLOAT, name, byte_sz, 0);
+}
+
 /*
  * Append new data section variable information entry for current DATASEC type:
  *   - *var_type_id* - type ID, describing type of the variable;
@@ -3351,8 +3384,8 @@ static bool btf_equal_common(struct btf_type *t1, struct btf_type *t2)
 	       t1->size == t2->size;
 }
 
-/* Calculate type signature hash of INT. */
-static long btf_hash_int(struct btf_type *t)
+/* Calculate type signature hash of an INT or a FLOAT. */
+static long btf_hash_int_float(struct btf_type *t)
 {
 	__u32 info = *(__u32 *)(t + 1);
 	long h;
@@ -3362,8 +3395,8 @@ static long btf_hash_int(struct btf_type *t)
 	return h;
 }
 
-/* Check structural equality of two INTs. */
-static bool btf_equal_int(struct btf_type *t1, struct btf_type *t2)
+/* Check structural equality of two INTs or two FLOATs. */
+static bool btf_equal_int_float(struct btf_type *t1, struct btf_type *t2)
 {
 	__u32 info1, info2;
 
@@ -3629,7 +3662,8 @@ static int btf_dedup_prep(struct btf_dedup *d)
 			h = btf_hash_common(t);
 			break;
 		case BTF_KIND_INT:
-			h = btf_hash_int(t);
+		case BTF_KIND_FLOAT:
+			h = btf_hash_int_float(t);
 			break;
 		case BTF_KIND_ENUM:
 			h = btf_hash_enum(t);
@@ -3687,11 +3721,12 @@ static int btf_dedup_prim_type(struct btf_dedup *d, __u32 type_id)
 		return 0;
 
 	case BTF_KIND_INT:
-		h = btf_hash_int(t);
+	case BTF_KIND_FLOAT:
+		h = btf_hash_int_float(t);
 		for_each_dedup_cand(d, hash_entry, h) {
 			cand_id = (__u32)(long)hash_entry->value;
 			cand = btf_type_by_id(d->btf, cand_id);
-			if (btf_equal_int(t, cand)) {
+			if (btf_equal_int_float(t, cand)) {
 				new_id = cand_id;
 				break;
 			}
@@ -3974,7 +4009,8 @@ static int btf_dedup_is_equiv(struct btf_dedup *d, __u32 cand_id,
 
 	switch (cand_kind) {
 	case BTF_KIND_INT:
-		return btf_equal_int(cand_type, canon_type);
+	case BTF_KIND_FLOAT:
+		return btf_equal_int_float(cand_type, canon_type);
 
 	case BTF_KIND_ENUM:
 		if (d->opts.dont_resolve_fwds)
@@ -4479,6 +4515,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..0f38bdc375bc 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));
@@ -362,6 +370,11 @@ btf_var_secinfos(const struct btf_type *t)
 	return (struct btf_var_secinfo *)(t + 1);
 }
 
+static inline __u8 btf_float_bits(const struct btf_type *t)
+{
+	return BTF_FLOAT_BITS(*(__u32 *)(t + 1));
+}
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif
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 2abbc3800568..ae1cdab156d6 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,12 @@ 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 */
+			__u8 nr_bits = btf_float_bits(t);
+
+			t->info = BTF_INFO_ENC(BTF_KIND_INT, 0, 0);
+			*(int *)(t + 1) = BTF_INT_ENC(0, 0, nr_bits);
 		}
 	}
 }
@@ -3882,6 +3893,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, 32, 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 +4084,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 +4966,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 btf_float_bits(local_type) == btf_float_bits(targ_type);
 	default:
 		pr_warn("unexpected kind %d relocated, local [%d], target [%d]\n",
 			btf_kind(local_type), local_id, targ_id);
@@ -5122,6 +5150,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 btf_float_bits(local_type) == btf_float_bits(targ_type);
 	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..6e440b88c010 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -31,6 +31,10 @@
 #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_FLOAT_ENC(nr_bits) nr_bits
+#define BTF_TYPE_FLOAT_ENC(name, bits, sz)				\
+	BTF_TYPE_ENC(name, BTF_INFO_ENC(BTF_KIND_FLOAT, 0, 0), sz),	\
+	BTF_FLOAT_ENC(bits)
 
 #ifndef likely
 #define likely(x) __builtin_expect(!!(x), 1)
-- 
2.29.2


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

* [PATCH RFC 3/6] tools/bpftool: Add BTF_KIND_FLOAT support
  2021-02-10  3:03 [PATCH RFC 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
  2021-02-10  3:03 ` [PATCH RFC 1/6] bpf: Add BTF_KIND_FLOAT to uapi Ilya Leoshkevich
  2021-02-10  3:03 ` [PATCH RFC 2/6] libbpf: Add BTF_KIND_FLOAT support Ilya Leoshkevich
@ 2021-02-10  3:03 ` Ilya Leoshkevich
  2021-02-10  3:03 ` [PATCH RFC 4/6] bpf: " Ilya Leoshkevich
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Ilya Leoshkevich @ 2021-02-10  3:03 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        | 13 +++++++++++++
 tools/bpf/bpftool/btf_dumper.c |  1 +
 2 files changed, 14 insertions(+)

diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
index fe9e7b3a4b50..7e72527b0409 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,18 @@ static int dump_btf_type(const struct btf *btf, __u32 id,
 			jsonw_end_array(w);
 		break;
 	}
+	case BTF_KIND_FLOAT: {
+		__u32 v = *(__u32 *)(t + 1);
+
+		if (json_output) {
+			jsonw_uint_field(w, "size", t->size);
+			jsonw_uint_field(w, "nr_bits", BTF_FLOAT_BITS(v));
+		} else {
+			printf(" size=%u nr_bits=%u",
+			       t->size, BTF_FLOAT_BITS(v));
+		}
+		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] 13+ messages in thread

* [PATCH RFC 4/6] bpf: Add BTF_KIND_FLOAT support
  2021-02-10  3:03 [PATCH RFC 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
                   ` (2 preceding siblings ...)
  2021-02-10  3:03 ` [PATCH RFC 3/6] tools/bpftool: " Ilya Leoshkevich
@ 2021-02-10  3:03 ` Ilya Leoshkevich
  2021-02-11  0:20   ` Andrii Nakryiko
  2021-02-10  3:03 ` [PATCH RFC 5/6] selftest/bpf: Add BTF_KIND_FLOAT tests Ilya Leoshkevich
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Ilya Leoshkevich @ 2021-02-10  3:03 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 | 101 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 99 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 756a93f534b6..6f27b7b59d77 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -173,8 +173,9 @@
 #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_FLOAT_MASK 0x000000ff
 #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 +281,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 +576,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;
 	}
 
@@ -614,6 +617,11 @@ static const struct btf_var *btf_type_var(const struct btf_type *t)
 	return (const struct btf_var *)(t + 1);
 }
 
+static u32 btf_type_float(const struct btf_type *t)
+{
+	return *(u32 *)(t + 1);
+}
+
 static const struct btf_kind_operations *btf_type_ops(const struct btf_type *t)
 {
 	return kind_ops[BTF_INFO_KIND(t->info)];
@@ -1704,6 +1712,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 +1858,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 +3684,93 @@ 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)
+{
+	u32 float_data, nr_bits, meta_needed = sizeof(float_data);
+
+	if (meta_left < meta_needed) {
+		btf_verifier_log_basic(env, t,
+				       "meta_left:%u meta_needed:%u",
+				       meta_left, meta_needed);
+		return -EINVAL;
+	}
+
+	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;
+	}
+
+	float_data = btf_type_float(t);
+	if (float_data & ~BTF_FLOAT_MASK) {
+		btf_verifier_log_basic(env, t, "Invalid float_data:%x",
+				       float_data);
+		return -EINVAL;
+	}
+
+	nr_bits = BTF_FLOAT_BITS(float_data);
+
+	if (BITS_ROUNDUP_BYTES(nr_bits) > t->size) {
+		btf_verifier_log_type(env, t, "nr_bits exceeds type_size");
+		return -EINVAL;
+	}
+
+	btf_verifier_log_type(env, t, NULL);
+
+	return meta_needed;
+}
+
+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;
+	u32 float_data;
+	u64 size_bits;
+
+	float_data = btf_type_float(member_type);
+	size_bits = BTF_FLOAT_BITS(float_data);
+	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)
+{
+	int float_data = btf_type_float(t);
+
+	btf_verifier_log(env,
+			 "size=%u nr_bits=%u",
+			 t->size, BTF_FLOAT_BITS(float_data));
+}
+
+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 +3904,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] 13+ messages in thread

* [PATCH RFC 5/6] selftest/bpf: Add BTF_KIND_FLOAT tests
  2021-02-10  3:03 [PATCH RFC 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
                   ` (3 preceding siblings ...)
  2021-02-10  3:03 ` [PATCH RFC 4/6] bpf: " Ilya Leoshkevich
@ 2021-02-10  3:03 ` Ilya Leoshkevich
  2021-02-10  3:03 ` [PATCH RFC 6/6] bpf: Document BTF_KIND_FLOAT in btf.rst Ilya Leoshkevich
  2021-02-11  1:31 ` [PATCH RFC 0/6] Add BTF_KIND_FLOAT support Andrii Nakryiko
  6 siblings, 0 replies; 13+ messages in thread
From: Ilya Leoshkevich @ 2021-02-10  3:03 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 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 | 145 +++++++++++++++++++
 tools/testing/selftests/bpf/test_btf.h       |   5 +
 3 files changed, 154 insertions(+)

diff --git a/tools/testing/selftests/bpf/btf_helpers.c b/tools/testing/selftests/bpf/btf_helpers.c
index 48f90490f922..179ec89c4277 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 nr_bits=%u", t->size, btf_float_bits(t));
+		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..d350577ee88a 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf.c
@@ -3531,6 +3531,150 @@ 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, 16, 2),			/* [2] */
+		BTF_TYPE_FLOAT_ENC(10, 32, 4),			/* [3] */
+		BTF_TYPE_FLOAT_ENC(16, 64, 8),			/* [4] */
+		BTF_TYPE_FLOAT_ENC(23, 128, 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, truncated",
+	.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, 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 = "meta_left:0 meta_needed:4",
+},
+{
+	.descr = "float test #3, 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),
+		BTF_FLOAT_ENC(32),
+								/* [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 #4, 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),
+		BTF_FLOAT_ENC(32),
+								/* [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 #5, invalid float_data",
+	.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, 0), 4),
+		BTF_FLOAT_ENC(32) | 0x100,
+								/* [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 float_data:120",
+},
+{
+	.descr = "float test #6, invalid nr_bits",
+	.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, 0), 4),
+		BTF_FLOAT_ENC(64),
+								/* [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 = "nr_bits exceeds type_size",
+},
+{
+	.descr = "float test #7, member does not fit",
+	.raw_types = {
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),	/* [1] */
+		BTF_TYPE_FLOAT_ENC(1, 32, 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 +6776,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..0585b4e31d6e 100644
--- a/tools/testing/selftests/bpf/test_btf.h
+++ b/tools/testing/selftests/bpf/test_btf.h
@@ -66,4 +66,9 @@
 #define BTF_FUNC_ENC(name, func_proto) \
 	BTF_TYPE_ENC(name, BTF_INFO_ENC(BTF_KIND_FUNC, 0, 0), func_proto)
 
+#define BTF_FLOAT_ENC(nr_bits) nr_bits
+#define BTF_TYPE_FLOAT_ENC(name, bits, sz)				\
+	BTF_TYPE_ENC(name, BTF_INFO_ENC(BTF_KIND_FLOAT, 0, 0), sz),	\
+	BTF_FLOAT_ENC(bits)
+
 #endif /* _TEST_BTF_H */
-- 
2.29.2


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

* [PATCH RFC 6/6] bpf: Document BTF_KIND_FLOAT in btf.rst
  2021-02-10  3:03 [PATCH RFC 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
                   ` (4 preceding siblings ...)
  2021-02-10  3:03 ` [PATCH RFC 5/6] selftest/bpf: Add BTF_KIND_FLOAT tests Ilya Leoshkevich
@ 2021-02-10  3:03 ` Ilya Leoshkevich
  2021-02-11  1:31 ` [PATCH RFC 0/6] Add BTF_KIND_FLOAT support Andrii Nakryiko
  6 siblings, 0 replies; 13+ messages in thread
From: Ilya Leoshkevich @ 2021-02-10  3:03 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 description is based on BTF_KIND_INT. Also document the expansion
of the kind bitfield.

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

diff --git a/Documentation/bpf/btf.rst b/Documentation/bpf/btf.rst
index 44dc789de2b4..d1d88d3a5d3f 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,20 @@ 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.
+
+``btf_type`` is followed by a ``u32`` with the following bits arrangement::
+
+  #define BTF_FLOAT_BITS(VAL)     ((VAL)  & 0x000000ff)
+
 3. BTF Kernel API
 *****************
 
-- 
2.29.2


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

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

On Tue, Feb 9, 2021 at 7:04 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> The logic follows that of BTF_KIND_INT most of the time, some functions
> are even unified to work on both. 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             | 85 +++++++++++++++++++++++----------
>  tools/lib/bpf/btf.h             | 13 +++++
>  tools/lib/bpf/btf_dump.c        |  4 ++
>  tools/lib/bpf/libbpf.c          | 32 ++++++++++++-
>  tools/lib/bpf/libbpf.map        |  5 ++
>  tools/lib/bpf/libbpf_internal.h |  4 ++
>  6 files changed, 118 insertions(+), 25 deletions(-)
>

[...]

> @@ -2445,6 +2450,12 @@ 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 */
> +                       __u8 nr_bits = btf_float_bits(t);
> +

nit: no need for extra variable, just use it inline below

> +                       t->info = BTF_INFO_ENC(BTF_KIND_INT, 0, 0);
> +                       *(int *)(t + 1) = BTF_INT_ENC(0, 0, nr_bits);
>                 }
>         }
>  }
> @@ -3882,6 +3893,18 @@ static int probe_kern_btf_datasec(void)
>                                              strs, sizeof(strs)));
>  }

[...]

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

* Re: [PATCH RFC 1/6] bpf: Add BTF_KIND_FLOAT to uapi
  2021-02-10  3:03 ` [PATCH RFC 1/6] bpf: Add BTF_KIND_FLOAT to uapi Ilya Leoshkevich
@ 2021-02-11  0:19   ` Andrii Nakryiko
  2021-02-11 21:26     ` Ilya Leoshkevich
  0 siblings, 1 reply; 13+ messages in thread
From: Andrii Nakryiko @ 2021-02-11  0:19 UTC (permalink / raw)
  To: Ilya Leoshkevich
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Arnaldo Carvalho de Melo, bpf, Heiko Carstens,
	Vasily Gorbik

On Tue, Feb 9, 2021 at 7:03 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> Add a new kind value, expand the kind bitfield, add a macro for
> parsing the additional u32.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  include/uapi/linux/btf.h       | 10 ++++++++--
>  tools/include/uapi/linux/btf.h | 10 ++++++++--
>  2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
> index 5a667107ad2c..e713430cb033 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
> @@ -169,4 +170,9 @@ struct btf_var_secinfo {
>         __u32   size;
>  };
>
> +/* BTF_KIND_FLOAT is followed by a u32 and the following


what's the point of that u32, if BTF_FLOAT_BITS() is just t->size * 8?
Why adding this complexity. BTF_KIND_INT has bits because we had an
inconvenient bitfield encoding as a special BTF_KIND_INT types, which
we since stopped using in favor of encoding bitfield sizes and offsets
inside struct/union fields. I don't think there is any need for that
with FLOAT, so why waste space and add complexity and possibility for
inconsistencies?

Disclaimer: I'm in a "just BTF_KIND_INT encoding bit for
floating-point numbers" camp.

> + * is the 32 bits arrangement:
> + */
> +#define BTF_FLOAT_BITS(VAL)    ((VAL)  & 0x000000ff)
> +
>  #endif /* _UAPI__LINUX_BTF_H__ */
> diff --git a/tools/include/uapi/linux/btf.h b/tools/include/uapi/linux/btf.h
> index 5a667107ad2c..e713430cb033 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
> @@ -169,4 +170,9 @@ struct btf_var_secinfo {
>         __u32   size;
>  };
>
> +/* BTF_KIND_FLOAT is followed by a u32 and the following
> + * is the 32 bits arrangement:
> + */
> +#define BTF_FLOAT_BITS(VAL)    ((VAL)  & 0x000000ff)
> +
>  #endif /* _UAPI__LINUX_BTF_H__ */
> --
> 2.29.2
>

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

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

On Tue, Feb 9, 2021 at 7:04 PM Ilya Leoshkevich <iii@linux.ibm.com> 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 | 101 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 99 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 756a93f534b6..6f27b7b59d77 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -173,8 +173,9 @@
>  #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_FLOAT_MASK 0x000000ff
>  #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 +281,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 +576,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;
>         }
>
> @@ -614,6 +617,11 @@ static const struct btf_var *btf_type_var(const struct btf_type *t)
>         return (const struct btf_var *)(t + 1);
>  }
>
> +static u32 btf_type_float(const struct btf_type *t)
> +{
> +       return *(u32 *)(t + 1);
> +}
> +
>  static const struct btf_kind_operations *btf_type_ops(const struct btf_type *t)
>  {
>         return kind_ops[BTF_INFO_KIND(t->info)];
> @@ -1704,6 +1712,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 +1858,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 +3684,93 @@ 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)
> +{
> +       u32 float_data, nr_bits, meta_needed = sizeof(float_data);
> +
> +       if (meta_left < meta_needed) {
> +               btf_verifier_log_basic(env, t,
> +                                      "meta_left:%u meta_needed:%u",
> +                                      meta_left, meta_needed);
> +               return -EINVAL;
> +       }
> +
> +       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;
> +       }
> +
> +       float_data = btf_type_float(t);
> +       if (float_data & ~BTF_FLOAT_MASK) {
> +               btf_verifier_log_basic(env, t, "Invalid float_data:%x",
> +                                      float_data);
> +               return -EINVAL;
> +       }
> +
> +       nr_bits = BTF_FLOAT_BITS(float_data);
> +
> +       if (BITS_ROUNDUP_BYTES(nr_bits) > t->size) {

what's the case where nr_bits != t->size * 8 ? Is that even possible in C?

> +               btf_verifier_log_type(env, t, "nr_bits exceeds type_size");
> +               return -EINVAL;
> +       }
> +
> +       btf_verifier_log_type(env, t, NULL);
> +
> +       return meta_needed;
> +}
> +
> +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;
> +       u32 float_data;
> +       u64 size_bits;
> +
> +       float_data = btf_type_float(member_type);
> +       size_bits = BTF_FLOAT_BITS(float_data);
> +       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)
> +{
> +       int float_data = btf_type_float(t);
> +
> +       btf_verifier_log(env,
> +                        "size=%u nr_bits=%u",
> +                        t->size, BTF_FLOAT_BITS(float_data));
> +}
> +
> +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 +3904,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	[flat|nested] 13+ messages in thread

* Re: [PATCH RFC 0/6] Add BTF_KIND_FLOAT support
  2021-02-10  3:03 [PATCH RFC 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
                   ` (5 preceding siblings ...)
  2021-02-10  3:03 ` [PATCH RFC 6/6] bpf: Document BTF_KIND_FLOAT in btf.rst Ilya Leoshkevich
@ 2021-02-11  1:31 ` Andrii Nakryiko
  6 siblings, 0 replies; 13+ messages in thread
From: Andrii Nakryiko @ 2021-02-11  1:31 UTC (permalink / raw)
  To: Ilya Leoshkevich
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Arnaldo Carvalho de Melo, bpf, Heiko Carstens,
	Vasily Gorbik

On Tue, Feb 9, 2021 at 7:03 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> 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-v1
> * https://reviews.llvm.org/D83289
>
> but they should go in after the libbpf part is integrated.
>
> There is also an open question: should we go forward with
> BTF_KIND_FLOAT, or should this be merely a BTF_KIND_INT encoding? The
> argument for BTF_KIND_FLOAT is that it's more explicit and therefore
> less prone to unintentional mixups. The argument for BTF_KIND_INT
> encoding is that there would be less code and the overall integration
> process would be simpler.

Less code is not the only or main motivation for representing floats
as just an encoding of BTF_KIND_INT. I think float is not sufficiently
different from bool and int to warrant its own type (kind). And BTF,
in general, is pretty good about having only essential types (kinds).
Float/double is a primitive type, just like bool or char/int/long,
supported by the compiler natively and it represents some indivisible
arrangement of bits in memory.

Surely, there are some semantic differences in how compiler and CPU
are handling such types, but that's none of type system's concern.
E.g., ABI calling conventions dictating which registers or stack
locations arguments go into are not described by BTF. Also, consider
bool. You can treat it as a single byte integer, but it's not just
that: compiler treats it specially (with the 0 and 1 regularization,
when converting to integer representation). Similarly for floats, code
generated for use of floats will be different from integers, but
that's none of type system's concerns. But when using BTF types to
describe memory contents, bool, int, and float are exactly the same
thing: a set of bytes in memory, which are up to interpretation. And
that's why it leads to less code and overall simpler integration.

The only place I can think of where BPF verifier would care about
float vs int, is when processing function signature, because (at least
for some architectures) float will be put in different registers than
non-floats. But BPF verifier can easily distinguish that case by
checking the encoding, so I don't buy the "unintentional mixups"
argument.


>
> 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                    |  19 ++-
>  include/uapi/linux/btf.h                     |  10 +-
>  kernel/bpf/btf.c                             | 101 ++++++++++++-
>  tools/bpf/bpftool/btf.c                      |  13 ++
>  tools/bpf/bpftool/btf_dumper.c               |   1 +
>  tools/include/uapi/linux/btf.h               |  10 +-
>  tools/lib/bpf/btf.c                          |  85 ++++++++---
>  tools/lib/bpf/btf.h                          |  13 ++
>  tools/lib/bpf/btf_dump.c                     |   4 +
>  tools/lib/bpf/libbpf.c                       |  32 +++-
>  tools/lib/bpf/libbpf.map                     |   5 +
>  tools/lib/bpf/libbpf_internal.h              |   4 +
>  tools/testing/selftests/bpf/btf_helpers.c    |   4 +
>  tools/testing/selftests/bpf/prog_tests/btf.c | 145 +++++++++++++++++++
>  tools/testing/selftests/bpf/test_btf.h       |   5 +
>  15 files changed, 418 insertions(+), 33 deletions(-)
>
> --
> 2.29.2
>

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

* Re: [PATCH RFC 1/6] bpf: Add BTF_KIND_FLOAT to uapi
  2021-02-11  0:19   ` Andrii Nakryiko
@ 2021-02-11 21:26     ` Ilya Leoshkevich
  2021-02-11 23:11       ` Alexei Starovoitov
  0 siblings, 1 reply; 13+ messages in thread
From: Ilya Leoshkevich @ 2021-02-11 21:26 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Yonghong Song, Arnaldo Carvalho de Melo, bpf, Heiko Carstens,
	Vasily Gorbik

On Wed, 2021-02-10 at 16:19 -0800, Andrii Nakryiko wrote:
> On Tue, Feb 9, 2021 at 7:03 PM Ilya Leoshkevich <iii@linux.ibm.com>
> wrote:
> > 
> > Add a new kind value, expand the kind bitfield, add a macro for
> > parsing the additional u32.
> > 
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >  include/uapi/linux/btf.h       | 10 ++++++++--
> >  tools/include/uapi/linux/btf.h | 10 ++++++++--
> >  2 files changed, 16 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
> > index 5a667107ad2c..e713430cb033 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
> > @@ -169,4 +170,9 @@ struct btf_var_secinfo {
> >         __u32   size;
> >  };
> > 
> > +/* BTF_KIND_FLOAT is followed by a u32 and the following
> 
> 
> what's the point of that u32, if BTF_FLOAT_BITS() is just t->size *
> 8?
> Why adding this complexity. BTF_KIND_INT has bits because we had an
> inconvenient bitfield encoding as a special BTF_KIND_INT types, which
> we since stopped using in favor of encoding bitfield sizes and
> offsets
> inside struct/union fields. I don't think there is any need for that
> with FLOAT, so why waste space and add complexity and possibility for
> inconsistencies?

You are right, this is not necessary. I don't think something like a
floating-point bitfield exists in the first place.

> Disclaimer: I'm in a "just BTF_KIND_INT encoding bit for
> floating-point numbers" camp.

Despite me being the guy, who sent this series, I like such a simpler
approach as well. In fact, my first attempt at this was even simpler -
just a char[] - but this didn't let us distinguish floats from "real"
byte arrays, which BTF_KIND_INT encoding does. But I think we need to
convince Alexey that this would be OK? :-) If that helps, I can
implement the BTF_KIND_INT encoding variant, so that we could compare
both approaches. What do you think?

> > + * is the 32 bits arrangement:
> > + */
> > +#define BTF_FLOAT_BITS(VAL)    ((VAL)  & 0x000000ff)
> > +
> >  #endif /* _UAPI__LINUX_BTF_H__ */
> > diff --git a/tools/include/uapi/linux/btf.h
> > b/tools/include/uapi/linux/btf.h
> > index 5a667107ad2c..e713430cb033 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
> > @@ -169,4 +170,9 @@ struct btf_var_secinfo {
> >         __u32   size;
> >  };
> > 
> > +/* BTF_KIND_FLOAT is followed by a u32 and the following
> > + * is the 32 bits arrangement:
> > + */
> > +#define BTF_FLOAT_BITS(VAL)    ((VAL)  & 0x000000ff)
> > +
> >  #endif /* _UAPI__LINUX_BTF_H__ */
> > --
> > 2.29.2
> > 



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

* Re: [PATCH RFC 1/6] bpf: Add BTF_KIND_FLOAT to uapi
  2021-02-11 21:26     ` Ilya Leoshkevich
@ 2021-02-11 23:11       ` Alexei Starovoitov
  0 siblings, 0 replies; 13+ messages in thread
From: Alexei Starovoitov @ 2021-02-11 23:11 UTC (permalink / raw)
  To: Ilya Leoshkevich
  Cc: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Yonghong Song, Arnaldo Carvalho de Melo, bpf,
	Heiko Carstens, Vasily Gorbik

On Thu, Feb 11, 2021 at 1:26 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> On Wed, 2021-02-10 at 16:19 -0800, Andrii Nakryiko wrote:
> > On Tue, Feb 9, 2021 at 7:03 PM Ilya Leoshkevich <iii@linux.ibm.com>
> > wrote:
> > >
> > > Add a new kind value, expand the kind bitfield, add a macro for
> > > parsing the additional u32.
> > >
> > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > > ---
> > >  include/uapi/linux/btf.h       | 10 ++++++++--
> > >  tools/include/uapi/linux/btf.h | 10 ++++++++--
> > >  2 files changed, 16 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
> > > index 5a667107ad2c..e713430cb033 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
> > > @@ -169,4 +170,9 @@ struct btf_var_secinfo {
> > >         __u32   size;
> > >  };
> > >
> > > +/* BTF_KIND_FLOAT is followed by a u32 and the following
> >
> >
> > what's the point of that u32, if BTF_FLOAT_BITS() is just t->size *
> > 8?
> > Why adding this complexity. BTF_KIND_INT has bits because we had an
> > inconvenient bitfield encoding as a special BTF_KIND_INT types, which
> > we since stopped using in favor of encoding bitfield sizes and
> > offsets
> > inside struct/union fields. I don't think there is any need for that
> > with FLOAT, so why waste space and add complexity and possibility for
> > inconsistencies?
>
> You are right, this is not necessary. I don't think something like a
> floating-point bitfield exists in the first place.
>
> > Disclaimer: I'm in a "just BTF_KIND_INT encoding bit for
> > floating-point numbers" camp.
>
> Despite me being the guy, who sent this series, I like such a simpler
> approach as well. In fact, my first attempt at this was even simpler -
> just a char[] - but this didn't let us distinguish floats from "real"
> byte arrays, which BTF_KIND_INT encoding does. But I think we need to
> convince Alexey that this would be OK? :-) If that helps, I can
> implement the BTF_KIND_INT encoding variant, so that we could compare
> both approaches. What do you think?

Sorry to crash your party :)
I'm very much against calling "float" a different kind of "int".
BTF is not equivalent to dwarf. It's not a traditional debug format
which purpose is to describe the types, line info, etc.
BTF is used in verification and its correctness is crucial.
Imagine a function with KIND_INT argument if there is no KIND_FLOAT
btf_func_model construction will silently consume 'int' which is actually
'float' which will cause kernel crash.
There is a wip on "unstable helpers". It needs accurate argument processing too.
The same thing will apply there. If KIND_FLOAT is mistaken for KIND_INT
the kernel will crash again, but in a different way.
The usage of floating point in the kernel is minimal, but the point stands.

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

end of thread, other threads:[~2021-02-11 23:12 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-10  3:03 [PATCH RFC 0/6] Add BTF_KIND_FLOAT support Ilya Leoshkevich
2021-02-10  3:03 ` [PATCH RFC 1/6] bpf: Add BTF_KIND_FLOAT to uapi Ilya Leoshkevich
2021-02-11  0:19   ` Andrii Nakryiko
2021-02-11 21:26     ` Ilya Leoshkevich
2021-02-11 23:11       ` Alexei Starovoitov
2021-02-10  3:03 ` [PATCH RFC 2/6] libbpf: Add BTF_KIND_FLOAT support Ilya Leoshkevich
2021-02-11  0:15   ` Andrii Nakryiko
2021-02-10  3:03 ` [PATCH RFC 3/6] tools/bpftool: " Ilya Leoshkevich
2021-02-10  3:03 ` [PATCH RFC 4/6] bpf: " Ilya Leoshkevich
2021-02-11  0:20   ` Andrii Nakryiko
2021-02-10  3:03 ` [PATCH RFC 5/6] selftest/bpf: Add BTF_KIND_FLOAT tests Ilya Leoshkevich
2021-02-10  3:03 ` [PATCH RFC 6/6] bpf: Document BTF_KIND_FLOAT in btf.rst Ilya Leoshkevich
2021-02-11  1:31 ` [PATCH RFC 0/6] Add BTF_KIND_FLOAT support Andrii Nakryiko

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