All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/9] libbpf random fixes
@ 2019-05-29  1:14 Andrii Nakryiko
  2019-05-29  1:14 ` [PATCH bpf-next 1/9] libbpf: fix detection of corrupted BPF instructions section Andrii Nakryiko
                   ` (8 more replies)
  0 siblings, 9 replies; 20+ messages in thread
From: Andrii Nakryiko @ 2019-05-29  1:14 UTC (permalink / raw)
  To: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team; +Cc: Andrii Nakryiko

This patch set is a collection of unrelated fixes for libbpf.

Patch #1 fixes detection of corrupted BPF section w/ instructions.
Patch #2 fixes possible errno clobbering.
Patch #3 simplifies endianness check and brings it in line with few other
similar checks in libbpf.
Patch #4 adds check for failed map name retrieval from ELF symbol name.
Patch #5 fixes return error code to be negative.
Patch #6 fixes using valid fd (0) as a marker of missing associated BTF.
Patch #7 removes redundant logic in two places.
Patch #8 fixes typos in comments and debug output, and fixes formatting.
Patch #9 unwraps a bunch of multi-line statements and comments.

If patches #8 and #9 create too much history noise, I can drop them, they
don't have functional changes.

Andrii Nakryiko (9):
  libbpf: fix detection of corrupted BPF instructions section
  libbpf: preserve errno before calling into user callback
  libbpf: simplify endianness check
  libbpf: check map name retrieved from ELF
  libbpf: fix error code returned on corrupted ELF
  libbpf: use negative fd to specify missing BTF
  libbpf: simplify two pieces of logic
  libbpf: typo and formatting fixes
  libbpf: reduce unnecessary line wrapping

 tools/lib/bpf/libbpf.c | 148 +++++++++++++++++------------------------
 1 file changed, 60 insertions(+), 88 deletions(-)

-- 
2.17.1


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

* [PATCH bpf-next 1/9] libbpf: fix detection of corrupted BPF instructions section
  2019-05-29  1:14 [PATCH bpf-next 0/9] libbpf random fixes Andrii Nakryiko
@ 2019-05-29  1:14 ` Andrii Nakryiko
  2019-05-29 17:01   ` Song Liu
  2019-05-29  1:14 ` [PATCH bpf-next 2/9] libbpf: preserve errno before calling into user callback Andrii Nakryiko
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Andrii Nakryiko @ 2019-05-29  1:14 UTC (permalink / raw)
  To: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team; +Cc: Andrii Nakryiko

Ensure that size of a section w/ BPF instruction is exactly a multiple
of BPF instruction size.

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

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index ca4432f5b067..05a73223e524 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -349,8 +349,11 @@ static int
 bpf_program__init(void *data, size_t size, char *section_name, int idx,
 		  struct bpf_program *prog)
 {
-	if (size < sizeof(struct bpf_insn)) {
-		pr_warning("corrupted section '%s'\n", section_name);
+	const size_t bpf_insn_sz = sizeof(struct bpf_insn);
+
+	if (size < bpf_insn_sz || size % bpf_insn_sz) {
+		pr_warning("corrupted section '%s', size: %zu\n",
+			   section_name, size);
 		return -EINVAL;
 	}
 
@@ -376,9 +379,8 @@ bpf_program__init(void *data, size_t size, char *section_name, int idx,
 			   section_name);
 		goto errout;
 	}
-	prog->insns_cnt = size / sizeof(struct bpf_insn);
-	memcpy(prog->insns, data,
-	       prog->insns_cnt * sizeof(struct bpf_insn));
+	prog->insns_cnt = size / bpf_insn_sz;
+	memcpy(prog->insns, data, prog->insns_cnt * bpf_insn_sz);
 	prog->idx = idx;
 	prog->instances.fds = NULL;
 	prog->instances.nr = -1;
-- 
2.17.1


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

* [PATCH bpf-next 2/9] libbpf: preserve errno before calling into user callback
  2019-05-29  1:14 [PATCH bpf-next 0/9] libbpf random fixes Andrii Nakryiko
  2019-05-29  1:14 ` [PATCH bpf-next 1/9] libbpf: fix detection of corrupted BPF instructions section Andrii Nakryiko
@ 2019-05-29  1:14 ` Andrii Nakryiko
  2019-05-29 17:03   ` Song Liu
  2019-05-29  1:14 ` [PATCH bpf-next 3/9] libbpf: simplify endianness check Andrii Nakryiko
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Andrii Nakryiko @ 2019-05-29  1:14 UTC (permalink / raw)
  To: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team; +Cc: Andrii Nakryiko

pr_warning ultimately may call into user-provided callback function,
which can clobber errno value, so we need to save it before that.

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

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 05a73223e524..7b80b9ae8a1f 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -565,12 +565,12 @@ static int bpf_object__elf_init(struct bpf_object *obj)
 	} else {
 		obj->efile.fd = open(obj->path, O_RDONLY);
 		if (obj->efile.fd < 0) {
-			char errmsg[STRERR_BUFSIZE];
-			char *cp = libbpf_strerror_r(errno, errmsg,
-						     sizeof(errmsg));
+			char errmsg[STRERR_BUFSIZE], *cp;
 
+			err = -errno;
+			cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg));
 			pr_warning("failed to open %s: %s\n", obj->path, cp);
-			return -errno;
+			return err;
 		}
 
 		obj->efile.elf = elf_begin(obj->efile.fd,
-- 
2.17.1


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

* [PATCH bpf-next 3/9] libbpf: simplify endianness check
  2019-05-29  1:14 [PATCH bpf-next 0/9] libbpf random fixes Andrii Nakryiko
  2019-05-29  1:14 ` [PATCH bpf-next 1/9] libbpf: fix detection of corrupted BPF instructions section Andrii Nakryiko
  2019-05-29  1:14 ` [PATCH bpf-next 2/9] libbpf: preserve errno before calling into user callback Andrii Nakryiko
@ 2019-05-29  1:14 ` Andrii Nakryiko
  2019-05-29 17:09   ` Song Liu
  2019-05-29  1:14 ` [PATCH bpf-next 4/9] libbpf: check map name retrieved from ELF Andrii Nakryiko
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Andrii Nakryiko @ 2019-05-29  1:14 UTC (permalink / raw)
  To: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team; +Cc: Andrii Nakryiko

Rewrite endianness check to use "more canonical" way, using
compiler-defined macros, similar to few other places in libbpf. It also
is more obvious and shorter.

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

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 7b80b9ae8a1f..c98f9942fba4 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -607,31 +607,18 @@ static int bpf_object__elf_init(struct bpf_object *obj)
 	return err;
 }
 
