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

aya: Add from_pin for Programs #496

Merged
merged 1 commit into from
Feb 9, 2023

Conversation

dave-tucker
Copy link
Member

This commit adds from_pin() which allows the creation of a Program from a path on bpffs. This is useful to be able to call attach or other APIs for programs that are already loaded to the kernel.

This differs from #444 since it implements this on the concrete program type, not the Program enum, allowing the user to pass in any additional context that isn't available from bpf_prog_info.

Signed-off-by: Dave Tucker dave@dtucker.co.uk

@netlify
Copy link

netlify bot commented Jan 19, 2023

Deploy Preview for aya-rs-docs ready!

Name Link
🔨 Latest commit 7a720ab
🔍 Latest deploy log https://app.netlify.com/sites/aya-rs-docs/deploys/63e43943f8e9590008bb0ea3
😎 Deploy Preview https://deploy-preview-496--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 settings.

Copy link
Collaborator

@alessandrod alessandrod left a comment

Choose a reason for hiding this comment

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

This seems largely ok! See comments

aya/src/programs/cgroup_skb.rs Outdated Show resolved Hide resolved
aya/src/programs/cgroup_sock.rs Outdated Show resolved Hide resolved
aya/src/programs/mod.rs Outdated Show resolved Hide resolved
aya/src/programs/lirc_mode2.rs Outdated Show resolved Hide resolved
aya/src/programs/perf_event.rs Outdated Show resolved Hide resolved
aya/src/programs/socket_filter.rs Outdated Show resolved Hide resolved
@dave-tucker dave-tucker force-pushed the program-from-pinned3 branch 2 times, most recently from 3027156 to 6289b77 Compare February 8, 2023 19:14
@dave-tucker
Copy link
Member Author

@alessandrod changes addressed.

  • Path was moved to ProgramData and program_data_from_path was made a function instead of a macro
  • from_pin was added using a macro impl_from_pin for simple cases and manually otherwise

@dave-tucker dave-tucker force-pushed the program-from-pinned3 branch 2 times, most recently from 45923f6 to 75e6444 Compare February 8, 2023 22:56
@dave-tucker
Copy link
Member Author

Updated to fix CI...
I had to move the Drop trait from the Program enum to the concrete program type (i.e XDP) for this to behave as expected. For example:

{
    let prog = Xdp::from_pin("/path/to/prog").unwrap();
    prog.attach("eth0");
}
// Link should get detached here and program fd should be closed

When I moved Drop, clippy was kind enough to point out that we have both Program::unload and Xdp::unload - the latter was pub where the former was pub (crate) and only called via the drop implementation. I went ahead and made Program::unload public since to me it makes sense to have it there as a convenience.

@alessandrod
Copy link
Collaborator

Updated to fix CI... I had to move the Drop trait from the Program enum to the concrete program type (i.e XDP) for this to behave as expected. For example:

{
    let prog = Xdp::from_pin("/path/to/prog").unwrap();
    prog.attach("eth0");
}
// Link should get detached here and program fd should be closed

When I moved Drop, clippy was kind enough to point out that we have both Program::unload and Xdp::unload - the latter was pub where the former was pub (crate) and only called via the drop implementation. I went ahead and made Program::unload public since to me it makes sense to have it there as a convenience.

This now will not drop programs unless you access them through concrete program types tho right? Eg if you Bpf::load and never create a concrete program

aya/src/programs/mod.rs Outdated Show resolved Hide resolved
pub(crate) fn from_pinned_path<P: AsRef<Path>>(
path: P,
) -> Result<ProgramData<T>, ProgramError> {
let path_string = std::ffi::CString::new(path.as_ref().to_str().unwrap()).unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should use to_string_lossy here

Copy link
Collaborator

Choose a reason for hiding this comment

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

also import types, don't use full qualified paths

Copy link
Member Author

Choose a reason for hiding this comment

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

Ugh, so Path -> Cstring isn't exactly easy, but this is what I settled on.

CString::new(path.as_ref().as_os_str().to_string_lossy().as_bytes())

aya/src/programs/mod.rs Outdated Show resolved Hide resolved
aya/src/programs/mod.rs Outdated Show resolved Hide resolved
aya/src/programs/mod.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@alessandrod alessandrod left a comment

Choose a reason for hiding this comment

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

Almost there!

This commit adds from_pin() which allows the creation of a Program
from a path on bpffs. This is useful to be able to call `attach` or
other APIs for programs that are already loaded to the kernel.

This differs from aya-rs#444 since it implements this on the concrete program
type, not the Program enum, allowing the user to pass in any additional
context that isn't available from bpf_prog_info.

Signed-off-by: Dave Tucker <dave@dtucker.co.uk>
Copy link
Collaborator

@alessandrod alessandrod left a comment

Choose a reason for hiding this comment

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

LGTM! Feel free to merge on green

@dave-tucker dave-tucker merged commit 811ab29 into aya-rs:main Feb 9, 2023
@dave-tucker dave-tucker deleted the program-from-pinned3 branch February 9, 2023 00:14
@astoycos astoycos mentioned this pull request Feb 10, 2023
@dave-tucker dave-tucker added feature A PR that implements a new feature or enhancement aya This is about aya (userspace) labels Feb 23, 2023
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) feature A PR that implements a new feature or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants