Skip to content

[TransferEngine] Bandwidth-aware NIC selection + DMA-BUF and IPv6 binding fixes - #1974

Open
Nishikant-K-P wants to merge 2 commits into
kvcache-ai:mainfrom
Nishikant-K-P:fix/pci-topology-and-dmabuf-validation
Open

Nishikant-K-P wants to merge 2 commits into
kvcache-ai:mainfrom
Nishikant-K-P:fix/pci-topology-and-dmabuf-validation

Conversation

@Nishikant-K-P

@Nishikant-K-P Nishikant-K-P commented Apr 25, 2026 •

Copy link
Copy Markdown
Contributor

Description

Two-commit series. Replaces the legacy getPciDistance GPU↔NIC selection
heuristic with NCCL-style PCI ancestor-chain walking and bandwidth-aware
ranking, plus 23 hardware-free unit tests for the new logic. Each commit
is self-contained and bisectable.

Note: This PR was originally a four-commit series. Per reviewer
request to split, the IPv6 RPC binding fix moved to #2014 and the
DMA-BUF CUDA device index fix moved to #2015. This PR is now scoped to
the topology rewrite + tests, with all reviewer feedback applied.

  1. [TE] Replace getPciDistance with NCCL-style PCI topology and
    bandwidth-aware NIC selection.

    The old metric was a string-prefix-of-sysfs heuristic that broke on
    GB200/GB300 (GPUs and NICs on separate PCI domains share only
    /sys/devices/pci000 as common prefix → flat artifact distances → only
    1 NIC per NUMA selected as preferred). Replaced with NCCL-style PCI
    ancestor-chain walking + path-type classification (PIX / PXB / PHB /
    NODE / SYS) and a 3-level NIC ranking (path type → hops → PCIe bandwidth
    from current_link_speed * current_link_width). The bandwidth tiebreaker
    auto-filters lower-bandwidth front-end NICs to avail_hca without
    requiring MC_CUSTOM_TOPO_JSON or a manual blacklist. Pure-logic helpers
    (PciPathType, InfinibandDevice, normalizePciBusId,
    classifyGpuNicPath, selectPreferredHcas) moved to
    include/pci_topology.h for testability and shared use between
    discoverCudaTopology and the unit tests.

  2. [TE] Add 23 PCI topology unit tests.
    Hardware-free tests in pci_topology_test.cpp covering
    normalizePciBusId (5), classifyGpuNicPath (10 — all 5 path types
    plus edge cases), NIC selection ranking via the production
    selectPreferredHcas helper (6), and fallback paths (2).

Reviewer feedback addressed

  • Selection ranking extracted into selectPreferredHcas() in
    pci_topology.h; tests now exercise production code instead of a
    duplicate copy.
  • HCA ancestor chains pre-computed once before the GPU loop in
    discoverCudaTopology (was O(N_gpu × N_hca) sysfs walks, now
    O(N_gpu + N_hca)).
  • Redundant <ctype.h> removed from topology.cpp (kept <cctype>
    since the new code uses std::tolower / std::isxdigit).

Module

  • Transfer Engine (mooncake-transfer-engine)

Type of Change

  • Bug fix
  • Refactor

