Skip to content

Commit

Permalink
nvme: Add transport controller ready step
Browse files Browse the repository at this point in the history
This step allows custom transports to perform extra actions or checks
at controller initialization and fail initialization if required.

Signed-off-by: Evgeniy Kochetov <evgeniik@nvidia.com>
Change-Id: Ic7cadae5398a35903917ceace3828f4371be63a3
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/12631
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <smatsumoto@nvidia.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@nvidia.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
  • Loading branch information
EugeneKochetov authored and tomzawadzki committed Aug 4, 2022
1 parent 44cbea4 commit 3dd0bc9
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 3 deletions.
2 changes: 2 additions & 0 deletions include/spdk/nvme.h
Original file line number Diff line number Diff line change
Expand Up @@ -3954,6 +3954,8 @@ struct spdk_nvme_transport_ops {
int (*ctrlr_get_memory_domains)(const struct spdk_nvme_ctrlr *ctrlr,
struct spdk_memory_domain **domains,
int array_size);

int (*ctrlr_ready)(struct spdk_nvme_ctrlr *ctrlr);
};

/**
Expand Down
18 changes: 15 additions & 3 deletions lib/nvme/nvme_ctrlr.c
Original file line number Diff line number Diff line change
Expand Up @@ -1395,6 +1395,8 @@ nvme_ctrlr_state_string(enum nvme_ctrlr_state state)
return "set host ID";
case NVME_CTRLR_STATE_WAIT_FOR_HOST_ID:
return "wait for set host ID";
case NVME_CTRLR_STATE_TRANSPORT_READY:
return "transport ready";
case NVME_CTRLR_STATE_READY:
return "ready";
case NVME_CTRLR_STATE_ERROR:
Expand Down Expand Up @@ -2906,7 +2908,7 @@ nvme_ctrlr_set_host_id_done(void *arg, const struct spdk_nvme_cpl *cpl)
NVME_CTRLR_DEBUGLOG(ctrlr, "Set Features - Host ID was successful\n");
}

nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_READY, NVME_TIMEOUT_INFINITE);
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_TRANSPORT_READY, ctrlr->opts.admin_timeout_ms);
}

static int
Expand All @@ -2922,7 +2924,7 @@ nvme_ctrlr_set_host_id(struct spdk_nvme_ctrlr *ctrlr)
* Set Features - Host Identifier after Connect, so we don't need to do anything here.
*/
NVME_CTRLR_DEBUGLOG(ctrlr, "NVMe-oF transport - not sending Set Features - Host ID\n");
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_READY, NVME_TIMEOUT_INFINITE);
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_TRANSPORT_READY, ctrlr->opts.admin_timeout_ms);
return 0;
}

Expand All @@ -2939,7 +2941,7 @@ nvme_ctrlr_set_host_id(struct spdk_nvme_ctrlr *ctrlr)
/* If the user specified an all-zeroes host identifier, don't send the command. */
if (spdk_mem_all_zero(host_id, host_id_size)) {
NVME_CTRLR_DEBUGLOG(ctrlr, "User did not specify host ID - not sending Set Features - Host ID\n");
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_READY, NVME_TIMEOUT_INFINITE);
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_TRANSPORT_READY, ctrlr->opts.admin_timeout_ms);
return 0;
}

Expand Down Expand Up @@ -3948,6 +3950,16 @@ nvme_ctrlr_process_init(struct spdk_nvme_ctrlr *ctrlr)
rc = nvme_ctrlr_set_host_id(ctrlr);
break;

case NVME_CTRLR_STATE_TRANSPORT_READY:
rc = nvme_transport_ctrlr_ready(ctrlr);
if (rc) {
NVME_CTRLR_ERRLOG(ctrlr, "Transport controller ready step failed: rc %d\n", rc);
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_ERROR, NVME_TIMEOUT_INFINITE);
} else {
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_READY, NVME_TIMEOUT_INFINITE);
}
break;

