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

Add InitGlobalVariable to support setting global variables in bpf code #258

Merged
merged 8 commits into from
Oct 18, 2022

Conversation

mozillazg
Copy link
Contributor

closes #27

@mozillazg mozillazg marked this pull request as ready for review October 6, 2022 08:48
libbpfgo.go Show resolved Hide resolved
libbpfgo.go Outdated Show resolved Hide resolved
@grantseltzer grantseltzer added this to the v0.4.2-libbpf-1.0.0 milestone Oct 10, 2022
@mozillazg mozillazg changed the title Support setting global variables in bpf code Add InitGlobalVariable to support setting global variables in bpf code Oct 11, 2022
@grantseltzer
Copy link
Contributor

I think this approach isn't quite correct. When I've used global variables like this in pure C libbpf, the variables are stored as key/value pairs in a map just called .rodata. They way you show your approach working in the selftest creates a new map for each variable with one entry in each map. Screenshot below is testing with a libbpf C native approach:

image

@mozillazg
Copy link
Contributor Author

I think this approach isn't quite correct. When I've used global variables like this in pure C libbpf, the variables are stored as key/value pairs in a map just called .rodata. The way you show your approach working in the selftest creates a new map for each variable with one entry in each map.

@grantseltzer We can't init multiple global variables like the pure C way with skel->rodata->foobar due to can't update the internet map with a global variable name, so I use multiple internal maps to workaround. Do you know any better way to support multiple global variables?

@grantseltzer
Copy link
Contributor

grantseltzer commented Oct 12, 2022 via email

@grantseltzer
Copy link
Contributor

grantseltzer commented Oct 13, 2022

@mozillazg I've opened mozillazg#2 with a POC of what I have in mind. It works with the selftest as is, but it fails if you change one of the data types to not match the others (bpf_map__set_initial_value returns errno 22 (invalid arg))

Any thoughts?

@mozillazg
Copy link
Contributor Author

mozillazg commented Oct 14, 2022

Is it possible to get a handle on the '.rodata' map and iterate over the
keys

We can't iterate the map before load bpf object. The '.rodata' map only has one element and the value don't have a well-known struct

I've opened mozillazg#2 with a POC of what I have in mind. It works with the selftest as is, but it fails if you change one of the data types to not match the others (bpf_map__set_initial_value returns errno 22 (invalid arg)). Any thoughts?

It looks like we can't use normal way to update the map value. Is it possible to update the memory of value?

# parse elf info
elf_info = parse_object("main.bpf.o")

# get global variable info
offset, size = get_global_variable_info(elf_info, "foobar")    # how ??

# get address of map value via bpf_map__initial_value
map_value_pointer = bpf_map__initial_value(...)

# update value of global variable via update memory 
update_global_variable(map_value_pointer, offset, &new_foo_value, size)  # how ??

# maybe need to put it back?
bpf_map__set_initial_value(...)

@grantseltzer
Copy link
Contributor

The error in my approach appears to be an issue with struct memory alignment. If the types add up to a perfect alignment of 8 bytes it works, otherwise it gives the "invalid argument" error.

@mozillazg
Copy link
Contributor Author

The error in my approach appears to be an issue with struct memory alignment. If the types add up to a perfect alignment of 8 bytes it works, otherwise it gives the "invalid argument" error.

@grantseltzer 👌

It looks like we can't use normal way to update the map value. Is it possible to update the memory of value?

# parse elf info
elf_info = parse_object("main.bpf.o")

# get global variable info
offset, size = get_global_variable_info(elf_info, "foobar")    # how ??

# get address of map value via bpf_map__initial_value
map_value_pointer = bpf_map__initial_value(...)

# update value of global variable via update memory 
update_global_variable(map_value_pointer, offset, &new_foo_value, size)  # how ??

# maybe need to put it back?
bpf_map__set_initial_value(...)

A POC of this way: mozillazg#3

image

@mozillazg
Copy link
Contributor Author

mozillazg commented Oct 15, 2022

@grantseltzer Which solution would you prefer?

@grantseltzer
Copy link
Contributor

@mozillazg Your solution in mozillazg#3 works great! Nicely done, really enjoyed collaborating with you on this!

Only thing is I kindly ask you remove the print statements and get the selftest working (remove time.Sleep). Also my suggestion for the GoDoc comment about InitGlobalVariable to something like:

// InitGlobalVariable sets global variables (defined in .data or .rodata)
// in bpf code. It must be called before the BPF object is loaded.

@mozillazg
Copy link
Contributor Author

@grantseltzer updated codes to use solution like mozillazg#3 . please review it again.

Copy link
Contributor

@grantseltzer grantseltzer left a comment

Choose a reason for hiding this comment

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

Looks great, well done and thank you very much @mozillazg!

@grantseltzer grantseltzer merged commit 57b6f6d into aquasecurity:main Oct 18, 2022
bobrik added a commit to bobrik/ebpf_exporter that referenced this pull request Oct 18, 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.

eBPF CO-RE improvements: Add support for setting global variables in bpf code
3 participants