How Has This Been Tested?

  • Unit tests: 23 new tests in pci_topology_test.cpp (no hardware /
    sysfs dependency). All pass via ctest -R pci_topology.

  • GB300, single-host VRAM read (GPU 1 → GPU 0, auto-discovery, no custom
    topo):
    41.22 GB/s @ 64 MB block. The full 18 → 41 GB/s improvement
    requires this PR together with the companion DMA-BUF fix ([TE] Fix DMA-BUF validation using wrong CUDA device index #2015) —
    without that fix, only NICs at verbs index 0–3 pass DMA-BUF validation;
    with both PRs applied, all 10 NICs register and the topology rewrite
    promotes 8 of them to preferred_hca.

    BlkSize BW (GB/s) Avg Lat (μs) P99 Tx (μs)
    4 KB 0.51 8.1 8
    64 KB 6.10 10.7 11
    1 MB 29.10 36.0 36
    16 MB 40.27 416.7 421
    64 MB 41.22 1628.0 1635
  • GB300, same-node DRAM, auto-discovery: 41.34 GB/s @ 64 MB block;
    bandwidth tiebreaker correctly demotes Gen4 front-end NICs to
    avail_hca.

  • GB200 (2 GPUs, 6 NICs, same-node DRAM): auto-discovery with all 6
    NICs reaches 35.5 GB/s; flat topo with 4 backend NICs reaches 39.3 GB/s.

Checklist

  • I have performed a self-review of my own code.
  • I have formatted my own code using ./scripts/code_format.sh before submitting.
  • I have updated the documentation. (no user-facing API change; the
    bandwidth tiebreaker removes the need for MC_CUSTOM_TOPO_JSON on
    GB200/GB300, but the env var continues to work for manual overrides)
  • I have added tests to prove my changes are effective.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request implements a robust PCI topology discovery system, replacing basic distance metrics with ancestor chain classification and PCIe bandwidth analysis to better identify preferred GPU-NIC pairings. It also refines DMA-BUF support verification in the RDMA transport and adds comprehensive unit tests. Feedback suggests optimizing performance by hoisting CUDA driver initialization and pre-calculating PCI ancestor chains to reduce redundant sysfs operations.

Comment on lines 629 to 659
for (int cuda_device : mapped_cuda_devices) {
CUdevice cuDevice;
CUresult result = cuInit(0);
if (result != CUDA_SUCCESS) {
LOG(ERROR) << "Failed to initialize CUDA driver for RNIC "
<< device_name;
goto cleanup_context_and_devices;
}
result = cuDeviceGet(&cuDevice, cuda_device);
if (result != CUDA_SUCCESS) {
LOG(ERROR) << "Failed to query CUDA device " << cuda_device
<< " for RNIC " << device_name;
goto cleanup_context_and_devices;
}
int dmaBufSupported;
result = cuDeviceGetAttribute(&dmaBufSupported,
CU_DEVICE_ATTRIBUTE_DMA_BUF_SUPPORTED,
cuDevice);
if (result != CUDA_SUCCESS) {
LOG(ERROR) << "Failed to query CUDA device attributes for "
<< "CUDA device " << cuda_device << " and RNIC "
<< device_name;
goto cleanup_context_and_devices;
}
if (!dmaBufSupported) {
LOG(ERROR) << "DMA BUF supported required for GPU RDMA without "
"nvidia-peermem on CUDA device "
<< cuda_device << " mapped to RNIC " << device_name;
goto cleanup_context_and_devices;
}
}

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.

medium

cuInit(0) is a global initialization for the CUDA driver and only needs to be called once per process. Calling it inside the loop for every mapped CUDA device is redundant. It should be moved outside the loop to ensure it's only executed once if there are mapped devices.

        if (!mapped_cuda_devices.empty()) {
            CUresult result = cuInit(0);
            if (result != CUDA_SUCCESS) {
                LOG(ERROR) << "Failed to initialize CUDA driver for RNIC "
                           << device_name;
                goto cleanup_context_and_devices;
            }
            for (int cuda_device : mapped_cuda_devices) {
                CUdevice cuDevice;
                result = cuDeviceGet(&cuDevice, cuda_device);
                if (result != CUDA_SUCCESS) {
                    LOG(ERROR) << "Failed to query CUDA device " << cuda_device
                               << " for RNIC " << device_name;
                    goto cleanup_context_and_devices;
                }
                int dmaBufSupported;
                result = cuDeviceGetAttribute(
                    &dmaBufSupported, CU_DEVICE_ATTRIBUTE_DMA_BUF_SUPPORTED,
                    cuDevice);
                if (result != CUDA_SUCCESS) {
                    LOG(ERROR) << "Failed to query CUDA device attributes for "
                               << "CUDA device " << cuda_device << " and RNIC "
                               << device_name;
                    goto cleanup_context_and_devices;
                }
                if (!dmaBufSupported) {
                    LOG(ERROR)
                        << "DMA BUF supported required for GPU RDMA without "
                           "nvidia-peermem on CUDA device "
                        << cuda_device << " mapped to RNIC " << device_name;
                    goto cleanup_context_and_devices;
                }
            }
        }

Comment on lines +431 to 434
int device_count = 0;
if (cudaGetDeviceCount(&device_count) != cudaSuccess) {
device_count = 0;
}

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.

medium

To improve performance, pre-calculate the PCI ancestor chains for all HCAs once before entering the GPU loop. This avoids redundant sysfs lookups and realpath calls for each GPU-HCA pair, which can be significant in systems with many GPUs and NICs.

    int device_count = 0;
    if (cudaGetDeviceCount(&device_count) != cudaSuccess) {
        device_count = 0;
    }

    std::vector<std::vector<std::string>> nic_ancestor_chains;
    nic_ancestor_chains.reserve(all_hca.size());
    for (const auto &hca : all_hca) {
        nic_ancestor_chains.push_back(buildPciAncestorChain(hca.pci_bus_id));
    }

Comment on lines 462 to 483
for (const auto &hca : all_hca) {
if (isSameNumaNode(hca.pci_bus_id.c_str(), pci_bus_id)) {
same_numa_hca.push_back(hca);
auto nic_ancestor_chain = buildPciAncestorChain(hca.pci_bus_id);
auto [path_type, hops] = classifyGpuNicPath(
gpu_ancestor_chain, gpu_ancestors, gpu_numa_node, hca.numa_node,
nic_ancestor_chain);
hca_paths.push_back(HcaPathInfo{.hca = &hca,
.path_type = path_type,
.hops = hops,
.pci_bw = hca.pci_bw_gbps});

int hop_rank = hops >= 0 ? hops : INT_MAX;
if (static_cast<int>(path_type) <
static_cast<int>(best_path_type) ||
(path_type == best_path_type && hop_rank < best_hops)) {
best_path_type = path_type;
best_hops = hop_rank;
best_bw = hca.pci_bw_gbps;
} else if (path_type == best_path_type && hop_rank == best_hops &&
hca.pci_bw_gbps > best_bw) {
best_bw = hca.pci_bw_gbps;
}
}

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.

medium

Use the pre-calculated NIC ancestor chains instead of calling buildPciAncestorChain repeatedly inside the nested loop. This reduces the complexity of PCI topology discovery from $O(N_{gpu} \times N_{hca})$ to $O(N_{gpu} + N_{hca})$ expensive sysfs operations.

        for (size_t j = 0; j < all_hca.size(); ++j) {
            const auto &hca = all_hca[j];
            const auto &nic_ancestor_chain = nic_ancestor_chains[j];
            auto [path_type, hops] = classifyGpuNicPath(
                gpu_ancestor_chain, gpu_ancestors, gpu_numa_node, hca.numa_node,
                nic_ancestor_chain);
            hca_paths.push_back(HcaPathInfo{.hca = &hca,
                                            .path_type = path_type,
                                            .hops = hops,
                                            .pci_bw = hca.pci_bw_gbps});

            int hop_rank = hops >= 0 ? hops : INT_MAX;
            if (static_cast<int>(path_type) <
                    static_cast<int>(best_path_type) ||
                (path_type == best_path_type && hop_rank < best_hops)) {
                best_path_type = path_type;
                best_hops = hop_rank;
                best_bw = hca.pci_bw_gbps;
            } else if (path_type == best_path_type && hop_rank == best_hops &&
                       hca.pci_bw_gbps > best_bw) {
                best_bw = hca.pci_bw_gbps;
            }
        }

@codecov-commenter

codecov-commenter commented Apr 25, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 95.36424% with 14 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
mooncake-transfer-engine/src/topology.cpp 12.50% 14 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copilot AI 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.

Pull request overview

This PR improves Transfer Engine auto-discovery correctness on modern multi-NIC/multi-GPU systems by fixing RPC IPv6 binding, correcting DMA-BUF validation device mapping, and replacing PCI-distance heuristics with a topology-aware + bandwidth-aware NIC ranking (with new unit tests).

Changes:

  • Fix TENT RPC server binding to honor IPv6-only hosts ("::" vs 0.0.0.0).
  • Fix DMA-BUF validation to validate against topology-mapped CUDA devices (preferred + avail) instead of verbs enumeration index.
  • Add NCCL-style PCI ancestor-chain classification and bandwidth-aware NIC selection, plus new GTest coverage.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
mooncake-transfer-engine/tests/pci_topology_test.cpp Adds unit tests for PCI normalization/classification and NIC selection behaviors.
mooncake-transfer-engine/tests/CMakeLists.txt Adds pci_topology_test target to the test suite.
mooncake-transfer-engine/tent/src/rpc/rpc.cpp Fixes RPC server bind address selection for IPv6-only environments.
mooncake-transfer-engine/src/transport/rdma_transport/rdma_context.cpp Fixes DMA-BUF validation to use topology-mapped CUDA device indices (and initializes CUDA).
mooncake-transfer-engine/src/topology.cpp Introduces PCI ancestor-chain topology discovery and bandwidth-aware NIC ranking.
mooncake-transfer-engine/include/pci_topology.h Adds testable, pure PCI topology helpers (normalization + path classification) and InfinibandDevice definition.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


TEST(NormalizePciBusIdTest, LowercaseConversion) {
EXPECT_EQ(normalizePciBusId("0000:03:00.0"), "0000:03:00.0");
EXPECT_EQ(normalizePciBusId("0000:03:00.0"), "0000:03:00.0");

Copilot AI Apr 26, 2026

Copy link

Choose a reason for hiding this comment

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

NormalizePciBusIdTest.LowercaseConversion contains a duplicated assertion (same input/output repeated), which adds noise without increasing coverage. Remove the redundant EXPECT_EQ or replace it with a distinct mixed-case/non-normalized case.

Suggested change
EXPECT_EQ(normalizePciBusId("0000:03:00.0"), "0000:03:00.0");
EXPECT_EQ(normalizePciBusId("0000:03:0A.0"), "0000:03:0a.0");

Copilot uses AI. Check for mistakes.
Comment on lines +209 to +268
// Helper: replicate the selection logic from discoverCudaTopology to pick
// preferred vs avail NICs given a GPU and a set of HCAs.
struct NicSelection {
std::vector<std::string> preferred;
std::vector<std::string> avail;
};

static NicSelection selectPreferredNics(
const std::vector<std::string> &gpu_ancestor_chain, int gpu_numa_node,
const std::vector<std::pair<mooncake::InfinibandDevice,
std::vector<std::string>>> &hcas_with_chains) {
auto gpu_ancestors = toSet(gpu_ancestor_chain);

struct HcaPathInfo {
const mooncake::InfinibandDevice *hca;
PciPathType path_type;
int hops;
double pci_bw;
};

std::vector<HcaPathInfo> hca_paths;
PciPathType best_path_type = PciPathType::DIS;
int best_hops = INT_MAX;
double best_bw = 0.0;

for (const auto &[hca, nic_chain] : hcas_with_chains) {
auto [path_type, hops] =
classifyGpuNicPath(gpu_ancestor_chain, gpu_ancestors, gpu_numa_node,
hca.numa_node, nic_chain);
hca_paths.push_back({&hca, path_type, hops, hca.pci_bw_gbps});

int hop_rank = hops >= 0 ? hops : INT_MAX;
if (static_cast<int>(path_type) < static_cast<int>(best_path_type) ||
(path_type == best_path_type && hop_rank < best_hops)) {
best_path_type = path_type;
best_hops = hop_rank;
best_bw = hca.pci_bw_gbps;
} else if (path_type == best_path_type && hop_rank == best_hops &&
hca.pci_bw_gbps > best_bw) {
best_bw = hca.pci_bw_gbps;
}
}

NicSelection result;
for (const auto &info : hca_paths) {
bool is_preferred = info.path_type == best_path_type;
if (is_preferred && best_hops != INT_MAX) {
is_preferred = info.hops == best_hops;
}
if (is_preferred && best_bw > 0.0) {
is_preferred = info.pci_bw >= best_bw;
}
if (is_preferred) {
result.preferred.push_back(info.hca->name);
} else {
result.avail.push_back(info.hca->name);
}
}
return result;
}

Copilot AI Apr 26, 2026

Copy link

Choose a reason for hiding this comment

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

This test file re-implements the NIC selection logic from discoverCudaTopology in selectPreferredNics(). As written, it doesn't exercise the production selection code and can silently diverge if the real algorithm changes. Consider extracting the ranking/partitioning logic into a shared pure helper (e.g., in pci_topology.h/.cpp) and calling it from both discoverCudaTopology and these tests.

Copilot uses AI. Check for mistakes.
Comment on lines +39 to +40
double
pci_bw_gbps; // PCIe bandwidth in GT/s * width (e.g. 1024 for Gen5 x16)

Copilot AI Apr 26, 2026

Copy link

Choose a reason for hiding this comment

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

pci_bw_gbps is documented/used as a bandwidth value, but it is computed as current_link_speed (GT/s) * current_link_width (a relative metric, not Gbps). The field name and example ("1024 for Gen5 x16") are misleading and can confuse future consumers. Consider renaming the field (e.g., pci_bw_score) and/or clarifying the units and example to match the actual calculation.

Suggested change
double
pci_bw_gbps; // PCIe bandwidth in GT/s * width (e.g. 1024 for Gen5 x16)
double pci_bw_gbps; // Relative PCIe link score: current_link_speed (GT/s)
// * current_link_width; not literal Gbps
// (e.g. 512 for Gen5 x16).

Copilot uses AI. Check for mistakes.
Comment on lines +191 to +207
// Read PCIe link speed and width to compute bandwidth.
// Used to distinguish high-bandwidth backend NICs from
// lower-bandwidth front-end/management NICs.
double pci_bw_gbps = 0.0;
{
double speed_gts = 0.0;
int width = 0;
snprintf(path, sizeof(path), "%s/current_link_speed",
resolved_path);
std::ifstream speed_file(path);
if (speed_file.is_open()) speed_file >> speed_gts;
snprintf(path, sizeof(path), "%s/current_link_width",
resolved_path);
std::ifstream width_file(path);
if (width_file.is_open()) width_file >> width;
pci_bw_gbps = speed_gts * width;
}

Copilot AI Apr 26, 2026

Copy link

Choose a reason for hiding this comment

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

PCIe “bandwidth” is computed as speed_gts * width, but the variable is named pci_bw_gbps, which implies a real Gbps value. Since this value is used as a ranking/tiebreaker, consider renaming it (or adjusting the comment) to reflect that it’s a relative score derived from link speed/width, not an actual bandwidth figure.

Copilot uses AI. Check for mistakes.
Comment on lines +629 to +636
for (int cuda_device : mapped_cuda_devices) {
CUdevice cuDevice;
CUresult result = cuInit(0);
if (result != CUDA_SUCCESS) {
LOG(ERROR) << "Failed to initialize CUDA driver for RNIC "
<< device_name;
goto cleanup_context_and_devices;
}

Copilot AI Apr 26, 2026

Copy link

Choose a reason for hiding this comment

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

cuInit(0) is called inside the loop for each mapped CUDA device. cuInit is process-global and idempotent, so repeatedly calling it here adds avoidable overhead on every RNIC open. Initialize CUDA once before the for (int cuda_device : mapped_cuda_devices) loop (or guard with a static/once_flag) and keep per-device checks inside the loop.

Suggested change
for (int cuda_device : mapped_cuda_devices) {
CUdevice cuDevice;
CUresult result = cuInit(0);
if (result != CUDA_SUCCESS) {
LOG(ERROR) << "Failed to initialize CUDA driver for RNIC "
<< device_name;
goto cleanup_context_and_devices;
}
CUresult result = cuInit(0);
if (result != CUDA_SUCCESS) {
LOG(ERROR) << "Failed to initialize CUDA driver for RNIC "
<< device_name;
goto cleanup_context_and_devices;
}
for (int cuda_device : mapped_cuda_devices) {
CUdevice cuDevice;

Copilot uses AI. Check for mistakes.
#include <unordered_set>
#include <utility>
#include <vector>
#include <ctype.h>

Copilot AI Apr 26, 2026

Copy link

Choose a reason for hiding this comment

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

topology.cpp now includes both <cctype> and <ctype.h>, which is redundant. Since the new code uses the C++ wrappers (std::tolower, std::isxdigit), prefer keeping <cctype> and dropping <ctype.h> to avoid mixing C/C++ headers.

Suggested change
#include <ctype.h>

Copilot uses AI. Check for mistakes.
if (result != CUDA_SUCCESS) {
LOG(ERROR) << "Failed to query CUDA device";
return ERR_CONTEXT;
// Verify DMA-BUF support against the CUDA device(s) that the local

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not sure whether other (domestic) GPU vendors could support this. Apply this update to NVIDIA only.

@staryxchen

Copy link
Copy Markdown
Collaborator

I suggest splitting this into multiple PRs, for two reasons: 1. It makes review easier; 2. We typically use squash merges, which combine multiple commits into a single commit in main branch, making it difficult to bisect them later.

@Nishikant-K-P
Nishikant-K-P force-pushed the fix/pci-topology-and-dmabuf-validation branch from e44bceb to 198d713 Compare April 29, 2026 16:13
…h-aware NIC selection

The old GPU-to-NIC topology used getPciDistance(), which resolved sysfs
paths for both devices, found their common string prefix, and counted
'/' separators in the remaining tails as a "distance" metric. This had
two problems:

1. On GB200/GB300 where GPUs and NICs are on separate PCI domains (e.g.
   pci0008:00 vs pci0000:00), the common prefix is just "/sys/devices/
   pci000" — a string artifact, not a topological relationship. NICs
   with shorter sysfs paths got artificially lower distances, causing
   only 1 NIC per NUMA to be selected as preferred.

2. The flat distance number lost the semantic distinction between NICs
   on the same PCI switch (very fast) vs same host bridge (moderate) vs
   same NUMA with no PCI relationship (slower).

Replace with a PCI ancestor chain walking approach inspired by NCCL:
- buildPciAncestorChain() walks up the sysfs tree via realpath("../")
  to build the full chain of PCI ancestors for each device
- classifyGpuNicPath() finds the lowest common ancestor between GPU
  and NIC chains, counts hops on each side, and classifies the path:
  - PIX (total hops <= 2): same PCI switch
  - PXB (total hops <= 4): crossing PCI bridges
  - PHB (total hops > 4):  crossing host bridge
  - NODE: same NUMA, no common PCI ancestor
  - SYS:  different NUMA nodes

Redesign NIC selection as a 3-level preference hierarchy replacing the
old single-metric min-distance approach:
  1. Best (lowest) PCI path type wins
  2. Fewest hops within the same path type
  3. Highest PCIe bandwidth (link speed * width from sysfs)

NODE ranking uses a fixed hop count of 0 so that all same-NUMA NICs
without a common PCI ancestor are equally preferred. PCI chain length
is not a meaningful proximity metric when there is no shared ancestor.

Include PCI root bus names (e.g. "pci0008:00") in ancestor chains by
also accepting "pci"-prefixed directory names in getPciParent(). This
allows devices behind different root ports on the same root complex to
find their common ancestor and be correctly classified as PHB instead
of falling through to NODE.

The PCIe bandwidth tiebreaker auto-filters lower-bandwidth front-end
NICs. On GB300, front-end NICs (mlx5_4, mlx5_9) are Gen4 x16 (32 GT/s)
while backend NICs are Gen5 x16 (64 GT/s). Both classify as NODE with
the same hop count, but the bandwidth tiebreaker moves front-end NICs
to avail_hca without requiring MC_CUSTOM_TOPO_JSON or a manual
blacklist. The same filter is applied to CPU topology discovery.

Extract pure-logic helpers to include/pci_topology.h for testability:
PciPathType enum, InfinibandDevice struct, normalizePciBusId(), and
classifyGpuNicPath(). The sysfs-dependent helpers remain static in
topology.cpp.

Tested on:
- GB300 (4 GPUs, 10 NICs, same-node DRAM, auto-discovery, no custom
  topo): selects 8 Gen5 backend NICs as preferred, filters 2 Gen4
  front-end NICs to avail. 41 GB/s vs old code's 18 GB/s (2.2x).
- GB200 (2 GPUs, 6 NICs, same-node DRAM): auto-discovery with all
  6 NICs reaches 35.5 GB/s; flat topo with 4 backend NICs reaches
  39.3 GB/s.
- GB300 single-host VRAM read (GPU 1 -> GPU 0, auto-discovery, no
  custom topo): 41.22 GB/s @ 64MB block. mlx5_4 and mlx5_9 register
  last in the avail_hca tier, reflecting the bandwidth tiebreaker
  classifying them as front-end (also visible by their GID prefix
  belonging to a distinct subnet from the 8 backend NICs).

    BlkSize       BW (GB/s)    Avg Lat (us)    P99 Tx (us)
    4 KB          0.51         8.1             8
    64 KB         6.10         10.7            11
    1 MB          29.10        36.0            36
    16 MB        40.27         416.7           421
    64 MB        41.22         1628.0          1635

  Without the topology rewrite, the old getPciDistance() metric
  selected only 1 NIC per NUMA on GB300 (4 NICs total), capping
  throughput at ~18 GB/s.
Add 23 unit tests in pci_topology_test.cpp covering the PCI topology
classification and NIC selection logic. All tests are pure-logic with
no hardware or sysfs dependency.

NormalizePciBusIdTest (5 tests):
- Lowercase conversion
- 8-digit to 4-digit PCI domain stripping (CUDA vs sysfs format)
- Already-normalized passthrough
- Non-zero leading digits preserved
- Mixed case with domain stripping

ClassifyGpuNicPathTest (10 tests):
- PIX: same PCI switch (hops <= 2)
- PXB: crossing PCI bridges (hops 3-4)
- PHB: crossing host bridge (hops > 4)
- NODE: same NUMA, no common ancestor
- SYS: different NUMA nodes
- PHB via shared root bus with different root ports
- NODE with different root buses (separate PCI domains)
- Unknown NUMA (-1) edge cases

NicSelectionTest (6 tests):
- GB200/GB300: all same-NUMA NICs preferred (equal bandwidth)
- GB300: bandwidth tiebreaker filters out Gen4 front-end NICs
- GB300: equal bandwidth means all same-NUMA NICs preferred
- All NICs under same switch (PIX, all preferred)
- PIX preferred over NODE in mixed topology
- PHB preferred over NODE with shared root bus (H100-like)

FallbackPathTest (2 tests):
- Disabled preferred NIC falls back to avail_hca via selectDevice
- Avail NIC present in topology for DMA-BUF validation coverage
@Nishikant-K-P
Nishikant-K-P force-pushed the fix/pci-topology-and-dmabuf-validation branch from 198d713 to 4d93c9a Compare April 29, 2026 16:19
@Nishikant-K-P

Copy link
Copy Markdown
Contributor Author

@staryxchen just following up on the review

@staryxchen

Copy link
Copy Markdown
Collaborator

@staryxchen just following up on the review

Sorry for late reply. Could you push an empty commit to re-run CI? And update PR title to reflect actual code changes.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

This PR has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Please update the PR or comment to keep it open.

@github-actions github-actions Bot added the stale label Oct 7, 2026

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants