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.

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

Ignore trailing kernel module annotation #1413

Merged
merged 5 commits into from
Jul 8, 2020

Conversation

danobi
Copy link
Member

@danobi danobi commented Jul 8, 2020

Before, find_wildcard_matches would choke on lines
available_filter_functions like:

ehci_disable_ASE [ehci_hcd]

This patch has k[ret]probes ignore the module annotation.

This closes #1036 .

Checklist
  • Language changes are updated in docs/reference_guide.md
  • User-visible and non-trivial changes updated in CHANGELOG.md

Does the same thing and less code to maintain.
This helper makes a lot of assumptions about the input and probably
shouldn't be a public API if we can help it.

Also rename the lower level helper b/c it doesn't look like function
overloading works correctly with class member and non-class member.
Before, `find_wildcard_matches` would choke on lines in
available_filter_functions like:

    ehci_disable_ASE [ehci_hcd]

This patch has k[ret]probes ignore the module annotation.

This closes bpftrace#1036 .
@fbs
Copy link
Member

fbs commented Jul 8, 2020

Ah nice you beat me to it. Wonder if it makes sense to define kprobes like kprobe:module:func to be able to attach to "module functions" that overlap in name

@danobi
Copy link
Member Author

danobi commented Jul 8, 2020

Yeah, I think kprobe:module:func would work. And I think it could be done as a followup PR.

Do you think the following rules make sense?

  • kprobe:foo*
    • ignore module name (module implicitly wildcarded)
  • 'kprobe:*:foo*
    • ignore module name (module explicitly wildcarded)
  • kprobe:bar*:foo*
    • don't ignore module name (not full wildcard)
  • non-wildcard cases work as you'd expect -- full text match only

Or is this too complicated? The alternative is to abandon the logic changes in this PR (the cleanup commits I think are still good) and always require module param to be specified if you want to include module kprobes.

@fbs
Copy link
Member

fbs commented Jul 8, 2020

Or is this too complicated?

Sounds good

The alternative is to abandon the logic changes in this PR (the cleanup commits I think are still good) and always require module param to be specified if you want to include module kprobes.

I think we should land this asap, we can always refactor it later. There is quite a lot of duplicate code between the listing and the function finding and kaddr/ksym expansion that we can clean up and simplify.

@danobi danobi merged commit 5975972 into bpftrace:master Jul 8, 2020
@zhangjaycee
Copy link

Yeah, I think kprobe:module:func would work. And I think it could be done as a followup PR.

Do you think the following rules make sense?

  • kprobe:foo*

    • ignore module name (module implicitly wildcarded)
  • 'kprobe:*:foo*

    • ignore module name (module explicitly wildcarded)
  • kprobe:bar*:foo*

    • don't ignore module name (not full wildcard)
  • non-wildcard cases work as you'd expect -- full text match only

Or is this too complicated? The alternative is to abandon the logic changes in this PR (the cleanup commits I think are still good) and always require module param to be specified if you want to include module kprobes.

Hi, I wonder if there has been any progress or plans on this feature? It is exactly what we need recently. Thanks!

@viktormalik
Copy link
Contributor

Hi, there is now support for this syntax for kfunc probes, i.e. kfunc:module:func. Is that sufficient for your needs?

@zhangjaycee
Copy link

zhangjaycee commented May 15, 2023

Hi, there is now support for this syntax for kfunc probes, i.e. kfunc:module:func. Is that sufficient for your needs?

Functionally, kfunc:module:func is good. But I found it requires relatively new kernel version support. So I wonder if anyone plans to implement kprobe:module:func. If it is not in process, I would also appreciate it if anyone could give some suggestions on how to implement it. Thanks!

@viktormalik
Copy link
Contributor

Functionally, kfunc:module:func is good. But I found it requires relatively new kernel version support. So I wonder if anyone plans to implement kprobe:module:func. If it is not in process, I would also appreciate it if anyone could give some suggestions on how to implement it. Thanks!

Yes, it requires kernel 5.5+ with BTF support, if you call that new :-).

Since we have this functionality, we don't plan to duplicate it for kprobes. If you want to do it yourself, you can get inspired by the kfunc implementation for some parts. The main difference will be in the way probes are attached. While kfunc specifies the target module/function upon BPF program loading, kprobes do that during the attachment itself. So, you'll have to check if (and how) BCC's bpf_attach_kprobe supports specifying the module to attach to.

@zhangjaycee
Copy link

Functionally, kfunc:module:func is good. But I found it requires relatively new kernel version support. So I wonder if anyone plans to implement kprobe:module:func. If it is not in process, I would also appreciate it if anyone could give some suggestions on how to implement it. Thanks!

Yes, it requires kernel 5.5+ with BTF support, if you call that new :-).

Since we have this functionality, we don't plan to duplicate it for kprobes. If you want to do it yourself, you can get inspired by the kfunc implementation for some parts. The main difference will be in the way probes are attached. While kfunc specifies the target module/function upon BPF program loading, kprobes do that during the attachment itself. So, you'll have to check if (and how) BCC's bpf_attach_kprobe supports specifying the module to attach to.

Thanks for the information :-). I will give feedback if there is any progress on this in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kprobe pattern matching returns "Invalid argument" for some probes
4 participants