Open tohojo opened 3 years ago
I didn't know libxdp did this - seems very clever. I think it should be feasible and I would love to see it implemented!
Alessandro Decina @.***> writes:
I didn't know libxdp did this - seems very clever. I think it should be feasible and I would love to see it implemented!
Awesome! I'm a bit of a rust newbie myself, still, so not sure if I can contribute code myself, but I can certainly review if you take a crack at it! :)
To get started, I took a first pass at writing the dispatcher with Aya:
#![no_std]
#![no_main]
use aya_bpf::{
bindings::xdp_action,
macros::xdp,
programs::XdpContext,
};
const XDP_METADATA_SECTION: &str = "xdp_metadata";
const XDP_DISPATCHER_VERSION: u32 = 1;
const XDP_DISPATCHER_RETVAL: u32 = 31;
const MAX_DISPATCHER_ACTIONS: usize = 10;
#[repr(C)]
pub struct XdpDispatcherConfig {
num_progs_enabled: u8,
chain_call_actions: [u32;MAX_DISPATCHER_ACTIONS],
run_prios: [u32;MAX_DISPATCHER_ACTIONS],
}
// this was static volatile const in C
static CONFIG: XdpDispatcherConfig = XdpDispatcherConfig{
num_progs_enabled: 0,
chain_call_actions: [0; MAX_DISPATCHER_ACTIONS],
run_prios: [0; MAX_DISPATCHER_ACTIONS],
};
#[no_mangle]
pub fn prog0(_ctx: XdpContext) -> u32 {
let ret = XDP_DISPATCHER_RETVAL;
// TODO: Check XdpContext is valid, if not abort
return ret;
}
#[xdp]
fn dispatcher(ctx: XdpContext) -> u32 {
let num_progs_enabled = CONFIG.num_progs_enabled;
if num_progs_enabled < 1 {
return xdp_action::XDP_PASS;
}
let ret = prog0(ctx);
if (1 << ret) & CONFIG.chain_call_actions[0] == 0 {
return ret
};
return xdp_action::XDP_PASS;
}
#[panic_handler]
fn panic(_info: &core::panic::PanicInfo) -> ! {
unreachable!()
}
There are a couple of things that I think we're going to need in order for this to land:
I'm a bit stumped on the Rust alternative to a static volatile const
in C. I'm guessing static
is correct as const
will be inlined, but not sure if we should access using core::ptr::read_volatile
. As for the const
portion, it seems necessary for it to be placed in the .ro-data
section (see: BPF Support For Global Data. As we don't want to put all statics in .ro-data
I'll assume we might need a #[global]
annotation so bpf-linker could add this to .ro-data
?
We'll probably need to implement #37 as the xdp_metadata is encoded in a similar way
We may want to consider adding bindings for the xdp_helpers
I think we'll also need #45
Also it might be nice to use macros or templating to produce the dispatcher code since there is a lot of repetition.
We'll also want to figure out we'd ship a precompiled version of the dispatcher alongside Aya :thinking:
To get started, I took a first pass at writing the dispatcher with Aya:
This looks reasonable at a first pass. Needs more slots, but as you say, that's a good candidate for some kind of templating :)
Also, note the compat_test()
function in the libxdp version of the dispatcher. We use that as an "extra slot" that we can attach a program to when we do feature detection in the library. This is needed because the freplace support is fairly new in the kernel, and we want to be able to detect if it's present (and fall back to single-program loading if it's not). Depending on your kernel feature support goals for Aya this may or may not be necessary I suppose.
There are a couple of things that I think we're going to need in order for this to land:
1. I'm a bit stumped on the Rust alternative to a `static volatile const` in C. I'm guessing `static` is correct as `const` will be inlined, but not sure if we should access using [`core::ptr::read_volatile`](https://doc.rust-lang.org/core/ptr/fn.read_volatile.html). As for the `const` portion, it seems necessary for it to be placed in the `.ro-data` section (see: [BPF Support For Global Data](https://lwn.net/Articles/784936/). As we don't want to put all statics in `.ro-data` I'll assume we might need a `#[global]` annotation so bpf-linker could add this to `.ro-data`?
So the static const
bit is to have the struct put into the .ro_data section, which is needed for the verifier to do dead code removal on load (so that we can define the dispatcher statically with 10 slots, but the unused slots will be removed from the program at load time).
The volatile
is there to prevent the compiler from optimising away all the of the values (since it thinks it can prove what the results of each comparison will be). So if the rust compiler doesn't do this, that won't be necessary, of course.
2. We'll probably need to implement [Include BTF Map information in the ELF file #37](https://github.com/aya-rs/aya/issues/37) as the xdp_metadata is encoded in a similar way
Yup. Turns out BTF can actually be used as a general-purpose key-value store :)
3. We may want to consider adding bindings for the `xdp_helpers`
That would be useful, I think.
4. I think we'll also need [Add support for map and program pinning #45](https://github.com/aya-rs/aya/issues/45)
Yeah, the pinning of maps and programs is how libxdp synchronises across processes, and makes sure that programs stay attached, so that bit is definitely needed.
Also it might be nice to use macros or templating to produce the dispatcher code since there is a lot of repetition.
Agreed - when building the dispatcher in libxdp we use M4 to template it, but I'd expect that Rust can do better here :)
We'll also want to figure out we'd ship a precompiled version of the dispatcher alongside Aya
Being able to use a precompiled dispatcher was an explicit design goal (which the dead code elimination trick helps with). We ship it as /usr/lib/bpf/xdp-dispatcher.o when we package libxdp. I suppose Aya could even re-use that if it's present (but not sure if that's a good idea).
There are a couple of things that I think we're going to need in order for this to land:
1. I'm a bit stumped on the Rust alternative to a `static volatile const` in C. I'm guessing `static` is correct as `const` will be inlined, but not sure if we should access using [`core::ptr::read_volatile`](https://doc.rust-lang.org/core/ptr/fn.read_volatile.html). As for the `const` portion, it seems necessary for it to be placed in the `.ro-data` section (see: [BPF Support For Global Data](https://lwn.net/Articles/784936/). As we don't want to put all statics in `.ro-data` I'll assume we might need a `#[global]` annotation so bpf-linker could add this to `.ro-data`?
I think #[no_mangle] static
+ read_volatile()
for access is enough. Aya already relocates .rodata
correctly using a map (a byte array), although we'll probably have to add some API to make it easy to patch the array with the dispatcher config. I think you can already do that now but you have to know the offset of the config in the section.
- We'll probably need to implement Include BTF Map information in the ELF file #37 as the xdp_metadata is encoded in a similar way
Ah the BTF hack! Yeah we need to implement this anyway.
- We may want to consider adding bindings for the
xdp_helpers
TIL. And yes!
4. I think we'll also need [Add support for map and program pinning #45](https://github.com/aya-rs/aya/issues/45)
I've been dragging my feet on this but it's probably time we implement it, yes.
I think #[no_mangle] static + read_volatile() for access is enough. Aya already relocates .rodata correctly using a map (a byte array), although we'll probably have to add some API to make it easy to patch the array with the dispatcher config. I think you can already do that now but you have to know the offset of the config in the section
My code (excuse the unsafe-ty):
#[no_mangle]
static CONFIG: XdpDispatcherConfig = XdpDispatcherConfig{
num_progs_enabled: 0,
chain_call_actions: [0; MAX_DISPATCHER_ACTIONS],
run_prios: [0; MAX_DISPATCHER_ACTIONS],
};
#[xdp]
fn dispatcher(ctx: XdpContext) -> u32 {
let cfg = &CONFIG as *const XdpDispatcherConfig;
let current_cfg = unsafe { core::ptr::read_volatile(&cfg) };
let num_progs_enabled = unsafe { (*current_cfg).num_progs_enabled };
if num_progs_enabled < 1 {
return xdp_action::XDP_PASS;
}
let ret = prog0(ctx);
if (1 << ret) & unsafe { (*current_cfg).chain_call_actions[0] } == 0 {
return ret
};
return xdp_action::XDP_PASS;
}
and I do indeed see something in .rodata
:tada: (which was not the case before)
Contents of section .rodata:
0000 00000000 00000000 00000000 00000000 ................
0010 00000000 00000000 00000000 00000000 ................
0020 00000000 00000000 00000000 00000000 ................
0030 00000000 00000000 00000000 00000000 ................
0040 00000000 00000000 00000000 00000000 ................
0050 00000000 ....
Is this still being developed / does support exist for it now?
It would be useful if Aya could support loading multiple XDP programs on an interface. We have a solution for that in libxdp, which defines a protocol for creating and managing a dispatcher program that calls the component programs.
That protocol is described here: https://github.com/xdp-project/xdp-tools/blob/master/lib/libxdp/protocol.org
Since Aya is doing its own loading, I guess it would make most sense to re-implement the protocol in Rust rather than linking to libxdp. Do you think that would be feasible?