From 5b4b66bab95d61792d07768219190ded252037d2 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Thu, 17 Aug 2017 16:28:00 -0700 Subject: [PATCH] nvmf: move admin processing to ctrlr.c Now that the discovery controller is using the common admin command functions, move all of them into the common ctrlr.c file. This also eliminates the subsystem ops, which are now just direct calls. Change-Id: I0a25a61e0ad8742d3d76a3cacd46db4701fc7d63 Signed-off-by: Daniel Verkamp Reviewed-on: https://review.gerrithub.io/374733 Tested-by: SPDK Automated Test System Reviewed-by: Ben Walker Reviewed-by: Jim Harris --- include/spdk/nvmf.h | 3 - lib/nvmf/ctrlr.c | 198 ++++++++++++++++-- lib/nvmf/ctrlr.h | 20 +- lib/nvmf/ctrlr_bdev.c | 172 +-------------- lib/nvmf/ctrlr_discovery.c | 54 ----- lib/nvmf/nvmf_internal.h | 27 --- lib/nvmf/request.c | 4 +- lib/nvmf/subsystem.c | 21 +- lib/nvmf/subsystem.h | 4 +- test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c | 6 + .../lib/nvmf/ctrlr_bdev.c/ctrlr_bdev_ut.c | 76 ------- .../ctrlr_discovery.c/ctrlr_discovery_ut.c | 42 +--- test/unit/lib/nvmf/request.c/request_ut.c | 12 ++ test/unit/lib/nvmf/subsystem.c/subsystem_ut.c | 14 +- 14 files changed, 233 insertions(+), 420 deletions(-) diff --git a/include/spdk/nvmf.h b/include/spdk/nvmf.h index 952af17e8..893bd9668 100644 --- a/include/spdk/nvmf.h +++ b/include/spdk/nvmf.h @@ -61,7 +61,6 @@ struct spdk_nvmf_qpair; struct spdk_nvmf_request; struct spdk_bdev; struct spdk_nvmf_request; -struct spdk_nvmf_ctrlr_ops; typedef void (*spdk_nvmf_subsystem_connect_fn)(void *cb_ctx, struct spdk_nvmf_request *req); typedef void (*spdk_nvmf_subsystem_disconnect_fn)(void *cb_ctx, struct spdk_nvmf_qpair *qpair); @@ -100,8 +99,6 @@ struct spdk_nvmf_subsystem { uint32_t max_nsid; } dev; - const struct spdk_nvmf_ctrlr_ops *ops; - void *cb_ctx; spdk_nvmf_subsystem_connect_fn connect_cb; spdk_nvmf_subsystem_disconnect_fn disconnect_cb; diff --git a/lib/nvmf/ctrlr.c b/lib/nvmf/ctrlr.c index 4573e81c3..a520e6b74 100644 --- a/lib/nvmf/ctrlr.c +++ b/lib/nvmf/ctrlr.c @@ -356,7 +356,7 @@ spdk_nvmf_ctrlr_get_qpair(struct spdk_nvmf_ctrlr *ctrlr, uint16_t qid) return NULL; } -struct spdk_nvmf_request * +static struct spdk_nvmf_request * spdk_nvmf_qpair_get_request(struct spdk_nvmf_qpair *qpair, uint16_t cid) { /* TODO: track list of outstanding requests in qpair? */ @@ -599,7 +599,7 @@ spdk_nvmf_ctrlr_poll(struct spdk_nvmf_ctrlr *ctrlr) return 0; } -int +static int spdk_nvmf_ctrlr_set_features_host_identifier(struct spdk_nvmf_request *req) { struct spdk_nvme_cpl *response = &req->rsp->nvme_cpl; @@ -609,7 +609,7 @@ spdk_nvmf_ctrlr_set_features_host_identifier(struct spdk_nvmf_request *req) return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; } -int +static int spdk_nvmf_ctrlr_get_features_host_identifier(struct spdk_nvmf_request *req) { struct spdk_nvmf_ctrlr *ctrlr = req->qpair->ctrlr; @@ -634,7 +634,7 @@ spdk_nvmf_ctrlr_get_features_host_identifier(struct spdk_nvmf_request *req) return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; } -int +static int spdk_nvmf_ctrlr_set_features_keep_alive_timer(struct spdk_nvmf_request *req) { struct spdk_nvmf_ctrlr *ctrlr = req->qpair->ctrlr; @@ -656,7 +656,7 @@ spdk_nvmf_ctrlr_set_features_keep_alive_timer(struct spdk_nvmf_request *req) return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; } -int +static int spdk_nvmf_ctrlr_get_features_keep_alive_timer(struct spdk_nvmf_request *req) { struct spdk_nvmf_ctrlr *ctrlr = req->qpair->ctrlr; @@ -667,7 +667,7 @@ spdk_nvmf_ctrlr_get_features_keep_alive_timer(struct spdk_nvmf_request *req) return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; } -int +static int spdk_nvmf_ctrlr_set_features_number_of_queues(struct spdk_nvmf_request *req) { struct spdk_nvmf_ctrlr *ctrlr = req->qpair->ctrlr; @@ -693,7 +693,7 @@ spdk_nvmf_ctrlr_set_features_number_of_queues(struct spdk_nvmf_request *req) return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; } -int +static int spdk_nvmf_ctrlr_get_features_number_of_queues(struct spdk_nvmf_request *req) { struct spdk_nvmf_ctrlr *ctrlr = req->qpair->ctrlr; @@ -711,7 +711,17 @@ spdk_nvmf_ctrlr_get_features_number_of_queues(struct spdk_nvmf_request *req) return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; } -int +static int +spdk_nvmf_ctrlr_get_features_write_cache(struct spdk_nvmf_request *req) +{ + struct spdk_nvme_cpl *rsp = &req->rsp->nvme_cpl; + + SPDK_TRACELOG(SPDK_TRACE_NVMF, "Get Features - Write Cache\n"); + rsp->cdw0 = 1; + return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; +} + +static int spdk_nvmf_ctrlr_set_features_async_event_configuration(struct spdk_nvmf_request *req) { struct spdk_nvmf_ctrlr *ctrlr = req->qpair->ctrlr; @@ -723,7 +733,7 @@ spdk_nvmf_ctrlr_set_features_async_event_configuration(struct spdk_nvmf_request return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; } -int +static int spdk_nvmf_ctrlr_get_features_async_event_configuration(struct spdk_nvmf_request *req) { struct spdk_nvmf_ctrlr *ctrlr = req->qpair->ctrlr; @@ -734,7 +744,7 @@ spdk_nvmf_ctrlr_get_features_async_event_configuration(struct spdk_nvmf_request return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; } -int +static int spdk_nvmf_ctrlr_async_event_request(struct spdk_nvmf_request *req) { struct spdk_nvmf_ctrlr *ctrlr = req->qpair->ctrlr; @@ -754,7 +764,7 @@ spdk_nvmf_ctrlr_async_event_request(struct spdk_nvmf_request *req) return SPDK_NVMF_REQUEST_EXEC_STATUS_ASYNCHRONOUS; } -int +static int spdk_nvmf_ctrlr_get_log_page(struct spdk_nvmf_request *req) { struct spdk_nvmf_subsystem *subsystem = req->qpair->ctrlr->subsys; @@ -951,7 +961,7 @@ spdk_nvmf_ctrlr_identify_active_ns_list(struct spdk_nvmf_subsystem *subsystem, return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; } -int +static int spdk_nvmf_ctrlr_identify(struct spdk_nvmf_request *req) { uint8_t cns; @@ -996,3 +1006,167 @@ invalid_cns: rsp->status.sc = SPDK_NVME_SC_INVALID_FIELD; return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; } + +static int +spdk_nvmf_ctrlr_abort(struct spdk_nvmf_request *req) +{ + struct spdk_nvmf_ctrlr *ctrlr = req->qpair->ctrlr; + struct spdk_nvme_cpl *rsp = &req->rsp->nvme_cpl; + struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd; + uint32_t cdw10 = cmd->cdw10; + uint16_t cid = cdw10 >> 16; + uint16_t sqid = cdw10 & 0xFFFFu; + struct spdk_nvmf_qpair *qpair; + struct spdk_nvmf_request *req_to_abort; + + SPDK_TRACELOG(SPDK_TRACE_NVMF, "abort sqid=%u cid=%u\n", sqid, cid); + + rsp->cdw0 = 1; /* Command not aborted */ + + qpair = spdk_nvmf_ctrlr_get_qpair(ctrlr, sqid); + if (qpair == NULL) { + SPDK_TRACELOG(SPDK_TRACE_NVMF, "sqid %u not found\n", sqid); + rsp->status.sct = SPDK_NVME_SCT_GENERIC; + rsp->status.sc = SPDK_NVME_SC_INVALID_FIELD; + return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; + } + + /* + * NOTE: This relies on the assumption that all connections for a ctrlr will be handled + * on the same thread. If this assumption becomes untrue, this will need to pass a message + * to the thread handling qpair, and the abort will need to be asynchronous. + */ + req_to_abort = spdk_nvmf_qpair_get_request(qpair, cid); + if (req_to_abort == NULL) { + SPDK_TRACELOG(SPDK_TRACE_NVMF, "cid %u not found\n", cid); + rsp->status.sct = SPDK_NVME_SCT_GENERIC; + rsp->status.sc = SPDK_NVME_SC_INVALID_FIELD; + return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; + } + + if (spdk_nvmf_request_abort(req_to_abort) == 0) { + SPDK_TRACELOG(SPDK_TRACE_NVMF, "abort ctrlr=%p req=%p sqid=%u cid=%u successful\n", + ctrlr, req_to_abort, sqid, cid); + rsp->cdw0 = 0; /* Command successfully aborted */ + } + rsp->status.sct = SPDK_NVME_SCT_GENERIC; + rsp->status.sc = SPDK_NVME_SC_SUCCESS; + return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; +} + +static int +spdk_nvmf_ctrlr_get_features(struct spdk_nvmf_request *req) +{ + uint8_t feature; + struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd; + struct spdk_nvme_cpl *response = &req->rsp->nvme_cpl; + + feature = cmd->cdw10 & 0xff; /* mask out the FID value */ + switch (feature) { + case SPDK_NVME_FEAT_NUMBER_OF_QUEUES: + return spdk_nvmf_ctrlr_get_features_number_of_queues(req); + case SPDK_NVME_FEAT_VOLATILE_WRITE_CACHE: + return spdk_nvmf_ctrlr_get_features_write_cache(req); + case SPDK_NVME_FEAT_KEEP_ALIVE_TIMER: + return spdk_nvmf_ctrlr_get_features_keep_alive_timer(req); + case SPDK_NVME_FEAT_ASYNC_EVENT_CONFIGURATION: + return spdk_nvmf_ctrlr_get_features_async_event_configuration(req); + case SPDK_NVME_FEAT_HOST_IDENTIFIER: + return spdk_nvmf_ctrlr_get_features_host_identifier(req); + default: + SPDK_ERRLOG("Get Features command with unsupported feature ID 0x%02x\n", feature); + response->status.sc = SPDK_NVME_SC_INVALID_FIELD; + return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; + } +} + +static int +spdk_nvmf_ctrlr_set_features(struct spdk_nvmf_request *req) +{ + uint8_t feature; + struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd; + struct spdk_nvme_cpl *response = &req->rsp->nvme_cpl; + + feature = cmd->cdw10 & 0xff; /* mask out the FID value */ + switch (feature) { + case SPDK_NVME_FEAT_NUMBER_OF_QUEUES: + return spdk_nvmf_ctrlr_set_features_number_of_queues(req); + case SPDK_NVME_FEAT_KEEP_ALIVE_TIMER: + return spdk_nvmf_ctrlr_set_features_keep_alive_timer(req); + case SPDK_NVME_FEAT_ASYNC_EVENT_CONFIGURATION: + return spdk_nvmf_ctrlr_set_features_async_event_configuration(req); + case SPDK_NVME_FEAT_HOST_IDENTIFIER: + return spdk_nvmf_ctrlr_set_features_host_identifier(req); + default: + SPDK_ERRLOG("Set Features command with unsupported feature ID 0x%02x\n", feature); + response->status.sc = SPDK_NVME_SC_INVALID_FIELD; + return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; + } +} + +static int +spdk_nvmf_ctrlr_keep_alive(struct spdk_nvmf_request *req) +{ + SPDK_TRACELOG(SPDK_TRACE_NVMF, "Keep Alive\n"); + /* + * To handle keep alive just clear or reset the + * ctrlr based keep alive duration counter. + * When added, a separate timer based process + * will monitor if the time since last recorded + * keep alive has exceeded the max duration and + * take appropriate action. + */ + return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; +} + +int +spdk_nvmf_ctrlr_process_admin_cmd(struct spdk_nvmf_request *req) +{ + struct spdk_nvmf_subsystem *subsystem = req->qpair->ctrlr->subsys; + struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd; + struct spdk_nvme_cpl *response = &req->rsp->nvme_cpl; + + if (subsystem->subtype == SPDK_NVMF_SUBTYPE_DISCOVERY) { + /* Discovery controllers only support Get Log Page and Identify */ + switch (cmd->opc) { + case SPDK_NVME_OPC_IDENTIFY: + case SPDK_NVME_OPC_GET_LOG_PAGE: + break; + default: + goto invalid_opcode; + } + } + + switch (cmd->opc) { + case SPDK_NVME_OPC_GET_LOG_PAGE: + return spdk_nvmf_ctrlr_get_log_page(req); + case SPDK_NVME_OPC_IDENTIFY: + return spdk_nvmf_ctrlr_identify(req); + case SPDK_NVME_OPC_ABORT: + return spdk_nvmf_ctrlr_abort(req); + case SPDK_NVME_OPC_GET_FEATURES: + return spdk_nvmf_ctrlr_get_features(req); + case SPDK_NVME_OPC_SET_FEATURES: + return spdk_nvmf_ctrlr_set_features(req); + case SPDK_NVME_OPC_ASYNC_EVENT_REQUEST: + return spdk_nvmf_ctrlr_async_event_request(req); + case SPDK_NVME_OPC_KEEP_ALIVE: + return spdk_nvmf_ctrlr_keep_alive(req); + + case SPDK_NVME_OPC_CREATE_IO_SQ: + case SPDK_NVME_OPC_CREATE_IO_CQ: + case SPDK_NVME_OPC_DELETE_IO_SQ: + case SPDK_NVME_OPC_DELETE_IO_CQ: + /* Create and Delete I/O CQ/SQ not allowed in NVMe-oF */ + goto invalid_opcode; + + default: + goto invalid_opcode; + } + +invalid_opcode: + SPDK_ERRLOG("Unsupported admin opcode 0x%x\n", cmd->opc); + response->status.sct = SPDK_NVME_SCT_GENERIC; + response->status.sc = SPDK_NVME_SC_INVALID_OPCODE; + return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; +} diff --git a/lib/nvmf/ctrlr.h b/lib/nvmf/ctrlr.h index b8a6506fd..f1143233a 100644 --- a/lib/nvmf/ctrlr.h +++ b/lib/nvmf/ctrlr.h @@ -105,8 +105,6 @@ void spdk_nvmf_ctrlr_connect(struct spdk_nvmf_qpair *qpair, struct spdk_nvmf_qpair *spdk_nvmf_ctrlr_get_qpair(struct spdk_nvmf_ctrlr *ctrlr, uint16_t qid); -struct spdk_nvmf_request *spdk_nvmf_qpair_get_request(struct spdk_nvmf_qpair *qpair, uint16_t cid); - void spdk_nvmf_property_get(struct spdk_nvmf_ctrlr *ctrlr, struct spdk_nvmf_fabric_prop_get_cmd *cmd, @@ -121,23 +119,9 @@ int spdk_nvmf_ctrlr_poll(struct spdk_nvmf_ctrlr *ctrlr); void spdk_nvmf_ctrlr_destruct(struct spdk_nvmf_ctrlr *ctrlr); -int spdk_nvmf_ctrlr_set_features_host_identifier(struct spdk_nvmf_request *req); -int spdk_nvmf_ctrlr_get_features_host_identifier(struct spdk_nvmf_request *req); +int spdk_nvmf_ctrlr_process_admin_cmd(struct spdk_nvmf_request *req); -int spdk_nvmf_ctrlr_set_features_keep_alive_timer(struct spdk_nvmf_request *req); -int spdk_nvmf_ctrlr_get_features_keep_alive_timer(struct spdk_nvmf_request *req); - -int spdk_nvmf_ctrlr_set_features_number_of_queues(struct spdk_nvmf_request *req); -int spdk_nvmf_ctrlr_get_features_number_of_queues(struct spdk_nvmf_request *req); - -int spdk_nvmf_ctrlr_set_features_async_event_configuration(struct spdk_nvmf_request *req); -int spdk_nvmf_ctrlr_get_features_async_event_configuration(struct spdk_nvmf_request *req); - -int spdk_nvmf_ctrlr_async_event_request(struct spdk_nvmf_request *req); - -int spdk_nvmf_ctrlr_get_log_page(struct spdk_nvmf_request *req); - -int spdk_nvmf_ctrlr_identify(struct spdk_nvmf_request *req); +int spdk_nvmf_ctrlr_process_io_cmd(struct spdk_nvmf_request *req); bool spdk_nvmf_ctrlr_dsm_supported(struct spdk_nvmf_ctrlr *ctrlr); diff --git a/lib/nvmf/ctrlr_bdev.c b/lib/nvmf/ctrlr_bdev.c index 899a173d0..ef647eb28 100644 --- a/lib/nvmf/ctrlr_bdev.c +++ b/lib/nvmf/ctrlr_bdev.c @@ -83,12 +83,6 @@ spdk_nvmf_ctrlr_dsm_supported(struct spdk_nvmf_ctrlr *ctrlr) return true; } -static void -nvmf_bdev_ctrlr_poll_for_completions(struct spdk_nvmf_subsystem *subsystem) -{ - return; -} - static void nvmf_bdev_ctrlr_complete_cmd(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg) @@ -123,152 +117,6 @@ spdk_nvmf_bdev_ctrlr_identify_ns(struct spdk_bdev *bdev, struct spdk_nvme_ns_dat return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; } -static int -nvmf_bdev_ctrlr_abort(struct spdk_nvmf_request *req) -{ - struct spdk_nvmf_ctrlr *ctrlr = req->qpair->ctrlr; - struct spdk_nvme_cpl *rsp = &req->rsp->nvme_cpl; - struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd; - uint32_t cdw10 = cmd->cdw10; - uint16_t cid = cdw10 >> 16; - uint16_t sqid = cdw10 & 0xFFFFu; - struct spdk_nvmf_qpair *qpair; - struct spdk_nvmf_request *req_to_abort; - - SPDK_TRACELOG(SPDK_TRACE_NVMF, "abort sqid=%u cid=%u\n", sqid, cid); - - rsp->cdw0 = 1; /* Command not aborted */ - - qpair = spdk_nvmf_ctrlr_get_qpair(ctrlr, sqid); - if (qpair == NULL) { - SPDK_TRACELOG(SPDK_TRACE_NVMF, "sqid %u not found\n", sqid); - rsp->status.sct = SPDK_NVME_SCT_GENERIC; - rsp->status.sc = SPDK_NVME_SC_INVALID_FIELD; - return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; - } - - /* - * NOTE: This relies on the assumption that all connections for a ctrlr will be handled - * on the same thread. If this assumption becomes untrue, this will need to pass a message - * to the thread handling qpair, and the abort will need to be asynchronous. - */ - req_to_abort = spdk_nvmf_qpair_get_request(qpair, cid); - if (req_to_abort == NULL) { - SPDK_TRACELOG(SPDK_TRACE_NVMF, "cid %u not found\n", cid); - rsp->status.sct = SPDK_NVME_SCT_GENERIC; - rsp->status.sc = SPDK_NVME_SC_INVALID_FIELD; - return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; - } - - if (spdk_nvmf_request_abort(req_to_abort) == 0) { - SPDK_TRACELOG(SPDK_TRACE_NVMF, "abort ctrlr=%p req=%p sqid=%u cid=%u successful\n", - ctrlr, req_to_abort, sqid, cid); - rsp->cdw0 = 0; /* Command successfully aborted */ - } - rsp->status.sct = SPDK_NVME_SCT_GENERIC; - rsp->status.sc = SPDK_NVME_SC_SUCCESS; - return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; -} - -static int -nvmf_bdev_ctrlr_get_features(struct spdk_nvmf_request *req) -{ - uint8_t feature; - struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd; - struct spdk_nvme_cpl *response = &req->rsp->nvme_cpl; - - feature = cmd->cdw10 & 0xff; /* mask out the FID value */ - switch (feature) { - case SPDK_NVME_FEAT_NUMBER_OF_QUEUES: - return spdk_nvmf_ctrlr_get_features_number_of_queues(req); - case SPDK_NVME_FEAT_VOLATILE_WRITE_CACHE: - response->cdw0 = 1; - return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; - case SPDK_NVME_FEAT_KEEP_ALIVE_TIMER: - return spdk_nvmf_ctrlr_get_features_keep_alive_timer(req); - case SPDK_NVME_FEAT_ASYNC_EVENT_CONFIGURATION: - return spdk_nvmf_ctrlr_get_features_async_event_configuration(req); - case SPDK_NVME_FEAT_HOST_IDENTIFIER: - return spdk_nvmf_ctrlr_get_features_host_identifier(req); - default: - SPDK_ERRLOG("Get Features command with unsupported feature ID 0x%02x\n", feature); - response->status.sc = SPDK_NVME_SC_INVALID_FIELD; - return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; - } -} - -static int -nvmf_bdev_ctrlr_set_features(struct spdk_nvmf_request *req) -{ - uint8_t feature; - struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd; - struct spdk_nvme_cpl *response = &req->rsp->nvme_cpl; - - feature = cmd->cdw10 & 0xff; /* mask out the FID value */ - switch (feature) { - case SPDK_NVME_FEAT_NUMBER_OF_QUEUES: - return spdk_nvmf_ctrlr_set_features_number_of_queues(req); - case SPDK_NVME_FEAT_KEEP_ALIVE_TIMER: - return spdk_nvmf_ctrlr_set_features_keep_alive_timer(req); - case SPDK_NVME_FEAT_ASYNC_EVENT_CONFIGURATION: - return spdk_nvmf_ctrlr_set_features_async_event_configuration(req); - case SPDK_NVME_FEAT_HOST_IDENTIFIER: - return spdk_nvmf_ctrlr_set_features_host_identifier(req); - default: - SPDK_ERRLOG("Set Features command with unsupported feature ID 0x%02x\n", feature); - response->status.sc = SPDK_NVME_SC_INVALID_FIELD; - return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; - } -} - -static int -nvmf_bdev_ctrlr_process_admin_cmd(struct spdk_nvmf_request *req) -{ - struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd; - struct spdk_nvme_cpl *response = &req->rsp->nvme_cpl; - - /* pre-set response details for this command */ - response->status.sc = SPDK_NVME_SC_SUCCESS; - - switch (cmd->opc) { - case SPDK_NVME_OPC_GET_LOG_PAGE: - return spdk_nvmf_ctrlr_get_log_page(req); - case SPDK_NVME_OPC_IDENTIFY: - return spdk_nvmf_ctrlr_identify(req); - case SPDK_NVME_OPC_ABORT: - return nvmf_bdev_ctrlr_abort(req); - case SPDK_NVME_OPC_GET_FEATURES: - return nvmf_bdev_ctrlr_get_features(req); - case SPDK_NVME_OPC_SET_FEATURES: - return nvmf_bdev_ctrlr_set_features(req); - case SPDK_NVME_OPC_ASYNC_EVENT_REQUEST: - return spdk_nvmf_ctrlr_async_event_request(req); - case SPDK_NVME_OPC_KEEP_ALIVE: - SPDK_TRACELOG(SPDK_TRACE_NVMF, "Keep Alive\n"); - /* - * To handle keep alive just clear or reset the - * ctrlr based keep alive duration counter. - * When added, a separate timer based process - * will monitor if the time since last recorded - * keep alive has exceeded the max duration and - * take appropriate action. - */ - return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; - - case SPDK_NVME_OPC_CREATE_IO_SQ: - case SPDK_NVME_OPC_CREATE_IO_CQ: - case SPDK_NVME_OPC_DELETE_IO_SQ: - case SPDK_NVME_OPC_DELETE_IO_CQ: - SPDK_ERRLOG("Admin opc 0x%02X not allowed in NVMf\n", cmd->opc); - response->status.sc = SPDK_NVME_SC_INVALID_OPCODE; - return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; - default: - SPDK_ERRLOG("Unsupported admin command\n"); - return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; - } - -} - static int nvmf_bdev_ctrlr_rw_cmd(struct spdk_bdev *bdev, struct spdk_bdev_desc *desc, struct spdk_io_channel *ch, struct spdk_nvmf_request *req) @@ -444,8 +292,8 @@ nvmf_bdev_ctrlr_nvme_passthru_io(struct spdk_bdev *bdev, struct spdk_bdev_desc * return SPDK_NVMF_REQUEST_EXEC_STATUS_ASYNCHRONOUS; } -static int -nvmf_bdev_ctrlr_process_io_cmd(struct spdk_nvmf_request *req) +int +spdk_nvmf_ctrlr_process_io_cmd(struct spdk_nvmf_request *req) { uint32_t nsid; struct spdk_bdev *bdev; @@ -486,8 +334,8 @@ nvmf_bdev_ctrlr_process_io_cmd(struct spdk_nvmf_request *req) } } -static int -nvmf_bdev_ctrlr_attach(struct spdk_nvmf_subsystem *subsystem) +int +spdk_nvmf_subsystem_bdev_attach(struct spdk_nvmf_subsystem *subsystem) { struct spdk_bdev *bdev; struct spdk_io_channel *ch; @@ -510,8 +358,8 @@ nvmf_bdev_ctrlr_attach(struct spdk_nvmf_subsystem *subsystem) return 0; } -static void -nvmf_bdev_ctrlr_detach(struct spdk_nvmf_subsystem *subsystem) +void +spdk_nvmf_subsystem_bdev_detach(struct spdk_nvmf_subsystem *subsystem) { uint32_t i; @@ -530,11 +378,3 @@ nvmf_bdev_ctrlr_detach(struct spdk_nvmf_subsystem *subsystem) } subsystem->dev.max_nsid = 0; } - -const struct spdk_nvmf_ctrlr_ops spdk_nvmf_bdev_ctrlr_ops = { - .attach = nvmf_bdev_ctrlr_attach, - .process_admin_cmd = nvmf_bdev_ctrlr_process_admin_cmd, - .process_io_cmd = nvmf_bdev_ctrlr_process_io_cmd, - .poll_for_completions = nvmf_bdev_ctrlr_poll_for_completions, - .detach = nvmf_bdev_ctrlr_detach, -}; diff --git a/lib/nvmf/ctrlr_discovery.c b/lib/nvmf/ctrlr_discovery.c index d12e0d258..ff010a4a3 100644 --- a/lib/nvmf/ctrlr_discovery.c +++ b/lib/nvmf/ctrlr_discovery.c @@ -143,57 +143,3 @@ spdk_nvmf_get_discovery_log_page(void *buffer, uint64_t offset, uint32_t length) /* We should have copied or zeroed every byte of the output buffer. */ assert(copy_len + zero_len == length); } - -static int -nvmf_discovery_ctrlr_process_admin_cmd(struct spdk_nvmf_request *req) -{ - struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd; - struct spdk_nvme_cpl *response = &req->rsp->nvme_cpl; - - /* pre-set response details for this command */ - response->status.sc = SPDK_NVME_SC_SUCCESS; - - if (req->data == NULL) { - SPDK_ERRLOG("discovery command with no buffer\n"); - response->status.sc = SPDK_NVME_SC_INVALID_FIELD; - return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; - } - - switch (cmd->opc) { - case SPDK_NVME_OPC_IDENTIFY: - return spdk_nvmf_ctrlr_identify(req); - case SPDK_NVME_OPC_GET_LOG_PAGE: - return spdk_nvmf_ctrlr_get_log_page(req); - default: - SPDK_ERRLOG("Unsupported Opcode 0x%x for Discovery service\n", cmd->opc); - response->status.sc = SPDK_NVME_SC_INVALID_OPCODE; - return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; - } - - return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; -} - -static int -nvmf_discovery_ctrlr_process_io_cmd(struct spdk_nvmf_request *req) -{ - /* Discovery controllers do not support I/O queues, so this code should be unreachable. */ - abort(); -} - -static void -nvmf_discovery_ctrlr_detach(struct spdk_nvmf_subsystem *subsystem) -{ -} - -static int -nvmf_discovery_ctrlr_attach(struct spdk_nvmf_subsystem *subsystem) -{ - return 0; -} - -const struct spdk_nvmf_ctrlr_ops spdk_nvmf_discovery_ctrlr_ops = { - .attach = nvmf_discovery_ctrlr_attach, - .process_admin_cmd = nvmf_discovery_ctrlr_process_admin_cmd, - .process_io_cmd = nvmf_discovery_ctrlr_process_io_cmd, - .detach = nvmf_discovery_ctrlr_detach, -}; diff --git a/lib/nvmf/nvmf_internal.h b/lib/nvmf/nvmf_internal.h index 5c2a13918..85e40e8c3 100644 --- a/lib/nvmf/nvmf_internal.h +++ b/lib/nvmf/nvmf_internal.h @@ -44,33 +44,6 @@ #define SPDK_NVMF_DEFAULT_NUM_CTRLRS_PER_LCORE 1 -struct spdk_nvmf_ctrlr_ops { - /** - * Initialize the controller. - */ - int (*attach)(struct spdk_nvmf_subsystem *subsystem); - - /** - * Process admin command. - */ - int (*process_admin_cmd)(struct spdk_nvmf_request *req); - - /** - * Process IO command. - */ - int (*process_io_cmd)(struct spdk_nvmf_request *req); - - /** - * Poll for completions. - */ - void (*poll_for_completions)(struct spdk_nvmf_subsystem *subsystem); - - /** - * Detach the controller. - */ - void (*detach)(struct spdk_nvmf_subsystem *subsystem); -}; - struct spdk_nvmf_tgt { uint16_t max_queue_depth; uint16_t max_qpairs_per_ctrlr; diff --git a/lib/nvmf/request.c b/lib/nvmf/request.c index 9e21bfbe0..ad5658e69 100644 --- a/lib/nvmf/request.c +++ b/lib/nvmf/request.c @@ -293,9 +293,9 @@ spdk_nvmf_request_exec(struct spdk_nvmf_request *req) rsp->status.sc = SPDK_NVME_SC_ABORTED_BY_REQUEST; status = SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; } else if (req->qpair->type == QPAIR_TYPE_AQ) { - status = subsystem->ops->process_admin_cmd(req); + status = spdk_nvmf_ctrlr_process_admin_cmd(req); } else { - status = subsystem->ops->process_io_cmd(req); + status = spdk_nvmf_ctrlr_process_io_cmd(req); } } diff --git a/lib/nvmf/subsystem.c b/lib/nvmf/subsystem.c index d8ead5c8e..ae5830614 100644 --- a/lib/nvmf/subsystem.c +++ b/lib/nvmf/subsystem.c @@ -106,7 +106,7 @@ spdk_nvmf_subsystem_host_allowed(struct spdk_nvmf_subsystem *subsystem, const ch int spdk_nvmf_subsystem_start(struct spdk_nvmf_subsystem *subsystem) { - return subsystem->ops->attach(subsystem); + return spdk_nvmf_subsystem_bdev_attach(subsystem); } static bool @@ -133,20 +133,13 @@ spdk_nvmf_subsystem_poll(struct spdk_nvmf_subsystem *subsystem) { struct spdk_nvmf_ctrlr *ctrlr; - /* Check the backing physical device for completions. */ - if (subsystem->ops->poll_for_completions) { - subsystem->ops->poll_for_completions(subsystem); - } - TAILQ_FOREACH(ctrlr, &subsystem->ctrlrs, link) { /* For each connection in the ctrlr, check for completions */ spdk_nvmf_ctrlr_poll(ctrlr); } if (nvmf_subsystem_removable(subsystem)) { - if (subsystem->ops->detach) { - subsystem->ops->detach(subsystem); - } + spdk_nvmf_subsystem_bdev_detach(subsystem); } } @@ -206,12 +199,6 @@ spdk_nvmf_create_subsystem(const char *nqn, TAILQ_INIT(&subsystem->hosts); TAILQ_INIT(&subsystem->ctrlrs); - if (type == SPDK_NVMF_SUBTYPE_DISCOVERY) { - subsystem->ops = &spdk_nvmf_discovery_ctrlr_ops; - } else { - subsystem->ops = &spdk_nvmf_bdev_ctrlr_ops; - } - TAILQ_INSERT_TAIL(&g_nvmf_tgt.subsystems, subsystem, entries); g_nvmf_tgt.discovery_genctr++; @@ -248,9 +235,7 @@ spdk_nvmf_delete_subsystem(struct spdk_nvmf_subsystem *subsystem) spdk_nvmf_ctrlr_destruct(ctrlr); } - if (subsystem->ops->detach) { - subsystem->ops->detach(subsystem); - } + spdk_nvmf_subsystem_bdev_detach(subsystem); TAILQ_REMOVE(&g_nvmf_tgt.subsystems, subsystem, entries); g_nvmf_tgt.discovery_genctr++; diff --git a/lib/nvmf/subsystem.h b/lib/nvmf/subsystem.h index 5af220762..23a71443a 100644 --- a/lib/nvmf/subsystem.h +++ b/lib/nvmf/subsystem.h @@ -43,7 +43,7 @@ struct spdk_nvmf_subsystem *spdk_nvmf_find_subsystem_with_cntlid(uint16_t cntlid void spdk_nvmf_get_discovery_log_page(void *buffer, uint64_t offset, uint32_t length); -extern const struct spdk_nvmf_ctrlr_ops spdk_nvmf_bdev_ctrlr_ops; -extern const struct spdk_nvmf_ctrlr_ops spdk_nvmf_discovery_ctrlr_ops; +int spdk_nvmf_subsystem_bdev_attach(struct spdk_nvmf_subsystem *subsystem); +void spdk_nvmf_subsystem_bdev_detach(struct spdk_nvmf_subsystem *subsystem); #endif /* SPDK_NVMF_SUBSYSTEM_H */ diff --git a/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c b/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c index f9aa3a8ca..a22879fee 100644 --- a/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c +++ b/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c @@ -128,6 +128,12 @@ spdk_nvmf_request_complete(struct spdk_nvmf_request *req) return -1; } +int +spdk_nvmf_request_abort(struct spdk_nvmf_request *req) +{ + return -1; +} + static void test_get_log_page(void) { diff --git a/test/unit/lib/nvmf/ctrlr_bdev.c/ctrlr_bdev_ut.c b/test/unit/lib/nvmf/ctrlr_bdev.c/ctrlr_bdev_ut.c index 4aee977f4..897e43f12 100644 --- a/test/unit/lib/nvmf/ctrlr_bdev.c/ctrlr_bdev_ut.c +++ b/test/unit/lib/nvmf/ctrlr_bdev.c/ctrlr_bdev_ut.c @@ -46,88 +46,12 @@ spdk_nvmf_ctrlr_get_qpair(struct spdk_nvmf_ctrlr *ctrlr, uint16_t qid) return NULL; } -struct spdk_nvmf_request * -spdk_nvmf_qpair_get_request(struct spdk_nvmf_qpair *qpair, uint16_t cid) -{ - return NULL; -} - -int -spdk_nvmf_ctrlr_get_features_number_of_queues(struct spdk_nvmf_request *req) -{ - return -1; -} - -int spdk_nvmf_ctrlr_set_features_number_of_queues(struct spdk_nvmf_request *req) -{ - return -1; -} - -int -spdk_nvmf_ctrlr_set_features_host_identifier(struct spdk_nvmf_request *req) -{ - return -1; -} - -int -spdk_nvmf_ctrlr_get_features_host_identifier(struct spdk_nvmf_request *req) -{ - return -1; -} -int -spdk_nvmf_ctrlr_set_features_keep_alive_timer(struct spdk_nvmf_request *req) -{ - return -1; -} - -int -spdk_nvmf_ctrlr_get_features_keep_alive_timer(struct spdk_nvmf_request *req) -{ - return -1; -} - -int -spdk_nvmf_ctrlr_set_features_async_event_configuration(struct spdk_nvmf_request *req) -{ - return -1; -} - -int -spdk_nvmf_ctrlr_get_features_async_event_configuration(struct spdk_nvmf_request *req) -{ - return -1; -} - -int -spdk_nvmf_ctrlr_async_event_request(struct spdk_nvmf_request *req) -{ - return -1; -} - -int -spdk_nvmf_ctrlr_get_log_page(struct spdk_nvmf_request *req) -{ - return -1; -} - -int -spdk_nvmf_ctrlr_identify(struct spdk_nvmf_request *req) -{ - return -1; -} - int spdk_nvmf_request_complete(struct spdk_nvmf_request *req) { return -1; } -int -spdk_nvmf_request_abort(struct spdk_nvmf_request *req) -{ - return -1; -} - const char * spdk_bdev_get_name(const struct spdk_bdev *bdev) { diff --git a/test/unit/lib/nvmf/ctrlr_discovery.c/ctrlr_discovery_ut.c b/test/unit/lib/nvmf/ctrlr_discovery.c/ctrlr_discovery_ut.c index fea8bb8a7..bbcf77025 100644 --- a/test/unit/lib/nvmf/ctrlr_discovery.c/ctrlr_discovery_ut.c +++ b/test/unit/lib/nvmf/ctrlr_discovery.c/ctrlr_discovery_ut.c @@ -39,8 +39,6 @@ SPDK_LOG_REGISTER_TRACE_FLAG("nvmf", SPDK_TRACE_NVMF) -const struct spdk_nvmf_ctrlr_ops spdk_nvmf_bdev_ctrlr_ops; - struct spdk_nvmf_tgt g_nvmf_tgt = { .subsystems = TAILQ_HEAD_INITIALIZER(g_nvmf_tgt.subsystems) }; @@ -156,47 +154,14 @@ spdk_nvmf_ctrlr_poll(struct spdk_nvmf_ctrlr *ctrlr) } int -spdk_nvmf_ctrlr_get_log_page(struct spdk_nvmf_request *req) +spdk_nvmf_subsystem_bdev_attach(struct spdk_nvmf_subsystem *subsystem) { - abort(); return -1; } -int -spdk_nvmf_ctrlr_identify(struct spdk_nvmf_request *req) +void +spdk_nvmf_subsystem_bdev_detach(struct spdk_nvmf_subsystem *subsystem) { - abort(); - return -1; -} - -static void -test_process_discovery_cmd(void) -{ - struct spdk_nvmf_request req = {}; - int ret; - struct spdk_nvmf_qpair req_qpair = {}; - struct spdk_nvmf_ctrlr req_ctrlr = {}; - struct spdk_nvme_ctrlr_data req_data = {}; - union nvmf_h2c_msg req_cmd = {}; - union nvmf_c2h_msg req_rsp = {}; - - req.qpair = &req_qpair; - req.cmd = &req_cmd; - req.rsp = &req_rsp; - - /* no request data check */ - ret = nvmf_discovery_ctrlr_process_admin_cmd(&req); - CU_ASSERT_EQUAL(ret, SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE); - CU_ASSERT_EQUAL(req.rsp->nvme_cpl.status.sc, SPDK_NVME_SC_INVALID_FIELD); - - req.qpair->ctrlr = &req_ctrlr; - req.data = &req_data; - - /* Invalid opcode return value check */ - req.cmd->nvme_cmd.opc = 100; - ret = nvmf_discovery_ctrlr_process_admin_cmd(&req); - CU_ASSERT_EQUAL(req.rsp->nvme_cpl.status.sc, SPDK_NVME_SC_INVALID_OPCODE); - CU_ASSERT_EQUAL(ret, SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE); } static bool @@ -301,7 +266,6 @@ int main(int argc, char **argv) } if ( - CU_add_test(suite, "process_discovery_command", test_process_discovery_cmd) == NULL || CU_add_test(suite, "discovery_log", test_discovery_log) == NULL) { CU_cleanup_registry(); return CU_get_error(); diff --git a/test/unit/lib/nvmf/request.c/request_ut.c b/test/unit/lib/nvmf/request.c/request_ut.c index 785c5dadf..1db259927 100644 --- a/test/unit/lib/nvmf/request.c/request_ut.c +++ b/test/unit/lib/nvmf/request.c/request_ut.c @@ -58,6 +58,18 @@ spdk_nvmf_ctrlr_connect(struct spdk_nvmf_qpair *qpair, { } +int +spdk_nvmf_ctrlr_process_admin_cmd(struct spdk_nvmf_request *req) +{ + return -1; +} + +int +spdk_nvmf_ctrlr_process_io_cmd(struct spdk_nvmf_request *req) +{ + return -1; +} + int spdk_nvme_ctrlr_cmd_admin_raw(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_cmd *cmd, diff --git a/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c b/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c index 9a77172c6..982c7dbcf 100644 --- a/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c +++ b/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c @@ -36,15 +36,23 @@ #include "spdk_cunit.h" #include "subsystem.h" -const struct spdk_nvmf_ctrlr_ops spdk_nvmf_bdev_ctrlr_ops; -const struct spdk_nvmf_ctrlr_ops spdk_nvmf_discovery_ctrlr_ops; - #include "subsystem.c" SPDK_LOG_REGISTER_TRACE_FLAG("nvmf", SPDK_TRACE_NVMF) struct spdk_nvmf_tgt g_nvmf_tgt; +int +spdk_nvmf_subsystem_bdev_attach(struct spdk_nvmf_subsystem *subsystem) +{ + return -1; +} + +void +spdk_nvmf_subsystem_bdev_detach(struct spdk_nvmf_subsystem *subsystem) +{ +} + struct spdk_nvmf_listen_addr * spdk_nvmf_listen_addr_create(struct spdk_nvme_transport_id *trid) {