Skip to content

[rocshmem] GDA Multi-NIC support - #4652

Merged
amd-arozanov merged 6 commits into
developfrom
users/arozanov/rocshmem-multi-nic
Apr 17, 2026
Merged

amd-arozanov merged 6 commits into
developfrom
users/arozanov/rocshmem-multi-nic

Conversation

@amd-arozanov

@amd-arozanov amd-arozanov commented Apr 2, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

rocSHMEM currently uses a single NIC per GPU. To enable NIC fusion (aggregating bandwidth across multiple NICs), the host-side infrastructure needs to support selecting, opening, and creating RDMA resources (PD, MR, CQ, QP) for more than one NIC. Additionally, users need control over how QPs are distributed across NICs.

Default behavior is unchanged: without new env vars, each PE selects exactly one NIC as before.

Technical Details

  • New env vars:
    • ROCSHMEM_GDA_MERGE_NICS - enables/disables multi-NIC selection
    • ROCSHMEM_GDA_NET_MERGE_LEVEL - controls the merge level:
      • PIX - same PCIe switch
      • PXB - same PCIe root complex
      • PHB - same NUMA node
      • SYS - all available NICs
    • ROCSHMEM_GDA_NET_FORCE_MERGE - comma-separated list of NIC device names; semicolon-separated rank groups.
      Example: "mlx5_0,mlx5_1;mlx5_2,mlx5_3"
      Local PE 0 will select mlx5_0 and mlx5_1
      Local PE 1 will select mlx5_2 and mlx5_3
    • ROCSHMEM_GDA_NIC_POLICY - NIC-to-QP binding mode:
      • ROUND_ROBIN (default): QP rows within a context are interleaved across NICs
        Requires NUM_QPS_PER_PE_DEFAULT_CTX or NUM_QPS_PER_PE_USR_CTX to be set to at least the number of selected NICs. If fewer QP rows are available than NICs, num_nics is automatically reduced to the max QP-per-PE count so that each active NIC has at least one QP row.
        Example with 4 NICs and 4 QPs per PE:
        ROCSHMEM_GDA_MERGE_NICS=1
        ROCSHMEM_GDA_NET_MERGE_LEVEL=PHB
        ROCSHMEM_GDA_NIC_POLICY=ROUND_ROBIN
        ROCSHMEM_GDA_NUM_QPS_PER_PE_DEFAULT_CTX=4
        # All contexts will have: QP row 0 -> NIC 0, row 1 -> NIC 1, row 2 -> NIC 2, row 3 -> NIC 3
        
      • PER_CONTEXT: all QPs in a context use one NIC; multi-NIC usage is achieved via multiple contexts. Does not require multiple QPs per PE.
        Example with 4 NICs and 4 contexts:
        ROCSHMEM_GDA_MERGE_NICS=1
        ROCSHMEM_GDA_NET_MERGE_LEVEL=PHB
        ROCSHMEM_GDA_NIC_POLICY=PER_CONTEXT
        ROCSHMEM_MAX_NUM_CONTEXTS=4
        # Context 0 -> NIC 0, Context 1 -> NIC 1, Context 2 -> NIC 2, Context 3 -> NIC 3
        
  • Add GetClosestNicsToGpu(), which returns multiple NICs sorted by PCIe proximity and filtered by merge level
  • Store selected NICs and RDMA objects in nic_devices_ vector (NicDevice struct with per-NIC context, PD, MR, port info, GID)
  • Add nic_for_qp() / nic_idx_for_qp(), which map flat QP index to the correct NicDevice based on active NIC policy
  • Loop over NICs in open_ib_device(), setup_heap_memory_rkey(), QP create/modify/destroy, and provider backends (BNXT, IONIC, MLX5)
  • Add unit tests for topology helpers, NIC list parsing, rank groups, filtered addresses, and ComputeNicIdxForQp (both modes)

JIRA ID

AICOMNET-69

AICOMNET-85 - functional and performance tests results

AIROCSHMEM-223

