-
Notifications
You must be signed in to change notification settings - Fork 87
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
[kprobe] add support for kprobes #33
Conversation
Codecov Report
@@ Coverage Diff @@
## master #33 +/- ##
==========================================
- Coverage 20.34% 17.44% -2.90%
==========================================
Files 14 15 +1
Lines 1170 1364 +194
==========================================
Hits 238 238
- Misses 918 1112 +194
Partials 14 14
Continue to review full report at Codecov.
|
pe.handlers = make([]*perfEventHandler, nCpus) | ||
for cpu := 0; cpu < nCpus; cpu++ { | ||
handler, err := newPerfEventHandler(cpu, -1, bufferSize) // All processes | ||
handler, err = newPerfEventHandler(cpu, -1, bufferSize) // All processes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was wrong with :=
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was shadowing the err
variable from outside the loop which then broke the error handling which relies on it checking the value of err
after the loop exits.
ebpf.go
Outdated
@@ -35,6 +35,8 @@ type Program interface { | |||
Detach() error | |||
// Returns program name as it defined in C code | |||
GetName() string | |||
// Returns program target symbol as defined in the section name e.g. kprobe/SyS_execve | |||
GetTarget() string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe GetNameExtras
or similar to make it kinda generic and possibly reusable for future types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed this to be more generically applicable as GetSection
to return the section name used for the program.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting close! :)
Few action items:
- Revert format for
bpf_helpers.h
? - Add at least basic documentation to
doc.go
? - Update readme with super simple example?
- Update readme under
examples
directory?
- added simple example to README.md - added basic info to doc.go - updated examples/README.md - updated BaseProgram usage to be by value - improved kprobe code comments and godoc - increased timeout from 1 to 3 seconds for unit test
Build is succeeding on tip but seems to be running into an issue with testify on older go versions - you had any experience with this before? |
hmm, interesting, let me check it as well |
Now all version passes.. :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job! :)
Co-authored-by: Konstantin Belyalov <[email protected]>
Initial support for
kprobes
withexec_dump
example.