Skip to content

GPUNETIO: progress independent QPs with CPU-local completion reporting - #2222

Draft
foraxe wants to merge 25 commits into
ai-dynamo:mainfrom
foraxe:gpunetio-independent-qp-progress
Draft

foraxe wants to merge 25 commits into
ai-dynamo:mainfrom
foraxe:gpunetio-independent-qp-progress

Conversation

@foraxe

@foraxe foraxe commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

What?

Allow independent GPUNETIO QPs to make completion progress without waiting for
the oldest entry in a global completion list. Preserve FIFO retirement within
each QP and aggregate every chunk of a logical NIXL request before completion.

Why?

An unfinished bulk transfer can delay reporting a completed small transfer on
another QP. Removing that cross-QP dependency improves completion isolation.

On the matched H20 component workload below, small-transfer p99 decreases from
113.439 to 29.251 µs. The tradeoff is not hidden: single-active-QP 4 KiB WRITE
p50/p99 remain approximately 4.1%/4.0% above the original baseline.

How?

  • Assign per-QP tickets in actual GPU SQ-reservation order and retire them FIFO.
  • Progress data and notification completions without blocking unrelated QPs.
  • Keep scheduler/ticket state in GPU memory. Publish generation-tagged terminal
    records into GPU-accessible pinned host memory, so CPU status checks do not
    repeatedly read GPU-mapped fields over PCIe.
  • Cache CPU-owned request metadata, including the QP pointer and generations.
  • Release-publish the host terminal state last, after GPU ownership ends;
    CPU consumers acquire it and verify the generation.
  • Mirror fatal errors to CPU/GPU views. Defer partial-launch errors until owned
    work drains, and reject pending cancellation with a negative status so the
    frontend retains its handle.
  • Select the engine's CUDA device before QP/CQ allocation on listener threads.

This does not add QP striping, peer-MR, a priority API, or a new congestion-control
protocol. No artificial delay or completion observer is used in performance runs.

Dependencies and review scope

Depends on #2175, whose request-ring ownership changes are included in this
branch. The core-agent and existing agent-test changes belong to that
prerequisite. The progress-specific diff starts at 1ca6efcc.

The tested runtime additionally composes #2052 and #2174 for DOCA 3.1 and the
paired OOB setup. Both benchmark arms include those same prerequisites and the
QP-allocation device-selection correction. These runtime overlays are separate
from the proposed feature diff.

This is a substantial backend change with a paired integration harness, intended
for design review rather than immediate merge. Upstream design discussion and
dependency review remain prerequisites under CONTRIBUTING.md.

Component results

H20, CUDA 12.8, DOCA 3.1; one source GPU and two target GPUs across two nodes;
one data QP per peer. The mixed workload pairs a 2 MiB bulk transfer with an
independent 4 KiB transfer. Each transfer also includes an 8-byte epoch marker.

Three independent process pairs per arm/shape, 20 warmup + 100 measured iterations,
with rotating arm order. Both arms use the same harness and NIXL-core binaries;
only the GPUNETIO plugin differs. Values are medians of per-run metrics.

Metric Original baseline Candidate Change
Mixed small-transfer p99 113.439 µs 29.251 µs −74.21%
Mixed bulk-transfer p99 115.112 µs 114.562 µs −0.48%
Mixed transfer-window rate 18.224 GB/s 18.823 GB/s +3.29%
Single-active-QP 4 KiB WRITE p50 22.963 µs 23.899 µs +4.08%
Single-active-QP 4 KiB WRITE p99 24.650 µs 25.643 µs +4.03%
Single-active-QP 4 KiB READ p50 24.941 µs 24.540 µs −1.61%
Single-active-QP 4 KiB READ p99 33.486 µs 31.331 µs −6.44%
Single-active-QP 2 MiB WRITE p50 109.290 µs 111.859 µs +2.35%
Single-active-QP 2 MiB WRITE p99 111.610 µs 114.505 µs +2.59%
Single-active-QP 2 MiB READ p50 141.656 µs 142.817 µs +0.82%
Single-active-QP 2 MiB READ p99 163.586 µs 161.755 µs −1.12%

Latency is source post-to-NIXL-API-completion, not a receiver-kernel or NIC-CQ
timestamp. Transfer-window rate includes software completion overhead; it is
not a raw NIC-bandwidth measurement. READ tails vary across runs. No serving,
DeepEP, or end-to-end wall-throughput improvement is claimed.

