feat(rdmacm): support setting private data in rdmacm conn param#114
feat(rdmacm): support setting private data in rdmacm conn param#114dragonJACson wants to merge 2 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the Cirrus CI configuration, adds new keywords to Cargo.toml, and introduces helper methods in src/rdmacm/communication_manager.rs to retrieve event private data and configure connection parameters. Feedback on the changes highlights two critical safety issues: first, unconditionally accessing the param.conn union member in Event::private_data can cause undefined behavior for datagram port spaces or invalid event types; second, storing a raw pointer to a short-lived slice in ConnectionParameter::setup_private_data without lifetime tracking can lead to use-after-free bugs, meaning the function should be marked as unsafe or refactored.
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.
Codecov Report❌ Patch coverage is
... and 7 files with indirect coverage changes 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR extends the RDMA CM wrapper to support sending and receiving connection private data via RDMA CM connection parameters/events, and updates CI metadata to better validate RDMA functionality.
Changes:
- Add
Event::private_data()to expose peer-provided private data on connection events. - Add
ConnectionParameter::setup_private_data()to configure private data forconnect/accept. - Update CI/config metadata (GitHub Actions RXE test job, crate keywords, remove Cirrus CI config).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
src/rdmacm/communication_manager.rs |
Adds APIs to set connection private data and to read private data from connection events (plus extensive docs). |
Cargo.toml |
Extends crate keywords for discoverability. |
.github/workflows/test.yml |
Adds a new RXE-based CI job to run RDMA tests in a simulated environment and upload coverage. |
.cirrus.yml |
Removes Cirrus CI configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dragonJACson
force-pushed
the
dev/add-private-data
branch
from
4a52554 to
ca671fd
Compare
dragonJACson
force-pushed
the
dev/add-private-data
branch
3 times, most recently
from
d44d8da to
6b42c39
Compare
| /// A connection paramter used for configure the communication when connecting or establishing | ||
| /// datagram communication. Used in [`Identifier::connect`] and [`Identifier::accept`]. | ||
| pub struct ConnectionParameter(rdma_conn_param); | ||
| pub struct ConnectionParameter(rdma_conn_param, Vec<u8>); |
There was a problem hiding this comment.
The readability of such definition is low. Maybe we should prefer normal structs which have explicit name for each fields rather than using tuple structs here.
There was a problem hiding this comment.
Renamed the first field to conn_param, the second field to private_data
dragonJACson
force-pushed
the
dev/add-private-data
branch
from
6b42c39 to
08c8613
Compare
Signed-off-by: Luke Yue <lukedyue@gmail.com>
Signed-off-by: Luke Yue <lukedyue@gmail.com>
dragonJACson
force-pushed
the
dev/add-private-data
branch
from
08c8613 to
ee24cd6
Compare
For #111