fix(deepep-v2): use the EFA installer bundled aws-ofi-nccl plugin - #1239
Conversation
KeitaW
left a comment
There was a problem hiding this comment.
Review Batch 1/3 — What the container still needs beyond the installer
I validated this change end to end rather than on reading alone: I built the image from this branch at de11533, built a second image with --build-arg EFA_INSTALLER_VERSION=1.49.0 to exercise the fail-closed path, and ran the two-node internode benchmark on 2x p6-b200.48xlarge. The core substitution is correct and I could not break it — details and numbers in batch 3.
The two findings here are both in prose this PR newly wrote, and they point the same way: the image needs more than the installer, and the docs now read as though it does not.
KeitaW
left a comment
There was a problem hiding this comment.
Review Batch 2/3 — Build gate ergonomics
Four small things in the block this PR already touches. None affects a successful default build.
Two smaller notes
ENV EFA_PREFIXlost its only in-build consumer.deepep.Dockerfile:108setsEFA_PREFIX=/opt/amazon/efa; its only user in the build was--with-libfabric=${EFA_PREFIX}on theconfigureline this PR deletes. It is still reasonable to bake into the image env for anyone compiling against libfabric inside the container — but if it stays, it is now an image contract rather than a build variable, and a short comment saying so would keep the next reader from treating it as dead.- The gate proves the GIN ABI but not the Net ABI. The README invites overriding
NCCL_REFandEFA_INSTALLER_VERSIONindependently (lines 93-99), while the gate asserts onlyncclGinPlugin_v14. The default pair is fine — I checked that NCCLv2.31.2-1probes GIN{14, 13}and Net{12...6}, and the bundled plugin exportsncclGinPlugin_v11/v13/v14plusncclNetPlugin_v6...v12. But a future NCCL bump that moves the Net floor to v13 would sail through this gate. AddingncclNetPlugin_v12to the samegrep, or noting the two versions as a tested pair rather than free knobs, would cover it.
KeitaW
left a comment
There was a problem hiding this comment.
Review Batch 3/3 — Validation, positives & sources
End-to-end validation of this branch
I ran the flow slurm/test-internode.sbatch defines, with its environment replicated exactly (NCCL_GIN_TYPE=5, NCCL_SYM_GIN_KERNELS_ENABLE=0, EP_NCCL_ROOT_DIR=$NCCL_HOME, LD_PRELOAD=$NCCL_HOME/lib/libnccl.so.2) plus NCCL_DEBUG=INFO, on 2x p6-b200.48xlarge (8x B200 + 8x EFA per node, EFA kernel driver 3.3.0g, gdrdrv loaded), image pinned by digest:
Ranks: 2 x 8 | Experts: 8/256 | Tokens: 8192, hidden: 7168 | #SM: 12, #QPs: 11/11
dispatch 82 GB/s (SO) / 265-268 GB/s (SU)
combine 68-70 GB/s (SO) / 220-230 GB/s (SU)
reduced combine 57-63 GB/s (SO) / 186-203 GB/s (SU)
RUN_EXIT=0 on both nodes; no Traceback / AssertionError / CUDA error on any rank.
(B200 numbers — not comparable with the B300 figures in the #1234 thread. Different silicon, not a regression.)
The part worth confirming for this change is which object served that run. On all 16 ranks across both nodes:
NCCL INFO GIN/Plugin: Successfully loaded external plugin /opt/amazon/ofi-nccl/lib/libnccl-net-ofi.so
NCCL INFO GIN/Plugin: Loaded gin plugin Libfabric_GDAKI (v14)
NCCL INFO GIN/Plugin: Skipping plugin Libfabric index 3 type 2: NCCL_GIN_TYPE=5 requested
NCCL INFO NET/OFI Initializing aws-ofi-nccl 1.21.1
NCCL INFO NET/OFI Plugin selected platform: AWS
NCCL INFO NET/Plugin: Loaded net plugin Libfabric (v12)
The installer's copy is the one loaded, the EFA-GDA op-table is the one bound, and the CPU-proxy backend is explicitly skipped. EFA hardware counters moved on all 8 devices per node (~26.3 GB tx and ~26.3 GB rx each, ~210 GB per node), so the traffic really went over the fabric.
Supporting checks:
- The in-container
libnccl-net-ofi.sohashes to4f5aaedbf62db3d9229d1107812813bfbf6197dbc3650ea00fbd249a444c001c, identical to the payload oflibnccl-ofi_1.21.1-1_amd64.debextracted straight from the installer tarball — the image runs the installer's binary, unmodified. - The gate genuinely discriminates: installer 1.50.0 bundles plugin 1.21.1, which exports
ncclGinPlugin_v14; 1.49.0 bundles 1.20.0, which exports onlyv11/v13. The 1.49.0 build fails at the gate, the 1.50.0 build passes. --disable-ngcdoes not skip the plugin — it only disables NGC auto-detection (efa_installer.sh:773-779), so the regularlibnccl-ofipackage installs. The build log showsSetting up libnccl-ofi (1.21.1-1).- Architecture-portable: the aarch64 deb installs to the same
/opt/amazon/ofi-nccl/lib/libnccl-net-ofi.soand also exportsncclGinPlugin_v14.
Things That Look Great
- This closes the exit condition the previous revision wrote for itself. #1234's README said the source build could be dropped "once an installer ships a GDAKI-enabled plugin"; 1.50.0 ships it and this PR acts on that instead of letting the sentence go stale.
- It removes a real duplicate-plugin hazard, not just a build step. Before this change the image contained two files named
libnccl-net-ofi.so— the installer's at/opt/amazon/ofi-nccl/liband the source build's at/opt/aws-ofi-nccl/lib— with both directories registered inld.so.conf.d. Anything resolving by bare filename (the tuner plugin, for one) took whicheverldconfigordered first. After this change a filesystem-wide search returns exactly one plugin directory. That is a correctness improvement on top of the simplification. - Dropping the source build also drops a build-time
git clone --recursiveof an external repo, which is both a supply-chain surface and a large chunk of build time. - The discriminator choice is right and I could not fool it.
ncclGinPlugin_v11/v13are exported by proxy-only builds too; onlyv14is EFA-GDA-specific. - Both halves of the plugin variable pair are set (lines 130-131), so the "use the image directly" flow is correctly configured now, not just the launcher flow.
NET/OFI Plugin selected platform: AWSconfirms the bundled build carries the platform-AWS support the deleted--enable-platform-awswas there to get — worth knowing, since that flag disappearing is the kind of thing that silently changes behaviour.
Suggested priority
Nothing here blocks the substitution, which I consider proven. The two documentation items in batch 1 are the ones I would fix before merge — both are in text this PR newly wrote, and both under-state what the container needs. Batch 2 is optional cleanup in the block you are already touching.
Sources
- Installer contents and plugin symbol tables —
aws-efa-installer-1.50.0.tar.gz(DEBS/UBUNTU2204/{x86_64,aarch64}/libnccl-ofi_1.21.1-1_*.deb) andaws-efa-installer-1.49.0.tar.gz(libnccl-ofi_1.20.0-1_amd64.deb); read withnm -D,readelf -d,strings. Verified live, 2026-08-25. - NCCL plugin ABI —
src/plugin/gin.ccandsrc/plugin/net.ccat the pinnedv2.31.2-1. Verified live, 2026-08-25. - EFA installer changelog (1.50.0: libfabric 2.6.0amzn1.0, OFI NCCL plugin 1.21.1; 1.49.0: libfabric 2.4.0amzn5.0, plugin 1.20.0) —
ChangeLog.mdin each tarball. Verified live, 2026-08-25. - Two-node run on 2x
p6-b200.48xlarge, image pinned by digestsha256:24343eea4db90de3e9ef43c47a77b2f2ea04c6114a93021d155a74e9d837eeef, all-rankNCCL_DEBUG=INFOlogs and before/after EFAhw_counters. Verified live, 2026-08-25. - Fail-closed build:
docker build --build-arg EFA_INSTALLER_VERSION=1.49.0exits 1 at the gate after#13 DONE 161.7s. Verified live, 2026-08-25.
1c5db7b to
a34d00b
Compare
Installer 1.50.0 bundles an EFA-GDA-capable aws-ofi-nccl (verified in PR 1234 review), so drop the source build and apply the ncclGinPlugin_v14 gate to the bundled binary instead. Fold the libfabric and aws-ofi-nccl version bullets into the EFA installer bullet, and state explicitly that the installer must run on the node itself (not the container) to provide the kernel driver. Addresses post-merge review comments on PR 1234. Co-authored-by: Vladimir Aerov <vaerov@amazon.com>
a34d00b to
232e9b7
Compare
Installer 1.50.0 bundles an EFA-GDA-capable aws-ofi-nccl (verified in PR 1234 review), so drop the source build and apply the ncclGinPlugin_v14 gate to the bundled binary instead. Fold the libfabric and aws-ofi-nccl version bullets into the EFA installer bullet, and state explicitly that the installer must run on the node itself (not the container) to provide the kernel driver.
Addresses post-merge review comments on PR 1234.