fix: clock time detection #4513
Conversation
pkg/ebpf/tracee.go
Outdated
| // Use monotonic time if the helper isn't supported. | ||
| usedClockID = traceetime.CLOCK_MONOTONIC |
There was a problem hiding this comment.
What if BPFHelperIsSupported returns supported == false and errno == EPERM? Shouldn't we set it as monotonic in this case? As this wrapped helper is kinda "returning wrong errno", what about just logger.Debug the errno, I mean... we're under caps EBPF, so EPERM shouldn't be returned if the libbpf helper were returning a right errno, am I following?
If we can rely on the supported value, we should stick with it and log the errno just for the sake of debugging.
P.S.: the link of the latest libbpfgo commit related to this fix as comments would be a must.
There was a problem hiding this comment.
Yaniv and I discussed this today, we should treat any error as making the supported essentially indeterminable. Therefore we should short circuit and return at that point, I think with a WARN log.
There was a problem hiding this comment.
But supported will only be 1 if really supported. The errno in that case, if set is only garbage.
I guess that the order of checking would always be 1. the real return (libbpf takes care of it correctly) and 2. in the case of error, errno should be complementary information (wrong or not).
There was a problem hiding this comment.
I am assuming that the default return of supported is false. So if there is an error it will be false. And in any case an error should indicate that we don't actually know the support check worked. That is why the case of err!=nil is its own thing, and should stay with BOOT for the heuristic reason mentioned.
There was a problem hiding this comment.
Copying from my comment to geyslan:
Yaniv and I discussed this today, we should treat any error as making the supported essentially indeterminable. Therefore we should short circuit and return at that point, I think with a WARN log. This is because it is more likely these days for a kernel to run with BOOT support. Then if we know there is no error, we can treat the !supported case and move to MONOTONIC.
rscampos
force-pushed
the
fix_clock_time_detection
branch
from
9a14480 to
0e12925
Compare
|
Thank you for the discussion, everyone. @geyslan and I had an offline discussion and revised the logic (in libbpfgo and Tracee) after identifying some issues with libbpf. I'll provide a detailed explanation of the issue, but the following is the logic explanation change. Since this feature probe in Tracee runs with elevated capabilities, it can rely on the |
rscampos
force-pushed
the
fix_clock_time_detection
branch
7 times, most recently
from
5f66fec to
277ef87
Compare
- The clock time detection was rework since the libbpfgo func BPFHelperIsSupported changed; - Unless it is explicitly marked as unsupported (supported=false), it will default to BOOTTIME.
rscampos
force-pushed
the
fix_clock_time_detection
branch
from
277ef87 to
23d8ea1
Compare
This comment was marked as resolved.
This comment was marked as resolved.
1. Explain what the PR does
23d8ea1 chore(go.mod): bump libbpfgo
16d1e19 fix: clock time detection
16d1e19 fix: clock time detection
2. Explain how to test it
3. Other comments
fix: #4505
Most of the change is located in libbpfgo, but I've inverted the order of the checking since most of the time we support boottime.
What changed and why in the libbpfgo: https://github.com/aquasecurity/libbpfgo/blob/45a155ff1a362156b5c7d1b75bc3db5f8cb5e526/libbpfgo.go#L104-L116