bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/5] Add CO-RE support for field existence relos
@ 2019-10-14 23:49 Andrii Nakryiko
  2019-10-14 23:49 ` [PATCH bpf-next 1/5] libbpf: update BTF reloc support to latest Clang format Andrii Nakryiko
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Andrii Nakryiko @ 2019-10-14 23:49 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

This patch set generalizes libbpf's CO-RE relocation support. It not supports field existence relocations, in addition to existing field's byte offset relocation.

This patch set upgrades the format of .BTF.ext's relocation record to match
latest Clang's format (12 -> 16 bytes). This is not a breaking change, as the
previous format hasn't been released yet as part of official Clang version
release.

Andrii Nakryiko (5):
  libbpf: update BTF reloc support to latest Clang format
  libbpf: refactor bpf_object__open APIs to use common opts
  libbpf: add support for field existance CO-RE relocation
  libbpf: add BPF-side definitions of supported field relocation kinds
  selftests/bpf: add field existence CO-RE relocs tests

 tools/lib/bpf/bpf_core_read.h                 |  12 ++
 tools/lib/bpf/btf.c                           |  16 +-
 tools/lib/bpf/btf.h                           |   4 +-
 tools/lib/bpf/libbpf.c                        | 169 +++++++++++-------
 tools/lib/bpf/libbpf.h                        |   4 +-
 tools/lib/bpf/libbpf_internal.h               |  25 ++-
 .../selftests/bpf/prog_tests/core_reloc.c     |  76 +++++++-
 .../bpf/progs/btf__core_reloc_existence.c     |   3 +
 ...ore_reloc_existence___err_wrong_arr_kind.c |   3 +
 ...loc_existence___err_wrong_arr_value_type.c |   3 +
 ...ore_reloc_existence___err_wrong_int_kind.c |   3 +
 ..._core_reloc_existence___err_wrong_int_sz.c |   3 +
 ...ore_reloc_existence___err_wrong_int_type.c |   3 +
 ..._reloc_existence___err_wrong_struct_type.c |   3 +
 .../btf__core_reloc_existence___minimal.c     |   3 +
 .../selftests/bpf/progs/core_reloc_types.h    |  56 ++++++
 .../bpf/progs/test_core_reloc_existence.c     |  79 ++++++++
 17 files changed, 381 insertions(+), 84 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_existence.c
 create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_arr_kind.c
 create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_arr_value_type.c
 create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_int_kind.c
 create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_int_sz.c
 create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_int_type.c
 create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_struct_type.c
 create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_existence___minimal.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_core_reloc_existence.c

-- 
2.17.1


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

* [PATCH bpf-next 1/5] libbpf: update BTF reloc support to latest Clang format
  2019-10-14 23:49 [PATCH bpf-next 0/5] Add CO-RE support for field existence relos Andrii Nakryiko
@ 2019-10-14 23:49 ` Andrii Nakryiko
  2019-10-15 15:15   ` Alexei Starovoitov
  2019-10-14 23:49 ` [PATCH bpf-next 2/5] libbpf: refactor bpf_object__open APIs to use common opts Andrii Nakryiko
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2019-10-14 23:49 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

BTF offset reloc was generalized in recent Clang into field relocation,
capturing extra u32 field, specifying what aspect of captured field
needs to be relocated. This changes .BTF.ext's record size for this
relocation from 12 bytes to 16 bytes. Given these format changes
happened in Clang before official released version, it's ok to not
support outdated 12-byte record size w/o breaking ABI.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/btf.c             | 16 ++++++++--------
 tools/lib/bpf/btf.h             |  4 ++--
 tools/lib/bpf/libbpf.c          | 24 ++++++++++++------------
 tools/lib/bpf/libbpf_internal.h | 25 ++++++++++++++++++-------
 4 files changed, 40 insertions(+), 29 deletions(-)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 1aa189a9112a..3eae8d1addfa 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -888,14 +888,14 @@ static int btf_ext_setup_line_info(struct btf_ext *btf_ext)
 	return btf_ext_setup_info(btf_ext, &param);
 }
 
-static int btf_ext_setup_offset_reloc(struct btf_ext *btf_ext)
+static int btf_ext_setup_field_reloc(struct btf_ext *btf_ext)
 {
 	struct btf_ext_sec_setup_param param = {
-		.off = btf_ext->hdr->offset_reloc_off,
-		.len = btf_ext->hdr->offset_reloc_len,
-		.min_rec_size = sizeof(struct bpf_offset_reloc),
-		.ext_info = &btf_ext->offset_reloc_info,
-		.desc = "offset_reloc",
+		.off = btf_ext->hdr->field_reloc_off,
+		.len = btf_ext->hdr->field_reloc_len,
+		.min_rec_size = sizeof(struct bpf_field_reloc),
+		.ext_info = &btf_ext->field_reloc_info,
+		.desc = "field_reloc",
 	};
 
 	return btf_ext_setup_info(btf_ext, &param);
@@ -975,9 +975,9 @@ struct btf_ext *btf_ext__new(__u8 *data, __u32 size)
 		goto done;
 
 	if (btf_ext->hdr->hdr_len <
-	    offsetofend(struct btf_ext_header, offset_reloc_len))
+	    offsetofend(struct btf_ext_header, field_reloc_len))
 		goto done;
-	err = btf_ext_setup_offset_reloc(btf_ext);
+	err = btf_ext_setup_field_reloc(btf_ext);
 	if (err)
 		goto done;
 
diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index 9cb44b4fbf60..b18994116a44 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -60,8 +60,8 @@ struct btf_ext_header {
 	__u32	line_info_len;
 
 	/* optional part of .BTF.ext header */
-	__u32	offset_reloc_off;
-	__u32	offset_reloc_len;
+	__u32	field_reloc_off;
+	__u32	field_reloc_len;
 };
 
 LIBBPF_API void btf__free(struct btf *btf);
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index a02cdedc4e3f..d6c7699de405 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2326,7 +2326,7 @@ static bool str_is_empty(const char *s)
 }
 
 /*
- * Turn bpf_offset_reloc into a low- and high-level spec representation,
+ * Turn bpf_field_reloc into a low- and high-level spec representation,
  * validating correctness along the way, as well as calculating resulting
  * field offset (in bytes), specified by accessor string. Low-level spec
  * captures every single level of nestedness, including traversing anonymous
@@ -2977,7 +2977,7 @@ static void *u32_as_hash_key(__u32 x)
  *    types should be compatible (see bpf_core_fields_are_compat for details).
  * 3. It is supported and expected that there might be multiple flavors
  *    matching the spec. As long as all the specs resolve to the same set of
- *    offsets across all candidates, there is not error. If there is any
+ *    offsets across all candidates, there is no error. If there is any
  *    ambiguity, CO-RE relocation will fail. This is necessary to accomodate
  *    imprefection of BTF deduplication, which can cause slight duplication of
  *    the same BTF type, if some directly or indirectly referenced (by
@@ -2992,12 +2992,12 @@ static void *u32_as_hash_key(__u32 x)
  *    CPU-wise compared to prebuilding a map from all local type names to
  *    a list of candidate type names. It's also sped up by caching resolved
  *    list of matching candidates per each local "root" type ID, that has at
- *    least one bpf_offset_reloc associated with it. This list is shared
+ *    least one bpf_field_reloc associated with it. This list is shared
  *    between multiple relocations for the same type ID and is updated as some
  *    of the candidates are pruned due to structural incompatibility.
  */
