[PATCH bpf-next 0/3] bpf: fix formatting for eBPF helper doc

[PATCH bpf-next 0/3] bpf: fix formatting for eBPF helper doc

[Quentin Monnet]

Hi,
Here is a follow-up set for eBPF helper documentation, with two patches to
fix formatting issues:

- One to fix an error I introduced with the initial set for the doc.
- One for the newly added bpf_get_stack(), that is currently not parsed
  correctly with the Python script (function signature is fine, but
  description and return value appear as empty).

There is no change to text contents in this set.

Quentin Monnet (3):
  bpf: fix formatting for bpf_perf_event_read() helper doc
  bpf: fix formatting for bpf_get_stack() helper documentation
  bpf: update bpf.h uapi header for tools

 include/uapi/linux/bpf.h       | 62 +++++++++++++++++++++---------------------
 tools/include/uapi/linux/bpf.h | 62 +++++++++++++++++++++---------------------
 2 files changed, 62 insertions(+), 62 deletions(-)

-- 
2.14.1

[PATCH bpf-next 1/3] bpf: fix formatting for bpf_perf_event_read() helper doc

[Quentin Monnet]

Some edits brought to the last iteration of BPF helper functions
documentation introduced an error with RST formatting. As a result, most
of one paragraph is rendered in bold text when only the name of a helper
should be. Fix it, and fix formatting of another function name in the
same paragraph.

Fixes: c6b5fb8690fa ("bpf: add documentation for eBPF helpers (42-50)")
Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 include/uapi/linux/bpf.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 23b334bba1a6..530ff6588d8f 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -828,12 +828,12 @@ union bpf_attr {
  *
  * 		Also, be aware that the newer helper
  * 		**bpf_perf_event_read_value**\ () is recommended over
- * 		**bpf_perf_event_read*\ () in general. The latter has some ABI
+ * 		**bpf_perf_event_read**\ () in general. The latter has some ABI
  * 		quirks where error and counter value are used as a return code
  * 		(which is wrong to do since ranges may overlap). This issue is
- * 		fixed with bpf_perf_event_read_value(), which at the same time
- * 		provides more features over the **bpf_perf_event_read**\ ()
- * 		interface. Please refer to the description of
+ * 		fixed with **bpf_perf_event_read_value**\ (), which at the same
+ * 		time provides more features over the **bpf_perf_event_read**\
+ * 		() interface. Please refer to the description of
  * 		**bpf_perf_event_read_value**\ () for details.
  * 	Return
  * 		The value of the perf event counter read from the map, or a
-- 
2.14.1

[PATCH bpf-next 2/3] bpf: fix formatting for bpf_get_stack() helper doc

[Quentin Monnet]

Fix formatting (indent) for bpf_get_stack() helper documentation, so
that the doc is rendered correctly with the Python script.

Fixes: c195651e565a ("bpf: add bpf_get_stack helper")
Cc: Yonghong Song <yhs@fb.com>
Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
---

Note: The error was a missing space between the '*' marking the
comments, and the tabs. This expected mixed indent comes from the fact I
started to write the doc as a RST, then copied my contents (tabs
included) in the header file and added a " * " (with a space) prefix
everywhere.

On a second thought, using such indent style was maybe... not my best idea
ever. Anyway, if indent for documenting eBPF helpers really gets to painful, we
could relax parsing rules in the Python script to make things easier.
---
 include/uapi/linux/bpf.h | 54 ++++++++++++++++++++++++------------------------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 530ff6588d8f..8daef7326bb7 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1770,33 +1770,33 @@ union bpf_attr {
  *
  * int bpf_get_stack(struct pt_regs *regs, void *buf, u32 size, u64 flags)
  * 	Description
- *		Return a user or a kernel stack in bpf program provided buffer.
- *		To achieve this, the helper needs *ctx*, which is a pointer
- *		to the context on which the tracing program is executed.
- *		To store the stacktrace, the bpf program provides *buf* with
- *		a nonnegative *size*.
- *
- *		The last argument, *flags*, holds the number of stack frames to
- *		skip (from 0 to 255), masked with
- *		**BPF_F_SKIP_FIELD_MASK**. The next bits can be used to set
- *		the following flags:
- *
- *		**BPF_F_USER_STACK**
- *			Collect a user space stack instead of a kernel stack.
- *		**BPF_F_USER_BUILD_ID**
- *			Collect buildid+offset instead of ips for user stack,
- *			only valid if **BPF_F_USER_STACK** is also specified.
- *
- *		**bpf_get_stack**\ () can collect up to
- *		**PERF_MAX_STACK_DEPTH** both kernel and user frames, subject
- *		to sufficient large buffer size. Note that
- *		this limit can be controlled with the **sysctl** program, and
- *		that it should be manually increased in order to profile long
- *		user stacks (such as stacks for Java programs). To do so, use:
- *
- *	::
- *
- *		# sysctl kernel.perf_event_max_stack=<new value>
+ * 		Return a user or a kernel stack in bpf program provided buffer.
+ * 		To achieve this, the helper needs *ctx*, which is a pointer
+ * 		to the context on which the tracing program is executed.
+ * 		To store the stacktrace, the bpf program provides *buf* with
+ * 		a nonnegative *size*.
+ *
+ * 		The last argument, *flags*, holds the number of stack frames to
+ * 		skip (from 0 to 255), masked with
+ * 		**BPF_F_SKIP_FIELD_MASK**. The next bits can be used to set
+ * 		the following flags:
+ *
+ * 		**BPF_F_USER_STACK**
+ * 			Collect a user space stack instead of a kernel stack.
+ * 		**BPF_F_USER_BUILD_ID**
+ * 			Collect buildid+offset instead of ips for user stack,
+ * 			only valid if **BPF_F_USER_STACK** is also specified.
+ *
+ * 		**bpf_get_stack**\ () can collect up to
+ * 		**PERF_MAX_STACK_DEPTH** both kernel and user frames, subject
+ * 		to sufficient large buffer size. Note that
+ * 		this limit can be controlled with the **sysctl** program, and
+ * 		that it should be manually increased in order to profile long
+ * 		user stacks (such as stacks for Java programs). To do so, use:
+ *
+ * 		::
+ *
+ * 			# sysctl kernel.perf_event_max_stack=<new value>
  *
  * 	Return
  * 		a non-negative value equal to or less than size on success, or
-- 
2.14.1

Re: [PATCH bpf-next 2/3] bpf: fix formatting for bpf_get_stack() helper doc

[Alexei Starovoitov]

On Mon, Apr 30, 2018 at 11:39:04AM +0100, Quentin Monnet wrote:
> Fix formatting (indent) for bpf_get_stack() helper documentation, so
> that the doc is rendered correctly with the Python script.
> 
> Fixes: c195651e565a ("bpf: add bpf_get_stack helper")
> Cc: Yonghong Song <yhs@fb.com>
> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
> ---
> 
> Note: The error was a missing space between the '*' marking the
> comments, and the tabs. This expected mixed indent comes from the fact I
> started to write the doc as a RST, then copied my contents (tabs
> included) in the header file and added a " * " (with a space) prefix
> everywhere.
> 
> On a second thought, using such indent style was maybe... not my best idea
> ever. Anyway, if indent for documenting eBPF helpers really gets to painful, we
> could relax parsing rules in the Python script to make things easier.
> ---
>  include/uapi/linux/bpf.h | 54 ++++++++++++++++++++++++------------------------
>  1 file changed, 27 insertions(+), 27 deletions(-)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 530ff6588d8f..8daef7326bb7 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1770,33 +1770,33 @@ union bpf_attr {
>   *
>   * int bpf_get_stack(struct pt_regs *regs, void *buf, u32 size, u64 flags)
>   * 	Description
> - *		Return a user or a kernel stack in bpf program provided buffer.
> - *		To achieve this, the helper needs *ctx*, which is a pointer
> + * 		Return a user or a kernel stack in bpf program provided buffer.
> + * 		To achieve this, the helper needs *ctx*, which is a pointer

I still don't quite get the difference.
It's replacing 2 tabs in above with 1 space + 2 tabs ?
Can you please teach the python script to accept both?
I bet that will be recurring mistake and it's impossible to spot in code review.

Re: [PATCH bpf-next 2/3] bpf: fix formatting for bpf_get_stack() helper doc

[David Ahern]

On 4/30/18 9:08 AM, Alexei Starovoitov wrote:
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 530ff6588d8f..8daef7326bb7 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -1770,33 +1770,33 @@ union bpf_attr {
>>   *
>>   * int bpf_get_stack(struct pt_regs *regs, void *buf, u32 size, u64 flags)
>>   * 	Description
>> - *		Return a user or a kernel stack in bpf program provided buffer.
>> - *		To achieve this, the helper needs *ctx*, which is a pointer
>> + * 		Return a user or a kernel stack in bpf program provided buffer.
>> + * 		To achieve this, the helper needs *ctx*, which is a pointer
> 
> I still don't quite get the difference.
> It's replacing 2 tabs in above with 1 space + 2 tabs ?
> Can you please teach the python script to accept both?
> I bet that will be recurring mistake and it's impossible to spot in code review.
> 

And checkpatch throws an error on the 1 space + 2 tabs so it gets
confusing on which format should be used.

Re: [PATCH bpf-next 2/3] bpf: fix formatting for bpf_get_stack() helper doc

[Quentin Monnet]

2018-04-30 09:12 UTC-0600 ~ David Ahern <dsahern@gmail.com>
> On 4/30/18 9:08 AM, Alexei Starovoitov wrote:
>>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>>> index 530ff6588d8f..8daef7326bb7 100644
>>> --- a/include/uapi/linux/bpf.h
>>> +++ b/include/uapi/linux/bpf.h
>>> @@ -1770,33 +1770,33 @@ union bpf_attr {
>>>   *
>>>   * int bpf_get_stack(struct pt_regs *regs, void *buf, u32 size, u64 flags)
>>>   * 	Description
>>> - *		Return a user or a kernel stack in bpf program provided buffer.
>>> - *		To achieve this, the helper needs *ctx*, which is a pointer
>>> + * 		Return a user or a kernel stack in bpf program provided buffer.
>>> + * 		To achieve this, the helper needs *ctx*, which is a pointer
>> I still don't quite get the difference.
>> It's replacing 2 tabs in above with 1 space + 2 tabs ?

Yes, exactly (Plus in this case, the "::" a few line below has a missing
tab).

>> Can you please teach the python script to accept both?
>> I bet that will be recurring mistake and it's impossible to spot in code review.
> And checkpatch throws an error on the 1 space + 2 tabs so it gets
> confusing on which format should be used.

Sorry about that :/. I will send a patch to make the script more flexible.

Quentin

[PATCH bpf-next 3/3] bpf: update bpf.h uapi header for tools

[Quentin Monnet]

Bring fixes for eBPF helper documentation formatting to bpf.h under
tools/ as well.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/include/uapi/linux/bpf.h | 62 +++++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 23b334bba1a6..8daef7326bb7 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -828,12 +828,12 @@ union bpf_attr {
  *
  * 		Also, be aware that the newer helper
  * 		**bpf_perf_event_read_value**\ () is recommended over
- * 		**bpf_perf_event_read*\ () in general. The latter has some ABI
+ * 		**bpf_perf_event_read**\ () in general. The latter has some ABI
  * 		quirks where error and counter value are used as a return code
  * 		(which is wrong to do since ranges may overlap). This issue is
- * 		fixed with bpf_perf_event_read_value(), which at the same time
- * 		provides more features over the **bpf_perf_event_read**\ ()
- * 		interface. Please refer to the description of
+ * 		fixed with **bpf_perf_event_read_value**\ (), which at the same
+ * 		time provides more features over the **bpf_perf_event_read**\
+ * 		() interface. Please refer to the description of
  * 		**bpf_perf_event_read_value**\ () for details.
  * 	Return
  * 		The value of the perf event counter read from the map, or a
@@ -1770,33 +1770,33 @@ union bpf_attr {
  *
  * int bpf_get_stack(struct pt_regs *regs, void *buf, u32 size, u64 flags)
  * 	Description
- *		Return a user or a kernel stack in bpf program provided buffer.
- *		To achieve this, the helper needs *ctx*, which is a pointer
- *		to the context on which the tracing program is executed.
- *		To store the stacktrace, the bpf program provides *buf* with
- *		a nonnegative *size*.
- *
- *		The last argument, *flags*, holds the number of stack frames to
- *		skip (from 0 to 255), masked with
- *		**BPF_F_SKIP_FIELD_MASK**. The next bits can be used to set
- *		the following flags:
- *
- *		**BPF_F_USER_STACK**
- *			Collect a user space stack instead of a kernel stack.
- *		**BPF_F_USER_BUILD_ID**
- *			Collect buildid+offset instead of ips for user stack,
- *			only valid if **BPF_F_USER_STACK** is also specified.
- *
- *		**bpf_get_stack**\ () can collect up to
- *		**PERF_MAX_STACK_DEPTH** both kernel and user frames, subject
- *		to sufficient large buffer size. Note that
- *		this limit can be controlled with the **sysctl** program, and
- *		that it should be manually increased in order to profile long
- *		user stacks (such as stacks for Java programs). To do so, use:
- *
- *	::
- *
- *		# sysctl kernel.perf_event_max_stack=<new value>
+ * 		Return a user or a kernel stack in bpf program provided buffer.
+ * 		To achieve this, the helper needs *ctx*, which is a pointer
+ * 		to the context on which the tracing program is executed.
+ * 		To store the stacktrace, the bpf program provides *buf* with
+ * 		a nonnegative *size*.
+ *
+ * 		The last argument, *flags*, holds the number of stack frames to
+ * 		skip (from 0 to 255), masked with
+ * 		**BPF_F_SKIP_FIELD_MASK**. The next bits can be used to set
+ * 		the following flags:
+ *
+ * 		**BPF_F_USER_STACK**
+ * 			Collect a user space stack instead of a kernel stack.
+ * 		**BPF_F_USER_BUILD_ID**
+ * 			Collect buildid+offset instead of ips for user stack,
+ * 			only valid if **BPF_F_USER_STACK** is also specified.
+ *
+ * 		**bpf_get_stack**\ () can collect up to
+ * 		**PERF_MAX_STACK_DEPTH** both kernel and user frames, subject
+ * 		to sufficient large buffer size. Note that
+ * 		this limit can be controlled with the **sysctl** program, and
+ * 		that it should be manually increased in order to profile long
+ * 		user stacks (such as stacks for Java programs). To do so, use:
+ *
+ * 		::
+ *
+ * 			# sysctl kernel.perf_event_max_stack=<new value>
  *
  * 	Return
  * 		a non-negative value equal to or less than size on success, or
-- 
2.14.1

Re: [PATCH bpf-next 0/3] bpf: fix formatting for eBPF helper doc

[Daniel Borkmann]

On 04/30/2018 12:39 PM, Quentin Monnet wrote:
> Here is a follow-up set for eBPF helper documentation, with two patches to
> fix formatting issues:
> 
> - One to fix an error I introduced with the initial set for the doc.
> - One for the newly added bpf_get_stack(), that is currently not parsed
>   correctly with the Python script (function signature is fine, but
>   description and return value appear as empty).
> 
> There is no change to text contents in this set.

Applied to bpf-next, thanks Quentin!