kernel-janitors.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Shuyi Cheng <chengshuyi@linux.alibaba.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>, Martin Lau <kafai@fb.com>,
	Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
	john fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@kernel.org>,
	Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>,
	kernel-janitors@vger.kernel.org
Subject: Re: [PATCH bpf-next v3 0/2] libbpf: Introduce 'btf_custom_path' to 'bpf_obj_open_opts'
Date: Mon, 12 Jul 2021 17:01:59 -0700	[thread overview]
Message-ID: <CAEf4BzY2cdT44bfbMus=gei27ViqGE1BtGo6XrErSsOCnqtVJg@mail.gmail.com> (raw)
In-Reply-To: <1625798873-55442-1-git-send-email-chengshuyi@linux.alibaba.com>

On Thu, Jul 8, 2021 at 7:48 PM Shuyi Cheng <chengshuyi@linux.alibaba.com> wrote:
>
> Patch 1: Add 'btf_custom_path' to 'bpf_obj_open_opts', allow developers
> to use custom btf to perform CO-RE relocation.
>
> Patch 2: Fixed the memory leak problem pointed out by Andrii.
>

Please note that the cover letter should have a high-level overview of
what the set of patches you are sending is doing, not just a
changelog. So in this case, as an example for the future
contributions, I'd write something like this:

```
This patch set adds the ability to point to a custom BTF for the
purposes of BPF CO-RE relocations. This is useful for using BPF CO-RE
on old kernels that don't yet natively support kernel (vmlinux) BTF
and thus libbpf needs application's help in locating kernel BTF
generated separately from the kernel itself. This was already possible
to do through bpf_object__load's attribute struct, but that makes it
inconvenient to use with BPF skeleton, which only allows to specify
bpf_object_open_opts during the open step. Thus, add the ability to
override vmlinux BTF at open time.

Patch #1 adds libbpf changes. Patch #2 fixes pre-existing memory leak
detected during the code review. Patch #3 switches existing selftests
to using open_opts for custom BTF.
```

BTW, see the description above about selftests (fictional patch #3).
Please update selftests core_autosize.c and core_reloc.c to use the
new functionality instead of load_attr.target_btf_path. It's a general
rule to always add a new test or update existing test to utilize newly
added functionality. That way we can know that it actually works as
expected.


> Changelog:
> ----------
>
> v2: https://lore.kernel.org/bpf/CAEf4Bza_ua+tjxdhyy4nZ8Boeo+scipWmr_1xM1pC6N5wyuhAA@mail.gmail.com/T/#mf9cf86ae0ffa96180ac29e4fd12697eb70eccd0f
> v2->v3:
> --- Load the BTF specified by btf_custom_path to btf_vmlinux_override
>     instead of btf_bmlinux.
> --- Fix the memory leak that may be introduced by the second version
>     of the patch.
> --- Add a new patch to fix the possible memory leak caused by
>     obj->kconfig.
>
> v1: https://lore.kernel.org/bpf/CAEf4BzaGjEC4t1OefDo11pj2-HfNy0BLhs_G2UREjRNTmb2u=A@mail.gmail.com/t/#m4d9f7c6761fbd2b436b5dfe491cd864b70225804
> v1->v2:
> -- Change custom_btf_path to btf_custom_path.
> -- If the length of btf_custom_path of bpf_obj_open_opts is too long,
>    return ERR_PTR(-ENAMETOOLONG).
> -- Add `custom BTF is in addition to vmlinux BTF`
>    with btf_custom_path field.
>
> Shuyi Cheng (2):
>   libbpf: Introduce 'btf_custom_path' to 'bpf_obj_open_opts'
>   libbpf: Fix the possible memory leak caused by obj->kconfig
>
>  tools/lib/bpf/libbpf.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++----
>  tools/lib/bpf/libbpf.h |  6 +++++-
>  2 files changed, 53 insertions(+), 5 deletions(-)
>
> --
> 1.8.3.1
>

      parent reply	other threads:[~2021-07-13  0:02 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-09  2:47 [PATCH bpf-next v3 0/2] libbpf: Introduce 'btf_custom_path' to 'bpf_obj_open_opts' Shuyi Cheng
2021-07-09  2:47 ` [PATCH bpf-next v3 1/2] " Shuyi Cheng
2021-07-12 23:53   ` Andrii Nakryiko
2021-07-09  2:47 ` [PATCH bpf-next v3 2/2] libbpf: Fix the possible memory leak caused by obj->kconfig Shuyi Cheng
2021-07-10 14:42   ` Dan Carpenter
2021-07-11  1:27     ` Shuyi Cheng
2021-07-13  0:01 ` Andrii Nakryiko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAEf4BzY2cdT44bfbMus=gei27ViqGE1BtGo6XrErSsOCnqtVJg@mail.gmail.com' \
    --to=andrii.nakryiko@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=chengshuyi@linux.alibaba.com \
    --cc=daniel@iogearbox.net \
    --cc=john.fastabend@gmail.com \
    --cc=kafai@fb.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=songliubraving@fb.com \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).