From e865a52415d0493bea85d7a918bdb66f120e5004 Mon Sep 17 00:00:00 2001 From: Lance Hartmann Date: Wed, 5 Dec 2018 16:57:53 -0500 Subject: [PATCH] nvme: Eliminate identify errors to Discovery ctrlr The nvme/identify cmd issued some cmds to a ctrlr irrespective of its type, and when the target was a Discovery ctrlr which only accepts a very limited cmd set, that would result in errors observable both on the initiator side (from nvme/identify) and in the output on the target (nvmf_tgt). Introduce new API, spdk_nvme_ctrlr_is_discovery(), and alter identify to make use of that in determining which commands to send to the target. Change-Id: I974a569843f1d2b9e1ece7bd3bf9ceee1bfae872 Signed-off-by: Lance Hartmann Reviewed-on: https://review.gerrithub.io/436225 Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Shuhei Matsumoto Reviewed-by: Ben Walker Chandler-Test-Pool: SPDK Automated Test System --- CHANGELOG.md | 3 ++ examples/nvme/identify/identify.c | 49 +++++++++++++++++++++---------- include/spdk/nvme.h | 9 ++++++ lib/nvme/nvme_ctrlr.c | 9 ++++++ 4 files changed, 54 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f684342a..462d038d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,9 @@ Add a new TCP/IP transport(located in lib/nvme/nvme_tcp.c) in nvme driver. With this new transport, it can be used to connect the NVMe-oF target with the same TCP/IP support. +Added API, spdk_nvme_ctrlr_is_discovery(), to indicate whether the ctrlr +arg refers to a Discovery Controller or not. + ### NVMe-oF Target The `spdk_nvmf_tgt_opts` struct has been deprecated in favor of `spdk_nvmf_transport_opts`. diff --git a/examples/nvme/identify/identify.c b/examples/nvme/identify/identify.c index 9cfe47d4f..45e2e00e2 100644 --- a/examples/nvme/identify/identify.c +++ b/examples/nvme/identify/identify.c @@ -405,25 +405,34 @@ get_log_pages(struct spdk_nvme_ctrlr *ctrlr) { const struct spdk_nvme_ctrlr_data *cdata; outstanding_commands = 0; + bool is_discovery = spdk_nvme_ctrlr_is_discovery(ctrlr); cdata = spdk_nvme_ctrlr_get_data(ctrlr); - if (get_error_log_page(ctrlr) == 0) { - outstanding_commands++; - } else { - printf("Get Error Log Page failed\n"); - } + if (!is_discovery) { + /* + * Only attempt to retrieve the following log pages + * when the NVM subsystem that's being targeted is + * NOT the Discovery Controller which only fields + * a Discovery Log Page. + */ + if (get_error_log_page(ctrlr) == 0) { + outstanding_commands++; + } else { + printf("Get Error Log Page failed\n"); + } - if (get_health_log_page(ctrlr) == 0) { - outstanding_commands++; - } else { - printf("Get Log Page (SMART/health) failed\n"); - } + if (get_health_log_page(ctrlr) == 0) { + outstanding_commands++; + } else { + printf("Get Log Page (SMART/health) failed\n"); + } - if (get_firmware_log_page(ctrlr) == 0) { - outstanding_commands++; - } else { - printf("Get Log Page (Firmware Slot Information) failed\n"); + if (get_firmware_log_page(ctrlr) == 0) { + outstanding_commands++; + } else { + printf("Get Log Page (Firmware Slot Information) failed\n"); + } } if (cdata->lpa.celp) { @@ -459,7 +468,7 @@ get_log_pages(struct spdk_nvme_ctrlr *ctrlr) } - if (get_discovery_log_page(ctrlr) == 0) { + if (is_discovery && (get_discovery_log_page(ctrlr) == 0)) { outstanding_commands++; } @@ -867,7 +876,15 @@ print_controller(struct spdk_nvme_ctrlr *ctrlr, const struct spdk_nvme_transport cap = spdk_nvme_ctrlr_get_regs_cap(ctrlr); vs = spdk_nvme_ctrlr_get_regs_vs(ctrlr); - get_features(ctrlr); + if (!spdk_nvme_ctrlr_is_discovery(ctrlr)) { + /* + * Discovery Controller only supports the + * IDENTIFY and GET_LOG_PAGE cmd set, so only + * attempt GET_FEATURES when NOT targeting a + * Discovery Controller. + */ + get_features(ctrlr); + } get_log_pages(ctrlr); cdata = spdk_nvme_ctrlr_get_data(ctrlr); diff --git a/include/spdk/nvme.h b/include/spdk/nvme.h index e034a6aee..c212f5d1a 100644 --- a/include/spdk/nvme.h +++ b/include/spdk/nvme.h @@ -183,6 +183,15 @@ struct spdk_nvme_ctrlr_opts { bool data_digest; }; +/** + * Indicate whether a ctrlr handle is associated with a Discovery controller. + * + * \param ctrlr Opaque handle to NVMe controller. + * + * \return true if a discovery controller, else false. + */ +bool spdk_nvme_ctrlr_is_discovery(struct spdk_nvme_ctrlr *ctrlr); + /** * Get the default options for the creation of a specific NVMe controller. * diff --git a/lib/nvme/nvme_ctrlr.c b/lib/nvme/nvme_ctrlr.c index a291d7d92..263217655 100644 --- a/lib/nvme/nvme_ctrlr.c +++ b/lib/nvme/nvme_ctrlr.c @@ -2733,3 +2733,12 @@ spdk_nvme_ctrlr_free_cmb_io_buffer(struct spdk_nvme_ctrlr *ctrlr, void *buf, siz nvme_robust_mutex_unlock(&ctrlr->ctrlr_lock); } } + +bool +spdk_nvme_ctrlr_is_discovery(struct spdk_nvme_ctrlr *ctrlr) +{ + assert(ctrlr); + + return !strncmp(ctrlr->trid.subnqn, SPDK_NVMF_DISCOVERY_NQN, + strlen(SPDK_NVMF_DISCOVERY_NQN)); +}