From 59f3cdacb13bd2a19c4a86be04792b0ee4491172 Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Fri, 3 Dec 2021 22:17:14 +0000 Subject: [PATCH] nvmf: don't always update discovery log when adding hosts If a subsystem has no listeners, then there is no need to update the discovery log when adding a host, or setting a subsystem to allow all hosts. This eliminates some unnecessary discovery log update notifications, especially when setting 'allow any hosts' on a subsystem immediately after it is created (and before it has any listeners). Update unit test to check the adding a host to a subsystem without listeners does not rev the genctr. Signed-off-by: Jim Harris Change-Id: I63dab5df564269e574bb925890088f52063aa378 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10546 Tested-by: SPDK CI Jenkins Reviewed-by: Changpeng Liu Reviewed-by: Dong Yi Reviewed-by: Jacek Kalwas Reviewed-by: Aleksey Marchuk --- lib/nvmf/subsystem.c | 8 ++++++-- .../lib/nvmf/ctrlr_discovery.c/ctrlr_discovery_ut.c | 12 +++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/lib/nvmf/subsystem.c b/lib/nvmf/subsystem.c index 668c7664e..bf01bffd0 100644 --- a/lib/nvmf/subsystem.c +++ b/lib/nvmf/subsystem.c @@ -837,7 +837,9 @@ spdk_nvmf_subsystem_add_host(struct spdk_nvmf_subsystem *subsystem, const char * TAILQ_INSERT_HEAD(&subsystem->hosts, host, link); - nvmf_update_discovery_log(subsystem->tgt, hostnqn); + if (!TAILQ_EMPTY(&subsystem->listeners)) { + nvmf_update_discovery_log(subsystem->tgt, hostnqn); + } pthread_mutex_unlock(&subsystem->mutex); @@ -946,7 +948,9 @@ spdk_nvmf_subsystem_set_allow_any_host(struct spdk_nvmf_subsystem *subsystem, bo { pthread_mutex_lock(&subsystem->mutex); subsystem->flags.allow_any_host = allow_any_host; - nvmf_update_discovery_log(subsystem->tgt, NULL); + if (!TAILQ_EMPTY(&subsystem->listeners)) { + nvmf_update_discovery_log(subsystem->tgt, NULL); + } pthread_mutex_unlock(&subsystem->mutex); return 0; 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 de3f4d9f7..f7433db74 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 @@ -313,9 +313,19 @@ test_discovery_log(void) /* Add one subsystem and verify that the discovery log contains it */ subsystem = spdk_nvmf_subsystem_create(&tgt, "nqn.2016-06.io.spdk:subsystem1", SPDK_NVMF_SUBTYPE_NVME, 0); - subsystem->flags.allow_any_host = true; SPDK_CU_ASSERT_FATAL(subsystem != NULL); + rc = spdk_nvmf_subsystem_add_host(subsystem, hostnqn); + CU_ASSERT(rc == 0); + + /* Get only genctr (first field in the header) */ + memset(buffer, 0xCC, sizeof(buffer)); + disc_log = (struct spdk_nvmf_discovery_log_page *)buffer; + nvmf_get_discovery_log_page(&tgt, hostnqn, &iov, 1, 0, sizeof(disc_log->genctr), + &trid); + /* No listeners yet on new subsystem, so genctr should still be 0. */ + CU_ASSERT(disc_log->genctr == 0); + test_gen_trid(&trid, SPDK_NVME_TRANSPORT_RDMA, SPDK_NVMF_ADRFAM_IPV4, "1234", "5678"); spdk_nvmf_subsystem_add_listener(subsystem, &trid, _subsystem_add_listen_done, NULL); subsystem->state = SPDK_NVMF_SUBSYSTEM_ACTIVE;