-
Notifications
You must be signed in to change notification settings - Fork 136
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for this, it looks great.
Apart from the non-trivial change of changing parsing code to unsafe
, my only other ask is to re-push the contents of the patchset to please the DCO bot.
You can see more info about that process here
Signed-off-by: Mikhail Trishchenkov <kriomant@gmail.com>
9b4be7a
to
929c067
Compare
Looking at the CI, we'll need to bump the build images to include |
I am very confused why the CI doesn't pass on this with the exception of 20.04. Will need to fire up a bunch of Dockers to dig deeper. |
@rhdxmr CI seems to fail with an error saying On aarch64 the error seems to be a bit more straightforward
|
@rsdy Okay I'll look into this problem and fix the issue.
I'll update this PR if I encounter another problem or find solutions. |
btf_dump__new is passed btf_dump_printf_fn_t that requires va_list. The implementation of va_list differs depending on architectures but libbpf-sys only supports amd64 bindings. The actual type of va_list in x86-64 is struct __va_list_tag. And va_list in aarch64 is [u64; 4usize]. For now It is uncertain that using __va_list_tag in aarch64 is okay. So keep using libbpf bindings only for functions relevant with va_list. Bump up libbpf to v0.6.1 since libbpf-sys uses it. Signed-off-by: Junyeong Jeong <rhdxmr@gmail.com>
Signed-off-by: Junyeong Jeong <rhdxmr@gmail.com>
688e253
to
f41b29c
Compare
Signed-off-by: Junyeong Jeong <rhdxmr@gmail.com>
541a72c
to
c2b4dcb
Compare
@rsdy |
Sorry for sitting on this review for so long. I wasn't super happy with the end implementation here, but after looking through libbpf-sys and weighing our options, it's not clear if this is a bug or known limitation on their end. I raised an issue there so we can hopefully migrate completely off of |
No description provided.