Tcx

[astoycos]

Fixes #918

This initial attempt works with the following example -> https://github.com/astoycos/tcxtest

I'm not at all happy with the API yet so leaving this as a draft for now, once we iron that out I'll also implement integration tests (which would be much easier with the work from #915)

Some open design notes/questions....

• For this first pass I exposed a tightly controlled way of ordering links that can be used anywhere since this API is expected to be expanded to other program types in the future, i.e

/// Defines how to assign a link priorty when using the kernels mprog feature.
pub enum LinkOrdering {
    /// Assign the link as the first link.
    First,
    /// Assign the link as the last link.
    Last,
    /// Assign the link before the given link specified by it's file descriptor.
    BeforeLink(FdLink),
    /// Assign the link after the given link specified by it's file descriptor.
    AfterLink(FdLink),
    /// Assign the link before the given link specified by it's id.
    BeforeLinkId(u32),
    /// Assign the link after the given link specified by it's id.
    AfterLinkId(u32),
    /// Assign the link before the given program specified by it's file descriptor.
    BeforeProgram(Program),
    /// Assign the link after the given program specified by it's file descriptor.
    AfterProgram(Program),
    /// Assign the link before the given program specified by it's id.
    BeforeProgramID(u32),
    /// Assign the link after the given program specified by it's id.
    AfterProgramID(u32),
}

• The tc link is modeled very similar to our XdpLink i.e

#[derive(Debug)]
pub(crate) enum TcLinkInner {
    TcxLink(FdLink),
    NlLink(NlLink),
}

However the way we model links seems to be becoming a bit messy and I'm wondering if we could refactor this to have a simple generic Link type rather then having every program have specific variants 🤔

• To express the desire to load a tc program via TCX rather than via netlink currently the user must use SchedClassifier.attach_with_options() with the new options type

/// Options for SchedClassifer attach via TCX.
#[derive(Debug)]
pub struct TcxOptions {
    /// Ordering defines the tcx link ordering relative to other links on the
    /// same interface.
    pub ordering: LinkOrdering,
    /// Expected revision is used to ensure that tcx link order setting is not
    /// racing with another process.
    pub expected_revision: Option<u64>,
}

However I'm starting to think that deprecating attach_with_options, and removing the TcxOptions, and NlOptions structs would be better. Instead just having attach_tc(ARGS) and attach_netlink(ARGS) would be alot cleaner for the user, the standard attach can maintain the same behavior (default netlink) for now

Basic testing just exercises First/Last

astoycos@nfvsdn-02-oot:~/go/src/github.com/aya-rs/tcxtest$ RUST_LOG=debug cargo xtask run -- --iface eno3
[2024-04-05T15:14:35Z DEBUG aya::bpf] BPF Feature Detection: Features {
        bpf_name: true,
        bpf_probe_read_kernel: true,
        bpf_perf_link: true,
        bpf_global_data: true,
        bpf_cookie: true,
        cpumap_prog_id: true,
        devmap_prog_id: true,
        btf: Some(
            BtfFeatures {
                btf_func: true,
                btf_func_global: true,
                btf_datasec: true,
                btf_float: true,
                btf_decl_tag: true,
                btf_type_tag: true,
                btf_enum64: true,
            },
        ),
    }
[2024-04-05T15:14:35Z DEBUG aya_obj::relocation] relocating map by section index 5, kind Rodata at insn 135 in section 3
[2024-04-05T15:14:35Z DEBUG aya_obj::relocation] relocating map by symbol index Some(14), kind Maps at insn 20 in section 3
[2024-04-05T15:14:35Z DEBUG aya_obj::relocation] relocating map by section index 5, kind Rodata at insn 82 in section 3
[2024-04-05T15:14:35Z DEBUG aya_obj::relocation] relocating map by section index 5, kind Rodata at insn 34 in section 3
[2024-04-05T15:14:35Z DEBUG aya_obj::relocation] relocating map by symbol index Some(15), kind Maps at insn 183 in section 3
[2024-04-05T15:14:35Z DEBUG aya_obj::relocation] relocating map by symbol index Some(14), kind Maps at insn 20 in section 3
[2024-04-05T15:14:35Z DEBUG aya_obj::relocation] relocating map by section index 5, kind Rodata at insn 135 in section 3
[2024-04-05T15:14:35Z DEBUG aya_obj::relocation] relocating map by section index 5, kind Rodata at insn 82 in section 3
[2024-04-05T15:14:35Z DEBUG aya_obj::relocation] relocating map by section index 5, kind Rodata at insn 34 in section 3
[2024-04-05T15:14:35Z DEBUG aya_obj::relocation] relocating map by symbol index Some(15), kind Maps at insn 185 in section 3
[2024-04-05T15:14:35Z DEBUG aya_obj::relocation] relocating program `tcxtestfirst` function `tcxtestfirst` size 194
[2024-04-05T15:14:35Z DEBUG aya_obj::relocation] finished relocating program `tcxtestfirst` function `tcxtestfirst`
[2024-04-05T15:14:35Z DEBUG aya_obj::relocation] relocating program `tcxtestlast` function `tcxtestlast` size 192
[2024-04-05T15:14:35Z DEBUG aya_obj::relocation] finished relocating program `tcxtestlast` function `tcxtestlast`
[2024-04-05T15:14:35Z INFO  tcxtest] Waiting for Ctrl-C...
[2024-04-05T15:14:40Z INFO  tcxtest] received a packet first
[2024-04-05T15:14:40Z INFO  tcxtest] received a packet last
[2024-04-05T15:14:41Z INFO  tcxtest] received a packet first
[2024-04-05T15:14:41Z INFO  tcxtest] received a packet last
[2024-04-05T15:14:42Z INFO  tcxtest] received a packet first
[2024-04-05T15:14:42Z INFO  tcxtest] received a packet last
^C[2024-04-05T15:14:44Z INFO  tcxtest] Exiting...

[astoycos]

cc @dave-tucker For some eyes

[netlify]

✅ Deploy Preview for aya-rs-docs ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	fc9f947
🔍 Latest deploy log	https://app.netlify.com/sites/aya-rs-docs/deploys/66103d68a3e5d90008a22845
😎 Deploy Preview	https://deploy-preview-921--aya-rs-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[mergify]

Hey @alessandrod, this pull request changes the Aya Public API and requires your review.

[alessandrod]

Fixes #918

Thanks so much for doing all this work!

This initial attempt works with the following example -> https://github.com/astoycos/tcxtest

I'm not at all happy with the API yet so leaving this as a draft for now, once we iron that out I'll also implement integration tests (which would be much easier with the work from #915)

Some open design notes/questions....

* For this first pass I exposed a tightly controlled way of ordering links that can be used anywhere since this API is expected to be expanded to other program types in the future,  i.e

/// Defines how to assign a link priorty when using the kernels mprog feature.
pub enum LinkOrdering {
    /// Assign the link as the first link.
    First,
    /// Assign the link as the last link.
    Last,
    /// Assign the link before the given link specified by it's file descriptor.
    BeforeLink(FdLink),
    /// Assign the link after the given link specified by it's file descriptor.
    AfterLink(FdLink),
    /// Assign the link before the given link specified by it's id.
    BeforeLinkId(u32),
    /// Assign the link after the given link specified by it's id.
    AfterLinkId(u32),
    /// Assign the link before the given program specified by it's file descriptor.
    BeforeProgram(Program),
    /// Assign the link after the given program specified by it's file descriptor.
    AfterProgram(Program),
    /// Assign the link before the given program specified by it's id.
    BeforeProgramID(u32),
    /// Assign the link after the given program specified by it's id.
    AfterProgramID(u32),
}

This seems fine, except the current definition isn't type safe:

LinkOrder::AfterProgram(KprobeProgram)

This shouldn't be possible. We should try to make the API as type safe as possible. Among other things we should probably have a wrapper type for program ids, so that LinkOrdering::BeforeProgramId(42) is not possible, but LinkOrdering::BeforeProgramId(unsafe { ProgramId::new(42) }) is.

* The tc link is modeled very similar to our XdpLink i.e

#[derive(Debug)]
pub(crate) enum TcLinkInner {
    TcxLink(FdLink),
    NlLink(NlLink),
}

This also seems fine

However the way we model links seems to be becoming a bit messy and I'm wondering if we could refactor this to have a simple generic Link type rather then having every program have specific variants 🤔

I don't think we can do this without giving up on a lot of type safety. If you have only one Link type, then all operations become possilble on all links: xdp operations on tc links, kprobe operations on xdp links, etc. But if you have something in mind that would work I'd love to see it!

* To express the desire to load a tc program via TCX rather than via netlink currently the user must use `SchedClassifier.attach_with_options()` with the new options type

/// Options for SchedClassifer attach via TCX.
#[derive(Debug)]
pub struct TcxOptions {
    /// Ordering defines the tcx link ordering relative to other links on the
    /// same interface.
    pub ordering: LinkOrdering,
    /// Expected revision is used to ensure that tcx link order setting is not
    /// racing with another process.
    pub expected_revision: Option<u64>,
}

However I'm starting to think that deprecating attach_with_options, and removing the TcxOptions, and NlOptions structs would be better. Instead just having attach_tc(ARGS) and attach_netlink(ARGS) would be alot cleaner for the user, the standard attach can maintain the same behavior (default netlink) for now

As an user of aya, I probably don't know anything about netlink or TCX and I shouldn't really learn anything about it. I think we should make this as transparent as possible to users. Like we do for xdp, we should probably detect whether we can attach as TCX or fallback to netlink, and in the default/common case, have sensible defaults for LinkOrdering and everything else.