netdev - Re: [PATCH RFC bpf-next 1/3] libbpf: Add new bpf_object_

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: <87eewz2rvb.fsf@toke.dk>
Date:   Fri, 20 Dec 2019 11:50:16 +0100
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        David Miller <davem@...emloft.net>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH RFC bpf-next 1/3] libbpf: Add new bpf_object__load2() using new-style opts

Andrii Nakryiko <andrii.nakryiko@...il.com> writes:

> On Thu, Dec 19, 2019 at 6:29 AM Toke Høiland-Jørgensen <toke@...hat.com> wrote:
>>
>> From: Toke Høiland-Jørgensen <toke@...hat.com>
>>
>> Since we introduced DECLARE_LIBBPF_OPTS and related macros for declaring
>> function options, that is now the preferred way to extend APIs. Introduce a
>> variant of the bpf_object__load() function that uses this function, and
>> deprecate the _xattr variant. Since all the good function names were taken,
>> the new function is unimaginatively called bpf_object__load2().
>>
>> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
>> ---
>
> I've been thinking about options for load quite a bit lately, and I'm
> leaning towards an opinion, that bpf_object__load() shouldn't take any
> options, and all the various per-bpf_object options have to be
> specified in bpf_object_open_opts and stored, if necessary for
> load/attach phase. So I'd rather move target_btf_path and log_level to
> open_opts instead.

Hmm, yeah, don't really object to that. I do think the 'log_level' is a
bit of an odd parameter in any case, though. If I turn on verbose
logging using the log_level parameter, that won't affect the logging of
libbpf itself, which was certainly surprising to me when I first
discovered it. So maybe rename it when adding it as an open option
("verbose_verifier" or something along those lines?).

Anyhow, given your idea with having a separate bpf_linker__() type, this
is not really needed for linking in any case, so I'll just drop this
patch for now...

-Toke