Skip to content

feat(container): libfabric2.5.1 for EFA and TRTLLM #10047

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.

Sign up for GitHub

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

Merged
dillon-cullinan merged 1 commit into from
May 28, 2026
Merged

feat(container): libfabric2.5.1 for EFA and TRTLLM #10047

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 14 additions & 23 deletions container/templates/aws.Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,7 @@ RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \
apt-get update && \
./efa_installer.sh -y --skip-kmod --skip-limit-conf --no-verify && \
rm -rf /tmp/efa && \
# Disable the EFA installer's aws-ofi-nccl plugin: it crashes TRT-LLM at engine init.
# The plugin is installed at /opt/amazon/ofi-nccl (no `aws-` prefix), but ld.so picks
# it up via /etc/ld.so.conf.d/aws-ofi-nccl.conf (which DOES carry the `aws-` prefix).
# Remove both, and also the cuda-dl-base location /opt/amazon/aws-ofi-nccl if present,
# before re-running ldconfig.
rm -rf /opt/amazon/aws-ofi-nccl /opt/amazon/ofi-nccl \
/etc/ld.so.conf.d/aws-ofi-nccl.conf && \
rm -rf /opt/amazon/aws-ofi-nccl /etc/ld.so.conf.d/aws-ofi-nccl.conf && \

@coderabbitai coderabbitai Bot May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix the aws-ofi-nccl cleanup path.

The EFA images elsewhere in this repo use /opt/aws-ofi-nccl, so this currently removes the .conf file but leaves the installed plugin directory behind.

Suggested fix 
-    rm -rf /opt/amazon/aws-ofi-nccl /etc/ld.so.conf.d/aws-ofi-nccl.conf && \
+    rm -rf /opt/aws-ofi-nccl /etc/ld.so.conf.d/aws-ofi-nccl.conf && \
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@container/templates/aws.Dockerfile` at line 39, The cleanup command is
removing /etc/ld.so.conf.d/aws-ofi-nccl.conf but targeting the wrong plugin
directory (/opt/amazon/aws-ofi-nccl); update the Dockerfile cleanup to remove
the correct installed path (/opt/aws-ofi-nccl) instead of
/opt/amazon/aws-ofi-nccl so both the plugin directory and the .conf file are
deleted (adjust the rm -rf target from "/opt/amazon/aws-ofi-nccl" to
"/opt/aws-ofi-nccl" while keeping "/etc/ld.so.conf.d/aws-ofi-nccl.conf").

@erezzarum erezzarum May 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We never use /opt/aws-ofi-nccl can you confirm this?

@coderabbitai coderabbitai Bot May 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for all references to aws-ofi-nccl paths in the repository
rg -n "aws-ofi-nccl" --type=dockerfile
rg -n "ofi-nccl" --type=dockerfile
# Also search in shell scripts and other files
rg -n "aws-ofi-nccl" -g "*.sh" -g "*.yml" -g "*.yaml" -g "*.json" -g "*.py" -g "*.txt" -g "*.md" 2>/dev/null || true
rg -rn "opt/aws-ofi-nccl" 2>/dev/null | head -30
rg -rn "opt/amazon/aws-ofi-nccl" 2>/dev/null | head -30
rg -rn "opt/amazon/ofi-nccl" 2>/dev/null | head -30

Length of output: 3490

@erezzarum You are correct — my suggestion was wrong. The path /opt/aws-ofi-nccl does exist in the repo, but only in an unrelated recipe (recipes/glm-5-nvfp4/sglang/disagg/efa/) and docs — it has no connection to this Dockerfile.

Looking at the diff more carefully, the current code (rm -rf /opt/amazon/aws-ofi-nccl) makes sense in context: the pre-PR code removed both /opt/amazon/aws-ofi-nccl and /opt/amazon/ofi-nccl (plus the conf file) to fully disable aws-ofi-nccl. This PR intentionally drops the /opt/amazon/ofi-nccl removal so the EFA installer's aws-ofi-nccl is preserved, while still cleaning up the duplicate /opt/amazon/aws-ofi-nccl path. That's consistent with the PR goal of "switching to the EFA installer aws-ofi-nccl."

My original review comment is invalid. Apologies for the noise!

✏️ Learnings added 
Learnt from: erezzarum
Repo: ai-dynamo/dynamo PR: 10047
File: container/templates/aws.Dockerfile:39-39
Timestamp: 2026-05-27T16:26:36.525Z
Learning: In the ai-dynamo/dynamo repository, the EFA installer installs aws-ofi-nccl primarily at `/opt/amazon/ofi-nccl` (without the `aws-` prefix). The path `/opt/amazon/aws-ofi-nccl` is a secondary/duplicate location. The path `/opt/aws-ofi-nccl` is used only in unrelated recipes (e.g., `recipes/glm-5-nvfp4/sglang/disagg/efa/`) and is not related to `container/templates/aws.Dockerfile`. Do not confuse these paths when reviewing EFA-related Dockerfile cleanup commands.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used 
Learnt from: yuanwu2017
Repo: ai-dynamo/dynamo PR: 7889
File: container/templates/dev.Dockerfile:221-225
Timestamp: 2026-04-04T09:52:48.051Z
Learning: For CPU-target build Dockerfiles in ai-dynamo/dynamo (e.g., templates used for CPU device builds), treat hard-coded `x86_64-linux-gnu` multiarch triplets in `NIXL_LIB_DIR` and `NIXL_PLUGIN_DIR` paths as intentional. Do not raise a multi-arch portability/architecture issue for these paths when the target is explicitly x86_64-only for CPU builds.

ldconfig

ENV EFA_VERSION="${EFA_VERSION}"
Expand All @@ -61,11 +55,11 @@ RUN --mount=from=wheel_builder,source=/usr/local/libfabric,target=/tmp/libfabric
if [ -n "$EFA_LIBFABRIC_VER" ] && [ -n "$REF_VER" ] && \
[ "$(printf '%s\n' "$EFA_LIBFABRIC_VER" "$REF_VER" | sort -V | head -n1)" = "$EFA_LIBFABRIC_VER" ] && \
[ "$EFA_LIBFABRIC_VER" != "$REF_VER" ]; then \
cp -Pf /tmp/libfabric_build/lib/libfabric.so* /opt/amazon/efa/lib/ && \
if [ -d /opt/amazon/efa/lib64 ]; then \
cp -Pf /tmp/libfabric_build/lib/libfabric.so* /opt/amazon/efa/lib64/; \
fi && \
cp -f /tmp/libfabric_build/bin/fi_info /opt/amazon/efa/bin/fi_info && \
rm -rf /opt/amazon/efa && \
cp -Pfr /tmp/libfabric_build /opt/amazon/efa && \
sed -i 's|^prefix=.*|prefix=/opt/amazon/efa|' /opt/amazon/efa/lib/pkgconfig/libfabric.pc && \
echo "/opt/amazon/efa/lib" > /etc/ld.so.conf.d/000_efa.conf && \
rm -f /etc/ld.so.conf.d/efa.conf && \
Comment thread
erezzarum marked this conversation as resolved.
ldconfig && \
echo "[aws] libfabric overlay: ${REF_VER} (overwrites EFA stock ${EFA_LIBFABRIC_RAW})"; \
else \
Expand All @@ -84,18 +78,15 @@ RUN --mount=from=wheel_builder,source=/usr/local/libfabric,target=/tmp/libfabric
# Dynamo-built NIXL 0.10.1 plugins). LIBFABRIC goes through libfabric directly
# (not UCX), so it is unaffected by the UCX 1.20.0 hang that LD_PRELOAD works
# around — and LIBFABRIC is the recommended backend for EFA.
RUN set -e && \
arch_libdir=$(find /opt/nvidia/nvda_nixl/lib -maxdepth 1 -type d -name '*-linux-gnu' | head -1) && \
[ -n "$arch_libdir" ] || { echo "ERROR: no arch-specific NIXL plugin dir under /opt/nvidia/nvda_nixl/lib" >&2; exit 1; } && \
venv_lf=$(find /opt/dynamo/venv -path '*.nixl_cu13.mesonpy.libs/plugins/libplugin_LIBFABRIC.so' | head -1) && \
[ -n "$venv_lf" ] || { echo "ERROR: no libplugin_LIBFABRIC.so under /opt/dynamo/venv" >&2; exit 1; } && \
cp -Pf "$venv_lf" "$arch_libdir/plugins/" && \
ln -sfT "$arch_libdir/plugins" /opt/nvidia/nvda_nixl/plugins && \
[ -f /opt/nvidia/nvda_nixl/plugins/libplugin_LIBFABRIC.so ] || { echo "ERROR: LIBFABRIC plugin not visible via /opt/nvidia/nvda_nixl/plugins" >&2; ls -la /opt/nvidia/nvda_nixl/plugins/ >&2; exit 1; } && \
echo "[aws] NIXL plugins consolidated under /opt/nvidia/nvda_nixl/plugins -> $arch_libdir/plugins"
RUN --mount=from=wheel_builder,source=/opt/nvidia/nvda_nixl,target=/tmp/nvda_nixl \
rm -rf /opt/nvidia/nvda_nixl && \
cp -Pfr /tmp/nvda_nixl /opt/nvidia/nvda_nixl && \
export LD_PRELOAD=/opt/nvidia/nvda_nixl/lib64/libnixl.so && \
export NIXL_PLUGIN_DIR=/opt/nvidia/nvda_nixl/lib64/plugins && \
ldconfig

ENV LD_PRELOAD=""
ENV NIXL_PLUGIN_DIR=/opt/nvidia/nvda_nixl/plugins
ENV LD_PRELOAD=/opt/nvidia/nvda_nixl/lib64/libnixl.so
ENV NIXL_PLUGIN_DIR=/opt/nvidia/nvda_nixl/lib64/plugins
{% endif %}

{% if target == "runtime" %}
Expand Down
Loading