bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 bpf-next 0/4] Add libbpf-provided extern variables support
@ 2019-12-14  1:47 Andrii Nakryiko
  2019-12-14  1:47 ` [PATCH v4 bpf-next 1/4] libbpf: extract internal map names into constants Andrii Nakryiko
                   ` (4 more replies)
  0 siblings, 5 replies; 21+ messages in thread
From: Andrii Nakryiko @ 2019-12-14  1:47 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

It's often important for BPF program to know kernel version or some specific
config values (e.g., CONFIG_HZ to convert jiffies to seconds) and change or
adjust program logic based on their values. As of today, any such need has to
be resolved by recompiling BPF program for specific kernel and kernel
configuration. In practice this is usually achieved by using BCC and its
embedded LLVM/Clang. With such set up #ifdef CONFIG_XXX and similar
compile-time constructs allow to deal with kernel varieties.

With CO-RE (Compile Once – Run Everywhere) approach, this is not an option,
unfortunately. All such logic variations have to be done as a normal
C language constructs (i.e., if/else, variables, etc), not a preprocessor
directives. This patch series add support for such advanced scenarios through
C extern variables. These extern variables will be recognized by libbpf and
supplied through extra .extern internal map, similarly to global data. This
.extern map is read-only, which allows BPF verifier to track its content
precisely as constants. That gives an opportunity to have pre-compiled BPF
program, which can potentially use BPF functionality (e.g., BPF helpers) or
kernel features (types, fields, etc), that are available only on a subset of
targeted kernels, while effectively eleminating (through verifier's dead code
detection) such unsupported functionality for other kernels (typically, older
versions). Patch #3 explicitly tests a scenario of using unsupported BPF
helper, to validate the approach.

This patch set heavily relies on BTF type information emitted by compiler for
each extern variable declaration. Based on specific types, libbpf does strict
checks of config data values correctness. See patch #1 for details.

Outline of the patch set:
- patch #1 does a small clean up of internal map names contants;
- patch #2 adds all of the libbpf internal machinery for externs support,
  including setting up BTF information for .extern data section;
- patch #3 adds support for .extern into BPF skeleton;
- patch #4 adds externs selftests, as well as enhances test_skeleton.c test to
  validate mmap()-ed .extern datasection functionality.

v3->v4:
- clean up copyrights and rebase onto latest skeleton patches (Alexei);

v2->v3:
- truncate too long strings (Alexei);
- clean ups, adding comments (Alexei);

v1->v2:
- use BTF type information for externs (Alexei);
- add strings support;
- add BPF skeleton support for .extern.

Andrii Nakryiko (4):
  libbpf: extract internal map names into constants
  libbpf: support libbpf-provided extern variables
  bpftool: generate externs datasec in BPF skeleton
  selftests/bpf: add tests for libbpf-provided externs

 include/uapi/linux/btf.h                      |   3 +-
 tools/bpf/bpftool/gen.c                       |   4 +
 tools/include/uapi/linux/btf.h                |   7 +-
 tools/lib/bpf/Makefile                        |  15 +-
 tools/lib/bpf/bpf_helpers.h                   |   9 +
 tools/lib/bpf/btf.c                           |   9 +-
 tools/lib/bpf/libbpf.c                        | 771 ++++++++++++++++--
 tools/lib/bpf/libbpf.h                        |  12 +-
 tools/testing/selftests/bpf/Makefile          |   2 +-
 .../selftests/bpf/prog_tests/core_extern.c    | 195 +++++
 .../selftests/bpf/prog_tests/skeleton.c       |  18 +-
 .../selftests/bpf/progs/test_core_extern.c    |  62 ++
 .../selftests/bpf/progs/test_skeleton.c       |   9 +
 13 files changed, 1035 insertions(+), 81 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/core_extern.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_core_extern.c

-- 
2.17.1


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

* [PATCH v4 bpf-next 1/4] libbpf: extract internal map names into constants
  2019-12-14  1:47 [PATCH v4 bpf-next 0/4] Add libbpf-provided extern variables support Andrii Nakryiko
@ 2019-12-14  1:47 ` Andrii Nakryiko
  2019-12-14  1:47 ` [PATCH v4 bpf-next 2/4] libbpf: support libbpf-provided extern variables Andrii Nakryiko
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 21+ messages in thread
From: Andrii Nakryiko @ 2019-12-14  1:47 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Instead of duplicating string literals, keep them in one place and consistent.

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

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index a1a902fa6e0c..f6c135869701 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -195,6 +195,10 @@ struct bpf_program {
 	__u32 prog_flags;
 };
 
