Removes source compilation of nixl dependency #24874

bbartels · 2025-09-15T10:33:43Z

Purpose

Test Plan

Test Result

Essential Elements of an Effective PR Description Checklist

The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
The test plan, such as providing test command.
The test results, such as pasting the results comparison before and after, or e2e results
(Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
(Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

Signed-off-by: bbartels <[email protected]>

bbartels · 2025-09-15T10:34:18Z

@NickLucche @hmellor Apologies, messed up the commits in the previous PR. Please have a look here :)

pytorch-bot · 2025-09-15T10:34:21Z

No ciflow labels are configured for this repo.
For information on how to enable CIFlow bot see this wiki

gemini-code-assist

Code Review

This pull request aims to simplify the installation of the nixl dependency by removing source compilation in favor of a pre-built package. While this is a good goal, the implementation has a critical issue in the Dockerfile where it attempts to install an Ubuntu 24.04 package on an Ubuntu 22.04 base image, which will likely break the build. The documentation has also been updated with installation instructions that are not portable and will fail for users on different systems. I've provided detailed feedback and suggestions to address these issues.

gemini-code-assist · 2025-09-15T10:35:20Z

docker/Dockerfile

+RUN curl https://developer.download.nvidia.com/compute/redist/gdrcopy/CUDA%2012.8/ubuntu24_04/x64/libgdrapi_2.5.1-1_amd64.Ubuntu24_04.deb --output libgdrapi_2.5.1-1_amd64.Ubuntu24_04.deb \
+    && apt install ./libgdrapi_2.5.1-1_amd64.Ubuntu24_04.deb \
+    && rm libgdrapi_2.5.1-1_amd64.Ubuntu24_04.deb


There's a critical issue with the installation of libgdrapi. The Docker image is based on Ubuntu 22.04 (from FINAL_BASE_IMAGE=nvidia/cuda:${CUDA_VERSION}-devel-ubuntu22.04), but the .deb package being downloaded is for Ubuntu 24.04. This version mismatch will likely cause the build to fail or lead to runtime issues due to dependency conflicts.

Additionally, the CUDA version (12.8) is hardcoded in the URL, which makes it brittle if the CUDA_VERSION build argument changes.

NVIDIA does not seem to provide a pre-built libgdrapi package for CUDA 12.8 on Ubuntu 22.04. To resolve this, you could consider one of the following:

Change the FINAL_BASE_IMAGE to an Ubuntu 24.04-based image, like nvidia/cuda:${CUDA_VERSION}-devel-ubuntu24.04, and verify that all other dependencies are compatible.

Revert to compiling gdrcopy from source for Ubuntu 22.04, even though the goal of this PR is to remove source compilation.

Make the URL and package name dynamic based on the OS and CUDA version, and add logic to handle cases where a pre-built package is not available.

Given the options, switching the base image to Ubuntu 24.04 might be the most straightforward solution if it doesn't introduce other compatibility problems.

gemini-code-assist · 2025-09-15T10:35:20Z

docs/serving/expert_parallel_deployment.md

+    RUN curl https://developer.download.nvidia.com/compute/redist/gdrcopy/CUDA%2012.8/ubuntu24_04/x64/libgdrapi_2.5.1-1_amd64.Ubuntu24_04.deb --output libgdrapi_2.5.1-1_amd64.Ubuntu24_04.deb \
+        && apt install ./libgdrapi_2.5.1-1_amd64.Ubuntu24_04.deb \
+        && rm libgdrapi_2.5.1-1_amd64.Ubuntu24_04.deb


The provided installation command for the gdrcopy dependency is highly specific to a single environment (Ubuntu 24.04 with CUDA 12.8) and will fail for most users. This can be very frustrating.

The documentation should provide more general guidance. For example, you could instruct users to install libgdrapi and link to the official gdrcopy installation page, while providing the specific command as an example for a particular setup.

Also, using RUN suggests a Docker command, which might be confusing for users setting up a host environment. It would be clearer to use commands appropriate for a user's shell (e.g., with sudo).

gemini-code-assist · 2025-09-15T10:35:20Z

docs/serving/expert_parallel_deployment.md

+    RUN curl https://developer.download.nvidia.com/compute/redist/gdrcopy/CUDA%2012.8/ubuntu24_04/x64/libgdrapi_2.5.1-1_amd64.Ubuntu24_04.deb --output libgdrapi_2.5.1-1_amd64.Ubuntu24_04.deb \
+        && apt install ./libgdrapi_2.5.1-1_amd64.Ubuntu24_04.deb \
+        && rm libgdrapi_2.5.1-1_amd64.Ubuntu24_04.deb


Similar to the previous comment, this installation command for gdrcopy is not portable and is specific to Ubuntu 24.04 with CUDA 12.8. This will cause issues for users with different setups. Please provide more general installation guidance, for instance by pointing to the official gdrcopy documentation and using this command as a specific example.

hmellor · 2025-09-15T10:40:27Z

Gemini's comments seem valid

Signed-off-by: bbartels <[email protected]>

pytorch-bot · 2025-09-15T10:45:20Z

No ciflow labels are configured for this repo.
For information on how to enable CIFlow bot see this wiki

Signed-off-by: bbartels <[email protected]>

pytorch-bot · 2025-09-15T11:30:59Z

No ciflow labels are configured for this repo.
For information on how to enable CIFlow bot see this wiki

bbartels · 2025-09-15T11:31:27Z

Gemini's comments seem valid

Added correct version of OS/uarch to dockefile

Signed-off-by: bbartels <[email protected]>

pytorch-bot · 2025-09-15T11:33:52Z

No ciflow labels are configured for this repo.
For information on how to enable CIFlow bot see this wiki

Signed-off-by: bbartels <[email protected]>

pytorch-bot · 2025-09-15T11:34:27Z

No ciflow labels are configured for this repo.
For information on how to enable CIFlow bot see this wiki

Signed-off-by: bbartels <[email protected]>

pytorch-bot · 2025-09-15T11:39:12Z

No ciflow labels are configured for this repo.
For information on how to enable CIFlow bot see this wiki

pytorch-bot · 2025-09-15T15:04:26Z

No ciflow labels are configured for this repo.
For information on how to enable CIFlow bot see this wiki

docker/Dockerfile

Signed-off-by: Benjamin Bartels <[email protected]>

Signed-off-by: bbartels <[email protected]>

tools/install_gdrcopy.sh

docker/Dockerfile

Signed-off-by: Benjamin Bartels <[email protected]>

bbartels · 2025-09-16T01:05:45Z

@NickLucche @hmellor Any further comments? Seems to have all passed now!

NickLucche

LGTM, thanks for your work @bbartels @cjackal.
Just left one note on the docs about GDRCopy being optional.

NickLucche · 2025-09-16T09:50:29Z

docs/serving/expert_parallel_deployment.md

 1. **Install DeepEP and pplx-kernels**: Set up host environment following vLLM's guide for EP kernels [here](gh-file:tools/ep_kernels).
 2. **Install DeepGEMM library**: Follow the [official instructions](https://github.com/deepseek-ai/DeepGEMM#installation).
-3. **For disaggregated serving**: Install UCX and NIXL following the [script](gh-file:tools/install_nixl.sh).
+3. **For disaggregated serving**: Install `gdrcopy` by running the [`install_gdrcopy.sh`](gh-file:tools/install_gdrcopy.sh) script (e.g., `install_gdrcopy.sh "${GDRCOPY_OS_VERSION}" "12.8" "x64"`). You can find available OS versions [here](https://developer.download.nvidia.com/compute/redist/gdrcopy/CUDA%2012.8/).


you're right thanks!

NickLucche · 2025-09-16T09:54:43Z

docs/serving/expert_parallel_deployment.md

 ### Setup Steps

-1. **Install KV Connector**: Install NIXL using the [installation script](gh-file:tools/install_nixl.sh)
+1. **Install gdrcopy/ucx/nixl**: Run the [`install_gdrcopy.sh`](gh-file:tools/install_gdrcopy.sh) script to install `gdrcopy` (e.g., `install_gdrcopy.sh "${GDRCOPY_OS_VERSION}" "12.8" "x64"`). You can find available OS versions [here](https://developer.download.nvidia.com/compute/redist/gdrcopy/CUDA%2012.8/). `nixl` and `ucx` are installed as dependencies via pip.


we should mention gdrcopy is for maximum performance, this should work even without it (plain pip install nixl)

docs/serving/expert_parallel_deployment.md

Signed-off-by: Benjamin Bartels <[email protected]>

Signed-off-by: bbartels <[email protected]> Signed-off-by: Benjamin Bartels <[email protected]> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Daniele <[email protected]>

Signed-off-by: bbartels <[email protected]> Signed-off-by: Benjamin Bartels <[email protected]> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Daniele <[email protected]> Signed-off-by: charlifu <[email protected]>

Signed-off-by: bbartels <[email protected]> Signed-off-by: Benjamin Bartels <[email protected]> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Daniele <[email protected]> Signed-off-by: xuebwang-amd <[email protected]>

Signed-off-by: bbartels <[email protected]> Signed-off-by: Benjamin Bartels <[email protected]> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Daniele <[email protected]>

Signed-off-by: bbartels <[email protected]> Signed-off-by: Benjamin Bartels <[email protected]> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Daniele <[email protected]> Signed-off-by: xuebwang-amd <[email protected]>

Removes source compilation of nixl dependency

ae8d4d4

Signed-off-by: bbartels <[email protected]>

bbartels requested review from NickLucche and hmellor as code owners September 15, 2025 10:33

mergify bot added the documentation Improvements or additions to documentation label Sep 15, 2025

mergify bot added the ci/build label Sep 15, 2025

gemini-code-assist bot reviewed Sep 15, 2025

View reviewed changes

Fixes ubuntu version

3231595

Signed-off-by: bbartels <[email protected]>

pytorch-bot bot removed the ci/build label Sep 15, 2025

mergify bot added the ci/build label Sep 15, 2025

Fixes aarch64

0a60259

Signed-off-by: bbartels <[email protected]>

pytorch-bot bot removed the ci/build label Sep 15, 2025

mergify bot added the ci/build label Sep 15, 2025

Fixes os version interpolation

2ef2f7d

Signed-off-by: bbartels <[email protected]>

pytorch-bot bot removed the ci/build label Sep 15, 2025

mergify bot added the ci/build label Sep 15, 2025

Fixes

83020eb

Signed-off-by: bbartels <[email protected]>

pytorch-bot bot removed the ci/build label Sep 15, 2025

mergify bot added the ci/build label Sep 15, 2025

Fixes path for arm

6cbed0e

Signed-off-by: bbartels <[email protected]>

pytorch-bot bot removed the ci/build label Sep 15, 2025

mergify bot added the ci/build label Sep 15, 2025

bbartels commented Sep 15, 2025

View reviewed changes

docker/Dockerfile Outdated Show resolved Hide resolved

bbartels added 3 commits September 15, 2025 16:47

Apply suggestions from code review

1c2eaec

Signed-off-by: Benjamin Bartels <[email protected]>

Merge branch 'main' into nixl-new

44250a5

Update Dockerfile

e69eb8d

Signed-off-by: bbartels <[email protected]>

bbartels force-pushed the nixl-new branch from 4436efd to e69eb8d Compare September 15, 2025 16:36

bbartels commented Sep 15, 2025

View reviewed changes

tools/install_gdrcopy.sh Show resolved Hide resolved

bbartels commented Sep 15, 2025

View reviewed changes

docker/Dockerfile Outdated Show resolved Hide resolved

bbartels added 2 commits September 15, 2025 23:29

Apply suggestions from code review

3d544a9

Signed-off-by: Benjamin Bartels <[email protected]>

Merge branch 'main' into nixl-new

2de1a78

bbartels requested a review from NickLucche September 15, 2025 22:35

NickLucche approved these changes Sep 16, 2025

View reviewed changes

bbartels commented Sep 16, 2025

View reviewed changes

docs/serving/expert_parallel_deployment.md Outdated Show resolved Hide resolved

bbartels added 2 commits September 16, 2025 11:27

Apply suggestions from code review

2923619

Signed-off-by: Benjamin Bartels <[email protected]>

Merge branch 'main' into nixl-new

bbb7c82

hmellor enabled auto-merge (squash) September 16, 2025 10:54

github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 16, 2025

bbartels added 3 commits September 16, 2025 12:39

Merge branch 'main' into nixl-new

6980c02

Merge branch 'main' into nixl-new

17ad028

Merge branch 'main' into nixl-new

beb3340

hmellor merged commit 64ad551 into vllm-project:main Sep 17, 2025
79 checks passed

cjackal mentioned this pull request Sep 18, 2025

[CI/build] Moves source compilation to build stage #25177

Closed

5 tasks

Uh oh!

Removes source compilation of nixl dependency #24874

Removes source compilation of nixl dependency #24874

Uh oh!

Conversation

bbartels commented Sep 15, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Test Plan

Test Result

Uh oh!

bbartels commented Sep 15, 2025

Uh oh!

pytorch-bot bot commented Sep 15, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

hmellor commented Sep 15, 2025

Uh oh!

pytorch-bot bot commented Sep 15, 2025

Uh oh!

pytorch-bot bot commented Sep 15, 2025

Uh oh!

bbartels commented Sep 15, 2025

Uh oh!

pytorch-bot bot commented Sep 15, 2025

Uh oh!

pytorch-bot bot commented Sep 15, 2025

Uh oh!

pytorch-bot bot commented Sep 15, 2025

Uh oh!

pytorch-bot bot commented Sep 15, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bbartels commented Sep 16, 2025

Uh oh!

NickLucche left a comment

Choose a reason for hiding this comment

Uh oh!

NickLucche Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

NickLucche Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

bbartels Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bbartels commented Sep 15, 2025 •

edited by github-actions bot

Loading