* [PATCH bpf-next 1/3] tools: bpftool: fix formatting in bpftool-link documentation
2020-09-04 16:14 [PATCH bpf-next 0/3] bpf: format fixes for BPF helpers and bpftool documentation Quentin Monnet
@ 2020-09-04 16:14 ` Quentin Monnet
2020-09-04 16:14 ` [PATCH bpf-next 2/3] bpf: fix formatting in documentation for BPF helpers Quentin Monnet
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Quentin Monnet @ 2020-09-04 16:14 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: bpf, netdev, Quentin Monnet
Fix a formatting error in the documentation for bpftool-link, so that
the man page can build correctly.
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
tools/bpf/bpftool/Documentation/bpftool-link.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-link.rst b/tools/bpf/bpftool/Documentation/bpftool-link.rst
index 4a52e7a93339..dc7693b5e606 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-link.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-link.rst
@@ -21,7 +21,7 @@ LINK COMMANDS
| **bpftool** **link { show | list }** [*LINK*]
| **bpftool** **link pin** *LINK* *FILE*
-| **bpftool** **link detach *LINK*
+| **bpftool** **link detach** *LINK*
| **bpftool** **link help**
|
| *LINK* := { **id** *LINK_ID* | **pinned** *FILE* }
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH bpf-next 2/3] bpf: fix formatting in documentation for BPF helpers
2020-09-04 16:14 [PATCH bpf-next 0/3] bpf: format fixes for BPF helpers and bpftool documentation Quentin Monnet
2020-09-04 16:14 ` [PATCH bpf-next 1/3] tools: bpftool: fix formatting in bpftool-link documentation Quentin Monnet
@ 2020-09-04 16:14 ` Quentin Monnet
2020-09-04 16:14 ` [PATCH bpf-next 3/3] tools, bpf: synchronise BPF UAPI header with tools Quentin Monnet
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Quentin Monnet @ 2020-09-04 16:14 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: bpf, netdev, Quentin Monnet
Fix a formatting error in the description of bpf_load_hdr_opt() (rst2man
complains about a wrong indentation, but what is missing is actually a
blank line before the bullet list).
Fix and harmonise the formatting for other helpers.
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
include/uapi/linux/bpf.h | 87 +++++++++++++++++++++-------------------
1 file changed, 45 insertions(+), 42 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 8dda13880957..90359cab501d 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -3349,38 +3349,38 @@ union bpf_attr {
* Description
* Dynamically cast a *sk* pointer to a *tcp6_sock* pointer.
* Return
- * *sk* if casting is valid, or NULL otherwise.
+ * *sk* if casting is valid, or **NULL** otherwise.
*
* struct tcp_sock *bpf_skc_to_tcp_sock(void *sk)
* Description
* Dynamically cast a *sk* pointer to a *tcp_sock* pointer.
* Return
- * *sk* if casting is valid, or NULL otherwise.
+ * *sk* if casting is valid, or **NULL** otherwise.
*
* struct tcp_timewait_sock *bpf_skc_to_tcp_timewait_sock(void *sk)
* Description
* Dynamically cast a *sk* pointer to a *tcp_timewait_sock* pointer.
* Return
- * *sk* if casting is valid, or NULL otherwise.
+ * *sk* if casting is valid, or **NULL** otherwise.
*
* struct tcp_request_sock *bpf_skc_to_tcp_request_sock(void *sk)
* Description
* Dynamically cast a *sk* pointer to a *tcp_request_sock* pointer.
* Return
- * *sk* if casting is valid, or NULL otherwise.
+ * *sk* if casting is valid, or **NULL** otherwise.
*
* struct udp6_sock *bpf_skc_to_udp6_sock(void *sk)
* Description
* Dynamically cast a *sk* pointer to a *udp6_sock* pointer.
* Return
- * *sk* if casting is valid, or NULL otherwise.
+ * *sk* if casting is valid, or **NULL** otherwise.
*
* long bpf_get_task_stack(struct task_struct *task, 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 *task*, which is a valid
- * pointer to struct task_struct. To store the stacktrace, the
- * bpf program provides *buf* with a nonnegative *size*.
+ * pointer to **struct task_struct**. 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
@@ -3410,12 +3410,12 @@ union bpf_attr {
* long bpf_load_hdr_opt(struct bpf_sock_ops *skops, void *searchby_res, u32 len, u64 flags)
* Description
* Load header option. Support reading a particular TCP header
- * option for bpf program (BPF_PROG_TYPE_SOCK_OPS).
+ * option for bpf program (**BPF_PROG_TYPE_SOCK_OPS**).
*
* If *flags* is 0, it will search the option from the
- * sock_ops->skb_data. The comment in "struct bpf_sock_ops"
+ * *skops*\ **->skb_data**. The comment in **struct bpf_sock_ops**
* has details on what skb_data contains under different
- * sock_ops->op.
+ * *skops*\ **->op**.
*
* The first byte of the *searchby_res* specifies the
* kind that it wants to search.
@@ -3435,7 +3435,7 @@ union bpf_attr {
* [ 254, 4, 0xeB, 0x9F, 0, 0, .... 0 ].
*
* To search for the standard window scale option (3),
- * the searchby_res should be [ 3, 0, 0, .... 0 ].
+ * the *searchby_res* should be [ 3, 0, 0, .... 0 ].
* Note, kind-length must be 0 for regular option.
*
* Searching for No-Op (0) and End-of-Option-List (1) are
@@ -3445,27 +3445,30 @@ union bpf_attr {
* of a header option.
*
* Supported flags:
+ *
* * **BPF_LOAD_HDR_OPT_TCP_SYN** to search from the
* saved_syn packet or the just-received syn packet.
*
* Return
- * >0 when found, the header option is copied to *searchby_res*.
- * The return value is the total length copied.
+ * > 0 when found, the header option is copied to *searchby_res*.
+ * The return value is the total length copied. On failure, a
+ * negative error code is returned:
*
- * **-EINVAL** If param is invalid
+ * **-EINVAL** if a parameter is invalid.
*
- * **-ENOMSG** The option is not found
+ * **-ENOMSG** if the option is not found.
*
- * **-ENOENT** No syn packet available when
- * **BPF_LOAD_HDR_OPT_TCP_SYN** is used
+ * **-ENOENT** if no syn packet is available when
+ * **BPF_LOAD_HDR_OPT_TCP_SYN** is used.
*
- * **-ENOSPC** Not enough space. Only *len* number of
- * bytes are copied.
+ * **-ENOSPC** if there is not enough space. Only *len* number of
+ * bytes are copied.
*
- * **-EFAULT** Cannot parse the header options in the packet
+ * **-EFAULT** on failure to parse the header options in the
+ * packet.
*
- * **-EPERM** This helper cannot be used under the
- * current sock_ops->op.
+ * **-EPERM** if the helper cannot be used under the current
+ * *skops*\ **->op**.
*
* long bpf_store_hdr_opt(struct bpf_sock_ops *skops, const void *from, u32 len, u64 flags)
* Description
@@ -3483,44 +3486,44 @@ union bpf_attr {
* by searching the same option in the outgoing skb.
*
* This helper can only be called during
- * BPF_SOCK_OPS_WRITE_HDR_OPT_CB.
+ * **BPF_SOCK_OPS_WRITE_HDR_OPT_CB**.
*
* Return
* 0 on success, or negative error in case of failure:
*
- * **-EINVAL** If param is invalid
+ * **-EINVAL** If param is invalid.
*
- * **-ENOSPC** Not enough space in the header.
- * Nothing has been written
+ * **-ENOSPC** if there is not enough space in the header.
+ * Nothing has been written
*
- * **-EEXIST** The option has already existed
+ * **-EEXIST** if the option already exists.
*
- * **-EFAULT** Cannot parse the existing header options
+ * **-EFAULT** on failrue to parse the existing header options.
*
- * **-EPERM** This helper cannot be used under the
- * current sock_ops->op.
+ * **-EPERM** if the helper cannot be used under the current
+ * *skops*\ **->op**.
*
* long bpf_reserve_hdr_opt(struct bpf_sock_ops *skops, u32 len, u64 flags)
* Description
* Reserve *len* bytes for the bpf header option. The
- * space will be used by bpf_store_hdr_opt() later in
- * BPF_SOCK_OPS_WRITE_HDR_OPT_CB.
+ * space will be used by **bpf_store_hdr_opt**\ () later in
+ * **BPF_SOCK_OPS_WRITE_HDR_OPT_CB**.
*
- * If bpf_reserve_hdr_opt() is called multiple times,
+ * If **bpf_reserve_hdr_opt**\ () is called multiple times,
* the total number of bytes will be reserved.
*
* This helper can only be called during
- * BPF_SOCK_OPS_HDR_OPT_LEN_CB.
+ * **BPF_SOCK_OPS_HDR_OPT_LEN_CB**.
*
* Return
* 0 on success, or negative error in case of failure:
*
- * **-EINVAL** if param is invalid
+ * **-EINVAL** if a parameter is invalid.
*
- * **-ENOSPC** Not enough space in the header.
+ * **-ENOSPC** if there is not enough space in the header.
*
- * **-EPERM** This helper cannot be used under the
- * current sock_ops->op.
+ * **-EPERM** if the helper cannot be used under the current
+ * *skops*\ **->op**.
*
* void *bpf_inode_storage_get(struct bpf_map *map, void *inode, void *value, u64 flags)
* Description
@@ -3560,9 +3563,9 @@ union bpf_attr {
*
* long bpf_d_path(struct path *path, char *buf, u32 sz)
* Description
- * Return full path for given 'struct path' object, which
- * needs to be the kernel BTF 'path' object. The path is
- * returned in the provided buffer 'buf' of size 'sz' and
+ * Return full path for given **struct path** object, which
+ * needs to be the kernel BTF *path* object. The path is
+ * returned in the provided buffer *buf* of size *sz* and
* is zero terminated.
*
* Return
@@ -3573,7 +3576,7 @@ union bpf_attr {
* long bpf_copy_from_user(void *dst, u32 size, const void *user_ptr)
* Description
* Read *size* bytes from user space address *user_ptr* and store
- * the data in *dst*. This is a wrapper of copy_from_user().
+ * the data in *dst*. This is a wrapper of **copy_from_user**\ ().
* Return
* 0 on success, or a negative error in case of failure.
*/
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH bpf-next 3/3] tools, bpf: synchronise BPF UAPI header with tools
2020-09-04 16:14 [PATCH bpf-next 0/3] bpf: format fixes for BPF helpers and bpftool documentation Quentin Monnet
2020-09-04 16:14 ` [PATCH bpf-next 1/3] tools: bpftool: fix formatting in bpftool-link documentation Quentin Monnet
2020-09-04 16:14 ` [PATCH bpf-next 2/3] bpf: fix formatting in documentation for BPF helpers Quentin Monnet
@ 2020-09-04 16:14 ` Quentin Monnet
2020-09-04 21:40 ` [PATCH bpf-next 0/3] bpf: format fixes for BPF helpers and bpftool documentation Andrii Nakryiko
2020-09-07 14:35 ` Daniel Borkmann
4 siblings, 0 replies; 8+ messages in thread
From: Quentin Monnet @ 2020-09-04 16:14 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: bpf, netdev, Quentin Monnet
Synchronise the bpf.h header under tools, to report the fixes recently
brought to the documentation for the BPF helpers.
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
tools/include/uapi/linux/bpf.h | 87 ++++++++++++++++++----------------
1 file changed, 45 insertions(+), 42 deletions(-)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 8dda13880957..90359cab501d 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -3349,38 +3349,38 @@ union bpf_attr {
* Description
* Dynamically cast a *sk* pointer to a *tcp6_sock* pointer.
* Return
- * *sk* if casting is valid, or NULL otherwise.
+ * *sk* if casting is valid, or **NULL** otherwise.
*
* struct tcp_sock *bpf_skc_to_tcp_sock(void *sk)
* Description
* Dynamically cast a *sk* pointer to a *tcp_sock* pointer.
* Return
- * *sk* if casting is valid, or NULL otherwise.
+ * *sk* if casting is valid, or **NULL** otherwise.
*
* struct tcp_timewait_sock *bpf_skc_to_tcp_timewait_sock(void *sk)
* Description
* Dynamically cast a *sk* pointer to a *tcp_timewait_sock* pointer.
* Return
- * *sk* if casting is valid, or NULL otherwise.
+ * *sk* if casting is valid, or **NULL** otherwise.
*
* struct tcp_request_sock *bpf_skc_to_tcp_request_sock(void *sk)
* Description
* Dynamically cast a *sk* pointer to a *tcp_request_sock* pointer.
* Return
- * *sk* if casting is valid, or NULL otherwise.
+ * *sk* if casting is valid, or **NULL** otherwise.
*
* struct udp6_sock *bpf_skc_to_udp6_sock(void *sk)
* Description
* Dynamically cast a *sk* pointer to a *udp6_sock* pointer.
* Return
- * *sk* if casting is valid, or NULL otherwise.
+ * *sk* if casting is valid, or **NULL** otherwise.
*
* long bpf_get_task_stack(struct task_struct *task, 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 *task*, which is a valid
- * pointer to struct task_struct. To store the stacktrace, the
- * bpf program provides *buf* with a nonnegative *size*.
+ * pointer to **struct task_struct**. 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
@@ -3410,12 +3410,12 @@ union bpf_attr {
* long bpf_load_hdr_opt(struct bpf_sock_ops *skops, void *searchby_res, u32 len, u64 flags)
* Description
* Load header option. Support reading a particular TCP header
- * option for bpf program (BPF_PROG_TYPE_SOCK_OPS).
+ * option for bpf program (**BPF_PROG_TYPE_SOCK_OPS**).
*
* If *flags* is 0, it will search the option from the
- * sock_ops->skb_data. The comment in "struct bpf_sock_ops"
+ * *skops*\ **->skb_data**. The comment in **struct bpf_sock_ops**
* has details on what skb_data contains under different
- * sock_ops->op.
+ * *skops*\ **->op**.
*
* The first byte of the *searchby_res* specifies the
* kind that it wants to search.
@@ -3435,7 +3435,7 @@ union bpf_attr {
* [ 254, 4, 0xeB, 0x9F, 0, 0, .... 0 ].
*
* To search for the standard window scale option (3),
- * the searchby_res should be [ 3, 0, 0, .... 0 ].
+ * the *searchby_res* should be [ 3, 0, 0, .... 0 ].
* Note, kind-length must be 0 for regular option.
*
* Searching for No-Op (0) and End-of-Option-List (1) are
@@ -3445,27 +3445,30 @@ union bpf_attr {
* of a header option.
*
* Supported flags:
+ *
* * **BPF_LOAD_HDR_OPT_TCP_SYN** to search from the
* saved_syn packet or the just-received syn packet.
*
* Return
- * >0 when found, the header option is copied to *searchby_res*.
- * The return value is the total length copied.
+ * > 0 when found, the header option is copied to *searchby_res*.
+ * The return value is the total length copied. On failure, a
+ * negative error code is returned:
*
- * **-EINVAL** If param is invalid
+ * **-EINVAL** if a parameter is invalid.
*
- * **-ENOMSG** The option is not found
+ * **-ENOMSG** if the option is not found.
*
- * **-ENOENT** No syn packet available when
- * **BPF_LOAD_HDR_OPT_TCP_SYN** is used
+ * **-ENOENT** if no syn packet is available when
+ * **BPF_LOAD_HDR_OPT_TCP_SYN** is used.
*
- * **-ENOSPC** Not enough space. Only *len* number of
- * bytes are copied.
+ * **-ENOSPC** if there is not enough space. Only *len* number of
+ * bytes are copied.
*
- * **-EFAULT** Cannot parse the header options in the packet
+ * **-EFAULT** on failure to parse the header options in the
+ * packet.
*
- * **-EPERM** This helper cannot be used under the
- * current sock_ops->op.
+ * **-EPERM** if the helper cannot be used under the current
+ * *skops*\ **->op**.
*
* long bpf_store_hdr_opt(struct bpf_sock_ops *skops, const void *from, u32 len, u64 flags)
* Description
@@ -3483,44 +3486,44 @@ union bpf_attr {
* by searching the same option in the outgoing skb.
*
* This helper can only be called during
- * BPF_SOCK_OPS_WRITE_HDR_OPT_CB.
+ * **BPF_SOCK_OPS_WRITE_HDR_OPT_CB**.
*
* Return
* 0 on success, or negative error in case of failure:
*
- * **-EINVAL** If param is invalid
+ * **-EINVAL** If param is invalid.
*
- * **-ENOSPC** Not enough space in the header.
- * Nothing has been written
+ * **-ENOSPC** if there is not enough space in the header.
+ * Nothing has been written
*
- * **-EEXIST** The option has already existed
+ * **-EEXIST** if the option already exists.
*
- * **-EFAULT** Cannot parse the existing header options
+ * **-EFAULT** on failrue to parse the existing header options.
*
- * **-EPERM** This helper cannot be used under the
- * current sock_ops->op.
+ * **-EPERM** if the helper cannot be used under the current
+ * *skops*\ **->op**.
*
* long bpf_reserve_hdr_opt(struct bpf_sock_ops *skops, u32 len, u64 flags)
* Description
* Reserve *len* bytes for the bpf header option. The
- * space will be used by bpf_store_hdr_opt() later in
- * BPF_SOCK_OPS_WRITE_HDR_OPT_CB.
+ * space will be used by **bpf_store_hdr_opt**\ () later in
+ * **BPF_SOCK_OPS_WRITE_HDR_OPT_CB**.
*
- * If bpf_reserve_hdr_opt() is called multiple times,
+ * If **bpf_reserve_hdr_opt**\ () is called multiple times,
* the total number of bytes will be reserved.
*
* This helper can only be called during
- * BPF_SOCK_OPS_HDR_OPT_LEN_CB.
+ * **BPF_SOCK_OPS_HDR_OPT_LEN_CB**.
*
* Return
* 0 on success, or negative error in case of failure:
*
- * **-EINVAL** if param is invalid
+ * **-EINVAL** if a parameter is invalid.
*
- * **-ENOSPC** Not enough space in the header.
+ * **-ENOSPC** if there is not enough space in the header.
*
- * **-EPERM** This helper cannot be used under the
- * current sock_ops->op.
+ * **-EPERM** if the helper cannot be used under the current
+ * *skops*\ **->op**.
*
* void *bpf_inode_storage_get(struct bpf_map *map, void *inode, void *value, u64 flags)
* Description
@@ -3560,9 +3563,9 @@ union bpf_attr {
*
* long bpf_d_path(struct path *path, char *buf, u32 sz)
* Description
- * Return full path for given 'struct path' object, which
- * needs to be the kernel BTF 'path' object. The path is
- * returned in the provided buffer 'buf' of size 'sz' and
+ * Return full path for given **struct path** object, which
+ * needs to be the kernel BTF *path* object. The path is
+ * returned in the provided buffer *buf* of size *sz* and
* is zero terminated.
*
* Return
@@ -3573,7 +3576,7 @@ union bpf_attr {
* long bpf_copy_from_user(void *dst, u32 size, const void *user_ptr)
* Description
* Read *size* bytes from user space address *user_ptr* and store
- * the data in *dst*. This is a wrapper of copy_from_user().
+ * the data in *dst*. This is a wrapper of **copy_from_user**\ ().
* Return
* 0 on success, or a negative error in case of failure.
*/
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH bpf-next 0/3] bpf: format fixes for BPF helpers and bpftool documentation
2020-09-04 16:14 [PATCH bpf-next 0/3] bpf: format fixes for BPF helpers and bpftool documentation Quentin Monnet
` (2 preceding siblings ...)
2020-09-04 16:14 ` [PATCH bpf-next 3/3] tools, bpf: synchronise BPF UAPI header with tools Quentin Monnet
@ 2020-09-04 21:40 ` Andrii Nakryiko
2020-09-07 14:48 ` Quentin Monnet
2020-09-07 14:35 ` Daniel Borkmann
4 siblings, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2020-09-04 21:40 UTC (permalink / raw)
To: Quentin Monnet; +Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Networking
On Fri, Sep 4, 2020 at 9:15 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> This series contains minor fixes (or harmonisation edits) for the
> bpftool-link documentation (first patch) and BPF helpers documentation
> (last two patches), so that all related man pages can build without errors.
>
> Quentin Monnet (3):
> tools: bpftool: fix formatting in bpftool-link documentation
> bpf: fix formatting in documentation for BPF helpers
> tools, bpf: synchronise BPF UAPI header with tools
>
> include/uapi/linux/bpf.h | 87 ++++++++++---------
> .../bpftool/Documentation/bpftool-link.rst | 2 +-
> tools/include/uapi/linux/bpf.h | 87 ++++++++++---------
> 3 files changed, 91 insertions(+), 85 deletions(-)
>
> --
> 2.20.1
>
This obviously looks good to me:
Acked-by: Andrii Nakryiko <andriin@fb.com>
But do you think we can somehow prevent issues like this? Consider
adding building/testing of documentation to selftests or something.
Not sure if that will catch all the issues you've fixed, but that
would be a good start.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH bpf-next 0/3] bpf: format fixes for BPF helpers and bpftool documentation
2020-09-04 21:40 ` [PATCH bpf-next 0/3] bpf: format fixes for BPF helpers and bpftool documentation Andrii Nakryiko
@ 2020-09-07 14:48 ` Quentin Monnet
2020-09-08 20:09 ` Andrii Nakryiko
0 siblings, 1 reply; 8+ messages in thread
From: Quentin Monnet @ 2020-09-07 14:48 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Networking
On 04/09/2020 22:40, Andrii Nakryiko wrote:
> On Fri, Sep 4, 2020 at 9:15 AM Quentin Monnet <quentin@isovalent.com> wrote:
>>
>> This series contains minor fixes (or harmonisation edits) for the
>> bpftool-link documentation (first patch) and BPF helpers documentation
>> (last two patches), so that all related man pages can build without errors.
>>
>> Quentin Monnet (3):
>> tools: bpftool: fix formatting in bpftool-link documentation
>> bpf: fix formatting in documentation for BPF helpers
>> tools, bpf: synchronise BPF UAPI header with tools
>>
>> include/uapi/linux/bpf.h | 87 ++++++++++---------
>> .../bpftool/Documentation/bpftool-link.rst | 2 +-
>> tools/include/uapi/linux/bpf.h | 87 ++++++++++---------
>> 3 files changed, 91 insertions(+), 85 deletions(-)
>>
>> --
>> 2.20.1
>>
>
> This obviously looks good to me:
>
> Acked-by: Andrii Nakryiko <andriin@fb.com>
>
> But do you think we can somehow prevent issues like this? Consider
> adding building/testing of documentation to selftests or something.
> Not sure if that will catch all the issues you've fixed, but that
> would be a good start.
>
Thanks for the review.
As for preventing future issues, I see two cases. Some minor fixes done
to harmonise the look of the description for the different helpers could
be checked with some kind of dedicated checkpatch-like script that would
validate new helpers I suppose, but I'm not sure whether that's worth
the trouble of creating that script, creating rules and then enforcing them.
The issues that do raise warnings are more important to fix, and easier
to detect. We could simply build bpftool's documentation (which also
happens to build the doc for eBPF helpers) and checks for warnings. We
already have a script to test bpftool build in the selftests, so I can
add it there as a follow-up and make doc build fail on warnings.
On a somewhat related note I also started to work on a script to check
that bpftool is correctly sync'ed (with kernel regarding prog names /
map names etc., and between source code / doc / bash completion) but I
haven't found the time to finish that work yet.
Quentin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next 0/3] bpf: format fixes for BPF helpers and bpftool documentation
2020-09-07 14:48 ` Quentin Monnet
@ 2020-09-08 20:09 ` Andrii Nakryiko
0 siblings, 0 replies; 8+ messages in thread
From: Andrii Nakryiko @ 2020-09-08 20:09 UTC (permalink / raw)
To: Quentin Monnet; +Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Networking
On Mon, Sep 7, 2020 at 7:48 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> On 04/09/2020 22:40, Andrii Nakryiko wrote:
> > On Fri, Sep 4, 2020 at 9:15 AM Quentin Monnet <quentin@isovalent.com> wrote:
> >>
> >> This series contains minor fixes (or harmonisation edits) for the
> >> bpftool-link documentation (first patch) and BPF helpers documentation
> >> (last two patches), so that all related man pages can build without errors.
> >>
> >> Quentin Monnet (3):
> >> tools: bpftool: fix formatting in bpftool-link documentation
> >> bpf: fix formatting in documentation for BPF helpers
> >> tools, bpf: synchronise BPF UAPI header with tools
> >>
> >> include/uapi/linux/bpf.h | 87 ++++++++++---------
> >> .../bpftool/Documentation/bpftool-link.rst | 2 +-
> >> tools/include/uapi/linux/bpf.h | 87 ++++++++++---------
> >> 3 files changed, 91 insertions(+), 85 deletions(-)
> >>
> >> --
> >> 2.20.1
> >>
> >
> > This obviously looks good to me:
> >
> > Acked-by: Andrii Nakryiko <andriin@fb.com>
> >
> > But do you think we can somehow prevent issues like this? Consider
> > adding building/testing of documentation to selftests or something.
> > Not sure if that will catch all the issues you've fixed, but that
> > would be a good start.
> >
>
> Thanks for the review.
>
> As for preventing future issues, I see two cases. Some minor fixes done
> to harmonise the look of the description for the different helpers could
> be checked with some kind of dedicated checkpatch-like script that would
> validate new helpers I suppose, but I'm not sure whether that's worth
> the trouble of creating that script, creating rules and then enforcing them.
>
> The issues that do raise warnings are more important to fix, and easier
> to detect. We could simply build bpftool's documentation (which also
> happens to build the doc for eBPF helpers) and checks for warnings. We
> already have a script to test bpftool build in the selftests, so I can
> add it there as a follow-up and make doc build fail on warnings.
Yeah, that would be good. I constantly forget to try building
documentation for bpftool, so having this automated would be an
improvement (provided building docs doesn't require unreasonable
dependencies).
>
> On a somewhat related note I also started to work on a script to check
> that bpftool is correctly sync'ed (with kernel regarding prog names /
> map names etc., and between source code / doc / bash completion) but I
> haven't found the time to finish that work yet.
>
> Quentin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next 0/3] bpf: format fixes for BPF helpers and bpftool documentation
2020-09-04 16:14 [PATCH bpf-next 0/3] bpf: format fixes for BPF helpers and bpftool documentation Quentin Monnet
` (3 preceding siblings ...)
2020-09-04 21:40 ` [PATCH bpf-next 0/3] bpf: format fixes for BPF helpers and bpftool documentation Andrii Nakryiko
@ 2020-09-07 14:35 ` Daniel Borkmann
4 siblings, 0 replies; 8+ messages in thread
From: Daniel Borkmann @ 2020-09-07 14:35 UTC (permalink / raw)
To: Quentin Monnet, Alexei Starovoitov; +Cc: bpf, netdev
On 9/4/20 6:14 PM, Quentin Monnet wrote:
> This series contains minor fixes (or harmonisation edits) for the
> bpftool-link documentation (first patch) and BPF helpers documentation
> (last two patches), so that all related man pages can build without errors.
>
> Quentin Monnet (3):
> tools: bpftool: fix formatting in bpftool-link documentation
> bpf: fix formatting in documentation for BPF helpers
> tools, bpf: synchronise BPF UAPI header with tools
>
> include/uapi/linux/bpf.h | 87 ++++++++++---------
> .../bpftool/Documentation/bpftool-link.rst | 2 +-
> tools/include/uapi/linux/bpf.h | 87 ++++++++++---------
> 3 files changed, 91 insertions(+), 85 deletions(-)
>
Applied, thanks!
^ permalink raw reply [flat|nested] 8+ messages in thread