From 79aba95edb31b6ff94d9f80bb0ff6537730aa98d Mon Sep 17 00:00:00 2001 From: Ben Walker Date: Thu, 29 Aug 2019 09:58:40 -0700 Subject: [PATCH] Revert "nvme: small code cleanup for nvme_transport_ctrlr_scan" This reverts commit 6129e78d262d21e3e3dd70ac74a3989b97748515. When the initiator sends the discovery log page, if the log page exceeds the size of its data buffer, it will break it up into multiple log page commands with appropriate offsets. However, supporting offsets in log pages is an optional feature in NVMe and reported by the EDLP bit in the identify data. This commit changed the discovery process to no longer send an identify command prior to doing the discovery log page command, so the values in the identify data are always 0. If the discovery log page exceeds the size of the data buffer (4k), it will then fail to send the second log page with an offset because it believes the controller does not support the feature. Revert this change to fix it. An identify should always be sent as part of the discovery process. A test case is included in a follow up patch the demonstrates the bug. Reported-by: Zahra Khatami Reported-by: Akshay Shah Signed-off-by: Ben Walker Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/466819 (master) (cherry picked from commit 647afdec4472a05adcf856081cea8d49eaded8f0) Change-Id: Iefd512a7521e0fea90541b3eb547671cfa816ea6 Signed-off-by: Tomasz Zawadzki Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/467946 Reviewed-by: Ben Walker Reviewed-by: Jim Harris Tested-by: SPDK CI Jenkins --- lib/nvme/nvme_rdma.c | 28 ++++++++++++++-------------- lib/nvme/nvme_tcp.c | 32 ++++++++++++++++++-------------- 2 files changed, 32 insertions(+), 28 deletions(-) diff --git a/lib/nvme/nvme_rdma.c b/lib/nvme/nvme_rdma.c index 2062f2a96..6bd7ac696 100644 --- a/lib/nvme/nvme_rdma.c +++ b/lib/nvme/nvme_rdma.c @@ -1428,22 +1428,22 @@ nvme_rdma_ctrlr_scan(struct spdk_nvme_probe_ctx *probe_ctx, return -1; } + /* get the cdata info */ + rc = nvme_ctrlr_cmd_identify(discovery_ctrlr, SPDK_NVME_IDENTIFY_CTRLR, 0, 0, + &discovery_ctrlr->cdata, sizeof(discovery_ctrlr->cdata), + nvme_completion_poll_cb, &status); + if (rc != 0) { + SPDK_ERRLOG("Failed to identify cdata\n"); + return rc; + } + + if (spdk_nvme_wait_for_completion(discovery_ctrlr->adminq, &status)) { + SPDK_ERRLOG("nvme_identify_controller failed!\n"); + return -ENXIO; + } + /* Direct attach through spdk_nvme_connect() API */ if (direct_connect == true) { - /* get the cdata info */ - rc = nvme_ctrlr_cmd_identify(discovery_ctrlr, SPDK_NVME_IDENTIFY_CTRLR, 0, 0, - &discovery_ctrlr->cdata, sizeof(discovery_ctrlr->cdata), - nvme_completion_poll_cb, &status); - if (rc != 0) { - SPDK_ERRLOG("Failed to identify cdata\n"); - return rc; - } - - if (spdk_nvme_wait_for_completion(discovery_ctrlr->adminq, &status)) { - SPDK_ERRLOG("nvme_identify_controller failed!\n"); - return -ENXIO; - } - /* Set the ready state to skip the normal init process */ discovery_ctrlr->state = NVME_CTRLR_STATE_READY; nvme_ctrlr_connected(probe_ctx, discovery_ctrlr); diff --git a/lib/nvme/nvme_tcp.c b/lib/nvme/nvme_tcp.c index a7d7c181e..9ea3e3901 100644 --- a/lib/nvme/nvme_tcp.c +++ b/lib/nvme/nvme_tcp.c @@ -310,22 +310,26 @@ nvme_tcp_ctrlr_scan(struct spdk_nvme_probe_ctx *probe_ctx, return -1; } + /* get the cdata info */ + status.done = false; + rc = nvme_ctrlr_cmd_identify(discovery_ctrlr, SPDK_NVME_IDENTIFY_CTRLR, 0, 0, + &discovery_ctrlr->cdata, sizeof(discovery_ctrlr->cdata), + nvme_completion_poll_cb, &status); + if (rc != 0) { + SPDK_ERRLOG("Failed to identify cdata\n"); + return rc; + } + + while (status.done == false) { + spdk_nvme_qpair_process_completions(discovery_ctrlr->adminq, 0); + } + if (spdk_nvme_cpl_is_error(&status.cpl)) { + SPDK_ERRLOG("nvme_identify_controller failed!\n"); + return -ENXIO; + } + /* Direct attach through spdk_nvme_connect() API */ if (direct_connect == true) { - /* get the cdata info */ - status.done = false; - rc = nvme_ctrlr_cmd_identify(discovery_ctrlr, SPDK_NVME_IDENTIFY_CTRLR, 0, 0, - &discovery_ctrlr->cdata, sizeof(discovery_ctrlr->cdata), - nvme_completion_poll_cb, &status); - if (rc != 0) { - SPDK_ERRLOG("Failed to identify cdata\n"); - return rc; - } - - if (spdk_nvme_wait_for_completion(discovery_ctrlr->adminq, &status)) { - SPDK_ERRLOG("nvme_identify_controller failed!\n"); - return -ENXIO; - } /* Set the ready state to skip the normal init process */ discovery_ctrlr->state = NVME_CTRLR_STATE_READY; nvme_ctrlr_connected(probe_ctx, discovery_ctrlr);