[PATCH 0/8] tools lib bpf: fixes and functional upgrade

[PATCH 0/8] tools lib bpf: fixes and functional upgrade

[Eric Leblond]

Hello,

Here's a patchset on the libbpf library that can be found in
tools/lib/bpf.

Patch 0 to patch 4 add a new function to be able to set the BPF
program type. Till then program type such as network filter can't
be loaded by the library:

* tools lib bpf: add error functions
* uapi linux bpf: add max value to enum
* tools: Sync tools/include/uapi/linux/bpf.h with the
* tools lib bpf: export function to set type

Patch 5 is adding functions that were missing to handle maps in
userspace.

* tools lib bpf: add missing functions

Patch 7 fixes a bug in the parsing of BPF ELF file.

* tools lib bpf: fix maps resolution

Patch 8 update 'make install' to install the header on the system.

* tools lib bpf: install header file


Patchset statistics:
 include/uapi/linux/bpf.h       |  1 +
 tools/include/uapi/linux/bpf.h | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 tools/lib/bpf/Makefile         | 11 +++++++++--
 tools/lib/bpf/bpf.c            | 35 ++++++++++++++++++++++++++++++++++-
 tools/lib/bpf/bpf.h            |  2 --
 tools/lib/bpf/libbpf.c         | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------
 tools/lib/bpf/libbpf.h         | 12 +++++++++++-
 7 files changed, 166 insertions(+), 34 deletions(-)

Best regards,
--
Eric Leblond

[PATCH 1/8] tools lib bpf: add error functions

[Eric Leblond]

The include of err.h is not explicitely needed in exported
functions and it was causing include conflict with some existing
code due to redefining some macros.

To fix this, let's have error handling functions provided by the
library. Furthermore this will allow user to have an homogeneous
API.

Signed-off-by: Eric Leblond <eric@regit.org>
---
 tools/lib/bpf/libbpf.c | 11 +++++++++++
 tools/lib/bpf/libbpf.h |  4 +++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index b699aea..90932f1 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -31,6 +31,7 @@
 #include <linux/kernel.h>
 #include <linux/bpf.h>
 #include <linux/list.h>
+#include <linux/err.h>
 #include <libelf.h>
 #include <gelf.h>
 
@@ -1447,3 +1448,13 @@ bpf_object__find_map_by_name(struct bpf_object *obj, const char *name)
 	}
 	return NULL;
 }
+
+bool bpf__is_error(const void *ptr)
+{
+	return IS_ERR(ptr);
+}
+
+long bpf__get_error(const void *ptr)
+{
+	return PTR_ERR(ptr);
+}
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index dd7a513..e40c8d3 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -23,7 +23,6 @@
 
 #include <stdio.h>
 #include <stdbool.h>
-#include <linux/err.h>
 
 enum libbpf_errno {
 	__LIBBPF_ERRNO__START = 4000,
@@ -211,4 +210,7 @@ int bpf_map__set_priv(struct bpf_map *map, void *priv,
 		      bpf_map_clear_priv_t clear_priv);
 void *bpf_map__priv(struct bpf_map *map);
 
+bool bpf__is_error(const void *ptr);
+long bpf__get_error(const void *ptr);
+
 #endif
-- 
2.9.3

[PATCH 2/8] uapi linux bpf: add max value to enum

[Eric Leblond]

It will be used to detect userspace trying to set invalid value.

Signed-off-by: Eric Leblond <eric@regit.org>
---
 include/uapi/linux/bpf.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index f09c70b..570287f 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -96,6 +96,7 @@ enum bpf_prog_type {
 	BPF_PROG_TYPE_TRACEPOINT,
 	BPF_PROG_TYPE_XDP,
 	BPF_PROG_TYPE_PERF_EVENT,
+	__MAX_BPF_PROG_TYPE,
 };
 
 #define BPF_PSEUDO_MAP_FD	1
-- 
2.9.3

[PATCH 3/8] tools: Sync tools/include/uapi/linux/bpf.h with the kernel

[Eric Leblond]

Signed-off-by: Eric Leblond <eric@regit.org>
---
 tools/include/uapi/linux/bpf.h | 52 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 9e5fc16..570287f 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -95,6 +95,8 @@ enum bpf_prog_type {
 	BPF_PROG_TYPE_SCHED_ACT,
 	BPF_PROG_TYPE_TRACEPOINT,
 	BPF_PROG_TYPE_XDP,
+	BPF_PROG_TYPE_PERF_EVENT,
+	__MAX_BPF_PROG_TYPE,
 };
 
 #define BPF_PSEUDO_MAP_FD	1
@@ -375,6 +377,56 @@ enum bpf_func_id {
 	 */
 	BPF_FUNC_probe_write_user,
 
+	/**
+	 * bpf_current_task_under_cgroup(map, index) - Check cgroup2 membership of current task
+	 * @map: pointer to bpf_map in BPF_MAP_TYPE_CGROUP_ARRAY type
+	 * @index: index of the cgroup in the bpf_map
+	 * Return:
+	 *   == 0 current failed the cgroup2 descendant test
+	 *   == 1 current succeeded the cgroup2 descendant test
+	 *    < 0 error
+	 */
+	BPF_FUNC_current_task_under_cgroup,
+
+	/**
+	 * bpf_skb_change_tail(skb, len, flags)
+	 * The helper will resize the skb to the given new size,
+	 * to be used f.e. with control messages.
+	 * @skb: pointer to skb
+	 * @len: new skb length
+	 * @flags: reserved
+	 * Return: 0 on success or negative error
+	 */
+	BPF_FUNC_skb_change_tail,
+
+	/**
+	 * bpf_skb_pull_data(skb, len)
+	 * The helper will pull in non-linear data in case the
+	 * skb is non-linear and not all of len are part of the
+	 * linear section. Only needed for read/write with direct
+	 * packet access.
+	 * @skb: pointer to skb
+	 * @len: len to make read/writeable
+	 * Return: 0 on success or negative error
+	 */
+	BPF_FUNC_skb_pull_data,
+
+	/**
+	 * bpf_csum_update(skb, csum)
+	 * Adds csum into skb->csum in case of CHECKSUM_COMPLETE.
+	 * @skb: pointer to skb
+	 * @csum: csum to add
+	 * Return: csum on success or negative error
+	 */
+	BPF_FUNC_csum_update,
+
+	/**
+	 * bpf_set_hash_invalid(skb)
+	 * Invalidate current skb>hash.
+	 * @skb: pointer to skb
+	 */
+	BPF_FUNC_set_hash_invalid,
+
 	__BPF_FUNC_MAX_ID,
 };
 
-- 
2.9.3

[PATCH 4/8] tools lib bpf: export function to set type

[Eric Leblond]

Current API was not allowing the user to set a type like socket
filter. To avoid a setter function for each type, the patch simply
exports a set function that takes the type in parameter.

Signed-off-by: Eric Leblond <eric@regit.org>
---
 tools/lib/bpf/libbpf.c | 19 +++++++++----------
 tools/lib/bpf/libbpf.h |  3 +++
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 90932f1..7cd341e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1336,26 +1336,25 @@ int bpf_program__nth_fd(struct bpf_program *prog, int n)
 	return fd;
 }
 
