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

inet_csk_accept ebpf program doesn't work properly #715

Open
rectified95 opened this issue Sep 10, 2024 · 0 comments
Open

inet_csk_accept ebpf program doesn't work properly #715

rectified95 opened this issue Sep 10, 2024 · 0 comments

Comments

@rectified95
Copy link
Contributor

rectified95 commented Sep 10, 2024

kretprobe/inet_csk_accept (reason TCP_ACCEPT_BASIC) has two big issues:

  1. it doesn't track bytes dropped for any packet it processes (it calls get_packet_from_sock which doesn't populate the packet's skb_len, since that value is not available in the socket struct this ebpf hook receives as input; this means that the subsequent call to update_metrics_map doesn't increase the drop bytes metrics)
  2. it misses some drops (it does a single rather than double pointer dereference of the value retrieved from accept_pids map; the rerieved pointer (and not underlying value) is assigned to int which given 2's complement notation means that anytime the memory address of the err value has the most significant bit == 0 (positive number), it returns early thinking the underlying function didn't indicate a drop)

Solutions:

  1. We'd need to figure out how to get the sk_buff from the socket.
  2. I tried 2 approaches unsuccessfully so far: doing the second pointer deref (ebpf verifier failure); storing the actual error value from inet_csk_accept rather than the pointer, as a signed int, which obviates the need for the double deref, but also got a verifier error.
github-merge-queue bot pushed a commit that referenced this issue Sep 13, 2024
# Description

For all eBPF program in the DropReason plugin (except `inet_csk_accept`
which has issue we need to investigate
#715):
- only make ebpf map calls when necessary
- omit setting some packet fields to 0 right after `memset` is called on
the entire struct

**Details**:
Previously, we did a map lookup regardless of whether the input `retVal`
indicated a drop. Now, only for drops.
We also skip a map delete when there wasn't a earlier kprobe that saved
the corresponding PID.

## Checklist

- [x] I have read the [contributing
documentation](https://retina.sh/docs/contributing).
- [x] I signed and signed-off the commits (`git commit -S -s ...`). See
[this
documentation](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification)
on signing commits.
- [x] I have correctly attributed the author(s) of the code.
- [x] I have tested the changes locally.
- [x] I have followed the project's style guidelines.
- [x] I have updated the documentation, if necessary.
- [x] I have added tests, if applicable.

Signed-off-by: Igor Klemenski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

1 participant