From 25bc221ceeaa11efcd95d2f65f4bb8a03ee4eaa2 Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Thu, 10 Feb 2022 10:37:04 +0000 Subject: [PATCH] nvmf: deprecate automatic discovery listener Currently we accept connections to the discovery subsystem on any listener that has been added to any subsystem (not just the discovery subsystem). This is not proper behavior, especially for TCP. TCP defines port 8009 (not 4420) as the discovery port, so the current behavior means that if NVM subsystems are listening on port 4420, then the discovery subsystem by default is listening there too. For now, continue to allow connections, but print a warning message when someone connects to the discovery subsystem on a listener trid that wasn't previously added. Signed-off-by: Jim Harris Change-Id: I734bc49d1a21b2edfb675aef4b8551e2d0ccd4d7 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/11539 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Aleksey Marchuk Reviewed-by: Shuhei Matsumoto --- CHANGELOG.md | 5 +++++ lib/nvmf/subsystem.c | 13 +++++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7452bc6b..31ce176e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,11 @@ remove NVMe error injections. Removed deprecated max_qpairs_per_ctrlr parameter from nvmf_create_transport RPC. Use max_io_qpairs_per_ctrlr instead. +Deprecated the ability for hosts to connect to the discovery subsystem automatically on any +existing listener. Users should now explicitly add listeners for the discovery subsystem. +Host can still connect to the discovery subsystem as before, but a warning message will be +emitted if no listener was configured for the transport ID of the incoming connection. + ### thread Added `spdk_thread_exec_msg()` API. diff --git a/lib/nvmf/subsystem.c b/lib/nvmf/subsystem.c index 29cbef097..4ceba5bcb 100644 --- a/lib/nvmf/subsystem.c +++ b/lib/nvmf/subsystem.c @@ -1205,16 +1205,21 @@ spdk_nvmf_subsystem_listener_allowed(struct spdk_nvmf_subsystem *subsystem, { struct spdk_nvmf_subsystem_listener *listener; - if (!strcmp(subsystem->subnqn, SPDK_NVMF_DISCOVERY_NQN)) { - return true; - } - TAILQ_FOREACH(listener, &subsystem->listeners, link) { if (spdk_nvme_transport_id_compare(listener->trid, trid) == 0) { return true; } } + if (!strcmp(subsystem->subnqn, SPDK_NVMF_DISCOVERY_NQN)) { + SPDK_WARNLOG("Allowing connection to discovery subsystem on %s/%s/%s, " + "even though this listener was not added to the discovery " + "subsystem. This behavior is deprecated and will be removed " + "in a future release.\n", + spdk_nvme_transport_id_trtype_str(trid->trtype), trid->traddr, trid->trsvcid); + return true; + } + return false; }