-static void bpf_program__set_type(struct bpf_program *prog,
-				  enum bpf_prog_type type)
+int bpf_program__set_type(struct bpf_program *prog, unsigned int type)
 {
+	if (!prog)
+		return -EINVAL;
+	if (type >= __MAX_BPF_PROG_TYPE)
+		return -EINVAL;
+
 	prog->type = type;
+	return 0;
 }
 
 int bpf_program__set_tracepoint(struct bpf_program *prog)
 {
-	if (!prog)
-		return -EINVAL;
-	bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
-	return 0;
+	return bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
 }
 
 int bpf_program__set_kprobe(struct bpf_program *prog)
 {
-	if (!prog)
-		return -EINVAL;
-	bpf_program__set_type(prog, BPF_PROG_TYPE_KPROBE);
-	return 0;
+	return bpf_program__set_type(prog, BPF_PROG_TYPE_KPROBE);
 }
 
 static bool bpf_program__is_type(struct bpf_program *prog,
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index e40c8d3..a18783b 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -173,6 +173,9 @@ int bpf_program__set_kprobe(struct bpf_program *prog);
 bool bpf_program__is_tracepoint(struct bpf_program *prog);
 bool bpf_program__is_kprobe(struct bpf_program *prog);
 
+int bpf_program__set_type(struct bpf_program *prog,
+				  unsigned int type);
+
 /*
  * We don't need __attribute__((packed)) now since it is
  * unnecessary for 'bpf_map_def' because they are all aligned.
-- 
2.9.3

[PATCH 5/8] tools lib bpf: add missing functions

[Eric Leblond]

Some functions were missing in the library to be able to use it
in the case where the userspace is handling the maps in kernel.

The patch also renames functions to have a homogeneous naming
convention.

Signed-off-by: Eric Leblond <eric@regit.org>
---
 tools/lib/bpf/bpf.c    | 35 ++++++++++++++++++++++++++++++++++-
 tools/lib/bpf/bpf.h    |  2 --
 tools/lib/bpf/libbpf.h |  5 +++++
 3 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 4212ed6..c0e07bd 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -25,6 +25,7 @@
 #include <asm/unistd.h>
 #include <linux/bpf.h>
 #include "bpf.h"
+#include "libbpf.h"
 
 /*
  * When building perf, unistd.h is overrided. __NR_bpf is
@@ -97,7 +98,7 @@ int bpf_load_program(enum bpf_prog_type type, struct bpf_insn *insns,
 	return sys_bpf(BPF_PROG_LOAD, &attr, sizeof(attr));
 }
 
-int bpf_map_update_elem(int fd, void *key, void *value,
+int bpf_map__update_elem(int fd, void *key, void *value,
 			u64 flags)
 {
 	union bpf_attr attr;
@@ -110,3 +111,35 @@ int bpf_map_update_elem(int fd, void *key, void *value,
 
 	return sys_bpf(BPF_MAP_UPDATE_ELEM, &attr, sizeof(attr));
 }
+
+int bpf_map__lookup_elem(int fd, void *key, void *value)
+{
+	union bpf_attr attr = {
+		.map_fd = fd,
+		.key = ptr_to_u64(key),
+		.value = ptr_to_u64(value),
+	};
+
+	return sys_bpf(BPF_MAP_LOOKUP_ELEM, &attr, sizeof(attr));
+}
+
+int bpf_map__delete_elem(int fd, void *key)
+{
+	union bpf_attr attr = {
+		.map_fd = fd,
+		.key = ptr_to_u64(key),
+	};
+
+	return sys_bpf(BPF_MAP_DELETE_ELEM, &attr, sizeof(attr));
+}
+
+int bpf_map__get_next_key(int fd, void *key, void *next_key)
+{
+	union bpf_attr attr = {
+		.map_fd = fd,
+		.key = ptr_to_u64(key),
+		.next_key = ptr_to_u64(next_key),
+	};
+
+	return sys_bpf(BPF_MAP_GET_NEXT_KEY, &attr, sizeof(attr));
+}
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index e8ba540..5ca834a 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -33,6 +33,4 @@ int bpf_load_program(enum bpf_prog_type type, struct bpf_insn *insns,
 		     u32 kern_version, char *log_buf,
 		     size_t log_buf_sz);
 
-int bpf_map_update_elem(int fd, void *key, void *value,
-			u64 flags);
 #endif
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index a18783b..dfb46d0 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -207,6 +207,11 @@ bpf_map__next(struct bpf_map *map, struct bpf_object *obj);
 int bpf_map__fd(struct bpf_map *map);
 const struct bpf_map_def *bpf_map__def(struct bpf_map *map);
 const char *bpf_map__name(struct bpf_map *map);
+int bpf_map__update_elem(int fd, void *key, void *value,
+			uint64_t flags);
+int bpf_map__lookup_elem(int fd, void *key, void *value);
+int bpf_map__delete_elem(int fd, void *key);
+int bpf_map__get_next_key(int fd, void *key, void *next_key);
 
 typedef void (*bpf_map_clear_priv_t)(struct bpf_map *, void *);
 int bpf_map__set_priv(struct bpf_map *map, void *priv,
-- 
2.9.3

[PATCH 6/8] tools lib bpf: improve warning

[Eric Leblond]

Signed-off-by: Eric Leblond <eric@regit.org>
---
 tools/lib/bpf/libbpf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 7cd341e..1fe4532 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -802,7 +802,8 @@ bpf_object__create_maps(struct bpf_object *obj)
 			size_t j;
 			int err = *pfd;
 
-			pr_warning("failed to create map: %s\n",
+			pr_warning("failed to create map (name: '%s'): %s\n",
+				   obj->maps[i].name,
 				   strerror(errno));
 			for (j = 0; j < i; j++)
 				zclose(obj->maps[j].fd);
-- 
2.9.3

[PATCH 7/8] tools lib bpf: fix maps resolution

[Eric Leblond]

It is not correct to assimilate the elf data of the maps section
to an array of map definition. In fact the sizes differ. The
offset provided in the symbol section has to be used instead.

This patch fixes a bug causing a elf with two maps not to load
correctly.

Signed-off-by: Eric Leblond <eric@regit.org>
---
 tools/lib/bpf/libbpf.c | 50 +++++++++++++++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 15 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 1fe4532..f72628b 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -186,6 +186,7 @@ struct bpf_program {
 struct bpf_map {
 	int fd;
 	char *name;
+	size_t offset;
 	struct bpf_map_def def;
 	void *priv;
 	bpf_map_clear_priv_t clear_priv;
@@ -529,13 +530,6 @@ bpf_object__init_maps(struct bpf_object *obj, void *data,
 
 	pr_debug("maps in %s: %zd bytes\n", obj->path, size);
 
-	obj->maps = calloc(nr_maps, sizeof(obj->maps[0]));
-	if (!obj->maps) {
-		pr_warning("alloc maps for object failed\n");
-		return -ENOMEM;
-	}
-	obj->nr_maps = nr_maps;
-
 	for (i = 0; i < nr_maps; i++) {
 		struct bpf_map_def *def = &obj->maps[i].def;
 
@@ -547,23 +541,42 @@ bpf_object__init_maps(struct bpf_object *obj, void *data,
 		obj->maps[i].fd = -1;
 
 		/* Save map definition into obj->maps */
-		*def = ((struct bpf_map_def *)data)[i];
+		*def = *(struct bpf_map_def *)(data + obj->maps[i].offset);
 	}
 	return 0;
 }
 
 static int
