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

WIP: auto-attach tracepoints based on SEC() data #1057

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Ablu
Copy link

@Ablu Ablu commented Oct 15, 2024

This is mostly a conversation starter. The work originated in me asking about this on Discord [1].

I can try to flesh this out to a full solution, but I have some questions on what the ideal solution looks like:

Question 1: Currently I am mostly seeing these SEC() annotations in C bpf programs. The automatic template generator of the aya-ebpf tracepoints does not seem to populate the fields (tough it looks like they are supported!). Should we populate those fields there too?
libbpf-tools generates those .skel.h headers that allow automating the attach. I find that a nice QoL improvement over manually having to attach all the probes.

Question 2: I assume we also want to expose public getters for these fields?

Question 3: Do we want to extend this to all of the other "well-known" SEC() annotations that libbpf-tools uses? This would allow us to have a central auto_attach() that simply iterates over all programs and attaches them all?

Open TODOs:

  • tests
  • better documentation
  • review contribution guidelines in detail

[1] https://discord.com/channels/855676609003651072/855676609003651075/1288759816612741120


This change is Reviewable

Copy link

netlify bot commented Oct 15, 2024

Deploy Preview for aya-rs-docs ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 471e822
🔍 Latest deploy log https://app.netlify.com/sites/aya-rs-docs/deploys/670e1910d9be4200081d570b
😎 Deploy Preview https://deploy-preview-1057--aya-rs-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@mergify mergify bot added aya This is about aya (userspace) aya-obj Relating to the aya-obj crate labels Oct 15, 2024
@alessandrod
Copy link
Collaborator

This is mostly a conversation starter. The work originated in me asking about this on Discord [1].

Hey! Thanks for the PR!

I can try to flesh this out to a full solution, but I have some questions on what the ideal solution looks like:

Question 1: Currently I am mostly seeing these SEC() annotations in C bpf programs. The automatic template generator of the aya-ebpf tracepoints does not seem to populate the fields (tough it looks like they are supported!). Should we populate those fields there too? libbpf-tools generates those .skel.h headers that allow automating the attach. I find that a nice QoL improvement over manually having to attach all the probes.

For things where it makes sense - for example tracepoints that in most cases have exactly 1 attach target - yes I think it makes sense.

Question 2: I assume we also want to expose public getters for these fields?

Yes, see CgroupSkb where we expose already https://docs.rs/aya/latest/aya/programs/cgroup_skb/struct.CgroupSkb.html#method.expected_attach_type

Question 3: Do we want to extend this to all of the other "well-known" SEC() annotations that libbpf-tools uses?

This for sure - we want to be as compatible with libbpf as possible.

This would allow us to have a central auto_attach() that simply iterates over all programs and attaches them all?

This is a separate question, and I think it requires a bit more thinking, eg:

let ebpf = Ebpf::load(...);
ebpf.auto_attach()?; // what happens to the programs that _can't_ be auto attached? 

I think it's counter intuitive that some programs wouldn't get attached. Not sure what the solution would be. What does libbpf do?

@Ablu
Copy link
Author

Ablu commented Oct 15, 2024

For things where it makes sense - for example tracepoints that in most cases have exactly 1 attach target - yes I think it makes sense.

I would add k(ret)probe's to the list then :). I can do some research on which programs can be auto-attached in libbpf.

I think it's counter intuitive that some programs wouldn't get attached. Not sure what the solution would be. What does libbpf do?

The code is here: https://github.com/libbpf/libbpf/blob/7984737fbf3b2a14a86321387bb62abb16cfc4ed/src/libbpf.c#L12365-L12404

So it looks like some conventions to prevent autoattachment: https://github.com/libbpf/libbpf/blob/7984737fbf3b2a14a86321387bb62abb16cfc4ed/src/libbpf.c#L747-L757 and some program types are simply not autoattached. Otherwise, a fail to auto-attach will simply cause an error.

That overall seems like reasonable to me!

@alessandrod
Copy link
Collaborator

For things where it makes sense - for example tracepoints that in most cases have exactly 1 attach target - yes I think it makes sense.

I would add k(ret)probe's to the list then :). I can do some research on which programs can be auto-attached in libbpf.

Yep for sure - ideally we get to 100% libbpf compatibility.

I think it's counter intuitive that some programs wouldn't get attached. Not sure what the solution would be. What does libbpf do?

The code is here: https://github.com/libbpf/libbpf/blob/7984737fbf3b2a14a86321387bb62abb16cfc4ed/src/libbpf.c#L12365-L12404

So it looks like some conventions to prevent autoattachment: https://github.com/libbpf/libbpf/blob/7984737fbf3b2a14a86321387bb62abb16cfc4ed/src/libbpf.c#L747-L757 and some program types are simply not autoattached. Otherwise, a fail to auto-attach will simply cause an error.

That overall seems like reasonable to me!

I don't like that in the case of auto_attach() failure some things might remain attached, but I don't have a solution, and I like being "more compatible" with libbpf. If we do implement auto_attach(), we should also add program.links() and program.links_mut() so one can retrieve the attached links and detach.

Copy link

mergify bot commented Nov 24, 2024

@Ablu, this pull request is now in conflict and requires a rebase.

@mergify mergify bot added the needs-rebase label Nov 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aya This is about aya (userspace) aya-obj Relating to the aya-obj crate needs-rebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants