diff --git a/lib/nvme/nvme.c b/lib/nvme/nvme.c index 819f0d6c9..5f34451ea 100644 --- a/lib/nvme/nvme.c +++ b/lib/nvme/nvme.c @@ -244,6 +244,7 @@ nvme_completion_poll_cb(void *arg, const struct spdk_nvme_cpl *cpl) if (status->timed_out) { /* There is no routine waiting for the completion of this request, free allocated memory */ + spdk_free(status->dma_data); free(status); return; } diff --git a/lib/nvme/nvme_fabric.c b/lib/nvme/nvme_fabric.c index 4a3c4c193..6f4896fc8 100644 --- a/lib/nvme/nvme_fabric.c +++ b/lib/nvme/nvme_fabric.c @@ -416,6 +416,8 @@ nvme_fabric_qpair_connect(struct spdk_nvme_qpair *qpair, uint32_t num_entries) return -ENOMEM; } + status->dma_data = nvmf_data; + memset(&cmd, 0, sizeof(cmd)); cmd.opcode = SPDK_NVME_OPC_FABRIC; cmd.fctype = SPDK_NVMF_FABRIC_COMMAND_CONNECT; @@ -441,16 +443,23 @@ nvme_fabric_qpair_connect(struct spdk_nvme_qpair *qpair, uint32_t num_entries) nvme_completion_poll_cb, status); if (rc < 0) { SPDK_ERRLOG("Failed to allocate/submit FABRIC_CONNECT command, rc %d\n", rc); - spdk_free(nvmf_data); + spdk_free(status->dma_data); free(status); return rc; } /* If we time out, the qpair will abort the request upon destruction. */ - rc = nvme_wait_for_completion_timeout(qpair, status, ctrlr->opts.fabrics_connect_timeout_us); - if (rc) { + if (ctrlr->opts.fabrics_connect_timeout_us > 0) { + status->timeout_tsc = spdk_get_ticks() + ctrlr->opts.fabrics_connect_timeout_us * + spdk_get_ticks_hz() / SPDK_SEC_TO_USEC; + } + + /* Wait until the command completes or times out */ + while (nvme_wait_for_completion_robust_lock_timeout_poll(qpair, status, NULL) == -EAGAIN) {} + + if (status->timed_out || spdk_nvme_cpl_is_error(&status->cpl)) { SPDK_ERRLOG("Connect command failed, rc %d, trtype:%s adrfam:%s traddr:%s trsvcid:%s subnqn:%s\n", - rc, + status->timed_out ? -ECANCELED : -EIO, spdk_nvme_transport_id_trtype_str(ctrlr->trid.trtype), spdk_nvme_transport_id_adrfam_str(ctrlr->trid.adrfam), ctrlr->trid.traddr, @@ -460,8 +469,8 @@ nvme_fabric_qpair_connect(struct spdk_nvme_qpair *qpair, uint32_t num_entries) SPDK_ERRLOG("Connect command completed with error: sct %d, sc %d\n", status->cpl.status.sct, status->cpl.status.sc); } - spdk_free(nvmf_data); if (!status->timed_out) { + spdk_free(status->dma_data); free(status); } return -EIO; @@ -473,7 +482,7 @@ nvme_fabric_qpair_connect(struct spdk_nvme_qpair *qpair, uint32_t num_entries) SPDK_DEBUGLOG(nvme, "CNTLID 0x%04" PRIx16 "\n", ctrlr->cntlid); } - spdk_free(nvmf_data); + spdk_free(status->dma_data); free(status); return 0; } diff --git a/lib/nvme/nvme_internal.h b/lib/nvme/nvme_internal.h index f954d1959..e7a84885a 100644 --- a/lib/nvme/nvme_internal.h +++ b/lib/nvme/nvme_internal.h @@ -383,6 +383,11 @@ struct nvme_request { struct nvme_completion_poll_status { struct spdk_nvme_cpl cpl; uint64_t timeout_tsc; + /** + * DMA buffer retained throughout the duration of the command. It'll be released + * automatically if the command times out, otherwise the user is responsible for freeing it. + */ + void *dma_data; bool done; /* This flag indicates that the request has been timed out and the memory must be freed in a completion callback */ diff --git a/test/unit/lib/nvme/nvme_fabric.c/nvme_fabric_ut.c b/test/unit/lib/nvme/nvme_fabric.c/nvme_fabric_ut.c index 0b5c8a6c1..67c8ecd85 100644 --- a/test/unit/lib/nvme/nvme_fabric.c/nvme_fabric_ut.c +++ b/test/unit/lib/nvme/nvme_fabric.c/nvme_fabric_ut.c @@ -42,8 +42,6 @@ pid_t g_spdk_nvme_pid; struct spdk_nvmf_fabric_prop_set_cmd g_ut_cmd = {}; struct spdk_nvmf_fabric_prop_get_rsp g_ut_response = {}; -DEFINE_STUB_V(nvme_completion_poll_cb, (void *arg, const struct spdk_nvme_cpl *cpl)); - DEFINE_STUB_V(spdk_nvme_ctrlr_get_default_ctrlr_opts, (struct spdk_nvme_ctrlr_opts *opts, size_t opts_size)); @@ -82,6 +80,7 @@ DEFINE_STUB(spdk_nvme_transport_id_adrfam_str, const char *, DEFINE_STUB(nvme_ctrlr_process_init, int, (struct spdk_nvme_ctrlr *ctrlr), 0); static struct spdk_nvmf_fabric_connect_data g_nvmf_data; +static struct nvme_request *g_request; int spdk_nvme_ctrlr_cmd_io_raw(struct spdk_nvme_ctrlr *ctrlr, @@ -100,15 +99,30 @@ spdk_nvme_ctrlr_cmd_io_raw(struct spdk_nvme_ctrlr *ctrlr, memcpy(&req->cmd, cmd, sizeof(req->cmd)); memcpy(&g_nvmf_data, buf, sizeof(g_nvmf_data)); + g_request = req; return 0; } -DEFINE_RETURN_MOCK(nvme_wait_for_completion_timeout, int); +void +nvme_completion_poll_cb(void *arg, const struct spdk_nvme_cpl *cpl) +{ + struct nvme_completion_poll_status *status = arg; + + if (status->timed_out) { + spdk_free(status->dma_data); + free(status); + } + + g_request = NULL; +} + +static bool g_nvme_wait_for_completion_timeout; + int -nvme_wait_for_completion_timeout(struct spdk_nvme_qpair *qpair, - struct nvme_completion_poll_status *status, - uint64_t timeout_in_usecs) +nvme_wait_for_completion_robust_lock_timeout_poll(struct spdk_nvme_qpair *qpair, + struct nvme_completion_poll_status *status, + pthread_mutex_t *robust_mutex) { struct spdk_nvmf_fabric_connect_rsp *rsp = (void *)&status->cpl; @@ -116,8 +130,7 @@ nvme_wait_for_completion_timeout(struct spdk_nvme_qpair *qpair, rsp->status_code_specific.success.cntlid = 1; } - status->timed_out = false; - HANDLE_RETURN_MOCK(nvme_wait_for_completion_timeout); + status->timed_out = g_nvme_wait_for_completion_timeout; return 0; } @@ -218,6 +231,19 @@ spdk_nvme_ctrlr_cmd_admin_raw(struct spdk_nvme_ctrlr *ctrlr, return 0; } +static void +abort_request(struct nvme_request *request) +{ + struct spdk_nvme_cpl cpl = { + .status = { + .sct = SPDK_NVME_SCT_GENERIC, + .sc = SPDK_NVME_SC_ABORTED_SQ_DELETION, + } + }; + + request->cb_fn(request->cb_arg, &cpl); +} + static void test_nvme_fabric_prop_set_cmd(void) { @@ -403,11 +429,12 @@ test_nvme_fabric_qpair_connect(void) /* Wait_for completion timeout */ STAILQ_INSERT_HEAD(&qpair.free_req, &req, stailq); - MOCK_SET(nvme_wait_for_completion_timeout, 1); + g_nvme_wait_for_completion_timeout = true; rc = nvme_fabric_qpair_connect(&qpair, 1); CU_ASSERT(rc == -EIO); - MOCK_CLEAR(nvme_wait_for_completion_timeout); + g_nvme_wait_for_completion_timeout = false; + abort_request(g_request); /* Input parameters invalid */ rc = nvme_fabric_qpair_connect(&qpair, 0);