Independent run values (runs 1 / 2 / 3)
Metric Baseline runs Candidate runs
Mixed small p99 (µs) 113.268 / 113.439 / 113.556 29.251 / 26.740 / 30.569
Mixed bulk p99 (µs) 115.272 / 114.531 / 115.112 115.390 / 113.540 / 114.562
Mixed transfer-window GB/s 18.268 / 18.224 / 18.213 18.809 / 18.852 / 18.823
4 KiB WRITE p50 (µs) 22.754 / 22.963 / 23.002 24.108 / 23.899 / 23.309
4 KiB WRITE p99 (µs) 24.281 / 24.650 / 24.694 27.693 / 25.643 / 25.355
4 KiB READ p50 (µs) 24.941 / 24.936 / 25.169 25.021 / 24.540 / 24.138
4 KiB READ p99 (µs) 29.229 / 33.486 / 44.622 42.630 / 26.544 / 31.331
2 MiB WRITE p50 (µs) 108.935 / 109.290 / 109.754 111.859 / 111.923 / 111.358
2 MiB WRITE p99 (µs) 111.876 / 111.610 / 111.583 114.505 / 115.782 / 113.600
2 MiB READ p50 (µs) 141.656 / 141.642 / 142.146 142.817 / 142.865 / 141.789
2 MiB READ p99 (µs) 163.586 / 180.684 / 144.404 154.086 / 171.157 / 161.755

Validation

31 paired runs passed: 27 performance runs across the original baseline,
pre-fix implementation, and final implementation; plus four final correctness
cases. Both endpoints exited cleanly. The correctness cases cover:

  • 513-descriptor WRITE/READ, 14 outstanding requests occupying 28 ring slots,
    129 subsequent reuse requests, and data-coupled/standalone notifications;
  • prepared-handle reuse with changing payload bytes and epochs;
  • stale remote-MR errors reaching NIXL with safe release;
  • CPU pre-enqueue errors for transfers and notifications: pending ownership is
    retained, queued work subsequently aborts, and receiver sentinels stay unchanged.

The final payload oracle mixes the full source/epoch seed and byte offset.
Every arm was measured using that oracle. Expected injected errors are confined
to fault cases. Arbitrary fatal CUDA device faults are not claimed as tested.

Reproduction

The committed test/gtest/plugins/gpunetio/README.md contains the build commands,
required environment variables, role setup, and output definitions. Configure
both endpoints with matching settings and a fresh metadata directory for every
process pair. An environment-related SKIPPED result is not a pass.

Pinned references:

  • PR branch: gpunetio-independent-qp-progress, 292249b8.
  • Runtime baseline: gpunetio-progress-runtime-foundation, 1901c6d0.
  • Runtime candidate: gpunetio-progress-runtime-candidate, 4db224b5.

The runtime refs reproduce the common platform prerequisites. Build the candidate
harness once and use that same executable/core installation for both arms,
selecting the baseline or candidate plugin with NIXL_PLUGIN_DIR. Use identical
compiler options for both plugin builds. The final reference updates after
measured b260dd16/f6db60a7 are README-only.

# After building and exporting the paired role/environment from the README:
export QP_TEST="$PWD/build/test/gtest/plugins/gpunetio/gpunetio_qp_progress_gtest"
: "${ARM_PLUGIN_DIR:?Set the baseline or candidate GPUNETIO plugin directory}"
test -f "$ARM_PLUGIN_DIR/libplugin_GPUNETIO.so"
sha256sum "$QP_TEST" "$ARM_PLUGIN_DIR/libplugin_GPUNETIO.so"

# Run on both endpoints with their respective source/target roles.
NIXL_PLUGIN_DIR="$ARM_PLUGIN_DIR" \
  "$QP_TEST" --gtest_filter=QpProgress.PerformanceMixed2MiBAnd4KiB

NIXL_PLUGIN_DIR="$ARM_PLUGIN_DIR" NIXL_QP_PROGRESS_CONTROL_BYTES=4096 \
  "$QP_TEST" --gtest_filter=QpProgress.SingleQpReadWriteControl
NIXL_PLUGIN_DIR="$ARM_PLUGIN_DIR" NIXL_QP_PROGRESS_CONTROL_BYTES=2097152 \
  "$QP_TEST" --gtest_filter=QpProgress.SingleQpReadWriteControl

