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

[Bagas Sanjaya <bagasdotme@gmail.com>]

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

On Fri, Mar 10, 2023 at 10:09:28AM -0800, Sreevani Sreejith wrote:
> From: Sreevani <ssreevani@meta.com>
> 
> Summary: Document that provides an overview of libbpf features for BPF
> application development.

It seems like you ignore some of my reviews at [1]. Anyway, I repeat
them here, augmenting my new comments.

The patch description should have been "Document overview of libbpf,
including its features for developing BPF programs.".

> +######
>  libbpf
> -======
> +######

Why did you add heading overline and change the heading character marker?

> +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 */

You may want to also add :lineos: option or manually add line numbers
if you add :emphasize-lines: so that readers can see the line number
it refers to.

> +Also, find the libbpf API documentation `here
> +<https://libbpf.readthedocs.io/en/latest/api.html>`_

"See also `libbpf API documentation <link>`_".

> +
> +libbpf and Rust
> +===============
> +
> +If you are building BPF applications in Rust, it is recommended to use the
> +`Libbpf-rs <https://github.com/libbpf/libbpf-rs>`_ library instead of bindgen
> +bindings directly to libbpf. Libbpf-rs wraps libbpf functionality in
> +Rust-idiomatic interfaces and provides libbpf-cargo plugin to handle BPF code
> +compilation and skeleton generation. Using Libbpf-rs will make building user
> +space part of the BPF application easier. Note that the BPF program themselves
> +must still be written in plain C.

BPF apps are application that use BPF program, right? I thought that
despite there is libbpf-rs, I still have to develop BPF apps in C.

Thanks.

[1]: https://lore.kernel.org/linux-doc/ZAqzeQZLNMyaZOck@debian.me/

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

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