[PATCH v1 0/1] nvme-tcp: fix wrong status on deferred digest error

[PATCH v1 0/1] nvme-tcp: fix wrong status on deferred digest error

[Xixin Liu]

Hi,

A C2HData digest error stores a host status code in the per-request
status field. Without DATA_SUCCESS the request is completed later from
the rsp path. That path used to pass the stored value straight into
complete, which expects CQE status field encoding and shifts right, so
DATA_XFER_ERROR could be reported as INVALID_FIELD.

This series keeps the per-request status as a host status code. On the
rsp path, shift left when a host error was already stored, otherwise use
the completion status field.

To hit the TCP DATA_XFER_ERROR path without DATA_SUCCESS, enable data
digest and temporarily change the target in
drivers/nvme/target/tcp.c, nvmet_setup_c2h_data_pdu, after the digest
is calculated:

  if (queue->data_digest) {
          pdu->hdr.flags |= NVME_TCP_F_DDGST;
          nvmet_tcp_calc_ddgst(cmd);
          cmd->exp_ddgst ^= cpu_to_le32(1);
  }

The default target path does not set DATA_SUCCESS on C2HData, so the
host stores the digest error and completes from the later rsp.

Verified on linux-next with an nvmet-tcp loopback target. Unpatched:

  nvme nvme0: data digest error: recv 0x2542708 expected 0x2542709
  nvme nvme0: Identify Controller failed (2)

Patched:

  nvme nvme0: data digest error: recv 0x7ebd5719 expected 0x7ebd5718
  nvme nvme0: Identify Controller failed (4)

Status code 0x2 is INVALID_FIELD. Status code 0x4 is DATA_XFER_ERROR.
recv and expected differ by one in both runs.

Thanks,
Xixin Liu

---

Xixin Liu (1):
  nvme-tcp: fix wrong status on deferred digest error

 drivers/nvme/host/tcp.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

-- 
2.53.0

[PATCH v1 1/1] nvme-tcp: fix wrong status on deferred digest error

[Xixin Liu]

A C2HData digest error stores a host status code in req status. Without
DATA_SUCCESS the request is completed later from the rsp path, which
passed that value straight into complete and could report the wrong
status code.

Keep req status as a host status code. On the rsp path, shift left when
a host error was already stored, otherwise use the completion status
field.

Fixes: 1ba2e507f55c ("nvme-tcp: Do not reset transport on data digest errors")
Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
---
 drivers/nvme/host/tcp.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

--- a/drivers/nvme/host/tcp.c	2026-08-26 09:24:50.988099281 +0800
+++ b/drivers/nvme/host/tcp.c	2026-08-26 09:25:12.474510519 +0800
@@ -73,7 +73,7 @@
 	u32			h2cdata_left;
 	u32			h2cdata_offset;
 	u16			ttag;
-	__le16			status;
+	u16			status;
 	struct list_head	entry;
 	struct llist_node	lentry;
 	__le32			ddgst;
@@ -617,6 +617,7 @@
 {
 	struct nvme_tcp_request *req;
 	struct request *rq;
+	__le16 status;
 
 	rq = nvme_find_rq(nvme_tcp_tagset(queue), cqe->command_id);
 	if (!rq) {
@@ -628,10 +629,12 @@
 	}
 
 	req = blk_mq_rq_to_pdu(rq);
-	if (req->status == cpu_to_le16(NVME_SC_SUCCESS))
-		req->status = cqe->status;
+	if (req->status != NVME_SC_SUCCESS)
+		status = cpu_to_le16(req->status << 1);
+	else
+		status = cqe->status;
 
-	if (!nvme_try_complete_req(rq, req->status, cqe->result))
+	if (!nvme_try_complete_req(rq, status, cqe->result))
 		nvme_complete_rq(rq);
 	queue->nr_cqe++;
 
@@ -961,8 +964,7 @@
 			queue->ddgst_remaining = NVME_TCP_DIGEST_LENGTH;
 		} else {
 			if (pdu->hdr.flags & NVME_TCP_F_DATA_SUCCESS) {
-				nvme_tcp_end_request(rq,
-						le16_to_cpu(req->status));
+				nvme_tcp_end_request(rq, req->status);
 				queue->nr_cqe++;
 			}
 			nvme_tcp_init_recv_ctx(queue);
@@ -996,7 +998,7 @@
 					pdu->command_id);
 		struct nvme_tcp_request *req = blk_mq_rq_to_pdu(rq);
 
-		req->status = cpu_to_le16(NVME_SC_DATA_XFER_ERROR);
+		req->status = NVME_SC_DATA_XFER_ERROR;
 
 		dev_err(queue->ctrl->ctrl.device,
 			"data digest error: recv %#x expected %#x\n",
@@ -1009,7 +1011,7 @@
 					pdu->command_id);
 		struct nvme_tcp_request *req = blk_mq_rq_to_pdu(rq);
 
-		nvme_tcp_end_request(rq, le16_to_cpu(req->status));
+		nvme_tcp_end_request(rq, req->status);
 		queue->nr_cqe++;
 	}
 
