RoCEv2 host-side RC receive path + cleanup#1270
Merged
Merged
Conversation
3921a4c to
e64ec80
Compare
…_ROCEV2 opt-out Server.cpp: - Receive RDMA-SEND-with-immediate (IBV_WC_RECV + IBV_WC_WITH_IMM) instead of RDMA-WRITE-with-immediate; decode the network-order immediate (channel + ring slot) and land the payload in the consumed recv-WR's slab buffer. - Batch the CQ poll: drain up to 16 CQEs per ibv_poll_cq() to amortize the per-call provider cost at line rate. - Set rnr_retry=7 (host SQ never transmits; cosmetic consistency). Build: - Add -DNO_ROCEV2 opt-out: gate libibverbs discovery/linking, the rocev2 source subdir, and the ROGUE_ROCEV2 define (module.cpp now keys the rocev2 setup_module on ROGUE_ROCEV2, not ROGUE_LINUX_BUILD). - Add -DROGUE_SKIP_PIP_INSTALL for packaging systems that install the python package themselves. - conda-recipe: add rdma-core to Linux host deps for libibverbs at build time.
…host-driven bring-up protocols/_RoCEv2.py: - Drop the FPGA metadata-bus codec and the engine child device; the codec now lives in surf (single source of truth). RoCEv2Server is a thin host-side Device — _start() is a host-side no-op and _stop() only tears down the C++ server; the Root drives the host<->FPGA handshake. - Add RoCEv2ServerCfg (host-NIC knobs) and RoCEv2TransportCfg (pmtu/minRnrTimer/ rnrRetry/retryCount, forwarded into both engine.setupConnection() and server.completeConnection()), with __post_init__ validation. - Add RoCEv2HostParams NamedTuple + getHostParams() accessor, completeConnection() wrapper with relocated validation + summary log, and printConnInfo(). - Add RoCEv2RnrTimer enum + code->ms map; drop magic timer codes. Auto GID-index detection, default resolution, HostIp local variable. Drop ConnectionState. _Process.py: - Add a configurable UpdatePeriod LocalVariable for the updateGroup leak period (the base _run() already wraps _process() in the updateGroup context).
- Delete test_rocev2_corner_cases.py and slim test_rocev2.py / conftest.py: the metadata-bus codec they exercised now lives in surf, so the codec/QP state-machine tests are no longer this repo's responsibility. - Gate the C++ rocev2 test subdir on (NOT NO_ROCEV2) so a -DNO_ROCEV2 build does not try to compile tests against absent ibverbs sources.
…g + enums, build opt-outs - Rewrite the RoCEv2 conceptual + API pages for the refactor: engine is no longer a child device; the Root sequences getHostParams -> setupConnection -> completeConnection. Python Example mirrors the rocev2PrbsTest.py idiom with RoCEv2ServerCfg + RoCEv2TransportCfg. - Fix the data-path wording from RDMA-WRITE to RDMA-SEND-with-immediate across the conceptual and C++ API pages; note the two-sided RNR-NAK flow control. - Document RoCEv2ServerCfg, RoCEv2TransportCfg, RoCEv2Mtu, RoCEv2RnrTimer via autoclass; drop ConnectionState, add HostIp. - process.rst: document the UpdatePeriod variable and base-class updateGroup. - DEVELOPMENT.md + yocto.rst: document -DNO_ROCEV2 and -DROGUE_SKIP_PIP_INSTALL.
e64ec80 to
81c5de2
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates Rogue’s RoCEv2 support to a host-side RC receive path built around RDMA SEND-with-immediate, moves the FPGA metadata-bus codec responsibility to surf, and refreshes tests/docs accordingly.
Changes:
- Updated the C++
rocev2::Serverreceive loop for RDMA SEND-with-immediate (batched CQ polling, immediate decoding, SEND semantics). - Refactored the Python wrapper to split configuration into
RoCEv2ServerCfg+RoCEv2TransportCfg, and to exposegetHostParams()/completeConnection()sequencing driven by the applicationRoot. - Simplified Rogue-side tests to focus on host-side helpers and hardware-gated smoke tests, removing codec/transport-helper tests that moved to surf.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/protocols/test_rocev2.py | Removes codec/transport-helper unit tests; keeps host-side helper + hardware-gated coverage. |
| tests/protocols/test_rocev2_corner_cases.py | Removes corner-case tests tied to the relocated FPGA metadata-bus codec/helpers. |
| tests/protocols/conftest.py | Removes shared response-pack helpers that were only needed by the removed codec tests. |
| src/rogue/protocols/rocev2/Server.cpp | Implements SEND-with-immediate receive semantics and CQ poll batching. |
| python/pyrogue/protocols/_RoCEv2.py | Introduces cfg dataclasses and new host↔FPGA bring-up API; removes embedded codec/helpers. |
| docs/src/built_in_modules/protocols/rocev2/index.rst | Updates module docs for new bring-up sequencing and SEND-based datapath. |
| docs/src/api/python/pyrogue/protocols/rocev2.rst | Updates Python API docs for new variables and configuration dataclasses. |
| docs/src/api/cpp/protocols/rocev2/server.rst | Updates C++ API docs for SEND wording and MR description. |
| docs/src/api/cpp/protocols/rocev2/core.rst | Updates core docs wording from WRITE to SEND. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…END wording - Server.cpp: a negative ibv_poll_cq() return is a non-transient error; stop the poll thread (mirroring the WR_FLUSH_ERR path) instead of spin-retrying, which would burn CPU and flood the log. - _RoCEv2.py: update RoCEv2ServerCfg docstrings, the MaxPayload LocalVariable description, and the connection-summary log to say RDMA SEND (receive slots) rather than the old RDMA WRITE wording; note the 4096 maxPayload default diverges from the C++ DefaultMaxPayload (9000). - docs index.rst: expand getHostParams() via ._asdict() in the prose example so it does not imply passing a NamedTuple straight into **kwargs.
…annel bounds tests - RoCEv2ServerCfg.maxPayload/rxQueueDepth annotated int|None to match the documented None sentinel (maps to C++ defaults). - MrAddr/MrRkey LocalVariable descriptions reworded for the RDMA-SEND receive path (no remote WRITE into the MR; rkey is local-only). - getChannel() docstring updated to RDMA SEND-with-Immediate and the ntohl(wc.imm_data) & 0xFF decode used by Server.cpp::runThread. - Restored getChannel bounds coverage (edge ids 0/255, out-of-range -1/256/1000) in hardware-free CI. - docs: attribute the 32-bit MR-length guard to host getHostParams() (raises first) with the surf engine as second line of defense.
… detectGidIndex/_host_ip_from_gid tests - docs: RoCEv2ServerCfg.maxPayload defaults to 4096 (one MTU_4096 PMTU), not the C++ DefaultMaxPayload of 9000; document maxPayload=None opt-in and fix the MR-length example (~1.0 MB). Correct stale 'RDMA WRITEs' wording to 'RDMA SENDs' in the same section. - gidIndex docstring: auto-detection scans sysfs (gid_attrs/types + gids), not ibv_devinfo; point at RoCEv2Server.detectGidIndex. - tests: hardware-free coverage for _host_ip_from_gid (round-trip + malformed-GID '' returns) and detectGidIndex (RoCE v2 IPv4 subnet match, skips RoCE v1 / link-local, None when off-subnet) via a temp sysfs tree.
Replace the receive thread's ibv_poll_cq + 100us sleep busy-loop with a completion-channel design. The CQ is bound to an ibv_comp_channel; the thread active-polls while completions flow (no sleep, and no per-batch interrupt latency since ibv_poll_cq reads the CQ from memory) and only arms + blocks in select() on the channel fd once the CQ has stayed empty for a bounded spin budget (genuinely idle). A self-pipe (wakeFd_) lets stop() break the select() immediately for prompt, sleepless shutdown. Per-completion handling is factored into processCompletion(). Throughput-equivalent to the prior loop (sustains line rate while busy, parks with no CPU spin when idle); removes the sleep() from the hot path.
bengineerd
approved these changes
Jun 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the host-side RoCEv2 RDMA receive path to
pyrogue.protocols. The branch builds the RC-modeRoCEv2Server(C++ ibverbs + PyRogue wrapper), the host↔FPGA QP bring-up/teardown sequencing, and theRoCEv2TransportCfgtuning bundle, then removes the now-relocated FPGA metadata-bus codec (which lives insurf).What's in this PR
getChannel()filters, auto GID-index detection, soft-reset on reconnect.getHostParams()/RoCEv2HostParams,completeConnection(),printConnInfo(), and theRoCEv2TransportCfgdataclass (pmtu / minRnrTimer / rnrRetry / retryCount) forwarded into both the FPGA engine and the host server so they cannot drift.surf(surf.ethernet.roce._RoCEv2Protocol/_RoCEv2Engine), which is now its single source of truth and owns its tests.