-bpf_object__init_maps_name(struct bpf_object *obj)
+bpf_object__init_maps_symbol(struct bpf_object *obj)
 {
 	int i;
+	int nr_maps = 0;
 	Elf_Data *symbols = obj->efile.symbols;
+	size_t map_idx = 0;
 
 	if (!symbols || obj->efile.maps_shndx < 0)
 		return -EINVAL;
 
+	/* get the number of maps */
+	for (i = 0; i < symbols->d_size / sizeof(GElf_Sym); i++) {
+		GElf_Sym sym;
+
+		if (!gelf_getsym(symbols, i, &sym))
+			continue;
+		if (sym.st_shndx != obj->efile.maps_shndx)
+			continue;
+		nr_maps++;
+	}
+
+	obj->maps = calloc(nr_maps, sizeof(obj->maps[0]));
+	if (!obj->maps) {
+		pr_warning("alloc maps for object failed\n");
+		return -ENOMEM;
+	}
+	obj->nr_maps = nr_maps;
+
 	for (i = 0; i < symbols->d_size / sizeof(GElf_Sym); i++) {
 		GElf_Sym sym;
-		size_t map_idx;
 		const char *map_name;
 
 		if (!gelf_getsym(symbols, i, &sym))
@@ -574,12 +587,12 @@ bpf_object__init_maps_name(struct bpf_object *obj)
 		map_name = elf_strptr(obj->efile.elf,
 				      obj->efile.strtabidx,
 				      sym.st_name);
-		map_idx = sym.st_value / sizeof(struct bpf_map_def);
 		if (map_idx >= obj->nr_maps) {
 			pr_warning("index of map \"%s\" is buggy: %zu > %zu\n",
 				   map_name, map_idx, obj->nr_maps);
 			continue;
 		}
+		obj->maps[map_idx].offset = sym.st_value;
 		obj->maps[map_idx].name = strdup(map_name);
 		if (!obj->maps[map_idx].name) {
 			pr_warning("failed to alloc map name\n");
@@ -587,6 +600,7 @@ bpf_object__init_maps_name(struct bpf_object *obj)
 		}
 		pr_debug("map %zu is \"%s\"\n", map_idx,
 			 obj->maps[map_idx].name);
+		map_idx++;
 	}
 	return 0;
 }
@@ -647,8 +661,6 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
 							data->d_buf,
 							data->d_size);
 		else if (strcmp(name, "maps") == 0) {
-			err = bpf_object__init_maps(obj, data->d_buf,
-						    data->d_size);
 			obj->efile.maps_shndx = idx;
 		} else if (sh.sh_type == SHT_SYMTAB) {
 			if (obj->efile.symbols) {
@@ -698,8 +710,16 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
 		pr_warning("Corrupted ELF file: index of strtab invalid\n");
 		return LIBBPF_ERRNO__FORMAT;
 	}
-	if (obj->efile.maps_shndx >= 0)
-		err = bpf_object__init_maps_name(obj);
+	if (obj->efile.maps_shndx >= 0) {
+		Elf_Data *data;
+		err = bpf_object__init_maps_symbol(obj);
+		if (err)
+			goto out;
+
+		scn = elf_getscn(elf, obj->efile.maps_shndx);
+		data = elf_getdata(scn, 0);
+		err = bpf_object__init_maps(obj, data->d_buf, data->d_size);
+	}
 out:
 	return err;
 }
-- 
2.9.3

[PATCH 8/8] tools lib bpf: install header file

[Eric Leblond]

Makefile was not installing the header file of the library and a
manual copy was needed to have a usable library on the system.

Signed-off-by: Eric Leblond <eric@regit.org>
---
 tools/lib/bpf/Makefile | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index 62d89d5..9525956 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -47,6 +47,7 @@ endif
 
 prefix ?= /usr/local
 libdir = $(prefix)/$(libdir_relative)
+includedir = $(prefix)/include/bpf
 man_dir = $(prefix)/share/man
 man_dir_SQ = '$(subst ','\'',$(man_dir))'
 
@@ -87,14 +88,16 @@ include $(FEATURES_DUMP)
 endif
 endif
 
-export prefix libdir src obj
+export prefix libdir includedir src obj
 
 # Shell quotes
 libdir_SQ = $(subst ','\'',$(libdir))
 libdir_relative_SQ = $(subst ','\'',$(libdir_relative))
+includedir_SQ = $(subst ','\'',$(includedir))
 plugin_dir_SQ = $(subst ','\'',$(plugin_dir))
 
 LIB_FILE = libbpf.a libbpf.so
+HEADER_FILE = libbpf.h
 
 VERSION		= $(BPF_VERSION)
 PATCHLEVEL	= $(BPF_PATCHLEVEL)
@@ -189,7 +192,11 @@ install_lib: all_cmd
 	$(call QUIET_INSTALL, $(LIB_FILE)) \
 		$(call do_install,$(LIB_FILE),$(libdir_SQ))
 
-install: install_lib
+install_header: all_cmd
+	$(call QUIET_INSTALL, $(HEADER_FILE)) \
+		$(call do_install,$(HEADER_FILE),$(includedir_SQ))
+
+install: install_lib install_header
 
 ### Cleaning rules
 
-- 
2.9.3

Re: [PATCH 1/8] tools lib bpf: add error functions

[Wangnan (F)]

On 2016/10/17 5:18, Eric Leblond wrote:
> The include of err.h is not explicitely needed in exported
> functions and it was causing include conflict with some existing
> code due to redefining some macros.
>
> To fix this, let's have error handling functions provided by the
> library. Furthermore this will allow user to have an homogeneous
> API.
>
> Signed-off-by: Eric Leblond <eric@regit.org>
> ---
>   tools/lib/bpf/libbpf.c | 11 +++++++++++
>   tools/lib/bpf/libbpf.h |  4 +++-
>   2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index b699aea..90932f1 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -31,6 +31,7 @@
>   #include <linux/kernel.h>
>   #include <linux/bpf.h>
>   #include <linux/list.h>
> +#include <linux/err.h>
>   #include <libelf.h>
>   #include <gelf.h>
>   
> @@ -1447,3 +1448,13 @@ bpf_object__find_map_by_name(struct bpf_object *obj, const char *name)
>   	}
>   	return NULL;
>   }
> +
> +bool bpf__is_error(const void *ptr)

Please use libbpf_is_error(), like libbpf_set_print. We use '__' because 
we want
to use the OO concept. This utility is not OO.

> +{
> +	return IS_ERR(ptr);
> +}
> +
> +long bpf__get_error(const void *ptr)

Same, please call it libbpf_get_error().

Thank you.

Re: [PATCH 3/8] tools: Sync tools/include/uapi/linux/bpf.h with the kernel

[Wangnan (F)]

On 2016/10/17 5:18, Eric Leblond wrote:
> Signed-off-by: Eric Leblond <eric@regit.org>

Commit message is required.

Thank you.

> ---
>   tools/include/uapi/linux/bpf.h | 52 ++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 52 insertions(+)
>
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 9e5fc16..570287f 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -95,6 +95,8 @@ enum bpf_prog_type {
>   	BPF_PROG_TYPE_SCHED_ACT,
>   	BPF_PROG_TYPE_TRACEPOINT,
>   	BPF_PROG_TYPE_XDP,
> +	BPF_PROG_TYPE_PERF_EVENT,
> +	__MAX_BPF_PROG_TYPE,
>   };
>   
>   #define BPF_PSEUDO_MAP_FD	1
> @@ -375,6 +377,56 @@ enum bpf_func_id {
>   	 */
>   	BPF_FUNC_probe_write_user,
>   
> +	/**
> +	 * bpf_current_task_under_cgroup(map, index) - Check cgroup2 membership of current task
> +	 * @map: pointer to bpf_map in BPF_MAP_TYPE_CGROUP_ARRAY type
> +	 * @index: index of the cgroup in the bpf_map
> +	 * Return:
> +	 *   == 0 current failed the cgroup2 descendant test
> +	 *   == 1 current succeeded the cgroup2 descendant test
> +	 *    < 0 error
> +	 */
> +	BPF_FUNC_current_task_under_cgroup,
> +
> +	/**
> +	 * bpf_skb_change_tail(skb, len, flags)
> +	 * The helper will resize the skb to the given new size,
> +	 * to be used f.e. with control messages.
> +	 * @skb: pointer to skb
> +	 * @len: new skb length
> +	 * @flags: reserved
> +	 * Return: 0 on success or negative error
> +	 */
> +	BPF_FUNC_skb_change_tail,
> +
> +	/**
> +	 * bpf_skb_pull_data(skb, len)
> +	 * The helper will pull in non-linear data in case the
> +	 * skb is non-linear and not all of len are part of the
> +	 * linear section. Only needed for read/write with direct
> +	 * packet access.
> +	 * @skb: pointer to skb
> +	 * @len: len to make read/writeable
> +	 * Return: 0 on success or negative error
> +	 */
> +	BPF_FUNC_skb_pull_data,
> +
> +	/**
> +	 * bpf_csum_update(skb, csum)
> +	 * Adds csum into skb->csum in case of CHECKSUM_COMPLETE.
> +	 * @skb: pointer to skb
> +	 * @csum: csum to add
> +	 * Return: csum on success or negative error
> +	 */
> +	BPF_FUNC_csum_update,
> +
> +	/**
> +	 * bpf_set_hash_invalid(skb)
> +	 * Invalidate current skb>hash.
> +	 * @skb: pointer to skb
> +	 */
> +	BPF_FUNC_set_hash_invalid,
> +
>   	__BPF_FUNC_MAX_ID,
>   };
>   

Re: [PATCH 4/8] tools lib bpf: export function to set type

[Wangnan (F)]

On 2016/10/17 5:18, Eric Leblond wrote:
> Current API was not allowing the user to set a type like socket
> filter. To avoid a setter function for each type, the patch simply
> exports a set function that takes the type in parameter.
>
> Signed-off-by: Eric Leblond <eric@regit.org>
> ---
>   tools/lib/bpf/libbpf.c | 19 +++++++++----------
>   tools/lib/bpf/libbpf.h |  3 +++
>   2 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 90932f1..7cd341e 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1336,26 +1336,25 @@ int bpf_program__nth_fd(struct bpf_program *prog, int n)
>   	return fd;
>   }
>   
> -static void bpf_program__set_type(struct bpf_program *prog,
> -				  enum bpf_prog_type type)
> +int bpf_program__set_type(struct bpf_program *prog, unsigned int type)
>   {
> +	if (!prog)
> +		return -EINVAL;
> +	if (type >= __MAX_BPF_PROG_TYPE)
> +		return -EINVAL;
> +
>   	prog->type = type;
> +	return 0;
>   }
>   
>   int bpf_program__set_tracepoint(struct bpf_program *prog)
>   {
> -	if (!prog)
> -		return -EINVAL;
> -	bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> -	return 0;
> +	return bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
>   }
>   
>   int bpf_program__set_kprobe(struct bpf_program *prog)
>   {
> -	if (!prog)
> -		return -EINVAL;
> -	bpf_program__set_type(prog, BPF_PROG_TYPE_KPROBE);
> -	return 0;
> +	return bpf_program__set_type(prog, BPF_PROG_TYPE_KPROBE);
>   }
>   
>   static bool bpf_program__is_type(struct bpf_program *prog,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index e40c8d3..a18783b 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -173,6 +173,9 @@ int bpf_program__set_kprobe(struct bpf_program *prog);
>   bool bpf_program__is_tracepoint(struct bpf_program *prog);
>   bool bpf_program__is_kprobe(struct bpf_program *prog);
>   
> +int bpf_program__set_type(struct bpf_program *prog,
> +				  unsigned int type);
> +

Although you don't include uapi/linux/bpf.h in this patch, logically
you add this dependency.

Please continously add bpf_program__set_socket_filter() and
bpf_program__is_socket_filter() like what we do for tracepoint.
This way libbpf.h is indenpendent from kernel header.

We can use macro in both .h and .c.

Thank you.

Re: [PATCH 5/8] tools lib bpf: add missing functions

[Wangnan (F)]

On 2016/10/17 5:18, Eric Leblond wrote:
> Some functions were missing in the library to be able to use it
> in the case where the userspace is handling the maps in kernel.
>
> The patch also renames functions to have a homogeneous naming
> convention.
>
> Signed-off-by: Eric Leblond <eric@regit.org>
> ---
>   tools/lib/bpf/bpf.c    | 35 ++++++++++++++++++++++++++++++++++-
>   tools/lib/bpf/bpf.h    |  2 --
>   tools/lib/bpf/libbpf.h |  5 +++++
>   3 files changed, 39 insertions(+), 3 deletions(-)
>
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 4212ed6..c0e07bd 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -25,6 +25,7 @@
>   #include <asm/unistd.h>
>   #include <linux/bpf.h>
>   #include "bpf.h"
> +#include "libbpf.h"
>   
>   /*
>    * When building perf, unistd.h is overrided. __NR_bpf is
> @@ -97,7 +98,7 @@ int bpf_load_program(enum bpf_prog_type type, struct bpf_insn *insns,
>   	return sys_bpf(BPF_PROG_LOAD, &attr, sizeof(attr));
>   }
>   
> -int bpf_map_update_elem(int fd, void *key, void *value,
> +int bpf_map__update_elem(int fd, void *key, void *value,
>   			u64 flags)

Please don't use '__' style API here. It is easily be confused with
bpf_map__*() in libbpf.h. They are APIs at different level.

bpf_map__*() are APIs for 'struct bpf_map's, they are object introduced
by libbpf, defined in libbpf.h. bpf_map_*() APIs operate on fd, they are
objects defined by kernel. bpf_map_*() APIs are declared in bpf.h.

In libbpf, bpf.h directly operates on kernel objects (fd), APIs in it
are named bpf_map_*(); libbpf.h operates on 'struct bpf_map' object,
APIs in it are named using bpf_map__*(). libbpf.h and bpf.h are independent
with each other.

>   {
>   	union bpf_attr attr;
> @@ -110,3 +111,35 @@ int bpf_map_update_elem(int fd, void *key, void *value,
>   
>   	return sys_bpf(BPF_MAP_UPDATE_ELEM, &attr, sizeof(attr));
>   }
> +
> +int bpf_map__lookup_elem(int fd, void *key, void *value)
> +{
> +	union bpf_attr attr = {
> +		.map_fd = fd,
> +		.key = ptr_to_u64(key),
> +		.value = ptr_to_u64(value),
> +	};
> +
> +	return sys_bpf(BPF_MAP_LOOKUP_ELEM, &attr, sizeof(attr));
> +}
> +
> +int bpf_map__delete_elem(int fd, void *key)
> +{
> +	union bpf_attr attr = {
> +		.map_fd = fd,
> +		.key = ptr_to_u64(key),
> +	};
> +
> +	return sys_bpf(BPF_MAP_DELETE_ELEM, &attr, sizeof(attr));
> +}
> +
> +int bpf_map__get_next_key(int fd, void *key, void *next_key)
> +{
> +	union bpf_attr attr = {
> +		.map_fd = fd,
> +		.key = ptr_to_u64(key),
> +		.next_key = ptr_to_u64(next_key),
> +	};
> +
> +	return sys_bpf(BPF_MAP_GET_NEXT_KEY, &attr, sizeof(attr));
> +}
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index e8ba540..5ca834a 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -33,6 +33,4 @@ int bpf_load_program(enum bpf_prog_type type, struct bpf_insn *insns,
>   		     u32 kern_version, char *log_buf,
>   		     size_t log_buf_sz);
>   
> -int bpf_map_update_elem(int fd, void *key, void *value,
> -			u64 flags);
>   #endif
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index a18783b..dfb46d0 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -207,6 +207,11 @@ bpf_map__next(struct bpf_map *map, struct bpf_object *obj);
>   int bpf_map__fd(struct bpf_map *map);
>   const struct bpf_map_def *bpf_map__def(struct bpf_map *map);
>   const char *bpf_map__name(struct bpf_map *map);
> +int bpf_map__update_elem(int fd, void *key, void *value,
> +			uint64_t flags);
> +int bpf_map__lookup_elem(int fd, void *key, void *value);
> +int bpf_map__delete_elem(int fd, void *key);
> +int bpf_map__get_next_key(int fd, void *key, void *next_key);

As what we have discussed, the newly introduced functions should be added
in bpf.h.

Thank you.

Re: [PATCH 6/8] tools lib bpf: improve warning

[Wangnan (F)]

On 2016/10/17 5:18, Eric Leblond wrote:
> Signed-off-by: Eric Leblond <eric@regit.org>

Please add some commit messages.

Thank you.

> ---
>   tools/lib/bpf/libbpf.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 7cd341e..1fe4532 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -802,7 +802,8 @@ bpf_object__create_maps(struct bpf_object *obj)
>   			size_t j;
>   			int err = *pfd;
>   
> -			pr_warning("failed to create map: %s\n",
> +			pr_warning("failed to create map (name: '%s'): %s\n",
> +				   obj->maps[i].name,
>   				   strerror(errno));
>   			for (j = 0; j < i; j++)
>   				zclose(obj->maps[j].fd);

Re: [PATCH 7/8] tools lib bpf: fix maps resolution

[Wangnan (F)]

On 2016/10/17 5:18, Eric Leblond wrote:
> It is not correct to assimilate the elf data of the maps section
> to an array of map definition. In fact the sizes differ. The
> offset provided in the symbol section has to be used instead.
>
> This patch fixes a bug causing a elf with two maps not to load
> correctly.

Could you please give an example so we can understand why
section 'maps' is not an array?

Thank you.

Re: [PATCH 1/8] tools lib bpf: add error functions

[Joe Stringer]

On 16 October 2016 at 14:18, Eric Leblond <eric@regit.org> wrote:
> The include of err.h is not explicitely needed in exported
> functions and it was causing include conflict with some existing
> code due to redefining some macros.
>
> To fix this, let's have error handling functions provided by the
> library. Furthermore this will allow user to have an homogeneous
> API.
>
> Signed-off-by: Eric Leblond <eric@regit.org>

Does it need to return the error like this or should we just fix up
the bpf_object__open() API to return errors in a simpler form?

There's already libbpf_set_print(...) for outputting errors, is it
reasonable to just change the library to return NULLs in error cases
instead?

Re: [PATCH 1/8] tools lib bpf: add error functions

[Wangnan (F)]

On 2016/10/19 6:52, Joe Stringer wrote:
> On 16 October 2016 at 14:18, Eric Leblond <eric@regit.org> wrote:
>> The include of err.h is not explicitely needed in exported
>> functions and it was causing include conflict with some existing
>> code due to redefining some macros.
>>
>> To fix this, let's have error handling functions provided by the
>> library. Furthermore this will allow user to have an homogeneous
>> API.
>>
>> Signed-off-by: Eric Leblond <eric@regit.org>
> Does it need to return the error like this or should we just fix up
> the bpf_object__open() API to return errors in a simpler form?
>
> There's already libbpf_set_print(...) for outputting errors, is it
> reasonable to just change the library to return NULLs in error cases
> instead?

Returning error code to caller so caller knows what happen.
Other subsystems in perf also do this.

Perf hides libbpf's error output (make it silent unless -v),
so it needs a way for receiving libbpf's error code.

I think this patch is good, decouple libbpf.h and kernel headers.

Thank you.

Re: [PATCH 7/8] tools lib bpf: fix maps resolution

[Wangnan (F)]

Hi Eric,

Are you still working in this patch set?

Now I know why maps section is not a simple array
from a patch set from Joe Stringer:

https://www.mail-archive.com/netdev@vger.kernel.org/msg135088.html

So I think this patch is really useful.

Are you going to resend the whole patch set? If not, let me collect
this patch 7/8 into my local code base and send to Arnaldo
with my other patches.

Thank you.

On 2016/10/17 5:18, Eric Leblond wrote:
> It is not correct to assimilate the elf data of the maps section
> to an array of map definition. In fact the sizes differ. The
> offset provided in the symbol section has to be used instead.
>
> This patch fixes a bug causing a elf with two maps not to load
> correctly.
>
> Signed-off-by: Eric Leblond <eric@regit.org>
> ---
>   tools/lib/bpf/libbpf.c | 50 +++++++++++++++++++++++++++++++++++---------------
>   1 file changed, 35 insertions(+), 15 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 1fe4532..f72628b 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -186,6 +186,7 @@ struct bpf_program {
>   struct bpf_map {
>   	int fd;
>   	char *name;
> +	size_t offset;
>   	struct bpf_map_def def;
>   	void *priv;
>   	bpf_map_clear_priv_t clear_priv;
> @@ -529,13 +530,6 @@ bpf_object__init_maps(struct bpf_object *obj, void *data,
>   
>   	pr_debug("maps in %s: %zd bytes\n", obj->path, size);
>   
> -	obj->maps = calloc(nr_maps, sizeof(obj->maps[0]));
> -	if (!obj->maps) {
> -		pr_warning("alloc maps for object failed\n");
> -		return -ENOMEM;
> -	}
> -	obj->nr_maps = nr_maps;
> -
>   	for (i = 0; i < nr_maps; i++) {
>   		struct bpf_map_def *def = &obj->maps[i].def;
>   
> @@ -547,23 +541,42 @@ bpf_object__init_maps(struct bpf_object *obj, void *data,
>   		obj->maps[i].fd = -1;
>   
>   		/* Save map definition into obj->maps */
> -		*def = ((struct bpf_map_def *)data)[i];
> +		*def = *(struct bpf_map_def *)(data + obj->maps[i].offset);
>   	}
>   	return 0;
>   }
>   
>   static int
> -bpf_object__init_maps_name(struct bpf_object *obj)
> +bpf_object__init_maps_symbol(struct bpf_object *obj)
>   {
>   	int i;
> +	int nr_maps = 0;
>   	Elf_Data *symbols = obj->efile.symbols;
> +	size_t map_idx = 0;
>   
>   	if (!symbols || obj->efile.maps_shndx < 0)
>   		return -EINVAL;
>   
> +	/* get the number of maps */
> +	for (i = 0; i < symbols->d_size / sizeof(GElf_Sym); i++) {
> +		GElf_Sym sym;
> +
> +		if (!gelf_getsym(symbols, i, &sym))
> +			continue;
> +		if (sym.st_shndx != obj->efile.maps_shndx)
> +			continue;
> +		nr_maps++;
> +	}
> +
> +	obj->maps = calloc(nr_maps, sizeof(obj->maps[0]));
> +	if (!obj->maps) {
> +		pr_warning("alloc maps for object failed\n");
> +		return -ENOMEM;
> +	}
> +	obj->nr_maps = nr_maps;
> +
>   	for (i = 0; i < symbols->d_size / sizeof(GElf_Sym); i++) {
>   		GElf_Sym sym;
> -		size_t map_idx;
>   		const char *map_name;
>   
>   		if (!gelf_getsym(symbols, i, &sym))
> @@ -574,12 +587,12 @@ bpf_object__init_maps_name(struct bpf_object *obj)
>   		map_name = elf_strptr(obj->efile.elf,
>   				      obj->efile.strtabidx,
>   				      sym.st_name);
> -		map_idx = sym.st_value / sizeof(struct bpf_map_def);
>   		if (map_idx >= obj->nr_maps) {
>   			pr_warning("index of map \"%s\" is buggy: %zu > %zu\n",
>   				   map_name, map_idx, obj->nr_maps);
>   			continue;
>   		}
> +		obj->maps[map_idx].offset = sym.st_value;
>   		obj->maps[map_idx].name = strdup(map_name);
>   		if (!obj->maps[map_idx].name) {
>   			pr_warning("failed to alloc map name\n");
> @@ -587,6 +600,7 @@ bpf_object__init_maps_name(struct bpf_object *obj)
>   		}
>   		pr_debug("map %zu is \"%s\"\n", map_idx,
>   			 obj->maps[map_idx].name);
> +		map_idx++;
>   	}
>   	return 0;
>   }
> @@ -647,8 +661,6 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
>   							data->d_buf,
>   							data->d_size);
>   		else if (strcmp(name, "maps") == 0) {
> -			err = bpf_object__init_maps(obj, data->d_buf,
> -						    data->d_size);
>   			obj->efile.maps_shndx = idx;
>   		} else if (sh.sh_type == SHT_SYMTAB) {
>   			if (obj->efile.symbols) {
> @@ -698,8 +710,16 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
>   		pr_warning("Corrupted ELF file: index of strtab invalid\n");
>   		return LIBBPF_ERRNO__FORMAT;
>   	}
> -	if (obj->efile.maps_shndx >= 0)
> -		err = bpf_object__init_maps_name(obj);
> +	if (obj->efile.maps_shndx >= 0) {
> +		Elf_Data *data;
> +		err = bpf_object__init_maps_symbol(obj);
> +		if (err)
> +			goto out;
> +
> +		scn = elf_getscn(elf, obj->efile.maps_shndx);
> +		data = elf_getdata(scn, 0);
> +		err = bpf_object__init_maps(obj, data->d_buf, data->d_size);
> +	}
>   out:
>   	return err;
>   }