case NVME_CTRLR_STATE_READY:
NVME_CTRLR_DEBUGLOG(ctrlr, "Ctrlr already in ready state\n");
return 0;
Expand Down
6 changes: 6 additions & 0 deletions lib/nvme/nvme_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,11 @@ enum nvme_ctrlr_state {
*/
NVME_CTRLR_STATE_WAIT_FOR_HOST_ID,

/**
* Let transport layer do its part of initialization.
*/
NVME_CTRLR_STATE_TRANSPORT_READY,

/**
* Controller initialization has completed and the controller is ready.
*/
Expand Down Expand Up @@ -1470,6 +1475,7 @@ struct spdk_nvme_ctrlr *nvme_transport_ctrlr_construct(const struct spdk_nvme_tr
int nvme_transport_ctrlr_destruct(struct spdk_nvme_ctrlr *ctrlr);
int nvme_transport_ctrlr_scan(struct spdk_nvme_probe_ctx *probe_ctx, bool direct_connect);
int nvme_transport_ctrlr_enable(struct spdk_nvme_ctrlr *ctrlr);
int nvme_transport_ctrlr_ready(struct spdk_nvme_ctrlr *ctrlr);
int nvme_transport_ctrlr_set_reg_4(struct spdk_nvme_ctrlr *ctrlr, uint32_t offset, uint32_t value);
int nvme_transport_ctrlr_set_reg_8(struct spdk_nvme_ctrlr *ctrlr, uint32_t offset, uint64_t value);
int nvme_transport_ctrlr_get_reg_4(struct spdk_nvme_ctrlr *ctrlr, uint32_t offset, uint32_t *value);
Expand Down
13 changes: 13 additions & 0 deletions lib/nvme/nvme_transport.c
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,19 @@ nvme_transport_ctrlr_enable(struct spdk_nvme_ctrlr *ctrlr)
return transport->ops.ctrlr_enable(ctrlr);
}

int
nvme_transport_ctrlr_ready(struct spdk_nvme_ctrlr *ctrlr)
{
const struct spdk_nvme_transport *transport = nvme_get_transport(ctrlr->trid.trstring);

assert(transport != NULL);
if (transport->ops.ctrlr_ready) {
return transport->ops.ctrlr_ready(ctrlr);
}

return 0;
}

int
nvme_transport_ctrlr_set_reg_4(struct spdk_nvme_ctrlr *ctrlr, uint32_t offset, uint32_t value)
{
Expand Down
27 changes: 27 additions & 0 deletions test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ nvme_transport_ctrlr_get_memory_domains(const struct spdk_nvme_ctrlr *ctrlr,
return 0;
}

DEFINE_RETURN_MOCK(nvme_transport_ctrlr_ready, int);
int
nvme_transport_ctrlr_ready(struct spdk_nvme_ctrlr *ctrlr)
{
HANDLE_RETURN_MOCK(nvme_transport_ctrlr_ready);
return 0;
}

struct spdk_nvme_ctrlr *nvme_transport_ctrlr_construct(const struct spdk_nvme_transport_id *trid,
const struct spdk_nvme_ctrlr_opts *opts,
void *devhandle)
Expand Down Expand Up @@ -3276,6 +3284,24 @@ test_nvme_ctrlr_get_memory_domains(void)
MOCK_CLEAR(nvme_transport_ctrlr_get_memory_domains);
}

static void
test_nvme_transport_ctrlr_ready(void)
{
DECLARE_AND_CONSTRUCT_CTRLR();

/* Transport init succeeded */
ctrlr.state = NVME_CTRLR_STATE_TRANSPORT_READY;
SPDK_CU_ASSERT_FATAL(nvme_ctrlr_process_init(&ctrlr) == 0);
CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_READY);

/* Transport init failed */
ctrlr.state = NVME_CTRLR_STATE_TRANSPORT_READY;
MOCK_SET(nvme_transport_ctrlr_ready, -1);
SPDK_CU_ASSERT_FATAL(nvme_ctrlr_process_init(&ctrlr) == -1);
CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_ERROR);
MOCK_CLEAR(nvme_transport_ctrlr_ready);
}

int
main(int argc, char **argv)
{
Expand Down Expand Up @@ -3331,6 +3357,7 @@ main(int argc, char **argv)
CU_ADD_TEST(suite, test_nvme_ctrlr_parse_ana_log_page);
CU_ADD_TEST(suite, test_nvme_ctrlr_ana_resize);
CU_ADD_TEST(suite, test_nvme_ctrlr_get_memory_domains);
CU_ADD_TEST(suite, test_nvme_transport_ctrlr_ready);

CU_basic_set_mode(CU_BRM_VERBOSE);
CU_basic_run_tests();
Expand Down

0 comments on commit 3dd0bc9

Please sign in to comment.