Skip to content

Add CPU RoCE to playback emulator - #834

Open
eliotheinrich wants to merge 8 commits into
NVIDIA:mainfrom
eliotheinrich:pr-playback-cpuroce
Open

eliotheinrich wants to merge 8 commits into
NVIDIA:mainfrom
eliotheinrich:pr-playback-cpuroce

Conversation

@eliotheinrich

@eliotheinrich eliotheinrich commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR adds a CPU RoCE session backend to the playback emulator.

Docs will follow (#835).

Runtime / performance impact

N/A

Self-review checklist

Please confirm each item before requesting review. Check [x] or strike
through and explain.

Before requesting review

  • I reviewed my own full diff in GitHub or my editor.
  • PR is in Draft if it is not yet ready for review.
  • Temporary / debugging changes have been removed.
  • Local test logs reviewed; no unexplained warnings or errors.
  • CI logs reviewed; no unexplained warnings or errors.
  • Full CI has been run.

Scope and size

  • PR is under ~1000 lines, or an exception is justified in the description.
  • Refactoring-only changes are isolated in their own PR(s).
  • No existing tests were disabled or modified just to make this PR pass
    (if so, an issue has been raised).

Tests

  • New functionality has new tests.
  • Tests fail if the new functionality is broken (including crashes), not
    just when it is missing.
  • Negative tests added where exceptions are expected.
  • Truth data added where simple EXPECT_* / assert checks are
    insufficient for algorithmic correctness.
  • CI runtime impact considered; team notified if significant.

Documentation

  • Public-facing APIs have Doxygen docs.
  • User-visible behavior changes have public docs, or a follow-up is
    tracked.

Code style

  • Naming follows the existing convention (snake_case vs camelCase) for
    the area being modified.

Dependencies

  • No new third-party dependencies, or the team has been notified and
    OSRB tickets filed.

Signed-off-by: Eliot Heinrich <eheinrich@nvidia.com>
Signed-off-by: Eliot Heinrich <eheinrich@nvidia.com>
Signed-off-by: Eliot Heinrich <eheinrich@nvidia.com>
Signed-off-by: Eliot Heinrich <eheinrich@nvidia.com>
Signed-off-by: Eliot Heinrich <eheinrich@nvidia.com>
Signed-off-by: Eliot Heinrich <eheinrich@nvidia.com>
@eliotheinrich
eliotheinrich marked this pull request as ready for review September 17, 2026 17:19
rx_data_ + static_cast<std::size_t>(slot) * stride_, stride_);
__atomic_store_n(&rx_flags_[slot], 0ull, __ATOMIC_RELEASE); // re-arm
cursor_ = (cursor_ + 1) & slot_mask_;
slot_owner_[slot] = kNoOwner;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

CUDA-Q’s existing RingSlotChannel serializes response-bearing calls and exits the dispatch on a timeout, while this implementation frees timed-out slots and continues publishing requests. A late unmatched response could therefore clear slot_owner_[slot] after that slot has been assigned to a newer request. Would it make sense to update ownership only after matching the response ID and confirming that slot_owner_[it->second.slot] still equals that ID? It may also be worth adding a delayed-response-after-timeout-and-ring-wrap test.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, good catch; addressed in 85e1b1f

worker_ = std::thread([this] { worker_loop(); });
}

void send(const frame &f, tag t) override {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we also validate f.size <= stride_ in send()? The current playback paths check max_frame_bytes upstream, but this session is the component that owns the fixed RDMA slot size, and try_publish() later copies frame_len bytes into that slot without another bound check. Keeping the invariant at this boundary would make the backend safer to reuse and straightforward to test with a slot_size + 1 frame.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in 85e1b1f

Signed-off-by: Eliot Heinrich <eheinrich@nvidia.com>
Signed-off-by: Eliot Heinrich <eheinrich@nvidia.com>
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