[PATCH bpf-next v3] bpf: Fix a few typos in BPF helpers documentation

[PATCH bpf-next v3] bpf: Fix a few typos in BPF helpers documentation

[Quentin Monnet]

Address a few typos in the documentation for the BPF helper functions.
They were reported by Jakub [0], who ran spell checkers on the generated
man page [1].

[0] https://lore.kernel.org/linux-man/d22dcd47-023c-8f52-d369-7b5308e6c842@gmail.com/T/#mb02e7d4b7fb61d98fa914c77b581184e9a9537af
[1] https://lore.kernel.org/linux-man/eb6a1e41-c48e-ac45-5154-ac57a2c76108@gmail.com/T/#m4a8d1b003616928013ffcd1450437309ab652f9f

v3: Do not copy unrelated (and breaking) elements to tools/ header
v2: Turn a ',' into a ';'

Cc: Alejandro Colomar <alx.manpages@gmail.com>
Cc: Jakub Wilk <jwilk@jwilk.net>
Cc: Jesper Dangaard Brouer <brouer@redhat.com>
Cc: linux-man@vger.kernel.org
Reported-by: Jakub Wilk <jwilk@jwilk.net>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
 include/uapi/linux/bpf.h       | 16 ++++++++--------
 tools/include/uapi/linux/bpf.h | 16 ++++++++--------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 0f61f09f467a..01c54a462352 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -4456,7 +4456,7 @@ union bpf_attr {
  *
  *		**-EEXIST** if the option already exists.
  *
- *		**-EFAULT** on failrue to parse the existing header options.
+ *		**-EFAULT** on failure to parse the existing header options.
  *
  *		**-EPERM** if the helper cannot be used under the current
  *		*skops*\ **->op**.
@@ -4665,7 +4665,7 @@ union bpf_attr {
  *		a *map* with *task* as the **key**.  From this
  *		perspective,  the usage is not much different from
  *		**bpf_map_lookup_elem**\ (*map*, **&**\ *task*) except this
- *		helper enforces the key must be an task_struct and the map must also
+ *		helper enforces the key must be a task_struct and the map must also
  *		be a **BPF_MAP_TYPE_TASK_STORAGE**.
  *
  *		Underneath, the value is stored locally at *task* instead of
@@ -4723,7 +4723,7 @@ union bpf_attr {
  *
  * long bpf_ima_inode_hash(struct inode *inode, void *dst, u32 size)
  *	Description
- *		Returns the stored IMA hash of the *inode* (if it's avaialable).
+ *		Returns the stored IMA hash of the *inode* (if it's available).
  *		If the hash is larger than *size*, then only *size*
  *		bytes will be copied to *dst*
  *	Return
@@ -4747,12 +4747,12 @@ union bpf_attr {
  *
  *		The argument *len_diff* can be used for querying with a planned
  *		size change. This allows to check MTU prior to changing packet
- *		ctx. Providing an *len_diff* adjustment that is larger than the
+ *		ctx. Providing a *len_diff* adjustment that is larger than the
  *		actual packet size (resulting in negative packet size) will in
- *		principle not exceed the MTU, why it is not considered a
- *		failure.  Other BPF-helpers are needed for performing the
- *		planned size change, why the responsability for catch a negative
- *		packet size belong in those helpers.
+ *		principle not exceed the MTU, which is why it is not considered
+ *		a failure.  Other BPF helpers are needed for performing the
+ *		planned size change; therefore the responsibility for catching
+ *		a negative packet size belongs in those helpers.
  *
  *		Specifying *ifindex* zero means the MTU check is performed
  *		against the current net device.  This is practical if this isn't
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 5056cef2112f..d45dda46aa42 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -4456,7 +4456,7 @@ union bpf_attr {
  *
  *		**-EEXIST** if the option already exists.
  *
- *		**-EFAULT** on failrue to parse the existing header options.
+ *		**-EFAULT** on failure to parse the existing header options.
  *
  *		**-EPERM** if the helper cannot be used under the current
  *		*skops*\ **->op**.
@@ -4665,7 +4665,7 @@ union bpf_attr {
  *		a *map* with *task* as the **key**.  From this
  *		perspective,  the usage is not much different from
  *		**bpf_map_lookup_elem**\ (*map*, **&**\ *task*) except this
- *		helper enforces the key must be an task_struct and the map must also
+ *		helper enforces the key must be a task_struct and the map must also
  *		be a **BPF_MAP_TYPE_TASK_STORAGE**.
  *
  *		Underneath, the value is stored locally at *task* instead of
@@ -4723,7 +4723,7 @@ union bpf_attr {
  *
  * long bpf_ima_inode_hash(struct inode *inode, void *dst, u32 size)
  *	Description
- *		Returns the stored IMA hash of the *inode* (if it's avaialable).
+ *		Returns the stored IMA hash of the *inode* (if it's available).
  *		If the hash is larger than *size*, then only *size*
  *		bytes will be copied to *dst*
  *	Return
@@ -4747,12 +4747,12 @@ union bpf_attr {
  *
  *		The argument *len_diff* can be used for querying with a planned
  *		size change. This allows to check MTU prior to changing packet
- *		ctx. Providing an *len_diff* adjustment that is larger than the
+ *		ctx. Providing a *len_diff* adjustment that is larger than the
  *		actual packet size (resulting in negative packet size) will in
- *		principle not exceed the MTU, why it is not considered a
- *		failure.  Other BPF-helpers are needed for performing the
- *		planned size change, why the responsability for catch a negative
- *		packet size belong in those helpers.
+ *		principle not exceed the MTU, which is why it is not considered
+ *		a failure.  Other BPF helpers are needed for performing the
+ *		planned size change; therefore the responsibility for catching
+ *		a negative packet size belongs in those helpers.
  *
  *		Specifying *ifindex* zero means the MTU check is performed
  *		against the current net device.  This is practical if this isn't
-- 
2.34.1

Re: [PATCH bpf-next v3] bpf: Fix a few typos in BPF helpers documentation

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 6486 bytes --]

Hi Quentin,

On 8/26/22 00:08, Quentin Monnet wrote:
> Address a few typos in the documentation for the BPF helper functions.
> They were reported by Jakub [0], who ran spell checkers on the generated
> man page [1].
> 
> [0] https://lore.kernel.org/linux-man/d22dcd47-023c-8f52-d369-7b5308e6c842@gmail.com/T/#mb02e7d4b7fb61d98fa914c77b581184e9a9537af
> [1] https://lore.kernel.org/linux-man/eb6a1e41-c48e-ac45-5154-ac57a2c76108@gmail.com/T/#m4a8d1b003616928013ffcd1450437309ab652f9f
> 
> v3: Do not copy unrelated (and breaking) elements to tools/ header
> v2: Turn a ',' into a ';'
> 
> Cc: Alejandro Colomar <alx.manpages@gmail.com>
> Cc: Jakub Wilk <jwilk@jwilk.net>
> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
> Cc: linux-man@vger.kernel.org
> Reported-by: Jakub Wilk <jwilk@jwilk.net>
> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
> ---
>   include/uapi/linux/bpf.h       | 16 ++++++++--------
>   tools/include/uapi/linux/bpf.h | 16 ++++++++--------
>   2 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 0f61f09f467a..01c54a462352 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -4456,7 +4456,7 @@ union bpf_attr {
>    *
>    *		**-EEXIST** if the option already exists.
>    *
> - *		**-EFAULT** on failrue to parse the existing header options.
> + *		**-EFAULT** on failure to parse the existing header options.
>    *
>    *		**-EPERM** if the helper cannot be used under the current
>    *		*skops*\ **->op**.
> @@ -4665,7 +4665,7 @@ union bpf_attr {
>    *		a *map* with *task* as the **key**.  From this
>    *		perspective,  the usage is not much different from
>    *		**bpf_map_lookup_elem**\ (*map*, **&**\ *task*) except this
> - *		helper enforces the key must be an task_struct and the map must also
> + *		helper enforces the key must be a task_struct and the map must also
>    *		be a **BPF_MAP_TYPE_TASK_STORAGE**.
>    *
>    *		Underneath, the value is stored locally at *task* instead of
> @@ -4723,7 +4723,7 @@ union bpf_attr {
>    *
>    * long bpf_ima_inode_hash(struct inode *inode, void *dst, u32 size)
>    *	Description
> - *		Returns the stored IMA hash of the *inode* (if it's avaialable).
> + *		Returns the stored IMA hash of the *inode* (if it's available).
>    *		If the hash is larger than *size*, then only *size*
>    *		bytes will be copied to *dst*
>    *	Return
> @@ -4747,12 +4747,12 @@ union bpf_attr {
>    *
>    *		The argument *len_diff* can be used for querying with a planned
>    *		size change. This allows to check MTU prior to changing packet
> - *		ctx. Providing an *len_diff* adjustment that is larger than the

I just noticed:  groff(1) uses double spaces after an end-of-sentence 
period.  Otherwise, it is understood as something like initials, or an 
abbreviature, and it causes some issues.  Please check the whole 
document, as I've seen a mix of styles.

Search for something like '.\. [^ ]'

Cheers,

Alex

> + *		ctx. Providing a *len_diff* adjustment that is larger than the
>    *		actual packet size (resulting in negative packet size) will in
> - *		principle not exceed the MTU, why it is not considered a
> - *		failure.  Other BPF-helpers are needed for performing the
> - *		planned size change, why the responsability for catch a negative
> - *		packet size belong in those helpers.
> + *		principle not exceed the MTU, which is why it is not considered
> + *		a failure.  Other BPF helpers are needed for performing the
> + *		planned size change; therefore the responsibility for catching
> + *		a negative packet size belongs in those helpers.
>    *
>    *		Specifying *ifindex* zero means the MTU check is performed
>    *		against the current net device.  This is practical if this isn't
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 5056cef2112f..d45dda46aa42 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -4456,7 +4456,7 @@ union bpf_attr {
>    *
>    *		**-EEXIST** if the option already exists.
>    *
> - *		**-EFAULT** on failrue to parse the existing header options.
> + *		**-EFAULT** on failure to parse the existing header options.
>    *
>    *		**-EPERM** if the helper cannot be used under the current
>    *		*skops*\ **->op**.
> @@ -4665,7 +4665,7 @@ union bpf_attr {
>    *		a *map* with *task* as the **key**.  From this
>    *		perspective,  the usage is not much different from
>    *		**bpf_map_lookup_elem**\ (*map*, **&**\ *task*) except this
> - *		helper enforces the key must be an task_struct and the map must also
> + *		helper enforces the key must be a task_struct and the map must also
>    *		be a **BPF_MAP_TYPE_TASK_STORAGE**.
>    *
>    *		Underneath, the value is stored locally at *task* instead of
> @@ -4723,7 +4723,7 @@ union bpf_attr {
>    *
>    * long bpf_ima_inode_hash(struct inode *inode, void *dst, u32 size)
>    *	Description
> - *		Returns the stored IMA hash of the *inode* (if it's avaialable).
> + *		Returns the stored IMA hash of the *inode* (if it's available).
>    *		If the hash is larger than *size*, then only *size*
>    *		bytes will be copied to *dst*
>    *	Return
> @@ -4747,12 +4747,12 @@ union bpf_attr {
>    *
>    *		The argument *len_diff* can be used for querying with a planned
>    *		size change. This allows to check MTU prior to changing packet
> - *		ctx. Providing an *len_diff* adjustment that is larger than the
> + *		ctx. Providing a *len_diff* adjustment that is larger than the
>    *		actual packet size (resulting in negative packet size) will in
> - *		principle not exceed the MTU, why it is not considered a
> - *		failure.  Other BPF-helpers are needed for performing the
> - *		planned size change, why the responsability for catch a negative
> - *		packet size belong in those helpers.
> + *		principle not exceed the MTU, which is why it is not considered
> + *		a failure.  Other BPF helpers are needed for performing the
> + *		planned size change; therefore the responsibility for catching
> + *		a negative packet size belongs in those helpers.
>    *
>    *		Specifying *ifindex* zero means the MTU check is performed
>    *		against the current net device.  This is practical if this isn't

-- 
Alejandro Colomar
<http://www.alejandro-colomar.es/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH bpf-next v3] bpf: Fix a few typos in BPF helpers documentation

[Quentin Monnet]

On 25/08/2022 23:12, Alejandro Colomar wrote:
> Hi Quentin,
> 

>> - *        ctx. Providing an *len_diff* adjustment that is larger than
>> the
> 
> I just noticed:  groff(1) uses double spaces after an end-of-sentence
> period.  Otherwise, it is understood as something like initials, or an
> abbreviature, and it causes some issues.  Please check the whole
> document, as I've seen a mix of styles.
> 
> Search for something like '.\. [^ ]'

This is a strange restriction in my opinion, but I can look into this as
a follow-up. I've not noticed issues with the rendered page so far, out
of curiosity what issues are we talking about?

Also before that, it would be good to sync and see what other formatting
elements need be addressed on the page, so we can fix them in a batch
rather than submitting them one after the other like we're doing.

Quentin

Re: [PATCH bpf-next v3] bpf: Fix a few typos in BPF helpers documentation

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 3129 bytes --]

Hi Quentin,

On 8/26/22 11:44, Quentin Monnet wrote:
> On 25/08/2022 23:12, Alejandro Colomar wrote:
>> Hi Quentin,
>>
> 
>>> - *        ctx. Providing an *len_diff* adjustment that is larger than
>>> the
>>
>> I just noticed:  groff(1) uses double spaces after an end-of-sentence
>> period.  Otherwise, it is understood as something like initials, or an
>> abbreviature, and it causes some issues.  Please check the whole
>> document, as I've seen a mix of styles.
>>
>> Search for something like '.\. [^ ]'
> 
> This is a strange restriction in my opinion, but I can look into this as
> a follow-up. I've not noticed issues with the rendered page so far, out
> of curiosity what issues are we talking about?

It's not so visible, and I'm not a groff(1) expert, so maybe there are 
more issues than the ones I know, but I'll explain it as I understand it:

For groff's output, there are two kinds of spaces: interword and 
intersentence spaces.  Interword space is normally a single character in 
monospaced fonts.  Intersentence is also a single space by default in 
monospace fonts, but it is not substituting interword space, but rather 
adding to it, so effectively the intersentence separation is two spaces 
in a monospaced font.  That can be configured, and one can for example 
ask their intersentence space to be 2 chars, and therefore have an 
intersentence effective sepparation of 3 chars.

In PDF output, the difference may be also noticeable slightly differently.

I prepared a simple file that will show you how it can make sentences 
much more readable, even if the theoretical difference might not be 
noticeable at first glance to the untrained eye:

$ cat sp.man
.TH spaces 7 today experiments
.SH correct spacing
Hello world!  Today is Friday.  This are extra words to fill.
And even more words.
.SH incorrect spacing
Hello world! Today is Monday. This are extra words to fill. And even 
more words.
$ man -P cat ./sp.man
spaces(7)          Miscellaneous Information Manual          spaces(7)

correct spacing
        Hello  world!   Today is Friday.  This are extra words to fill.
        And even more words.

incorrect spacing
        Hello world! Today is Monday. This are extra words to fill. And
        even more words.

experiments                      today                       spaces(7)



Notice how the first one is much more nicely rendered.  I rendered it in 
a 72-col terminal because my mailer would wrap at that boundary anyway. 
You can render the file at 80 columns and see a different rendering, 
where it is even bigger the difference in favor of the correctly written 
one.


> 
> Also before that, it would be good to sync and see what other formatting
> elements need be addressed on the page, so we can fix them in a batch
> rather than submitting them one after the other like we're doing.

Sure!  It'll take some time from my side, but I'll try to come up with a 
list of issues in that page.

> 
> Quentin


Cheers,

Alex

-- 
Alejandro Colomar
<http://www.alejandro-colomar.es/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH bpf-next v3] bpf: Fix a few typos in BPF helpers documentation

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Thu, 25 Aug 2022 23:08:06 +0100 you wrote:
> Address a few typos in the documentation for the BPF helper functions.
> They were reported by Jakub [0], who ran spell checkers on the generated
> man page [1].
> 
> [0] https://lore.kernel.org/linux-man/d22dcd47-023c-8f52-d369-7b5308e6c842@gmail.com/T/#mb02e7d4b7fb61d98fa914c77b581184e9a9537af
> [1] https://lore.kernel.org/linux-man/eb6a1e41-c48e-ac45-5154-ac57a2c76108@gmail.com/T/#m4a8d1b003616928013ffcd1450437309ab652f9f
> 
> [...]

Here is the summary with links:
  - [bpf-next,v3] bpf: Fix a few typos in BPF helpers documentation
    https://git.kernel.org/bpf/bpf-next/c/aa75622c3be4

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html