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

Set attach type during load for BPF_PROG_TYPE_CGROUP_SKB #263

Merged
merged 2 commits into from May 20, 2022

Conversation

nak3
Copy link
Contributor

@nak3 nak3 commented May 19, 2022

As per title, this patch sets expected_attach_type during load.

Fix #262

@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 5d22869

As per title, this patch sets `expected_attach_type` during load.
@dave-tucker
Copy link
Member

@nak3 Thanks for the PR!

Does the current behaviour cause an issue (other than misleading strace output)?
Since CgroupSkb programs can be attached in either direction, and you have to provide a direction on attach anyway,, I guess this change is just cosmetic?

I'm not against the change at all... I just want to understand the motivation.

@@ -62,6 +62,11 @@ pub struct CgroupSkb {
impl CgroupSkb {
/// Loads the program inside the kernel.
pub fn load(&mut self) -> Result<(), ProgramError> {
self.data.expected_attach_type = match self.expected_attach_type {
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 map()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! updated.

@nak3
Copy link
Contributor Author

nak3 commented May 20, 2022

Hi @dave-tucker

Yes, one very small issue is caused. (Sorry I should explain it in the linked issue.)
As Kernel code allows only Egress to use the return values (2 or 3), currently attach type "Egress" causes the following error:

#[cgroup_skb(name="cgroup_skb_tmp", attach="egress")]
pub fn cgroup_skb_tmp(ctx: SkBuffContext) -> i32 {
    3
}

this (return 3) causes the following error:

Error: the BPF_PROG_LOAD syscall failed. Verifier output: func#0 @0
0: R1=ctx(id=0,off=0,imm=0) R10=fp0
0: (b7) r0 = 3                        ; R0_w=inv3
1: (95) exit
At program exit the register R0 has value (0x3; 0x0) should have been in (0x0; 0x1)
verification time 10 usec
stack depth 0
processed 2 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0


Caused by:
    Invalid argument (os error 22)

After applied this patch, the return value 2 or 3 for Egress is allowed.

@alessandrod alessandrod merged commit 63b6286 into aya-rs:main May 20, 2022
@nak3 nak3 deleted the cgroup-skb-attach-type branch May 20, 2022 03:31
@dave-tucker dave-tucker added fix A PR that is a small change or fixes a bug 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) fix A PR that is a small change or fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BPF_PROG_TYPE_CGROUP_SKB always calls BPF_PROG_LOAD with Ingress attach type
4 participants