netdev - Re: [PATCH bpf-next 0/5] libbpf: error reporting changes for v1.0

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]

Date:   Mon, 24 May 2021 23:34:13 +0200
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     John Fastabend <john.fastabend@...il.com>,
        Stanislav Fomichev <sdf@...gle.com>,
        Andrii Nakryiko <andrii@...nel.org>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>
Subject: Re: [PATCH bpf-next 0/5] libbpf: error reporting changes for v1.0

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

> On Mon, May 24, 2021 at 1:53 PM Toke Høiland-Jørgensen <toke@...hat.com> wrote:
>>
>> Andrii Nakryiko <andrii.nakryiko@...il.com> writes:
>>
>> > On Sun, May 23, 2021 at 11:36 PM John Fastabend
>> > <john.fastabend@...il.com> wrote:
>> >>
>> >> Andrii Nakryiko wrote:
>> >> > Implement error reporting changes discussed in "Libbpf: the road to v1.0"
>> >> > ([0]) document.
>> >> >
>> >> > Libbpf gets a new API, libbpf_set_strict_mode() which accepts a set of flags
>> >> > that turn on a set of libbpf 1.0 changes, that might be potentially breaking.
>> >> > It's possible to opt-in into all current and future 1.0 features by specifying
>> >> > LIBBPF_STRICT_ALL flag.
>> >> >
>> >> > When some of the 1.0 "features" are requested, libbpf APIs might behave
>> >> > differently. In this patch set a first set of changes are implemented, all
>> >> > related to the way libbpf returns errors. See individual patches for details.
>> >> >
>> >> > Patch #1 adds a no-op libbpf_set_strict_mode() functionality to enable
>> >> > updating selftests.
>> >> >
>> >> > Patch #2 gets rid of all the bad code patterns that will break in libbpf 1.0
>> >> > (exact -1 comparison for low-level APIs, direct IS_ERR() macro usage to check
>> >> > pointer-returning APIs for error, etc). These changes make selftest work in
>> >> > both legacy and 1.0 libbpf modes. Selftests also opt-in into 100% libbpf 1.0
>> >> > mode to automatically gain all the subsequent changes, which will come in
>> >> > follow up patches.
>> >> >
>> >> > Patch #3 streamlines error reporting for low-level APIs wrapping bpf() syscall.
>> >> >
>> >> > Patch #4 streamlines errors for all the rest APIs.
>> >> >
>> >> > Patch #5 ensures that BPF skeletons propagate errors properly as well, as
>> >> > currently on error some APIs will return NULL with no way of checking exact
>> >> > error code.
>> >> >
>> >> >   [0] https://docs.google.com/document/d/1UyjTZuPFWiPFyKk1tV5an11_iaRuec6U-ZESZ54nNTY
>> >> >
>> >> > Andrii Nakryiko (5):
>> >> >   libbpf: add libbpf_set_strict_mode() API to turn on libbpf 1.0
>> >> >     behaviors
>> >> >   selftests/bpf: turn on libbpf 1.0 mode and fix all IS_ERR checks
>> >> >   libbpf: streamline error reporting for low-level APIs
>> >> >   libbpf: streamline error reporting for high-level APIs
>> >> >   bpftool: set errno on skeleton failures and propagate errors
>> >> >
>> >>
>> >> LGTM for the series,
>> >>
>> >> Acked-by: John Fastabend <john.fastabend@...il.com>
>> >
>> > Thanks, John!
>> >
>> > Toke, Stanislav, you cared about these aspects of libbpf 1.0 (by
>> > commenting on the doc itself), do you mind also taking a brief look
>> > and letting me know if this works for your use cases? Thanks!
>>
>> Changes LGTM:
>>
>> Acked-by: Toke Høiland-Jørgensen <toke@...hat.com>
>>
>> As a side note, the series seems to have been chopped up into individual
>> emails with no threading; was a bit weird that I had to go hunting for
>> the individual patches in my mailbox...
>>
>
> That's my bad, I messed up and sent them individually and probably
> that's why they weren't threaded properly.

Right, OK, I'll stop looking for bugs on my end, then :)

BTW, one more thing that just came to mind: since that gdoc is not
likely to be around forever, would it be useful to make the reference in
the commit message(s) point to something more stable? IDK what that
shoul be, really. Maybe just pasting (an abbreviated outline of?) the
text in the document into the cover letter / merge commit could work?

-Toke