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

Add Support for BPF_PROG_TYPE_CGROUP_SOCK_ADDR #261

Merged
merged 2 commits into from May 20, 2022

Conversation

dave-tucker
Copy link
Member

@netlify
Copy link

netlify bot commented May 19, 2022

👷 Deploy request for aya-rs pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 2bac924

@@ -257,6 +280,7 @@ impl CgroupSkb {
pub fn from_syn(mut args: Args, item: ItemFn) -> Result<CgroupSkb> {
let name = pop_arg(&mut args, "name");
let expected_attach_type = pop_arg(&mut args, "attach");
err_on_unknown_args(&args)?;
Copy link
Member Author

Choose a reason for hiding this comment

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

not related to this PR, but I noticed we didn't try and consume any remaining args in a few macros so figured I'd add it while I was here.

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.

Great stuff! See comments.

Also meta comment: I think this should have Cgroup and cgroup_ prefixes like the other CGROUP_ programs?

aya/src/obj/mod.rs Show resolved Hide resolved
aya/src/programs/sock_addr.rs Outdated Show resolved Hide resolved
"sendmsg6" => Ok(SockAddrAttachType::UDPSendMsg6),
"recvmsg4" => Ok(SockAddrAttachType::UDPRecvMsg4),
"recvmsg6" => Ok(SockAddrAttachType::UDPRecvMsg6),
_ => Err(ProgramError::InvalidAttachType {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should use a custom pub(crate) struct InvalidAttachType(String). No need to add a public ProgramError::InvalidAttachType that is used only internally.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've moved it to ParseError, which isn't "exactly" what you asked 😆 but is the right place for this error IMHO.
clippy complained that the pub(crate) struct InvalidAttachType would be leaked so wouldn't allow your suggestion.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd make it pub(crate) try_from at the impl CgroupSockAddrAttachType and return a pub(crate) error (without actually implementing TryFrom for CgroupSockAddrAttachType). We are now exposing a public variant for an internally used error, no bueno.

aya/src/programs/mod.rs Outdated Show resolved Hide resolved
aya/src/programs/sock_addr.rs Outdated Show resolved Hide resolved
aya/src/programs/sock_addr.rs Outdated Show resolved Hide resolved
@dave-tucker
Copy link
Member Author

@alessandrod I've addressed most of your comments if you'd like to take another look.

Signed-off-by: Dave Tucker <dave@dtucker.co.uk>
Signed-off-by: Dave Tucker <dave@dtucker.co.uk>
@alessandrod alessandrod merged commit 8fd8816 into aya-rs:main May 20, 2022
@dave-tucker dave-tucker added feature A PR that implements a new feature or enhancement aya-bpf This is about aya-bpf (kernel) aya This is about aya (userspace) labels May 24, 2022
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-bpf This is about aya-bpf (kernel) feature A PR that implements a new feature or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for BPF_PROG_TYPE_CGROUP_SOCK_ADDR
2 participants