Re: [PATCH 7/8] tools lib bpf: fix maps resolution

[Eric Leblond]

Hi,

On Tue, 2016-11-08 at 02:23 +0800, Wangnan (F) wrote:
> Hi Eric,
> 
> Are you still working in this patch set?

Sorry to lag on this, I've been taken by a series of other projects. I
did not yet reworked it yet but I was planning to do a bit on it this
week.

> 
> Now I know why maps section is not a simple array
> from a patch set from Joe Stringer:
> 
> https://www.mail-archive.com/netdev@vger.kernel.org/msg135088.html
> 
> So I think this patch is really useful.
> 
> Are you going to resend the whole patch set? If not, let me collect
> this patch 7/8 into my local code base and send to Arnaldo
> with my other patches.

If ok with you, I propose that you collect patch 7/8 it you have no
news from me on Friday. If an issue for you, just collect it now and I
will synchronize with updated code when resending my patchset.

BR,
-- 
Eric Leblond <eric@regit.org>
Blog: https://home.regit.org/

Re: [PATCH 7/8] tools lib bpf: fix maps resolution

[Wangnan (F)]

Hi Eric,

During testing this patch I find a segfault, please see inline comment.

In addition, since both the BPF map array and map names should be done
after symbol table is collected, merging bpf_object__init_maps and
bpf_object__init_maps_name would be a good practice, making code
simpler.

