Re: [PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-13 Thread Alexei Starovoitov
On Mon, Dec 14, 2015 at 12:39:40PM +0800, Wangnan (F) wrote:
> 
> And what do you think about the BPF function prototype? Should we put them
> into kernel headers? What about::
> +#define DEFINE_BPF_FUNC(rettype, name, arglist...) static rettype
> (*name)(arglist) = (void *)BPF_FUNC_##name

tldr: let's keep it as a part of user headers until better
solution found.

frankly
static void *(*bpf_map_lookup_elem)(void *map, void *key) =
(void *) BPF_FUNC_map_lookup_elem;
was llvm hack that I thought will be fixed quickly.
That was the easiest way to make C/llvm/bpf_loader to agree on
passing 'bpf_call #num' insn into the kernel.
It works, but it works only with -O2 and higher.
At lower optimization levels llvm generates load of constant
into register and indirect call by register, so that's not suitable
as clean api. bcc with clang::rewriter can solve it, but we don't
want to always depend on that, so currently it's a status quo.
Don't mess with what ain't broken.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-13 Thread Wangnan (F)



On 2015/12/14 12:28, Alexei Starovoitov wrote:

On Mon, Dec 14, 2015 at 11:27:36AM +0800, Wangnan (F) wrote:


On 2015/12/12 2:21, Alexei Starovoitov wrote:

On Fri, Dec 11, 2015 at 08:39:35PM +0800, pi3orama wrote:

static u64 (*bpf_ktime_get_ns)(void) =
 (void *)5;
static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
 (void *)6;
static int (*bpf_get_smp_processor_id)(void) =
 (void *)8;
static int (*bpf_perf_event_output)(void *, struct bpf_map_def *, int,
void *, unsigned long) =
 (void *)23;

Where can I get this magical mistery table? Could this be hidden away in
some .h file automagically included in bpf scriptlets so that n00bies
like me don't have to be wtf'ing?


They are function numbers defined in bpf.h and bpf-common.h, but they are Linux
headers. Directly include them causes many error for llvm. Also, the function
prototypes are BPF specific and can't included in Linux source. We should have
a place holds those indices and prototypes together.

wait, what kind of errors?
they are in uapi, so gets installed into /usr/include eventually
and I haven't seen any erros either with gcc or clang.


Sorry. I saw error because I use

#include 

It is okay if I use

#include 

then let's use that instead of copy-paste. thanks


And what do you think about the BPF function prototype? Should we put them
into kernel headers? What about::

diff --git a/include/uapi/linux/bpf_functions.h 
b/include/uapi/linux/bpf_functions.h

new file mode 100644
index 000..3a562d4
--- /dev/null
+++ b/include/uapi/linux/bpf_functions.h
@@ -0,0 +1,2 @@
+DEFINE_BPF_FUNC(void *, map_lookup_elem, void *, void *)
+DEFINE_BPF_FUNC(int, map_update_elem, void *, void *, void *, int)
[SNIP]
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 9ea2d22..2f2f05f 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -133,143 +133,23 @@ union bpf_attr {
};
 } __attribute__((aligned(8)));

+#define DEFINE_BPF_FUNC(rettype, name, arglist...) BPF_FUNC_##name
+
+enum bpf_func_id {
+BPF_FUNC_unspec,
+#include "bpf_functions.h"
+__BPF_FUNC_MAX_ID,
+};
+
+#ifdef __BPF_SOURCE__
+#undef DEFINE_BPF_FUNC
+#define DEFINE_BPF_FUNC(rettype, name, arglist...) static rettype 
(*name)(arglist) = (void *)BPF_FUNC_##name

+#include "bpf_functions.h"
+#endif
 /* integer value in 'imm' field of BPF_CALL instruction selects which 
helper

  * function eBPF program intends to call
  */
 enum bpf_func_id {
-   BPF_FUNC_unspec,
[SNIP]

And when compiling BPF source file we add a __BPF_SOURCE__ directive?

Thank you.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-13 Thread Alexei Starovoitov
On Mon, Dec 14, 2015 at 11:27:36AM +0800, Wangnan (F) wrote:
> 
> 
> On 2015/12/12 2:21, Alexei Starovoitov wrote:
> >On Fri, Dec 11, 2015 at 08:39:35PM +0800, pi3orama wrote:
> >>>static u64 (*bpf_ktime_get_ns)(void) =
> >>> (void *)5;
> >>>static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
> >>> (void *)6;
> >>>static int (*bpf_get_smp_processor_id)(void) =
> >>> (void *)8;
> >>>static int (*bpf_perf_event_output)(void *, struct bpf_map_def *, int,
> >>>void *, unsigned long) =
> >>> (void *)23;
> >>>
> >>>Where can I get this magical mistery table? Could this be hidden away in
> >>>some .h file automagically included in bpf scriptlets so that n00bies
> >>>like me don't have to be wtf'ing?
> >>>
> >>They are function numbers defined in bpf.h and bpf-common.h, but they are 
> >>Linux
> >>headers. Directly include them causes many error for llvm. Also, the 
> >>function
> >>prototypes are BPF specific and can't included in Linux source. We should 
> >>have
> >>a place holds those indices and prototypes together.
> >wait, what kind of errors?
> >they are in uapi, so gets installed into /usr/include eventually
> >and I haven't seen any erros either with gcc or clang.
> >
> Sorry. I saw error because I use
> 
> #include 
> 
> It is okay if I use
> 
> #include 

then let's use that instead of copy-paste. thanks

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-13 Thread Wangnan (F)



On 2015/12/12 2:21, Alexei Starovoitov wrote:

On Fri, Dec 11, 2015 at 08:39:35PM +0800, pi3orama wrote:

static u64 (*bpf_ktime_get_ns)(void) =
 (void *)5;
static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
 (void *)6;
static int (*bpf_get_smp_processor_id)(void) =
 (void *)8;
static int (*bpf_perf_event_output)(void *, struct bpf_map_def *, int,
void *, unsigned long) =
 (void *)23;

Where can I get this magical mistery table? Could this be hidden away in
some .h file automagically included in bpf scriptlets so that n00bies
like me don't have to be wtf'ing?


They are function numbers defined in bpf.h and bpf-common.h, but they are Linux
headers. Directly include them causes many error for llvm. Also, the function
prototypes are BPF specific and can't included in Linux source. We should have
a place holds those indices and prototypes together.

wait, what kind of errors?
they are in uapi, so gets installed into /usr/include eventually
and I haven't seen any erros either with gcc or clang.


Sorry. I saw error because I use

#include 

It is okay if I use

#include 

Thank you.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-13 Thread Alexei Starovoitov
On Mon, Dec 14, 2015 at 11:27:36AM +0800, Wangnan (F) wrote:
> 
> 
> On 2015/12/12 2:21, Alexei Starovoitov wrote:
> >On Fri, Dec 11, 2015 at 08:39:35PM +0800, pi3orama wrote:
> >>>static u64 (*bpf_ktime_get_ns)(void) =
> >>> (void *)5;
> >>>static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
> >>> (void *)6;
> >>>static int (*bpf_get_smp_processor_id)(void) =
> >>> (void *)8;
> >>>static int (*bpf_perf_event_output)(void *, struct bpf_map_def *, int,
> >>>void *, unsigned long) =
> >>> (void *)23;
> >>>
> >>>Where can I get this magical mistery table? Could this be hidden away in
> >>>some .h file automagically included in bpf scriptlets so that n00bies
> >>>like me don't have to be wtf'ing?
> >>>
> >>They are function numbers defined in bpf.h and bpf-common.h, but they are 
> >>Linux
> >>headers. Directly include them causes many error for llvm. Also, the 
> >>function
> >>prototypes are BPF specific and can't included in Linux source. We should 
> >>have
> >>a place holds those indices and prototypes together.
> >wait, what kind of errors?
> >they are in uapi, so gets installed into /usr/include eventually
> >and I haven't seen any erros either with gcc or clang.
> >
> Sorry. I saw error because I use
> 
> #include 
> 
> It is okay if I use
> 
> #include 

then let's use that instead of copy-paste. thanks

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-13 Thread Wangnan (F)



On 2015/12/14 12:28, Alexei Starovoitov wrote:

On Mon, Dec 14, 2015 at 11:27:36AM +0800, Wangnan (F) wrote:


On 2015/12/12 2:21, Alexei Starovoitov wrote:

On Fri, Dec 11, 2015 at 08:39:35PM +0800, pi3orama wrote:

static u64 (*bpf_ktime_get_ns)(void) =
 (void *)5;
static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
 (void *)6;
static int (*bpf_get_smp_processor_id)(void) =
 (void *)8;
static int (*bpf_perf_event_output)(void *, struct bpf_map_def *, int,
void *, unsigned long) =
 (void *)23;

Where can I get this magical mistery table? Could this be hidden away in
some .h file automagically included in bpf scriptlets so that n00bies
like me don't have to be wtf'ing?


They are function numbers defined in bpf.h and bpf-common.h, but they are Linux
headers. Directly include them causes many error for llvm. Also, the function
prototypes are BPF specific and can't included in Linux source. We should have
a place holds those indices and prototypes together.

wait, what kind of errors?
they are in uapi, so gets installed into /usr/include eventually
and I haven't seen any erros either with gcc or clang.


Sorry. I saw error because I use

#include 

It is okay if I use

#include 

then let's use that instead of copy-paste. thanks


And what do you think about the BPF function prototype? Should we put them
into kernel headers? What about::

diff --git a/include/uapi/linux/bpf_functions.h 
b/include/uapi/linux/bpf_functions.h

new file mode 100644
index 000..3a562d4
--- /dev/null
+++ b/include/uapi/linux/bpf_functions.h
@@ -0,0 +1,2 @@
+DEFINE_BPF_FUNC(void *, map_lookup_elem, void *, void *)
+DEFINE_BPF_FUNC(int, map_update_elem, void *, void *, void *, int)
[SNIP]
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 9ea2d22..2f2f05f 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -133,143 +133,23 @@ union bpf_attr {
};
 } __attribute__((aligned(8)));

+#define DEFINE_BPF_FUNC(rettype, name, arglist...) BPF_FUNC_##name
+
+enum bpf_func_id {
+BPF_FUNC_unspec,
+#include "bpf_functions.h"
+__BPF_FUNC_MAX_ID,
+};
+
+#ifdef __BPF_SOURCE__
+#undef DEFINE_BPF_FUNC
+#define DEFINE_BPF_FUNC(rettype, name, arglist...) static rettype 
(*name)(arglist) = (void *)BPF_FUNC_##name

+#include "bpf_functions.h"
+#endif
 /* integer value in 'imm' field of BPF_CALL instruction selects which 
helper

  * function eBPF program intends to call
  */
 enum bpf_func_id {
-   BPF_FUNC_unspec,
[SNIP]

And when compiling BPF source file we add a __BPF_SOURCE__ directive?

Thank you.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-13 Thread Alexei Starovoitov
On Mon, Dec 14, 2015 at 12:39:40PM +0800, Wangnan (F) wrote:
> 
> And what do you think about the BPF function prototype? Should we put them
> into kernel headers? What about::
> +#define DEFINE_BPF_FUNC(rettype, name, arglist...) static rettype
> (*name)(arglist) = (void *)BPF_FUNC_##name

tldr: let's keep it as a part of user headers until better
solution found.

frankly
static void *(*bpf_map_lookup_elem)(void *map, void *key) =
(void *) BPF_FUNC_map_lookup_elem;
was llvm hack that I thought will be fixed quickly.
That was the easiest way to make C/llvm/bpf_loader to agree on
passing 'bpf_call #num' insn into the kernel.
It works, but it works only with -O2 and higher.
At lower optimization levels llvm generates load of constant
into register and indirect call by register, so that's not suitable
as clean api. bcc with clang::rewriter can solve it, but we don't
want to always depend on that, so currently it's a status quo.
Don't mess with what ain't broken.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-13 Thread Wangnan (F)



On 2015/12/12 2:21, Alexei Starovoitov wrote:

On Fri, Dec 11, 2015 at 08:39:35PM +0800, pi3orama wrote:

static u64 (*bpf_ktime_get_ns)(void) =
 (void *)5;
static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
 (void *)6;
static int (*bpf_get_smp_processor_id)(void) =
 (void *)8;
static int (*bpf_perf_event_output)(void *, struct bpf_map_def *, int,
void *, unsigned long) =
 (void *)23;

Where can I get this magical mistery table? Could this be hidden away in
some .h file automagically included in bpf scriptlets so that n00bies
like me don't have to be wtf'ing?


They are function numbers defined in bpf.h and bpf-common.h, but they are Linux
headers. Directly include them causes many error for llvm. Also, the function
prototypes are BPF specific and can't included in Linux source. We should have
a place holds those indices and prototypes together.

wait, what kind of errors?
they are in uapi, so gets installed into /usr/include eventually
and I haven't seen any erros either with gcc or clang.


Sorry. I saw error because I use

#include 

It is okay if I use

#include 

Thank you.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-11 Thread Alexei Starovoitov
On Fri, Dec 11, 2015 at 08:39:35PM +0800, pi3orama wrote:
> > static u64 (*bpf_ktime_get_ns)(void) =
> > (void *)5;
> > static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
> > (void *)6;
> > static int (*bpf_get_smp_processor_id)(void) =
> > (void *)8;
> > static int (*bpf_perf_event_output)(void *, struct bpf_map_def *, int,
> > void *, unsigned long) =
> > (void *)23;
> > 
> > Where can I get this magical mistery table? Could this be hidden away in
> > some .h file automagically included in bpf scriptlets so that n00bies
> > like me don't have to be wtf'ing?
> > 
> 
> They are function numbers defined in bpf.h and bpf-common.h, but they are 
> Linux
> headers. Directly include them causes many error for llvm. Also, the function
> prototypes are BPF specific and can't included in Linux source. We should have
> a place holds those indices and prototypes together.

wait, what kind of errors?
they are in uapi, so gets installed into /usr/include eventually
and I haven't seen any erros either with gcc or clang.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-11 Thread pi3orama


发自我的 iPhone

> 在 2015年12月11日,下午8:47,Arnaldo Carvalho de Melo  写道:
> 
> Em Fri, Dec 11, 2015 at 08:39:35PM +0800, pi3orama escreveu:
>> 
>> 
>> 发自我的 iPhone
>> 
>>> 在 2015年12月11日,下午8:15,Arnaldo Carvalho de Melo  写道:
>>> 
>>> Em Fri, Dec 11, 2015 at 09:11:45AM -0300, Arnaldo Carvalho de Melo escreveu:
 Em Tue, Dec 08, 2015 at 02:25:37AM +, Wang Nan escreveu:
> This patch introduce a new syntax to perf event parser:
> 
> # perf record -e bpf_file.c/maps.mymap.value[0,3...5,7]=1234/ ...
 
 Is the above example valid? Wouldn't this be "maps:mymap.value" ?
 
> 
> By utilizing the basic facilities in bpf-loader.c which allow setting
> different slots in a BPF map separately, the newly introduced syntax
> allows perf to control specific elements in a BPF map.
> 
> Test result:
> 
> # cat ./test_bpf_map_3.c
> / BEGIN **/
> #define SEC(NAME) __attribute__((section(NAME), used))
> enum bpf_map_type {
>BPF_MAP_TYPE_ARRAY = 2,
> };
> struct bpf_map_def {
>unsigned int type;
>unsigned int key_size;
>unsigned int value_size;
>unsigned int max_entries;
> };
> static void *(*map_lookup_elem)(struct bpf_map_def *, void *) =
>(void *)1;
> static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
>(void *)6;
 
 Can you explain the above a bit more? What are the magic 1 and 6 values?
>>> 
>>> So, from another patch:
>>> 
>>> static u64 (*bpf_ktime_get_ns)(void) =
>>>(void *)5;
>>> static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
>>>(void *)6;
>>> static int (*bpf_get_smp_processor_id)(void) =
>>>(void *)8;
>>> static int (*bpf_perf_event_output)(void *, struct bpf_map_def *, int,
>>> void *, unsigned long) =
>>>(void *)23;
>>> 
>>> Where can I get this magical mistery table? Could this be hidden away in
>>> some .h file automagically included in bpf scriptlets so that n00bies
>>> like me don't have to be wtf'ing?
>> 
>> They are function numbers defined in bpf.h and bpf-common.h, but they are 
>> Linux
>> headers. Directly include them causes many error for llvm. Also, the function
>> prototypes are BPF specific and can't included in Linux source. We should 
>> have
>> a place holds those indices and prototypes together.
> 
> Sure, just please don't assume whoever is reading your patches has this
> background, provide comments above such places, so that reviewing gets
> facilitated.
> 
> I eventually figured this is some sort of trampoline to access kernel
> functions:
> 
> /* integer value in 'imm' field of BPF_CALL instruction selects which
> * helper function eBPF program intends to call
> */
> enum bpf_func_id {
>BPF_FUNC_unspec,
>BPF_FUNC_map_lookup_elem, /* void *map_lookup_elem(, ) */
>BPF_FUNC_map_update_elem, /* int map_update_elem(, , , 
> flags) */
>BPF_FUNC_map_delete_elem, /* int map_delete_elem(, ) */
>BPF_FUNC_probe_read,  /* int bpf_probe_read(void *dst, int size, 
> void *src) */
> 
> 
> But if you had just:
> 
> /*
> * See enum_bpf_func_id in ./include/uapi/linux/bpf.h
> */
> 
> That would've helped.
> 

Thank you, but I think this is a good chance to setup the policy about the 
header
files for BPF. I suggested to put BPF specific headers into Linux kernel 
include dir,
but we must find a way to avoid Linux includes them. Another useful structure is
pt_regs, however which is not as important as before because we have prologue
now.

I'd like to have a try next week.

Thank you.

> - Arnaldo

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-11 Thread Arnaldo Carvalho de Melo
Em Fri, Dec 11, 2015 at 08:39:35PM +0800, pi3orama escreveu:
> 
> 
> 发自我的 iPhone
> 
> > 在 2015年12月11日,下午8:15,Arnaldo Carvalho de Melo  写道:
> > 
> > Em Fri, Dec 11, 2015 at 09:11:45AM -0300, Arnaldo Carvalho de Melo escreveu:
> >> Em Tue, Dec 08, 2015 at 02:25:37AM +, Wang Nan escreveu:
> >>> This patch introduce a new syntax to perf event parser:
> >>> 
> >>> # perf record -e bpf_file.c/maps.mymap.value[0,3...5,7]=1234/ ...
> >> 
> >> Is the above example valid? Wouldn't this be "maps:mymap.value" ?
> >> 
> >>> 
> >>> By utilizing the basic facilities in bpf-loader.c which allow setting
> >>> different slots in a BPF map separately, the newly introduced syntax
> >>> allows perf to control specific elements in a BPF map.
> >>> 
> >>> Test result:
> >>> 
> >>> # cat ./test_bpf_map_3.c
> >>> / BEGIN **/
> >>> #define SEC(NAME) __attribute__((section(NAME), used))
> >>> enum bpf_map_type {
> >>> BPF_MAP_TYPE_ARRAY = 2,
> >>> };
> >>> struct bpf_map_def {
> >>> unsigned int type;
> >>> unsigned int key_size;
> >>> unsigned int value_size;
> >>> unsigned int max_entries;
> >>> };
> >>> static void *(*map_lookup_elem)(struct bpf_map_def *, void *) =
> >>> (void *)1;
> >>> static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
> >>> (void *)6;
> >> 
> >> Can you explain the above a bit more? What are the magic 1 and 6 values?
> > 
> > So, from another patch:
> > 
> > static u64 (*bpf_ktime_get_ns)(void) =
> > (void *)5;
> > static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
> > (void *)6;
> > static int (*bpf_get_smp_processor_id)(void) =
> > (void *)8;
> > static int (*bpf_perf_event_output)(void *, struct bpf_map_def *, int,
> > void *, unsigned long) =
> > (void *)23;
> > 
> > Where can I get this magical mistery table? Could this be hidden away in
> > some .h file automagically included in bpf scriptlets so that n00bies
> > like me don't have to be wtf'ing?
> > 
> 
> They are function numbers defined in bpf.h and bpf-common.h, but they are 
> Linux
> headers. Directly include them causes many error for llvm. Also, the function
> prototypes are BPF specific and can't included in Linux source. We should have
> a place holds those indices and prototypes together.

Sure, just please don't assume whoever is reading your patches has this
background, provide comments above such places, so that reviewing gets
facilitated.

I eventually figured this is some sort of trampoline to access kernel
functions:

/* integer value in 'imm' field of BPF_CALL instruction selects which
 * helper function eBPF program intends to call
 */
enum bpf_func_id {
BPF_FUNC_unspec,
BPF_FUNC_map_lookup_elem, /* void *map_lookup_elem(, ) */
BPF_FUNC_map_update_elem, /* int map_update_elem(, , , 
flags) */
BPF_FUNC_map_delete_elem, /* int map_delete_elem(, ) */
BPF_FUNC_probe_read,  /* int bpf_probe_read(void *dst, int size, 
void *src) */


But if you had just:

/*
 * See enum_bpf_func_id in ./include/uapi/linux/bpf.h
 */

That would've helped.

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-11 Thread pi3orama


发自我的 iPhone

> 在 2015年12月11日,下午8:15,Arnaldo Carvalho de Melo  写道:
> 
> Em Fri, Dec 11, 2015 at 09:11:45AM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Tue, Dec 08, 2015 at 02:25:37AM +, Wang Nan escreveu:
>>> This patch introduce a new syntax to perf event parser:
>>> 
>>> # perf record -e bpf_file.c/maps.mymap.value[0,3...5,7]=1234/ ...
>> 
>> Is the above example valid? Wouldn't this be "maps:mymap.value" ?
>> 
>>> 
>>> By utilizing the basic facilities in bpf-loader.c which allow setting
>>> different slots in a BPF map separately, the newly introduced syntax
>>> allows perf to control specific elements in a BPF map.
>>> 
>>> Test result:
>>> 
>>> # cat ./test_bpf_map_3.c
>>> / BEGIN **/
>>> #define SEC(NAME) __attribute__((section(NAME), used))
>>> enum bpf_map_type {
>>> BPF_MAP_TYPE_ARRAY = 2,
>>> };
>>> struct bpf_map_def {
>>> unsigned int type;
>>> unsigned int key_size;
>>> unsigned int value_size;
>>> unsigned int max_entries;
>>> };
>>> static void *(*map_lookup_elem)(struct bpf_map_def *, void *) =
>>> (void *)1;
>>> static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
>>> (void *)6;
>> 
>> Can you explain the above a bit more? What are the magic 1 and 6 values?
> 
> So, from another patch:
> 
> static u64 (*bpf_ktime_get_ns)(void) =
> (void *)5;
> static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
> (void *)6;
> static int (*bpf_get_smp_processor_id)(void) =
> (void *)8;
> static int (*bpf_perf_event_output)(void *, struct bpf_map_def *, int,
> void *, unsigned long) =
> (void *)23;
> 
> Where can I get this magical mistery table? Could this be hidden away in
> some .h file automagically included in bpf scriptlets so that n00bies
> like me don't have to be wtf'ing?
> 

They are function numbers defined in bpf.h and bpf-common.h, but they are Linux
headers. Directly include them causes many error for llvm. Also, the function
prototypes are BPF specific and can't included in Linux source. We should have
a place holds those indices and prototypes together.

> - Arnaldo

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-11 Thread Arnaldo Carvalho de Melo
Em Fri, Dec 11, 2015 at 09:11:45AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Dec 08, 2015 at 02:25:37AM +, Wang Nan escreveu:
> > This patch introduce a new syntax to perf event parser:
> > 
> >  # perf record -e bpf_file.c/maps.mymap.value[0,3...5,7]=1234/ ...
> 
> Is the above example valid? Wouldn't this be "maps:mymap.value" ?
> 
> > 
> > By utilizing the basic facilities in bpf-loader.c which allow setting
> > different slots in a BPF map separately, the newly introduced syntax
> > allows perf to control specific elements in a BPF map.
> > 
> > Test result:
> > 
> >  # cat ./test_bpf_map_3.c
> >  / BEGIN **/
> >  #define SEC(NAME) __attribute__((section(NAME), used))
> >  enum bpf_map_type {
> >  BPF_MAP_TYPE_ARRAY = 2,
> >  };
> >  struct bpf_map_def {
> >  unsigned int type;
> >  unsigned int key_size;
> >  unsigned int value_size;
> >  unsigned int max_entries;
> >  };
> >  static void *(*map_lookup_elem)(struct bpf_map_def *, void *) =
> >  (void *)1;
> >  static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
> >  (void *)6;
> 
> Can you explain the above a bit more? What are the magic 1 and 6 values?

So, from another patch:

 static u64 (*bpf_ktime_get_ns)(void) =
 (void *)5;
 static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
 (void *)6;
 static int (*bpf_get_smp_processor_id)(void) =
 (void *)8;
 static int (*bpf_perf_event_output)(void *, struct bpf_map_def *, int,
void *, unsigned long) =
 (void *)23;

Where can I get this magical mistery table? Could this be hidden away in
some .h file automagically included in bpf scriptlets so that n00bies
like me don't have to be wtf'ing?

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-11 Thread Arnaldo Carvalho de Melo
Em Tue, Dec 08, 2015 at 02:25:37AM +, Wang Nan escreveu:
> This patch introduce a new syntax to perf event parser:
> 
>  # perf record -e bpf_file.c/maps.mymap.value[0,3...5,7]=1234/ ...

Is the above example valid? Wouldn't this be "maps:mymap.value" ?

> 
> By utilizing the basic facilities in bpf-loader.c which allow setting
> different slots in a BPF map separately, the newly introduced syntax
> allows perf to control specific elements in a BPF map.
> 
> Test result:
> 
>  # cat ./test_bpf_map_3.c
>  / BEGIN **/
>  #define SEC(NAME) __attribute__((section(NAME), used))
>  enum bpf_map_type {
>  BPF_MAP_TYPE_ARRAY = 2,
>  };
>  struct bpf_map_def {
>  unsigned int type;
>  unsigned int key_size;
>  unsigned int value_size;
>  unsigned int max_entries;
>  };
>  static void *(*map_lookup_elem)(struct bpf_map_def *, void *) =
>  (void *)1;
>  static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
>  (void *)6;

Can you explain the above a bit more? What are the magic 1 and 6 values?

>  struct bpf_map_def SEC("maps") channel = {
>  .type = BPF_MAP_TYPE_ARRAY,
>  .key_size = sizeof(int),
>  .value_size = sizeof(unsigned char),
>  .max_entries = 100,
>  };

>  SEC("func=hrtimer_nanosleep rqtp->tv_nsec")
>  int func(void *ctx, int err, long nsec)
>  {
>  char fmt[] = "%ld\n";
>  long usec = nsec * 0x10624dd3 >> 38; // nsec / 1000
>  int key = (int)usec;
>  unsigned char *pval = map_lookup_elem(, );

So that "mymap.value" name doesn't needs to be specified here?

> 
>  if (!pval)
>  return 0;
>  bpf_trace_printk(fmt, sizeof(fmt), (unsigned char)*pval);
>  return 0;
>  }
>  char _license[] SEC("license") = "GPL";
>  int _version SEC("version") = LINUX_VERSION_CODE;
>  /* END ***/
> 
> Normal case:
>  # echo "" > /sys/kernel/debug/tracing/trace
>  # ./perf record -e './test_bpf_map_3.c/maps:channel.value[0,1,2,3...5]=101/' 
> usleep 2
>  [ perf record: Woken up 1 times to write data ]
>  [ perf record: Captured and wrote 0.012 MB perf.data ]
>  # cat /sys/kernel/debug/tracing/trace | grep usleep
>usleep-405   [004] d... 2745423.547822: : 101
>  # ./perf record -e 
> './test_bpf_map_3.c/maps:channel.value[0...9,20...29]=102,maps:channel.value[10...19]=103/'
>  usleep 3
>  [ perf record: Woken up 1 times to write data ]
>  [ perf record: Captured and wrote 0.012 MB perf.data ]
>  # ./perf record -e 
> './test_bpf_map_3.c/maps:channel.value[0...9,20...29]=102,maps:channel.value[10...19]=103/'
>  usleep 15
>  [ perf record: Woken up 1 times to write data ]
>  [ perf record: Captured and wrote 0.012 MB perf.data ]
>  # cat /sys/kernel/debug/tracing/trace | grep usleep
>usleep-405   [004] d... 2745423.547822: : 101
>usleep-655   [006] d... 2745434.122814: : 102
>usleep-904   [006] d... 2745439.916264: : 103
>  # ./perf record -e './test_bpf_map_3.c/maps:channel.value[all]=104/' usleep 
> 99
>  # cat /sys/kernel/debug/tracing/trace | grep usleep
>usleep-405   [004] d... 2745423.547822: : 101
>usleep-655   [006] d... 2745434.122814: : 102
>usleep-904   [006] d... 2745439.916264: : 103
>usleep-1537  [003] d... 2745538.053737: : 104
> 
> Error case:
>  # ./perf record -e './test_bpf_map_3.c/maps:channel.value[10...1000]=104/' 
> usleep 99
>  event syntax error: '..annel.value[10...1000]=104/'
>\___ Index too large
>  Hint:Valid config terms:
>   maps:[].value=[value]
>   maps:[].event=[event]
> 
>   where  is something like [0,3...5] or [all]
>   (add -v to see detail)
>  Run 'perf list' for a list of valid events
> 
>   Usage: perf record [] []
>  or: perf record [] --  []
> 
>  -e, --eventevent selector. use 'perf list' to list available 
> events
> 
> Signed-off-by: Wang Nan 
> Cc: Alexei Starovoitov 
> Cc: Arnaldo Carvalho de Melo 
> Cc: Masami Hiramatsu 
> Cc: Namhyung Kim 
> Cc: Zefan Li 
> Cc: pi3or...@163.com
> ---
>  tools/perf/util/parse-events.c |  5 ++-
>  tools/perf/util/parse-events.l | 13 ++-
>  tools/perf/util/parse-events.y | 85 
> ++
>  3 files changed, 100 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index af3d657..abdf551 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -660,9 +660,10 @@ parse_events_config_bpf(struct parse_events_evlist *data,
>sizeof(errbuf));
>   data->error->help = strdup(
>  "Hint:\tValid config terms:\n"
> -" \tmaps:[].value=[value]\n"
> -" \tmaps:[].event=[event]\n"
> +" \tmaps:[].value=[value]\n"
> +" \tmaps:[].event=[event]\n"
>  "\n"
> +" \twhere  is 

Re: [PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-11 Thread Arnaldo Carvalho de Melo
Em Fri, Dec 11, 2015 at 09:11:45AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Dec 08, 2015 at 02:25:37AM +, Wang Nan escreveu:
> > This patch introduce a new syntax to perf event parser:
> > 
> >  # perf record -e bpf_file.c/maps.mymap.value[0,3...5,7]=1234/ ...
> 
> Is the above example valid? Wouldn't this be "maps:mymap.value" ?
> 
> > 
> > By utilizing the basic facilities in bpf-loader.c which allow setting
> > different slots in a BPF map separately, the newly introduced syntax
> > allows perf to control specific elements in a BPF map.
> > 
> > Test result:
> > 
> >  # cat ./test_bpf_map_3.c
> >  / BEGIN **/
> >  #define SEC(NAME) __attribute__((section(NAME), used))
> >  enum bpf_map_type {
> >  BPF_MAP_TYPE_ARRAY = 2,
> >  };
> >  struct bpf_map_def {
> >  unsigned int type;
> >  unsigned int key_size;
> >  unsigned int value_size;
> >  unsigned int max_entries;
> >  };
> >  static void *(*map_lookup_elem)(struct bpf_map_def *, void *) =
> >  (void *)1;
> >  static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
> >  (void *)6;
> 
> Can you explain the above a bit more? What are the magic 1 and 6 values?

So, from another patch:

 static u64 (*bpf_ktime_get_ns)(void) =
 (void *)5;
 static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
 (void *)6;
 static int (*bpf_get_smp_processor_id)(void) =
 (void *)8;
 static int (*bpf_perf_event_output)(void *, struct bpf_map_def *, int,
void *, unsigned long) =
 (void *)23;

Where can I get this magical mistery table? Could this be hidden away in
some .h file automagically included in bpf scriptlets so that n00bies
like me don't have to be wtf'ing?

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-11 Thread Arnaldo Carvalho de Melo
Em Tue, Dec 08, 2015 at 02:25:37AM +, Wang Nan escreveu:
> This patch introduce a new syntax to perf event parser:
> 
>  # perf record -e bpf_file.c/maps.mymap.value[0,3...5,7]=1234/ ...

Is the above example valid? Wouldn't this be "maps:mymap.value" ?

> 
> By utilizing the basic facilities in bpf-loader.c which allow setting
> different slots in a BPF map separately, the newly introduced syntax
> allows perf to control specific elements in a BPF map.
> 
> Test result:
> 
>  # cat ./test_bpf_map_3.c
>  / BEGIN **/
>  #define SEC(NAME) __attribute__((section(NAME), used))
>  enum bpf_map_type {
>  BPF_MAP_TYPE_ARRAY = 2,
>  };
>  struct bpf_map_def {
>  unsigned int type;
>  unsigned int key_size;
>  unsigned int value_size;
>  unsigned int max_entries;
>  };
>  static void *(*map_lookup_elem)(struct bpf_map_def *, void *) =
>  (void *)1;
>  static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
>  (void *)6;

Can you explain the above a bit more? What are the magic 1 and 6 values?

>  struct bpf_map_def SEC("maps") channel = {
>  .type = BPF_MAP_TYPE_ARRAY,
>  .key_size = sizeof(int),
>  .value_size = sizeof(unsigned char),
>  .max_entries = 100,
>  };

>  SEC("func=hrtimer_nanosleep rqtp->tv_nsec")
>  int func(void *ctx, int err, long nsec)
>  {
>  char fmt[] = "%ld\n";
>  long usec = nsec * 0x10624dd3 >> 38; // nsec / 1000
>  int key = (int)usec;
>  unsigned char *pval = map_lookup_elem(, );

So that "mymap.value" name doesn't needs to be specified here?

> 
>  if (!pval)
>  return 0;
>  bpf_trace_printk(fmt, sizeof(fmt), (unsigned char)*pval);
>  return 0;
>  }
>  char _license[] SEC("license") = "GPL";
>  int _version SEC("version") = LINUX_VERSION_CODE;
>  /* END ***/
> 
> Normal case:
>  # echo "" > /sys/kernel/debug/tracing/trace
>  # ./perf record -e './test_bpf_map_3.c/maps:channel.value[0,1,2,3...5]=101/' 
> usleep 2
>  [ perf record: Woken up 1 times to write data ]
>  [ perf record: Captured and wrote 0.012 MB perf.data ]
>  # cat /sys/kernel/debug/tracing/trace | grep usleep
>usleep-405   [004] d... 2745423.547822: : 101
>  # ./perf record -e 
> './test_bpf_map_3.c/maps:channel.value[0...9,20...29]=102,maps:channel.value[10...19]=103/'
>  usleep 3
>  [ perf record: Woken up 1 times to write data ]
>  [ perf record: Captured and wrote 0.012 MB perf.data ]
>  # ./perf record -e 
> './test_bpf_map_3.c/maps:channel.value[0...9,20...29]=102,maps:channel.value[10...19]=103/'
>  usleep 15
>  [ perf record: Woken up 1 times to write data ]
>  [ perf record: Captured and wrote 0.012 MB perf.data ]
>  # cat /sys/kernel/debug/tracing/trace | grep usleep
>usleep-405   [004] d... 2745423.547822: : 101
>usleep-655   [006] d... 2745434.122814: : 102
>usleep-904   [006] d... 2745439.916264: : 103
>  # ./perf record -e './test_bpf_map_3.c/maps:channel.value[all]=104/' usleep 
> 99
>  # cat /sys/kernel/debug/tracing/trace | grep usleep
>usleep-405   [004] d... 2745423.547822: : 101
>usleep-655   [006] d... 2745434.122814: : 102
>usleep-904   [006] d... 2745439.916264: : 103
>usleep-1537  [003] d... 2745538.053737: : 104
> 
> Error case:
>  # ./perf record -e './test_bpf_map_3.c/maps:channel.value[10...1000]=104/' 
> usleep 99
>  event syntax error: '..annel.value[10...1000]=104/'
>\___ Index too large
>  Hint:Valid config terms:
>   maps:[].value=[value]
>   maps:[].event=[event]
> 
>   where  is something like [0,3...5] or [all]
>   (add -v to see detail)
>  Run 'perf list' for a list of valid events
> 
>   Usage: perf record [] []
>  or: perf record [] --  []
> 
>  -e, --eventevent selector. use 'perf list' to list available 
> events
> 
> Signed-off-by: Wang Nan 
> Cc: Alexei Starovoitov 
> Cc: Arnaldo Carvalho de Melo 
> Cc: Masami Hiramatsu 
> Cc: Namhyung Kim 
> Cc: Zefan Li 
> Cc: pi3or...@163.com
> ---
>  tools/perf/util/parse-events.c |  5 ++-
>  tools/perf/util/parse-events.l | 13 ++-
>  tools/perf/util/parse-events.y | 85 
> ++
>  3 files changed, 100 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index af3d657..abdf551 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -660,9 +660,10 @@ parse_events_config_bpf(struct parse_events_evlist *data,
>sizeof(errbuf));
>   data->error->help = strdup(
>  "Hint:\tValid config terms:\n"
> -" \tmaps:[].value=[value]\n"
> -" 

Re: [PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-11 Thread pi3orama


发自我的 iPhone

> 在 2015年12月11日,下午8:15,Arnaldo Carvalho de Melo  写道:
> 
> Em Fri, Dec 11, 2015 at 09:11:45AM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Tue, Dec 08, 2015 at 02:25:37AM +, Wang Nan escreveu:
>>> This patch introduce a new syntax to perf event parser:
>>> 
>>> # perf record -e bpf_file.c/maps.mymap.value[0,3...5,7]=1234/ ...
>> 
>> Is the above example valid? Wouldn't this be "maps:mymap.value" ?
>> 
>>> 
>>> By utilizing the basic facilities in bpf-loader.c which allow setting
>>> different slots in a BPF map separately, the newly introduced syntax
>>> allows perf to control specific elements in a BPF map.
>>> 
>>> Test result:
>>> 
>>> # cat ./test_bpf_map_3.c
>>> / BEGIN **/
>>> #define SEC(NAME) __attribute__((section(NAME), used))
>>> enum bpf_map_type {
>>> BPF_MAP_TYPE_ARRAY = 2,
>>> };
>>> struct bpf_map_def {
>>> unsigned int type;
>>> unsigned int key_size;
>>> unsigned int value_size;
>>> unsigned int max_entries;
>>> };
>>> static void *(*map_lookup_elem)(struct bpf_map_def *, void *) =
>>> (void *)1;
>>> static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
>>> (void *)6;
>> 
>> Can you explain the above a bit more? What are the magic 1 and 6 values?
> 
> So, from another patch:
> 
> static u64 (*bpf_ktime_get_ns)(void) =
> (void *)5;
> static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
> (void *)6;
> static int (*bpf_get_smp_processor_id)(void) =
> (void *)8;
> static int (*bpf_perf_event_output)(void *, struct bpf_map_def *, int,
> void *, unsigned long) =
> (void *)23;
> 
> Where can I get this magical mistery table? Could this be hidden away in
> some .h file automagically included in bpf scriptlets so that n00bies
> like me don't have to be wtf'ing?
> 

They are function numbers defined in bpf.h and bpf-common.h, but they are Linux
headers. Directly include them causes many error for llvm. Also, the function
prototypes are BPF specific and can't included in Linux source. We should have
a place holds those indices and prototypes together.

> - Arnaldo

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-11 Thread pi3orama


发自我的 iPhone

> 在 2015年12月11日,下午8:47,Arnaldo Carvalho de Melo  写道:
> 
> Em Fri, Dec 11, 2015 at 08:39:35PM +0800, pi3orama escreveu:
>> 
>> 
>> 发自我的 iPhone
>> 
>>> 在 2015年12月11日,下午8:15,Arnaldo Carvalho de Melo  写道:
>>> 
>>> Em Fri, Dec 11, 2015 at 09:11:45AM -0300, Arnaldo Carvalho de Melo escreveu:
 Em Tue, Dec 08, 2015 at 02:25:37AM +, Wang Nan escreveu:
> This patch introduce a new syntax to perf event parser:
> 
> # perf record -e bpf_file.c/maps.mymap.value[0,3...5,7]=1234/ ...
 
 Is the above example valid? Wouldn't this be "maps:mymap.value" ?
 
> 
> By utilizing the basic facilities in bpf-loader.c which allow setting
> different slots in a BPF map separately, the newly introduced syntax
> allows perf to control specific elements in a BPF map.
> 
> Test result:
> 
> # cat ./test_bpf_map_3.c
> / BEGIN **/
> #define SEC(NAME) __attribute__((section(NAME), used))
> enum bpf_map_type {
>BPF_MAP_TYPE_ARRAY = 2,
> };
> struct bpf_map_def {
>unsigned int type;
>unsigned int key_size;
>unsigned int value_size;
>unsigned int max_entries;
> };
> static void *(*map_lookup_elem)(struct bpf_map_def *, void *) =
>(void *)1;
> static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
>(void *)6;
 
 Can you explain the above a bit more? What are the magic 1 and 6 values?
>>> 
>>> So, from another patch:
>>> 
>>> static u64 (*bpf_ktime_get_ns)(void) =
>>>(void *)5;
>>> static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
>>>(void *)6;
>>> static int (*bpf_get_smp_processor_id)(void) =
>>>(void *)8;
>>> static int (*bpf_perf_event_output)(void *, struct bpf_map_def *, int,
>>> void *, unsigned long) =
>>>(void *)23;
>>> 
>>> Where can I get this magical mistery table? Could this be hidden away in
>>> some .h file automagically included in bpf scriptlets so that n00bies
>>> like me don't have to be wtf'ing?
>> 
>> They are function numbers defined in bpf.h and bpf-common.h, but they are 
>> Linux
>> headers. Directly include them causes many error for llvm. Also, the function
>> prototypes are BPF specific and can't included in Linux source. We should 
>> have
>> a place holds those indices and prototypes together.
> 
> Sure, just please don't assume whoever is reading your patches has this
> background, provide comments above such places, so that reviewing gets
> facilitated.
> 
> I eventually figured this is some sort of trampoline to access kernel
> functions:
> 
> /* integer value in 'imm' field of BPF_CALL instruction selects which
> * helper function eBPF program intends to call
> */
> enum bpf_func_id {
>BPF_FUNC_unspec,
>BPF_FUNC_map_lookup_elem, /* void *map_lookup_elem(, ) */
>BPF_FUNC_map_update_elem, /* int map_update_elem(, , , 
> flags) */
>BPF_FUNC_map_delete_elem, /* int map_delete_elem(, ) */
>BPF_FUNC_probe_read,  /* int bpf_probe_read(void *dst, int size, 
> void *src) */
> 
> 
> But if you had just:
> 
> /*
> * See enum_bpf_func_id in ./include/uapi/linux/bpf.h
> */
> 
> That would've helped.
> 

Thank you, but I think this is a good chance to setup the policy about the 
header
files for BPF. I suggested to put BPF specific headers into Linux kernel 
include dir,
but we must find a way to avoid Linux includes them. Another useful structure is
pt_regs, however which is not as important as before because we have prologue
now.

I'd like to have a try next week.

Thank you.

> - Arnaldo

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-11 Thread Arnaldo Carvalho de Melo
Em Fri, Dec 11, 2015 at 08:39:35PM +0800, pi3orama escreveu:
> 
> 
> 发自我的 iPhone
> 
> > 在 2015年12月11日,下午8:15,Arnaldo Carvalho de Melo  写道:
> > 
> > Em Fri, Dec 11, 2015 at 09:11:45AM -0300, Arnaldo Carvalho de Melo escreveu:
> >> Em Tue, Dec 08, 2015 at 02:25:37AM +, Wang Nan escreveu:
> >>> This patch introduce a new syntax to perf event parser:
> >>> 
> >>> # perf record -e bpf_file.c/maps.mymap.value[0,3...5,7]=1234/ ...
> >> 
> >> Is the above example valid? Wouldn't this be "maps:mymap.value" ?
> >> 
> >>> 
> >>> By utilizing the basic facilities in bpf-loader.c which allow setting
> >>> different slots in a BPF map separately, the newly introduced syntax
> >>> allows perf to control specific elements in a BPF map.
> >>> 
> >>> Test result:
> >>> 
> >>> # cat ./test_bpf_map_3.c
> >>> / BEGIN **/
> >>> #define SEC(NAME) __attribute__((section(NAME), used))
> >>> enum bpf_map_type {
> >>> BPF_MAP_TYPE_ARRAY = 2,
> >>> };
> >>> struct bpf_map_def {
> >>> unsigned int type;
> >>> unsigned int key_size;
> >>> unsigned int value_size;
> >>> unsigned int max_entries;
> >>> };
> >>> static void *(*map_lookup_elem)(struct bpf_map_def *, void *) =
> >>> (void *)1;
> >>> static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
> >>> (void *)6;
> >> 
> >> Can you explain the above a bit more? What are the magic 1 and 6 values?
> > 
> > So, from another patch:
> > 
> > static u64 (*bpf_ktime_get_ns)(void) =
> > (void *)5;
> > static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
> > (void *)6;
> > static int (*bpf_get_smp_processor_id)(void) =
> > (void *)8;
> > static int (*bpf_perf_event_output)(void *, struct bpf_map_def *, int,
> > void *, unsigned long) =
> > (void *)23;
> > 
> > Where can I get this magical mistery table? Could this be hidden away in
> > some .h file automagically included in bpf scriptlets so that n00bies
> > like me don't have to be wtf'ing?
> > 
> 
> They are function numbers defined in bpf.h and bpf-common.h, but they are 
> Linux
> headers. Directly include them causes many error for llvm. Also, the function
> prototypes are BPF specific and can't included in Linux source. We should have
> a place holds those indices and prototypes together.

Sure, just please don't assume whoever is reading your patches has this
background, provide comments above such places, so that reviewing gets
facilitated.

I eventually figured this is some sort of trampoline to access kernel
functions:

/* integer value in 'imm' field of BPF_CALL instruction selects which
 * helper function eBPF program intends to call
 */
enum bpf_func_id {
BPF_FUNC_unspec,
BPF_FUNC_map_lookup_elem, /* void *map_lookup_elem(, ) */
BPF_FUNC_map_update_elem, /* int map_update_elem(, , , 
flags) */
BPF_FUNC_map_delete_elem, /* int map_delete_elem(, ) */
BPF_FUNC_probe_read,  /* int bpf_probe_read(void *dst, int size, 
void *src) */


But if you had just:

/*
 * See enum_bpf_func_id in ./include/uapi/linux/bpf.h
 */

That would've helped.

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-11 Thread Alexei Starovoitov
On Fri, Dec 11, 2015 at 08:39:35PM +0800, pi3orama wrote:
> > static u64 (*bpf_ktime_get_ns)(void) =
> > (void *)5;
> > static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
> > (void *)6;
> > static int (*bpf_get_smp_processor_id)(void) =
> > (void *)8;
> > static int (*bpf_perf_event_output)(void *, struct bpf_map_def *, int,
> > void *, unsigned long) =
> > (void *)23;
> > 
> > Where can I get this magical mistery table? Could this be hidden away in
> > some .h file automagically included in bpf scriptlets so that n00bies
> > like me don't have to be wtf'ing?
> > 
> 
> They are function numbers defined in bpf.h and bpf-common.h, but they are 
> Linux
> headers. Directly include them causes many error for llvm. Also, the function
> prototypes are BPF specific and can't included in Linux source. We should have
> a place holds those indices and prototypes together.

wait, what kind of errors?
they are in uapi, so gets installed into /usr/include eventually
and I haven't seen any erros either with gcc or clang.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-07 Thread Wang Nan
This patch introduce a new syntax to perf event parser:

 # perf record -e bpf_file.c/maps.mymap.value[0,3...5,7]=1234/ ...

By utilizing the basic facilities in bpf-loader.c which allow setting
different slots in a BPF map separately, the newly introduced syntax
allows perf to control specific elements in a BPF map.

Test result:

 # cat ./test_bpf_map_3.c
 / BEGIN **/
 #define SEC(NAME) __attribute__((section(NAME), used))
 enum bpf_map_type {
 BPF_MAP_TYPE_ARRAY = 2,
 };
 struct bpf_map_def {
 unsigned int type;
 unsigned int key_size;
 unsigned int value_size;
 unsigned int max_entries;
 };
 static void *(*map_lookup_elem)(struct bpf_map_def *, void *) =
 (void *)1;
 static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
 (void *)6;
 struct bpf_map_def SEC("maps") channel = {
 .type = BPF_MAP_TYPE_ARRAY,
 .key_size = sizeof(int),
 .value_size = sizeof(unsigned char),
 .max_entries = 100,
 };
 SEC("func=hrtimer_nanosleep rqtp->tv_nsec")
 int func(void *ctx, int err, long nsec)
 {
 char fmt[] = "%ld\n";
 long usec = nsec * 0x10624dd3 >> 38; // nsec / 1000
 int key = (int)usec;
 unsigned char *pval = map_lookup_elem(, );

 if (!pval)
 return 0;
 bpf_trace_printk(fmt, sizeof(fmt), (unsigned char)*pval);
 return 0;
 }
 char _license[] SEC("license") = "GPL";
 int _version SEC("version") = LINUX_VERSION_CODE;
 /* END ***/

Normal case:
 # echo "" > /sys/kernel/debug/tracing/trace
 # ./perf record -e './test_bpf_map_3.c/maps:channel.value[0,1,2,3...5]=101/' 
usleep 2
 [ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.012 MB perf.data ]
 # cat /sys/kernel/debug/tracing/trace | grep usleep
   usleep-405   [004] d... 2745423.547822: : 101
 # ./perf record -e 
'./test_bpf_map_3.c/maps:channel.value[0...9,20...29]=102,maps:channel.value[10...19]=103/'
 usleep 3
 [ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.012 MB perf.data ]
 # ./perf record -e 
'./test_bpf_map_3.c/maps:channel.value[0...9,20...29]=102,maps:channel.value[10...19]=103/'
 usleep 15
 [ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.012 MB perf.data ]
 # cat /sys/kernel/debug/tracing/trace | grep usleep
   usleep-405   [004] d... 2745423.547822: : 101
   usleep-655   [006] d... 2745434.122814: : 102
   usleep-904   [006] d... 2745439.916264: : 103
 # ./perf record -e './test_bpf_map_3.c/maps:channel.value[all]=104/' usleep 99
 # cat /sys/kernel/debug/tracing/trace | grep usleep
   usleep-405   [004] d... 2745423.547822: : 101
   usleep-655   [006] d... 2745434.122814: : 102
   usleep-904   [006] d... 2745439.916264: : 103
   usleep-1537  [003] d... 2745538.053737: : 104

Error case:
 # ./perf record -e './test_bpf_map_3.c/maps:channel.value[10...1000]=104/' 
usleep 99
 event syntax error: '..annel.value[10...1000]=104/'
   \___ Index too large
 Hint:  Valid config terms:
maps:[].value=[value]
maps:[].event=[event]

where  is something like [0,3...5] or [all]
(add -v to see detail)
 Run 'perf list' for a list of valid events

  Usage: perf record [] []
 or: perf record [] --  []

 -e, --eventevent selector. use 'perf list' to list available 
events

Signed-off-by: Wang Nan 
Cc: Alexei Starovoitov 
Cc: Arnaldo Carvalho de Melo 
Cc: Masami Hiramatsu 
Cc: Namhyung Kim 
Cc: Zefan Li 
Cc: pi3or...@163.com
---
 tools/perf/util/parse-events.c |  5 ++-
 tools/perf/util/parse-events.l | 13 ++-
 tools/perf/util/parse-events.y | 85 ++
 3 files changed, 100 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index af3d657..abdf551 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -660,9 +660,10 @@ parse_events_config_bpf(struct parse_events_evlist *data,
 sizeof(errbuf));
data->error->help = strdup(
 "Hint:\tValid config terms:\n"
-" \tmaps:[].value=[value]\n"
-" \tmaps:[].event=[event]\n"
+" \tmaps:[].value=[value]\n"
+" \tmaps:[].event=[event]\n"
 "\n"
+" \twhere  is something like [0,3...5] or [all]\n"
 " \t(add -v to see detail)");
data->error->str = strdup(errbuf);
if (err == -BPF_LOADER_ERRNO__OBJCONF_MAP_VALUE)
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 4387728..8bb3437 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -9,8 +9,8 @@
 %{
 #include 
 #include "../perf.h"
-#include "parse-events-bison.h"
 #include "parse-events.h"
+#include "parse-events-bison.h"
 
 char 

[PATCH v4 09/16] perf tools: Enable indices setting syntax for BPF maps

2015-12-07 Thread Wang Nan
This patch introduce a new syntax to perf event parser:

 # perf record -e bpf_file.c/maps.mymap.value[0,3...5,7]=1234/ ...

By utilizing the basic facilities in bpf-loader.c which allow setting
different slots in a BPF map separately, the newly introduced syntax
allows perf to control specific elements in a BPF map.

Test result:

 # cat ./test_bpf_map_3.c
 / BEGIN **/
 #define SEC(NAME) __attribute__((section(NAME), used))
 enum bpf_map_type {
 BPF_MAP_TYPE_ARRAY = 2,
 };
 struct bpf_map_def {
 unsigned int type;
 unsigned int key_size;
 unsigned int value_size;
 unsigned int max_entries;
 };
 static void *(*map_lookup_elem)(struct bpf_map_def *, void *) =
 (void *)1;
 static int (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
 (void *)6;
 struct bpf_map_def SEC("maps") channel = {
 .type = BPF_MAP_TYPE_ARRAY,
 .key_size = sizeof(int),
 .value_size = sizeof(unsigned char),
 .max_entries = 100,
 };
 SEC("func=hrtimer_nanosleep rqtp->tv_nsec")
 int func(void *ctx, int err, long nsec)
 {
 char fmt[] = "%ld\n";
 long usec = nsec * 0x10624dd3 >> 38; // nsec / 1000
 int key = (int)usec;
 unsigned char *pval = map_lookup_elem(, );

 if (!pval)
 return 0;
 bpf_trace_printk(fmt, sizeof(fmt), (unsigned char)*pval);
 return 0;
 }
 char _license[] SEC("license") = "GPL";
 int _version SEC("version") = LINUX_VERSION_CODE;
 /* END ***/

Normal case:
 # echo "" > /sys/kernel/debug/tracing/trace
 # ./perf record -e './test_bpf_map_3.c/maps:channel.value[0,1,2,3...5]=101/' 
usleep 2
 [ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.012 MB perf.data ]
 # cat /sys/kernel/debug/tracing/trace | grep usleep
   usleep-405   [004] d... 2745423.547822: : 101
 # ./perf record -e 
'./test_bpf_map_3.c/maps:channel.value[0...9,20...29]=102,maps:channel.value[10...19]=103/'
 usleep 3
 [ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.012 MB perf.data ]
 # ./perf record -e 
'./test_bpf_map_3.c/maps:channel.value[0...9,20...29]=102,maps:channel.value[10...19]=103/'
 usleep 15
 [ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.012 MB perf.data ]
 # cat /sys/kernel/debug/tracing/trace | grep usleep
   usleep-405   [004] d... 2745423.547822: : 101
   usleep-655   [006] d... 2745434.122814: : 102
   usleep-904   [006] d... 2745439.916264: : 103
 # ./perf record -e './test_bpf_map_3.c/maps:channel.value[all]=104/' usleep 99
 # cat /sys/kernel/debug/tracing/trace | grep usleep
   usleep-405   [004] d... 2745423.547822: : 101
   usleep-655   [006] d... 2745434.122814: : 102
   usleep-904   [006] d... 2745439.916264: : 103
   usleep-1537  [003] d... 2745538.053737: : 104

Error case:
 # ./perf record -e './test_bpf_map_3.c/maps:channel.value[10...1000]=104/' 
usleep 99
 event syntax error: '..annel.value[10...1000]=104/'
   \___ Index too large
 Hint:  Valid config terms:
maps:[].value=[value]
maps:[].event=[event]

where  is something like [0,3...5] or [all]
(add -v to see detail)
 Run 'perf list' for a list of valid events

  Usage: perf record [] []
 or: perf record [] --  []

 -e, --eventevent selector. use 'perf list' to list available 
events

Signed-off-by: Wang Nan 
Cc: Alexei Starovoitov 
Cc: Arnaldo Carvalho de Melo 
Cc: Masami Hiramatsu 
Cc: Namhyung Kim 
Cc: Zefan Li 
Cc: pi3or...@163.com
---
 tools/perf/util/parse-events.c |  5 ++-
 tools/perf/util/parse-events.l | 13 ++-
 tools/perf/util/parse-events.y | 85 ++
 3 files changed, 100 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index af3d657..abdf551 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -660,9 +660,10 @@ parse_events_config_bpf(struct parse_events_evlist *data,
 sizeof(errbuf));
data->error->help = strdup(
 "Hint:\tValid config terms:\n"
-" \tmaps:[].value=[value]\n"
-" \tmaps:[].event=[event]\n"
+" \tmaps:[].value=[value]\n"
+" \tmaps:[].event=[event]\n"
 "\n"
+" \twhere  is something like [0,3...5] or [all]\n"
 " \t(add -v to see detail)");
data->error->str = strdup(errbuf);
if (err == -BPF_LOADER_ERRNO__OBJCONF_MAP_VALUE)
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 4387728..8bb3437 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -9,8 +9,8 @@
 %{
 #include