From 2167c68d1838111692d35339b2fece4a8f200f91 Mon Sep 17 00:00:00 2001 From: Jan Kryl Date: Thu, 2 Jan 2020 19:00:45 +0000 Subject: [PATCH] lib/nvmf: nvmf target stops to listen when subsystem is destroyed There is a spdk_nvmf_tgt_listen() which opens a port for specified transport (trid) which opens possibility to accept new connections from initiators. However there is no counterpart of this function (i.e. spdk_nvmf_tgt_stop_listen()), which would stop listening. Instead the current code relies on spdk_nvmf_subsystem_destroy() to stop the listener, which seems to be wrong. Fixes #1129 Change-Id: I6e73d8c234dc451f0fee8394132eae34cd4f4756 Signed-off-by: Jan Kryl Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/479873 Reviewed-by: Shuhei Matsumoto Reviewed-by: Alexey Marchuk Tested-by: SPDK CI Jenkins --- CHANGELOG.md | 3 +++ include/spdk/nvmf.h | 21 ++++++++++++++++++++- lib/nvmf/nvmf.c | 30 ++++++++++++++++++++++++++++++ lib/nvmf/nvmf_internal.h | 2 ++ lib/nvmf/nvmf_rpc.c | 4 ++++ lib/nvmf/subsystem.c | 29 ++++++++++++++++++++--------- 6 files changed, 79 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4cb30d06..0409b3e37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,9 @@ modified to take a string. A function table, `spdk_nvmf_transport_ops`, and macro, `SPDK_NVMF_TRANSPORT_REGISTER`, have been added which enable registering out of tree transports. +Add `spdk_nvmf_tgt_stop_listen()` that can be used to stop listening for +incoming connections for specified target and trid. Listener is not stopped +implicitly upon destruction of a subsystem any more. ### util diff --git a/include/spdk/nvmf.h b/include/spdk/nvmf.h index 1f20e994b..a905d0f22 100644 --- a/include/spdk/nvmf.h +++ b/include/spdk/nvmf.h @@ -359,6 +359,19 @@ void spdk_nvmf_tgt_listen(struct spdk_nvmf_tgt *tgt, spdk_nvmf_tgt_listen_done_fn cb_fn, void *cb_arg); +/** + * Stop accepting new connections at the provided address. + * + * This is a counterpart to spdk_nvmf_tgt_listen(). + * + * \param tgt The target associated with the listen address. + * \param trid The address to stop listening at. + * + * \return int. 0 on success or a negated errno on failure. + */ +int spdk_nvmf_tgt_stop_listen(struct spdk_nvmf_tgt *tgt, + struct spdk_nvme_transport_id *trid); + /** * Poll the target for incoming connections. * @@ -687,6 +700,8 @@ const char *spdk_nvmf_host_get_nqn(struct spdk_nvmf_host *host); /** * Accept new connections on the address provided. * + * This does not start the listener. Use spdk_nvmf_tgt_listen() for that. + * * May only be performed on subsystems in the PAUSED or INACTIVE states. * * \param subsystem Subsystem to add listener to. @@ -698,7 +713,11 @@ int spdk_nvmf_subsystem_add_listener(struct spdk_nvmf_subsystem *subsystem, struct spdk_nvme_transport_id *trid); /** - * Stop accepting new connections on the address provided + * Remove the listener from subsystem. + * + * New connections to the address won't be propagated to the subsystem. + * However to stop listening at target level one must use the + * spdk_nvmf_tgt_stop_listen(). * * May only be performed on subsystems in the PAUSED or INACTIVE states. * diff --git a/lib/nvmf/nvmf.c b/lib/nvmf/nvmf.c index 278016d55..ff214ed70 100644 --- a/lib/nvmf/nvmf.c +++ b/lib/nvmf/nvmf.c @@ -280,6 +280,7 @@ spdk_nvmf_tgt_destroy_cb(void *io_device) if (tgt->subsystems) { for (i = 0; i < tgt->max_subsystems; i++) { if (tgt->subsystems[i]) { + spdk_nvmf_subsystem_remove_all_listeners(tgt->subsystems[i], true); spdk_nvmf_subsystem_destroy(tgt->subsystems[i]); } } @@ -571,6 +572,35 @@ spdk_nvmf_tgt_listen(struct spdk_nvmf_tgt *tgt, } } +int +spdk_nvmf_tgt_stop_listen(struct spdk_nvmf_tgt *tgt, + struct spdk_nvme_transport_id *trid) +{ + struct spdk_nvmf_transport *transport; + const char *trtype; + int rc; + + transport = spdk_nvmf_tgt_get_transport(tgt, trid->trstring); + if (!transport) { + trtype = spdk_nvme_transport_id_trtype_str(trid->trtype); + if (trtype != NULL) { + SPDK_ERRLOG("Unable to stop listen on transport %s. The transport must be created first.\n", + trtype); + } else { + SPDK_ERRLOG("The specified trtype %d is unknown. Please make sure that it is properly registered.\n", + trid->trtype); + } + return -EINVAL; + } + + rc = spdk_nvmf_transport_stop_listen(transport, trid); + if (rc < 0) { + SPDK_ERRLOG("Failed to stop listening on address '%s'\n", trid->traddr); + return rc; + } + return 0; +} + struct spdk_nvmf_tgt_add_transport_ctx { struct spdk_nvmf_tgt *tgt; struct spdk_nvmf_transport *transport; diff --git a/lib/nvmf/nvmf_internal.h b/lib/nvmf/nvmf_internal.h index 06089dfa7..f83b116ca 100644 --- a/lib/nvmf/nvmf_internal.h +++ b/lib/nvmf/nvmf_internal.h @@ -434,6 +434,8 @@ int spdk_nvmf_subsystem_add_ctrlr(struct spdk_nvmf_subsystem *subsystem, struct spdk_nvmf_ctrlr *ctrlr); void spdk_nvmf_subsystem_remove_ctrlr(struct spdk_nvmf_subsystem *subsystem, struct spdk_nvmf_ctrlr *ctrlr); +void spdk_nvmf_subsystem_remove_all_listeners(struct spdk_nvmf_subsystem *subsystem, + bool stop); struct spdk_nvmf_ctrlr *spdk_nvmf_subsystem_get_ctrlr(struct spdk_nvmf_subsystem *subsystem, uint16_t cntlid); int spdk_nvmf_ctrlr_async_event_ns_notice(struct spdk_nvmf_ctrlr *ctrlr); diff --git a/lib/nvmf/nvmf_rpc.c b/lib/nvmf/nvmf_rpc.c index 8e66d6ecc..061de38d5 100644 --- a/lib/nvmf/nvmf_rpc.c +++ b/lib/nvmf/nvmf_rpc.c @@ -43,6 +43,8 @@ #include "spdk_internal/event.h" #include "spdk_internal/log.h" +#include "nvmf_internal.h" + static int json_write_hex_str(struct spdk_json_write_ctx *w, const void *data, size_t size) { @@ -458,6 +460,7 @@ spdk_rpc_nvmf_subsystem_stopped(struct spdk_nvmf_subsystem *subsystem, struct spdk_jsonrpc_request *request = cb_arg; struct spdk_json_write_ctx *w; + spdk_nvmf_subsystem_remove_all_listeners(subsystem, true); spdk_nvmf_subsystem_destroy(subsystem); w = spdk_jsonrpc_begin_result(request); @@ -661,6 +664,7 @@ nvmf_rpc_listen_paused(struct spdk_nvmf_subsystem *subsystem, "Invalid parameters"); ctx->response_sent = true; } + spdk_nvmf_tgt_stop_listen(ctx->tgt, &ctx->trid); } else { spdk_jsonrpc_send_error_response(ctx->request, SPDK_JSONRPC_ERROR_INVALID_PARAMS, "Invalid parameters"); diff --git a/lib/nvmf/subsystem.c b/lib/nvmf/subsystem.c index 245a91bb6..eace99f97 100644 --- a/lib/nvmf/subsystem.c +++ b/lib/nvmf/subsystem.c @@ -314,13 +314,16 @@ _spdk_nvmf_subsystem_remove_host(struct spdk_nvmf_subsystem *subsystem, struct s static void _nvmf_subsystem_remove_listener(struct spdk_nvmf_subsystem *subsystem, - struct spdk_nvmf_listener *listener) + struct spdk_nvmf_listener *listener, + bool stop) { struct spdk_nvmf_transport *transport; - transport = spdk_nvmf_tgt_get_transport(subsystem->tgt, listener->trid.trstring); - if (transport != NULL) { - spdk_nvmf_transport_stop_listen(transport, &listener->trid); + if (stop) { + transport = spdk_nvmf_tgt_get_transport(subsystem->tgt, listener->trid.trstring); + if (transport != NULL) { + spdk_nvmf_transport_stop_listen(transport, &listener->trid); + } } TAILQ_REMOVE(&subsystem->listeners, listener, link); @@ -330,7 +333,6 @@ _nvmf_subsystem_remove_listener(struct spdk_nvmf_subsystem *subsystem, void spdk_nvmf_subsystem_destroy(struct spdk_nvmf_subsystem *subsystem) { - struct spdk_nvmf_listener *listener, *listener_tmp; struct spdk_nvmf_host *host, *host_tmp; struct spdk_nvmf_ctrlr *ctrlr, *ctrlr_tmp; struct spdk_nvmf_ns *ns; @@ -343,9 +345,7 @@ spdk_nvmf_subsystem_destroy(struct spdk_nvmf_subsystem *subsystem) SPDK_DEBUGLOG(SPDK_LOG_NVMF, "subsystem is %p\n", subsystem); - TAILQ_FOREACH_SAFE(listener, &subsystem->listeners, link, listener_tmp) { - _nvmf_subsystem_remove_listener(subsystem, listener); - } + spdk_nvmf_subsystem_remove_all_listeners(subsystem, false); TAILQ_FOREACH_SAFE(host, &subsystem->hosts, link, host_tmp) { _spdk_nvmf_subsystem_remove_host(subsystem, host); @@ -800,11 +800,22 @@ spdk_nvmf_subsystem_remove_listener(struct spdk_nvmf_subsystem *subsystem, return -ENOENT; } - _nvmf_subsystem_remove_listener(subsystem, listener); + _nvmf_subsystem_remove_listener(subsystem, listener, false); return 0; } +void +spdk_nvmf_subsystem_remove_all_listeners(struct spdk_nvmf_subsystem *subsystem, + bool stop) +{ + struct spdk_nvmf_listener *listener, *listener_tmp; + + TAILQ_FOREACH_SAFE(listener, &subsystem->listeners, link, listener_tmp) { + _nvmf_subsystem_remove_listener(subsystem, listener, stop); + } +} + bool spdk_nvmf_subsystem_listener_allowed(struct spdk_nvmf_subsystem *subsystem, struct spdk_nvme_transport_id *trid)