-static int bpf_core_reloc_offset(struct bpf_program *prog,
-				 const struct bpf_offset_reloc *relo,
+static int bpf_core_reloc_field(struct bpf_program *prog,
+				 const struct bpf_field_reloc *relo,
 				 int relo_idx,
 				 const struct btf *local_btf,
 				 const struct btf *targ_btf,
@@ -3106,10 +3106,10 @@ static int bpf_core_reloc_offset(struct bpf_program *prog,
 }
 
 static int
-bpf_core_reloc_offsets(struct bpf_object *obj, const char *targ_btf_path)
+bpf_core_reloc_fields(struct bpf_object *obj, const char *targ_btf_path)
 {
 	const struct btf_ext_info_sec *sec;
-	const struct bpf_offset_reloc *rec;
+	const struct bpf_field_reloc *rec;
 	const struct btf_ext_info *seg;
 	struct hashmap_entry *entry;
 	struct hashmap *cand_cache = NULL;
@@ -3134,7 +3134,7 @@ bpf_core_reloc_offsets(struct bpf_object *obj, const char *targ_btf_path)
 		goto out;
 	}
 
-	seg = &obj->btf_ext->offset_reloc_info;
+	seg = &obj->btf_ext->field_reloc_info;
 	for_each_btf_ext_sec(seg, sec) {
 		sec_name = btf__name_by_offset(obj->btf, sec->sec_name_off);
 		if (str_is_empty(sec_name)) {
@@ -3153,8 +3153,8 @@ bpf_core_reloc_offsets(struct bpf_object *obj, const char *targ_btf_path)
 			 sec_name, sec->num_info);
 
 		for_each_btf_ext_rec(seg, sec, i, rec) {
-			err = bpf_core_reloc_offset(prog, rec, i, obj->btf,
-						    targ_btf, cand_cache);
+			err = bpf_core_reloc_field(prog, rec, i, obj->btf,
+						   targ_btf, cand_cache);
 			if (err) {
 				pr_warning("prog '%s': relo #%d: failed to relocate: %d\n",
 					   sec_name, i, err);
@@ -3179,8 +3179,8 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
 {
 	int err = 0;
 
-	if (obj->btf_ext->offset_reloc_info.len)
-		err = bpf_core_reloc_offsets(obj, targ_btf_path);
+	if (obj->btf_ext->field_reloc_info.len)
+		err = bpf_core_reloc_fields(obj, targ_btf_path);
 
 	return err;
 }
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index f51444fc7eb7..524a74a6ec81 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -110,7 +110,7 @@ struct btf_ext {
 	};
 	struct btf_ext_info func_info;
 	struct btf_ext_info line_info;
-	struct btf_ext_info offset_reloc_info;
+	struct btf_ext_info field_reloc_info;
 	__u32 data_size;
 };
 
@@ -135,13 +135,23 @@ struct bpf_line_info_min {
 	__u32	line_col;
 };
 
-/* The minimum bpf_offset_reloc checked by the loader
+/* bpf_field_reloc_kind encodes which aspect of captured field has to be
+ * adjusted by relocations. Currently supported values are:
+ *   - BPF_FI_BYTE_OFFSET: field offset (in bytes);
+ *   - BPF_FI_EXISTS: field existence (1 if field exists, 0 - otherwise);
+ */
+enum bpf_field_reloc_kind {
+	BPF_FRK_BYTE_OFFSET = 0,
+	BPF_FRK_EXISTS = 2,
+};
+
+/* The minimum bpf_field_reloc checked by the loader
  *
- * Offset relocation captures the following data:
+ * Field relocation captures the following data:
  * - insn_off - instruction offset (in bytes) within a BPF program that needs
- *   its insn->imm field to be relocated with actual offset;
+ *   its insn->imm field to be relocated with actual field info;
  * - type_id - BTF type ID of the "root" (containing) entity of a relocatable
- *   offset;
+ *   field;
  * - access_str_off - offset into corresponding .BTF string section. String
  *   itself encodes an accessed field using a sequence of field and array
  *   indicies, separated by colon (:). It's conceptually very close to LLVM's
@@ -172,15 +182,16 @@ struct bpf_line_info_min {
  * bpf_probe_read(&dst, sizeof(dst),
  *		  __builtin_preserve_access_index(&src->a.b.c));
  *
- * In this case Clang will emit offset relocation recording necessary data to
+ * In this case Clang will emit field relocation recording necessary data to
  * be able to find offset of embedded `a.b.c` field within `src` struct.
  *
  *   [0] https://llvm.org/docs/LangRef.html#getelementptr-instruction
  */
-struct bpf_offset_reloc {
+struct bpf_field_reloc {
 	__u32   insn_off;
 	__u32   type_id;
 	__u32   access_str_off;
+	enum bpf_field_reloc_kind kind;
 };
 
 #endif /* __LIBBPF_LIBBPF_INTERNAL_H */
-- 
2.17.1


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

* [PATCH bpf-next 2/5] libbpf: refactor bpf_object__open APIs to use common opts
  2019-10-14 23:49 [PATCH bpf-next 0/5] Add CO-RE support for field existence relos Andrii Nakryiko
  2019-10-14 23:49 ` [PATCH bpf-next 1/5] libbpf: update BTF reloc support to latest Clang format Andrii Nakryiko
@ 2019-10-14 23:49 ` Andrii Nakryiko
  2019-10-14 23:49 ` [PATCH bpf-next 3/5] libbpf: add support for field existance CO-RE relocation Andrii Nakryiko
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Andrii Nakryiko @ 2019-10-14 23:49 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Refactor all the various bpf_object__open variations to ultimately
specify common bpf_object_open_opts struct. This makes it easy to keep
extending this common struct w/ extra parameters without having to
update all the legacy APIs.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/libbpf.c | 71 +++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 36 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index d6c7699de405..db3308b91806 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1322,9 +1322,9 @@ static int bpf_object__init_user_btf_maps(struct bpf_object *obj, bool strict)
 	return 0;
 }
 
-static int bpf_object__init_maps(struct bpf_object *obj, int flags)
+static int bpf_object__init_maps(struct bpf_object *obj, bool relaxed_maps)
 {
-	bool strict = !(flags & MAPS_RELAX_COMPAT);
+	bool strict = !relaxed_maps;
 	int err;
 
 	err = bpf_object__init_user_maps(obj, strict);
@@ -1521,7 +1521,7 @@ static int bpf_object__sanitize_and_load_btf(struct bpf_object *obj)
 	return 0;
 }
 
-static int bpf_object__elf_collect(struct bpf_object *obj, int flags)
+static int bpf_object__elf_collect(struct bpf_object *obj, bool relaxed_maps)
 {
 	Elf *elf = obj->efile.elf;
 	GElf_Ehdr *ep = &obj->efile.ehdr;
@@ -1652,7 +1652,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj, int flags)
 	}
 	err = bpf_object__init_btf(obj, btf_data, btf_ext_data);
 	if (!err)
-		err = bpf_object__init_maps(obj, flags);
+		err = bpf_object__init_maps(obj, relaxed_maps);
 	if (!err)
 		err = bpf_object__sanitize_and_load_btf(obj);
 	if (!err)
@@ -3554,24 +3554,45 @@ bpf_object__load_progs(struct bpf_object *obj, int log_level)
 
 static struct bpf_object *
 __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
-		   const char *obj_name, int flags)
+		   struct bpf_object_open_opts *opts)
 {
 	struct bpf_object *obj;
+	const char *obj_name;
+	char tmp_name[64];
+	bool relaxed_maps;
 	int err;
 
 	if (elf_version(EV_CURRENT) == EV_NONE) {
-		pr_warning("failed to init libelf for %s\n", path);
+		pr_warning("failed to init libelf for %s\n",
+			   path ? : "(mem buf)");
 		return ERR_PTR(-LIBBPF_ERRNO__LIBELF);
 	}
 
+	if (!OPTS_VALID(opts, bpf_object_open_opts))
+		return ERR_PTR(-EINVAL);
+
+	obj_name = OPTS_GET(opts, object_name, path);
+	if (obj_buf) {
+		if (!obj_name) {
+			snprintf(tmp_name, sizeof(tmp_name), "%lx-%lx",
+				 (unsigned long)obj_buf,
+				 (unsigned long)obj_buf_sz);
+			obj_name = tmp_name;
+		}
+		path = obj_name;
+		pr_debug("loading object '%s' from buffer\n", obj_name);
+	}
+
 	obj = bpf_object__new(path, obj_buf, obj_buf_sz, obj_name);
 	if (IS_ERR(obj))
 		return obj;
 
+	relaxed_maps = OPTS_GET(opts, relaxed_maps, false);
+
 	CHECK_ERR(bpf_object__elf_init(obj), err, out);
 	CHECK_ERR(bpf_object__check_endianness(obj), err, out);
 	CHECK_ERR(bpf_object__probe_caps(obj), err, out);
-	CHECK_ERR(bpf_object__elf_collect(obj, flags), err, out);
+	CHECK_ERR(bpf_object__elf_collect(obj, relaxed_maps), err, out);
 	CHECK_ERR(bpf_object__collect_reloc(obj), err, out);
 
 	bpf_object__elf_finish(obj);
@@ -3584,13 +3605,16 @@ __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
 static struct bpf_object *
 __bpf_object__open_xattr(struct bpf_object_open_attr *attr, int flags)
 {
+	LIBBPF_OPTS(bpf_object_open_opts, opts,
+		.relaxed_maps = flags & MAPS_RELAX_COMPAT,
+	);
+
 	/* param validation */
 	if (!attr->file)
 		return NULL;
 
 	pr_debug("loading %s\n", attr->file);
-
-	return __bpf_object__open(attr->file, NULL, 0, NULL, flags);
+	return __bpf_object__open(attr->file, NULL, 0, &opts);
 }
 
 struct bpf_object *bpf_object__open_xattr(struct bpf_object_open_attr *attr)
@@ -3611,47 +3635,22 @@ struct bpf_object *bpf_object__open(const char *path)
 struct bpf_object *
 bpf_object__open_file(const char *path, struct bpf_object_open_opts *opts)
 {
-	const char *obj_name;
-	bool relaxed_maps;
-
-	if (!OPTS_VALID(opts, bpf_object_open_opts))
-		return ERR_PTR(-EINVAL);
 	if (!path)
 		return ERR_PTR(-EINVAL);
 
 	pr_debug("loading %s\n", path);
 
-	obj_name = OPTS_GET(opts, object_name, path);
-	relaxed_maps = OPTS_GET(opts, relaxed_maps, false);
-	return __bpf_object__open(path, NULL, 0, obj_name,
-				  relaxed_maps ? MAPS_RELAX_COMPAT : 0);
+	return __bpf_object__open(path, NULL, 0, opts);
 }
 
 struct bpf_object *
 bpf_object__open_mem(const void *obj_buf, size_t obj_buf_sz,
 		     struct bpf_object_open_opts *opts)
 {
-	char tmp_name[64];
-	const char *obj_name;
-	bool relaxed_maps;
-
-	if (!OPTS_VALID(opts, bpf_object_open_opts))
-		return ERR_PTR(-EINVAL);
 	if (!obj_buf || obj_buf_sz == 0)
 		return ERR_PTR(-EINVAL);
 
-	obj_name = OPTS_GET(opts, object_name, NULL);
-	if (!obj_name) {
-		snprintf(tmp_name, sizeof(tmp_name), "%lx-%lx",
-			 (unsigned long)obj_buf,
-			 (unsigned long)obj_buf_sz);
-		obj_name = tmp_name;
-	}
-	pr_debug("loading object '%s' from buffer\n", obj_name);
-
-	relaxed_maps = OPTS_GET(opts, relaxed_maps, false);
-	return __bpf_object__open(obj_name, obj_buf, obj_buf_sz, obj_name,
-				  relaxed_maps ? MAPS_RELAX_COMPAT : 0);
+	return __bpf_object__open(NULL, obj_buf, obj_buf_sz, opts);
 }
 
 struct bpf_object *
-- 
2.17.1


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

* [PATCH bpf-next 3/5] libbpf: add support for field existance CO-RE relocation
  2019-10-14 23:49 [PATCH bpf-next 0/5] Add CO-RE support for field existence relos Andrii Nakryiko
  2019-10-14 23:49 ` [PATCH bpf-next 1/5] libbpf: update BTF reloc support to latest Clang format Andrii Nakryiko
  2019-10-14 23:49 ` [PATCH bpf-next 2/5] libbpf: refactor bpf_object__open APIs to use common opts Andrii Nakryiko
@ 2019-10-14 23:49 ` Andrii Nakryiko
  2019-10-14 23:49 ` [PATCH bpf-next 4/5] libbpf: add BPF-side definitions of supported field relocation kinds Andrii Nakryiko
  2019-10-14 23:49 ` [PATCH bpf-next 5/5] selftests/bpf: add field existence CO-RE relocs tests Andrii Nakryiko
  4 siblings, 0 replies; 8+ messages in thread
From: Andrii Nakryiko @ 2019-10-14 23:49 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Add support for BPF_FRK_EXISTS relocation kind to detect existence of
captured field in a destination BTF, allowing conditional logic to
handle incompatible differences between kernels.

Also introduce opt-in relaxed CO-RE relocation handling option, which
makes libbpf emit warning for failed relocations, but proceed with other
relocations. Instruction, for which relocation failed, is patched with
(u32)-1 value.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/libbpf.c | 74 +++++++++++++++++++++++++++++++++---------
 tools/lib/bpf/libbpf.h |  4 ++-
 2 files changed, 61 insertions(+), 17 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index db3308b91806..bebd9eb352ee 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -249,6 +249,7 @@ struct bpf_object {
 
 	bool loaded;
 	bool has_pseudo_calls;
+	bool relaxed_core_relocs;
 
 	/*
 	 * Information when doing elf related work. Only valid if fd
@@ -2771,26 +2772,54 @@ static int bpf_core_spec_match(struct bpf_core_spec *local_spec,
 
 /*
  * Patch relocatable BPF instruction.
- * Expected insn->imm value is provided for validation, as well as the new
- * relocated value.
+ *
+ * Patched value is determined by relocation kind and target specification.
+ * For field existence relocation target spec will be NULL if field is not
+ * found.
+ * Expected insn->imm value is determined using relocation kind and local
+ * spec, and is checked before patching instruction. If actual insn->imm value
+ * is wrong, bail out with error.
  *
  * Currently three kinds of BPF instructions are supported:
  * 1. rX = <imm> (assignment with immediate operand);
  * 2. rX += <imm> (arithmetic operations with immediate operand);
- * 3. *(rX) = <imm> (indirect memory assignment with immediate operand).
- *
- * If actual insn->imm value is wrong, bail out.
  */
-static int bpf_core_reloc_insn(struct bpf_program *prog, int insn_off,
-			       __u32 orig_off, __u32 new_off)
+static int bpf_core_reloc_insn(struct bpf_program *prog,
+			       const struct bpf_field_reloc *relo,
+			       const struct bpf_core_spec *local_spec,
+			       const struct bpf_core_spec *targ_spec)
 {
+	__u32 orig_val, new_val;
 	struct bpf_insn *insn;
 	int insn_idx;
 	__u8 class;
 
-	if (insn_off % sizeof(struct bpf_insn))
+	if (relo->insn_off % sizeof(struct bpf_insn))
 		return -EINVAL;
-	insn_idx = insn_off / sizeof(struct bpf_insn);
+	insn_idx = relo->insn_off / sizeof(struct bpf_insn);
+
+	switch (relo->kind) {
+	case BPF_FRK_BYTE_OFFSET:
+		orig_val = local_spec->offset;
+		if (targ_spec) {
+			new_val = targ_spec->offset;
+		} else {
+			pr_warning("prog '%s': patching insn #%d w/ failed reloc, imm %d -> %d\n",
+				   bpf_program__title(prog, false), insn_idx,
+				   orig_val, -1);
+			new_val = (__u32)-1;
+		}
+		break;
+	case BPF_FRK_EXISTS:
+		orig_val = 1; /* can't generate EXISTS relo w/o local field */
+		new_val = targ_spec ? 1 : 0;
+		break;
+	default:
+		pr_warning("prog '%s': unknown relo %d at insn #%d'\n",
+			   bpf_program__title(prog, false),
+			   relo->kind, insn_idx);
+		return -EINVAL;
+	}
 
 	insn = &prog->insns[insn_idx];
 	class = BPF_CLASS(insn->code);
@@ -2798,12 +2827,12 @@ static int bpf_core_reloc_insn(struct bpf_program *prog, int insn_off,
 	if (class == BPF_ALU || class == BPF_ALU64) {
 		if (BPF_SRC(insn->code) != BPF_K)
 			return -EINVAL;
-		if (insn->imm != orig_off)
+		if (insn->imm != orig_val)
 			return -EINVAL;
-		insn->imm = new_off;
+		insn->imm = new_val;
 		pr_debug("prog '%s': patched insn #%d (ALU/ALU64) imm %d -> %d\n",
 			 bpf_program__title(prog, false),
-			 insn_idx, orig_off, new_off);
+			 insn_idx, orig_val, new_val);
 	} else {
 		pr_warning("prog '%s': trying to relocate unrecognized insn #%d, code:%x, src:%x, dst:%x, off:%x, imm:%x\n",
 			   bpf_program__title(prog, false),
@@ -2811,6 +2840,7 @@ static int bpf_core_reloc_insn(struct bpf_program *prog, int insn_off,
 			   insn->off, insn->imm);
 		return -EINVAL;
 	}
+
 	return 0;
 }
 
@@ -3087,15 +3117,26 @@ static int bpf_core_reloc_field(struct bpf_program *prog,
 		cand_ids->data[j++] = cand_spec.spec[0].type_id;
 	}
 
-	cand_ids->len = j;
-	if (cand_ids->len == 0) {
+	/*
+	 * For BPF_FRK_EXISTS relo or when relaxed CO-RE reloc mode is
+	 * requested, it's expected that we might not find any candidates.
+	 * In this case, if field wasn't found in any candidate, the list of
+	 * candidates shouldn't change at all, we'll just handle relocating
+	 * appropriately, depending on relo's kind.
+	 */
+	if (j > 0)
+		cand_ids->len = j;
+
+	if (j == 0 && !prog->obj->relaxed_core_relocs &&
+	    relo->kind != BPF_FRK_EXISTS) {
 		pr_warning("prog '%s': relo #%d: no matching targets found for [%d] %s + %s\n",
 			   prog_name, relo_idx, local_id, local_name, spec_str);
 		return -ESRCH;
 	}
 
-	err = bpf_core_reloc_insn(prog, relo->insn_off,
-				  local_spec.offset, targ_spec.offset);
+	/* bpf_core_reloc_insn should know how to handle missing targ_spec */
+	err = bpf_core_reloc_insn(prog, relo, &local_spec,
+				  j ? &targ_spec : NULL);
 	if (err) {
 		pr_warning("prog '%s': relo #%d: failed to patch insn at offset %d: %d\n",
 			   prog_name, relo_idx, relo->insn_off, err);
@@ -3587,6 +3628,7 @@ __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
 	if (IS_ERR(obj))
 		return obj;
 
+	obj->relaxed_core_relocs = OPTS_GET(opts, relaxed_core_relocs, false);
 	relaxed_maps = OPTS_GET(opts, relaxed_maps, false);
 
 	CHECK_ERR(bpf_object__elf_init(obj), err, out);
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 667e6853e51f..53ce212764e0 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -96,8 +96,10 @@ struct bpf_object_open_opts {
 	const char *object_name;
 	/* parse map definitions non-strictly, allowing extra attributes/data */
 	bool relaxed_maps;
+	/* process CO-RE relocations non-strictly, allowing them to fail */
+	bool relaxed_core_relocs;
 };
-#define bpf_object_open_opts__last_field relaxed_maps
+#define bpf_object_open_opts__last_field relaxed_core_relocs
 
 LIBBPF_API struct bpf_object *bpf_object__open(const char *path);
 LIBBPF_API struct bpf_object *
-- 
2.17.1


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

* [PATCH bpf-next 4/5] libbpf: add BPF-side definitions of supported field relocation kinds
  2019-10-14 23:49 [PATCH bpf-next 0/5] Add CO-RE support for field existence relos Andrii Nakryiko
                   ` (2 preceding siblings ...)
  2019-10-14 23:49 ` [PATCH bpf-next 3/5] libbpf: add support for field existance CO-RE relocation Andrii Nakryiko
@ 2019-10-14 23:49 ` Andrii Nakryiko
  2019-10-14 23:49 ` [PATCH bpf-next 5/5] selftests/bpf: add field existence CO-RE relocs tests Andrii Nakryiko
  4 siblings, 0 replies; 8+ messages in thread
From: Andrii Nakryiko @ 2019-10-14 23:49 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Add enum definition for Clang's __builtin_preserve_field_info()
second argument (info_kind). Currently only byte offset and existence
are supported. Corresponding Clang changes introducing this built-in can
be found at [0]

  [0] https://reviews.llvm.org/D67980

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/bpf_core_read.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/lib/bpf/bpf_core_read.h b/tools/lib/bpf/bpf_core_read.h
index 4daf04c25493..5a5cc88621a9 100644
--- a/tools/lib/bpf/bpf_core_read.h
+++ b/tools/lib/bpf/bpf_core_read.h
@@ -2,6 +2,18 @@
 #ifndef __BPF_CORE_READ_H__
 #define __BPF_CORE_READ_H__
 
+/* enum bpf_field_info_kind is passed as a second argument into
+ * __builtin_preserve_field_info() built-in to get a specific aspect of
+ * a field, captured as a first argument. __builtin_preserve_field_info(field,
+ * info_kind) returns __u32 integer and produces BTF field relocation, which
+ * is understood and processed by libbpf during BPF object loading. See
+ * selftests/bpf for examples.
+ */
+enum bpf_field_info_kind {
+	BPF_FI_BYTE_OFFSET = 0,	  /* field byte offset */
+	BPF_FI_EXISTS = 2,	  /* whether field exists in target kernel */
+};
+
 /*
  * bpf_core_read() abstracts away bpf_probe_read() call and captures offset
  * relocation for source address using __builtin_preserve_access_index()
-- 
2.17.1


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

* [PATCH bpf-next 5/5] selftests/bpf: add field existence CO-RE relocs tests
  2019-10-14 23:49 [PATCH bpf-next 0/5] Add CO-RE support for field existence relos Andrii Nakryiko
                   ` (3 preceding siblings ...)
  2019-10-14 23:49 ` [PATCH bpf-next 4/5] libbpf: add BPF-side definitions of supported field relocation kinds Andrii Nakryiko
@ 2019-10-14 23:49 ` Andrii Nakryiko
  4 siblings, 0 replies; 8+ messages in thread
From: Andrii Nakryiko @ 2019-10-14 23:49 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Add a bunch of tests validating CO-RE is handling field existence
relocation. Relaxed CO-RE relocation mode is activated for these new
tests to prevent libbpf from rejecting BPF object for no-match
relocation, even though test BPF program is not going to use that
relocation, if field is missing.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 .../selftests/bpf/prog_tests/core_reloc.c     | 76 +++++++++++++++++-
 .../bpf/progs/btf__core_reloc_existence.c     |  3 +
 ...ore_reloc_existence___err_wrong_arr_kind.c |  3 +
 ...loc_existence___err_wrong_arr_value_type.c |  3 +
 ...ore_reloc_existence___err_wrong_int_kind.c |  3 +
 ..._core_reloc_existence___err_wrong_int_sz.c |  3 +
 ...ore_reloc_existence___err_wrong_int_type.c |  3 +
 ..._reloc_existence___err_wrong_struct_type.c |  3 +
 .../btf__core_reloc_existence___minimal.c     |  3 +
 .../selftests/bpf/progs/core_reloc_types.h    | 56 +++++++++++++
 .../bpf/progs/test_core_reloc_existence.c     | 79 +++++++++++++++++++
 11 files changed, 233 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_existence.c
 create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_arr_kind.c
 create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_arr_value_type.c
 create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_int_kind.c
 create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_int_sz.c
 create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_int_type.c
 create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_struct_type.c
 create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_existence___minimal.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_core_reloc_existence.c

diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
index 21a0dff66241..7e2f5b4bf7f3 100644
--- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c
+++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
@@ -174,6 +174,21 @@
 	.fails = true,							\
 }
 
+#define EXISTENCE_DATA(struct_name) STRUCT_TO_CHAR_PTR(struct_name) {	\
+	.a = 42,							\
+}
+
+#define EXISTENCE_CASE_COMMON(name)					\
+	.case_name = #name,						\
+	.bpf_obj_file = "test_core_reloc_existence.o",			\
+	.btf_src_file = "btf__core_reloc_" #name ".o",			\
+	.relaxed_core_relocs = true					\
+
+#define EXISTENCE_ERR_CASE(name) {					\
+	EXISTENCE_CASE_COMMON(name),					\
+	.fails = true,							\
+}
+
 struct core_reloc_test_case {
 	const char *case_name;
 	const char *bpf_obj_file;
@@ -183,6 +198,7 @@ struct core_reloc_test_case {
 	const char *output;
 	int output_len;
 	bool fails;
+	bool relaxed_core_relocs;
 };
 
 static struct core_reloc_test_case test_cases[] = {
@@ -283,6 +299,59 @@ static struct core_reloc_test_case test_cases[] = {
 		},
 		.output_len = sizeof(struct core_reloc_misc_output),
 	},
+
+	/* validate field existence checks */
+	{
+		EXISTENCE_CASE_COMMON(existence),
+		.input = STRUCT_TO_CHAR_PTR(core_reloc_existence) {
+			.a = 1,
+			.b = 2,
+			.c = 3,
+			.arr = { 4 },
+			.s = { .x = 5 },
+		},
+		.input_len = sizeof(struct core_reloc_existence),
+		.output = STRUCT_TO_CHAR_PTR(core_reloc_existence_output) {
+			.a_exists = 1,
+			.b_exists = 1,
+			.c_exists = 1,
+			.arr_exists = 1,
+			.s_exists = 1,
+			.a_value = 1,
+			.b_value = 2,
+			.c_value = 3,
+			.arr_value = 4,
+			.s_value = 5,
+		},
+		.output_len = sizeof(struct core_reloc_existence_output),
+	},
+	{
+		EXISTENCE_CASE_COMMON(existence___minimal),
+		.input = STRUCT_TO_CHAR_PTR(core_reloc_existence___minimal) {
+			.a = 42,
+		},
+		.input_len = sizeof(struct core_reloc_existence),
+		.output = STRUCT_TO_CHAR_PTR(core_reloc_existence_output) {
+			.a_exists = 1,
+			.b_exists = 0,
+			.c_exists = 0,
+			.arr_exists = 0,
+			.s_exists = 0,
+			.a_value = 42,
+			.b_value = 0xff000002u,
+			.c_value = 0xff000003u,
+			.arr_value = 0xff000004u,
+			.s_value = 0xff000005u,
+		},
+		.output_len = sizeof(struct core_reloc_existence_output),
+	},
+
+	EXISTENCE_ERR_CASE(existence__err_int_sz),
+	EXISTENCE_ERR_CASE(existence__err_int_type),
+	EXISTENCE_ERR_CASE(existence__err_int_kind),
+	EXISTENCE_ERR_CASE(existence__err_arr_kind),
+	EXISTENCE_ERR_CASE(existence__err_arr_value_type),
+	EXISTENCE_ERR_CASE(existence__err_struct_type),
 };
 
 struct data {
@@ -305,11 +374,14 @@ void test_core_reloc(void)
 
 	for (i = 0; i < ARRAY_SIZE(test_cases); i++) {
 		test_case = &test_cases[i];
-
 		if (!test__start_subtest(test_case->case_name))
 			continue;
 
-		obj = bpf_object__open(test_case->bpf_obj_file);
+		LIBBPF_OPTS(bpf_object_open_opts, opts,
+			.relaxed_core_relocs = test_case->relaxed_core_relocs,
+		);
+
+		obj = bpf_object__open_file(test_case->bpf_obj_file, &opts);
 		if (CHECK(IS_ERR_OR_NULL(obj), "obj_open",
 			  "failed to open '%s': %ld\n",
 			  test_case->bpf_obj_file, PTR_ERR(obj)))
diff --git a/tools/testing/selftests/bpf/progs/btf__core_reloc_existence.c b/tools/testing/selftests/bpf/progs/btf__core_reloc_existence.c
new file mode 100644
index 000000000000..0b62315ad46c
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/btf__core_reloc_existence.c
@@ -0,0 +1,3 @@
+#include "core_reloc_types.h"
+
+void f(struct core_reloc_existence x) {}
diff --git a/tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_arr_kind.c b/tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_arr_kind.c
new file mode 100644
index 000000000000..dd0ffa518f36
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_arr_kind.c
@@ -0,0 +1,3 @@
+#include "core_reloc_types.h"
+
+void f(struct core_reloc_existence___err_wrong_arr_kind x) {}
diff --git a/tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_arr_value_type.c b/tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_arr_value_type.c
new file mode 100644
index 000000000000..bc83372088ad
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_arr_value_type.c
@@ -0,0 +1,3 @@
+#include "core_reloc_types.h"
+
+void f(struct core_reloc_existence___err_wrong_arr_value_type x) {}
diff --git a/tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_int_kind.c b/tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_int_kind.c
new file mode 100644
index 000000000000..917bec41be08
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_int_kind.c
@@ -0,0 +1,3 @@
+#include "core_reloc_types.h"
+
+void f(struct core_reloc_existence___err_wrong_int_kind x) {}
diff --git a/tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_int_sz.c b/tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_int_sz.c
new file mode 100644
index 000000000000..6ec7e6ec1c91
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_int_sz.c
@@ -0,0 +1,3 @@
+#include "core_reloc_types.h"
+
+void f(struct core_reloc_existence___err_wrong_int_sz x) {}
diff --git a/tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_int_type.c b/tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_int_type.c
new file mode 100644
index 000000000000..7bbcacf2b0d1
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_int_type.c
@@ -0,0 +1,3 @@
+#include "core_reloc_types.h"
+
+void f(struct core_reloc_existence___err_wrong_int_type x) {}
diff --git a/tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_struct_type.c b/tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_struct_type.c
new file mode 100644
index 000000000000..f384dd38ec70
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/btf__core_reloc_existence___err_wrong_struct_type.c
@@ -0,0 +1,3 @@
+#include "core_reloc_types.h"
+
+void f(struct core_reloc_existence___err_wrong_struct_type x) {}
diff --git a/tools/testing/selftests/bpf/progs/btf__core_reloc_existence___minimal.c b/tools/testing/selftests/bpf/progs/btf__core_reloc_existence___minimal.c
new file mode 100644
index 000000000000..aec2dec20e90
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/btf__core_reloc_existence___minimal.c
@@ -0,0 +1,3 @@
+#include "core_reloc_types.h"
+
+void f(struct core_reloc_existence___minimal x) {}
diff --git a/tools/testing/selftests/bpf/progs/core_reloc_types.h b/tools/testing/selftests/bpf/progs/core_reloc_types.h
index 9a6bdeb4894c..ad763ec0ba8f 100644
--- a/tools/testing/selftests/bpf/progs/core_reloc_types.h
+++ b/tools/testing/selftests/bpf/progs/core_reloc_types.h
@@ -674,3 +674,59 @@ struct core_reloc_misc_extensible {
 	int c;
 	int d;
 };
+
+/*
+ * EXISTENCE
+ */
+struct core_reloc_existence_output {
+	int a_exists;
+	int a_value;
+	int b_exists;
+	int b_value;
+	int c_exists;
+	int c_value;
+	int arr_exists;
+	int arr_value;
+	int s_exists;
+	int s_value;
+};
+
+struct core_reloc_existence {
+	int a;
+	struct {
+		int b;
+	};
+	int c;
+	int arr[1];
+	struct {
+		int x;
+	} s;
+};
+
+struct core_reloc_existence___minimal {
+	int a;
+};
+
+struct core_reloc_existence___err_wrong_int_sz {
+	short a;
+};
+
+struct core_reloc_existence___err_wrong_int_type {
+	int b[1];
+};
+
+struct core_reloc_existence___err_wrong_int_kind {
+	struct{ int x; } c;
+};
+
+struct core_reloc_existence___err_wrong_arr_kind {
+	int arr;
+};
+
+struct core_reloc_existence___err_wrong_arr_value_type {
+	short arr[1];
+};
+
+struct core_reloc_existence___err_wrong_struct_type {
+	int s;
+};
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_existence.c b/tools/testing/selftests/bpf/progs/test_core_reloc_existence.c
new file mode 100644
index 000000000000..fdf0fdc361e0
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_existence.c
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2019 Facebook
+
+#include <linux/bpf.h>
+#include <stdint.h>
+#include "bpf_helpers.h"
+#include "bpf_core_read.h"
+
+char _license[] SEC("license") = "GPL";
+
+static volatile struct data {
+	char in[256];
+	char out[256];
+} data;
+
+struct core_reloc_existence_output {
+	int a_exists;
+	int a_value;
+	int b_exists;
+	int b_value;
+	int c_exists;
+	int c_value;
+	int arr_exists;
+	int arr_value;
+	int s_exists;
+	int s_value;
+};
+
+struct core_reloc_existence {
+	struct {
+		int x;
+	} s;
+	int arr[1];
+	int a;
+	struct {
+		int b;
+	};
+	int c;
+};
+
+SEC("raw_tracepoint/sys_enter")
+int test_core_existence(void *ctx)
+{
+	struct core_reloc_existence *in = (void *)&data.in;
+	struct core_reloc_existence_output *out = (void *)&data.out;
+
+	out->a_exists = __builtin_preserve_field_info(in->a, BPF_FI_EXISTS);
+	if (__builtin_preserve_field_info(in->a, BPF_FI_EXISTS))
+		out->a_value = BPF_CORE_READ(in, a);
+	else
+		out->a_value = 0xff000001u;
+
+	out->b_exists = __builtin_preserve_field_info(in->b, BPF_FI_EXISTS);
+	if (__builtin_preserve_field_info(in->b, BPF_FI_EXISTS))
+		out->b_value = BPF_CORE_READ(in, b);
+	else
+		out->b_value = 0xff000002u;
+
+	out->c_exists = __builtin_preserve_field_info(in->c, BPF_FI_EXISTS);
+	if (__builtin_preserve_field_info(in->c, BPF_FI_EXISTS))
+		out->c_value = BPF_CORE_READ(in, c);
+	else
+		out->c_value = 0xff000003u;
+
+	out->arr_exists = __builtin_preserve_field_info(in->arr, BPF_FI_EXISTS);
+	if (__builtin_preserve_field_info(in->arr, BPF_FI_EXISTS))
+		out->arr_value = BPF_CORE_READ(in, arr[0]);
+	else
+		out->arr_value = 0xff000004u;
+
+	out->s_exists = __builtin_preserve_field_info(in->s, BPF_FI_EXISTS);
+	if (__builtin_preserve_field_info(in->s, BPF_FI_EXISTS))
+		out->s_value = BPF_CORE_READ(in, s.x);
+	else
+		out->s_value = 0xff000005u;
+
+	return 0;
+}
+
-- 
2.17.1


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

* Re: [PATCH bpf-next 1/5] libbpf: update BTF reloc support to latest Clang format
  2019-10-14 23:49 ` [PATCH bpf-next 1/5] libbpf: update BTF reloc support to latest Clang format Andrii Nakryiko
@ 2019-10-15 15:15   ` Alexei Starovoitov
  2019-10-15 16:14     ` Andrii Nakryiko
  0 siblings, 1 reply; 8+ messages in thread
From: Alexei Starovoitov @ 2019-10-15 15:15 UTC (permalink / raw)
  To: Andrii Nakryiko, bpf, netdev, daniel; +Cc: andrii.nakryiko, Kernel Team

On 10/14/19 4:49 PM, Andrii Nakryiko wrote:
> BTF offset reloc was generalized in recent Clang into field relocation,
> capturing extra u32 field, specifying what aspect of captured field
> needs to be relocated. This changes .BTF.ext's record size for this
> relocation from 12 bytes to 16 bytes. Given these format changes
> happened in Clang before official released version, it's ok to not
> support outdated 12-byte record size w/o breaking ABI.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
...> -/* The minimum bpf_offset_reloc checked by the loader
> +/* bpf_field_reloc_kind encodes which aspect of captured field has to be
> + * adjusted by relocations. Currently supported values are:
> + *   - BPF_FI_BYTE_OFFSET: field offset (in bytes);
> + *   - BPF_FI_EXISTS: field existence (1 if field exists, 0 - otherwise);
> + */
> +enum bpf_field_reloc_kind {
> +	BPF_FRK_BYTE_OFFSET = 0,
> +	BPF_FRK_EXISTS = 2,
> +};

Comment above doesn't match the enum.
Also in patch 4 :
+enum bpf_field_info_kind {
+	BPF_FI_BYTE_OFFSET = 0,	  /* field byte offset */
+	BPF_FI_EXISTS = 2,	  /* whether field exists in target kernel */
+};

Do you expect that .btf.ext encoding to be different from
argument passed into __builtin_field_info() ?
May be better to design the interface that it always matches
and keep single enum for both?
I don't like either FRK or FI abbreviations.
May be
BPF_FIELD_BYTE_OFFSET
BPF_FIELD_EXISTS ?

These constants would need to be exposed in bpf_core_read.h and
will become uapi as soon as we release next libbpf at
the end of this bpf-next cycle. At that point llvm side would have
to stick to these constants as well.
It would have to understand them as arguments and use the same in .btf.ext.
Does it make sense?


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

* Re: [PATCH bpf-next 1/5] libbpf: update BTF reloc support to latest Clang format
  2019-10-15 15:15   ` Alexei Starovoitov
@ 2019-10-15 16:14     ` Andrii Nakryiko
  0 siblings, 0 replies; 8+ messages in thread
From: Andrii Nakryiko @ 2019-10-15 16:14 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: Andrii Nakryiko, bpf, netdev, daniel, Kernel Team

On Tue, Oct 15, 2019 at 8:15 AM Alexei Starovoitov <ast@fb.com> wrote:
>
> On 10/14/19 4:49 PM, Andrii Nakryiko wrote:
> > BTF offset reloc was generalized in recent Clang into field relocation,
> > capturing extra u32 field, specifying what aspect of captured field
> > needs to be relocated. This changes .BTF.ext's record size for this
> > relocation from 12 bytes to 16 bytes. Given these format changes
> > happened in Clang before official released version, it's ok to not
> > support outdated 12-byte record size w/o breaking ABI.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ...> -/* The minimum bpf_offset_reloc checked by the loader
> > +/* bpf_field_reloc_kind encodes which aspect of captured field has to be
> > + * adjusted by relocations. Currently supported values are:
> > + *   - BPF_FI_BYTE_OFFSET: field offset (in bytes);
> > + *   - BPF_FI_EXISTS: field existence (1 if field exists, 0 - otherwise);
> > + */
> > +enum bpf_field_reloc_kind {
> > +     BPF_FRK_BYTE_OFFSET = 0,
> > +     BPF_FRK_EXISTS = 2,
> > +};
>
> Comment above doesn't match the enum.

he-he, you can see that I went back and forth with names :)

> Also in patch 4 :
> +enum bpf_field_info_kind {
> +       BPF_FI_BYTE_OFFSET = 0,   /* field byte offset */
> +       BPF_FI_EXISTS = 2,        /* whether field exists in target kernel */
> +};
>
> Do you expect that .btf.ext encoding to be different from
> argument passed into __builtin_field_info() ?
> May be better to design the interface that it always matches
> and keep single enum for both?
> I don't like either FRK or FI abbreviations.
> May be
> BPF_FIELD_BYTE_OFFSET
> BPF_FIELD_EXISTS ?
>
> These constants would need to be exposed in bpf_core_read.h and
> will become uapi as soon as we release next libbpf at
> the end of this bpf-next cycle. At that point llvm side would have
> to stick to these constants as well.
> It would have to understand them as arguments and use the same in .btf.ext.
> Does it make sense?

yeah, it does. My thinking was that we already have two built-ins that
share the same BTF relocation (__builtin_preserve_access_index() and
__builtin_preserve_field_info()), so I figured it might be worth-while
to not couple low-level relocation and bulit-in parameters together,
because it might be the case in the future where we'll be emitting
multiple relos from single built-in or some other arrangement.

But I think it's ok to couple enum definitions for now. In the end,
it's just numbers, so if we ever need to diverge, it will be possible.
The only exposed constants are those that are in bpf_core_read.h and
are only supposed to be passed into __builtin_preserve_field_info().
libbpf's enum bpf_field_reloc_kind is internal, so we can change it
whenever we need to.

btw, I'm going to add bpf_core_field_exists() macro to
bpf_core_read.h, similar to bpf_core_read() macro, hope it's not very
controversial.

>

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

end of thread, other threads:[~2019-10-15 16:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-14 23:49 [PATCH bpf-next 0/5] Add CO-RE support for field existence relos Andrii Nakryiko
2019-10-14 23:49 ` [PATCH bpf-next 1/5] libbpf: update BTF reloc support to latest Clang format Andrii Nakryiko
2019-10-15 15:15   ` Alexei Starovoitov
2019-10-15 16:14     ` Andrii Nakryiko
2019-10-14 23:49 ` [PATCH bpf-next 2/5] libbpf: refactor bpf_object__open APIs to use common opts Andrii Nakryiko
2019-10-14 23:49 ` [PATCH bpf-next 3/5] libbpf: add support for field existance CO-RE relocation Andrii Nakryiko
2019-10-14 23:49 ` [PATCH bpf-next 4/5] libbpf: add BPF-side definitions of supported field relocation kinds Andrii Nakryiko
2019-10-14 23:49 ` [PATCH bpf-next 5/5] selftests/bpf: add field existence CO-RE relocs tests Andrii Nakryiko

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