| Message ID | 20220825220806.107143-1-quentin@isovalent.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | aa75622c3be4d5819ce69c714acbcbd67bba5d65 |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next,v3] bpf: Fix a few typos in BPF helpers documentation | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-next-PR | fail | PR summary |
| bpf/vmtest-bpf-next-VM_Test-2 | fail | Logs for build for s390x with gcc |
| bpf/vmtest-bpf-next-VM_Test-3 | fail | Logs for build for x86_64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-6 | success | Logs for set-matrix |
| bpf/vmtest-bpf-next-VM_Test-1 | pending | Logs for build for s390x with gcc |
| bpf/vmtest-bpf-next-VM_Test-4 | success | Logs for llvm-toolchain |
| bpf/vmtest-bpf-next-VM_Test-5 | success | Logs for set-matrix |
| netdev/tree_selection | success | Clearly marked for bpf-next, async |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Single patches do not need cover letters |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 1697 this patch: 1697 |
| netdev/cc_maintainers | warning | 2 maintainers not CCed: song@kernel.org martin.lau@linux.dev |
| netdev/build_clang | success | Errors and warnings before: 173 this patch: 173 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 1691 this patch: 1691 |
| netdev/checkpatch | warning | WARNING: line length of 83 exceeds 80 columns |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
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
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
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
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!
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
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(-)