-static int
-bpf_object__check_endianness(struct bpf_object *obj)
-{
-	static unsigned int const endian = 1;
-
-	switch (obj->efile.ehdr.e_ident[EI_DATA]) {
-	case ELFDATA2LSB:
-		/* We are big endian, BPF obj is little endian. */
-		if (*(unsigned char const *)&endian != 1)
-			goto mismatch;
-		break;
-
-	case ELFDATA2MSB:
-		/* We are little endian, BPF obj is big endian. */
-		if (*(unsigned char const *)&endian != 0)
-			goto mismatch;
-		break;
-	default:
-		return -LIBBPF_ERRNO__ENDIAN;
-	}
-
-	return 0;
-
-mismatch:
-	pr_warning("Error: endianness mismatch.\n");
+static int bpf_object__check_endianness(struct bpf_object *obj)
+{
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+	if (obj->efile.ehdr.e_ident[EI_DATA] == ELFDATA2LSB)
+		return 0;
+#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+	if (obj->efile.ehdr.e_ident[EI_DATA] == ELFDATA2MSB)
+		return 0;
+#else
+# error "Unrecognized __BYTE_ORDER__"
+#endif
+	pr_warning("endianness mismatch.\n");
 	return -LIBBPF_ERRNO__ENDIAN;
 }
 
-- 
2.17.1


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

* [PATCH bpf-next 4/9] libbpf: check map name retrieved from ELF
  2019-05-29  1:14 [PATCH bpf-next 0/9] libbpf random fixes Andrii Nakryiko
                   ` (2 preceding siblings ...)
  2019-05-29  1:14 ` [PATCH bpf-next 3/9] libbpf: simplify endianness check Andrii Nakryiko
@ 2019-05-29  1:14 ` Andrii Nakryiko
  2019-05-29 17:11   ` Song Liu
  2019-05-29  1:14 ` [PATCH bpf-next 5/9] libbpf: fix error code returned on corrupted ELF Andrii Nakryiko
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Andrii Nakryiko @ 2019-05-29  1:14 UTC (permalink / raw)
  To: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team; +Cc: Andrii Nakryiko

Validate there was no error retrieving symbol name corresponding to
a BPF map.

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

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index c98f9942fba4..7abe71ee507a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -920,6 +920,11 @@ bpf_object__init_maps(struct bpf_object *obj, int flags)
 		map_name = elf_strptr(obj->efile.elf,
 				      obj->efile.strtabidx,
 				      sym.st_name);
+		if (!map_name) {
+			pr_warning("failed to get map #%d name sym string for obj %s\n",
+				   map_idx, obj->path);
+			return -LIBBPF_ERRNO__FORMAT;
+		}
 
 		obj->maps[map_idx].libbpf_type = LIBBPF_MAP_UNSPEC;
 		obj->maps[map_idx].offset = sym.st_value;
-- 
2.17.1


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

* [PATCH bpf-next 5/9] libbpf: fix error code returned on corrupted ELF
  2019-05-29  1:14 [PATCH bpf-next 0/9] libbpf random fixes Andrii Nakryiko
                   ` (3 preceding siblings ...)
  2019-05-29  1:14 ` [PATCH bpf-next 4/9] libbpf: check map name retrieved from ELF Andrii Nakryiko
@ 2019-05-29  1:14 ` Andrii Nakryiko
  2019-05-29 17:13   ` Song Liu
  2019-05-29  1:14 ` [PATCH bpf-next 6/9] libbpf: use negative fd to specify missing BTF Andrii Nakryiko
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Andrii Nakryiko @ 2019-05-29  1:14 UTC (permalink / raw)
  To: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team; +Cc: Andrii Nakryiko

All of libbpf errors are negative, except this one. Fix it.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 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 7abe71ee507a..9c45856e7fd6 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1221,7 +1221,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj, int flags)
 
 	if (!obj->efile.strtabidx || obj->efile.strtabidx >= idx) {
 		pr_warning("Corrupted ELF file: index of strtab invalid\n");
-		return LIBBPF_ERRNO__FORMAT;
+		return -LIBBPF_ERRNO__FORMAT;
 	}
 	if (btf_data) {
 		obj->btf = btf__new(btf_data->d_buf, btf_data->d_size);
-- 
2.17.1


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

* [PATCH bpf-next 6/9] libbpf: use negative fd to specify missing BTF
  2019-05-29  1:14 [PATCH bpf-next 0/9] libbpf random fixes Andrii Nakryiko
                   ` (4 preceding siblings ...)
  2019-05-29  1:14 ` [PATCH bpf-next 5/9] libbpf: fix error code returned on corrupted ELF Andrii Nakryiko
@ 2019-05-29  1:14 ` Andrii Nakryiko
  2019-05-29 17:23   ` Song Liu
  2019-05-29  1:14 ` [PATCH bpf-next 7/9] libbpf: simplify two pieces of logic Andrii Nakryiko
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Andrii Nakryiko @ 2019-05-29  1:14 UTC (permalink / raw)
  To: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team; +Cc: Andrii Nakryiko

0 is a valid FD, so it's better to initialize it to -1, as is done in
other places. Also, technically, BTF type ID 0 is valid (it's a VOID
type), so it's more reliable to check btf_fd, instead of
btf_key_type_id, to determine if there is any BTF associated with a map.

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

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 9c45856e7fd6..292ea9a2dc3d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1751,7 +1751,7 @@ bpf_object__create_maps(struct bpf_object *obj)
 		create_attr.key_size = def->key_size;
 		create_attr.value_size = def->value_size;
 		create_attr.max_entries = def->max_entries;
-		create_attr.btf_fd = 0;
+		create_attr.btf_fd = -1;
 		create_attr.btf_key_type_id = 0;
 		create_attr.btf_value_type_id = 0;
 		if (bpf_map_type__is_map_in_map(def->type) &&
@@ -1765,11 +1765,11 @@ bpf_object__create_maps(struct bpf_object *obj)
 		}
 
 		*pfd = bpf_create_map_xattr(&create_attr);
-		if (*pfd < 0 && create_attr.btf_key_type_id) {
+		if (*pfd < 0 && create_attr.btf_fd >= 0) {
 			cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
 			pr_warning("Error in bpf_create_map_xattr(%s):%s(%d). Retrying without BTF.\n",
 				   map->name, cp, errno);
-			create_attr.btf_fd = 0;
+			create_attr.btf_fd = -1;
 			create_attr.btf_key_type_id = 0;
 			create_attr.btf_value_type_id = 0;
 			map->btf_key_type_id = 0;
@@ -2053,6 +2053,9 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
 	char *log_buf;
 	int ret;
 
+	if (!insns || !insns_cnt)
+		return -EINVAL;
+
 	memset(&load_attr, 0, sizeof(struct bpf_load_program_attr));
 	load_attr.prog_type = prog->type;
 	load_attr.expected_attach_type = prog->expected_attach_type;
@@ -2063,7 +2066,7 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
 	load_attr.license = license;
 	load_attr.kern_version = kern_version;
 	load_attr.prog_ifindex = prog->prog_ifindex;
-	load_attr.prog_btf_fd = prog->btf_fd >= 0 ? prog->btf_fd : 0;
+	load_attr.prog_btf_fd = prog->btf_fd;
 	load_attr.func_info = prog->func_info;
 	load_attr.func_info_rec_size = prog->func_info_rec_size;
 	load_attr.func_info_cnt = prog->func_info_cnt;
@@ -2072,8 +2075,6 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
 	load_attr.line_info_cnt = prog->line_info_cnt;
 	load_attr.log_level = prog->log_level;
 	load_attr.prog_flags = prog->prog_flags;
-	if (!load_attr.insns || !load_attr.insns_cnt)
-		return -EINVAL;
 
 retry_load:
 	log_buf = malloc(log_buf_size);
-- 
2.17.1


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

* [PATCH bpf-next 7/9] libbpf: simplify two pieces of logic
  2019-05-29  1:14 [PATCH bpf-next 0/9] libbpf random fixes Andrii Nakryiko
                   ` (5 preceding siblings ...)
  2019-05-29  1:14 ` [PATCH bpf-next 6/9] libbpf: use negative fd to specify missing BTF Andrii Nakryiko
@ 2019-05-29  1:14 ` Andrii Nakryiko
  2019-05-29 17:24   ` Song Liu
  2019-05-29  1:14 ` [PATCH bpf-next 8/9] libbpf: typo and formatting fixes Andrii Nakryiko
  2019-05-29  1:14 ` [PATCH bpf-next 9/9] libbpf: reduce unnecessary line wrapping Andrii Nakryiko
  8 siblings, 1 reply; 20+ messages in thread
From: Andrii Nakryiko @ 2019-05-29  1:14 UTC (permalink / raw)
  To: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team; +Cc: Andrii Nakryiko

Extra check for type is unnecessary in first case.

Extra zeroing is unnecessary, as snprintf guarantees that it will
zero-terminate string.

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

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 292ea9a2dc3d..e3bc00933145 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1430,8 +1430,7 @@ bpf_program__collect_reloc(struct bpf_program *prog, GElf_Shdr *shdr,
 				if (maps[map_idx].libbpf_type != type)
 					continue;
 				if (type != LIBBPF_MAP_UNSPEC ||
-				    (type == LIBBPF_MAP_UNSPEC &&
-				     maps[map_idx].offset == sym.st_value)) {
+				    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;
@@ -2354,7 +2353,6 @@ struct bpf_object *bpf_object__open_buffer(void *obj_buf,
 		snprintf(tmp_name, sizeof(tmp_name), "%lx-%lx",
 			 (unsigned long)obj_buf,
 			 (unsigned long)obj_buf_sz);
-		tmp_name[sizeof(tmp_name) - 1] = '\0';
 		name = tmp_name;
 	}
 	pr_debug("loading object '%s' from buffer\n",
-- 
2.17.1


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

* [PATCH bpf-next 8/9] libbpf: typo and formatting fixes
  2019-05-29  1:14 [PATCH bpf-next 0/9] libbpf random fixes Andrii Nakryiko
                   ` (6 preceding siblings ...)
  2019-05-29  1:14 ` [PATCH bpf-next 7/9] libbpf: simplify two pieces of logic Andrii Nakryiko
@ 2019-05-29  1:14 ` Andrii Nakryiko
  2019-05-29 17:25   ` Song Liu
  2019-05-29  1:14 ` [PATCH bpf-next 9/9] libbpf: reduce unnecessary line wrapping Andrii Nakryiko
  8 siblings, 1 reply; 20+ messages in thread
From: Andrii Nakryiko @ 2019-05-29  1:14 UTC (permalink / raw)
  To: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team; +Cc: Andrii Nakryiko

A bunch of typo and formatting fixes.

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

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e3bc00933145..9d9c19a1b2fe 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -505,7 +505,7 @@ static struct bpf_object *bpf_object__new(const char *path,
 
 	obj->efile.fd = -1;
 	/*
-	 * Caller of this function should also calls
+	 * Caller of this function should also call
 	 * bpf_object__elf_finish() after data collection to return
 	 * obj_buf to user. If not, we should duplicate the buffer to
 	 * avoid user freeing them before elf finish.
@@ -574,8 +574,7 @@ static int bpf_object__elf_init(struct bpf_object *obj)
 		}
 
 		obj->efile.elf = elf_begin(obj->efile.fd,
-				LIBBPF_ELF_C_READ_MMAP,
-				NULL);
+					   LIBBPF_ELF_C_READ_MMAP, NULL);
 	}
 
 	if (!obj->efile.elf) {
@@ -594,9 +593,9 @@ static int bpf_object__elf_init(struct bpf_object *obj)
 	ep = &obj->efile.ehdr;
 
 	/* Old LLVM set e_machine to EM_NONE */
-	if ((ep->e_type != ET_REL) || (ep->e_machine && (ep->e_machine != EM_BPF))) {
-		pr_warning("%s is not an eBPF object file\n",
-			obj->path);
+	if (ep->e_type != ET_REL ||
+	    (ep->e_machine && ep->e_machine != EM_BPF)) {
+		pr_warning("%s is not an eBPF object file\n", obj->path);
 		err = -LIBBPF_ERRNO__FORMAT;
 		goto errout;
 	}
@@ -1438,7 +1437,7 @@ bpf_program__collect_reloc(struct bpf_program *prog, GElf_Shdr *shdr,
 			}
 
 			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;
 			}
@@ -1797,7 +1796,7 @@ bpf_object__create_maps(struct bpf_object *obj)
 			}
 		}
 
