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

[dmvevents]

Adds 3.test_cases/pytorch/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.

Two integration fixes are baked in + documented: (1) 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 deepseek-ai/DeepEP#727); (2) the gdrcopy forced-PCIe capability via aws-ofi-nccl#1354. Non-eager needs vLLM #46404+#46432 (merged) + a one-line meta guard, applied by recipe/apply-noneager-fix-stack.sh.

Follows the sglang exemplar layout + conventions: in-tree setup_deepep_v2_efa.sh (distinctly named — not vendor-synced to the 567632d NVSHMEM setup), ARG-pinned versions (no latest), user-supplied ECR, MIT-0 headers. Draft while I attach the raw benchmark logs + a fresh-pod re-measure.

Draft — feedback welcome, especially on placement + the eager-vs-non-eager framing.

[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

[dmvevents]

Round 2 pushed — 2abb4e4d (all 14 threads addressed)

Thanks for the thorough round-2 pass, @KeitaW — every comment reproduced cleanly on my side. Pushed as head 2abb4e4d; each thread has an in-line reply naming the exact fix. Summary:

Correctness (probes/guards/scripts):

• Worker probe tautology → [v]llm serve bracket on both startup + readiness probes
• DNS FATAL guard unreachable → || true on the substitution so the guard fires
• grep -q SIGPIPE-141 flake → draining grep -c form on all three checks (matches Layer 5b)
• Worker node-rank 0 collision → ${3:?…} + a case role-guard sibling to serve.sh
• Probe hardcoded model → os.environ.get("SERVE_MODEL", …)

Reproducibility / pins:

• PR pins fetched via moving ref → git fetch origin ${…_PR_SHA} direct-by-SHA (both aws-ofi-nccl + DeepEP)
• gdrcopy movable tag → ARG GDRCOPY_SHA=c91ad9f… (v2.5.2), fetched+checked-out by SHA — the whole sample is now SHA-pinned with no exception
• benchmark → ephemeral fs → env OUT_ROOT=/work/benchmarks in the exec command

Docs / hygiene: CMD banner → /opt/… paths · env_vars.example header + dropped the no-op disable hint · trust-remote-code rationale comment · chmod 755 on the two 644 scripts · no-alternative-backend caveat (honest: the alt sweep isn't practical with the pods down) · hugepages pre-alloc note · build_deepep.sh tee for failed-build diagnostics.

One note in the spirit of your live-repro discipline: pinning gdrcopy by commit surfaced a follow-on I caught pre-push — an inline # … == comment after the ARG line breaks docker build at parse time (ARG names can not be blank, since Docker re-tokenizes the rest of an ARG line rather than treating # as a comment). Moved the note to a full-line comment above the ARG; verified docker build parses clean (rc=0) on Docker 29.6.1 both BuildKit and legacy. Layer 5/5b (VLLM_SHA=14617c2b…, the E2E-verified pin) is untouched.

[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