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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZA7wm8scokV+XPav@debian.me>
Date:   Mon, 13 Mar 2023 16:44:59 +0700
From:   Bagas Sanjaya <bagasdotme@...il.com>
To:     Sreevani Sreejith <ssreevani@...a.com>, void@...ifault.com,
        psreep@...il.com, bpf@...r.kernel.org,
        Linux-kernel@...r.kernel.org, andrii@...nel.org, mykola@...a.com
Cc:     linux-doc@...r.kernel.org
Subject: Re: [PATCH V3 bpf-next] BPF, docs: libbpf Overview Document

On Fri, Mar 10, 2023 at 10:09:28AM -0800, Sreevani Sreejith wrote:
> From: Sreevani <ssreevani@...a.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

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)