Remove socket filter #394
Remove socket filter #394
Conversation
|
|
||
| bpf_map_update_elem(&active_ssl_handshakes, &id, &s, BPF_ANY); | ||
| // Important: We must work here to remember the iovec pointer, since the msghdr structure |
There was a problem hiding this comment.
Saving the msghdr pointer didn't work with requests that are split in few parts, i.e. large requests > 4K in size. Therefore, here I save now the iovec_ptr before we end up receiving the message, so we can read it properly when the first 4K chunk is processed.
| @@ -0,0 +1,10 @@ | |||
| with open('data.json', 'w') as f: | |||
| f.write('''{ | |||
There was a problem hiding this comment.
I made a small helper program to generate large requests. The earlier version of large_data was manually copy pasted duplicate keys, which prevented me from testing large response sizes, the JSON responder would remove the duplicates :).
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #394 +/- ##
===========================================
- Coverage 81.10% 42.87% -38.23%
===========================================
Files 55 53 -2
Lines 4318 4191 -127
===========================================
- Hits 3502 1797 -1705
- Misses 634 2284 +1650
+ Partials 182 110 -72
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
| @@ -258,7 +251,7 @@ func (p *Tracer) UProbes() map[string]map[string]ebpfcommon.FunctionPrograms { | |||
| } | |||
|
|
|||
| func (p *Tracer) SocketFilters() []*ebpf.Program { | |||
| return []*ebpf.Program{p.bpfObjects.SocketHttpFilter} | |||
| return nil | |||
There was a problem hiding this comment.
I wonder if we can now remove the SocketFilter method from the ebpf.Tracer interface and the logic that invokes it.
|
Please can we merge first this PR to make sure the DaemonSet scenario still keeps working? #399 |
|
Sure! let me resolve the conflicts. |
|
Thanks Mario! it's merged now |
|
Ouch! I meant to merge first the DaemonSet test 😅 before the current PR (my message writing was too ambiguous). I'll rerun the tests to make sure DaemonSet still works |
|
No worries, I can always revert the changes related to pid matching and we know we need something better anyway, which I'm working on next. |
|
No need to, the tests are passing: #399 |
This PR changes how we process kprobe based events to use tcp_sendmsg and tcp_recvmsg instead of the socket filter. I had initially added the probes for tcp_recvmsg/sendmsg to capture the traceparent header field, so it makes sense to actually use these now instead of having duplicate logic with the socket filter.
The PR also has some minor tweaks:
TODO: