nvme/tcp: enable interrupt mode support for TCP transport by c3y1huang · Pull Request #60 · longhorn/spdk

c3y1huang · 2025-07-03T01:52:20Z

Which issue(s) this PR fixes:

Issue longhorn/longhorn#9834

What this PR does / why we need it:

Support interrupt mode for NVMe TCP transport.

Special notes for your reviewer:

None

Additional documentation or context

longhorn/longhorn#9834 (comment)

PhanLe1010 · 2025-08-28T02:27:27Z

Hi @c3y1huang Do you have documents to quickly ramp up on nvme/tcp layer? Thank you

c3y1huang · 2025-09-01T02:26:11Z

Hi @c3y1huang Do you have documents to quickly ramp up on nvme/tcp layer? Thank you

Maybe longhorn/longhorn#9834 (comment) helps.

derekbit

In general, LGTM.

longhorn/longhorn-9834 Signed-off-by: Chin-Ya Huang <chin-ya.huang@suse.com>

…in interrupt mode longhorn/longhorn-9834 Signed-off-by: Chin-Ya Huang <chin-ya.huang@suse.com>

longhorn/longhorn-9834 Signed-off-by: Chin-Ya Huang <chin-ya.huang@suse.com>

… asynchronously longhorn/longhorn-9834 Signed-off-by: Chin-Ya Huang <chin-ya.huang@suse.com>

longhorn/longhorn-9834 Signed-off-by: Chin-Ya Huang <chin-ya.huang@suse.com>

…terrupt mode longhorn/longhorn-9834 Signed-off-by: Chin-Ya Huang <chin-ya.huang@suse.com>

…oid EBADF longhorn/longhorn-9834 Signed-off-by: Chin-Ya Huang <chin-ya.huang@suse.com>

derekbit · 2025-09-04T04:23:04Z

Hello @c3y1huang

In SPDK's NVMe/TCP transport, when a TCP socket is not immediately writable, the associated command data is buffered in sock->queued_reqs. These are internal queues managed by SPDK and are not part of the OS's TCP send buffer. This means they are not visible to the OS and require additional handling to ensure the data is eventually flushed and transmitted.
Unlike the PCIe transport, where write() operations immediately push NVMe/TCP transport requires explicit flush of the queued data. Until SPDK flushes the content of sock->queued_reqs into the OS's TCP send buffer, no data will be sent over the network.

Can you point out where the queued requests are being flushed? Thank you.

c3y1huang · 2025-09-04T04:30:53Z

Can you point out where the queued requests are being flushed? Thank you.

https://github.com/c3y1huang/longhorn-spdk/blob/8c25ff0e13b5181099a76a4fedc0d527e6d8b94b/lib/nvme/nvme_tcp.c#L2135

derekbit · 2025-09-04T05:47:41Z

