Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

Use libbpf-sys #278

Merged
merged 6 commits into from
Feb 21, 2022
Merged

Use libbpf-sys #278

merged 6 commits into from
Feb 21, 2022

Conversation

kriomant
Copy link
Contributor

@kriomant kriomant commented Feb 7, 2022

No description provided.

Copy link
Collaborator

@rsdy rsdy left a 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

redbpf/src/lib.rs Show resolved Hide resolved
@rsdy rsdy mentioned this pull request Feb 7, 2022
Signed-off-by: Mikhail Trishchenkov <kriomant@gmail.com>
@rsdy
Copy link
Collaborator

rsdy commented Feb 8, 2022

Looking at the CI, we'll need to bump the build images to include pkg-config to successfully build this this branch.

@rsdy
Copy link
Collaborator

rsdy commented Feb 10, 2022

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.

@rsdy
Copy link
Collaborator

rsdy commented Feb 15, 2022

@rhdxmr CI seems to fail with an error saying Feb 13 13:44:42.856 DEBUG build_script_build: Generating bindings with BTF of vmlinux. Do you think you could look into this?

On aarch64 the error seems to be a bit more straightforward

 error[E0412]: cannot find type `__gnuc_va_list` in module `super`
   --> bpf-sys/src/type_gen.rs:273:54
    |
273 |     #[cfg(not(target_env = "musl"))] va_list: super::__gnuc_va_list,
    |                                                      ^^^^^^^^^^^^^^ not found in `super`

@rhdxmr
Copy link
Collaborator

rhdxmr commented Feb 15, 2022

@rsdy Okay I'll look into this problem and fix the issue.
It seems that there are two problems..

  1. Failed to generate rust bindings for the Linux kernel structures from vmlinux BTF
  2. Failed to compile in aarch64

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>
Signed-off-by: Junyeong Jeong <rhdxmr@gmail.com>
Signed-off-by: Junyeong Jeong <rhdxmr@gmail.com>
@rhdxmr
Copy link
Collaborator

rhdxmr commented Feb 18, 2022

@rsdy
All problems are solved. I think it is okay now.
I double checked all modifications but it would be better if you check for the last time.

@rsdy rsdy merged commit af990ff into foniod:main Feb 21, 2022
@rsdy
Copy link
Collaborator

rsdy commented Feb 21, 2022

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 bpf-sys once we clear this up. Thanks both!

@kriomant kriomant deleted the feature/libbpf branch February 27, 2022 14:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants