Update bpf-helpers(7) man page

Update bpf-helpers(7) man page

[Rumen Telbizov]

Hi Michael, everyone,

I was directed to you by Quentin Monnet with regards to a missing
function in the bpf-helpers(7) Linux man page. I found out that the
man page doesn't have anything regarding bpf_redirect_neigh(). Quentin
mentioned that he has a script which generates the man page from the
comments in the source code, but then I am not sure how and when the
man page is being generated so that it gets out of date. This function
definitely has comments/documentation in the code and it works
properly but is missing in the man page.

Is this something you can help with?

Thank you,
Rumen Telbizov

Re: Update bpf-helpers(7) man page

[Alejandro Colomar]

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

Hi Rumen,

On 7/18/22 18:37, Rumen Telbizov wrote:
> Hi Michael, everyone,
> 
> I was directed to you by Quentin Monnet with regards to a missing
> function in the bpf-helpers(7) Linux man page. I found out that the
> man page doesn't have anything regarding bpf_redirect_neigh(). Quentin
> mentioned that he has a script which generates the man page from the
> comments in the source code, but then I am not sure how and when the
> man page is being generated so that it gets out of date. This function
> definitely has comments/documentation in the code and it works
> properly but is missing in the man page.
> 
> Is this something you can help with?

Yes, the page is generated:

alx@asus5775:~/src/linux/man-pages/man-pages$ cat MAINTAINER_NOTES
Externally generated pages
==========================

A few pages come from external sources. Fixes to the pages should really
go to the upstream source.