@@ -2733,7 +2735,7 @@
 		return ret;
 
 	req->state = NVME_TCP_SEND_CMD_PDU;
-	req->status = cpu_to_le16(NVME_SC_SUCCESS);
+	req->status = NVME_SC_SUCCESS;
 	req->offset = 0;
 	req->data_sent = 0;
 	req->pdu_len = 0;
-- 
2.53.0

Re: [PATCH v1 1/1] nvme-tcp: fix wrong status on deferred digest error

[Hannes Reinecke]

On 8/26/26 3:30 AM, Xixin Liu wrote:
> A C2HData digest error stores a host status code in req status. Without
> DATA_SUCCESS the request is completed later from the rsp path, which
> passed that value straight into complete and could report the wrong
> status code.
> 
> Keep req status as a host status code. On the rsp path, shift left when
> a host error was already stored, otherwise use the completion status
> field.
> 
I really don't see it. Where is the issue?
AFAICS we're just changing the type of 'status' from holding the
wire-value to the host value.

Can you make the patch simpler by just keeping 'status' as __le16?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH v1 0/1] nvme-tcp: fix wrong status on deferred digest error

[Xixin Liu]

Hi Hannes,

Thanks for the review. This is not a wire to host conversion. req->status
already mixes wire and host values. Digest error stores a host SC. u16
makes the host SC explicit. __le16 is endian packaging for wire layout.
The SUCCESS rsp branch overwrites with cqe->status, which is wire. A host
SC must be shifted left before nvme_try_complete_req.

Without DATA_SUCCESS, digest error is deferred to the rsp path:

	req->status = cpu_to_le16(NVME_SC_DATA_XFER_ERROR);   /* host SC */

	if (req->status == cpu_to_le16(NVME_SC_SUCCESS))
		req->status = cqe->status;   /* wire */
	if (!nvme_try_complete_req(rq, req->status, cqe->result))

Both branches pass req->status into nvme_try_complete_req, which expects
wire:

	nvme_req(req)->status = le16_to_cpu(status) >> 1;

SUCCESS is fine. Non-SUCCESS still holds host SC 04h Data Transfer Error,
so 0x4 becomes INVALID_FIELD, 0x2.

v1 changed req->status to u16. v2 keeps __le16 as you suggested. Please
take a look at both and say which you prefer for merge. I will send v2
shortly.

Thanks,
Xixin Liu

[PATCH v2 0/1] nvme-tcp: fix wrong status on deferred digest error

[Xixin Liu]

Hi,

Thanks Hannes for the review on v1.

Changes since v1:
- keep req->status as __le16
- only fix the rsp path: shift left when a deferred host error is stored

Same loopback test as v1, PASS.

Thanks,
Xixin Liu

---

Xixin Liu (1):
  nvme-tcp: fix wrong status on deferred digest error

 drivers/nvme/host/tcp.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

-- 
2.53.0

[PATCH v2 1/1] nvme-tcp: fix wrong status on deferred digest error

[Xixin Liu]

A C2HData digest error stores a host status code in req->status. Without
DATA_SUCCESS completion goes through the rsp path, which passed that host
value straight into complete as CQE Status wire encoding.

On the rsp path, shift left when a deferred host error is stored,
otherwise use cqe->status.

