Perf regression test

[daniel-noland]

Fix perf regression due to CPU pinning

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4e27c1e2-2408-4c32-9b13-7a1e3054eaa8

📥 Commits

Reviewing files that changed from the base of the PR and between 7de01aa and e3fdcd7.

📒 Files selected for processing (4)

• dataplane/src/runtime.rs
• dpdk/Cargo.toml
• dpdk/src/eal.rs
• dpdk/src/test_support.rs

🚧 Files skipped from review as they are similar to previous changes (4)

• dpdk/src/eal.rs
• dpdk/Cargo.toml
• dataplane/src/runtime.rs
• dpdk/src/test_support.rs

---

📝 Walkthrough

Walkthrough

DPDK CPU affinity is centralized in main_lcore_arg(), which formats an explicit --lcores mapping. Production and test EAL initialization now use this helper, with nix declared as a required dependency.

Changes

DPDK lcore affinity

Layer / File(s)	Summary
Affinity argument helper dpdk/src/eal.rs, dpdk/Cargo.toml	Adds main_lcore_arg() to format the calling thread’s CPU affinity as a DPDK --lcores argument and makes nix a required dependency.
EAL initialization wiring dataplane/src/runtime.rs, dpdk/src/test_support.rs	Uses the shared lcore argument in production and test EAL startup, removing the duplicate affinity helper.

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is related to the change set, but it is somewhat generic and does not mention the CPU pinning fix.
Description check	✅ Passed	The description clearly matches the change set by describing the CPU pinning performance regression fix.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

This PR addresses a DPDK-related performance regression by ensuring the DPDK EAL init path does not implicitly pin the calling thread (and therefore subsequently spawned threads) to a single CPU due to DPDK’s default main-lcore affinity behavior.

Changes:

• Introduces dpdk::eal::main_lcore_arg() to build a --lcores mapping that keeps the main lcore affinity aligned with the process/thread’s allowed CPU set.
• Uses --lcores <computed> in both the dataplane runtime EAL init and DPDK test EAL init helpers.
• Updates the CI workflow build matrix definition (notably expanding the build matrix inline rather than via an anchor).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
dpdk/src/test_support.rs	Switches test EAL startup to use the shared main_lcore_arg() helper instead of local affinity parsing.
dpdk/src/eal.rs	Adds main_lcore_arg() and documentation explaining why --lcores is needed to avoid single-core pinning.
dpdk/Cargo.toml	Makes nix a non-optional dependency (needed by main_lcore_arg()), and removes it from the test feature list.
dataplane/src/runtime.rs	Passes --lcores computed by main_lcore_arg() into early EAL init to avoid narrowing process CPU affinity.
.github/workflows/dev.yml	Adjusts matrix definitions for build jobs (but currently has an exclude that likely won’t match as intended).

Comments suppressed due to low confidence (2)

dpdk/src/eal.rs:134

confidence: 8
tags: [logic, style]

`main_lcore_arg` currently (a) suppresses potential errors from `CpuSet::is_set` via `unwrap_or(false)` and (b) uses a very terse `expect("sched_getaffinity")` message. If either error occurs, the function can silently drop CPUs and potentially produce an empty `0@()` mapping that will later fail in `rte_eal_init`, making the root cause hard to diagnose.

Handle `is_set` errors explicitly and use more descriptive panic messages so failures point directly at the affinity query.

#[allow(clippy::expect_used)]
pub fn main_lcore_arg() -> String {
use nix::sched::{CpuSet, sched_getaffinity};
use nix::unistd::Pid;
let set = sched_getaffinity(Pid::from_raw(0)).expect("sched_getaffinity");
let cpus = (0..CpuSet::count())

**.github/workflows/dev.yml:355**
* ```yaml
confidence: 9
tags: [other]

This matrix exclude entry is unlikely to match anything because matrix.build is an object (name/profile/sanitize/instrument), but the exclude only specifies build.name. In GitHub Actions, excludes must match the full axis value for object-valued dimensions, so this will effectively be a no-op and may unintentionally run the excluded job.

Specify the full build object in the exclude (or convert build to a scalar axis and use include).

        build:
          - name: "debug"
            profile: "debug"
            sanitize: ""
            instrument: "none"
          - name: "release"
            profile: "release"
            sanitize: ""
            instrument: "none"
          - name: "thread"
            profile: "fuzz"
            sanitize: "thread"
            instrument: "none"
        exclude:
          - nix-target: validator
            build:
              name: "debug"
    steps:

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

.github/workflows/dev.yml:355

confidence: 9
tags: [logic]

The build matrix now includes a `thread` (TSAN) variant, but the `validator` target uses `platform=wasm32-wasip1`. ThreadSanitizer flags aren’t supported for wasm builds (they propagate via `sanitize=${{ matrix.build.sanitize }}` into the Nix/Rust flags), so `validator/thread` is likely to fail. Exclude `validator` + `thread` from the matrix (similar to the existing `validator` + `debug` exclusion).

    build:
      - name: "debug"
        profile: "debug"
        sanitize: ""
        instrument: "none"
      - name: "release"
        profile: "release"
        sanitize: ""
        instrument: "none"
      - name: "thread"
        profile: "fuzz"
        sanitize: "thread"
        instrument: "none"
    exclude:
      - nix-target: validator
        build:
          name: "debug"
steps:

</details>

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (3)

nix/profiles.nix:148

confidence: 8
tags: [style]

`sanitize.leak` adds `-Cforce-frame-pointers=yes` for Rust but doesn’t add `-fno-omit-frame-pointer` for C/C++ like the address/thread sanitizer profiles do. This can reduce the quality of native stack traces and makes the leak profile inconsistent with the other sanitizers.

sanitize.leak.NIX_CFLAGS_COMPILE = [
"-fsanitize=leak"
];

**dpdk/src/eal.rs:132**
* ```yaml
confidence: 8
tags: [logic]

main_lcore_arg can produce an invalid --lcores value ("0@()") if the affinity query returns an empty CPU set, which would fail later with a less clear EAL error. Also, this function uses expect (and will need a direct panic! for the empty-set case); the dpdk crate’s top-level docs ask that #[allow(...)] panics/unwraps include a short justification comment (see dpdk/src/lib.rs:18-25).

#[must_use]
#[allow(clippy::expect_used)]
pub fn main_lcore_arg() -> String {
    use nix::sched::{CpuSet, sched_getaffinity};
    use nix::unistd::Pid;

.github/workflows/dev.yml:355

confidence: 9
tags: [logic]

The build matrix now includes a `thread` build with `sanitize: "thread"`, but the `validator` target is built for `wasm32-wasip1` (see the `platform` selection below). Thread sanitizer isn’t supported on wasm, so `validator/thread` is very likely to fail; it should be excluded from the matrix.

      - name: "thread"
        profile: "fuzz"
        sanitize: "thread"
        instrument: "none"
    exclude:
      - nix-target: validator
        build:
          name: "debug"
steps:

</details>

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

dpdk/src/eal.rs:129

confidence: 7
tags: [style]

This crate-level documentation asks that `expect`/`panic` usage be accompanied by an explicit `#[allow(...)]` with a short justification comment. Adding that justification here would make it clearer why panicking is acceptable in this API (and helps future reviewers keep clippy-lint exceptions intentional). See `dpdk/src/lib.rs` (crate docs) for the stated guidance.

#[allow(clippy::expect_used)]

</details>

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

dpdk/src/eal.rs:131

confidence: 9
tags: [style]

`main_lcore_arg` uses `#[allow(clippy::expect_used)]` but doesn’t include the required in-line justification comment for allowing panics/expect in library code (per `dpdk/src/lib.rs:18-25`). Add a short rationale on the allow attribute so the exception is self-documenting.

#[must_use]
#[allow(clippy::expect_used)]
pub fn main_lcore_arg() -> String {

**dpdk/src/eal.rs:138**
* ```yaml
confidence: 9
tags: [logic]

The docs say this function panics if the affinity resolves to an empty CPU set, but the current implementation would return "0@()" instead. Also, unwrap_or(false) can silently drop CPUs if is_set errors. Validate non-empty results and fail loudly if is_set fails so callers don’t pass an invalid/degenerate --lcores value.

    let set = sched_getaffinity(Pid::from_raw(0)).expect("sched_getaffinity");
    let cpus = (0..CpuSet::count())
        .filter(|&i| set.is_set(i).unwrap_or(false))
        .map(|x| x.to_string())
        .collect::<Vec<_>>()

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

dpdk/src/eal.rs:128

confidence: 9
tags: [docs, style]

The crate-level docs request a `# Safety` note describing the conditions that can lead to a panic when panics are unavoidable in library code (see `dpdk/src/lib.rs:18-25`). This doc block also has a grammar typo ("the are"). Consider updating the heading/wording to match the crate convention and fix the typo.

/// # Panics
///
/// - Panics if the calling thread's CPU affinity cannot be queried.
/// - Panics if the are no available CPUs in the result of sched_getaffinity.

**dpdk/src/eal.rs:131**
* ```yaml
confidence: 7
tags: [other]

main_lcore_arg is now relied on by both runtime and test EAL initialization, but there’s no smoke test ensuring it returns a non-empty, well-formed --lcores mapping string. Adding a simple format test helps prevent regressions (e.g. accidental 0@() or malformed delimiters) without needing DPDK initialized.

pub fn main_lcore_arg() -> String {

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

dpdk/src/eal.rs:131

confidence: 7
tags: [docs, style]

`main_lcore_arg` can panic (via `expect` and `assert_ne!`). `dpdk/src/lib.rs` asks that panicking library code include both an explicit allow and a `# Safety` note explaining the panic conditions. This function has a `# Panics` section but no `# Safety` section, and it doesn’t explicitly allow/justify the panic path used for the empty-affinity assertion.

/// # Panics
///
/// - Panics if the calling thread's CPU affinity cannot be queried.
/// - Panics if there are no available CPUs in the result of sched_getaffinity.
#[must_use]
#[allow(clippy::expect_used)]
pub fn main_lcore_arg() -> String {

</details>