aya: fix tc name limit

[pooladkhay]

The buffer for attributes should be sized to hold at least 256 bytes, based on CLS_BPF_NAME_LEN = 256 from the kernel: https://github.com/torvalds/linux/blob/02aee814/net/sched/cls_bpf.c#L28

This pull request also includes integration tests with an eBPF program of type classifier with a 256-byte-long name.

Fixes: #610

[netlify[bot]]

✅ Deploy Preview for aya-rs-docs ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	9606690a733b67de2fd57beb399fa0fc965918b4
🔍 Latest deploy log	https://app.netlify.com/sites/aya-rs-docs/deploys/656a23481a434f00080c2ef5
😎 Deploy Preview	https://deploy-preview-728--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.

[dave-tucker]

I'm happy with the change to up the buffer to the next nearest power of 2. Tamir's comments need addressing before merge.

[mergify[bot]]

@pooladkhay, this pull request is now in conflict and requires a rebase.

[alessandrod]

@pooladkhay ping?

[pooladkhay]

@pooladkhay ping?

@alessandrod pong! 😃

Life hasn't been easy lately, I should have notified earlier. I'll get back to this asap, please let it be open...

[alessandrod]

@pooladkhay ping?

@alessandrod pong! 😃

Life hasn't been easy lately, I should have notified earlier. I'll get back to this asap, please let it be open...

hugs ❤️ and sorry if I made you feel rushed it wasn't my intention

[pooladkhay]

@pooladkhay ping?

@alessandrod pong! 😃 Life hasn't been easy lately, I should have notified earlier. I'll get back to this asap, please let it be open...

hugs ❤️ and sorry if I made you feel rushed it wasn't my intention

No worries at all 🙋‍♂️

[mergify[bot]]

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

[mergify[bot]]

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

[pooladkhay]

Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @alessandrod, @dave-tucker, and @pooladkhay)

test/integration-test/src/tests/load.rs line 19 at r2 (raw file):

#[test]
fn tc_name_limit() {

Looks like these tests are failing:

failures:

---- tests::load::tc_name_limit stdout ----
thread 'tests::load::tc_name_limit' panicked at test/integration-test/src/tests/load.rs:31:32:
called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- tests::load::tc_name_limit_exceeded stdout ----
thread 'tests::load::tc_name_limit_exceeded' panicked at test/integration-test/src/tests/load.rs:60:32:
called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }

It's the qdisc_add_clsact("lo").unwrap(); calls that seem to be failing.

That's true, Two commands are being used to run the tests: cargo xtask integration-test local which succeeds, and cargo xtask integration-test vm <KERNEL_IMAGE> which is the faulty one. Running it on my local machine fails too.

Is it possible that the loop back interface which I'm trying to attach an eBPF program to, is not available on that image? (hence the Os { code: 2, kind: NotFound, message: "No such file or directory" } error)

[pooladkhay]

Reviewed 7 of 7 files at r6, all commit messages. Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @alessandrod, @dave-tucker, and @pooladkhay)

test/integration-test/src/tests/load.rs line 19 at r2 (raw file): Previously, pooladkhay (Mohammad Javad Pooladkhay) wrote…

Yep, seems likely that it is not available. Can we create a network namespace so that we aren't reliant on ambient resources?

Could you please give me more hints, and maybe an example?

[pooladkhay]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @alessandrod, @dave-tucker, and @pooladkhay)

test/integration-test/src/tests/load.rs line 19 at r2 (raw file): Previously, pooladkhay (Mohammad Javad Pooladkhay) wrote…

Could you please give me more hints, and maybe an example?

See

https://github.com/aya-rs/aya/blob/b1769678f48f7abf6c987a1d686bbaffd5d1e664/test/integration-test/src/tests/xdp.rs#L137

I tried that. Namespace is created but the test still fails with the same error:

test tests::load::tc_name_limit ... Entered network namespace aya-test-168-0
thread 'tests::load::tc_name_limit' panicked at test/integration-test/src/tests/load.rs:623:32:
called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Exited network namespace aya-test-168-0
FAILED

I wonder what is the purpose of this step in testing? What is the main difference between cargo xtask integration-test vm and cargo xtask integration-test local if the tests are running on a linux machine? Is this related to kernel versions? maybe being able to test on a specific version?

I took a look and the CI file and realised runs-on: ubuntu-22.04 is set at the top but there are also checks like this one: if: runner.os == 'macOS'. Considering those checks checking runner.os, maybe cargo xtask integration-test vm is used in a scenario where the runner is not a linux image? If that's the case, I'm unable to make sense of it because of runs-on: ubuntu-22.04.

Or maybe I'm missing something here...

[mergify[bot]]

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