* [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG
@ 2021-09-13 15:51 Yonghong Song
2021-09-13 15:51 ` [PATCH bpf-next v2 01/11] btf: change BTF_KIND_* macros to enums Yonghong Song
` (11 more replies)
0 siblings, 12 replies; 43+ messages in thread
From: Yonghong Song @ 2021-09-13 15:51 UTC (permalink / raw)
To: bpf; +Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
LLVM14 added support for a new C attribute ([1])
__attribute__((btf_tag("arbitrary_str")))
This attribute will be emitted to dwarf ([2]) and pahole
will convert it to BTF. Or for bpf target, this
attribute will be emitted to BTF directly ([3], [4]).
The attribute is intended to provide additional
information for
- struct/union type or struct/union member
- static/global variables
- static/global function or function parameter.
This new attribute can be used to add attributes
to kernel codes, e.g., pre- or post- conditions,
allow/deny info, or any other info in which only
the kernel is interested. Such attributes will
be processed by clang frontend and emitted to
dwarf, converting to BTF by pahole. Ultimiately
the verifier can use these information for
verification purpose.
The new attribute can also be used for bpf
programs, e.g., tagging with __user attributes
for function parameters, specifying global
function preconditions, etc. Such information
may help verifier to detect user program
bugs.
After this series, pahole dwarf->btf converter
will be enhanced to support new llvm tag
for btf_tag attribute. With pahole support,
we will then try to add a few real use case,
e.g., __user/__rcu tagging, allow/deny list,
some kernel function precondition, etc,
in the kernel.
In the rest of the series, Patches 1-2 had
kernel support. Patches 3-4 added
libbpf support. Patch 5 added bpftool
support. Patches 6-10 added various selftests.
Patch 11 added documentation for the new kind.
[1] https://reviews.llvm.org/D106614
[2] https://reviews.llvm.org/D106621
[3] https://reviews.llvm.org/D106622
[4] https://reviews.llvm.org/D109560
Changelog:
v1 -> v2:
- BTF ELF format changed in llvm ([4] above),
so cross-board change to use the new format.
- Clarified in commit message that BTF_KIND_TAG
is not emitted by bpftool btf dump format c.
- Fix various comments from Andrii.
Yonghong Song (11):
btf: change BTF_KIND_* macros to enums
bpf: support for new btf kind BTF_KIND_TAG
libbpf: rename btf_{hash,equal}_int to btf_{hash,equal}_int_tag
libbpf: add support for BTF_KIND_TAG
bpftool: add support for BTF_KIND_TAG
selftests/bpf: test libbpf API function btf__add_tag()
selftests/bpf: change NAME_NTH/IS_NAME_NTH for BTF_KIND_TAG format
selftests/bpf: add BTF_KIND_TAG unit tests
selftests/bpf: test BTF_KIND_TAG for deduplication
selftests/bpf: add a test with a bpf program with btf_tag attributes
docs/bpf: add documentation for BTF_KIND_TAG
Documentation/bpf/btf.rst | 27 +-
include/uapi/linux/btf.h | 52 +--
kernel/bpf/btf.c | 120 +++++++
tools/bpf/bpftool/btf.c | 12 +
tools/include/uapi/linux/btf.h | 52 +--
tools/lib/bpf/btf.c | 85 ++++-
tools/lib/bpf/btf.h | 15 +
tools/lib/bpf/btf_dump.c | 3 +
tools/lib/bpf/libbpf.c | 31 +-
tools/lib/bpf/libbpf.map | 5 +
tools/lib/bpf/libbpf_internal.h | 2 +
tools/testing/selftests/bpf/btf_helpers.c | 7 +-
tools/testing/selftests/bpf/prog_tests/btf.c | 318 ++++++++++++++++--
.../selftests/bpf/prog_tests/btf_tag.c | 14 +
.../selftests/bpf/prog_tests/btf_write.c | 21 ++
tools/testing/selftests/bpf/progs/tag.c | 39 +++
tools/testing/selftests/bpf/test_btf.h | 3 +
17 files changed, 736 insertions(+), 70 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/btf_tag.c
create mode 100644 tools/testing/selftests/bpf/progs/tag.c
--
2.30.2
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH bpf-next v2 01/11] btf: change BTF_KIND_* macros to enums
2021-09-13 15:51 [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG Yonghong Song
@ 2021-09-13 15:51 ` Yonghong Song
2021-09-14 4:59 ` Andrii Nakryiko
2021-09-13 15:51 ` [PATCH bpf-next v2 02/11] bpf: support for new btf kind BTF_KIND_TAG Yonghong Song
` (10 subsequent siblings)
11 siblings, 1 reply; 43+ messages in thread
From: Yonghong Song @ 2021-09-13 15:51 UTC (permalink / raw)
To: bpf; +Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
Change BTF_KIND_* macros to enums so they are encoded in dwarf and
appear in vmlinux.h. This will make it easier for bpf programs
to use these constants without macro definitions.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
include/uapi/linux/btf.h | 36 ++++++++++++++++++----------------
tools/include/uapi/linux/btf.h | 36 ++++++++++++++++++----------------
2 files changed, 38 insertions(+), 34 deletions(-)
diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
index d27b1708efe9..c32cd6697d63 100644
--- a/include/uapi/linux/btf.h
+++ b/include/uapi/linux/btf.h
@@ -56,23 +56,25 @@ struct btf_type {
#define BTF_INFO_VLEN(info) ((info) & 0xffff)
#define BTF_INFO_KFLAG(info) ((info) >> 31)
-#define BTF_KIND_UNKN 0 /* Unknown */
-#define BTF_KIND_INT 1 /* Integer */
-#define BTF_KIND_PTR 2 /* Pointer */
-#define BTF_KIND_ARRAY 3 /* Array */
-#define BTF_KIND_STRUCT 4 /* Struct */
-#define BTF_KIND_UNION 5 /* Union */
-#define BTF_KIND_ENUM 6 /* Enumeration */
-#define BTF_KIND_FWD 7 /* Forward */
-#define BTF_KIND_TYPEDEF 8 /* Typedef */
-#define BTF_KIND_VOLATILE 9 /* Volatile */
-#define BTF_KIND_CONST 10 /* Const */
-#define BTF_KIND_RESTRICT 11 /* Restrict */
-#define BTF_KIND_FUNC 12 /* Function */
-#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 */
+enum {
+ BTF_KIND_UNKN = 0, /* Unknown */
+ BTF_KIND_INT, /* Integer */
+ BTF_KIND_PTR, /* Pointer */
+ BTF_KIND_ARRAY, /* Array */
+ BTF_KIND_STRUCT, /* Struct */
+ BTF_KIND_UNION, /* Union */
+ BTF_KIND_ENUM, /* Enumeration */
+ BTF_KIND_FWD, /* Forward */
+ BTF_KIND_TYPEDEF, /* Typedef */
+ BTF_KIND_VOLATILE, /* Volatile */
+ BTF_KIND_CONST, /* Const */
+ BTF_KIND_RESTRICT, /* Restrict */
+ BTF_KIND_FUNC, /* Function */
+ BTF_KIND_FUNC_PROTO, /* Function Proto */
+ BTF_KIND_VAR, /* Variable */
+ BTF_KIND_DATASEC, /* Section */
+ BTF_KIND_FLOAT, /* Floating point */
+};
#define BTF_KIND_MAX BTF_KIND_FLOAT
#define NR_BTF_KINDS (BTF_KIND_MAX + 1)
diff --git a/tools/include/uapi/linux/btf.h b/tools/include/uapi/linux/btf.h
index d27b1708efe9..c32cd6697d63 100644
--- a/tools/include/uapi/linux/btf.h
+++ b/tools/include/uapi/linux/btf.h
@@ -56,23 +56,25 @@ struct btf_type {
#define BTF_INFO_VLEN(info) ((info) & 0xffff)
#define BTF_INFO_KFLAG(info) ((info) >> 31)
-#define BTF_KIND_UNKN 0 /* Unknown */
-#define BTF_KIND_INT 1 /* Integer */
-#define BTF_KIND_PTR 2 /* Pointer */
-#define BTF_KIND_ARRAY 3 /* Array */
-#define BTF_KIND_STRUCT 4 /* Struct */
-#define BTF_KIND_UNION 5 /* Union */
-#define BTF_KIND_ENUM 6 /* Enumeration */
-#define BTF_KIND_FWD 7 /* Forward */
-#define BTF_KIND_TYPEDEF 8 /* Typedef */
-#define BTF_KIND_VOLATILE 9 /* Volatile */
-#define BTF_KIND_CONST 10 /* Const */
-#define BTF_KIND_RESTRICT 11 /* Restrict */
-#define BTF_KIND_FUNC 12 /* Function */
-#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 */
+enum {
+ BTF_KIND_UNKN = 0, /* Unknown */
+ BTF_KIND_INT, /* Integer */
+ BTF_KIND_PTR, /* Pointer */
+ BTF_KIND_ARRAY, /* Array */
+ BTF_KIND_STRUCT, /* Struct */
+ BTF_KIND_UNION, /* Union */
+ BTF_KIND_ENUM, /* Enumeration */
+ BTF_KIND_FWD, /* Forward */
+ BTF_KIND_TYPEDEF, /* Typedef */
+ BTF_KIND_VOLATILE, /* Volatile */
+ BTF_KIND_CONST, /* Const */
+ BTF_KIND_RESTRICT, /* Restrict */
+ BTF_KIND_FUNC, /* Function */
+ BTF_KIND_FUNC_PROTO, /* Function Proto */
+ BTF_KIND_VAR, /* Variable */
+ BTF_KIND_DATASEC, /* Section */
+ BTF_KIND_FLOAT, /* Floating point */
+};
#define BTF_KIND_MAX BTF_KIND_FLOAT
#define NR_BTF_KINDS (BTF_KIND_MAX + 1)
--
2.30.2
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH bpf-next v2 02/11] bpf: support for new btf kind BTF_KIND_TAG
2021-09-13 15:51 [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG Yonghong Song
2021-09-13 15:51 ` [PATCH bpf-next v2 01/11] btf: change BTF_KIND_* macros to enums Yonghong Song
@ 2021-09-13 15:51 ` Yonghong Song
2021-09-14 5:08 ` Andrii Nakryiko
2021-09-13 15:51 ` [PATCH bpf-next v2 03/11] libbpf: rename btf_{hash,equal}_int to btf_{hash,equal}_int_tag Yonghong Song
` (9 subsequent siblings)
11 siblings, 1 reply; 43+ messages in thread
From: Yonghong Song @ 2021-09-13 15:51 UTC (permalink / raw)
To: bpf; +Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
LLVM14 added support for a new C attribute ([1])
__attribute__((btf_tag("arbitrary_str")))
This attribute will be emitted to dwarf ([2]) and pahole
will convert it to BTF. Or for bpf target, this
attribute will be emitted to BTF directly ([3], [4]).
The attribute is intended to provide additional
information for
- struct/union type or struct/union member
- static/global variables
- static/global function or function parameter.
For linux kernel, the btf_tag can be applied
in various places to specify user pointer,
function pre- or post- condition, function
allow/deny in certain context, etc. Such information
will be encoded in vmlinux BTF and can be used
by verifier.
The btf_tag can also be applied to bpf programs
to help global verifiable functions, e.g.,
specifying preconditions, etc.
This patch added basic parsing and checking support
in kernel for new BTF_KIND_TAG kind.
[1] https://reviews.llvm.org/D106614
[2] https://reviews.llvm.org/D106621
[3] https://reviews.llvm.org/D106622
[4] https://reviews.llvm.org/D109560
Signed-off-by: Yonghong Song <yhs@fb.com>
---
include/uapi/linux/btf.h | 16 ++++-
kernel/bpf/btf.c | 120 +++++++++++++++++++++++++++++++++
tools/include/uapi/linux/btf.h | 16 ++++-
3 files changed, 148 insertions(+), 4 deletions(-)
diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
index c32cd6697d63..52332fc144cf 100644
--- a/include/uapi/linux/btf.h
+++ b/include/uapi/linux/btf.h
@@ -43,7 +43,7 @@ struct btf_type {
* "size" tells the size of the type it is describing.
*
* "type" is used by PTR, TYPEDEF, VOLATILE, CONST, RESTRICT,
- * FUNC, FUNC_PROTO and VAR.
+ * FUNC, FUNC_PROTO, VAR and TAG.
* "type" is a type_id referring to another type.
*/
union {
@@ -74,8 +74,9 @@ enum {
BTF_KIND_VAR, /* Variable */
BTF_KIND_DATASEC, /* Section */
BTF_KIND_FLOAT, /* Floating point */
+ BTF_KIND_TAG, /* Tag */
};
-#define BTF_KIND_MAX BTF_KIND_FLOAT
+#define BTF_KIND_MAX BTF_KIND_TAG
#define NR_BTF_KINDS (BTF_KIND_MAX + 1)
/* For some specific BTF_KIND, "struct btf_type" is immediately
@@ -172,4 +173,15 @@ struct btf_var_secinfo {
__u32 size;
};
+/* BTF_KIND_TAG is followed by a single "struct btf_tag" to describe
+ * additional information related to the tag applied location.
+ * If component_idx == -1, the tag is applied to a struct, union,
+ * variable or function. Otherwise, it is applied to a struct/union
+ * member or a func argument, and component_idx indicates which member
+ * or argument (0 ... vlen-1).
+ */
+struct btf_tag {
+ __s32 component_idx;
+};
+
#endif /* _UAPI__LINUX_BTF_H__ */
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index dfe61df4f974..566f4e840f64 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -281,6 +281,7 @@ static const char * const btf_kind_str[NR_BTF_KINDS] = {
[BTF_KIND_VAR] = "VAR",
[BTF_KIND_DATASEC] = "DATASEC",
[BTF_KIND_FLOAT] = "FLOAT",
+ [BTF_KIND_TAG] = "TAG",
};
const char *btf_type_str(const struct btf_type *t)
@@ -459,6 +460,17 @@ static bool btf_type_is_datasec(const struct btf_type *t)
return BTF_INFO_KIND(t->info) == BTF_KIND_DATASEC;
}
+static bool btf_type_is_tag(const struct btf_type *t)
+{
+ return BTF_INFO_KIND(t->info) == BTF_KIND_TAG;
+}
+
+static bool btf_type_is_tag_target(const struct btf_type *t)
+{
+ return btf_type_is_func(t) || btf_type_is_struct(t) ||
+ btf_type_is_var(t);
+}
+
u32 btf_nr_types(const struct btf *btf)
{
u32 total = 0;
@@ -537,6 +549,7 @@ const struct btf_type *btf_type_resolve_func_ptr(const struct btf *btf,
static bool btf_type_is_resolve_source_only(const struct btf_type *t)
{
return btf_type_is_var(t) ||
+ btf_type_is_tag(t) ||
btf_type_is_datasec(t);
}
@@ -563,6 +576,7 @@ static bool btf_type_needs_resolve(const struct btf_type *t)
btf_type_is_struct(t) ||
btf_type_is_array(t) ||
btf_type_is_var(t) ||
+ btf_type_is_tag(t) ||
btf_type_is_datasec(t);
}
@@ -616,6 +630,11 @@ static const struct btf_var *btf_type_var(const struct btf_type *t)
return (const struct btf_var *)(t + 1);
}
+static const struct btf_tag *btf_type_tag(const struct btf_type *t)
+{
+ return (const struct btf_tag *)(t + 1);
+}
+
static const struct btf_kind_operations *btf_type_ops(const struct btf_type *t)
{
return kind_ops[BTF_INFO_KIND(t->info)];
@@ -3801,6 +3820,102 @@ static const struct btf_kind_operations float_ops = {
.show = btf_df_show,
};
+static s32 btf_tag_check_meta(struct btf_verifier_env *env,
+ const struct btf_type *t,
+ u32 meta_left)
+{
+ const struct btf_tag *tag;
+ u32 meta_needed = sizeof(*tag);
+ const char *value;
+
+ if (meta_left < meta_needed) {
+ btf_verifier_log_basic(env, t,
+ "meta_left:%u meta_needed:%u",
+ meta_left, meta_needed);
+ return -EINVAL;
+ }
+
+ value = btf_name_by_offset(env->btf, t->name_off);
+ if (!value || !value[0]) {
+ btf_verifier_log_type(env, t, "Invalid value");
+ 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;
+ }
+
+ btf_verifier_log_type(env, t, NULL);
+
+ return meta_needed;
+}
+
+static int btf_tag_resolve(struct btf_verifier_env *env,
+ const struct resolve_vertex *v)
+{
+ const struct btf_type *next_type;
+ const struct btf_type *t = v->t;
+ u32 next_type_id = t->type;
+ struct btf *btf = env->btf;
+ s32 component_idx;
+ u32 vlen;
+
+ next_type = btf_type_by_id(btf, next_type_id);
+ if (!next_type || !btf_type_is_tag_target(next_type)) {
+ btf_verifier_log_type(env, v->t, "Invalid type_id");
+ return -EINVAL;
+ }
+
+ if (!env_type_is_resolve_sink(env, next_type) &&
+ !env_type_is_resolved(env, next_type_id))
+ return env_stack_push(env, next_type, next_type_id);
+
+ component_idx = btf_type_tag(t)->component_idx;
+ if (component_idx != -1) {
+ if (btf_type_is_var(next_type) || component_idx < 0) {
+ btf_verifier_log_type(env, v->t, "Invalid component_idx");
+ return -EINVAL;
+ }
+
+ if (btf_type_is_struct(next_type)) {
+ vlen = btf_type_vlen(next_type);
+ } else {
+ next_type = btf_type_by_id(btf, next_type->type);
+ vlen = btf_type_vlen(next_type);
+ }
+
+ if ((u32)component_idx >= vlen) {
+ btf_verifier_log_type(env, v->t, "Invalid component_idx");
+ return -EINVAL;
+ }
+ }
+
+ env_stack_pop_resolved(env, next_type_id, 0);
+
+ return 0;
+}
+
+static void btf_tag_log(struct btf_verifier_env *env, const struct btf_type *t)
+{
+ btf_verifier_log(env, "type=%u component_idx=%d", t->type,
+ btf_type_tag(t)->component_idx);
+}
+
+static const struct btf_kind_operations tag_ops = {
+ .check_meta = btf_tag_check_meta,
+ .resolve = btf_tag_resolve,
+ .check_member = btf_df_check_member,
+ .check_kflag_member = btf_df_check_kflag_member,
+ .log_details = btf_tag_log,
+ .show = btf_df_show,
+};
+
static int btf_func_proto_check(struct btf_verifier_env *env,
const struct btf_type *t)
{
@@ -3935,6 +4050,7 @@ static const struct btf_kind_operations * const kind_ops[NR_BTF_KINDS] = {
[BTF_KIND_VAR] = &var_ops,
[BTF_KIND_DATASEC] = &datasec_ops,
[BTF_KIND_FLOAT] = &float_ops,
+ [BTF_KIND_TAG] = &tag_ops,
};
static s32 btf_check_meta(struct btf_verifier_env *env,
@@ -4019,6 +4135,10 @@ static bool btf_resolve_valid(struct btf_verifier_env *env,
return !btf_resolved_type_id(btf, type_id) &&
!btf_resolved_type_size(btf, type_id);
+ if (btf_type_is_tag(t))
+ return btf_resolved_type_id(btf, type_id) &&
+ !btf_resolved_type_size(btf, type_id);
+
if (btf_type_is_modifier(t) || btf_type_is_ptr(t) ||
btf_type_is_var(t)) {
t = btf_type_id_resolve(btf, &type_id);
diff --git a/tools/include/uapi/linux/btf.h b/tools/include/uapi/linux/btf.h
index c32cd6697d63..52332fc144cf 100644
--- a/tools/include/uapi/linux/btf.h
+++ b/tools/include/uapi/linux/btf.h
@@ -43,7 +43,7 @@ struct btf_type {
* "size" tells the size of the type it is describing.
*
* "type" is used by PTR, TYPEDEF, VOLATILE, CONST, RESTRICT,
- * FUNC, FUNC_PROTO and VAR.
+ * FUNC, FUNC_PROTO, VAR and TAG.
* "type" is a type_id referring to another type.
*/
union {
@@ -74,8 +74,9 @@ enum {
BTF_KIND_VAR, /* Variable */
BTF_KIND_DATASEC, /* Section */
BTF_KIND_FLOAT, /* Floating point */
+ BTF_KIND_TAG, /* Tag */
};
-#define BTF_KIND_MAX BTF_KIND_FLOAT
+#define BTF_KIND_MAX BTF_KIND_TAG
#define NR_BTF_KINDS (BTF_KIND_MAX + 1)
/* For some specific BTF_KIND, "struct btf_type" is immediately
@@ -172,4 +173,15 @@ struct btf_var_secinfo {
__u32 size;
};
+/* BTF_KIND_TAG is followed by a single "struct btf_tag" to describe
+ * additional information related to the tag applied location.
+ * If component_idx == -1, the tag is applied to a struct, union,
+ * variable or function. Otherwise, it is applied to a struct/union
+ * member or a func argument, and component_idx indicates which member
+ * or argument (0 ... vlen-1).
+ */
+struct btf_tag {
+ __s32 component_idx;
+};
+
#endif /* _UAPI__LINUX_BTF_H__ */
--
2.30.2
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH bpf-next v2 03/11] libbpf: rename btf_{hash,equal}_int to btf_{hash,equal}_int_tag
2021-09-13 15:51 [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG Yonghong Song
2021-09-13 15:51 ` [PATCH bpf-next v2 01/11] btf: change BTF_KIND_* macros to enums Yonghong Song
2021-09-13 15:51 ` [PATCH bpf-next v2 02/11] bpf: support for new btf kind BTF_KIND_TAG Yonghong Song
@ 2021-09-13 15:51 ` Yonghong Song
2021-09-13 15:51 ` [PATCH bpf-next v2 04/11] libbpf: add support for BTF_KIND_TAG Yonghong Song
` (8 subsequent siblings)
11 siblings, 0 replies; 43+ messages in thread
From: Yonghong Song @ 2021-09-13 15:51 UTC (permalink / raw)
To: bpf; +Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
This patch renames functions btf_{hash,equal}_int() to
btf_{hash,equal}_int_tag() so they can be reused for
BTF_KIND_TAG support. There is no functionality change for
this patch.
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
tools/lib/bpf/btf.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 77dc24d58302..7cb6ebf1be37 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -3256,8 +3256,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 INT or TAG. */
+static long btf_hash_int_tag(struct btf_type *t)
{
__u32 info = *(__u32 *)(t + 1);
long h;
@@ -3267,8 +3267,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 TAGs. */
+static bool btf_equal_int_tag(struct btf_type *t1, struct btf_type *t2)
{
__u32 info1, info2;
@@ -3535,7 +3535,7 @@ static int btf_dedup_prep(struct btf_dedup *d)
h = btf_hash_common(t);
break;
case BTF_KIND_INT:
- h = btf_hash_int(t);
+ h = btf_hash_int_tag(t);
break;
case BTF_KIND_ENUM:
h = btf_hash_enum(t);
@@ -3593,11 +3593,11 @@ static int btf_dedup_prim_type(struct btf_dedup *d, __u32 type_id)
return 0;
case BTF_KIND_INT:
- h = btf_hash_int(t);
+ h = btf_hash_int_tag(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_tag(t, cand)) {
new_id = cand_id;
break;
}
@@ -3881,7 +3881,7 @@ 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);
+ return btf_equal_int_tag(cand_type, canon_type);
case BTF_KIND_ENUM:
if (d->opts.dont_resolve_fwds)
--
2.30.2
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH bpf-next v2 04/11] libbpf: add support for BTF_KIND_TAG
2021-09-13 15:51 [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG Yonghong Song
` (2 preceding siblings ...)
2021-09-13 15:51 ` [PATCH bpf-next v2 03/11] libbpf: rename btf_{hash,equal}_int to btf_{hash,equal}_int_tag Yonghong Song
@ 2021-09-13 15:51 ` Yonghong Song
2021-09-14 5:15 ` Andrii Nakryiko
2021-09-13 15:51 ` [PATCH bpf-next v2 05/11] bpftool: " Yonghong Song
` (7 subsequent siblings)
11 siblings, 1 reply; 43+ messages in thread
From: Yonghong Song @ 2021-09-13 15:51 UTC (permalink / raw)
To: bpf; +Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
Add BTF_KIND_TAG support for parsing and dedup.
Also added sanitization for BTF_KIND_TAG. If BTF_KIND_TAG is not
supported in the kernel, sanitize it to INTs.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
tools/lib/bpf/btf.c | 69 +++++++++++++++++++++++++++++++++
tools/lib/bpf/btf.h | 15 +++++++
tools/lib/bpf/btf_dump.c | 3 ++
tools/lib/bpf/libbpf.c | 31 +++++++++++++--
tools/lib/bpf/libbpf.map | 5 +++
tools/lib/bpf/libbpf_internal.h | 2 +
6 files changed, 122 insertions(+), 3 deletions(-)
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 7cb6ebf1be37..f4dc3741af47 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -304,6 +304,8 @@ static int btf_type_size(const struct btf_type *t)
return base_size + sizeof(struct btf_var);
case BTF_KIND_DATASEC:
return base_size + vlen * sizeof(struct btf_var_secinfo);
+ case BTF_KIND_TAG:
+ return base_size + sizeof(struct btf_tag);
default:
pr_debug("Unsupported BTF_KIND:%u\n", btf_kind(t));
return -EINVAL;
@@ -376,6 +378,9 @@ static int btf_bswap_type_rest(struct btf_type *t)
v->size = bswap_32(v->size);
}
return 0;
+ case BTF_KIND_TAG:
+ btf_tag(t)->component_idx = bswap_32(btf_tag(t)->component_idx);
+ return 0;
default:
pr_debug("Unsupported BTF_KIND:%u\n", btf_kind(t));
return -EINVAL;
@@ -586,6 +591,7 @@ __s64 btf__resolve_size(const struct btf *btf, __u32 type_id)
case BTF_KIND_CONST:
case BTF_KIND_RESTRICT:
case BTF_KIND_VAR:
+ case BTF_KIND_TAG:
type_id = t->type;
break;
case BTF_KIND_ARRAY:
@@ -2440,6 +2446,49 @@ int btf__add_datasec_var_info(struct btf *btf, int var_type_id, __u32 offset, __
return 0;
}
+/*
+ * Append new BTF_KIND_TAG type with:
+ * - *value* - non-empty/non-NULL string;
+ * - *ref_type_id* - referenced type ID, it might not exist yet;
+ * - *component_idx* - -1 for tagging reference type, otherwise struct/union
+ * member or function argument index;
+ * Returns:
+ * - >0, type ID of newly added BTF type;
+ * - <0, on error.
+ */
+int btf__add_tag(struct btf *btf, const char *value, int ref_type_id,
+ int component_idx)
+{
+ bool for_ref_type = false;
+ struct btf_type *t;
+ int sz, value_off;
+
+ if (!value || !value[0] || component_idx < -1)
+ return libbpf_err(-EINVAL);
+
+ if (validate_type_id(ref_type_id))
+ return libbpf_err(-EINVAL);
+
+ if (btf_ensure_modifiable(btf))
+ return libbpf_err(-ENOMEM);
+
+ sz = sizeof(struct btf_type) + sizeof(struct btf_tag);
+ t = btf_add_type_mem(btf, sz);
+ if (!t)
+ return libbpf_err(-ENOMEM);
+
+ value_off = btf__add_str(btf, value);
+ if (value_off < 0)
+ return value_off;
+
+ t->name_off = value_off;
+ t->info = btf_type_info(BTF_KIND_TAG, 0, for_ref_type);
+ t->type = ref_type_id;
+ ((struct btf_tag *)(t + 1))->component_idx = component_idx;
+
+ return btf_commit_type(btf, sz);
+}
+
struct btf_ext_sec_setup_param {
__u32 off;
__u32 len;
@@ -3535,6 +3584,7 @@ static int btf_dedup_prep(struct btf_dedup *d)
h = btf_hash_common(t);
break;
case BTF_KIND_INT:
+ case BTF_KIND_TAG:
h = btf_hash_int_tag(t);
break;
case BTF_KIND_ENUM:
@@ -3590,6 +3640,7 @@ static int btf_dedup_prim_type(struct btf_dedup *d, __u32 type_id)
case BTF_KIND_FUNC_PROTO:
case BTF_KIND_VAR:
case BTF_KIND_DATASEC:
+ case BTF_KIND_TAG:
return 0;
case BTF_KIND_INT:
@@ -4210,6 +4261,23 @@ static int btf_dedup_ref_type(struct btf_dedup *d, __u32 type_id)
}
break;
+ case BTF_KIND_TAG:
+ ref_type_id = btf_dedup_ref_type(d, t->type);
+ if (ref_type_id < 0)
+ return ref_type_id;
+ t->type = ref_type_id;
+
+ h = btf_hash_int_tag(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_tag(t, cand)) {
+ new_id = cand_id;
+ break;
+ }
+ }
+ break;
+
case BTF_KIND_ARRAY: {
struct btf_array *info = btf_array(t);
@@ -4482,6 +4550,7 @@ int btf_type_visit_type_ids(struct btf_type *t, type_id_visit_fn visit, void *ct
case BTF_KIND_TYPEDEF:
case BTF_KIND_FUNC:
case BTF_KIND_VAR:
+ case BTF_KIND_TAG:
return visit(&t->type, ctx);
case BTF_KIND_ARRAY: {
diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index f2e2fab950b7..659ea8a2769b 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -143,6 +143,10 @@ 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);
+/* tag construction API */
+LIBBPF_API int btf__add_tag(struct btf *btf, const char *value, int ref_type_id,
+ int component_idx);
+
struct btf_dedup_opts {
unsigned int dedup_table_size;
bool dont_resolve_fwds;
@@ -330,6 +334,11 @@ static inline bool btf_is_float(const struct btf_type *t)
return btf_kind(t) == BTF_KIND_FLOAT;
}
+static inline bool btf_is_tag(const struct btf_type *t)
+{
+ return btf_kind(t) == BTF_KIND_TAG;
+}
+
static inline __u8 btf_int_encoding(const struct btf_type *t)
{
return BTF_INT_ENCODING(*(__u32 *)(t + 1));
@@ -398,6 +407,12 @@ btf_var_secinfos(const struct btf_type *t)
return (struct btf_var_secinfo *)(t + 1);
}
+struct btf_tag;
+static inline struct btf_tag *btf_tag(const struct btf_type *t)
+{
+ return (struct btf_tag *)(t + 1);
+}
+
#ifdef __cplusplus
} /* extern "C" */
#endif
diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index e4b483f15fb9..ad6df97295ae 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -316,6 +316,7 @@ static int btf_dump_mark_referenced(struct btf_dump *d)
case BTF_KIND_TYPEDEF:
case BTF_KIND_FUNC:
case BTF_KIND_VAR:
+ case BTF_KIND_TAG:
d->type_states[t->type].referenced = 1;
break;
@@ -583,6 +584,7 @@ static int btf_dump_order_type(struct btf_dump *d, __u32 id, bool through_ptr)
case BTF_KIND_FUNC:
case BTF_KIND_VAR:
case BTF_KIND_DATASEC:
+ case BTF_KIND_TAG:
d->type_states[id].order_state = ORDERED;
return 0;
@@ -2215,6 +2217,7 @@ static int btf_dump_dump_type_data(struct btf_dump *d,
case BTF_KIND_FWD:
case BTF_KIND_FUNC:
case BTF_KIND_FUNC_PROTO:
+ case BTF_KIND_TAG:
err = btf_dump_unsupported_data(d, t, id);
break;
case BTF_KIND_INT:
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 8f579c6666b2..4a62ef714562 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -195,6 +195,8 @@ enum kern_feature_id {
FEAT_BTF_FLOAT,
/* BPF perf link support */
FEAT_PERF_LINK,
+ /* BTF_KIND_ATTR support */
+ FEAT_BTF_TAG,
__FEAT_CNT,
};
@@ -1987,6 +1989,7 @@ static const char *__btf_kind_str(__u16 kind)
case BTF_KIND_VAR: return "var";
case BTF_KIND_DATASEC: return "datasec";
case BTF_KIND_FLOAT: return "float";
+ case BTF_KIND_TAG: return "tag";
default: return "unknown";
}
}
@@ -2486,8 +2489,9 @@ static bool btf_needs_sanitization(struct bpf_object *obj)
bool has_datasec = kernel_supports(obj, FEAT_BTF_DATASEC);
bool has_float = kernel_supports(obj, FEAT_BTF_FLOAT);
bool has_func = kernel_supports(obj, FEAT_BTF_FUNC);
+ bool has_tag = kernel_supports(obj, FEAT_BTF_TAG);
- return !has_func || !has_datasec || !has_func_global || !has_float;
+ return !has_func || !has_datasec || !has_func_global || !has_float || !has_tag;
}
static void bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
@@ -2496,14 +2500,15 @@ static void bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
bool has_datasec = kernel_supports(obj, FEAT_BTF_DATASEC);
bool has_float = kernel_supports(obj, FEAT_BTF_FLOAT);
bool has_func = kernel_supports(obj, FEAT_BTF_FUNC);
+ bool has_tag = kernel_supports(obj, FEAT_BTF_TAG);
struct btf_type *t;
int i, j, vlen;
for (i = 1; i <= btf__get_nr_types(btf); i++) {
t = (struct btf_type *)btf__type_by_id(btf, i);
- if (!has_datasec && btf_is_var(t)) {
- /* replace VAR with INT */
+ if ((!has_datasec && btf_is_var(t)) || (!has_tag && btf_is_tag(t))) {
+ /* replace VAR/TAG with INT */
t->info = BTF_INFO_ENC(BTF_KIND_INT, 0, 0);
/*
* using size = 1 is the safest choice, 4 will be too
@@ -4213,6 +4218,23 @@ static int probe_kern_btf_float(void)
strs, sizeof(strs)));
}
+static int probe_kern_btf_tag(void)
+{
+ static const char strs[] = "\0tag";
+ __u32 types[] = {
+ /* int */
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ /* VAR x */ /* [2] */
+ BTF_TYPE_ENC(1, BTF_INFO_ENC(BTF_KIND_VAR, 0, 0), 1),
+ BTF_VAR_STATIC,
+ /* attr */
+ BTF_TYPE_TAG_ENC(1, 2, -1),
+ };
+
+ 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 = {
@@ -4429,6 +4451,9 @@ static struct kern_feature_desc {
[FEAT_PERF_LINK] = {
"BPF perf link support", probe_perf_link,
},
+ [FEAT_BTF_TAG] = {
+ "BTF_KIND_TAG support", probe_kern_btf_tag,
+ },
};
static bool kernel_supports(const struct bpf_object *obj, enum kern_feature_id feat_id)
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index bbc53bb25f68..9e649cf9e771 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -386,3 +386,8 @@ LIBBPF_0.5.0 {
btf_dump__dump_type_data;
libbpf_set_strict_mode;
} LIBBPF_0.4.0;
+
+LIBBPF_0.6.0 {
+ global:
+ btf__add_tag;
+} LIBBPF_0.5.0;
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 4f6ff5c23695..ceb0c98979bc 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -69,6 +69,8 @@
#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)
+#define BTF_TYPE_TAG_ENC(value, type, component_idx) \
+ BTF_TYPE_ENC(value, BTF_INFO_ENC(BTF_KIND_TAG, 0, 0), type), (component_idx)
#ifndef likely
#define likely(x) __builtin_expect(!!(x), 1)
--
2.30.2
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH bpf-next v2 05/11] bpftool: add support for BTF_KIND_TAG
2021-09-13 15:51 [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG Yonghong Song
` (3 preceding siblings ...)
2021-09-13 15:51 ` [PATCH bpf-next v2 04/11] libbpf: add support for BTF_KIND_TAG Yonghong Song
@ 2021-09-13 15:51 ` Yonghong Song
2021-09-14 5:16 ` Andrii Nakryiko
2021-09-13 15:51 ` [PATCH bpf-next v2 06/11] selftests/bpf: test libbpf API function btf__add_tag() Yonghong Song
` (6 subsequent siblings)
11 siblings, 1 reply; 43+ messages in thread
From: Yonghong Song @ 2021-09-13 15:51 UTC (permalink / raw)
To: bpf; +Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
Added bpftool support to dump BTF_KIND_TAG information.
The new bpftool will be used in later patches to dump
btf in the test bpf program object file.
Currently, the tags are not emitted with
bpftool btf dump file <path> format c
and they are silently ignored. The tag information is
mostly used in the kernel for verification purpose and the kernel
uses its own btf to check. With adding these tags
to vmlinux.h, tags will be encoded in program's btf but
they will not be used by the kernel, at least for now.
So let us delay adding these tags to format C header files
until there is a real need.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
tools/bpf/bpftool/btf.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
index f7e5ff3586c9..49743ad96851 100644
--- a/tools/bpf/bpftool/btf.c
+++ b/tools/bpf/bpftool/btf.c
@@ -37,6 +37,7 @@ static const char * const btf_kind_str[NR_BTF_KINDS] = {
[BTF_KIND_VAR] = "VAR",
[BTF_KIND_DATASEC] = "DATASEC",
[BTF_KIND_FLOAT] = "FLOAT",
+ [BTF_KIND_TAG] = "TAG",
};
struct btf_attach_table {
@@ -347,6 +348,17 @@ static int dump_btf_type(const struct btf *btf, __u32 id,
printf(" size=%u", t->size);
break;
}
+ case BTF_KIND_TAG: {
+ const struct btf_tag *tag = (const void *)(t + 1);
+
+ if (json_output) {
+ jsonw_uint_field(w, "type_id", t->type);
+ jsonw_int_field(w, "component_idx", tag->component_idx);
+ } else {
+ printf(" type_id=%u component_idx=%d", t->type, tag->component_idx);
+ }
+ break;
+ }
default:
break;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH bpf-next v2 06/11] selftests/bpf: test libbpf API function btf__add_tag()
2021-09-13 15:51 [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG Yonghong Song
` (4 preceding siblings ...)
2021-09-13 15:51 ` [PATCH bpf-next v2 05/11] bpftool: " Yonghong Song
@ 2021-09-13 15:51 ` Yonghong Song
2021-09-14 5:18 ` Andrii Nakryiko
2021-09-13 15:52 ` [PATCH bpf-next v2 07/11] selftests/bpf: change NAME_NTH/IS_NAME_NTH for BTF_KIND_TAG format Yonghong Song
` (5 subsequent siblings)
11 siblings, 1 reply; 43+ messages in thread
From: Yonghong Song @ 2021-09-13 15:51 UTC (permalink / raw)
To: bpf; +Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
Add btf_write tests with btf__add_tag() function.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
tools/testing/selftests/bpf/btf_helpers.c | 7 ++++++-
.../selftests/bpf/prog_tests/btf_write.c | 21 +++++++++++++++++++
2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/btf_helpers.c b/tools/testing/selftests/bpf/btf_helpers.c
index b692e6ead9b5..ce103fb0ad1b 100644
--- a/tools/testing/selftests/bpf/btf_helpers.c
+++ b/tools/testing/selftests/bpf/btf_helpers.c
@@ -24,11 +24,12 @@ static const char * const btf_kind_str_mapping[] = {
[BTF_KIND_VAR] = "VAR",
[BTF_KIND_DATASEC] = "DATASEC",
[BTF_KIND_FLOAT] = "FLOAT",
+ [BTF_KIND_TAG] = "TAG",
};
static const char *btf_kind_str(__u16 kind)
{
- if (kind > BTF_KIND_DATASEC)
+ if (kind > BTF_KIND_TAG)
return "UNKNOWN";
return btf_kind_str_mapping[kind];
}
@@ -177,6 +178,10 @@ int fprintf_btf_type_raw(FILE *out, const struct btf *btf, __u32 id)
case BTF_KIND_FLOAT:
fprintf(out, " size=%u", t->size);
break;
+ case BTF_KIND_TAG:
+ fprintf(out, " type_id=%u component_idx=%d",
+ t->type, btf_tag(t)->component_idx);
+ break;
default:
break;
}
diff --git a/tools/testing/selftests/bpf/prog_tests/btf_write.c b/tools/testing/selftests/bpf/prog_tests/btf_write.c
index 022c7d89d6f4..76548eecce2c 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_write.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_write.c
@@ -281,5 +281,26 @@ void test_btf_write() {
"[17] DATASEC 'datasec1' size=12 vlen=1\n"
"\ttype_id=1 offset=4 size=8", "raw_dump");
+ /* TAG */
+ id = btf__add_tag(btf, "tag1", 16, -1);
+ ASSERT_EQ(id, 18, "tag_id");
+ t = btf__type_by_id(btf, 18);
+ ASSERT_STREQ(btf__str_by_offset(btf, t->name_off), "tag1", "tag_value");
+ ASSERT_EQ(btf_kind(t), BTF_KIND_TAG, "tag_kind");
+ ASSERT_EQ(t->type, 16, "tag_type");
+ ASSERT_EQ(btf_tag(t)->component_idx, -1, "tag_component_idx");
+ ASSERT_STREQ(btf_type_raw_dump(btf, 18),
+ "[18] TAG 'tag1' type_id=16 component_idx=-1", "raw_dump");
+
+ id = btf__add_tag(btf, "tag2", 14, 1);
+ ASSERT_EQ(id, 19, "tag_id");
+ t = btf__type_by_id(btf, 19);
+ ASSERT_STREQ(btf__str_by_offset(btf, t->name_off), "tag2", "tag_value");
+ ASSERT_EQ(btf_kind(t), BTF_KIND_TAG, "tag_kind");
+ ASSERT_EQ(t->type, 14, "tag_type");
+ ASSERT_EQ(btf_tag(t)->component_idx, 1, "tag_component_idx");
+ ASSERT_STREQ(btf_type_raw_dump(btf, 19),
+ "[19] TAG 'tag2' type_id=14 component_idx=1", "raw_dump");
+
btf__free(btf);
}
--
2.30.2
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH bpf-next v2 07/11] selftests/bpf: change NAME_NTH/IS_NAME_NTH for BTF_KIND_TAG format
2021-09-13 15:51 [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG Yonghong Song
` (5 preceding siblings ...)
2021-09-13 15:51 ` [PATCH bpf-next v2 06/11] selftests/bpf: test libbpf API function btf__add_tag() Yonghong Song
@ 2021-09-13 15:52 ` Yonghong Song
2021-09-14 5:23 ` Andrii Nakryiko
2021-09-13 15:52 ` [PATCH bpf-next v2 08/11] selftests/bpf: add BTF_KIND_TAG unit tests Yonghong Song
` (4 subsequent siblings)
11 siblings, 1 reply; 43+ messages in thread
From: Yonghong Song @ 2021-09-13 15:52 UTC (permalink / raw)
To: bpf; +Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
BTF_KIND_TAG ELF format has a component_idx which might have value -1.
test_btf may confuse it with common_type.name as NAME_NTH checkes
high 16bit to be 0xffff. Change NAME_NTH high 16bit check to be
0xfffe so it won't confuse with component_idx.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
tools/testing/selftests/bpf/prog_tests/btf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
index 649f87382c8d..ad39f4d588d0 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf.c
@@ -39,8 +39,8 @@ static bool always_log;
#define BTF_END_RAW 0xdeadbeef
#define NAME_TBD 0xdeadb33f
-#define NAME_NTH(N) (0xffff0000 | N)
-#define IS_NAME_NTH(X) ((X & 0xffff0000) == 0xffff0000)
+#define NAME_NTH(N) (0xfffe0000 | N)
+#define IS_NAME_NTH(X) ((X & 0xffff0000) == 0xfffe0000)
#define GET_NAME_NTH_IDX(X) (X & 0x0000ffff)
#define MAX_NR_RAW_U32 1024
--
2.30.2
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH bpf-next v2 08/11] selftests/bpf: add BTF_KIND_TAG unit tests
2021-09-13 15:51 [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG Yonghong Song
` (6 preceding siblings ...)
2021-09-13 15:52 ` [PATCH bpf-next v2 07/11] selftests/bpf: change NAME_NTH/IS_NAME_NTH for BTF_KIND_TAG format Yonghong Song
@ 2021-09-13 15:52 ` Yonghong Song
2021-09-14 5:31 ` Andrii Nakryiko
2021-09-13 15:52 ` [PATCH bpf-next v2 09/11] selftests/bpf: test BTF_KIND_TAG for deduplication Yonghong Song
` (3 subsequent siblings)
11 siblings, 1 reply; 43+ messages in thread
From: Yonghong Song @ 2021-09-13 15:52 UTC (permalink / raw)
To: bpf; +Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
Test good and bad variants of BTF_KIND_TAG encoding.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
tools/testing/selftests/bpf/prog_tests/btf.c | 223 +++++++++++++++++++
tools/testing/selftests/bpf/test_btf.h | 3 +
2 files changed, 226 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
index ad39f4d588d0..21b122f72a55 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf.c
@@ -3661,6 +3661,227 @@ static struct btf_raw_test raw_tests[] = {
.err_str = "Invalid type_size",
},
+{
+ .descr = "tag test #1, struct/member, well-formed",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_STRUCT_ENC(0, 2, 8), /* [2] */
+ BTF_MEMBER_ENC(NAME_TBD, 1, 0),
+ BTF_MEMBER_ENC(NAME_TBD, 1, 32),
+ BTF_TAG_ENC(NAME_TBD, 2, -1),
+ BTF_TAG_ENC(NAME_TBD, 2, 0),
+ BTF_TAG_ENC(NAME_TBD, 2, 1),
+ BTF_END_RAW,
+ },
+ BTF_STR_SEC("\0m1\0m2\0tag1\0tag2\0tag3"),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "tag_type_check_btf",
+ .key_size = sizeof(int),
+ .value_size = 8,
+ .key_type_id = 1,
+ .value_type_id = 2,
+ .max_entries = 1,
+},
+{
+ .descr = "tag test #2, union/member, well-formed",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_STRUCT_ENC(NAME_TBD, 2, 4), /* [2] */
+ BTF_MEMBER_ENC(NAME_TBD, 1, 0),
+ BTF_MEMBER_ENC(NAME_TBD, 1, 0),
+ BTF_TAG_ENC(NAME_TBD, 2, -1),
+ BTF_TAG_ENC(NAME_TBD, 2, 0),
+ BTF_TAG_ENC(NAME_TBD, 2, 1),
+ BTF_END_RAW,
+ },
+ BTF_STR_SEC("\0t\0m1\0m2\0tag1\0tag2\0tag3"),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "tag_type_check_btf",
+ .key_size = sizeof(int),
+ .value_size = 4,
+ .key_type_id = 1,
+ .value_type_id = 2,
+ .max_entries = 1,
+},
+{
+ .descr = "tag test #3, variable, well-formed",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_VAR_ENC(NAME_TBD, 1, 0), /* [2] */
+ BTF_VAR_ENC(NAME_TBD, 1, 1), /* [3] */
+ BTF_TAG_ENC(NAME_TBD, 2, -1),
+ BTF_TAG_ENC(NAME_TBD, 3, -1),
+ BTF_END_RAW,
+ },
+ BTF_STR_SEC("\0local\0global\0tag1\0tag2"),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "tag_type_check_btf",
+ .key_size = sizeof(int),
+ .value_size = 4,
+ .key_type_id = 1,
+ .value_type_id = 1,
+ .max_entries = 1,
+},
+{
+ .descr = "tag test #4, func/parameter, well-formed",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_FUNC_PROTO_ENC(0, 2), /* [2] */
+ BTF_FUNC_PROTO_ARG_ENC(NAME_TBD, 1),
+ BTF_FUNC_PROTO_ARG_ENC(NAME_TBD, 1),
+ BTF_FUNC_ENC(NAME_TBD, 2), /* [3] */
+ BTF_TAG_ENC(NAME_TBD, 3, -1),
+ BTF_TAG_ENC(NAME_TBD, 3, 0),
+ BTF_TAG_ENC(NAME_TBD, 3, 1),
+ BTF_END_RAW,
+ },
+ BTF_STR_SEC("\0arg1\0arg2\0f\0tag1\0tag2\0tag3"),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "tag_type_check_btf",
+ .key_size = sizeof(int),
+ .value_size = 4,
+ .key_type_id = 1,
+ .value_type_id = 1,
+ .max_entries = 1,
+},
+{
+ .descr = "tag test #5, invalid value",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_VAR_ENC(NAME_TBD, 1, 0), /* [2] */
+ BTF_TAG_ENC(0, 2, -1),
+ BTF_END_RAW,
+ },
+ BTF_STR_SEC("\0local\0tag"),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "tag_type_check_btf",
+ .key_size = sizeof(int),
+ .value_size = 4,
+ .key_type_id = 1,
+ .value_type_id = 1,
+ .max_entries = 1,
+ .btf_load_err = true,
+ .err_str = "Invalid value",
+},
+{
+ .descr = "tag test #6, invalid target type",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_TAG_ENC(NAME_TBD, 1, -1),
+ BTF_END_RAW,
+ },
+ BTF_STR_SEC("\0tag1"),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "tag_type_check_btf",
+ .key_size = sizeof(int),
+ .value_size = 4,
+ .key_type_id = 1,
+ .value_type_id = 1,
+ .max_entries = 1,
+ .btf_load_err = true,
+ .err_str = "Invalid type",
+},
+{
+ .descr = "tag test #7, invalid vlen",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_VAR_ENC(NAME_TBD, 1, 0), /* [2] */
+ BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_TAG, 1, 1), 2), (0),
+ BTF_END_RAW,
+ },
+ BTF_STR_SEC("\0local\0tag1"),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "tag_type_check_btf",
+ .key_size = sizeof(int),
+ .value_size = 4,
+ .key_type_id = 1,
+ .value_type_id = 1,
+ .max_entries = 1,
+ .btf_load_err = true,
+ .err_str = "vlen != 0",
+},
+{
+ .descr = "tag test #8, invalid kflag",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_VAR_ENC(NAME_TBD, 1, 0), /* [2] */
+ BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_TAG, 1, 0), 2), (-1),
+ BTF_END_RAW,
+ },
+ BTF_STR_SEC("\0local\0tag1"),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "tag_type_check_btf",
+ .key_size = sizeof(int),
+ .value_size = 4,
+ .key_type_id = 1,
+ .value_type_id = 1,
+ .max_entries = 1,
+ .btf_load_err = true,
+ .err_str = "Invalid btf_info kind_flag",
+},
+{
+ .descr = "tag test #9, var, invalid component_idx",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_VAR_ENC(NAME_TBD, 1, 0), /* [2] */
+ BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_TAG, 0, 0), 2), (0),
+ BTF_END_RAW,
+ },
+ BTF_STR_SEC("\0local\0tag"),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "tag_type_check_btf",
+ .key_size = sizeof(int),
+ .value_size = 4,
+ .key_type_id = 1,
+ .value_type_id = 1,
+ .max_entries = 1,
+ .btf_load_err = true,
+ .err_str = "Invalid component_idx",
+},
+{
+ .descr = "tag test #10, struct member, invalid component_idx",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_STRUCT_ENC(0, 2, 8), /* [2] */
+ BTF_MEMBER_ENC(NAME_TBD, 1, 0),
+ BTF_MEMBER_ENC(NAME_TBD, 1, 32),
+ BTF_TAG_ENC(NAME_TBD, 2, 2),
+ BTF_END_RAW,
+ },
+ BTF_STR_SEC("\0m1\0m2\0tag"),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "tag_type_check_btf",
+ .key_size = sizeof(int),
+ .value_size = 8,
+ .key_type_id = 1,
+ .value_type_id = 2,
+ .max_entries = 1,
+ .btf_load_err = true,
+ .err_str = "Invalid component_idx",
+},
+{
+ .descr = "tag test #11, func parameter, invalid component_idx",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_FUNC_PROTO_ENC(0, 2), /* [2] */
+ BTF_FUNC_PROTO_ARG_ENC(NAME_TBD, 1),
+ BTF_FUNC_PROTO_ARG_ENC(NAME_TBD, 1),
+ BTF_FUNC_ENC(NAME_TBD, 2), /* [3] */
+ BTF_TAG_ENC(NAME_TBD, 3, 2),
+ BTF_END_RAW,
+ },
+ BTF_STR_SEC("\0arg1\0arg2\0f\0tag"),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "tag_type_check_btf",
+ .key_size = sizeof(int),
+ .value_size = 4,
+ .key_type_id = 1,
+ .value_type_id = 1,
+ .max_entries = 1,
+ .btf_load_err = true,
+ .err_str = "Invalid component_idx",
+},
+
}; /* struct btf_raw_test raw_tests[] */
static const char *get_next_str(const char *start, const char *end)
@@ -6801,6 +7022,8 @@ static int btf_type_size(const struct btf_type *t)
return base_size + sizeof(struct btf_var);
case BTF_KIND_DATASEC:
return base_size + vlen * sizeof(struct btf_var_secinfo);
+ case BTF_KIND_TAG:
+ return base_size + sizeof(struct btf_tag);
default:
fprintf(stderr, "Unsupported BTF_KIND:%u\n", kind);
return -EINVAL;
diff --git a/tools/testing/selftests/bpf/test_btf.h b/tools/testing/selftests/bpf/test_btf.h
index e2394eea4b7f..0619e06d745e 100644
--- a/tools/testing/selftests/bpf/test_btf.h
+++ b/tools/testing/selftests/bpf/test_btf.h
@@ -69,4 +69,7 @@
#define BTF_TYPE_FLOAT_ENC(name, sz) \
BTF_TYPE_ENC(name, BTF_INFO_ENC(BTF_KIND_FLOAT, 0, 0), sz)
+#define BTF_TAG_ENC(value, type, component_idx) \
+ BTF_TYPE_ENC(value, BTF_INFO_ENC(BTF_KIND_TAG, 0, 0), type), (component_idx)
+
#endif /* _TEST_BTF_H */
--
2.30.2
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH bpf-next v2 09/11] selftests/bpf: test BTF_KIND_TAG for deduplication
2021-09-13 15:51 [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG Yonghong Song
` (7 preceding siblings ...)
2021-09-13 15:52 ` [PATCH bpf-next v2 08/11] selftests/bpf: add BTF_KIND_TAG unit tests Yonghong Song
@ 2021-09-13 15:52 ` Yonghong Song
2021-09-14 5:38 ` Andrii Nakryiko
2021-09-13 15:52 ` [PATCH bpf-next v2 10/11] selftests/bpf: add a test with a bpf program with btf_tag attributes Yonghong Song
` (2 subsequent siblings)
11 siblings, 1 reply; 43+ messages in thread
From: Yonghong Song @ 2021-09-13 15:52 UTC (permalink / raw)
To: bpf; +Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
Add unit tests for BTF_KIND_TAG deduplication for
- struct and struct member
- variable
- func and func argument
Signed-off-by: Yonghong Song <yhs@fb.com>
---
tools/testing/selftests/bpf/prog_tests/btf.c | 91 ++++++++++++++++----
1 file changed, 74 insertions(+), 17 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
index 21b122f72a55..1ef0346be7b4 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf.c
@@ -6642,27 +6642,33 @@ const struct btf_dedup_test dedup_tests[] = {
BTF_MEMBER_ENC(NAME_NTH(4), 5, 64), /* const int *a; */
BTF_MEMBER_ENC(NAME_NTH(5), 2, 128), /* int b[16]; */
BTF_MEMBER_ENC(NAME_NTH(6), 1, 640), /* int c; */
- BTF_MEMBER_ENC(NAME_NTH(8), 13, 672), /* float d; */
+ BTF_MEMBER_ENC(NAME_NTH(8), 15, 672), /* float d; */
/* ptr -> [3] struct s */
BTF_PTR_ENC(3), /* [4] */
/* ptr -> [6] const int */
BTF_PTR_ENC(6), /* [5] */
/* const -> [1] int */
BTF_CONST_ENC(1), /* [6] */
+ /* tag -> [3] struct s */
+ BTF_TAG_ENC(NAME_NTH(2), 3, -1), /* [7] */
+ /* tag -> [3] struct s, member 1 */
+ BTF_TAG_ENC(NAME_NTH(2), 3, 1), /* [8] */
/* full copy of the above */
- BTF_TYPE_INT_ENC(NAME_NTH(1), BTF_INT_SIGNED, 0, 32, 4), /* [7] */
- BTF_TYPE_ARRAY_ENC(7, 7, 16), /* [8] */
- BTF_STRUCT_ENC(NAME_NTH(2), 5, 88), /* [9] */
- BTF_MEMBER_ENC(NAME_NTH(3), 10, 0),
- BTF_MEMBER_ENC(NAME_NTH(4), 11, 64),
- BTF_MEMBER_ENC(NAME_NTH(5), 8, 128),
- BTF_MEMBER_ENC(NAME_NTH(6), 7, 640),
- BTF_MEMBER_ENC(NAME_NTH(8), 13, 672),
- BTF_PTR_ENC(9), /* [10] */
- BTF_PTR_ENC(12), /* [11] */
- BTF_CONST_ENC(7), /* [12] */
- BTF_TYPE_FLOAT_ENC(NAME_NTH(7), 4), /* [13] */
+ BTF_TYPE_INT_ENC(NAME_NTH(1), BTF_INT_SIGNED, 0, 32, 4), /* [9] */
+ BTF_TYPE_ARRAY_ENC(9, 9, 16), /* [10] */
+ BTF_STRUCT_ENC(NAME_NTH(2), 5, 88), /* [11] */
+ BTF_MEMBER_ENC(NAME_NTH(3), 12, 0),
+ BTF_MEMBER_ENC(NAME_NTH(4), 13, 64),
+ BTF_MEMBER_ENC(NAME_NTH(5), 10, 128),
+ BTF_MEMBER_ENC(NAME_NTH(6), 9, 640),
+ BTF_MEMBER_ENC(NAME_NTH(8), 15, 672),
+ BTF_PTR_ENC(11), /* [12] */
+ BTF_PTR_ENC(14), /* [13] */
+ BTF_CONST_ENC(9), /* [14] */
+ BTF_TYPE_FLOAT_ENC(NAME_NTH(7), 4), /* [15] */
+ BTF_TAG_ENC(NAME_NTH(2), 11, -1), /* [16] */
+ BTF_TAG_ENC(NAME_NTH(2), 11, 1), /* [17] */
BTF_END_RAW,
},
BTF_STR_SEC("\0int\0s\0next\0a\0b\0c\0float\0d"),
@@ -6679,14 +6685,16 @@ const struct btf_dedup_test dedup_tests[] = {
BTF_MEMBER_ENC(NAME_NTH(1), 5, 64), /* const int *a; */
BTF_MEMBER_ENC(NAME_NTH(2), 2, 128), /* int b[16]; */
BTF_MEMBER_ENC(NAME_NTH(3), 1, 640), /* int c; */
- BTF_MEMBER_ENC(NAME_NTH(4), 7, 672), /* float d; */
+ BTF_MEMBER_ENC(NAME_NTH(4), 9, 672), /* float d; */
/* ptr -> [3] struct s */
BTF_PTR_ENC(3), /* [4] */
/* ptr -> [6] const int */
BTF_PTR_ENC(6), /* [5] */
/* const -> [1] int */
BTF_CONST_ENC(1), /* [6] */
- BTF_TYPE_FLOAT_ENC(NAME_NTH(7), 4), /* [7] */
+ BTF_TAG_ENC(NAME_NTH(2), 3, -1), /* [7] */
+ BTF_TAG_ENC(NAME_NTH(2), 3, 1), /* [8] */
+ BTF_TYPE_FLOAT_ENC(NAME_NTH(7), 4), /* [9] */
BTF_END_RAW,
},
BTF_STR_SEC("\0a\0b\0c\0d\0int\0float\0next\0s"),
@@ -6811,9 +6819,11 @@ const struct btf_dedup_test dedup_tests[] = {
BTF_FUNC_PROTO_ARG_ENC(NAME_TBD, 8),
BTF_FUNC_ENC(NAME_TBD, 12), /* [13] func */
BTF_TYPE_FLOAT_ENC(NAME_TBD, 2), /* [14] float */
+ BTF_TAG_ENC(NAME_TBD, 13, -1), /* [15] tag */
+ BTF_TAG_ENC(NAME_TBD, 13, 1), /* [16] tag */
BTF_END_RAW,
},
- BTF_STR_SEC("\0A\0B\0C\0D\0E\0F\0G\0H\0I\0J\0K\0L\0M\0N"),
+ BTF_STR_SEC("\0A\0B\0C\0D\0E\0F\0G\0H\0I\0J\0K\0L\0M\0N\0O\0P"),
},
.expect = {
.raw_types = {
@@ -6837,9 +6847,11 @@ const struct btf_dedup_test dedup_tests[] = {
BTF_FUNC_PROTO_ARG_ENC(NAME_TBD, 8),
BTF_FUNC_ENC(NAME_TBD, 12), /* [13] func */
BTF_TYPE_FLOAT_ENC(NAME_TBD, 2), /* [14] float */
+ BTF_TAG_ENC(NAME_TBD, 13, -1), /* [15] tag */
+ BTF_TAG_ENC(NAME_TBD, 13, 1), /* [16] tag */
BTF_END_RAW,
},
- BTF_STR_SEC("\0A\0B\0C\0D\0E\0F\0G\0H\0I\0J\0K\0L\0M\0N"),
+ BTF_STR_SEC("\0A\0B\0C\0D\0E\0F\0G\0H\0I\0J\0K\0L\0M\0N\0O\0P"),
},
.opts = {
.dont_resolve_fwds = false,
@@ -6988,6 +7000,51 @@ const struct btf_dedup_test dedup_tests[] = {
.dedup_table_size = 1
},
},
+{
+ .descr = "dedup: func/func_arg/var tags",
+ .input = {
+ .raw_types = {
+ /* int */
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ /* static int t */
+ BTF_VAR_ENC(NAME_NTH(1), 1, 0), /* [2] */
+ /* void f(int a1, int a2) */
+ BTF_FUNC_PROTO_ENC(0, 2), /* [3] */
+ BTF_FUNC_PROTO_ARG_ENC(NAME_NTH(2), 1),
+ BTF_FUNC_PROTO_ARG_ENC(NAME_NTH(3), 1),
+ BTF_FUNC_ENC(NAME_NTH(4), 2), /* [4] */
+ /* tag -> t */
+ BTF_TAG_ENC(NAME_NTH(5), 2, -1), /* [5] */
+ BTF_TAG_ENC(NAME_NTH(5), 2, -1), /* [6] */
+ /* tag -> func */
+ BTF_TAG_ENC(NAME_NTH(5), 4, -1), /* [7] */
+ BTF_TAG_ENC(NAME_NTH(5), 4, -1), /* [8] */
+ /* tag -> func arg a1 */
+ BTF_TAG_ENC(NAME_NTH(5), 4, 1), /* [9] */
+ BTF_TAG_ENC(NAME_NTH(5), 4, 1), /* [10] */
+ BTF_END_RAW,
+ },
+ BTF_STR_SEC("\0t\0a1\0a2\0f\0tag"),
+ },
+ .expect = {
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_VAR_ENC(NAME_NTH(1), 1, 0), /* [2] */
+ BTF_FUNC_PROTO_ENC(0, 2), /* [3] */
+ BTF_FUNC_PROTO_ARG_ENC(NAME_NTH(2), 1),
+ BTF_FUNC_PROTO_ARG_ENC(NAME_NTH(3), 1),
+ BTF_FUNC_ENC(NAME_NTH(4), 2), /* [4] */
+ BTF_TAG_ENC(NAME_NTH(5), 2, -1), /* [5] */
+ BTF_TAG_ENC(NAME_NTH(5), 4, -1), /* [6] */
+ BTF_TAG_ENC(NAME_NTH(5), 4, 1), /* [7] */
+ BTF_END_RAW,
+ },
+ BTF_STR_SEC("\0t\0a1\0a2\0f\0tag"),
+ },
+ .opts = {
+ .dont_resolve_fwds = false,
+ },
+},
};
--
2.30.2
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH bpf-next v2 10/11] selftests/bpf: add a test with a bpf program with btf_tag attributes
2021-09-13 15:51 [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG Yonghong Song
` (8 preceding siblings ...)
2021-09-13 15:52 ` [PATCH bpf-next v2 09/11] selftests/bpf: test BTF_KIND_TAG for deduplication Yonghong Song
@ 2021-09-13 15:52 ` Yonghong Song
2021-09-13 15:52 ` [PATCH bpf-next v2 11/11] docs/bpf: add documentation for BTF_KIND_TAG Yonghong Song
2021-09-13 16:08 ` [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG Yonghong Song
11 siblings, 0 replies; 43+ messages in thread
From: Yonghong Song @ 2021-09-13 15:52 UTC (permalink / raw)
To: bpf; +Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
Add a bpf program with btf_tag attributes. The program is
loaded successfully with the kernel. With the command
bpftool btf dump file ./tag.o
the following dump shows that tags are properly encoded:
[8] STRUCT 'key_t' size=12 vlen=3
'a' type_id=2 bits_offset=0
'b' type_id=2 bits_offset=32
'c' type_id=2 bits_offset=64
[9] TAG 'tag1' type_id=8 component_id=-1
[10] TAG 'tag2' type_id=8 component_id=-1
[11] TAG 'tag1' type_id=8 component_id=1
[12] TAG 'tag2' type_id=8 component_id=1
...
[21] FUNC_PROTO '(anon)' ret_type_id=2 vlen=1
'x' type_id=2
[22] FUNC 'foo' type_id=21 linkage=static
[23] TAG 'tag1' type_id=22 component_id=0
[24] TAG 'tag2' type_id=22 component_id=0
[25] TAG 'tag1' type_id=22 component_id=-1
[26] TAG 'tag2' type_id=22 component_id=-1
...
[29] VAR 'total' type_id=27, linkage=global
[30] TAG 'tag1' type_id=29 component_id=-1
[31] TAG 'tag2' type_id=29 component_id=-1
If an old clang compiler, which does not support btf_tag attribute,
is used, these btf_tag attributes will be silently ignored.
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
.../selftests/bpf/prog_tests/btf_tag.c | 14 +++++++
tools/testing/selftests/bpf/progs/tag.c | 39 +++++++++++++++++++
2 files changed, 53 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/btf_tag.c
create mode 100644 tools/testing/selftests/bpf/progs/tag.c
diff --git a/tools/testing/selftests/bpf/prog_tests/btf_tag.c b/tools/testing/selftests/bpf/prog_tests/btf_tag.c
new file mode 100644
index 000000000000..f939527ede77
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/btf_tag.c
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2021 Facebook */
+#include <test_progs.h>
+#include "tag.skel.h"
+
+void test_btf_tag(void)
+{
+ struct tag *skel;
+
+ skel = tag__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "btf_tag"))
+ return;
+ tag__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/tag.c b/tools/testing/selftests/bpf/progs/tag.c
new file mode 100644
index 000000000000..17f88c58a6c5
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/tag.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2021 Facebook */
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+#define __tag1 __attribute__((btf_tag("tag1")))
+#define __tag2 __attribute__((btf_tag("tag2")))
+
+struct key_t {
+ int a;
+ int b __tag1 __tag2;
+ int c;
+} __tag1 __tag2;
+
+struct {
+ __uint(type, BPF_MAP_TYPE_HASH);
+ __uint(max_entries, 3);
+ __type(key, struct key_t);
+ __type(value, __u64);
+} hashmap1 SEC(".maps");
+
+__u32 total __tag1 __tag2 = 0;
+
+static __noinline int foo(int x __tag1 __tag2) __tag1 __tag2
+{
+ struct key_t key;
+ __u64 val = 1;
+
+ key.a = key.b = key.c = x;
+ bpf_map_update_elem(&hashmap1, &key, &val, 0);
+ return 0;
+}
+
+SEC("fentry/bpf_fentry_test1")
+int BPF_PROG(sub, int x)
+{
+ return foo(x);
+}
--
2.30.2
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH bpf-next v2 11/11] docs/bpf: add documentation for BTF_KIND_TAG
2021-09-13 15:51 [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG Yonghong Song
` (9 preceding siblings ...)
2021-09-13 15:52 ` [PATCH bpf-next v2 10/11] selftests/bpf: add a test with a bpf program with btf_tag attributes Yonghong Song
@ 2021-09-13 15:52 ` Yonghong Song
2021-09-14 5:40 ` Andrii Nakryiko
2021-09-13 16:08 ` [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG Yonghong Song
11 siblings, 1 reply; 43+ messages in thread
From: Yonghong Song @ 2021-09-13 15:52 UTC (permalink / raw)
To: bpf; +Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
Add BTF_KIND_TAG documentation in btf.rst.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
Documentation/bpf/btf.rst | 27 ++++++++++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/Documentation/bpf/btf.rst b/Documentation/bpf/btf.rst
index 846354cd2d69..9fff578a0a35 100644
--- a/Documentation/bpf/btf.rst
+++ b/Documentation/bpf/btf.rst
@@ -85,6 +85,7 @@ sequentially and type id is assigned to each recognized type starting from id
#define BTF_KIND_VAR 14 /* Variable */
#define BTF_KIND_DATASEC 15 /* Section */
#define BTF_KIND_FLOAT 16 /* Floating point */
+ #define BTF_KIND_TAG 17 /* Tag */
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.
@@ -106,7 +107,7 @@ Each type contains the following common data::
* "size" tells the size of the type it is describing.
*
* "type" is used by PTR, TYPEDEF, VOLATILE, CONST, RESTRICT,
- * FUNC and FUNC_PROTO.
+ * FUNC, FUNC_PROTO and TAG.
* "type" is a type_id referring to another type.
*/
union {
@@ -465,6 +466,30 @@ map definition.
No additional type data follow ``btf_type``.
+2.2.17 BTF_KIND_TAG
+~~~~~~~~~~~~~~~~~~~
+
+``struct btf_type`` encoding requirement:
+ * ``name_off``: offset to a non-empty string
+ * ``info.kind_flag``: 0
+ * ``info.kind``: BTF_KIND_TAG
+ * ``info.vlen``: 0
+ * ``type``: ``struct``, ``union``, ``func`` or ``var``
+
+``btf_type`` is followed by ``struct btf_tag``.::
+
+ struct btf_tag {
+ __u32 component_idx;
+ };
+
+The ``name_off`` encodes btf_tag attribute string.
+The ``type`` should be ``struct``, ``union``, ``func`` or ``var``.
+If ``btf_tag.component_idx = -1``, the btf_tag attribute is
+applied to a valid ``type``. Otherwise, the btf_tag attribute is
+applied to a ``struct``/``union`` member or a ``func`` argument,
+and ``btf_tag.component_idx`` should be a valid index (starting from 0)
+pointing to a member or an argument.
+
3. BTF Kernel API
*****************
--
2.30.2
^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG
2021-09-13 15:51 [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG Yonghong Song
` (10 preceding siblings ...)
2021-09-13 15:52 ` [PATCH bpf-next v2 11/11] docs/bpf: add documentation for BTF_KIND_TAG Yonghong Song
@ 2021-09-13 16:08 ` Yonghong Song
2021-09-13 16:40 ` Jose E. Marchesi
11 siblings, 1 reply; 43+ messages in thread
From: Yonghong Song @ 2021-09-13 16:08 UTC (permalink / raw)
To: bpf, Jose E. Marchesi
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
cc Jose E. Marchesi
Hi, Jose, just let you know that the BTF format for BTF_KIND_TAG is
changed since v1 as the new format can simplify kernel/libbpf
implementation. Thanks!
On 9/13/21 8:51 AM, Yonghong Song wrote:
> LLVM14 added support for a new C attribute ([1])
> __attribute__((btf_tag("arbitrary_str")))
> This attribute will be emitted to dwarf ([2]) and pahole
> will convert it to BTF. Or for bpf target, this
> attribute will be emitted to BTF directly ([3], [4]).
> The attribute is intended to provide additional
> information for
> - struct/union type or struct/union member
> - static/global variables
> - static/global function or function parameter.
>
> This new attribute can be used to add attributes
> to kernel codes, e.g., pre- or post- conditions,
> allow/deny info, or any other info in which only
> the kernel is interested. Such attributes will
> be processed by clang frontend and emitted to
> dwarf, converting to BTF by pahole. Ultimiately
> the verifier can use these information for
> verification purpose.
>
> The new attribute can also be used for bpf
> programs, e.g., tagging with __user attributes
> for function parameters, specifying global
> function preconditions, etc. Such information
> may help verifier to detect user program
> bugs.
>
> After this series, pahole dwarf->btf converter
> will be enhanced to support new llvm tag
> for btf_tag attribute. With pahole support,
> we will then try to add a few real use case,
> e.g., __user/__rcu tagging, allow/deny list,
> some kernel function precondition, etc,
> in the kernel.
>
> In the rest of the series, Patches 1-2 had
> kernel support. Patches 3-4 added
> libbpf support. Patch 5 added bpftool
> support. Patches 6-10 added various selftests.
> Patch 11 added documentation for the new kind.
>
> [1] https://reviews.llvm.org/D106614
> [2] https://reviews.llvm.org/D106621
> [3] https://reviews.llvm.org/D106622
> [4] https://reviews.llvm.org/D109560
>
> Changelog:
> v1 -> v2:
> - BTF ELF format changed in llvm ([4] above),
> so cross-board change to use the new format.
> - Clarified in commit message that BTF_KIND_TAG
> is not emitted by bpftool btf dump format c.
> - Fix various comments from Andrii.
>
> Yonghong Song (11):
> btf: change BTF_KIND_* macros to enums
> bpf: support for new btf kind BTF_KIND_TAG
> libbpf: rename btf_{hash,equal}_int to btf_{hash,equal}_int_tag
> libbpf: add support for BTF_KIND_TAG
> bpftool: add support for BTF_KIND_TAG
> selftests/bpf: test libbpf API function btf__add_tag()
> selftests/bpf: change NAME_NTH/IS_NAME_NTH for BTF_KIND_TAG format
> selftests/bpf: add BTF_KIND_TAG unit tests
> selftests/bpf: test BTF_KIND_TAG for deduplication
> selftests/bpf: add a test with a bpf program with btf_tag attributes
> docs/bpf: add documentation for BTF_KIND_TAG
>
> Documentation/bpf/btf.rst | 27 +-
> include/uapi/linux/btf.h | 52 +--
> kernel/bpf/btf.c | 120 +++++++
> tools/bpf/bpftool/btf.c | 12 +
> tools/include/uapi/linux/btf.h | 52 +--
> tools/lib/bpf/btf.c | 85 ++++-
> tools/lib/bpf/btf.h | 15 +
> tools/lib/bpf/btf_dump.c | 3 +
> tools/lib/bpf/libbpf.c | 31 +-
> tools/lib/bpf/libbpf.map | 5 +
> tools/lib/bpf/libbpf_internal.h | 2 +
> tools/testing/selftests/bpf/btf_helpers.c | 7 +-
> tools/testing/selftests/bpf/prog_tests/btf.c | 318 ++++++++++++++++--
> .../selftests/bpf/prog_tests/btf_tag.c | 14 +
> .../selftests/bpf/prog_tests/btf_write.c | 21 ++
> tools/testing/selftests/bpf/progs/tag.c | 39 +++
> tools/testing/selftests/bpf/test_btf.h | 3 +
> 17 files changed, 736 insertions(+), 70 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/btf_tag.c
> create mode 100644 tools/testing/selftests/bpf/progs/tag.c
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG
2021-09-13 16:08 ` [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG Yonghong Song
@ 2021-09-13 16:40 ` Jose E. Marchesi
2021-12-16 21:52 ` Yonghong Song
0 siblings, 1 reply; 43+ messages in thread
From: Jose E. Marchesi @ 2021-09-13 16:40 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
> cc Jose E. Marchesi
>
> Hi, Jose, just let you know that the BTF format for BTF_KIND_TAG is
> changed since v1 as the new format can simplify kernel/libbpf
> implementation. Thanks!
Noted. Thanks for the update.
>
> On 9/13/21 8:51 AM, Yonghong Song wrote:
>> LLVM14 added support for a new C attribute ([1])
>> __attribute__((btf_tag("arbitrary_str")))
>> This attribute will be emitted to dwarf ([2]) and pahole
>> will convert it to BTF. Or for bpf target, this
>> attribute will be emitted to BTF directly ([3], [4]).
>> The attribute is intended to provide additional
>> information for
>> - struct/union type or struct/union member
>> - static/global variables
>> - static/global function or function parameter.
>> This new attribute can be used to add attributes
>> to kernel codes, e.g., pre- or post- conditions,
>> allow/deny info, or any other info in which only
>> the kernel is interested. Such attributes will
>> be processed by clang frontend and emitted to
>> dwarf, converting to BTF by pahole. Ultimiately
>> the verifier can use these information for
>> verification purpose.
>> The new attribute can also be used for bpf
>> programs, e.g., tagging with __user attributes
>> for function parameters, specifying global
>> function preconditions, etc. Such information
>> may help verifier to detect user program
>> bugs.
>> After this series, pahole dwarf->btf converter
>> will be enhanced to support new llvm tag
>> for btf_tag attribute. With pahole support,
>> we will then try to add a few real use case,
>> e.g., __user/__rcu tagging, allow/deny list,
>> some kernel function precondition, etc,
>> in the kernel.
>> In the rest of the series, Patches 1-2 had
>> kernel support. Patches 3-4 added
>> libbpf support. Patch 5 added bpftool
>> support. Patches 6-10 added various selftests.
>> Patch 11 added documentation for the new kind.
>> [1] https://reviews.llvm.org/D106614
>> [2] https://reviews.llvm.org/D106621
>> [3] https://reviews.llvm.org/D106622
>> [4] https://reviews.llvm.org/D109560
>> Changelog:
>> v1 -> v2:
>> - BTF ELF format changed in llvm ([4] above),
>> so cross-board change to use the new format.
>> - Clarified in commit message that BTF_KIND_TAG
>> is not emitted by bpftool btf dump format c.
>> - Fix various comments from Andrii.
>> Yonghong Song (11):
>> btf: change BTF_KIND_* macros to enums
>> bpf: support for new btf kind BTF_KIND_TAG
>> libbpf: rename btf_{hash,equal}_int to btf_{hash,equal}_int_tag
>> libbpf: add support for BTF_KIND_TAG
>> bpftool: add support for BTF_KIND_TAG
>> selftests/bpf: test libbpf API function btf__add_tag()
>> selftests/bpf: change NAME_NTH/IS_NAME_NTH for BTF_KIND_TAG format
>> selftests/bpf: add BTF_KIND_TAG unit tests
>> selftests/bpf: test BTF_KIND_TAG for deduplication
>> selftests/bpf: add a test with a bpf program with btf_tag attributes
>> docs/bpf: add documentation for BTF_KIND_TAG
>> Documentation/bpf/btf.rst | 27 +-
>> include/uapi/linux/btf.h | 52 +--
>> kernel/bpf/btf.c | 120 +++++++
>> tools/bpf/bpftool/btf.c | 12 +
>> tools/include/uapi/linux/btf.h | 52 +--
>> tools/lib/bpf/btf.c | 85 ++++-
>> tools/lib/bpf/btf.h | 15 +
>> tools/lib/bpf/btf_dump.c | 3 +
>> tools/lib/bpf/libbpf.c | 31 +-
>> tools/lib/bpf/libbpf.map | 5 +
>> tools/lib/bpf/libbpf_internal.h | 2 +
>> tools/testing/selftests/bpf/btf_helpers.c | 7 +-
>> tools/testing/selftests/bpf/prog_tests/btf.c | 318 ++++++++++++++++--
>> .../selftests/bpf/prog_tests/btf_tag.c | 14 +
>> .../selftests/bpf/prog_tests/btf_write.c | 21 ++
>> tools/testing/selftests/bpf/progs/tag.c | 39 +++
>> tools/testing/selftests/bpf/test_btf.h | 3 +
>> 17 files changed, 736 insertions(+), 70 deletions(-)
>> create mode 100644 tools/testing/selftests/bpf/prog_tests/btf_tag.c
>> create mode 100644 tools/testing/selftests/bpf/progs/tag.c
>>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 01/11] btf: change BTF_KIND_* macros to enums
2021-09-13 15:51 ` [PATCH bpf-next v2 01/11] btf: change BTF_KIND_* macros to enums Yonghong Song
@ 2021-09-14 4:59 ` Andrii Nakryiko
2021-09-14 15:53 ` Yonghong Song
0 siblings, 1 reply; 43+ messages in thread
From: Andrii Nakryiko @ 2021-09-14 4:59 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kernel Team
On Mon, Sep 13, 2021 at 8:51 AM Yonghong Song <yhs@fb.com> wrote:
>
> Change BTF_KIND_* macros to enums so they are encoded in dwarf and
> appear in vmlinux.h. This will make it easier for bpf programs
> to use these constants without macro definitions.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
> include/uapi/linux/btf.h | 36 ++++++++++++++++++----------------
> tools/include/uapi/linux/btf.h | 36 ++++++++++++++++++----------------
> 2 files changed, 38 insertions(+), 34 deletions(-)
>
> diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
> index d27b1708efe9..c32cd6697d63 100644
> --- a/include/uapi/linux/btf.h
> +++ b/include/uapi/linux/btf.h
> @@ -56,23 +56,25 @@ struct btf_type {
> #define BTF_INFO_VLEN(info) ((info) & 0xffff)
> #define BTF_INFO_KFLAG(info) ((info) >> 31)
>
> -#define BTF_KIND_UNKN 0 /* Unknown */
> -#define BTF_KIND_INT 1 /* Integer */
> -#define BTF_KIND_PTR 2 /* Pointer */
> -#define BTF_KIND_ARRAY 3 /* Array */
> -#define BTF_KIND_STRUCT 4 /* Struct */
> -#define BTF_KIND_UNION 5 /* Union */
> -#define BTF_KIND_ENUM 6 /* Enumeration */
> -#define BTF_KIND_FWD 7 /* Forward */
> -#define BTF_KIND_TYPEDEF 8 /* Typedef */
> -#define BTF_KIND_VOLATILE 9 /* Volatile */
> -#define BTF_KIND_CONST 10 /* Const */
> -#define BTF_KIND_RESTRICT 11 /* Restrict */
> -#define BTF_KIND_FUNC 12 /* Function */
> -#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 */
> +enum {
> + BTF_KIND_UNKN = 0, /* Unknown */
> + BTF_KIND_INT, /* Integer */
> + BTF_KIND_PTR, /* Pointer */
> + BTF_KIND_ARRAY, /* Array */
> + BTF_KIND_STRUCT, /* Struct */
> + BTF_KIND_UNION, /* Union */
> + BTF_KIND_ENUM, /* Enumeration */
> + BTF_KIND_FWD, /* Forward */
> + BTF_KIND_TYPEDEF, /* Typedef */
> + BTF_KIND_VOLATILE, /* Volatile */
> + BTF_KIND_CONST, /* Const */
> + BTF_KIND_RESTRICT, /* Restrict */
> + BTF_KIND_FUNC, /* Function */
> + BTF_KIND_FUNC_PROTO, /* Function Proto */
> + BTF_KIND_VAR, /* Variable */
> + BTF_KIND_DATASEC, /* Section */
> + BTF_KIND_FLOAT, /* Floating point */
Can you please leave explicit integer values specified? It's extremely
helpful and much easier in practice, compared to having to count the
number of rows from BTF_KIND_UNKN. Had to do it multiple times with
other BPF constants and was happy I didn't have to do that for
BTF_KIND enums.
> +};
> #define BTF_KIND_MAX BTF_KIND_FLOAT
> #define NR_BTF_KINDS (BTF_KIND_MAX + 1)
these two can be just an enum values as well, and actually will be
"auto-updated", if done this way (I think, haven't really tested)
BTF_KIND_FLOAT = 16,
NR_BTF_KINDS,
BTF_KIND_MAX = NR_BTF_KINDS - 1,
... won't it?
[...]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 02/11] bpf: support for new btf kind BTF_KIND_TAG
2021-09-13 15:51 ` [PATCH bpf-next v2 02/11] bpf: support for new btf kind BTF_KIND_TAG Yonghong Song
@ 2021-09-14 5:08 ` Andrii Nakryiko
2021-09-14 15:59 ` Yonghong Song
0 siblings, 1 reply; 43+ messages in thread
From: Andrii Nakryiko @ 2021-09-14 5:08 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kernel Team
On Mon, Sep 13, 2021 at 8:51 AM Yonghong Song <yhs@fb.com> wrote:
>
> LLVM14 added support for a new C attribute ([1])
> __attribute__((btf_tag("arbitrary_str")))
> This attribute will be emitted to dwarf ([2]) and pahole
> will convert it to BTF. Or for bpf target, this
> attribute will be emitted to BTF directly ([3], [4]).
> The attribute is intended to provide additional
> information for
> - struct/union type or struct/union member
> - static/global variables
> - static/global function or function parameter.
>
> For linux kernel, the btf_tag can be applied
> in various places to specify user pointer,
> function pre- or post- condition, function
> allow/deny in certain context, etc. Such information
> will be encoded in vmlinux BTF and can be used
> by verifier.
>
> The btf_tag can also be applied to bpf programs
> to help global verifiable functions, e.g.,
> specifying preconditions, etc.
>
> This patch added basic parsing and checking support
> in kernel for new BTF_KIND_TAG kind.
>
> [1] https://reviews.llvm.org/D106614
> [2] https://reviews.llvm.org/D106621
> [3] https://reviews.llvm.org/D106622
> [4] https://reviews.llvm.org/D109560
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
> include/uapi/linux/btf.h | 16 ++++-
> kernel/bpf/btf.c | 120 +++++++++++++++++++++++++++++++++
> tools/include/uapi/linux/btf.h | 16 ++++-
> 3 files changed, 148 insertions(+), 4 deletions(-)
>
[...]
>
> +static s32 btf_tag_check_meta(struct btf_verifier_env *env,
> + const struct btf_type *t,
> + u32 meta_left)
> +{
> + const struct btf_tag *tag;
> + u32 meta_needed = sizeof(*tag);
> + const char *value;
> +
> + if (meta_left < meta_needed) {
> + btf_verifier_log_basic(env, t,
> + "meta_left:%u meta_needed:%u",
> + meta_left, meta_needed);
> + return -EINVAL;
> + }
> +
> + value = btf_name_by_offset(env->btf, t->name_off);
> + if (!value || !value[0]) {
> + btf_verifier_log_type(env, t, "Invalid value");
> + 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;
> + }
> +
probably need to enforce that component_idx is >= -1? -2 is not a
valid supported value right now.
> + btf_verifier_log_type(env, t, NULL);
> +
> + return meta_needed;
> +}
> +
> +static int btf_tag_resolve(struct btf_verifier_env *env,
> + const struct resolve_vertex *v)
> +{
> + const struct btf_type *next_type;
> + const struct btf_type *t = v->t;
> + u32 next_type_id = t->type;
> + struct btf *btf = env->btf;
> + s32 component_idx;
> + u32 vlen;
> +
> + next_type = btf_type_by_id(btf, next_type_id);
> + if (!next_type || !btf_type_is_tag_target(next_type)) {
> + btf_verifier_log_type(env, v->t, "Invalid type_id");
> + return -EINVAL;
> + }
> +
> + if (!env_type_is_resolve_sink(env, next_type) &&
> + !env_type_is_resolved(env, next_type_id))
> + return env_stack_push(env, next_type, next_type_id);
> +
> + component_idx = btf_type_tag(t)->component_idx;
> + if (component_idx != -1) {
so here, if it's -2, that should be an error, but currently will be
ignored, right?
> + if (btf_type_is_var(next_type) || component_idx < 0) {
if is_var(next_type) then component_idx should only be -1, nothing
else. Or am I missing some convention?
> + btf_verifier_log_type(env, v->t, "Invalid component_idx");
> + return -EINVAL;
> + }
> +
> + if (btf_type_is_struct(next_type)) {
> + vlen = btf_type_vlen(next_type);
> + } else {
> + next_type = btf_type_by_id(btf, next_type->type);
> + vlen = btf_type_vlen(next_type);
> + }
> +
> + if ((u32)component_idx >= vlen) {
> + btf_verifier_log_type(env, v->t, "Invalid component_idx");
> + return -EINVAL;
> + }
> + }
> +
> + env_stack_pop_resolved(env, next_type_id, 0);
> +
> + return 0;
> +}
> +
[...]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 04/11] libbpf: add support for BTF_KIND_TAG
2021-09-13 15:51 ` [PATCH bpf-next v2 04/11] libbpf: add support for BTF_KIND_TAG Yonghong Song
@ 2021-09-14 5:15 ` Andrii Nakryiko
2021-09-14 16:42 ` Yonghong Song
0 siblings, 1 reply; 43+ messages in thread
From: Andrii Nakryiko @ 2021-09-14 5:15 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kernel Team
On Mon, Sep 13, 2021 at 8:51 AM Yonghong Song <yhs@fb.com> wrote:
>
> Add BTF_KIND_TAG support for parsing and dedup.
> Also added sanitization for BTF_KIND_TAG. If BTF_KIND_TAG is not
> supported in the kernel, sanitize it to INTs.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
> tools/lib/bpf/btf.c | 69 +++++++++++++++++++++++++++++++++
> tools/lib/bpf/btf.h | 15 +++++++
> tools/lib/bpf/btf_dump.c | 3 ++
> tools/lib/bpf/libbpf.c | 31 +++++++++++++--
> tools/lib/bpf/libbpf.map | 5 +++
> tools/lib/bpf/libbpf_internal.h | 2 +
> 6 files changed, 122 insertions(+), 3 deletions(-)
>
Just a few small nits.
[...]
> @@ -2440,6 +2446,49 @@ int btf__add_datasec_var_info(struct btf *btf, int var_type_id, __u32 offset, __
> return 0;
> }
>
> +/*
> + * Append new BTF_KIND_TAG type with:
> + * - *value* - non-empty/non-NULL string;
> + * - *ref_type_id* - referenced type ID, it might not exist yet;
> + * - *component_idx* - -1 for tagging reference type, otherwise struct/union
> + * member or function argument index;
> + * Returns:
> + * - >0, type ID of newly added BTF type;
> + * - <0, on error.
> + */
> +int btf__add_tag(struct btf *btf, const char *value, int ref_type_id,
> + int component_idx)
> +{
> + bool for_ref_type = false;
leftovers from the previous revision? let's just use 0 for kflag argument below
> + struct btf_type *t;
> + int sz, value_off;
> +
> + if (!value || !value[0] || component_idx < -1)
> + return libbpf_err(-EINVAL);
> +
> + if (validate_type_id(ref_type_id))
> + return libbpf_err(-EINVAL);
> +
> + if (btf_ensure_modifiable(btf))
> + return libbpf_err(-ENOMEM);
> +
> + sz = sizeof(struct btf_type) + sizeof(struct btf_tag);
> + t = btf_add_type_mem(btf, sz);
> + if (!t)
> + return libbpf_err(-ENOMEM);
> +
> + value_off = btf__add_str(btf, value);
> + if (value_off < 0)
> + return value_off;
> +
> + t->name_off = value_off;
> + t->info = btf_type_info(BTF_KIND_TAG, 0, for_ref_type);
> + t->type = ref_type_id;
> + ((struct btf_tag *)(t + 1))->component_idx = component_idx;
nit: btf_tag(t)->component_idx
> +
> + return btf_commit_type(btf, sz);
> +}
> +
> struct btf_ext_sec_setup_param {
> __u32 off;
> __u32 len;
[...]
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 8f579c6666b2..4a62ef714562 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -195,6 +195,8 @@ enum kern_feature_id {
> FEAT_BTF_FLOAT,
> /* BPF perf link support */
> FEAT_PERF_LINK,
> + /* BTF_KIND_ATTR support */
s/BTF_KIND_ATTR/BTF_KIND_TAG/
> + FEAT_BTF_TAG,
> __FEAT_CNT,
> };
>
[...]
> static bool kernel_supports(const struct bpf_object *obj, enum kern_feature_id feat_id)
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index bbc53bb25f68..9e649cf9e771 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -386,3 +386,8 @@ LIBBPF_0.5.0 {
> btf_dump__dump_type_data;
> libbpf_set_strict_mode;
> } LIBBPF_0.4.0;
> +
> +LIBBPF_0.6.0 {
> + global:
> + btf__add_tag;
> +} LIBBPF_0.5.0;
you'll need a rebase for this due to my patch for libbpf_version.h, sorry
> diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
> index 4f6ff5c23695..ceb0c98979bc 100644
> --- a/tools/lib/bpf/libbpf_internal.h
> +++ b/tools/lib/bpf/libbpf_internal.h
> @@ -69,6 +69,8 @@
> #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)
> +#define BTF_TYPE_TAG_ENC(value, type, component_idx) \
> + BTF_TYPE_ENC(value, BTF_INFO_ENC(BTF_KIND_TAG, 0, 0), type), (component_idx)
>
> #ifndef likely
> #define likely(x) __builtin_expect(!!(x), 1)
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 05/11] bpftool: add support for BTF_KIND_TAG
2021-09-13 15:51 ` [PATCH bpf-next v2 05/11] bpftool: " Yonghong Song
@ 2021-09-14 5:16 ` Andrii Nakryiko
0 siblings, 0 replies; 43+ messages in thread
From: Andrii Nakryiko @ 2021-09-14 5:16 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kernel Team
On Mon, Sep 13, 2021 at 8:51 AM Yonghong Song <yhs@fb.com> wrote:
>
> Added bpftool support to dump BTF_KIND_TAG information.
> The new bpftool will be used in later patches to dump
> btf in the test bpf program object file.
>
> Currently, the tags are not emitted with
> bpftool btf dump file <path> format c
> and they are silently ignored. The tag information is
> mostly used in the kernel for verification purpose and the kernel
> uses its own btf to check. With adding these tags
> to vmlinux.h, tags will be encoded in program's btf but
> they will not be used by the kernel, at least for now.
> So let us delay adding these tags to format C header files
> until there is a real need.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> tools/bpf/bpftool/btf.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
> index f7e5ff3586c9..49743ad96851 100644
> --- a/tools/bpf/bpftool/btf.c
> +++ b/tools/bpf/bpftool/btf.c
> @@ -37,6 +37,7 @@ static const char * const btf_kind_str[NR_BTF_KINDS] = {
> [BTF_KIND_VAR] = "VAR",
> [BTF_KIND_DATASEC] = "DATASEC",
> [BTF_KIND_FLOAT] = "FLOAT",
> + [BTF_KIND_TAG] = "TAG",
> };
>
> struct btf_attach_table {
> @@ -347,6 +348,17 @@ static int dump_btf_type(const struct btf *btf, __u32 id,
> printf(" size=%u", t->size);
> break;
> }
> + case BTF_KIND_TAG: {
> + const struct btf_tag *tag = (const void *)(t + 1);
> +
> + if (json_output) {
> + jsonw_uint_field(w, "type_id", t->type);
> + jsonw_int_field(w, "component_idx", tag->component_idx);
> + } else {
> + printf(" type_id=%u component_idx=%d", t->type, tag->component_idx);
> + }
> + break;
> + }
> default:
> break;
> }
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 06/11] selftests/bpf: test libbpf API function btf__add_tag()
2021-09-13 15:51 ` [PATCH bpf-next v2 06/11] selftests/bpf: test libbpf API function btf__add_tag() Yonghong Song
@ 2021-09-14 5:18 ` Andrii Nakryiko
0 siblings, 0 replies; 43+ messages in thread
From: Andrii Nakryiko @ 2021-09-14 5:18 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kernel Team
On Mon, Sep 13, 2021 at 8:52 AM Yonghong Song <yhs@fb.com> wrote:
>
> Add btf_write tests with btf__add_tag() function.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
LGTM.
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> tools/testing/selftests/bpf/btf_helpers.c | 7 ++++++-
> .../selftests/bpf/prog_tests/btf_write.c | 21 +++++++++++++++++++
> 2 files changed, 27 insertions(+), 1 deletion(-)
>
[...]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 07/11] selftests/bpf: change NAME_NTH/IS_NAME_NTH for BTF_KIND_TAG format
2021-09-13 15:52 ` [PATCH bpf-next v2 07/11] selftests/bpf: change NAME_NTH/IS_NAME_NTH for BTF_KIND_TAG format Yonghong Song
@ 2021-09-14 5:23 ` Andrii Nakryiko
0 siblings, 0 replies; 43+ messages in thread
From: Andrii Nakryiko @ 2021-09-14 5:23 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kernel Team
On Mon, Sep 13, 2021 at 8:52 AM Yonghong Song <yhs@fb.com> wrote:
>
> BTF_KIND_TAG ELF format has a component_idx which might have value -1.
> test_btf may confuse it with common_type.name as NAME_NTH checkes
> high 16bit to be 0xffff. Change NAME_NTH high 16bit check to be
> 0xfffe so it won't confuse with component_idx.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> tools/testing/selftests/bpf/prog_tests/btf.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
> index 649f87382c8d..ad39f4d588d0 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf.c
> @@ -39,8 +39,8 @@ static bool always_log;
> #define BTF_END_RAW 0xdeadbeef
> #define NAME_TBD 0xdeadb33f
>
> -#define NAME_NTH(N) (0xffff0000 | N)
> -#define IS_NAME_NTH(X) ((X & 0xffff0000) == 0xffff0000)
> +#define NAME_NTH(N) (0xfffe0000 | N)
> +#define IS_NAME_NTH(X) ((X & 0xffff0000) == 0xfffe0000)
> #define GET_NAME_NTH_IDX(X) (X & 0x0000ffff)
>
> #define MAX_NR_RAW_U32 1024
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 08/11] selftests/bpf: add BTF_KIND_TAG unit tests
2021-09-13 15:52 ` [PATCH bpf-next v2 08/11] selftests/bpf: add BTF_KIND_TAG unit tests Yonghong Song
@ 2021-09-14 5:31 ` Andrii Nakryiko
2021-09-14 17:00 ` Yonghong Song
0 siblings, 1 reply; 43+ messages in thread
From: Andrii Nakryiko @ 2021-09-14 5:31 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kernel Team
On Mon, Sep 13, 2021 at 8:52 AM Yonghong Song <yhs@fb.com> wrote:
>
> Test good and bad variants of BTF_KIND_TAG encoding.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
> tools/testing/selftests/bpf/prog_tests/btf.c | 223 +++++++++++++++++++
> tools/testing/selftests/bpf/test_btf.h | 3 +
> 2 files changed, 226 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
> index ad39f4d588d0..21b122f72a55 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf.c
> @@ -3661,6 +3661,227 @@ static struct btf_raw_test raw_tests[] = {
> .err_str = "Invalid type_size",
> },
>
> +{
> + .descr = "tag test #1, struct/member, well-formed",
> + .raw_types = {
> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
> + BTF_STRUCT_ENC(0, 2, 8), /* [2] */
> + BTF_MEMBER_ENC(NAME_TBD, 1, 0),
> + BTF_MEMBER_ENC(NAME_TBD, 1, 32),
> + BTF_TAG_ENC(NAME_TBD, 2, -1),
> + BTF_TAG_ENC(NAME_TBD, 2, 0),
> + BTF_TAG_ENC(NAME_TBD, 2, 1),
> + BTF_END_RAW,
> + },
> + BTF_STR_SEC("\0m1\0m2\0tag1\0tag2\0tag3"),
> + .map_type = BPF_MAP_TYPE_ARRAY,
> + .map_name = "tag_type_check_btf",
> + .key_size = sizeof(int),
> + .value_size = 8,
> + .key_type_id = 1,
> + .value_type_id = 2,
> + .max_entries = 1,
> +},
> +{
> + .descr = "tag test #2, union/member, well-formed",
> + .raw_types = {
> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
> + BTF_STRUCT_ENC(NAME_TBD, 2, 4), /* [2] */
this is not a union. Other tests open-code union, but it's probably a
good idea to add BTF_UNION_ENC
> + BTF_MEMBER_ENC(NAME_TBD, 1, 0),
> + BTF_MEMBER_ENC(NAME_TBD, 1, 0),
> + BTF_TAG_ENC(NAME_TBD, 2, -1),
> + BTF_TAG_ENC(NAME_TBD, 2, 0),
> + BTF_TAG_ENC(NAME_TBD, 2, 1),
> + BTF_END_RAW,
> + },
> + BTF_STR_SEC("\0t\0m1\0m2\0tag1\0tag2\0tag3"),
> + .map_type = BPF_MAP_TYPE_ARRAY,
> + .map_name = "tag_type_check_btf",
> + .key_size = sizeof(int),
> + .value_size = 4,
> + .key_type_id = 1,
> + .value_type_id = 2,
> + .max_entries = 1,
> +},
[...]
> +{
> + .descr = "tag test #7, invalid vlen",
> + .raw_types = {
> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
> + BTF_VAR_ENC(NAME_TBD, 1, 0), /* [2] */
> + BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_TAG, 1, 1), 2), (0),
here you have both kflag and vlen specified, so it's not clear which
one is rejected. Please keep kflag at 0 for this one.
> + BTF_END_RAW,
> + },
> + BTF_STR_SEC("\0local\0tag1"),
> + .map_type = BPF_MAP_TYPE_ARRAY,
> + .map_name = "tag_type_check_btf",
> + .key_size = sizeof(int),
> + .value_size = 4,
> + .key_type_id = 1,
> + .value_type_id = 1,
> + .max_entries = 1,
> + .btf_load_err = true,
> + .err_str = "vlen != 0",
> +},
> +{
> + .descr = "tag test #8, invalid kflag",
> + .raw_types = {
> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
> + BTF_VAR_ENC(NAME_TBD, 1, 0), /* [2] */
> + BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_TAG, 1, 0), 2), (-1),
> + BTF_END_RAW,
> + },
> + BTF_STR_SEC("\0local\0tag1"),
> + .map_type = BPF_MAP_TYPE_ARRAY,
> + .map_name = "tag_type_check_btf",
> + .key_size = sizeof(int),
> + .value_size = 4,
> + .key_type_id = 1,
> + .value_type_id = 1,
> + .max_entries = 1,
> + .btf_load_err = true,
> + .err_str = "Invalid btf_info kind_flag",
> +},
> +{
> + .descr = "tag test #9, var, invalid component_idx",
> + .raw_types = {
> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
> + BTF_VAR_ENC(NAME_TBD, 1, 0), /* [2] */
> + BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_TAG, 0, 0), 2), (0),
nit: could have used BTF_TAG_ENC?
> + BTF_END_RAW,
> + },
> + BTF_STR_SEC("\0local\0tag"),
> + .map_type = BPF_MAP_TYPE_ARRAY,
> + .map_name = "tag_type_check_btf",
> + .key_size = sizeof(int),
> + .value_size = 4,
> + .key_type_id = 1,
> + .value_type_id = 1,
> + .max_entries = 1,
> + .btf_load_err = true,
> + .err_str = "Invalid component_idx",
> +},
> +{
> + .descr = "tag test #10, struct member, invalid component_idx",
> + .raw_types = {
> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
> + BTF_STRUCT_ENC(0, 2, 8), /* [2] */
> + BTF_MEMBER_ENC(NAME_TBD, 1, 0),
> + BTF_MEMBER_ENC(NAME_TBD, 1, 32),
> + BTF_TAG_ENC(NAME_TBD, 2, 2),
> + BTF_END_RAW,
> + },
> + BTF_STR_SEC("\0m1\0m2\0tag"),
> + .map_type = BPF_MAP_TYPE_ARRAY,
> + .map_name = "tag_type_check_btf",
> + .key_size = sizeof(int),
> + .value_size = 8,
> + .key_type_id = 1,
> + .value_type_id = 2,
> + .max_entries = 1,
> + .btf_load_err = true,
> + .err_str = "Invalid component_idx",
> +},
> +{
> + .descr = "tag test #11, func parameter, invalid component_idx",
> + .raw_types = {
> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
> + BTF_FUNC_PROTO_ENC(0, 2), /* [2] */
> + BTF_FUNC_PROTO_ARG_ENC(NAME_TBD, 1),
> + BTF_FUNC_PROTO_ARG_ENC(NAME_TBD, 1),
> + BTF_FUNC_ENC(NAME_TBD, 2), /* [3] */
> + BTF_TAG_ENC(NAME_TBD, 3, 2),
> + BTF_END_RAW,
> + },
> + BTF_STR_SEC("\0arg1\0arg2\0f\0tag"),
> + .map_type = BPF_MAP_TYPE_ARRAY,
> + .map_name = "tag_type_check_btf",
> + .key_size = sizeof(int),
> + .value_size = 4,
> + .key_type_id = 1,
> + .value_type_id = 1,
> + .max_entries = 1,
> + .btf_load_err = true,
> + .err_str = "Invalid component_idx",
> +},
> +
please also add invalid negative component_idx test (e.g., -2)
> }; /* struct btf_raw_test raw_tests[] */
>
> static const char *get_next_str(const char *start, const char *end)
> @@ -6801,6 +7022,8 @@ static int btf_type_size(const struct btf_type *t)
> return base_size + sizeof(struct btf_var);
> case BTF_KIND_DATASEC:
> return base_size + vlen * sizeof(struct btf_var_secinfo);
> + case BTF_KIND_TAG:
> + return base_size + sizeof(struct btf_tag);
> default:
> fprintf(stderr, "Unsupported BTF_KIND:%u\n", kind);
> return -EINVAL;
> diff --git a/tools/testing/selftests/bpf/test_btf.h b/tools/testing/selftests/bpf/test_btf.h
> index e2394eea4b7f..0619e06d745e 100644
> --- a/tools/testing/selftests/bpf/test_btf.h
> +++ b/tools/testing/selftests/bpf/test_btf.h
> @@ -69,4 +69,7 @@
> #define BTF_TYPE_FLOAT_ENC(name, sz) \
> BTF_TYPE_ENC(name, BTF_INFO_ENC(BTF_KIND_FLOAT, 0, 0), sz)
>
> +#define BTF_TAG_ENC(value, type, component_idx) \
> + BTF_TYPE_ENC(value, BTF_INFO_ENC(BTF_KIND_TAG, 0, 0), type), (component_idx)
> +
> #endif /* _TEST_BTF_H */
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 09/11] selftests/bpf: test BTF_KIND_TAG for deduplication
2021-09-13 15:52 ` [PATCH bpf-next v2 09/11] selftests/bpf: test BTF_KIND_TAG for deduplication Yonghong Song
@ 2021-09-14 5:38 ` Andrii Nakryiko
2021-09-14 17:15 ` Yonghong Song
2021-09-14 19:39 ` Yonghong Song
0 siblings, 2 replies; 43+ messages in thread
From: Andrii Nakryiko @ 2021-09-14 5:38 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kernel Team
On Mon, Sep 13, 2021 at 8:52 AM Yonghong Song <yhs@fb.com> wrote:
>
> Add unit tests for BTF_KIND_TAG deduplication for
> - struct and struct member
> - variable
> - func and func argument
>
Can you please also add tests where you have duplicated struct,
variable, and func (three different tests), and each copy has two
tags: one with common value (e.g., common_val) and one with unique
value (uniq_val1 and uniq_val2, one for each copy of a
struct/var/func). End result should be a single struct/var/func with
three different tags pointing to it (e.g., common_val, uniq_val1,
uniq_val2). I.e., those tags are "inherited" by the deduplicated
entity and only a unique set of them is left.
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
> tools/testing/selftests/bpf/prog_tests/btf.c | 91 ++++++++++++++++----
> 1 file changed, 74 insertions(+), 17 deletions(-)
>
[...]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 11/11] docs/bpf: add documentation for BTF_KIND_TAG
2021-09-13 15:52 ` [PATCH bpf-next v2 11/11] docs/bpf: add documentation for BTF_KIND_TAG Yonghong Song
@ 2021-09-14 5:40 ` Andrii Nakryiko
0 siblings, 0 replies; 43+ messages in thread
From: Andrii Nakryiko @ 2021-09-14 5:40 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kernel Team
On Mon, Sep 13, 2021 at 8:53 AM Yonghong Song <yhs@fb.com> wrote:
>
> Add BTF_KIND_TAG documentation in btf.rst.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
Probably worth it to mention that component_idx for tag for VAR has to be -1.
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> Documentation/bpf/btf.rst | 27 ++++++++++++++++++++++++++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/bpf/btf.rst b/Documentation/bpf/btf.rst
> index 846354cd2d69..9fff578a0a35 100644
> --- a/Documentation/bpf/btf.rst
> +++ b/Documentation/bpf/btf.rst
> @@ -85,6 +85,7 @@ sequentially and type id is assigned to each recognized type starting from id
> #define BTF_KIND_VAR 14 /* Variable */
> #define BTF_KIND_DATASEC 15 /* Section */
> #define BTF_KIND_FLOAT 16 /* Floating point */
> + #define BTF_KIND_TAG 17 /* Tag */
>
> 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.
> @@ -106,7 +107,7 @@ Each type contains the following common data::
> * "size" tells the size of the type it is describing.
> *
> * "type" is used by PTR, TYPEDEF, VOLATILE, CONST, RESTRICT,
> - * FUNC and FUNC_PROTO.
> + * FUNC, FUNC_PROTO and TAG.
> * "type" is a type_id referring to another type.
> */
> union {
> @@ -465,6 +466,30 @@ map definition.
>
> No additional type data follow ``btf_type``.
>
> +2.2.17 BTF_KIND_TAG
> +~~~~~~~~~~~~~~~~~~~
> +
> +``struct btf_type`` encoding requirement:
> + * ``name_off``: offset to a non-empty string
> + * ``info.kind_flag``: 0
> + * ``info.kind``: BTF_KIND_TAG
> + * ``info.vlen``: 0
> + * ``type``: ``struct``, ``union``, ``func`` or ``var``
> +
> +``btf_type`` is followed by ``struct btf_tag``.::
> +
> + struct btf_tag {
> + __u32 component_idx;
> + };
> +
> +The ``name_off`` encodes btf_tag attribute string.
> +The ``type`` should be ``struct``, ``union``, ``func`` or ``var``.
> +If ``btf_tag.component_idx = -1``, the btf_tag attribute is
> +applied to a valid ``type``. Otherwise, the btf_tag attribute is
> +applied to a ``struct``/``union`` member or a ``func`` argument,
> +and ``btf_tag.component_idx`` should be a valid index (starting from 0)
> +pointing to a member or an argument.
> +
> 3. BTF Kernel API
> *****************
>
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 01/11] btf: change BTF_KIND_* macros to enums
2021-09-14 4:59 ` Andrii Nakryiko
@ 2021-09-14 15:53 ` Yonghong Song
0 siblings, 0 replies; 43+ messages in thread
From: Yonghong Song @ 2021-09-14 15:53 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kernel Team
On 9/13/21 9:59 PM, Andrii Nakryiko wrote:
> On Mon, Sep 13, 2021 at 8:51 AM Yonghong Song <yhs@fb.com> wrote:
>>
>> Change BTF_KIND_* macros to enums so they are encoded in dwarf and
>> appear in vmlinux.h. This will make it easier for bpf programs
>> to use these constants without macro definitions.
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>> include/uapi/linux/btf.h | 36 ++++++++++++++++++----------------
>> tools/include/uapi/linux/btf.h | 36 ++++++++++++++++++----------------
>> 2 files changed, 38 insertions(+), 34 deletions(-)
>>
>> diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
>> index d27b1708efe9..c32cd6697d63 100644
>> --- a/include/uapi/linux/btf.h
>> +++ b/include/uapi/linux/btf.h
>> @@ -56,23 +56,25 @@ struct btf_type {
>> #define BTF_INFO_VLEN(info) ((info) & 0xffff)
>> #define BTF_INFO_KFLAG(info) ((info) >> 31)
>>
>> -#define BTF_KIND_UNKN 0 /* Unknown */
>> -#define BTF_KIND_INT 1 /* Integer */
>> -#define BTF_KIND_PTR 2 /* Pointer */
>> -#define BTF_KIND_ARRAY 3 /* Array */
>> -#define BTF_KIND_STRUCT 4 /* Struct */
>> -#define BTF_KIND_UNION 5 /* Union */
>> -#define BTF_KIND_ENUM 6 /* Enumeration */
>> -#define BTF_KIND_FWD 7 /* Forward */
>> -#define BTF_KIND_TYPEDEF 8 /* Typedef */
>> -#define BTF_KIND_VOLATILE 9 /* Volatile */
>> -#define BTF_KIND_CONST 10 /* Const */
>> -#define BTF_KIND_RESTRICT 11 /* Restrict */
>> -#define BTF_KIND_FUNC 12 /* Function */
>> -#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 */
>> +enum {
>> + BTF_KIND_UNKN = 0, /* Unknown */
>> + BTF_KIND_INT, /* Integer */
>> + BTF_KIND_PTR, /* Pointer */
>> + BTF_KIND_ARRAY, /* Array */
>> + BTF_KIND_STRUCT, /* Struct */
>> + BTF_KIND_UNION, /* Union */
>> + BTF_KIND_ENUM, /* Enumeration */
>> + BTF_KIND_FWD, /* Forward */
>> + BTF_KIND_TYPEDEF, /* Typedef */
>> + BTF_KIND_VOLATILE, /* Volatile */
>> + BTF_KIND_CONST, /* Const */
>> + BTF_KIND_RESTRICT, /* Restrict */
>> + BTF_KIND_FUNC, /* Function */
>> + BTF_KIND_FUNC_PROTO, /* Function Proto */
>> + BTF_KIND_VAR, /* Variable */
>> + BTF_KIND_DATASEC, /* Section */
>> + BTF_KIND_FLOAT, /* Floating point */
>
> Can you please leave explicit integer values specified? It's extremely
> helpful and much easier in practice, compared to having to count the
> number of rows from BTF_KIND_UNKN. Had to do it multiple times with
> other BPF constants and was happy I didn't have to do that for
> BTF_KIND enums.
Sure. Will do.
>
>> +};
>> #define BTF_KIND_MAX BTF_KIND_FLOAT
>> #define NR_BTF_KINDS (BTF_KIND_MAX + 1)
>
> these two can be just an enum values as well, and actually will be
> "auto-updated", if done this way (I think, haven't really tested)
>
> BTF_KIND_FLOAT = 16,
> NR_BTF_KINDS,
> BTF_KIND_MAX = NR_BTF_KINDS - 1,
This should work. Previously, I tried to give a name to
the enum. If that is case, e.g., put NR_BTF_KINDS and
BTF_KIND_MAX inside the enum will not be a good idea
since then you have some case not a real type or
you have duplicated case number (e.g, BTF_KIND_FLOAT 16
and default (which covers BTF_KIND_MAX) also contains 16)
and compiler may warn.
But eventually I did not put an enum name here since
a common name like btf_kind may have been used by
application.
So will make the change as we don't name the enum.
>
> ... won't it?
>
> [...]
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 02/11] bpf: support for new btf kind BTF_KIND_TAG
2021-09-14 5:08 ` Andrii Nakryiko
@ 2021-09-14 15:59 ` Yonghong Song
2021-09-14 23:30 ` Andrii Nakryiko
0 siblings, 1 reply; 43+ messages in thread
From: Yonghong Song @ 2021-09-14 15:59 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kernel Team
On 9/13/21 10:08 PM, Andrii Nakryiko wrote:
> On Mon, Sep 13, 2021 at 8:51 AM Yonghong Song <yhs@fb.com> wrote:
>>
>> LLVM14 added support for a new C attribute ([1])
>> __attribute__((btf_tag("arbitrary_str")))
>> This attribute will be emitted to dwarf ([2]) and pahole
>> will convert it to BTF. Or for bpf target, this
>> attribute will be emitted to BTF directly ([3], [4]).
>> The attribute is intended to provide additional
>> information for
>> - struct/union type or struct/union member
>> - static/global variables
>> - static/global function or function parameter.
>>
>> For linux kernel, the btf_tag can be applied
>> in various places to specify user pointer,
>> function pre- or post- condition, function
>> allow/deny in certain context, etc. Such information
>> will be encoded in vmlinux BTF and can be used
>> by verifier.
>>
>> The btf_tag can also be applied to bpf programs
>> to help global verifiable functions, e.g.,
>> specifying preconditions, etc.
>>
>> This patch added basic parsing and checking support
>> in kernel for new BTF_KIND_TAG kind.
>>
>> [1] https://reviews.llvm.org/D106614
>> [2] https://reviews.llvm.org/D106621
>> [3] https://reviews.llvm.org/D106622
>> [4] https://reviews.llvm.org/D109560
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>> include/uapi/linux/btf.h | 16 ++++-
>> kernel/bpf/btf.c | 120 +++++++++++++++++++++++++++++++++
>> tools/include/uapi/linux/btf.h | 16 ++++-
>> 3 files changed, 148 insertions(+), 4 deletions(-)
>>
>
> [...]
>
>>
>> +static s32 btf_tag_check_meta(struct btf_verifier_env *env,
>> + const struct btf_type *t,
>> + u32 meta_left)
>> +{
>> + const struct btf_tag *tag;
>> + u32 meta_needed = sizeof(*tag);
>> + const char *value;
>> +
>> + if (meta_left < meta_needed) {
>> + btf_verifier_log_basic(env, t,
>> + "meta_left:%u meta_needed:%u",
>> + meta_left, meta_needed);
>> + return -EINVAL;
>> + }
>> +
>> + value = btf_name_by_offset(env->btf, t->name_off);
>> + if (!value || !value[0]) {
>> + btf_verifier_log_type(env, t, "Invalid value");
>> + 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;
>> + }
>> +
>
> probably need to enforce that component_idx is >= -1? -2 is not a
> valid supported value right now.
I tested below. But I can test here for kernel practice, testing error
case earlier.
>
>> + btf_verifier_log_type(env, t, NULL);
>> +
>> + return meta_needed;
>> +}
>> +
>> +static int btf_tag_resolve(struct btf_verifier_env *env,
>> + const struct resolve_vertex *v)
>> +{
>> + const struct btf_type *next_type;
>> + const struct btf_type *t = v->t;
>> + u32 next_type_id = t->type;
>> + struct btf *btf = env->btf;
>> + s32 component_idx;
>> + u32 vlen;
>> +
>> + next_type = btf_type_by_id(btf, next_type_id);
>> + if (!next_type || !btf_type_is_tag_target(next_type)) {
>> + btf_verifier_log_type(env, v->t, "Invalid type_id");
>> + return -EINVAL;
>> + }
>> +
>> + if (!env_type_is_resolve_sink(env, next_type) &&
>> + !env_type_is_resolved(env, next_type_id))
>> + return env_stack_push(env, next_type, next_type_id);
>> +
>> + component_idx = btf_type_tag(t)->component_idx;
>> + if (component_idx != -1) {
>
> so here, if it's -2, that should be an error, but currently will be
> ignored, right?
It is not. See below. At this point, component_idx could be -2 or 0 or 1 ...
>
>> + if (btf_type_is_var(next_type) || component_idx < 0) {
>
> if is_var(next_type) then component_idx should only be -1, nothing
> else. Or am I missing some convention?
So if it is a variable, the error will return.
If it is not a variable and component_idx < 0 (-2 in this case), return
error. So we do test -2 here.
I will restructure the code to test < -1 earlier, so we won't have
confusion here.
>
>> + btf_verifier_log_type(env, v->t, "Invalid component_idx");
>> + return -EINVAL;
>> + }
>> +
>> + if (btf_type_is_struct(next_type)) {
>> + vlen = btf_type_vlen(next_type);
>> + } else {
>> + next_type = btf_type_by_id(btf, next_type->type);
>> + vlen = btf_type_vlen(next_type);
>> + }
>> +
>> + if ((u32)component_idx >= vlen) {
>> + btf_verifier_log_type(env, v->t, "Invalid component_idx");
>> + return -EINVAL;
>> + }
>> + }
>> +
>> + env_stack_pop_resolved(env, next_type_id, 0);
>> +
>> + return 0;
>> +}
>> +
>
> [...]
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 04/11] libbpf: add support for BTF_KIND_TAG
2021-09-14 5:15 ` Andrii Nakryiko
@ 2021-09-14 16:42 ` Yonghong Song
0 siblings, 0 replies; 43+ messages in thread
From: Yonghong Song @ 2021-09-14 16:42 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kernel Team
On 9/13/21 10:15 PM, Andrii Nakryiko wrote:
> On Mon, Sep 13, 2021 at 8:51 AM Yonghong Song <yhs@fb.com> wrote:
>>
>> Add BTF_KIND_TAG support for parsing and dedup.
>> Also added sanitization for BTF_KIND_TAG. If BTF_KIND_TAG is not
>> supported in the kernel, sanitize it to INTs.
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>> tools/lib/bpf/btf.c | 69 +++++++++++++++++++++++++++++++++
>> tools/lib/bpf/btf.h | 15 +++++++
>> tools/lib/bpf/btf_dump.c | 3 ++
>> tools/lib/bpf/libbpf.c | 31 +++++++++++++--
>> tools/lib/bpf/libbpf.map | 5 +++
>> tools/lib/bpf/libbpf_internal.h | 2 +
>> 6 files changed, 122 insertions(+), 3 deletions(-)
>>
>
> Just a few small nits.
Ack for all nits. All these are indeed missed leftovers from previous
patchset/changes.
>
> [...]
>
>> @@ -2440,6 +2446,49 @@ int btf__add_datasec_var_info(struct btf *btf, int var_type_id, __u32 offset, __
>> return 0;
>> }
>>
>> +/*
>> + * Append new BTF_KIND_TAG type with:
>> + * - *value* - non-empty/non-NULL string;
>> + * - *ref_type_id* - referenced type ID, it might not exist yet;
>> + * - *component_idx* - -1 for tagging reference type, otherwise struct/union
>> + * member or function argument index;
>> + * Returns:
>> + * - >0, type ID of newly added BTF type;
>> + * - <0, on error.
>> + */
>> +int btf__add_tag(struct btf *btf, const char *value, int ref_type_id,
>> + int component_idx)
>> +{
>> + bool for_ref_type = false;
>
> leftovers from the previous revision? let's just use 0 for kflag argument below
>
>> + struct btf_type *t;
>> + int sz, value_off;
>> +
>> + if (!value || !value[0] || component_idx < -1)
>> + return libbpf_err(-EINVAL);
>> +
>> + if (validate_type_id(ref_type_id))
>> + return libbpf_err(-EINVAL);
>> +
>> + if (btf_ensure_modifiable(btf))
>> + return libbpf_err(-ENOMEM);
>> +
>> + sz = sizeof(struct btf_type) + sizeof(struct btf_tag);
>> + t = btf_add_type_mem(btf, sz);
>> + if (!t)
>> + return libbpf_err(-ENOMEM);
>> +
>> + value_off = btf__add_str(btf, value);
>> + if (value_off < 0)
>> + return value_off;
>> +
>> + t->name_off = value_off;
>> + t->info = btf_type_info(BTF_KIND_TAG, 0, for_ref_type);
>> + t->type = ref_type_id;
>> + ((struct btf_tag *)(t + 1))->component_idx = component_idx;
>
> nit: btf_tag(t)->component_idx
>
>> +
>> + return btf_commit_type(btf, sz);
>> +}
>> +
>> struct btf_ext_sec_setup_param {
>> __u32 off;
>> __u32 len;
>
> [...]
>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index 8f579c6666b2..4a62ef714562 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -195,6 +195,8 @@ enum kern_feature_id {
>> FEAT_BTF_FLOAT,
>> /* BPF perf link support */
>> FEAT_PERF_LINK,
>> + /* BTF_KIND_ATTR support */
>
> s/BTF_KIND_ATTR/BTF_KIND_TAG/
>
>> + FEAT_BTF_TAG,
>> __FEAT_CNT,
>> };
>>
>
> [...]
>
>> static bool kernel_supports(const struct bpf_object *obj, enum kern_feature_id feat_id)
>> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
>> index bbc53bb25f68..9e649cf9e771 100644
>> --- a/tools/lib/bpf/libbpf.map
>> +++ b/tools/lib/bpf/libbpf.map
>> @@ -386,3 +386,8 @@ LIBBPF_0.5.0 {
>> btf_dump__dump_type_data;
>> libbpf_set_strict_mode;
>> } LIBBPF_0.4.0;
>> +
>> +LIBBPF_0.6.0 {
>> + global:
>> + btf__add_tag;
>> +} LIBBPF_0.5.0;
>
> you'll need a rebase for this due to my patch for libbpf_version.h, sorry
>
>> diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
>> index 4f6ff5c23695..ceb0c98979bc 100644
>> --- a/tools/lib/bpf/libbpf_internal.h
>> +++ b/tools/lib/bpf/libbpf_internal.h
>> @@ -69,6 +69,8 @@
>> #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)
>> +#define BTF_TYPE_TAG_ENC(value, type, component_idx) \
>> + BTF_TYPE_ENC(value, BTF_INFO_ENC(BTF_KIND_TAG, 0, 0), type), (component_idx)
>>
>> #ifndef likely
>> #define likely(x) __builtin_expect(!!(x), 1)
>> --
>> 2.30.2
>>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 08/11] selftests/bpf: add BTF_KIND_TAG unit tests
2021-09-14 5:31 ` Andrii Nakryiko
@ 2021-09-14 17:00 ` Yonghong Song
0 siblings, 0 replies; 43+ messages in thread
From: Yonghong Song @ 2021-09-14 17:00 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kernel Team
On 9/13/21 10:31 PM, Andrii Nakryiko wrote:
> On Mon, Sep 13, 2021 at 8:52 AM Yonghong Song <yhs@fb.com> wrote:
>>
>> Test good and bad variants of BTF_KIND_TAG encoding.
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>> tools/testing/selftests/bpf/prog_tests/btf.c | 223 +++++++++++++++++++
>> tools/testing/selftests/bpf/test_btf.h | 3 +
>> 2 files changed, 226 insertions(+)
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
>> index ad39f4d588d0..21b122f72a55 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/btf.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/btf.c
>> @@ -3661,6 +3661,227 @@ static struct btf_raw_test raw_tests[] = {
>> .err_str = "Invalid type_size",
>> },
>>
>> +{
>> + .descr = "tag test #1, struct/member, well-formed",
>> + .raw_types = {
>> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
>> + BTF_STRUCT_ENC(0, 2, 8), /* [2] */
>> + BTF_MEMBER_ENC(NAME_TBD, 1, 0),
>> + BTF_MEMBER_ENC(NAME_TBD, 1, 32),
>> + BTF_TAG_ENC(NAME_TBD, 2, -1),
>> + BTF_TAG_ENC(NAME_TBD, 2, 0),
>> + BTF_TAG_ENC(NAME_TBD, 2, 1),
>> + BTF_END_RAW,
>> + },
>> + BTF_STR_SEC("\0m1\0m2\0tag1\0tag2\0tag3"),
>> + .map_type = BPF_MAP_TYPE_ARRAY,
>> + .map_name = "tag_type_check_btf",
>> + .key_size = sizeof(int),
>> + .value_size = 8,
>> + .key_type_id = 1,
>> + .value_type_id = 2,
>> + .max_entries = 1,
>> +},
>> +{
>> + .descr = "tag test #2, union/member, well-formed",
>> + .raw_types = {
>> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
>> + BTF_STRUCT_ENC(NAME_TBD, 2, 4), /* [2] */
>
> this is not a union. Other tests open-code union, but it's probably a
> good idea to add BTF_UNION_ENC
This intended to be a union. So will fix.
>
>> + BTF_MEMBER_ENC(NAME_TBD, 1, 0),
>> + BTF_MEMBER_ENC(NAME_TBD, 1, 0),
>> + BTF_TAG_ENC(NAME_TBD, 2, -1),
>> + BTF_TAG_ENC(NAME_TBD, 2, 0),
>> + BTF_TAG_ENC(NAME_TBD, 2, 1),
>> + BTF_END_RAW,
>> + },
>> + BTF_STR_SEC("\0t\0m1\0m2\0tag1\0tag2\0tag3"),
>> + .map_type = BPF_MAP_TYPE_ARRAY,
>> + .map_name = "tag_type_check_btf",
>> + .key_size = sizeof(int),
>> + .value_size = 4,
>> + .key_type_id = 1,
>> + .value_type_id = 2,
>> + .max_entries = 1,
>> +},
>
> [...]
>
>> +{
>> + .descr = "tag test #7, invalid vlen",
>> + .raw_types = {
>> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
>> + BTF_VAR_ENC(NAME_TBD, 1, 0), /* [2] */
>> + BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_TAG, 1, 1), 2), (0),
>
> here you have both kflag and vlen specified, so it's not clear which
> one is rejected. Please keep kflag at 0 for this one.
Ack.
>
>> + BTF_END_RAW,
>> + },
>> + BTF_STR_SEC("\0local\0tag1"),
>> + .map_type = BPF_MAP_TYPE_ARRAY,
>> + .map_name = "tag_type_check_btf",
>> + .key_size = sizeof(int),
>> + .value_size = 4,
>> + .key_type_id = 1,
>> + .value_type_id = 1,
>> + .max_entries = 1,
>> + .btf_load_err = true,
>> + .err_str = "vlen != 0",
>> +},
>> +{
>> + .descr = "tag test #8, invalid kflag",
>> + .raw_types = {
>> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
>> + BTF_VAR_ENC(NAME_TBD, 1, 0), /* [2] */
>> + BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_TAG, 1, 0), 2), (-1),
>> + BTF_END_RAW,
>> + },
>> + BTF_STR_SEC("\0local\0tag1"),
>> + .map_type = BPF_MAP_TYPE_ARRAY,
>> + .map_name = "tag_type_check_btf",
>> + .key_size = sizeof(int),
>> + .value_size = 4,
>> + .key_type_id = 1,
>> + .value_type_id = 1,
>> + .max_entries = 1,
>> + .btf_load_err = true,
>> + .err_str = "Invalid btf_info kind_flag",
>> +},
>> +{
>> + .descr = "tag test #9, var, invalid component_idx",
>> + .raw_types = {
>> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
>> + BTF_VAR_ENC(NAME_TBD, 1, 0), /* [2] */
>> + BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_TAG, 0, 0), 2), (0),
>
> nit: could have used BTF_TAG_ENC?
Ack.
>
>> + BTF_END_RAW,
>> + },
>> + BTF_STR_SEC("\0local\0tag"),
>> + .map_type = BPF_MAP_TYPE_ARRAY,
>> + .map_name = "tag_type_check_btf",
>> + .key_size = sizeof(int),
>> + .value_size = 4,
>> + .key_type_id = 1,
>> + .value_type_id = 1,
>> + .max_entries = 1,
>> + .btf_load_err = true,
>> + .err_str = "Invalid component_idx",
>> +},
>> +{
>> + .descr = "tag test #10, struct member, invalid component_idx",
>> + .raw_types = {
>> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
>> + BTF_STRUCT_ENC(0, 2, 8), /* [2] */
>> + BTF_MEMBER_ENC(NAME_TBD, 1, 0),
>> + BTF_MEMBER_ENC(NAME_TBD, 1, 32),
>> + BTF_TAG_ENC(NAME_TBD, 2, 2),
>> + BTF_END_RAW,
>> + },
>> + BTF_STR_SEC("\0m1\0m2\0tag"),
>> + .map_type = BPF_MAP_TYPE_ARRAY,
>> + .map_name = "tag_type_check_btf",
>> + .key_size = sizeof(int),
>> + .value_size = 8,
>> + .key_type_id = 1,
>> + .value_type_id = 2,
>> + .max_entries = 1,
>> + .btf_load_err = true,
>> + .err_str = "Invalid component_idx",
>> +},
>> +{
>> + .descr = "tag test #11, func parameter, invalid component_idx",
>> + .raw_types = {
>> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
>> + BTF_FUNC_PROTO_ENC(0, 2), /* [2] */
>> + BTF_FUNC_PROTO_ARG_ENC(NAME_TBD, 1),
>> + BTF_FUNC_PROTO_ARG_ENC(NAME_TBD, 1),
>> + BTF_FUNC_ENC(NAME_TBD, 2), /* [3] */
>> + BTF_TAG_ENC(NAME_TBD, 3, 2),
>> + BTF_END_RAW,
>> + },
>> + BTF_STR_SEC("\0arg1\0arg2\0f\0tag"),
>> + .map_type = BPF_MAP_TYPE_ARRAY,
>> + .map_name = "tag_type_check_btf",
>> + .key_size = sizeof(int),
>> + .value_size = 4,
>> + .key_type_id = 1,
>> + .value_type_id = 1,
>> + .max_entries = 1,
>> + .btf_load_err = true,
>> + .err_str = "Invalid component_idx",
>> +},
>> +
>
> please also add invalid negative component_idx test (e.g., -2)
Ack.
>
>> }; /* struct btf_raw_test raw_tests[] */
>>
>> static const char *get_next_str(const char *start, const char *end)
>> @@ -6801,6 +7022,8 @@ static int btf_type_size(const struct btf_type *t)
>> return base_size + sizeof(struct btf_var);
>> case BTF_KIND_DATASEC:
>> return base_size + vlen * sizeof(struct btf_var_secinfo);
>> + case BTF_KIND_TAG:
>> + return base_size + sizeof(struct btf_tag);
>> default:
>> fprintf(stderr, "Unsupported BTF_KIND:%u\n", kind);
>> return -EINVAL;
>> diff --git a/tools/testing/selftests/bpf/test_btf.h b/tools/testing/selftests/bpf/test_btf.h
>> index e2394eea4b7f..0619e06d745e 100644
>> --- a/tools/testing/selftests/bpf/test_btf.h
>> +++ b/tools/testing/selftests/bpf/test_btf.h
>> @@ -69,4 +69,7 @@
>> #define BTF_TYPE_FLOAT_ENC(name, sz) \
>> BTF_TYPE_ENC(name, BTF_INFO_ENC(BTF_KIND_FLOAT, 0, 0), sz)
>>
>> +#define BTF_TAG_ENC(value, type, component_idx) \
>> + BTF_TYPE_ENC(value, BTF_INFO_ENC(BTF_KIND_TAG, 0, 0), type), (component_idx)
>> +
>> #endif /* _TEST_BTF_H */
>> --
>> 2.30.2
>>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 09/11] selftests/bpf: test BTF_KIND_TAG for deduplication
2021-09-14 5:38 ` Andrii Nakryiko
@ 2021-09-14 17:15 ` Yonghong Song
2021-09-14 19:39 ` Yonghong Song
1 sibling, 0 replies; 43+ messages in thread
From: Yonghong Song @ 2021-09-14 17:15 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kernel Team
On 9/13/21 10:38 PM, Andrii Nakryiko wrote:
> On Mon, Sep 13, 2021 at 8:52 AM Yonghong Song <yhs@fb.com> wrote:
>>
>> Add unit tests for BTF_KIND_TAG deduplication for
>> - struct and struct member
>> - variable
>> - func and func argument
>>
>
> Can you please also add tests where you have duplicated struct,
> variable, and func (three different tests), and each copy has two
> tags: one with common value (e.g., common_val) and one with unique
> value (uniq_val1 and uniq_val2, one for each copy of a
> struct/var/func). End result should be a single struct/var/func with
> three different tags pointing to it (e.g., common_val, uniq_val1,
> uniq_val2). I.e., those tags are "inherited" by the deduplicated
> entity and only a unique set of them is left.
Sure. Will do.
>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>> tools/testing/selftests/bpf/prog_tests/btf.c | 91 ++++++++++++++++----
>> 1 file changed, 74 insertions(+), 17 deletions(-)
>>
>
> [...]
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 09/11] selftests/bpf: test BTF_KIND_TAG for deduplication
2021-09-14 5:38 ` Andrii Nakryiko
2021-09-14 17:15 ` Yonghong Song
@ 2021-09-14 19:39 ` Yonghong Song
2021-09-14 23:31 ` Andrii Nakryiko
1 sibling, 1 reply; 43+ messages in thread
From: Yonghong Song @ 2021-09-14 19:39 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kernel Team
On 9/13/21 10:38 PM, Andrii Nakryiko wrote:
> On Mon, Sep 13, 2021 at 8:52 AM Yonghong Song <yhs@fb.com> wrote:
>>
>> Add unit tests for BTF_KIND_TAG deduplication for
>> - struct and struct member
>> - variable
>> - func and func argument
>>
>
> Can you please also add tests where you have duplicated struct,
> variable, and func (three different tests), and each copy has two
currently, variable won't be deduplicated so I will skip variable
and add tests for func/argument and struct/member.
> tags: one with common value (e.g., common_val) and one with unique
> value (uniq_val1 and uniq_val2, one for each copy of a
> struct/var/func). End result should be a single struct/var/func with
> three different tags pointing to it (e.g., common_val, uniq_val1,
> uniq_val2). I.e., those tags are "inherited" by the deduplicated
> entity and only a unique set of them is left.
>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>> tools/testing/selftests/bpf/prog_tests/btf.c | 91 ++++++++++++++++----
>> 1 file changed, 74 insertions(+), 17 deletions(-)
>>
>
> [...]
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 02/11] bpf: support for new btf kind BTF_KIND_TAG
2021-09-14 15:59 ` Yonghong Song
@ 2021-09-14 23:30 ` Andrii Nakryiko
0 siblings, 0 replies; 43+ messages in thread
From: Andrii Nakryiko @ 2021-09-14 23:30 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kernel Team
On Tue, Sep 14, 2021 at 8:59 AM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 9/13/21 10:08 PM, Andrii Nakryiko wrote:
> > On Mon, Sep 13, 2021 at 8:51 AM Yonghong Song <yhs@fb.com> wrote:
> >>
> >> LLVM14 added support for a new C attribute ([1])
> >> __attribute__((btf_tag("arbitrary_str")))
> >> This attribute will be emitted to dwarf ([2]) and pahole
> >> will convert it to BTF. Or for bpf target, this
> >> attribute will be emitted to BTF directly ([3], [4]).
> >> The attribute is intended to provide additional
> >> information for
> >> - struct/union type or struct/union member
> >> - static/global variables
> >> - static/global function or function parameter.
> >>
> >> For linux kernel, the btf_tag can be applied
> >> in various places to specify user pointer,
> >> function pre- or post- condition, function
> >> allow/deny in certain context, etc. Such information
> >> will be encoded in vmlinux BTF and can be used
> >> by verifier.
> >>
> >> The btf_tag can also be applied to bpf programs
> >> to help global verifiable functions, e.g.,
> >> specifying preconditions, etc.
> >>
> >> This patch added basic parsing and checking support
> >> in kernel for new BTF_KIND_TAG kind.
> >>
> >> [1] https://reviews.llvm.org/D106614
> >> [2] https://reviews.llvm.org/D106621
> >> [3] https://reviews.llvm.org/D106622
> >> [4] https://reviews.llvm.org/D109560
> >>
> >> Signed-off-by: Yonghong Song <yhs@fb.com>
> >> ---
> >> include/uapi/linux/btf.h | 16 ++++-
> >> kernel/bpf/btf.c | 120 +++++++++++++++++++++++++++++++++
> >> tools/include/uapi/linux/btf.h | 16 ++++-
> >> 3 files changed, 148 insertions(+), 4 deletions(-)
> >>
> >
> > [...]
> >
> >>
> >> +static s32 btf_tag_check_meta(struct btf_verifier_env *env,
> >> + const struct btf_type *t,
> >> + u32 meta_left)
> >> +{
> >> + const struct btf_tag *tag;
> >> + u32 meta_needed = sizeof(*tag);
> >> + const char *value;
> >> +
> >> + if (meta_left < meta_needed) {
> >> + btf_verifier_log_basic(env, t,
> >> + "meta_left:%u meta_needed:%u",
> >> + meta_left, meta_needed);
> >> + return -EINVAL;
> >> + }
> >> +
> >> + value = btf_name_by_offset(env->btf, t->name_off);
> >> + if (!value || !value[0]) {
> >> + btf_verifier_log_type(env, t, "Invalid value");
> >> + 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;
> >> + }
> >> +
> >
> > probably need to enforce that component_idx is >= -1? -2 is not a
> > valid supported value right now.
>
> I tested below. But I can test here for kernel practice, testing error
> case earlier.
>
> >
> >> + btf_verifier_log_type(env, t, NULL);
> >> +
> >> + return meta_needed;
> >> +}
> >> +
> >> +static int btf_tag_resolve(struct btf_verifier_env *env,
> >> + const struct resolve_vertex *v)
> >> +{
> >> + const struct btf_type *next_type;
> >> + const struct btf_type *t = v->t;
> >> + u32 next_type_id = t->type;
> >> + struct btf *btf = env->btf;
> >> + s32 component_idx;
> >> + u32 vlen;
> >> +
> >> + next_type = btf_type_by_id(btf, next_type_id);
> >> + if (!next_type || !btf_type_is_tag_target(next_type)) {
> >> + btf_verifier_log_type(env, v->t, "Invalid type_id");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + if (!env_type_is_resolve_sink(env, next_type) &&
> >> + !env_type_is_resolved(env, next_type_id))
> >> + return env_stack_push(env, next_type, next_type_id);
> >> +
> >> + component_idx = btf_type_tag(t)->component_idx;
> >> + if (component_idx != -1) {
> >
> > so here, if it's -2, that should be an error, but currently will be
> > ignored, right?
>
> It is not. See below. At this point, component_idx could be -2 or 0 or 1 ...
>
> >
> >> + if (btf_type_is_var(next_type) || component_idx < 0) {
> >
> > if is_var(next_type) then component_idx should only be -1, nothing
> > else. Or am I missing some convention?
>
> So if it is a variable, the error will return.
>
> If it is not a variable and component_idx < 0 (-2 in this case), return
> error. So we do test -2 here.
>
> I will restructure the code to test < -1 earlier, so we won't have
> confusion here.
Oh, I've read this a few times and every single time I read it as
(btf_type_is_var() && component_idx < 0). It makes sense now, but it
is a bit convoluted to follow the checks. Thanks for improving!
>
> >
> >> + btf_verifier_log_type(env, v->t, "Invalid component_idx");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + if (btf_type_is_struct(next_type)) {
> >> + vlen = btf_type_vlen(next_type);
> >> + } else {
> >> + next_type = btf_type_by_id(btf, next_type->type);
> >> + vlen = btf_type_vlen(next_type);
> >> + }
> >> +
> >> + if ((u32)component_idx >= vlen) {
> >> + btf_verifier_log_type(env, v->t, "Invalid component_idx");
> >> + return -EINVAL;
> >> + }
> >> + }
> >> +
> >> + env_stack_pop_resolved(env, next_type_id, 0);
> >> +
> >> + return 0;
> >> +}
> >> +
> >
> > [...]
> >
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 09/11] selftests/bpf: test BTF_KIND_TAG for deduplication
2021-09-14 19:39 ` Yonghong Song
@ 2021-09-14 23:31 ` Andrii Nakryiko
0 siblings, 0 replies; 43+ messages in thread
From: Andrii Nakryiko @ 2021-09-14 23:31 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Kernel Team
On Tue, Sep 14, 2021 at 12:39 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 9/13/21 10:38 PM, Andrii Nakryiko wrote:
> > On Mon, Sep 13, 2021 at 8:52 AM Yonghong Song <yhs@fb.com> wrote:
> >>
> >> Add unit tests for BTF_KIND_TAG deduplication for
> >> - struct and struct member
> >> - variable
> >> - func and func argument
> >>
> >
> > Can you please also add tests where you have duplicated struct,
> > variable, and func (three different tests), and each copy has two
>
> currently, variable won't be deduplicated so I will skip variable
> and add tests for func/argument and struct/member.
oh, right, yeap, it makes sense only for struct/union and func
>
> > tags: one with common value (e.g., common_val) and one with unique
> > value (uniq_val1 and uniq_val2, one for each copy of a
> > struct/var/func). End result should be a single struct/var/func with
> > three different tags pointing to it (e.g., common_val, uniq_val1,
> > uniq_val2). I.e., those tags are "inherited" by the deduplicated
> > entity and only a unique set of them is left.
> >
> >> Signed-off-by: Yonghong Song <yhs@fb.com>
> >> ---
> >> tools/testing/selftests/bpf/prog_tests/btf.c | 91 ++++++++++++++++----
> >> 1 file changed, 74 insertions(+), 17 deletions(-)
> >>
> >
> > [...]
> >
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG
2021-09-13 16:40 ` Jose E. Marchesi
@ 2021-12-16 21:52 ` Yonghong Song
2021-12-17 10:40 ` Jose E. Marchesi
0 siblings, 1 reply; 43+ messages in thread
From: Yonghong Song @ 2021-12-16 21:52 UTC (permalink / raw)
To: Jose E. Marchesi
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team
On 9/13/21 9:40 AM, Jose E. Marchesi wrote:
>
>> cc Jose E. Marchesi
>>
>> Hi, Jose, just let you know that the BTF format for BTF_KIND_TAG is
>> changed since v1 as the new format can simplify kernel/libbpf
>> implementation. Thanks!
>
> Noted. Thanks for the update.
Hi, Jose,
This is just another update on btf_tag development.
Now, btf_tag is divided into btf_decl_tag and btf_type_tag
for tagging declarations and types as clang compiler prefers
not to mix them with each other. All compiler works in llvm
has done and you can check upstream llvm-project "main" branch
for implementation.
The patch set below (under review)
https://lore.kernel.org/bpf/20211209173537.1525283-1-yhs@fb.com/
actually tried to use btf_type_tag for linux kernel __user
annotation so bpf verifier can use it.
Another question from Omar (open source drgn maintainer)
https://developers.facebook.com/blog/post/2021/12/09/drgn-how-linux-kernel-team-meta-debugs-kernel-scale/
mentioned that btf_tag information will also help drgn since it
can then especially distinguish between __percpu pointer from
other pointers. Currently drgn is using dwarf, clang compiled
kernel puts btf_tag information in dwarf. Based on our earlier
discussion, gcc intends to generate btf tags for BTF only. Maybe
we could discuss to also generate for dwarf? Do we need a flag?
Please let me know if you have any questions.
Happy to help in whatever way to get gcc also implementing btf tag
support.
Thanks!
Yonghong
>
>>
>> On 9/13/21 8:51 AM, Yonghong Song wrote:
>>> LLVM14 added support for a new C attribute ([1])
>>> __attribute__((btf_tag("arbitrary_str")))
>>> This attribute will be emitted to dwarf ([2]) and pahole
>>> will convert it to BTF. Or for bpf target, this
>>> attribute will be emitted to BTF directly ([3], [4]).
>>> The attribute is intended to provide additional
>>> information for
>>> - struct/union type or struct/union member
>>> - static/global variables
>>> - static/global function or function parameter.
>>> This new attribute can be used to add attributes
>>> to kernel codes, e.g., pre- or post- conditions,
>>> allow/deny info, or any other info in which only
>>> the kernel is interested. Such attributes will
>>> be processed by clang frontend and emitted to
>>> dwarf, converting to BTF by pahole. Ultimiately
>>> the verifier can use these information for
>>> verification purpose.
>>> The new attribute can also be used for bpf
>>> programs, e.g., tagging with __user attributes
>>> for function parameters, specifying global
>>> function preconditions, etc. Such information
>>> may help verifier to detect user program
>>> bugs.
>>> After this series, pahole dwarf->btf converter
>>> will be enhanced to support new llvm tag
>>> for btf_tag attribute. With pahole support,
>>> we will then try to add a few real use case,
>>> e.g., __user/__rcu tagging, allow/deny list,
>>> some kernel function precondition, etc,
>>> in the kernel.
>>> In the rest of the series, Patches 1-2 had
>>> kernel support. Patches 3-4 added
>>> libbpf support. Patch 5 added bpftool
>>> support. Patches 6-10 added various selftests.
>>> Patch 11 added documentation for the new kind.
>>> [1] https://reviews.llvm.org/D106614
>>> [2] https://reviews.llvm.org/D106621
>>> [3] https://reviews.llvm.org/D106622
>>> [4] https://reviews.llvm.org/D109560
>>> Changelog:
>>> v1 -> v2:
>>> - BTF ELF format changed in llvm ([4] above),
>>> so cross-board change to use the new format.
>>> - Clarified in commit message that BTF_KIND_TAG
>>> is not emitted by bpftool btf dump format c.
>>> - Fix various comments from Andrii.
>>> Yonghong Song (11):
>>> btf: change BTF_KIND_* macros to enums
>>> bpf: support for new btf kind BTF_KIND_TAG
>>> libbpf: rename btf_{hash,equal}_int to btf_{hash,equal}_int_tag
>>> libbpf: add support for BTF_KIND_TAG
>>> bpftool: add support for BTF_KIND_TAG
>>> selftests/bpf: test libbpf API function btf__add_tag()
>>> selftests/bpf: change NAME_NTH/IS_NAME_NTH for BTF_KIND_TAG format
>>> selftests/bpf: add BTF_KIND_TAG unit tests
>>> selftests/bpf: test BTF_KIND_TAG for deduplication
>>> selftests/bpf: add a test with a bpf program with btf_tag attributes
>>> docs/bpf: add documentation for BTF_KIND_TAG
>>> Documentation/bpf/btf.rst | 27 +-
>>> include/uapi/linux/btf.h | 52 +--
>>> kernel/bpf/btf.c | 120 +++++++
>>> tools/bpf/bpftool/btf.c | 12 +
>>> tools/include/uapi/linux/btf.h | 52 +--
>>> tools/lib/bpf/btf.c | 85 ++++-
>>> tools/lib/bpf/btf.h | 15 +
>>> tools/lib/bpf/btf_dump.c | 3 +
>>> tools/lib/bpf/libbpf.c | 31 +-
>>> tools/lib/bpf/libbpf.map | 5 +
>>> tools/lib/bpf/libbpf_internal.h | 2 +
>>> tools/testing/selftests/bpf/btf_helpers.c | 7 +-
>>> tools/testing/selftests/bpf/prog_tests/btf.c | 318 ++++++++++++++++--
>>> .../selftests/bpf/prog_tests/btf_tag.c | 14 +
>>> .../selftests/bpf/prog_tests/btf_write.c | 21 ++
>>> tools/testing/selftests/bpf/progs/tag.c | 39 +++
>>> tools/testing/selftests/bpf/test_btf.h | 3 +
>>> 17 files changed, 736 insertions(+), 70 deletions(-)
>>> create mode 100644 tools/testing/selftests/bpf/prog_tests/btf_tag.c
>>> create mode 100644 tools/testing/selftests/bpf/progs/tag.c
>>>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG
2021-12-16 21:52 ` Yonghong Song
@ 2021-12-17 10:40 ` Jose E. Marchesi
2021-12-18 1:44 ` Alexei Starovoitov
0 siblings, 1 reply; 43+ messages in thread
From: Jose E. Marchesi @ 2021-12-17 10:40 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
kernel-team, Mark Wielaard
Hi Yonghong.
> On 9/13/21 9:40 AM, Jose E. Marchesi wrote:
>>
>>> cc Jose E. Marchesi
>>>
>>> Hi, Jose, just let you know that the BTF format for BTF_KIND_TAG is
>>> changed since v1 as the new format can simplify kernel/libbpf
>>> implementation. Thanks!
>> Noted. Thanks for the update.
>
> Hi, Jose,
>
> This is just another update on btf_tag development.
> Now, btf_tag is divided into btf_decl_tag and btf_type_tag
> for tagging declarations and types as clang compiler prefers
> not to mix them with each other. All compiler works in llvm
> has done and you can check upstream llvm-project "main" branch
> for implementation.
>
> The patch set below (under review)
> https://lore.kernel.org/bpf/20211209173537.1525283-1-yhs@fb.com/
> actually tried to use btf_type_tag for linux kernel __user
> annotation so bpf verifier can use it.
Noted, thanks for the heads up :)
We have not yet started to implement this.
> Another question from Omar (open source drgn maintainer)
>
> https://developers.facebook.com/blog/post/2021/12/09/drgn-how-linux-kernel-team-meta-debugs-kernel-scale/
> mentioned that btf_tag information will also help drgn since it
> can then especially distinguish between __percpu pointer from
> other pointers. Currently drgn is using dwarf, clang compiled
> kernel puts btf_tag information in dwarf. Based on our earlier
> discussion, gcc intends to generate btf tags for BTF only. Maybe
> we could discuss to also generate for dwarf? Do we need a flag?
It seems to me that there are three different orthogonal issues/topics
here, even if somehow related. Each would require a separated
discussion, probably on different contexts:
[Please let me know if I am wrong on any detail in the summary below.
In part I am writing it down as a recap to help myself :)]
1) The need for BTF to convey free-text tags on certain elements, such
as members of struct types.
IMO there is not much to discuss about this one. The specification
is straightforward as is the implementation. We will be adding it to
GCC soon.
Note that:
- This is obviously BTF specific.
- This is not strictly BPF specific, as GCC can generate BTF for any
supported target and not just BPF. I think you have patches for
LLVM to the same effect.
2) The need for DWARF to convey free-text tags on certain elements, such
as members of struct types.
The motivation for this was originally the way the Linux kernel
generates its BTF information, using pahole, using DWARF as a source.
As we discussed in our last exchange on this topic, this is
accidental, i.e. if the kernel switched to generate BTF directly from
the compiler and the linker could merge/deduplicate BTF, there would
be no need for using DWARF to act as the "unwilling conveyer" of this
information. There are additional benefits of this second approach.
Thats why we didn't plan to add these extended DWARF DIEs to GCC.
However, it now seems that a DWARF consumer, the drgn project, would
also benefit from having such a support in DWARF to distinguish
between different kind of pointers.
So it seems to me that now we have two use-cases for adding support
for these free-text tags to DWARF, as a proper extension to the
format, strictly unrelated to BTF, BPF or even the kernel, since:
- This is not kernel specific.
- This is not directly related to BTF.
- This is not directly related to BPF.
Therefore I would avoid any reference to BTF in the proposal to keep
it general, like in the name of DIE types and so on. This would
Whom to involve to discuss this? Well, I would say that at a minimum
we need to involve GCC people, LLVM people and DWARF people, and the
consensum of all the three parties. Once agreed, we can all
implement the same DIEs counting on the next version of the DWARF
spec will catch with them. I would really avoid rushed solutions
based on compiler-specific extensions.
Where to discuss this? I don't know. In some DWARF forum? Or
cross-posting gcc-patches and whatever LLVM list uses for
development? I am CCing Mark Wielaard who is a DWARF wizard... any
suggestion?
3) Addition of C-family language-level constructions to specify
free-text tags on certain language elements, such as struct fields.
These are the attributes, or built-ins or whatever syntax.
Note that, strictly speaking:
- This is orthogonal to both DWARF and BTF, and any other supported
debugging format, which may or may not be expressive enough to
convey the free-form text tag.
- This is not specific to BPF.
Therefore I would avoid any reference to BTF or BPF in the attribute
names. Something like `__attribute__((btf_tag("arbitrary_str")))'
makes very little sense to me; the attribute name ought to be more
generic.
Whom to involve to discuss this? Definitely, the front-end chaps of
both GCC and LLVM will have something to say about this, particularly
the ones in charge of the C-like language front-ends like C and C++.
A consensum among them would be ideal and would avoid
compiler-specific hacks.
Where to discuss this? Again, it seems that we need some neutral
ground to discuss inter-operability issues between the different free
software compilers...
In any case we are more than willing to help and participate in the
discussions :)
> Please let me know if you have any questions. Happy to help in
> whatever way to get gcc also implementing btf tag support.
>
> Thanks!
>
> Yonghong
>
>>
>>>
>>> On 9/13/21 8:51 AM, Yonghong Song wrote:
>>>> LLVM14 added support for a new C attribute ([1])
>>>> __attribute__((btf_tag("arbitrary_str")))
>>>> This attribute will be emitted to dwarf ([2]) and pahole
>>>> will convert it to BTF. Or for bpf target, this
>>>> attribute will be emitted to BTF directly ([3], [4]).
>>>> The attribute is intended to provide additional
>>>> information for
>>>> - struct/union type or struct/union member
>>>> - static/global variables
>>>> - static/global function or function parameter.
>>>> This new attribute can be used to add attributes
>>>> to kernel codes, e.g., pre- or post- conditions,
>>>> allow/deny info, or any other info in which only
>>>> the kernel is interested. Such attributes will
>>>> be processed by clang frontend and emitted to
>>>> dwarf, converting to BTF by pahole. Ultimiately
>>>> the verifier can use these information for
>>>> verification purpose.
>>>> The new attribute can also be used for bpf
>>>> programs, e.g., tagging with __user attributes
>>>> for function parameters, specifying global
>>>> function preconditions, etc. Such information
>>>> may help verifier to detect user program
>>>> bugs.
>>>> After this series, pahole dwarf->btf converter
>>>> will be enhanced to support new llvm tag
>>>> for btf_tag attribute. With pahole support,
>>>> we will then try to add a few real use case,
>>>> e.g., __user/__rcu tagging, allow/deny list,
>>>> some kernel function precondition, etc,
>>>> in the kernel.
>>>> In the rest of the series, Patches 1-2 had
>>>> kernel support. Patches 3-4 added
>>>> libbpf support. Patch 5 added bpftool
>>>> support. Patches 6-10 added various selftests.
>>>> Patch 11 added documentation for the new kind.
>>>> [1] https://reviews.llvm.org/D106614
>>>> [2] https://reviews.llvm.org/D106621
>>>> [3] https://reviews.llvm.org/D106622
>>>> [4] https://reviews.llvm.org/D109560
>>>> Changelog:
>>>> v1 -> v2:
>>>> - BTF ELF format changed in llvm ([4] above),
>>>> so cross-board change to use the new format.
>>>> - Clarified in commit message that BTF_KIND_TAG
>>>> is not emitted by bpftool btf dump format c.
>>>> - Fix various comments from Andrii.
>>>> Yonghong Song (11):
>>>> btf: change BTF_KIND_* macros to enums
>>>> bpf: support for new btf kind BTF_KIND_TAG
>>>> libbpf: rename btf_{hash,equal}_int to btf_{hash,equal}_int_tag
>>>> libbpf: add support for BTF_KIND_TAG
>>>> bpftool: add support for BTF_KIND_TAG
>>>> selftests/bpf: test libbpf API function btf__add_tag()
>>>> selftests/bpf: change NAME_NTH/IS_NAME_NTH for BTF_KIND_TAG format
>>>> selftests/bpf: add BTF_KIND_TAG unit tests
>>>> selftests/bpf: test BTF_KIND_TAG for deduplication
>>>> selftests/bpf: add a test with a bpf program with btf_tag attributes
>>>> docs/bpf: add documentation for BTF_KIND_TAG
>>>> Documentation/bpf/btf.rst | 27 +-
>>>> include/uapi/linux/btf.h | 52 +--
>>>> kernel/bpf/btf.c | 120 +++++++
>>>> tools/bpf/bpftool/btf.c | 12 +
>>>> tools/include/uapi/linux/btf.h | 52 +--
>>>> tools/lib/bpf/btf.c | 85 ++++-
>>>> tools/lib/bpf/btf.h | 15 +
>>>> tools/lib/bpf/btf_dump.c | 3 +
>>>> tools/lib/bpf/libbpf.c | 31 +-
>>>> tools/lib/bpf/libbpf.map | 5 +
>>>> tools/lib/bpf/libbpf_internal.h | 2 +
>>>> tools/testing/selftests/bpf/btf_helpers.c | 7 +-
>>>> tools/testing/selftests/bpf/prog_tests/btf.c | 318 ++++++++++++++++--
>>>> .../selftests/bpf/prog_tests/btf_tag.c | 14 +
>>>> .../selftests/bpf/prog_tests/btf_write.c | 21 ++
>>>> tools/testing/selftests/bpf/progs/tag.c | 39 +++
>>>> tools/testing/selftests/bpf/test_btf.h | 3 +
>>>> 17 files changed, 736 insertions(+), 70 deletions(-)
>>>> create mode 100644 tools/testing/selftests/bpf/prog_tests/btf_tag.c
>>>> create mode 100644 tools/testing/selftests/bpf/progs/tag.c
>>>>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG
2021-12-17 10:40 ` Jose E. Marchesi
@ 2021-12-18 1:44 ` Alexei Starovoitov
2021-12-18 20:15 ` Yonghong Song
0 siblings, 1 reply; 43+ messages in thread
From: Alexei Starovoitov @ 2021-12-18 1:44 UTC (permalink / raw)
To: Jose E. Marchesi
Cc: Yonghong Song, bpf, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, kernel-team, Mark Wielaard
On Fri, Dec 17, 2021 at 11:40:10AM +0100, Jose E. Marchesi wrote:
>
> 2) The need for DWARF to convey free-text tags on certain elements, such
> as members of struct types.
>
> The motivation for this was originally the way the Linux kernel
> generates its BTF information, using pahole, using DWARF as a source.
> As we discussed in our last exchange on this topic, this is
> accidental, i.e. if the kernel switched to generate BTF directly from
> the compiler and the linker could merge/deduplicate BTF, there would
> be no need for using DWARF to act as the "unwilling conveyer" of this
> information. There are additional benefits of this second approach.
> Thats why we didn't plan to add these extended DWARF DIEs to GCC.
>
> However, it now seems that a DWARF consumer, the drgn project, would
> also benefit from having such a support in DWARF to distinguish
> between different kind of pointers.
drgn can use .percpu section in vmlinux for global percpu vars.
For pointers the annotation is indeed necessary.
> So it seems to me that now we have two use-cases for adding support
> for these free-text tags to DWARF, as a proper extension to the
> format, strictly unrelated to BTF, BPF or even the kernel, since:
> - This is not kernel specific.
> - This is not directly related to BTF.
> - This is not directly related to BPF.
__percpu annotation is kernel specific.
__user and __rcu are kernel specific too.
Only BPF and BTF can meaningfully consume all three.
drgn can consume __percpu.
In that sense if GCC follows LLVM and emits compiler specific DWARF tag
pahole can convert it to the same BTF regardless whether kernel
was compiled with clang or gcc.
drgn can consume dwarf generated by clang or gcc as well even when BTF
is not there. That is the fastest way forward.
In that sense it would be nice to have common DWARF tag for pointer
annotations, but it's not mandatory. The time is the most valuable asset.
Implementing GCC specific DWARF tag doesn't require committee voting
and the mailing list bikeshedding.
> 3) Addition of C-family language-level constructions to specify
> free-text tags on certain language elements, such as struct fields.
>
> These are the attributes, or built-ins or whatever syntax.
>
> Note that, strictly speaking:
> - This is orthogonal to both DWARF and BTF, and any other supported
> debugging format, which may or may not be expressive enough to
> convey the free-form text tag.
> - This is not specific to BPF.
>
> Therefore I would avoid any reference to BTF or BPF in the attribute
> names. Something like `__attribute__((btf_tag("arbitrary_str")))'
> makes very little sense to me; the attribute name ought to be more
> generic.
Let's agree to disagree.
When BPF ISA was designed we didn't go to Intel, Arm, Mips, etc in order to
come up with the best ISA that would JIT to those architectures the best
possible way. Same thing with btf_tag. Today it is specific to BTF and BPF
only. Hence it's called this way. Whenever actual users will appear that need
free-text tags on a struct field then and only then will be the time to discuss
generic tag name. Just because "free-text tag on a struct field" sounds generic
it doesn't mean that it has any use case beyond what we're using it for in BPF
land. It goes back to the point of coding now instead of talking about coding.
If gcc wants to call it __attribute__((my_precious_gcc_tag("arbitrary_str")))
go ahead and code it this way. The include/linux/compiler.h can accommodate it.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG
2021-12-18 1:44 ` Alexei Starovoitov
@ 2021-12-18 20:15 ` Yonghong Song
2021-12-20 9:49 ` Jose E. Marchesi
0 siblings, 1 reply; 43+ messages in thread
From: Yonghong Song @ 2021-12-18 20:15 UTC (permalink / raw)
To: Alexei Starovoitov, Jose E. Marchesi
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
kernel-team, Mark Wielaard
On 12/17/21 5:44 PM, Alexei Starovoitov wrote:
> On Fri, Dec 17, 2021 at 11:40:10AM +0100, Jose E. Marchesi wrote:
>>
>> 2) The need for DWARF to convey free-text tags on certain elements, such
>> as members of struct types.
>>
>> The motivation for this was originally the way the Linux kernel
>> generates its BTF information, using pahole, using DWARF as a source.
>> As we discussed in our last exchange on this topic, this is
>> accidental, i.e. if the kernel switched to generate BTF directly from
>> the compiler and the linker could merge/deduplicate BTF, there would
>> be no need for using DWARF to act as the "unwilling conveyer" of this
>> information. There are additional benefits of this second approach.
>> Thats why we didn't plan to add these extended DWARF DIEs to GCC.
>>
>> However, it now seems that a DWARF consumer, the drgn project, would
>> also benefit from having such a support in DWARF to distinguish
>> between different kind of pointers.
>
> drgn can use .percpu section in vmlinux for global percpu vars.
> For pointers the annotation is indeed necessary.
>
>> So it seems to me that now we have two use-cases for adding support
>> for these free-text tags to DWARF, as a proper extension to the
>> format, strictly unrelated to BTF, BPF or even the kernel, since:
>> - This is not kernel specific.
>> - This is not directly related to BTF.
>> - This is not directly related to BPF.
>
> __percpu annotation is kernel specific.
> __user and __rcu are kernel specific too.
> Only BPF and BTF can meaningfully consume all three.
> drgn can consume __percpu.
>
> In that sense if GCC follows LLVM and emits compiler specific DWARF tag
> pahole can convert it to the same BTF regardless whether kernel
> was compiled with clang or gcc.
> drgn can consume dwarf generated by clang or gcc as well even when BTF
> is not there. That is the fastest way forward.
>
> In that sense it would be nice to have common DWARF tag for pointer
> annotations, but it's not mandatory. The time is the most valuable asset.
> Implementing GCC specific DWARF tag doesn't require committee voting
> and the mailing list bikeshedding.
>
>> 3) Addition of C-family language-level constructions to specify
>> free-text tags on certain language elements, such as struct fields.
>>
>> These are the attributes, or built-ins or whatever syntax.
>>
>> Note that, strictly speaking:
>> - This is orthogonal to both DWARF and BTF, and any other supported
>> debugging format, which may or may not be expressive enough to
>> convey the free-form text tag.
>> - This is not specific to BPF.
>>
>> Therefore I would avoid any reference to BTF or BPF in the attribute
>> names. Something like `__attribute__((btf_tag("arbitrary_str")))'
>> makes very little sense to me; the attribute name ought to be more
>> generic.
>
> Let's agree to disagree.
> When BPF ISA was designed we didn't go to Intel, Arm, Mips, etc in order to
> come up with the best ISA that would JIT to those architectures the best
> possible way. Same thing with btf_tag. Today it is specific to BTF and BPF
> only. Hence it's called this way. Whenever actual users will appear that need
> free-text tags on a struct field then and only then will be the time to discuss
> generic tag name. Just because "free-text tag on a struct field" sounds generic
> it doesn't mean that it has any use case beyond what we're using it for in BPF
> land. It goes back to the point of coding now instead of talking about coding.
> If gcc wants to call it __attribute__((my_precious_gcc_tag("arbitrary_str")))
> go ahead and code it this way. The include/linux/compiler.h can accommodate it.
Just want to add a little bit context for this. In the beginning when we
proposed to add the attribute, we named as a generic name like 'tag' (or
something like that). But eventually upstream suggested 'btf_tag' since
the use case we proposed is for bpf. At that point, we don't know
drgn use cases yet. Even with that, the use cases are still just for
linux kernel.
At that time, some *similar* use cases did came up, e.g., for
swift<->C++ conversion encoding ("tag name", "attribute info") for
attributes in the source code, will help a lot. But they will use a
different "tag name" than btf_tag to differentiate.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG
2021-12-18 20:15 ` Yonghong Song
@ 2021-12-20 9:49 ` Jose E. Marchesi
2021-12-20 15:52 ` Yonghong Song
2022-01-25 3:58 ` Yonghong Song
0 siblings, 2 replies; 43+ messages in thread
From: Jose E. Marchesi @ 2021-12-20 9:49 UTC (permalink / raw)
To: Yonghong Song
Cc: Alexei Starovoitov, bpf, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, kernel-team, Mark Wielaard
> On 12/17/21 5:44 PM, Alexei Starovoitov wrote:
>> On Fri, Dec 17, 2021 at 11:40:10AM +0100, Jose E. Marchesi wrote:
>>>
>>> 2) The need for DWARF to convey free-text tags on certain elements, such
>>> as members of struct types.
>>>
>>> The motivation for this was originally the way the Linux kernel
>>> generates its BTF information, using pahole, using DWARF as a source.
>>> As we discussed in our last exchange on this topic, this is
>>> accidental, i.e. if the kernel switched to generate BTF directly from
>>> the compiler and the linker could merge/deduplicate BTF, there would
>>> be no need for using DWARF to act as the "unwilling conveyer" of this
>>> information. There are additional benefits of this second approach.
>>> Thats why we didn't plan to add these extended DWARF DIEs to GCC.
>>>
>>> However, it now seems that a DWARF consumer, the drgn project, would
>>> also benefit from having such a support in DWARF to distinguish
>>> between different kind of pointers.
>> drgn can use .percpu section in vmlinux for global percpu vars.
>> For pointers the annotation is indeed necessary.
>>
>>> So it seems to me that now we have two use-cases for adding support
>>> for these free-text tags to DWARF, as a proper extension to the
>>> format, strictly unrelated to BTF, BPF or even the kernel, since:
>>> - This is not kernel specific.
>>> - This is not directly related to BTF.
>>> - This is not directly related to BPF.
>> __percpu annotation is kernel specific.
>> __user and __rcu are kernel specific too.
>> Only BPF and BTF can meaningfully consume all three.
>> drgn can consume __percpu.
>> In that sense if GCC follows LLVM and emits compiler specific DWARF
>> tag
>> pahole can convert it to the same BTF regardless whether kernel
>> was compiled with clang or gcc.
>> drgn can consume dwarf generated by clang or gcc as well even when BTF
>> is not there. That is the fastest way forward.
>> In that sense it would be nice to have common DWARF tag for pointer
>> annotations, but it's not mandatory. The time is the most valuable asset.
>> Implementing GCC specific DWARF tag doesn't require committee voting
>> and the mailing list bikeshedding.
>>
>>> 3) Addition of C-family language-level constructions to specify
>>> free-text tags on certain language elements, such as struct fields.
>>>
>>> These are the attributes, or built-ins or whatever syntax.
>>>
>>> Note that, strictly speaking:
>>> - This is orthogonal to both DWARF and BTF, and any other supported
>>> debugging format, which may or may not be expressive enough to
>>> convey the free-form text tag.
>>> - This is not specific to BPF.
>>>
>>> Therefore I would avoid any reference to BTF or BPF in the attribute
>>> names. Something like `__attribute__((btf_tag("arbitrary_str")))'
>>> makes very little sense to me; the attribute name ought to be more
>>> generic.
>> Let's agree to disagree.
>> When BPF ISA was designed we didn't go to Intel, Arm, Mips, etc in order to
>> come up with the best ISA that would JIT to those architectures the best
>> possible way. Same thing with btf_tag. Today it is specific to BTF and BPF
>> only. Hence it's called this way. Whenever actual users will appear that need
>> free-text tags on a struct field then and only then will be the time to discuss
>> generic tag name. Just because "free-text tag on a struct field" sounds generic
>> it doesn't mean that it has any use case beyond what we're using it for in BPF
>> land. It goes back to the point of coding now instead of talking about coding.
>> If gcc wants to call it __attribute__((my_precious_gcc_tag("arbitrary_str")))
>> go ahead and code it this way. The include/linux/compiler.h can accommodate it.
>
> Just want to add a little bit context for this. In the beginning when
> we proposed to add the attribute, we named as a generic name like
> 'tag' (or something like that). But eventually upstream suggested
> 'btf_tag' since the use case we proposed is for bpf. At that point, we
> don't know drgn use cases yet. Even with that, the use cases are still
> just for linux kernel.
>
> At that time, some *similar* use cases did came up, e.g., for
> swift<->C++ conversion encoding ("tag name", "attribute info") for
> attributes in the source code, will help a lot. But they will use a
> different "tag name" than btf_tag to differentiate.
Thanks for the info.
I find it very interesting that the LLVM people prefers to have several
"use case specific" tag names instead of something more generic, which
is the exact opposite of what I would have done :) They may have
appealing reasons for doing so. Do you have a pointer to the dicussion
you had upstream at hand?
Anyway, I will taste the waters with the other GCC hackers about both
DIEs and attribute and see what we can come out with. Thanks again for
reaching out Yonghong.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG
2021-12-20 9:49 ` Jose E. Marchesi
@ 2021-12-20 15:52 ` Yonghong Song
2022-01-25 3:58 ` Yonghong Song
1 sibling, 0 replies; 43+ messages in thread
From: Yonghong Song @ 2021-12-20 15:52 UTC (permalink / raw)
To: Jose E. Marchesi
Cc: Alexei Starovoitov, bpf, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, kernel-team, Mark Wielaard
On 12/20/21 1:49 AM, Jose E. Marchesi wrote:
>
>> On 12/17/21 5:44 PM, Alexei Starovoitov wrote:
>>> On Fri, Dec 17, 2021 at 11:40:10AM +0100, Jose E. Marchesi wrote:
>>>>
>>>> 2) The need for DWARF to convey free-text tags on certain elements, such
>>>> as members of struct types.
>>>>
>>>> The motivation for this was originally the way the Linux kernel
>>>> generates its BTF information, using pahole, using DWARF as a source.
>>>> As we discussed in our last exchange on this topic, this is
>>>> accidental, i.e. if the kernel switched to generate BTF directly from
>>>> the compiler and the linker could merge/deduplicate BTF, there would
>>>> be no need for using DWARF to act as the "unwilling conveyer" of this
>>>> information. There are additional benefits of this second approach.
>>>> Thats why we didn't plan to add these extended DWARF DIEs to GCC.
>>>>
>>>> However, it now seems that a DWARF consumer, the drgn project, would
>>>> also benefit from having such a support in DWARF to distinguish
>>>> between different kind of pointers.
>>> drgn can use .percpu section in vmlinux for global percpu vars.
>>> For pointers the annotation is indeed necessary.
>>>
>>>> So it seems to me that now we have two use-cases for adding support
>>>> for these free-text tags to DWARF, as a proper extension to the
>>>> format, strictly unrelated to BTF, BPF or even the kernel, since:
>>>> - This is not kernel specific.
>>>> - This is not directly related to BTF.
>>>> - This is not directly related to BPF.
>>> __percpu annotation is kernel specific.
>>> __user and __rcu are kernel specific too.
>>> Only BPF and BTF can meaningfully consume all three.
>>> drgn can consume __percpu.
>>> In that sense if GCC follows LLVM and emits compiler specific DWARF
>>> tag
>>> pahole can convert it to the same BTF regardless whether kernel
>>> was compiled with clang or gcc.
>>> drgn can consume dwarf generated by clang or gcc as well even when BTF
>>> is not there. That is the fastest way forward.
>>> In that sense it would be nice to have common DWARF tag for pointer
>>> annotations, but it's not mandatory. The time is the most valuable asset.
>>> Implementing GCC specific DWARF tag doesn't require committee voting
>>> and the mailing list bikeshedding.
>>>
>>>> 3) Addition of C-family language-level constructions to specify
>>>> free-text tags on certain language elements, such as struct fields.
>>>>
>>>> These are the attributes, or built-ins or whatever syntax.
>>>>
>>>> Note that, strictly speaking:
>>>> - This is orthogonal to both DWARF and BTF, and any other supported
>>>> debugging format, which may or may not be expressive enough to
>>>> convey the free-form text tag.
>>>> - This is not specific to BPF.
>>>>
>>>> Therefore I would avoid any reference to BTF or BPF in the attribute
>>>> names. Something like `__attribute__((btf_tag("arbitrary_str")))'
>>>> makes very little sense to me; the attribute name ought to be more
>>>> generic.
>>> Let's agree to disagree.
>>> When BPF ISA was designed we didn't go to Intel, Arm, Mips, etc in order to
>>> come up with the best ISA that would JIT to those architectures the best
>>> possible way. Same thing with btf_tag. Today it is specific to BTF and BPF
>>> only. Hence it's called this way. Whenever actual users will appear that need
>>> free-text tags on a struct field then and only then will be the time to discuss
>>> generic tag name. Just because "free-text tag on a struct field" sounds generic
>>> it doesn't mean that it has any use case beyond what we're using it for in BPF
>>> land. It goes back to the point of coding now instead of talking about coding.
>>> If gcc wants to call it __attribute__((my_precious_gcc_tag("arbitrary_str")))
>>> go ahead and code it this way. The include/linux/compiler.h can accommodate it.
>>
>> Just want to add a little bit context for this. In the beginning when
>> we proposed to add the attribute, we named as a generic name like
>> 'tag' (or something like that). But eventually upstream suggested
>> 'btf_tag' since the use case we proposed is for bpf. At that point, we
>> don't know drgn use cases yet. Even with that, the use cases are still
>> just for linux kernel.
>>
>> At that time, some *similar* use cases did came up, e.g., for
>> swift<->C++ conversion encoding ("tag name", "attribute info") for
>> attributes in the source code, will help a lot. But they will use a
>> different "tag name" than btf_tag to differentiate.
>
> Thanks for the info.
>
> I find it very interesting that the LLVM people prefers to have several
> "use case specific" tag names instead of something more generic, which
> is the exact opposite of what I would have done :) They may have
> appealing reasons for doing so. Do you have a pointer to the dicussion
> you had upstream at hand?
Jose, the llvm-dev discussion link is below:
https://lists.llvm.org/pipermail/llvm-dev/2021-June/151009.html
>
> Anyway, I will taste the waters with the other GCC hackers about both
> DIEs and attribute and see what we can come out with. Thanks again for
> reaching out Yonghong.
Thanks!
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG
2021-12-20 9:49 ` Jose E. Marchesi
2021-12-20 15:52 ` Yonghong Song
@ 2022-01-25 3:58 ` Yonghong Song
2022-01-27 15:38 ` Jose E. Marchesi
1 sibling, 1 reply; 43+ messages in thread
From: Yonghong Song @ 2022-01-25 3:58 UTC (permalink / raw)
To: Jose E. Marchesi
Cc: Alexei Starovoitov, bpf, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, kernel-team, Mark Wielaard
On 12/20/21 1:49 AM, Jose E. Marchesi wrote:
>
>> On 12/17/21 5:44 PM, Alexei Starovoitov wrote:
>>> On Fri, Dec 17, 2021 at 11:40:10AM +0100, Jose E. Marchesi wrote:
>>>>
>>>> 2) The need for DWARF to convey free-text tags on certain elements, such
>>>> as members of struct types.
>>>>
>>>> The motivation for this was originally the way the Linux kernel
>>>> generates its BTF information, using pahole, using DWARF as a source.
>>>> As we discussed in our last exchange on this topic, this is
>>>> accidental, i.e. if the kernel switched to generate BTF directly from
>>>> the compiler and the linker could merge/deduplicate BTF, there would
>>>> be no need for using DWARF to act as the "unwilling conveyer" of this
>>>> information. There are additional benefits of this second approach.
>>>> Thats why we didn't plan to add these extended DWARF DIEs to GCC.
>>>>
>>>> However, it now seems that a DWARF consumer, the drgn project, would
>>>> also benefit from having such a support in DWARF to distinguish
>>>> between different kind of pointers.
>>> drgn can use .percpu section in vmlinux for global percpu vars.
>>> For pointers the annotation is indeed necessary.
>>>
>>>> So it seems to me that now we have two use-cases for adding support
>>>> for these free-text tags to DWARF, as a proper extension to the
>>>> format, strictly unrelated to BTF, BPF or even the kernel, since:
>>>> - This is not kernel specific.
>>>> - This is not directly related to BTF.
>>>> - This is not directly related to BPF.
>>> __percpu annotation is kernel specific.
>>> __user and __rcu are kernel specific too.
>>> Only BPF and BTF can meaningfully consume all three.
>>> drgn can consume __percpu.
>>> In that sense if GCC follows LLVM and emits compiler specific DWARF
>>> tag
>>> pahole can convert it to the same BTF regardless whether kernel
>>> was compiled with clang or gcc.
>>> drgn can consume dwarf generated by clang or gcc as well even when BTF
>>> is not there. That is the fastest way forward.
>>> In that sense it would be nice to have common DWARF tag for pointer
>>> annotations, but it's not mandatory. The time is the most valuable asset.
>>> Implementing GCC specific DWARF tag doesn't require committee voting
>>> and the mailing list bikeshedding.
>>>
>>>> 3) Addition of C-family language-level constructions to specify
>>>> free-text tags on certain language elements, such as struct fields.
>>>>
>>>> These are the attributes, or built-ins or whatever syntax.
>>>>
>>>> Note that, strictly speaking:
>>>> - This is orthogonal to both DWARF and BTF, and any other supported
>>>> debugging format, which may or may not be expressive enough to
>>>> convey the free-form text tag.
>>>> - This is not specific to BPF.
>>>>
>>>> Therefore I would avoid any reference to BTF or BPF in the attribute
>>>> names. Something like `__attribute__((btf_tag("arbitrary_str")))'
>>>> makes very little sense to me; the attribute name ought to be more
>>>> generic.
>>> Let's agree to disagree.
>>> When BPF ISA was designed we didn't go to Intel, Arm, Mips, etc in order to
>>> come up with the best ISA that would JIT to those architectures the best
>>> possible way. Same thing with btf_tag. Today it is specific to BTF and BPF
>>> only. Hence it's called this way. Whenever actual users will appear that need
>>> free-text tags on a struct field then and only then will be the time to discuss
>>> generic tag name. Just because "free-text tag on a struct field" sounds generic
>>> it doesn't mean that it has any use case beyond what we're using it for in BPF
>>> land. It goes back to the point of coding now instead of talking about coding.
>>> If gcc wants to call it __attribute__((my_precious_gcc_tag("arbitrary_str")))
>>> go ahead and code it this way. The include/linux/compiler.h can accommodate it.
>>
>> Just want to add a little bit context for this. In the beginning when
>> we proposed to add the attribute, we named as a generic name like
>> 'tag' (or something like that). But eventually upstream suggested
>> 'btf_tag' since the use case we proposed is for bpf. At that point, we
>> don't know drgn use cases yet. Even with that, the use cases are still
>> just for linux kernel.
>>
>> At that time, some *similar* use cases did came up, e.g., for
>> swift<->C++ conversion encoding ("tag name", "attribute info") for
>> attributes in the source code, will help a lot. But they will use a
>> different "tag name" than btf_tag to differentiate.
>
> Thanks for the info.
>
> I find it very interesting that the LLVM people prefers to have several
> "use case specific" tag names instead of something more generic, which
> is the exact opposite of what I would have done :) They may have
> appealing reasons for doing so. Do you have a pointer to the dicussion
> you had upstream at hand?
>
> Anyway, I will taste the waters with the other GCC hackers about both
> DIEs and attribute and see what we can come out with. Thanks again for
> reaching out Yonghong.
Hi, Jose,
Any progress on gcc btf_tag support discussion? If possible, could
you add me to the discussion mailing list so I may help to move
the project forward? Thanks a lot!
Yonghong
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG
2022-01-25 3:58 ` Yonghong Song
@ 2022-01-27 15:38 ` Jose E. Marchesi
2022-01-27 16:42 ` Yonghong Song
0 siblings, 1 reply; 43+ messages in thread
From: Jose E. Marchesi @ 2022-01-27 15:38 UTC (permalink / raw)
To: Yonghong Song
Cc: Alexei Starovoitov, bpf, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, kernel-team, Mark Wielaard
> On 12/20/21 1:49 AM, Jose E. Marchesi wrote:
>>
>>> On 12/17/21 5:44 PM, Alexei Starovoitov wrote:
>>>> On Fri, Dec 17, 2021 at 11:40:10AM +0100, Jose E. Marchesi wrote:
>>>>>
>>>>> 2) The need for DWARF to convey free-text tags on certain elements, such
>>>>> as members of struct types.
>>>>>
>>>>> The motivation for this was originally the way the Linux kernel
>>>>> generates its BTF information, using pahole, using DWARF as a source.
>>>>> As we discussed in our last exchange on this topic, this is
>>>>> accidental, i.e. if the kernel switched to generate BTF directly from
>>>>> the compiler and the linker could merge/deduplicate BTF, there would
>>>>> be no need for using DWARF to act as the "unwilling conveyer" of this
>>>>> information. There are additional benefits of this second approach.
>>>>> Thats why we didn't plan to add these extended DWARF DIEs to GCC.
>>>>>
>>>>> However, it now seems that a DWARF consumer, the drgn project, would
>>>>> also benefit from having such a support in DWARF to distinguish
>>>>> between different kind of pointers.
>>>> drgn can use .percpu section in vmlinux for global percpu vars.
>>>> For pointers the annotation is indeed necessary.
>>>>
>>>>> So it seems to me that now we have two use-cases for adding support
>>>>> for these free-text tags to DWARF, as a proper extension to the
>>>>> format, strictly unrelated to BTF, BPF or even the kernel, since:
>>>>> - This is not kernel specific.
>>>>> - This is not directly related to BTF.
>>>>> - This is not directly related to BPF.
>>>> __percpu annotation is kernel specific.
>>>> __user and __rcu are kernel specific too.
>>>> Only BPF and BTF can meaningfully consume all three.
>>>> drgn can consume __percpu.
>>>> In that sense if GCC follows LLVM and emits compiler specific DWARF
>>>> tag
>>>> pahole can convert it to the same BTF regardless whether kernel
>>>> was compiled with clang or gcc.
>>>> drgn can consume dwarf generated by clang or gcc as well even when BTF
>>>> is not there. That is the fastest way forward.
>>>> In that sense it would be nice to have common DWARF tag for pointer
>>>> annotations, but it's not mandatory. The time is the most valuable asset.
>>>> Implementing GCC specific DWARF tag doesn't require committee voting
>>>> and the mailing list bikeshedding.
>>>>
>>>>> 3) Addition of C-family language-level constructions to specify
>>>>> free-text tags on certain language elements, such as struct fields.
>>>>>
>>>>> These are the attributes, or built-ins or whatever syntax.
>>>>>
>>>>> Note that, strictly speaking:
>>>>> - This is orthogonal to both DWARF and BTF, and any other supported
>>>>> debugging format, which may or may not be expressive enough to
>>>>> convey the free-form text tag.
>>>>> - This is not specific to BPF.
>>>>>
>>>>> Therefore I would avoid any reference to BTF or BPF in the attribute
>>>>> names. Something like `__attribute__((btf_tag("arbitrary_str")))'
>>>>> makes very little sense to me; the attribute name ought to be more
>>>>> generic.
>>>> Let's agree to disagree.
>>>> When BPF ISA was designed we didn't go to Intel, Arm, Mips, etc in order to
>>>> come up with the best ISA that would JIT to those architectures the best
>>>> possible way. Same thing with btf_tag. Today it is specific to BTF and BPF
>>>> only. Hence it's called this way. Whenever actual users will appear that need
>>>> free-text tags on a struct field then and only then will be the time to discuss
>>>> generic tag name. Just because "free-text tag on a struct field" sounds generic
>>>> it doesn't mean that it has any use case beyond what we're using it for in BPF
>>>> land. It goes back to the point of coding now instead of talking about coding.
>>>> If gcc wants to call it __attribute__((my_precious_gcc_tag("arbitrary_str")))
>>>> go ahead and code it this way. The include/linux/compiler.h can accommodate it.
>>>
>>> Just want to add a little bit context for this. In the beginning when
>>> we proposed to add the attribute, we named as a generic name like
>>> 'tag' (or something like that). But eventually upstream suggested
>>> 'btf_tag' since the use case we proposed is for bpf. At that point, we
>>> don't know drgn use cases yet. Even with that, the use cases are still
>>> just for linux kernel.
>>>
>>> At that time, some *similar* use cases did came up, e.g., for
>>> swift<->C++ conversion encoding ("tag name", "attribute info") for
>>> attributes in the source code, will help a lot. But they will use a
>>> different "tag name" than btf_tag to differentiate.
>> Thanks for the info.
>> I find it very interesting that the LLVM people prefers to have
>> several
>> "use case specific" tag names instead of something more generic, which
>> is the exact opposite of what I would have done :) They may have
>> appealing reasons for doing so. Do you have a pointer to the dicussion
>> you had upstream at hand?
>> Anyway, I will taste the waters with the other GCC hackers about
>> both
>> DIEs and attribute and see what we can come out with. Thanks again for
>> reaching out Yonghong.
>
> Hi, Jose,
>
> Any progress on gcc btf_tag support discussion? If possible, could
> you add me to the discussion mailing list so I may help to move
> the project forward? Thanks a lot!
We are in the process of implementing the support of the BTF extensions
(which is done) and the C language attributes (which is WIP.)
I haven't started the discussion about DWARF yet. Will do shortly. You
will be in CC :)
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG
2022-01-27 15:38 ` Jose E. Marchesi
@ 2022-01-27 16:42 ` Yonghong Song
2022-02-17 13:20 ` Jose E. Marchesi
0 siblings, 1 reply; 43+ messages in thread
From: Yonghong Song @ 2022-01-27 16:42 UTC (permalink / raw)
To: Jose E. Marchesi
Cc: Alexei Starovoitov, bpf, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, kernel-team, Mark Wielaard
On 1/27/22 7:38 AM, Jose E. Marchesi wrote:
>
>> On 12/20/21 1:49 AM, Jose E. Marchesi wrote:
>>>
>>>> On 12/17/21 5:44 PM, Alexei Starovoitov wrote:
>>>>> On Fri, Dec 17, 2021 at 11:40:10AM +0100, Jose E. Marchesi wrote:
>>>>>>
>>>>>> 2) The need for DWARF to convey free-text tags on certain elements, such
>>>>>> as members of struct types.
>>>>>>
>>>>>> The motivation for this was originally the way the Linux kernel
>>>>>> generates its BTF information, using pahole, using DWARF as a source.
>>>>>> As we discussed in our last exchange on this topic, this is
>>>>>> accidental, i.e. if the kernel switched to generate BTF directly from
>>>>>> the compiler and the linker could merge/deduplicate BTF, there would
>>>>>> be no need for using DWARF to act as the "unwilling conveyer" of this
>>>>>> information. There are additional benefits of this second approach.
>>>>>> Thats why we didn't plan to add these extended DWARF DIEs to GCC.
>>>>>>
>>>>>> However, it now seems that a DWARF consumer, the drgn project, would
>>>>>> also benefit from having such a support in DWARF to distinguish
>>>>>> between different kind of pointers.
>>>>> drgn can use .percpu section in vmlinux for global percpu vars.
>>>>> For pointers the annotation is indeed necessary.
>>>>>
>>>>>> So it seems to me that now we have two use-cases for adding support
>>>>>> for these free-text tags to DWARF, as a proper extension to the
>>>>>> format, strictly unrelated to BTF, BPF or even the kernel, since:
>>>>>> - This is not kernel specific.
>>>>>> - This is not directly related to BTF.
>>>>>> - This is not directly related to BPF.
>>>>> __percpu annotation is kernel specific.
>>>>> __user and __rcu are kernel specific too.
>>>>> Only BPF and BTF can meaningfully consume all three.
>>>>> drgn can consume __percpu.
>>>>> In that sense if GCC follows LLVM and emits compiler specific DWARF
>>>>> tag
>>>>> pahole can convert it to the same BTF regardless whether kernel
>>>>> was compiled with clang or gcc.
>>>>> drgn can consume dwarf generated by clang or gcc as well even when BTF
>>>>> is not there. That is the fastest way forward.
>>>>> In that sense it would be nice to have common DWARF tag for pointer
>>>>> annotations, but it's not mandatory. The time is the most valuable asset.
>>>>> Implementing GCC specific DWARF tag doesn't require committee voting
>>>>> and the mailing list bikeshedding.
>>>>>
>>>>>> 3) Addition of C-family language-level constructions to specify
>>>>>> free-text tags on certain language elements, such as struct fields.
>>>>>>
>>>>>> These are the attributes, or built-ins or whatever syntax.
>>>>>>
>>>>>> Note that, strictly speaking:
>>>>>> - This is orthogonal to both DWARF and BTF, and any other supported
>>>>>> debugging format, which may or may not be expressive enough to
>>>>>> convey the free-form text tag.
>>>>>> - This is not specific to BPF.
>>>>>>
>>>>>> Therefore I would avoid any reference to BTF or BPF in the attribute
>>>>>> names. Something like `__attribute__((btf_tag("arbitrary_str")))'
>>>>>> makes very little sense to me; the attribute name ought to be more
>>>>>> generic.
>>>>> Let's agree to disagree.
>>>>> When BPF ISA was designed we didn't go to Intel, Arm, Mips, etc in order to
>>>>> come up with the best ISA that would JIT to those architectures the best
>>>>> possible way. Same thing with btf_tag. Today it is specific to BTF and BPF
>>>>> only. Hence it's called this way. Whenever actual users will appear that need
>>>>> free-text tags on a struct field then and only then will be the time to discuss
>>>>> generic tag name. Just because "free-text tag on a struct field" sounds generic
>>>>> it doesn't mean that it has any use case beyond what we're using it for in BPF
>>>>> land. It goes back to the point of coding now instead of talking about coding.
>>>>> If gcc wants to call it __attribute__((my_precious_gcc_tag("arbitrary_str")))
>>>>> go ahead and code it this way. The include/linux/compiler.h can accommodate it.
>>>>
>>>> Just want to add a little bit context for this. In the beginning when
>>>> we proposed to add the attribute, we named as a generic name like
>>>> 'tag' (or something like that). But eventually upstream suggested
>>>> 'btf_tag' since the use case we proposed is for bpf. At that point, we
>>>> don't know drgn use cases yet. Even with that, the use cases are still
>>>> just for linux kernel.
>>>>
>>>> At that time, some *similar* use cases did came up, e.g., for
>>>> swift<->C++ conversion encoding ("tag name", "attribute info") for
>>>> attributes in the source code, will help a lot. But they will use a
>>>> different "tag name" than btf_tag to differentiate.
>>> Thanks for the info.
>>> I find it very interesting that the LLVM people prefers to have
>>> several
>>> "use case specific" tag names instead of something more generic, which
>>> is the exact opposite of what I would have done :) They may have
>>> appealing reasons for doing so. Do you have a pointer to the dicussion
>>> you had upstream at hand?
>>> Anyway, I will taste the waters with the other GCC hackers about
>>> both
>>> DIEs and attribute and see what we can come out with. Thanks again for
>>> reaching out Yonghong.
>>
>> Hi, Jose,
>>
>> Any progress on gcc btf_tag support discussion? If possible, could
>> you add me to the discussion mailing list so I may help to move
>> the project forward? Thanks a lot!
>
> We are in the process of implementing the support of the BTF extensions
> (which is done) and the C language attributes (which is WIP.)
Sounds good. I am happy to answer questions if you have any.
>
> I haven't started the discussion about DWARF yet. Will do shortly. You
> will be in CC :)
Thanks a lot, Jose! I am looking forward to the discussion.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG
2022-01-27 16:42 ` Yonghong Song
@ 2022-02-17 13:20 ` Jose E. Marchesi
2022-02-17 15:28 ` Alexei Starovoitov
0 siblings, 1 reply; 43+ messages in thread
From: Jose E. Marchesi @ 2022-02-17 13:20 UTC (permalink / raw)
To: Yonghong Song
Cc: Alexei Starovoitov, bpf, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, kernel-team, Mark Wielaard, david.faust
> On 1/27/22 7:38 AM, Jose E. Marchesi wrote:
>>
>>> On 12/20/21 1:49 AM, Jose E. Marchesi wrote:
>>>>
>>>>> On 12/17/21 5:44 PM, Alexei Starovoitov wrote:
>>>>>> On Fri, Dec 17, 2021 at 11:40:10AM +0100, Jose E. Marchesi wrote:
>>>>>>>
>>>>>>> 2) The need for DWARF to convey free-text tags on certain elements, such
>>>>>>> as members of struct types.
>>>>>>>
>>>>>>> The motivation for this was originally the way the Linux kernel
>>>>>>> generates its BTF information, using pahole, using DWARF as a source.
>>>>>>> As we discussed in our last exchange on this topic, this is
>>>>>>> accidental, i.e. if the kernel switched to generate BTF directly from
>>>>>>> the compiler and the linker could merge/deduplicate BTF, there would
>>>>>>> be no need for using DWARF to act as the "unwilling conveyer" of this
>>>>>>> information. There are additional benefits of this second approach.
>>>>>>> Thats why we didn't plan to add these extended DWARF DIEs to GCC.
>>>>>>>
>>>>>>> However, it now seems that a DWARF consumer, the drgn project, would
>>>>>>> also benefit from having such a support in DWARF to distinguish
>>>>>>> between different kind of pointers.
>>>>>> drgn can use .percpu section in vmlinux for global percpu vars.
>>>>>> For pointers the annotation is indeed necessary.
>>>>>>
>>>>>>> So it seems to me that now we have two use-cases for adding support
>>>>>>> for these free-text tags to DWARF, as a proper extension to the
>>>>>>> format, strictly unrelated to BTF, BPF or even the kernel, since:
>>>>>>> - This is not kernel specific.
>>>>>>> - This is not directly related to BTF.
>>>>>>> - This is not directly related to BPF.
>>>>>> __percpu annotation is kernel specific.
>>>>>> __user and __rcu are kernel specific too.
>>>>>> Only BPF and BTF can meaningfully consume all three.
>>>>>> drgn can consume __percpu.
>>>>>> In that sense if GCC follows LLVM and emits compiler specific DWARF
>>>>>> tag
>>>>>> pahole can convert it to the same BTF regardless whether kernel
>>>>>> was compiled with clang or gcc.
>>>>>> drgn can consume dwarf generated by clang or gcc as well even when BTF
>>>>>> is not there. That is the fastest way forward.
>>>>>> In that sense it would be nice to have common DWARF tag for pointer
>>>>>> annotations, but it's not mandatory. The time is the most valuable asset.
>>>>>> Implementing GCC specific DWARF tag doesn't require committee voting
>>>>>> and the mailing list bikeshedding.
>>>>>>
>>>>>>> 3) Addition of C-family language-level constructions to specify
>>>>>>> free-text tags on certain language elements, such as struct fields.
>>>>>>>
>>>>>>> These are the attributes, or built-ins or whatever syntax.
>>>>>>>
>>>>>>> Note that, strictly speaking:
>>>>>>> - This is orthogonal to both DWARF and BTF, and any other supported
>>>>>>> debugging format, which may or may not be expressive enough to
>>>>>>> convey the free-form text tag.
>>>>>>> - This is not specific to BPF.
>>>>>>>
>>>>>>> Therefore I would avoid any reference to BTF or BPF in the attribute
>>>>>>> names. Something like `__attribute__((btf_tag("arbitrary_str")))'
>>>>>>> makes very little sense to me; the attribute name ought to be more
>>>>>>> generic.
>>>>>> Let's agree to disagree.
>>>>>> When BPF ISA was designed we didn't go to Intel, Arm, Mips, etc in order to
>>>>>> come up with the best ISA that would JIT to those architectures the best
>>>>>> possible way. Same thing with btf_tag. Today it is specific to BTF and BPF
>>>>>> only. Hence it's called this way. Whenever actual users will appear that need
>>>>>> free-text tags on a struct field then and only then will be the time to discuss
>>>>>> generic tag name. Just because "free-text tag on a struct field" sounds generic
>>>>>> it doesn't mean that it has any use case beyond what we're using it for in BPF
>>>>>> land. It goes back to the point of coding now instead of talking about coding.
>>>>>> If gcc wants to call it __attribute__((my_precious_gcc_tag("arbitrary_str")))
>>>>>> go ahead and code it this way. The include/linux/compiler.h can accommodate it.
>>>>>
>>>>> Just want to add a little bit context for this. In the beginning when
>>>>> we proposed to add the attribute, we named as a generic name like
>>>>> 'tag' (or something like that). But eventually upstream suggested
>>>>> 'btf_tag' since the use case we proposed is for bpf. At that point, we
>>>>> don't know drgn use cases yet. Even with that, the use cases are still
>>>>> just for linux kernel.
>>>>>
>>>>> At that time, some *similar* use cases did came up, e.g., for
>>>>> swift<->C++ conversion encoding ("tag name", "attribute info") for
>>>>> attributes in the source code, will help a lot. But they will use a
>>>>> different "tag name" than btf_tag to differentiate.
>>>> Thanks for the info.
>>>> I find it very interesting that the LLVM people prefers to have
>>>> several
>>>> "use case specific" tag names instead of something more generic, which
>>>> is the exact opposite of what I would have done :) They may have
>>>> appealing reasons for doing so. Do you have a pointer to the dicussion
>>>> you had upstream at hand?
>>>> Anyway, I will taste the waters with the other GCC hackers about
>>>> both
>>>> DIEs and attribute and see what we can come out with. Thanks again for
>>>> reaching out Yonghong.
>>>
>>> Hi, Jose,
>>>
>>> Any progress on gcc btf_tag support discussion? If possible, could
>>> you add me to the discussion mailing list so I may help to move
>>> the project forward? Thanks a lot!
>> We are in the process of implementing the support of the BTF
>> extensions
>> (which is done) and the C language attributes (which is WIP.)
>
> Sounds good. I am happy to answer questions if you have any.
>
>> I haven't started the discussion about DWARF yet. Will do shortly.
>> You
>> will be in CC :)
>
> Thanks a lot, Jose! I am looking forward to the discussion.
Just a heads-up.
We are still working on the GCC implementation of the tags. Having some
difficulties with the ordering of the C type attributes.
Regarding the DWARF part, GCC uses DWARF as the internal "canonical"
debug info, and the BTF is generated from it. This means we had to add
a DWARF DIE for the pointer tag qualifier anyway in order to convey the
info to BTF. So now it is just a matter of emitting it along with the
rest of the DWARF.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG
2022-02-17 13:20 ` Jose E. Marchesi
@ 2022-02-17 15:28 ` Alexei Starovoitov
2022-02-17 16:41 ` Jose E. Marchesi
0 siblings, 1 reply; 43+ messages in thread
From: Alexei Starovoitov @ 2022-02-17 15:28 UTC (permalink / raw)
To: Jose E. Marchesi
Cc: Yonghong Song, bpf, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Kernel Team, Mark Wielaard, david.faust
On Thu, Feb 17, 2022 at 5:20 AM Jose E. Marchesi
<jose.marchesi@oracle.com> wrote:
>
> Just a heads-up.
>
> We are still working on the GCC implementation of the tags. Having some
> difficulties with the ordering of the C type attributes.
>
> Regarding the DWARF part, GCC uses DWARF as the internal "canonical"
> debug info, and the BTF is generated from it. This means we had to add
> a DWARF DIE for the pointer tag qualifier anyway in order to convey the
> info to BTF. So now it is just a matter of emitting it along with the
> rest of the DWARF.
Thanks for the update!
Do you have an early git branch we can use to test building
the kernel with it?
Or is it not at this level yet?
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG
2022-02-17 15:28 ` Alexei Starovoitov
@ 2022-02-17 16:41 ` Jose E. Marchesi
0 siblings, 0 replies; 43+ messages in thread
From: Jose E. Marchesi @ 2022-02-17 16:41 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Yonghong Song, bpf, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Kernel Team, Mark Wielaard, david.faust
> On Thu, Feb 17, 2022 at 5:20 AM Jose E. Marchesi
> <jose.marchesi@oracle.com> wrote:
>>
>> Just a heads-up.
>>
>> We are still working on the GCC implementation of the tags. Having some
>> difficulties with the ordering of the C type attributes.
>>
>> Regarding the DWARF part, GCC uses DWARF as the internal "canonical"
>> debug info, and the BTF is generated from it. This means we had to add
>> a DWARF DIE for the pointer tag qualifier anyway in order to convey the
>> info to BTF. So now it is just a matter of emitting it along with the
>> rest of the DWARF.
>
> Thanks for the update!
> Do you have an early git branch we can use to test building
> the kernel with it?
> Or is it not at this level yet?
Not yet.
Once we have something working internally we will submit the patches to
gcc-patches for discussion. At that point we can put them in a branch
for early testing.
^ permalink raw reply [flat|nested] 43+ messages in thread
end of thread, other threads:[~2022-02-17 16:42 UTC | newest]
Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-13 15:51 [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG Yonghong Song
2021-09-13 15:51 ` [PATCH bpf-next v2 01/11] btf: change BTF_KIND_* macros to enums Yonghong Song
2021-09-14 4:59 ` Andrii Nakryiko
2021-09-14 15:53 ` Yonghong Song
2021-09-13 15:51 ` [PATCH bpf-next v2 02/11] bpf: support for new btf kind BTF_KIND_TAG Yonghong Song
2021-09-14 5:08 ` Andrii Nakryiko
2021-09-14 15:59 ` Yonghong Song
2021-09-14 23:30 ` Andrii Nakryiko
2021-09-13 15:51 ` [PATCH bpf-next v2 03/11] libbpf: rename btf_{hash,equal}_int to btf_{hash,equal}_int_tag Yonghong Song
2021-09-13 15:51 ` [PATCH bpf-next v2 04/11] libbpf: add support for BTF_KIND_TAG Yonghong Song
2021-09-14 5:15 ` Andrii Nakryiko
2021-09-14 16:42 ` Yonghong Song
2021-09-13 15:51 ` [PATCH bpf-next v2 05/11] bpftool: " Yonghong Song
2021-09-14 5:16 ` Andrii Nakryiko
2021-09-13 15:51 ` [PATCH bpf-next v2 06/11] selftests/bpf: test libbpf API function btf__add_tag() Yonghong Song
2021-09-14 5:18 ` Andrii Nakryiko
2021-09-13 15:52 ` [PATCH bpf-next v2 07/11] selftests/bpf: change NAME_NTH/IS_NAME_NTH for BTF_KIND_TAG format Yonghong Song
2021-09-14 5:23 ` Andrii Nakryiko
2021-09-13 15:52 ` [PATCH bpf-next v2 08/11] selftests/bpf: add BTF_KIND_TAG unit tests Yonghong Song
2021-09-14 5:31 ` Andrii Nakryiko
2021-09-14 17:00 ` Yonghong Song
2021-09-13 15:52 ` [PATCH bpf-next v2 09/11] selftests/bpf: test BTF_KIND_TAG for deduplication Yonghong Song
2021-09-14 5:38 ` Andrii Nakryiko
2021-09-14 17:15 ` Yonghong Song
2021-09-14 19:39 ` Yonghong Song
2021-09-14 23:31 ` Andrii Nakryiko
2021-09-13 15:52 ` [PATCH bpf-next v2 10/11] selftests/bpf: add a test with a bpf program with btf_tag attributes Yonghong Song
2021-09-13 15:52 ` [PATCH bpf-next v2 11/11] docs/bpf: add documentation for BTF_KIND_TAG Yonghong Song
2021-09-14 5:40 ` Andrii Nakryiko
2021-09-13 16:08 ` [PATCH bpf-next v2 00/11] bpf: add support for new btf kind BTF_KIND_TAG Yonghong Song
2021-09-13 16:40 ` Jose E. Marchesi
2021-12-16 21:52 ` Yonghong Song
2021-12-17 10:40 ` Jose E. Marchesi
2021-12-18 1:44 ` Alexei Starovoitov
2021-12-18 20:15 ` Yonghong Song
2021-12-20 9:49 ` Jose E. Marchesi
2021-12-20 15:52 ` Yonghong Song
2022-01-25 3:58 ` Yonghong Song
2022-01-27 15:38 ` Jose E. Marchesi
2022-01-27 16:42 ` Yonghong Song
2022-02-17 13:20 ` Jose E. Marchesi
2022-02-17 15:28 ` Alexei Starovoitov
2022-02-17 16:41 ` Jose E. Marchesi
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).