diff --git a/CHANGELOG.md b/CHANGELOG.md index c98ef7635..223f39cec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,9 @@ was used to format the namespace. Added two new APIs `spdk_nvme_ns_cmd_io_mgmt_recv` and `spdk_nvme_ns_cmd_io_mgmt_send` to receive and send the I/O management commands. +New `spdk_nvmf_transport_create_async` was added, it accepts a callback and callback argument. +`spdk_nvmf_transport_create` is marked deprecated. + ## v23.01 ### accel diff --git a/include/spdk/nvmf.h b/include/spdk/nvmf.h index f2bc551f9..026d889cc 100644 --- a/include/spdk/nvmf.h +++ b/include/spdk/nvmf.h @@ -968,7 +968,7 @@ spdk_nvmf_transport_opts_init(const char *transport_name, struct spdk_nvmf_transport_opts *opts, size_t opts_size); /** - * Create a protocol transport + * Create a protocol transport - deprecated, please use \ref spdk_nvmf_transport_create_async. * * \param transport_name The transport type to create * \param opts The transport options (e.g. max_io_size). It should not be NULL, and opts_size @@ -979,6 +979,27 @@ spdk_nvmf_transport_opts_init(const char *transport_name, struct spdk_nvmf_transport *spdk_nvmf_transport_create(const char *transport_name, struct spdk_nvmf_transport_opts *opts); +typedef void (*spdk_nvmf_transport_create_done_cb)(void *cb_arg, + struct spdk_nvmf_transport *transport); + +/** + * Create a protocol transport + * + * The callback will be executed asynchronously - i.e. spdk_nvmf_transport_create_async will always return + * prior to `cb_fn` being called. + * + * \param transport_name The transport type to create + * \param opts The transport options (e.g. max_io_size). It should not be NULL, and opts_size + * pointed in this structure should not be zero value. + * \param cb_fn A callback that will be called once the transport is created + * \param cb_arg A context argument passed to cb_fn. + * + * \return 0 on success, or negative errno on failure (`cb_fn` will not be executed then). + */ +int spdk_nvmf_transport_create_async(const char *transport_name, + struct spdk_nvmf_transport_opts *opts, + spdk_nvmf_transport_create_done_cb cb_fn, void *cb_arg); + typedef void (*spdk_nvmf_transport_destroy_done_cb)(void *cb_arg); /** diff --git a/include/spdk/nvmf_transport.h b/include/spdk/nvmf_transport.h index 9098357dd..edb054580 100644 --- a/include/spdk/nvmf_transport.h +++ b/include/spdk/nvmf_transport.h @@ -236,9 +236,12 @@ struct spdk_nvmf_transport_ops { void (*opts_init)(struct spdk_nvmf_transport_opts *opts); /** - * Create a transport for the given transport opts + * Create a transport for the given transport opts. Either synchronous + * or asynchronous version shall be implemented. */ struct spdk_nvmf_transport *(*create)(struct spdk_nvmf_transport_opts *opts); + int (*create_async)(struct spdk_nvmf_transport_opts *opts, spdk_nvmf_transport_create_done_cb cb_fn, + void *cb_arg); /** * Dump transport-specific opts into JSON diff --git a/lib/nvmf/nvmf_rpc.c b/lib/nvmf/nvmf_rpc.c index ce7b299bf..c69af44ca 100644 --- a/lib/nvmf/nvmf_rpc.c +++ b/lib/nvmf/nvmf_rpc.c @@ -1924,12 +1924,32 @@ nvmf_rpc_tgt_add_transport_done(void *cb_arg, int status) nvmf_rpc_create_transport_ctx_free(ctx); } +static void +nvmf_rpc_create_transport_done(void *cb_arg, struct spdk_nvmf_transport *transport) +{ + struct nvmf_rpc_create_transport_ctx *ctx = cb_arg; + + if (!transport) { + SPDK_ERRLOG("Failed to create transport.\n"); + spdk_jsonrpc_send_error_response(ctx->request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR, + "Failed to create transport."); + nvmf_rpc_create_transport_ctx_free(ctx); + return; + } + + ctx->transport = transport; + + spdk_nvmf_tgt_add_transport(spdk_nvmf_get_tgt(ctx->tgt_name), transport, + nvmf_rpc_tgt_add_transport_done, ctx); +} + static void rpc_nvmf_create_transport(struct spdk_jsonrpc_request *request, const struct spdk_json_val *params) { struct nvmf_rpc_create_transport_ctx *ctx; struct spdk_nvmf_tgt *tgt; + int rc; ctx = calloc(1, sizeof(*ctx)); if (!ctx) { @@ -1989,20 +2009,15 @@ rpc_nvmf_create_transport(struct spdk_jsonrpc_request *request, /* Transport can parse additional params themselves */ ctx->opts.transport_specific = params; + ctx->request = request; - ctx->transport = spdk_nvmf_transport_create(ctx->trtype, &ctx->opts); - - if (!ctx->transport) { + rc = spdk_nvmf_transport_create_async(ctx->trtype, &ctx->opts, nvmf_rpc_create_transport_done, ctx); + if (rc) { SPDK_ERRLOG("Transport type '%s' create failed\n", ctx->trtype); spdk_jsonrpc_send_error_response_fmt(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR, "Transport type '%s' create failed", ctx->trtype); nvmf_rpc_create_transport_ctx_free(ctx); - return; } - - /* add transport to target */ - ctx->request = request; - spdk_nvmf_tgt_add_transport(tgt, ctx->transport, nvmf_rpc_tgt_add_transport_done, ctx); } SPDK_RPC_REGISTER("nvmf_create_transport", rpc_nvmf_create_transport, SPDK_RPC_RUNTIME) diff --git a/lib/nvmf/spdk_nvmf.map b/lib/nvmf/spdk_nvmf.map index 411fbdd6d..29c8337e6 100644 --- a/lib/nvmf/spdk_nvmf.map +++ b/lib/nvmf/spdk_nvmf.map @@ -67,6 +67,7 @@ spdk_nvmf_subsystem_get_max_nsid; spdk_nvmf_transport_opts_init; spdk_nvmf_transport_create; + spdk_nvmf_transport_create_async; spdk_nvmf_transport_destroy; spdk_nvmf_tgt_get_transport; spdk_nvmf_transport_get_first; diff --git a/lib/nvmf/transport.c b/lib/nvmf/transport.c index fc23f364e..78041a7c3 100644 --- a/lib/nvmf/transport.c +++ b/lib/nvmf/transport.c @@ -166,72 +166,42 @@ nvmf_transport_opts_copy(struct spdk_nvmf_transport_opts *opts, #undef FILED_CHECK } -struct spdk_nvmf_transport * -spdk_nvmf_transport_create(const char *transport_name, struct spdk_nvmf_transport_opts *opts) +struct nvmf_transport_create_ctx { + const struct spdk_nvmf_transport_ops *ops; + struct spdk_nvmf_transport_opts opts; + void *cb_arg; + spdk_nvmf_transport_create_done_cb cb_fn; +}; + +static void +nvmf_transport_create_async_done(void *cb_arg, struct spdk_nvmf_transport *transport) { - const struct spdk_nvmf_transport_ops *ops = NULL; - struct spdk_nvmf_transport *transport; + struct nvmf_transport_create_ctx *ctx = cb_arg; char spdk_mempool_name[MAX_MEMPOOL_NAME_LENGTH]; int chars_written; - struct spdk_nvmf_transport_opts opts_local = {}; - if (!opts) { - SPDK_ERRLOG("opts should not be NULL\n"); - return NULL; - } - - if (!opts->opts_size) { - SPDK_ERRLOG("The opts_size in opts structure should not be zero\n"); - return NULL; - } - - ops = nvmf_get_transport_ops(transport_name); - if (!ops) { - SPDK_ERRLOG("Transport type '%s' unavailable.\n", transport_name); - return NULL; - } - nvmf_transport_opts_copy(&opts_local, opts, opts->opts_size); - - if (opts_local.max_io_size != 0 && (!spdk_u32_is_pow2(opts_local.max_io_size) || - opts_local.max_io_size < 8192)) { - SPDK_ERRLOG("max_io_size %u must be a power of 2 and be greater than or equal 8KB\n", - opts_local.max_io_size); - return NULL; - } - - if (opts_local.max_aq_depth < SPDK_NVMF_MIN_ADMIN_MAX_SQ_SIZE) { - SPDK_ERRLOG("max_aq_depth %u is less than minimum defined by NVMf spec, use min value\n", - opts_local.max_aq_depth); - opts_local.max_aq_depth = SPDK_NVMF_MIN_ADMIN_MAX_SQ_SIZE; - } - - transport = ops->create(&opts_local); if (!transport) { - SPDK_ERRLOG("Unable to create new transport of type %s\n", transport_name); - return NULL; + SPDK_ERRLOG("Failed to create transport.\n"); + goto err; } pthread_mutex_init(&transport->mutex, NULL); TAILQ_INIT(&transport->listeners); - - transport->ops = ops; - transport->opts = opts_local; - + transport->ops = ctx->ops; + transport->opts = ctx->opts; chars_written = snprintf(spdk_mempool_name, MAX_MEMPOOL_NAME_LENGTH, "%s_%s_%s", "spdk_nvmf", - transport_name, "data"); + transport->ops->name, "data"); if (chars_written < 0) { SPDK_ERRLOG("Unable to generate transport data buffer pool name.\n"); - ops->destroy(transport, NULL, NULL); - return NULL; + goto err; } - if (opts_local.num_shared_buffers) { + if (ctx->opts.num_shared_buffers) { transport->data_buf_pool = spdk_mempool_create(spdk_mempool_name, - opts_local.num_shared_buffers, - opts_local.io_unit_size + NVMF_DATA_BUFFER_ALIGNMENT, + ctx->opts.num_shared_buffers, + ctx->opts.io_unit_size + NVMF_DATA_BUFFER_ALIGNMENT, SPDK_MEMPOOL_DEFAULT_CACHE_SIZE, SPDK_ENV_SOCKET_ID_ANY); - if (!transport->data_buf_pool) { if (spdk_mempool_lookup(spdk_mempool_name) != NULL) { SPDK_ERRLOG("Unable to allocate poll group buffer pull: already exists\n"); @@ -240,11 +210,128 @@ spdk_nvmf_transport_create(const char *transport_name, struct spdk_nvmf_transpor } else { SPDK_ERRLOG("Unable to allocate buffer pool for poll group\n"); } - ops->destroy(transport, NULL, NULL); - return NULL; + goto err; } } + ctx->cb_fn(ctx->cb_arg, transport); + free(ctx); + return; + +err: + if (transport) { + transport->ops->destroy(transport, NULL, NULL); + } + + ctx->cb_fn(ctx->cb_arg, NULL); + free(ctx); +} + +static void +_nvmf_transport_create_done(void *ctx) +{ + struct nvmf_transport_create_ctx *_ctx = (struct nvmf_transport_create_ctx *)ctx; + + nvmf_transport_create_async_done(_ctx, _ctx->ops->create(&_ctx->opts)); +} + +static int +nvmf_transport_create(const char *transport_name, struct spdk_nvmf_transport_opts *opts, + spdk_nvmf_transport_create_done_cb cb_fn, void *cb_arg, bool sync) +{ + struct nvmf_transport_create_ctx *ctx; + int rc; + + ctx = calloc(1, sizeof(*ctx)); + if (!ctx) { + return -ENOMEM; + } + + if (!opts) { + SPDK_ERRLOG("opts should not be NULL\n"); + goto err; + } + + if (!opts->opts_size) { + SPDK_ERRLOG("The opts_size in opts structure should not be zero\n"); + goto err; + } + + ctx->ops = nvmf_get_transport_ops(transport_name); + if (!ctx->ops) { + SPDK_ERRLOG("Transport type '%s' unavailable.\n", transport_name); + goto err; + } + + nvmf_transport_opts_copy(&ctx->opts, opts, opts->opts_size); + if (ctx->opts.max_io_size != 0 && (!spdk_u32_is_pow2(ctx->opts.max_io_size) || + ctx->opts.max_io_size < 8192)) { + SPDK_ERRLOG("max_io_size %u must be a power of 2 and be greater than or equal 8KB\n", + ctx->opts.max_io_size); + goto err; + } + + if (ctx->opts.max_aq_depth < SPDK_NVMF_MIN_ADMIN_MAX_SQ_SIZE) { + SPDK_ERRLOG("max_aq_depth %u is less than minimum defined by NVMf spec, use min value\n", + ctx->opts.max_aq_depth); + ctx->opts.max_aq_depth = SPDK_NVMF_MIN_ADMIN_MAX_SQ_SIZE; + } + + ctx->cb_fn = cb_fn; + ctx->cb_arg = cb_arg; + + /* Prioritize sync create operation. */ + if (ctx->ops->create) { + if (sync) { + _nvmf_transport_create_done(ctx); + return 0; + } + + rc = spdk_thread_send_msg(spdk_get_thread(), _nvmf_transport_create_done, ctx); + if (rc) { + goto err; + } + + return 0; + } + + assert(ctx->ops->create_async); + rc = ctx->ops->create_async(&ctx->opts, nvmf_transport_create_async_done, ctx); + if (rc) { + SPDK_ERRLOG("Unable to create new transport of type %s\n", transport_name); + goto err; + } + + return 0; +err: + free(ctx); + return -1; +} + +int +spdk_nvmf_transport_create_async(const char *transport_name, struct spdk_nvmf_transport_opts *opts, + spdk_nvmf_transport_create_done_cb cb_fn, void *cb_arg) +{ + return nvmf_transport_create(transport_name, opts, cb_fn, cb_arg, false); +} + +static void +nvmf_transport_create_sync_done(void *cb_arg, struct spdk_nvmf_transport *transport) +{ + struct spdk_nvmf_transport **_transport = cb_arg; + + *_transport = transport; +} + +struct spdk_nvmf_transport * +spdk_nvmf_transport_create(const char *transport_name, struct spdk_nvmf_transport_opts *opts) +{ + struct spdk_nvmf_transport *transport = NULL; + + /* Current implementation supports synchronous version of create operation only. */ + assert(nvmf_get_transport_ops(transport_name) && nvmf_get_transport_ops(transport_name)->create); + + nvmf_transport_create(transport_name, opts, nvmf_transport_create_sync_done, &transport, true); return transport; } 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 0bf11fa9d..c3300b5aa 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 @@ -150,15 +150,17 @@ static struct spdk_nvmf_transport g_transport = { .ops = &g_transport_ops }; -struct spdk_nvmf_transport * -spdk_nvmf_transport_create(const char *transport_name, - struct spdk_nvmf_transport_opts *tprt_opts) +int +spdk_nvmf_transport_create_async(const char *transport_name, + struct spdk_nvmf_transport_opts *tprt_opts, + spdk_nvmf_transport_create_done_cb cb_fn, void *cb_arg) { if (strcasecmp(transport_name, spdk_nvme_transport_id_trtype_str(SPDK_NVME_TRANSPORT_RDMA))) { - return &g_transport; + cb_fn(cb_arg, &g_transport); + return 0; } - return NULL; + return -1; } struct spdk_nvmf_subsystem * diff --git a/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c b/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c index 5ad132cef..3e04fb58d 100644 --- a/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c +++ b/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c @@ -97,15 +97,17 @@ nvmf_transport_listener_discover(struct spdk_nvmf_transport *transport, static struct spdk_nvmf_transport g_transport = {}; -struct spdk_nvmf_transport * -spdk_nvmf_transport_create(const char *transport_name, - struct spdk_nvmf_transport_opts *tprt_opts) +int +spdk_nvmf_transport_create_async(const char *transport_name, + struct spdk_nvmf_transport_opts *tprt_opts, + spdk_nvmf_transport_create_done_cb cb_fn, void *cb_arg) { if (strcasecmp(transport_name, spdk_nvme_transport_id_trtype_str(SPDK_NVME_TRANSPORT_RDMA))) { - return &g_transport; + cb_fn(cb_arg, &g_transport); + return 0; } - return NULL; + return -1; } struct spdk_nvmf_subsystem * diff --git a/test/unit/lib/nvmf/transport.c/transport_ut.c b/test/unit/lib/nvmf/transport.c/transport_ut.c index fd7e8ade4..4ff050a7f 100644 --- a/test/unit/lib/nvmf/transport.c/transport_ut.c +++ b/test/unit/lib/nvmf/transport.c/transport_ut.c @@ -37,8 +37,6 @@ DEFINE_STUB_V(spdk_nvme_trid_populate_transport, (struct spdk_nvme_transport_id enum spdk_nvme_transport_type trtype)); DEFINE_STUB(nvmf_ctrlr_abort_request, int, (struct spdk_nvmf_request *req), 0); DEFINE_STUB(spdk_nvmf_request_complete, int, (struct spdk_nvmf_request *req), 0); -DEFINE_STUB(ut_transport_create, struct spdk_nvmf_transport *, - (struct spdk_nvmf_transport_opts *opts), NULL); DEFINE_STUB(ut_transport_destroy, int, (struct spdk_nvmf_transport *transport, spdk_nvmf_transport_destroy_done_cb cb_fn, void *cb_arg), 0); DEFINE_STUB(ibv_get_device_name, const char *, (struct ibv_device *device), NULL); @@ -108,29 +106,49 @@ ibv_reg_mr(struct ibv_pd *pd, void *addr, size_t length, int access) } } +struct spdk_nvmf_transport ut_transport = {}; + +static int +ut_transport_create(struct spdk_nvmf_transport_opts *opts, spdk_nvmf_transport_create_done_cb cb_fn, + void *cb_arg) +{ + cb_fn(cb_arg, &ut_transport); + return 0; +} + +static void +test_nvmf_create_transport_done(void *cb_arg, struct spdk_nvmf_transport *transport) +{ + struct spdk_nvmf_transport **ctx = cb_arg; + + *ctx = transport; +} + static void test_spdk_nvmf_transport_create(void) { int rc; - struct spdk_nvmf_transport ut_transport = {}; struct spdk_nvmf_transport *transport = NULL; struct nvmf_transport_ops_list_element *ops_element; struct spdk_nvmf_transport_ops ops = { .name = "new_ops", .type = (enum spdk_nvme_transport_type)SPDK_NVMF_TRTYPE_RDMA, - .create = ut_transport_create, + .create_async = ut_transport_create, .destroy = ut_transport_destroy }; /* No available ops element */ - transport = spdk_nvmf_transport_create("new_ops", &g_rdma_ut_transport_opts); + rc = spdk_nvmf_transport_create_async("new_ops", &g_rdma_ut_transport_opts, + test_nvmf_create_transport_done, &transport); + CU_ASSERT(rc != 0); CU_ASSERT(transport == NULL); /* Create transport successfully */ - MOCK_SET(ut_transport_create, &ut_transport); spdk_nvmf_transport_register(&ops); - transport = spdk_nvmf_transport_create("new_ops", &g_rdma_ut_transport_opts); + rc = spdk_nvmf_transport_create_async("new_ops", &g_rdma_ut_transport_opts, + test_nvmf_create_transport_done, &transport); + CU_ASSERT(rc == 0); CU_ASSERT(transport == &ut_transport); CU_ASSERT(!memcmp(&transport->opts, &g_rdma_ut_transport_opts, sizeof(g_rdma_ut_transport_opts))); CU_ASSERT(!memcmp(transport->ops, &ops, sizeof(ops))); @@ -140,9 +158,12 @@ test_spdk_nvmf_transport_create(void) CU_ASSERT(rc == 0); /* transport_opts parameter invalid */ + transport = NULL; g_rdma_ut_transport_opts.max_io_size = 4096; - transport = spdk_nvmf_transport_create("new_ops", &g_rdma_ut_transport_opts); + rc = spdk_nvmf_transport_create_async("new_ops", &g_rdma_ut_transport_opts, + test_nvmf_create_transport_done, &transport); + CU_ASSERT(rc != 0); CU_ASSERT(transport == NULL); g_rdma_ut_transport_opts.max_io_size = (SPDK_NVMF_RDMA_MIN_IO_BUFFER_SIZE * RDMA_UT_UNITS_IN_MAX_IO); @@ -150,7 +171,6 @@ test_spdk_nvmf_transport_create(void) ops_element = TAILQ_LAST(&g_spdk_nvmf_transport_ops, nvmf_transport_ops_list); TAILQ_REMOVE(&g_spdk_nvmf_transport_ops, ops_element, link); free(ops_element); - MOCK_CLEAR(ut_transport_create); } static struct spdk_nvmf_transport_poll_group * @@ -211,22 +231,22 @@ test_spdk_nvmf_transport_opts_init(void) int rc; bool rcbool; size_t opts_size; - struct spdk_nvmf_transport rtransport = {}; struct spdk_nvmf_transport *transport = NULL; struct spdk_nvmf_transport_opts opts = {}; const struct spdk_nvmf_transport_ops *tops; struct spdk_nvmf_transport_ops ops = { .name = "ut_ops", .type = (enum spdk_nvme_transport_type)SPDK_NVMF_TRTYPE_RDMA, - .create = ut_transport_create, + .create_async = ut_transport_create, .destroy = ut_transport_destroy, .opts_init = ut_opts_init }; - MOCK_SET(ut_transport_create, &rtransport); spdk_nvmf_transport_register(&ops); - transport = spdk_nvmf_transport_create("ut_ops", &g_rdma_ut_transport_opts); - CU_ASSERT(transport == &rtransport); + rc = spdk_nvmf_transport_create_async("ut_ops", &g_rdma_ut_transport_opts, + test_nvmf_create_transport_done, &transport); + CU_ASSERT(rc == 0); + CU_ASSERT(transport == &ut_transport); tops = nvmf_get_transport_ops(ops.name); CU_ASSERT(memcmp(tops, &ops, sizeof(struct spdk_nvmf_transport_ops)) == 0); @@ -266,7 +286,6 @@ static void test_spdk_nvmf_transport_listen_ext(void) { int rc; - struct spdk_nvmf_transport rtransport = {}; struct spdk_nvmf_transport *transport = NULL; struct spdk_nvme_transport_id trid1 = {}; struct spdk_nvme_transport_id trid2 = {}; @@ -275,7 +294,7 @@ test_spdk_nvmf_transport_listen_ext(void) struct spdk_nvmf_transport_ops ops = { .name = "ut_ops1", .type = (enum spdk_nvme_transport_type)SPDK_NVMF_TRTYPE_RDMA, - .create = ut_transport_create, + .create_async = ut_transport_create, .destroy = ut_transport_destroy, .opts_init = ut_opts_init, .listen = ut_transport_listen, @@ -288,9 +307,11 @@ test_spdk_nvmf_transport_listen_ext(void) memcpy(trid1.traddr, "192.168.100.72", sizeof("192.168.100.72")); memcpy(trid1.trsvcid, "4420", sizeof("4420")); - MOCK_SET(ut_transport_create, &rtransport); spdk_nvmf_transport_register(&ops); - transport = spdk_nvmf_transport_create("ut_ops1", &g_rdma_ut_transport_opts); + rc = spdk_nvmf_transport_create_async("ut_ops1", &g_rdma_ut_transport_opts, + test_nvmf_create_transport_done, &transport); + CU_ASSERT(rc == 0); + CU_ASSERT(transport == &ut_transport); /* Test1: Execute listen failed */ MOCK_SET(ut_transport_listen, -1);