* [PATCH v2 1/4] bpf: clarify description for CONFIG_BPF_EVENTS
2019-08-20 23:08 [PATCH v2 0/4] BPF-related documentation fixes Peter Wu
@ 2019-08-20 23:08 ` Peter Wu
2019-08-20 23:08 ` [PATCH v2 2/4] bpf: fix 'struct pt_reg' typo in documentation Peter Wu
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Peter Wu @ 2019-08-20 23:08 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, bpf
PERF_EVENT_IOC_SET_BPF supports uprobes since v4.3, and tracepoints
since v4.7 via commit 04a22fae4cbc ("tracing, perf: Implement BPF
programs attached to uprobes"), and commit 98b5c2c65c29 ("perf, bpf:
allow bpf programs attach to tracepoints") respectively.
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
---
kernel/trace/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 98da8998c25c..b09d7b1ffffd 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -520,7 +520,8 @@ config BPF_EVENTS
bool
default y
help
- This allows the user to attach BPF programs to kprobe events.
+ This allows the user to attach BPF programs to kprobe, uprobe, and
+ tracepoint events.
config DYNAMIC_EVENTS
def_bool n
--
2.22.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 2/4] bpf: fix 'struct pt_reg' typo in documentation
2019-08-20 23:08 [PATCH v2 0/4] BPF-related documentation fixes Peter Wu
2019-08-20 23:08 ` [PATCH v2 1/4] bpf: clarify description for CONFIG_BPF_EVENTS Peter Wu
@ 2019-08-20 23:08 ` Peter Wu
2019-08-21 10:28 ` Quentin Monnet
2019-08-20 23:08 ` [PATCH v2 3/4] bpf: clarify when bpf_trace_printk discards lines Peter Wu
2019-08-20 23:09 ` [PATCH v2 4/4] bpf: sync bpf.h to tools/ Peter Wu
3 siblings, 1 reply; 9+ messages in thread
From: Peter Wu @ 2019-08-20 23:08 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, bpf
There is no 'struct pt_reg'.
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
---
include/uapi/linux/bpf.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index fa1c753dcdbc..9ca333c3ce91 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1013,7 +1013,7 @@ union bpf_attr {
* The realm of the route for the packet associated to *skb*, or 0
* if none was found.
*
- * int bpf_perf_event_output(struct pt_reg *ctx, struct bpf_map *map, u64 flags, void *data, u64 size)
+ * int bpf_perf_event_output(struct pt_regs *ctx, struct bpf_map *map, u64 flags, void *data, u64 size)
* Description
* Write raw *data* blob into a special BPF perf event held by
* *map* of type **BPF_MAP_TYPE_PERF_EVENT_ARRAY**. This perf
@@ -1075,7 +1075,7 @@ union bpf_attr {
* Return
* 0 on success, or a negative error in case of failure.
*
- * int bpf_get_stackid(struct pt_reg *ctx, struct bpf_map *map, u64 flags)
+ * int bpf_get_stackid(struct pt_regs *ctx, struct bpf_map *map, u64 flags)
* Description
* Walk a user or a kernel stack and return its id. To achieve
* this, the helper needs *ctx*, which is a pointer to the context
@@ -1724,7 +1724,7 @@ union bpf_attr {
* Return
* 0 on success, or a negative error in case of failure.
*
- * int bpf_override_return(struct pt_reg *regs, u64 rc)
+ * int bpf_override_return(struct pt_regs *regs, u64 rc)
* Description
* Used for error injection, this helper uses kprobes to override
* the return value of the probed function, and to set it to *rc*.
--
2.22.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 3/4] bpf: clarify when bpf_trace_printk discards lines
2019-08-20 23:08 [PATCH v2 0/4] BPF-related documentation fixes Peter Wu
2019-08-20 23:08 ` [PATCH v2 1/4] bpf: clarify description for CONFIG_BPF_EVENTS Peter Wu
2019-08-20 23:08 ` [PATCH v2 2/4] bpf: fix 'struct pt_reg' typo in documentation Peter Wu
@ 2019-08-20 23:08 ` Peter Wu
2019-08-20 23:22 ` Alexei Starovoitov
2019-08-20 23:09 ` [PATCH v2 4/4] bpf: sync bpf.h to tools/ Peter Wu
3 siblings, 1 reply; 9+ messages in thread
From: Peter Wu @ 2019-08-20 23:08 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, bpf
I opened /sys/kernel/tracing/trace once and kept reading from it.
bpf_trace_printk somehow did not seem to work, no entries were appended
to that trace file. It turns out that tracing is disabled when that file
is open. Save the next person some time and document this.
The trace file is described in Documentation/trace/ftrace.rst, however
the implication "tracing is disabled" did not immediate translate to
"bpf_trace_printk silently discards entries".
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
---
include/uapi/linux/bpf.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 9ca333c3ce91..e4236e357ed9 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -575,6 +575,8 @@ union bpf_attr {
* limited to five).
*
* Each time the helper is called, it appends a line to the trace.
+ * Lines are discarded while *\/sys/kernel/debug/tracing/trace* is
+ * open, use *\/sys/kernel/debug/tracing/trace_pipe* to avoid this.
* The format of the trace is customizable, and the exact output
* one will get depends on the options set in
* *\/sys/kernel/debug/tracing/trace_options* (see also the
--
2.22.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 3/4] bpf: clarify when bpf_trace_printk discards lines
2019-08-20 23:08 ` [PATCH v2 3/4] bpf: clarify when bpf_trace_printk discards lines Peter Wu
@ 2019-08-20 23:22 ` Alexei Starovoitov
2019-08-21 0:04 ` Peter Wu
0 siblings, 1 reply; 9+ messages in thread
From: Alexei Starovoitov @ 2019-08-20 23:22 UTC (permalink / raw)
To: Peter Wu; +Cc: Alexei Starovoitov, Daniel Borkmann, netdev, bpf
On Wed, Aug 21, 2019 at 12:08:59AM +0100, Peter Wu wrote:
> I opened /sys/kernel/tracing/trace once and kept reading from it.
> bpf_trace_printk somehow did not seem to work, no entries were appended
> to that trace file. It turns out that tracing is disabled when that file
> is open. Save the next person some time and document this.
>
> The trace file is described in Documentation/trace/ftrace.rst, however
> the implication "tracing is disabled" did not immediate translate to
> "bpf_trace_printk silently discards entries".
>
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> ---
> include/uapi/linux/bpf.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 9ca333c3ce91..e4236e357ed9 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -575,6 +575,8 @@ union bpf_attr {
> * limited to five).
> *
> * Each time the helper is called, it appends a line to the trace.
> + * Lines are discarded while *\/sys/kernel/debug/tracing/trace* is
> + * open, use *\/sys/kernel/debug/tracing/trace_pipe* to avoid this.
that's not quite correct.
Having 'trace' file open doesn't discard lines.
I think this type of comment in uapi header makes more confusion than helps.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 3/4] bpf: clarify when bpf_trace_printk discards lines
2019-08-20 23:22 ` Alexei Starovoitov
@ 2019-08-21 0:04 ` Peter Wu
2019-08-21 17:23 ` Alexei Starovoitov
0 siblings, 1 reply; 9+ messages in thread
From: Peter Wu @ 2019-08-21 0:04 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Alexei Starovoitov, Daniel Borkmann, netdev, bpf
On Tue, Aug 20, 2019 at 04:22:23PM -0700, Alexei Starovoitov wrote:
> On Wed, Aug 21, 2019 at 12:08:59AM +0100, Peter Wu wrote:
> > I opened /sys/kernel/tracing/trace once and kept reading from it.
> > bpf_trace_printk somehow did not seem to work, no entries were appended
> > to that trace file. It turns out that tracing is disabled when that file
> > is open. Save the next person some time and document this.
> >
> > The trace file is described in Documentation/trace/ftrace.rst, however
> > the implication "tracing is disabled" did not immediate translate to
> > "bpf_trace_printk silently discards entries".
> >
> > Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> > ---
> > include/uapi/linux/bpf.h | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 9ca333c3ce91..e4236e357ed9 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -575,6 +575,8 @@ union bpf_attr {
> > * limited to five).
> > *
> > * Each time the helper is called, it appends a line to the trace.
> > + * Lines are discarded while *\/sys/kernel/debug/tracing/trace* is
> > + * open, use *\/sys/kernel/debug/tracing/trace_pipe* to avoid this.
>
> that's not quite correct.
> Having 'trace' file open doesn't discard lines.
> I think this type of comment in uapi header makes more confusion than helps.
Having the 'trace' file open for reading results in discarding lines. It
took me a while to figure that out. At first I was not even sure whether
my eBPF program was executed or not due to lack of entries in the
'trace' file.
I ended up setting a breakpoint and ended up with this call stack:
- bpf_trace_printk
- ____bpf_trace_printk
- __trace_printk
- trace_vprintk
- trace_array_vprintk
- __trace_array_vprintk
- __trace_array_vprintk
- __trace_buffer_lock_reserve
- ring_buffer_lock_reserve
The function ends up skipping the even because record_disabled == 1:
if (unlikely(atomic_read(&buffer->record_disabled)))
goto out;
Why is that? Well, I guessed that ring_buffer_record_disable and
ring_buffer_record_enable would be related. Sure enough, the first one
was hit when the 'trace' file is opened for reading while the latter is
called when the file is closed.
The relevant code from kernel/trace/trace.c (__tracing_open), "snapshot"
is true when "trace" is opened, and "false" when "trace_pipe" is used:
/* stop the trace while dumping if we are not opening "snapshot" */
if (!iter->snapshot)
tracing_stop_tr(tr);
So I think this supports the claim that lines are discarded. Do you
think this is not the case?
--
Kind regards,
Peter Wu
https://lekensteyn.nl
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 3/4] bpf: clarify when bpf_trace_printk discards lines
2019-08-21 0:04 ` Peter Wu
@ 2019-08-21 17:23 ` Alexei Starovoitov
0 siblings, 0 replies; 9+ messages in thread
From: Alexei Starovoitov @ 2019-08-21 17:23 UTC (permalink / raw)
To: Peter Wu; +Cc: Alexei Starovoitov, Daniel Borkmann, Network Development, bpf
On Tue, Aug 20, 2019 at 5:04 PM Peter Wu <peter@lekensteyn.nl> wrote:
>
> On Tue, Aug 20, 2019 at 04:22:23PM -0700, Alexei Starovoitov wrote:
> > On Wed, Aug 21, 2019 at 12:08:59AM +0100, Peter Wu wrote:
> > > I opened /sys/kernel/tracing/trace once and kept reading from it.
> > > bpf_trace_printk somehow did not seem to work, no entries were appended
> > > to that trace file. It turns out that tracing is disabled when that file
> > > is open. Save the next person some time and document this.
> > >
> > > The trace file is described in Documentation/trace/ftrace.rst, however
> > > the implication "tracing is disabled" did not immediate translate to
> > > "bpf_trace_printk silently discards entries".
> > >
> > > Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> > > ---
> > > include/uapi/linux/bpf.h | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > > index 9ca333c3ce91..e4236e357ed9 100644
> > > --- a/include/uapi/linux/bpf.h
> > > +++ b/include/uapi/linux/bpf.h
> > > @@ -575,6 +575,8 @@ union bpf_attr {
> > > * limited to five).
> > > *
> > > * Each time the helper is called, it appends a line to the trace.
> > > + * Lines are discarded while *\/sys/kernel/debug/tracing/trace* is
> > > + * open, use *\/sys/kernel/debug/tracing/trace_pipe* to avoid this.
> >
> > that's not quite correct.
> > Having 'trace' file open doesn't discard lines.
> > I think this type of comment in uapi header makes more confusion than helps.
>
> Having the 'trace' file open for reading results in discarding lines. It
> took me a while to figure that out. At first I was not even sure whether
> my eBPF program was executed or not due to lack of entries in the
> 'trace' file.
>
> I ended up setting a breakpoint and ended up with this call stack:
>
> - bpf_trace_printk
> - ____bpf_trace_printk
> - __trace_printk
> - trace_vprintk
> - trace_array_vprintk
> - __trace_array_vprintk
> - __trace_array_vprintk
> - __trace_buffer_lock_reserve
> - ring_buffer_lock_reserve
>
> The function ends up skipping the even because record_disabled == 1:
>
> if (unlikely(atomic_read(&buffer->record_disabled)))
> goto out;
>
> Why is that? Well, I guessed that ring_buffer_record_disable and
> ring_buffer_record_enable would be related. Sure enough, the first one
> was hit when the 'trace' file is opened for reading while the latter is
> called when the file is closed.
>
> The relevant code from kernel/trace/trace.c (__tracing_open), "snapshot"
> is true when "trace" is opened, and "false" when "trace_pipe" is used:
>
> /* stop the trace while dumping if we are not opening "snapshot" */
> if (!iter->snapshot)
> tracing_stop_tr(tr);
>
> So I think this supports the claim that lines are discarded. Do you
> think this is not the case?
Indeed.
I missed "(opened)" part in Documentation/trace/ftrace.rst:
trace:
This file holds the output of the trace in a human
readable format (described below). Note, tracing is temporarily
disabled while this file is being read (opened).
I always thought that reading disables it.
It's indeed odd part of the ftrace implementation that
worth documenting here.
Applied the set to bpf-next. Thanks
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 4/4] bpf: sync bpf.h to tools/
2019-08-20 23:08 [PATCH v2 0/4] BPF-related documentation fixes Peter Wu
` (2 preceding siblings ...)
2019-08-20 23:08 ` [PATCH v2 3/4] bpf: clarify when bpf_trace_printk discards lines Peter Wu
@ 2019-08-20 23:09 ` Peter Wu
3 siblings, 0 replies; 9+ messages in thread
From: Peter Wu @ 2019-08-20 23:09 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, bpf
Fix a 'struct pt_reg' typo and clarify when bpf_trace_printk discards
lines. Affects documentation only.
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
---
tools/include/uapi/linux/bpf.h | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 4e455018da65..58bdb89599c9 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -575,6 +575,8 @@ union bpf_attr {
* limited to five).
*
* Each time the helper is called, it appends a line to the trace.
+ * Lines are discarded while *\/sys/kernel/debug/tracing/trace* is
+ * open, use *\/sys/kernel/debug/tracing/trace_pipe* to avoid this.
* The format of the trace is customizable, and the exact output
* one will get depends on the options set in
* *\/sys/kernel/debug/tracing/trace_options* (see also the
@@ -1013,7 +1015,7 @@ union bpf_attr {
* The realm of the route for the packet associated to *skb*, or 0
* if none was found.
*
- * int bpf_perf_event_output(struct pt_reg *ctx, struct bpf_map *map, u64 flags, void *data, u64 size)
+ * int bpf_perf_event_output(struct pt_regs *ctx, struct bpf_map *map, u64 flags, void *data, u64 size)
* Description
* Write raw *data* blob into a special BPF perf event held by
* *map* of type **BPF_MAP_TYPE_PERF_EVENT_ARRAY**. This perf
@@ -1075,7 +1077,7 @@ union bpf_attr {
* Return
* 0 on success, or a negative error in case of failure.
*
- * int bpf_get_stackid(struct pt_reg *ctx, struct bpf_map *map, u64 flags)
+ * int bpf_get_stackid(struct pt_regs *ctx, struct bpf_map *map, u64 flags)
* Description
* Walk a user or a kernel stack and return its id. To achieve
* this, the helper needs *ctx*, which is a pointer to the context
@@ -1721,7 +1723,7 @@ union bpf_attr {
* Return
* 0 on success, or a negative error in case of failure.
*
- * int bpf_override_return(struct pt_reg *regs, u64 rc)
+ * int bpf_override_return(struct pt_regs *regs, u64 rc)
* Description
* Used for error injection, this helper uses kprobes to override
* the return value of the probed function, and to set it to *rc*.
--
2.22.0
^ permalink raw reply related [flat|nested] 9+ messages in thread