drivers/nvme/host/tcp.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
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
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
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
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.0A 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.0On 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()
© 2016 - 2026 Red Hat, Inc.