feat(spdk): thread NVMe-oF transport type through engine/replica create by abonillabeeche · Pull Request #654 · longhorn/longhorn-spdk-engine

abonillabeeche · 2026-08-20T04:16:46Z

Part of longhorn/longhorn#13796. Depends on longhorn/types#121 and longhorn/go-spdk-helper#308.

Carry the DataEngineTransport from the spdkrpc create requests into replica and engine setup: create the matching NVMe-oF transport (RDMA or TCP) on the SPDK target, add the listener with the right trtype, and connect the engine's controllers over the selected transport. RDMA uses RoCEv2 (adrfam=ipv4). Unset means TCP; existing volumes and the host frontend are unaffected (frontend stays TCP).

Files: pkg/spdk/{engine,replica,server,server_engine,util,shard,shardgroup}.go, pkg/client/{client_engine,client_replica}.go.

shuo-wu · 2026-08-20T19:53:48Z

+		SalvageRequested:    salvageRequested,
+		SnapshotMaxCount:    snapshotMaxCount,
+		DataLayoutType:      dataLayoutType,
+		DataEngineTransport: dataEngineTransport,


NIT: TransportType may be better. No need to add prefix DataEngine

dropping the DataEngine prefix: the remaining DataEngineTransport here is the generated proto field from longhorn/types#121. Renaming it to transport_type is a clean change but cascades to longhorn-manager and instance-manager. Want me to rename it in #121 for consistency, or keep the proto field namespaced and only use TransportType on the Go structs?

shuo-wu · 2026-08-20T19:54:26Z

+	// data_engine_transport; unset (empty) is treated as TCP for backward
+	// compatibility. Only the internal engine<->replica data fabric honors this;
+	// the host-facing NVMe-TCP frontend and all EC/transient paths stay TCP.
+	Trtype spdktypes.NvmeTransportType


NIT: TransportType

shuo-wu · 2026-08-20T20:48:06Z

 		e.Name, e.NvmeTcpTarget.IP, e.NvmeTcpTarget.Port, initialANAState, cntlid, nsUUID)
 	if err := spdkClient.StartExposeBdevWithANAState(e.NvmeTcpTarget.Nqn, e.Name, e.NvmeTcpTarget.Nguid, nsUUID,
-		e.NvmeTcpTarget.IP, strconv.Itoa(int(e.NvmeTcpTarget.Port)), spdkANAState, cntlid, cntlid); err != nil {
+		e.NvmeTcpTarget.IP, strconv.Itoa(int(e.NvmeTcpTarget.Port)), spdktypes.NvmeTransportTypeTCP, spdkANAState, cntlid, cntlid); err != nil {


NIT: Add a TODO comment for the Frontend adaptation

renamed the struct field to TransportType on both Engine and Replica, and the client-method params to transportType

shuo-wu · 2026-08-20T20:51:36Z

 		e.backends[replicaName] = backendFactory(replicaName, replicaAddr)

-		bdevName, err := connectNVMfBdev(spdkClient, replicaName, replicaAddr, e.ctrlrLossTimeout, e.fastIOFailTimeoutSec, maxRetries, retryInterval)
+		bdevName, err := connectNVMfBdev(spdkClient, replicaName, replicaAddr, e.Trtype, e.ctrlrLossTimeout, e.fastIOFailTimeoutSec, maxRetries, retryInterval)


Probably we need to validate whether the replicas are using the same tytype with this engine. The validation can be applied by refactoring func validateReplicaSize

shuo-wu · 2026-08-20T20:55:26Z

+	// Select the internal engine<->replica NVMe-oF transport. Unset (TCP=0)
+	// preserves historical TCP behavior. Must match the transport the replicas
+	// used to expose their heads.
+	e.Trtype = nvmeTransportFromProto(req.DataEngineTransport)
+


It's better to directly add an input TransportType for the above NewEngine instead

TransportType is now an argument to NewEngine(...) and set at construction; dropped the post-construct assignment.

shuo-wu · 2026-08-20T20:56:49Z

 	if err := spdkClient.StartExposeBdev(helpertypes.GetNQN(r.snapshotCloningDstCache.cloningLvol.Name),
 		r.snapshotCloningDstCache.cloningLvol.UUID, generateNGUID(r.snapshotCloningDstCache.cloningLvol.Name), r.IP,
-		strconv.Itoa(int(r.snapshotCloningDstCache.cloningPort)), helpertypes.InternalHostNQN); err != nil {
+		strconv.Itoa(int(r.snapshotCloningDstCache.cloningPort)), spdktypes.NvmeTransportTypeTCP, helpertypes.InternalHostNQN); err != nil {


Can we use RDMA here (clone) or in the below rebuilding/expansion? Or you can add TODO comments first.

added TODOs.

shuo-wu · 2026-08-20T21:05:06Z

Besides, I am thinking whether Longhorn needs to detect the availability of RDMA hardware before accepting the transport type RDMA.
The detection can be in this spdk-server or in the upper longhorn-manager. Currently, I may prefer to let longhorn-manager detect that. Then, Longhorn can reject the invalid engine earlier. Besides, if only some of the nodes have RDMA hardware, the validation will be more complicated.

Add an RDMA option to the internal engine<->replica data path of the v2 data engine. Adds a TransportType field (default TCP) to the Engine and Replica objects, set from the new spdkrpc DataEngineTransport field, and uses it at the two sites that cross the network: replica head export (StartExposeBdev) and engine->replica connect (connectNVMfBdev). The engine's TransportType is now passed into NewEngine at construction rather than set on the returned object. All other exposure/connect sites (frontend, transient, clone, rebuild-src, external snapshot, EC shard/shardgroup) stay pinned to TCP, with TODOs on the frontend and transient paths for future RDMA support. validateNvmeTransport relaxed to accept TCP or RDMA. Default behavior is unchanged (unset => TCP). Ref: longhorn/longhorn#13796 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Alejandro Bonilla <abonilla@suse.com>

abonillabeeche · 2026-08-21T02:05:34Z

Thanks for the review — pushed a revision addressing the NITs.

engine.go:133 / field naming — renamed the struct field to TransportType on both Engine and Replica, and the client-method params to transportType.
server_engine.go:50 — TransportType is now an argument to NewEngine(...) and set at construction; dropped the post-construct assignment.
engine.go:379 (frontend) and replica.go:2542 (clone/rebuild/expansion) — added TODOs. These transient/host-facing paths stay on NVMe-TCP for this MVP; the internal engine↔replica head fabric is the only transport-selectable path. Honoring RDMA on the frontend also needs the initiator/host side to support it, so I'd rather do it as a follow-up than half-wire it here.
client_engine.go:48 — dropping the DataEngine prefix: the remaining DataEngineTransport here is the generated proto field from feat(proto): add TransportType (TCP/RDMA) to spdkrpc + imrpc types#121. Renaming it to transport_type is a clean change but cascades to longhorn-manager and instance-manager. Want me to rename it in Fix incorrect major and minor numbers of getDiskID() #121 for consistency, or keep the proto field namespaced and only use TransportType on the Go structs?
engine.go:401 — validating engine and replicas agree on trtype: good idea. Right now validateNvmeTransport accepts TCP or RDMA but doesn't cross-check that a replica's transport matches the engine's, because the engine doesn't yet see each replica's declared transport at connect time. I'll do this as a follow-up: surface the replica transport (via BackendView) and add the equality check by refactoring validateReplicaSize as you suggested. Flagging it on the tracking issue.
RDMA hardware detection: agreed it belongs in longhorn-manager rather than here — it can reject an RDMA engine before scheduling, and it's the right place to reason about mixed clusters where only some nodes have RoCE-capable NICs. I'll add node-level RDMA-capability detection + validation on the manager side (feat(volume): RDMA (RoCEv2) transport support for the v2 data engine longhorn-manager#5107, tracked in [FEATURE] V2 Engine: RDMA (RoCEv2) NVMe-oF transport — working implementation + arm64 benchmarks longhorn#13796) rather than in the spdk-server.

Default behavior is unchanged: an unset transport maps to TCP, so an older control plane keeps working. Not e2e-tested in this revision.

Rename the struct field to TransportType on Engine and Replica and the client-method params to transportType, and consume the renamed spdkrpc.TransportType enum. No behavior change. Ref: longhorn/longhorn#13796 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Alejandro Bonilla <abonilla@suse.com>

Address review on longhorn/longhorn-spdk-engine#654: this proto lives in a v2-data-engine-only surface, and this repo's convention is TransportType, so drop the DataEngine prefix on the generated type. - imrpc/spdkrpc: enum DataEngineTransport -> TransportType, values DATA_ENGINE_TRANSPORT_{TCP,RDMA} -> TRANSPORT_TYPE_{TCP,RDMA}, field data_engine_transport -> transport_type. Wire-compatible: field numbers and enum values are unchanged, so already deployed peers keep interoperating; only source that references the Go/py identifiers must be recompiled. Ref: longhorn/longhorn#13796 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Alejandro Bonilla <abonilla@suse.com>

…for v2 Address review on longhorn/longhorn-spdk-engine#654: detect RDMA hardware availability in longhorn-manager so an invalid RDMA volume is rejected early, and handle mixed clusters where only some nodes are RoCE-capable. Node detection: - environment check monitor reports a new RDMACapable node condition for v2-enabled nodes, based on whether /sys/class/infiniband enumerates any device. Validation and scheduling: - volume validator rejects dataEngineTransport=rdma unless the data engine is v2 and at least NumberOfReplicas RDMA-capable nodes exist (checked on create and on replica scale-up). - replica scheduler confines RDMA-volume replicas to RDMA-capable nodes, keeping mixed clusters correct. Device access: - new opt-in setting v2-data-engine-rdma-device-resource. When set to an extended resource advertised by an RDMA shared device plugin (e.g. k8s-rdma-shared-dev-plugin), the v2 instance-manager pod requests it, gaining declarative shared access to the RoCE verbs device without a bond and preserving per-pod RoCEv2 GIDs. Empty (default) keeps the prior privileged host-mount behavior. Ref: longhorn/longhorn#13796 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Alejandro Bonilla <abonilla@suse.com>

This was referenced Aug 20, 2026

feat(instance): forward v2 data-engine transport to SPDK create calls longhorn/longhorn-instance-manager#1616

Draft

[FEATURE] V2 Engine: RDMA (RoCEv2) NVMe-oF transport — working implementation + arm64 benchmarks longhorn/longhorn#13796

Open

abonillabeeche force-pushed the rdma-transport branch from b956331 to 1ec2e82 Compare August 20, 2026 05:18

abonillabeeche changed the title ~~spdk: thread NVMe-oF transport type through engine/replica create~~ feat(spdk): thread NVMe-oF transport type through engine/replica create Aug 20, 2026

shuo-wu reviewed Aug 20, 2026

View reviewed changes

abonillabeeche force-pushed the rdma-transport branch from 1ec2e82 to dfc5f2e Compare August 21, 2026 02:05

abonillabeeche mentioned this pull request Aug 22, 2026

feat(proto): add TransportType (TCP/RDMA) to spdkrpc + imrpc longhorn/types#121

Draft

Navigation Menu

Uh oh!

feat(spdk): thread NVMe-oF transport type through engine/replica create - #654

feat(spdk): thread NVMe-oF transport type through engine/replica create#654
abonillabeeche wants to merge 2 commits into
longhorn:masterfrom
abonillabeeche:rdma-transport

abonillabeeche commented Aug 20, 2026

Uh oh!

shuo-wu Aug 20, 2026

Uh oh!

abonillabeeche Aug 21, 2026

Uh oh!

shuo-wu Aug 20, 2026

Uh oh!

shuo-wu Aug 20, 2026

Uh oh!

abonillabeeche Aug 21, 2026

Uh oh!

shuo-wu Aug 20, 2026

Uh oh!

shuo-wu Aug 20, 2026

Uh oh!

abonillabeeche Aug 21, 2026

Uh oh!

shuo-wu Aug 20, 2026

Uh oh!

abonillabeeche Aug 21, 2026

Uh oh!

shuo-wu commented Aug 20, 2026

Uh oh!

abonillabeeche commented Aug 21, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

abonillabeeche commented Aug 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shuo-wu commented Aug 20, 2026

Uh oh!

abonillabeeche commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

abonillabeeche commented Aug 21, 2026 •

edited

Loading