Re: [PATCH V2 bpf-next] BPF, docs: libbpf Overview Document

[Bagas Sanjaya <bagasdotme@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3113 bytes --]

On Thu, Mar 09, 2023 at 02:49:30PM -0800, Sreevani Sreejith wrote:
> From: Sreevani <ssreevani@meta.com>
> 
> Summary: Document that provides an overview of libbpf features for BPF
> application development.
> 
> Reviewers:
> 
> Subscribers:

Please remove the boilerplate above. The summary should become patch
description.

> +======
>  libbpf
>  ======

Why did you promote the heading above to title heading?

> diff --git a/Documentation/bpf/libbpf/libbpf_overview.rst b/Documentation/bpf/libbpf/libbpf_overview.rst
> new file mode 100644
> index 000000000000..549469aa2d3b
> --- /dev/null
> +++ b/Documentation/bpf/libbpf/libbpf_overview.rst
> @@ -0,0 +1,238 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +.._libbpf-overview-label:

Missing space before label name. Or should the label name be
"_libbpf-overview"?

> +#################################
> +BPF App Lifecycle and libbpf APIs
> +#################################
> <snipped> ...
> +########################
> +BPF Object Skeleton File
> +########################
> <snipped> ...
> +---------------------------------------
> +Other Advantages of Using Skeleton File
> +---------------------------------------
> +
> <snipped> ...
> +###########
> +BPF Helpers
> +###########
> <snipped> ...
> +#########################################
> +BPF CO-RE (Compile Once – Run Everywhere)
> +#########################################
> <snipped> ...

See the bottom.

> +You can generate the BTF information for the running kernel with the following
> +command:
> +
> +::
> +
> +$ bpftool btf dump file /sys/kernel/btf/vmlinux format c > vmlinux.h

My htmldocs build renders the code block above fine, but IMO the
convention here is to indent it.

> +The following code snippet shows how to read the parent field of a kernel
> +``task_struct`` using BPF CO-RE and libbf. The basic helper to read a field in a
> +CO-RE relocatable manner is ``bpf_core_read(dst, sz, src)``, which will read
> +``sz`` bytes from the field referenced by ``src`` into the memory pointed to by
> +``dst``.
> +
> +  .. code-block:: C
> +    :emphasize-lines: 6
> +
> +    //...
> +    struct task_struct *task = (void *)bpf_get_current_task();
> +    struct task_struct *parent_task;
> +    int err;
> +
> +    err = bpf_core_read(&parent_task, sizeof(void *), &task->parent);
> +    if (err) {
> +      /* handle error */
> +    }
> +
> +    /* parent_task contains the value of task->parent pointer */

I guess :linenos: option also helps locating the highlighted line, right?

> +###########################
> +Getting Started with libbpf
> +###########################
> <snipped> ...
> +###############
> +libbpf and Rust
> +###############
> <snipped> ...
> +########################
> +Additional Documentation
> +########################

For section headings above, I'd like to strip the overline. Yet,
Sphinx is smart when discriminating headings based on underline/overline
character.

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]