-		pr_debug("create map %s: fd=%d\n", map->name, *pfd);
+		pr_debug("created map %s: fd=%d\n", map->name, *pfd);
 	}
 
 	return 0;
-- 
2.17.1


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

* [PATCH bpf-next 9/9] libbpf: reduce unnecessary line wrapping
  2019-05-29  1:14 [PATCH bpf-next 0/9] libbpf random fixes Andrii Nakryiko
                   ` (7 preceding siblings ...)
  2019-05-29  1:14 ` [PATCH bpf-next 8/9] libbpf: typo and formatting fixes Andrii Nakryiko
@ 2019-05-29  1:14 ` Andrii Nakryiko
  2019-05-29 17:27   ` Song Liu
  8 siblings, 1 reply; 20+ messages in thread
From: Andrii Nakryiko @ 2019-05-29  1:14 UTC (permalink / raw)
  To: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team; +Cc: Andrii Nakryiko

There are a bunch of lines of code or comments that are unnecessary
wrapped into multi-lines. Fix that without violating any code
guidelines.

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

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 9d9c19a1b2fe..2c576843ea40 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -497,8 +497,7 @@ static struct bpf_object *bpf_object__new(const char *path,
 
 	strcpy(obj->path, path);
 	/* Using basename() GNU version which doesn't modify arg. */
-	strncpy(obj->name, basename((void *)path),
-		sizeof(obj->name) - 1);
+	strncpy(obj->name, basename((void *)path), sizeof(obj->name) - 1);
 	end = strchr(obj->name, '.');
 	if (end)
 		*end = 0;
@@ -578,15 +577,13 @@ static int bpf_object__elf_init(struct bpf_object *obj)
 	}
 
 	if (!obj->efile.elf) {
-		pr_warning("failed to open %s as ELF file\n",
-				obj->path);
+		pr_warning("failed to open %s as ELF file\n", obj->path);
 		err = -LIBBPF_ERRNO__LIBELF;
 		goto errout;
 	}
 
 	if (!gelf_getehdr(obj->efile.elf, &obj->efile.ehdr)) {
-		pr_warning("failed to get EHDR from %s\n",
-				obj->path);
+		pr_warning("failed to get EHDR from %s\n", obj->path);
 		err = -LIBBPF_ERRNO__FORMAT;
 		goto errout;
 	}
@@ -622,18 +619,15 @@ static int bpf_object__check_endianness(struct bpf_object *obj)
 }
 
 static int
-bpf_object__init_license(struct bpf_object *obj,
-			 void *data, size_t size)
+bpf_object__init_license(struct bpf_object *obj, void *data, size_t size)
 {
-	memcpy(obj->license, data,
-	       min(size, sizeof(obj->license) - 1));
+	memcpy(obj->license, data, min(size, sizeof(obj->license) - 1));
 	pr_debug("license of %s is %s\n", obj->path, obj->license);
 	return 0;
 }
 
 static int
-bpf_object__init_kversion(struct bpf_object *obj,
-			  void *data, size_t size)
+bpf_object__init_kversion(struct bpf_object *obj, void *data, size_t size)
 {
 	__u32 kver;
 
@@ -643,8 +637,7 @@ bpf_object__init_kversion(struct bpf_object *obj,
 	}
 	memcpy(&kver, data, sizeof(kver));
 	obj->kern_version = kver;
-	pr_debug("kernel version of %s is %x\n", obj->path,
-		 obj->kern_version);
+	pr_debug("kernel version of %s is %x\n", obj->path, obj->kern_version);
 	return 0;
 }
 
@@ -800,8 +793,7 @@ bpf_object__init_internal_map(struct bpf_object *obj, struct bpf_map *map,
 	def->key_size = sizeof(int);
 	def->value_size = data->d_size;
 	def->max_entries = 1;
-	def->map_flags = type == LIBBPF_MAP_RODATA ?
-			 BPF_F_RDONLY_PROG : 0;
+	def->map_flags = type == LIBBPF_MAP_RODATA ? BPF_F_RDONLY_PROG : 0;
 	if (data_buff) {
 		*data_buff = malloc(data->d_size);
 		if (!*data_buff) {
@@ -816,8 +808,7 @@ bpf_object__init_internal_map(struct bpf_object *obj, struct bpf_map *map,
 	return 0;
 }
 
-static int
-bpf_object__init_maps(struct bpf_object *obj, int flags)
+static int bpf_object__init_maps(struct bpf_object *obj, int flags)
 {
 	int i, map_idx, map_def_sz = 0, nr_syms, nr_maps = 0, nr_maps_glob = 0;
 	bool strict = !(flags & MAPS_RELAX_COMPAT);
@@ -1098,8 +1089,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj, int flags)
 
 	/* Elf is corrupted/truncated, avoid calling elf_strptr. */
 	if (!elf_rawdata(elf_getscn(elf, ep->e_shstrndx), NULL)) {
-		pr_warning("failed to get e_shstrndx from %s\n",
-			   obj->path);
+		pr_warning("failed to get e_shstrndx from %s\n", obj->path);
 		return -LIBBPF_ERRNO__FORMAT;
 	}
 
@@ -1340,8 +1330,7 @@ bpf_program__collect_reloc(struct bpf_program *prog, GElf_Shdr *shdr,
 	size_t nr_maps = obj->nr_maps;
 	int i, nrels;
 
-	pr_debug("collecting relocating info for: '%s'\n",
-		 prog->section_name);
+	pr_debug("collecting relocating info for: '%s'\n", prog->section_name);
 	nrels = shdr->sh_size / shdr->sh_entsize;
 
 	prog->reloc_desc = malloc(sizeof(*prog->reloc_desc) * nrels);
@@ -1366,9 +1355,7 @@ bpf_program__collect_reloc(struct bpf_program *prog, GElf_Shdr *shdr,
 			return -LIBBPF_ERRNO__FORMAT;
 		}
 
-		if (!gelf_getsym(symbols,
-				 GELF_R_SYM(rel.r_info),
-				 &sym)) {
+		if (!gelf_getsym(symbols, GELF_R_SYM(rel.r_info), &sym)) {
 			pr_warning("relocation: symbol %"PRIx64" not found\n",
 				   GELF_R_SYM(rel.r_info));
 			return -LIBBPF_ERRNO__FORMAT;
@@ -1817,18 +1804,14 @@ check_btf_ext_reloc_err(struct bpf_program *prog, int err,
 	if (btf_prog_info) {
 		/*
 		 * Some info has already been found but has problem
-		 * in the last btf_ext reloc.  Must have to error
-		 * out.
+		 * in the last btf_ext reloc. Must have to error out.
 		 */
 		pr_warning("Error in relocating %s for sec %s.\n",
 			   info_name, prog->section_name);
 		return err;
 	}
 
-	/*
-	 * Have problem loading the very first info.  Ignore
-	 * the rest.
-	 */
+	/* Have problem loading the very first info. Ignore the rest. */
 	pr_warning("Cannot find %s for main program sec %s. Ignore all %s.\n",
 		   info_name, prog->section_name, info_name);
 	return 0;
@@ -2032,9 +2015,7 @@ static int bpf_object__collect_reloc(struct bpf_object *obj)
 			return -LIBBPF_ERRNO__RELOC;
 		}
 
-		err = bpf_program__collect_reloc(prog,
-						 shdr, data,
-						 obj);
+		err = bpf_program__collect_reloc(prog, shdr, data, obj);
 		if (err)
 			return err;
 	}
@@ -2354,8 +2335,7 @@ struct bpf_object *bpf_object__open_buffer(void *obj_buf,
 			 (unsigned long)obj_buf_sz);
 		name = tmp_name;
 	}
-	pr_debug("loading object '%s' from buffer\n",
-		 name);
+	pr_debug("loading object '%s' from buffer\n", name);
 
 	return __bpf_object__open(name, obj_buf, obj_buf_sz, true, true);
 }
-- 
2.17.1


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

* Re: [PATCH bpf-next 1/9] libbpf: fix detection of corrupted BPF instructions section
  2019-05-29  1:14 ` [PATCH bpf-next 1/9] libbpf: fix detection of corrupted BPF instructions section Andrii Nakryiko
@ 2019-05-29 17:01   ` Song Liu
  2019-05-29 17:10     ` Andrii Nakryiko
  0 siblings, 1 reply; 20+ messages in thread
From: Song Liu @ 2019-05-29 17:01 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, Networking, bpf, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team

On Tue, May 28, 2019 at 6:14 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> Ensure that size of a section w/ BPF instruction is exactly a multiple
> of BPF instruction size.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>  tools/lib/bpf/libbpf.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index ca4432f5b067..05a73223e524 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -349,8 +349,11 @@ static int
>  bpf_program__init(void *data, size_t size, char *section_name, int idx,
>                   struct bpf_program *prog)
>  {
> -       if (size < sizeof(struct bpf_insn)) {
> -               pr_warning("corrupted section '%s'\n", section_name);
> +       const size_t bpf_insn_sz = sizeof(struct bpf_insn);
> +
> +       if (size < bpf_insn_sz || size % bpf_insn_sz) {

how about
           if (!size || size % bpf_insn_sz)

> +               pr_warning("corrupted section '%s', size: %zu\n",
> +                          section_name, size);
>                 return -EINVAL;
>         }
>
> @@ -376,9 +379,8 @@ bpf_program__init(void *data, size_t size, char *section_name, int idx,
>                            section_name);
>                 goto errout;
>         }
> -       prog->insns_cnt = size / sizeof(struct bpf_insn);
> -       memcpy(prog->insns, data,
> -              prog->insns_cnt * sizeof(struct bpf_insn));
> +       prog->insns_cnt = size / bpf_insn_sz;
> +       memcpy(prog->insns, data, prog->insns_cnt * bpf_insn_sz);

Given the check above, we can just use size in memcpy, right?

Thanks,
Song

>         prog->idx = idx;
>         prog->instances.fds = NULL;
>         prog->instances.nr = -1;
> --
> 2.17.1
>

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

* Re: [PATCH bpf-next 2/9] libbpf: preserve errno before calling into user callback
  2019-05-29  1:14 ` [PATCH bpf-next 2/9] libbpf: preserve errno before calling into user callback Andrii Nakryiko
@ 2019-05-29 17:03   ` Song Liu
  0 siblings, 0 replies; 20+ messages in thread
From: Song Liu @ 2019-05-29 17:03 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, Networking, bpf, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team

On Tue, May 28, 2019 at 6:14 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> pr_warning ultimately may call into user-provided callback function,
> which can clobber errno value, so we need to save it before that.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Acked-by: Song Liu <songliubraving@fb.com>


> ---
>  tools/lib/bpf/libbpf.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 05a73223e524..7b80b9ae8a1f 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -565,12 +565,12 @@ static int bpf_object__elf_init(struct bpf_object *obj)
>         } else {
>                 obj->efile.fd = open(obj->path, O_RDONLY);
>                 if (obj->efile.fd < 0) {
> -                       char errmsg[STRERR_BUFSIZE];
> -                       char *cp = libbpf_strerror_r(errno, errmsg,
> -                                                    sizeof(errmsg));
> +                       char errmsg[STRERR_BUFSIZE], *cp;
>
> +                       err = -errno;
> +                       cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg));
>                         pr_warning("failed to open %s: %s\n", obj->path, cp);
> -                       return -errno;
> +                       return err;
>                 }
>
>                 obj->efile.elf = elf_begin(obj->efile.fd,
> --
> 2.17.1
>

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

* Re: [PATCH bpf-next 3/9] libbpf: simplify endianness check
  2019-05-29  1:14 ` [PATCH bpf-next 3/9] libbpf: simplify endianness check Andrii Nakryiko
@ 2019-05-29 17:09   ` Song Liu
  0 siblings, 0 replies; 20+ messages in thread
From: Song Liu @ 2019-05-29 17:09 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, Networking, bpf, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team

On Tue, May 28, 2019 at 6:14 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> Rewrite endianness check to use "more canonical" way, using
> compiler-defined macros, similar to few other places in libbpf. It also
> is more obvious and shorter.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>

> ---
>  tools/lib/bpf/libbpf.c | 37 ++++++++++++-------------------------
>  1 file changed, 12 insertions(+), 25 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 7b80b9ae8a1f..c98f9942fba4 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -607,31 +607,18 @@ static int bpf_object__elf_init(struct bpf_object *obj)
>         return err;
>  }
>
> -static int
> -bpf_object__check_endianness(struct bpf_object *obj)
> -{
> -       static unsigned int const endian = 1;
> -
> -       switch (obj->efile.ehdr.e_ident[EI_DATA]) {
> -       case ELFDATA2LSB:
> -               /* We are big endian, BPF obj is little endian. */
> -               if (*(unsigned char const *)&endian != 1)
> -                       goto mismatch;
> -               break;
> -
> -       case ELFDATA2MSB:
> -               /* We are little endian, BPF obj is big endian. */
> -               if (*(unsigned char const *)&endian != 0)
> -                       goto mismatch;
> -               break;
> -       default:
> -               return -LIBBPF_ERRNO__ENDIAN;
> -       }
> -
> -       return 0;
> -
> -mismatch:
> -       pr_warning("Error: endianness mismatch.\n");
> +static int bpf_object__check_endianness(struct bpf_object *obj)
> +{
> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> +       if (obj->efile.ehdr.e_ident[EI_DATA] == ELFDATA2LSB)
> +               return 0;
> +#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
> +       if (obj->efile.ehdr.e_ident[EI_DATA] == ELFDATA2MSB)
> +               return 0;
> +#else
> +# error "Unrecognized __BYTE_ORDER__"
> +#endif
> +       pr_warning("endianness mismatch.\n");
>         return -LIBBPF_ERRNO__ENDIAN;
>  }
>
> --
> 2.17.1
>

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

* Re: [PATCH bpf-next 1/9] libbpf: fix detection of corrupted BPF instructions section
  2019-05-29 17:01   ` Song Liu
@ 2019-05-29 17:10     ` Andrii Nakryiko
  0 siblings, 0 replies; 20+ messages in thread
From: Andrii Nakryiko @ 2019-05-29 17:10 UTC (permalink / raw)
  To: Song Liu
  Cc: Andrii Nakryiko, Networking, bpf, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team

On Wed, May 29, 2019 at 10:01 AM Song Liu <liu.song.a23@gmail.com> wrote:
>
> On Tue, May 28, 2019 at 6:14 PM Andrii Nakryiko <andriin@fb.com> wrote:
> >
> > Ensure that size of a section w/ BPF instruction is exactly a multiple
> > of BPF instruction size.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> >  tools/lib/bpf/libbpf.c | 12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index ca4432f5b067..05a73223e524 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -349,8 +349,11 @@ static int
> >  bpf_program__init(void *data, size_t size, char *section_name, int idx,
> >                   struct bpf_program *prog)
> >  {
> > -       if (size < sizeof(struct bpf_insn)) {
> > -               pr_warning("corrupted section '%s'\n", section_name);
> > +       const size_t bpf_insn_sz = sizeof(struct bpf_insn);
> > +
> > +       if (size < bpf_insn_sz || size % bpf_insn_sz) {
>
> how about
>            if (!size || size % bpf_insn_sz)

sure, why not.

>
> > +               pr_warning("corrupted section '%s', size: %zu\n",
> > +                          section_name, size);
> >                 return -EINVAL;
> >         }
> >
> > @@ -376,9 +379,8 @@ bpf_program__init(void *data, size_t size, char *section_name, int idx,
> >                            section_name);
> >                 goto errout;
> >         }
> > -       prog->insns_cnt = size / sizeof(struct bpf_insn);
> > -       memcpy(prog->insns, data,
> > -              prog->insns_cnt * sizeof(struct bpf_insn));
> > +       prog->insns_cnt = size / bpf_insn_sz;
> > +       memcpy(prog->insns, data, prog->insns_cnt * bpf_insn_sz);
>
> Given the check above, we can just use size in memcpy, right?

yep, good point, will update

>
> Thanks,
> Song
>
> >         prog->idx = idx;
> >         prog->instances.fds = NULL;
> >         prog->instances.nr = -1;
> > --
> > 2.17.1
> >

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

* Re: [PATCH bpf-next 4/9] libbpf: check map name retrieved from ELF
  2019-05-29  1:14 ` [PATCH bpf-next 4/9] libbpf: check map name retrieved from ELF Andrii Nakryiko
@ 2019-05-29 17:11   ` Song Liu
  0 siblings, 0 replies; 20+ messages in thread
From: Song Liu @ 2019-05-29 17:11 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, Networking, bpf, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team

On Tue, May 28, 2019 at 6:15 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> Validate there was no error retrieving symbol name corresponding to
> a BPF map.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Acked-by: Song Liu <songliubraving@fb.com>


> ---
>  tools/lib/bpf/libbpf.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index c98f9942fba4..7abe71ee507a 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -920,6 +920,11 @@ bpf_object__init_maps(struct bpf_object *obj, int flags)
>                 map_name = elf_strptr(obj->efile.elf,
>                                       obj->efile.strtabidx,
>                                       sym.st_name);
> +               if (!map_name) {
> +                       pr_warning("failed to get map #%d name sym string for obj %s\n",
> +                                  map_idx, obj->path);
> +                       return -LIBBPF_ERRNO__FORMAT;
> +               }
>
>                 obj->maps[map_idx].libbpf_type = LIBBPF_MAP_UNSPEC;
>                 obj->maps[map_idx].offset = sym.st_value;
> --
> 2.17.1
>

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

* Re: [PATCH bpf-next 5/9] libbpf: fix error code returned on corrupted ELF
  2019-05-29  1:14 ` [PATCH bpf-next 5/9] libbpf: fix error code returned on corrupted ELF Andrii Nakryiko
@ 2019-05-29 17:13   ` Song Liu
  0 siblings, 0 replies; 20+ messages in thread
From: Song Liu @ 2019-05-29 17:13 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, Networking, bpf, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team

On Tue, May 28, 2019 at 6:14 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> All of libbpf errors are negative, except this one. Fix it.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>

> ---
>  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 7abe71ee507a..9c45856e7fd6 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1221,7 +1221,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj, int flags)
>
>         if (!obj->efile.strtabidx || obj->efile.strtabidx >= idx) {
>                 pr_warning("Corrupted ELF file: index of strtab invalid\n");
> -               return LIBBPF_ERRNO__FORMAT;
> +               return -LIBBPF_ERRNO__FORMAT;
>         }
>         if (btf_data) {
>                 obj->btf = btf__new(btf_data->d_buf, btf_data->d_size);
> --
> 2.17.1
>

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

* Re: [PATCH bpf-next 6/9] libbpf: use negative fd to specify missing BTF
  2019-05-29  1:14 ` [PATCH bpf-next 6/9] libbpf: use negative fd to specify missing BTF Andrii Nakryiko
@ 2019-05-29 17:23   ` Song Liu
  0 siblings, 0 replies; 20+ messages in thread
From: Song Liu @ 2019-05-29 17:23 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, Networking, bpf, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team

On Tue, May 28, 2019 at 6:14 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> 0 is a valid FD, so it's better to initialize it to -1, as is done in
> other places. Also, technically, BTF type ID 0 is valid (it's a VOID
> type), so it's more reliable to check btf_fd, instead of
> btf_key_type_id, to determine if there is any BTF associated with a map.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>

> ---
>  tools/lib/bpf/libbpf.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 9c45856e7fd6..292ea9a2dc3d 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1751,7 +1751,7 @@ bpf_object__create_maps(struct bpf_object *obj)
>                 create_attr.key_size = def->key_size;
>                 create_attr.value_size = def->value_size;
>                 create_attr.max_entries = def->max_entries;
> -               create_attr.btf_fd = 0;
> +               create_attr.btf_fd = -1;
>                 create_attr.btf_key_type_id = 0;
>                 create_attr.btf_value_type_id = 0;
>                 if (bpf_map_type__is_map_in_map(def->type) &&
> @@ -1765,11 +1765,11 @@ bpf_object__create_maps(struct bpf_object *obj)
>                 }
>
>                 *pfd = bpf_create_map_xattr(&create_attr);
> -               if (*pfd < 0 && create_attr.btf_key_type_id) {
> +               if (*pfd < 0 && create_attr.btf_fd >= 0) {
>                         cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
>                         pr_warning("Error in bpf_create_map_xattr(%s):%s(%d). Retrying without BTF.\n",
>                                    map->name, cp, errno);
> -                       create_attr.btf_fd = 0;
> +                       create_attr.btf_fd = -1;
>                         create_attr.btf_key_type_id = 0;
>                         create_attr.btf_value_type_id = 0;
>                         map->btf_key_type_id = 0;
> @@ -2053,6 +2053,9 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
>         char *log_buf;
>         int ret;
>
> +       if (!insns || !insns_cnt)
> +               return -EINVAL;
> +
>         memset(&load_attr, 0, sizeof(struct bpf_load_program_attr));
>         load_attr.prog_type = prog->type;
>         load_attr.expected_attach_type = prog->expected_attach_type;
> @@ -2063,7 +2066,7 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
>         load_attr.license = license;
>         load_attr.kern_version = kern_version;
>         load_attr.prog_ifindex = prog->prog_ifindex;
> -       load_attr.prog_btf_fd = prog->btf_fd >= 0 ? prog->btf_fd : 0;
> +       load_attr.prog_btf_fd = prog->btf_fd;
>         load_attr.func_info = prog->func_info;
>         load_attr.func_info_rec_size = prog->func_info_rec_size;
>         load_attr.func_info_cnt = prog->func_info_cnt;
> @@ -2072,8 +2075,6 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
>         load_attr.line_info_cnt = prog->line_info_cnt;
>         load_attr.log_level = prog->log_level;
>         load_attr.prog_flags = prog->prog_flags;
> -       if (!load_attr.insns || !load_attr.insns_cnt)
> -               return -EINVAL;
>
>  retry_load:
>         log_buf = malloc(log_buf_size);
> --
> 2.17.1
>

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

* Re: [PATCH bpf-next 7/9] libbpf: simplify two pieces of logic
  2019-05-29  1:14 ` [PATCH bpf-next 7/9] libbpf: simplify two pieces of logic Andrii Nakryiko
@ 2019-05-29 17:24   ` Song Liu
  0 siblings, 0 replies; 20+ messages in thread
From: Song Liu @ 2019-05-29 17:24 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, Networking, bpf, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team

On Tue, May 28, 2019 at 6:14 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> Extra check for type is unnecessary in first case.
>
> Extra zeroing is unnecessary, as snprintf guarantees that it will
> zero-terminate string.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>

> ---
>  tools/lib/bpf/libbpf.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 292ea9a2dc3d..e3bc00933145 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1430,8 +1430,7 @@ bpf_program__collect_reloc(struct bpf_program *prog, GElf_Shdr *shdr,
>                                 if (maps[map_idx].libbpf_type != type)
>                                         continue;
>                                 if (type != LIBBPF_MAP_UNSPEC ||
> -                                   (type == LIBBPF_MAP_UNSPEC &&
> -                                    maps[map_idx].offset == sym.st_value)) {
> +                                   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;
> @@ -2354,7 +2353,6 @@ struct bpf_object *bpf_object__open_buffer(void *obj_buf,
>                 snprintf(tmp_name, sizeof(tmp_name), "%lx-%lx",
>                          (unsigned long)obj_buf,
>                          (unsigned long)obj_buf_sz);
> -               tmp_name[sizeof(tmp_name) - 1] = '\0';
>                 name = tmp_name;
>         }
>         pr_debug("loading object '%s' from buffer\n",
> --
> 2.17.1
>

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

* Re: [PATCH bpf-next 8/9] libbpf: typo and formatting fixes
  2019-05-29  1:14 ` [PATCH bpf-next 8/9] libbpf: typo and formatting fixes Andrii Nakryiko
@ 2019-05-29 17:25   ` Song Liu
  0 siblings, 0 replies; 20+ messages in thread
From: Song Liu @ 2019-05-29 17:25 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, Networking, bpf, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team

On Tue, May 28, 2019 at 6:14 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> A bunch of typo and formatting fixes.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>

> ---
>  tools/lib/bpf/libbpf.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index e3bc00933145..9d9c19a1b2fe 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -505,7 +505,7 @@ static struct bpf_object *bpf_object__new(const char *path,
>
>         obj->efile.fd = -1;
>         /*
> -        * Caller of this function should also calls
> +        * Caller of this function should also call
>          * bpf_object__elf_finish() after data collection to return
>          * obj_buf to user. If not, we should duplicate the buffer to
>          * avoid user freeing them before elf finish.
> @@ -574,8 +574,7 @@ static int bpf_object__elf_init(struct bpf_object *obj)
>                 }
>
>                 obj->efile.elf = elf_begin(obj->efile.fd,
> -                               LIBBPF_ELF_C_READ_MMAP,
> -                               NULL);
> +                                          LIBBPF_ELF_C_READ_MMAP, NULL);
>         }
>
>         if (!obj->efile.elf) {
> @@ -594,9 +593,9 @@ static int bpf_object__elf_init(struct bpf_object *obj)
>         ep = &obj->efile.ehdr;
>
>         /* Old LLVM set e_machine to EM_NONE */
> -       if ((ep->e_type != ET_REL) || (ep->e_machine && (ep->e_machine != EM_BPF))) {
> -               pr_warning("%s is not an eBPF object file\n",
> -                       obj->path);
> +       if (ep->e_type != ET_REL ||
> +           (ep->e_machine && ep->e_machine != EM_BPF)) {
> +               pr_warning("%s is not an eBPF object file\n", obj->path);
>                 err = -LIBBPF_ERRNO__FORMAT;
>                 goto errout;
>         }
> @@ -1438,7 +1437,7 @@ bpf_program__collect_reloc(struct bpf_program *prog, GElf_Shdr *shdr,
>                         }
>
>                         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;
>                         }
> @@ -1797,7 +1796,7 @@ bpf_object__create_maps(struct bpf_object *obj)
>                         }
>                 }
>
> -               pr_debug("create map %s: fd=%d\n", map->name, *pfd);
> +               pr_debug("created map %s: fd=%d\n", map->name, *pfd);
>         }
>
>         return 0;
> --
> 2.17.1
>

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

* Re: [PATCH bpf-next 9/9] libbpf: reduce unnecessary line wrapping
  2019-05-29  1:14 ` [PATCH bpf-next 9/9] libbpf: reduce unnecessary line wrapping Andrii Nakryiko
@ 2019-05-29 17:27   ` Song Liu
  0 siblings, 0 replies; 20+ messages in thread
From: Song Liu @ 2019-05-29 17:27 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, Networking, bpf, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team

On Tue, May 28, 2019 at 6:14 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> There are a bunch of lines of code or comments that are unnecessary
> wrapped into multi-lines. Fix that without violating any code
> guidelines.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Acked-by: Song Liu <songliubraving@fb.com>


> ---
>  tools/lib/bpf/libbpf.c | 52 +++++++++++++-----------------------------
>  1 file changed, 16 insertions(+), 36 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 9d9c19a1b2fe..2c576843ea40 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -497,8 +497,7 @@ static struct bpf_object *bpf_object__new(const char *path,
>
>         strcpy(obj->path, path);
>         /* Using basename() GNU version which doesn't modify arg. */
> -       strncpy(obj->name, basename((void *)path),
> -               sizeof(obj->name) - 1);
> +       strncpy(obj->name, basename((void *)path), sizeof(obj->name) - 1);
>         end = strchr(obj->name, '.');
>         if (end)
>                 *end = 0;
> @@ -578,15 +577,13 @@ static int bpf_object__elf_init(struct bpf_object *obj)
>         }
>
>         if (!obj->efile.elf) {
> -               pr_warning("failed to open %s as ELF file\n",
> -                               obj->path);
> +               pr_warning("failed to open %s as ELF file\n", obj->path);
>                 err = -LIBBPF_ERRNO__LIBELF;
>                 goto errout;
>         }
>
>         if (!gelf_getehdr(obj->efile.elf, &obj->efile.ehdr)) {
> -               pr_warning("failed to get EHDR from %s\n",
> -                               obj->path);
> +               pr_warning("failed to get EHDR from %s\n", obj->path);
>                 err = -LIBBPF_ERRNO__FORMAT;
>                 goto errout;
>         }
> @@ -622,18 +619,15 @@ static int bpf_object__check_endianness(struct bpf_object *obj)
>  }
>
>  static int
> -bpf_object__init_license(struct bpf_object *obj,
> -                        void *data, size_t size)
> +bpf_object__init_license(struct bpf_object *obj, void *data, size_t size)
>  {
> -       memcpy(obj->license, data,
> -              min(size, sizeof(obj->license) - 1));
> +       memcpy(obj->license, data, min(size, sizeof(obj->license) - 1));
>         pr_debug("license of %s is %s\n", obj->path, obj->license);
>         return 0;
>  }
>
>  static int
> -bpf_object__init_kversion(struct bpf_object *obj,
> -                         void *data, size_t size)
> +bpf_object__init_kversion(struct bpf_object *obj, void *data, size_t size)
>  {
>         __u32 kver;
>
> @@ -643,8 +637,7 @@ bpf_object__init_kversion(struct bpf_object *obj,
>         }
>         memcpy(&kver, data, sizeof(kver));
>         obj->kern_version = kver;
> -       pr_debug("kernel version of %s is %x\n", obj->path,
> -                obj->kern_version);
> +       pr_debug("kernel version of %s is %x\n", obj->path, obj->kern_version);
>         return 0;
>  }
>
> @@ -800,8 +793,7 @@ bpf_object__init_internal_map(struct bpf_object *obj, struct bpf_map *map,
>         def->key_size = sizeof(int);
>         def->value_size = data->d_size;
>         def->max_entries = 1;
> -       def->map_flags = type == LIBBPF_MAP_RODATA ?
> -                        BPF_F_RDONLY_PROG : 0;
> +       def->map_flags = type == LIBBPF_MAP_RODATA ? BPF_F_RDONLY_PROG : 0;
>         if (data_buff) {
>                 *data_buff = malloc(data->d_size);
>                 if (!*data_buff) {
> @@ -816,8 +808,7 @@ bpf_object__init_internal_map(struct bpf_object *obj, struct bpf_map *map,
>         return 0;
>  }
>
> -static int
> -bpf_object__init_maps(struct bpf_object *obj, int flags)
> +static int bpf_object__init_maps(struct bpf_object *obj, int flags)
>  {
>         int i, map_idx, map_def_sz = 0, nr_syms, nr_maps = 0, nr_maps_glob = 0;
>         bool strict = !(flags & MAPS_RELAX_COMPAT);
> @@ -1098,8 +1089,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj, int flags)
>
>         /* Elf is corrupted/truncated, avoid calling elf_strptr. */
>         if (!elf_rawdata(elf_getscn(elf, ep->e_shstrndx), NULL)) {
> -               pr_warning("failed to get e_shstrndx from %s\n",
> -                          obj->path);
> +               pr_warning("failed to get e_shstrndx from %s\n", obj->path);
>                 return -LIBBPF_ERRNO__FORMAT;
>         }
>
> @@ -1340,8 +1330,7 @@ bpf_program__collect_reloc(struct bpf_program *prog, GElf_Shdr *shdr,
>         size_t nr_maps = obj->nr_maps;
>         int i, nrels;
>
> -       pr_debug("collecting relocating info for: '%s'\n",
> -                prog->section_name);
> +       pr_debug("collecting relocating info for: '%s'\n", prog->section_name);
>         nrels = shdr->sh_size / shdr->sh_entsize;
>
>         prog->reloc_desc = malloc(sizeof(*prog->reloc_desc) * nrels);
> @@ -1366,9 +1355,7 @@ bpf_program__collect_reloc(struct bpf_program *prog, GElf_Shdr *shdr,
>                         return -LIBBPF_ERRNO__FORMAT;
>                 }
>
> -               if (!gelf_getsym(symbols,
> -                                GELF_R_SYM(rel.r_info),
> -                                &sym)) {
> +               if (!gelf_getsym(symbols, GELF_R_SYM(rel.r_info), &sym)) {
>                         pr_warning("relocation: symbol %"PRIx64" not found\n",
>                                    GELF_R_SYM(rel.r_info));
>                         return -LIBBPF_ERRNO__FORMAT;
> @@ -1817,18 +1804,14 @@ check_btf_ext_reloc_err(struct bpf_program *prog, int err,
>         if (btf_prog_info) {
>                 /*
>                  * Some info has already been found but has problem
> -                * in the last btf_ext reloc.  Must have to error
> -                * out.
> +                * in the last btf_ext reloc. Must have to error out.
>                  */
>                 pr_warning("Error in relocating %s for sec %s.\n",
>                            info_name, prog->section_name);
>                 return err;
>         }
>
> -       /*
> -        * Have problem loading the very first info.  Ignore
> -        * the rest.
> -        */
> +       /* Have problem loading the very first info. Ignore the rest. */
>         pr_warning("Cannot find %s for main program sec %s. Ignore all %s.\n",
>                    info_name, prog->section_name, info_name);
>         return 0;
> @@ -2032,9 +2015,7 @@ static int bpf_object__collect_reloc(struct bpf_object *obj)
>                         return -LIBBPF_ERRNO__RELOC;
>                 }
>
> -               err = bpf_program__collect_reloc(prog,
> -                                                shdr, data,
> -                                                obj);
> +               err = bpf_program__collect_reloc(prog, shdr, data, obj);
>                 if (err)
>                         return err;
>         }
> @@ -2354,8 +2335,7 @@ struct bpf_object *bpf_object__open_buffer(void *obj_buf,
>                          (unsigned long)obj_buf_sz);
>                 name = tmp_name;
>         }
> -       pr_debug("loading object '%s' from buffer\n",
> -                name);
> +       pr_debug("loading object '%s' from buffer\n", name);
>
>         return __bpf_object__open(name, obj_buf, obj_buf_sz, true, true);
>  }
> --
> 2.17.1
>

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

end of thread, other threads:[~2019-05-29 17:27 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-29  1:14 [PATCH bpf-next 0/9] libbpf random fixes Andrii Nakryiko
2019-05-29  1:14 ` [PATCH bpf-next 1/9] libbpf: fix detection of corrupted BPF instructions section Andrii Nakryiko
2019-05-29 17:01   ` Song Liu
2019-05-29 17:10     ` Andrii Nakryiko
2019-05-29  1:14 ` [PATCH bpf-next 2/9] libbpf: preserve errno before calling into user callback Andrii Nakryiko
2019-05-29 17:03   ` Song Liu
2019-05-29  1:14 ` [PATCH bpf-next 3/9] libbpf: simplify endianness check Andrii Nakryiko
2019-05-29 17:09   ` Song Liu
2019-05-29  1:14 ` [PATCH bpf-next 4/9] libbpf: check map name retrieved from ELF Andrii Nakryiko
2019-05-29 17:11   ` Song Liu
2019-05-29  1:14 ` [PATCH bpf-next 5/9] libbpf: fix error code returned on corrupted ELF Andrii Nakryiko
2019-05-29 17:13   ` Song Liu
2019-05-29  1:14 ` [PATCH bpf-next 6/9] libbpf: use negative fd to specify missing BTF Andrii Nakryiko
2019-05-29 17:23   ` Song Liu
2019-05-29  1:14 ` [PATCH bpf-next 7/9] libbpf: simplify two pieces of logic Andrii Nakryiko
2019-05-29 17:24   ` Song Liu
2019-05-29  1:14 ` [PATCH bpf-next 8/9] libbpf: typo and formatting fixes Andrii Nakryiko
2019-05-29 17:25   ` Song Liu
2019-05-29  1:14 ` [PATCH bpf-next 9/9] libbpf: reduce unnecessary line wrapping Andrii Nakryiko
2019-05-29 17:27   ` Song Liu

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