Fixes: 1ba2e507f55c ("nvme-tcp: Do not reset transport on data digest errors")
Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
---
 drivers/nvme/host/tcp.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -617,6 +617,7 @@ static int nvme_tcp_process_nvme_cqe(struct nvme_tcp_queue *queue,
 {
 	struct nvme_tcp_request *req;
 	struct request *rq;
+	__le16 status;
 
 	rq = nvme_find_rq(nvme_tcp_tagset(queue), cqe->command_id);
 	if (!rq) {
@@ -628,10 +629,12 @@ static int nvme_tcp_process_nvme_cqe(struct nvme_tcp_queue *queue,
 	}
 
 	req = blk_mq_rq_to_pdu(rq);
 	if (req->status == cpu_to_le16(NVME_SC_SUCCESS))
-		req->status = cqe->status;
+		status = cqe->status;
+	else
+		status = cpu_to_le16(le16_to_cpu(req->status) << 1);
 
-	if (!nvme_try_complete_req(rq, req->status, cqe->result))
+	if (!nvme_try_complete_req(rq, status, cqe->result))
 		nvme_complete_rq(rq);
 	queue->nr_cqe++;
 
-- 
2.53.0

Re: [PATCH v2 1/1] nvme-tcp: fix wrong status on deferred digest error

[Sagi Grimberg]

On 27/08/2026 4:05, Xixin Liu wrote:
> A C2HData digest error stores a host status code in req->status. Without
> DATA_SUCCESS completion goes through the rsp path, which passed that host
> value straight into complete as CQE Status wire encoding.
>
> On the rsp path, shift left when a deferred host error is stored,
> otherwise use cqe->status.

Why not just assign with a shifted status?
We already do this in nvme_tcp_end_request()

Re: [PATCH v2 1/1] nvme-tcp: fix wrong status on deferred digest error

[Xixin Liu]

Hi Sagi,

Thanks for the review. v3 follows your suggestion: host status on the rsp
path, complete through nvme_tcp_end_request with cqe->result. I will send
v3 shortly.

Thanks,
Xixin Liu

[PATCH v3 0/1] nvme-tcp: fix wrong status on deferred digest error

[Xixin Liu]

Hi,

Thanks for the review on v2.

Changes since v2:
- complete the rsp path through nvme_tcp_end_request with host status

Thanks,
Xixin Liu

---

Xixin Liu (1):
  nvme-tcp: fix wrong status on deferred digest error

 drivers/nvme/host/tcp.c | 36 ++++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

-- 
2.53.0

[PATCH v3 1/1] nvme-tcp: fix wrong status on deferred digest error

[Xixin Liu]

A C2HData digest error stores a host status code in req->status. Without
DATA_SUCCESS completion goes through the rsp path, which passed that host
value straight into complete as CQE Status wire encoding.

On the rsp path, pick host status from the CQE status field or from the
stored deferred error, then complete through nvme_tcp_end_request with
the CQE result.

Fixes: 1ba2e507f55c ("nvme-tcp: Do not reset transport on data digest errors")
Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
---
 drivers/nvme/host/tcp.c | 36 +++++++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 15 deletions(-)

--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -612,11 +612,19 @@ static void nvme_tcp_error_recovery(struct nvme_ctrl *ctrl)
 	queue_work(nvme_reset_wq, &to_tcp_ctrl(ctrl)->err_work);
 }
 
+static inline void nvme_tcp_end_request(struct request *rq, u16 status,
+					union nvme_result result)
+{
+	if (!nvme_try_complete_req(rq, cpu_to_le16(status << 1), result))
+		nvme_complete_rq(rq);
+}
+
 static int nvme_tcp_process_nvme_cqe(struct nvme_tcp_queue *queue,
 		struct nvme_completion *cqe)
 {
 	struct nvme_tcp_request *req;
 	struct request *rq;
+	u16 status;
 
 	rq = nvme_find_rq(nvme_tcp_tagset(queue), cqe->command_id);
 	if (!rq) {
@@ -629,10 +637,11 @@ static int nvme_tcp_process_nvme_cqe(struct nvme_tcp_queue *queue,
 
 	req = blk_mq_rq_to_pdu(rq);
 	if (req->status == cpu_to_le16(NVME_SC_SUCCESS))
-		req->status = cqe->status;
+		status = le16_to_cpu(cqe->status) >> 1;
+	else
+		status = le16_to_cpu(req->status);
 
-	if (!nvme_try_complete_req(rq, req->status, cqe->result))
-		nvme_complete_rq(rq);
+	nvme_tcp_end_request(rq, status, cqe->result);
 	queue->nr_cqe++;
 
 	return 0;
@@ -893,14 +902,6 @@ static int nvme_tcp_recv_pdu(struct nvme_tcp_queue *queue, struct sk_buff *skb,
 	return -EINVAL;
 }
 
-static inline void nvme_tcp_end_request(struct request *rq, u16 status)
-{
-	union nvme_result res = {};
-
-	if (!nvme_try_complete_req(rq, cpu_to_le16(status << 1), res))
-		nvme_complete_rq(rq);
-}
-
 static int nvme_tcp_recv_data(struct nvme_tcp_queue *queue, struct sk_buff *skb,
 			      unsigned int *offset, size_t *len)
 {
@@ -961,8 +962,9 @@ static int nvme_tcp_recv_data(struct nvme_tcp_queue *queue, struct sk_buff *skb,
 			queue->ddgst_remaining = NVME_TCP_DIGEST_LENGTH;
 		} else {
 			if (pdu->hdr.flags & NVME_TCP_F_DATA_SUCCESS) {
-				nvme_tcp_end_request(rq,
-						le16_to_cpu(req->status));
+				union nvme_result res = {};
+
+				nvme_tcp_end_request(rq, le16_to_cpu(req->status), res);
 				queue->nr_cqe++;
 			}
 			nvme_tcp_init_recv_ctx(queue);
@@ -1009,7 +1011,9 @@ static int nvme_tcp_recv_ddgst(struct nvme_tcp_queue *queue,
 					pdu->command_id);
 		struct nvme_tcp_request *req = blk_mq_rq_to_pdu(rq);
 
-		nvme_tcp_end_request(rq, le16_to_cpu(req->status));
+		union nvme_result res = {};
+
+		nvme_tcp_end_request(rq, le16_to_cpu(req->status), res);
 		queue->nr_cqe++;
 	}
 
@@ -1124,8 +1128,10 @@ static void nvme_tcp_fail_request(struct nvme_tcp_request *req)
 		nvme_complete_async_event(&req->queue->ctrl->ctrl,
 				cpu_to_le16(NVME_SC_HOST_PATH_ERROR), &res);
 	} else {
+		union nvme_result res = {};
+
 		nvme_tcp_end_request(blk_mq_rq_from_pdu(req),
-				NVME_SC_HOST_PATH_ERROR);
+				NVME_SC_HOST_PATH_ERROR, res);
 	}
 }
 
-- 
2.53.0