From 4a24f581d6a490b3a4dbdbbdf75f4c01323b8ef6 Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Thu, 30 Jun 2022 21:15:58 +0000 Subject: [PATCH] nvme: add cmd/cpl printing for tcp errors This follows similar logic in the pcie completion path, including omitting error messages when aborting aers by adding a print_on_error parameter to the completion function. Signed-off-by: Jim Harris Change-Id: I96df72280bb8fcbee3847fdc27f38e14a1bf3251 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/13522 Tested-by: SPDK CI Jenkins Reviewed-by: Changpeng Liu Reviewed-by: Shuhei Matsumoto Community-CI: Broadcom CI --- lib/nvme/nvme_tcp.c | 23 ++++++++++++++++----- test/unit/lib/nvme/nvme_tcp.c/nvme_tcp_ut.c | 13 ++++++++++-- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/lib/nvme/nvme_tcp.c b/lib/nvme/nvme_tcp.c index 3a29723fb..0a1394705 100644 --- a/lib/nvme/nvme_tcp.c +++ b/lib/nvme/nvme_tcp.c @@ -153,7 +153,7 @@ static int64_t nvme_tcp_poll_group_process_completions(struct spdk_nvme_transpor *tgroup, uint32_t completions_per_qpair, spdk_nvme_disconnected_qpair_cb disconnected_qpair_cb); static void nvme_tcp_icresp_handle(struct nvme_tcp_qpair *tqpair, struct nvme_tcp_pdu *pdu); static void nvme_tcp_req_complete(struct nvme_tcp_req *tcp_req, struct nvme_tcp_qpair *tqpair, - struct spdk_nvme_cpl *rsp); + struct spdk_nvme_cpl *rsp, bool print_on_error); static inline struct nvme_tcp_qpair * nvme_tcp_qpair(struct spdk_nvme_qpair *qpair) @@ -629,7 +629,7 @@ nvme_tcp_req_complete_safe(struct nvme_tcp_req *tcp_req) tcp_req->tqpair->async_complete++; } - nvme_tcp_req_complete(tcp_req, tcp_req->tqpair, &tcp_req->rsp); + nvme_tcp_req_complete(tcp_req, tcp_req->tqpair, &tcp_req->rsp, true); return true; } @@ -744,13 +744,15 @@ nvme_tcp_qpair_reset(struct spdk_nvme_qpair *qpair) static void nvme_tcp_req_complete(struct nvme_tcp_req *tcp_req, struct nvme_tcp_qpair *tqpair, - struct spdk_nvme_cpl *rsp) + struct spdk_nvme_cpl *rsp, + bool print_on_error) { struct spdk_nvme_cpl cpl; spdk_nvme_cmd_cb user_cb; void *user_cb_arg; struct spdk_nvme_qpair *qpair; struct nvme_request *req; + bool error, print_error; assert(tcp_req->req != NULL); req = tcp_req->req; @@ -761,6 +763,17 @@ nvme_tcp_req_complete(struct nvme_tcp_req *tcp_req, user_cb_arg = req->cb_arg; qpair = req->qpair; + error = spdk_nvme_cpl_is_error(rsp); + print_error = error && print_on_error && !qpair->ctrlr->opts.disable_error_logging; + + if (print_error) { + spdk_nvme_qpair_print_command(qpair, &req->cmd); + } + + if (print_error || SPDK_DEBUGLOG_FLAG_ENABLED("nvme")) { + spdk_nvme_qpair_print_completion(qpair, rsp); + } + TAILQ_REMOVE(&tcp_req->tqpair->outstanding_reqs, tcp_req, link); nvme_tcp_req_put(tqpair, tcp_req); nvme_free_request(req); @@ -779,7 +792,7 @@ nvme_tcp_qpair_abort_reqs(struct spdk_nvme_qpair *qpair, uint32_t dnr) cpl.status.dnr = dnr; TAILQ_FOREACH_SAFE(tcp_req, &tqpair->outstanding_reqs, link, tmp) { - nvme_tcp_req_complete(tcp_req, tqpair, &cpl); + nvme_tcp_req_complete(tcp_req, tqpair, &cpl, true); } } @@ -2197,7 +2210,7 @@ nvme_tcp_admin_qpair_abort_aers(struct spdk_nvme_qpair *qpair) continue; } - nvme_tcp_req_complete(tcp_req, tqpair, &cpl); + nvme_tcp_req_complete(tcp_req, tqpair, &cpl, false); } } diff --git a/test/unit/lib/nvme/nvme_tcp.c/nvme_tcp_ut.c b/test/unit/lib/nvme/nvme_tcp.c/nvme_tcp_ut.c index ee3abb477..fa802b2b1 100644 --- a/test/unit/lib/nvme/nvme_tcp.c/nvme_tcp_ut.c +++ b/test/unit/lib/nvme/nvme_tcp.c/nvme_tcp_ut.c @@ -39,6 +39,12 @@ DEFINE_STUB(spdk_nvme_poll_group_process_completions, int64_t, (struct spdk_nvme DEFINE_STUB(nvme_poll_group_connect_qpair, int, (struct spdk_nvme_qpair *qpair), 0); DEFINE_STUB_V(nvme_qpair_resubmit_requests, (struct spdk_nvme_qpair *qpair, uint32_t num_requests)); +DEFINE_STUB_V(spdk_nvme_qpair_print_command, (struct spdk_nvme_qpair *qpair, + struct spdk_nvme_cmd *cmd)); + +DEFINE_STUB_V(spdk_nvme_qpair_print_completion, (struct spdk_nvme_qpair *qpair, + struct spdk_nvme_cpl *cpl)); + static void test_nvme_tcp_pdu_set_data_buf(void) { @@ -1333,6 +1339,7 @@ static void test_nvme_tcp_capsule_resp_hdr_handle(void) { struct nvme_tcp_qpair tqpair = {}; + struct spdk_nvme_ctrlr ctrlr = {}; struct spdk_nvme_tcp_stat stats = {}; struct nvme_request req = {}; struct spdk_nvme_cpl rccqe_tgt = {}; @@ -1344,6 +1351,7 @@ test_nvme_tcp_capsule_resp_hdr_handle(void) tqpair.num_entries = 1; tqpair.stats = &stats; req.qpair = &tqpair.qpair; + req.qpair->ctrlr = &ctrlr; rc = nvme_tcp_alloc_reqs(&tqpair); SPDK_CU_ASSERT_FATAL(rc == 0); @@ -1520,7 +1528,7 @@ test_nvme_tcp_ctrlr_create_io_qpair(void) static void test_nvme_tcp_ctrlr_delete_io_qpair(void) { - struct spdk_nvme_ctrlr *ctrlr = (struct spdk_nvme_ctrlr *)0xdeadbeef; + struct spdk_nvme_ctrlr ctrlr = {}; struct spdk_nvme_qpair *qpair; struct nvme_tcp_qpair *tqpair; struct nvme_tcp_req tcp_req = {}; @@ -1532,6 +1540,7 @@ test_nvme_tcp_ctrlr_delete_io_qpair(void) tqpair->send_pdus = calloc(1, sizeof(struct nvme_tcp_pdu)); tqpair->qpair.trtype = SPDK_NVME_TRANSPORT_TCP; qpair = &tqpair->qpair; + qpair->ctrlr = &ctrlr; tcp_req.req = &req; tcp_req.req->qpair = &tqpair->qpair; tcp_req.req->cb_fn = ut_nvme_complete_request; @@ -1540,7 +1549,7 @@ test_nvme_tcp_ctrlr_delete_io_qpair(void) TAILQ_INIT(&tqpair->outstanding_reqs); TAILQ_INSERT_TAIL(&tcp_req.tqpair->outstanding_reqs, &tcp_req, link); - rc = nvme_tcp_ctrlr_delete_io_qpair(ctrlr, qpair); + rc = nvme_tcp_ctrlr_delete_io_qpair(&ctrlr, qpair); CU_ASSERT(rc == 0); }