Message-ID: <8f999e0a-1c75-6443-ee1e-2913e3cf313e@netronome.com>
Date:   Wed, 11 Apr 2018 16:41:04 +0100
From:   Quentin Monnet <quentin.monnet@...ronome.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     daniel@...earbox.net, ast@...nel.org, netdev@...r.kernel.org,
        oss-drivers@...ronome.com, linux-doc@...r.kernel.org,
        linux-man@...r.kernel.org
Subject: [RFC bpf-next v2 1/8] bpf: add script and prepare bpf.h for new
 helpers documentation

2018-04-10 11:16 UTC-0700 ~ Alexei Starovoitov
<alexei.starovoitov@...il.com>
> On Tue, Apr 10, 2018 at 03:41:50PM +0100, Quentin Monnet wrote:
>> Remove previous "overview" of eBPF helpers from user bpf.h header.
>> Replace it by a comment explaining how to process the new documentation
>> (to come in following patches) with a Python script to produce RST, then
>> man page documentation.
>>
>> Also add the aforementioned Python script under scripts/. It is used to
>> process include/uapi/linux/bpf.h and to extract helper descriptions, to
>> turn it into a RST document that can further be processed with rst2man
>> to produce a man page. The script takes one "--filename <path/to/file>"
>> option. If the script is launched from scripts/ in the kernel root
>> directory, it should be able to find the location of the header to
>> parse, and "--filename <path/to/file>" is then optional. If it cannot
>> find the file, then the option becomes mandatory. RST-formatted
>> documentation is printed to standard output.
>>
>> Typical workflow for producing the final man page would be:
>>
>>     $ ./scripts/bpf_helpers_doc.py \
>>             --filename include/uapi/linux/bpf.h > /tmp/bpf-helpers.rst
>>     $ rst2man /tmp/bpf-helpers.rst > /tmp/bpf-helpers.7
>>     $ man /tmp/bpf-helpers.7
>>
>> Note that the tool kernel-doc cannot be used to document eBPF helpers,
>> whose signatures are not available directly in the header files
>> (pre-processor directives are used to produce them at the beginning of
>> the compilation process).
>>
>> Signed-off-by: Quentin Monnet <quentin.monnet@...ronome.com>
>> ---
>>  include/uapi/linux/bpf.h   | 406 ++------------------------------------------
>>  scripts/bpf_helpers_doc.py | 414 +++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 430 insertions(+), 390 deletions(-)
>>  create mode 100755 scripts/bpf_helpers_doc.py
>>

[...]

>> diff --git a/scripts/bpf_helpers_doc.py b/scripts/bpf_helpers_doc.py
>> new file mode 100755
>> index 000000000000..3a15ba3f0a83
>> --- /dev/null
>> +++ b/scripts/bpf_helpers_doc.py
>> @@ -0,0 +1,414 @@
>> +#!/usr/bin/python3
>> +#
>> +# Copyright (C) 2018 Netronome Systems, Inc.
>> +#
>> +# This software is licensed under the GNU General License Version 2,
>> +# June 1991 as shown in the file COPYING in the top-level directory of this
>> +# source tree.
> 
> please use SPDX instead.
> 

Same as for bpftool, our layers remain a bit cautious about it. I'd be
happy to change it for SPDX as a follow-up when I get the green light.

>> +#
>> +# THE COPYRIGHT HOLDERS AND/OR OTHER PARTIES PROVIDE THE PROGRAM "AS IS"
>> +# WITHOUT WARRANTY OF ANY KIND, EITHER EXPRESSED OR IMPLIED, INCLUDING,
>> +# BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
>> +# FOR A PARTICULAR PURPOSE. THE ENTIRE RISK AS TO THE QUALITY AND PERFORMANCE
>> +# OF THE PROGRAM IS WITH YOU. SHOULD THE PROGRAM PROVE DEFECTIVE, YOU ASSUME
>> +# THE COST OF ALL NECESSARY SERVICING, REPAIR OR CORRECTION.
>> +

