On Wed, Jun 2, 2021 at 4:36 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Mon, May 31, 2021 at 12:56 PM grantseltzer <grantseltzer@xxxxxxxxx> 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@xxxxxxxxx>
> > ---
>
> 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.
> > +
>
> [...]
Thanks for the feedback on this, I've fixed them on a local copy and
will incorporate them into my final patch/PR depending on what we go
with. See my most recent response on '[PATCH bpf-next 0/3]
Autogenerating API documentation'
