Skip to content

Dynamic Loading IB verbs#253

Open
AtlantaPepsi wants to merge 2 commits into
ROCm:candidate-1.68from
AtlantaPepsi:ibdl
Open

Dynamic Loading IB verbs#253
AtlantaPepsi wants to merge 2 commits into
ROCm:candidate-1.68from
AtlantaPepsi:ibdl

Conversation

@AtlantaPepsi

@AtlantaPepsi AtlantaPepsi commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

Motivation

  • Dynamically loading ib verbs symbols
  • Minor refactor on dmabuf detection and export

Technical Details

  • Created a top level third-party/ folder for ibverbs related files. Will also harbor future external source which TransferBench depends
  • Created a separate minimal header IbvHeader.hpp for ib verbs structs and IbvDynLoad.hpp for dynamic loading and status report for ib verbs functionality.
  • Dynamic loading is a singleton and done once per process, and TransferBench header will probe in runtime if basic ibverbs function as well as dmabuf export is supported.
  • Also got rid HAVE_DMABUF_SUPPORT macro. Got rid of redundant dependency check on hsa header and rocr binaries (they are mandatory for AMD platform) in build process. Similar to ibv, it now dynamically checks for hsa_amd_portable_export_dmabuf symbol as part of check kernel support, and returns dmabuf support in runtime.

Test Plan

Test Result

Submission Checklist

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 introduces an IBV_DIRECT build/runtime switch intended to support resolving libibverbs symbols either via direct linkage or via dlopen/dlsym, and adds a runtime check to block NIC executor usage when verbs aren’t available.

Changes:

  • Added IBV_DIRECT mode plumbing (Makefile + CMake option) to toggle direct symbol binding vs dlsym resolution.
  • Introduced pfn_* function pointers and updated some verbs call sites/macros to call through them.
  • Added System::IbvLoaded() and a guard to error out when NIC executor is requested but verbs aren’t loaded.

Reviewed changes

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

File Description
src/header/TransferBench.hpp Adds libibverbs dynamic-loading infrastructure (pfn_*, Ibvdl()), runtime loaded-state tracking, and uses pfn_* in some verbs calls.
Makefile Adds DISABLE_IBV_DIRECT flag and sets -DIBV_DIRECT=1 by default when NIC exec is enabled.
CMakeLists.txt Adds ENABLE_IBV_DIRECT option and defines IBV_DIRECT=1 when enabled.

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

Comment thread src/header/TransferBench.hpp Outdated
Comment thread src/header/TransferBench.hpp Outdated
Comment thread src/header/TransferBench.hpp Outdated
Comment thread src/header/TransferBench.hpp Outdated
Comment thread src/header/TransferBench.hpp Outdated
Comment thread Makefile Outdated
Comment thread CMakeLists.txt Outdated
Comment thread src/header/TransferBench.hpp
Comment thread src/header/TransferBench.hpp Outdated
@AtlantaPepsi AtlantaPepsi changed the base branch from candidate to merge/TransferBench-v1.67.0 April 27, 2026 06:33
@AtlantaPepsi AtlantaPepsi marked this pull request as ready for review April 27, 2026 16:02
@AtlantaPepsi AtlantaPepsi requested review from a team as code owners April 27, 2026 16:02
@nileshnegi

Copy link
Copy Markdown
Collaborator

i'd say just merge into candidate for now.
@gilbertlee-amd mentioned we should get the other pod-presets merged into candidate as well, before promoting to develop

@AtlantaPepsi AtlantaPepsi changed the base branch from merge/TransferBench-v1.67.0 to candidate April 27, 2026 16:19
@AtlantaPepsi AtlantaPepsi marked this pull request as draft May 6, 2026 20:00
Copilot AI review requested due to automatic review settings June 9, 2026 21:11
@AtlantaPepsi AtlantaPepsi changed the base branch from candidate to candidate-1.68 June 9, 2026 21:12

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 6 out of 6 changed files in this pull request and generated 5 comments.

Comment thread Makefile Outdated
Comment thread CMakeLists.txt Outdated
Comment thread third-party/ibverbs/IbvDynLoad.hpp
Comment thread third-party/ibverbs/IbvDynLoad.hpp
Comment thread src/header/TransferBench.hpp
@AtlantaPepsi AtlantaPepsi marked this pull request as ready for review June 10, 2026 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants