All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next v3 00/10] Introduce type match support
@ 2022-06-28 16:01 Daniel Müller
  2022-06-28 16:01 ` [PATCH bpf-next v3 01/10] bpf: Introduce TYPE_MATCH related constants/macros Daniel Müller
                   ` (11 more replies)
  0 siblings, 12 replies; 18+ messages in thread
From: Daniel Müller @ 2022-06-28 16:01 UTC (permalink / raw)
  To: bpf, ast, andrii, daniel, kernel-team; +Cc: joannelkoong

This patch set proposes the addition of a new way for performing type queries to
BPF. It introduces the "type matches" relation, similar to what is already
present with "type exists" (in the form of bpf_core_type_exists).

"type exists" performs fairly superficial checking, mostly concerned with
whether a type exists in the kernel and is of the same kind (enum/struct/...).
Notably, compatibility checks for members of composite types is lacking.

The newly introduced "type matches" (bpf_core_type_matches) fills this gap in
that it performs stricter checks: compatibility of members and existence of
similarly named enum variants is checked as well. E.g., given these definitions:

	struct task_struct___og { int pid; int tgid; };

	struct task_struct___foo { int foo; }

'task_struct___og' would "match" the kernel type 'task_struct', because the
members match up, while 'task_struct___foo' would not match, because the
kernel's 'task_struct' has no member named 'foo'.

More precisely, the "type match" relation is defined as follows (copied from
source):
- modifiers and typedefs are stripped (and, hence, effectively ignored)
- generally speaking types need to be of same kind (struct vs. struct, union
  vs. union, etc.)
  - exceptions are struct/union behind a pointer which could also match a
    forward declaration of a struct or union, respectively, and enum vs.
    enum64 (see below)
Then, depending on type:
- integers:
  - match if size and signedness match
- arrays & pointers:
  - target types are recursively matched
- structs & unions:
  - local members need to exist in target with the same name
  - for each member we recursively check match unless it is already behind a
    pointer, in which case we only check matching names and compatible kind
- enums:
  - local variants have to have a match in target by symbolic name (but not
    numeric value)
  - size has to match (but enum may match enum64 and vice versa)
- function pointers:
  - number and position of arguments in local type has to match target
  - for each argument and the return value we recursively check match

Enabling this feature requires a new relocation to be made known to the
compiler. This is being taken care of for LLVM as part of
https://reviews.llvm.org/D126838.

If applied, among other things, usage of this functionality could have helped
flag issues such as the one discussed here
https://lore.kernel.org/all/93a20759600c05b6d9e4359a1517c88e06b44834.camel@fb.com/
earlier.

Suggested-by: Andrii Nakryiko <andrii@kernel.org>
---
Changelog:
v2 -> v3:
- renamed btfgen_mark_types_match
- covered BTF_KIND_RESTRICT in type match marking logic
- used bpf_core_names_match in more places
- reworked "behind pointer" logic
- added test using live task_struct

v1 -> v2:
- deduplicated and moved core algorithm into relo_core.c
- adjusted bpf_core_names_match to get btf_type passed in
- removed some length equality checks before strncmp usage
- correctly use kflag from targ_t instead of local_t
- added comment for meaning of kflag w/ FWD kind
- __u32 -> u32
- handle BTF_KIND_FWD properly in bpftool marking logic
- rebased

Daniel Müller (10):
  bpf: Introduce TYPE_MATCH related constants/macros
  bpftool: Honor BPF_CORE_TYPE_MATCHES relocation
  bpf: Introduce btf_int_bits() function
  libbpf: Add type match support
  bpf: Add type match support
  libbpf: Honor TYPE_MATCH relocation
  selftests/bpf: Add type-match checks to type-based tests
  selftests/bpf: Add test checking more characteristics
  selftests/bpf: Add nested type to type based tests
  selftests/bpf: Add type match test against kernel's task_struct

 include/linux/btf.h                           |   5 +
 include/uapi/linux/bpf.h                      |   1 +
 kernel/bpf/btf.c                              |   9 +
 tools/bpf/bpftool/gen.c                       | 108 +++++++
 tools/include/uapi/linux/bpf.h                |   1 +
 tools/lib/bpf/bpf_core_read.h                 |  11 +
 tools/lib/bpf/libbpf.c                        |   6 +
 tools/lib/bpf/relo_core.c                     | 284 +++++++++++++++++-
 tools/lib/bpf/relo_core.h                     |   4 +
 .../selftests/bpf/prog_tests/core_reloc.c     |  73 ++++-
 .../progs/btf__core_reloc_type_based___diff.c |   3 +
 .../selftests/bpf/progs/core_reloc_types.h    | 108 ++++++-
 .../bpf/progs/test_core_reloc_kernel.c        |  11 +
 .../bpf/progs/test_core_reloc_type_based.c    |  44 ++-
 14 files changed, 650 insertions(+), 18 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_type_based___diff.c

-- 
2.30.2


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

* [PATCH bpf-next v3 01/10] bpf: Introduce TYPE_MATCH related constants/macros
  2022-06-28 16:01 [PATCH bpf-next v3 00/10] Introduce type match support Daniel Müller
@ 2022-06-28 16:01 ` Daniel Müller
  2022-06-28 16:01 ` [PATCH bpf-next v3 02/10] bpftool: Honor BPF_CORE_TYPE_MATCHES relocation Daniel Müller
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Daniel Müller @ 2022-06-28 16:01 UTC (permalink / raw)
  To: bpf, ast, andrii, daniel, kernel-team; +Cc: joannelkoong

In order to provide type match support we require a new type of
relocation which, in turn, requires toolchain support. Recent LLVM/Clang
versions support a new value for the last argument to the
__builtin_preserve_type_info builtin, for example.
With this change we introduce the necessary constants into relevant
header files, mirroring what the compiler may support.

Signed-off-by: Daniel Müller <deso@posteo.net>
---
 include/uapi/linux/bpf.h       | 1 +
 tools/include/uapi/linux/bpf.h | 1 +
 tools/lib/bpf/bpf_core_read.h  | 1 +
 3 files changed, 3 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index e81362..42605c 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -6782,6 +6782,7 @@ enum bpf_core_relo_kind {
 	BPF_CORE_TYPE_SIZE = 9,              /* type size in bytes */
 	BPF_CORE_ENUMVAL_EXISTS = 10,        /* enum value existence in target kernel */
 	BPF_CORE_ENUMVAL_VALUE = 11,         /* enum value integer value */
+	BPF_CORE_TYPE_MATCHES = 12,          /* type match in target kernel */
 };
 
 /*
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index e81362..42605c 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -6782,6 +6782,7 @@ enum bpf_core_relo_kind {
 	BPF_CORE_TYPE_SIZE = 9,              /* type size in bytes */
 	BPF_CORE_ENUMVAL_EXISTS = 10,        /* enum value existence in target kernel */
 	BPF_CORE_ENUMVAL_VALUE = 11,         /* enum value integer value */
+	BPF_CORE_TYPE_MATCHES = 12,          /* type match in target kernel */
 };
 
 /*
diff --git a/tools/lib/bpf/bpf_core_read.h b/tools/lib/bpf/bpf_core_read.h
index fd48b1..2308f49 100644
--- a/tools/lib/bpf/bpf_core_read.h
+++ b/tools/lib/bpf/bpf_core_read.h
@@ -29,6 +29,7 @@ enum bpf_type_id_kind {
 enum bpf_type_info_kind {
 	BPF_TYPE_EXISTS = 0,		/* type existence in target kernel */
 	BPF_TYPE_SIZE = 1,		/* type size in target kernel */
+	BPF_TYPE_MATCHES = 2,		/* type match in target kernel */
 };
 
 /* second argument to __builtin_preserve_enum_value() built-in */
-- 
2.30.2


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

* [PATCH bpf-next v3 02/10] bpftool: Honor BPF_CORE_TYPE_MATCHES relocation
  2022-06-28 16:01 [PATCH bpf-next v3 00/10] Introduce type match support Daniel Müller
  2022-06-28 16:01 ` [PATCH bpf-next v3 01/10] bpf: Introduce TYPE_MATCH related constants/macros Daniel Müller
@ 2022-06-28 16:01 ` Daniel Müller
  2022-06-28 16:52   ` Quentin Monnet
  2022-07-06  4:16   ` Andrii Nakryiko
  2022-06-28 16:01 ` [PATCH bpf-next v3 03/10] bpf: Introduce btf_int_bits() function Daniel Müller
                   ` (9 subsequent siblings)
  11 siblings, 2 replies; 18+ messages in thread
From: Daniel Müller @ 2022-06-28 16:01 UTC (permalink / raw)
  To: bpf, ast, andrii, daniel, kernel-team; +Cc: joannelkoong, Quentin Monnet

bpftool needs to know about the newly introduced BPF_CORE_TYPE_MATCHES
relocation for its 'gen min_core_btf' command to work properly in the
present of this relocation.
Specifically, we need to make sure to mark types and fields so that they
are present in the minimized BTF for "type match" checks to work out.
However, contrary to the existing btfgen_record_field_relo, we need to
rely on the BTF -- and not the spec -- to find fields. With this change
we handle this new variant correctly. The functionality will be tested
with follow on changes to BPF selftests, which already run against a
minimized BTF created with bpftool.

Cc: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Daniel Müller <deso@posteo.net>
---
 tools/bpf/bpftool/gen.c | 108 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 108 insertions(+)

diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 480cbd8..a30328c 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -1856,6 +1856,112 @@ static int btfgen_record_field_relo(struct btfgen_info *info, struct bpf_core_sp
 	return 0;
 }
 
