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.

Sign up for GitHub

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

Jump to bottom

Add support to retrieve missed counts from probes #1295

Merged
merged 3 commits into from
Apr 9, 2024
Merged

Add support to retrieve missed counts from probes #1295

merged 3 commits into from
Apr 9, 2024

Conversation

olsajiri
Copy link
Contributor

@olsajiri olsajiri commented Jan 5, 2024 •
edited
Loading

adding support to get info data from kprobe_multi/perf_events probes, using it in tetragon 1941

@olsajiri olsajiri force-pushed the missed_counts branch 4 times, most recently from 118316b to ecb4d92 Compare February 2, 2024 11:08
@olsajiri olsajiri marked this pull request as ready for review February 2, 2024 11:26
@olsajiri olsajiri requested a review from a team as a code owner February 2, 2024 11:26
@olsajiri olsajiri mentioned this pull request Feb 2, 2024
Merged
@olsajiri olsajiri marked this pull request as draft February 10, 2024 23:36
@olsajiri olsajiri force-pushed the missed_counts branch 2 times, most recently from 447e99d to e84d2f0 Compare February 12, 2024 01:02
@olsajiri olsajiri marked this pull request as ready for review February 12, 2024 11:22
rgo3
rgo3 previously requested changes Feb 16, 2024
View reviewed changes
Copy link
Contributor

@rgo3 rgo3 left a comment

Choose a reason for hiding this comment

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

I've reviewed this PR with Lorenz yesterday and we concluded that we might need to figure out a better way of dealing with the bpf_link_info UAPI in ebpf-go before your proposed changes can be implemented. I've pointed out two specific problems in the PR where we hope a better UAPI representation will enable better solutions to retrieve and write metadata on LinkInfo objects. Lorenz and I will spend an hour today to see what we can come up with.

internal/cmd/gentypes/main.go Outdated Show resolved Hide resolved
link/kprobe_multi.go Outdated Show resolved Hide resolved
@rgo3
Copy link
Contributor

rgo3 commented Feb 16, 2024

I will spend some more time next week to work on the ideas Lorenz had to change how ebpf-go represents the bpf_link_info struct in Go.

@olsajiri
Copy link
Contributor Author

I will spend some more time next week to work on the ideas Lorenz had to change how ebpf-go represents the bpf_link_info struct in Go.

sounds great, please let me know if I can help in any way

@rgo3 rgo3 mentioned this pull request Feb 23, 2024
Merged
@olsajiri olsajiri requested a review from mmat11 as a code owner March 28, 2024 12:32
@olsajiri olsajiri force-pushed the missed_counts branch 5 times, most recently from 850d561 to 53e95f9 Compare March 28, 2024 14:37
@olsajiri
Copy link
Contributor Author

@lmb @rgo3 hey guys, could you plz check on the new changes?
I rebased that on the latest and it works now with tetragon cilium/tetragon#1941 thanks

@lmb lmb force-pushed the missed_counts branch from 53e95f9 to 5cda353 Compare April 2, 2024 14:06
lmb
lmb requested changes Apr 2, 2024
View reviewed changes
Copy link
Collaborator

@lmb lmb left a comment

Choose a reason for hiding this comment

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

I pushed some fixups in order to get this merged quickly. Main changes:

  • We can't just return ErrNotSupported. Instead I added getters on KprobeInfo which allow the caller to detect whether we could get the info from the kernel. This is how MapInfo and ProgramInfo work already.
  • Add sys.PerfEventLinkInfo and clean up usage. Yes, another syscall. I've outlined the way to fix this in Robin's last PR. The current approach is cheating and will have to change as soon as the next perf event info thingie lands.

Please take a look, and let me know whether you're OK with dropping exporting the new constants from package link.

internal/sys/types.go Outdated Show resolved Hide resolved
internal/sys/types.go Outdated Show resolved Hide resolved
link/link.go Outdated Show resolved Hide resolved
link/link.go Outdated Show resolved Hide resolved
link/link.go Outdated Show resolved Hide resolved
link/syscalls.go Outdated Show resolved Hide resolved
@lmb lmb force-pushed the missed_counts branch from 5cda353 to e00dbe0 Compare April 2, 2024 14:13
@lmb lmb dismissed rgo3’s stale review April 4, 2024 09:42

Superseded by my review

lmb
lmb reviewed Apr 4, 2024
View reviewed changes
link/link.go Outdated Show resolved Hide resolved
@olsajiri olsajiri force-pushed the missed_counts branch 3 times, most recently from de6a6bc to c677521 Compare April 5, 2024 07:45
@lmb
Copy link
Collaborator

lmb commented Apr 5, 2024

@olsajiri good to go?

@olsajiri
Copy link
Contributor Author

olsajiri commented Apr 5, 2024

@lmb yes, I have tetragon changes on top of this and it looks good, so looks good to me

olsajiri added 3 commits April 9, 2024 11:17
@olsajiri @lmb
sys: Generate kprobe link info structures
5500140 
Generate needs link info structures and related PerfEventType
enum to retrieve kprobe info data from both perf event kprobe
and kprobe multi.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
@olsajiri @lmb
link: Add Info interface support for kprobe multi link
f1d4828 
Implementing Info function (Link interface) for kprobe multi
link, it's now possibe to retrieve link's info data with:

  kmulti := info.KprobeMulti()
  addresses, ok := kmulti.Addresses()
  flags, ok = kmulti.Flags()
  missed, ok = kmulti.Missed()

At the moment we support/return only Count/Flags/Missed data
returned by Addresses/Flags/Missed KprobeMultiInfo methods
respectively.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
@olsajiri @lmb
link: Add Info interface support for perf event kprobe link
d698b6b 
Implementing Info function (Link interface) for perf event
kprobe link, it's now possibe to retrieve link's data with:

  pevent := info.PerfEvent()
  switch pevent.Type {
  case unix.BPF_PERF_EVENT_KPROBE, unix.BPF_PERF_EVENT_KRETPROBE:
          kp := pevent.Kprobe()
          addr, ok := kp.Address()
          addr, ok := kp.Flags()
          missed, ok := kp.Missed()
  }

At the moment we support/return only Addr/Flags/Missed data
returned by Address/Flags/Missed KprobeMultiInfo methods
respectively.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
@lmb lmb force-pushed the missed_counts branch from c677521 to d698b6b Compare April 9, 2024 09:17
lmb
lmb approved these changes Apr 9, 2024
View reviewed changes
Copy link
Collaborator

@lmb lmb left a comment

Choose a reason for hiding this comment

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

Renamed MultiInfo.Addresses to MultiInfo.AddressCount.

@lmb lmb merged commit e678d6e into cilium:main Apr 9, 2024
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lmb lmb lmb approved these changes

@rgo3 rgo3 rgo3 left review comments

@mmat11 mmat11 Awaiting requested review from mmat11 mmat11 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@olsajiri @rgo3 @lmb