tzfile(5), zdump(8), and zic(8) come from the tz project
(https://www.iana.org/time-zones).

bpf-helpers(7) is autogenerated from the kernel sources using scripts.
See man-pages commit 53666f6c30451cde022f65d35a8d448f5a7132ba for
details.



If Quentin has the script, he could send it to me as a patch to the 
man-pages repo, so that it's added to our scripts/ directory.  I can 
regenerate the page whenever I'm requested to do so, but I've never done 
it, so I need the script and a simple explanation to use it.


Thanks,

Alex

> 
> Thank you,
> Rumen Telbizov

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

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

Re: Update bpf-helpers(7) man page

[Rumen Telbizov]

Hi Alejandro,

Thanks for following up on this.
Quentin will send you the script these days for you to rerun.
However, I'm wondering if there's a way to run it automatically when a change is
detected or otherwise without needing manual intervention? This way
the published
page will not get out of date. I am not sure what that mechanism might be but
just a thought.

Cheers



On Tue, Jul 19, 2022 at 7:42 AM Alejandro Colomar
<alx.manpages@gmail.com> wrote:
>
> Hi Rumen,
>
> On 7/18/22 18:37, Rumen Telbizov wrote:
> > Hi Michael, everyone,
> >
> > I was directed to you by Quentin Monnet with regards to a missing
> > function in the bpf-helpers(7) Linux man page. I found out that the
> > man page doesn't have anything regarding bpf_redirect_neigh(). Quentin
> > mentioned that he has a script which generates the man page from the
> > comments in the source code, but then I am not sure how and when the
> > man page is being generated so that it gets out of date. This function
> > definitely has comments/documentation in the code and it works
> > properly but is missing in the man page.
> >
> > Is this something you can help with?
>
> Yes, the page is generated:
>
> alx@asus5775:~/src/linux/man-pages/man-pages$ cat MAINTAINER_NOTES
> Externally generated pages
> ==========================
>
> A few pages come from external sources. Fixes to the pages should really
> go to the upstream source.
>
> tzfile(5), zdump(8), and zic(8) come from the tz project
> (https://www.iana.org/time-zones).
>
> bpf-helpers(7) is autogenerated from the kernel sources using scripts.
> See man-pages commit 53666f6c30451cde022f65d35a8d448f5a7132ba for
> details.
>
>
>
> If Quentin has the script, he could send it to me as a patch to the
> man-pages repo, so that it's added to our scripts/ directory. I can
> regenerate the page whenever I'm requested to do so, but I've never done
> it, so I need the script and a simple explanation to use it.
>
>
> Thanks,
>
> Alex
>
> >
> > Thank you,
> > Rumen Telbizov
>
> --
> Alejandro Colomar
> <http://www.alejandro-colomar.es/>

Re: Update bpf-helpers(7) man page

[Alejandro Colomar]

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

Hi Rumen,

On 7/19/22 19:21, Rumen Telbizov wrote:
> Hi Alejandro,
> 
> Thanks for following up on this.
> Quentin will send you the script these days for you to rerun.
> However, I'm wondering if there's a way to run it automatically when a change is
> detected or otherwise without needing manual intervention? This way
> the published
> page will not get out of date. I am not sure what that mechanism might be but
> just a thought.

I'm not sure an automated mechanism would be easy to set up.
But, I'm planning to add a RELEASE file to the man-pages repo with 
instructions to make a release.  I can add there a step that reminds to 
refresh the bpf-helpers(7) manual page before every release.


Cheers,

Alex

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

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

Re: Update bpf-helpers(7) man page

[Quentin Monnet]

On Wed, 20 Jul 2022 at 10:50, Alejandro Colomar <alx.manpages@gmail.com> wrote:
>
> Hi Rumen,
>
> On 7/19/22 19:21, Rumen Telbizov wrote:
> > Hi Alejandro,
> >
> > Thanks for following up on this.
> > Quentin will send you the script these days for you to rerun.
> > However, I'm wondering if there's a way to run it automatically when a change is
> > detected or otherwise without needing manual intervention? This way
> > the published
> > page will not get out of date. I am not sure what that mechanism might be but
> > just a thought.
>
> I'm not sure an automated mechanism would be easy to set up.
> But, I'm planning to add a RELEASE file to the man-pages repo with
> instructions to make a release.  I can add there a step that reminds to
> refresh the bpf-helpers(7) manual page before every release.

Hi Alejandro, Rumen,

Thank you Rumen for raising this. Yes, the bpf-helpers(7) man page is
generated from a script: it's scripts/bpf_doc.py under the kernel
repository [0], which parses the UAPI header at
include/uapi/linux/bpf.h [1] to generate a rST file that can be
converted to a man page. From the root of the Linux repository, one
can generate and read the manual page with the following command:

    $ ./scripts/bpf_doc.py helpers | rst2man | man -l -

(Note that the name of the script has changed since man-page commit
53666f6c3045.)

Given that the script is maintained in the Linux repository (it is run
through the BPF CI [2], and it is also used to generate a C header
that is shipped along with libbpf [3]), I would recommend against
copying it to the man-pages repository, so that we avoid getting two
copies out-of-sync. It is probably best to pick it up from the Linux
repo (since the UAPI header is also required anyway) when regenerating
the page.

If automation is not doable, I would be very happy to have someone
running this step before each project release.

Many thanks,
Quentin

[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/bpf_doc.py?h=v5.18
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/bpf.h?h=v5.18#n1526
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/testing/selftests/bpf/Makefile.docs?h=v5.18
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/lib/bpf/Makefile?h=v5.18#n159

Re: Update bpf-helpers(7) man page

[Alejandro Colomar]

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

Hi Quentin,

On 7/20/22 22:40, Quentin Monnet wrote:
> On Wed, 20 Jul 2022 at 10:50, Alejandro Colomar <alx.manpages@gmail.com> wrote:
>>
>> Hi Rumen,
>>
>> On 7/19/22 19:21, Rumen Telbizov wrote:
>>> Hi Alejandro,
>>>
>>> Thanks for following up on this.
>>> Quentin will send you the script these days for you to rerun.
>>> However, I'm wondering if there's a way to run it automatically when a change is
>>> detected or otherwise without needing manual intervention? This way
>>> the published
>>> page will not get out of date. I am not sure what that mechanism might be but
>>> just a thought.
>>
>> I'm not sure an automated mechanism would be easy to set up.
>> But, I'm planning to add a RELEASE file to the man-pages repo with
>> instructions to make a release.  I can add there a step that reminds to
>> refresh the bpf-helpers(7) manual page before every release.
> 
> Hi Alejandro, Rumen,
> 
> Thank you Rumen for raising this. Yes, the bpf-helpers(7) man page is
> generated from a script: it's scripts/bpf_doc.py under the kernel
> repository [0], which parses the UAPI header at
> include/uapi/linux/bpf.h [1] to generate a rST file that can be
> converted to a man page. From the root of the Linux repository, one
> can generate and read the manual page with the following command:
> 
>      $ ./scripts/bpf_doc.py helpers | rst2man | man -l -
> 
> (Note that the name of the script has changed since man-page commit
> 53666f6c3045.)

Okay, that makes sense (I tried to find the script mentioned in that 
commit, and din't find it).
> 
> Given that the script is maintained in the Linux repository (it is run
> through the BPF CI [2], and it is also used to generate a C header
> that is shipped along with libbpf [3]), I would recommend against
> copying it to the man-pages repository, so that we avoid getting two
> copies out-of-sync. It is probably best to pick it up from the Linux
> repo (since the UAPI header is also required anyway) when regenerating
> the page.
> 

Yes, having it in the kernel, since you also use it for other thing, 
makes sense to me.  I can make a note of that in our future RELEASE 
instructions.  For now, I'll document it in a new commit message.

> If automation is not doable, I would be very happy to have someone
> running this step before each project release.

If you send a man-pages patch after every kernel release, that would be 
great.  We can also do that ourselves, as long as the tools are there; 
but if something changes in that script, it would be nice to notify us, 
if we're using them.

Whatever you prefer.

Thanks,

Alex

> 
> Many thanks,
> Quentin
> 
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/bpf_doc.py?h=v5.18
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/bpf.h?h=v5.18#n1526
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/testing/selftests/bpf/Makefile.docs?h=v5.18
> [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/lib/bpf/Makefile?h=v5.18#n159

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

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

Re: Update bpf-helpers(7) man page

[Quentin Monnet]

On Wed, 20 Jul 2022 at 22:07, Alejandro Colomar <alx.manpages@gmail.com> wrote:
>
> Hi Quentin,
>
> On 7/20/22 22:40, Quentin Monnet wrote:
> > On Wed, 20 Jul 2022 at 10:50, Alejandro Colomar <alx.manpages@gmail.com> wrote:
> >>
> >> Hi Rumen,
> >>
> >> On 7/19/22 19:21, Rumen Telbizov wrote:
> >>> Hi Alejandro,
> >>>
> >>> Thanks for following up on this.
> >>> Quentin will send you the script these days for you to rerun.
> >>> However, I'm wondering if there's a way to run it automatically when a change is
> >>> detected or otherwise without needing manual intervention? This way
> >>> the published
> >>> page will not get out of date. I am not sure what that mechanism might be but
> >>> just a thought.
> >>
> >> I'm not sure an automated mechanism would be easy to set up.
> >> But, I'm planning to add a RELEASE file to the man-pages repo with
> >> instructions to make a release.  I can add there a step that reminds to
> >> refresh the bpf-helpers(7) manual page before every release.
> >
> > Hi Alejandro, Rumen,
> >
> > Thank you Rumen for raising this. Yes, the bpf-helpers(7) man page is
> > generated from a script: it's scripts/bpf_doc.py under the kernel
> > repository [0], which parses the UAPI header at
> > include/uapi/linux/bpf.h [1] to generate a rST file that can be
> > converted to a man page. From the root of the Linux repository, one
> > can generate and read the manual page with the following command:
> >
> >      $ ./scripts/bpf_doc.py helpers | rst2man | man -l -
> >
> > (Note that the name of the script has changed since man-page commit
> > 53666f6c3045.)
>
> Okay, that makes sense (I tried to find the script mentioned in that
> commit, and din't find it).
> >
> > Given that the script is maintained in the Linux repository (it is run
> > through the BPF CI [2], and it is also used to generate a C header
> > that is shipped along with libbpf [3]), I would recommend against
> > copying it to the man-pages repository, so that we avoid getting two
> > copies out-of-sync. It is probably best to pick it up from the Linux
> > repo (since the UAPI header is also required anyway) when regenerating
> > the page.
> >
>
> Yes, having it in the kernel, since you also use it for other thing,
> makes sense to me.  I can make a note of that in our future RELEASE
> instructions.  For now, I'll document it in a new commit message.
>
> > If automation is not doable, I would be very happy to have someone
> > running this step before each project release.
>
> If you send a man-pages patch after every kernel release, that would be
> great.  We can also do that ourselves, as long as the tools are there;

Since you're offering, I wouldn't mind leaving it to you to run the
script before each release. I'm willing to help, but I don't really
trust myself with remembering this every time, so having it written
down as one of the steps in your RELEASE file is probably the best way
to make sure the update happens.

Please do reach out if the script didn't behave as expected, though.
I'll be happy to assist.

> but if something changes in that script, it would be nice to notify us,
> if we're using them.

I do realise we failed to send a notice when we renamed the script,
please accept my apologies for that. I'll be more careful in the
future.

Thanks,
Quentin

Re: Update bpf-helpers(7) man page

[Alejandro Colomar]

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

Hi Quentin,

On 7/20/22 23:44, Quentin Monnet wrote:
> Since you're offering, I wouldn't mind leaving it to you to run the
> script before each release. I'm willing to help, but I don't really
> trust myself with remembering this every time, so having it written
> down as one of the steps in your RELEASE file is probably the best way
> to make sure the update happens.

Yep.

> 
> Please do reach out if the script didn't behave as expected, though.
> I'll be happy to assist.

Okay, I'll try it and will come back if I need help.

Thanks!

> 
>> but if something changes in that script, it would be nice to notify us,
>> if we're using them.
> 
> I do realise we failed to send a notice when we renamed the script,
> please accept my apologies for that. I'll be more careful in the
> future.

I wasn't yet involved in maintaining the man-pages at the time the 
script changed, so no need to apologize to me :p

Cheers,

Alex

> 
> Thanks,
> Quentin

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

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

Re: Update bpf-helpers(7) man page

[Rumen Telbizov]

Thanks for handling that folks.
I'll keep an eye on the man page updating.

Alejandro, do you have an idea when it might be that you would
run the script and update the page? I can verify afterwards.

Re: Update bpf-helpers(7) man page

[Alejandro Colomar]

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

Hi Rumen,

On 7/21/22 00:44, Rumen Telbizov wrote:
> Thanks for handling that folks.
> I'll keep an eye on the man page updating.
> 
> Alejandro, do you have an idea when it might be that you would
> run the script and update the page? I can verify afterwards.

Done.  Check the last two commits.

Cheers,

Alex

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

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

Re: Update bpf-helpers(7) man page

[Rumen Telbizov]

Thanks Alex and Quentin for all your efforts.

I hope https://man7.org/linux/man-pages/man7/bpf-helpers.7.html picks
it up soon too.

On Thu, Jul 21, 2022 at 4:28 AM Alejandro Colomar
<alx.manpages@gmail.com> wrote:
>
> Hi Rumen,
>
> On 7/21/22 00:44, Rumen Telbizov wrote:
> > Thanks for handling that folks.
> > I'll keep an eye on the man page updating.
> >
> > Alejandro, do you have an idea when it might be that you would
> > run the script and update the page? I can verify afterwards.
>
> Done. Check the last two commits.
>
> Cheers,
>
> Alex
>
> --
> Alejandro Colomar
> <http://www.alejandro-colomar.es/>

Re: Update bpf-helpers(7) man page

[Jakub Wilk]

The latest bpf-helpers(7) man page update introduced a few misspellings:

>-EFAULT on failrue to parse the existing header options.

failrue -> failure

>this helper enforces the key must be an task_struct

an -> a

>Returns the stored IMA hash of the inode (if it's avaialable)

avaialable -> available

>Providing an 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.

an -> a

But I can't parse the subordinate clause. I think it should be:

..., which is 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.

I can't parse this subordinate clause either. I guess it was meant to say:

..., which is why the responsibility for catching a negative packet size 
belongs in those helpers.

Any volunteers to fix the misspellings on the kernel side?

-- 
Jakub Wilk

Re: Update bpf-helpers(7) man page

[Quentin Monnet]

On 24/08/2022 17:02, Jakub Wilk wrote:
> The latest bpf-helpers(7) man page update introduced a few misspellings:
> 
>> -EFAULT on failrue to parse the existing header options.
> 
> failrue -> failure
> 
>> this helper enforces the key must be an task_struct
> 
> an -> a
> 
>> Returns the stored IMA hash of the inode (if it's avaialable)
> 
> avaialable -> available
> 
>> Providing an 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.
> 
> an -> a
> 
> But I can't parse the subordinate clause. I think it should be:
> 
> ..., which is 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.
> 
> I can't parse this subordinate clause either. I guess it was meant to say:
> 
> ..., which is why the responsibility for catching a negative packet size
> belongs in those helpers.
> 
> Any volunteers to fix the misspellings on the kernel side?
> 

I'll have a look at these and send a fix. It's likely that some of those
are my mistakes anyway.

Thanks for the report,
Quentin

Re: Update bpf-helpers(7) man page

[Quentin Monnet]

On Wed, 24 Aug 2022 at 17:48, Quentin Monnet <quentin@isovalent.com> wrote:
>
> On 24/08/2022 17:02, Jakub Wilk wrote:
> > The latest bpf-helpers(7) man page update introduced a few misspellings:
> >
> >> -EFAULT on failrue to parse the existing header options.
> >
> > failrue -> failure
> >
> >> this helper enforces the key must be an task_struct
> >
> > an -> a
> >
> >> Returns the stored IMA hash of the inode (if it's avaialable)
> >
> > avaialable -> available
> >
> >> Providing an 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.
> >
> > an -> a
> >
> > But I can't parse the subordinate clause. I think it should be:
> >
> > ..., which is 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.
> >
> > I can't parse this subordinate clause either. I guess it was meant to say:
> >
> > ..., which is why the responsibility for catching a negative packet size
> > belongs in those helpers.
> >
> > Any volunteers to fix the misspellings on the kernel side?
> >
>
> I'll have a look at these and send a fix. It's likely that some of those
> are my mistakes anyway.

Out of curiosity, did you spot these mistakes by reading the page, or
did you run a tool on the document?

Quentin

Re: Update bpf-helpers(7) man page

[Alejandro Colomar]

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

Hi Quentin,

On 8/25/22 00:07, Quentin Monnet wrote:
> 
> Out of curiosity, did you spot these mistakes by reading the page, or
> did you run a tool on the document?

I asked the same thing a few days ago :)

Answer is in linux-man@:

<https://lore.kernel.org/linux-man/eb6a1e41-c48e-ac45-5154-ac57a2c76108@gmail.com/T/#m4a8d1b003616928013ffcd1450437309ab652f9f>

Cheers,

Alex


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

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