Skip to content

feat: add rdma_reject, rdma_get_devices and address getters - #116

Open
dragonJACson wants to merge 2 commits into
mainfrom
sideway-cm-improve
Open

feat: add rdma_reject, rdma_get_devices and address getters#116
dragonJACson wants to merge 2 commits into
mainfrom
sideway-cm-improve

Conversation

@dragonJACson

Copy link
Copy Markdown
Contributor

No description provided.

@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 adds support for rejecting connections, querying RDMA devices, and retrieving local and remote ports and socket addresses from an Identifier. The review feedback highlights several critical compilation issues, including calls to an undefined helper function port_from_raw_addr in tests, and incorrect pointer usage (self.cm_id.as_ref() instead of self.cm_id.as_ptr()) when calling FFI functions. Additionally, the get_devices function needs to be updated to correctly handle cases where no RDMA devices are present on the system without returning an error.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/rdmacm/communication_manager.rs Outdated
Comment thread src/rdmacm/communication_manager.rs
Comment thread src/rdmacm/communication_manager.rs Outdated
Comment thread src/rdmacm/communication_manager.rs
Comment thread src/rdmacm/communication_manager.rs
Comment thread src/rdmacm/communication_manager.rs
@codecov

codecov Bot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.87805% with 33 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/rdmacm/communication_manager.rs 79.87% 33 Missing ⚠️
Files with missing lines Coverage Δ
src/rdmacm/communication_manager.rs 86.80% <79.87%> (-1.87%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dragonJACson
dragonJACson force-pushed the sideway-cm-improve branch 3 times, most recently from 36de44f to d10e5f0 Compare July 1, 2026 01:28
@dragonJACson
dragonJACson requested a review from Copilot July 1, 2026 01:29

Copilot AI left a comment

Copy link
Copy Markdown

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 expands the RDMA CM wrapper API by adding connection rejection support, device enumeration via RDMA CM, and convenient accessors for local/peer addresses and ports, while reusing the existing global DeviceContext cache.

Changes:

  • Add Identifier::reject() wrapping rdma_reject, with a dedicated RejectError.
  • Add get_devices() wrapping rdma_get_devices and integrating results with the global DeviceContext cache.
  • Add Identifier getters for local/peer SocketAddr and for src/dst ports; update/add tests to cover new helpers and improve robustness.

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

Comment thread src/rdmacm/communication_manager.rs
@dragonJACson
dragonJACson changed the base branch from dev/add-private-data to main August 13, 2026 01:37
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.

2 participants