Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1576090 > unrolled thread

[PATCH net-next v3 04/11] bpf: Use bpf_load_program() from the library

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH net-next v3 04/11] bpf: Use bpf_load_program() from the library Mickaël Salaün <mic@digikod.net> - 2017-02-07 22:50 +0100
    Re: [PATCH net-next v3 04/11] bpf: Use bpf_load_program() from the  library Arnaldo Carvalho de Melo <acme@redhat.com> - 2017-02-08 20:50 +0100
    Re: [PATCH net-next v3 04/11] bpf: Use bpf_load_program() from the  library Arnaldo Carvalho de Melo <acme@redhat.com> - 2017-02-08 21:00 +0100
      Re: [PATCH net-next v3 04/11] bpf: Use bpf_load_program() from the  library Mickaël Salaün <mic@digikod.net> - 2017-02-08 21:10 +0100

#1576090 — [PATCH net-next v3 04/11] bpf: Use bpf_load_program() from the library

Replace bpf_prog_load() with bpf_load_program() calls.

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/lib/bpf/bpf.c                         |  9 ++++-----
 tools/lib/bpf/bpf.h                         |  4 ++--
 tools/testing/selftests/bpf/Makefile        |  4 +++-
 tools/testing/selftests/bpf/bpf_sys.h       | 21 ---------------------
 tools/testing/selftests/bpf/test_tag.c      |  6 ++++--
 tools/testing/selftests/bpf/test_verifier.c |  8 +++++---
 6 files changed, 18 insertions(+), 34 deletions(-)

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 3ddb58a36d3c..e96e2a9a7742 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -42,7 +42,7 @@
 # endif
 #endif
 
-static __u64 ptr_to_u64(void *ptr)
+static __u64 ptr_to_u64(const void *ptr)
 {
 	return (__u64) (unsigned long) ptr;
 }
@@ -69,14 +69,13 @@ int bpf_create_map(enum bpf_map_type map_type, int key_size,
 	return sys_bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
 }
 
-int bpf_load_program(enum bpf_prog_type type, struct bpf_insn *insns,
-		     size_t insns_cnt, char *license,
+int bpf_load_program(enum bpf_prog_type type, const struct bpf_insn *insns,
+		     size_t insns_cnt, const char *license,
 		     __u32 kern_version, char *log_buf, size_t log_buf_sz)
 {
 	int fd;
-	union bpf_attr attr;
+	union bpf_attr attr = {};
 
-	bzero(&attr, sizeof(attr));
 	attr.prog_type = type;
 	attr.insn_cnt = (__u32)insns_cnt;
 	attr.insns = ptr_to_u64(insns);
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index a2f9853dd882..bc959a2de023 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -28,8 +28,8 @@ int bpf_create_map(enum bpf_map_type map_type, int key_size, int value_size,
 
 /* Recommend log buffer size */
 #define BPF_LOG_BUF_SIZE 65536
-int bpf_load_program(enum bpf_prog_type type, struct bpf_insn *insns,
-		     size_t insns_cnt, char *license,
+int bpf_load_program(enum bpf_prog_type type, const struct bpf_insn *insns,
+		     size_t insns_cnt, const char *license,
 		     __u32 kern_version, char *log_buf,
 		     size_t log_buf_sz);
 
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index f3d65ad53494..a35f564f66a1 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -1,4 +1,4 @@
-CFLAGS += -Wall -O2 -lcap -I../../../include/uapi
+CFLAGS += -Wall -O2 -lcap -I../../../include/uapi -I../../../lib
 
 test_objs = test_verifier test_tag test_maps test_lru_map test_lpm_map
 
@@ -7,6 +7,8 @@ TEST_FILES := $(test_objs)
 
 all: $(test_objs)
 
+$(test_objs): ../../../lib/bpf/bpf.o
+
 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 878bd60da376..3bdd1ddda6c1 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -24,6 +24,8 @@
 #include <linux/bpf_perf_event.h>
 #include <linux/bpf.h>
 
+#include <bpf/bpf.h>
+
 #include "../../../include/linux/filter.h"
 
 #include "bpf_sys.h"
@@ -4535,9 +4537,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

[toc] | [next] | [standalone]

#1576992 — Re: [PATCH net-next v3 04/11] bpf: Use bpf_load_program() from the library

Em Tue, Feb 07, 2017 at 03:17:43PM -0800, Alexei Starovoitov escreveu:
> On 2/7/17 1:44 PM, Mickaël Salaün wrote:
> >-	union bpf_attr attr;
> >+	union bpf_attr attr = {};
> >
> >-	bzero(&attr, sizeof(attr));
 
> I think somebody mentioned that there are compilers out there
> that don't do it correctly, hence it was done with explicit bzero.
> Arnaldo, Wang, do you remember the details?

Yeah, lemme dig it...

- Arnaldo

[toc] | [prev] | [next] | [standalone]

#1577003 — Re: [PATCH net-next v3 04/11] bpf: Use bpf_load_program() from the library

Em Tue, Feb 07, 2017 at 03:17:43PM -0800, Alexei Starovoitov escreveu:
> On 2/7/17 1:44 PM, Mickaël Salaün wrote:
> >-	union bpf_attr attr;
> >+	union bpf_attr attr = {};
> >
> >-	bzero(&attr, sizeof(attr));
> 
> I think somebody mentioned that there are compilers out there
> that don't do it correctly, hence it was done with explicit bzero.
> Arnaldo, Wang, do you remember the details?

https://www.spinics.net/lists/netdev/msg411144.html

But this was when some named initializers are used in a union with
unnamed members like 'union bpf_attr', unsure if this would break with
the above case where no named initializers are being used.

Having that said, the above is gratuitous, the code that is being
replaced is not related to the patch at hand, and conceptually the end
result should be the same.

So, please, just leave it as is, i.e. using bzero() and make your patch
a bit smaller, remember, small is good, smaller is even better ;-)

- Arnaldo

[toc] | [prev] | [next] | [standalone]

#1577012 — Re: [PATCH net-next v3 04/11] bpf: Use bpf_load_program() from the library

[Multipart message — attachments visible in raw view] — view raw

On 08/02/2017 20:46, Arnaldo Carvalho de Melo wrote:
> Em Tue, Feb 07, 2017 at 03:17:43PM -0800, Alexei Starovoitov escreveu:
>> On 2/7/17 1:44 PM, Mickaël Salaün wrote:
>>> -	union bpf_attr attr;
>>> +	union bpf_attr attr = {};
>>>
>>> -	bzero(&attr, sizeof(attr));
>>
>> I think somebody mentioned that there are compilers out there
>> that don't do it correctly, hence it was done with explicit bzero.
>> Arnaldo, Wang, do you remember the details?
> 
> https://www.spinics.net/lists/netdev/msg411144.html
> 
> But this was when some named initializers are used in a union with
> unnamed members like 'union bpf_attr', unsure if this would break with
> the above case where no named initializers are being used.
> 
> Having that said, the above is gratuitous, the code that is being
> replaced is not related to the patch at hand, and conceptually the end
> result should be the same.
> 
> So, please, just leave it as is, i.e. using bzero() and make your patch
> a bit smaller, remember, small is good, smaller is even better ;-)
> 
> - Arnaldo
> 

Right, I'll do it.

Thanks

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web