Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vendor deprecated btf_ext APIs and structs #3787

Merged

Conversation

davemarchevsky
Copy link
Collaborator

Some btf_ext-related APIs in libbpf are being deprecated because they
make incorrect assumptions. They're being used only by bcc currently, so
vendor them before they get deleted.

After / as part of #3660, may need to revisit the incorrect assumptions
being made here.

The functions and structs were ripped directly from libbpf with minimal
changes:

  • Change void* arithmetic to uint8_t
  • __u32 -> uint32_t and similar
  • Add a wrapping namespace
  • rec_size functions were not needed - just grab the rec_size
    directly since type is no longer opaque to bcc

@davemarchevsky
Copy link
Collaborator Author

davemarchevsky commented Jan 4, 2022

need to remove __bcc prefix from things

edit: did so

@davemarchevsky davemarchevsky force-pushed the davemarchevsky_btf_ext branch from d0d478d to 3a17a38 Compare January 4, 2022 22:42
Copy link
Collaborator

@yonghong-song yonghong-song left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btf_load() is also deprecated.

/home/yhs/work/bcc/src/cc/bcc_btf.cc:600:20: warning: ‘int btf__load(btf*)’ is deprecated: libbpf v0.6+: use btf__load_into_kernel instead [-Wdeprecated-declarations]

Maybe to check to use btf__load_into_kernel()?

If we want to accommodate the external libbpf build, you can copy btf__load() function as well. Hopefully, eventually all these (bpf_ext* and btf__load) are gone and handled by libbpf library directly.

I did a simple test with profile.py with this patch and dump program xlated code. It appears the patch is working and I can see proper source annotation in xlated code.

done:
if (err) {
btf_ext__free(btf_ext);
return (struct btf_ext*)err;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

err is 'int' type, converting to 'struct btf_ext *' will trigger the warning:

/home/yhs/work/bcc/src/cc/bcc_btf.cc: In function ‘btf_ext_vendored::btf_ext* btf_ext_vendored::btf_ext__new(const uint8_t*, uint32_t)’:                                            
/home/yhs/work/bcc/src/cc/bcc_btf.cc:276:41: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]                                                        
                 return (struct btf_ext*)err;                                                                                                                                       
                                         ^~~ 

You can do (struct btf_ext *)(long)err.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed, but with a uintptr_t cast instead of long

@chenhengqi
Copy link
Collaborator

Three libbpf APIs used by BCC are deprecated.

  • bpf_create_map_xattr -> bpf_map_create
  • bpf_load_program_xattr -> bpf_prog_load
  • btf__load -> btf__load_into_kernel

@davemarchevsky Are you going to address these in this PR ?

Or I may pick this up.

@davemarchevsky
Copy link
Collaborator Author

Maybe to check to use btf__load_into_kernel()?

@davemarchevsky Are you going to address these in this PR ?

I think those migrations away from other deprecated stuff should be addressed in a separate PR. @chenhengqi if you'd like to do it plz feel free, otherwise I will pick up late this week / early next.

@yonghong-song
Copy link
Collaborator

I think those migrations away from other deprecated stuff should be addressed in a separate PR. @chenhengqi if you'd like to do it plz feel free, otherwise I will pick up late this week / early next.

okay. let us address btf__load API in the separate PR. Please fix the warning and I think we will be good to go.

@davemarchevsky davemarchevsky force-pushed the davemarchevsky_btf_ext branch from 3a17a38 to 57dbccf Compare January 5, 2022 02:39
Some btf_ext-related APIs in libbpf are being deprecated because they
make incorrect assumptions. They're being used only by bcc currently, so
vendor them before they get deleted.

After / as part of iovisor#3660, may need to revisit the incorrect assumptions
being made here.

The functions and structs were ripped directly from libbpf with minimal
changes:
  * Change void* arithmetic to uint8_t
  * __u32 -> uint32_t and similar
  * Add a wrapping namespace
  * `rec_size` functions were not needed - just grab the rec_size
directly since type is no longer opaque to bcc
@davemarchevsky davemarchevsky force-pushed the davemarchevsky_btf_ext branch from 57dbccf to 94d4491 Compare January 5, 2022 02:47
@yonghong-song yonghong-song merged commit 9fce505 into iovisor:master Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants