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

Introduce bpf tail call to increase the size of grpc parsing loop #978

Merged
merged 7 commits into from
Jul 2, 2024

Conversation

grcevski
Copy link
Contributor

Until we recently added the looped reading of the iovec, we didn't need to split the krpobes tcp_sendmsg and tcp_recvmsg into separate bpf programs. However now we hit instruction limits on certain kernels and this PR splits the protocol parsing into separate programs.

I used this opportunity to break up http_sock.h into specific protocol files.

@grcevski grcevski requested review from mariomac and marctc as code owners June 27, 2024 18:16
}

u32 remaining = IO_VEC_MAX_LEN > tot_len ? (IO_VEC_MAX_LEN - tot_len) : 0;
u32 iov_size = vec.iov_len < l ? vec.iov_len : l;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made one more fix here in the read loop. Sometimes the max_len size is small, e.g. 25 bytes, but the vec.iov_len is large, e.g. 65535, so the loop would terminate on line 200, not reading a thing.

I noticed this on older kernels for receive messages.

@grcevski
Copy link
Contributor Author

Looks like a large PR, but most of the code changes are just moved code into separate files. The main change is the storing of the arguments of the handle_buf_with_connection function call to map memory and then instead of the inlined protocol parsing code, we now use bpf_tail_call to jump into separate BPF programs.

@codecov-commenter
Copy link

codecov-commenter commented Jun 27, 2024

Codecov Report

Attention: Patch coverage is 86.36364% with 6 lines in your changes missing coverage. Please review.

Project coverage is 80.59%. Comparing base (fd90b82) to head (090e9b3).

Files Patch % Lines
pkg/internal/ebpf/httpfltr/httpfltr.go 88.88% 1 Missing and 1 partial ⚠️
pkg/internal/ebpf/httpssl/httpssl.go 88.88% 1 Missing and 1 partial ⚠️
pkg/internal/ebpf/logger/logger.go 0.00% 1 Missing ⚠️
pkg/internal/ebpf/watcher/watcher.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #978      +/-   ##
==========================================
+ Coverage   75.58%   80.59%   +5.00%     
==========================================
  Files         134      134              
  Lines       10601    10645      +44     
==========================================
+ Hits         8013     8579     +566     
+ Misses       2093     1561     -532     
- Partials      495      505      +10     
Flag Coverage Δ
integration-test 55.88% <81.81%> (+0.10%) ⬆️
k8s-integration-test 59.11% <81.81%> (?)
oats-test 36.31% <84.09%> (+0.19%) ⬆️
unittests 50.40% <0.00%> (-0.20%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@marctc marctc merged commit bdd38fa into grafana:main Jul 2, 2024
6 checks passed
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.

4 participants