lizrice / learning-ebpf

Learning eBPF, published by O'Reilly - out now! Here's where you'll find a VM config for the examples, and more
https://www.amazon.com/Learning-eBPF-Programming-Observability-Networking/dp/1098135121
Apache License 2.0
1.19k stars 255 forks source link

Fix extracting pid form bpf_get_current_pid_tgid() #36

Closed AliJavidiCS closed 5 months ago

AliJavidiCS commented 9 months ago

Based on Issue #35, pid must be extracted from the return value by using an and operation with 0xFFFFFFFF.

parttimenerd commented 7 months ago

Why? The documentation tells us:

Returns the process ID in the lower 32 bits (kernel's view of the PID, which in user space is usually presented as the thread ID), and the thread group ID in the upper 32 bits (what user space often thinks of as the PID). By directly setting this to a u32, we discard the upper 32 bits.

Therefore the current code seems to be correct.

AliJavidiCS commented 7 months ago

I agree, The documentation states that PID is stored in the lower 32 bits. However the code actually throws away the lower 32 bits as in the line of code below: data.pid = bpf_get_current_pid_tgid() >> 32;

If we insist on shifting the result to get the PID we might use something like this: data.pid = (bpf_get_current_pid_tgid() << 32) >> 32;

Besides, the book refers to the higher 32 bits as PID as I mentioned in #35 and the current code would be correct if that was the case.

lizrice commented 5 months ago

Thanks for this @AliJavidiCS. Per the comments in #35 I think the code is actually correct here, though the footnote needs a better explanation!