Skip to content

docs(eks): remove Known Issues block now that libfabric + GDRCopy fixes are merged - #9831

Open
yifjiang wants to merge 2 commits into
ai-dynamo:mainfrom
yifjiang:yifjiang/efa-doc-libfabric-workaround
Open

docs(eks): remove Known Issues block now that libfabric + GDRCopy fixes are merged#9831
yifjiang wants to merge 2 commits into
ai-dynamo:mainfrom
yifjiang:yifjiang/efa-doc-libfabric-workaround

Conversation

@yifjiang

@yifjiang yifjiang commented May 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Removes the "Known Issues" section from docs/kubernetes/cloud-providers/eks/efa.md and prunes the two now-obsolete rows from "Common Failure Modes". Assumes #9703, #9704, #9705, and #9727 are all merged — after those land, the issues this section documented (GB200 fi_mr_reg(VRAM) failure on the EFA installer's stock libfabric, and GDRCopy v2.5.1 kmod build failure on kernel ≥ 6.15) no longer affect default --make-efa builds, so the inline workarounds the section provided would be misleading.

Depends on #9703 + #9704 + #9705 + #9727 merging first. If any of those slip, this PR should be held until they all land. Marked draft for review.

What changes

docs/kubernetes/cloud-providers/eks/efa.md:

  • ## Known Issues section deleted entirely. Both Issue 1 (libfabric CUDA dmabuf bug) and the inline Dockerfile workaround it carried are no longer needed: feat(container): configurable libfabric repo + v2.5.1 overlay for EFA #9727 internalizes the patched libfabric into the --make-efa aws stage. (The previous text on main only documented Issue 1; the GDRCopy v2.5.1 / kernel-6.15 issue was never on main — it landed in this branch earlier as Issue 2 during my expansion. Both go away now.)
  • ## Common Failure Modes table cleaned up:
    • Removed row "Pod fails at startup with fi_mr_reg EFAULT on GB200 → Issue 1 → Apply patch or use ofiwg/libfabric v2.5.1" (issue is fixed in main; references a nonexistent ## Known Issues > Issue 1).
    • Removed row "Throughput below EFA line rate, /dev/gdrdrv missing on host → Issue 2 → Bump nixl_gdrcopy_ref to v2.5.2" (issue is fixed by build(container): bump nixl_gdrcopy_ref to v2.5.2 (kernel >=6.15 fix) #9705 / dynamo ships v2.5.2 source by default).
    • Merged the two remaining fi_mr_reg EFAULT rows (GB200 and x86_64) into one more general "missing privileged: true or efa_nv_peermem" entry — that's the only fi_mr_reg EFAULT cause left once the libfabric bug is fixed upstream.
  • References list cleaned up: removed the orphaned [ofiwg/libfabric#12019] link (was only referenced from the deleted Issue 1).

Net diff: 1 line added, 34 deleted.

Validation evidence

The v4 PR-internalized image (built from a branch that has all four merged PRs cherry-picked) passes end-to-end with no inline workaround:

  • scripts/probe_nixl_libfabric_vram.py (standalone NIXL register_memory(VRAM, LIBFABRIC) probe) returns === libfabric VRAM registration: PASSED === on dev-01 GB200 (the cluster where the dropped Issue 1 originally manifested).
  • Live TRT-LLM disagg DGD on dev-01 with Qwen3-Coder-480B-A35B-Instruct-FP4, READY 3/3, 0 restarts. Per-rank logs show using NIXL backend: LIBFABRIC.
  • See prs-internalized-v4-validation-2026-05-21.md for the full validation report (3-way A/B with prior images, TTFT sweep, etc.).

Sequencing

Merge order required:

  1. fix(container): remove correct ofi-nccl paths in aws.Dockerfile #9703 (ofi-nccl rm path)
  2. feat(container): add --has-trtllm-context flag to render.py [RFC] #9704 (--has-trtllm-context flag)
  3. build(container): bump nixl_gdrcopy_ref to v2.5.2 (kernel >=6.15 fix) #9705 (gdrcopy v2.5.2 bump)
  4. feat(container): configurable libfabric repo + v2.5.1 overlay for EFA #9727 (ofiwg/libfabric v2.5.1 overlay in aws stage)
  5. this PR (delete the now-obsolete docs)

(Any merge order among 1–4 works; this docs-cleanup PR must come last.)

Risk

LOW. Documentation-only. No code paths affected. If a reader builds from an older dynamo (pre-merge of the four PRs) they may hit issues this section used to document — but the appropriate fix is to upgrade dynamo, not re-add the inline workaround section.

Test plan

  • Renders cleanly in GitHub markdown preview.
  • No dangling cross-references to deleted sections.
  • CI pre-commit, lychee, fern-docs pass.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Simplified the EKS EFA guide by removing a lengthy “Known Issues” section.
    • Updated troubleshooting guidance for startup failures related to fi_mr_reg EFAULT with clearer cause-and-fix steps.
    • Refined references by removing an outdated external issue link.

@github-actions

github-actions Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot added docs external-contribution Pull request is from an external contributor documentation Improvements or additions to documentation labels May 21, 2026
@yifjiang
yifjiang force-pushed the yifjiang/efa-doc-libfabric-workaround branch from dbae7d4 to 7f7a37a Compare May 21, 2026 17:26
@pull-request-size pull-request-size Bot added size/M and removed size/S labels May 21, 2026
@yifjiang yifjiang changed the title docs(eks): expand libfabric CUDA dmabuf workaround with SONAME fix + build-time validation docs(eks): document three known issues for default-built dynamo EFA images May 21, 2026
@yifjiang
yifjiang force-pushed the yifjiang/efa-doc-libfabric-workaround branch from 7f7a37a to 54cf069 Compare May 21, 2026 17:42
@yifjiang yifjiang changed the title docs(eks): document three known issues for default-built dynamo EFA images docs(eks): expand libfabric CUDA dmabuf workaround + add GDRCopy kernel-6.15 known issue May 21, 2026
@yifjiang

Copy link
Copy Markdown
Contributor Author

/ok to test 54cf069

…es are merged

Removes the "Known Issues" section from docs/kubernetes/cloud-providers/eks/efa.md
and prunes the two now-obsolete rows from "Common Failure Modes". Assumes
ai-dynamo#9703, ai-dynamo#9704, ai-dynamo#9705, and ai-dynamo#9727 are all merged — after those land, the issues
this section documented (GB200 fi_mr_reg(VRAM) failure on the EFA installer's
stock libfabric, and GDRCopy v2.5.1 kmod build failure on kernel >= 6.15) no
longer affect default --make-efa builds, so the inline workarounds the section
provided would mislead readers.

Also removes the ofiwg/libfabric#12019 reference from the bottom links list
since it points at the same now-resolved upstream issue.

Net diff: -34 / +1.

Signed-off-by: Yifan Jiang <yifjiang@users.noreply.github.com>
@yifjiang

Copy link
Copy Markdown
Contributor Author

/ok to test 78d0621

@yifjiang
yifjiang force-pushed the yifjiang/efa-doc-libfabric-workaround branch from 54cf069 to 78d0621 Compare May 22, 2026 03:08
@yifjiang yifjiang changed the title docs(eks): expand libfabric CUDA dmabuf workaround + add GDRCopy kernel-6.15 known issue docs(eks): remove Known Issues block now that libfabric + GDRCopy fixes are merged May 22, 2026
@yifjiang
yifjiang force-pushed the yifjiang/efa-doc-libfabric-workaround branch from 78d0621 to 9b52cac Compare May 23, 2026 07:09
@copy-pr-bot

copy-pr-bot Bot commented May 23, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot added the Stale label Jun 23, 2026
@rmccorm4
rmccorm4 temporarily deployed to external_collaborator June 30, 2026 20:03 — with GitHub Actions Inactive
@rmccorm4

Copy link
Copy Markdown
Contributor

/ok to test 066d516

@rmccorm4
rmccorm4 marked this pull request as ready for review June 30, 2026 20:04
@rmccorm4
rmccorm4 requested a review from a team as a code owner June 30, 2026 20:04
@rmccorm4
rmccorm4 requested a review from jh-nv June 30, 2026 20:05
@rmccorm4
rmccorm4 enabled auto-merge (squash) June 30, 2026 20:05

@devin-ai-integration devin-ai-integration Bot left a comment

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.

Devin Review found 2 potential issues.

Open in Devin Review

Comment on lines -214 to -243
## Known Issues

One issue currently affects default-built Dynamo EFA images.

### Issue 1: libfabric on GB200 fails `fi_mr_reg` on CUDA VRAM

**Known affected platforms:** GB200.

**Symptom:** Worker pod fails at startup with `fi_mr_reg` returning EFAULT during NIXL initialization. NIXL VRAM registration fails; depending on the framework, the worker either crashes or silently falls back to TCP.

**Root cause:** The libfabric version (versions lower than 2.5.x) bundled with the EFA installer (up to currently latest 1.48.0) lacks a CUDA branch in the dmabuf-eligibility check in `prov/efa/src/efa_mr.c`. On x86_64 hosts the legacy `ibv_reg_mr` path handles CUDA pointers natively, so the bug doesn't surface. On arm64 64K-page kernels (GB200), the legacy path returns EFAULT for CUDA VRAM. Tracked in [ofiwg/libfabric#12019](https://github.com/ofiwg/libfabric/issues/12019).

**Upstream status:** The bug is resolved in `ofiwg/libfabric` main and v2.5.x via a more comprehensive rewrite of `efa_mr_reg_ibv_mr()`. AWS's `aws/libfabric` fork has not picked up the upstream rewrite; the latest EFA installer (1.48.0) still ships `v2.4.0amzn3.0` with the older code path.

**Workarounds:**

1. **Apply the one-line patch to the bundled libfabric.** During image build, replace the `aws.Dockerfile` install step with a custom build:
```dockerfile
RUN git clone --depth 1 --branch v2.4.0amzn3.0 https://github.com/aws/libfabric.git /tmp/libfabric && \
cd /tmp/libfabric && \
sed -i 's/efa_mr_is_neuron(efa_mr) || efa_mr_is_rocr(efa_mr)/efa_mr_is_neuron(efa_mr) || efa_mr_is_rocr(efa_mr) || efa_mr_is_cuda(efa_mr)/' prov/efa/src/efa_mr.c && \
./autogen.sh && \
CPPFLAGS="-I/usr/local/cuda/include" \
LDFLAGS="-L/usr/local/cuda/lib64 -L/usr/local/cuda/lib64/stubs -Wl,-rpath,/usr/local/cuda/lib64" \
./configure --prefix=/opt/amazon/efa --enable-efa --with-cuda=/usr/local/cuda --enable-cuda-dlopen && \
make -j$(nproc) && make install
# Then rebuild aws-ofi-nccl from source against the patched libfabric (do not mix versions)
```
2. **Replace bundled libfabric with `ofiwg/libfabric@v2.5.1`** (or newer). The upstream rewrite is already present; no patch needed. Rebuild `aws-ofi-nccl` against it.

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.

🟡 Dangling anchor link to a removed section leaves readers clicking a dead link

The "Known Issues" section was removed (efa.md:214-243, LEFT) but the admonition at docs/kubernetes/cloud-providers/eks/efa.md:139 still references [Known Issues](#known-issues), so readers who click it land nowhere.

Impact: Users following the important callout in Step 3 hit a broken in-page link and cannot find the referenced information.

Broken anchor details and style-guide rule

Line 139 contains:

See Known Issues below for one case where the default-built image does not produce a working EFA deployment out of the box (GB200 / arm64 64K-page kernels).

The ## Known Issues heading that provided the #known-issues anchor was entirely deleted in this PR (old lines 214–243), but the > [!IMPORTANT] admonition referencing it was not updated or removed.

This also violates the docs style guide at docs/documentation-style-guide.md:169: "Every internal link and #anchor must resolve to a real file or heading."

The entire admonition block at line 139 should either be removed (since the known issue it describes was removed) or updated to point to valid content.

Prompt for agents
The PR removed the '## Known Issues' section (old lines 214-243) from docs/kubernetes/cloud-providers/eks/efa.md but left an admonition at line 139 that still references [Known Issues](#known-issues). This anchor is now broken. The admonition reads: 'See [Known Issues](#known-issues) below for one case where the default-built image does not produce a working EFA deployment out of the box (GB200 / arm64 64K-page kernels).' Since the Known Issues section and its content about the GB200 libfabric CUDA dmabuf bug have been removed, this entire admonition block (the > [!IMPORTANT] block at line 138-139) should also be removed, or rewritten to reference whatever content (if any) replaced the removed section.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

| TTFT ~10 s, throughput 1–5 GB/s | UCX host-staged (no GPU-Direct on kernel ≥ 6.8) | Switch to LIBFABRIC backend |
| Pod fails at startup with `fi_mr_reg` EFAULT on GB200 | Issue 1 (libfabric CUDA dmabuf bug) | Apply patch or use ofiwg/libfabric v2.5.1 |
| Pod fails at startup with `fi_mr_reg` EFAULT on x86_64 | `privileged: true` missing OR `efa_nv_peermem` missing on old kernel | Verify Step 5 security context |
| Pod fails at startup with `fi_mr_reg` EFAULT | `privileged: true` missing OR (on pre-DMA-BUF kernels) `efa_nv_peermem` not loaded | Verify Step 5 security context; verify Step 2 kernel/module state |

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.

🚩 Removal of Known Issues section drops actionable workaround content

The removed "Known Issues" section (old lines 214–243) contained a concrete one-line sed patch and an alternative workaround (switching to ofiwg/libfabric@v2.5.1) for a GB200-specific fi_mr_reg EFAULT during NIXL initialization. The consolidated table row at docs/kubernetes/cloud-providers/eks/efa.md:284 now attributes the symptom generically to missing privileged: true or efa_nv_peermem, which doesn't cover the libfabric CUDA dmabuf bug that was documented. If that upstream bug is still relevant for users on older libfabric versions, removing the workaround without a replacement could leave GB200 users without guidance. If the bug has been fixed in the libfabric versions shipped by the EFA installer, the removal is correct — but that context isn't stated in the PR.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 36e89b6f-beb5-4dbf-8780-1be7831dae41

📥 Commits

Reviewing files that changed from the base of the PR and between dac7dfb and 066d516.

📒 Files selected for processing (1)
  • docs/kubernetes/cloud-providers/eks/efa.md

Walkthrough

The EKS EFA documentation was updated to remove the "Known Issues" section detailing a libfabric CUDA dmabuf registration failure, simplify the "Common Failure Modes" table entry for fi_mr_reg EFAULT errors with updated cause/fix guidance, and remove an upstream libfabric issue reference link.

Changes

EKS EFA Troubleshooting Docs Update

Layer / File(s) Summary
Common Failure Modes table update
docs/kubernetes/cloud-providers/eks/efa.md
The fi_mr_reg EFAULT failure mode row now points to missing privileged: true or unloaded efa_nv_peermem module, referencing Steps 2 and 5; the "Known Issues" section and the upstream libfabric issue reference link were removed.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is detailed, but it does not follow the required template and omits the required Related Issues section. Add the template headings (Overview, Details, Where should reviewer start?, Related Issues) and include the required issue link or no-issue confirmation.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the doc cleanup and removal of outdated Known Issues content.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@jh-nv jh-nv left a comment

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.

Thanks.

The PR description says #9703/#9704/#9705/#9727 must all be merged first, but #9703 and #9704 are currently closed unmerged. just want to make sure those were superseded by #9727 and are no longer required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs documentation Improvements or additions to documentation external-contribution Pull request is from an external contributor size/M Stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants