[DTrace-devel] [PATCH 08/12] Remove libbpf dependency

[Kris Van Hees]

On Sat, Jul 16, 2022 at 01:52:15PM -0700, Eugene Loh via DTrace-devel wrote:
> Again, sorry for the weirdly formatted replies:  this is another patch that
> never appeared in my inbox.
> 
> Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
> 
> A few comments:
> 
> *)  In file CODING-STYLE, the last three lines can be removed.
>     They are (including a blank line):
> 
>         Code in libbpf/ need not conform to the guidelines in this document,
> since
>         we expect not to retain that code in the long term.

Ah yes, thanks.

> *)  In libdtrace/dt_bpf.c, you comment out
> 
>         /* #include <bpf.h> */
> 
>     That line can be removed entirely.

Yes, I forgot to do that.

> *)  In libdtrace/dt_bpf.c, in dt_bpf_prog_load(), you have
> 
>         do {
>                 fd = bpf(BPF_PROG_LOAD, &attr);
>         } while (fd < 0 && errno == EAGAIN);
> 
>     I do not see the EAGAIN case documented in the bpf() man page.
>     Where is this behavior described?

bpf() is a syscall, and therefore it can fail with errno EAGAIN.

>     Also, do we really want an endless loop here?  Maybe there should
>     be a time-out or a limited number of retries?

It is almost certainly safe, but I agree that it is even safer if we put a
limit on the number of retries.  I'll add that.

> *)  Finally, it's a little confusing to have both dt_bpf_load_prog() and
>     dt_bpf_prog_load(), but perhaps the confusion is worth the hassle of
>     coming up with different names and changing stuff.  I don't know.
>     It looks funny, but can stay the way it is.

Yes, the naming gets a bit strange but I don't think it is that difficult to
figure out either and I honestly don't know what names would make it much
better.  I'll stick with it as-is for now.