Skip to content

Conversation

@dragonJACson
Copy link
Contributor

@dragonJACson dragonJACson commented Nov 23, 2025

  • Add helper functions to create AddressHandle, setup_ud_addr.
  • Fix segfault when ibv_create_qp_ex returns NULL.
  • Provide an initial demo for UD pingpong.

This should close #64

@dragonJACson dragonJACson requested a review from FujiZ November 23, 2025 08:55
@gemini-code-assist
Copy link

Summary of Changes

Hello @dragonJACson, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the sideway RDMA library by introducing initial support for User Datagram (UD) communication. It provides a concrete example of UD ping-pong, alongside the necessary API extensions for managing RDMA Address Handles and configuring UD-specific send operations. These changes lay the groundwork for building applications that leverage unreliable datagrams over RDMA, expanding the library's utility for various high-performance networking scenarios.

Highlights

  • New UD Ping-Pong Example: Introduced a new example, ud_pingpong_split.rs, which demonstrates User Datagram (UD) ping-pong communication using the sideway RDMA library. This example includes full setup, connection, and performance measurement capabilities.
  • Address Handle Management: Added a new AddressHandle struct and associated error handling (CreateAddressHandleError) to src/ibverbs/address.rs, allowing for proper encapsulation and management of RDMA Address Handles.
  • User Datagram (UD) Send Support: Extended the QueuePair and PostSendGuard functionalities to support UD send operations, including methods to specify the Address Handle, remote Queue Pair Number (QPN), and Queue Key (QKey) for UD packets.
  • Improved Queue Pair Creation: Added a null check during the creation of QueuePair instances using ibv_create_qp_ex to provide more robust error handling.
  • Port and Queue Key Accessors: Added lid() to PortAttr for retrieving the Local Identifier and setup_qkey()/qkey() to QueuePairAttribute for managing the Queue Key.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@codecov
Copy link

codecov bot commented Nov 23, 2025

Codecov Report

❌ Patch coverage is 72.72727% with 63 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/ibverbs/queue_pair.rs 6.45% 29 Missing ⚠️
src/ibverbs/address.rs 86.45% 26 Missing ⚠️
src/ibverbs/protection_domain.rs 0.00% 5 Missing ⚠️
src/ibverbs/device_context.rs 0.00% 3 Missing ⚠️
Files with missing lines Coverage Δ
src/ibverbs/device_context.rs 85.54% <0.00%> (-0.63%) ⬇️
src/ibverbs/protection_domain.rs 71.42% <0.00%> (-15.53%) ⬇️
src/ibverbs/address.rs 85.39% <86.45%> (+1.65%) ⬆️
src/ibverbs/queue_pair.rs 68.68% <6.45%> (-1.92%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

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 introduces an initial version of an Unreliable Datagram (UD) ping-pong example, along with the necessary API extensions in the sideway library to support UD queue pairs. The changes to the core library are well-structured, adding support for address handles and UD-specific send operations with proper error handling and resource management. The new ud_pingpong_split.rs example, however, could be improved. It currently relies heavily on unwrap(), expect(), and panic!() for error handling, which makes the code brittle. Switching to Result propagation would make the example more robust and a better showcase of idiomatic Rust. There are also a few other areas for improvement in the example, such as handling an ignored Result and simplifying a function with too many arguments.

Comment on lines 250 to 258
fn parse_single_work_completion(
&self, wc: &ExtendedWorkCompletion, ts_param: &mut TimeStamps, scnt: &mut u32, rcnt: &mut u32,
outstanding_send: &mut bool, rout: &mut u32, rx_depth: u32, need_post_recv: &mut bool, to_post_recv: &mut u32,
use_ts: bool,
) {

Choose a reason for hiding this comment

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

medium

The function parse_single_work_completion has a large number of arguments (11), which harms readability and maintainability. This is why clippy::too_many_arguments is disabled for this file. Consider grouping related arguments into a state-holding struct. For instance, counters (scnt, rcnt, rout), flags (outstanding_send, need_post_recv), and post-receive parameters (to_post_recv) could be encapsulated within a PingPongState struct passed by mutable reference. This would simplify the function signature and improve code organization.

@dragonJACson dragonJACson marked this pull request as draft November 23, 2025 08:58
@dragonJACson
Copy link
Contributor Author

I provide an initial support for UD QP in this commit, while a lot of interface design left to be discussed.

  1. I add setup_ud_send and setup_ud_send_imm for passing AddressHandle when creating a send handle, does this look good to you @FujiZ? We don't have compile time check for users to create a RDMA WRITE handle on UD QP though, I guess that would force us to separate QPs into RC QP, UD QP and UC QP.
  2. User need to treat the first 40 bytes of recv buffer as global routing header, should we provide a helper function for that? Or just let user handle it?

@dragonJACson dragonJACson changed the title WIP: feat: provide an initial version of UD pingpong WIP: feat: provide vital things to make UD work Nov 23, 2025
@dragonJACson dragonJACson added this to the Release v0.5.0 milestone Nov 23, 2025
@dragonJACson dragonJACson force-pushed the dev/ud-and-uc-adaption branch 4 times, most recently from 7780437 to 5fd86de Compare November 28, 2025 01:47
- Surface port LIDs from `PortAttr`.
- Add `ProtectionDomain::create_ah` for creating address handle.
- Extend queue-pair attributes/guards with QKey accessors and new
`setup_ud_send{,_imm}` helpers, wiring them through basic, extended,
and generic post-send guards via `ibv_wr_set_ud_addr`

Signed-off-by: Luke Yue <lukedyue@gmail.com>
`ibv_qp_to_qp_ex` would dereference the result of `ibv_create_qp_ex`, a
null pointer dereferencing would prevent `NonNull::new().ok_or()?`
returning a `CreateQueuePairError`.

Checking the pointer before passing it to `ibv_qp_to_qp_ex` to fix it.

Signed-off-by: Luke Yue <lukedyue@gmail.com>
@dragonJACson dragonJACson force-pushed the dev/ud-and-uc-adaption branch 3 times, most recently from 87c7460 to 397dac1 Compare November 28, 2025 13:37
Signed-off-by: Luke Yue <lukedyue@gmail.com>
@dragonJACson dragonJACson force-pushed the dev/ud-and-uc-adaption branch 2 times, most recently from e8e4b54 to 20f0dc7 Compare November 28, 2025 13:51
…uffers

Add a smoltcp-style `GlobalRoutingHeader<T>` wrapper that provides
zero-copy read/write access to Global Routing Header fields directly
from byte buffers.

- `new_unchecked` / `new_checked` constructors for buffer wrapping
- Field accessors: version, traffic_class, flow_label, payload_length,
  next_header, hop_limit, source_gid, destination_gid
- Mutable setters for all fields when buffer is writable
- `as_ptr` / `as_mut_ptr` for FFI interop
- `grh` for copying to owned `ibv_grh` struct

Also add `--debug-grh` flag to ud_pingpong_split example to print
received GRH on each iteration for debugging purposes.

Signed-off-by: Luke Yue <lukedyue@gmail.com>
@dragonJACson dragonJACson force-pushed the dev/ud-and-uc-adaption branch from 20f0dc7 to 51b8f00 Compare November 28, 2025 13:51
@dragonJACson dragonJACson changed the title WIP: feat: provide vital things to make UD work feat: provide vital things to make UD work Nov 29, 2025
@dragonJACson dragonJACson marked this pull request as ready for review November 29, 2025 04:01
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.

Add initial support for RDMA UD QP creation and post_send / post_recv, with a demo

2 participants