[PATCH bpf-next 0/5] tools: bpftool: fix minor issues in bpftool and its documentation

[PATCH bpf-next 0/5] tools: bpftool: fix minor issues in bpftool and its documentation

[Quentin Monnet]

Hi,
This series brings a number of small fixes to bpftool and the related
manual pages.

The first two patches are on bpftool itself (remove blank lines when
dumping programs, do not set errno when showing a tree of cgroup programs).

The next two patches are fixes for the manual pages (missing keyword, wrong
option name for showing bpftool version). The last one is not exactly a
fix, but rather an addition to the man page, explaining how to dump program
statistics.

Quentin Monnet (5):
  tools: bpftool: remove blank line after btf_id when listing programs
  tools: bpftool: reset errno for "bpftool cgroup tree"
  tools: bpftool: fix man page documentation for "pinmaps" keyword
  tools: bpftool: fix short option name for printing version in man
    pages
  tools: bpftool: add a note on program statistics in man page

 tools/bpf/bpftool/Documentation/bpftool-cgroup.rst  |  2 +-
 tools/bpf/bpftool/Documentation/bpftool-feature.rst |  2 +-
 tools/bpf/bpftool/Documentation/bpftool-map.rst     |  2 +-
 tools/bpf/bpftool/Documentation/bpftool-net.rst     |  2 +-
 tools/bpf/bpftool/Documentation/bpftool-perf.rst    |  2 +-
 tools/bpf/bpftool/Documentation/bpftool-prog.rst    | 12 ++++++++++--
 tools/bpf/bpftool/Documentation/bpftool.rst         |  2 +-
 tools/bpf/bpftool/cgroup.c                          |  7 +++++++
 tools/bpf/bpftool/prog.c                            |  2 +-
 9 files changed, 24 insertions(+), 9 deletions(-)

-- 
2.17.1

[PATCH bpf-next 1/5] tools: bpftool: remove blank line after btf_id when listing programs

[Quentin Monnet]

Commit 569b0c77735d ("tools/bpftool: show btf id in program information")
made bpftool print an empty line after each program entry when listing
the BPF programs loaded on the system (plain output). This is especially
confusing when some programs have an associated BTF id, and others
don't. Let's remove the blank line.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/bpf/bpftool/prog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 81067803189e..4b7a307b9fc9 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -323,7 +323,7 @@ static void print_prog_plain(struct bpf_prog_info *info, int fd)
 	}
 
 	if (info->btf_id)
-		printf("\n\tbtf_id %d\n", info->btf_id);
+		printf("\n\tbtf_id %d", info->btf_id);
 
 	printf("\n");
 }
-- 
2.17.1

[PATCH bpf-next 2/5] tools: bpftool: reset errno for "bpftool cgroup tree"

[Quentin Monnet]

When trying to dump the tree of all cgroups under a given root node,
bpftool attempts to query programs of all available attach types. Some
of those attach types do not support queries, therefore several of the
calls are actually expected to fail.

Those calls set errno to EINVAL, which has no consequence for dumping
the rest of the tree. It does have consequences however if errno is
inspected at a later time. For example, bpftool batch mode relies on
errno to determine whether a command has succeeded, and whether it
should carry on with the next command. Setting errno to EINVAL when
everything worked as expected would therefore make such command fail:

        # echo 'cgroup tree \n net show' | \
                bpftool batch file -

To improve this, reset errno when its value is EINVAL after attempting
to show programs for all existing attach types in do_show_tree_fn().

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/bpf/bpftool/cgroup.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
index 4b5c8da2a7c0..a81b34343eb8 100644
--- a/tools/bpf/bpftool/cgroup.c
+++ b/tools/bpf/bpftool/cgroup.c
@@ -248,6 +248,13 @@ static int do_show_tree_fn(const char *fpath, const struct stat *sb,
 	for (type = 0; type < __MAX_BPF_ATTACH_TYPE; type++)
 		show_attached_bpf_progs(cgroup_fd, type, ftw->level);
 
+	if (errno == EINVAL)
+		/* Last attach type does not support query.
+		 * Do not report an error for this, especially because batch
+		 * mode would stop processing commands.
+		 */
+		errno = 0;
+
 	if (json_output) {
 		jsonw_end_array(json_wtr);
 		jsonw_end_object(json_wtr);
-- 
2.17.1

[PATCH bpf-next 3/5] tools: bpftool: fix man page documentation for "pinmaps" keyword

[Quentin Monnet]

The "pinmaps" keyword is present in the man page, in the verbose
description of the "bpftool prog load" command. However, it is missing
from the summary of available commands at the beginning of the file. Add
it there as well.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/bpf/bpftool/Documentation/bpftool-prog.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 9386bd6e0396..49c33939239f 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -25,7 +25,7 @@ PROG COMMANDS
 |	**bpftool** **prog dump xlated** *PROG* [{**file** *FILE* | **opcodes** | **visual** | **linum**}]
 |	**bpftool** **prog dump jited**  *PROG* [{**file** *FILE* | **opcodes** | **linum**}]
 |	**bpftool** **prog pin** *PROG* *FILE*
-|	**bpftool** **prog { load | loadall }** *OBJ* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
+|	**bpftool** **prog { load | loadall }** *OBJ* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*] [**pinmaps** *MAP_DIR*]
 |	**bpftool** **prog attach** *PROG* *ATTACH_TYPE* [*MAP*]
 |	**bpftool** **prog detach** *PROG* *ATTACH_TYPE* [*MAP*]
 |	**bpftool** **prog tracelog**
-- 
2.17.1

[PATCH bpf-next 4/5] tools: bpftool: fix short option name for printing version in man pages

[Quentin Monnet]

Manual pages would tell that option "-v" (lower case) would print the
version number for bpftool. This is wrong: the short name of the option
is "-V" (upper case). Fix the documentation accordingly.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/bpf/bpftool/Documentation/bpftool-cgroup.rst  | 2 +-
 tools/bpf/bpftool/Documentation/bpftool-feature.rst | 2 +-
 tools/bpf/bpftool/Documentation/bpftool-map.rst     | 2 +-
 tools/bpf/bpftool/Documentation/bpftool-net.rst     | 2 +-
 tools/bpf/bpftool/Documentation/bpftool-perf.rst    | 2 +-
 tools/bpf/bpftool/Documentation/bpftool-prog.rst    | 2 +-
 tools/bpf/bpftool/Documentation/bpftool.rst         | 2 +-
 7 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
index 9bb9ace54ba8..5e3b7d9d7599 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
@@ -99,7 +99,7 @@ OPTIONS
 	-h, --help
 		  Print short generic help message (similar to **bpftool help**).
 
-	-v, --version
+	-V, --version
 		  Print version number (similar to **bpftool version**).
 
 	-j, --json
diff --git a/tools/bpf/bpftool/Documentation/bpftool-feature.rst b/tools/bpf/bpftool/Documentation/bpftool-feature.rst
index 82de03dd8f52..10177343b7ce 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-feature.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-feature.rst
@@ -63,7 +63,7 @@ OPTIONS
 	-h, --help
 		  Print short generic help message (similar to **bpftool help**).
 
-	-v, --version
+	-V, --version
 		  Print version number (similar to **bpftool version**).
 
 	-j, --json
diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst
index 5c984ffc9f01..55ecf80ca03e 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
@@ -135,7 +135,7 @@ OPTIONS
 	-h, --help
 		  Print short generic help message (similar to **bpftool help**).
 
-	-v, --version
+	-V, --version
 		  Print version number (similar to **bpftool version**).
 
 	-j, --json
diff --git a/tools/bpf/bpftool/Documentation/bpftool-net.rst b/tools/bpf/bpftool/Documentation/bpftool-net.rst
index 779dab3650ee..6b692b428373 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-net.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-net.rst
@@ -55,7 +55,7 @@ OPTIONS
 	-h, --help
 		  Print short generic help message (similar to **bpftool help**).
 
-	-v, --version
+	-V, --version
 		  Print version number (similar to **bpftool version**).
 
 	-j, --json
diff --git a/tools/bpf/bpftool/Documentation/bpftool-perf.rst b/tools/bpf/bpftool/Documentation/bpftool-perf.rst
index bca5590a80d0..86154740fabb 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-perf.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-perf.rst
@@ -43,7 +43,7 @@ OPTIONS
 	-h, --help
 		  Print short generic help message (similar to **bpftool help**).
 
-	-v, --version
+	-V, --version
 		  Print version number (similar to **bpftool version**).
 
 	-j, --json
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 49c33939239f..f2beb79a77e7 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -144,7 +144,7 @@ OPTIONS
 	-h, --help
 		  Print short generic help message (similar to **bpftool help**).
 
-	-v, --version
+	-V, --version
 		  Print version number (similar to **bpftool version**).
 
 	-j, --json
diff --git a/tools/bpf/bpftool/Documentation/bpftool.rst b/tools/bpf/bpftool/Documentation/bpftool.rst
index 4f2188845dd8..deb2ca911ddf 100644
--- a/tools/bpf/bpftool/Documentation/bpftool.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool.rst
@@ -49,7 +49,7 @@ OPTIONS
 	-h, --help
 		  Print short help message (similar to **bpftool help**).
 
-	-v, --version
+	-V, --version
 		  Print version number (similar to **bpftool version**).
 
 	-j, --json
-- 
2.17.1

[PATCH bpf-next 5/5] tools: bpftool: add a note on program statistics in man page

[Quentin Monnet]

Linux kernel now supports statistics for BPF programs, and bpftool is
able to dump them. However, these statistics are not enabled by default,
and administrators may not know how to access them.

Add a paragraph in bpftool documentation, under the description of the
"bpftool prog show" command, to explain that such statistics are
available and that their collection is controlled via a dedicated sysctl
knob.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/bpf/bpftool/Documentation/bpftool-prog.rst | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index f2beb79a77e7..bb9bb00c0c2c 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -56,6 +56,14 @@ DESCRIPTION
 		  Output will start with program ID followed by program type and
 		  zero or more named attributes (depending on kernel version).
 
+		  Since Linux 5.1 the kernel can collect statistics on BPF
+		  programs (such as the total time spent running the program,
+		  and the number of times it was run). If available, bpftool
+		  shows such statistics. However, the kernel does not collect
+		  them by defaults, as it slightly impacts performance on each
+		  program run. Activation or deactivation of the feature is
+		  performed via the **kernel.bpf_stats_enabled** sysctl knob.
+
 	**bpftool prog dump xlated** *PROG* [{ **file** *FILE* | **opcodes** | **visual** | **linum** }]
 		  Dump eBPF instructions of the program from the kernel. By
 		  default, eBPF will be disassembled and printed to standard
-- 
2.17.1

Re: [PATCH bpf-next 0/5] tools: bpftool: fix minor issues in bpftool and its documentation

[Song Liu]

On Fri, Apr 12, 2019 at 6:30 AM Quentin Monnet
<quentin.monnet@netronome.com> wrote:
>
> Hi,
> This series brings a number of small fixes to bpftool and the related
> manual pages.
>
> The first two patches are on bpftool itself (remove blank lines when
> dumping programs, do not set errno when showing a tree of cgroup programs).
>
> The next two patches are fixes for the manual pages (missing keyword, wrong
> option name for showing bpftool version). The last one is not exactly a
> fix, but rather an addition to the man page, explaining how to dump program
> statistics.

For the set:

Acked-by: Song Liu <songliubraving@fb.com>

>
> Quentin Monnet (5):
>   tools: bpftool: remove blank line after btf_id when listing programs
>   tools: bpftool: reset errno for "bpftool cgroup tree"
>   tools: bpftool: fix man page documentation for "pinmaps" keyword
>   tools: bpftool: fix short option name for printing version in man
>     pages
>   tools: bpftool: add a note on program statistics in man page
>
>  tools/bpf/bpftool/Documentation/bpftool-cgroup.rst  |  2 +-
>  tools/bpf/bpftool/Documentation/bpftool-feature.rst |  2 +-
>  tools/bpf/bpftool/Documentation/bpftool-map.rst     |  2 +-
>  tools/bpf/bpftool/Documentation/bpftool-net.rst     |  2 +-
>  tools/bpf/bpftool/Documentation/bpftool-perf.rst    |  2 +-
>  tools/bpf/bpftool/Documentation/bpftool-prog.rst    | 12 ++++++++++--
>  tools/bpf/bpftool/Documentation/bpftool.rst         |  2 +-
>  tools/bpf/bpftool/cgroup.c                          |  7 +++++++
>  tools/bpf/bpftool/prog.c                            |  2 +-
>  9 files changed, 24 insertions(+), 9 deletions(-)
>
> --
> 2.17.1
>

Re: [PATCH bpf-next 0/5] tools: bpftool: fix minor issues in bpftool and its documentation

[Daniel Borkmann]

On 04/12/2019 03:29 PM, Quentin Monnet wrote:
> Hi,
> This series brings a number of small fixes to bpftool and the related
> manual pages.
> 
> The first two patches are on bpftool itself (remove blank lines when
> dumping programs, do not set errno when showing a tree of cgroup programs).
> 
> The next two patches are fixes for the manual pages (missing keyword, wrong
> option name for showing bpftool version). The last one is not exactly a
> fix, but rather an addition to the man page, explaining how to dump program
> statistics.
> 
> Quentin Monnet (5):
>   tools: bpftool: remove blank line after btf_id when listing programs
>   tools: bpftool: reset errno for "bpftool cgroup tree"
>   tools: bpftool: fix man page documentation for "pinmaps" keyword
>   tools: bpftool: fix short option name for printing version in man
>     pages
>   tools: bpftool: add a note on program statistics in man page
> 
>  tools/bpf/bpftool/Documentation/bpftool-cgroup.rst  |  2 +-
>  tools/bpf/bpftool/Documentation/bpftool-feature.rst |  2 +-
>  tools/bpf/bpftool/Documentation/bpftool-map.rst     |  2 +-
>  tools/bpf/bpftool/Documentation/bpftool-net.rst     |  2 +-
>  tools/bpf/bpftool/Documentation/bpftool-perf.rst    |  2 +-
>  tools/bpf/bpftool/Documentation/bpftool-prog.rst    | 12 ++++++++++--
>  tools/bpf/bpftool/Documentation/bpftool.rst         |  2 +-
>  tools/bpf/bpftool/cgroup.c                          |  7 +++++++
>  tools/bpf/bpftool/prog.c                            |  2 +-
>  9 files changed, 24 insertions(+), 9 deletions(-)
> 

Applied, thanks!