rust-bpf / rust-bcc

user-friendly rust bindings for the bpf compiler collection
MIT License
475 stars 54 forks source link

Add cflags support #127

Closed javierhonduco closed 4 years ago

javierhonduco commented 4 years ago

This PR adds support for passing a vector of cflags that will be passed to the bcc backend and processed by clang's preprocessor

Template

The BCC Python and C++ bindings allow passing CFLAGS to the BPF program. We are using this feature in a project with a Python BCC driver and are currently experimenting with Rust so we would love to be able to use it :smile:

Moves bpf_module_create_c_from_string to its own function, leaving new with the same signature, and adds new_with_cflags, to which you can pass CFLAGS has a second argument

It should not break backwards compatibility. I am a not experienced with Rust (first Rust PR!), so I am unsure if there is a better way to do this!

Test plan

Added a simple smoke test

➜  rust-bcc git:(cflags-support) ✗ cargo build --examples
    Finished dev [unoptimized + debuginfo] target(s) in 0.02s
➜  rust-bcc git:(cflags-support) ✗ sudo target/debug/examples/smoketest 
smoketest: empty table
smoketest: sized histogram
smoketest: hash insert and delete
smoketest: array
smoketest: cflags
smoketest passed

Thanks!

javierhonduco commented 4 years ago

cc @rdelfin In case you have any feedback!

brayniac commented 4 years ago

Thanks for opening this! Seems like a good feature to add.

I'd rather see this implemented using the builder pattern instead of adding a dedicated function to create w/ CFLAGs. This will give us more flexibility going forward and addresses the concern you had around adding more args in the future.

I'll see if I can take this on.

brayniac commented 4 years ago

@javierhonduco - can you see if the version in master since #128 landed meets your current needs?

javierhonduco commented 4 years ago

Sorry for the delay, just came from holidays :)

I'd rather see this implemented using the builder pattern instead of adding a dedicated function to create w/ CFLAGs. This will give us more flexibility going forward and addresses the concern you had around adding more args in the future.

Totally, this is way better! Just left some comments in #128! :)