Test Plan

  1. CI tests - existing functional test suite (driver.sh all)
  2. NIC selection - verified correct NIC selection across configurations:
    • MERGE_NICS=0 (default, single NIC)
    • MERGE_NICS=1 with MERGE_LEVEL=PIX, PXB, PHB, SYS
    • FORCE_MERGE with explicit NIC names
    • NIC_POLICY=ROUND_ROBIN and PER_CONTEXT
    • ppn = 1, 2, 4, 8
    • NICs: Thor2, AINIC
  3. Unit tests (topology_gtest):

Test Result

Passed

Submission Checklist

@amd-arozanov amd-arozanov self-assigned this Apr 2, 2026
@amd-arozanov
amd-arozanov requested a review from a team as a code owner April 2, 2026 16:48
Copilot AI review requested due to automatic review settings April 2, 2026 16:48

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 adds host-side infrastructure to enable selecting and initializing multiple RDMA NICs per GPU (as a prerequisite for NIC fusion), including topology-based NIC ranking/filtering, env-var driven selection, and per-NIC RDMA resource allocation.

Changes:

  • Introduces NIC path/merge concepts (PIX/PXB/PHB/SYS), parsing helpers, and multi-NIC selection via GetClosestNicsToGpu().
  • Refactors the GDA backend to manage per-NIC contexts/PDs/MRs and map QPs to the correct NIC (nic_devices_, nic_for_qp()).
  • Adds/expands unit tests for topology helpers, NIC filtering, and multi-NIC selection behavior.

Reviewed changes

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

Show a summary per file
File Description
projects/rocshmem/tests/unit_tests/topology_gtest.cpp Adds unit tests for merge-level parsing, NIC list parsing, filtered NIC address building, and multi-NIC selection.
projects/rocshmem/src/gda/topology.hpp Adds NicPathType and new public APIs/helpers for merge parsing, NIC filtering, path classification, and multi-NIC selection.
projects/rocshmem/src/gda/topology.cpp Implements new parsing/filtering/path classification helpers and GetClosestNicsToGpu().
projects/rocshmem/src/gda/backend_gda.hpp Introduces NicDevice, tracks multiple NICs, and adds QP→NIC mapping helpers.
projects/rocshmem/src/gda/backend_gda.cpp Implements multi-NIC selection, per-NIC device open/PD/MR setup, QP init using per-NIC keys, and per-NIC teardown.
projects/rocshmem/src/gda/mlx5/backend_gda_mlx5.cpp Routes MLX5 QP creation and key usage through the correct per-NIC context/PD/MR.
projects/rocshmem/src/gda/ionic/backend_gda_ionic.cpp Routes IONIC CQ/QP creation and key usage through the correct per-NIC context/PD/MR; moves parent-domain setup per NIC.
projects/rocshmem/src/gda/bnxt/backend_gda_bnxt.cpp Routes BNXT CQ/QP creation and key usage through the correct per-NIC context/PD/MR and port attributes.
projects/rocshmem/src/envvar.hpp Declares new GDA env vars for multi-NIC behavior.
projects/rocshmem/src/envvar.cpp Defines new GDA env vars (MERGE_NICS, NET_MERGE_LEVEL, NET_FORCE_MERGE).

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

@amd-arozanov amd-arozanov changed the title [rocshmem] Multi-NIC support: host-side topology, NIC selection, and per-NIC resource allocation [rocshmem] Multi-NIC support: host-side topology, NIC selection, and per-NIC resource allocation [WIP] Apr 7, 2026
@amd-arozanov
amd-arozanov force-pushed the users/arozanov/rocshmem-multi-nic branch from df87748 to 496bccb Compare April 7, 2026 12:37
@amd-arozanov
amd-arozanov force-pushed the users/arozanov/rocshmem-multi-nic branch 2 times, most recently from 9ee8b5e to 039d5a6 Compare April 10, 2026 08:37
@amd-arozanov
amd-arozanov requested a review from Copilot April 10, 2026 09:02

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

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


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

