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

[RFC] Get values from per-cpu maps #2

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

levaitamas
Copy link
Contributor

Per-cpu maps are special, they store multiple values to a single key; a value for each CPU core. The bpf_map_lookup_elem() returns all these values. But when I call a get() on a RawMap, it returns a single value only since it allocates a buffer which is as long as the map's value size. To handle per-cpu maps, I propose a new getPerCPU() function which returns a buffer containing all values. wdyt?

@mildsunrise
Copy link
Owner

Hi! As you see there's no per-cpu support yet, I think the way to go about it would be:

  • get() returns an array of buffers/values for per-cpu maps. I imagine set() and other operations would have to be modified similarly. or,

  • forbid the current classes from being used on per-cpu maps and create new ones (ConvPerCpuMap, RawPerCpuMap, ConvPerCpuArrayMap, RawPerCpuArrayMap)

the rationale for this is that right now get() causes a buffer overflow and I don't want users to be able to segfault node. the first option would require less code duplication, but it's not type safe. the second option would allow us to handle the CPU flag better

@@ -315,6 +316,11 @@ Napi::Value BpfObjGet(const CallbackInfo& info) {
return ToStatus(env, bpf_obj_get(path.c_str()));
}

Napi::Value GetNumPossibleCPUs(const CallbackInfo& info) {
Napi::Env env = info.Env();
return ToStatus(env, libbpf_num_possible_cpus());
Copy link
Owner

@mildsunrise mildsunrise Jan 21, 2021

Choose a reason for hiding this comment

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

nit: we can't use ToStatus here, the function returns the error code directly (not through errno)

@levaitamas
Copy link
Contributor Author

Thanks, that sounds like a great plan! Unfortunately, I have to focus on some other stuff, so let me just put this on my weekend project list.

rg0now pushed a commit to rg0now/l7mp that referenced this pull request Jun 7, 2021
The node_bpf packages has no support for per-cpu bpf maps yet.  As a
quick workaround we stick to non percpu-maps.  See
mildsunrise/node_bpf#2 for details.
levaitamas added a commit to l7mp/l7mp that referenced this pull request Jun 7, 2021
The node_bpf packages has no support for per-cpu bpf maps yet.  As a
quick workaround we stick to non percpu-maps.  See
mildsunrise/node_bpf#2 for details.
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.

2 participants