Date:   Fri, 10 Mar 2023 11:35:05 +0700
From:   Bagas Sanjaya <bagasdotme@gmail.com>
To:     Sreevani Sreejith <ssreevani@meta.com>, void@manifault.com,
        psreep@gmail.com, Linux BPF <bpf@vger.kernel.org>,
        Linux-kernel@vger.kernel.org, andrii@kernel.org, mykola@meta.com
Cc:     Linux Documentation <linux-doc@vger.kernel.org>
Subject: Re: [PATCH V2 bpf-next] BPF, docs: libbpf Overview Document
Message-ID: <ZAqzeQZLNMyaZOck@debian.me>
References: <20230309224930.4106982-1-ssreevani@meta.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="CrzSA/fAm0tW3LNz"
Content-Disposition: inline
In-Reply-To: <20230309224930.4106982-1-ssreevani@meta.com>
Precedence: bulk


--CrzSA/fAm0tW3LNz
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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

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

> +=3D=3D=3D=3D=3D=3D
>  libbpf
>  =3D=3D=3D=3D=3D=3D

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 =E2=80=93 Run Everywhere)
> +#########################################
> <snipped> ...

See the bottom.

> +You can generate the BTF information for the running kernel with the fol=
lowing
> +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 fi=
eld 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 pointe=
d to by
> +``dst``.
> +
> +  .. code-block:: C
> +    :emphasize-lines: 6
> +
> +    //...
> +    struct task_struct *task =3D (void *)bpf_get_current_task();
> +    struct task_struct *parent_task;
> +    int err;
> +
> +    err =3D 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.

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

--CrzSA/fAm0tW3LNz
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQSSYQ6Cy7oyFNCHrUH2uYlJVVFOowUCZAqzdQAKCRD2uYlJVVFO
o6OcAQClt3tCiKoO8NuaMrOiu4gmgX5OHuWjIm/L7+HHVI7MagD/ZgpKdhv+5eCn
5ITdBbMueewsYJm/fgQmt/UsinBnMQg=
=mWJu
-----END PGP SIGNATURE-----

--CrzSA/fAm0tW3LNz--