Comment thread projects/rocshmem/src/gda/topology.cpp Resolved
@amd-arozanov
amd-arozanov force-pushed the users/arozanov/rocshmem-multi-nic branch from 039d5a6 to 7c12f1a Compare April 10, 2026 09:40
@amd-arozanov
amd-arozanov requested a review from Copilot April 10, 2026 09:43
@amd-arozanov amd-arozanov changed the title [rocshmem] Multi-NIC support: host-side topology, NIC selection, and per-NIC resource allocation [WIP] [rocshmem] GDA Multi-NIC support Apr 10, 2026

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

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


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

Comment thread projects/rocshmem/src/gda/topology.cpp Resolved
@amd-arozanov
amd-arozanov requested review from a team and Yiltan April 10, 2026 10:57
@amd-arozanov
amd-arozanov force-pushed the users/arozanov/rocshmem-multi-nic branch from 7c12f1a to ddaa80b Compare April 13, 2026 11:51
@amd-arozanov

Copy link
Copy Markdown
Contributor Author

PR is ready for review

@amd-arozanov
amd-arozanov force-pushed the users/arozanov/rocshmem-multi-nic branch from 1d08518 to e216070 Compare April 14, 2026 14:50

@Yiltan Yiltan 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.

LGTM

Note: you may want to rebase on develop -Werror was introduce (just want to make sure things compile after merge)

@amd-arozanov
amd-arozanov force-pushed the users/arozanov/rocshmem-multi-nic branch from e216070 to 5574d00 Compare April 14, 2026 14:54
@amd-arozanov

Copy link
Copy Markdown
Contributor Author

@Yiltan, @abouteiller, @edgargabriel, can I merge this PR, or are we waiting more reviews?

@amd-arozanov

Copy link
Copy Markdown
Contributor Author

@Yiltan, I rebased this PR and the build with -Werror passed

Add topology-based multi-NIC selection and per-NIC IB resource allocation.

Default behavior unchanged - single closest NIC is selected unless ROCSHMEM_GDA_MERGE_NICS=1 is set.

Key changes:
- NicDevice struct and nic_devices_ vector for per-NIC resources
- PCIe topology functions (ComputeGpuNicPathType, GetClosestNicsToGpu)
- NIC filtering (include/exclude lists, merge level, force-merge)
- Enforce num_qps_per_pe >= num_nics_ so each NIC gets a QP row
- Provider backends (BNXT, IONIC, MLX5) use nic_for_qp() for per-NIC access
- Unit tests for topology and NIC selection helpers
Add ROCSHMEM_GDA_NIC_POLICY env var with two modes:
- ROUND_ROBIN (default): QPs within a context spread across NICs
- PER_CONTEXT: all QPs in a context use one NIC, multi-NIC via
  multiple contexts (ctx_id % num_nics)

Extract ComputeNicIdxForQp() into nic_policy.hpp for testability.
Remove auto-increase of num_qps_per_pe to max(env, num_nics).

Add multi-context put functional test (MultiCtxPutTestType=94) that
creates 4 contexts and performs put_nbi through each. Add unit tests
for ComputeNicIdxForQp covering both modes.

Made-with: Cursor
@amd-arozanov
amd-arozanov force-pushed the users/arozanov/rocshmem-multi-nic branch 3 times, most recently from 3defa04 to 5d6a521 Compare April 15, 2026 15:52
Comment thread projects/rocshmem/src/envvar.cpp Resolved
Comment thread projects/rocshmem/src/gda/topology.cpp Resolved
@amd-arozanov
amd-arozanov force-pushed the users/arozanov/rocshmem-multi-nic branch from 5d6a521 to bff376e Compare April 16, 2026 08:33
@amd-arozanov
amd-arozanov merged commit bf861a3 into develop Apr 17, 2026
11 checks passed
@amd-arozanov
amd-arozanov deleted the users/arozanov/rocshmem-multi-nic branch April 17, 2026 08:06
@amd-arozanov
amd-arozanov restored the users/arozanov/rocshmem-multi-nic branch April 17, 2026 08:06
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.

4 participants