Re: [PATCH bpf-next] tools: bpftool: add a command to dump the trace pipe
On Thu, Dec 06, 2018 at 05:20:54PM +, Quentin Monnet wrote: > 2018-12-05 19:18 UTC-0800 ~ Alexei Starovoitov > > > On Wed, Dec 05, 2018 at 06:15:23PM +, Quentin Monnet wrote: > + > +/* Allow room for NULL terminating byte and pipe file name */ > +snprintf(format, sizeof(format), "%%*s %%%zds %%99s %%*s %%*d > %%*d\\n", > + PATH_MAX - strlen(pipe_name) - 1); > >>> > >>> before scanning trace_pipe could you add a check that trace_options are > >>> compatible? > >>> Otherwise there will be a lot of garbage printed. > >>> afaik default is rarely changed, so the patch is ok as-is. > >>> The followup some time in the future would be perfect. > >> > >> Sure. What do you mean exactly by compatible options? I can check that > >> "trace_printk" is set, is there any other option that would be relevant? > > > > See Documentation/trace/ftrace.rst > > a lot of the flags will change the format significantly. > > Like 'bin' will make it binary. > > I'm not suggesting to support all possible output formats. > > Only to check that trace flags match scanf. > > fscanf() is only used to retrieve the name of the sysfs directory where > the pipe is located, when listing all the mount points on the system. It > is not used to dump the content from the pipe (which is done with > getline(), so formatting does not matter much). > > If the "bin" option is set, "bpftool prog tracelog" will dump the same > binary content as "cat /sys/kernel/debug/tracing/trace_pipe", which is > the expected behaviour (at least with the current patch). Let me know if > you would like me to change this somehow. I misread the patch :) thanks for explaining. all good then.
Re: [PATCH bpf-next] tools: bpftool: add a command to dump the trace pipe
2018-12-05 19:18 UTC-0800 ~ Alexei Starovoitov > On Wed, Dec 05, 2018 at 06:15:23PM +, Quentin Monnet wrote: + + /* Allow room for NULL terminating byte and pipe file name */ + snprintf(format, sizeof(format), "%%*s %%%zds %%99s %%*s %%*d %%*d\\n", + PATH_MAX - strlen(pipe_name) - 1); >>> >>> before scanning trace_pipe could you add a check that trace_options are >>> compatible? >>> Otherwise there will be a lot of garbage printed. >>> afaik default is rarely changed, so the patch is ok as-is. >>> The followup some time in the future would be perfect. >> >> Sure. What do you mean exactly by compatible options? I can check that >> "trace_printk" is set, is there any other option that would be relevant? > > See Documentation/trace/ftrace.rst > a lot of the flags will change the format significantly. > Like 'bin' will make it binary. > I'm not suggesting to support all possible output formats. > Only to check that trace flags match scanf. fscanf() is only used to retrieve the name of the sysfs directory where the pipe is located, when listing all the mount points on the system. It is not used to dump the content from the pipe (which is done with getline(), so formatting does not matter much). If the "bin" option is set, "bpftool prog tracelog" will dump the same binary content as "cat /sys/kernel/debug/tracing/trace_pipe", which is the expected behaviour (at least with the current patch). Let me know if you would like me to change this somehow. Thanks, Quentin
Re: [PATCH bpf-next] tools: bpftool: add a command to dump the trace pipe
On Wed, Dec 05, 2018 at 06:15:23PM +, Quentin Monnet wrote: > > > + > > > + /* Allow room for NULL terminating byte and pipe file name */ > > > + snprintf(format, sizeof(format), "%%*s %%%zds %%99s %%*s %%*d %%*d\\n", > > > + PATH_MAX - strlen(pipe_name) - 1); > > > > before scanning trace_pipe could you add a check that trace_options are > > compatible? > > Otherwise there will be a lot of garbage printed. > > afaik default is rarely changed, so the patch is ok as-is. > > The followup some time in the future would be perfect. > > Sure. What do you mean exactly by compatible options? I can check that > "trace_printk" is set, is there any other option that would be relevant? See Documentation/trace/ftrace.rst a lot of the flags will change the format significantly. Like 'bin' will make it binary. I'm not suggesting to support all possible output formats. Only to check that trace flags match scanf.
Re: [PATCH bpf-next] tools: bpftool: add a command to dump the trace pipe
2018-12-05 08:50 UTC-0800 ~ Alexei Starovoitov
On Wed, Dec 05, 2018 at 10:28:24AM +, Quentin Monnet wrote:
BPF programs can use the bpf_trace_printk() helper to print debug
information into the trace pipe. Add a subcommand
"bpftool prog tracelog" to simply dump this pipe to the console.
This is for a good part copied from iproute2, where the feature is
available with "tc exec bpf dbg". Changes include dumping pipe content
to stdout instead of stderr and adding JSON support (content is dumped
as an array of strings, one per line read from the pipe). This version
is dual-licensed, with Daniel's permission.
Cc: Daniel Borkmann
Suggested-by: Daniel Borkmann
Signed-off-by: Quentin Monnet
Reviewed-by: Jakub Kicinski
---
+static bool find_tracefs_pipe(char *mnt)
+{
+ static const char * const known_mnts[] = {
+ "/sys/kernel/debug/tracing",
+ "/sys/kernel/tracing",
+ "/tracing",
+ "/trace",
I wonder where this list came from?
I only knew of 1st one.
I only met the first form too. I took the list from iproute2. I can
change it in the future if we find out trying all these paths is not
relevant.
+ };
+ const char *pipe_name = "/trace_pipe";
+ const char *fstype = "tracefs";
+ char type[100], format[32];
+ const char * const *ptr;
+ bool found = false;
+ FILE *fp;
+
+ for (ptr = known_mnts; ptr < known_mnts + ARRAY_SIZE(known_mnts); ptr++)
+ if (find_tracefs_mnt_single(TRACEFS_MAGIC, mnt, *ptr))
+ goto exit_found;
+
+ fp = fopen("/proc/mounts", "r");
+ if (!fp)
+ return false;
+
+ /* Allow room for NULL terminating byte and pipe file name */
+ snprintf(format, sizeof(format), "%%*s %%%zds %%99s %%*s %%*d %%*d\\n",
+PATH_MAX - strlen(pipe_name) - 1);
before scanning trace_pipe could you add a check that trace_options are
compatible?
Otherwise there will be a lot of garbage printed.
afaik default is rarely changed, so the patch is ok as-is.
The followup some time in the future would be perfect.
Sure. What do you mean exactly by compatible options? I can check that
"trace_printk" is set, is there any other option that would be relevant?
Thanks,
Quentin
Re: [PATCH bpf-next] tools: bpftool: add a command to dump the trace pipe
On Wed, Dec 05, 2018 at 10:28:24AM +, Quentin Monnet wrote:
> BPF programs can use the bpf_trace_printk() helper to print debug
> information into the trace pipe. Add a subcommand
> "bpftool prog tracelog" to simply dump this pipe to the console.
>
> This is for a good part copied from iproute2, where the feature is
> available with "tc exec bpf dbg". Changes include dumping pipe content
> to stdout instead of stderr and adding JSON support (content is dumped
> as an array of strings, one per line read from the pipe). This version
> is dual-licensed, with Daniel's permission.
>
> Cc: Daniel Borkmann
> Suggested-by: Daniel Borkmann
> Signed-off-by: Quentin Monnet
> Reviewed-by: Jakub Kicinski
> ---
> tools/bpf/bpftool/Documentation/bpftool-prog.rst | 13 +-
> tools/bpf/bpftool/bash-completion/bpftool| 5 +-
> tools/bpf/bpftool/main.h | 1 +
> tools/bpf/bpftool/prog.c | 4 +-
> tools/bpf/bpftool/tracelog.c | 157
> +++
> 5 files changed, 176 insertions(+), 4 deletions(-)
> create mode 100644 tools/bpf/bpftool/tracelog.c
>
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> index ab36e920e552..5524b6dccd85 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> @@ -26,8 +26,9 @@ MAP COMMANDS
> |**bpftool** **prog dump jited** *PROG* [{**file** *FILE* |
> **opcodes**}]
> |**bpftool** **prog pin** *PROG* *FILE*
> |**bpftool** **prog { load | loadall }** *OBJ* *PATH* [**type** *TYPE*]
> [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> -| **bpftool** **prog attach** *PROG* *ATTACH_TYPE* [*MAP*]
> -| **bpftool** **prog detach** *PROG* *ATTACH_TYPE* [*MAP*]
> +|**bpftool** **prog attach** *PROG* *ATTACH_TYPE* [*MAP*]
> +|**bpftool** **prog detach** *PROG* *ATTACH_TYPE* [*MAP*]
> +|**bpftool** **prog tracelog**
> |**bpftool** **prog help**
> |
> |*MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
> @@ -117,6 +118,14 @@ DESCRIPTION
> parameter, with the exception of *flow_dissector* which is
> detached from the current networking name space.
>
> + **bpftool prog tracelog**
> + Dump the trace pipe of the system to the console (stdout).
> + Hit to stop printing. BPF programs can write to this
> + trace pipe at runtime with the **bpf_trace_printk()** helper.
> + This should be used only for debugging purposes. For
> + streaming data from BPF programs to user space, one can use
> + perf events (see also **bpftool-map**\ (8)).
> +
> **bpftool prog help**
> Print short help message.
>
> diff --git a/tools/bpf/bpftool/bash-completion/bpftool
> b/tools/bpf/bpftool/bash-completion/bpftool
> index 9a60080f085f..44c189ba072a 100644
> --- a/tools/bpf/bpftool/bash-completion/bpftool
> +++ b/tools/bpf/bpftool/bash-completion/bpftool
> @@ -398,10 +398,13 @@ _bpftool()
> ;;
> esac
> ;;
> +tracelog)
> +return 0
> +;;
> *)
> [[ $prev == $object ]] && \
> COMPREPLY=( $( compgen -W 'dump help pin attach
> detach load \
> -show list' -- "$cur" ) )
> +show list tracelog' -- "$cur" ) )
> ;;
> esac
> ;;
> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> index 2761981669c8..0be0dd8f467f 100644
> --- a/tools/bpf/bpftool/main.h
> +++ b/tools/bpf/bpftool/main.h
> @@ -167,6 +167,7 @@ int do_event_pipe(int argc, char **argv);
> int do_cgroup(int argc, char **arg);
> int do_perf(int argc, char **arg);
> int do_net(int argc, char **arg);
> +int do_tracelog(int argc, char **arg);
>
> int parse_u32_arg(int *argc, char ***argv, __u32 *val, const char *what);
> int prog_parse_fd(int *argc, char ***argv);
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 56db61c5a91f..54c8dbf05c9c 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -1140,6 +1140,7 @@ static int do_help(int argc, char **argv)
> " [pinmaps MAP_DIR]\n"
> " %s %s attach PROG ATTACH_TYPE [MAP]\n"
> " %s %s detach PROG ATTACH_TYPE [MAP]\n"
> + " %s %s tracelog\n"
> " %s %s help\n"
> "\n"
> " " HELP_SPEC_MAP "\n"
> @@ -1158,7 +1159,7 @@ static int do_help(int argc, char **argv)
> "",
> bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
> bin_name, argv[-2], bin_name,
Re: [PATCH bpf-next] tools: bpftool: add a command to dump the trace pipe
On 12/05/2018 11:28 AM, Quentin Monnet wrote: > BPF programs can use the bpf_trace_printk() helper to print debug > information into the trace pipe. Add a subcommand > "bpftool prog tracelog" to simply dump this pipe to the console. > > This is for a good part copied from iproute2, where the feature is > available with "tc exec bpf dbg". Changes include dumping pipe content > to stdout instead of stderr and adding JSON support (content is dumped > as an array of strings, one per line read from the pipe). This version > is dual-licensed, with Daniel's permission. > > Cc: Daniel Borkmann > Suggested-by: Daniel Borkmann > Signed-off-by: Quentin Monnet > Reviewed-by: Jakub Kicinski Looks good, applied, thanks!
[PATCH bpf-next] tools: bpftool: add a command to dump the trace pipe
BPF programs can use the bpf_trace_printk() helper to print debug
information into the trace pipe. Add a subcommand
"bpftool prog tracelog" to simply dump this pipe to the console.
This is for a good part copied from iproute2, where the feature is
available with "tc exec bpf dbg". Changes include dumping pipe content
to stdout instead of stderr and adding JSON support (content is dumped
as an array of strings, one per line read from the pipe). This version
is dual-licensed, with Daniel's permission.
Cc: Daniel Borkmann
Suggested-by: Daniel Borkmann
Signed-off-by: Quentin Monnet
Reviewed-by: Jakub Kicinski
---
tools/bpf/bpftool/Documentation/bpftool-prog.rst | 13 +-
tools/bpf/bpftool/bash-completion/bpftool| 5 +-
tools/bpf/bpftool/main.h | 1 +
tools/bpf/bpftool/prog.c | 4 +-
tools/bpf/bpftool/tracelog.c | 157 +++
5 files changed, 176 insertions(+), 4 deletions(-)
create mode 100644 tools/bpf/bpftool/tracelog.c
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index ab36e920e552..5524b6dccd85 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -26,8 +26,9 @@ MAP COMMANDS
| **bpftool** **prog dump jited** *PROG* [{**file** *FILE* |
**opcodes**}]
| **bpftool** **prog pin** *PROG* *FILE*
| **bpftool** **prog { load | loadall }** *OBJ* *PATH* [**type** *TYPE*]
[**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
-| **bpftool** **prog attach** *PROG* *ATTACH_TYPE* [*MAP*]
-| **bpftool** **prog detach** *PROG* *ATTACH_TYPE* [*MAP*]
+| **bpftool** **prog attach** *PROG* *ATTACH_TYPE* [*MAP*]
+| **bpftool** **prog detach** *PROG* *ATTACH_TYPE* [*MAP*]
+| **bpftool** **prog tracelog**
| **bpftool** **prog help**
|
| *MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
@@ -117,6 +118,14 @@ DESCRIPTION
parameter, with the exception of *flow_dissector* which is
detached from the current networking name space.
+ **bpftool prog tracelog**
+ Dump the trace pipe of the system to the console (stdout).
+ Hit to stop printing. BPF programs can write to this
+ trace pipe at runtime with the **bpf_trace_printk()** helper.
+ This should be used only for debugging purposes. For
+ streaming data from BPF programs to user space, one can use
+ perf events (see also **bpftool-map**\ (8)).
+
**bpftool prog help**
Print short help message.
diff --git a/tools/bpf/bpftool/bash-completion/bpftool
b/tools/bpf/bpftool/bash-completion/bpftool
index 9a60080f085f..44c189ba072a 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -398,10 +398,13 @@ _bpftool()
;;
esac
;;
+tracelog)
+return 0
+;;
*)
[[ $prev == $object ]] && \
COMPREPLY=( $( compgen -W 'dump help pin attach detach
load \
-show list' -- "$cur" ) )
+show list tracelog' -- "$cur" ) )
;;
esac
;;
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 2761981669c8..0be0dd8f467f 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -167,6 +167,7 @@ int do_event_pipe(int argc, char **argv);
int do_cgroup(int argc, char **arg);
int do_perf(int argc, char **arg);
int do_net(int argc, char **arg);
+int do_tracelog(int argc, char **arg);
int parse_u32_arg(int *argc, char ***argv, __u32 *val, const char *what);
int prog_parse_fd(int *argc, char ***argv);
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 56db61c5a91f..54c8dbf05c9c 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -1140,6 +1140,7 @@ static int do_help(int argc, char **argv)
" [pinmaps MAP_DIR]\n"
" %s %s attach PROG ATTACH_TYPE [MAP]\n"
" %s %s detach PROG ATTACH_TYPE [MAP]\n"
+ " %s %s tracelog\n"
" %s %s help\n"
"\n"
" " HELP_SPEC_MAP "\n"
@@ -1158,7 +1159,7 @@ static int do_help(int argc, char **argv)
"",
bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
- bin_name, argv[-2], bin_name, argv[-2]);
+ bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2]);
return 0;
}
@@ -1173,6 +1174,7 @@ static const