ARM_PLUGIN_DIR selects the plugin build under test; keep the common core-library
search path unchanged. Repeat each performance shape with three fresh process
pairs per arm. The harness writes qp_progress_performance.json or
qp_progress_single_qp_control.json into the configured metadata directory.

Run the four candidate correctness cases as separate fresh pairs:

"$QP_TEST" --gtest_filter=QpProgress.OutstandingDescriptorsAndAttachedStreamOrder
"$QP_TEST" --gtest_filter=QpProgress.RemoteDeregisterReturnsBackendError
"$QP_TEST" --gtest_filter=QpProgress.CpuFatalLatchRejectsQueuedTransfer
"$QP_TEST" --gtest_filter=QpProgress.CpuFatalLatchRejectsQueuedNotification

Use target-fault for the remote-MR case only; the CPU pre-error cases use normal
source/target roles. Keep the candidate plugin selected for correctness tests.

foraxe and others added 25 commits September 7, 2026 18:14
Signed-off-by: foraxe <foraxe@users.noreply.github.com>
(cherry picked from commit 8a61ec6)
Signed-off-by: foraxe <foraxe@users.noreply.github.com>
(cherry picked from commit c68db92)
Signed-off-by: foraxe <foraxe@users.noreply.github.com>
(cherry picked from commit d2f1557)
Signed-off-by: foraxe <foraxe@users.noreply.github.com>
(cherry picked from commit 8b0883b)
Signed-off-by: foraxe <foraxe@users.noreply.github.com>
(cherry picked from commit dd8d094)
Signed-off-by: foraxe <foraxe@users.noreply.github.com>
(cherry picked from commit 4e6d61c)
Signed-off-by: foraxe <foraxe@users.noreply.github.com>
(cherry picked from commit 6846c18)
Signed-off-by: foraxe <foraxe@users.noreply.github.com>
(cherry picked from commit 57dfc94)
…ocking

Signed-off-by: Yunxiao Ning <ningyunxiao.nyx@antgroup.com>
Signed-off-by: Yunxiao Ning <ningyunxiao.nyx@antgroup.com>
Signed-off-by: Yunxiao Ning <ningyunxiao.nyx@antgroup.com>
Signed-off-by: Yunxiao Ning <ningyunxiao.nyx@antgroup.com>
Signed-off-by: Yunxiao Ning <ningyunxiao.nyx@antgroup.com>
Signed-off-by: Yunxiao Ning <ningyunxiao.nyx@antgroup.com>
Signed-off-by: Yunxiao Ning <ningyunxiao.nyx@antgroup.com>
Signed-off-by: Yunxiao Ning <ningyunxiao.nyx@antgroup.com>
Signed-off-by: Yunxiao Ning <ningyunxiao.nyx@antgroup.com>
Signed-off-by: Yunxiao Ning <ningyunxiao.nyx@antgroup.com>
Signed-off-by: Yunxiao Ning <ningyunxiao.nyx@antgroup.com>
Signed-off-by: Yunxiao Ning <ningyunxiao.nyx@antgroup.com>
Signed-off-by: Yunxiao Ning <ningyunxiao.nyx@antgroup.com>
Signed-off-by: Yunxiao Ning <ningyunxiao.nyx@antgroup.com>
Signed-off-by: Yunxiao Ning <ningyunxiao.nyx@antgroup.com>
Signed-off-by: Yunxiao Ning <ningyunxiao.nyx@antgroup.com>
Signed-off-by: Yunxiao Ning <ningyunxiao.nyx@antgroup.com>
@copy-pr-bot

copy-pr-bot Bot commented Sep 8, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown

👋 Hi foraxe! Thank you for contributing to ai-dynamo/nixl.

Your PR reviewers will review your contribution then trigger the CI to test your changes.

🚀

@svc-nixl

svc-nixl commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentPR Size Check · commit 292249b8

TL;DR: This isn't a build or test breakage — the PR Size Check workflow deliberately failed PR #2222 because it adds 1048 lines to existing files, more than double the hard 500-line policy limit. The fix is to split the PR into smaller reviewable pieces (or get the size policy waived/adjusted).

Full analysis

Summary: Job check-pr-size in .github/workflows/pr-size-check.yml exited 1 with "❌ PR size check failed! This PR adds 1048 lines (excluding subprojects), which exceeds the maximum of 500 lines."

Root cause: The workflow's single step computes added lines with git show --numstat --pretty="" --diff-filter=M -- . ':(exclude)subprojects/*' on the PR merge commit and hard-fails when the sum exceeds 500. For merge commit 1b5c652 (PR head 292249b8, branch gpunetio-independent-qp-progress) the sum was 1048, so the [[ "$LINES_CHANGED" -gt 500 ]] branch ran exit 1. There is no infrastructure or toolchain problem: checkout succeeded, the whole job took ~2 seconds (12:00:32 → 12:00:34) with no gaps, and the only error in the log is this intentional policy exit. Note the metric counts added lines in modified files only (--diff-filter=M), so newly added files don't count — 1048 lines of growth is all in pre-existing files.

Implicated commit: [REDACTED:Hex High Entropy String] (PR #2222 head; merge commit [REDACTED:Hex High Entropy String]). The workflow itself is unchanged and not at fault.

File: .github/workflows/pr-size-check.yml:21-26 (check logic); the oversized change is in PR #2222's diff against base [REDACTED:Hex High Entropy String].

Suggested fix: Split PR #2222 into several PRs each under 500 added lines in modified files — for the GPUNetIO independent-QP-progress work, natural splits are (a) plumbing/struct and header changes, (b) the QP progress engine itself, (c) tests and examples. Run git show --numstat --pretty="" --diff-filter=M -- . ':(exclude)subprojects/*' | sort -k1 -rn | head locally to see which files dominate the 1048 lines and peel those off first. If the change genuinely cannot be decomposed, ask a maintainer for an explicit exception rather than editing the threshold — and if exceptions are expected to be routine, add a bypass label check (e.g. skip when the PR carries a large-pr-approved label) to the workflow so exemptions are auditable instead of raising the global 500-line limit.

Related: PR #2222 (this PR); PR #2215 "[GPUNetIO] Add optional multi-QP WRITE striping" appears to be adjacent GPUNetIO multi-QP work and may be a candidate to stack this change onto or to split against.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id d197cf9b-4306-4c48-a3cd-1165dd7d91a3 in the triage console for the audit trail.

@svc-nixl

svc-nixl commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentBlossom-CI · commit 292249b8

TL;DR: The Blossom-CI Authorization job failed only because the blossom-ci AUTH gate declined to auto-trigger an unsigned head commit (292249b8) and exited 255 — no build or test ever ran. Fix by re-triggering with a /build comment from an authorized maintainer (or pushing GPG/SSH-signed commits); the workflow should not report a declined auto-trigger as a failure.

Full analysis

Summary: Blossom-CI / Authorization (GHA run 34223682847) failed with ##[error]Process completed with exit code 255 at the blossom-ci AUTH step; the pipeline stopped before Vulnerability-scan / Job-trigger.

Root cause: The run was started by the automatic pull_request_target/poll path (log: Evaluating: (success() && (((github.event.comment.body == '/build') || ((github.event_name == 'pull_request_target')))))Result: true). The blossom-ci AUTH helper then checked the PR's commits and rejected the automatic trigger:

  • Commit signature not verified (reason=unsigned); declining auto-trigger
  • PR State: open
  • Auto-trigger declined: use manual comment trigger
  • ##[error]Process completed with exit code 255.

So this is an authorization/policy gate outcome on commit [REDACTED:Hex High Entropy String] (branch gpunetio-independent-qp-progress, PR #2222) being unsigned — not a defect in nixl code, the build, or the test infrastructure. The gate signals "declined" with a non-zero exit, which GitHub surfaces as a red check. Note also a config mismatch: the checked-in workflow's on: list contains only issue_comment and workflow_dispatch and the job guard is if: github.event.comment.body == '/build', yet the executed workflow (resolved from refs/heads/main at run time) also permits pull_request_target — that extra auto-trigger path is exactly what reached the signature check and failed.

Implicated commit: [REDACTED:Hex High Entropy String] (PR #2222 head, unsigned) is what the gate rejected. The auto-trigger behavior traces to the workflow trigger configuration last touched by 77b10ee6 — "CI: Blossom ci separate checks (#1133)", Alexey Rivkin (earlier attempts at this same auto-trigger path: 531e8038, reverted by 8d78f896, Daniel Pressler).

File: .github/workflows/blossom-ci.yml:24-38 (Authorization job / OPERATION: 'AUTH' step); trigger list at .github/workflows/blossom-ci.yml:12-22

Suggested fix:

  1. Immediate unblock — have an authorized maintainer post a /build comment on PR GPUNETIO: progress independent QPs with CPU-local completion reporting #2222 (the manual path the log explicitly recommends), or have the PR author re-push with verified GPG/SSH-signed commits so the auto-trigger is accepted.
  2. Durable fix — stop a declined auto-trigger from reporting as a build failure. Either:
    • Remove the pull_request_target auto-trigger from the deployed workflow so it matches the checked-in if: github.event.comment.body == '/build' guard (as was done in revert 8d78f896), or
    • Make the AUTH step exit 0 / skip cleanly when the only reason is "unsigned commit, use manual comment trigger" — e.g. guard the step with continue-on-error: true for the auto-trigger event, or gate the job with an additional if on the commit-verification state — so unsigned PRs simply don't start CI instead of posting a failed check.
  3. If signed commits are intended to be a requirement, document it in CONTRIBUTING so contributors know unsigned pushes will never auto-trigger CI.

Related: PR #2222 (this PR); workflow history: #1133 (77b10ee6), #771 (531e8038) and its revert #775 (8d78f896), #748 (d899d0fe) — all previous iterations of blossom-ci auto-trigger-without-comment behavior.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id dc072f5a-c9b4-4267-8abb-2c1339069ed7 in the triage console for the audit trail.

@svc-nixl

svc-nixl commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentRun Pre-Commit Hooks · commit 292249b8

TL;DR: The pre-commit codespell hook failed with 10 false positives on the camelCase identifier NowNs (now + nanoseconds) in the new file test/gtest/plugins/gpunetio/qp_progress_gtest.cu, which codespell reads as a misspelling of "knowns/nouns"; rename the identifier to now_ns (or add nowns to a codespell ignore list).

Full analysis

Summary: GitHub Actions job "Run Pre-Commit Hooks" (run 34223682974, PR #2222) failed at the pre-commit run --files ... step — hook codespell exited 65; all other hooks passed or were skipped.

Root cause: codespell's dictionary contains the entry nowns -> knowns, nouns. codespell splits camelCase words, so the identifier fragment NowNs — a "now timestamp in nanoseconds" name introduced in the new test file added by this PR — matches that dictionary entry. It fires 10 times (lines 82, 618, 628, 738, 743, 749, 750, 757, 763, 780 of test/gtest/plugins/gpunetio/qp_progress_gtest.cu), each reported as NowNs ==> knowns, nouns. This is a spelling-lint false positive, not a functional defect. The hook is configured with args: ["--toml", "pyproject.toml"] in .pre-commit-config.yaml:42-47, but pyproject.toml (51 lines, ending at [tool.meson-python.args]) has no [tool.codespell] section at all — so there is no ignore-words-list to suppress it, and the exclude regex only covers .md, .rs, stemmer/stop-words files and src/utils/serdes/serdes.h, not .cu test sources.

Implicated commit: [REDACTED:Hex High Entropy String] (PR #2222, branch gpunetio-independent-qp-progress) — the commit that adds test/gtest/plugins/gpunetio/qp_progress_gtest.cu

File: test/gtest/plugins/gpunetio/qp_progress_gtest.cu:82 (and 618, 628, 738, 743, 749, 750, 757, 763, 780); config at .pre-commit-config.yaml:42-47 and pyproject.toml (missing [tool.codespell])

Suggested fix: Preferred — rename the identifier in the new test so codespell no longer sees the NowNs token, e.g. getNowNs()/nowNsget_now_ns()/now_ns or nowNsec/nowTimeNs; codespell tokenizes on underscores, so the snake_case form does not trigger the entry. If the camelCase naming must be kept for consistency with the surrounding gpunetio code, add to pyproject.toml:

[tool.codespell]
ignore-words-list = "nowns"

(this also makes the existing --toml pyproject.toml argument meaningful, since the file currently supplies codespell no configuration). Then run pre-commit run --all-files locally before pushing to confirm the hook is green.

Related: none found — no existing issue/PR in ai-dynamo/nixl tracks a codespell nowns false positive or a [tool.codespell] ignore list.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 2a562b77-9516-4344-9dae-1e126805a2c8 in the triage console for the audit trail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants