Hello Quentin, Daniel,
On 05/23/2018 09:03 PM, Quentin Monnet wrote:
Hi Michael,
Thanks a lot for your review and feedback! Please find some comments
inline below.
2018-05-23 17:46 UTC+0200 ~ Michael Kerrisk (man-pages)
<mtk.manpages@xxxxxxxxx>
Hello Quentin,
On 30 April 2018 at 18:37, Quentin Monnet <quentin.monnet@xxxxxxxxxxxxx> wrote:
eBPF sub-system on Linux can use "helper functions", functions
implemented in the kernel that can be called from within a eBPF program
injected by a user on Linux. The kernel already supports a long list of
such helpers (sixty-seven at this time, new ones are under review).
Therefore, it is proposed to create a new manual page, separate from
bpf(2), to document those helpers for people willing to develop new eBPF
programs.
Thank you for all of this work! And, agreed, a separate manual page is best.
Additionally, in an effort to keep this documentation in synchronisation
with what is implemented in the kernel, it is further proposed to keep
the documentation itself in the kernel sources, as comments in file
"include/uapi/linux/bpf.h", and to generate the man page from there.
Can you say some more about this. I mean, putting the man-page source
in has some disadvantages from my point of view. So, I want to ask:
will putting all the documentation in a .h file really assist
synchronization (with reasoning please! ;-)).
To keep the man page up-to-date, the best is obviously to edit it each
time a kernel patch updates the eBPF helpers. The idea here is to have
authors of the patches related to these helpers to update the
documentation when they submit their code. Since the first chunks of the
documentation were merged on the kernel side, eBPF maintainers have been
able to ask this of people who proposed new helpers. As a result, since
I posted the present patch for the manual page, five more helper
functions have been added and documented this way. We did not have to
update the descriptions of existing functions in that time interval, but
occasional changes of behaviour (new flags supported, new eBPF map types
supported, etc.) have occurred in the past and will likely happen again
from time to time.
If the documentation was instead a straight manual page in the man-pages
repository, I am very unsure it would be up-to-date even as of today.
From my point of view (which seems to be shared by other eBPF
maintainers?), it is more delicate to ask to the authors of those
functions to go and update the documentation in a *separate* repository:
- It adds some overhead to the process, by making them send a patch set
to another project, and to follow-up for reviews after that.
- Using groff is, well, not exactly attracting. If documentation cannot
be added in the same patch set as the code, I fear it may deter authors
from updating the page. reStructuredText seems complex enough already,
we had to fix a couple of formatting mistakes in the description of the
last helpers.
- With all due respect, I believe that netdev mailing list is the best
place to get technical reviews on the contents of this documentation.
Having it in the kernel makes it easy for eBPF contributors to review
the changes.
So in short: I expect most of the contributors to this documentation
will be people updating those helpers, and it makes it much easier, in
terms of process as in terms of technical reviews, to ask them to edit
the description if the doc is at the same location as the actual code.
There are some downsides, from my point of view:
* The generated man page source is kind of ugly.
I agree (although raw groff pages in general rarely look appealing to
me). But if the page is to be automatically generated anyway, this is
not really a major issue in my opinion.
* The generated man page renders better than I would expect, but there
are places where things are a bit wrong. For example, using ".sp" as a
paragrap separator renders poorly when producing PostScript/PDF
versions of the page.
This is more annoying. I would be ready to work to improve this. I
suppose we could post-process the page; adding a third tool to the work
flow may not be ideal, but might be worth if it really improves the
final output.
That was the only issue I noticed so far. Maybe there are others
that I'll see later, but as I said, the page looks better than I might
have expected for a generated page. The incorrect use of ".sp" just
bothers me because a year or so back I fixed that issue in hundreds
of the existing pages.
* People are likely to send me patches for the generated page. I won't
be able to accept those. Instead someone (possibly me), will have to
turn them into patches against the .h file. (This will be amplified by
the fact that many more people are likely to look at the manual page
than the at the kernel source file.)
Could you maybe redirect those people to the .h file instead of
translating the patches yourself? I certainly do not have your
experience as a project maintainer, maybe redirecting volunteer
contributors proposing their patches is more delicate than I can see?
On a side note, I believe that most contributors to this page are likely
to be more or less familiar with contributing to the kernel anyway.
Not to be too pessimistic, but I've found it's often a bit of a
challenge to get man-pages contributions from kernel developers...
I
had a look at the list of people who edited the bpf(2) manual page, and
it seems that most of them could have sent their patch against the .h
with little to no difficulty: I would expect most contributors to
bpf-helpers(2) to be in a similar situation, or even to be the very
authors of the documented changes.
Overall, I think I'd prefer to have this as a straight manual page,
rather than something generated from a kernel source file. My
reasoning is that I'm not convinced about the argument that putting
the documentation into a .h file will really hellp in synchronizing
with the implementation. And having a true manual page would allow me
to avoid the issues mentioned above (as well as some others that I
probably haven't thought of right now). Obviously I can't insist on
this, but it would I think make my life easier.
I certainly do not want to make things harder for you. And in the other
way round, I cannot and (do not want) to force you to take the generated
page either :).
To be honest I started to write the documentation as a straight manual
page, in groff. I did not mind going all the way with it, but after
several discussions with eBPF folks (mostly Alexei and Daniel, both
CC-ed) came the idea of moving it to reStructuredText, and placing it in
the bpf.h header file. The consensus, if I remember correctly, was that
it would be easier to have all the documentation in one place,
preferably in the kernel (we discussed placing it under Documentation/,
then moved to bpf.h where a brief description of the helpers was present
already), for the reasons I provided above.
Although not relevant here, another advantage of parsing this
description is the possibility to reuse the contents to produce other
kinds of output (for example, the list of function prototypes can be
used to automatically generate a new, up-to-date .h header file usable
with eBPF programs).
I'd be happy to work with you on just converting this to a straight
manual page, which might simply mean that I edit the currently
generated version to produce something that fits better with current
norms for the man-pages project.
We could come to that if you do not want the generated page. But it
seems to me that creating it from the kernel side is the best compromise
we (on the eBPF side) could find. Again, I do not want to impose it on
you. But I would really like to convince you it is worth it! :) Please
let me know what you think of those arguments, or if you have further
questions.
So, I think we should try it your way. I guess we'll need a while
to see how well things work, and if need be, we can revisit how we do
things.
This patch adds the new man page, generated from kernel sources, to the
man-pages repository. For each eBPF helper function, a description of
the helper, of its arguments and of the return value is provided. The
idea is that all future changes for this page should be redirected to
the kernel file "include/uapi/linux/bpf.h", and the modified page
generated from there.
Generating the page itself is a two-step process. First, the
documentation is extracted from include/uapi/linux/bpf.h, and converted
to a RST (reStructuredText-formatted) page, with the relevant script
from Linux sources:
$ ./scripts/bpf_helpers_doc.py > /tmp/bpf-helpers.rst
The second step consists in turning the RST document into the final man
page, with rst2man:
$ rst2man /tmp/bpf-helpers.rst > bpf-helpers.7
The bpf.h file was taken at kernel commit 081023a31129 (that should be
in Linux 4.18).
So, I suppose the procedure is that I should resync by performing this
step every couple of months?
Cheers,
Michael
Cc: Alexei Starovoitov <ast@xxxxxxxxxx>
Cc: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
Signed-off-by: Quentin Monnet <quentin.monnet@xxxxxxxxxxxxx>
---
man2/bpf.2 | 1 +
man7/bpf-helpers.7 | 1983 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 1984 insertions(+)
create mode 100644 man7/bpf-helpers.7
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