+/* Mark types, members, and member types. Compared to btfgen_record_field_relo,
+ * this function does not rely on the target spec for inferring members, but
+ * uses the associated BTF.
+ *
+ * The `behind_ptr` argument is used to stop marking of composite types reached
+ * through a pointer. This way, we can keep BTF size in check while providing
+ * reasonable match semantics.
+ */
+static int btfgen_mark_type_match(struct btfgen_info *info, __u32 type_id, bool behind_ptr)
+{
+	const struct btf_type *btf_type;
+	struct btf *btf = info->src_btf;
+	struct btf_type *cloned_type;
+	int i, err;
+
+	if (type_id == 0)
+		return 0;
+
+	btf_type = btf__type_by_id(btf, type_id);
+	/* mark type on cloned BTF as used */
+	cloned_type = (struct btf_type *)btf__type_by_id(info->marked_btf, type_id);
+	cloned_type->name_off = MARKED;
+
+	switch (btf_kind(btf_type)) {
+	case BTF_KIND_UNKN:
+	case BTF_KIND_INT:
+	case BTF_KIND_FLOAT:
+	case BTF_KIND_ENUM:
+	case BTF_KIND_ENUM64:
+		break;
+	case BTF_KIND_STRUCT:
+	case BTF_KIND_UNION: {
+		struct btf_member *m = btf_members(btf_type);
+		__u16 vlen = btf_vlen(btf_type);
+
+		if (behind_ptr)
+			break;
+
+		for (i = 0; i < vlen; i++, m++) {
+			/* mark member */
+			btfgen_mark_member(info, type_id, i);
+
+			/* mark member's type */
+			err = btfgen_mark_type_match(info, m->type, false);
+			if (err)
+				return err;
+		}
+		break;
+	}
+	case BTF_KIND_CONST:
+	case BTF_KIND_FWD:
+	case BTF_KIND_RESTRICT:
+	case BTF_KIND_TYPEDEF:
+	case BTF_KIND_VOLATILE:
+		return btfgen_mark_type_match(info, btf_type->type, false);
+	case BTF_KIND_PTR:
+		return btfgen_mark_type_match(info, btf_type->type, true);
+	case BTF_KIND_ARRAY: {
+		struct btf_array *array;
+
+		array = btf_array(btf_type);
+		/* mark array type */
+		err = btfgen_mark_type_match(info, array->type, false);
+		/* mark array's index type */
+		err = err ? : btfgen_mark_type_match(info, array->index_type, false);
+		if (err)
+			return err;
+		break;
+	}
+	case BTF_KIND_FUNC_PROTO: {
+		__u16 vlen = btf_vlen(btf_type);
+		struct btf_param *param;
+
+		/* mark ret type */
+		err = btfgen_mark_type_match(info, btf_type->type, false);
+		if (err)
+			return err;
+
+		/* mark parameters types */
+		param = btf_params(btf_type);
+		for (i = 0; i < vlen; i++) {
+			err = btfgen_mark_type_match(info, param->type, false);
+			if (err)
+				return err;
+			param++;
+		}
+		break;
+	}
+	/* tells if some other type needs to be handled */
+	default:
+		p_err("unsupported kind: %s (%d)", btf_kind_str(btf_type), type_id);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/* Mark types, members, and member types. Compared to btfgen_record_field_relo,
+ * this function does not rely on the target spec for inferring members, but
+ * uses the associated BTF.
+ */
+static int btfgen_record_type_match_relo(struct btfgen_info *info, struct bpf_core_spec *targ_spec)
+{
+	return btfgen_mark_type_match(info, targ_spec->root_type_id, false);
+}
+
 static int btfgen_record_type_relo(struct btfgen_info *info, struct bpf_core_spec *targ_spec)
 {
 	return btfgen_mark_type(info, targ_spec->root_type_id, true);
@@ -1882,6 +1988,8 @@ static int btfgen_record_reloc(struct btfgen_info *info, struct bpf_core_spec *r
 	case BPF_CORE_TYPE_EXISTS:
 	case BPF_CORE_TYPE_SIZE:
 		return btfgen_record_type_relo(info, res);
+	case BPF_CORE_TYPE_MATCHES:
+		return btfgen_record_type_match_relo(info, res);
 	case BPF_CORE_ENUMVAL_EXISTS:
 	case BPF_CORE_ENUMVAL_VALUE:
 		return btfgen_record_enumval_relo(info, res);
-- 
2.30.2


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

* [PATCH bpf-next v3 03/10] bpf: Introduce btf_int_bits() function
  2022-06-28 16:01 [PATCH bpf-next v3 00/10] Introduce type match support Daniel Müller
  2022-06-28 16:01 ` [PATCH bpf-next v3 01/10] bpf: Introduce TYPE_MATCH related constants/macros Daniel Müller
  2022-06-28 16:01 ` [PATCH bpf-next v3 02/10] bpftool: Honor BPF_CORE_TYPE_MATCHES relocation Daniel Müller
@ 2022-06-28 16:01 ` Daniel Müller
  2022-06-28 16:01 ` [PATCH bpf-next v3 04/10] libbpf: Add type match support Daniel Müller
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Daniel Müller @ 2022-06-28 16:01 UTC (permalink / raw)
  To: bpf, ast, andrii, daniel, kernel-team; +Cc: joannelkoong

This change adds the btf_int_bits() function to include/linux/btf.h. It
mirrors what already exists in user space and will be required by follow
on changes.

Signed-off-by: Daniel Müller <deso@posteo.net>
---
 include/linux/btf.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/btf.h b/include/linux/btf.h
index 1bfed7..54a65a 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -242,6 +242,11 @@ static inline u8 btf_int_offset(const struct btf_type *t)
 	return BTF_INT_OFFSET(*(u32 *)(t + 1));
 }
 
+static inline u8 btf_int_bits(const struct btf_type *t)
+{
+	return BTF_INT_BITS(*(u32 *)(t + 1));
+}
+
 static inline u8 btf_int_encoding(const struct btf_type *t)
 {
 	return BTF_INT_ENCODING(*(u32 *)(t + 1));
-- 
2.30.2


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

* [PATCH bpf-next v3 04/10] libbpf: Add type match support
  2022-06-28 16:01 [PATCH bpf-next v3 00/10] Introduce type match support Daniel Müller
                   ` (2 preceding siblings ...)
  2022-06-28 16:01 ` [PATCH bpf-next v3 03/10] bpf: Introduce btf_int_bits() function Daniel Müller
@ 2022-06-28 16:01 ` Daniel Müller
  2022-07-06  4:16   ` Andrii Nakryiko
  2022-06-28 16:01 ` [PATCH bpf-next v3 05/10] bpf: " Daniel Müller
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 18+ messages in thread
From: Daniel Müller @ 2022-06-28 16:01 UTC (permalink / raw)
  To: bpf, ast, andrii, daniel, kernel-team; +Cc: joannelkoong

This patch adds support for the proposed type match relation to
relo_core where it is shared between userspace and kernel. A bit more
plumbing is necessary and will arrive with subsequent changes to
actually use it -- this patch only introduces the main matching
algorithm.

The matching relation is defined as follows (copy from source):
- modifiers and typedefs are stripped (and, hence, effectively ignored)
- generally speaking types need to be of same kind (struct vs. struct, union
  vs. union, etc.)
  - exceptions are struct/union behind a pointer which could also match a
    forward declaration of a struct or union, respectively, and enum vs.
    enum64 (see below)
Then, depending on type:
- integers:
  - match if size and signedness match
- arrays & pointers:
  - target types are recursively matched
- structs & unions:
  - local members need to exist in target with the same name
  - for each member we recursively check match unless it is already behind a
    pointer, in which case we only check matching names and compatible kind
- enums:
  - local variants have to have a match in target by symbolic name (but not
    numeric value)
  - size has to match (but enum may match enum64 and vice versa)
- function pointers:
  - number and position of arguments in local type has to match target
  - for each argument and the return value we recursively check match

Signed-off-by: Daniel Müller <deso@posteo.net>
---
 tools/lib/bpf/relo_core.c | 268 ++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/relo_core.h |   2 +
 2 files changed, 270 insertions(+)

diff --git a/tools/lib/bpf/relo_core.c b/tools/lib/bpf/relo_core.c
index e070123..b3f5d7e 100644
--- a/tools/lib/bpf/relo_core.c
+++ b/tools/lib/bpf/relo_core.c
@@ -1410,3 +1410,271 @@ int bpf_core_calc_relo_insn(const char *prog_name,
 
 	return 0;
 }
+
+static bool bpf_core_names_match(const struct btf *local_btf, size_t local_name_off,
+				 const struct btf *targ_btf, size_t targ_name_off)
+{
+	const char *local_n, *targ_n;
+	size_t local_len, targ_len;
+
+	local_n = btf__name_by_offset(local_btf, local_name_off);
+	targ_n = btf__name_by_offset(targ_btf, targ_name_off);
+
+	if (str_is_empty(targ_n))
+		return str_is_empty(local_n);
+
+	targ_len = bpf_core_essential_name_len(targ_n);
+	local_len = bpf_core_essential_name_len(local_n);
+
+	return targ_len == local_len && strncmp(local_n, targ_n, local_len) == 0;
+}
+
+static int bpf_core_enums_match(const struct btf *local_btf, const struct btf_type *local_t,
+				const struct btf *targ_btf, const struct btf_type *targ_t)
+{
+	__u16 local_vlen = btf_vlen(local_t);
+	__u16 targ_vlen = btf_vlen(targ_t);
+	int i, j;
+
+	if (local_t->size != targ_t->size)
+		return 0;
+
+	if (local_vlen > targ_vlen)
+		return 0;
+
+	/* iterate over the local enum's variants and make sure each has
+	 * a symbolic name correspondent in the target
+	 */
+	for (i = 0; i < local_vlen; i++) {
+		bool matched = false;
+		__u32 local_n_off;
+
+		local_n_off = btf_is_enum(local_t) ? btf_enum(local_t)[i].name_off :
+						     btf_enum64(local_t)[i].name_off;
+
+		for (j = 0; j < targ_vlen; j++) {
+			__u32 targ_n_off;
+
+			targ_n_off = btf_is_enum(targ_t) ? btf_enum(targ_t)[j].name_off :
+							   btf_enum64(targ_t)[j].name_off;
+
+			if (bpf_core_names_match(local_btf, local_n_off, targ_btf, targ_n_off)) {
+				matched = true;
+				break;
+			}
+		}
+
+		if (!matched)
+			return 0;
+	}
+	return 1;
+}
+
+static int bpf_core_composites_match(const struct btf *local_btf, const struct btf_type *local_t,
+				     const struct btf *targ_btf, const struct btf_type *targ_t,
+				     bool behind_ptr, int level)
+{
+	const struct btf_member *local_m = btf_members(local_t);
+	__u16 local_vlen = btf_vlen(local_t);
+	__u16 targ_vlen = btf_vlen(targ_t);
+	int i, j, err;
+
+	if (local_vlen > targ_vlen)
+		return 0;
+
+	/* check that all local members have a match in the target */
+	for (i = 0; i < local_vlen; i++, local_m++) {
+		const struct btf_member *targ_m = btf_members(targ_t);
+		bool matched = false;
+
+		for (j = 0; j < targ_vlen; j++, targ_m++) {
+			if (!bpf_core_names_match(local_btf, local_m->name_off, targ_btf,
+						  targ_m->name_off))
+				continue;
+
+			err = __bpf_core_types_match(local_btf, local_m->type, targ_btf,
+						     targ_m->type, behind_ptr, level - 1);
+			if (err > 0) {
+				matched = true;
+				break;
+			}
+		}
+
+		if (!matched)
+			return 0;
+	}
+	return 1;
+}
+
+/* Check that two types "match".
+ *
+ * The matching relation is defined as follows:
+ * - modifiers and typedefs are stripped (and, hence, effectively ignored)
+ * - generally speaking types need to be of same kind (struct vs. struct, union
+ *   vs. union, etc.)
+ *   - exceptions are struct/union behind a pointer which could also match a
+ *     forward declaration of a struct or union, respectively, and enum vs.
+ *     enum64 (see below)
+ * Then, depending on type:
+ * - integers:
+ *   - match if size and signedness match
+ * - arrays & pointers:
+ *   - target types are recursively matched
+ * - structs & unions:
+ *   - local members need to exist in target with the same name
+ *   - for each member we recursively check match unless it is already behind a
+ *     pointer, in which case we only check matching names and compatible kind
+ * - enums:
+ *   - local variants have to have a match in target by symbolic name (but not
+ *     numeric value)
+ *   - size has to match (but enum may match enum64 and vice versa)
+ * - function pointers:
+ *   - number and position of arguments in local type has to match target
+ *   - for each argument and the return value we recursively check match
+ */
+int __bpf_core_types_match(const struct btf *local_btf, __u32 local_id, const struct btf *targ_btf,
+			   __u32 targ_id, bool behind_ptr, int level)
+{
+	const struct btf_type *local_t, *targ_t;
+	int depth = 32; /* max recursion depth */
+	__u16 local_k;
+
+	if (level <= 0)
+		return -EINVAL;
+
+	local_t = btf_type_by_id(local_btf, local_id);
+	targ_t = btf_type_by_id(targ_btf, targ_id);
+
+recur:
+	depth--;
+	if (depth < 0)
+		return -EINVAL;
+
+	local_t = skip_mods_and_typedefs(local_btf, local_id, &local_id);
+	targ_t = skip_mods_and_typedefs(targ_btf, targ_id, &targ_id);
+	if (!local_t || !targ_t)
+		return -EINVAL;
+
+	if (!bpf_core_names_match(local_btf, local_t->name_off, targ_btf, targ_t->name_off))
+		return 0;
+
+	local_k = btf_kind(local_t);
+
+	switch (local_k) {
+	case BTF_KIND_UNKN:
+		return local_k == btf_kind(targ_t);
+	case BTF_KIND_FWD: {
+		bool local_f = BTF_INFO_KFLAG(local_t->info);
+		__u16 targ_k = btf_kind(targ_t);
+
+		if (behind_ptr) {
+			if (local_k == targ_k)
+				return local_f == BTF_INFO_KFLAG(targ_t->info);
+
+			/* for forward declarations kflag dictates whether the
+			 * target is a struct (0) or union (1)
+			 */
+			return (targ_k == BTF_KIND_STRUCT && !local_f) ||
+			       (targ_k == BTF_KIND_UNION && local_f);
+		} else {
+			if (local_k != targ_k)
+				return 0;
+
+			/* match if the forward declaration is for the same kind */
+			return local_f == BTF_INFO_KFLAG(targ_t->info);
+		}
+	}
+	case BTF_KIND_ENUM:
+	case BTF_KIND_ENUM64:
+		if (!btf_is_any_enum(targ_t))
+			return 0;
+
+		return bpf_core_enums_match(local_btf, local_t, targ_btf, targ_t);
+	case BTF_KIND_STRUCT:
+	case BTF_KIND_UNION: {
+		__u16 targ_k = btf_kind(targ_t);
+
+		if (behind_ptr) {
+			bool targ_f = BTF_INFO_KFLAG(targ_t->info);
+
+			if (local_k == targ_k)
+				return 1;
+
+			if (targ_k != BTF_KIND_FWD)
+				return 0;
+
+			return (local_k == BTF_KIND_UNION) == targ_f;
+		} else {
+			if (local_k != targ_k)
+				return 0;
+
+			return bpf_core_composites_match(local_btf, local_t, targ_btf, targ_t,
+							 behind_ptr, level);
+		}
+	}
+	case BTF_KIND_INT: {
+		__u8 local_sgn;
+		__u8 targ_sgn;
+
+		if (local_k != btf_kind(targ_t))
+			return 0;
+
+		local_sgn = btf_int_encoding(local_t) & BTF_INT_SIGNED;
+		targ_sgn = btf_int_encoding(targ_t) & BTF_INT_SIGNED;
+
+		return btf_int_bits(local_t) == btf_int_bits(targ_t) && local_sgn == targ_sgn;
+	}
+	case BTF_KIND_PTR:
+		if (local_k != btf_kind(targ_t))
+			return 0;
+
+		behind_ptr = true;
+
+		local_id = local_t->type;
+		targ_id = targ_t->type;
+		goto recur;
+	case BTF_KIND_ARRAY: {
+		const struct btf_array *local_array = btf_array(local_t);
+		const struct btf_array *targ_array = btf_array(targ_t);
+
+		if (local_k != btf_kind(targ_t))
+			return 0;
+
+		if (local_array->nelems != targ_array->nelems)
+			return 0;
+
+		local_id = local_array->type;
+		targ_id = targ_array->type;
+		goto recur;
+	}
+	case BTF_KIND_FUNC_PROTO: {
+		struct btf_param *local_p = btf_params(local_t);
+		struct btf_param *targ_p = btf_params(targ_t);
+		__u16 local_vlen = btf_vlen(local_t);
+		__u16 targ_vlen = btf_vlen(targ_t);
+		int i, err;
+
+		if (local_k != btf_kind(targ_t))
+			return 0;
+
+		if (local_vlen != targ_vlen)
+			return 0;
+
+		for (i = 0; i < local_vlen; i++, local_p++, targ_p++) {
+			err = __bpf_core_types_match(local_btf, local_p->type, targ_btf,
+						     targ_p->type, behind_ptr, level - 1);
+			if (err <= 0)
+				return err;
+		}
+
+		/* tail recurse for return type check */
+		local_id = local_t->type;
+		targ_id = targ_t->type;
+		goto recur;
+	}
+	default:
+		pr_warn("unexpected kind %s relocated, local [%d], target [%d]\n",
+			btf_kind_str(local_t), local_id, targ_id);
+		return 0;
+	}
+}
diff --git a/tools/lib/bpf/relo_core.h b/tools/lib/bpf/relo_core.h
index 3fd384..8462e0a 100644
--- a/tools/lib/bpf/relo_core.h
+++ b/tools/lib/bpf/relo_core.h
@@ -72,6 +72,8 @@ int __bpf_core_types_are_compat(const struct btf *local_btf, __u32 local_id,
 				const struct btf *targ_btf, __u32 targ_id, int level);
 int bpf_core_types_are_compat(const struct btf *local_btf, __u32 local_id,
 			      const struct btf *targ_btf, __u32 targ_id);
+int __bpf_core_types_match(const struct btf *local_btf, __u32 local_id, const struct btf *targ_btf,
+			   __u32 targ_id, bool behind_ptr, int level);
 
 size_t bpf_core_essential_name_len(const char *name);
 
-- 
2.30.2


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

* [PATCH bpf-next v3 05/10] bpf: Add type match support
  2022-06-28 16:01 [PATCH bpf-next v3 00/10] Introduce type match support Daniel Müller
                   ` (3 preceding siblings ...)
  2022-06-28 16:01 ` [PATCH bpf-next v3 04/10] libbpf: Add type match support Daniel Müller
@ 2022-06-28 16:01 ` Daniel Müller
  2022-06-28 16:01 ` [PATCH bpf-next v3 06/10] libbpf: Honor TYPE_MATCH relocation Daniel Müller
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Daniel Müller @ 2022-06-28 16:01 UTC (permalink / raw)
  To: bpf, ast, andrii, daniel, kernel-team; +Cc: joannelkoong

This change implements the kernel side of the "type matches" support,
just calling the previously added core logic in relo_core.c.

Signed-off-by: Daniel Müller <deso@posteo.net>
---
 kernel/bpf/btf.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 2e2066..8923b5 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -7442,6 +7442,15 @@ int bpf_core_types_are_compat(const struct btf *local_btf, __u32 local_id,
 					   MAX_TYPES_ARE_COMPAT_DEPTH);
 }
 
+#define MAX_TYPES_MATCH_DEPTH 2
+
+int bpf_core_types_match(const struct btf *local_btf, u32 local_id,
+			 const struct btf *targ_btf, u32 targ_id)
+{
+	return __bpf_core_types_match(local_btf, local_id, targ_btf, targ_id, false,
+				      MAX_TYPES_MATCH_DEPTH);
+}
+
 static bool bpf_core_is_flavor_sep(const char *s)
 {
 	/* check X___Y name pattern, where X and Y are not underscores */
-- 
2.30.2


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

* [PATCH bpf-next v3 06/10] libbpf: Honor TYPE_MATCH relocation
  2022-06-28 16:01 [PATCH bpf-next v3 00/10] Introduce type match support Daniel Müller
                   ` (4 preceding siblings ...)
  2022-06-28 16:01 ` [PATCH bpf-next v3 05/10] bpf: " Daniel Müller
@ 2022-06-28 16:01 ` Daniel Müller
  2022-06-28 16:01 ` [PATCH bpf-next v3 07/10] selftests/bpf: Add type-match checks to type-based tests Daniel Müller
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Daniel Müller @ 2022-06-28 16:01 UTC (permalink / raw)
  To: bpf, ast, andrii, daniel, kernel-team; +Cc: joannelkoong

This patch finalizes support for the proposed type match relation in
libbpf by hooking it up to the TYPE_MATCH relocation. For this
relocation to be handled correctly by LLVM we have D126838
(https://reviews.llvm.org/D126838).
The main functionality is present behind the newly introduced
bpf_core_type_matches macro (similar to bpf_core_type_exists). This
macro can be used to check whether a local type has a "match" in a
target BTF.

Signed-off-by: Daniel Müller <deso@posteo.net>
---
 tools/lib/bpf/bpf_core_read.h | 10 ++++++++++
 tools/lib/bpf/libbpf.c        |  6 ++++++
 tools/lib/bpf/relo_core.c     | 16 ++++++++++++----
 tools/lib/bpf/relo_core.h     |  2 ++
 4 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/tools/lib/bpf/bpf_core_read.h b/tools/lib/bpf/bpf_core_read.h
index 2308f49..496e6a 100644
--- a/tools/lib/bpf/bpf_core_read.h
+++ b/tools/lib/bpf/bpf_core_read.h
@@ -184,6 +184,16 @@ enum bpf_enum_value_kind {
 #define bpf_core_type_exists(type)					    \
 	__builtin_preserve_type_info(*(typeof(type) *)0, BPF_TYPE_EXISTS)
 
+/*
+ * Convenience macro to check that provided named type
+ * (struct/union/enum/typedef) "matches" that in a target kernel.
+ * Returns:
+ *    1, if the type matches in the target kernel's BTF;
+ *    0, if the type does not match any in the target kernel
+ */
+#define bpf_core_type_matches(type)					    \
+	__builtin_preserve_type_info(*(typeof(type) *)0, BPF_TYPE_MATCHES)
+
 /*
  * Convenience macro to get the byte size of a provided named type
  * (struct/union/enum/typedef) in a target kernel.
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 335467..92fc399 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -5735,6 +5735,12 @@ int bpf_core_types_are_compat(const struct btf *local_btf, __u32 local_id,
 	return __bpf_core_types_are_compat(local_btf, local_id, targ_btf, targ_id, 32);
 }
 
+int bpf_core_types_match(const struct btf *local_btf, __u32 local_id,
+			 const struct btf *targ_btf, __u32 targ_id)
+{
+	return __bpf_core_types_match(local_btf, local_id, targ_btf, targ_id, false, 32);
+}
+
 static size_t bpf_core_hash_fn(const void *key, void *ctx)
 {
 	return (size_t)key;
diff --git a/tools/lib/bpf/relo_core.c b/tools/lib/bpf/relo_core.c
index b3f5d7e..5f1294c7 100644
--- a/tools/lib/bpf/relo_core.c
+++ b/tools/lib/bpf/relo_core.c
@@ -95,6 +95,7 @@ static const char *core_relo_kind_str(enum bpf_core_relo_kind kind)
 	case BPF_CORE_TYPE_ID_LOCAL: return "local_type_id";
 	case BPF_CORE_TYPE_ID_TARGET: return "target_type_id";
 	case BPF_CORE_TYPE_EXISTS: return "type_exists";
+	case BPF_CORE_TYPE_MATCHES: return "type_matches";
 	case BPF_CORE_TYPE_SIZE: return "type_size";
 	case BPF_CORE_ENUMVAL_EXISTS: return "enumval_exists";
 	case BPF_CORE_ENUMVAL_VALUE: return "enumval_value";
@@ -123,6 +124,7 @@ static bool core_relo_is_type_based(enum bpf_core_relo_kind kind)
 	case BPF_CORE_TYPE_ID_LOCAL:
 	case BPF_CORE_TYPE_ID_TARGET:
 	case BPF_CORE_TYPE_EXISTS:
+	case BPF_CORE_TYPE_MATCHES:
 	case BPF_CORE_TYPE_SIZE:
 		return true;
 	default:
@@ -251,7 +253,7 @@ int __bpf_core_types_are_compat(const struct btf *local_btf, __u32 local_id,
  *   - field 'a' access (corresponds to '2' in low-level spec);
  *   - array element #3 access (corresponds to '3' in low-level spec).
  *
- * Type-based relocations (TYPE_EXISTS/TYPE_SIZE,
+ * Type-based relocations (TYPE_EXISTS/TYPE_MATCHES/TYPE_SIZE,
  * TYPE_ID_LOCAL/TYPE_ID_TARGET) don't capture any field information. Their
  * spec and raw_spec are kept empty.
  *
@@ -568,9 +570,14 @@ static int bpf_core_spec_match(struct bpf_core_spec *local_spec,
 	targ_spec->relo_kind = local_spec->relo_kind;
 
 	if (core_relo_is_type_based(local_spec->relo_kind)) {
-		return bpf_core_types_are_compat(local_spec->btf,
-						 local_spec->root_type_id,
-						 targ_btf, targ_id);
+		if (local_spec->relo_kind == BPF_CORE_TYPE_MATCHES)
+			return bpf_core_types_match(local_spec->btf,
+						    local_spec->root_type_id,
+						    targ_btf, targ_id);
+		else
+			return bpf_core_types_are_compat(local_spec->btf,
+							 local_spec->root_type_id,
+							 targ_btf, targ_id);
 	}
 
 	local_acc = &local_spec->spec[0];
@@ -819,6 +826,7 @@ static int bpf_core_calc_type_relo(const struct bpf_core_relo *relo,
 			*validate = false;
 		break;
 	case BPF_CORE_TYPE_EXISTS:
+	case BPF_CORE_TYPE_MATCHES:
 		*val = 1;
 		break;
 	case BPF_CORE_TYPE_SIZE:
diff --git a/tools/lib/bpf/relo_core.h b/tools/lib/bpf/relo_core.h
index 8462e0a..1c0566d 100644
--- a/tools/lib/bpf/relo_core.h
+++ b/tools/lib/bpf/relo_core.h
@@ -74,6 +74,8 @@ int bpf_core_types_are_compat(const struct btf *local_btf, __u32 local_id,
 			      const struct btf *targ_btf, __u32 targ_id);
 int __bpf_core_types_match(const struct btf *local_btf, __u32 local_id, const struct btf *targ_btf,
 			   __u32 targ_id, bool behind_ptr, int level);
+int bpf_core_types_match(const struct btf *local_btf, __u32 local_id, const struct btf *targ_btf,
+			 __u32 targ_id);
 
 size_t bpf_core_essential_name_len(const char *name);
 
-- 
2.30.2


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

* [PATCH bpf-next v3 07/10] selftests/bpf: Add type-match checks to type-based tests
  2022-06-28 16:01 [PATCH bpf-next v3 00/10] Introduce type match support Daniel Müller
                   ` (5 preceding siblings ...)
  2022-06-28 16:01 ` [PATCH bpf-next v3 06/10] libbpf: Honor TYPE_MATCH relocation Daniel Müller
@ 2022-06-28 16:01 ` Daniel Müller
  2022-06-28 16:01 ` [PATCH bpf-next v3 08/10] selftests/bpf: Add test checking more characteristics Daniel Müller
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Daniel Müller @ 2022-06-28 16:01 UTC (permalink / raw)
  To: bpf, ast, andrii, daniel, kernel-team; +Cc: joannelkoong

Now that we have type-match logic in both libbpf and the kernel, this
change adjusts the existing BPF self tests to check this functionality.
Specifically, we extend the existing type-based tests to check the
previously introduced bpf_core_type_matches macro.

Signed-off-by: Daniel Müller <deso@posteo.net>
---
 .../selftests/bpf/prog_tests/core_reloc.c     | 31 ++++++++++++++++--
 .../selftests/bpf/progs/core_reloc_types.h    | 14 +++++++-
 .../bpf/progs/test_core_reloc_type_based.c    | 32 ++++++++++++++++++-
 3 files changed, 73 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
index 2f92fe..328dd7 100644
--- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c
+++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
@@ -543,7 +543,6 @@ static int __trigger_module_test_read(const struct core_reloc_test_case *test)
 	return 0;
 }
 
-
 static const struct core_reloc_test_case test_cases[] = {
 	/* validate we can find kernel image and use its BTF for relocs */
 	{
@@ -752,7 +751,7 @@ static const struct core_reloc_test_case test_cases[] = {
 	SIZE_CASE(size___diff_offs),
 	SIZE_ERR_CASE(size___err_ambiguous),
 
-	/* validate type existence and size relocations */
+	/* validate type existence, match, and size relocations */
 	TYPE_BASED_CASE(type_based, {
 		.struct_exists = 1,
 		.union_exists = 1,
@@ -765,6 +764,19 @@ static const struct core_reloc_test_case test_cases[] = {
 		.typedef_void_ptr_exists = 1,
 		.typedef_func_proto_exists = 1,
 		.typedef_arr_exists = 1,
+
+		.struct_matches = 1,
+		.union_matches = 1,
+		.enum_matches = 1,
+		.typedef_named_struct_matches = 1,
+		.typedef_anon_struct_matches = 1,
+		.typedef_struct_ptr_matches = 1,
+		.typedef_int_matches = 1,
+		.typedef_enum_matches = 1,
+		.typedef_void_ptr_matches = 1,
+		.typedef_func_proto_matches = 1,
+		.typedef_arr_matches = 1,
+
 		.struct_sz = sizeof(struct a_struct),
 		.union_sz = sizeof(union a_union),
 		.enum_sz = sizeof(enum an_enum),
@@ -792,6 +804,19 @@ static const struct core_reloc_test_case test_cases[] = {
 		.typedef_void_ptr_exists = 1,
 		.typedef_func_proto_exists = 1,
 		.typedef_arr_exists = 1,
+
+		.struct_matches = 0,
+		.union_matches = 0,
+		.enum_matches = 0,
+		.typedef_named_struct_matches = 0,
+		.typedef_anon_struct_matches = 0,
+		.typedef_struct_ptr_matches = 1,
+		.typedef_int_matches = 0,
+		.typedef_enum_matches = 0,
+		.typedef_void_ptr_matches = 1,
+		.typedef_func_proto_matches = 0,
+		.typedef_arr_matches = 0,
+
 		.struct_sz = sizeof(struct a_struct___diff_sz),
 		.union_sz = sizeof(union a_union___diff_sz),
 		.enum_sz = sizeof(enum an_enum___diff_sz),
@@ -806,10 +831,12 @@ static const struct core_reloc_test_case test_cases[] = {
 	}),
 	TYPE_BASED_CASE(type_based___incompat, {
 		.enum_exists = 1,
+		.enum_matches = 1,
 		.enum_sz = sizeof(enum an_enum),
 	}),
 	TYPE_BASED_CASE(type_based___fn_wrong_args, {
 		.struct_exists = 1,
+		.struct_matches = 1,
 		.struct_sz = sizeof(struct a_struct),
 	}),
 
diff --git a/tools/testing/selftests/bpf/progs/core_reloc_types.h b/tools/testing/selftests/bpf/progs/core_reloc_types.h
index 26e1033..6a44f3e 100644
--- a/tools/testing/selftests/bpf/progs/core_reloc_types.h
+++ b/tools/testing/selftests/bpf/progs/core_reloc_types.h
@@ -860,7 +860,7 @@ struct core_reloc_size___err_ambiguous2 {
 };
 
 /*
- * TYPE EXISTENCE & SIZE
+ * TYPE EXISTENCE, MATCH & SIZE
  */
 struct core_reloc_type_based_output {
 	bool struct_exists;
@@ -875,6 +875,18 @@ struct core_reloc_type_based_output {
 	bool typedef_func_proto_exists;
 	bool typedef_arr_exists;
 
+	bool struct_matches;
+	bool union_matches;
+	bool enum_matches;
+	bool typedef_named_struct_matches;
+	bool typedef_anon_struct_matches;
+	bool typedef_struct_ptr_matches;
+	bool typedef_int_matches;
+	bool typedef_enum_matches;
+	bool typedef_void_ptr_matches;
+	bool typedef_func_proto_matches;
+	bool typedef_arr_matches;
+
 	int struct_sz;
 	int union_sz;
 	int enum_sz;
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_type_based.c b/tools/testing/selftests/bpf/progs/test_core_reloc_type_based.c
index fb60f8..325ead 100644
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_type_based.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_type_based.c
@@ -61,6 +61,18 @@ struct core_reloc_type_based_output {
 	bool typedef_func_proto_exists;
 	bool typedef_arr_exists;
 
+	bool struct_matches;
+	bool union_matches;
+	bool enum_matches;
+	bool typedef_named_struct_matches;
+	bool typedef_anon_struct_matches;
+	bool typedef_struct_ptr_matches;
+	bool typedef_int_matches;
+	bool typedef_enum_matches;
+	bool typedef_void_ptr_matches;
+	bool typedef_func_proto_matches;
+	bool typedef_arr_matches;
+
 	int struct_sz;
 	int union_sz;
 	int enum_sz;
@@ -77,7 +89,13 @@ struct core_reloc_type_based_output {
 SEC("raw_tracepoint/sys_enter")
 int test_core_type_based(void *ctx)
 {
-#if __has_builtin(__builtin_preserve_type_info)
+	/* Support for the BPF_TYPE_MATCHES argument to the
+	 * __builtin_preserve_type_info builtin was added at some point during
+	 * development of clang 15 and it's what we require for this test. Part of it
+	 * could run with merely __builtin_preserve_type_info (which could be checked
+	 * separately), but we have to find an upper bound.
+	 */
+#if __has_builtin(__builtin_preserve_type_info) && __clang_major__ >= 15
 	struct core_reloc_type_based_output *out = (void *)&data.out;
 
 	out->struct_exists = bpf_core_type_exists(struct a_struct);
@@ -92,6 +110,18 @@ int test_core_type_based(void *ctx)
 	out->typedef_func_proto_exists = bpf_core_type_exists(func_proto_typedef);
 	out->typedef_arr_exists = bpf_core_type_exists(arr_typedef);
 
+	out->struct_matches = bpf_core_type_matches(struct a_struct);
+	out->union_matches = bpf_core_type_matches(union a_union);
+	out->enum_matches = bpf_core_type_matches(enum an_enum);
+	out->typedef_named_struct_matches = bpf_core_type_matches(named_struct_typedef);
+	out->typedef_anon_struct_matches = bpf_core_type_matches(anon_struct_typedef);
+	out->typedef_struct_ptr_matches = bpf_core_type_matches(struct_ptr_typedef);
+	out->typedef_int_matches = bpf_core_type_matches(int_typedef);
+	out->typedef_enum_matches = bpf_core_type_matches(enum_typedef);
+	out->typedef_void_ptr_matches = bpf_core_type_matches(void_ptr_typedef);
+	out->typedef_func_proto_matches = bpf_core_type_matches(func_proto_typedef);
+	out->typedef_arr_matches = bpf_core_type_matches(arr_typedef);
+
 	out->struct_sz = bpf_core_type_size(struct a_struct);
 	out->union_sz = bpf_core_type_size(union a_union);
 	out->enum_sz = bpf_core_type_size(enum an_enum);
-- 
2.30.2


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

* [PATCH bpf-next v3 08/10] selftests/bpf: Add test checking more characteristics
  2022-06-28 16:01 [PATCH bpf-next v3 00/10] Introduce type match support Daniel Müller
                   ` (6 preceding siblings ...)
  2022-06-28 16:01 ` [PATCH bpf-next v3 07/10] selftests/bpf: Add type-match checks to type-based tests Daniel Müller
@ 2022-06-28 16:01 ` Daniel Müller
  2022-06-28 16:01 ` [PATCH bpf-next v3 09/10] selftests/bpf: Add nested type to type based tests Daniel Müller
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Daniel Müller @ 2022-06-28 16:01 UTC (permalink / raw)
  To: bpf, ast, andrii, daniel, kernel-team; +Cc: joannelkoong

This change adds another type-based self-test that specifically aims to
test some more characteristics of the TYPE_MATCH logic. Specifically, it
covers a few more potential differences between types, such as different
orders, enum variant values, and integer signedness.

Signed-off-by: Daniel Müller <deso@posteo.net>
---
 .../selftests/bpf/prog_tests/core_reloc.c     | 37 ++++++++++++++
 .../progs/btf__core_reloc_type_based___diff.c |  3 ++
 .../selftests/bpf/progs/core_reloc_types.h    | 51 +++++++++++++++++++
 3 files changed, 91 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_type_based___diff.c

diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
index 328dd7..eb47bf 100644
--- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c
+++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
@@ -792,6 +792,43 @@ static const struct core_reloc_test_case test_cases[] = {
 	TYPE_BASED_CASE(type_based___all_missing, {
 		/* all zeros */
 	}),
+	TYPE_BASED_CASE(type_based___diff, {
+		.struct_exists = 1,
+		.union_exists = 1,
+		.enum_exists = 1,
+		.typedef_named_struct_exists = 1,
+		.typedef_anon_struct_exists = 1,
+		.typedef_struct_ptr_exists = 1,
+		.typedef_int_exists = 1,
+		.typedef_enum_exists = 1,
+		.typedef_void_ptr_exists = 1,
+		.typedef_func_proto_exists = 1,
+		.typedef_arr_exists = 1,
+
+		.struct_matches = 1,
+		.union_matches = 1,
+		.enum_matches = 1,
+		.typedef_named_struct_matches = 1,
+		.typedef_anon_struct_matches = 1,
+		.typedef_struct_ptr_matches = 1,
+		.typedef_int_matches = 0,
+		.typedef_enum_matches = 1,
+		.typedef_void_ptr_matches = 1,
+		.typedef_func_proto_matches = 0,
+		.typedef_arr_matches = 0,
+
+		.struct_sz = sizeof(struct a_struct___diff),
+		.union_sz = sizeof(union a_union___diff),
+		.enum_sz = sizeof(enum an_enum___diff),
+		.typedef_named_struct_sz = sizeof(named_struct_typedef___diff),
+		.typedef_anon_struct_sz = sizeof(anon_struct_typedef___diff),
+		.typedef_struct_ptr_sz = sizeof(struct_ptr_typedef___diff),
+		.typedef_int_sz = sizeof(int_typedef___diff),
+		.typedef_enum_sz = sizeof(enum_typedef___diff),
+		.typedef_void_ptr_sz = sizeof(void_ptr_typedef___diff),
+		.typedef_func_proto_sz = sizeof(func_proto_typedef___diff),
+		.typedef_arr_sz = sizeof(arr_typedef___diff),
+	}),
 	TYPE_BASED_CASE(type_based___diff_sz, {
 		.struct_exists = 1,
 		.union_exists = 1,
diff --git a/tools/testing/selftests/bpf/progs/btf__core_reloc_type_based___diff.c b/tools/testing/selftests/bpf/progs/btf__core_reloc_type_based___diff.c
new file mode 100644
index 0000000..57ae2c
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/btf__core_reloc_type_based___diff.c
@@ -0,0 +1,3 @@
+#include "core_reloc_types.h"
+
+void f(struct core_reloc_type_based___diff x) {}
diff --git a/tools/testing/selftests/bpf/progs/core_reloc_types.h b/tools/testing/selftests/bpf/progs/core_reloc_types.h
index 6a44f3e..e326b6 100644
--- a/tools/testing/selftests/bpf/progs/core_reloc_types.h
+++ b/tools/testing/selftests/bpf/progs/core_reloc_types.h
@@ -951,6 +951,57 @@ struct core_reloc_type_based {
 struct core_reloc_type_based___all_missing {
 };
 
+/* different member orders, enum variant values, signedness, etc */
+struct a_struct___diff {
+	int x;
+	int a;
+};
+
+union a_union___diff {
+	int z;
+	int y;
+};
+
+typedef struct a_struct___diff named_struct_typedef___diff;
+
+typedef struct { int z, x, y; } anon_struct_typedef___diff;
+
+typedef struct {
+	int c;
+	int b;
+	int a;
+} *struct_ptr_typedef___diff;
+
+enum an_enum___diff {
+	AN_ENUM_VAL2___diff = 0,
+	AN_ENUM_VAL1___diff = 42,
+	AN_ENUM_VAL3___diff = 1,
+};
+
+typedef unsigned int int_typedef___diff;
+
+typedef enum { TYPEDEF_ENUM_VAL2___diff, TYPEDEF_ENUM_VAL1___diff = 50 } enum_typedef___diff;
+
+typedef const void *void_ptr_typedef___diff;
+
+typedef int_typedef___diff (*func_proto_typedef___diff)(long);
+
+typedef char arr_typedef___diff[3];
+
+struct core_reloc_type_based___diff {
+	struct a_struct___diff f1;
+	union a_union___diff f2;
+	enum an_enum___diff f3;
+	named_struct_typedef___diff f4;
+	anon_struct_typedef___diff f5;
+	struct_ptr_typedef___diff f6;
+	int_typedef___diff f7;
+	enum_typedef___diff f8;
+	void_ptr_typedef___diff f9;
+	func_proto_typedef___diff f10;
+	arr_typedef___diff f11;
+};
+
 /* different type sizes, extra modifiers, anon vs named enums, etc */
 struct a_struct___diff_sz {
 	long x;
-- 
2.30.2


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

* [PATCH bpf-next v3 09/10] selftests/bpf: Add nested type to type based tests
  2022-06-28 16:01 [PATCH bpf-next v3 00/10] Introduce type match support Daniel Müller
                   ` (7 preceding siblings ...)
  2022-06-28 16:01 ` [PATCH bpf-next v3 08/10] selftests/bpf: Add test checking more characteristics Daniel Müller
@ 2022-06-28 16:01 ` Daniel Müller
  2022-06-28 16:01 ` [PATCH bpf-next v3 10/10] selftests/bpf: Add type match test against kernel's task_struct Daniel Müller
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Daniel Müller @ 2022-06-28 16:01 UTC (permalink / raw)
  To: bpf, ast, andrii, daniel, kernel-team; +Cc: joannelkoong

This change extends the type based tests with another struct type (in
addition to a_struct) to check relocations against: a_complex_struct.
This type is nested more deeply to provide additional coverage of
certain paths in the type match logic.

Signed-off-by: Daniel Müller <deso@posteo.net>
---
 .../selftests/bpf/prog_tests/core_reloc.c     |  4 ++
 .../selftests/bpf/progs/core_reloc_types.h    | 62 +++++++++++++------
 .../bpf/progs/test_core_reloc_type_based.c    | 12 ++++
 3 files changed, 58 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
index eb47bf..8882c9c 100644
--- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c
+++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
@@ -754,6 +754,7 @@ static const struct core_reloc_test_case test_cases[] = {
 	/* validate type existence, match, and size relocations */
 	TYPE_BASED_CASE(type_based, {
 		.struct_exists = 1,
+		.complex_struct_exists = 1,
 		.union_exists = 1,
 		.enum_exists = 1,
 		.typedef_named_struct_exists = 1,
@@ -766,6 +767,7 @@ static const struct core_reloc_test_case test_cases[] = {
 		.typedef_arr_exists = 1,
 
 		.struct_matches = 1,
+		.complex_struct_matches = 1,
 		.union_matches = 1,
 		.enum_matches = 1,
 		.typedef_named_struct_matches = 1,
@@ -794,6 +796,7 @@ static const struct core_reloc_test_case test_cases[] = {
 	}),
 	TYPE_BASED_CASE(type_based___diff, {
 		.struct_exists = 1,
+		.complex_struct_exists = 1,
 		.union_exists = 1,
 		.enum_exists = 1,
 		.typedef_named_struct_exists = 1,
@@ -806,6 +809,7 @@ static const struct core_reloc_test_case test_cases[] = {
 		.typedef_arr_exists = 1,
 
 		.struct_matches = 1,
+		.complex_struct_matches = 1,
 		.union_matches = 1,
 		.enum_matches = 1,
 		.typedef_named_struct_matches = 1,
diff --git a/tools/testing/selftests/bpf/progs/core_reloc_types.h b/tools/testing/selftests/bpf/progs/core_reloc_types.h
index e326b6..474411 100644
--- a/tools/testing/selftests/bpf/progs/core_reloc_types.h
+++ b/tools/testing/selftests/bpf/progs/core_reloc_types.h
@@ -864,6 +864,7 @@ struct core_reloc_size___err_ambiguous2 {
  */
 struct core_reloc_type_based_output {
 	bool struct_exists;
+	bool complex_struct_exists;
 	bool union_exists;
 	bool enum_exists;
 	bool typedef_named_struct_exists;
@@ -876,6 +877,7 @@ struct core_reloc_type_based_output {
 	bool typedef_arr_exists;
 
 	bool struct_matches;
+	bool complex_struct_matches;
 	bool union_matches;
 	bool enum_matches;
 	bool typedef_named_struct_matches;
@@ -904,6 +906,14 @@ struct a_struct {
 	int x;
 };
 
+struct a_complex_struct {
+	union {
+		struct a_struct * restrict a;
+		void *b;
+	} x;
+	volatile long y;
+};
+
 union a_union {
 	int y;
 	int z;
@@ -935,16 +945,17 @@ typedef char arr_typedef[20];
 
 struct core_reloc_type_based {
 	struct a_struct f1;
-	union a_union f2;
-	enum an_enum f3;
-	named_struct_typedef f4;
-	anon_struct_typedef f5;
-	struct_ptr_typedef f6;
-	int_typedef f7;
-	enum_typedef f8;
-	void_ptr_typedef f9;
-	func_proto_typedef f10;
-	arr_typedef f11;
+	struct a_complex_struct f2;
+	union a_union f3;
+	enum an_enum f4;
+	named_struct_typedef f5;
+	anon_struct_typedef f6;
+	struct_ptr_typedef f7;
+	int_typedef f8;
+	enum_typedef f9;
+	void_ptr_typedef f10;
+	func_proto_typedef f11;
+	arr_typedef f12;
 };
 
 /* no types in target */
@@ -957,6 +968,16 @@ struct a_struct___diff {
 	int a;
 };
 
+struct a_struct___forward;
+
+struct a_complex_struct___diff {
+	union {
+		struct a_struct___forward *a;
+		void *b;
+	} x;
+	volatile long y;
+};
+
 union a_union___diff {
 	int z;
 	int y;
@@ -990,16 +1011,17 @@ typedef char arr_typedef___diff[3];
 
 struct core_reloc_type_based___diff {
 	struct a_struct___diff f1;
-	union a_union___diff f2;
-	enum an_enum___diff f3;
-	named_struct_typedef___diff f4;
-	anon_struct_typedef___diff f5;
-	struct_ptr_typedef___diff f6;
-	int_typedef___diff f7;
-	enum_typedef___diff f8;
-	void_ptr_typedef___diff f9;
-	func_proto_typedef___diff f10;
-	arr_typedef___diff f11;
+	struct a_complex_struct___diff f2;
+	union a_union___diff f3;
+	enum an_enum___diff f4;
+	named_struct_typedef___diff f5;
+	anon_struct_typedef___diff f6;
+	struct_ptr_typedef___diff f7;
+	int_typedef___diff f8;
+	enum_typedef___diff f9;
+	void_ptr_typedef___diff f10;
+	func_proto_typedef___diff f11;
+	arr_typedef___diff f12;
 };
 
 /* different type sizes, extra modifiers, anon vs named enums, etc */
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_type_based.c b/tools/testing/selftests/bpf/progs/test_core_reloc_type_based.c
index 325ead..d95bc08 100644
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_type_based.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_type_based.c
@@ -19,6 +19,14 @@ struct a_struct {
 	int x;
 };
 
+struct a_complex_struct {
+	union {
+		struct a_struct *a;
+		void *b;
+	} x;
+	volatile long y;
+};
+
 union a_union {
 	int y;
 	int z;
@@ -50,6 +58,7 @@ typedef char arr_typedef[20];
 
 struct core_reloc_type_based_output {
 	bool struct_exists;
+	bool complex_struct_exists;
 	bool union_exists;
 	bool enum_exists;
 	bool typedef_named_struct_exists;
@@ -62,6 +71,7 @@ struct core_reloc_type_based_output {
 	bool typedef_arr_exists;
 
 	bool struct_matches;
+	bool complex_struct_matches;
 	bool union_matches;
 	bool enum_matches;
 	bool typedef_named_struct_matches;
@@ -99,6 +109,7 @@ int test_core_type_based(void *ctx)
 	struct core_reloc_type_based_output *out = (void *)&data.out;
 
 	out->struct_exists = bpf_core_type_exists(struct a_struct);
+	out->complex_struct_exists = bpf_core_type_exists(struct a_complex_struct);
 	out->union_exists = bpf_core_type_exists(union a_union);
 	out->enum_exists = bpf_core_type_exists(enum an_enum);
 	out->typedef_named_struct_exists = bpf_core_type_exists(named_struct_typedef);
@@ -111,6 +122,7 @@ int test_core_type_based(void *ctx)
 	out->typedef_arr_exists = bpf_core_type_exists(arr_typedef);
 
 	out->struct_matches = bpf_core_type_matches(struct a_struct);
+	out->complex_struct_matches = bpf_core_type_matches(struct a_complex_struct);
 	out->union_matches = bpf_core_type_matches(union a_union);
 	out->enum_matches = bpf_core_type_matches(enum an_enum);
 	out->typedef_named_struct_matches = bpf_core_type_matches(named_struct_typedef);
-- 
2.30.2


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

* [PATCH bpf-next v3 10/10] selftests/bpf: Add type match test against kernel's task_struct
  2022-06-28 16:01 [PATCH bpf-next v3 00/10] Introduce type match support Daniel Müller
                   ` (8 preceding siblings ...)
  2022-06-28 16:01 ` [PATCH bpf-next v3 09/10] selftests/bpf: Add nested type to type based tests Daniel Müller
@ 2022-06-28 16:01 ` Daniel Müller
  2022-07-05 21:07 ` [PATCH bpf-next v3 00/10] Introduce type match support Daniel Müller
  2022-07-06  4:20 ` patchwork-bot+netdevbpf
  11 siblings, 0 replies; 18+ messages in thread
From: Daniel Müller @ 2022-06-28 16:01 UTC (permalink / raw)
  To: bpf, ast, andrii, daniel, kernel-team; +Cc: joannelkoong

This change extends the existing core_reloc/kernel test to include a
type match check of a local task_struct against the kernel's definition
-- which we assume to succeed.

Signed-off-by: Daniel Müller <deso@posteo.net>
---
 .../selftests/bpf/prog_tests/core_reloc.c     |  1 +
 .../selftests/bpf/progs/core_reloc_types.h    |  1 +
 .../bpf/progs/test_core_reloc_kernel.c        | 19 +++++++++++++++++++
 3 files changed, 21 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
index 8882c9c..a6f65e2 100644
--- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c
+++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
@@ -555,6 +555,7 @@ static const struct core_reloc_test_case test_cases[] = {
 			.valid = { 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, },
 			.comm = "test_progs",
 			.comm_len = sizeof("test_progs"),
+			.local_task_struct_matches = true,
 		},
 		.output_len = sizeof(struct core_reloc_kernel_output),
 		.raw_tp_name = "sys_enter",
diff --git a/tools/testing/selftests/bpf/progs/core_reloc_types.h b/tools/testing/selftests/bpf/progs/core_reloc_types.h
index 474411..7ef91d 100644
--- a/tools/testing/selftests/bpf/progs/core_reloc_types.h
+++ b/tools/testing/selftests/bpf/progs/core_reloc_types.h
@@ -13,6 +13,7 @@ struct core_reloc_kernel_output {
 	int valid[10];
 	char comm[sizeof("test_progs")];
 	int comm_len;
+	bool local_task_struct_matches;
 };
 
 /*
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
index 145028..a17dd8 100644
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_kernel.c
@@ -21,6 +21,7 @@ struct core_reloc_kernel_output {
 	/* we have test_progs[-flavor], so cut flavor part */
 	char comm[sizeof("test_progs")];
 	int comm_len;
+	bool local_task_struct_matches;
 };
 
 struct task_struct {
@@ -30,11 +31,25 @@ struct task_struct {
 	struct task_struct *group_leader;
 };
 
+struct mm_struct___wrong {
+    int abc_whatever_should_not_exist;
+};
+
+struct task_struct___local {
+    int pid;
+    struct mm_struct___wrong *mm;
+};
+
 #define CORE_READ(dst, src) bpf_core_read(dst, sizeof(*(dst)), src)
 
 SEC("raw_tracepoint/sys_enter")
 int test_core_kernel(void *ctx)
 {
+	/* Support for the BPF_TYPE_MATCHES argument to the
+	 * __builtin_preserve_type_info builtin was added at some point during
+	 * development of clang 15 and it's what we require for this test.
+	 */
+#if __has_builtin(__builtin_preserve_type_info) && __clang_major__ >= 15
 	struct task_struct *task = (void *)bpf_get_current_task();
 	struct core_reloc_kernel_output *out = (void *)&data.out;
 	uint64_t pid_tgid = bpf_get_current_pid_tgid();
@@ -93,6 +108,10 @@ int test_core_kernel(void *ctx)
 		group_leader, group_leader, group_leader, group_leader,
 		comm);
 
+	out->local_task_struct_matches = bpf_core_type_matches(struct task_struct___local);
+#else
+	data.skip = true;
+#endif
 	return 0;
 }
 
-- 
2.30.2


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

* Re: [PATCH bpf-next v3 02/10] bpftool: Honor BPF_CORE_TYPE_MATCHES relocation
  2022-06-28 16:01 ` [PATCH bpf-next v3 02/10] bpftool: Honor BPF_CORE_TYPE_MATCHES relocation Daniel Müller
@ 2022-06-28 16:52   ` Quentin Monnet
  2022-07-06  4:16   ` Andrii Nakryiko
  1 sibling, 0 replies; 18+ messages in thread
From: Quentin Monnet @ 2022-06-28 16:52 UTC (permalink / raw)
  To: Daniel Müller, bpf, ast, andrii, daniel, kernel-team; +Cc: joannelkoong

2022-06-28 16:01 UTC+0000 ~ Daniel Müller <deso@posteo.net>
> bpftool needs to know about the newly introduced BPF_CORE_TYPE_MATCHES
> relocation for its 'gen min_core_btf' command to work properly in the
> present of this relocation.
> Specifically, we need to make sure to mark types and fields so that they
> are present in the minimized BTF for "type match" checks to work out.
> However, contrary to the existing btfgen_record_field_relo, we need to
> rely on the BTF -- and not the spec -- to find fields. With this change
> we handle this new variant correctly. The functionality will be tested
> with follow on changes to BPF selftests, which already run against a
> minimized BTF created with bpftool.
> 
> Cc: Quentin Monnet <quentin@isovalent.com>
> Signed-off-by: Daniel Müller <deso@posteo.net>

Acked-by: Quentin Monnet <quentin@isovalent.com>

Thanks!


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

* Re: [PATCH bpf-next v3 00/10] Introduce type match support
  2022-06-28 16:01 [PATCH bpf-next v3 00/10] Introduce type match support Daniel Müller
                   ` (9 preceding siblings ...)
  2022-06-28 16:01 ` [PATCH bpf-next v3 10/10] selftests/bpf: Add type match test against kernel's task_struct Daniel Müller
@ 2022-07-05 21:07 ` Daniel Müller
  2022-07-06  4:16   ` Andrii Nakryiko
  2022-07-06  4:20 ` patchwork-bot+netdevbpf
  11 siblings, 1 reply; 18+ messages in thread
From: Daniel Müller @ 2022-07-05 21:07 UTC (permalink / raw)
  To: bpf, ast, andrii, daniel, kernel-team; +Cc: joannelkoong

On Tue, Jun 28, 2022 at 04:01:17PM +0000, Daniel Müller wrote:
> This patch set proposes the addition of a new way for performing type queries to
> BPF. It introduces the "type matches" relation, similar to what is already
> present with "type exists" (in the form of bpf_core_type_exists).
> 
> "type exists" performs fairly superficial checking, mostly concerned with
> whether a type exists in the kernel and is of the same kind (enum/struct/...).
> Notably, compatibility checks for members of composite types is lacking.
> 
> The newly introduced "type matches" (bpf_core_type_matches) fills this gap in
> that it performs stricter checks: compatibility of members and existence of
> similarly named enum variants is checked as well. E.g., given these definitions:
> 
> 	struct task_struct___og { int pid; int tgid; };
> 
> 	struct task_struct___foo { int foo; }
> 
> 'task_struct___og' would "match" the kernel type 'task_struct', because the
> members match up, while 'task_struct___foo' would not match, because the
> kernel's 'task_struct' has no member named 'foo'.
> 
> More precisely, the "type match" relation is defined as follows (copied from
> source):
> - modifiers and typedefs are stripped (and, hence, effectively ignored)
> - generally speaking types need to be of same kind (struct vs. struct, union
>   vs. union, etc.)
>   - exceptions are struct/union behind a pointer which could also match a
>     forward declaration of a struct or union, respectively, and enum vs.
>     enum64 (see below)
> Then, depending on type:
> - integers:
>   - match if size and signedness match
> - arrays & pointers:
>   - target types are recursively matched
> - structs & unions:
>   - local members need to exist in target with the same name
>   - for each member we recursively check match unless it is already behind a
>     pointer, in which case we only check matching names and compatible kind
> - enums:
>   - local variants have to have a match in target by symbolic name (but not
>     numeric value)
>   - size has to match (but enum may match enum64 and vice versa)
> - function pointers:
>   - number and position of arguments in local type has to match target
>   - for each argument and the return value we recursively check match
> 
> Enabling this feature requires a new relocation to be made known to the
> compiler. This is being taken care of for LLVM as part of
> https://reviews.llvm.org/D126838.

To give an update here, LLVM changes have been merged and, to the best of my
knowledge, are being used by BPF CI (tests that failed earlier are now passing).

Thanks,
Daniel

[...]

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

* Re: [PATCH bpf-next v3 00/10] Introduce type match support
  2022-07-05 21:07 ` [PATCH bpf-next v3 00/10] Introduce type match support Daniel Müller
@ 2022-07-06  4:16   ` Andrii Nakryiko
  2022-07-06 16:06     ` Daniel Müller
  0 siblings, 1 reply; 18+ messages in thread
From: Andrii Nakryiko @ 2022-07-06  4:16 UTC (permalink / raw)
  To: Daniel Müller
  Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Kernel Team, Joanne Koong

On Tue, Jul 5, 2022 at 2:07 PM Daniel Müller <deso@posteo.net> wrote:
>
> On Tue, Jun 28, 2022 at 04:01:17PM +0000, Daniel Müller wrote:
> > This patch set proposes the addition of a new way for performing type queries to
> > BPF. It introduces the "type matches" relation, similar to what is already
> > present with "type exists" (in the form of bpf_core_type_exists).
> >
> > "type exists" performs fairly superficial checking, mostly concerned with
> > whether a type exists in the kernel and is of the same kind (enum/struct/...).
> > Notably, compatibility checks for members of composite types is lacking.
> >
> > The newly introduced "type matches" (bpf_core_type_matches) fills this gap in
> > that it performs stricter checks: compatibility of members and existence of
> > similarly named enum variants is checked as well. E.g., given these definitions:
> >
> >       struct task_struct___og { int pid; int tgid; };
> >
> >       struct task_struct___foo { int foo; }
> >
> > 'task_struct___og' would "match" the kernel type 'task_struct', because the
> > members match up, while 'task_struct___foo' would not match, because the
> > kernel's 'task_struct' has no member named 'foo'.
> >
> > More precisely, the "type match" relation is defined as follows (copied from
> > source):
> > - modifiers and typedefs are stripped (and, hence, effectively ignored)
> > - generally speaking types need to be of same kind (struct vs. struct, union
> >   vs. union, etc.)
> >   - exceptions are struct/union behind a pointer which could also match a
> >     forward declaration of a struct or union, respectively, and enum vs.
> >     enum64 (see below)
> > Then, depending on type:
> > - integers:
> >   - match if size and signedness match
> > - arrays & pointers:
> >   - target types are recursively matched
> > - structs & unions:
> >   - local members need to exist in target with the same name
> >   - for each member we recursively check match unless it is already behind a
> >     pointer, in which case we only check matching names and compatible kind
> > - enums:
> >   - local variants have to have a match in target by symbolic name (but not
> >     numeric value)
> >   - size has to match (but enum may match enum64 and vice versa)
> > - function pointers:
> >   - number and position of arguments in local type has to match target
> >   - for each argument and the return value we recursively check match
> >
> > Enabling this feature requires a new relocation to be made known to the
> > compiler. This is being taken care of for LLVM as part of
> > https://reviews.llvm.org/D126838.
>
> To give an update here, LLVM changes have been merged and, to the best of my
> knowledge, are being used by BPF CI (tests that failed earlier are now passing).
>

I did a few small changes and combined patches 4-6 together (because
they add the same functionality to both libbpf and kernel
simultaneously, there were compilation warnings about non-static
functions not having a proper prototype defined). But I've split out
the bpf_core_type_matches() macro in bpf_core_read.h into a separate
patch. I also dropped patch #3 as it wasn't needed anymore.

Please see comments I left for two further follow ups.

> Thanks,
> Daniel
>
> [...]

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

* Re: [PATCH bpf-next v3 02/10] bpftool: Honor BPF_CORE_TYPE_MATCHES relocation
  2022-06-28 16:01 ` [PATCH bpf-next v3 02/10] bpftool: Honor BPF_CORE_TYPE_MATCHES relocation Daniel Müller
  2022-06-28 16:52   ` Quentin Monnet
@ 2022-07-06  4:16   ` Andrii Nakryiko
  1 sibling, 0 replies; 18+ messages in thread
From: Andrii Nakryiko @ 2022-07-06  4:16 UTC (permalink / raw)
  To: Daniel Müller
  Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Kernel Team, Joanne Koong, Quentin Monnet

On Tue, Jun 28, 2022 at 9:01 AM Daniel Müller <deso@posteo.net> wrote:
>
> bpftool needs to know about the newly introduced BPF_CORE_TYPE_MATCHES
> relocation for its 'gen min_core_btf' command to work properly in the
> present of this relocation.
> Specifically, we need to make sure to mark types and fields so that they
> are present in the minimized BTF for "type match" checks to work out.
> However, contrary to the existing btfgen_record_field_relo, we need to
> rely on the BTF -- and not the spec -- to find fields. With this change
> we handle this new variant correctly. The functionality will be tested
> with follow on changes to BPF selftests, which already run against a
> minimized BTF created with bpftool.
>
> Cc: Quentin Monnet <quentin@isovalent.com>
> Signed-off-by: Daniel Müller <deso@posteo.net>
> ---
>  tools/bpf/bpftool/gen.c | 108 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 108 insertions(+)
>
> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> index 480cbd8..a30328c 100644
> --- a/tools/bpf/bpftool/gen.c
> +++ b/tools/bpf/bpftool/gen.c
> @@ -1856,6 +1856,112 @@ static int btfgen_record_field_relo(struct btfgen_info *info, struct bpf_core_sp
>         return 0;
>  }
>
> +/* Mark types, members, and member types. Compared to btfgen_record_field_relo,
> + * this function does not rely on the target spec for inferring members, but
> + * uses the associated BTF.
> + *
> + * The `behind_ptr` argument is used to stop marking of composite types reached
> + * through a pointer. This way, we can keep BTF size in check while providing
> + * reasonable match semantics.
> + */
> +static int btfgen_mark_type_match(struct btfgen_info *info, __u32 type_id, bool behind_ptr)
> +{
> +       const struct btf_type *btf_type;
> +       struct btf *btf = info->src_btf;
> +       struct btf_type *cloned_type;
> +       int i, err;
> +
> +       if (type_id == 0)
> +               return 0;
> +
> +       btf_type = btf__type_by_id(btf, type_id);
> +       /* mark type on cloned BTF as used */
> +       cloned_type = (struct btf_type *)btf__type_by_id(info->marked_btf, type_id);
> +       cloned_type->name_off = MARKED;
> +
> +       switch (btf_kind(btf_type)) {
> +       case BTF_KIND_UNKN:
> +       case BTF_KIND_INT:
> +       case BTF_KIND_FLOAT:
> +       case BTF_KIND_ENUM:
> +       case BTF_KIND_ENUM64:
> +               break;
> +       case BTF_KIND_STRUCT:
> +       case BTF_KIND_UNION: {
> +               struct btf_member *m = btf_members(btf_type);
> +               __u16 vlen = btf_vlen(btf_type);
> +
> +               if (behind_ptr)
> +                       break;
> +
> +               for (i = 0; i < vlen; i++, m++) {
> +                       /* mark member */
> +                       btfgen_mark_member(info, type_id, i);
> +
> +                       /* mark member's type */
> +                       err = btfgen_mark_type_match(info, m->type, false);
> +                       if (err)
> +                               return err;
> +               }
> +               break;
> +       }
> +       case BTF_KIND_CONST:
> +       case BTF_KIND_FWD:
> +       case BTF_KIND_RESTRICT:
> +       case BTF_KIND_TYPEDEF:
> +       case BTF_KIND_VOLATILE:
> +               return btfgen_mark_type_match(info, btf_type->type, false);

this should have preserved behind_ptr instead of resetting it to false
(i.e. `const struct blah *` should be treated similarly to `struct
blah *`). I've fixed it up while applying.

> +       case BTF_KIND_PTR:
> +               return btfgen_mark_type_match(info, btf_type->type, true);
> +       case BTF_KIND_ARRAY: {
> +               struct btf_array *array;
> +
> +               array = btf_array(btf_type);
> +               /* mark array type */
> +               err = btfgen_mark_type_match(info, array->type, false);
> +               /* mark array's index type */
> +               err = err ? : btfgen_mark_type_match(info, array->index_type, false);
> +               if (err)
> +                       return err;
> +               break;

[...]

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

* Re: [PATCH bpf-next v3 04/10] libbpf: Add type match support
  2022-06-28 16:01 ` [PATCH bpf-next v3 04/10] libbpf: Add type match support Daniel Müller
@ 2022-07-06  4:16   ` Andrii Nakryiko
  0 siblings, 0 replies; 18+ messages in thread
From: Andrii Nakryiko @ 2022-07-06  4:16 UTC (permalink / raw)
  To: Daniel Müller
  Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Kernel Team, Joanne Koong

On Tue, Jun 28, 2022 at 9:02 AM Daniel Müller <deso@posteo.net> wrote:
>
> This patch adds support for the proposed type match relation to
> relo_core where it is shared between userspace and kernel. A bit more
> plumbing is necessary and will arrive with subsequent changes to
> actually use it -- this patch only introduces the main matching
> algorithm.
>
> The matching relation is defined as follows (copy from source):
> - modifiers and typedefs are stripped (and, hence, effectively ignored)
> - generally speaking types need to be of same kind (struct vs. struct, union
>   vs. union, etc.)
>   - exceptions are struct/union behind a pointer which could also match a
>     forward declaration of a struct or union, respectively, and enum vs.
>     enum64 (see below)
> Then, depending on type:
> - integers:
>   - match if size and signedness match
> - arrays & pointers:
>   - target types are recursively matched
> - structs & unions:
>   - local members need to exist in target with the same name
>   - for each member we recursively check match unless it is already behind a
>     pointer, in which case we only check matching names and compatible kind
> - enums:
>   - local variants have to have a match in target by symbolic name (but not
>     numeric value)
>   - size has to match (but enum may match enum64 and vice versa)
> - function pointers:
>   - number and position of arguments in local type has to match target
>   - for each argument and the return value we recursively check match
>
> Signed-off-by: Daniel Müller <deso@posteo.net>
> ---
>  tools/lib/bpf/relo_core.c | 268 ++++++++++++++++++++++++++++++++++++++
>  tools/lib/bpf/relo_core.h |   2 +
>  2 files changed, 270 insertions(+)
>

[...]

> +static int bpf_core_composites_match(const struct btf *local_btf, const struct btf_type *local_t,
> +                                    const struct btf *targ_btf, const struct btf_type *targ_t,
> +                                    bool behind_ptr, int level)
> +{
> +       const struct btf_member *local_m = btf_members(local_t);
> +       __u16 local_vlen = btf_vlen(local_t);
> +       __u16 targ_vlen = btf_vlen(targ_t);
> +       int i, j, err;
> +
> +       if (local_vlen > targ_vlen)
> +               return 0;
> +
> +       /* check that all local members have a match in the target */
> +       for (i = 0; i < local_vlen; i++, local_m++) {
> +               const struct btf_member *targ_m = btf_members(targ_t);
> +               bool matched = false;
> +
> +               for (j = 0; j < targ_vlen; j++, targ_m++) {
> +                       if (!bpf_core_names_match(local_btf, local_m->name_off, targ_btf,
> +                                                 targ_m->name_off))
> +                               continue;
> +
> +                       err = __bpf_core_types_match(local_btf, local_m->type, targ_btf,
> +                                                    targ_m->type, behind_ptr, level - 1);
> +                       if (err > 0) {

this seems a bit too permissive, if we get an error, we should error
out instead of ignoring this. I left this for a follow up, though

> +                               matched = true;
> +                               break;
> +                       }
> +               }
> +
> +               if (!matched)
> +                       return 0;
> +       }
> +       return 1;
> +}
> +
> +/* Check that two types "match".
> + *
> + * The matching relation is defined as follows:
> + * - modifiers and typedefs are stripped (and, hence, effectively ignored)
> + * - generally speaking types need to be of same kind (struct vs. struct, union
> + *   vs. union, etc.)
> + *   - exceptions are struct/union behind a pointer which could also match a
> + *     forward declaration of a struct or union, respectively, and enum vs.
> + *     enum64 (see below)
> + * Then, depending on type:
> + * - integers:
> + *   - match if size and signedness match
> + * - arrays & pointers:
> + *   - target types are recursively matched
> + * - structs & unions:
> + *   - local members need to exist in target with the same name
> + *   - for each member we recursively check match unless it is already behind a
> + *     pointer, in which case we only check matching names and compatible kind
> + * - enums:
> + *   - local variants have to have a match in target by symbolic name (but not
> + *     numeric value)
> + *   - size has to match (but enum may match enum64 and vice versa)
> + * - function pointers:
> + *   - number and position of arguments in local type has to match target
> + *   - for each argument and the return value we recursively check match
> + */
> +int __bpf_core_types_match(const struct btf *local_btf, __u32 local_id, const struct btf *targ_btf,
> +                          __u32 targ_id, bool behind_ptr, int level)
> +{
> +       const struct btf_type *local_t, *targ_t;
> +       int depth = 32; /* max recursion depth */
> +       __u16 local_k;
> +
> +       if (level <= 0)
> +               return -EINVAL;
> +
> +       local_t = btf_type_by_id(local_btf, local_id);
> +       targ_t = btf_type_by_id(targ_btf, targ_id);
> +
> +recur:
> +       depth--;
> +       if (depth < 0)
> +               return -EINVAL;
> +
> +       local_t = skip_mods_and_typedefs(local_btf, local_id, &local_id);
> +       targ_t = skip_mods_and_typedefs(targ_btf, targ_id, &targ_id);
> +       if (!local_t || !targ_t)
> +               return -EINVAL;
> +
> +       if (!bpf_core_names_match(local_btf, local_t->name_off, targ_btf, targ_t->name_off))
> +               return 0;

so the location of this check bothers me

Think about the case when we have on one side

typedef struct { /* something */ } abc;

and this on the other side:

typedef struct { /* something */ } def;

As this is written right now, we'll just ignore "abc" and "def" names,
which seems wrong. I haven't touched this part, but let's think what
to do about that and have a follow up patch.

> +
> +       local_k = btf_kind(local_t);
> +
> +       switch (local_k) {

[...]

> +
> +               return bpf_core_enums_match(local_btf, local_t, targ_btf, targ_t);
> +       case BTF_KIND_STRUCT:
> +       case BTF_KIND_UNION: {
> +               __u16 targ_k = btf_kind(targ_t);

you use targ_k almost in every case, so it would be cleaner to just
calculated right next to local_k, IMO (so that's what I did when
applying)

> +
> +               if (behind_ptr) {
> +                       bool targ_f = BTF_INFO_KFLAG(targ_t->info);

[...]

> +
> +               local_sgn = btf_int_encoding(local_t) & BTF_INT_SIGNED;
> +               targ_sgn = btf_int_encoding(targ_t) & BTF_INT_SIGNED;
> +
> +               return btf_int_bits(local_t) == btf_int_bits(targ_t) && local_sgn == targ_sgn;

nit: should probably be local_t->size == targ_t->size instead of
btf_int_bits() (which is kind of deprecated)



> +       }
> +       case BTF_KIND_PTR:
> +               if (local_k != btf_kind(targ_t))
> +                       return 0;
> +
> +               behind_ptr = true;
> +
> +               local_id = local_t->type;
> +               targ_id = targ_t->type;
> +               goto recur;

[...]

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

* Re: [PATCH bpf-next v3 00/10] Introduce type match support
  2022-06-28 16:01 [PATCH bpf-next v3 00/10] Introduce type match support Daniel Müller
                   ` (10 preceding siblings ...)
  2022-07-05 21:07 ` [PATCH bpf-next v3 00/10] Introduce type match support Daniel Müller
@ 2022-07-06  4:20 ` patchwork-bot+netdevbpf
  11 siblings, 0 replies; 18+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-07-06  4:20 UTC (permalink / raw)
  To: =?utf-8?q?Daniel_M=C3=BCller_=3Cdeso=40posteo=2Enet=3E?=
  Cc: bpf, ast, andrii, daniel, kernel-team, joannelkoong

Hello:

This series was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Tue, 28 Jun 2022 16:01:17 +0000 you wrote:
> This patch set proposes the addition of a new way for performing type queries to
> BPF. It introduces the "type matches" relation, similar to what is already
> present with "type exists" (in the form of bpf_core_type_exists).
> 
> "type exists" performs fairly superficial checking, mostly concerned with
> whether a type exists in the kernel and is of the same kind (enum/struct/...).
> Notably, compatibility checks for members of composite types is lacking.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v3,01/10] bpf: Introduce TYPE_MATCH related constants/macros
    https://git.kernel.org/bpf/bpf-next/c/3c660a5d86f4
  - [bpf-next,v3,02/10] bpftool: Honor BPF_CORE_TYPE_MATCHES relocation
    https://git.kernel.org/bpf/bpf-next/c/633e7ceb2cbb
  - [bpf-next,v3,03/10] bpf: Introduce btf_int_bits() function
    (no matching commit)
  - [bpf-next,v3,04/10] libbpf: Add type match support
    https://git.kernel.org/bpf/bpf-next/c/ec6209c8d42f
  - [bpf-next,v3,05/10] bpf: Add type match support
    (no matching commit)
  - [bpf-next,v3,06/10] libbpf: Honor TYPE_MATCH relocation
    (no matching commit)
  - [bpf-next,v3,07/10] selftests/bpf: Add type-match checks to type-based tests
    https://git.kernel.org/bpf/bpf-next/c/67d8ed429525
  - [bpf-next,v3,08/10] selftests/bpf: Add test checking more characteristics
    https://git.kernel.org/bpf/bpf-next/c/bed56a6dd4cb
  - [bpf-next,v3,09/10] selftests/bpf: Add nested type to type based tests
    https://git.kernel.org/bpf/bpf-next/c/537905c4b68f
  - [bpf-next,v3,10/10] selftests/bpf: Add type match test against kernel's task_struct
    https://git.kernel.org/bpf/bpf-next/c/950b34778722

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

* Re: [PATCH bpf-next v3 00/10] Introduce type match support
  2022-07-06  4:16   ` Andrii Nakryiko
@ 2022-07-06 16:06     ` Daniel Müller
  0 siblings, 0 replies; 18+ messages in thread
From: Daniel Müller @ 2022-07-06 16:06 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Kernel Team, Joanne Koong

On Tue, Jul 05, 2022 at 09:16:27PM -0700, Andrii Nakryiko wrote:
> On Tue, Jul 5, 2022 at 2:07 PM Daniel Müller <deso@posteo.net> wrote:
> >
> > On Tue, Jun 28, 2022 at 04:01:17PM +0000, Daniel Müller wrote:
> > > This patch set proposes the addition of a new way for performing type queries to
> > > BPF. It introduces the "type matches" relation, similar to what is already
> > > present with "type exists" (in the form of bpf_core_type_exists).
> > >
> > > "type exists" performs fairly superficial checking, mostly concerned with
> > > whether a type exists in the kernel and is of the same kind (enum/struct/...).
> > > Notably, compatibility checks for members of composite types is lacking.
> > >
> > > The newly introduced "type matches" (bpf_core_type_matches) fills this gap in
> > > that it performs stricter checks: compatibility of members and existence of
> > > similarly named enum variants is checked as well. E.g., given these definitions:
> > >
> > >       struct task_struct___og { int pid; int tgid; };
> > >
> > >       struct task_struct___foo { int foo; }
> > >
> > > 'task_struct___og' would "match" the kernel type 'task_struct', because the
> > > members match up, while 'task_struct___foo' would not match, because the
> > > kernel's 'task_struct' has no member named 'foo'.
> > >
> > > More precisely, the "type match" relation is defined as follows (copied from
> > > source):
> > > - modifiers and typedefs are stripped (and, hence, effectively ignored)
> > > - generally speaking types need to be of same kind (struct vs. struct, union
> > >   vs. union, etc.)
> > >   - exceptions are struct/union behind a pointer which could also match a
> > >     forward declaration of a struct or union, respectively, and enum vs.
> > >     enum64 (see below)
> > > Then, depending on type:
> > > - integers:
> > >   - match if size and signedness match
> > > - arrays & pointers:
> > >   - target types are recursively matched
> > > - structs & unions:
> > >   - local members need to exist in target with the same name
> > >   - for each member we recursively check match unless it is already behind a
> > >     pointer, in which case we only check matching names and compatible kind
> > > - enums:
> > >   - local variants have to have a match in target by symbolic name (but not
> > >     numeric value)
> > >   - size has to match (but enum may match enum64 and vice versa)
> > > - function pointers:
> > >   - number and position of arguments in local type has to match target
> > >   - for each argument and the return value we recursively check match
> > >
> > > Enabling this feature requires a new relocation to be made known to the
> > > compiler. This is being taken care of for LLVM as part of
> > > https://reviews.llvm.org/D126838.
> >
> > To give an update here, LLVM changes have been merged and, to the best of my
> > knowledge, are being used by BPF CI (tests that failed earlier are now passing).
> >
> 
> I did a few small changes and combined patches 4-6 together (because
> they add the same functionality to both libbpf and kernel
> simultaneously, there were compilation warnings about non-static
> functions not having a proper prototype defined). But I've split out
> the bpf_core_type_matches() macro in bpf_core_read.h into a separate
> patch. I also dropped patch #3 as it wasn't needed anymore.
> 
> Please see comments I left for two further follow ups.

Sounds good. Will address your comments soon. Thanks for merging!

Daniel

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

end of thread, other threads:[~2022-07-06 16:06 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-28 16:01 [PATCH bpf-next v3 00/10] Introduce type match support Daniel Müller
2022-06-28 16:01 ` [PATCH bpf-next v3 01/10] bpf: Introduce TYPE_MATCH related constants/macros Daniel Müller
2022-06-28 16:01 ` [PATCH bpf-next v3 02/10] bpftool: Honor BPF_CORE_TYPE_MATCHES relocation Daniel Müller
2022-06-28 16:52   ` Quentin Monnet
2022-07-06  4:16   ` Andrii Nakryiko
2022-06-28 16:01 ` [PATCH bpf-next v3 03/10] bpf: Introduce btf_int_bits() function Daniel Müller
2022-06-28 16:01 ` [PATCH bpf-next v3 04/10] libbpf: Add type match support Daniel Müller
2022-07-06  4:16   ` Andrii Nakryiko
2022-06-28 16:01 ` [PATCH bpf-next v3 05/10] bpf: " Daniel Müller
2022-06-28 16:01 ` [PATCH bpf-next v3 06/10] libbpf: Honor TYPE_MATCH relocation Daniel Müller
2022-06-28 16:01 ` [PATCH bpf-next v3 07/10] selftests/bpf: Add type-match checks to type-based tests Daniel Müller
2022-06-28 16:01 ` [PATCH bpf-next v3 08/10] selftests/bpf: Add test checking more characteristics Daniel Müller
2022-06-28 16:01 ` [PATCH bpf-next v3 09/10] selftests/bpf: Add nested type to type based tests Daniel Müller
2022-06-28 16:01 ` [PATCH bpf-next v3 10/10] selftests/bpf: Add type match test against kernel's task_struct Daniel Müller
2022-07-05 21:07 ` [PATCH bpf-next v3 00/10] Introduce type match support Daniel Müller
2022-07-06  4:16   ` Andrii Nakryiko
2022-07-06 16:06     ` Daniel Müller
2022-07-06  4:20 ` patchwork-bot+netdevbpf

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