[PATCH net-next v1 1/7] bpf: Add missing header to the library

[PATCH net-next v1 1/7] bpf: Add missing header to the library

[Mickaël Salaün]

Including stddef.h is needed to define size_t.

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Wang Nan <wangnan0@huawei.com>
---
 tools/lib/bpf/bpf.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index a2f9853dd882..df6e186da788 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -22,6 +22,7 @@
 #define __BPF_BPF_H
 
 #include <linux/bpf.h>
+#include <stddef.h>
 
 int bpf_create_map(enum bpf_map_type map_type, int key_size, int value_size,
 		   int max_entries, __u32 map_flags);
-- 
2.11.0

[PATCH net-next v1 2/7] samples/bpf: Ignore already processed ELF sections

[Mickaël Salaün]

Add a missing check for the map fixup loop.

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
---
 samples/bpf/bpf_load.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
index 396e204888b3..e04fe09d7c2e 100644
--- a/samples/bpf/bpf_load.c
+++ b/samples/bpf/bpf_load.c
@@ -328,6 +328,8 @@ int load_bpf_file(char *path)
 
 	/* load programs that need map fixup (relocations) */
 	for (i = 1; i < ehdr.e_shnum; i++) {
+		if (processed_sec[i])
+			continue;
 
 		if (get_sec(elf, i, &ehdr, &shname, &shdr, &data))
 			continue;
-- 
2.11.0

[PATCH net-next v1 3/7] samples/bpf: Reset global variables

[Mickaël Salaün]

Before loading a new ELF, clean previous kernel version, license and
processed sections.

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
---
 samples/bpf/bpf_load.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
index e04fe09d7c2e..b86ee54da2d1 100644
--- a/samples/bpf/bpf_load.c
+++ b/samples/bpf/bpf_load.c
@@ -277,6 +277,11 @@ int load_bpf_file(char *path)
 	Elf_Data *data, *data_prog, *symbols = NULL;
 	char *shname, *shname_prog;
 
+	/* reset global variables */
+	kern_version = 0;
+	memset(license, 0, sizeof(license));
+	memset(processed_sec, 0, sizeof(processed_sec));
+
 	if (elf_version(EV_CURRENT) == EV_NONE)
 		return 1;
 
-- 
2.11.0

[PATCH net-next v1 4/7] tools: Sync {,tools/}include/uapi/linux/bpf.h

[Mickaël Salaün]

The tools version of this header is out of date; update it to the latest
version from kernel header.

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
---
 tools/include/uapi/linux/bpf.h | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 0eb0e87dbe9f..e07fd5a324e6 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -63,6 +63,12 @@ struct bpf_insn {
 	__s32	imm;		/* signed immediate constant */
 };
 
+/* Key of an a BPF_MAP_TYPE_LPM_TRIE entry */
+struct bpf_lpm_trie_key {
+	__u32	prefixlen;	/* up to 32 for AF_INET, 128 for AF_INET6 */
+	__u8	data[0];	/* Arbitrary size */
+};
+
 /* BPF syscall commands, see bpf(2) man-page for details. */
 enum bpf_cmd {
 	BPF_MAP_CREATE,
@@ -89,6 +95,7 @@ enum bpf_map_type {
 	BPF_MAP_TYPE_CGROUP_ARRAY,
 	BPF_MAP_TYPE_LRU_HASH,
 	BPF_MAP_TYPE_LRU_PERCPU_HASH,
+	BPF_MAP_TYPE_LPM_TRIE,
 };
 
 enum bpf_prog_type {
@@ -430,6 +437,18 @@ union bpf_attr {
  *     @xdp_md: pointer to xdp_md
  *     @delta: An positive/negative integer to be added to xdp_md.data
  *     Return: 0 on success or negative on error
+ *
+ * int bpf_probe_read_str(void *dst, int size, const void *unsafe_ptr)
+ *     Copy a NUL terminated string from unsafe address. In case the string
+ *     length is smaller than size, the target is not padded with further NUL
+ *     bytes. In case the string length is larger than size, just count-1
+ *     bytes are copied and the last byte is set to NUL.
+ *     @dst: destination address
+ *     @size: maximum number of bytes to copy, including the trailing NUL
+ *     @unsafe_ptr: unsafe address
+ *     Return:
+ *       > 0 length of the string including the trailing NUL on success
+ *       < 0 error
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -476,7 +495,8 @@ union bpf_attr {
 	FN(set_hash_invalid),		\
 	FN(get_numa_node_id),		\
 	FN(skb_change_head),		\
-	FN(xdp_adjust_head),
+	FN(xdp_adjust_head),		\
+	FN(probe_read_str),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
@@ -502,6 +522,7 @@ enum bpf_func_id {
 /* BPF_FUNC_l4_csum_replace flags. */
 #define BPF_F_PSEUDO_HDR		(1ULL << 4)
 #define BPF_F_MARK_MANGLED_0		(1ULL << 5)
+#define BPF_F_MARK_ENFORCE		(1ULL << 6)
 
 /* BPF_FUNC_clone_redirect and BPF_FUNC_redirect flags. */
 #define BPF_F_INGRESS			(1ULL << 0)
-- 
2.11.0

Re: [PATCH net-next v1 4/7] tools: Sync {,tools/}include/uapi/linux/bpf.h

[Mickaël Salaün]

[-- Attachment #1.1: Type: text/plain, Size: 2976 bytes --]

This patch only make sense in net-next, however.

On 06/02/2017 00:14, Mickaël Salaün wrote:
> The tools version of this header is out of date; update it to the latest
> version from kernel header.
> 
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Cc: Alexei Starovoitov <ast@fb.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: David S. Miller <davem@davemloft.net>
> ---
>  tools/include/uapi/linux/bpf.h | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 0eb0e87dbe9f..e07fd5a324e6 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -63,6 +63,12 @@ struct bpf_insn {
>  	__s32	imm;		/* signed immediate constant */
>  };
>  
> +/* Key of an a BPF_MAP_TYPE_LPM_TRIE entry */
> +struct bpf_lpm_trie_key {
> +	__u32	prefixlen;	/* up to 32 for AF_INET, 128 for AF_INET6 */
> +	__u8	data[0];	/* Arbitrary size */
> +};
> +
>  /* BPF syscall commands, see bpf(2) man-page for details. */
>  enum bpf_cmd {
>  	BPF_MAP_CREATE,
> @@ -89,6 +95,7 @@ enum bpf_map_type {
>  	BPF_MAP_TYPE_CGROUP_ARRAY,
>  	BPF_MAP_TYPE_LRU_HASH,
>  	BPF_MAP_TYPE_LRU_PERCPU_HASH,
> +	BPF_MAP_TYPE_LPM_TRIE,
>  };
>  
>  enum bpf_prog_type {
> @@ -430,6 +437,18 @@ union bpf_attr {
>   *     @xdp_md: pointer to xdp_md
>   *     @delta: An positive/negative integer to be added to xdp_md.data
>   *     Return: 0 on success or negative on error
> + *
> + * int bpf_probe_read_str(void *dst, int size, const void *unsafe_ptr)
> + *     Copy a NUL terminated string from unsafe address. In case the string
> + *     length is smaller than size, the target is not padded with further NUL
> + *     bytes. In case the string length is larger than size, just count-1
> + *     bytes are copied and the last byte is set to NUL.
> + *     @dst: destination address
> + *     @size: maximum number of bytes to copy, including the trailing NUL
> + *     @unsafe_ptr: unsafe address
> + *     Return:
> + *       > 0 length of the string including the trailing NUL on success
> + *       < 0 error
>   */
>  #define __BPF_FUNC_MAPPER(FN)		\
>  	FN(unspec),			\
> @@ -476,7 +495,8 @@ union bpf_attr {
>  	FN(set_hash_invalid),		\
>  	FN(get_numa_node_id),		\
>  	FN(skb_change_head),		\
> -	FN(xdp_adjust_head),
> +	FN(xdp_adjust_head),		\
> +	FN(probe_read_str),
>  
>  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>   * function eBPF program intends to call
> @@ -502,6 +522,7 @@ enum bpf_func_id {
>  /* BPF_FUNC_l4_csum_replace flags. */
>  #define BPF_F_PSEUDO_HDR		(1ULL << 4)
>  #define BPF_F_MARK_MANGLED_0		(1ULL << 5)
> +#define BPF_F_MARK_ENFORCE		(1ULL << 6)
>  
>  /* BPF_FUNC_clone_redirect and BPF_FUNC_redirect flags. */
>  #define BPF_F_INGRESS			(1ULL << 0)
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH net-next v1 5/7] bpf: Simplify bpf_load_program() error handling in the library

[Mickaël Salaün]

Do not call a second time bpf(2) when a program load failed.

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Wang Nan <wangnan0@huawei.com>
---
 tools/lib/bpf/bpf.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 3ddb58a36d3c..fda3f494f1cd 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -73,7 +73,6 @@ int bpf_load_program(enum bpf_prog_type type, struct bpf_insn *insns,
 		     size_t insns_cnt, char *license,
 		     __u32 kern_version, char *log_buf, size_t log_buf_sz)
 {
-	int fd;
 	union bpf_attr attr;
 
 	bzero(&attr, sizeof(attr));
@@ -81,20 +80,15 @@ int bpf_load_program(enum bpf_prog_type type, struct bpf_insn *insns,
 	attr.insn_cnt = (__u32)insns_cnt;
 	attr.insns = ptr_to_u64(insns);
 	attr.license = ptr_to_u64(license);
-	attr.log_buf = ptr_to_u64(NULL);
-	attr.log_size = 0;
-	attr.log_level = 0;
+	attr.log_buf = ptr_to_u64(log_buf);
+	attr.log_size = log_buf_sz;
 	attr.kern_version = kern_version;
 
-	fd = sys_bpf(BPF_PROG_LOAD, &attr, sizeof(attr));
-	if (fd >= 0 || !log_buf || !log_buf_sz)
-		return fd;
+	if (log_buf && log_buf_sz > 0) {
+		attr.log_level = 1;
+		log_buf[0] = 0;
+	}
 
-	/* Try again with log */
-	attr.log_buf = ptr_to_u64(log_buf);
-	attr.log_size = log_buf_sz;
-	attr.log_level = 1;
-	log_buf[0] = 0;
 	return sys_bpf(BPF_PROG_LOAD, &attr, sizeof(attr));
 }
 
-- 
2.11.0

[PATCH net-next v1 6/7] bpf: Use the bpf_load_program() from the library

[Mickaël Salaün]

Replace bpf_prog_load() with bpf_load_program() calls.

Use the tools include directory instead of the installed one to allow
builds from other kernels.

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Shuah Khan <shuah@kernel.org>
---
 tools/testing/selftests/bpf/Makefile        |  6 +++++-
 tools/testing/selftests/bpf/bpf_sys.h       | 21 ---------------------
 tools/testing/selftests/bpf/test_tag.c      |  6 ++++--
 tools/testing/selftests/bpf/test_verifier.c |  8 +++++---
 4 files changed, 14 insertions(+), 27 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 769a6cb42b4b..712861492278 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -1,12 +1,16 @@
-CFLAGS += -Wall -O2 -I../../../../usr/include
+CFLAGS += -Wall -O2 -I../../../include/uapi -I../../../lib
 
 test_objs = test_verifier test_tag test_maps test_lru_map test_lpm_map
 
 TEST_PROGS := $(test_objs) test_kmod.sh
 TEST_FILES := $(test_objs)
+LIBBPF := ../../../lib/bpf/bpf.o
 
 all: $(test_objs)
 
+test_verifier: $(LIBBPF)
+test_tag: $(LIBBPF)
+
 include ../lib.mk
 
 clean:
diff --git a/tools/testing/selftests/bpf/bpf_sys.h b/tools/testing/selftests/bpf/bpf_sys.h
index 6b4565f2a3f2..e7bbe3e5402e 100644
--- a/tools/testing/selftests/bpf/bpf_sys.h
+++ b/tools/testing/selftests/bpf/bpf_sys.h
@@ -84,25 +84,4 @@ static inline int bpf_map_create(enum bpf_map_type type, uint32_t size_key,
 	return bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
 }
 
-static inline int bpf_prog_load(enum bpf_prog_type type,
-				const struct bpf_insn *insns, size_t size_insns,
-				const char *license, char *log, size_t size_log)
-{
-	union bpf_attr attr = {};
-
-	attr.prog_type = type;
-	attr.insns = bpf_ptr_to_u64(insns);
-	attr.insn_cnt = size_insns / sizeof(struct bpf_insn);
-	attr.license = bpf_ptr_to_u64(license);
-
-	if (size_log > 0) {
-		attr.log_buf = bpf_ptr_to_u64(log);
-		attr.log_size = size_log;
-		attr.log_level = 1;
-		log[0] = 0;
-	}
-
-	return bpf(BPF_PROG_LOAD, &attr, sizeof(attr));
-}
-
 #endif /* __BPF_SYS__ */
diff --git a/tools/testing/selftests/bpf/test_tag.c b/tools/testing/selftests/bpf/test_tag.c
index 5f7c602f47d1..b77dc4b03e77 100644
--- a/tools/testing/selftests/bpf/test_tag.c
+++ b/tools/testing/selftests/bpf/test_tag.c
@@ -16,6 +16,8 @@
 #include <linux/bpf.h>
 #include <linux/if_alg.h>
 
+#include <bpf/bpf.h>
+
 #include "../../../include/linux/filter.h"
 
 #include "bpf_sys.h"
@@ -55,8 +57,8 @@ static int bpf_try_load_prog(int insns, int fd_map,
 	int fd_prog;
 
 	bpf_filler(insns, fd_map);
-	fd_prog = bpf_prog_load(BPF_PROG_TYPE_SCHED_CLS, prog, insns *
-				sizeof(struct bpf_insn), "", NULL, 0);
+	fd_prog = bpf_load_program(BPF_PROG_TYPE_SCHED_CLS, prog, insns, "", 0,
+			NULL, 0);
 	assert(fd_prog > 0);
 	if (fd_map > 0)
 		bpf_filler(insns, 0);
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 0d0912c7f03c..04a549e54f61 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -23,6 +23,8 @@
 #include <linux/bpf_perf_event.h>
 #include <linux/bpf.h>
 
+#include <bpf/bpf.h>
+
 #include "../../../include/linux/filter.h"
 
 #include "bpf_sys.h"
@@ -4456,9 +4458,9 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
 
 	do_test_fixup(test, prog, &fd_f1, &fd_f2, &fd_f3);
 
-	fd_prog = bpf_prog_load(prog_type ? : BPF_PROG_TYPE_SOCKET_FILTER,
-				prog, prog_len * sizeof(struct bpf_insn),
-				"GPL", bpf_vlog, sizeof(bpf_vlog));
+	fd_prog = bpf_load_program(prog_type ? : BPF_PROG_TYPE_SOCKET_FILTER,
+				prog, prog_len, "GPL", 0, bpf_vlog,
+				sizeof(bpf_vlog));
 
 	expected_ret = unpriv && test->result_unpriv != UNDEF ?
 		       test->result_unpriv : test->result;
-- 
2.11.0

Re: [PATCH net-next v1 6/7] bpf: Use the bpf_load_program() from the library

[Daniel Borkmann]

On 02/06/2017 12:14 AM, Mickaël Salaün wrote:
> Replace bpf_prog_load() with bpf_load_program() calls.
>
> Use the tools include directory instead of the installed one to allow
> builds from other kernels.
>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Cc: Alexei Starovoitov <ast@fb.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Shuah Khan <shuah@kernel.org>
> ---
>   tools/testing/selftests/bpf/Makefile        |  6 +++++-
>   tools/testing/selftests/bpf/bpf_sys.h       | 21 ---------------------
>   tools/testing/selftests/bpf/test_tag.c      |  6 ++++--
>   tools/testing/selftests/bpf/test_verifier.c |  8 +++++---
>   4 files changed, 14 insertions(+), 27 deletions(-)

No objections, but if so, can't we add the remaining missing
pieces to bpf lib, so we can remove bpf_sys.h altogether?

Thanks,
Daniel

Re: [PATCH net-next v1 6/7] bpf: Use the bpf_load_program() from the library

[Mickaël Salaün]

[-- Attachment #1.1: Type: text/plain, Size: 1011 bytes --]


On 06/02/2017 16:30, Daniel Borkmann wrote:
> On 02/06/2017 12:14 AM, Mickaël Salaün wrote:
>> Replace bpf_prog_load() with bpf_load_program() calls.
>>
>> Use the tools include directory instead of the installed one to allow
>> builds from other kernels.
>>
>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>> Cc: Alexei Starovoitov <ast@fb.com>
>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: Shuah Khan <shuah@kernel.org>
>> ---
>>   tools/testing/selftests/bpf/Makefile        |  6 +++++-
>>   tools/testing/selftests/bpf/bpf_sys.h       | 21 ---------------------
>>   tools/testing/selftests/bpf/test_tag.c      |  6 ++++--
>>   tools/testing/selftests/bpf/test_verifier.c |  8 +++++---
>>   4 files changed, 14 insertions(+), 27 deletions(-)
> 
> No objections, but if so, can't we add the remaining missing
> pieces to bpf lib, so we can remove bpf_sys.h altogether?
> 
> Thanks,
> Daniel
> 

OK, I'll send a new patch replacing bpf_sys.h entirely.

 Mickaël


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH net-next v1 6/7] bpf: Use the bpf_load_program() from the library

[Daniel Borkmann]

On 02/06/2017 08:16 PM, Mickaël Salaün wrote:
> On 06/02/2017 16:30, Daniel Borkmann wrote:
>> On 02/06/2017 12:14 AM, Mickaël Salaün wrote:
>>> Replace bpf_prog_load() with bpf_load_program() calls.
>>>
>>> Use the tools include directory instead of the installed one to allow
>>> builds from other kernels.
>>>
>>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>>> Cc: Alexei Starovoitov <ast@fb.com>
>>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>>> Cc: Shuah Khan <shuah@kernel.org>
>>> ---
>>>    tools/testing/selftests/bpf/Makefile        |  6 +++++-
>>>    tools/testing/selftests/bpf/bpf_sys.h       | 21 ---------------------
>>>    tools/testing/selftests/bpf/test_tag.c      |  6 ++++--
>>>    tools/testing/selftests/bpf/test_verifier.c |  8 +++++---
>>>    4 files changed, 14 insertions(+), 27 deletions(-)
>>
>> No objections, but if so, can't we add the remaining missing
>> pieces to bpf lib, so we can remove bpf_sys.h altogether?
>
> OK, I'll send a new patch replacing bpf_sys.h entirely.

Sounds great, thanks!

Re: [PATCH net-next v1 6/7] bpf: Use the bpf_load_program() from the library

[Mickaël Salaün]

[-- Attachment #1.1: Type: text/plain, Size: 1218 bytes --]


On 06/02/2017 20:18, Daniel Borkmann wrote:
> On 02/06/2017 08:16 PM, Mickaël Salaün wrote:
>> On 06/02/2017 16:30, Daniel Borkmann wrote:
>>> On 02/06/2017 12:14 AM, Mickaël Salaün wrote:
>>>> Replace bpf_prog_load() with bpf_load_program() calls.
>>>>
>>>> Use the tools include directory instead of the installed one to allow
>>>> builds from other kernels.
>>>>
>>>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>>>> Cc: Alexei Starovoitov <ast@fb.com>
>>>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>>>> Cc: Shuah Khan <shuah@kernel.org>
>>>> ---
>>>>    tools/testing/selftests/bpf/Makefile        |  6 +++++-
>>>>    tools/testing/selftests/bpf/bpf_sys.h       | 21
>>>> ---------------------
>>>>    tools/testing/selftests/bpf/test_tag.c      |  6 ++++--
>>>>    tools/testing/selftests/bpf/test_verifier.c |  8 +++++---
>>>>    4 files changed, 14 insertions(+), 27 deletions(-)
>>>
>>> No objections, but if so, can't we add the remaining missing
>>> pieces to bpf lib, so we can remove bpf_sys.h altogether?
>>
>> OK, I'll send a new patch replacing bpf_sys.h entirely.
> 
> Sounds great, thanks!
> 

Do you prefer a big patch or one for each replaced function?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH net-next v1 6/7] bpf: Use the bpf_load_program() from the library

[Daniel Borkmann]

On 02/06/2017 10:30 PM, Mickaël Salaün wrote:
> On 06/02/2017 20:18, Daniel Borkmann wrote:
>> On 02/06/2017 08:16 PM, Mickaël Salaün wrote:
>>> On 06/02/2017 16:30, Daniel Borkmann wrote:
>>>> On 02/06/2017 12:14 AM, Mickaël Salaün wrote:
>>>>> Replace bpf_prog_load() with bpf_load_program() calls.
>>>>>
>>>>> Use the tools include directory instead of the installed one to allow
>>>>> builds from other kernels.
>>>>>
>>>>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>>>>> Cc: Alexei Starovoitov <ast@fb.com>
>>>>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>>>>> Cc: Shuah Khan <shuah@kernel.org>
>>>>> ---
>>>>>     tools/testing/selftests/bpf/Makefile        |  6 +++++-
>>>>>     tools/testing/selftests/bpf/bpf_sys.h       | 21
>>>>> ---------------------
>>>>>     tools/testing/selftests/bpf/test_tag.c      |  6 ++++--
>>>>>     tools/testing/selftests/bpf/test_verifier.c |  8 +++++---
>>>>>     4 files changed, 14 insertions(+), 27 deletions(-)
>>>>
>>>> No objections, but if so, can't we add the remaining missing
>>>> pieces to bpf lib, so we can remove bpf_sys.h altogether?
>>>
>>> OK, I'll send a new patch replacing bpf_sys.h entirely.
>>
>> Sounds great, thanks!
>
> Do you prefer a big patch or one for each replaced function?

I think it makes sense to split it into two: i) this patch as-is
for the prog part, and ii) rest for maps.

Re: [PATCH net-next v1 6/7] bpf: Use the bpf_load_program() from the library

[Mickaël Salaün]

[-- Attachment #1.1: Type: text/plain, Size: 1582 bytes --]



On 06/02/2017 23:44, Daniel Borkmann wrote:
> On 02/06/2017 10:30 PM, Mickaël Salaün wrote:
>> On 06/02/2017 20:18, Daniel Borkmann wrote:
>>> On 02/06/2017 08:16 PM, Mickaël Salaün wrote:
>>>> On 06/02/2017 16:30, Daniel Borkmann wrote:
>>>>> On 02/06/2017 12:14 AM, Mickaël Salaün wrote:
>>>>>> Replace bpf_prog_load() with bpf_load_program() calls.
>>>>>>
>>>>>> Use the tools include directory instead of the installed one to allow
>>>>>> builds from other kernels.
>>>>>>
>>>>>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>>>>>> Cc: Alexei Starovoitov <ast@fb.com>
>>>>>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>>>>>> Cc: Shuah Khan <shuah@kernel.org>
>>>>>> ---
>>>>>>     tools/testing/selftests/bpf/Makefile        |  6 +++++-
>>>>>>     tools/testing/selftests/bpf/bpf_sys.h       | 21
>>>>>> ---------------------
>>>>>>     tools/testing/selftests/bpf/test_tag.c      |  6 ++++--
>>>>>>     tools/testing/selftests/bpf/test_verifier.c |  8 +++++---
>>>>>>     4 files changed, 14 insertions(+), 27 deletions(-)
>>>>>
>>>>> No objections, but if so, can't we add the remaining missing
>>>>> pieces to bpf lib, so we can remove bpf_sys.h altogether?
>>>>
>>>> OK, I'll send a new patch replacing bpf_sys.h entirely.
>>>
>>> Sounds great, thanks!
>>
>> Do you prefer a big patch or one for each replaced function?
> 
> I think it makes sense to split it into two: i) this patch as-is
> for the prog part, and ii) rest for maps.
> 

Hum, I already split them to ease the review. I'm going to send this
series now.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH net-next v1 7/7] bpf: Always test unprivileged programs

[Mickaël Salaün]

If selftests are run as root, then execute the unprivileged checks as
well. This switch from 240 to 364 tests.

The test numbers are suffixed with "/u" when executed as unprivileged or
with "/p" when executed as privileged.

The geteuid() check is replaced with a capability check.

Handling capabilities require the libcap dependency.

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Shuah Khan <shuah@kernel.org>
---
 tools/testing/selftests/bpf/Makefile        |  2 +-
 tools/testing/selftests/bpf/test_verifier.c | 68 ++++++++++++++++++++++++++---
 2 files changed, 64 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 712861492278..30bb40261692 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -1,4 +1,4 @@
-CFLAGS += -Wall -O2 -I../../../include/uapi -I../../../lib
+CFLAGS += -Wall -O2 -lcap -I../../../include/uapi -I../../../lib
 
 test_objs = test_verifier test_tag test_maps test_lru_map test_lpm_map
 
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 04a549e54f61..aa42aef22b85 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -16,6 +16,7 @@
 #include <stdbool.h>
 #include <sched.h>
 
+#include <sys/capability.h>
 #include <sys/resource.h>
 
 #include <linux/unistd.h>
@@ -4498,6 +4499,55 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
 	goto close_fds;
 }
 
+static bool is_admin(void)
+{
+	cap_t caps;
+	cap_flag_value_t sysadmin = CAP_CLEAR;
+	const cap_value_t cap_val = CAP_SYS_ADMIN;
+
+	if (!CAP_IS_SUPPORTED(CAP_SETFCAP)) {
+		perror("cap_get_flag");
+		return false;
+	}
+	caps = cap_get_proc();
+	if (!caps) {
+		perror("cap_get_proc");
+		return false;
+	}
+	if (cap_get_flag(caps, cap_val, CAP_EFFECTIVE, &sysadmin))
+		perror("cap_get_flag");
+	if (cap_free(caps) == -1)
+		perror("cap_free");
+	return (sysadmin == CAP_SET);
+}
+
+static int set_admin(bool admin)
+{
+	cap_t caps;
+	const cap_value_t cap_val = CAP_SYS_ADMIN;
+	int ret = -1;
+
+	caps = cap_get_proc();
+	if (!caps) {
+		perror("cap_get_proc");
+		return -1;
+	}
+	if (cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap_val,
+				admin ? CAP_SET : CAP_CLEAR)) {
+		perror("cap_set_flag");
+		goto out;
+	}
+	if (cap_set_proc(caps)) {
+		perror("cap_set_proc");
+		goto out;
+	}
+	ret = 0;
+out:
+	if (cap_free(caps) == -1)
+		perror("cap_free");
+	return ret;
+}
+
 static int do_test(bool unpriv, unsigned int from, unsigned int to)
 {
 	int i, passes = 0, errors = 0;
@@ -4508,11 +4558,19 @@ static int do_test(bool unpriv, unsigned int from, unsigned int to)
 		/* Program types that are not supported by non-root we
 		 * skip right away.
 		 */
-		if (unpriv && test->prog_type)
-			continue;
+		if (!test->prog_type) {
+			if (!unpriv)
+				set_admin(false);
+			printf("#%d/u %s ", i, test->descr);
+			do_test_single(test, true, &passes, &errors);
+			if (!unpriv)
+				set_admin(true);
+		}
 
-		printf("#%d %s ", i, test->descr);
-		do_test_single(test, unpriv, &passes, &errors);
+		if (!unpriv) {
+			printf("#%d/p %s ", i, test->descr);
+			do_test_single(test, false, &passes, &errors);
+		}
 	}
 
 	printf("Summary: %d PASSED, %d FAILED\n", passes, errors);
@@ -4524,7 +4582,7 @@ int main(int argc, char **argv)
 	struct rlimit rinf = { RLIM_INFINITY, RLIM_INFINITY };
 	struct rlimit rlim = { 1 << 20, 1 << 20 };
 	unsigned int from = 0, to = ARRAY_SIZE(tests);
-	bool unpriv = geteuid() != 0;
+	bool unpriv = !is_admin();
 
 	if (argc == 3) {
 		unsigned int l = atoi(argv[argc - 2]);
-- 
2.11.0

Re: [PATCH net-next v1 7/7] bpf: Always test unprivileged programs

[Daniel Borkmann]

On 02/06/2017 12:14 AM, Mickaël Salaün wrote:
> If selftests are run as root, then execute the unprivileged checks as
> well. This switch from 240 to 364 tests.
>
> The test numbers are suffixed with "/u" when executed as unprivileged or
> with "/p" when executed as privileged.
>
> The geteuid() check is replaced with a capability check.
>
> Handling capabilities require the libcap dependency.
>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Cc: Alexei Starovoitov <ast@fb.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Shuah Khan <shuah@kernel.org>

Very useful! Could probably also go as stand-alone to net-next,
but how you prefer.

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

Re: [PATCH net-next v1 7/7] bpf: Always test unprivileged programs

[Alexei Starovoitov]

On 2/5/17 3:14 PM, Mickaël Salaün wrote:
> -		if (unpriv && test->prog_type)
> -			continue;
> +		if (!test->prog_type) {
> +			if (!unpriv)
> +				set_admin(false);
> +			printf("#%d/u %s ", i, test->descr);
> +			do_test_single(test, true, &passes, &errors);
> +			if (!unpriv)
> +				set_admin(true);
> +		}
>
> -		printf("#%d %s ", i, test->descr);
> -		do_test_single(test, unpriv, &passes, &errors);
> +		if (!unpriv) {
> +			printf("#%d/p %s ", i, test->descr);
> +			do_test_single(test, false, &passes, &errors);
> +		}

great idea.
Acked-by: Alexei Starovoitov <ast@kernel.org>

as far as other patches.. we need to figure out how to avoid conflicts
between net-next and Arnaldo's tree where Joe's patches went.

Mickael,
can you see some way of splitting the patch set between trees?
Like above test_verfier.c improvement needs to go into net-next.
The rest can go via perf

Re: [PATCH net-next v1 7/7] bpf: Always test unprivileged programs

[Mickaël Salaün]

[-- Attachment #1.1: Type: text/plain, Size: 1446 bytes --]


On 06/02/2017 17:09, Alexei Starovoitov wrote:
> On 2/5/17 3:14 PM, Mickaël Salaün wrote:
>> -        if (unpriv && test->prog_type)
>> -            continue;
>> +        if (!test->prog_type) {
>> +            if (!unpriv)
>> +                set_admin(false);
>> +            printf("#%d/u %s ", i, test->descr);
>> +            do_test_single(test, true, &passes, &errors);
>> +            if (!unpriv)
>> +                set_admin(true);
>> +        }
>>
>> -        printf("#%d %s ", i, test->descr);
>> -        do_test_single(test, unpriv, &passes, &errors);
>> +        if (!unpriv) {
>> +            printf("#%d/p %s ", i, test->descr);
>> +            do_test_single(test, false, &passes, &errors);
>> +        }
> 
> great idea.
> Acked-by: Alexei Starovoitov <ast@kernel.org>
> 
> as far as other patches.. we need to figure out how to avoid conflicts
> between net-next and Arnaldo's tree where Joe's patches went.

A merge between this series and Arnaldo's tree works fine. The only
dependency is between patches 6 and 7.

> 
> Mickael,
> can you see some way of splitting the patch set between trees?
> Like above test_verfier.c improvement needs to go into net-next.
> The rest can go via perf
> 
> 

OK, I'll send a first series with the patches from 1 to 5 for the perf
tree and a second series with the 6th and 7th patches (touching
tools/testing/selftests/bpf only) to net-next.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]