[...]

>> +class PrinterRST(Printer):
>> +    """
>> +    A printer for dumping collected information about helpers as a ReStructured
>> +    Text page compatible with the rst2man program, which can be used to
>> +    generate a manual page for the helpers.
>> +    @helpers: array of Helper objects to print to standard output
>> +    """
>> +    def print_header(self):
>> +        header = '''\
>> +.. Copyright (C) 2018 Netronome Systems, Inc.
> 
> I think would be good to capture copyrights of all authors that added
> the helpers being documented. Since a lot of text was copied from commit
> logs it's only fair to preserve the copyrights.
> Such man page file is automatically generated by the python script
> and script itself is copyrighted by Netronome. That's fine, but the text
> of man page is not netronome only.
> I'm not sure what would be the solution. May be something like:
> "
> Copyright (C) All BPF authors and contributors from 2011 to present
> See git log include/uapi/linux/bpf.h for details
> "
> ?

Seems fair indeed. I do not have a better suggestion myself, so I will
stick to yours.

Out of curiosity, why 2011 for the year? I thought you introduced eBPF
in the kernel in 2014 (bd4cf0ed331a), and I do not believe helpers have
any link with cBPF?

>> +.. 
>> +.. %%%LICENSE_START(VERBATIM)
>> +.. Permission is granted to make and distribute verbatim copies of this
>> +.. manual provided the copyright notice and this permission notice are
>> +.. preserved on all copies.
>> +.. 
>> +.. Permission is granted to copy and distribute modified versions of this
>> +.. manual under the conditions for verbatim copying, provided that the
>> +.. entire resulting derived work is distributed under the terms of a
>> +.. permission notice identical to this one.
>> +.. 
>> +.. Since the Linux kernel and libraries are constantly changing, this
>> +.. manual page may be incorrect or out-of-date.  The author(s) assume no
>> +.. responsibility for errors or omissions, or for damages resulting from
>> +.. the use of the information contained herein.  The author(s) may not
>> +.. have taken the same level of care in the production of this manual,
>> +.. which is licensed free of charge, as they might when working
>> +.. professionally.
>> +.. 
>> +.. Formatted or processed versions of this manual, if unaccompanied by
>> +.. the source, must acknowledge the copyright and authors of this work.
>> +.. %%%LICENSE_END
>> +.. 
>> +.. Please do not edit this file. It was generated from the documentation
>> +.. located in file include/uapi/linux/bpf.h of the Linux kernel sources
>> +.. (helpers description), and from scripts/bpf_helpers_doc.py in the same
>> +.. repository (header and footer).
>> +
>> +===========
>> +BPF-HELPERS
>> +===========
>> +-------------------------------------------------------------------------------
>> +list of eBPF helper functions
>> +-------------------------------------------------------------------------------
>> +
>> +:Manual section: 7
>> +
>> +DESCRIPTION
>> +===========
>> +
>> +The extended Berkeley Packet Filter (eBPF) subsystem consists in programs
>> +written in a pseudo-assembly language, then attached to one of the several
>> +kernel hooks and run in reaction of specific events. This framework differs
>> +from the older, "classic" BPF (or "cBPF") in several aspects, one of them being
>> +the ability to call special functions (or "helpers") from within a program. For
>> +security reasons, these functions are restricted to a white-list of helpers
>> +defined in the kernel.
> 
> 'for security reasons' sounds a bit odd. May be 'for safety reasons' ?
> Or drop that part.

I'll drop it and keep "These functions are restricted to a white-list of
helpers defined in the kernel."

>> +
>> +These helpers are used by eBPF programs to interact with the system, or with
>> +the context in which they work. For instance, they can be used to print
>> +debugging messages, to get the time since the system was booted, to interact
>> +with eBPF maps, or to manipulate network packets metadata. Since there are
> 
> s/packets metadata/packets/

Ok.

>> +several eBPF program types, and that they do not run in the same context, each
>> +program type can only call a subset of those helpers.
>> +
>> +Due to eBPF conventions, a helper can not have more than five arguments.
>> +
>> +This document is an attempt to list and document the helpers available to eBPF
>> +developers. They are sorted by chronological order (the oldest helpers in the
>> +kernel at the top).
>> +
>> +HELPERS
>> +=======
>> +'''
>> +        print(header)
>> +
>> +    def print_footer(self):
>> +        footer = '''
>> +NOTES
>> +=====
>> +
>> +On the performance side, eBPF programs move to the stack all arguments to pass
>> +to the helpers, and call directly into the compiled helper functions without
> 
> "move to the stack all arguments" ?! I'm not sure what you're trying to say.
> The arguments stay in registers for the call.
> 
>> +requiring any foreign-function interface. As a result, calling helpers
>> +introduce very little overhead.
> 
> not true. it's zero overhead. Literally. Very little is not the same as zero.

Not the same indeed :). I will fix with "no overhead".

I'm not too sure either what I meant when I wrote the thing about moving
arguments to the stack... In fact this "NOTES" section is short and not
really relevant. I will probably delete it and add a line in page header
about helpers being called with no overhead.

>> +
>> +EXAMPLES
>> +========
>> +
>> +Example usage for most of the eBPF helpers listed in this manual page are
>> +available within the Linux kernel sources, at the following locations:
>> +
>> +* *samples/bpf/*
>> +* *tools/testing/selftests/bpf/*
>> +
>> +IMPLEMENTATION
>> +==============
>> +
>> +This manual page is an effort to document the existing eBPF helper functions.
>> +But as of this writing, the BPF sub-system is under heavy development. New eBPF
>> +program or map types are added, along with new helper functions. Some helpers
>> +are occasionally made available for additional program types. So in spite of
>> +the efforts of the community, this page might not be up-to-date. If you want to
>> +check by yourself what helper functions exist in your kernel, or what types of
>> +programs they can support, here are some files among the kernel tree that you
>> +may be interested in:
>> +
>> +* *include/uapi/linux/bpf.h* contains the full list of all helper functions.
>> +* *net/core/filter.c* contains the definition of most network-related helper
>> +  functions, and the list of program types from which they can be used.
>> +* *kernel/trace/bpf_trace.c* is the equivalent for most tracing program-related
>> +  helpers.
>> +* *kernel/bpf/verifier.c* contains the functions used to check that valid types
>> +  of eBPF maps are used with a given helper function.
>> +* *kernel/bpf/* directory contains other files in which additional helpers are
>> +  defined (for cgroups, sockmaps, etc.).
>> +
>> +Compatibility between helper functions and program types can generally be found
>> +in the files where helper functions are defined. Look for the **struct
>> +bpf_func_proto** objects and for functions returning them: these functions
>> +contain a list of helpers that a given program type can call. Note that the
>> +**default:** label of the **switch ... case** used to filter helpers can call
>> +other functions, themselves allowing access to additional helpers. The
>> +requirement for GPL license is also in those **struct bpf_func_proto**.
> 
> I think here would be good to add that most networking helpers are non-GPL
> because they operate on packets which are abstract bytes on the wire,
> whereas most tracing helpers are GPL, since they inspect the guts of
> the linux kernel which is GPL itself.
> That's the main reason why adding extra 'gpl=yes/no' for each helper
> description is redundant.
> 

I removed information from the page header about licensing since v1, I
may reintroduce some of it and tell about the difference between
networking and tracing programs, as you suggest.

I understand this difference and I see that specifying GPL requirement
for each individual helper is redundant. And yet I still believe that
for newcomers, it remains easier to have the indication for the specific
helper they are reading about in the man page rather than to take a
guess ("Is this helper for networking only?"). But I do not intend to
add it back to this set anyway, so let's keep this for future
discussions :).

Thanks for the review!
Quentin

Powered by blists - more mailing lists