[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20181205165008.eonv4pcvoobymjvb@ast-mbp.dhcp.thefacebook.com>
Date: Wed, 5 Dec 2018 08:50:10 -0800
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Quentin Monnet <quentin.monnet@...ronome.com>
Cc: Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>, netdev@...r.kernel.org,
oss-drivers@...ronome.com
Subject: Re: [PATCH bpf-next] tools: bpftool: add a command to dump the trace
pipe
On Wed, Dec 05, 2018 at 10:28:24AM +0000, 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 <daniel@...earbox.net>
> Suggested-by: Daniel Borkmann <daniel@...earbox.net>
> Signed-off-by: Quentin Monnet <quentin.monnet@...ronome.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
> ---
> 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 <Ctrl+C> 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 struct cmd cmds[] = {
> { "loadall", do_loadall },
> { "attach", do_attach },
> { "detach", do_detach },
> + { "tracelog", do_tracelog },
> { 0 }
> };
>
> diff --git a/tools/bpf/bpftool/tracelog.c b/tools/bpf/bpftool/tracelog.c
> new file mode 100644
> index 000000000000..1fa8e513f590
> --- /dev/null
> +++ b/tools/bpf/bpftool/tracelog.c
> @@ -0,0 +1,157 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +/* Copyright (c) 2015-2017 Daniel Borkmann */
> +/* Copyright (c) 2018 Netronome Systems, Inc. */
> +
> +#include <errno.h>
> +#include <limits.h>
> +#include <signal.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <linux/magic.h>
> +#include <sys/fcntl.h>
> +#include <sys/vfs.h>
> +
> +#include "main.h"
> +
> +#ifndef TRACEFS_MAGIC
> +# define TRACEFS_MAGIC 0x74726163
> +#endif
> +
> +#define _textify(x) #x
> +#define textify(x) _textify(x)
> +
> +FILE *trace_pipe_fd;
> +char *buff;
> +
> +static int validate_tracefs_mnt(const char *mnt, unsigned long magic)
> +{
> + struct statfs st_fs;
> +
> + if (statfs(mnt, &st_fs) < 0)
> + return -ENOENT;
> + if ((unsigned long)st_fs.f_type != magic)
> + return -ENOENT;
> +
> + return 0;
> +}
> +
> +static bool
> +find_tracefs_mnt_single(unsigned long magic, char *mnt, const char *mntpt)
> +{
> + size_t src_len;
> +
> + if (validate_tracefs_mnt(mntpt, magic))
> + return false;
> +
> + src_len = strlen(mntpt);
> + if (src_len + 1 >= PATH_MAX) {
> + p_err("tracefs mount point name too long");
> + return false;
> + }
> +
> + strcpy(mnt, mntpt);
> + return true;
> +}
> +
> +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.
> + };
> + 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.
Powered by blists - more mailing lists