Re: [PATCH v3 2/5] bpf: Simplify bpf_load_program() error handling in the library

2017-02-08 Thread Mickaël Salaün 

On 08/02/2017 03:35, Wangnan (F) wrote:
> 
> 
> On 2017/2/8 4:56, Mickaël Salaün wrote:
>> Do not call a second time bpf(2) when a program load failed.
> 
> BPF_PROG_LOAD should success most of the time. Setting log_level to
> 0 by default and require log buffer when failure can make it faster
> in normal case.

OK, I'm not sure if this is relevant for a test but I'm going to remove
this patch.

Thanks

> 
> Thank you.
> 
>> Signed-off-by: Mickaël Salaün 
>> Cc: Alexei Starovoitov 
>> Cc: Arnaldo Carvalho de Melo 
>> Cc: Daniel Borkmann 
>> Cc: Wang Nan 
>> ---
>>   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));
>>   }
>>   
> 
> 
> 



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v3 2/5] bpf: Simplify bpf_load_program() error handling in the library

2017-02-07 Thread Wangnan (F) 



On 2017/2/8 4:56, Mickaël Salaün wrote:

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


BPF_PROG_LOAD should success most of the time. Setting log_level to
0 by default and require log buffer when failure can make it faster
in normal case.

Thank you.


Signed-off-by: Mickaël Salaün 
Cc: Alexei Starovoitov 
Cc: Arnaldo Carvalho de Melo 
Cc: Daniel Borkmann 
Cc: Wang Nan 
---
  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));
  }

[PATCH v3 2/5] bpf: Simplify bpf_load_program() error handling in the library

2017-02-07 Thread Mickaël Salaün 
Do not call a second time bpf(2) when a program load failed.

Signed-off-by: Mickaël Salaün 
Cc: Alexei Starovoitov 
Cc: Arnaldo Carvalho de Melo 
Cc: Daniel Borkmann 
Cc: Wang Nan 
---
 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