Re: [PATCH bpf-next 1/2] Add documentation for libbpf including API autogen

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Mon, May 31, 2021 at 12:56 PM grantseltzer <grantseltzer@gmail.com> wrote:
>
> This adds rst files containing documentation for libbpf. This includes
> the addition of libbpf_api.rst which pulls comment documentation from
> header files in libbpf under tools/lib/bpf/. The comment docs would be
> of the standard kernel doc format.
>
> Signed-off-by: grantseltzer <grantseltzer@gmail.com>
> ---

Looks good, thanks! See few comments below. Let's figure out what to
do with libbpf docs versioning and land it through bpf-next tree.

>  Documentation/bpf/index.rst                   |  13 ++
>  Documentation/bpf/libbpf.rst                  |  14 ++
>  Documentation/bpf/libbpf_api.rst              |  18 ++
>  Documentation/bpf/libbpf_build.rst            |  37 ++++
>  .../bpf/libbpf_naming_convention.rst          | 170 ++++++++++++++++++
>  5 files changed, 252 insertions(+)
>  create mode 100644 Documentation/bpf/libbpf.rst
>  create mode 100644 Documentation/bpf/libbpf_api.rst
>  create mode 100644 Documentation/bpf/libbpf_build.rst
>  create mode 100644 Documentation/bpf/libbpf_naming_convention.rst
>

[...]

> +API
> +===
> +
> +This documentation is autogenerated from header files in libbpf, tools/lib/bpf
> +
> +.. kernel-doc:: tools/lib/bpf/libbpf.h
> +   :internal:
> +
> +.. kernel-doc:: tools/lib/bpf/bpf.h
> +   :internal:
> +
> +.. kernel-doc:: tools/lib/bpf/btf.h
> +   :internal:
> +
> +.. kernel-doc:: tools/lib/bpf/xsk.h
> +   :internal:

Libbpf API has a BPF side as well (bpf_helpers.h which pulls in
auto-generated bpf_helper_defs.h with all BPF helper definitions,
bpf_tracing.h, bpf_core_read.h, bpf_endian.h), we should probably
expose them as well?

> diff --git a/Documentation/bpf/libbpf_build.rst b/Documentation/bpf/libbpf_build.rst
> new file mode 100644
> index 000000000..b8240eaaa
> --- /dev/null
> +++ b/Documentation/bpf/libbpf_build.rst
> @@ -0,0 +1,37 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +Building libbpf
> +===============
> +
> +libelf is an internal dependency of libbpf and thus it is required to link

zlib is another dependency, can you please mention it as well?

> +against and must be installed on the system for applications to work.
> +pkg-config is used by default to find libelf, and the program called
> +can be overridden with PKG_CONFIG.
> +

[...]

> +API naming convention
> +=====================
> +
> +libbpf API provides access to a few logically separated groups of
> +functions and types. Every group has its own naming convention
> +described here. It's recommended to follow these conventions whenever a
> +new function or type is added to keep libbpf API clean and consistent.
> +
> +All types and functions provided by libbpf API should have one of the
> +following prefixes: ``bpf_``, ``btf_``, ``libbpf_``, ``xsk_``,
> +``perf_buffer_``.

ring_buffer_ and btf_dump_ are two others that we use. But I don't
know how important it is to have an exhaustive list here.

> +
> +System call wrappers
> +--------------------
> +
> +System call wrappers are simple wrappers for commands supported by
> +sys_bpf system call. These wrappers should go to ``bpf.h`` header file
> +and map one-on-one to corresponding commands.

typo: one-to-one?

> +
> +For example ``bpf_map_lookup_elem`` wraps ``BPF_MAP_LOOKUP_ELEM``
> +command of sys_bpf, ``bpf_prog_attach`` wraps ``BPF_PROG_ATTACH``, etc.
> +
> +Objects
> +-------
> +
> +Another class of types and functions provided by libbpf API is "objects"
> +and functions to work with them. Objects are high-level abstractions
> +such as BPF program or BPF map. They're represented by corresponding
> +structures such as ``struct bpf_object``, ``struct bpf_program``,
> +``struct bpf_map``, etc.
> +
> +Structures are forward declared and access to their fields should be
> +provided via corresponding getters and setters rather than directly.
> +
> +These objects are associated with corresponding parts of ELF object that
> +contains compiled BPF programs.
> +
> +For example ``struct bpf_object`` represents ELF object itself created
> +from an ELF file or from a buffer, ``struct bpf_program`` represents a
> +program in ELF object and ``struct bpf_map`` is a map.
> +
> +Functions that work with an object have names built from object name,
> +double underscore and part that describes function purpose.
> +
> +For example ``bpf_object__open`` consists of the name of corresponding
> +object, ``bpf_object``, double underscore and ``open`` that defines the
> +purpose of the function to open ELF file and create ``bpf_object`` from
> +it.
> +
> +Another example: ``bpf_program__load`` is named for corresponding
> +object, ``bpf_program``, that is separated from other part of the name
> +by double underscore.

let's drop this example, bpf_program__load is a bad example and is
going to be deprecated.  We can use btf__parse() as an example here.

> +
> +All objects and corresponding functions other than BTF related should go
> +to ``libbpf.h``. BTF types and functions should go to ``btf.h``.
> +
> +Auxiliary functions
> +-------------------
> +
> +Auxiliary functions and types that don't fit well in any of categories
> +described above should have ``libbpf_`` prefix, e.g.
> +``libbpf_get_error`` or ``libbpf_prog_type_by_name``.
> +
> +AF_XDP functions
> +-------------------
> +
> +AF_XDP functions should have an ``xsk_`` prefix, e.g.
> +``xsk_umem__get_data`` or ``xsk_umem__create``. The interface consists
> +of both low-level ring access functions and high-level configuration
> +functions. These can be mixed and matched. Note that these functions
> +are not reentrant for performance reasons.
> +
> +Please take a look at Documentation/networking/af_xdp.rst in the Linux
> +kernel source tree on how to use XDP sockets and for some common
> +mistakes in case you do not get any traffic up to user space.

I'd probably drop this section, given we move xsk.{c,h} into libxdp.

> +
> +ABI
> +==========
> +
> +libbpf can be both linked statically or used as DSO. To avoid possible
> +conflicts with other libraries an application is linked with, all
> +non-static libbpf symbols should have one of the prefixes mentioned in
> +API documentation above. See API naming convention to choose the right
> +name for a new symbol.
> +

[...]