[RFC]: Decouple NVMe-oF Replicas from SPDK behind an Initiator Interface

# RFC: Decouple NVMe-oF Replicas from SPDK behind an Initiator Interface

- Related:
- Issue [#3131](https://github.com/kvcache-ai/Mooncake/issues/3131) — NoF writes fail: user buffers never registered with SPDK's RDMA translation table
- PR [#3251](https://github.com/kvcache-ai/Mooncake/pull/3251) — block-unaligned I/O for NoF replicas (SGL-based); still open as of 2026-08-12
- RFC [#3360](https://github.com/kvcache-ai/Mooncake/issues/3360) — Refactor Mooncake Store segment management around SegmentPool, esp. sub-issue [#3364](https://github.com/kvcache-ai/Mooncake/issues/3364) (Integrate NoF mounted regions into SegmentPool). Orthogonal and independent of this RFC; see §4.
- Companion docs: `nof-call-chains.md` (verified call chains + implicit contracts C1–C16), `spdk-nof-review.md` (review findings M1–M10 / E1–E4), `nof-spdk-impl-plan.md` (detailed implementation plan), `nof-refactor-diagrams.md` (full mermaid diagram set)

---

## 1. Summary

Today "NoF replica" and "SPDK" are the same thing in Mooncake Store: every component on the NoF data and control plane calls SPDK C APIs through a global singleton (`SpdkWrapper`), and SPDK types (`spdk_nvme_qpair`, `spdk_nvme_ns`, `spdk_nvme_cmd_cb`) leak into public headers consumed by the client, master and benchmarks.

This RFC proposes introducing a small, stable abstraction — an `NVMeoFInitiator` interface plus a separate `DmaBufferAllocator` interface — moving all SPDK code into a single `SpdkInitiator` implementation behind those interfaces, and injecting instances instead of reaching for a global singleton. No wire protocol, replica-type, or segment-management changes are involved.

This is a data-plane refactor. A separate community RFC ([#3360](https://github.com/kvcache-ai/Mooncake/issues/3360)) covers control-plane ownership of NoF mounted regions in the master; the two are orthogonal (§4).

---

## 2. Problem Statement

Today "NoF replica" and "SPDK" are the same thing in Mooncake Store. `SpdkWrapper::GetInstance()` is reached directly from 6 files, 14 call sites spanning client, master and benchmarks; the public header `include/spdk/spdk_wrapper.h` leaks SPDK types (`spdk_nvme_qpair*`, `spdk_nvme_ns*`, `spdk_nvme_cmd_cb`), so every translation unit that touches NoF compiles against SPDK headers; and 38 SPDK/DPDK static libs + 15 system libs are linked `PUBLIC` into `mooncake_store`, of which the master needs exactly one heartbeat probe — its segment mount/unmount/eviction logic is pure metadata. Replacing SPDK with another initiator (kernel nvme, libnvme, a vendor SDK) would currently require touching ~15 files and ~2700 lines, most of them unrelated to I/O semantics.

Worse than the explicit coupling is the implicit kind: 16 unwritten contracts (C1–C16, see `nof-call-chains.md`) that no type or comment protects. The load-bearing three: `SpdkNofTask::io_count` points at a worker-thread stack variable, safe only because SPDK runs completion callbacks synchronously on the polling thread; `te_endpoint` is simultaneously the master segment name, the mount dedup key, and the transport-id parse input (an invariant shared with RFC #3364); buffers from `spdk_zmalloc` must be freed with `spdk_free`, or glibc aborts. Any refactor that only wraps the API calls will break these behind the new interface — the design below makes them explicit.

---

## 3. Why Now: Community Evidence

### 3.1 Issue #3131 — the missing half of memory registration is a live user bug

A user running `ReplicateConfig{replica_num=0, nof_replica_num=1}` (pure-NoF writes, LMCache deployment, SPDK v23.01.1, ConnectX-6 RoCE) gets `TRANSFER_FAIL` on every write: SPDK's NVMe-oF RDMA transport keeps its own memory translation table (`nvme_rdma_qpair::rkey_map`), independent of Mooncake Transfer Engine's MR registry. User buffers registered via `register_buffer()` → `ibv_reg_mr()` are invisible to SPDK, so `spdk_rdma_get_translation()` fails with "No translation for ptr".

Root cause in code: `transfer_task.cpp` hands the user buffer pointer straight to `spdk_nvme_ns_cmd_write()`; nothing ever calls `spdk_mem_register()`. The whole codebase contains zero `spdk_mem_register` calls.

This is exactly the gap this RFC's `RegisterMemory`/`UnregisterMemory` interface methods close (§5.4). It also proves the gap is not theoretical — it breaks the flagship pure-NoF configuration for real users today.

### 3.2 PR #3251 — unaligned I/O support is rewriting the same code, and deepening the coupling

PR #3251 (block-unaligned NoF I/O via SPDK SGL, open as of 2026-08-12) is a valuable feature, but in its current form it makes the coupling worse, on exactly the files this RFC touches:

- `transfer_task.cpp` gains a direct `spdk_vtophys()` call (in `AppendDmaSges()`) — the first time SPDK API is used in the data plane *outside* `SpdkWrapper`;
- `SpdkWrapper::SubmitSglRequest()` leaks two more SPDK callback types (`spdk_nvme_req_reset_sgl_cb`, `spdk_nvme_req_next_sge_cb`) into the public header;
- `SpdkNofTask` grows an SGE vector plus `buffer_owners` keep-alives for padding/discard DMA scratch;
- `NoFDescriptor`/`NoFSegment`/`ssd_register_client`/pybind all grow `block_size` plumbing — touching files this RFC otherwise considers stable.

Two consequences:

1. Urgency. Every month the refactor waits, more SPDK surface accretes into `transfer_task.cpp` and the public headers. The interface must be designed *now* so that #3251's SGL machinery has a proper home (inside the initiator implementation) instead of becoming permanent residents of the transfer layer.
2. Sequencing. #3251 rewrites the exact data-plane code the refactor relocates (`SpdkNofTask`, the worker submit loop, `SpdkWrapper::SubmitRequest`). Landing it first and refactoring on top is cheaper than asking it to rebase onto a brand-new interface — but treat this as a preference, not a hard gate: #3251 is an open, moving PR, and its metadata-side `block_size` plumbing (`NoFSegment`, `replica.h`, master, register client, pybind) is outside this RFC's scope in either order. Whichever side lands second, the interface — byte-oriented and SGL-ready from day one (§5.3) — is re-validated against it. Independently, the #3131 fix (`spdk_mem_register` in the buffer-registration path) should ship immediately as a small standalone PR: it is a prerequisite for #3251 too (its `spdk_vtophys` SGL splitting fails on buffers SPDK doesn't know), and it must not wait for a multi-week refactor.

---

## 4. Goals and Non-Goals

### Goals

1. Introduce an `NVMeoFInitiator` abstract interface hiding the NVMe-oF initiator behind well-defined virtual methods with explicit contracts (callback threading, polling semantics, handle lifetime, transport-string ownership).
2. Move all SPDK code into a concrete `SpdkInitiator` (pimpl) — SPDK headers and types visible only inside that implementation.
3. Make the segment handle opaque: callers never see `spdk_nvme_qpair`/`ns`.
4. Separate DMA buffer allocation into a `DmaBufferAllocator` interface (`SpdkDmaAllocator`, `SystemDmaAllocator`).
5. Fix the #3131 memory-registration gap via `RegisterMemory`/`UnregisterMemory`.
6. Replace the global singleton with dependency injection (a factory is the only `#ifdef USE_NOF` point that touches SPDK types).
7. Propagate data-plane error detail (`sc`/`sct`/status string) to callers instead of collapsing everything to `TRANSFER_FAIL` — today only the probe path keeps the SPDK status string.
8. Converge SPDK/DPDK libraries to a `PRIVATE` link dependency of the initiator implementation library.

### Non-Goals

- No change to NoF segment behavior: mount/unmount/remount RPC chain and semantics, replica type system (`ReplicaType::NOF_SSD` stays), heartbeat state machine and thresholds, eviction triggers, wire protocol, RPC layer.
- No second initiator implementation in this work (kernel-nvme/libnvme becomes *possible*, not built).
- No change to the worker thread model, seg→worker affinity, busy-polling, or QoS behavior — these become explicit interface contracts instead of SPDK side-effects.
- Feature-gating `#ifdef USE_NOF` blocks in master/client (mount rejection, heartbeat config, eviction hooks — roughly 12 sites) stay; only the two sites touching SPDK *types* converge into the factory.

> RFC [#3360](https://github.com/kvcache-ai/Mooncake/issues/3360) / [#3364](https://github.com/kvcache-ai/Mooncake/issues/3364) refactors master-side NoF metadata ownership into SegmentPool. That work is orthogonal to this RFC — it touches control-plane cataloging, this touches data-plane SPDK coupling. The shared invariant is the `te_endpoint` string format (C3); neither RFC changes it. No sequencing dependency exists in either direction.

---

## 5. Design Overview

### 5.1 Two interfaces, because SPDK is two things

SPDK serves Mooncake as (a) an NVMe-oF initiator and (b) a hugepage DMA memory allocator. The call sites of the two roles never overlap. A future kernel-nvme initiator would do I/O but not DMA allocation, so the roles get separate interfaces:

- `NVMeoFInitiator` — segment open/probe, I/O submit/poll, memory registration (environment lifecycle is owned internally, not exposed on the interface; §5.6);
- `DmaBufferAllocator` — `Alloc`/`Free` only. As a side benefit, the alloc/free mirror contract (C5) and the static-destruction-order hazard (C7) disappear: an allocator object remembers who it is and outlives its users by construction (`shared_ptr`).

### 5.2 Opaque handle, caching semantics inside the implementation

`NofSegmentHandle` is a forward-declared class. `OpenSegment(transport_str)` returns a handle whose *identity* is stable per endpoint: the current `SpdkWrapper` caches `ctrlr_info`/`nof_seg_handle` per endpoint and callers depend on pointer stability (the handle pointer is the worker-affinity and QoS map key, C10). That caching moves into `SpdkInitiator::Impl`; the interface contract states that repeated `OpenSegment` with the same transport string returns handles bound to the same underlying queue pair, and that handles live as long as the initiator.

### 5.3 Byte-oriented, SGL-ready I/O contract

`SubmitIO` takes `(handle, buffer, byte_offset, byte_length, op, callback, ctx)` rather than `(lba, lba_count)`:

- LBA conversion moves into the implementation, which owns block-size knowledge anyway;
- today's alignment gate (offset/size/ptr must be block-aligned) stays as an explicit interface precondition, enforced at submit time;
- when #3251 lands, its data-plane unaligned handling — physical-range round-up, zero-padding write scratch, head/tail discard read scratch, `spdk_vtophys` SGL splitting — is designed to drop *inside* `SpdkInitiator::Impl` without interface change (its metadata-side `block_size` plumbing through `NoFSegment`/replica/pybind lives outside the initiator and is unaffected either way). A `GetCapabilities()` query (SGL support, DWORD alignment) is part of the interface from the start so #3251's validation has a neutral home.

Scratch DMA buffers needed to satisfy an I/O (padding/discard) are owned by the implementation and released before the completion callback returns — callers never see them (this is where #3251's `buffer_owners` keep-alive semantics get contractually pinned).

### 5.4 Memory registration (closes #3131)

```cpp
virtual ErrorCode RegisterMemory(void* ptr, size_t size) = 0;
virtual ErrorCode UnregisterMemory(void* ptr) = 0;
```

`SpdkInitiator` implements these with `spdk_mem_register`/`spdk_mem_unregister` (compile-time feature-detected for older SPDK; graceful no-op-with-warning degradation). `RealClient::register_buffer_internal` calls TE registration first, then initiator registration; on initiator failure the TE registration is rolled back, so no half-registered state exists. For non-RDMA initiators these are no-ops.

### 5.5 Neutral completion callback

```cpp
struct NofIOCompletion { bool success; int sc; int sct; std::string error_string; };
using NofIOCallback = void(*)(void* ctx, const NofIOCompletion&);
```

No `spdk_nvme_cpl` outside the implementation. The error string is populated from `spdk_nvme_cpl_get_status_string` — turning today's log-only data-plane errors into diagnosable results (Goal 7). A raw function pointer + `void*` is used instead of `std::function` so the callback itself carries no hidden allocation. That alone does not make the hot path allocation-free, and this RFC does not claim it does: the (callback, ctx) pair must reach the completion trampoline through SPDK's single `void*` cb_arg, and packing that pair into a per-call heap adaptor would *regress* today's design, where sub-tasks come from a pre-allocated chunked pool and completion is a plain `reinterpret_cast` plus pool return. The contract is therefore explicit: in steady state, submit and completion perform no per-sub-IO heap allocation — the two-slot callback adaptor is embedded in caller-owned (pooled) sub-task storage, a caller-side layout detail that leaves the interface signature unchanged. A transitional heap-adaptor form may appear in early implementation milestones for simplicity, but it is gated by the before/after bench and is not the intended steady state.

### 5.6 Dependency injection via a single factory

`CreateNofRuntime()` — a free function compiled unconditionally, and the only site whose definition is `#ifdef USE_NOF`-gated — returns `NofRuntime{shared_ptr initiator, shared_ptr dma_allocator}`. In non-USE_NOF builds it returns `{nullptr, SystemDmaAllocator}`, so callers express the behavior difference through the injected objects (null-check the initiator) instead of scattering type-level `#ifdef`.

The returned objects are **ready to use: no caller ever invokes a setup method.** `Initialize()`/`Shutdown()` are therefore not part of the public interface — the SPDK environment lifecycle is owned inside the implementation (a shared, refcounted env guard) and acquired lazily on first use (`OpenSegment`/`ProbeSegment`/DMA `Alloc`), preserving today's lazy behavior and sparing non-NoF clients the EAL-init + hugepage cost (§8). `initiator == nullptr` uniformly means "NoF is unavailable in this process" — a non-USE_NOF build today, or a future eager-init initiator whose factory-time init failed; lazy-path resource failures surface as errors from the operation that triggers them, exactly as today's DMA-alloc/probe paths report them. Future initiators that *cannot* initialize lazily (native DPDK needing EAL args, GDS needing a CUDA device ordinal and context ordering) do so inside the factory — configuration is a factory concern, invisible to callers. Three further properties:

- **One env per process, many runtimes allowed.** DPDK EAL init is process-global; the shared, refcounted env guard lets any number of runtimes coexist (client plus in-process bench, or several `Client` instances) without double init, and lets the env outlive all users. Each initiator instance keeps its own connection/handle cache, so handle identity — the QoS and worker-affinity key — is scoped per instance; the intended usage remains one runtime per process role.
- **`RealClient` owns, everyone else borrows.** `RealClient::setup` holds the pair, passes the initiator down to `TransferSubmitter` (via a new optional trailing parameter on `Client::Create` — a setter cannot work because `InitTransferSubmitter` runs inside `Client::Create`), and hands the allocator straight to `ClientBufferAllocator`.
- **Master reuses its existing seam.** The master keeps only the initiator and routes `ProbeSegment` through the existing `NoFProbeFn` test seam — mechanism unchanged, `nof_heartbeat_test.cpp` untouched; probe DMA buffers are handled inside the implementation.

The Python-exported free functions (`hugepage_memory_alloc/free`) keep their exact signatures; internally they delegate to a lazily-initialized process-wide default DMA allocator — the one deliberate exception to DI, since their ABI has no injection point.

---

## 6. Architecture

### 6.1 Before (measured)

```mermaid
flowchart TB
subgraph CP["client process"]
PUT["Client::Put/Get"] --> TS["TransferSubmitter"]
TS --> WP["SpdkNofWorkerPool"]
RC["RealClient::setup"]
CBA["ClientBufferAllocator"] --> UM["utils / memory_alloc"]
end
subgraph MP["master process"]
HB["nof_heartbeat_thread
(nof_probe_fn_)"]
end
BN["nof_worker_pool_bench"]

TS -. "GetInstance() ×6" .-> SW
WP -. "GetInstance()" .-> SW
RC -. "GetInstance()" .-> SW
UM -. "GetInstance() ×2 each" .-> SW
HB -. "GetInstance() ×2" .-> SW
BN -. "GetInstance()" .-> SW

SW["SpdkWrapper — Meyers singleton
env init + ctrlr/ns/qpair cache
+ IO submit/poll + DMA alloc + probe
public header leaks spdk/env.h, spdk/nvme.h"]
LNK["38 SPDK/DPDK static libs + 15 system libs
linked PUBLIC → client / bench inherit all"]
SW --- LNK
```

### 6.2 After

```mermaid
flowchart TB
subgraph CP["client process"]
RC["RealClient::setup
CreateNofRuntime()
← the ONE #ifdef USE_NOF-gated site"]
RC -->|"shared_ptr inject"| TS["TransferSubmitter
└ NofWorkerPool → SubmitIO / PollCompletion"]
RC -->|"shared_ptr inject"| CBA["ClientBufferAllocator
(use_hugepage_ precedence kept; Python ABI kept)"]
end
subgraph MP["master process"]
HB["nof_probe_fn_ default lambda
(existing test seam reused;
heartbeat logic untouched)"]
end

TS -->|"virtual calls"| IF
HB -->|"ProbeSegment"| IF
CBA -->|"Alloc / Free"| DA

IF["NVMeoFInitiator
«interface»"]
DA["DmaBufferAllocator
«interface»"]
IF --> SI["SpdkInitiator — Impl (pimpl)
spdk_env lifecycle
ctrlr/ns/qpair cache (same endpoint ⇒ same qpair)
spdk_mem_register
(future: vtophys SGL, pad/discard scratch)"]
DA --> SD["SpdkDmaAllocator
spdk_zmalloc / spdk_free"]
DA --> SY["SystemDmaAllocator
aligned_alloc"]
LNK["SPDK/DPDK libs → PRIVATE
to the initiator library only"]
SI --- LNK
```

### 6.3 Data-plane flow (Put/Get) after

```mermaid
sequenceDiagram
autonumber
participant C as caller thread (Client::Put)
participant TS as TransferSubmitter
participant W as worker thread (bound to handle)
participant I as SpdkInitiator::Impl

C->>TS: submit
TS->>I: OpenSegment(endpoint) → cached handle (Impl cache)
TS->>I: GetBlockSize(handle)
Note over TS: alignment gate in bytes (interface precondition)
TS->>W: pool.submitTask(NofTask) — per-worker queue + cv
W->>I: SubmitIO(handle, buf, byte_off, byte_len, op, cb, ctx)
I->>I: spdk_nvme_ns_cmd_*
loop busy poll (unchanged)
W->>I: PollCompletion(handle, 0)
end
I-->>W: nof_io_complete(NofIOCompletion sc / sct / error_string)
W-->>C: set_completed(OK or TRANSFER_FAIL + detail) + cv notify
C->>C: TransferFuture.wait returns
```

The queueing, affinity, busy-poll and cv-wakeup skeleton is byte-for-byte today's behavior; the only runtime deltas are one virtual call per operation and error detail flowing back.

### 6.4 What does not change

Worker count / NUMA binding / seg→worker affinity (SPDK qpair single-thread constraint — now an explicit contract); `OperationState` mutex+cv and `TransferFuture`; mount/unmount/remount RPC chain and behavior; heartbeat state machine and thresholds; eviction triggers; `te_endpoint` string contract (an invariant shared with RFC #3364: format owned by `ssd_register_client`, parsing owned by each initiator); Python ABI (`hugepage_memory_alloc/free` keep symbol and signature, internals delegate); `use_hugepage_` precedence over the DMA allocator; only client local buffers use SPDK DMA — store segment allocation keeps `use_spdk_dma=false`.

---

## 7. Impact on Existing SPDK Code

| Area | Impact |
|---|---|
| `spdk_wrapper.{h,cpp}` | Migrated into `SpdkInitiator::Impl` + `SpdkDmaAllocator` essentially verbatim (ctrlr/ns cache, env-init double-check, probe machinery), then deprecated and removed in the final phase. Two ride-along fixes: probe-ctx recycle race (recycle only after the caller has read results) and `GetBlockSize` caching at QoS creation. |
| `transfer_task.{h,cpp}` | `SpdkNofTask`→`NofTask` (opaque handle, byte offsets, `IOOp` enum class indexed QoS arrays, `io_count` becomes `shared_ptr>` — removing the stack-pointer time bomb C2), worker loop calls the interface. |
| `real_client.{h,cpp}` | Owns factory-created initiator+allocator; the eager setup-time `InitializeEnv` call disappears — the env is acquired lazily inside the implementation; `register_buffer_internal` gains initiator registration with rollback. |
| `utils.{h,cpp}`, `memory_alloc.{h,cpp}`, `client_buffer.{h,cpp}` | `use_spdk_dma` bool replaced by injected `DmaBufferAllocator`; Python-exported symbols keep their exact signatures. |
| `master_service.{h,cpp}` | Only the default `nof_probe_fn_` lambda body changes (singleton → injected initiator). Heartbeat logic untouched. |
| Build | New `mooncake_nof_spdk` static lib; `${SPDK_STATIC_LIBS}` moves from `mooncake_store` PUBLIC to that lib's PRIVATE. Client/master binaries no longer transitively link DPDK/SPDK. |
| Bench | `nof_worker_pool_bench.cpp` switches from singleton to factory-created instances. |
| Tests | Existing tests keep passing (`nof_heartbeat_test.cpp`, `client_buffer_test.cpp`, e2e heartbeat script). New `nvmeof_initiator_test.cpp` adds MockInitiator/MockDmaAllocator coverage of the worker pool and QoS. |

Behavioral compatibility: no on-wire or on-disk format changes; no API/ABI change except internal C++ interfaces; Python bindings unchanged.

---

## 8. Risks and Mitigations

| Risk | Mitigation |
|---|---|
| Callback threading assumptions change behavior | Interface contract mandates synchronous completion on the polling thread per handle; MockInitiator implements the same contract so tests are meaningful |
| `max_completions==0` means "all" (SPDK dialect) | Contract defines it explicitly; other implementers must follow |
| SPDK version lacks `spdk_mem_register` | Compile-time feature detection; warn-and-continue degradation |
| Hot-path regression (virtual call, byte→LBA math, callback path) | Block size cached at QoS creation (removes a per-loop query that exists today); §5.5 requires allocation-free steady-state submit/completion (adaptor embedded in pooled sub-task storage, no per-sub-IO heap alloc); bench gate before merge |
| DPDK EAL init + hugepage reservation hits non-NoF clients in USE_NOF builds | Known constraint, changed for the better by this refactor: today the client setup path runs `spdk_env_init` eagerly even when NoF is never used, while DMA-alloc and probe already initialize lazily. The design removes that eager call entirely — there is no `Initialize()` on the interface, the env is acquired lazily at first use (§5.6), so pure RDMA/TCP deployments pay nothing; init failures surface at first use, which those paths already report as errors today |
| Coupling with PR #3251 (open, moving target) | Landing the #3131 fix and #3251 first or landing this RFC first are both OK |
| Rebase collision with RFC #3360 sub-issues | File overlap is limited to `master_service.{h,cpp}`: this RFC edits only the probe lambda body, while #3364 (if it lands) touches the NoF mount/unmount/heartbeat region. Either order is rebase-cheap |
| Stale base branch | The working branch lags `upstream/main` significantly, with heavy drift in `master_service.{h,cpp}`; rebasing onto current main is a hard precondition before any phase starts |

Start with the companion docs `nof-call-chains.md`, `spdk-nof-review.md`, `nof-spdk-impl-plan.md`, and `nof-refactor-diagrams.md`, then inspect `transfer_task.cpp` and `include/spdk/spdk_wrapper.h`. Trace `SpdkWrapper` call sites and `RealClient::register_buffer_internal`; done means the proposed initiator and allocator boundaries, memory-registration behavior, and dependency-injection scope are validated without changing the listed non-goals.

Written by the indexing model from the issue text.