[PATCH perf/core 0/6] Libbpf improvements

[PATCH perf/core 0/6] Libbpf improvements

[Joe Stringer]

Patch 1 fixes an issue when using drastically different BPF map definitions
inside ELFs from a client using libbpf, vs the map definition libbpf uses.

Patch 2 is a trivial typo fix.

Patches 3-5 add some simple, useful helper functions for setting prog type
and retrieving libbpf errors without depending on kernel headers from
userspace programs.

Patch 6 adds a new pinning functionality for an entire object. Calling
bpf_object__pin(obj, subpath) will mount all programs from the BPF object
to $bpf_fs_path/$subpath/progs/$progname, and all maps from the BPF object
to $bpf_fs_path/$subpath/maps/$mapname. The first program with a particular
name will be mounted with its progname and the suffix "_0"; subsequent
programs with the same will have "_1", and so on; duplicate maps with the
same name are disallowed.

Joe Stringer (6):
  tools lib bpf: Fix map offsets in relocation
  tools lib bpf: Fix grammar in map_idx warning
  tools lib bpf: Define prog_type fns with macro
  tools lib bpf: Add set/is helpers for all prog types
  tools lib bpf: Add libbpf_get_error()
  tools lib bpf: Add bpf_object__pin()

 tools/lib/bpf/libbpf.c  | 197 +++++++++++++++++++++++++++++++++++++++++-------
 tools/lib/bpf/libbpf.h  |  15 +++-
 tools/perf/tests/llvm.c |   2 +-
 3 files changed, 185 insertions(+), 29 deletions(-)

-- 
2.11.0

[PATCH perf/core 1/6] tools lib bpf: Fix map offsets in relocation

[Joe Stringer]

Commit 4708bbda5cb2 ("tools lib bpf: Fix maps resolution") attempted to
fix map resolution by identifying the number of symbols that point to
maps, and using this number to resolve each of the maps.

However, during relocation the original definition of the map size was
still in use. For up to two maps, the calculation was correct if there
was a small difference in size between the map definition in libbpf and
the one that the client library uses. However if the difference was
large, particularly if more than two maps were used in the BPF program,
the relocation would fail.

For example, when using a map definition with size 28, with three maps,
map relocation would count
    (sym_offset / sizeof(struct bpf_map_def) => map_idx)
    (0 / 16 => 0), ie map_idx = 0
    (28 / 16 => 1), ie map_idx = 1
    (56 / 16 => 3), ie map_idx = 3

So, libbpf reports:
    libbpf: bpf relocation: map_idx 3 large than 2

Fix map relocation by tracking the size of the map definition during
map counting, then reuse this instead of the size of the libbpf map
structure. With this patch applied, libbpf will accept such ELFs.