+#define DATA_SEC ".data"
+#define BSS_SEC ".bss"
+#define RODATA_SEC ".rodata"
+
 enum libbpf_map_type {
 	LIBBPF_MAP_UNSPEC,
 	LIBBPF_MAP_DATA,
@@ -203,9 +207,9 @@ enum libbpf_map_type {
 };
 
 static const char * const libbpf_type_to_btf_name[] = {
-	[LIBBPF_MAP_DATA]	= ".data",
-	[LIBBPF_MAP_BSS]	= ".bss",
-	[LIBBPF_MAP_RODATA]	= ".rodata",
+	[LIBBPF_MAP_DATA]	= DATA_SEC,
+	[LIBBPF_MAP_BSS]	= BSS_SEC,
+	[LIBBPF_MAP_RODATA]	= RODATA_SEC,
 };
 
 struct bpf_map {
@@ -736,13 +740,13 @@ int bpf_object__section_size(const struct bpf_object *obj, const char *name,
 	*size = 0;
 	if (!name) {
 		return -EINVAL;
-	} else if (!strcmp(name, ".data")) {
+	} else if (!strcmp(name, DATA_SEC)) {
 		if (obj->efile.data)
 			*size = obj->efile.data->d_size;
-	} else if (!strcmp(name, ".bss")) {
+	} else if (!strcmp(name, BSS_SEC)) {
 		if (obj->efile.bss)
 			*size = obj->efile.bss->d_size;
-	} else if (!strcmp(name, ".rodata")) {
+	} else if (!strcmp(name, RODATA_SEC)) {
 		if (obj->efile.rodata)
 			*size = obj->efile.rodata->d_size;
 	} else {
@@ -1684,10 +1688,10 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
 						name, obj->path, cp);
 					return err;
 				}
-			} else if (strcmp(name, ".data") == 0) {
+			} else if (strcmp(name, DATA_SEC) == 0) {
 				obj->efile.data = data;
 				obj->efile.data_shndx = idx;
-			} else if (strcmp(name, ".rodata") == 0) {
+			} else if (strcmp(name, RODATA_SEC) == 0) {
 				obj->efile.rodata = data;
 				obj->efile.rodata_shndx = idx;
 			} else {
@@ -1717,7 +1721,8 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
 
 			obj->efile.reloc_sects[nr_sects].shdr = sh;
 			obj->efile.reloc_sects[nr_sects].data = data;
-		} else if (sh.sh_type == SHT_NOBITS && strcmp(name, ".bss") == 0) {
+		} else if (sh.sh_type == SHT_NOBITS &&
+			   strcmp(name, BSS_SEC) == 0) {
 			obj->efile.bss = data;
 			obj->efile.bss_shndx = idx;
 		} else {
-- 
2.17.1


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

* [PATCH v4 bpf-next 2/4] libbpf: support libbpf-provided extern variables
  2019-12-14  1:47 [PATCH v4 bpf-next 0/4] Add libbpf-provided extern variables support Andrii Nakryiko
  2019-12-14  1:47 ` [PATCH v4 bpf-next 1/4] libbpf: extract internal map names into constants Andrii Nakryiko
@ 2019-12-14  1:47 ` Andrii Nakryiko
  2019-12-14 12:50   ` Toke Høiland-Jørgensen
                     ` (2 more replies)
  2019-12-14  1:47 ` [PATCH v4 bpf-next 3/4] bpftool: generate externs datasec in BPF skeleton Andrii Nakryiko
                   ` (2 subsequent siblings)
  4 siblings, 3 replies; 21+ messages in thread
From: Andrii Nakryiko @ 2019-12-14  1:47 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Add support for extern variables, provided to BPF program by libbpf. Currently
the following extern variables are supported:
  - LINUX_KERNEL_VERSION; version of a kernel in which BPF program is
    executing, follows KERNEL_VERSION() macro convention, can be 4- and 8-byte
    long;
  - CONFIG_xxx values; a set of values of actual kernel config. Tristate,
    boolean, strings, and integer values are supported.

Set of possible values is determined by declared type of extern variable.
Supported types of variables are:
- Tristate values. Are represented as `enum libbpf_tristate`. Accepted values
  are **strictly** 'y', 'n', or 'm', which are represented as TRI_YES, TRI_NO,
  or TRI_MODULE, respectively.
- Boolean values. Are represented as bool (_Bool) types. Accepted values are
  'y' and 'n' only, turning into true/false values, respectively.
- Single-character values. Can be used both as a substritute for
  bool/tristate, or as a small-range integer:
  - 'y'/'n'/'m' are represented as is, as characters 'y', 'n', or 'm';
  - integers in a range [-128, 127] or [0, 255] (depending on signedness of
    char in target architecture) are recognized and represented with
    respective values of char type.
- Strings. String values are declared as fixed-length char arrays. String of
  up to that length will be accepted and put in first N bytes of char array,
  with the rest of bytes zeroed out. If config string value is longer than
  space alloted, it will be truncated and warning message emitted. Char array
  is always zero terminated. String literals in config have to be enclosed in
  double quotes, just like C-style string literals.
- Integers. 8-, 16-, 32-, and 64-bit integers are supported, both signed and
  unsigned variants. Libbpf enforces parsed config value to be in the
  supported range of corresponding integer type. Integers values in config can
  be:
  - decimal integers, with optional + and - signs;
  - hexadecimal integers, prefixed with 0x or 0X;
  - octal integers, starting with 0.

Config file itself is searched in /boot/config-$(uname -r) location with
fallback to /proc/config.gz, unless config path is specified explicitly
through bpf_object_open_opts' kernel_config_path option. Both gzipped and
plain text formats are supported. Libbpf adds explicit dependency on zlib
because of this, but this shouldn't be a problem, given libelf already depends
on zlib.

All detected extern variables, are put into a separate .extern internal map.
It, similarly to .rodata map, is marked as read-only from BPF program side, as
well as is frozen on load. This allows BPF verifier to track extern values as
constants and perform enhanced branch prediction and dead code elimination.
This can be relied upon for doing kernel version/feature detection and using
potentially unsupported field relocations or BPF helpers in a CO-RE-based BPF
program, while still having a single version of BPF program running on old and
new kernels. Selftests are validating this explicitly for unexisting BPF
helper.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 include/uapi/linux/btf.h             |   3 +-
 tools/include/uapi/linux/btf.h       |   7 +-
 tools/lib/bpf/Makefile               |  15 +-
 tools/lib/bpf/bpf_helpers.h          |   9 +
 tools/lib/bpf/btf.c                  |   9 +-
 tools/lib/bpf/libbpf.c               | 738 +++++++++++++++++++++++++--
 tools/lib/bpf/libbpf.h               |  12 +-
 tools/testing/selftests/bpf/Makefile |   2 +-
 8 files changed, 729 insertions(+), 66 deletions(-)

diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
index c02dec97e1ce..1a2898c482ee 100644
--- a/include/uapi/linux/btf.h
+++ b/include/uapi/linux/btf.h
@@ -142,7 +142,8 @@ struct btf_param {
 
 enum {
 	BTF_VAR_STATIC = 0,
-	BTF_VAR_GLOBAL_ALLOCATED,
+	BTF_VAR_GLOBAL_ALLOCATED = 1,
+	BTF_VAR_GLOBAL_EXTERN = 2,
 };
 
 /* BTF_KIND_VAR is followed by a single "struct btf_var" to describe
diff --git a/tools/include/uapi/linux/btf.h b/tools/include/uapi/linux/btf.h
index 63ae4a39e58b..1a2898c482ee 100644
--- a/tools/include/uapi/linux/btf.h
+++ b/tools/include/uapi/linux/btf.h
@@ -22,9 +22,9 @@ struct btf_header {
 };
 
 /* Max # of type identifier */
-#define BTF_MAX_TYPE	0x0000ffff
+#define BTF_MAX_TYPE	0x000fffff
 /* Max offset into the string section */
-#define BTF_MAX_NAME_OFFSET	0x0000ffff
+#define BTF_MAX_NAME_OFFSET	0x00ffffff
 /* Max # of struct/union/enum members or func args */
 #define BTF_MAX_VLEN	0xffff
 
@@ -142,7 +142,8 @@ struct btf_param {
 
 enum {
 	BTF_VAR_STATIC = 0,
-	BTF_VAR_GLOBAL_ALLOCATED,
+	BTF_VAR_GLOBAL_ALLOCATED = 1,
+	BTF_VAR_GLOBAL_EXTERN = 2,
 };
 
 /* BTF_KIND_VAR is followed by a single "struct btf_var" to describe
diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index defae23a0169..c3fdb2907b96 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -56,8 +56,8 @@ ifndef VERBOSE
 endif
 
 FEATURE_USER = .libbpf
-FEATURE_TESTS = libelf libelf-mmap bpf reallocarray
-FEATURE_DISPLAY = libelf bpf
+FEATURE_TESTS = libelf libelf-mmap zlib bpf reallocarray
+FEATURE_DISPLAY = libelf zlib bpf
 
 INCLUDES = -I. -I$(srctree)/tools/include -I$(srctree)/tools/arch/$(ARCH)/include/uapi -I$(srctree)/tools/include/uapi
 FEATURE_CHECK_CFLAGS-bpf = $(INCLUDES)
@@ -159,7 +159,7 @@ all: fixdep
 
 all_cmd: $(CMD_TARGETS) check
 
-$(BPF_IN_SHARED): force elfdep bpfdep bpf_helper_defs.h
+$(BPF_IN_SHARED): force elfdep zdep bpfdep bpf_helper_defs.h
 	@(test -f ../../include/uapi/linux/bpf.h -a -f ../../../include/uapi/linux/bpf.h && ( \
 	(diff -B ../../include/uapi/linux/bpf.h ../../../include/uapi/linux/bpf.h >/dev/null) || \
 	echo "Warning: Kernel ABI header at 'tools/include/uapi/linux/bpf.h' differs from latest version at 'include/uapi/linux/bpf.h'" >&2 )) || true
@@ -177,7 +177,7 @@ $(BPF_IN_SHARED): force elfdep bpfdep bpf_helper_defs.h
 	echo "Warning: Kernel ABI header at 'tools/include/uapi/linux/if_xdp.h' differs from latest version at 'include/uapi/linux/if_xdp.h'" >&2 )) || true
 	$(Q)$(MAKE) $(build)=libbpf OUTPUT=$(SHARED_OBJDIR) CFLAGS="$(CFLAGS) $(SHLIB_FLAGS)"
 
-$(BPF_IN_STATIC): force elfdep bpfdep bpf_helper_defs.h
+$(BPF_IN_STATIC): force elfdep zdep bpfdep bpf_helper_defs.h
 	$(Q)$(MAKE) $(build)=libbpf OUTPUT=$(STATIC_OBJDIR)
 
 bpf_helper_defs.h: $(srctree)/tools/include/uapi/linux/bpf.h
@@ -189,7 +189,7 @@ $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION)
 $(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN_SHARED)
 	$(QUIET_LINK)$(CC) $(LDFLAGS) \
 		--shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
-		-Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
+		-Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -lz -o $@
 	@ln -sf $(@F) $(OUTPUT)libbpf.so
 	@ln -sf $(@F) $(OUTPUT)libbpf.so.$(LIBBPF_MAJOR_VERSION)
 
@@ -277,12 +277,15 @@ clean:
 
 
 
-PHONY += force elfdep bpfdep cscope tags
+PHONY += force elfdep zdep bpfdep cscope tags
 force:
 
 elfdep:
 	@if [ "$(feature-libelf)" != "1" ]; then echo "No libelf found"; exit 1 ; fi
 
+zdep:
+	@if [ "$(feature-zlib)" != "1" ]; then echo "No zlib found"; exit 1 ; fi
+
 bpfdep:
 	@if [ "$(feature-bpf)" != "1" ]; then echo "BPF API too old"; exit 1 ; fi
 
diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
index 0c7d28292898..aa46700075e1 100644
--- a/tools/lib/bpf/bpf_helpers.h
+++ b/tools/lib/bpf/bpf_helpers.h
@@ -25,6 +25,9 @@
 #ifndef __always_inline
 #define __always_inline __attribute__((always_inline))
 #endif
+#ifndef __weak
+#define __weak __attribute__((weak))
+#endif
 
 /*
  * Helper structure used by eBPF C program
@@ -44,4 +47,10 @@ enum libbpf_pin_type {
 	LIBBPF_PIN_BY_NAME,
 };
 
+enum libbpf_tristate {
+	TRI_NO = 0,
+	TRI_YES = 1,
+	TRI_MODULE = 2,
+};
+
 #endif
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 84fe82f27bef..520021939d81 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -578,6 +578,12 @@ static int btf_fixup_datasec(struct bpf_object *obj, struct btf *btf,
 		return -ENOENT;
 	}
 
+	/* .extern datasec size and var offsets were set correctly during
+	 * extern collection step, so just skip straight to sorting variables
+	 */
+	if (t->size)
+		goto sort_vars;
+
 	ret = bpf_object__section_size(obj, name, &size);
 	if (ret || !size || (t->size && t->size != size)) {
 		pr_debug("Invalid size for section %s: %u bytes\n", name, size);
@@ -614,7 +620,8 @@ static int btf_fixup_datasec(struct bpf_object *obj, struct btf *btf,
 		vsi->offset = off;
 	}
 
-	qsort(t + 1, vars, sizeof(*vsi), compare_vsi_off);
+sort_vars:
+	qsort(btf_var_secinfos(t), vars, sizeof(*vsi), compare_vsi_off);
 	return 0;
 }
 
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index f6c135869701..26144706ba0b 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -44,6 +44,7 @@
 #include <tools/libc_compat.h>
 #include <libelf.h>
 #include <gelf.h>
+#include <zlib.h>
 
 #include "libbpf.h"
 #include "bpf.h"
@@ -139,6 +140,20 @@ struct bpf_capabilities {
 	__u32 array_mmap:1;
 };
 
+enum reloc_type {
+	RELO_LD64,
+	RELO_CALL,
+	RELO_DATA,
+	RELO_EXTERN,
+};
+
+struct reloc_desc {
+	enum reloc_type type;
+	int insn_idx;
+	int map_idx;
+	int sym_off;
+};
+
 /*
  * bpf_prog should be a better name but it has been used in
  * linux/filter.h.
@@ -157,16 +172,7 @@ struct bpf_program {
 	size_t insns_cnt, main_prog_cnt;
 	enum bpf_prog_type type;
 
-	struct reloc_desc {
-		enum {
-			RELO_LD64,
-			RELO_CALL,
-			RELO_DATA,
-		} type;
-		int insn_idx;
-		int map_idx;
-		int sym_off;
-	} *reloc_desc;
+	struct reloc_desc *reloc_desc;
 	int nr_reloc;
 	int log_level;
 
@@ -198,18 +204,21 @@ struct bpf_program {
 #define DATA_SEC ".data"
 #define BSS_SEC ".bss"
 #define RODATA_SEC ".rodata"
+#define EXTERN_SEC ".extern"
 
 enum libbpf_map_type {
 	LIBBPF_MAP_UNSPEC,
 	LIBBPF_MAP_DATA,
 	LIBBPF_MAP_BSS,
 	LIBBPF_MAP_RODATA,
+	LIBBPF_MAP_EXTERN,
 };
 
 static const char * const libbpf_type_to_btf_name[] = {
 	[LIBBPF_MAP_DATA]	= DATA_SEC,
 	[LIBBPF_MAP_BSS]	= BSS_SEC,
 	[LIBBPF_MAP_RODATA]	= RODATA_SEC,
+	[LIBBPF_MAP_EXTERN]	= EXTERN_SEC,
 };
 
 struct bpf_map {
@@ -231,6 +240,28 @@ struct bpf_map {
 	bool reused;
 };
 
+enum extern_type {
+	EXT_UNKNOWN,
+	EXT_CHAR,
+	EXT_BOOL,
+	EXT_INT,
+	EXT_TRISTATE,
+	EXT_CHAR_ARR,
+};
+
+struct extern_desc {
+	const char *name;
+	int sym_idx;
+	int btf_id;
+	enum extern_type type;
+	int sz;
+	int align;
+	int data_off;
+	bool is_signed;
+	bool is_weak;
+	bool is_set;
+};
+
 static LIST_HEAD(bpf_objects_list);
 
 struct bpf_object {
@@ -244,6 +275,11 @@ struct bpf_object {
 	size_t nr_maps;
 	size_t maps_cap;
 
+	char *kconfig_path;
+	struct extern_desc *externs;
+	int nr_extern;
+	int extern_map_idx;
+
 	bool loaded;
 	bool has_pseudo_calls;
 	bool relaxed_core_relocs;
@@ -271,6 +307,7 @@ struct bpf_object {
 		int maps_shndx;
 		int btf_maps_shndx;
 		int text_shndx;
+		int symbols_shndx;
 		int data_shndx;
 		int rodata_shndx;
 		int bss_shndx;
@@ -542,6 +579,7 @@ static struct bpf_object *bpf_object__new(const char *path,
 	obj->efile.data_shndx = -1;
 	obj->efile.rodata_shndx = -1;
 	obj->efile.bss_shndx = -1;
+	obj->extern_map_idx = -1;
 
 	obj->kern_version = get_kernel_version();
 	obj->loaded = false;
@@ -866,7 +904,8 @@ bpf_object__init_internal_map(struct bpf_object *obj, enum libbpf_map_type type,
 	def->key_size = sizeof(int);
 	def->value_size = data_sz;
 	def->max_entries = 1;
-	def->map_flags = type == LIBBPF_MAP_RODATA ? BPF_F_RDONLY_PROG : 0;
+	def->map_flags = type == LIBBPF_MAP_RODATA || type == LIBBPF_MAP_EXTERN
+			 ? BPF_F_RDONLY_PROG : 0;
 	def->map_flags |= BPF_F_MMAPABLE;
 
 	pr_debug("map '%s' (global data): at sec_idx %d, offset %zu, flags %x.\n",
@@ -883,7 +922,7 @@ bpf_object__init_internal_map(struct bpf_object *obj, enum libbpf_map_type type,
 		return err;
 	}
 
-	if (type != LIBBPF_MAP_BSS)
+	if (data)
 		memcpy(map->mmaped, data, data_sz);
 
 	pr_debug("map %td is \"%s\"\n", map - obj->maps, map->name);
@@ -924,6 +963,271 @@ static int bpf_object__init_global_data_maps(struct bpf_object *obj)
 	return 0;
 }
 
+
+static struct extern_desc *find_extern_by_name(const struct bpf_object *obj,
+					       const void *name)
+{
+	int i;
+
+	for (i = 0; i < obj->nr_extern; i++) {
+		if (strcmp(obj->externs[i].name, name) == 0)
+			return &obj->externs[i];
+	}
+	return NULL;
+}
+
+static int set_ext_value_tri(struct extern_desc *ext, void *ext_val,
+			     char value)
+{
+	switch (ext->type) {
+	case EXT_BOOL:
+		if (value == 'm') {
+			pr_warn("extern %s=%c should be tristate or char\n",
+				ext->name, value);
+			return -EINVAL;
+		}
+		*(bool *)ext_val = value == 'y' ? true : false;
+		break;
+	case EXT_TRISTATE:
+		if (value == 'y')
+			*(enum libbpf_tristate *)ext_val = TRI_YES;
+		else if (value == 'm')
+			*(enum libbpf_tristate *)ext_val = TRI_MODULE;
+		else /* value == 'n' */
+			*(enum libbpf_tristate *)ext_val = TRI_NO;
+		break;
+	case EXT_CHAR:
+		*(char *)ext_val = value;
+		break;
+	case EXT_UNKNOWN:
+	case EXT_INT:
+	case EXT_CHAR_ARR:
+	default:
+		pr_warn("extern %s=%c should be bool, tristate, or char\n",
+			ext->name, value);
+		return -EINVAL;
+	}
+	ext->is_set = true;
+	return 0;
+}
+
+static int set_ext_value_str(struct extern_desc *ext, char *ext_val,
+			     const char *value)
+{
+	size_t len;
+
+	if (ext->type != EXT_CHAR_ARR) {
+		pr_warn("extern %s=%s should char array\n", ext->name, value);
+		return -EINVAL;
+	}
+
+	len = strlen(value);
+	if (value[len - 1] != '"') {
+		pr_warn("extern '%s': invalid string config '%s'\n",
+			ext->name, value);
+		return -EINVAL;
+	}
+
+	/* strip quotes */
+	len -= 2;
+	if (len >= ext->sz) {
+		pr_warn("extern '%s': long string config %s of (%zu bytes) truncated to %d bytes\n",
+			ext->name, value, len, ext->sz - 1);
+		len = ext->sz - 1;
+	}
+	memcpy(ext_val, value + 1, len);
+	ext_val[len] = '\0';
+	ext->is_set = true;
+	return 0;
+}
+
+static int parse_u64(const char *value, __u64 *res)
+{
+	char *value_end;
+	int err;
+
+	errno = 0;
+	*res = strtoull(value, &value_end, 0);
+	if (errno) {
+		err = -errno;
+		pr_warn("failed to parse '%s' as integer: %d\n", value, err);
+		return err;
+	}
+	if (*value_end) {
+		pr_warn("failed to parse '%s' as integer completely\n", value);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static bool is_ext_value_in_range(const struct extern_desc *ext, __u64 v)
+{
+	int bit_sz = ext->sz * 8;
+
+	if (ext->sz == 8)
+		return true;
+
+	/* Validate that value stored in u64 fits in integer of `ext->sz`
+	 * bytes size without any loss of information. If the target integer
+	 * is signed, we rely on the following limits of integer type of
+	 * Y bits and subsequent transformation:
+	 *
+	 *     -2^(Y-1) <= X           <= 2^(Y-1) - 1
+	 *            0 <= X + 2^(Y-1) <= 2^Y - 1
+	 *            0 <= X + 2^(Y-1) <  2^Y
+	 *
+	 *  For unsigned target integer, check that all the (64 - Y) bits are
+	 *  zero.
+	 */
+	if (ext->is_signed)
+		return v + (1ULL << (bit_sz - 1)) < (1ULL << bit_sz);
+	else
+		return (v >> bit_sz) == 0;
+}
+
+static int set_ext_value_num(struct extern_desc *ext, void *ext_val,
+			     __u64 value)
+{
+	if (ext->type != EXT_INT && ext->type != EXT_CHAR) {
+		pr_warn("extern %s=%llu should be integer\n",
+			ext->name, value);
+		return -EINVAL;
+	}
+	if (!is_ext_value_in_range(ext, value)) {
+		pr_warn("extern %s=%llu value doesn't fit in %d bytes\n",
+			ext->name, value, ext->sz);
+		return -ERANGE;
+	}
+	switch (ext->sz) {
+		case 1: *(__u8 *)ext_val = value; break;
+		case 2: *(__u16 *)ext_val = value; break;
+		case 4: *(__u32 *)ext_val = value; break;
+		case 8: *(__u64 *)ext_val = value; break;
+		default:
+			return -EINVAL;
+	}
+	ext->is_set = true;
+	return 0;
+}
+
+static int bpf_object__read_kernel_config(struct bpf_object *obj,
+					  const char *config_path,
+					  void *data)
+{
+	char buf[PATH_MAX], *sep, *value;
+	struct extern_desc *ext;
+	int len, err = 0;
+	void *ext_val;
+	__u64 num;
+	gzFile file;
+
+	if (config_path) {
+		file = gzopen(config_path, "r");
+	} else {
+		struct utsname uts;
+
+		uname(&uts);
+		len = snprintf(buf, PATH_MAX, "/boot/config-%s", uts.release);
+		if (len < 0)
+			return -EINVAL;
+		else if (len >= PATH_MAX)
+			return -ENAMETOOLONG;
+		/* gzopen also accepts uncompressed files. */
+		file = gzopen(buf, "r");
+		if (!file)
+			file = gzopen("/proc/config.gz", "r");
+	}
+	if (!file) {
+		pr_warn("failed to read kernel config at '%s'\n", config_path);
+		return -ENOENT;
+	}
+
+	while (gzgets(file, buf, sizeof(buf))) {
+		if (strncmp(buf, "CONFIG_", 7))
+			continue;
+
+		sep = strchr(buf, '=');
+		if (!sep) {
+			err = -EINVAL;
+			pr_warn("failed to parse '%s': no separator\n", buf);
+			goto out;
+		}
+		/* Trim ending '\n' */
+		len = strlen(buf);
+		if (buf[len - 1] == '\n')
+			buf[len - 1] = '\0';
+		/* Split on '=' and ensure that a value is present. */
+		*sep = '\0';
+		if (!sep[1]) {
+			err = -EINVAL;
+			*sep = '=';
+			pr_warn("failed to parse '%s': no value\n", buf);
+			goto out;
+		}
+
+		ext = find_extern_by_name(obj, buf);
+		if (!ext)
+			continue;
+		if (ext->is_set) {
+			err = -EINVAL;
+			pr_warn("re-defining extern '%s' not allowed\n", buf);
+			goto out;
+		}
+
+		ext_val = data + ext->data_off;
+		value = sep + 1;
+
+		switch (*value) {
+		case 'y': case 'n': case 'm':
+			err = set_ext_value_tri(ext, ext_val, *value);
+			break;
+		case '"':
+			err = set_ext_value_str(ext, ext_val, value);
+			break;
+		default:
+			/* assume integer */
+			err = parse_u64(value, &num);
+			if (err) {
+				pr_warn("extern %s=%s should be integer\n",
+					ext->name, value);
+				goto out;
+			}
+			err = set_ext_value_num(ext, ext_val, num);
+			break;
+		}
+		if (err)
+			goto out;
+		pr_debug("extern %s=%s\n", ext->name, value);
+	}
+
+out:
+	gzclose(file);
+	return err;
+}
+
+static int bpf_object__init_extern_map(struct bpf_object *obj)
+{
+	struct extern_desc *last_ext;
+	size_t map_sz;
+	int err;
+
+	if (obj->nr_extern == 0)
+		return 0;
+
+	last_ext = &obj->externs[obj->nr_extern - 1];
+	map_sz = last_ext->data_off + last_ext->sz;
+
+	err = bpf_object__init_internal_map(obj, LIBBPF_MAP_EXTERN,
+					    obj->efile.symbols_shndx,
+					    NULL, map_sz);
+	if (err)
+		return err;
+
+	obj->extern_map_idx = obj->nr_maps - 1;
+
+	return 0;
+}
+
 static int bpf_object__init_user_maps(struct bpf_object *obj, bool strict)
 {
 	Elf_Data *symbols = obj->efile.symbols;
@@ -1401,19 +1705,17 @@ static int bpf_object__init_user_btf_maps(struct bpf_object *obj, bool strict,
 static int bpf_object__init_maps(struct bpf_object *obj,
 				 const struct bpf_object_open_opts *opts)
 {
-	const char *pin_root_path = OPTS_GET(opts, pin_root_path, NULL);
-	bool strict = !OPTS_GET(opts, relaxed_maps, false);
+	const char *pin_root_path;
+	bool strict;
 	int err;
 
-	err = bpf_object__init_user_maps(obj, strict);
-	if (err)
-		return err;
+	strict = !OPTS_GET(opts, relaxed_maps, false);
+	pin_root_path = OPTS_GET(opts, pin_root_path, NULL);
 
-	err = bpf_object__init_user_btf_maps(obj, strict, pin_root_path);
-	if (err)
-		return err;
-
-	err = bpf_object__init_global_data_maps(obj);
+	err = bpf_object__init_user_maps(obj, strict);
+	err = err ?: bpf_object__init_user_btf_maps(obj, strict, pin_root_path);
+	err = err ?: bpf_object__init_global_data_maps(obj);
+	err = err ?: bpf_object__init_extern_map(obj);
 	if (err)
 		return err;
 
@@ -1532,11 +1834,6 @@ static int bpf_object__init_btf(struct bpf_object *obj,
 				BTF_ELF_SEC, err);
 			goto out;
 		}
-		err = btf__finalize_data(obj, obj->btf);
-		if (err) {
-			pr_warn("Error finalizing %s: %d.\n", BTF_ELF_SEC, err);
-			goto out;
-		}
 	}
 	if (btf_ext_data) {
 		if (!obj->btf) {
@@ -1570,6 +1867,30 @@ static int bpf_object__init_btf(struct bpf_object *obj,
 	return 0;
 }
 
+static int bpf_object__finalize_btf(struct bpf_object *obj)
+{
+	int err;
+
+	if (!obj->btf)
+		return 0;
+
+	err = btf__finalize_data(obj, obj->btf);
+	if (!err)
+		return 0;
+
+	pr_warn("Error finalizing %s: %d.\n", BTF_ELF_SEC, err);
+	btf__free(obj->btf);
+	obj->btf = NULL;
+	btf_ext__free(obj->btf_ext);
+	obj->btf_ext = NULL;
+
+	if (bpf_object__is_btf_mandatory(obj)) {
+		pr_warn("BTF is required, but is missing or corrupted.\n");
+		return -ENOENT;
+	}
+	return 0;
+}
+
 static int bpf_object__sanitize_and_load_btf(struct bpf_object *obj)
 {
 	int err = 0;
@@ -1670,6 +1991,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
 				return -LIBBPF_ERRNO__FORMAT;
 			}
 			obj->efile.symbols = data;
+			obj->efile.symbols_shndx = idx;
 			obj->efile.strtabidx = sh.sh_link;
 		} else if (sh.sh_type == SHT_PROGBITS && data->d_size > 0) {
 			if (sh.sh_flags & SHF_EXECINSTR) {
@@ -1737,6 +2059,216 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
 	return bpf_object__init_btf(obj, btf_data, btf_ext_data);
 }
 
+static bool sym_is_extern(const GElf_Sym *sym)
+{
+	int bind = GELF_ST_BIND(sym->st_info);
+	/* externs are symbols w/ type=NOTYPE, bind=GLOBAL|WEAK, section=UND */
+	return sym->st_shndx == SHN_UNDEF &&
+	       (bind == STB_GLOBAL || bind == STB_WEAK) &&
+	       GELF_ST_TYPE(sym->st_info) == STT_NOTYPE;
+}
+
+static int find_extern_btf_id(const struct btf *btf, const char *ext_name)
+{
+	const struct btf_type *t;
+	const char *var_name;
+	int i, n;
+
+	if (!btf)
+		return -ESRCH;
+
+	n = btf__get_nr_types(btf);
+	for (i = 1; i <= n; i++) {
+		t = btf__type_by_id(btf, i);
+
+		if (!btf_is_var(t))
+			continue;
+
+		var_name = btf__name_by_offset(btf, t->name_off);
+		if (strcmp(var_name, ext_name))
+			continue;
+
+		if (btf_var(t)->linkage != BTF_VAR_GLOBAL_EXTERN)
+			return -EINVAL;
+
+		return i;
+	}
+
+	return -ENOENT;
+}
+
+static enum extern_type find_extern_type(const struct btf *btf, int id,
+					 bool *is_signed)
+{
+	const struct btf_type *t;
+	const char *name;
+
+	t = skip_mods_and_typedefs(btf, id, NULL);
+	name = btf__name_by_offset(btf, t->name_off);
+
+	if (is_signed)
+		*is_signed = false;
+	switch (btf_kind(t)) {
+	case BTF_KIND_INT: {
+		int enc = btf_int_encoding(t);
+
+		if (enc & BTF_INT_BOOL)
+			return t->size == 1 ? EXT_BOOL : EXT_UNKNOWN;
+		if (is_signed)
+			*is_signed = enc & BTF_INT_SIGNED;
+		if (t->size == 1)
+			return EXT_CHAR;
+		if (t->size < 1 || t->size > 8 || (t->size & (t->size - 1)))
+			return EXT_UNKNOWN;
+		return EXT_INT;
+	}
+	case BTF_KIND_ENUM:
+		if (t->size != 4)
+			return EXT_UNKNOWN;
+		if (strcmp(name, "libbpf_tristate"))
+			return EXT_UNKNOWN;
+		return EXT_TRISTATE;
+	case BTF_KIND_ARRAY:
+		if (btf_array(t)->nelems == 0)
+			return EXT_UNKNOWN;
+		if (find_extern_type(btf, btf_array(t)->type, NULL) != EXT_CHAR)
+			return EXT_UNKNOWN;
+		return EXT_CHAR_ARR;
+	default:
+		return EXT_UNKNOWN;
+	}
+}
+
+static int cmp_externs(const void *_a, const void *_b)
+{
+	const struct extern_desc *a = _a;
+	const struct extern_desc *b = _b;
+
+	/* descending order by alignment requirements */
+	if (a->align != b->align)
+		return a->align > b->align ? -1 : 1;
+	/* ascending order by size, within same alignment class */
+	if (a->sz != b->sz)
+		return a->sz < b->sz ? -1 : 1;
+	/* resolve ties by name */
+	return strcmp(a->name, b->name);
+}
+
+static int bpf_object__collect_externs(struct bpf_object *obj)
+{
+	const struct btf_type *t;
+	struct extern_desc *ext;
+	int i, n, off, btf_id;
+	struct btf_type *sec;
+	const char *ext_name;
+	Elf_Scn *scn;
+	GElf_Shdr sh;
+
+	if (!obj->efile.symbols)
+		return 0;
+
+	scn = elf_getscn(obj->efile.elf, obj->efile.symbols_shndx);
+	if (!scn)
+		return -LIBBPF_ERRNO__FORMAT;
+	if (gelf_getshdr(scn, &sh) != &sh)
+		return -LIBBPF_ERRNO__FORMAT;
+	n = sh.sh_size / sh.sh_entsize;
+
+	pr_debug("looking for externs among %d symbols...\n", n);
+	for (i = 0; i < n; i++) {
+		GElf_Sym sym;
+
+		if (!gelf_getsym(obj->efile.symbols, i, &sym))
+			return -LIBBPF_ERRNO__FORMAT;
+		if (!sym_is_extern(&sym))
+			continue;
+		ext_name = elf_strptr(obj->efile.elf, obj->efile.strtabidx,
+				      sym.st_name);
+		if (!ext_name || !ext_name[0])
+			continue;
+
+		ext = obj->externs;
+		ext = reallocarray(ext, obj->nr_extern + 1, sizeof(*ext));
+		if (!ext)
+			return -ENOMEM;
+		obj->externs = ext;
+		ext = &ext[obj->nr_extern];
+		memset(ext, 0, sizeof(*ext));
+		obj->nr_extern++;
+
+		ext->btf_id = find_extern_btf_id(obj->btf, ext_name);
+		if (ext->btf_id <= 0) {
+			pr_warn("failed to find BTF for extern '%s': %d\n",
+				ext_name, ext->btf_id);
+			return ext->btf_id;
+		}
+		t = btf__type_by_id(obj->btf, ext->btf_id);
+		ext->name = btf__name_by_offset(obj->btf, t->name_off);
+		ext->sym_idx = i;
+		ext->is_weak = GELF_ST_BIND(sym.st_info) == STB_WEAK;
+		ext->sz = btf__resolve_size(obj->btf, t->type);
+		if (ext->sz <= 0) {
+			pr_warn("failed to resolve size of extern '%s': %d\n",
+				ext_name, ext->sz);
+			return ext->sz;
+		}
+		ext->align = btf__align_of(obj->btf, t->type);
+		if (ext->align <= 0) {
+			pr_warn("failed to determine alignment of extern '%s': %d\n",
+				ext_name, ext->align);
+			return -EINVAL;
+		}
+		ext->type = find_extern_type(obj->btf, t->type,
+					     &ext->is_signed);
+		if (ext->type == EXT_UNKNOWN) {
+			pr_warn("extern '%s' type is unsupported\n", ext_name);
+			return -ENOTSUP;
+		}
+	}
+	pr_debug("collected %d externs total\n", obj->nr_extern);
+
+	if (!obj->nr_extern)
+		return 0;
+
+	/* sort externs by (alignment, size, name) and calculate their offsets
+	 * within a map */
+	qsort(obj->externs, obj->nr_extern, sizeof(*ext), cmp_externs);
+	off = 0;
+	for (i = 0; i < obj->nr_extern; i++) {
+		ext = &obj->externs[i];
+		ext->data_off = roundup(off, ext->align);
+		off = ext->data_off + ext->sz;
+		pr_debug("extern #%d: symbol %d, off %u, name %s\n",
+			 i, ext->sym_idx, ext->data_off, ext->name);
+	}
+
+	btf_id = btf__find_by_name(obj->btf, EXTERN_SEC);
+	if (btf_id <= 0) {
+		pr_warn("no BTF info found for '%s' datasec\n", EXTERN_SEC);
+		return -ESRCH;
+	}
+
+	sec = (struct btf_type *)btf__type_by_id(obj->btf, btf_id);
+	sec->size = off;
+	n = btf_vlen(sec);
+	for (i = 0; i < n; i++) {
+		struct btf_var_secinfo *vs = btf_var_secinfos(sec) + i;
+
+		t = btf__type_by_id(obj->btf, vs->type);
+		ext_name = btf__name_by_offset(obj->btf, t->name_off);
+		ext = find_extern_by_name(obj, ext_name);
+		if (!ext) {
+			pr_warn("failed to find extern definition for BTF var '%s'\n",
+				ext_name);
+			return -ESRCH;
+		}
+		vs->offset = ext->data_off;
+		btf_var(t)->linkage = BTF_VAR_GLOBAL_ALLOCATED;
+	}
+
+	return 0;
+}
+
 static struct bpf_program *
 bpf_object__find_prog_by_idx(struct bpf_object *obj, int idx)
 {
@@ -1801,6 +2333,8 @@ bpf_object__section_to_libbpf_map_type(const struct bpf_object *obj, int shndx)
 		return LIBBPF_MAP_BSS;
 	else if (shndx == obj->efile.rodata_shndx)
 		return LIBBPF_MAP_RODATA;
+	else if (shndx == obj->efile.symbols_shndx)
+		return LIBBPF_MAP_EXTERN;
 	else
 		return LIBBPF_MAP_UNSPEC;
 }
@@ -1845,6 +2379,30 @@ static int bpf_program__record_reloc(struct bpf_program *prog,
 			insn_idx, insn->code);
 		return -LIBBPF_ERRNO__RELOC;
 	}
+
+	if (sym_is_extern(sym)) {
+		int sym_idx = GELF_R_SYM(rel->r_info);
+		int i, n = obj->nr_extern;
+		struct extern_desc *ext;
+
+		for (i = 0; i < n; i++) {
+			ext = &obj->externs[i];
+			if (ext->sym_idx == sym_idx)
+				break;
+		}
+		if (i >= n) {
+			pr_warn("extern relo failed to find extern for sym %d\n",
+				sym_idx);
+			return -LIBBPF_ERRNO__RELOC;
+		}
+		pr_debug("found extern #%d '%s' (sym %d, off %u) for insn %u\n",
+			 i, ext->name, ext->sym_idx, ext->data_off, insn_idx);
+		reloc_desc->type = RELO_EXTERN;
+		reloc_desc->insn_idx = insn_idx;
+		reloc_desc->sym_off = ext->data_off;
+		return 0;
+	}
+
 	if (!shdr_idx || shdr_idx >= SHN_LORESERVE) {
 		pr_warn("invalid relo for \'%s\' in special section 0x%x; forgot to initialize global var?..\n",
 			name, shdr_idx);
@@ -2306,11 +2864,12 @@ bpf_object__reuse_map(struct bpf_map *map)
 static int
 bpf_object__populate_internal_map(struct bpf_object *obj, struct bpf_map *map)
 {
+	enum libbpf_map_type map_type = map->libbpf_type;
 	char *cp, errmsg[STRERR_BUFSIZE];
 	int err, zero = 0;
 
-	/* Nothing to do here since kernel already zero-initializes .bss map. */
-	if (map->libbpf_type == LIBBPF_MAP_BSS)
+	/* kernel already zero-initializes .bss map. */
+	if (map_type == LIBBPF_MAP_BSS)
 		return 0;
 
 	err = bpf_map_update_elem(map->fd, &zero, map->mmaped, 0);
@@ -2322,8 +2881,8 @@ bpf_object__populate_internal_map(struct bpf_object *obj, struct bpf_map *map)
 		return err;
 	}
 
-	/* Freeze .rodata map as read-only from syscall side. */
-	if (map->libbpf_type == LIBBPF_MAP_RODATA) {
+	/* Freeze .rodata and .extern map as read-only from syscall side. */
+	if (map_type == LIBBPF_MAP_RODATA || map_type == LIBBPF_MAP_EXTERN) {
 		err = bpf_map_freeze(map->fd);
 		if (err) {
 			err = -errno;
@@ -3572,9 +4131,6 @@ bpf_program__reloc_text(struct bpf_program *prog, struct bpf_object *obj,
 	size_t new_cnt;
 	int err;
 
-	if (relo->type != RELO_CALL)
-		return -LIBBPF_ERRNO__RELOC;
-
 	if (prog->idx == obj->efile.text_shndx) {
 		pr_warn("relo in .text insn %d into off %d (insn #%d)\n",
 			relo->insn_idx, relo->sym_off, relo->sym_off / 8);
@@ -3636,27 +4192,37 @@ bpf_program__relocate(struct bpf_program *prog, struct bpf_object *obj)
 
 	for (i = 0; i < prog->nr_reloc; i++) {
 		struct reloc_desc *relo = &prog->reloc_desc[i];
+		struct bpf_insn *insn = &prog->insns[relo->insn_idx];
 
-		if (relo->type == RELO_LD64 || relo->type == RELO_DATA) {
-			struct bpf_insn *insn = &prog->insns[relo->insn_idx];
-
-			if (relo->insn_idx + 1 >= (int)prog->insns_cnt) {
-				pr_warn("relocation out of range: '%s'\n",
-					prog->section_name);
-				return -LIBBPF_ERRNO__RELOC;
-			}
+		if (relo->insn_idx + 1 >= (int)prog->insns_cnt) {
+			pr_warn("relocation out of range: '%s'\n",
+				prog->section_name);
+			return -LIBBPF_ERRNO__RELOC;
+		}
 
-			if (relo->type != RELO_DATA) {
-				insn[0].src_reg = BPF_PSEUDO_MAP_FD;
-			} else {
-				insn[0].src_reg = BPF_PSEUDO_MAP_VALUE;
-				insn[1].imm = insn[0].imm + relo->sym_off;
-			}
+		switch (relo->type) {
+		case RELO_LD64:
+			insn[0].src_reg = BPF_PSEUDO_MAP_FD;
+			insn[0].imm = obj->maps[relo->map_idx].fd;
+			break;
+		case RELO_DATA:
+			insn[0].src_reg = BPF_PSEUDO_MAP_VALUE;
+			insn[1].imm = insn[0].imm + relo->sym_off;
 			insn[0].imm = obj->maps[relo->map_idx].fd;
-		} else if (relo->type == RELO_CALL) {
+			break;
+		case RELO_EXTERN:
+			insn[0].src_reg = BPF_PSEUDO_MAP_VALUE;
+			insn[0].imm = obj->maps[obj->extern_map_idx].fd;
+			insn[1].imm = relo->sym_off;
+			break;
+		case RELO_CALL:
 			err = bpf_program__reloc_text(prog, obj, relo);
 			if (err)
 				return err;
+			break;
+		default:
+			pr_warn("relo #%d: bad relo type %d\n", i, relo->type);
+			return -EINVAL;
 		}
 	}
 
@@ -3936,11 +4502,10 @@ static struct bpf_object *
 __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
 		   const struct bpf_object_open_opts *opts)
 {
+	const char *obj_name, *kconfig_path;
 	struct bpf_program *prog;
 	struct bpf_object *obj;
-	const char *obj_name;
 	char tmp_name[64];
-	__u32 attach_prog_fd;
 	int err;
 
 	if (elf_version(EV_CURRENT) == EV_NONE) {
@@ -3969,11 +4534,18 @@ __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
 		return obj;
 
 	obj->relaxed_core_relocs = OPTS_GET(opts, relaxed_core_relocs, false);
-	attach_prog_fd = OPTS_GET(opts, attach_prog_fd, 0);
+	kconfig_path = OPTS_GET(opts, kconfig_path, NULL);
+	if (kconfig_path) {
+		obj->kconfig_path = strdup(kconfig_path);
+		if (!obj->kconfig_path)
+			return ERR_PTR(-ENOMEM);
+	}
 
 	err = bpf_object__elf_init(obj);
 	err = err ? : bpf_object__check_endianness(obj);
 	err = err ? : bpf_object__elf_collect(obj);
+	err = err ? : bpf_object__collect_externs(obj);
+	err = err ? : bpf_object__finalize_btf(obj);
 	err = err ? : bpf_object__init_maps(obj, opts);
 	err = err ? : bpf_object__init_prog_names(obj);
 	err = err ? : bpf_object__collect_reloc(obj);
@@ -3996,7 +4568,7 @@ __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
 		bpf_program__set_type(prog, prog_type);
 		bpf_program__set_expected_attach_type(prog, attach_type);
 		if (prog_type == BPF_PROG_TYPE_TRACING)
-			prog->attach_prog_fd = attach_prog_fd;
+			prog->attach_prog_fd = OPTS_GET(opts, attach_prog_fd, 0);
 	}
 
 	return obj;
@@ -4107,6 +4679,61 @@ static int bpf_object__sanitize_maps(struct bpf_object *obj)
 	return 0;
 }
 
+static int bpf_object__resolve_externs(struct bpf_object *obj,
+				       const char *config_path)
+{
+	bool need_config = false;
+	struct extern_desc *ext;
+	int err, i;
+	void *data;
+
+	if (obj->nr_extern == 0)
+		return 0;
+
+	data = obj->maps[obj->extern_map_idx].mmaped;
+
+	for (i = 0; i < obj->nr_extern; i++) {
+		ext = &obj->externs[i];
+
+		if (strcmp(ext->name, "LINUX_KERNEL_VERSION") == 0) {
+			void *ext_val = data + ext->data_off;
+			__u32 kver = get_kernel_version();
+
+			if (!kver) {
+				pr_warn("failed to get kernel version\n");
+				return -EINVAL;
+			}
+			err = set_ext_value_num(ext, ext_val, kver);
+			if (err)
+				return err;
+			pr_debug("extern %s=0x%x\n", ext->name, kver);
+		} else if (strncmp(ext->name, "CONFIG_", 7) == 0) {
+			need_config = true;
+		} else {
+			pr_warn("unrecognized extern '%s'\n", ext->name);
+			return -EINVAL;
+		}
+	}
+	if (need_config) {
+		err = bpf_object__read_kernel_config(obj, config_path, data);
+		if (err)
+			return -EINVAL;
+	}
+	for (i = 0; i < obj->nr_extern; i++) {
+		ext = &obj->externs[i];
+
+		if (!ext->is_set && !ext->is_weak) {
+			pr_warn("extern %s (strong) not resolved\n", ext->name);
+			return -ESRCH;
+		} else if (!ext->is_set) {
+			pr_debug("extern %s (weak) not resolved, defaulting to zero\n",
+				 ext->name);
+		}
+	}
+
+	return 0;
+}
+
 int bpf_object__load_xattr(struct bpf_object_load_attr *attr)
 {
 	struct bpf_object *obj;
@@ -4126,6 +4753,7 @@ int bpf_object__load_xattr(struct bpf_object_load_attr *attr)
 	obj->loaded = true;
 
 	err = bpf_object__probe_caps(obj);
+	err = err ? : bpf_object__resolve_externs(obj, obj->kconfig_path);
 	err = err ? : bpf_object__sanitize_and_load_btf(obj);
 	err = err ? : bpf_object__sanitize_maps(obj);
 	err = err ? : bpf_object__create_maps(obj);
@@ -4719,6 +5347,10 @@ void bpf_object__close(struct bpf_object *obj)
 		zfree(&map->pin_path);
 	}
 
+	zfree(&obj->kconfig_path);
+	zfree(&obj->externs);
+	obj->nr_extern = 0;
+
 	zfree(&obj->maps);
 	obj->nr_maps = 0;
 
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 623191e71415..6340823871e2 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -85,8 +85,12 @@ struct bpf_object_open_opts {
 	 */
 	const char *pin_root_path;
 	__u32 attach_prog_fd;
+	/* kernel config file path override (for CONFIG_ externs); can point
+	 * to either uncompressed text file or .gz file
+	 */
+	const char *kconfig_path;
 };
-#define bpf_object_open_opts__last_field attach_prog_fd
+#define bpf_object_open_opts__last_field kconfig_path
 
 LIBBPF_API struct bpf_object *bpf_object__open(const char *path);
 LIBBPF_API struct bpf_object *
@@ -669,6 +673,12 @@ LIBBPF_API int bpf_object__attach_skeleton(struct bpf_object_skeleton *s);
 LIBBPF_API void bpf_object__detach_skeleton(struct bpf_object_skeleton *s);
 LIBBPF_API void bpf_object__destroy_skeleton(struct bpf_object_skeleton *s);
 
+enum libbpf_tristate {
+	TRI_NO = 0,
+	TRI_YES = 1,
+	TRI_MODULE = 2,
+};
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index f70c8e735120..ba90621617a8 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -24,7 +24,7 @@ CFLAGS += -g -Wall -O2 $(GENFLAGS) -I$(APIDIR) -I$(LIBDIR) -I$(BPFDIR)	\
 	  -I$(GENDIR) -I$(TOOLSINCDIR) -I$(CURDIR)			\
 	  -Dbpf_prog_load=bpf_prog_test_load				\
 	  -Dbpf_load_program=bpf_test_load_program
-LDLIBS += -lcap -lelf -lrt -lpthread
+LDLIBS += -lcap -lelf -lz -lrt -lpthread
 
 # Order correspond to 'make run_tests' order
 TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \
-- 
2.17.1


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

* [PATCH v4 bpf-next 3/4] bpftool: generate externs datasec in BPF skeleton
  2019-12-14  1:47 [PATCH v4 bpf-next 0/4] Add libbpf-provided extern variables support Andrii Nakryiko
  2019-12-14  1:47 ` [PATCH v4 bpf-next 1/4] libbpf: extract internal map names into constants Andrii Nakryiko
  2019-12-14  1:47 ` [PATCH v4 bpf-next 2/4] libbpf: support libbpf-provided extern variables Andrii Nakryiko
@ 2019-12-14  1:47 ` Andrii Nakryiko
  2019-12-14  1:47 ` [PATCH v4 bpf-next 4/4] selftests/bpf: add tests for libbpf-provided externs Andrii Nakryiko
  2019-12-16  0:52 ` [PATCH v4 bpf-next 0/4] Add libbpf-provided extern variables support Alexei Starovoitov
  4 siblings, 0 replies; 21+ messages in thread
From: Andrii Nakryiko @ 2019-12-14  1:47 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Add support for generation of mmap()-ed read-only view of libbpf-provided
extern variables. As externs are not supposed to be provided by user code
(that's what .data, .bss, and .rodata is for), don't mmap() it initially. Only
after skeleton load is performed, map .extern contents as read-only memory.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/bpf/bpftool/gen.c |  4 ++++
 tools/lib/bpf/libbpf.c  | 10 +++++-----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 7379dae35dca..a07c80429c7a 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -82,6 +82,8 @@ static const char *get_map_ident(const struct bpf_map *map)
 		return "rodata";
 	else if (str_has_suffix(name, ".bss"))
 		return "bss";
+	else if (str_has_suffix(name, ".extern"))
+		return "externs"; /* extern is a C keyword */
 	else
 		return NULL;
 }
@@ -109,6 +111,8 @@ static int codegen_datasec_def(struct bpf_object *obj,
 		sec_ident = "bss";
 	else if (strcmp(sec_name, ".rodata") == 0)
 		sec_ident = "rodata";
+	else if (strcmp(sec_name, ".extern") == 0)
+		sec_ident = "externs"; /* extern is a C keyword */
 	else
 		return 0;
 
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 26144706ba0b..c9f90bfd2f0d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -7472,7 +7472,8 @@ int bpf_object__open_skeleton(struct bpf_object_skeleton *s,
 			return -ESRCH;
 		}
 
-		if (mmaped)
+		/* externs shouldn't be pre-setup from user code */
+		if (mmaped && (*map)->libbpf_type != LIBBPF_MAP_EXTERN)
 			*mmaped = (*map)->mmaped;
 	}
 
@@ -7505,7 +7506,6 @@ int bpf_object__load_skeleton(struct bpf_object_skeleton *s)
 		size_t mmap_sz = bpf_map_mmap_sz(map);
 		int prot, map_fd = bpf_map__fd(map);
 		void **mmaped = s->maps[i].mmaped;
-		void *remapped;
 
 		if (!mmaped)
 			continue;
@@ -7530,9 +7530,9 @@ int bpf_object__load_skeleton(struct bpf_object_skeleton *s)
 		 * as per normal clean up procedure, so we don't need to worry
 		 * about it from skeleton's clean up perspective.
 		 */
-		remapped = mmap(*mmaped, mmap_sz, prot, MAP_SHARED | MAP_FIXED,
-				map_fd, 0);
-		if (remapped == MAP_FAILED) {
+		*mmaped = mmap(map->mmaped, mmap_sz, prot,
+				MAP_SHARED | MAP_FIXED, map_fd, 0);
+		if (*mmaped == MAP_FAILED) {
 			err = -errno;
 			*mmaped = NULL;
 			pr_warn("failed to re-mmap() map '%s': %d\n",
-- 
2.17.1


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

* [PATCH v4 bpf-next 4/4] selftests/bpf: add tests for libbpf-provided externs
  2019-12-14  1:47 [PATCH v4 bpf-next 0/4] Add libbpf-provided extern variables support Andrii Nakryiko
                   ` (2 preceding siblings ...)
  2019-12-14  1:47 ` [PATCH v4 bpf-next 3/4] bpftool: generate externs datasec in BPF skeleton Andrii Nakryiko
@ 2019-12-14  1:47 ` Andrii Nakryiko
  2019-12-16  0:52 ` [PATCH v4 bpf-next 0/4] Add libbpf-provided extern variables support Alexei Starovoitov
  4 siblings, 0 replies; 21+ messages in thread
From: Andrii Nakryiko @ 2019-12-14  1:47 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Add a set of tests validating libbpf-provided extern variables. One crucial
feature that's tested is dead code elimination together with using invalid BPF
helper. CONFIG_MISSING is not supposed to exist and should always be specified
by libbpf as zero, which allows BPF verifier to correctly do branch pruning
and not fail validation, when invalid BPF helper is called from dead if branch.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 .../selftests/bpf/prog_tests/core_extern.c    | 195 ++++++++++++++++++
 .../selftests/bpf/prog_tests/skeleton.c       |  18 +-
 .../selftests/bpf/progs/test_core_extern.c    |  62 ++++++
 .../selftests/bpf/progs/test_skeleton.c       |   9 +
 4 files changed, 283 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/core_extern.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_core_extern.c

diff --git a/tools/testing/selftests/bpf/prog_tests/core_extern.c b/tools/testing/selftests/bpf/prog_tests/core_extern.c
new file mode 100644
index 000000000000..30a7972e9012
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/core_extern.c
@@ -0,0 +1,195 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2019 Facebook */
+
+#include <test_progs.h>
+#include <sys/mman.h>
+#include <sys/utsname.h>
+#include <linux/version.h>
+#include "test_core_extern.skel.h"
+
+static uint32_t get_kernel_version(void)
+{
+	uint32_t major, minor, patch;
+	struct utsname info;
+
+	uname(&info);
+	if (sscanf(info.release, "%u.%u.%u", &major, &minor, &patch) != 3)
+		return 0;
+	return KERNEL_VERSION(major, minor, patch);
+}
+
+#define CFG "CONFIG_BPF_SYSCALL=n\n"
+
+static struct test_case {
+	const char *name;
+	const char *cfg;
+	const char *cfg_path;
+	bool fails;
+	struct test_core_extern__data data;
+} test_cases[] = {
+	{ .name = "default search path", .cfg_path = NULL,
+	  .data = { .bpf_syscall = true } },
+	{ .name = "/proc/config.gz", .cfg_path = "/proc/config.gz",
+	  .data = { .bpf_syscall = true } },
+	{ .name = "missing config", .fails = true,
+	  .cfg_path = "/proc/invalid-config.gz" },
+	{
+		.name = "custom values",
+		.cfg = "CONFIG_BPF_SYSCALL=y\n"
+		       "CONFIG_TRISTATE=m\n"
+		       "CONFIG_BOOL=y\n"
+		       "CONFIG_CHAR=100\n"
+		       "CONFIG_USHORT=30000\n"
+		       "CONFIG_INT=123456\n"
+		       "CONFIG_ULONG=0xDEADBEEFC0DE\n"
+		       "CONFIG_STR=\"abracad\"\n"
+		       "CONFIG_MISSING=0",
+		.data = {
+			.bpf_syscall = true,
+			.tristate_val = TRI_MODULE,
+			.bool_val = true,
+			.char_val = 100,
+			.ushort_val = 30000,
+			.int_val = 123456,
+			.ulong_val = 0xDEADBEEFC0DE,
+			.str_val = "abracad",
+		},
+	},
+	/* TRISTATE */
+	{ .name = "tristate (y)", .cfg = CFG"CONFIG_TRISTATE=y\n",
+	  .data = { .tristate_val = TRI_YES } },
+	{ .name = "tristate (n)", .cfg = CFG"CONFIG_TRISTATE=n\n",
+	  .data = { .tristate_val = TRI_NO } },
+	{ .name = "tristate (m)", .cfg = CFG"CONFIG_TRISTATE=m\n",
+	  .data = { .tristate_val = TRI_MODULE } },
+	{ .name = "tristate (int)", .fails = 1, .cfg = CFG"CONFIG_TRISTATE=1" },
+	{ .name = "tristate (bad)", .fails = 1, .cfg = CFG"CONFIG_TRISTATE=M" },
+	/* BOOL */
+	{ .name = "bool (y)", .cfg = CFG"CONFIG_BOOL=y\n",
+	  .data = { .bool_val = true } },
+	{ .name = "bool (n)", .cfg = CFG"CONFIG_BOOL=n\n",
+	  .data = { .bool_val = false } },
+	{ .name = "bool (tristate)", .fails = 1, .cfg = CFG"CONFIG_BOOL=m" },
+	{ .name = "bool (int)", .fails = 1, .cfg = CFG"CONFIG_BOOL=1" },
+	/* CHAR */
+	{ .name = "char (tristate)", .cfg = CFG"CONFIG_CHAR=m\n",
+	  .data = { .char_val = 'm' } },
+	{ .name = "char (bad)", .fails = 1, .cfg = CFG"CONFIG_CHAR=q\n" },
+	{ .name = "char (empty)", .fails = 1, .cfg = CFG"CONFIG_CHAR=\n" },
+	{ .name = "char (str)", .fails = 1, .cfg = CFG"CONFIG_CHAR=\"y\"\n" },
+	/* STRING */
+	{ .name = "str (empty)", .cfg = CFG"CONFIG_STR=\"\"\n",
+	  .data = { .str_val = "\0\0\0\0\0\0\0" } },
+	{ .name = "str (padded)", .cfg = CFG"CONFIG_STR=\"abra\"\n",
+	  .data = { .str_val = "abra\0\0\0" } },
+	{ .name = "str (too long)", .cfg = CFG"CONFIG_STR=\"abracada\"\n",
+	  .data = { .str_val = "abracad" } },
+	{ .name = "str (no value)", .fails = 1, .cfg = CFG"CONFIG_STR=\n" },
+	{ .name = "str (bad value)", .fails = 1, .cfg = CFG"CONFIG_STR=bla\n" },
+	/* INTEGERS */
+	{
+		.name = "integer forms",
+		.cfg = CFG
+		       "CONFIG_CHAR=0xA\n"
+		       "CONFIG_USHORT=0462\n"
+		       "CONFIG_INT=-100\n"
+		       "CONFIG_ULONG=+1000000000000",
+		.data = {
+			.char_val = 0xA,
+			.ushort_val = 0462,
+			.int_val = -100,
+			.ulong_val = 1000000000000,
+		},
+	},
+	{ .name = "int (bad)", .fails = 1, .cfg = CFG"CONFIG_INT=abc" },
+	{ .name = "int (str)", .fails = 1, .cfg = CFG"CONFIG_INT=\"abc\"" },
+	{ .name = "int (empty)", .fails = 1, .cfg = CFG"CONFIG_INT=" },
+	{ .name = "int (mixed)", .fails = 1, .cfg = CFG"CONFIG_INT=123abc" },
+	{ .name = "int (max)", .cfg = CFG"CONFIG_INT=2147483647",
+	  .data = { .int_val = 2147483647 } },
+	{ .name = "int (min)", .cfg = CFG"CONFIG_INT=-2147483648",
+	  .data = { .int_val = -2147483648 } },
+	{ .name = "int (max+1)", .fails = 1, .cfg = CFG"CONFIG_INT=2147483648" },
+	{ .name = "int (min-1)", .fails = 1, .cfg = CFG"CONFIG_INT=-2147483649" },
+	{ .name = "ushort (max)", .cfg = CFG"CONFIG_USHORT=65535",
+	  .data = { .ushort_val = 65535 } },
+	{ .name = "ushort (min)", .cfg = CFG"CONFIG_USHORT=0",
+	  .data = { .ushort_val = 0 } },
+	{ .name = "ushort (max+1)", .fails = 1, .cfg = CFG"CONFIG_USHORT=65536" },
+	{ .name = "ushort (min-1)", .fails = 1, .cfg = CFG"CONFIG_USHORT=-1" },
+	{ .name = "u64 (max)", .cfg = CFG"CONFIG_ULONG=0xffffffffffffffff",
+	  .data = { .ulong_val = 0xffffffffffffffff } },
+	{ .name = "u64 (min)", .cfg = CFG"CONFIG_ULONG=0",
+	  .data = { .ulong_val = 0 } },
+	{ .name = "u64 (max+1)", .fails = 1, .cfg = CFG"CONFIG_ULONG=0x10000000000000000" },
+};
+
+BPF_EMBED_OBJ(core_extern, "test_core_extern.o");
+
+void test_core_extern(void)
+{
+	const uint32_t kern_ver = get_kernel_version();
+	int err, duration = 0, i, j;
+	struct test_core_extern *skel = NULL;
+	uint64_t *got, *exp;
+	int n = sizeof(*skel->data) / sizeof(uint64_t);
+
+	for (i = 0; i < ARRAY_SIZE(test_cases); i++) {
+		char tmp_cfg_path[] = "/tmp/test_core_extern_cfg.XXXXXX";
+		struct test_case *t = &test_cases[i];
+		DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts,
+			.kconfig_path = t->cfg_path,
+		);
+
+		if (!test__start_subtest(t->name))
+			continue;
+
+		if (t->cfg) {
+			size_t n = strlen(t->cfg) + 1;
+			int fd = mkstemp(tmp_cfg_path);
+			int written;
+
+			if (CHECK(fd < 0, "mkstemp", "errno: %d\n", errno))
+				continue;
+			printf("using '%s' as config file\n", tmp_cfg_path);
+			written = write(fd, t->cfg, n);
+			close(fd);
+			if (CHECK_FAIL(written != n))
+				goto cleanup;
+			opts.kconfig_path = tmp_cfg_path;
+		}
+
+		skel = test_core_extern__open_opts(&core_extern_embed, &opts);
+		if (CHECK(!skel, "skel_open", "skeleton open failed\n"))
+			goto cleanup;
+		err = test_core_extern__load(skel);
+		if (t->fails) {
+			CHECK(!err, "skel_load",
+			      "shouldn't succeed open/load of skeleton\n");
+			goto cleanup;
+		} else if (CHECK(err, "skel_load",
+				 "failed to open/load skeleton\n")) {
+			goto cleanup;
+		}
+		err = test_core_extern__attach(skel);
+		if (CHECK(err, "attach_raw_tp", "failed attach: %d\n", err))
+			goto cleanup;
+
+		usleep(1);
+
+		t->data.kern_ver = kern_ver;
+		t->data.missing_val = 0xDEADC0DE;
+		got = (uint64_t *)skel->data;
+		exp = (uint64_t *)&t->data;
+		for (j = 0; j < n; j++) {
+			CHECK(got[j] != exp[j], "check_res",
+			      "result #%d: expected %lx, but got %lx\n",
+			       j, exp[j], got[j]);
+		}
+cleanup:
+		if (t->cfg)
+			unlink(tmp_cfg_path);
+		test_core_extern__destroy(skel);
+		skel = NULL;
+	}
+}
diff --git a/tools/testing/selftests/bpf/prog_tests/skeleton.c b/tools/testing/selftests/bpf/prog_tests/skeleton.c
index 79f8d13e6740..151cdad3ad0d 100644
--- a/tools/testing/selftests/bpf/prog_tests/skeleton.c
+++ b/tools/testing/selftests/bpf/prog_tests/skeleton.c
@@ -17,11 +17,21 @@ void test_skeleton(void)
 	int duration = 0, err;
 	struct test_skeleton* skel;
 	struct test_skeleton__bss *bss;
+	struct test_skeleton__externs *exts;
 
-	skel = test_skeleton__open_and_load(&skeleton_embed);
+	skel = test_skeleton__open(&skeleton_embed);
 	if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
 		return;
 
+	printf("EXTERNS BEFORE: %p\n", skel->externs);
+	if (CHECK(skel->externs, "skel_externs", "externs are mmaped()!\n"))
+		goto cleanup;
+
+	err = test_skeleton__load(skel);
+	if (CHECK(err, "skel_load", "failed to load skeleton: %d\n", err))
+		goto cleanup;
+	printf("EXTERNS AFTER: %p\n", skel->externs);
+
 	bss = skel->bss;
 	bss->in1 = 1;
 	bss->in2 = 2;
@@ -29,6 +39,7 @@ void test_skeleton(void)
 	bss->in4 = 4;
 	bss->in5.a = 5;
 	bss->in5.b = 6;
+	exts = skel->externs;
 
 	err = test_skeleton__attach(skel);
 	if (CHECK(err, "skel_attach", "skeleton attach failed: %d\n", err))
@@ -46,6 +57,11 @@ void test_skeleton(void)
 	CHECK(bss->handler_out5.b != 6, "res6", "got %lld != exp %d\n",
 	      bss->handler_out5.b, 6);
 
+	CHECK(bss->bpf_syscall != exts->CONFIG_BPF_SYSCALL, "ext1",
+	      "got %d != exp %d\n", bss->bpf_syscall, exts->CONFIG_BPF_SYSCALL);
+	CHECK(bss->kern_ver != exts->LINUX_KERNEL_VERSION, "ext2",
+	      "got %d != exp %d\n", bss->kern_ver, exts->LINUX_KERNEL_VERSION);
+
 cleanup:
 	test_skeleton__destroy(skel);
 }
diff --git a/tools/testing/selftests/bpf/progs/test_core_extern.c b/tools/testing/selftests/bpf/progs/test_core_extern.c
new file mode 100644
index 000000000000..e12f09f9e881
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_core_extern.c
@@ -0,0 +1,62 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2019 Facebook */
+
+#include <stdint.h>
+#include <stdbool.h>
+#include <linux/ptrace.h>
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+/* non-existing BPF helper, to test dead code elimination */
+static int (*bpf_missing_helper)(const void *arg1, int arg2) = (void *) 999;
+
+extern int LINUX_KERNEL_VERSION;
+extern bool CONFIG_BPF_SYSCALL; /* strong */
+extern enum libbpf_tristate CONFIG_TRISTATE __weak;
+extern bool CONFIG_BOOL __weak;
+extern char CONFIG_CHAR __weak;
+extern uint16_t CONFIG_USHORT __weak;
+extern int CONFIG_INT __weak;
+extern uint64_t CONFIG_ULONG __weak;
+extern const char CONFIG_STR[8] __weak;
+extern uint64_t CONFIG_MISSING __weak;
+
+uint64_t kern_ver = -1;
+uint64_t bpf_syscall = -1;
+uint64_t tristate_val = -1;
+uint64_t bool_val = -1;
+uint64_t char_val = -1;
+uint64_t ushort_val = -1;
+uint64_t int_val = -1;
+uint64_t ulong_val = -1;
+char str_val[8] = {-1, -1, -1, -1, -1, -1, -1, -1};
+uint64_t missing_val = -1;
+
+SEC("raw_tp/sys_enter")
+int handle_sys_enter(struct pt_regs *ctx)
+{
+	int i;
+
+	kern_ver = LINUX_KERNEL_VERSION;
+	bpf_syscall = CONFIG_BPF_SYSCALL;
+	tristate_val = CONFIG_TRISTATE;
+	bool_val = CONFIG_BOOL;
+	char_val = CONFIG_CHAR;
+	ushort_val = CONFIG_USHORT;
+	int_val = CONFIG_INT;
+	ulong_val = CONFIG_ULONG;
+
+	for (i = 0; i < sizeof(CONFIG_STR); i++) {
+		str_val[i] = CONFIG_STR[i];
+	}
+
+	if (CONFIG_MISSING)
+		/* invalid, but dead code - never executed */
+		missing_val = bpf_missing_helper(ctx, 123);
+	else
+		missing_val = 0xDEADC0DE;
+
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/test_skeleton.c b/tools/testing/selftests/bpf/progs/test_skeleton.c
index c0013d2e16f2..9caa44758ea2 100644
--- a/tools/testing/selftests/bpf/progs/test_skeleton.c
+++ b/tools/testing/selftests/bpf/progs/test_skeleton.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) 2019 Facebook */
 
+#include <stdbool.h>
 #include <linux/bpf.h>
 #include "bpf_helpers.h"
 
@@ -20,6 +21,10 @@ char out3 = 0;
 long long out4 = 0;
 int out1 = 0;
 
+extern bool CONFIG_BPF_SYSCALL;
+extern int LINUX_KERNEL_VERSION;
+bool bpf_syscall = 0;
+int kern_ver = 0;
 
 SEC("raw_tp/sys_enter")
 int handler(const void *ctx)
@@ -31,6 +36,10 @@ int handler(const void *ctx)
 	out3 = in3;
 	out4 = in4;
 	out5 = in5;
+
+	bpf_syscall = CONFIG_BPF_SYSCALL;
+	kern_ver = LINUX_KERNEL_VERSION;
+
 	return 0;
 }
 
-- 
2.17.1


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

* Re: [PATCH v4 bpf-next 2/4] libbpf: support libbpf-provided extern variables
  2019-12-14  1:47 ` [PATCH v4 bpf-next 2/4] libbpf: support libbpf-provided extern variables Andrii Nakryiko
@ 2019-12-14 12:50   ` Toke Høiland-Jørgensen
  2019-12-14 20:27     ` Yonghong Song
  2019-12-16 11:17   ` Daniel Borkmann
  2019-12-16 12:43   ` Daniel Borkmann
  2 siblings, 1 reply; 21+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-12-14 12:50 UTC (permalink / raw)
  To: Andrii Nakryiko, bpf, netdev, ast, daniel
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

[...]

> +static bool sym_is_extern(const GElf_Sym *sym)
> +{
> +	int bind = GELF_ST_BIND(sym->st_info);
> +	/* externs are symbols w/ type=NOTYPE, bind=GLOBAL|WEAK, section=UND */
> +	return sym->st_shndx == SHN_UNDEF &&
> +	       (bind == STB_GLOBAL || bind == STB_WEAK) &&
> +	       GELF_ST_TYPE(sym->st_info) == STT_NOTYPE;
> +}

Will this also match function declarations marked as extern? I've
started looking into how to handle this for the static/dynamic linking
use cases and am wondering whether it makes sense to pull in this series
and build on that?

-Toke


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

* Re: [PATCH v4 bpf-next 2/4] libbpf: support libbpf-provided extern variables
  2019-12-14 12:50   ` Toke Høiland-Jørgensen
@ 2019-12-14 20:27     ` Yonghong Song
  0 siblings, 0 replies; 21+ messages in thread
From: Yonghong Song @ 2019-12-14 20:27 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, Andrii Nakryiko, bpf, netdev,
	Alexei Starovoitov, daniel
  Cc: andrii.nakryiko, Kernel Team



On 12/14/19 4:50 AM, Toke Høiland-Jørgensen wrote:
> [...]
> 
>> +static bool sym_is_extern(const GElf_Sym *sym)
>> +{
>> +	int bind = GELF_ST_BIND(sym->st_info);
>> +	/* externs are symbols w/ type=NOTYPE, bind=GLOBAL|WEAK, section=UND */
>> +	return sym->st_shndx == SHN_UNDEF &&
>> +	       (bind == STB_GLOBAL || bind == STB_WEAK) &&
>> +	       GELF_ST_TYPE(sym->st_info) == STT_NOTYPE;
>> +}
> 
> Will this also match function declarations marked as extern? I've

Yes. They are treated the same as other extern variables in symbol table.

> started looking into how to handle this for the static/dynamic linking
> use cases and am wondering whether it makes sense to pull in this series
> and build on that?
> 
> -Toke
> 

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

* Re: [PATCH v4 bpf-next 0/4] Add libbpf-provided extern variables support
  2019-12-14  1:47 [PATCH v4 bpf-next 0/4] Add libbpf-provided extern variables support Andrii Nakryiko
                   ` (3 preceding siblings ...)
  2019-12-14  1:47 ` [PATCH v4 bpf-next 4/4] selftests/bpf: add tests for libbpf-provided externs Andrii Nakryiko
@ 2019-12-16  0:52 ` Alexei Starovoitov
  2019-12-16  1:47   ` Andrii Nakryiko
  4 siblings, 1 reply; 21+ messages in thread
From: Alexei Starovoitov @ 2019-12-16  0:52 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: bpf, netdev, ast, daniel, andrii.nakryiko, kernel-team

On Fri, Dec 13, 2019 at 05:47:06PM -0800, Andrii Nakryiko wrote:
> It's often important for BPF program to know kernel version or some specific
> config values (e.g., CONFIG_HZ to convert jiffies to seconds) and change or
> adjust program logic based on their values. As of today, any such need has to
> be resolved by recompiling BPF program for specific kernel and kernel
> configuration. In practice this is usually achieved by using BCC and its
> embedded LLVM/Clang. With such set up #ifdef CONFIG_XXX and similar
> compile-time constructs allow to deal with kernel varieties.
> 
> With CO-RE (Compile Once – Run Everywhere) approach, this is not an option,
> unfortunately. All such logic variations have to be done as a normal
> C language constructs (i.e., if/else, variables, etc), not a preprocessor
> directives. This patch series add support for such advanced scenarios through
> C extern variables. These extern variables will be recognized by libbpf and
> supplied through extra .extern internal map, similarly to global data. This
> .extern map is read-only, which allows BPF verifier to track its content
> precisely as constants. That gives an opportunity to have pre-compiled BPF
> program, which can potentially use BPF functionality (e.g., BPF helpers) or
> kernel features (types, fields, etc), that are available only on a subset of
> targeted kernels, while effectively eleminating (through verifier's dead code
> detection) such unsupported functionality for other kernels (typically, older
> versions). Patch #3 explicitly tests a scenario of using unsupported BPF
> helper, to validate the approach.
> 
> This patch set heavily relies on BTF type information emitted by compiler for
> each extern variable declaration. Based on specific types, libbpf does strict
> checks of config data values correctness. See patch #1 for details.
> 
> Outline of the patch set:
> - patch #1 does a small clean up of internal map names contants;
> - patch #2 adds all of the libbpf internal machinery for externs support,
>   including setting up BTF information for .extern data section;
> - patch #3 adds support for .extern into BPF skeleton;
> - patch #4 adds externs selftests, as well as enhances test_skeleton.c test to
>   validate mmap()-ed .extern datasection functionality.

Applied. Thanks.

Looking at the tests that do mkstemp()+write() just to pass a file path
as .kconfig_path option into bpf_object_open_opts() it feels that file only
support for externs is unnecessary limiting. I think it will simplify
tests and will make the whole extern support more flexible if in addition to
kconfig_path bpf_object_open_opts() would support in-memory configuration.

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

* Re: [PATCH v4 bpf-next 0/4] Add libbpf-provided extern variables support
  2019-12-16  0:52 ` [PATCH v4 bpf-next 0/4] Add libbpf-provided extern variables support Alexei Starovoitov
@ 2019-12-16  1:47   ` Andrii Nakryiko
  2019-12-16  4:42     ` Alexei Starovoitov
  0 siblings, 1 reply; 21+ messages in thread
From: Andrii Nakryiko @ 2019-12-16  1:47 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team

On Sun, Dec 15, 2019 at 4:52 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Dec 13, 2019 at 05:47:06PM -0800, Andrii Nakryiko wrote:
> > It's often important for BPF program to know kernel version or some specific
> > config values (e.g., CONFIG_HZ to convert jiffies to seconds) and change or
> > adjust program logic based on their values. As of today, any such need has to
> > be resolved by recompiling BPF program for specific kernel and kernel
> > configuration. In practice this is usually achieved by using BCC and its
> > embedded LLVM/Clang. With such set up #ifdef CONFIG_XXX and similar
> > compile-time constructs allow to deal with kernel varieties.
> >
> > With CO-RE (Compile Once – Run Everywhere) approach, this is not an option,
> > unfortunately. All such logic variations have to be done as a normal
> > C language constructs (i.e., if/else, variables, etc), not a preprocessor
> > directives. This patch series add support for such advanced scenarios through
> > C extern variables. These extern variables will be recognized by libbpf and
> > supplied through extra .extern internal map, similarly to global data. This
> > .extern map is read-only, which allows BPF verifier to track its content
> > precisely as constants. That gives an opportunity to have pre-compiled BPF
> > program, which can potentially use BPF functionality (e.g., BPF helpers) or
> > kernel features (types, fields, etc), that are available only on a subset of
> > targeted kernels, while effectively eleminating (through verifier's dead code
> > detection) such unsupported functionality for other kernels (typically, older
> > versions). Patch #3 explicitly tests a scenario of using unsupported BPF
> > helper, to validate the approach.
> >
> > This patch set heavily relies on BTF type information emitted by compiler for
> > each extern variable declaration. Based on specific types, libbpf does strict
> > checks of config data values correctness. See patch #1 for details.
> >
> > Outline of the patch set:
> > - patch #1 does a small clean up of internal map names contants;
> > - patch #2 adds all of the libbpf internal machinery for externs support,
> >   including setting up BTF information for .extern data section;
> > - patch #3 adds support for .extern into BPF skeleton;
> > - patch #4 adds externs selftests, as well as enhances test_skeleton.c test to
> >   validate mmap()-ed .extern datasection functionality.
>
> Applied. Thanks.

Great, thanks!

>
> Looking at the tests that do mkstemp()+write() just to pass a file path
> as .kconfig_path option into bpf_object_open_opts() it feels that file only
> support for externs is unnecessary limiting. I think it will simplify

yeah, it was a bit painful :)

> tests and will make the whole extern support more flexible if in addition to
> kconfig_path bpf_object_open_opts() would support in-memory configuration.

I wanted to keep it simple for users, in case libbpf can't find config
file, to just specify its location. But given your feedback here, and
you mentioned previously that it would be nice to allow users to
specify custom kconfig-like configuration to be exposed as externs as
well, how about replacing .kconfig_path, which is a patch to config
file, with just .kconfig, which is the **contents** of config file.
That way we can support all of the above scenarios, if maybe sometime
with a tiny bit of extra work for users:

1. Override real kconfig with custom config (e.g., for testing
purposes) - just specify alternative contents.
2. Extend kconfig with some extra configuration - user will have to
read real kconfig, then append (or prepend, doesn't matter) custom
contents.

What I want to avoid is having multiple ways to do this, having to
decide whether to augment real Kconfig or completely override it, etc.
So one string-based config override is preferable for simplicity.
Agreed?

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

* Re: [PATCH v4 bpf-next 0/4] Add libbpf-provided extern variables support
  2019-12-16  1:47   ` Andrii Nakryiko
@ 2019-12-16  4:42     ` Alexei Starovoitov
  2019-12-16 19:34       ` Andrii Nakryiko
  0 siblings, 1 reply; 21+ messages in thread
From: Alexei Starovoitov @ 2019-12-16  4:42 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team

On Sun, Dec 15, 2019 at 05:47:01PM -0800, Andrii Nakryiko wrote:
> On Sun, Dec 15, 2019 at 4:52 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Fri, Dec 13, 2019 at 05:47:06PM -0800, Andrii Nakryiko wrote:
> > > It's often important for BPF program to know kernel version or some specific
> > > config values (e.g., CONFIG_HZ to convert jiffies to seconds) and change or
> > > adjust program logic based on their values. As of today, any such need has to
> > > be resolved by recompiling BPF program for specific kernel and kernel
> > > configuration. In practice this is usually achieved by using BCC and its
> > > embedded LLVM/Clang. With such set up #ifdef CONFIG_XXX and similar
> > > compile-time constructs allow to deal with kernel varieties.
> > >
> > > With CO-RE (Compile Once – Run Everywhere) approach, this is not an option,
> > > unfortunately. All such logic variations have to be done as a normal
> > > C language constructs (i.e., if/else, variables, etc), not a preprocessor
> > > directives. This patch series add support for such advanced scenarios through
> > > C extern variables. These extern variables will be recognized by libbpf and
> > > supplied through extra .extern internal map, similarly to global data. This
> > > .extern map is read-only, which allows BPF verifier to track its content
> > > precisely as constants. That gives an opportunity to have pre-compiled BPF
> > > program, which can potentially use BPF functionality (e.g., BPF helpers) or
> > > kernel features (types, fields, etc), that are available only on a subset of
> > > targeted kernels, while effectively eleminating (through verifier's dead code
> > > detection) such unsupported functionality for other kernels (typically, older
> > > versions). Patch #3 explicitly tests a scenario of using unsupported BPF
> > > helper, to validate the approach.
> > >
> > > This patch set heavily relies on BTF type information emitted by compiler for
> > > each extern variable declaration. Based on specific types, libbpf does strict
> > > checks of config data values correctness. See patch #1 for details.
> > >
> > > Outline of the patch set:
> > > - patch #1 does a small clean up of internal map names contants;
> > > - patch #2 adds all of the libbpf internal machinery for externs support,
> > >   including setting up BTF information for .extern data section;
> > > - patch #3 adds support for .extern into BPF skeleton;
> > > - patch #4 adds externs selftests, as well as enhances test_skeleton.c test to
> > >   validate mmap()-ed .extern datasection functionality.
> >
> > Applied. Thanks.
> 
> Great, thanks!
> 
> >
> > Looking at the tests that do mkstemp()+write() just to pass a file path
> > as .kconfig_path option into bpf_object_open_opts() it feels that file only
> > support for externs is unnecessary limiting. I think it will simplify
> 
> yeah, it was a bit painful :)
> 
> > tests and will make the whole extern support more flexible if in addition to
> > kconfig_path bpf_object_open_opts() would support in-memory configuration.
> 
> I wanted to keep it simple for users, in case libbpf can't find config
> file, to just specify its location. But given your feedback here, and
> you mentioned previously that it would be nice to allow users to
> specify custom kconfig-like configuration to be exposed as externs as
> well, how about replacing .kconfig_path, which is a patch to config
> file, with just .kconfig, which is the **contents** of config file.
> That way we can support all of the above scenarios, if maybe sometime
> with a tiny bit of extra work for users:
> 
> 1. Override real kconfig with custom config (e.g., for testing
> purposes) - just specify alternative contents.
> 2. Extend kconfig with some extra configuration - user will have to
> read real kconfig, then append (or prepend, doesn't matter) custom
> contents.
> 
> What I want to avoid is having multiple ways to do this, having to
> decide whether to augment real Kconfig or completely override it, etc.
> So one string-based config override is preferable for simplicity.
> Agreed?

I think user experience would be better if users don't have to know that
/proc/config.gz exists and even more so if they don't need to read it. By
default libbpf should pick all CONFIG_* from known locations and in addition if
extra text is specified for bpf_object_open_opts() the libbpf can take the
values from there. So may be .kconfig_path can be replaced with
.additional_key_value_pairs ? I think override of /proc/config.gz is
unnecessary. Whereas additional predefined externs are useful for testing and
passing load-time configuration to bpf progs. Like IP addresses, etc.

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

* Re: [PATCH v4 bpf-next 2/4] libbpf: support libbpf-provided extern variables
  2019-12-14  1:47 ` [PATCH v4 bpf-next 2/4] libbpf: support libbpf-provided extern variables Andrii Nakryiko
  2019-12-14 12:50   ` Toke Høiland-Jørgensen
@ 2019-12-16 11:17   ` Daniel Borkmann
  2019-12-16 19:29     ` Andrii Nakryiko
  2019-12-16 12:43   ` Daniel Borkmann
  2 siblings, 1 reply; 21+ messages in thread
From: Daniel Borkmann @ 2019-12-16 11:17 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: bpf, netdev, ast, andrii.nakryiko, kernel-team

On Fri, Dec 13, 2019 at 05:47:08PM -0800, Andrii Nakryiko wrote:
> Add support for extern variables, provided to BPF program by libbpf. Currently
> the following extern variables are supported:
>   - LINUX_KERNEL_VERSION; version of a kernel in which BPF program is
>     executing, follows KERNEL_VERSION() macro convention, can be 4- and 8-byte
>     long;
>   - CONFIG_xxx values; a set of values of actual kernel config. Tristate,
>     boolean, strings, and integer values are supported.
> 
[...]
> 
> All detected extern variables, are put into a separate .extern internal map.
> It, similarly to .rodata map, is marked as read-only from BPF program side, as
> well as is frozen on load. This allows BPF verifier to track extern values as
> constants and perform enhanced branch prediction and dead code elimination.
> This can be relied upon for doing kernel version/feature detection and using
> potentially unsupported field relocations or BPF helpers in a CO-RE-based BPF
> program, while still having a single version of BPF program running on old and
> new kernels. Selftests are validating this explicitly for unexisting BPF
> helper.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
[...]
> +static int bpf_object__resolve_externs(struct bpf_object *obj,
> +				       const char *config_path)
> +{
> +	bool need_config = false;
> +	struct extern_desc *ext;
> +	int err, i;
> +	void *data;
> +
> +	if (obj->nr_extern == 0)
> +		return 0;
> +
> +	data = obj->maps[obj->extern_map_idx].mmaped;
> +
> +	for (i = 0; i < obj->nr_extern; i++) {
> +		ext = &obj->externs[i];
> +
> +		if (strcmp(ext->name, "LINUX_KERNEL_VERSION") == 0) {
> +			void *ext_val = data + ext->data_off;
> +			__u32 kver = get_kernel_version();
> +
> +			if (!kver) {
> +				pr_warn("failed to get kernel version\n");
> +				return -EINVAL;
> +			}
> +			err = set_ext_value_num(ext, ext_val, kver);
> +			if (err)
> +				return err;
> +			pr_debug("extern %s=0x%x\n", ext->name, kver);
> +		} else if (strncmp(ext->name, "CONFIG_", 7) == 0) {
> +			need_config = true;
> +		} else {
> +			pr_warn("unrecognized extern '%s'\n", ext->name);
> +			return -EINVAL;
> +		}

I don't quite like that this is (mainly) tracing-only specific, and that
for everything else we just bail out - there is much more potential than
just completing above vars. But also, there is also no way to opt-out
for application developers of /this specific/ semi-magic auto-completion
of externs.

bpf_object__resolve_externs() should be changed instead to invoke a
callback obj->resolve_externs(). Former can be passed by the application
developer to allow them to take care of extern resolution all by themself,
and if no callback has been passed, then we default to the one above
being set as obj->resolve_externs.

> +	}
> +	if (need_config) {
> +		err = bpf_object__read_kernel_config(obj, config_path, data);
> +		if (err)
> +			return -EINVAL;
> +	}
> +	for (i = 0; i < obj->nr_extern; i++) {
> +		ext = &obj->externs[i];
> +
> +		if (!ext->is_set && !ext->is_weak) {
> +			pr_warn("extern %s (strong) not resolved\n", ext->name);
> +			return -ESRCH;
> +		} else if (!ext->is_set) {
> +			pr_debug("extern %s (weak) not resolved, defaulting to zero\n",
> +				 ext->name);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  int bpf_object__load_xattr(struct bpf_object_load_attr *attr)
>  {
>  	struct bpf_object *obj;
> @@ -4126,6 +4753,7 @@ int bpf_object__load_xattr(struct bpf_object_load_attr *attr)
>  	obj->loaded = true;
>  
>  	err = bpf_object__probe_caps(obj);
> +	err = err ? : bpf_object__resolve_externs(obj, obj->kconfig_path);
>  	err = err ? : bpf_object__sanitize_and_load_btf(obj);
>  	err = err ? : bpf_object__sanitize_maps(obj);
>  	err = err ? : bpf_object__create_maps(obj);
[...]

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

* Re: [PATCH v4 bpf-next 2/4] libbpf: support libbpf-provided extern variables
  2019-12-14  1:47 ` [PATCH v4 bpf-next 2/4] libbpf: support libbpf-provided extern variables Andrii Nakryiko
  2019-12-14 12:50   ` Toke Høiland-Jørgensen
  2019-12-16 11:17   ` Daniel Borkmann
@ 2019-12-16 12:43   ` Daniel Borkmann
  2019-12-16 18:19     ` Andrii Nakryiko
  2 siblings, 1 reply; 21+ messages in thread
From: Daniel Borkmann @ 2019-12-16 12:43 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: bpf, netdev, ast, andrii.nakryiko, kernel-team

On Fri, Dec 13, 2019 at 05:47:08PM -0800, Andrii Nakryiko wrote:
[...]
> Config file itself is searched in /boot/config-$(uname -r) location with
> fallback to /proc/config.gz, unless config path is specified explicitly
> through bpf_object_open_opts' kernel_config_path option. Both gzipped and
> plain text formats are supported. Libbpf adds explicit dependency on zlib
> because of this, but this shouldn't be a problem, given libelf already depends
> on zlib.

Hm, given this seems to break the build and is not an essential feature,
can't we use the feature detection from tooling infra which you invoke
anyway to compile out bpf_object__read_kernel_config() internals and return
an error there? Build could warn perf-style what won't be available for
the user in that case.

https://patchwork.ozlabs.org/patch/1210213/

Also, does libbpf.pc.template need updating wrt zlib?

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

* Re: [PATCH v4 bpf-next 2/4] libbpf: support libbpf-provided extern variables
  2019-12-16 12:43   ` Daniel Borkmann
@ 2019-12-16 18:19     ` Andrii Nakryiko
  0 siblings, 0 replies; 21+ messages in thread
From: Andrii Nakryiko @ 2019-12-16 18:19 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov, Kernel Team

On Mon, Dec 16, 2019 at 4:43 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On Fri, Dec 13, 2019 at 05:47:08PM -0800, Andrii Nakryiko wrote:
> [...]
> > Config file itself is searched in /boot/config-$(uname -r) location with
> > fallback to /proc/config.gz, unless config path is specified explicitly
> > through bpf_object_open_opts' kernel_config_path option. Both gzipped and
> > plain text formats are supported. Libbpf adds explicit dependency on zlib
> > because of this, but this shouldn't be a problem, given libelf already depends
> > on zlib.
>
> Hm, given this seems to break the build and is not an essential feature,
> can't we use the feature detection from tooling infra which you invoke
> anyway to compile out bpf_object__read_kernel_config() internals and return
> an error there? Build could warn perf-style what won't be available for
> the user in that case.
>
> https://patchwork.ozlabs.org/patch/1210213/

libz is a dependency of libelf, so this doesn't really add any new
dependencies. Everywhere where libbpf could be built, both libelf and
libz should be present already. Unfortunately now that libz is
directly used by libbpf, though, it needs to be specified explicitly
in compiler invocation, which I missed for samples/bpf, sorry about
that.

>
> Also, does libbpf.pc.template need updating wrt zlib?

Yeah, wasn't aware of it, will post a follow-up patch adding -lz there. Thanks!

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

* Re: [PATCH v4 bpf-next 2/4] libbpf: support libbpf-provided extern variables
  2019-12-16 11:17   ` Daniel Borkmann
@ 2019-12-16 19:29     ` Andrii Nakryiko
  2019-12-17 14:42       ` Daniel Borkmann
  0 siblings, 1 reply; 21+ messages in thread
From: Andrii Nakryiko @ 2019-12-16 19:29 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov, Kernel Team

On Mon, Dec 16, 2019 at 3:17 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On Fri, Dec 13, 2019 at 05:47:08PM -0800, Andrii Nakryiko wrote:
> > Add support for extern variables, provided to BPF program by libbpf. Currently
> > the following extern variables are supported:
> >   - LINUX_KERNEL_VERSION; version of a kernel in which BPF program is
> >     executing, follows KERNEL_VERSION() macro convention, can be 4- and 8-byte
> >     long;
> >   - CONFIG_xxx values; a set of values of actual kernel config. Tristate,
> >     boolean, strings, and integer values are supported.
> >
> [...]
> >
> > All detected extern variables, are put into a separate .extern internal map.
> > It, similarly to .rodata map, is marked as read-only from BPF program side, as
> > well as is frozen on load. This allows BPF verifier to track extern values as
> > constants and perform enhanced branch prediction and dead code elimination.
> > This can be relied upon for doing kernel version/feature detection and using
> > potentially unsupported field relocations or BPF helpers in a CO-RE-based BPF
> > program, while still having a single version of BPF program running on old and
> > new kernels. Selftests are validating this explicitly for unexisting BPF
> > helper.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> [...]
> > +static int bpf_object__resolve_externs(struct bpf_object *obj,
> > +                                    const char *config_path)
> > +{
> > +     bool need_config = false;
> > +     struct extern_desc *ext;
> > +     int err, i;
> > +     void *data;
> > +
> > +     if (obj->nr_extern == 0)
> > +             return 0;
> > +
> > +     data = obj->maps[obj->extern_map_idx].mmaped;
> > +
> > +     for (i = 0; i < obj->nr_extern; i++) {
> > +             ext = &obj->externs[i];
> > +
> > +             if (strcmp(ext->name, "LINUX_KERNEL_VERSION") == 0) {
> > +                     void *ext_val = data + ext->data_off;
> > +                     __u32 kver = get_kernel_version();
> > +
> > +                     if (!kver) {
> > +                             pr_warn("failed to get kernel version\n");
> > +                             return -EINVAL;
> > +                     }
> > +                     err = set_ext_value_num(ext, ext_val, kver);
> > +                     if (err)
> > +                             return err;
> > +                     pr_debug("extern %s=0x%x\n", ext->name, kver);
> > +             } else if (strncmp(ext->name, "CONFIG_", 7) == 0) {
> > +                     need_config = true;
> > +             } else {
> > +                     pr_warn("unrecognized extern '%s'\n", ext->name);
> > +                     return -EINVAL;
> > +             }
>
> I don't quite like that this is (mainly) tracing-only specific, and that
> for everything else we just bail out - there is much more potential than
> just completing above vars. But also, there is also no way to opt-out
> for application developers of /this specific/ semi-magic auto-completion
> of externs.

What makes you think it's tracing only? While non-tracing apps
probably don't need to care about LINUX_KERNEL_VERSION, all of the
CONFIG_ stuff is useful and usable for any type of application.

As for opt-out, you can easily opt out by not using extern variables.

>
> bpf_object__resolve_externs() should be changed instead to invoke a
> callback obj->resolve_externs(). Former can be passed by the application
> developer to allow them to take care of extern resolution all by themself,
> and if no callback has been passed, then we default to the one above
> being set as obj->resolve_externs.

Can you elaborate on the use case you have in mind? The way I always
imagined BPF applications provide custom read-only parameters to BPF
side is through using .rodata variables. With skeleton it's super easy
to initialize them before BPF program is loaded, and their values will
be well-known by verifier and potentially optimized.

E.g., with skeleton, it becomes trivial. E.g., on BPF side:


const volatile int custom_ipv4;
const volatile bool feature_X_enabled;

...

if (custom_ipv4 && in_ipv4 != custom_ipv4)
  return 0;

if (feature_X_enabled) {
  /* do something fancy */
}

Then on userspace side:

/* instantiate skeleton */
skel = my_prog__open();

skel->rodata->custom_ipv4 = IP_AS_INT(1, 2, 3, 4);
if (/* should enable feature X*/)
    skel->rodata->feature_X_enabled = true;

my_prog__load(); /* load, verify, eliminate dead code and optimize */

So for application-specific stuff, there isn't really a need to use
externs to do that. Furthermore, I think allowing using externs as
just another way to specify application-specific configuration is
going to create a problem, potentially, as we'll have higher
probability of collisions with kernel-provided extersn (variables
and/or functions), or even externs provided by other
dynamically/statically linked BPF programs (once we have dynamic and
static linking, of course).

So if you still insist we need user to provide custom extern-parsing
logic, can you please elaborate on the use case details?

BTW, from discussion w/ Alexei on another thread, I think I'm going to
change kconfig_path option to just `kconfig`, which will specify
additional config in Kconfig format. This could be used by
applications to provide their own config, augmenting Kconfig with
custom overrides.


>
> > +     }
> > +     if (need_config) {
> > +             err = bpf_object__read_kernel_config(obj, config_path, data);
> > +             if (err)
> > +                     return -EINVAL;
> > +     }
> > +     for (i = 0; i < obj->nr_extern; i++) {
> > +             ext = &obj->externs[i];
> > +
> > +             if (!ext->is_set && !ext->is_weak) {
> > +                     pr_warn("extern %s (strong) not resolved\n", ext->name);
> > +                     return -ESRCH;
> > +             } else if (!ext->is_set) {
> > +                     pr_debug("extern %s (weak) not resolved, defaulting to zero\n",
> > +                              ext->name);
> > +             }
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> >  int bpf_object__load_xattr(struct bpf_object_load_attr *attr)
> >  {
> >       struct bpf_object *obj;
> > @@ -4126,6 +4753,7 @@ int bpf_object__load_xattr(struct bpf_object_load_attr *attr)
> >       obj->loaded = true;
> >
> >       err = bpf_object__probe_caps(obj);
> > +     err = err ? : bpf_object__resolve_externs(obj, obj->kconfig_path);
> >       err = err ? : bpf_object__sanitize_and_load_btf(obj);
> >       err = err ? : bpf_object__sanitize_maps(obj);
> >       err = err ? : bpf_object__create_maps(obj);
> [...]

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

* Re: [PATCH v4 bpf-next 0/4] Add libbpf-provided extern variables support
  2019-12-16  4:42     ` Alexei Starovoitov
@ 2019-12-16 19:34       ` Andrii Nakryiko
  0 siblings, 0 replies; 21+ messages in thread
From: Andrii Nakryiko @ 2019-12-16 19:34 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team

On Sun, Dec 15, 2019 at 8:42 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Sun, Dec 15, 2019 at 05:47:01PM -0800, Andrii Nakryiko wrote:
> > On Sun, Dec 15, 2019 at 4:52 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Fri, Dec 13, 2019 at 05:47:06PM -0800, Andrii Nakryiko wrote:
> > > > It's often important for BPF program to know kernel version or some specific
> > > > config values (e.g., CONFIG_HZ to convert jiffies to seconds) and change or
> > > > adjust program logic based on their values. As of today, any such need has to
> > > > be resolved by recompiling BPF program for specific kernel and kernel
> > > > configuration. In practice this is usually achieved by using BCC and its
> > > > embedded LLVM/Clang. With such set up #ifdef CONFIG_XXX and similar
> > > > compile-time constructs allow to deal with kernel varieties.
> > > >
> > > > With CO-RE (Compile Once – Run Everywhere) approach, this is not an option,
> > > > unfortunately. All such logic variations have to be done as a normal
> > > > C language constructs (i.e., if/else, variables, etc), not a preprocessor
> > > > directives. This patch series add support for such advanced scenarios through
> > > > C extern variables. These extern variables will be recognized by libbpf and
> > > > supplied through extra .extern internal map, similarly to global data. This
> > > > .extern map is read-only, which allows BPF verifier to track its content
> > > > precisely as constants. That gives an opportunity to have pre-compiled BPF
> > > > program, which can potentially use BPF functionality (e.g., BPF helpers) or
> > > > kernel features (types, fields, etc), that are available only on a subset of
> > > > targeted kernels, while effectively eleminating (through verifier's dead code
> > > > detection) such unsupported functionality for other kernels (typically, older
> > > > versions). Patch #3 explicitly tests a scenario of using unsupported BPF
> > > > helper, to validate the approach.
> > > >
> > > > This patch set heavily relies on BTF type information emitted by compiler for
> > > > each extern variable declaration. Based on specific types, libbpf does strict
> > > > checks of config data values correctness. See patch #1 for details.
> > > >
> > > > Outline of the patch set:
> > > > - patch #1 does a small clean up of internal map names contants;
> > > > - patch #2 adds all of the libbpf internal machinery for externs support,
> > > >   including setting up BTF information for .extern data section;
> > > > - patch #3 adds support for .extern into BPF skeleton;
> > > > - patch #4 adds externs selftests, as well as enhances test_skeleton.c test to
> > > >   validate mmap()-ed .extern datasection functionality.
> > >
> > > Applied. Thanks.
> >
> > Great, thanks!
> >
> > >
> > > Looking at the tests that do mkstemp()+write() just to pass a file path
> > > as .kconfig_path option into bpf_object_open_opts() it feels that file only
> > > support for externs is unnecessary limiting. I think it will simplify
> >
> > yeah, it was a bit painful :)
> >
> > > tests and will make the whole extern support more flexible if in addition to
> > > kconfig_path bpf_object_open_opts() would support in-memory configuration.
> >
> > I wanted to keep it simple for users, in case libbpf can't find config
> > file, to just specify its location. But given your feedback here, and
> > you mentioned previously that it would be nice to allow users to
> > specify custom kconfig-like configuration to be exposed as externs as
> > well, how about replacing .kconfig_path, which is a patch to config
> > file, with just .kconfig, which is the **contents** of config file.
> > That way we can support all of the above scenarios, if maybe sometime
> > with a tiny bit of extra work for users:
> >
> > 1. Override real kconfig with custom config (e.g., for testing
> > purposes) - just specify alternative contents.
> > 2. Extend kconfig with some extra configuration - user will have to
> > read real kconfig, then append (or prepend, doesn't matter) custom
> > contents.
> >
> > What I want to avoid is having multiple ways to do this, having to
> > decide whether to augment real Kconfig or completely override it, etc.
> > So one string-based config override is preferable for simplicity.
> > Agreed?
>
> I think user experience would be better if users don't have to know that
> /proc/config.gz exists and even more so if they don't need to read it. By
> default libbpf should pick all CONFIG_* from known locations and in addition if
> extra text is specified for bpf_object_open_opts() the libbpf can take the
> values from there. So may be .kconfig_path can be replaced with
> .additional_key_value_pairs ? I think override of /proc/config.gz is
> unnecessary. Whereas additional predefined externs are useful for testing and
> passing load-time configuration to bpf progs. Like IP addresses, etc.

Yes, agree about usability issues w/ user having to read and unzip
/proc/config.gz. I will add kconfig (const char*) to mean
additions/overrides to Kconfig, which applications could use to
override and augment Kconfig options for their needs. I'd still like
to keep the convention of CONFIG_ and reserve everything else for
static/dynamic linking use case (and kernel-provided externs, as a
special case of dynamic linking). Makes sense?

Daniel expressed concern about opting out of Kconfig parsing
altogether, so as a way to do this, I can keep kconfig_path option
anyways, and define that empty string means "no Kconfig parsing". But
let's see if Daniel still thinks it a problem.

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

* Re: [PATCH v4 bpf-next 2/4] libbpf: support libbpf-provided extern variables
  2019-12-16 19:29     ` Andrii Nakryiko
@ 2019-12-17 14:42       ` Daniel Borkmann
  2019-12-17 19:03         ` Andrii Nakryiko
  0 siblings, 1 reply; 21+ messages in thread
From: Daniel Borkmann @ 2019-12-17 14:42 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov, Kernel Team

On 12/16/19 8:29 PM, Andrii Nakryiko wrote:
> On Mon, Dec 16, 2019 at 3:17 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>> On Fri, Dec 13, 2019 at 05:47:08PM -0800, Andrii Nakryiko wrote:
>>> Add support for extern variables, provided to BPF program by libbpf. Currently
>>> the following extern variables are supported:
>>>    - LINUX_KERNEL_VERSION; version of a kernel in which BPF program is
>>>      executing, follows KERNEL_VERSION() macro convention, can be 4- and 8-byte
>>>      long;
>>>    - CONFIG_xxx values; a set of values of actual kernel config. Tristate,
>>>      boolean, strings, and integer values are supported.
>>>
>> [...]
>>>
>>> All detected extern variables, are put into a separate .extern internal map.
>>> It, similarly to .rodata map, is marked as read-only from BPF program side, as
>>> well as is frozen on load. This allows BPF verifier to track extern values as
>>> constants and perform enhanced branch prediction and dead code elimination.
>>> This can be relied upon for doing kernel version/feature detection and using
>>> potentially unsupported field relocations or BPF helpers in a CO-RE-based BPF
>>> program, while still having a single version of BPF program running on old and
>>> new kernels. Selftests are validating this explicitly for unexisting BPF
>>> helper.
>>>
>>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>> [...]
>>> +static int bpf_object__resolve_externs(struct bpf_object *obj,
>>> +                                    const char *config_path)
>>> +{
>>> +     bool need_config = false;
>>> +     struct extern_desc *ext;
>>> +     int err, i;
>>> +     void *data;
>>> +
>>> +     if (obj->nr_extern == 0)
>>> +             return 0;
>>> +
>>> +     data = obj->maps[obj->extern_map_idx].mmaped;
>>> +
>>> +     for (i = 0; i < obj->nr_extern; i++) {
>>> +             ext = &obj->externs[i];
>>> +
>>> +             if (strcmp(ext->name, "LINUX_KERNEL_VERSION") == 0) {
>>> +                     void *ext_val = data + ext->data_off;
>>> +                     __u32 kver = get_kernel_version();
>>> +
>>> +                     if (!kver) {
>>> +                             pr_warn("failed to get kernel version\n");
>>> +                             return -EINVAL;
>>> +                     }
>>> +                     err = set_ext_value_num(ext, ext_val, kver);
>>> +                     if (err)
>>> +                             return err;
>>> +                     pr_debug("extern %s=0x%x\n", ext->name, kver);
>>> +             } else if (strncmp(ext->name, "CONFIG_", 7) == 0) {
>>> +                     need_config = true;
>>> +             } else {
>>> +                     pr_warn("unrecognized extern '%s'\n", ext->name);
>>> +                     return -EINVAL;
>>> +             }
>>
>> I don't quite like that this is (mainly) tracing-only specific, and that
>> for everything else we just bail out - there is much more potential than
>> just completing above vars. But also, there is also no way to opt-out
>> for application developers of /this specific/ semi-magic auto-completion
>> of externs.
> 
> What makes you think it's tracing only? While non-tracing apps
> probably don't need to care about LINUX_KERNEL_VERSION, all of the
> CONFIG_ stuff is useful and usable for any type of application.

Ok, just curious, do you have a concrete example e.g. for tc/xdp networking
programs where you have a specific CONFIG_* extern that you're using in your
programs currently?

> As for opt-out, you can easily opt out by not using extern variables.
> 
>> bpf_object__resolve_externs() should be changed instead to invoke a
>> callback obj->resolve_externs(). Former can be passed by the application
>> developer to allow them to take care of extern resolution all by themself,
>> and if no callback has been passed, then we default to the one above
>> being set as obj->resolve_externs.
> 
> Can you elaborate on the use case you have in mind? The way I always
> imagined BPF applications provide custom read-only parameters to BPF
> side is through using .rodata variables. With skeleton it's super easy
> to initialize them before BPF program is loaded, and their values will
> be well-known by verifier and potentially optimized.

We do set the map with .extern contents as read-only memory as well in
libbpf as I understand it. So same dead code optimization from the
verifier as well in this case. It feels the line with passing external
config via .rodata variables vs .extern variables gets therefore a bit
blurry.

Are we saying that in libbpf convention is that .extern contents must
*only* ever be kernel-provided but cannot come from some other configuration
data such as env/daemon?

[...]
> So for application-specific stuff, there isn't really a need to use
> externs to do that. Furthermore, I think allowing using externs as
> just another way to specify application-specific configuration is
> going to create a problem, potentially, as we'll have higher
> probability of collisions with kernel-provided extersn (variables
> and/or functions), or even externs provided by other
> dynamically/statically linked BPF programs (once we have dynamic and
> static linking, of course).

Yes, that makes more sense, but then we are already potentially colliding
with current CONFIG_* variables once we handle dynamically / statically
linked BPF programs. Perhaps that's my confusion in the first place. Would
have been good if 166750bc1dd2 had a discussion on that as part of the
commit message.

So, naive question, what's the rationale of not using .rodata variables
for CONFIG_* case and how do we handle these .extern collisions in future?
Should these vars rather have had some sort of annotation or be moved into
special ".extern.config" section or the like where we explicitly know that
these are handled differently so they don't collide with future ".extern"
content once we have linked BPF programs?

> So if you still insist we need user to provide custom extern-parsing
> logic, can you please elaborate on the use case details?
> 
> BTW, from discussion w/ Alexei on another thread, I think I'm going to
> change kconfig_path option to just `kconfig`, which will specify
> additional config in Kconfig format. This could be used by
> applications to provide their own config, augmenting Kconfig with
> custom overrides.

Ok.

Thanks,
Daniel

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

* Re: [PATCH v4 bpf-next 2/4] libbpf: support libbpf-provided extern variables
  2019-12-17 14:42       ` Daniel Borkmann
@ 2019-12-17 19:03         ` Andrii Nakryiko
  2019-12-17 19:50           ` Daniel Borkmann
  0 siblings, 1 reply; 21+ messages in thread
From: Andrii Nakryiko @ 2019-12-17 19:03 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov, Kernel Team

On Tue, Dec 17, 2019 at 6:42 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 12/16/19 8:29 PM, Andrii Nakryiko wrote:
> > On Mon, Dec 16, 2019 at 3:17 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> >> On Fri, Dec 13, 2019 at 05:47:08PM -0800, Andrii Nakryiko wrote:
> >>> Add support for extern variables, provided to BPF program by libbpf. Currently
> >>> the following extern variables are supported:
> >>>    - LINUX_KERNEL_VERSION; version of a kernel in which BPF program is
> >>>      executing, follows KERNEL_VERSION() macro convention, can be 4- and 8-byte
> >>>      long;
> >>>    - CONFIG_xxx values; a set of values of actual kernel config. Tristate,
> >>>      boolean, strings, and integer values are supported.
> >>>
> >> [...]
> >>>
> >>> All detected extern variables, are put into a separate .extern internal map.
> >>> It, similarly to .rodata map, is marked as read-only from BPF program side, as
> >>> well as is frozen on load. This allows BPF verifier to track extern values as
> >>> constants and perform enhanced branch prediction and dead code elimination.
> >>> This can be relied upon for doing kernel version/feature detection and using
> >>> potentially unsupported field relocations or BPF helpers in a CO-RE-based BPF
> >>> program, while still having a single version of BPF program running on old and
> >>> new kernels. Selftests are validating this explicitly for unexisting BPF
> >>> helper.
> >>>
> >>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> >> [...]
> >>> +static int bpf_object__resolve_externs(struct bpf_object *obj,
> >>> +                                    const char *config_path)
> >>> +{
> >>> +     bool need_config = false;
> >>> +     struct extern_desc *ext;
> >>> +     int err, i;
> >>> +     void *data;
> >>> +
> >>> +     if (obj->nr_extern == 0)
> >>> +             return 0;
> >>> +
> >>> +     data = obj->maps[obj->extern_map_idx].mmaped;
> >>> +
> >>> +     for (i = 0; i < obj->nr_extern; i++) {
> >>> +             ext = &obj->externs[i];
> >>> +
> >>> +             if (strcmp(ext->name, "LINUX_KERNEL_VERSION") == 0) {
> >>> +                     void *ext_val = data + ext->data_off;
> >>> +                     __u32 kver = get_kernel_version();
> >>> +
> >>> +                     if (!kver) {
> >>> +                             pr_warn("failed to get kernel version\n");
> >>> +                             return -EINVAL;
> >>> +                     }
> >>> +                     err = set_ext_value_num(ext, ext_val, kver);
> >>> +                     if (err)
> >>> +                             return err;
> >>> +                     pr_debug("extern %s=0x%x\n", ext->name, kver);
> >>> +             } else if (strncmp(ext->name, "CONFIG_", 7) == 0) {
> >>> +                     need_config = true;
> >>> +             } else {
> >>> +                     pr_warn("unrecognized extern '%s'\n", ext->name);
> >>> +                     return -EINVAL;
> >>> +             }
> >>
> >> I don't quite like that this is (mainly) tracing-only specific, and that
> >> for everything else we just bail out - there is much more potential than
> >> just completing above vars. But also, there is also no way to opt-out
> >> for application developers of /this specific/ semi-magic auto-completion
> >> of externs.
> >
> > What makes you think it's tracing only? While non-tracing apps
> > probably don't need to care about LINUX_KERNEL_VERSION, all of the
> > CONFIG_ stuff is useful and usable for any type of application.
>
> Ok, just curious, do you have a concrete example e.g. for tc/xdp networking
> programs where you have a specific CONFIG_* extern that you're using in your
> programs currently?

So one good example will be CONFIG_HZ, used by latest Martin's work on
tcp_congestion_ops, as well as used by other networking BPF programs
for network stats.

>
> > As for opt-out, you can easily opt out by not using extern variables.
> >
> >> bpf_object__resolve_externs() should be changed instead to invoke a
> >> callback obj->resolve_externs(). Former can be passed by the application
> >> developer to allow them to take care of extern resolution all by themself,
> >> and if no callback has been passed, then we default to the one above
> >> being set as obj->resolve_externs.
> >
> > Can you elaborate on the use case you have in mind? The way I always
> > imagined BPF applications provide custom read-only parameters to BPF
> > side is through using .rodata variables. With skeleton it's super easy
> > to initialize them before BPF program is loaded, and their values will
> > be well-known by verifier and potentially optimized.
>
> We do set the map with .extern contents as read-only memory as well in
> libbpf as I understand it. So same dead code optimization from the
> verifier as well in this case. It feels the line with passing external
> config via .rodata variables vs .extern variables gets therefore a bit
> blurry.
>
> Are we saying that in libbpf convention is that .extern contents must
> *only* ever be kernel-provided but cannot come from some other configuration
> data such as env/daemon?

You are right that .rodata and .extern are both read-only and allow
dead code optimization, which was a requirement for both. The line is
between who "owns" the data. .rodata should be parameters owned and
set up by BPF program and its userspace counterpart. While .extern is
something that's owned and provided by some external party. It is
libbpf for Kconfig, in the future it might be also kernel variables
(e.g., jiffies). With static and dynamic linking, extern variables
will be variables defined in some other BPF object, that we are
linking with.

>
> [...]
> > So for application-specific stuff, there isn't really a need to use
> > externs to do that. Furthermore, I think allowing using externs as
> > just another way to specify application-specific configuration is
> > going to create a problem, potentially, as we'll have higher
> > probability of collisions with kernel-provided extersn (variables
> > and/or functions), or even externs provided by other
> > dynamically/statically linked BPF programs (once we have dynamic and
> > static linking, of course).
>
> Yes, that makes more sense, but then we are already potentially colliding
> with current CONFIG_* variables once we handle dynamically / statically
> linked BPF programs. Perhaps that's my confusion in the first place. Would
> have been good if 166750bc1dd2 had a discussion on that as part of the
> commit message.
>
> So, naive question, what's the rationale of not using .rodata variables
> for CONFIG_* case and how do we handle these .extern collisions in future?
> Should these vars rather have had some sort of annotation or be moved into
> special ".extern.config" section or the like where we explicitly know that
> these are handled differently so they don't collide with future ".extern"
> content once we have linked BPF programs?

Yes, name collision is a possibility, which means users should
restrain from using LINUX_KERNEL_VERSION and CONFIG_XXX names for
their variables. But if that is ever actually the problem, the way to
resolve this collision/ambiguity would be to put externs in a separate
sections. It's possible to annotate extern variable with custom
section.

But I guess putting Kconfig-provided externs into ".extern.kconfig"
might be a good idea, actually. That will make it possible to have
writable externs in the future.

>
> > So if you still insist we need user to provide custom extern-parsing
> > logic, can you please elaborate on the use case details?
> >
> > BTW, from discussion w/ Alexei on another thread, I think I'm going to
> > change kconfig_path option to just `kconfig`, which will specify
> > additional config in Kconfig format. This could be used by
> > applications to provide their own config, augmenting Kconfig with
> > custom overrides.
>
> Ok.
>
> Thanks,
> Daniel

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

* Re: [PATCH v4 bpf-next 2/4] libbpf: support libbpf-provided extern variables
  2019-12-17 19:03         ` Andrii Nakryiko
@ 2019-12-17 19:50           ` Daniel Borkmann
  2019-12-17 20:16             ` Alexei Starovoitov
  0 siblings, 1 reply; 21+ messages in thread
From: Daniel Borkmann @ 2019-12-17 19:50 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov, Kernel Team

On 12/17/19 8:03 PM, Andrii Nakryiko wrote:
> On Tue, Dec 17, 2019 at 6:42 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>> On 12/16/19 8:29 PM, Andrii Nakryiko wrote:
>>> On Mon, Dec 16, 2019 at 3:17 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>>>> On Fri, Dec 13, 2019 at 05:47:08PM -0800, Andrii Nakryiko wrote:
[...]
>>> So for application-specific stuff, there isn't really a need to use
>>> externs to do that. Furthermore, I think allowing using externs as
>>> just another way to specify application-specific configuration is
>>> going to create a problem, potentially, as we'll have higher
>>> probability of collisions with kernel-provided extersn (variables
>>> and/or functions), or even externs provided by other
>>> dynamically/statically linked BPF programs (once we have dynamic and
>>> static linking, of course).
>>
>> Yes, that makes more sense, but then we are already potentially colliding
>> with current CONFIG_* variables once we handle dynamically / statically
>> linked BPF programs. Perhaps that's my confusion in the first place. Would
>> have been good if 166750bc1dd2 had a discussion on that as part of the
>> commit message.
>>
>> So, naive question, what's the rationale of not using .rodata variables
>> for CONFIG_* case and how do we handle these .extern collisions in future?
>> Should these vars rather have had some sort of annotation or be moved into
>> special ".extern.config" section or the like where we explicitly know that
>> these are handled differently so they don't collide with future ".extern"
>> content once we have linked BPF programs?
> 
> Yes, name collision is a possibility, which means users should
> restrain from using LINUX_KERNEL_VERSION and CONFIG_XXX names for
> their variables. But if that is ever actually the problem, the way to
> resolve this collision/ambiguity would be to put externs in a separate
> sections. It's possible to annotate extern variable with custom
> section.
> 
> But I guess putting Kconfig-provided externs into ".extern.kconfig"
> might be a good idea, actually. That will make it possible to have
> writable externs in the future.

Yep, and as mentioned it will make it more clear that these get special
loader treatment as opposed to regular externs we need to deal with in
future. A '.extern.kconfig' section sounds good to me and the BPF helper
header could provide a __kconfig annotation for that as well.

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

* Re: [PATCH v4 bpf-next 2/4] libbpf: support libbpf-provided extern variables
  2019-12-17 19:50           ` Daniel Borkmann
@ 2019-12-17 20:16             ` Alexei Starovoitov
  2019-12-17 23:37               ` Daniel Borkmann
  0 siblings, 1 reply; 21+ messages in thread
From: Alexei Starovoitov @ 2019-12-17 20:16 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Andrii Nakryiko, Andrii Nakryiko, bpf, Networking,
	Alexei Starovoitov, Kernel Team

On Tue, Dec 17, 2019 at 08:50:31PM +0100, Daniel Borkmann wrote:
> > 
> > Yes, name collision is a possibility, which means users should
> > restrain from using LINUX_KERNEL_VERSION and CONFIG_XXX names for
> > their variables. But if that is ever actually the problem, the way to
> > resolve this collision/ambiguity would be to put externs in a separate
> > sections. It's possible to annotate extern variable with custom
> > section.
> > 
> > But I guess putting Kconfig-provided externs into ".extern.kconfig"
> > might be a good idea, actually. That will make it possible to have
> > writable externs in the future.
> 
> Yep, and as mentioned it will make it more clear that these get special
> loader treatment as opposed to regular externs we need to deal with in
> future. A '.extern.kconfig' section sounds good to me and the BPF helper
> header could provide a __kconfig annotation for that as well.

I think annotating all extern vars into special section name will be quite
cumbersome from bpf program writer pov.
imo capital case extern variables LINUX_KERNEL_VERSION and CONFIG_XXX are
distinct enough and make it clear they should come from something other than
normal C. Traditional C coding style uses all capital letters for macroses. So
all capital extern variables are unlikely to conflict with any normal extern
vars. Like vars in vmlinux and vars in other bpf elf files.

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

* Re: [PATCH v4 bpf-next 2/4] libbpf: support libbpf-provided extern variables
  2019-12-17 20:16             ` Alexei Starovoitov
@ 2019-12-17 23:37               ` Daniel Borkmann
  2019-12-18  0:08                 ` Andrii Nakryiko
  0 siblings, 1 reply; 21+ messages in thread
From: Daniel Borkmann @ 2019-12-17 23:37 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andrii Nakryiko, Andrii Nakryiko, bpf, Networking,
	Alexei Starovoitov, Kernel Team

On 12/17/19 9:16 PM, Alexei Starovoitov wrote:
> On Tue, Dec 17, 2019 at 08:50:31PM +0100, Daniel Borkmann wrote:
>>>
>>> Yes, name collision is a possibility, which means users should
>>> restrain from using LINUX_KERNEL_VERSION and CONFIG_XXX names for
>>> their variables. But if that is ever actually the problem, the way to
>>> resolve this collision/ambiguity would be to put externs in a separate
>>> sections. It's possible to annotate extern variable with custom
>>> section.
>>>
>>> But I guess putting Kconfig-provided externs into ".extern.kconfig"
>>> might be a good idea, actually. That will make it possible to have
>>> writable externs in the future.
>>
>> Yep, and as mentioned it will make it more clear that these get special
>> loader treatment as opposed to regular externs we need to deal with in
>> future. A '.extern.kconfig' section sounds good to me and the BPF helper
>> header could provide a __kconfig annotation for that as well.
> 
> I think annotating all extern vars into special section name will be quite
> cumbersome from bpf program writer pov.
> imo capital case extern variables LINUX_KERNEL_VERSION and CONFIG_XXX are
> distinct enough and make it clear they should come from something other than
> normal C. Traditional C coding style uses all capital letters for macroses. So
> all capital extern variables are unlikely to conflict with any normal extern
> vars. Like vars in vmlinux and vars in other bpf elf files.

But still, how many of the LINUX_KERNEL_VERSION or CONFIG_XXX vars are actually
used per program. I bet just a handful. And I don't think adding a __kconfig is
cumbersome, it would make it more self-documenting in fact, denoting that this
var is not treated the usual way once prog linking is in place. Even if all
capital letters. Tomorrow, we'd be adding 'extern unsigned long jiffies' as
another potential example, and then it gets even more confusing on the 'collision'
side with regular BPF ELF. Same here, instead of __kconfig, this could have a
__vmlinux or __kernel annotation in order to document its source for the loader
(and developer) more clearly and also gives flexibility wrt ".extern.xyz"
subsections on how we want to map them.

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

* Re: [PATCH v4 bpf-next 2/4] libbpf: support libbpf-provided extern variables
  2019-12-17 23:37               ` Daniel Borkmann
@ 2019-12-18  0:08                 ` Andrii Nakryiko
  0 siblings, 0 replies; 21+ messages in thread
From: Andrii Nakryiko @ 2019-12-18  0:08 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Networking,
	Alexei Starovoitov, Kernel Team

On Tue, Dec 17, 2019 at 3:37 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 12/17/19 9:16 PM, Alexei Starovoitov wrote:
> > On Tue, Dec 17, 2019 at 08:50:31PM +0100, Daniel Borkmann wrote:
> >>>
> >>> Yes, name collision is a possibility, which means users should
> >>> restrain from using LINUX_KERNEL_VERSION and CONFIG_XXX names for
> >>> their variables. But if that is ever actually the problem, the way to
> >>> resolve this collision/ambiguity would be to put externs in a separate
> >>> sections. It's possible to annotate extern variable with custom
> >>> section.
> >>>
> >>> But I guess putting Kconfig-provided externs into ".extern.kconfig"
> >>> might be a good idea, actually. That will make it possible to have
> >>> writable externs in the future.
> >>
> >> Yep, and as mentioned it will make it more clear that these get special
> >> loader treatment as opposed to regular externs we need to deal with in
> >> future. A '.extern.kconfig' section sounds good to me and the BPF helper
> >> header could provide a __kconfig annotation for that as well.
> >
> > I think annotating all extern vars into special section name will be quite
> > cumbersome from bpf program writer pov.
> > imo capital case extern variables LINUX_KERNEL_VERSION and CONFIG_XXX are
> > distinct enough and make it clear they should come from something other than
> > normal C. Traditional C coding style uses all capital letters for macroses. So
> > all capital extern variables are unlikely to conflict with any normal extern
> > vars. Like vars in vmlinux and vars in other bpf elf files.
>
> But still, how many of the LINUX_KERNEL_VERSION or CONFIG_XXX vars are actually
> used per program. I bet just a handful. And I don't think adding a __kconfig is
> cumbersome, it would make it more self-documenting in fact, denoting that this
> var is not treated the usual way once prog linking is in place. Even if all
> capital letters. Tomorrow, we'd be adding 'extern unsigned long jiffies' as
> another potential example, and then it gets even more confusing on the 'collision'
> side with regular BPF ELF. Same here, instead of __kconfig, this could have a
> __vmlinux or __kernel annotation in order to document its source for the loader
> (and developer) more clearly and also gives flexibility wrt ".extern.xyz"
> subsections on how we want to map them.

Sounds reasonable and clean enough. Let me play a bit with this and
see how this all plays together.

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

end of thread, other threads:[~2019-12-18  0:08 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-14  1:47 [PATCH v4 bpf-next 0/4] Add libbpf-provided extern variables support Andrii Nakryiko
2019-12-14  1:47 ` [PATCH v4 bpf-next 1/4] libbpf: extract internal map names into constants Andrii Nakryiko
2019-12-14  1:47 ` [PATCH v4 bpf-next 2/4] libbpf: support libbpf-provided extern variables Andrii Nakryiko
2019-12-14 12:50   ` Toke Høiland-Jørgensen
2019-12-14 20:27     ` Yonghong Song
2019-12-16 11:17   ` Daniel Borkmann
2019-12-16 19:29     ` Andrii Nakryiko
2019-12-17 14:42       ` Daniel Borkmann
2019-12-17 19:03         ` Andrii Nakryiko
2019-12-17 19:50           ` Daniel Borkmann
2019-12-17 20:16             ` Alexei Starovoitov
2019-12-17 23:37               ` Daniel Borkmann
2019-12-18  0:08                 ` Andrii Nakryiko
2019-12-16 12:43   ` Daniel Borkmann
2019-12-16 18:19     ` Andrii Nakryiko
2019-12-14  1:47 ` [PATCH v4 bpf-next 3/4] bpftool: generate externs datasec in BPF skeleton Andrii Nakryiko
2019-12-14  1:47 ` [PATCH v4 bpf-next 4/4] selftests/bpf: add tests for libbpf-provided externs Andrii Nakryiko
2019-12-16  0:52 ` [PATCH v4 bpf-next 0/4] Add libbpf-provided extern variables support Alexei Starovoitov
2019-12-16  1:47   ` Andrii Nakryiko
2019-12-16  4:42     ` Alexei Starovoitov
2019-12-16 19:34       ` 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).