iovisor / bcc

BCC - Tools for BPF-based Linux IO analysis, networking, monitoring, and more
Apache License 2.0
20.59k stars 3.88k forks source link

Allow to set single values on percpu maps with python #1886

Open banh-gao opened 6 years ago

banh-gao commented 6 years ago

I'm using a percpu_hash and I need to modify the value on a map specific to a cpu from the userland.

As introduced with the PR #458 the map can be modified via the python binding but the entry of all maps has to be modified at the same time. This is something I need to avoid since I don't want to modify the entries of maps belonging to other CPUs.

Currently I'm doing something like:

global_value = bpf_map[key]
global_value[cpu] = 0
bpf_map[key] = global_value

However between the read at line 1 and the write at line 3 the value of the maps not belonging to cpu and copied in global_value could have been changed.

Is there a way to only update the map belonging to cpu?

mauriciovasquezbernal commented 6 years ago

I also hit the same problem using the C++ API. The kernel implementation only allows to update the value for all CPUs from userspace. It is done in bpf_percpu_hash_update [1].

@4ast do you have any thoughts on this from the kernel side?

[1] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/tree/kernel/bpf/syscall.c#n752

4ast commented 6 years ago

we need to extend kernel api to lookup/update only specific cpu in per-cpu maps. How do you propose syscall api would look like? kernel patches are welcome. @borkmann

palmtenor commented 6 years ago

Maybe we can use the flags in bpf_map_*_elem's bpf_attr to specify which CPU to use...

4ast commented 6 years ago

good idea! Looks like we cannot reuse BPF_F_CURRENT_CPU for the same purpose. But something like BPF_F_CPU 4 and some other bits encoding the cpu should work for both lookup and update.

mauriciovasquezbernal commented 6 years ago

I am interested in implementing it. I am pretty new at kernel developing so I'd like to ask some questions here before spamming the mailing lists.

Thanks, Mauricio

yonghong-song commented 6 years ago

Maybe the format bpf_map_update_elem(..., cpu_id << 32 | BPF_CPU) is better? I am using BPF_CPU to be consistent with other flags BPF_ANY/NOEXIST/EXIST. You may want to define BPF_CPU_MASK (0xFFFFFFFF00000000ULL) so kernel can use the mask for verification purpose.

We cannot add flags to bpf_map_lookup_elem helper for backward compatibility reasons. This may not have a lot of values anyway since user space can easily get the pointer to the cpu value through &vals[cpu].

pchaigno commented 4 years ago

I have submitted a patchset upstream to implement this feature: https://lore.kernel.org/bpf/cover.1576673841.git.paul.chaignon@orange.com.

yonghong-song commented 4 years ago

Great. Thanks!