feat(deepep-efa): vLLM DeepEP-V2 MoE all-to-all over EFA (eager + non-eager)

[dmvevents]

Adds examples/inference/vllm/deepep-v2-efa/ — the V2/NCCL-GIN counterpart to the merged sglang/dsr1-deepep-efa (#1215). Serves an MoE on vLLM with DeepEP-V2 ElasticBuffer all-to-all over EFA via the aws-ofi-nccl GIN CPU-proxy (NCCL_GIN_TYPE=2), no NVSHMEM/IBGDA.

Both execution modes measured on 2× p5en (H200), Qwen3-30B-A3B-FP8 DP16/EP16: eager 4.8→301.8 tok/s (121/121 HTTP 200), non-eager 5.0→239.7 tok/s (153/153 HTTP 200). Full tables + environment provenance in benchmarks/README.md — including the fact that the recipe's DeepEP source and GIN plugin have moved since those tables were taken (see "Verification level").

What the recipe is today (after four review rounds)

• GIN plugin = the EFA installer's bundled aws-ofi-nccl 1.21.1 (installer >= 1.50.0, GIN GA since v1.21.0). Not built from source; the Dockerfile gates fail-loud on the bundled plugin's ncclGinPlugin_v14 export and on gdrcopy having been compiled in. The gdrdrv-2.4 forced-PCIe override this sample once cherry-picked (gdrcopy: add OFI_NCCL_GDRCOPY_FORCED_PCIE_COPY override for the forced-PCIe capability probe aws/aws-ofi-nccl#1351) was declined upstream, so host gdrcopy/gdrdrv >= 2.5 is a documented prerequisite instead of a container workaround.
• DeepEP-V2 source = the amazon-contributing/DeepEP fork at one immutable SHA (97d8f9bc) — the tree AWS points to for DeepEP-V2 on EFA and the one the repo's canonical deepep-v2-benchmark provisioner pins; the sibling vllm/deepep-v2-gdaki-efa and nvidia-dynamo/deepep-v2-efa samples pin the same SHA. No PR merge, no local patch: the fork carries the in-tree successors of aws-efa: cap auto-QP at 2 on EFA to avoid 128-slot GIN ring overflow deepseek-ai/DeepEP#612's EFA work (QP count clamped from _C runtime constants, RDMA link rate probed from sysfs), so no EP_EFA_MAX_QPS / EP_EFA_RDMA_GBS env exists in this sample.
• One integration fix baked in + documented: EP_REUSE_NCCL_COMM=0 (stock DeepEP-V2 null-derefs ncclTeamWorld under vLLM's lazy torch comms — deterministic serve-init segfault; the guard is also filed upstream as utils: only reuse PyTorch's NCCL comm when _comm_ptr() is non-null deepseek-ai/DeepEP#727).
• Zero build-time patches. Default (CUDA-graph) compilation works at the shipped vLLM pin (14617c2b, the merge commit of [Bugfix] DeepEP-V2: expert_tokens_meta must be None on the decode/cudagraph path (empty recv_expert_num_tokens) vllm-project/vllm#52632) with no patch step; eager stays the shipped default because it is what the published tables were measured with.
• Transport env contract = NCCL params (NCCL_GIN_TYPE=2, NCCL_GIN_ENABLE=1, …) + libfabric/aws-ofi-nccl params that exist at the pinned versions. Knobs earlier revisions exported but that nothing reads (OFI_NCCL_GIN_GDAKI, OFI_NCCL_GIN_MAX_REQUESTS — not in aws-ofi-nccl 1.21.1's parameter table; DEEP_EP_BACKEND — read by neither DeepEP nor vLLM) are gone.
• Follows the sglang exemplar layout + conventions: in-tree setup_deepep_v2_efa.sh (distinctly named — not vendor-synced to the 567632d NVSHMEM setup; it only stages source, the one DeepEP build is in-pod on first boot), ARG-pinned versions (no latest), user-supplied ECR, MIT-0 headers, fail-loud verify-image.sh (includes a fork-discriminator check) and run-kernel-test.sh gates.

Verification level, stated plainly

• The benchmarks/ tables were measured on the previous substrate (vLLM e2f993dc4, DeepEP b306af06+PR#612, source-built aws-ofi-nccl 9c44d34+#1351); the provenance table says so row by row.
• The current recipe is verified at build + symbol level (clean docker build: installer 1.50.0 plugin gates, fork-discriminator check, submodule init, NCCL 2.30.4 re-pin assert). The 2-node EFA E2E (kernel smoke + serve + sweep) has not been re-run on the current DeepEP pin + bundled plugin; that re-run is the named remaining step and the tables will be refreshed from it.

[dmvevents]

Re-scoped per the repo's no-patches rule — thank you for stating it plainly rather than letting it surface in review.

What changed (this push):

• recipe/apply-noneager-fix-stack.sh (cherry-pick + build-time source rewrite) is removed. The case now ships zero patches.
• Eager (--enforce-eager) is the shipped + supported path — it is also our production recommendation today.
• Default compilation (CUDA graphs) is documented as pending one upstream fix: the empty-ExpertTokensMetadata guard, now filed as vllm-project/vllm#52632. Once it merges, a vLLM pin bump enables non-eager here with no recipe change — exactly the pin-bump-survivable shape the rule requires.
• The non-eager numbers in benchmarks/ are retained but relabeled as historical measurements taken with the then-unmerged guard.

This keeps the case alive across pin bumps by construction: nothing in the build rewrites upstream source.

[dmvevents]

Follow-up: the re-scoped eager path is now live-tested end-to-end from this branch — image built exactly per the README (setup/build-push.sh), deployed with kubernetes/vllm-deepep-v2-2node.yaml on 2× p5en.48xlarge (H200), DP16/EP16, Qwen/Qwen3-30B-A3B-FP8: all 16 API servers up, EP split confirmed ([EP Rank 0/16] ... 8/128 experts), chat completions HTTP 200 with coherent output, and node-level EFA rdma_write_bytes moving ~0.4 GB/node per 48-token request (the MoE all-to-all on the wire, no TCP/SHM fallback).

The live gate caught four defects in the case as previously pushed, all fixed in the four commits above:

1. 7921f4e9 — the NCCL linker-precedence assert used a pattern ldconfig -p can never print, so the image could not build at all.
2. 5adc7783 — the FORCED_PCIE param moved from aws-ofi-nccl#1354 to #1351 when we scoped #1354 down last week; the pin now names #1351 at its immutable head, baked as the Dockerfile default (no more "look up the current PR head" step — fully reproducible docker build .).
3. e3df2294 — the vLLM wheel's resolver silently downgraded nvidia-nccl-cu13 back to 2.28.9 (dropping the GIN/LSA symbols DeepEP compiles against); re-pinned after the last pip layer with a fail-loud symbol assert.
4. 33a5ec6c — gdrcopy userspace was missing, so the GIN plugin built without GDRCopy support and ElasticBuffer failed at serve; added the gdrcopy layer (mirrors the sibling GDAKI case) plus an assert that a GDRCopy-less plugin build can never ship silently.

Still zero patches: every fix is Dockerfile/pin hygiene; nothing rewrites upstream source.

[KeitaW]

Review Batch 1/6: Pre-flight Gates & Verification Scripts

The two scripts the README tells a reader to run before committing a node to a multi-hundred-GB weight load cannot report success as written. This is the batch that most determines the verdict: everything else in the sample is in good shape, and these gates are the sample's own safety story.

[KeitaW]

Review Batch 2/6: Benchmark Methodology & Measurement Integrity

The tables are honest about what was run and the caveats section is better than most. These findings are about what the numbers can support, not about whether they were taken in good faith.

No warmup, so the conc=1 row absorbs one-time initialization

The first level runs against a server that has not yet served a request, so it carries first-request initialization that later levels don't. Since conc=1 is also the row the PR body quotes as the low endpoint, an unmeasured warmup request before the sweep would make the levels comparable to each other and across the two execution modes.

The comparison a reader most wants is the one measurement not present

serve.sh hardcodes --all2all-backend deepep_v2, and every table measures that one backend, so the sample can't answer the question a reader arrives with: is DeepEP-V2 over EFA faster than what vLLM already does by default on this hardware? Because a test case in this repo functions as a recommendation, the absence reads as an implied yes.

Worth sharing as a data point rather than a correction, since our hardware differs: on 2× p6-b200 with DeepSeek-V2-Lite-Chat at DP16/EP16, the same vLLM pin (e2f993dc4), both arms --enforce-eager, EP_EFA_MAX_QPS=2, n=3, we measured deepep_v2 against allgather_reducescatter and the stock backend was not the strawman one might expect. On short-prefill agent traffic it was near parity on throughput (deepep_v2 345.9 vs allgather 336.9 tok/s, +2.7% for deepep_v2) with allgather 25-30% lower p50. On long-prefill RAG-shape traffic allgather won outright: 449.25 vs 329.80 tok/s (+36.2%) and p50 4.50 s vs 6.18 s. Zero request failures in every run of every arm.

Different GPU generation, different model (64-expert with shared experts vs your 128-expert Qwen3), different traffic shape, and both arms paying the eager tax, so this predicts nothing about your p5en/Qwen3 configuration. But it is enough to say that adding one allgather_reducescatter sweep on the same pods would turn this from "how to enable DeepEP-V2 on EFA" into "and here is when it helps", which is the more useful sample. Failing that, a caveat noting no alternative-backend baseline was measured would keep a reader from over-reading the tables.

[KeitaW]

Review Batch 3/6: Reproducibility & Cross-File Consistency

[KeitaW]

Review Batch 4/6: Container Build & Pin Hygiene

The pin discipline here is genuinely strong, see batch 6. These are the rough edges around it.

Truncating the build log hides the diagnostic a failed build needs

python3 setup.py build_ext --inplace 2>&1 | tail -30 keeps only the last 30 lines, which for an nvcc failure is usually the generic "error: command failed with exit status 1" tail rather than the actual compiler diagnostic. Writing the full output to a file and tailing that would keep the console tidy without losing the evidence.

[KeitaW]

Review Batch 5/6: Kubernetes Manifest & Runtime Posture

The headless Service advertises port 8000 on workers that run no API server

The Service selects app: vllm-deepep-v2, which matches both replicas, and publishes port 8000. But workers launch serve.sh worker … --headless, which starts no HTTP server, so resolving the Service name for HTTP round-robins into connection-refused roughly half the time. Keeping the headless Service for dp-rpc and adding a second Service selecting statefulset.kubernetes.io/pod-name: vllm-deepep-v2-0 for HTTP would make the endpoint predictable.

hugepages-2Mi: 5120Mi is an unusual requirement to carry without a note

This is the only manifest in 3.test_cases/ that requests hugepages, and hugepages-2Mi is only allocatable if the node pre-allocated 2 MiB hugepages at boot, otherwise the pod sits Pending with no obvious explanation. I want to be careful not to overstate this: on the p6-b300 nodes I tested, hugepages were pre-allocated (42,242 Mi allocatable) and the pod scheduled without trouble (verified live, 2026-08-18). So this is not universally broken, and if EFA/libfabric needs it in your configuration that is worth stating. A one-line comment on what pre-allocates them would save the next reader a confusing Pending.

Two smaller manifest notes

privileged: true already grants all capabilities, so the explicit capabilities.add: [IPC_LOCK] is redundant (harmless, but it reads as though it were doing work). And the work emptyDir's sizeLimit: 900Gi draws on node ephemeral storage, on an AMI whose /var/lib/kubelet sits on the root EBS volume that is either unsatisfiable or an eviction mid-run; a pointer to the instance-store mount would make the intent explicit.

[KeitaW]

Review Batch 6/6: Tuning, Positives & Sources

Things That Look Great

I built this image from the branch and ran what I could of it, so these are checked rather than skimmed.

The pin discipline is the best I've seen in a test case here. Every pin is justified inline, both PR references use immutable head SHAs with the moving-ref trap called out explicitly in the comments, and the git merge of DeepEP #612 at 28d1f7fb reproduces exactly (the staged tree's git log shows the three #612 commits). I checked the whole reference chain and it holds: DeepEP #727 and vLLM #52632 are both open and authored by you, aws-ofi-nccl #1351 is at c2e773df and is a single commit, so the git cherry-pick you use for it is correct, not a partial application, the FALLBACK_V1_FOR_GDRDRV_24 marker really is present at 9c44d34, and the pinned vLLM wheel URL returns HTTP 200 (verified live, 2026-08-18).

The Layer 5b re-pin is the kind of thing that saves the next person a day. I watched it happen during my build: the vLLM wheel layer pulled nvidia-nccl-cu13 back down to 2.28.9 and nvshmem to 3.4.5, and Layer 5b restored 2.30.4/3.6.5 with the symbol assert passing. Documenting it as "any pip layer added later must keep this re-pin LAST" rather than just fixing it silently is what makes it durable, we hit this same transitive downgrade independently and would have saved the debugging had this comment existed.

The fail-loud build asserts are placed at the right seams, and one of them is load-bearing in a way that's easy to miss: the check that GDRCopy support not available at compile time does not appear in the built plugin. I confirmed it comes out clean (0 occurrences), along with ncclGinPlugin and the GIN/LSA symbols. That assert is exactly what stops the failure your last commit fixed from shipping silently again.

Both integration fixes went upstream instead of into local patch files, and the re-scope to a zero-patch sample is the right call for a repo where pins move, a .patch applied at build time rots silently when upstream shifts, whereas a pin bump either works or fails loudly. Documenting the non-eager path as pending an upstream merge, with a refusal gate in serve.sh behind an explicit acknowledgement variable, is a more honest shape than shipping it half-working.

The EP-divisibility preflight is a real gate, not decoration. I ran it three ways against the live Hub: Qwen/Qwen3-30B-A3B-FP8 at DP16 passes with 128 routed experts, Qwen/Qwen1.5-MoE-A2.7B is correctly refused at 60 experts (reproducing the worked example in your own comment), and a dense Qwen3-8B is correctly refused with the not-a-routed-MoE message. Both failure modes fire, which is more than most preflights manage. It also confirms the README's claim that the default model is public and needs no token.

The measurement reporting is more honest than the norm here, HTTP-200 counts alongside throughput rather than throughput alone, the c=64 non-eager regression reported against interest and explicitly marked as not root-caused, and a caveats section that names pod age, the single sweep, and the absence of variance bars. The batch-2 findings are about what the numbers can support; none of them are about good faith.

The manifest documents its own portability seams, per-instance EFA NIC counts, the EP32 scale seam, and a measured justification for privileged: true with the unprivileged alternative named. That last one is exactly the "one-line rationale for a deliberate security-relevant choice" pattern this repo asks for.

Placement and layout are right. It mirrors the merged sglang/dsr1-deepep-efa exemplar, the index table row is added, MIT-0 headers are on all 15 new files including .gitignore (I checked all of them, not a sample), and the reasoning for keeping setup_deepep_v2_efa.sh out of the NVSHMEM vendor-sync workflow is sound and stated where the next person will find it.

One thing outside this PR's scope, offered only as an FYI since your "H200 only, no Blackwell serving run" limitation is well placed: I tried this on 2× p6-b300 and DeepEP's runtime JIT can't produce a loadable kernel there at this pin. get_arch maps every major-10 device to sm_100f and ptxas rejects the generated PTX (Arguments mismatch for instruction 'mov'); forcing sm_100a assembles but won't load (CUDA_ERROR_NO_BINARY_FOR_GPU, since a targets are arch-specific), and forcing native sm_103a hits the same ptxas error. I checked before attributing it: get_arch is unchanged on DeepEP main, nothing since b306af06 touches it, and no DeepEP issue mentions that ptxas error. Scoping it honestly, issues #590/#608/#622 show people getting real numbers on B300 via the Hybrid-EP paths, so this is "closed on sm_103 with CUDA 13.0 at this pin", not "B300 is unsupported". Nothing here touches your p5en results.

Sources

Upstream code and PRs

• DeepEP tests/elastic/test_ep.py and deep_ep/utils/envs.py at the pinned b306af06, https://github.com/deepseek-ai/DeepEP/blob/b306af06afd412c88e51e71802951606e40b7358/tests/elastic/test_ep.py
• vLLM #52632 (empty-ExpertTokensMetadata guard), vllm-project/vllm#52632
• vLLM #47785 (expert_id < 0 guard in moe_align_sum_kernels.cu, merged 2026-07-10), vllm-project/vllm#47785
• vLLM #46432 / #46404, vllm-project/vllm#46432 · vllm-project/vllm#46404
• DeepEP #612 (EFA auto-QP cap) and #727, deepseek-ai/DeepEP#612 · deepseek-ai/DeepEP#727
• aws-ofi-nccl #1351 and the 6e504db seq-window redesign, aws/aws-ofi-nccl#1351

Verified live, 2026-08-18

• grep -ciE "passed|\bPASS\b|all tests? ok" on test_ep.py @ b306af06 → 0 (no pass marker exists for run-kernel-test.sh:41 to match)
• docker build of this branch's Dockerfile → succeeds; python3 -c "import deep_ep" in the resulting image → ModuleNotFoundError, /opt/DeepEP absent from sys.path
• pip3 --version in the built image → pip 22.0.2; pip3 install --help → 0 occurrences of break-system-packages
• EFA installer 1.49.0 on nvcr.io/nvidia/cuda:13.0.0-devel-ubuntu22.04 → installs libfabric1-aws 2.4.0amzn5.0, ibverbs-providers 63.0-1, and libnccl-ofi-ngc-v3 1.20.0-1 (the NGC branch fires; the build still succeeds)
• gh api repos/aws/aws-ofi-nccl/compare/6e504db...9c44d34 → ahead_by: 76
• gh api repos/aws/aws-ofi-nccl/pulls/1351/commits → length 1 (single-commit PR; the cherry-pick is complete)
• Pinned vLLM wheel URL → HTTP 200, 274,075,589 bytes
• cuobjdump --list-elf on the wheel's _C.abi3.so → SASS sm_75/80/86/89/90/90a/100/120; PTX sm_75/80/90 only
• EP-divisibility preflight against the live Hub → Qwen3-30B-A3B-FP8 (128 experts) passes at DP16; Qwen1.5-MoE-A2.7B (60) and dense Qwen3-8B both correctly refused
• 2× p6-b300 run of this branch: manifest schedules, fi_info reports efa-direct, NET/OFI Selected provider is efa, fabric is efa-direct (found 16 nics), build_deepep.sh produces an sm_103 _C.so with ElasticBuffer: OK; run-kernel-test.sh fails at init_process_group and passes that point with --nproc-per-node=1

[KeitaW]

Thank you! Few comments.

[dmvevents]

All 28 threads from the 08-18 round are answered inline and the fixes are on the branch at 0e5f902 (5d133f7 transport/image gates, eb72b3f benchmark methodology, 0e5f902 reproducibility). Re-verifying my own replies against the tree before asking for another look, since a "fixed in X" that isn't true at head wastes your round:

• Gates — run-kernel-test.sh is one torchrun proc/node with --num-processes "$GPUS_PER_NODE", gates on exit code, and the transport assert is provider-specific (efa-direct|Selected Provider is efa), so a tcp;ofi_rxm banner no longer satisfies it. verify-image.sh dropped the import deep_ep assert and now asserts ncclGetLsaDevicePointer on the winning libnccl.so.2 rather than the path.
• Benchmark — 5× requests/level, successes-only percentiles, ignore_eos, per-request unique prefix, 200-without-usage counted as a failure, exit 0 only if ok == n at every level, and benchmark.sh defers to that exit code. The published tables are labelled as measured with the old one-shot probe and not byte-reproducible under the new one.
• Reproducibility — one image name from setup/env_vars, wheel filename derived from the per-commit index so bumping VLLM_SHA alone is sufficient, dated tag instead of latest, --disable-ngc explicit, ninja baked, no --break-system-packages anywhere, set -euo pipefail + fatal empty-LEADER_IP guard, Guaranteed QoS, SERVE_DP_LOCAL driving the worker start-rank.

Two things I checked rather than assumed, both from your findings:

1. /opt/nvidia/nvidia_entrypoint.sh does exist on nvcr.io/nvidia/cuda:13.0.0-devel-ubuntu22.04 — your NGC auto-detect finding reproduces on the base, which is why --disable-ngc is now explicit rather than relying on the normal path.
2. The probes I added in response to your readiness comment depend on pgrep, which Layer 1 never installs. procps turns out to be in the base image, and I tested the worker probe three ways in a clean container: exit 1 with no process, exit 0 with a vllm serve argv alive, exit 1 again after it dies — no self-match against the probe's own shell. Also confirmed set -e doesn't break the DNS retry loop (false && break in a loop body doesn't trip it) while the empty-LEADER_IP guard still fires.

Still open and yours to call: EP_EFA_MAX_QPS=2 stays the default so the sample reproduces its own tables, with your +29%/−23% B200 datapoint recorded in serve.sh as a tuning note. I can't produce the p5en A/B this round — those pods are down — so I'd rather leave the knob at the measured value than ship a number I haven't taken. Branch is 1 commit behind main; happy to rebase whenever it's convenient for you. Ready for re-review.

[KeitaW]

Review Batch 1/4 — Response-Round Scoreboard & Two Defects Introduced by the Fixes

This is an exemplary response round: three commits (5d133f7, eb72b3f, 0e5f902) resolving all 28 inline findings from round 1, each reply citing its commit. I verified every one against the code rather than the replies, and they all hold — the kernel-test launcher, the exit-code gates, the probe methodology (successes-only percentiles, ignore_eos, unique prompt prefixes, 5× requests per level), the single image name across all five surfaces, the derived wheel URL, --disable-ngc, the baked ninja, Guaranteed QoS, and the honest relabeling of the benchmark tables all check out. The 153-request reconciliation is arithmetically consistent (121 + 31 warm-up + 1 coherence check).

Of the round-1 body findings, the warmup point is partially covered by the 5× multiplier, and the docs now route all HTTP through the leader pod by name (which defuses the Service-port-8000 round-robin in every documented path — a one-line comment on the Service would close the residual trap for anyone who targets the Service name directly). Four smaller body notes are still open; they're carried in batch 4.

Two of the fixes introduced new defects, both in the manifest — the two comments in this batch. Nothing here rises to must-fix, and none of it invalidates the response round.

[KeitaW]

Review Batch 2/4 — Verification Gates & Benchmark Plumbing

The gates are much stronger than round 1's — these are the remaining seams, all small.

[KeitaW]

Review Batch 3/4 — Pinning & Config Surface

The pin discipline remains the sample's strongest suit (see batch 4). These close the last gaps between the doctrine and the mechanics.

[KeitaW]

Review Batch 4/4 — Carried-Forward from Round 1, Positives & Sources

Four round-1 body-level notes are still open. Each is a small, bounded edit; none blocks.

Remaining small notes, grouped

Three leftovers from round 1's smaller notes, none load-bearing: capabilities.add: [IPC_LOCK] is still redundant under privileged: true (harmless; if it's kept deliberately to support the drop-privileged path the header describes, a three-word comment would say so); the work emptyDir's sizeLimit: 900Gi still draws on node ephemeral storage with no pointer to where that lands on the target AMI; and the probe still has no explicit unmeasured warmup request — the 5× multiplier dilutes first-request initialization at conc=1 to one observation in five, which is much better than round 1, so consider this one mostly settled and purely optional.

Things That Look Great

I re-verified the response round item-by-item against the code, not the replies — these are checked.

The response round itself is the model for this repo. Three focused commits, 28 of 28 inline findings resolved, every reply citing its commit, and several fixes going beyond the ask: the exit gate landed as ok == n at every level (stronger than the suggested ok == conc), the wheel-URL fix derives the filename from the per-commit index rather than just documenting the coupling (I re-derived it live: the index href resolves to the exact pinned wheel, HTTP 200, 274 MB — verified 2026-08-19), and the 153-request discrepancy was reconciled with a full arithmetic breakdown rather than a hand-wave — one that checks out (121 + 31 + 1, and every table row's tok/s × wall ≈ n × 128).

The benchmarks README now says exactly what the numbers are. The old probe's three methodology limits are disclosed as caveats on the published tables instead of silently retrofitted, the "per-stream latency floor, not a throughput ceiling" framing is front and center, and the new probe enforces what the old one assumed (ignore_eos, unique prefixes, successes-only accounting, 200-no-usage as a counted failure). The percentile function is a correct linear interpolation. This is now the most honest benchmarks doc in the vLLM tree.

The probe and gate rewrites are real fixes, not appeasements. The kernel test now launches one torchrun per node with the fan-out delegated to the test's own spawner, gates on the exit code, and accepts only provider-specific EFA banners; verify-image.sh asserts what the image actually stages and discriminates the NCCL downgrade by symbol rather than path. The reasoning is written into the scripts as comments, so the next editor knows why each gate has the shape it has.

The leader probe + publishNotReadyAddresses pairing is exactly right — the rendezvous-vs-readiness interaction round 1 worried about is handled, with the reasoning documented in the manifest. (The worker half needs the batch-1 fix, but the structure is correct.)

The pin discipline held through the response round. --disable-ngc matches the sibling samples and is a real flag doing what the comment says (verified in the 1.49.0 installer source: auto-detect defaults on, --disable-ngc is the supported off switch — 2026-08-19); ninja is baked with the air-gap rationale written down; the immutable tag default (v1-20260818) documents the :latest+IfNotPresent trap it replaces; and the new deep_ep/buffers/elastic.py assertion in verify-image.sh targets a path that really exists at the pinned DeepEP SHA (verified live, 2026-08-19).

Sources

Upstream code and PRs

• aws-ofi-nccl PR #1351 (pinned c2e773d; head now 63698ea), aws/aws-ofi-nccl#1351
• DeepEP PR #612 (pinned = current head 28d1f7fb), deepseek-ai/DeepEP#612
• DeepEP deep_ep/buffers/elastic.py at the pinned b306af06, https://github.com/deepseek-ai/DeepEP/tree/b306af06afd412c88e51e71802951606e40b7358/deep_ep/buffers
• EFA installer 1.49.0 (efa_installer.sh: DISABLE_NGC default 0, --disable-ngc documented off switch), https://efa-installer.amazonaws.com/aws-efa-installer-1.49.0.tar.gz

Verified live, 2026-08-19

• pgrep -af 'vllm serve' invoked via bash -c → matches its own parent shell (/bin/bash -c pgrep -af 'vllm serve' …) on a host with no vLLM process; the bracketed pgrep -f '[v]llm serve' form → no match from the same wrapper
• bash -c 'set -euo pipefail; LEADER_IP=$(getent hosts nonexistent.invalid | awk … | head -1); test -n …' → exits 2, FATAL line never prints; with || true inside the substitution → FATAL prints, exits 1
• seq 1 200000 | grep -q 5 under set -o pipefail → exit 141 (SIGPIPE)
• gh api repos/aws/aws-ofi-nccl/pulls/1351 → head 63698ea ≠ pinned c2e773d; git fetch origin c2e773df… (direct SHA fetch) succeeds against GitHub
• gh api repos/deepseek-ai/DeepEP/pulls/612 → head 28d1f7fb = the pin (still additive)
• gh api repos/NVIDIA/gdrcopy/commits/v2.5.2 → c91ad9f178e5 (the tag's current commit)
• Wheel index for e2f993dc4 → href ../../e2f993dc4…/vllm-0.22.1rc1.dev283%2Bge2f993dc4-cp38-abi3-manylinux_2_28_x86_64.whl, resolves HTTP 200, 274,075,589 bytes
• benchmarks/README.md arithmetic recomputed: 121 = 1+8+16+32+64; 153 = 121+31+1; per-row tok/s × wall ≈ n × 128; all five delta percentages recompute correctly

[KeitaW]

Review Batch 1/5 — Aligning the environment with the canonical deepep-v2-benchmark

Since round 2, main gained a canonical DeepEP-V2 / NCCL-GIN asset: micro-benchmarks/expert-parallelism/deepep-v2-benchmark (PR #1234, merged 2026-08-24), carrying setup_deepep_gin.sh and a reference deepep.Dockerfile. This sample should be built on that asset rather than beside it — the environment setup needs to align with it.

Its README states the intended consumption pattern in almost exactly this sample's terms: the script installs DeepEP V2 "into your container or environment (for example a vLLM image that already ships a DeepEP)", invoked as ./setup_deepep_gin.sh --nccl-root /path/to/gin-capable/nccl. That is this sample's use case verbatim.

Before the asks, the carve-outs — a wholesale adoption would break this sample, and I want to be precise about which canonical constraints do not transfer. The canonical measures EFA-GDA (NCCL_GIN_TYPE=5, GPU-initiated); this sample deliberately uses the CPU-proxy backend (NCCL_GIN_TYPE=2), and the canonical README says the CPU-proxy path "has no such floor." So these canonical requirements are not binding here: EFA installer ≥ 1.50 / libfabric ≥ 2.5, the host EFA driver ≥ 3.3.0 (efadv_create_comp_cntr), and the ncclGinPlugin_v14 export gate. Nor is the canonical's aws-ofi-nccl pin adoptable: v1.21.1 does not contain the OFI_NCCL_GDRCOPY_FORCED_PCIE_COPY parameter this sample depends on — aws-ofi-nccl #1351 is still open/unmerged (verified live, 2026-08-25), so the cherry-pick must stay. Please keep all four of those as they are.

[KeitaW]

Review Batch 2/5 — The vLLM pin bump: the docs say unblocked, the code still refuses

c0259466 bumps VLLM_SHA to 14617c2b, which I verified is exactly the merge commit of vLLM #52632 (merged 2026-08-20) — the fix this sample has been waiting on. The claim is sound and the READMEs were updated. But the bump only landed in the docs: the runtime still enforces the old pin's policy, and the sample's headline new capability is blocked by its own gate.

What run demonstrates default compilation at the new pin?

The eager/non-eager table now states default compilation "works without --enforce-eager" at this pin, and the round-2 comment refers to 14617c2b as "the E2E-verified pin", but I couldn't find a posted run at it — the sweeps in benchmarks/ are explicitly historical (old pin, then-unmerged guard). Given the version jump, could you say what was actually exercised: a full serve, or the import/preflight path? Two things I checked that support the bump and narrow what's left to verify — every flag serve.sh passes (--all2all-backend, --data-parallel-start-rank, --data-parallel-size-local, --enable-expert-parallel, --data-parallel-backend) still exists in arg_utils.py at 14617c2b, DeepEPV2All2AllManager is still at the path the preflight imports, and the wheel index resolves (HTTP 200) for that SHA (all verified live, 2026-08-25). So the remaining question is runtime behaviour, not the interface. If a fresh non-eager run isn't practical right now, softening to "expected to work at this pin; not yet re-measured" would carry the same information honestly.

[KeitaW]

Review Batch 3/5 — Where my round-2 fixes landed but didn't finish the job

Three of the round-2 fixes are correct as far as they go and still leave the original problem standing. I'd rather flag these than let the checkmarks imply more than they do — in each case the fix addressed the mechanism I named while the property I was actually asking for is still unmet.

[KeitaW]

Review Batch 4/5 — Strict-mode and residual correctness

[KeitaW]

Review Batch 5/5 — Round-2 verification, Positives & Sources

Things That Look Great

Round 2 was resolved completely, and I checked the code rather than the replies. All 14 findings are fixed at 2abb4e4d — each addressing the mechanism I named, though three of them leave the underlying property still unmet (batch 3): the bracket trick on both probe branches, || true on the DNS assignment so the FATAL guard is reachable, the draining grep -c form on all three image checks, ${3:?} plus a case role-guard in run-kernel-test.sh, os.environ.get("SERVE_MODEL", …) in the probe, OUT_ROOT=/work/benchmarks on the in-pod exec, direct-SHA fetches for both PR pins, gdrcopy pinned to c91ad9f, the /opt-correct CMD banner, the trust-remote-code rationale, chmod 755 on the two scripts, and the three carried-forward doc items. Two of those went beyond the ask — the role-guard was volunteered as a sibling to serve.sh, and serve.sh picked up the same treatment.

The ARG parse trap you caught pre-push is a genuinely good find, and the mechanism is sharper than the note suggests. I reproduced it: a plain trailing comment after ARG parses fine on both builders, but a comment containing == fails with dockerfile parse error: ARG names can not be blank — because ARG accepts multiple space-separated names and == tokenizes to one with an empty name. That's why the sample built for months with four other inline ARG comments and only the gdrcopy one broke. I ran docker build --check against the shipped Dockerfile and it's clean, so the remaining inline comments are safe. Worth keeping the full-line-comment habit anyway, since the trap is invisible until it fires. (An independent pass flagged all inline ARG comments as build-breaking; that's the over-general version of your finding, and the empirical check rules it out.)

The pin-bump reasoning checks out where it can be verified. 14617c2b really is #52632's merge commit (merged 2026-08-20), the wheel index resolves for it, and — the part that's easy to get wrong — the shared-experts claim holds: vLLM #47785 is an ancestor of this pin, so the README's "any pin past #52632 also includes #47785" is correct rather than merely plausible (verified live, 2026-08-25). The serve.sh preflight that imports DeepEPV2All2AllManager is also exactly the right gate to have kept across a four-minor-version jump.

The verification scripts now discriminate. After round 2 the image checks assert the winning libnccl by symbol, the EFA banner by provider-specific pattern, and the staged DeepEP by ElasticBuffer class — and all of them drain their producers. That combination is what makes "ALL CHECKS PASS" mean something, and it's better than the canonical benchmark's own equivalent in one respect: it fails loudly on a partially-correct image rather than on a missing one.

Sources

Canonical asset (the alignment target)

• micro-benchmarks/expert-parallelism/deepep-v2-benchmark (PR #1234, merged 2026-08-24), https://github.com/awslabs/awsome-distributed-ai/tree/main/micro-benchmarks/expert-parallelism/deepep-v2-benchmark
• setup_deepep_gin.sh — "for example a vLLM image that already ships a DeepEP"; --nccl-root gate on include/nccl_device.h
• deepep.Dockerfile — ENV NCCL_SOCKET_IFNAME=^docker,lo,veth; CUDA 13.1.2 default; the ptxas 13.0.88 / st.bulk note
• Repo IFNAME convention: https://github.com/awslabs/awsome-distributed-ai/blob/main/micro-benchmarks/nccl-tests/nccl-tests.Dockerfile

Upstream

• vLLM #52632 (merge commit 14617c2b, merged 2026-08-20), vllm-project/vllm#52632
• vLLM #47785, vllm-project/vllm#47785
• aws-ofi-nccl #1351 — still open, aws/aws-ofi-nccl#1351
• amazon-contributing/DeepEP #3 (st.bulk 64-bit operand, merged 2026-08-24), amazon-contributing/DeepEP#3

Verified live, 2026-08-25

• gh api repos/vllm-project/vllm/commits/14617c2b… → "[Bugfix] DeepEP-V2: expert_tokens_meta must be None … (#52632)"; pulls/52632.merge_commit_sha = 14617c2b…
• compare/14617c2b…{#47785 merge} → ahead_by: 0 (⇒ #47785 is an ancestor of the pin)
• gh api repos/aws/aws-ofi-nccl/pulls/1351 → merged: false; nccl_ofi_param.h @ v1.21.1 → 0 occurrences of GDRCOPY_FORCED_PCIE_COPY
• nvidia_nccl_cu13-2.30.4 wheel → ships include/nccl_device.h + nccl_device/gin/proxy/gin_proxy.h
• vLLM arg_utils.py @ 14617c2b → all five serve.sh flags present; all2all.py → class DeepEPV2All2AllManager present
• Wheel index for 14617c2b → vllm-0.26.1rc1.dev1000+g14617c2b6-…x86_64.whl, HTTP 200
• ARG V=x # plain comment builds; ARG V=x # comment with == → dockerfile parse error: ARG names can not be blank (both BuildKit and legacy, Docker 29.7.2); docker build --check on the shipped Dockerfile → "Check complete, no warnings found"
• set -euo pipefail; V="$(ls -d <existing> <no-match-glob> 2>/dev/null | head -1)" → exits 2, recovery line never reached
• NCCL_SOCKET_IFNAME / NCCL_GIN_PLUGIN / nccl_device / amazon-contributing → 0 occurrences each in the PR diff

[dmvevents]

Re-review requested — head is now 8787c510 (3 changes since the round-2 wrap above)

Since the round-2 comment (2abb4e4d, all 14 threads addressed), three more changes landed, so I'm re-requesting a look:

• Migrated to examples/inference/vllm/deepep-v2-efa/ (5f9f6ec3) to follow the examples/inference/<engine>/<name>/ reorg (Reorg: de-number dirs + restructure examples (Option B); retain HyperPod LifecycleScripts paths #1119). This is why the paths in the older inline comments have moved — the file contents are unchanged by the move, only the location.
• Added .dockerignore (a0bfd21e) so setup/env_vars (which can hold an HF token) never enters the build context.
• recipe/run-kernel-test.sh now sets NCCL_SOCKET_IFNAME (51f56aa8) so the README's "VERBATIM from serve.sh" claim is actually true.

npx markdownlint-cli2 is clean on the moved tree, and there is no functional change to the DeepEP-V2 / EFA transport since round 2. Thanks again for the thorough round-2 pass, @KeitaW.

[paragao]

since the release of EFA installer 1.50.0, DeepEP V2 is natively supported on AWS EFA. Please, review this submission the light of that new support. If this PR is obsolete, then please cancel it.

Support for DeepEP V2: https://github.com/amazon-contributing/DeepEP

This is the official Dockerfile for native support of DeepEP V2 on EFA: https://github.com/awslabs/awsome-distributed-ai/blob/main/micro-benchmarks/expert-parallelism/deepep-v2-benchmark/README.md

And check this requirements for making sure NCCL GIN will work: https://github.com/aws/aws-ofi-nccl/blob/master/doc/gin-getting-started.md

[paragao]

bump this version to 1.50.0 and review the whole Dockerfile to make sure it is aligned with the new native support of DeepEP V2 on the EFA Installer.

[dmvevents]

Done in 5c5584a. EFA_INSTALLER_VER is now 1.50.0, and the whole Dockerfile is aligned with the bundled plugin: the from-source aws-ofi-nccl build is gone (Layer 4's AWS_OFI_NCCL_* args deleted), and the image consumes the installer's aws-ofi-nccl 1.21.1 at /opt/amazon/ofi-nccl/lib. The installer layer now gates fail-loud on the bundled plugin's ncclGinPlugin_v14 export — the same gate the canonical deepep-v2-benchmark adopted when it made this exact migration in #1239 — plus a check that the plugin was compiled with gdrcopy support.

One dependency this drops deliberately: the gdrdrv-2.4 forced-PCIe override (aws/aws-ofi-nccl#1351) this sample used to cherry-pick. Upstream declined that PR (gdrcopy 2.4.x has silent-data-corruption issues; their guidance is to move hosts to 2.5), so the README now documents host gdrcopy/gdrdrv >= 2.5 as a prerequisite instead of carrying a rejected workaround.

Verification level, stated plainly: this change is verified at build + symbol level — a clean docker build of the updated Dockerfile passes the new gates (plugin present, ncclGinPlugin_v14 exported, gdrcopy compiled in, libfabric 2.6.0amzn1.0), and the same was confirmed on a bare ubuntu:22.04 install of installer 1.50.0 with the Dockerfile's exact flags. The 2-node EFA E2E (kernel smoke + serve + sweep) has not been re-run on the bundled plugin yet; benchmarks/README.md now records that the published tables were measured on the previous source-built plugin and that a rebuild runs a different plugin build. That E2E re-validation is the named remaining step.

[paragao]

with the new EFA Installer 1.50.0 you don't need to build aws-ofi-nccl to supoprt DeepEP V2 anymore. The supported version is using EFA Installer 1.50.0.

[dmvevents]

Confirmed from primary sources, and fixed in 5c5584a — with one nuance worth recording for the next reader.

Verified: installer 1.50.0 bundles aws-ofi-nccl 1.21.1 (ChangeLog: "Upgrade to OFI NCCL Plugin 1.21.1"), and GIN support went GA in aws-ofi-nccl v1.21.0 (2026-08, host-proxy mode + EFA-GDA kernel backend). Symbol check on the bundled libnccl-ofi_1.21.1-1_amd64.deb: exports ncclGinPlugin_v11/_v13/_v14, compiled with gdrcopy support. So the from-source build is deleted from setup_deepep_v2_efa.sh; the script now only stages the pinned DeepEP-V2 source, and the plugin comes from the installer (gated in the Dockerfile on ncclGinPlugin_v14) — the same migration the canonical deepep-v2-benchmark made in #1239.

The nuance: the old source pin existed for a reason that has since been resolved against us, not silently fixed. It carried the gdrdrv-2.4 compat pieces (the aws/aws-ofi-nccl#1351 forced-PCIe param + the gdr_pin_buffer v1 fallback), which are not in 1.21.1 — upstream declined #1351 on 2026-08-21 (gdrcopy 2.4.x silent-data-corruption; "customers should update to 2.5 to use the GIN interface"). We agree that's the right call for a public sample, so the sample now requires host gdrcopy/gdrdrv >= 2.5 (README prerequisite + a node-side check command) instead of working around 2.4.

Verification level: build + symbol proven (clean docker build passes the gates); the 2-node EFA E2E has not been re-run on the bundled plugin — that is the remaining step, and the benchmarks provenance says so explicitly.

[paragao]

review this whole file in light of the new EFA Installer 1.50.0 native DeepEP V2 support. If the file is still needed, consider this being part of the "setup_deep_ep.sh" you have. From an user perspective, this is quite confusing as you are "building" DeepEP in multiple points.

[dmvevents]

Half fixed, half explained in 5c5584a — and the fix removes exactly the confusion you're pointing at.

Fixed: with the plugin now coming from the EFA installer (see the thread on setup_deepep_v2_efa.sh), the docker-build-time script no longer builds anything — it only stages pinned DeepEP-V2 source. So there is now exactly one build of DeepEP in the whole recipe: recipe/build_deepep.sh, in-pod on first boot.

Why that file can't fold into setup_deepep_v2_efa.sh: the two run at different lifecycle stages. setup_deepep_v2_efa.sh runs inside docker build, where there is no GPU and no CUDA context; the DeepEP _C.so compile needs a live CUDA context (and the exact torch ABI of the running pod), so it has to run at first boot. The headers of both scripts and the Dockerfile's Layer-4 comment now state this split in one line each ("this script only STAGES source — the one DeepEP BUILD is in-pod"), so the next reader doesn't have to reverse-engineer it.

[paragao]

Add a comment that these are just sample numbers the user can expect. They are not absolute "best/top" numbers. Each user experience can vary depending on the variables of their environment. Or just remove absolute numbers and add percentages to represent the scaling out.

[dmvevents]

Done in 5c5584a — both halves of your suggestion, keeping the measured numbers for provenance:

1. A prominent "Sample numbers, not absolutes" note under ## Results (plus a one-line reminder at each table): these are one measured sweep from the specific environment in the provenance table, what a user can expect to see in a comparable setup — not best/top numbers — and they will vary with driver/firmware, pin set, model, prompt shape, and cluster state.
2. A scaling column on both sweep tables giving the relative view you asked for — the multiple vs conc=1 and the % of ideal linear scaling, derived arithmetically from the numbers already in the table (e.g. eager c=64: 301.8 / 4.8 = 62.9× = 98.2% of ideal). No new measurements were invented; every percentage is computable from the committed values.

The absolutes stay because they are the provenance record of what this recipe was proven to do (and the raw JSONL they came from); the caveat + percentages now carry the "your numbers will differ" message.

[paragao]

Add a comment that these are just sample numbers the user can expect. They are not absolute "best/top" numbers. Each user experience can vary depending on the variables of their environment. Or just remove absolute numbers and add percentages to represent the scaling out.

[dmvevents]

Same fix as the line-53 thread, applied in 5c5584a: the "Sample numbers, not absolutes" note covers this table too, and it now carries its own derived scaling column (multiple vs conc=1 + % of ideal linear, computed from the committed values — e.g. non-eager c=64: 239.7 / 5.0 = 47.9× = 74.9% of ideal). Measured absolutes kept for provenance.

[paragao]

Add a comment that these are just sample numbers the user can expect. They are not absolute "best/top" numbers. Each user experience can vary depending on the variables of their environment. Or just remove absolute numbers and add percentages to represent the scaling out.

[dmvevents]

Same fix as the line-53 thread, applied in 5c5584a: this comparison table was already relative (the delta % column), and it now carries the single-environment caveat explicitly — one sweep per mode, deltas are datapoints not universals (the c=64 divergence was reproduced once), matching the "Reading" guidance below it.

[dmvevents]

Round 3 addressed — head is now 8d95f609 (fix commit 5c5584ac + a sync merge of upstream/main, 13 commits behind before). Thanks @paragao — the 1.50.0 pointer was right, and it let this sample drop a whole build stage.

Per thread:

• Dockerfile (installer pin) — EFA_INSTALLER_VER bumped to 1.50.0 and the recipe migrated to the installer-bundled aws-ofi-nccl 1.21.1 at /opt/amazon/ofi-nccl/lib, gated fail-loud on its ncclGinPlugin_v14 export + a gdrcopy-compiled-in check — the same migration the canonical deepep-v2-benchmark made in fix(deepep-v2): use the EFA installer bundled aws-ofi-nccl plugin #1239.
• setup_deepep_v2_efa.sh — the from-source aws-ofi-nccl build (and its gdrcopy: add OFI_NCCL_GDRCOPY_FORCED_PCIE_COPY override for the forced-PCIe capability probe aws/aws-ofi-nccl#1351 cherry-pick, since declined upstream) is deleted; the script now only stages pinned DeepEP-V2 source. Host gdrcopy/gdrdrv >= 2.5 is now a documented prerequisite (upstream's supported pairing for GIN).
• recipe/build_deepep.sh — kept, and it is now the single DeepEP build point in the recipe. It cannot fold into the docker-build-time script: the _C.so compile needs a live CUDA context (and the running pod's torch ABI), so it runs at first boot. Both script headers + the Dockerfile Layer-4 comment now state the split explicitly.
• benchmarks/README.md (3 threads) — added the "sample numbers, not absolutes" caveat (top note + per-table reminders) and derived scaling columns (multiple vs conc=1, % of ideal linear), every value computed from the numbers already committed; measured absolutes kept as provenance.

Also swept along: setup/env_vars.example + setup/build-push.sh lose the now-removed AWS_OFI_NCCL_PR_SHA knob, the example image tag is bumped to v3-20260916 (IfNotPresent caches by tag), and every NCCL_NET_PLUGIN/LD_LIBRARY_PATH reference now points at /opt/amazon/ofi-nccl/lib.

Verification level, stated plainly: this round is verified at build + symbol level — a clean docker build of the updated recipe passes all gates (ncclGinPlugin_v14 present, gdrcopy compiled in, libfabric 2.6.0amzn1.0, and the Layer-5b nvidia-nccl-cu13==2.30.4 re-pin asserts), and a bare ubuntu:22.04 run of installer 1.50.0 with the Dockerfile's exact flags confirms the bundled plugin independently. The 2-node EFA E2E (kernel smoke + serve + sweep) has not been re-run on the bundled plugin — the benchmarks provenance table records that the published tables were measured on the previous source-built plugin. That E2E re-validation is the named remaining step; I'll run it when cluster capacity frees up and update here.

[dmvevents]

@KeitaW — ready for another look when you have time. Head is 8d95f6094.

All 58
8 of your threads on this one are answered in-thread with the commit that addresses each. Flagging it here because my earlier round summary didn't @-mention you, so I don't think it ever resurfaced in your review queue — and as an outside contributor I can't re-request review directly (the API returns 404 without push access).

Nothing is failing on my side: no merge conflict, and the BLOCKED state comes with 0 check-runs, which I read as this repo's CI not triggering for outside contributors rather than a real failure. Happy to rebase or split anything if that's easier to review.