From 68f1681771de06761bfb7262a53b936edde7d8e2 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Thu, 3 Sep 2020 05:42:08 +0900 Subject: [PATCH] lib/nvmf: Control I/O and some admin commands according to ANA state For I/O commands, block them if ANA state is inaccessible, persistent loss, or change. For Identify command, clear capacity field (nuse) to 0 if ANA state is inaccessible or persistent loss. For Get Features command, block features, error recovery, write atomicity normal, reservation notification mask, and reservation persistence if ANA state is inaccessible, persistent loss, or change. For Get Log Page command, error information page does not return any data yet, and hence there is no change. For Set Features command, if ANA state is inaccessible or change, block the command if NSID is 0xFFFFFFFF or if feature is error recovery, write atomicity normal, reservation notification mask, or reservation persistence, or if ANA state is persistent loss, block the command. Signed-off-by: Shuhei Matsumoto Change-Id: I15dd593227e451aa2247c53da42b6acad1757907 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/4043 Community-CI: Mellanox Build Bot Community-CI: Broadcom CI Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Jim Harris Reviewed-by: Aleksey Marchuk --- lib/nvmf/ctrlr.c | 94 ++++++++++++++++++++++++++- test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c | 10 ++- 2 files changed, 102 insertions(+), 2 deletions(-) diff --git a/lib/nvmf/ctrlr.c b/lib/nvmf/ctrlr.c index 65112b8e4..023eb0e70 100644 --- a/lib/nvmf/ctrlr.c +++ b/lib/nvmf/ctrlr.c @@ -1996,6 +1996,11 @@ spdk_nvmf_ctrlr_identify_ns(struct spdk_nvmf_ctrlr *ctrlr, if (subsystem->ana_reporting) { /* ANA group ID matches NSID. */ nsdata->anagrpid = ns->nsid; + + if (ctrlr->listener->ana_state == SPDK_NVME_ANA_INACCESSIBLE_STATE || + ctrlr->listener->ana_state == SPDK_NVME_ANA_PERSISTENT_LOSS_STATE) { + nsdata->nuse = 0; + } } return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; @@ -2403,6 +2408,24 @@ get_features_generic(struct spdk_nvmf_request *req, uint32_t cdw0) return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; } +/* we have to use the typedef in the function declaration to appease astyle. */ +typedef enum spdk_nvme_path_status_code spdk_nvme_path_status_code_t; + +static spdk_nvme_path_status_code_t +_nvme_ana_state_to_path_status(enum spdk_nvme_ana_state ana_state) +{ + switch (ana_state) { + case SPDK_NVME_ANA_INACCESSIBLE_STATE: + return SPDK_NVME_SC_ASYMMETRIC_ACCESS_INACCESSIBLE; + case SPDK_NVME_ANA_PERSISTENT_LOSS_STATE: + return SPDK_NVME_SC_ASYMMETRIC_ACCESS_PERSISTENT_LOSS; + case SPDK_NVME_ANA_CHANGE_STATE: + return SPDK_NVME_SC_ASYMMETRIC_ACCESS_TRANSITION; + default: + return SPDK_NVME_SC_INTERNAL_PATH_ERROR; + } +} + static int nvmf_ctrlr_get_features(struct spdk_nvmf_request *req) { @@ -2410,8 +2433,31 @@ nvmf_ctrlr_get_features(struct spdk_nvmf_request *req) struct spdk_nvmf_ctrlr *ctrlr = req->qpair->ctrlr; struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd; struct spdk_nvme_cpl *response = &req->rsp->nvme_cpl; + enum spdk_nvme_ana_state ana_state; feature = cmd->cdw10_bits.get_features.fid; + + ana_state = ctrlr->listener->ana_state; + switch (ana_state) { + case SPDK_NVME_ANA_INACCESSIBLE_STATE: + case SPDK_NVME_ANA_PERSISTENT_LOSS_STATE: + case SPDK_NVME_ANA_CHANGE_STATE: + switch (feature) { + case SPDK_NVME_FEAT_ERROR_RECOVERY: + case SPDK_NVME_FEAT_WRITE_ATOMICITY: + case SPDK_NVME_FEAT_HOST_RESERVE_MASK: + case SPDK_NVME_FEAT_HOST_RESERVE_PERSIST: + response->status.sct = SPDK_NVME_SCT_PATH; + response->status.sc = _nvme_ana_state_to_path_status(ana_state); + return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; + default: + break; + } + break; + default: + break; + } + switch (feature) { case SPDK_NVME_FEAT_ARBITRATION: return get_features_generic(req, ctrlr->feat.arbitration.raw); @@ -2448,9 +2494,10 @@ static int nvmf_ctrlr_set_features(struct spdk_nvmf_request *req) { uint8_t feature, save; + struct spdk_nvmf_ctrlr *ctrlr = req->qpair->ctrlr; struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd; struct spdk_nvme_cpl *response = &req->rsp->nvme_cpl; - + enum spdk_nvme_ana_state ana_state; /* * Features are not saveable by the controller as indicated by * ONCS field of the Identify Controller data. @@ -2463,6 +2510,37 @@ nvmf_ctrlr_set_features(struct spdk_nvmf_request *req) } feature = cmd->cdw10_bits.set_features.fid; + + ana_state = ctrlr->listener->ana_state; + switch (ana_state) { + case SPDK_NVME_ANA_INACCESSIBLE_STATE: + case SPDK_NVME_ANA_CHANGE_STATE: + if (cmd->nsid == 0xffffffffu) { + response->status.sct = SPDK_NVME_SCT_PATH; + response->status.sc = _nvme_ana_state_to_path_status(ana_state); + return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; + } else { + switch (feature) { + case SPDK_NVME_FEAT_ERROR_RECOVERY: + case SPDK_NVME_FEAT_WRITE_ATOMICITY: + case SPDK_NVME_FEAT_HOST_RESERVE_MASK: + case SPDK_NVME_FEAT_HOST_RESERVE_PERSIST: + response->status.sct = SPDK_NVME_SCT_PATH; + response->status.sc = _nvme_ana_state_to_path_status(ana_state); + return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; + default: + break; + } + } + break; + case SPDK_NVME_ANA_PERSISTENT_LOSS_STATE: + response->status.sct = SPDK_NVME_SCT_PATH; + response->status.sc = SPDK_NVME_SC_ASYMMETRIC_ACCESS_PERSISTENT_LOSS; + return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; + default: + break; + } + switch (feature) { case SPDK_NVME_FEAT_ARBITRATION: return nvmf_ctrlr_set_features_arbitration(req); @@ -3047,6 +3125,7 @@ nvmf_ctrlr_process_io_cmd(struct spdk_nvmf_request *req) struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd; struct spdk_nvme_cpl *response = &req->rsp->nvme_cpl; struct spdk_nvmf_subsystem_pg_ns_info *ns_info; + enum spdk_nvme_ana_state ana_state; /* pre-set response details for this command */ response->status.sc = SPDK_NVME_SC_SUCCESS; @@ -3066,6 +3145,19 @@ nvmf_ctrlr_process_io_cmd(struct spdk_nvmf_request *req) return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; } + /* It will be lower overhead to check if ANA state is optimized or + * non-optimized. + */ + ana_state = ctrlr->listener->ana_state; + if (spdk_unlikely(ana_state != SPDK_NVME_ANA_OPTIMIZED_STATE && + ana_state != SPDK_NVME_ANA_NON_OPTIMIZED_STATE)) { + SPDK_DEBUGLOG(SPDK_LOG_NVMF, "Fail I/O command due to ANA state %d\n", + ana_state); + response->status.sct = SPDK_NVME_SCT_PATH; + response->status.sc = _nvme_ana_state_to_path_status(ana_state); + return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; + } + ns = _nvmf_subsystem_get_ns(ctrlr->subsys, nsid); if (ns == NULL || ns->bdev == NULL) { SPDK_ERRLOG("Unsuccessful query for nsid %u\n", cmd->nsid); diff --git a/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c b/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c index 75cb6a803..4878174fa 100644 --- a/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c +++ b/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c @@ -950,7 +950,10 @@ test_set_get_features(void) { struct spdk_nvmf_subsystem subsystem = {}; struct spdk_nvmf_qpair admin_qpair = {}; - struct spdk_nvmf_ctrlr ctrlr = { .subsys = &subsystem, .admin_qpair = &admin_qpair }; + struct spdk_nvmf_subsystem_listener listener = {}; + struct spdk_nvmf_ctrlr ctrlr = { + .subsys = &subsystem, .admin_qpair = &admin_qpair, .listener = &listener + }; union nvmf_h2c_msg cmd = {}; union nvmf_c2h_msg rsp = {}; struct spdk_nvmf_ns ns[3]; @@ -960,6 +963,7 @@ test_set_get_features(void) subsystem.ns = ns_arr; subsystem.max_nsid = SPDK_COUNTOF(ns_arr); + listener.ana_state = SPDK_NVME_ANA_OPTIMIZED_STATE; admin_qpair.ctrlr = &ctrlr; req.qpair = &admin_qpair; cmd.nvme_cmd.nsid = 1; @@ -1521,6 +1525,7 @@ test_fused_compare_and_write(void) struct spdk_nvmf_subsystem subsystem = {}; struct spdk_nvmf_ns ns = {}; struct spdk_nvmf_ns *subsys_ns[1] = {}; + struct spdk_nvmf_subsystem_listener listener = {}; struct spdk_bdev bdev = {}; struct spdk_nvmf_poll_group group = {}; @@ -1534,9 +1539,12 @@ test_fused_compare_and_write(void) subsys_ns[0] = &ns; subsystem.ns = (struct spdk_nvmf_ns **)&subsys_ns; + listener.ana_state = SPDK_NVME_ANA_OPTIMIZED_STATE; + /* Enable controller */ ctrlr.vcprop.cc.bits.en = 1; ctrlr.subsys = (struct spdk_nvmf_subsystem *)&subsystem; + ctrlr.listener = &listener; group.num_sgroups = 1; sgroups.state = SPDK_NVMF_SUBSYSTEM_ACTIVE;