Re: [PATCH bpf-next v2 3/5] bpftool: Update documentation of "bpftool feature" command

From: Quentin Monnet <quentin@isovalent.com>
To: Michal Rostecki <mrostecki@opensuse.org>, bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
	Yonghong Song <yhs@fb.com>, Andrii Nakryiko <andriin@fb.com>,
	Jakub Kicinski <kuba@kernel.org>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Shuah Khan <shuah@kernel.org>,
	"open list:KERNEL SELFTEST FRAMEWORK" 
	<linux-kselftest@vger.kernel.org>
Subject: Re: [PATCH bpf-next v2 3/5] bpftool: Update documentation of "bpftool feature" command
Date: Fri, 21 Feb 2020 11:28:21 +0000	[thread overview]
Message-ID: <85e7fcf3-0ad3-28e3-ef6f-a19669fd6c7f@isovalent.com> (raw)
In-Reply-To: <20200221031702.25292-4-mrostecki@opensuse.org>

2020-02-21 04:16 UTC+0100 ~ Michal Rostecki <mrostecki@opensuse.org>
> Update documentation of "bpftool feature" command with information about
> new arguments: "full".
> 
> Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
> ---
>   .../bpf/bpftool/Documentation/bpftool-feature.rst | 15 ++++++++++-----
>   1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-feature.rst b/tools/bpf/bpftool/Documentation/bpftool-feature.rst
> index 4d08f35034a2..2e8f66ee1e77 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-feature.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-feature.rst
> @@ -19,19 +19,24 @@ SYNOPSIS
>   FEATURE COMMANDS
>   ================
>   
> -|	**bpftool** **feature probe** [*COMPONENT*] [**macros** [**prefix** *PREFIX*]]
> +|	**bpftool** **feature probe** [*COMPONENT*] [**full**] [**macros** [**prefix** *PREFIX*]]
>   |	**bpftool** **feature help**
>   |
>   |	*COMPONENT* := { **kernel** | **dev** *NAME* }
>   
>   DESCRIPTION
>   ===========
> -	**bpftool feature probe** [**kernel**] [**macros** [**prefix** *PREFIX*]]
> +	**bpftool feature probe** [**kernel**] [**full**] [**macros** [**prefix** *PREFIX*]]
>   		  Probe the running kernel and dump a number of eBPF-related
>   		  parameters, such as availability of the **bpf()** system call,
>   		  JIT status, eBPF program types availability, eBPF helper
>   		  functions availability, and more.
>   
> +		  By default, bpftool does not run probes for
> +		  bpf_probe_write_user and bpf_trace_printk helpers which emit

Please use formatting on helper function names, for readability and 
consistency.

I would even be tempted to highlight part or all of the sentence, with 
caps or bold, as some users may be surprised not to see those helpers in 
the list of available helpers on their system.

> +		  dmesg warnings. To enable them and run all probes, the
> +		  **full** keyword should be used.
> +
>   		  If the **macros** keyword (but not the **-j** option) is
>   		  passed, a subset of the output is dumped as a list of
>   		  **#define** macros that are ready to be included in a C
> @@ -48,12 +53,12 @@ DESCRIPTION
>   		  **bpf_trace_printk**\ () or **bpf_probe_write_user**\ ()) may
>   		  print warnings to kernel logs.

This should maybe be moved upwards and combined with your new paragraph?

>   
> -	**bpftool feature probe dev** *NAME* [**macros** [**prefix** *PREFIX*]]
> +	**bpftool feature probe dev** *NAME* [**full**] [**macros** [**prefix** *PREFIX*]]
>   		  Probe network device for supported eBPF features and dump
>   		  results to the console.
>   
> -		  The two keywords **macros** and **prefix** have the same
> -		  role as when probing the kernel.
> +		  The keywords **full**, **macros** and **prefix** have the
> +		  same role as when probing the kernel.
>   
>   	**bpftool feature help**
>   		  Print short help message.
> 