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

Attaching a program to a cgroup via bpf_link_create throws if the caller passes in flags #1078

Open
cweld510 opened this issue Nov 5, 2024 · 2 comments

Comments

@cweld510
Copy link

cweld510 commented Nov 5, 2024

I ran into this problem specifically when trying to load and attach a cgroup_device program, with code that looks like the following:

    let mut bpf =
        EbpfLoader::new().load(aya::include_bytes_aligned!("../bpf/prog.o"))?;

    let cgroup_device: &mut CgroupDevice = bpf
        .program_mut("prog")
        .ok_or(anyhow!(
            "could not obtain reference to prog"
        ))?
        .try_into()?;
    cgroup_device.load()?;
    let cgroup_fd = File::open(cgroup_path)?;
    let link_id = cgroup_device.attach(cgroup_fd, aya::programs::CgroupAttachMode::Single)?;
   

On newer kernels that support bpf_link_create, the call to bpf_link_create will return EINVAL if you pass in any mode other than CgroupAttachMode::Single, because bpf_link_create when used on a cgroup doesn't seem to support flags (see this kernel code). In practice, the kernel defaults to BPF_F_ALLOW_MULTI.

Note that I'm using cgroupsv2.

The fix for this (which I'm happy to make!) seems to be that the link mode should not be passed to bpf_link_create for the cgroup methods, since it's not a valid argument. Does this sound reasonable?

@dave-tucker
Copy link
Member

🤦 wow. nice find.

Assuming you're replacing mode.into() with 0 the fix does sound reasonable.
I would add a warn! log above this though if the flags where anything other than CgroupAttachMode::default() since they won't be passed on to attach - which may, or may not be what the user expected.
It would be good to add a warning about this into the docstring for the attach functions affected also.

we'll keep the API as described in #985 since passing these flags is required to get these features on kernels still using perf_event_open.

@alessandrod
Copy link
Collaborator

It doesn't seem nice that someone passes Single and we just set it to Multi - what if they really want Single?

Wouldn't it be better to fallback to not using bpf_links if someone sets Single?

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

No branches or pull requests

3 participants