-
Notifications
You must be signed in to change notification settings - Fork 207
feat(deepep-efa): TensorRT-LLM NcclEP MoE all-to-all over EFA (NCCL-GIN CPU-proxy) #1240
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
dmvevents
wants to merge
11
commits into
awslabs:main
Choose a base branch
from
dmvevents:feat/trtllm-deepep-efa
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
bf5e5a1
feat(deepep-efa): TensorRT-LLM NcclEP MoE all-to-all over EFA (NCCL-G…
dmvevents 458eac1
fix(deepep-efa): honor pre-set APPLY_HT_FLAT_PATCH, add TRT-LLM engin…
dmvevents 558cd52
trtllm/nccl-ep-efa: migrate build to released aws-ofi-nccl v1.21.1 + …
dmvevents dc7120f
trtllm/nccl-ep-efa: RUN_SERVE gate, stable .svc DNS rendezvous, ephem…
dmvevents aafe187
trtllm/nccl-ep-efa: recipe robustness — fail-loud NCCL_LIB, collectiv…
dmvevents e8494dd
trtllm/nccl-ep-efa: docs — pins/diagram consistency, gdrdrv prereq, .…
dmvevents c813862
Merge remote-tracking branch 'upstream/main' into feat/trtllm-deepep-efa
dmvevents 71b43b2
refactor(inference): migrate tensorrt-llm/nccl-ep-efa to examples/inf…
dmvevents 596f7f4
fix(trtllm/nccl-ep-efa): EFA-installer 1.50 compat + record the 2.31.…
dmvevents ee0b062
fix(trtllm/nccl-ep-efa): guard probe's NcclEpContext._ep_algorithm (a…
dmvevents b1e44e5
docs(trtllm-nccl-ep-efa): record the concrete ABI reason for the NCCL…
dmvevents File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| <!-- | ||
| Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| SPDX-License-Identifier: MIT-0 | ||
| --> | ||
|
|
||
| # TensorRT-LLM test cases | ||
|
|
||
| [TensorRT-LLM](https://github.com/NVIDIA/TensorRT-LLM) is NVIDIA's open-source | ||
| inference engine for large language models on NVIDIA GPUs. The samples in this | ||
| directory deploy TensorRT-LLM on AWS with high-performance EFA networking and | ||
| expert-parallel MoE all-to-all. | ||
|
|
||
| ## Available test cases | ||
|
|
||
| | Test case | Orchestrator | Description | | ||
| | --- | --- | --- | | ||
| | [`nccl-ep-efa`](./nccl-ep-efa) | Kubernetes (2-node) | Wide-EP MoE dispatch/combine via TRT-LLM's **`NcclEP`** backend (`nccl.ep` / `libnccl_ep` — NOT the `deep_ep` package) over **AWS EFA**, using aws-ofi-nccl with **GIN** (GPU-Initiated Networking) CPU-proxy. Image built NGC-from-scratch from public sources; the recipe runs image build → transport smoke test → served `/v1/chat/completions` → concurrency benchmark. Validated on `p5en.48xlarge` (H200). | | ||
|
|
||
| For kernel-level expert-parallelism dispatch/combine benchmarks over EFA — | ||
| including a DeepEP V2 benchmark on the same NCCL-GIN substrate this test case | ||
| uses — see | ||
| [`micro-benchmarks/expert-parallelism`](../../../micro-benchmarks/expert-parallelism). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. SPDX-License-Identifier: MIT-0 | ||
| setup/env_vars | ||
| benchmarks/raw/ | ||
| *.log |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,186 @@ | ||
| # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. SPDX-License-Identifier: MIT-0 | ||
| # | ||
| # TensorRT-LLM + NcclEP MoE all-to-all over AWS EFA (NCCL-GIN CPU-proxy). | ||
| # TRT-LLM's NcclEP backend rides `nccl.ep` (nccl4py), which needs NCCL >= 2.30.4 and a | ||
| # GIN-capable network plugin — neither of which the NGC TRT-LLM container ships. This image | ||
| # adds, from PUBLIC sources only: EFA userspace, gdrcopy, the pinned NCCL + nccl4py pair, | ||
| # and aws-ofi-nccl's GIN plugin (built by setup_trtllm_nccl_ep_efa.sh — COPY'd, not curled, | ||
| # so it is in-tree + reviewable). | ||
| # | ||
| # setup/build-push.sh builds + pushes ${REGISTRY}/${IMAGE_NAME}:${IMAGE_TAG} (setup/env_vars); | ||
| # manual equivalent: DOCKER_BUILDKIT=1 docker build -t <registry>/trtllm-nccl-ep-efa:<tag> . | ||
| # | ||
| # Base pin rationale (GA-over-prerelease exception, documented): GA v1.2.1 does NOT contain | ||
| # the NcclEP backend at all (tensorrt_llm/_torch/modules/fused_moe/nccl_ep_utils.py is absent | ||
| # at that tag), so a 1.3.0rc pin is REQUIRED, not a preference. 1.3.0rc24 is the first NGC | ||
| # release container whose factory ships the NCCL_EP arm natively (TRTLLM_FORCE_COMM_METHOD); | ||
| # re-test and re-pin when a GA that carries the backend appears. | ||
| ARG TRTLLM_BASE=nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc24 | ||
| FROM ${TRTLLM_BASE} | ||
| ARG TRTLLM_BASE # re-declare: pre-FROM ARGs go out of scope after FROM (used in Layer 6 diagnostics) | ||
|
|
||
| LABEL org.opencontainers.image.description="TensorRT-LLM + NcclEP MoE all-to-all over AWS EFA (NCCL-GIN CPU-proxy)" | ||
| LABEL org.opencontainers.image.licenses="MIT-0" | ||
| LABEL org.opencontainers.image.source="https://github.com/awslabs/awsome-distributed-ai" | ||
| ENV DEBIAN_FRONTEND=noninteractive | ||
| SHELL ["/bin/bash", "-c"] | ||
| USER root | ||
|
|
||
| # ---- Layer 1: system + build deps ------------------------------------------- | ||
| # libevent-{core,pthreads} are prrte-aws deps the EFA installer needs; do NOT purge | ||
| # /var/lib/apt/lists here — the installer below runs its own apt-get install and fails | ||
| # with "held broken packages" against empty lists. Purged at the end of Layer 2 instead. | ||
| RUN apt-get update && apt-get install -y --no-install-recommends \ | ||
| build-essential autoconf automake libtool pkg-config git curl wget ca-certificates \ | ||
| libnuma-dev libhwloc-dev libudev-dev \ | ||
| libevent-core-2.1-7t64 libevent-pthreads-2.1-7t64 \ | ||
| pciutils environment-modules tcl udev dmidecode ethtool iproute2 kmod | ||
|
|
||
| # ---- Layer 2: AWS EFA (public installer) ---- | ||
| # 1.50.0 is the current EFA userspace and what the sibling deepep-v2-benchmark builds; its | ||
| # tarball resolves (verified) and it ships aws-ofi-nccl 1.21.1 in-box, lining this sample up | ||
| # with the rest of the repo. BUILD-PIN only: the 2026-08-07 correctness E2E (real | ||
| # trtllm-serve HTTP-200-correct + 16-rank cross-node dispatch/combine, IMA=0, efa-direct on | ||
| # every rank) ran on 1.48.0 — this assembly is not yet cluster-re-measured on 1.50.0, which | ||
| # is exactly what recipe/verify-image.sh + run-kernel-test.sh exist to re-verify on-cluster. | ||
| # --disable-ngc: the NGC base trips the installer's NGC auto-detect, which would silently | ||
| # reroute to the libnccl-ofi-ngc path — we build aws-ofi-nccl from source ourselves | ||
| # (Layer 5), same explicit choice as the sibling vllm/dsv3-uccl-nixl sample (which also | ||
| # passes --disable-ngc alone). Do NOT add --disable-build-ngc: that flag existed in | ||
| # aws-efa-installer 1.48.0 but was REMOVED in 1.49.0+ (the installer no longer auto-detects | ||
| # the build-ngc use case, so the flag became unnecessary and getopt now rejects it — an | ||
| # unknown long-opt makes efa_installer.sh print usage and exit 1, failing this layer). The | ||
| # 2026-08-07 E2E ran on 1.48.0 where both flags were valid; the 1.50.0 bump made the second | ||
| # one a build error, which is why it is dropped here. | ||
| ARG EFA_INSTALLER_VER=1.50.0 | ||
| RUN apt-get update \ | ||
| && curl -fsSL https://efa-installer.amazonaws.com/aws-efa-installer-${EFA_INSTALLER_VER}.tar.gz | tar -xzf - -C /tmp \ | ||
| && cd /tmp/aws-efa-installer \ | ||
| && ./efa_installer.sh -y --skip-kmod --skip-limit-conf --no-verify --disable-ngc \ | ||
| && echo "${EFA_INSTALLER_VER}" > /opt/efa-installer.version \ | ||
| && rm -rf /tmp/aws-efa-installer /var/lib/apt/lists/* | ||
| # Add EFA userspace (libfabric + efa provider) to PATH/LD, but deliberately NOT the EFA | ||
| # installer's bundled OpenMPI (/opt/amazon/openmpi). The NGC TRT-LLM base ships HPC-X OpenMPI | ||
| # (ldconfig default /opt/hpcx/ompi) and tensorrt_llm's MPI_Init resolves libopen-pal through it. | ||
| # The EFA installer's OpenMPI 4.1.7 ships a libopen-pal.so.40 that does NOT export | ||
| # opal_libevent2022_event_assign; prepending /opt/amazon/openmpi/lib here shadows the HPC-X | ||
| # libopen-pal under HPC-X's own mca_ess_hnp.so plugin → undefined symbol → MPI_Init aborts on | ||
| # `import tensorrt_llm`. Neither recipe uses EFA's mpirun (serve.sh is single-node; the probe | ||
| # uses torchrun), so the EFA OpenMPI is unnecessary and its lib on LD is actively harmful. | ||
| ENV PATH=/opt/amazon/efa/bin:$PATH | ||
| ENV LD_LIBRARY_PATH=/opt/amazon/efa/lib:${LD_LIBRARY_PATH:-} | ||
|
|
||
| # ---- Layer 3: gdrcopy userspace (PUBLIC: github.com/NVIDIA/gdrcopy) ---- | ||
| # aws-ofi-nccl's GIN path REQUIRES gdrapi.h at configure time — without it the plugin | ||
| # compiles with "GDRCopy support not available", nccl_ofi_gin_init fails at serve time, | ||
| # and NcclEP's group creation dies. gdrcopy v2.5.2 == commit c91ad9f: pin the commit, not | ||
| # the tag (a bare tag is a moving ref upstream can re-point). | ||
| ARG GDRCOPY_SHA=c91ad9f178e5fb729fc5b6dc62a77c3bb364d6c9 | ||
| RUN git clone https://github.com/NVIDIA/gdrcopy.git /tmp/gdrcopy \ | ||
| && cd /tmp/gdrcopy && git fetch origin ${GDRCOPY_SHA} && git checkout ${GDRCOPY_SHA} \ | ||
| && make prefix=/usr/local lib lib_install && ldconfig \ | ||
| && rm -rf /tmp/gdrcopy | ||
|
|
||
| # ---- Layer 4: NCCL 2.30.4 (over the container's baked NCCL) + nccl4py ---- | ||
| # THE crux: tensorrt_llm's is_nccl_ep_installed() gates on libnccl >= 2.30.4, while the NGC | ||
| # TRT-LLM container bakes an older nvidia-nccl-cu13 — so NcclEP is dead on arrival as | ||
| # shipped. --no-deps so pip does not drag torch/TRT-LLM's pinned dependency graph backwards; | ||
| # we are deliberately overriding exactly one pin. nccl4py 0.3.1 ships the `nccl.ep` python | ||
| # package + libnccl_ep.so 0.1.0 (the version whose HT-kernel int64/int32 ABI detail the README | ||
| # documents). This nccl4py pin is REQUIRED, not merely current: nccl4py 0.4.1 ships no `nccl.ep` | ||
| # package at all (only nccl.bindings + nccl.core), so a bump to latest breaks `import nccl.ep`. | ||
| # Why 2.30.4 and not the newer 2.31.2: 2.31.2 also clears the >= 2.30.4 floor, but libnccl_ep 0.1.0 | ||
| # (shipped by the nccl4py pin above) is built against the NCCL 2.30 device-side API that the GIN | ||
| # CPU-proxy path calls into — and that device API is NOT append-only across 2.30.4->2.31.2. In the | ||
| # device struct libnccl_ep dereferences by pointer (ncclDevComm), 2.31.2 inserts hybridDenseGinBarrier | ||
| # at field 10 (before lsaMultimem) + backendIndex mid-GIN-block, and shrinks the by-value | ||
| # resourceWindow_inlined member (dropped its reserved padding) — each shifts the byte offsets of the | ||
| # GIN fields the CPU-proxy kernels read (verified by diffing nccl_device/impl/impl_comm__types.h at | ||
| # tags v2.30.4-1 vs v2.31.2-1). ncclGinType_t is append-only (PROXY=2 unchanged, +EFA_GDA=5), so the | ||
| # enum is not the issue — the devComm layout is. This is the silent-corruption class, so a build-only | ||
| # check cannot catch it. 2.30.4 is the version the 2026-08-07 correctness E2E actually ran. | ||
| # Two single-variable 2.31.2 trials on cgk p5en (--build-arg NVIDIA_NCCL_CU13=2.31.2) established: | ||
| # (+) LIBRARY ABI is clean on 2.31.2 — is_nccl_ep_installed() passes and the CommunicationFactory | ||
| # selects NcclEP on all 16 ranks (no quiet symbol/binding break). MEASURED. | ||
| # (=) DEVICE dispatch/combine round-trip: NOT YET measured on either arm. First trial's GIN init | ||
| # hard-failed because cgk's host gdrdrv is 2.4 (< the sample's GDRCopy-2.5 prereq); a second | ||
| # trial (2026-09-01) with a throwaway forced_pcie_copy instrument patch DID init GIN on | ||
| # gdrdrv-2.4 and both arms reached NcclEP selection, but crashed one line before the first | ||
| # dispatch on a latent probe-diagnostic bug (recipe/probe_nccl_ep.py referenced a | ||
| # NcclEpContext._ep_algorithm attr absent on rc24 — now getattr-guarded). See VERDICT.md. | ||
| # So the 2.31.2 device-side ABI is NOT yet certified (nor refuted) at the kernel level. Held at | ||
| # 2.30.4 as the measured-matching floor, not an upper bound — to bump it, re-run verify-image.sh + | ||
| # run-kernel-test.sh (probe fix now landed) on a gdrdrv>=2.5 host (or with the instrument patch) so | ||
| # GIN inits and the dispatch/combine round-trip + oracle actually execute on both arms. | ||
| ARG NVIDIA_NCCL_CU13=2.30.4 | ||
| ARG NCCL4PY_VER=0.3.1 | ||
|
dmvevents marked this conversation as resolved.
|
||
| RUN pip3 install --no-cache-dir --no-deps --force-reinstall "nvidia-nccl-cu13==${NVIDIA_NCCL_CU13}" \ | ||
| && pip3 install --no-cache-dir "nccl4py==${NCCL4PY_VER}" \ | ||
| && NCCL_ROOT=$(python3 -c "import nvidia.nccl, pathlib; print(pathlib.Path(nvidia.nccl.__path__[0]))") \ | ||
| && ln -sf "$NCCL_ROOT/lib/libnccl.so.2" /usr/local/lib/libnccl.so.2 \ | ||
| && ln -sf "$NCCL_ROOT/lib/libnccl.so.2" /usr/local/lib/libnccl.so \ | ||
| && echo "$NCCL_ROOT/lib" > /etc/ld.so.conf.d/00-pip-nccl.conf && ldconfig \ | ||
| && [ "$(strings "$NCCL_ROOT/lib/libnccl.so.2" | grep -c "NCCL version ${NVIDIA_NCCL_CU13}")" -ge 1 ] | ||
| # The version assert uses the draining count form, not `grep -q` (SIGPIPE-141 flake under | ||
| # pipefail). Import checks (nccl.ep, is_nccl_ep_installed) live in recipe/verify-image.sh, | ||
| # which runs with GPUs — the build sandbox has none. | ||
|
|
||
| # ---- Layer 5: aws-ofi-nccl GIN plugin (in-tree script, COPY'd not curled) ---- | ||
| # Built from the released tag v1.21.1 (same tag the sibling deepep-v2-benchmark builds): it | ||
| # exports the CPU-proxy GIN op-tables (ncclGinPlugin_v11/_v13) NCCL_GIN_TYPE=2 uses, and its | ||
| # forced-PCIe-with-fallback gdrcopy path is the released default — so no closed-PR cherry-pick | ||
| # and no OFI_NCCL_GDRCOPY_FORCED_PCIE_COPY override are needed. gdrdrv >= 2.5 is a host | ||
| # precondition (README Prerequisites) rather than a private plugin fork. | ||
| COPY setup_trtllm_nccl_ep_efa.sh /opt/setup_trtllm_nccl_ep_efa.sh | ||
| ARG AWS_OFI_NCCL_REF=v1.21.1 | ||
| RUN chmod +x /opt/setup_trtllm_nccl_ep_efa.sh \ | ||
| && AWS_OFI_NCCL_REF=${AWS_OFI_NCCL_REF} /opt/setup_trtllm_nccl_ep_efa.sh | ||
| ENV LD_LIBRARY_PATH=/opt/aws-ofi-nccl/lib:${LD_LIBRARY_PATH} | ||
| # NCCL_GIN_PLUGIN pairs with NCCL_NET_PLUGIN — one .so supplies both the net and GIN tables; | ||
| # set both as ENV (matching the sibling deepep-v2-benchmark) so the image is correct for | ||
| # anyone running it outside the launchers. NVIDIA_GDRCOPY=enabled matches the two sibling | ||
| # GIN images and the non-privileged/device-plugin path the manifest header offers. | ||
| ENV NCCL_NET_PLUGIN=/opt/aws-ofi-nccl/lib/libnccl-net-ofi.so \ | ||
| NCCL_GIN_PLUGIN=/opt/aws-ofi-nccl/lib/libnccl-net-ofi.so \ | ||
| NVIDIA_GDRCOPY=enabled | ||
|
|
||
| # ---- Layer 6 (OPT-IN, default OFF): the HIGH_THROUGHPUT+FLAT selectability patch ---- | ||
| # Upstream's NcclEP hardcodes LOW_LATENCY + RANK_MAJOR. On THIS substrate (NCCL 2.30.4 + | ||
| # nccl_ep 0.1.0 + GIN CPU-proxy) the upstream default runs clean — measured both in a real | ||
| # serve and at 16 ranks cross-node — so the UNPATCHED image is the baseline and this layer | ||
| # defaults OFF. NVIDIA/TensorRT-LLM PR #17715 (open) makes algorithm/layout selectable via | ||
| # TRTLLM_NCCL_EP_ALGO / TRTLLM_NCCL_EP_LAYOUT; build with APPLY_HT_FLAT_PATCH=1 to bake the | ||
| # PR's three change commits (pinned at their immutable SHAs) into the container's site-packages. | ||
| # Fail-loud: if a hunk no longer applies against this base tag, the BUILD fails — do not | ||
| # ship an image whose patch state is ambiguous. Retire this layer when #17715 merges. | ||
| ARG APPLY_HT_FLAT_PATCH=0 | ||
| # The three single-parent commits of PR#17715 (NOT the branch-head merge commit | ||
| # e14a6f64 — GitHub's .patch endpoint 403s for a merge, so git format-patch emits nothing | ||
| # and the layer would die on it every time). | ||
| ARG HT_FLAT_PATCH_SHAS="6035d66353142d70ff41041b13dda1b3e788371a 2e3de8a3ccbb6bb1f78cd1504852045db989abfb 4e7cba789f4d928babc6199e895961f54ee30e11" | ||
| RUN set -euo pipefail; \ | ||
| if [ "${APPLY_HT_FLAT_PATCH}" = "1" ]; then \ | ||
| SP_PARENT=$(python3 -c "import tensorrt_llm, pathlib; print(pathlib.Path(tensorrt_llm.__file__).parent.parent)"); \ | ||
| cd "$SP_PARENT"; \ | ||
| for sha in ${HT_FLAT_PATCH_SHAS}; do \ | ||
| echo "== applying NVIDIA/TensorRT-LLM PR#17715 commit ${sha} =="; \ | ||
| curl -fsSL "https://github.com/NVIDIA/TensorRT-LLM/commit/${sha}.patch" > /tmp/${sha}.patch; \ | ||
| [ -s /tmp/${sha}.patch ] || { echo "FATAL: PR#17715 ${sha} yielded an empty patch (a merge commit has no format-patch output) — pin a single-parent commit"; exit 1; }; \ | ||
| git apply --include='tensorrt_llm/*' --check /tmp/${sha}.patch \ | ||
| || { echo "FATAL: PR#17715 ${sha} does not apply on ${TRTLLM_BASE} — base moved under the patch"; exit 1; }; \ | ||
| git apply --include='tensorrt_llm/*' /tmp/${sha}.patch; \ | ||
| rm -f /tmp/${sha}.patch; \ | ||
| done; \ | ||
| find "$SP_PARENT/tensorrt_llm/_torch/modules/fused_moe" -name '__pycache__' -prune -exec rm -rf {} + || true; \ | ||
| touch /opt/.ht-flat-patch-applied; \ | ||
| else echo "HT/FLAT patch layer skipped (APPLY_HT_FLAT_PATCH=0 — upstream LL/RANK_MAJOR default)"; fi | ||
|
|
||
| # ---- Layer 7: the recipe scripts (LAST — script iteration never invalidates heavy layers) ---- | ||
| COPY recipe/serve.sh /opt/serve.sh | ||
| COPY recipe/run-kernel-test.sh /opt/run-kernel-test.sh | ||
| COPY recipe/probe_nccl_ep.py /opt/probe_nccl_ep.py | ||
| COPY recipe/benchmark_probe.py /opt/benchmark_probe.py | ||
| COPY recipe/benchmark.sh /opt/benchmark.sh | ||
| RUN chmod 755 /opt/serve.sh /opt/run-kernel-test.sh /opt/benchmark.sh | ||
|
|
||
| CMD ["/bin/bash", "-lc", "echo 'run: /opt/serve.sh (single-node EP serve) | /opt/run-kernel-test.sh {leader|worker} <ip> (cross-node NcclEP probe)'; sleep infinity"] | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NVIDIA_NCCL_CU13=2.30.4— the one pin that may be genuinely forced; please measure it and say so (should fix)This is the pin I am least sure should move, and I want to be careful about it rather than lump it in with the two above.
The floor is not what holds it here:
nccl_ep_utils.pyatv1.3.0rc24has_MIN_NCCL_RUNTIME_VERSION = "2.30.4"compared withruntime < Version(...), so 2.31.2 satisfies TRT-LLM.nccl4pydoes not pin it either — its metadata declares a barenvidia-nccl-cu13under thecu13extra with no version bound, and the wheel bundles nolibnccl, linkinglibnccl.so.2by soname. Andnvidia-nccl-cu13==2.31.2is published.What might hold it is ABI:
libnccl_ep.so0.1.0's device kernels takencclDevComm*andncclWindow_vidmem*by pointer (visible in the exported symbol names), and NCCL's device-API structs did change between 2.30 and 2.31 —ncclDevCommRequirementsgained fields, and thencclGinType_tenum grewGPI = 4/EFA_GDA = 5. A prebuilt third-party binary compiled against the 2.30 device API running on a 2.31 runtime is exactly the case that can fail quietly rather than loudly.So the ask is a measurement, not an edit: try
2.31.2, and if it works, take it — if it does not, put that in the pin table ("held at 2.30.4 becauselibnccl_ep0.1.0 is built against the 2.30 device API; symptom: …"). Either outcome converts an unexplained old pin into a justified one, which is the part that matters. Note this is also the pin that decides the GIN backend menu —NCCL_GIN_TYPE_EFA_GDA = 5does not exist in 2.30.4'snccl_device/core.h— but since CPU-proxy is the intended mode, that is context rather than a reason to move.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Partially addressed — I took the "justify" fork of your ask, not yet the measurement. 558cd52 + e8494dd rewrite the Dockerfile comment and pin-table row to record exactly why it is held: 2.31.2 clears TRT-LLM's
_MIN_NCCL_RUNTIME_VERSIONfloor, butlibnccl_ep0.1.0 is a prebuilt binary against the 2.30 device API, and the 2.30→2.31 device-struct changes you list are exactly the fails-quietly class — so the row frames 2.30.4 as the measured-matching floor, not an upper bound, with the bump condition stated. The live 2.31.2 trial needs a cluster window (a build-only check cannot catch a quiet device-ABI failure); it is planned alongside the EFA 1.50.0 re-measure, and the row updates with whichever result it produces.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Held at 2.30.4 with the reason now recorded in 558cd52 (the Dockerfile comment): libnccl_ep 0.1.0 is built against the NCCL 2.30 device-side API the GIN CPU-proxy calls into, so moving to 2.31.x is a device-ABI change. I have not measured 2.31.2 on this substrate, so I am not moving the pin blind — it is documented as the measured-matching floor, to be bumped together with a libnccl_ep rebuilt on the newer device API and re-run through verify-image.sh + run-kernel-test.sh.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Measured on cgk 2× p5en.48xlarge (H200), 16 ranks cross-node. Splitting the answer along the two ABI layers your comment separates, because only one of them is settled:
Library / binding ABI on 2.31.2 — clean, no quiet break (measured). Built the image
--build-arg NVIDIA_NCCL_CU13=2.31.2as a single-variable A/B against a 2.30.4 control. On all 16 ranksis_nccl_ep_installed()passes (_get_nccl_ep_unavailable_reason→None) andCommunicationFactory.create_strategyreturnsNcclEP— no silent symbol/binding break at thenccl.epPython layer, andnvidia-nccl-cu13==2.31.2linkslibnccl.so.2by soname exactly as you noted. So the floor and the import/selection path are genuinely fine on 2.31.2.Device kernel ABI on 2.31.2 — NOT yet certified (this is precisely your
ncclDevComm*/ncclGinType_tconcern, and I don't want to overstate it). This is the layer that can fail quietly, and I can't yet claim it passes or fails. Two honest obstacles on this specific substrate:forced_pcie_copy()gates onmin(userspace, kernel) >= 2.5→ GIN init hard-fails version-independently on both arms (the sample documents gdrdrv ≥ 2.5 as a host prerequisite; this pair doesn't meet it).forced_pcie_copy() -> true, applied identically to both arms so the only differential stays the NCCL runtime version) GIN did initialize and both arms advanced toNcclEPselection — but the run then hit a latent bug in the probe's own diagnostic line (NcclEpContext._ep_algorithm, absent on 1.3.0rc24 — upstream hardcodes LOW_LATENCY and doesn't store the algo on the context) one statement before the firstdispatch(). So the device dispatch/combine round-trip was never exercised on either arm. That probe bug is nowgetattr-guarded (commitee0b062con this branch); certifying the device kernels needs a rebuild + re-run past that fix on a gdrdrv ≥ 2.5 host (or with the instrument patch).Pin decision, recorded as you asked. Held at
2.30.4— the version the 2026-08-07 correctness E2E (realtrtllm-serveHTTP-200-correct + 16-rank cross-node dispatch/combine, IMA=0, efa-direct on every rank) actually ran on — not as an upper bound but as the measured-matching floor. The Layer-4 Dockerfile comment now states this explicitly: library ABI clean on 2.31.2 (measured), device round-trip not yet measured on either arm, and the exact re-run needed to bump it.I'll update this thread with the device-level verdict once the rebuilt image runs on a gdrdrv ≥ 2.5 host — leaving it unresolved until there's a
PROBE-PASS/PROBE-MISMATCHon the device path, since that's the half your question actually turns on.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following up with the header-level answer to the part your comment actually turns on — the device-side
ncclDevComm*/ncclWindow_vidmem*ABI. I diffed the device-API headers betweenv2.30.4-1andv2.31.2-1(nccl_device/core.h,nccl_device/impl/impl_comm__types.h,.../impl_core__types.h), which lets me name the specific reason to hold, rather than leaving it as "built against the 2.30 API."ncclGinType_t— append-only;PROXY=2is byte-identical (agrees with your read). 2.30.4 has{NONE=0, PROXY=2, GDAKI=3}; 2.31.2 appends{GPI=4, EFA_GDA=5, MAX_TYPES=6}and leavesNCCL_GIN_TYPE_PROXY=2unchanged. So, as you said,EFA_GDA=5not existing at 2.30.4 is context, not a reason to move — the CPU-proxy value is stable across the bump.ncclDevComm— NOT append-only; this is the crux, and it's concrete. The struct grows 23→35 field-lines and, more importantly, the first divergence is mid-struct at field #10:… resourceWindow_inlined; lsaMultimem; lsaBarrier; railGinBarrier; ginConnectionCount; ginNetDeviceTypes[]; …… resourceWindow_inlined;hybridDenseGinBarrier;lsaMultimem; lsaBarrier; railGinBarrier; ginConnectionCount;backendIndex;ginNetDeviceTypes[]; …hybridDenseGinBarrier(ancclGinBarrierHandle_t) is inserted beforelsaMultimem, andbackendIndex(uint8_t) is inserted inside the GIN block, beforeginNetDeviceTypes. There's a third shift on the same struct:ginIsRailed(oneboolat 2.30.4) is replaced byginConnectionStride+ginContextStride(2×int) +ginStrongLegacySignals. And the inlined field #9 itself (resourceWindow_inlined, embedded by value) shrank — 2.30.4 definesncclResourceWindow_vidmemwith explicit reserved padding and the comment "Same size asncclWindow_vidmemfor backward compatibility"; 2.31.2 drops that padding to a bare{lsaFlatBase, stride4G, mcOffset4K}. Any one of these shifts the byte offsets of the GIN fields the CPU-proxy device code reads (railGinBarrier,ginHandles[],ginSignalShadows,ginContextCount, …); together they guarantee it. A prebuiltlibnccl_ep.so0.1.0 compiled against the 2.30 layout, handed ancclDevCommallocated by a 2.31.2 runtime, reads those fields at the wrong offsets — the fails-quietly case you flagged, now with a named field rather than a hand-wave.ncclWindow_vidmem— near-compatible. For completeness on the other pointer arg you named: this one is far tamer —ginWins[]→ginWinsDefaultBackend[]is a rename of the samencclGinWindow_t[NCCL_GIN_MAX_CONNECTIONS]type, andint cftFlatRankis appended at the tail. SoncclWindow_vidmemalone would be layout-compatible;ncclDevCommis the one that isn't.The honest bound — why this justifies the pin but the empirical run still certifies it. 2.31.2 did add version-negotiation machinery that 2.30.4 lacks entirely:
ncclDevCommRequirementsgainedbool useRuntimeVersion(+devCommRuntimeVersionSize), alongside the existingmagic/versionheader onncclDevComm. Its initializer defaultsuseRuntimeVersion=false— documented as the "device code is not the runtime version" (i.e. AOT/prebuilt) case, which is exactlylibnccl_ep's case. So NCCL 2.31 is aware of version skew and might lay out a back-compat devComm for an older-compiled kernel — but whether it does so correctly for a foreign prebuilt AOT binary is runtime-internal, not visible in the headers. So the header diff makes the risk structurally real and specific (it justifies holding the pin, which was your ask — "convert an unexplained old pin into a justified one"), and it also confirms why a green smoke test alone couldn't prove safety here. The device dispatch/combine round-trip on a gdrdrv ≥ 2.5 host — the run I still owe this thread — stays the empirical certifier, and I'll post thePROBE-PASS/PROBE-MISMATCHwhen that host is available.Net: held at
2.30.4, now with the concrete reason recorded —ncclDevCommis not append-only across 2.30.4→2.31.2 (hybridDenseGinBarrierinserted at field 10 beforelsaMultimem, plusbackendIndexmid-GIN-block), shifting the offsets of the GIN fields the CPU-proxy path dereferences. I'll fold that one-liner into the Layer-4 pin comment. Thanks for pushing on this one specifically — you were right that it was the pin that deserved a real answer.