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

[Mickaël Salaün]

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 
>> Cc: Alexei Starovoitov 
>> Cc: Daniel Borkmann 
>> Cc: Shuah Khan 
>> ---
>> 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.



signature.asc
Description: OpenPGP digital signature

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 
Cc: Alexei Starovoitov 
Cc: Daniel Borkmann 
Cc: Shuah Khan 
---
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]

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 
 Cc: Alexei Starovoitov 
 Cc: Daniel Borkmann 
 Cc: Shuah Khan 
 ---
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?



signature.asc
Description: OpenPGP digital signature

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 
Cc: Alexei Starovoitov 
Cc: Daniel Borkmann 
Cc: Shuah Khan 
---
   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

[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 
Cc: Alexei Starovoitov 
Cc: Daniel Borkmann 
Cc: Shuah Khan 
---
  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

[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 
Cc: Alexei Starovoitov 
Cc: Daniel Borkmann 
Cc: Shuah Khan 
---
 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, , 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, , 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 
 #include 
 
+#include 
+
 #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 
 #include 
 
+#include 
+
 #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, _f1, _f2, _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