-
Notifications
You must be signed in to change notification settings - Fork 712
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
Fallback to proc when ebpf timestamps are wrong #2336
Fallback to proc when ebpf timestamps are wrong #2336
Conversation
probe/endpoint/ebpf.go
Outdated
@@ -99,7 +100,11 @@ var lastTimestampV4 uint64 | |||
|
|||
func tcpEventCbV4(e tracer.TcpV4) { | |||
if lastTimestampV4 > e.Timestamp { | |||
log.Errorf("ERROR: late event!\n") | |||
// A kernel bug can cause the timestamps to be wrong, i.e. Ubuntu with Linux 4.4.0-47.68 | |||
// See https://github.com/iovisor/bcc/issues/790#issuecomment-263704235 |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
cba42e4
to
44c7c11
Compare
Works in a manual test like schu@6febb7d (https://circleci.com/gh/schu/scope/308) For an integration test we would have to prepare and use a separate node with an older kernel w/o the fix. Not worth the effort IMHO. |
probe/endpoint/connection_tracker.go
Outdated
t.ebpfTracker = nil | ||
} else { | ||
t.performEbpfTrack(rpt, hostNodeID) | ||
return |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
probe/endpoint/ebpf.go
Outdated
@@ -99,7 +100,10 @@ var lastTimestampV4 uint64 | |||
|
|||
func tcpEventCbV4(e tracer.TcpV4) { | |||
if lastTimestampV4 > e.Timestamp { | |||
log.Errorf("ERROR: late event!\n") | |||
// See https://github.com/weaveworks/scope/issues/2334 | |||
log.Debugf("tcp tracer received event with timestamp %v even though the last timestamp was %v. Stopping the eBPF tracker.", e.Timestamp, lastTimestampV4) |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
You could mock the failure and add a unit-test EDIT: only if it's not too much work |
.. and avoid nil pointer dereference. It can happen that `getWalkedProcPid` is called before the first `performWalk` finished.
44c7c11
to
5262e07
Compare
Updated.
Mocking the ebpf part would require too much work in my opinion (as we would have to mock most of it). I have added a simple test to see if we set the tracker to dead after receiving events out of order. |
This didn't work as expected. This is what I got when testing it in WeaveCloud's dev environment (the ebpf error continues in a loop).
|
cc @alban