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.0