Fixes: 4708bbda5cb2 ("tools lib bpf: Fix maps resolution")
Signed-off-by: Joe Stringer <joe@ovn.org>
---
 tools/lib/bpf/libbpf.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 84e6b35da4bd..350ee4c59f85 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -201,6 +201,7 @@ struct bpf_object {
 	size_t nr_programs;
 	struct bpf_map *maps;
 	size_t nr_maps;
+	size_t map_len;
 
 	bool loaded;
 
@@ -379,6 +380,7 @@ static struct bpf_object *bpf_object__new(const char *path,
 	obj->efile.maps_shndx = -1;
 
 	obj->loaded = false;
+	obj->map_len = sizeof(struct bpf_map_def);
 
 	INIT_LIST_HEAD(&obj->list);
 	list_add(&obj->list, &bpf_objects_list);
@@ -602,6 +604,7 @@ bpf_object__init_maps(struct bpf_object *obj)
 		return -ENOMEM;
 	}
 	obj->nr_maps = nr_maps;
+	obj->map_len = data->d_size / nr_maps;
 
 	/*
 	 * fill all fd with -1 so won't close incorrect
@@ -829,7 +832,7 @@ bpf_program__collect_reloc(struct bpf_program *prog,
 			return -LIBBPF_ERRNO__RELOC;
 		}
 
-		map_idx = sym.st_value / sizeof(struct bpf_map_def);
+		map_idx = sym.st_value / prog->obj->map_len;
 		if (map_idx >= nr_maps) {
 			pr_warning("bpf relocation: map_idx %d large than %d\n",
 				   (int)map_idx, (int)nr_maps - 1);
-- 
2.11.0

[PATCH perf/core 2/6] tools lib bpf: Fix grammar in map_idx warning

[Joe Stringer]

"Larger" is the comparative adjective form of "large".

Signed-off-by: Joe Stringer <joe@ovn.org>
---
 tools/lib/bpf/libbpf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 350ee4c59f85..653b1b368deb 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -834,7 +834,7 @@ bpf_program__collect_reloc(struct bpf_program *prog,
 
 		map_idx = sym.st_value / prog->obj->map_len;
 		if (map_idx >= nr_maps) {
-			pr_warning("bpf relocation: map_idx %d large than %d\n",
+			pr_warning("bpf relocation: map_idx %d larger than %d\n",
 				   (int)map_idx, (int)nr_maps - 1);
 			return -LIBBPF_ERRNO__RELOC;
 		}
-- 
2.11.0

[PATCH perf/core 3/6] tools lib bpf: Define prog_type fns with macro

[Joe Stringer]

Turning this into a macro allows future prog types to be added with a
single line per type.

Signed-off-by: Joe Stringer <joe@ovn.org>
---
 tools/lib/bpf/libbpf.c | 41 ++++++++++++++++-------------------------
 1 file changed, 16 insertions(+), 25 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 653b1b368deb..aa2a04bfd494 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1422,37 +1422,28 @@ static void bpf_program__set_type(struct bpf_program *prog,
 	prog->type = type;
 }
 
-int bpf_program__set_tracepoint(struct bpf_program *prog)
-{
-	if (!prog)
-		return -EINVAL;
-	bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
-	return 0;
-}
-
-int bpf_program__set_kprobe(struct bpf_program *prog)
-{
-	if (!prog)
-		return -EINVAL;
-	bpf_program__set_type(prog, BPF_PROG_TYPE_KPROBE);
-	return 0;
-}
-
 static bool bpf_program__is_type(struct bpf_program *prog,
 				 enum bpf_prog_type type)
 {
 	return prog ? (prog->type == type) : false;
 }
 
-bool bpf_program__is_tracepoint(struct bpf_program *prog)
-{
-	return bpf_program__is_type(prog, BPF_PROG_TYPE_TRACEPOINT);
-}
-
-bool bpf_program__is_kprobe(struct bpf_program *prog)
-{
-	return bpf_program__is_type(prog, BPF_PROG_TYPE_KPROBE);
-}
+#define BPF_PROG_TYPE_FNS(NAME, TYPE)			\
+int bpf_program__set_##NAME(struct bpf_program *prog)	\
+{							\
+	if (!prog)					\
+		return -EINVAL;				\
+	bpf_program__set_type(prog, TYPE);		\
+	return 0;					\
+}							\
+							\
+bool bpf_program__is_##NAME(struct bpf_program *prog)	\
+{							\
+	return bpf_program__is_type(prog, TYPE);	\
+}							\
+
+BPF_PROG_TYPE_FNS(kprobe, BPF_PROG_TYPE_KPROBE);
+BPF_PROG_TYPE_FNS(tracepoint, BPF_PROG_TYPE_TRACEPOINT);
 
 int bpf_map__fd(struct bpf_map *map)
 {
-- 
2.11.0

[PATCH perf/core 4/6] tools lib bpf: Add set/is helpers for all prog types

[Joe Stringer]

These bpf_prog_types were exposed in the uapi but there were no
corresponding functions to set these types for programs in libbpf.

Signed-off-by: Joe Stringer <joe@ovn.org>
---
 tools/lib/bpf/libbpf.c |  5 +++++
 tools/lib/bpf/libbpf.h | 10 ++++++++++
 2 files changed, 15 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index aa2a04bfd494..e9a177d5ab15 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1442,8 +1442,13 @@ bool bpf_program__is_##NAME(struct bpf_program *prog)	\
 	return bpf_program__is_type(prog, TYPE);	\
 }							\
 
+BPF_PROG_TYPE_FNS(socket_filter, BPF_PROG_TYPE_SOCKET_FILTER);
 BPF_PROG_TYPE_FNS(kprobe, BPF_PROG_TYPE_KPROBE);
+BPF_PROG_TYPE_FNS(sched_cls, BPF_PROG_TYPE_SCHED_CLS);
+BPF_PROG_TYPE_FNS(sched_act, BPF_PROG_TYPE_SCHED_ACT);
 BPF_PROG_TYPE_FNS(tracepoint, BPF_PROG_TYPE_TRACEPOINT);
+BPF_PROG_TYPE_FNS(xdp, BPF_PROG_TYPE_XDP);
+BPF_PROG_TYPE_FNS(perf_event, BPF_PROG_TYPE_PERF_EVENT);
 
 int bpf_map__fd(struct bpf_map *map)
 {
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index a5a8b86a06fe..2188ccdc0e2d 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -174,11 +174,21 @@ int bpf_program__nth_fd(struct bpf_program *prog, int n);
 /*
  * Adjust type of bpf program. Default is kprobe.
  */
+int bpf_program__set_socket_filter(struct bpf_program *prog);
 int bpf_program__set_tracepoint(struct bpf_program *prog);
 int bpf_program__set_kprobe(struct bpf_program *prog);
+int bpf_program__set_sched_cls(struct bpf_program *prog);
+int bpf_program__set_sched_act(struct bpf_program *prog);
+int bpf_program__set_xdp(struct bpf_program *prog);
+int bpf_program__set_perf_event(struct bpf_program *prog);
 
+bool bpf_program__is_socket_filter(struct bpf_program *prog);
 bool bpf_program__is_tracepoint(struct bpf_program *prog);
 bool bpf_program__is_kprobe(struct bpf_program *prog);
+bool bpf_program__is_sched_cls(struct bpf_program *prog);
+bool bpf_program__is_sched_act(struct bpf_program *prog);
+bool bpf_program__is_xdp(struct bpf_program *prog);
+bool bpf_program__is_perf_event(struct bpf_program *prog);
 
 /*
  * We don't need __attribute__((packed)) now since it is
-- 
2.11.0

[PATCH perf/core 5/6] tools lib bpf: Add libbpf_get_error()

[Joe Stringer]

This function will turn a libbpf pointer into a standard error code (or
0 if the pointer is valid). This also allows removal of the dependency
on linux/err.h in the public header file, which causes problems in
userspace programs built against libbpf.

Signed-off-by: Joe Stringer <joe@ovn.org>
---
 tools/lib/bpf/libbpf.c  | 8 ++++++++
 tools/lib/bpf/libbpf.h  | 4 +++-
 tools/perf/tests/llvm.c | 2 +-
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e9a177d5ab15..6b651c19870d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -28,6 +28,7 @@
 #include <fcntl.h>
 #include <errno.h>
 #include <asm/unistd.h>
+#include <linux/err.h>
 #include <linux/kernel.h>
 #include <linux/bpf.h>
 #include <linux/list.h>
@@ -1536,3 +1537,10 @@ bpf_object__find_map_by_offset(struct bpf_object *obj, size_t offset)
 	}
 	return ERR_PTR(-ENOENT);
 }
+
+long libbpf_get_error(const void *ptr)
+{
+	if (IS_ERR(ptr))
+		return PTR_ERR(ptr);
+	return 0;
+}
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 2188ccdc0e2d..4014d1ba5e3d 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -22,8 +22,8 @@
 #define __BPF_LIBBPF_H
 
 #include <stdio.h>
+#include <stdint.h>
 #include <stdbool.h>
-#include <linux/err.h>
 #include <sys/types.h>  // for size_t
 
 enum libbpf_errno {
@@ -234,4 +234,6 @@ 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);
 
+long libbpf_get_error(const void *ptr);
+
 #endif
diff --git a/tools/perf/tests/llvm.c b/tools/perf/tests/llvm.c
index 02a33ebcd992..d357dab72e68 100644
--- a/tools/perf/tests/llvm.c
+++ b/tools/perf/tests/llvm.c
@@ -13,7 +13,7 @@ static int test__bpf_parsing(void *obj_buf, size_t obj_buf_sz)
 	struct bpf_object *obj;
 
 	obj = bpf_object__open_buffer(obj_buf, obj_buf_sz, NULL);
-	if (IS_ERR(obj))
+	if (libbpf_get_error(obj))
 		return TEST_FAIL;
 	bpf_object__close(obj);
 	return TEST_OK;
-- 
2.11.0

[PATCH perf/core 6/6] tools lib bpf: Add bpf_object__pin()

[Joe Stringer]

Add a new API to pin a BPF object to the filesystem. The user can
specify a subdirectory under the BPF filesystem to pin these programs.

For example, with the subdirectory 'foo', programs and maps are pinned:
/sys/fs/bpf/foo/progs/PROG_NAME
/sys/fs/bpf/foo/maps/MAP_NAME

If the user has specified an alternative BPF filesystem mountpoint via
/proc/mounts, that will be read and used instead.

Signed-off-by: Joe Stringer <joe@ovn.org>
---
 tools/lib/bpf/libbpf.c | 136 +++++++++++++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h |   1 +
 2 files changed, 137 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 6b651c19870d..181dca0bdacb 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4,6 +4,7 @@
  * Copyright (C) 2013-2015 Alexei Starovoitov <ast@kernel.org>
  * Copyright (C) 2015 Wang Nan <wangnan0@huawei.com>
  * Copyright (C) 2015 Huawei Inc.
+ * Copyright (C) 2016 Nicira, Inc.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -31,7 +32,12 @@
 #include <linux/err.h>
 #include <linux/kernel.h>
 #include <linux/bpf.h>
+#include <linux/magic.h>
 #include <linux/list.h>
+#include <linux/limits.h>
+#include <sys/mount.h>
+#include <sys/stat.h>
+#include <sys/vfs.h>
 #include <libelf.h>
 #include <gelf.h>
 
@@ -1231,6 +1237,136 @@ int bpf_object__load(struct bpf_object *obj)
 	return err;
 }
 
+#define stringize(x) #x
+
+static int mount_bpf(char *path)
+{
+	const char *mounts_fmt = "%*s %"stringize(PATH_MAX)"s %#"stringize(NAME_MAX)"s %*s\n";
+	struct statfs st_fs;
+	char type[NAME_MAX];
+	int err = 0;
+	FILE *fp;
+
+	/* Populate 'path'. */
+	fp = fopen("/proc/mounts", "r");
+	if (fp) {
+		while (fscanf(fp, mounts_fmt, path, type) == 2) {
+			if (!strcmp(type, "bpf"))
+				break;
+		}
+		if (fclose(fp)) {
+			err = -errno;
+			pr_warning("failed to close /proc/mounts: %s\n",
+				   strerror(errno));
+		}
+		if (strcmp(type, "bpf")) {
+			err = -errno;
+			pr_debug("failed to find bpf mount\n");
+		}
+	} else {
+		err = -errno;
+		pr_warning("cannot open /proc/mounts: %s\n", strerror(errno));
+	}
+	if (err) {
+		pr_debug("using /sys/fs/bpf for BPF filesystem mountpoint\n");
+		strcpy(path, "/sys/fs/bpf");
+	}
+
+	if (!statfs(path, &st_fs) && st_fs.f_type == BPF_FS_MAGIC)
+		return 0;
+
+	if (mount("bpf", path, "bpf", 0, NULL)) {
+		pr_warning("failed to mount bpf: %s\n", strerror(errno));
+		return -errno;
+	}
+
+	return 0;
+}
+
+static int make_dirs(const char *path, const char *subdir, char *buf,
+		     size_t len)
+{
+	snprintf(buf, len, "%s/%s/", path, subdir);
+	if (mkdir(buf, 0700) && errno != EEXIST) {
+		pr_warning("failed to mkdir %s: %s\n", subdir, strerror(errno));
+		return -errno;
+	}
+
+	snprintf(buf, len, "%s/%s/maps/", path, subdir);
+	if (mkdir(buf, 0700) && errno != EEXIST) {
+		pr_warning("failed to mkdir map: %s\n", strerror(errno));
+		return -errno;
+	}
+
+	snprintf(buf, len, "%s/%s/progs/", path, subdir);
+	if (mkdir(buf, 0700) && errno != EEXIST) {
+		pr_warning("failed to mkdir prog: %s\n", strerror(errno));
+		return -errno;
+	}
+
+	return 0;
+}
+
+int bpf_object__pin(struct bpf_object *obj, const char *subdir)
+{
+	const size_t len = 255;
+	char path[PATH_MAX];
+	char buf[len];
+	size_t i, j;
+	int err = 0;
+
+	if (!obj)
+		return -ENOENT;
+
+	if (!obj->loaded) {
+		pr_warning("object not yet loaded; load it first\n");
+		return -ENOENT;
+	}
+
+	err = mount_bpf(path);
+	if (err)
+		return err;
+
+	err = make_dirs(path, subdir, buf, len);
+	if (err)
+		return err;
+
+	for (i = 0; i < obj->nr_maps; i++) {
+		struct bpf_map *map = &obj->maps[i];
+
+		snprintf(buf, len, "%s/%s/maps/%s", path, subdir,
+			 bpf_map__name(map));
+		if (bpf_obj_pin(map->fd, buf)) {
+			err = -errno;
+			pr_warning("failed to pin map: %s\n", strerror(errno));
+			goto out;
+		}
+		pr_debug("Loaded map '%s' at '%s'\n", bpf_map__name(map), buf);
+	}
+
+	if (obj->programs && obj->nr_programs) {
+		for (i = 0; i < obj->nr_programs; i++) {
+			struct bpf_program *prog = &obj->programs[i];
+
+			for (j = 0; j < prog->instances.nr; j++) {
+				snprintf(buf, len, "%s/%s/progs/%s_%zu", path,
+					 subdir, prog->section_name, j);
+				if (bpf_obj_pin(prog->instances.fds[j], buf)) {
+					err = -errno;
+					pr_warning("failed to pin prog: %s\n",
+						   strerror(errno));
+					goto out;
+				}
+				pr_debug("Loaded prog '%s' at '%s'\n",
+					 prog->section_name, buf);
+			}
+		}
+	}
+
+out:
+	return err;
+}
+
 void bpf_object__close(struct bpf_object *obj)
 {
 	size_t i;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 4014d1ba5e3d..920699a437be 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -65,6 +65,7 @@ struct bpf_object *bpf_object__open(const char *path);
 struct bpf_object *bpf_object__open_buffer(void *obj_buf,
 					   size_t obj_buf_sz,
 					   const char *name);
+int bpf_object__pin(struct bpf_object *object, const char *subdir);
 void bpf_object__close(struct bpf_object *object);
 
 /* Load/unload object into/from kernel */
-- 
2.11.0

Re: [PATCH perf/core 1/6] tools lib bpf: Fix map offsets in relocation

[Wangnan (F)]

On 2017/1/19 7:57, Joe Stringer wrote:
> Commit 4708bbda5cb2 ("tools lib bpf: Fix maps resolution") attempted to
> fix map resolution by identifying the number of symbols that point to
> maps, and using this number to resolve each of the maps.
>
> However, during relocation the original definition of the map size was
> still in use.


Oops... Didn't realize that.


>   For up to two maps, the calculation was correct if there
> was a small difference in size between the map definition in libbpf and
> the one that the client library uses. However if the difference was
> large, particularly if more than two maps were used in the BPF program,
> the relocation would fail.
>
> For example, when using a map definition with size 28, with three maps,
> map relocation would count
>      (sym_offset / sizeof(struct bpf_map_def) => map_idx)
>      (0 / 16 => 0), ie map_idx = 0
>      (28 / 16 => 1), ie map_idx = 1
>      (56 / 16 => 3), ie map_idx = 3
>
> So, libbpf reports:
>      libbpf: bpf relocation: map_idx 3 large than 2

Understand.

> Fix map relocation by tracking the size of the map definition during
> map counting, then reuse this instead of the size of the libbpf map
> structure.

Then we add a restriction that map size is fixed
in one object, but we don't have to. During relocating,
we can get exact map positioning information from
sym.st_value. I think finding a map with exact offset
from the map array would be better.

I'll write a patch to replace this one.

Thank you.

>   With this patch applied, libbpf will accept such ELFs.
>
> Fixes: 4708bbda5cb2 ("tools lib bpf: Fix maps resolution")
> Signed-off-by: Joe Stringer <joe@ovn.org>
> ---
>   tools/lib/bpf/libbpf.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 84e6b35da4bd..350ee4c59f85 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -201,6 +201,7 @@ struct bpf_object {
>   	size_t nr_programs;
>   	struct bpf_map *maps;
>   	size_t nr_maps;
> +	size_t map_len;
>   
>   	bool loaded;
>   
> @@ -379,6 +380,7 @@ static struct bpf_object *bpf_object__new(const char *path,
>   	obj->efile.maps_shndx = -1;
>   
>   	obj->loaded = false;
> +	obj->map_len = sizeof(struct bpf_map_def);
>   
>   	INIT_LIST_HEAD(&obj->list);
>   	list_add(&obj->list, &bpf_objects_list);
> @@ -602,6 +604,7 @@ bpf_object__init_maps(struct bpf_object *obj)
>   		return -ENOMEM;
>   	}
>   	obj->nr_maps = nr_maps;
> +	obj->map_len = data->d_size / nr_maps;
>   
>   	/*
>   	 * fill all fd with -1 so won't close incorrect
> @@ -829,7 +832,7 @@ bpf_program__collect_reloc(struct bpf_program *prog,
>   			return -LIBBPF_ERRNO__RELOC;
>   		}
>   
> -		map_idx = sym.st_value / sizeof(struct bpf_map_def);
> +		map_idx = sym.st_value / prog->obj->map_len;
>   		if (map_idx >= nr_maps) {
>   			pr_warning("bpf relocation: map_idx %d large than %d\n",
>   				   (int)map_idx, (int)nr_maps - 1);

[PATCH -improve] tools lib bpf: Fix map offsets in relocation

[Wang Nan]

From: Joe Stringer <joe@ovn.org>

Commit 4708bbda5cb2 ("tools lib bpf: Fix maps resolution") attempted to
fix map resolution by identifying the number of symbols that point to
maps, and using this number to resolve each of the maps.

However, during relocation the original definition of the map size was
still in use. For up to two maps, the calculation was correct if there
was a small difference in size between the map definition in libbpf and
the one that the client library uses. However if the difference was
large, particularly if more than two maps were used in the BPF program,
the relocation would fail.

For example, when using a map definition with size 28, with three maps,
map relocation would count
    (sym_offset / sizeof(struct bpf_map_def) => map_idx)
    (0 / 16 => 0), ie map_idx = 0
    (28 / 16 => 1), ie map_idx = 1
    (56 / 16 => 3), ie map_idx = 3

So, libbpf reports:
    libbpf: bpf relocation: map_idx 3 large than 2

Fix map relocation by checking the exact offset of maps when doing
relocation.

Fixes: 4708bbda5cb2 ("tools lib bpf: Fix maps resolution")
Signed-off-by: Joe Stringer <joe@ovn.org>
Signed-off-by: Wang Nan <wangnan0@huawei.com>
[Allow different map size in an object]
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/bpf/libbpf.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 84e6b35..671d5ad 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -779,7 +779,7 @@ static int
 bpf_program__collect_reloc(struct bpf_program *prog,
 			   size_t nr_maps, GElf_Shdr *shdr,
 			   Elf_Data *data, Elf_Data *symbols,
-			   int maps_shndx)
+			   int maps_shndx, struct bpf_map *maps)
 {
 	int i, nrels;
 
@@ -829,7 +829,15 @@ bpf_program__collect_reloc(struct bpf_program *prog,
 			return -LIBBPF_ERRNO__RELOC;
 		}
 
-		map_idx = sym.st_value / sizeof(struct bpf_map_def);
+		/* TODO: 'maps' is sorted. We can use bsearch to make it faster. */
+		for (map_idx = 0; map_idx < nr_maps; map_idx++) {
+			if (maps[map_idx].offset == sym.st_value) {
+				pr_debug("relocation: find map %zd (%s) for insn %u\n",
+					 map_idx, maps[map_idx].name, insn_idx);
+				break;
+			}
+		}
+
 		if (map_idx >= nr_maps) {
 			pr_warning("bpf relocation: map_idx %d large than %d\n",
 				   (int)map_idx, (int)nr_maps - 1);
@@ -953,7 +961,8 @@ static int bpf_object__collect_reloc(struct bpf_object *obj)
 		err = bpf_program__collect_reloc(prog, nr_maps,
 						 shdr, data,
 						 obj->efile.symbols,
-						 obj->efile.maps_shndx);
+						 obj->efile.maps_shndx,
+						 obj->maps);
 		if (err)
 			return err;
 	}
-- 
2.10.1

Re: [PATCH perf/core 2/6] tools lib bpf: Fix grammar in map_idx warning

[Wangnan (F)]

On 2017/1/19 7:57, Joe Stringer wrote:
> "Larger" is the comparative adjective form of "large".
>
> Signed-off-by: Joe Stringer <joe@ovn.org>
> ---
>   tools/lib/bpf/libbpf.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 350ee4c59f85..653b1b368deb 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -834,7 +834,7 @@ bpf_program__collect_reloc(struct bpf_program *prog,
>   
>   		map_idx = sym.st_value / prog->obj->map_len;
>   		if (map_idx >= nr_maps) {
> -			pr_warning("bpf relocation: map_idx %d large than %d\n",
> +			pr_warning("bpf relocation: map_idx %d larger than %d\n",

With my change to 1/6, this message become meaningless. We can
change it to:

     bpf relocation: failed to relocate instruction %u

Thank you.

>   				   (int)map_idx, (int)nr_maps - 1);
>   			return -LIBBPF_ERRNO__RELOC;
>   		}

Re: [PATCH perf/core 6/6] tools lib bpf: Add bpf_object__pin()

[Wangnan (F)]

On 2017/1/19 7:57, Joe Stringer wrote:
> Add a new API to pin a BPF object to the filesystem. The user can
> specify a subdirectory under the BPF filesystem to pin these programs.
>
> For example, with the subdirectory 'foo', programs and maps are pinned:
> /sys/fs/bpf/foo/progs/PROG_NAME
> /sys/fs/bpf/foo/maps/MAP_NAME
>
> If the user has specified an alternative BPF filesystem mountpoint via
> /proc/mounts, that will be read and used instead.
>
> Signed-off-by: Joe Stringer <joe@ovn.org>
> ---
>   tools/lib/bpf/libbpf.c | 136 +++++++++++++++++++++++++++++++++++++++++++++++++
>   tools/lib/bpf/libbpf.h |   1 +
>   2 files changed, 137 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 6b651c19870d..181dca0bdacb 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4,6 +4,7 @@
>    * Copyright (C) 2013-2015 Alexei Starovoitov <ast@kernel.org>
>    * Copyright (C) 2015 Wang Nan <wangnan0@huawei.com>
>    * Copyright (C) 2015 Huawei Inc.
> + * Copyright (C) 2016 Nicira, Inc.
>    *
>    * This program is free software; you can redistribute it and/or
>    * modify it under the terms of the GNU Lesser General Public
> @@ -31,7 +32,12 @@
>   #include <linux/err.h>
>   #include <linux/kernel.h>
>   #include <linux/bpf.h>
> +#include <linux/magic.h>
>   #include <linux/list.h>
> +#include <linux/limits.h>
> +#include <sys/mount.h>
> +#include <sys/stat.h>
> +#include <sys/vfs.h>
>   #include <libelf.h>
>   #include <gelf.h>
>   
> @@ -1231,6 +1237,136 @@ int bpf_object__load(struct bpf_object *obj)
>   	return err;
>   }
>   
> +#define stringize(x) #x
> +
> +static int mount_bpf(char *path)

We can use tools/lib/api/fs/fs.c to help us mounting filesystems.
Try not reinventing it.

> +{
> +	const char *mounts_fmt = "%*s %"stringize(PATH_MAX)"s %#"stringize(NAME_MAX)"s %*s\n";
> +	struct statfs st_fs;
> +	char type[NAME_MAX];
> +	int err = 0;
> +	FILE *fp;
> +
> +	/* Populate 'path'. */
> +	fp = fopen("/proc/mounts", "r");
> +	if (fp) {
> +		while (fscanf(fp, mounts_fmt, path, type) == 2) {
> +			if (!strcmp(type, "bpf"))
> +				break;
> +		}
> +		if (fclose(fp)) {
> +			err = -errno;
> +			pr_warning("failed to close /proc/mounts: %s\n",
> +				   strerror(errno));
> +		}
> +		if (strcmp(type, "bpf")) {
> +			err = -errno;
> +			pr_debug("failed to find bpf mount\n");
> +		}
> +	} else {
> +		err = -errno;
> +		pr_warning("cannot open /proc/mounts: %s\n", strerror(errno));
> +	}
> +	if (err) {
> +		pr_debug("using /sys/fs/bpf for BPF filesystem mountpoint\n");
> +		strcpy(path, "/sys/fs/bpf");
> +	}
> +
> +	if (!statfs(path, &st_fs) && st_fs.f_type == BPF_FS_MAGIC)
> +		return 0;
> +
> +	if (mount("bpf", path, "bpf", 0, NULL)) {
> +		pr_warning("failed to mount bpf: %s\n", strerror(errno));
> +		return -errno;
> +	}
> +
> +	return 0;
> +}
> +
> +static int make_dirs(const char *path, const char *subdir, char *buf,
> +		     size_t len)
> +{
> +	snprintf(buf, len, "%s/%s/", path, subdir);
> +	if (mkdir(buf, 0700) && errno != EEXIST) {
> +		pr_warning("failed to mkdir %s: %s\n", subdir, strerror(errno));
> +		return -errno;
> +	}
> +
> +	snprintf(buf, len, "%s/%s/maps/", path, subdir);
> +	if (mkdir(buf, 0700) && errno != EEXIST) {
> +		pr_warning("failed to mkdir map: %s\n", strerror(errno));
> +		return -errno;
> +	}
> +
> +	snprintf(buf, len, "%s/%s/progs/", path, subdir);
> +	if (mkdir(buf, 0700) && errno != EEXIST) {
> +		pr_warning("failed to mkdir prog: %s\n", strerror(errno));
> +		return -errno;
> +	}
> +
> +	return 0;
> +}
> +
> +int bpf_object__pin(struct bpf_object *obj, const char *subdir)
> +{

We must pin a whole object? I suggest we provide
bpf_program__pin and bpf_map__pin, and build bpf_object__pin
on top of them.

Thank you.

> +	const size_t len = 255;
> +	char path[PATH_MAX];
> +	char buf[len];
> +	size_t i, j;
> +	int err = 0;
> +
> +	if (!obj)
> +		return -ENOENT;
> +
> +	if (!obj->loaded) {
> +		pr_warning("object not yet loaded; load it first\n");
> +		return -ENOENT;
> +	}
> +
> +	err = mount_bpf(path);
> +	if (err)
> +		return err;
> +
> +	err = make_dirs(path, subdir, buf, len);
> +	if (err)
> +		return err;
> +
> +	for (i = 0; i < obj->nr_maps; i++) {
> +		struct bpf_map *map = &obj->maps[i];
> +
> +		snprintf(buf, len, "%s/%s/maps/%s", path, subdir,
> +			 bpf_map__name(map));
> +		if (bpf_obj_pin(map->fd, buf)) {
> +			err = -errno;
> +			pr_warning("failed to pin map: %s\n", strerror(errno));
> +			goto out;
> +		}
> +		pr_debug("Loaded map '%s' at '%s'\n", bpf_map__name(map), buf);
> +	}
> +
> +	if (obj->programs && obj->nr_programs) {
> +		for (i = 0; i < obj->nr_programs; i++) {
> +			struct bpf_program *prog = &obj->programs[i];
> +
> +			for (j = 0; j < prog->instances.nr; j++) {
> +				snprintf(buf, len, "%s/%s/progs/%s_%zu", path,
> +					 subdir, prog->section_name, j);
> +				if (bpf_obj_pin(prog->instances.fds[j], buf)) {
> +					err = -errno;
> +					pr_warning("failed to pin prog: %s\n",
> +						   strerror(errno));
> +					goto out;
> +				}
> +				pr_debug("Loaded prog '%s' at '%s'\n",
> +					 prog->section_name, buf);
> +			}
> +		}
> +	}
> +
> +out:
> +	return err;
> +}
> +
>   void bpf_object__close(struct bpf_object *obj)
>   {
>   	size_t i;
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 4014d1ba5e3d..920699a437be 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -65,6 +65,7 @@ struct bpf_object *bpf_object__open(const char *path);
>   struct bpf_object *bpf_object__open_buffer(void *obj_buf,
>   					   size_t obj_buf_sz,
>   					   const char *name);
> +int bpf_object__pin(struct bpf_object *object, const char *subdir);
>   void bpf_object__close(struct bpf_object *object);
>   
>   /* Load/unload object into/from kernel */

Re: [PATCH perf/core 0/6] Libbpf improvements

[Wangnan (F)]

On 2017/1/19 7:57, Joe Stringer wrote:
> Patch 1 fixes an issue when using drastically different BPF map definitions
> inside ELFs from a client using libbpf, vs the map definition libbpf uses.
>
> Patch 2 is a trivial typo fix.
>
> Patches 3-5 add some simple, useful helper functions for setting prog type
> and retrieving libbpf errors without depending on kernel headers from
> userspace programs.
>
> Patch 6 adds a new pinning functionality for an entire object. Calling
> bpf_object__pin(obj, subpath) will mount all programs from the BPF object
> to $bpf_fs_path/$subpath/progs/$progname, and all maps from the BPF object
> to $bpf_fs_path/$subpath/maps/$mapname. The first program with a particular
> name will be mounted with its progname and the suffix "_0"; subsequent
> programs with the same will have "_1", and so on; duplicate maps with the
> same name are disallowed.
>
> Joe Stringer (6):
>    tools lib bpf: Fix map offsets in relocation
>    tools lib bpf: Fix grammar in map_idx warning
>    tools lib bpf: Define prog_type fns with macro
>    tools lib bpf: Add set/is helpers for all prog types
>    tools lib bpf: Add libbpf_get_error()
>    tools lib bpf: Add bpf_object__pin()
>
>   tools/lib/bpf/libbpf.c  | 197 +++++++++++++++++++++++++++++++++++++++++-------
>   tools/lib/bpf/libbpf.h  |  15 +++-
>   tools/perf/tests/llvm.c |   2 +-
>   3 files changed, 185 insertions(+), 29 deletions(-)
>
For patch 3,4 and 5:

Acked-by: Wang Nan <wangnan0@huawei.com>

See my comment to commits 1, 2 and 6.

Thank you.

Re: [PATCH perf/core 6/6] tools lib bpf: Add bpf_object__pin()

[Joe Stringer]

On 19 January 2017 at 02:22, Wangnan (F) <wangnan0@huawei.com> wrote:
>
>
> On 2017/1/19 7:57, Joe Stringer wrote:
>>
>> Add a new API to pin a BPF object to the filesystem. The user can
>> specify a subdirectory under the BPF filesystem to pin these programs.
>>
>> For example, with the subdirectory 'foo', programs and maps are pinned:
>> /sys/fs/bpf/foo/progs/PROG_NAME
>> /sys/fs/bpf/foo/maps/MAP_NAME
>>
>> If the user has specified an alternative BPF filesystem mountpoint via
>> /proc/mounts, that will be read and used instead.
>>
>> Signed-off-by: Joe Stringer <joe@ovn.org>
>> ---
>>   tools/lib/bpf/libbpf.c | 136
>> +++++++++++++++++++++++++++++++++++++++++++++++++
>>   tools/lib/bpf/libbpf.h |   1 +
>>   2 files changed, 137 insertions(+)
>>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index 6b651c19870d..181dca0bdacb 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -4,6 +4,7 @@
>>    * Copyright (C) 2013-2015 Alexei Starovoitov <ast@kernel.org>
>>    * Copyright (C) 2015 Wang Nan <wangnan0@huawei.com>
>>    * Copyright (C) 2015 Huawei Inc.
>> + * Copyright (C) 2016 Nicira, Inc.
>>    *
>>    * This program is free software; you can redistribute it and/or
>>    * modify it under the terms of the GNU Lesser General Public
>> @@ -31,7 +32,12 @@
>>   #include <linux/err.h>
>>   #include <linux/kernel.h>
>>   #include <linux/bpf.h>
>> +#include <linux/magic.h>
>>   #include <linux/list.h>
>> +#include <linux/limits.h>
>> +#include <sys/mount.h>
>> +#include <sys/stat.h>
>> +#include <sys/vfs.h>
>>   #include <libelf.h>
>>   #include <gelf.h>
>>   @@ -1231,6 +1237,136 @@ int bpf_object__load(struct bpf_object *obj)
>>         return err;
>>   }
>>   +#define stringize(x) #x
>> +
>> +static int mount_bpf(char *path)
>
>
> We can use tools/lib/api/fs/fs.c to help us mounting filesystems.
> Try not reinventing it.

libbpf is LGPL2.1 and libapi doesn't state a license.

Instead, maybe I'll just drop this piece - libbpf user can mount the
filesystem (using libapi or something else) and provide fully
qualified path to bpf_{map,program}__pin().

Re: [PATCH perf/core 0/6] Libbpf improvements

[Joe Stringer]

On 19 January 2017 at 02:24, Wangnan (F) <wangnan0@huawei.com> wrote:
>
>
> On 2017/1/19 7:57, Joe Stringer wrote:
>>
>> Patch 1 fixes an issue when using drastically different BPF map
>> definitions
>> inside ELFs from a client using libbpf, vs the map definition libbpf uses.
>>
>> Patch 2 is a trivial typo fix.
>>
>> Patches 3-5 add some simple, useful helper functions for setting prog type
>> and retrieving libbpf errors without depending on kernel headers from
>> userspace programs.
>>
>> Patch 6 adds a new pinning functionality for an entire object. Calling
>> bpf_object__pin(obj, subpath) will mount all programs from the BPF object
>> to $bpf_fs_path/$subpath/progs/$progname, and all maps from the BPF object
>> to $bpf_fs_path/$subpath/maps/$mapname. The first program with a
>> particular
>> name will be mounted with its progname and the suffix "_0"; subsequent
>> programs with the same will have "_1", and so on; duplicate maps with the
>> same name are disallowed.
>>
>> Joe Stringer (6):
>>    tools lib bpf: Fix map offsets in relocation
>>    tools lib bpf: Fix grammar in map_idx warning
>>    tools lib bpf: Define prog_type fns with macro
>>    tools lib bpf: Add set/is helpers for all prog types
>>    tools lib bpf: Add libbpf_get_error()
>>    tools lib bpf: Add bpf_object__pin()
>>
>>   tools/lib/bpf/libbpf.c  | 197
>> +++++++++++++++++++++++++++++++++++++++++-------
>>   tools/lib/bpf/libbpf.h  |  15 +++-
>>   tools/perf/tests/llvm.c |   2 +-
>>   3 files changed, 185 insertions(+), 29 deletions(-)
>>
> For patch 3,4 and 5:
>
> Acked-by: Wang Nan <wangnan0@huawei.com>
>
> See my comment to commits 1, 2 and 6.

Thanks for the review, I addressed your comments and sent a v2 series.