Ergonomics: Treat PollCompletionQueueError::CompletionQueueEmpty as an empty iterator to simplify completion polling

[vtta]

This patch simplifies completion polling by allowing native iteration over the completion queue using for wc in cq.iter()?.
Previously, callers had to manually invoke poller.next(), which resulted in verbose and heavily nested code.
Treating PollCompletionQueueError::CompletionQueueEmpty as a natural “end of iterator” condition is appropriate here, an empty CQ simply behaves like an empty iterator. As a result, we now keep PollCompletionQueueError for actual Ibverbs errors and let normal iterator semantics handle the empty-queue case.

The resulting interface looks like:

for wc in cq.iter()? {
    // do something with wc
}

instead of

loop {
  match cq.start_poll() {
      Ok(mut poller) => {
          while let Some(wc) = poller.next() {
              // do something with wc
          }
      },
      Err(err) => {
          // continue or abort based on CompletionQueueEmpty or Ibverbs 
          continue;
      },
  }
}

[gemini-code-assist]

Summary of Changes

Hello @vtta, I'm Gemini Code Assist¹! 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 developer experience for handling RDMA completion queues by refactoring the polling mechanism into a more idiomatic Rust iterator pattern. This change eliminates verbose match statements and manual poller.next() calls, allowing developers to process work completions using simple for loops. The CompletionQueueEmpty error is now implicitly handled by the iterator's natural termination, streamlining error management and making the code cleaner and more concise.

Highlights

• Simplified Completion Polling: The start_poll() method has been replaced with an iter() method, allowing for native iteration over completion queues using for wc in cq.iter()?.
• Improved Error Handling: The PollCompletionQueueError::CompletionQueueEmpty variant has been removed, as an empty completion queue is now naturally handled by iterator semantics.
• New Iterator Structures: Introduced BasicCompletionQueueIter and ExtendedCompletionQueueIter to provide an iterator-based interface for polling work completions.
• Enhanced Port Attributes: Added a lid() method to the PortAttr struct for retrieving the local identifier of an RDMA port.
• Example and Test Updates: Updated example and test files (rc_pingpong.rs, rc_pingpong_split.rs, test_post_send.rs) to reflect the new ergonomic polling interface.

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. ↩

[gemini-code-assist]

Code Review

This pull request introduces a significant ergonomic improvement by treating an empty completion queue as an empty iterator, which greatly simplifies the polling logic in client code. The changes in src/ibverbs/completion.rs are well-conceived, and the updated examples clearly demonstrate the benefits of the new API. I've identified a potential panic in the new iterator implementation and a couple of copy-paste errors in the tests. After addressing these points, this will be a very solid contribution.

[gemini-code-assist]

The unreachable!() macro here can be reached. The ibv_poll_cq function can return a negative value on error. In that case, this _ match arm will be taken, causing a panic. This would crash the application instead of handling the error gracefully. While the Iterator trait prevents returning a Result from next(), the error should not cause a panic. A better approach would be to stop the iteration, similar to how an empty queue is handled.

                _ => {
                    // This is an error from ibv_poll_cq, so we stop iterating.
                    self.status = Empty;
                    None
                }

[dragonJACson]

Could you please fix this? I think we would reach here when hardware status is broken. @vtta

[codecov]

Codecov Report

❌ Patch coverage is 70.23810% with 25 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/ibverbs/completion.rs	72.83%	22 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/completion.rs	80.97% <72.83%> (-0.98%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[dragonJACson]

Generally looks good to me 👍, thanks for your contribution!

Could you please rebase your commits to squash cd39297 into the preceding functional commit? It’s a small typo fix and fits better together.

Also, please update the commit messages to be a bit more informative and consistent:

For 76455a2

chore(test): rename mr to send_mr for clarity in post send tests

For 250519c, You could mention the API behavior change in the message body.

That should make the history cleaner and commit messages more descriptive. cc @FujiZ

[dragonJACson]

Should be self.poll_batch.get().try_into().unwrap_unchecked()

[dragonJACson]

Could you please fix this? I think we would reach here when hardware status is broken. @vtta

[dragonJACson]

This means when status is Empty, we would never call ibv_poll_cq again, line 666 should be removed. Does this look good to you? @FujiZ