So I prepare a new patch. Please have a look at:

http://lkml.kernel.org/g/20161108215734.28905-1-wangnan0@huawei.com

New version ensure not crashing in any case user provides a corrupted
maps section, including array of bpf maps, maps with different definition
structures and very short map definition.

Thank you.

On 2016/10/16 14:18, Eric Leblond wrote:
> It is not correct to assimilate the elf data of the maps section
> to an array of map definition. In fact the sizes differ. The
> offset provided in the symbol section has to be used instead.
>
> This patch fixes a bug causing a elf with two maps not to load
> correctly.
>
> Signed-off-by: Eric Leblond <eric@regit.org>
> ---
>   tools/lib/bpf/libbpf.c | 50 +++++++++++++++++++++++++++++++++++---------------
>   1 file changed, 35 insertions(+), 15 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 1fe4532..f72628b 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -186,6 +186,7 @@ struct bpf_program {
>   struct bpf_map {
>   	int fd;
>   	char *name;
> +	size_t offset;
>   	struct bpf_map_def def;
>   	void *priv;
>   	bpf_map_clear_priv_t clear_priv;
> @@ -529,13 +530,6 @@ bpf_object__init_maps(struct bpf_object *obj, void *data,
>   
>   	pr_debug("maps in %s: %zd bytes\n", obj->path, size);
>   
> -	obj->maps = calloc(nr_maps, sizeof(obj->maps[0]));
> -	if (!obj->maps) {
> -		pr_warning("alloc maps for object failed\n");
> -		return -ENOMEM;
> -	}
> -	obj->nr_maps = nr_maps;
> -
>   	for (i = 0; i < nr_maps; i++) {
>   		struct bpf_map_def *def = &obj->maps[i].def;
>   
> @@ -547,23 +541,42 @@ bpf_object__init_maps(struct bpf_object *obj, void *data,
>   		obj->maps[i].fd = -1;
>   
>   		/* Save map definition into obj->maps */
> -		*def = ((struct bpf_map_def *)data)[i];
> +		*def = *(struct bpf_map_def *)(data + obj->maps[i].offset);
>   	}

Here, nr_maps is still size / sizeof(struct bpf_map_def), so obj->maps[i]
can be invalid.

>   	return 0;
>   }
>   
>   static int
> -bpf_object__init_maps_name(struct bpf_object *obj)
> +bpf_object__init_maps_symbol(struct bpf_object *obj)
>   {
>   	int i;
> +	int nr_maps = 0;
>   	Elf_Data *symbols = obj->efile.symbols;
> +	size_t map_idx = 0;
>   
>   	if (!symbols || obj->efile.maps_shndx < 0)
>   		return -EINVAL;
>   
> +	/* get the number of maps */
> +	for (i = 0; i < symbols->d_size / sizeof(GElf_Sym); i++) {
> +		GElf_Sym sym;
> +
> +		if (!gelf_getsym(symbols, i, &sym))
> +			continue;
> +		if (sym.st_shndx != obj->efile.maps_shndx)
> +			continue;
> +		nr_maps++;
> +	}
> +
> +	obj->maps = calloc(nr_maps, sizeof(obj->maps[0]));
> +	if (!obj->maps) {
> +		pr_warning("alloc maps for object failed\n");
> +		return -ENOMEM;
> +	}
> +	obj->nr_maps = nr_maps;
> +
>   	for (i = 0; i < symbols->d_size / sizeof(GElf_Sym); i++) {
>   		GElf_Sym sym;
> -		size_t map_idx;
>   		const char *map_name;
>   
>   		if (!gelf_getsym(symbols, i, &sym))
> @@ -574,12 +587,12 @@ bpf_object__init_maps_name(struct bpf_object *obj)
>   		map_name = elf_strptr(obj->efile.elf,
>   				      obj->efile.strtabidx,
>   				      sym.st_name);
> -		map_idx = sym.st_value / sizeof(struct bpf_map_def);
>   		if (map_idx >= obj->nr_maps) {
>   			pr_warning("index of map \"%s\" is buggy: %zu > %zu\n",
>   				   map_name, map_idx, obj->nr_maps);
>   			continue;
>   		}
> +		obj->maps[map_idx].offset = sym.st_value;
>   		obj->maps[map_idx].name = strdup(map_name);
>   		if (!obj->maps[map_idx].name) {
>   			pr_warning("failed to alloc map name\n");
> @@ -587,6 +600,7 @@ bpf_object__init_maps_name(struct bpf_object *obj)
>   		}
>   		pr_debug("map %zu is \"%s\"\n", map_idx,
>   			 obj->maps[map_idx].name);
> +		map_idx++;
>   	}
>   	return 0;
>   }
> @@ -647,8 +661,6 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
>   							data->d_buf,
>   							data->d_size);
>   		else if (strcmp(name, "maps") == 0) {
> -			err = bpf_object__init_maps(obj, data->d_buf,
> -						    data->d_size);
>   			obj->efile.maps_shndx = idx;
>   		} else if (sh.sh_type == SHT_SYMTAB) {
>   			if (obj->efile.symbols) {
> @@ -698,8 +710,16 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
>   		pr_warning("Corrupted ELF file: index of strtab invalid\n");
>   		return LIBBPF_ERRNO__FORMAT;
>   	}
> -	if (obj->efile.maps_shndx >= 0)
> -		err = bpf_object__init_maps_name(obj);
> +	if (obj->efile.maps_shndx >= 0) {
> +		Elf_Data *data;
> +		err = bpf_object__init_maps_symbol(obj);
> +		if (err)
> +			goto out;
> +
> +		scn = elf_getscn(elf, obj->efile.maps_shndx);
> +		data = elf_getdata(scn, 0);
> +		err = bpf_object__init_maps(obj, data->d_buf, data->d_size);
> +	}
>   out:
>   	return err;
>   }