| Message ID | 20210521234203.1283033-1-andrii@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | libbpf: error reporting changes for v1.0 | expand |
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@gmail.com>
On Sun, May 23, 2021 at 11:36 PM John Fastabend <john.fastabend@gmail.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@gmail.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!
On Mon, May 24, 2021 at 12:19 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Sun, May 23, 2021 at 11:36 PM John Fastabend > <john.fastabend@gmail.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@gmail.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! I took a quick look earlier today and everything looks good, thanks! I'll try to enable strict mode in our codebase in the coming weeks to see how it goes.
Andrii Nakryiko <andrii.nakryiko@gmail.com> writes: > On Sun, May 23, 2021 at 11:36 PM John Fastabend > <john.fastabend@gmail.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@gmail.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@redhat.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...
On Mon, May 24, 2021 at 1:35 PM Stanislav Fomichev <sdf@google.com> wrote: > > On Mon, May 24, 2021 at 12:19 PM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Sun, May 23, 2021 at 11:36 PM John Fastabend > > <john.fastabend@gmail.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@gmail.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! > > I took a quick look earlier today and everything looks good, thanks! Great, thanks for looking. > I'll try to enable strict mode in our codebase in the coming weeks to > see how it goes. Keep in mind, if you do libbpf_set_strict_mode(LIBBPF_STRICT_ALL) you are automatically opting in for all the future "features", so for production you might want to go conservative and start with specifying explicitly LIBBPF_STRICT_DIRECT_ERRS | LIBBPF_STRICT_CLEAN_PTRS, and then add more as you check that your code will handle new changes.
On Mon, May 24, 2021 at 1:53 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote: > > Andrii Nakryiko <andrii.nakryiko@gmail.com> writes: > > > On Sun, May 23, 2021 at 11:36 PM John Fastabend > > <john.fastabend@gmail.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@gmail.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@redhat.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.
Andrii Nakryiko <andrii.nakryiko@gmail.com> writes: > On Mon, May 24, 2021 at 1:53 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote: >> >> Andrii Nakryiko <andrii.nakryiko@gmail.com> writes: >> >> > On Sun, May 23, 2021 at 11:36 PM John Fastabend >> > <john.fastabend@gmail.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@gmail.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@redhat.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
On Mon, May 24, 2021 at 2:34 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote: > > Andrii Nakryiko <andrii.nakryiko@gmail.com> writes: > > > On Mon, May 24, 2021 at 1:53 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote: > >> > >> Andrii Nakryiko <andrii.nakryiko@gmail.com> writes: > >> > >> > On Sun, May 23, 2021 at 11:36 PM John Fastabend > >> > <john.fastabend@gmail.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@gmail.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@redhat.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? I was hoping Google won't deprecate Google Docs any time soon and I had no intention to remove that document. But I was also thinking to start wiki page at github.com/libbpf/libbpf with migration instructions, so once that is up and running I can link that from libbpf_set_strict_mode() doc comment. But I'd like to avoid blocking on that. > > -Toke >
Andrii Nakryiko <andrii.nakryiko@gmail.com> writes: > On Mon, May 24, 2021 at 2:34 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote: >> >> Andrii Nakryiko <andrii.nakryiko@gmail.com> writes: >> >> > On Mon, May 24, 2021 at 1:53 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote: >> >> >> >> Andrii Nakryiko <andrii.nakryiko@gmail.com> writes: >> >> >> >> > On Sun, May 23, 2021 at 11:36 PM John Fastabend >> >> > <john.fastabend@gmail.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@gmail.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@redhat.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? > > I was hoping Google won't deprecate Google Docs any time soon and I > had no intention to remove that document. But I was also thinking to > start wiki page at github.com/libbpf/libbpf with migration > instructions, so once that is up and running I can link that from > libbpf_set_strict_mode() doc comment. Right, that sounds reasonable :) > But I'd like to avoid blocking on that. Understandable; but just pasting an outline into the commit message (and keeping the link) could work in the meantime? -Toke
On Mon, May 24, 2021 at 3:20 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote: > > Andrii Nakryiko <andrii.nakryiko@gmail.com> writes: > > > On Mon, May 24, 2021 at 2:34 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote: > >> > >> Andrii Nakryiko <andrii.nakryiko@gmail.com> writes: > >> > >> > On Mon, May 24, 2021 at 1:53 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote: > >> >> > >> >> Andrii Nakryiko <andrii.nakryiko@gmail.com> writes: > >> >> > >> >> > On Sun, May 23, 2021 at 11:36 PM John Fastabend > >> >> > <john.fastabend@gmail.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@gmail.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@redhat.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? > > > > I was hoping Google won't deprecate Google Docs any time soon and I > > had no intention to remove that document. But I was also thinking to > > start wiki page at github.com/libbpf/libbpf with migration > > instructions, so once that is up and running I can link that from > > libbpf_set_strict_mode() doc comment. > > Right, that sounds reasonable :) > > > But I'd like to avoid blocking on that. > > Understandable; but just pasting an outline into the commit message (and > keeping the link) could work in the meantime? I'm not sure what are we trying to achieve by copy/pasting parts of that doc here. Each patch succinctly explains how each feature behaves, so it's completely self-describing. I put the link to the document for anyone that wants to read the entire discussion or leave some more comments, but it's not mandatory to understand this patch set. > > -Toke >
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 tools/bpf/bpftool/gen.c | 27 +- tools/lib/bpf/Makefile | 1 + tools/lib/bpf/bpf.c | 168 ++++-- tools/lib/bpf/bpf_prog_linfo.c | 18 +- tools/lib/bpf/btf.c | 302 +++++----- tools/lib/bpf/btf_dump.c | 14 +- tools/lib/bpf/libbpf.c | 519 ++++++++++-------- tools/lib/bpf/libbpf.h | 1 + tools/lib/bpf/libbpf.map | 5 + tools/lib/bpf/libbpf_errno.c | 7 +- tools/lib/bpf/libbpf_internal.h | 53 ++ tools/lib/bpf/libbpf_legacy.h | 59 ++ tools/lib/bpf/linker.c | 22 +- tools/lib/bpf/netlink.c | 81 +-- tools/lib/bpf/ringbuf.c | 26 +- tools/testing/selftests/bpf/bench.c | 1 + .../selftests/bpf/benchs/bench_rename.c | 2 +- .../selftests/bpf/benchs/bench_ringbufs.c | 6 +- .../selftests/bpf/benchs/bench_trigger.c | 2 +- .../selftests/bpf/prog_tests/attach_probe.c | 12 +- .../selftests/bpf/prog_tests/bpf_iter.c | 31 +- .../selftests/bpf/prog_tests/bpf_tcp_ca.c | 8 +- tools/testing/selftests/bpf/prog_tests/btf.c | 93 ++-- .../selftests/bpf/prog_tests/btf_dump.c | 8 +- .../selftests/bpf/prog_tests/btf_write.c | 4 +- .../bpf/prog_tests/cg_storage_multi.c | 84 +-- .../bpf/prog_tests/cgroup_attach_multi.c | 2 +- .../selftests/bpf/prog_tests/cgroup_link.c | 14 +- .../bpf/prog_tests/cgroup_skb_sk_lookup.c | 2 +- .../selftests/bpf/prog_tests/check_mtu.c | 2 +- .../selftests/bpf/prog_tests/core_reloc.c | 15 +- .../selftests/bpf/prog_tests/fexit_bpf2bpf.c | 25 +- .../selftests/bpf/prog_tests/flow_dissector.c | 2 +- .../bpf/prog_tests/flow_dissector_reattach.c | 10 +- .../bpf/prog_tests/get_stack_raw_tp.c | 10 +- .../prog_tests/get_stackid_cannot_attach.c | 9 +- .../selftests/bpf/prog_tests/hashmap.c | 9 +- .../selftests/bpf/prog_tests/kfree_skb.c | 19 +- .../selftests/bpf/prog_tests/ksyms_btf.c | 3 +- .../selftests/bpf/prog_tests/link_pinning.c | 7 +- .../selftests/bpf/prog_tests/obj_name.c | 8 +- .../selftests/bpf/prog_tests/perf_branches.c | 4 +- .../selftests/bpf/prog_tests/perf_buffer.c | 2 +- .../bpf/prog_tests/perf_event_stackmap.c | 3 +- .../selftests/bpf/prog_tests/probe_user.c | 7 +- .../selftests/bpf/prog_tests/prog_run_xattr.c | 4 +- .../bpf/prog_tests/raw_tp_test_run.c | 4 +- .../selftests/bpf/prog_tests/rdonly_maps.c | 7 +- .../bpf/prog_tests/reference_tracking.c | 2 +- .../selftests/bpf/prog_tests/resolve_btfids.c | 2 +- .../selftests/bpf/prog_tests/ringbuf_multi.c | 2 +- .../bpf/prog_tests/select_reuseport.c | 53 +- .../selftests/bpf/prog_tests/send_signal.c | 3 +- .../selftests/bpf/prog_tests/sk_lookup.c | 2 +- .../selftests/bpf/prog_tests/sock_fields.c | 14 +- .../selftests/bpf/prog_tests/sockmap_basic.c | 8 +- .../selftests/bpf/prog_tests/sockmap_ktls.c | 2 +- .../selftests/bpf/prog_tests/sockmap_listen.c | 10 +- .../bpf/prog_tests/stacktrace_build_id_nmi.c | 3 +- .../selftests/bpf/prog_tests/stacktrace_map.c | 2 +- .../bpf/prog_tests/stacktrace_map_raw_tp.c | 5 +- .../bpf/prog_tests/tcp_hdr_options.c | 15 +- .../selftests/bpf/prog_tests/test_overhead.c | 12 +- .../bpf/prog_tests/trampoline_count.c | 14 +- .../selftests/bpf/prog_tests/udp_limit.c | 7 +- .../selftests/bpf/prog_tests/xdp_bpf2bpf.c | 2 +- .../selftests/bpf/prog_tests/xdp_link.c | 8 +- tools/testing/selftests/bpf/test_maps.c | 168 +++--- tools/testing/selftests/bpf/test_progs.c | 3 + tools/testing/selftests/bpf/test_progs.h | 9 +- .../selftests/bpf/test_tcpnotify_user.c | 7 +- 71 files changed, 1123 insertions(+), 952 deletions(-) create mode 100644 tools/lib/bpf/libbpf_legacy.h