+        rc = spdk_nvme_poll_group_process_completions(ctx->poll_group, 0,
+                                                      nvme_transport_connect_qpair_fail);
+    } else {
+        rc = spdk_nvme_qpair_process_completions(qpair, 0);


Why do we need spdk_nvme_qpair_process_completions for none-fabrics case?

The start_async_qpair_connect function is introduced to resolve the blocking busy-wait loop used for connection completion, allowing async handling of the connection state changes.

The logic originates from

spdk/lib/nvme/nvme_transport.c

Lines 525 to 532 in ea6cb0b

while (nvme_qpair_get_state(qpair) == NVME_QPAIR_CONNECTING) {

if (qpair->poll_group && spdk_nvme_ctrlr_is_fabrics(ctrlr)) {

rc = spdk_nvme_poll_group_process_completions(

qpair->poll_group->group, 0,

nvme_transport_connect_qpair_fail);

} else {

rc = spdk_nvme_qpair_process_completions(qpair, 0);

}

, which relied on a busy-wait loop to poll for completion.

For TCP transport, this pattern is not strictly necessary since this PR introduces a poller to manage asynchronous connection states. However, I kept the unified logic across transports for consistency and to simplify the transport abstraction.

PhanLe1010 · 2025-09-09T02:56:05Z

+		} else {
+			/*
+			 * Non-blocking path for TCP transport. Use a poller to complete connection
+			 * asynchronously.
+			 */
+			rc = start_async_qpair_connect(qpair, nvme_qpair_connect_completion_cb, NULL);
+			if (rc != 0) {
+			    SPDK_ERRLOG("Failed to start async connect: %d\n", rc);
+			    goto err;


Sorry if my question does not make sense. Does it mean we also use poller for TCP transport even if in non-interrupt mode?

For non-interrupt mode (qpair->async), it wouldn't walk through the logic in here.

The poller you are referencing gets unregistered once the IO pair leaves the connecting state. That is to resolve the blocking busy-wait loop issue when the NVMe-oF target and initiator run in the same process: https://github.com/longhorn/spdk/pull/60/files#diff-7c17adad72a592dee70af34689ed1d1dafbd224d0d5574c2cc857265b4cf9b82R538-R541

That said, at this stage, interrupt mode still partially relies on the periodic poller for:

Admin Queue Operations: Still do periodic polling for keepalive and controller recovery.

I/O Queue Completion: Polling is used to for command completion. [FEATURE] V2 volumes support interrupt mode longhorn#9834 (comment)

We’ll improve this later in longhorn/longhorn#11662.

Got it. Thanks @c3y1huang !

c3y1huang self-assigned this Jul 3, 2025

c3y1huang force-pushed the 9834-v2-interrupt-mode-v25.05 branch 4 times, most recently from dee6812 to 0ef3b18 Compare July 7, 2025 08:09

c3y1huang force-pushed the 9834-v2-interrupt-mode-v25.05 branch 3 times, most recently from d32f6ad to aa845ff Compare July 22, 2025 00:51

c3y1huang force-pushed the 9834-v2-interrupt-mode-v25.05 branch 3 times, most recently from 44b5c47 to 9aaeca2 Compare July 28, 2025 07:22

c3y1huang force-pushed the 9834-v2-interrupt-mode-v25.05 branch 2 times, most recently from a1b4f6d to 00e9888 Compare August 6, 2025 01:55

c3y1huang force-pushed the 9834-v2-interrupt-mode-v25.05 branch from 00e9888 to d584f93 Compare August 20, 2025 00:38

c3y1huang marked this pull request as ready for review August 25, 2025 03:53

c3y1huang requested review from a team, DamiaSan, PhanLe1010, derekbit and shuo-wu August 25, 2025 03:55

c3y1huang mentioned this pull request Aug 25, 2025

[FEATURE] V2 volumes support interrupt mode longhorn/longhorn#9834

Closed

derekbit reviewed Sep 3, 2025

View reviewed changes

Comment thread lib/nvme/nvme_tcp.c Outdated

Comment thread lib/nvme/nvme_tcp.c Outdated

Comment thread lib/nvme/nvme_tcp.c Outdated

Comment thread lib/nvme/nvme_transport.c

Comment thread module/bdev/nvme/bdev_nvme.c

Comment thread module/bdev/nvme/bdev_nvme.c

c3y1huang force-pushed the 9834-v2-interrupt-mode-v25.05 branch from d584f93 to 8c25ff0 Compare September 3, 2025 03:48

c3y1huang requested a review from derekbit September 3, 2025 03:51

derekbit approved these changes Sep 3, 2025

View reviewed changes

Comment thread include/spdk/sock.h

Comment thread include/spdk_internal/sock_module.h

Comment thread lib/nvme/nvme_tcp.c

derekbit previously approved these changes Sep 3, 2025

View reviewed changes

c3y1huang added 2 commits September 4, 2025 08:57

nvme/tcp: enable interrupt mode support for TCP transport

c4ac0f7

longhorn/longhorn-9834 Signed-off-by: Chin-Ya Huang <chin-ya.huang@suse.com>

nvme/tcp: skip redundant TCP I/O qpair registration

a22c861

longhorn/longhorn-9834 Signed-off-by: Chin-Ya Huang <chin-ya.huang@suse.com>

c3y1huang added 7 commits September 4, 2025 08:57

bdev/nvme: enforce ioq polling period (100ms) for NVMe TCP transport …

058452f

…in interrupt mode longhorn/longhorn-9834 Signed-off-by: Chin-Ya Huang <chin-ya.huang@suse.com>

nvme: non-blocking I/O qpair connect for TCP in interrupt mode

ec8af32

longhorn/longhorn-9834 Signed-off-by: Chin-Ya Huang <chin-ya.huang@suse.com>

examples/vmd: link 'thread' lib

9645ba6

longhorn/longhorn-9834 Signed-off-by: Chin-Ya Huang <chin-ya.huang@suse.com>

nvme: replacing busy wait loop with poller to complete TCP connection…

0f898b6

… asynchronously longhorn/longhorn-9834 Signed-off-by: Chin-Ya Huang <chin-ya.huang@suse.com>

app/spdk_tgt: remove event_iscsi to support interrupt mode in Longhorn

1b75550

longhorn/longhorn-9834 Signed-off-by: Chin-Ya Huang <chin-ya.huang@suse.com>

bdev/nvme: enforce adminq polling period for NVMe TCP transport in in…

388d842

…terrupt mode longhorn/longhorn-9834 Signed-off-by: Chin-Ya Huang <chin-ya.huang@suse.com>

bdev/nvme: force adminq poll period for NVMe/TCP interrupt mode to av…

52394ba

…oid EBADF longhorn/longhorn-9834 Signed-off-by: Chin-Ya Huang <chin-ya.huang@suse.com>

c3y1huang dismissed derekbit’s stale review via 52394ba September 4, 2025 00:57

c3y1huang force-pushed the 9834-v2-interrupt-mode-v25.05 branch from 8c25ff0 to 52394ba Compare September 4, 2025 00:57

c3y1huang requested a review from derekbit September 4, 2025 00:58

derekbit reviewed Sep 4, 2025

View reviewed changes

c3y1huang requested a review from derekbit September 4, 2025 07:26

derekbit approved these changes Sep 4, 2025

View reviewed changes

derekbit merged commit 53eafc8 into longhorn:longhorn-v25.05 Sep 4, 2025
2 checks passed

c3y1huang deleted the 9834-v2-interrupt-mode-v25.05 branch September 4, 2025 08:29

PhanLe1010 reviewed Sep 9, 2025

View reviewed changes

	while (nvme_qpair_get_state(qpair) == NVME_QPAIR_CONNECTING) {
	if (qpair->poll_group && spdk_nvme_ctrlr_is_fabrics(ctrlr)) {
	rc = spdk_nvme_poll_group_process_completions(
	qpair->poll_group->group, 0,
	nvme_transport_connect_qpair_fail);
	} else {
	rc = spdk_nvme_qpair_process_completions(qpair, 0);
	}

Uh oh!

Conversation

c3y1huang commented Jul 3, 2025

Which issue(s) this PR fixes:

What this PR does / why we need it:

Special notes for your reviewer:

Additional documentation or context

Uh oh!

PhanLe1010 commented Aug 28, 2025

Uh oh!

c3y1huang commented Sep 1, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

derekbit left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

derekbit commented Sep 4, 2025

Uh oh!

c3y1huang commented Sep 4, 2025

Uh oh!

derekbit Sep 4, 2025

Choose a reason for hiding this comment

Uh oh!

c3y1huang Sep 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PhanLe1010 Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

c3y1huang Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PhanLe1010 Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

c3y1huang Sep 9, 2025 •

edited

Loading