aya-rs / aya

Aya is an eBPF library for the Rust programming language, built with a focus on developer experience and operability.
https://aya-rs.dev/book/
Apache License 2.0
3.25k stars 290 forks source link

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

Open cweld510 opened 3 weeks ago

cweld510 commented 3 weeks ago

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 commented 2 weeks ago

🤦 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 https://github.com/aya-rs/aya/pull/985 since passing these flags is required to get these features on kernels still using perf_event_open.

alessandrod commented 2 weeks ago

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?