nvmf: Solve subsystem add/delete issue

When we do frequent same subsystem add/delete,
we will face the adding issue. For example,
1  Add subsystem A
2  Delete subsystem A
3  Add subsystem A  (Fail in this step).
The reason is that we did not correctly free
the listener resources of subsystems, and this patch
can solve this issue.

Change-Id: I6765a306a3f10c9a0f38c95dbba12e2a4073e705
Signed-off-by: Ziye Yang <ziye.yang@intel.com>

app/nvmf_tgt/conf.c

@@ -517,8 +517,15 @@ spdk_nvmf_parse_subsystem(struct spdk_conf_section *sp)
 					     rte_lcore_to_socket_id(app_subsys->lcore));
 			}
 		}
-		spdk_nvmf_subsystem_add_listener(subsystem, transport_name, traddr, trsvcid);
 
+		ret = spdk_nvmf_subsystem_add_listener(subsystem, transport_name, traddr, trsvcid);
+
+		if (ret < 0) {
+			SPDK_ERRLOG("Failed to listen on traddr=%s, trsvcid=%s\n", traddr, trsvcid);
+			free(traddr);
+			free(trsvcid);
+			return -1;
+		}
 		free(traddr);
 		free(trsvcid);
 	}

lib/nvmf/nvmf.c

@@ -124,6 +124,12 @@ spdk_nvmf_listen_addr_create(char *trname, char *traddr, char *trsvcid)
 void
 spdk_nvmf_listen_addr_destroy(struct spdk_nvmf_listen_addr *addr)
 {
+	const struct spdk_nvmf_transport *transport;
+
+	transport = spdk_nvmf_transport_get(addr->trname);
+	assert(transport != NULL);
+	transport->listen_addr_remove(addr);
+
 	free(addr->trname);
 	free(addr->trsvcid);
 	free(addr->traddr);

lib/nvmf/rdma.c

@@ -151,6 +151,7 @@ struct spdk_nvmf_rdma_listen_addr {
 	struct rdma_cm_id			*id;
 	struct ibv_device_attr 			attr;
 	struct ibv_comp_channel			*comp_channel;
+	uint32_t				ref;
 	TAILQ_ENTRY(spdk_nvmf_rdma_listen_addr)	link;
 };
 
@@ -996,18 +997,21 @@ spdk_nvmf_rdma_init(uint16_t max_queue_depth, uint32_t max_io_size,
 	return 0;
 }
 
+static void
+spdk_nvmf_rdma_listen_addr_free(struct spdk_nvmf_rdma_listen_addr *addr)
+{
+	if (!addr) {
+		return;
+	}
+
+	free(addr->traddr);
+	free(addr->trsvcid);
+	free(addr);
+}
 static int
 spdk_nvmf_rdma_fini(void)
 {
-	struct spdk_nvmf_rdma_listen_addr *addr, *tmp;
-
 	pthread_mutex_lock(&g_rdma.lock);
-	TAILQ_FOREACH_SAFE(addr, &g_rdma.listen_addrs, link, tmp) {
-		TAILQ_REMOVE(&g_rdma.listen_addrs, addr, link);
-		ibv_destroy_comp_channel(addr->comp_channel);
-		rdma_destroy_id(addr->id);
-	}
-
 	if (g_rdma.event_channel != NULL) {
 		rdma_destroy_event_channel(g_rdma.event_channel);
 	}
@@ -1016,6 +1020,31 @@ spdk_nvmf_rdma_fini(void)
 	return 0;
 }
 
+static int
+spdk_nvmf_rdma_listen_remove(struct spdk_nvmf_listen_addr *listen_addr)
+{
+	struct spdk_nvmf_rdma_listen_addr *addr, *tmp;
+
+	pthread_mutex_lock(&g_rdma.lock);
+	TAILQ_FOREACH_SAFE(addr, &g_rdma.listen_addrs, link, tmp) {
+		if ((!strcasecmp(addr->traddr, listen_addr->traddr)) &&
+		    (!strcasecmp(addr->trsvcid, listen_addr->trsvcid))) {
+			assert(addr->ref > 0);
+			addr->ref--;
+			if (!addr->ref) {
+				TAILQ_REMOVE(&g_rdma.listen_addrs, addr, link);
+				ibv_destroy_comp_channel(addr->comp_channel);
+				rdma_destroy_id(addr->id);
+				spdk_nvmf_rdma_listen_addr_free(addr);
+			}
+			break;
+		}
+	}
+
+	pthread_mutex_unlock(&g_rdma.lock);
+	return 0;
+}
+
 static int
 spdk_nvmf_rdma_poll(struct spdk_nvmf_conn *conn);
 
@@ -1096,6 +1125,7 @@ spdk_nvmf_rdma_listen(struct spdk_nvmf_listen_addr *listen_addr)
 	TAILQ_FOREACH(addr, &g_rdma.listen_addrs, link) {
 		if ((!strcasecmp(addr->traddr, listen_addr->traddr)) &&
 		    (!strcasecmp(addr->trsvcid, listen_addr->trsvcid))) {
+			addr->ref++;
 			/* Already listening at this address */
 			pthread_mutex_unlock(&g_rdma.lock);
 			return 0;
@@ -1108,13 +1138,24 @@ spdk_nvmf_rdma_listen(struct spdk_nvmf_listen_addr *listen_addr)
 		return -1;
 	}
 
-	addr->traddr = listen_addr->traddr;
+	addr->traddr = strdup(listen_addr->traddr);
-	addr->trsvcid = listen_addr->trsvcid;
+	if (!addr->traddr) {
+		spdk_nvmf_rdma_listen_addr_free(addr);
+		pthread_mutex_unlock(&g_rdma.lock);
+		return -1;
+	}
+
+	addr->trsvcid = strdup(listen_addr->trsvcid);
+	if (!addr->trsvcid) {
+		spdk_nvmf_rdma_listen_addr_free(addr);
+		pthread_mutex_unlock(&g_rdma.lock);
+		return -1;
+	}
 
 	rc = rdma_create_id(g_rdma.event_channel, &addr->id, addr, RDMA_PS_TCP);
 	if (rc < 0) {
 		SPDK_ERRLOG("rdma_create_id() failed\n");
-		free(addr);
+		spdk_nvmf_rdma_listen_addr_free(addr);
 		pthread_mutex_unlock(&g_rdma.lock);
 		return -1;
 	}
@@ -1127,7 +1168,7 @@ spdk_nvmf_rdma_listen(struct spdk_nvmf_listen_addr *listen_addr)
 	if (rc < 0) {
 		SPDK_ERRLOG("rdma_bind_addr() failed\n");
 		rdma_destroy_id(addr->id);
-		free(addr);
+		spdk_nvmf_rdma_listen_addr_free(addr);
 		pthread_mutex_unlock(&g_rdma.lock);
 		return -1;
 	}
@@ -1136,7 +1177,7 @@ spdk_nvmf_rdma_listen(struct spdk_nvmf_listen_addr *listen_addr)
 	if (rc < 0) {
 		SPDK_ERRLOG("rdma_listen() failed\n");
 		rdma_destroy_id(addr->id);
-		free(addr);
+		spdk_nvmf_rdma_listen_addr_free(addr);
 		pthread_mutex_unlock(&g_rdma.lock);
 		return -1;
 	}
@@ -1145,7 +1186,7 @@ spdk_nvmf_rdma_listen(struct spdk_nvmf_listen_addr *listen_addr)
 	if (rc < 0) {
 		SPDK_ERRLOG("Failed to query RDMA device attributes.\n");
 		rdma_destroy_id(addr->id);
-		free(addr);
+		spdk_nvmf_rdma_listen_addr_free(addr);
 		pthread_mutex_unlock(&g_rdma.lock);
 		return -1;
 	}
@@ -1154,7 +1195,7 @@ spdk_nvmf_rdma_listen(struct spdk_nvmf_listen_addr *listen_addr)
 	if (!addr->comp_channel) {
 		SPDK_ERRLOG("Failed to create completion channel\n");
 		rdma_destroy_id(addr->id);
-		free(addr);
+		spdk_nvmf_rdma_listen_addr_free(addr);
 		pthread_mutex_unlock(&g_rdma.lock);
 		return -1;
 	}
@@ -1166,11 +1207,12 @@ spdk_nvmf_rdma_listen(struct spdk_nvmf_listen_addr *listen_addr)
 		SPDK_ERRLOG("fcntl to set comp channel to non-blocking failed\n");
 		rdma_destroy_id(addr->id);
 		ibv_destroy_comp_channel(addr->comp_channel);
-		free(addr);
+		spdk_nvmf_rdma_listen_addr_free(addr);
 		pthread_mutex_unlock(&g_rdma.lock);
 		return -1;
 	}
 
+	addr->ref = 1;
 	TAILQ_INSERT_TAIL(&g_rdma.listen_addrs, addr, link);
 	pthread_mutex_unlock(&g_rdma.lock);
 
@@ -1462,6 +1504,7 @@ const struct spdk_nvmf_transport spdk_nvmf_transport_rdma = {
 	.acceptor_poll = spdk_nvmf_rdma_acceptor_poll,
 
 	.listen_addr_add = spdk_nvmf_rdma_listen,
+	.listen_addr_remove = spdk_nvmf_rdma_listen_remove,
 	.listen_addr_discover = spdk_nvmf_rdma_discover,
 
 	.session_init = spdk_nvmf_rdma_session_init,

lib/nvmf/transport.h

@@ -68,6 +68,12 @@ struct spdk_nvmf_transport {
 	  */
 	int (*listen_addr_add)(struct spdk_nvmf_listen_addr *listen_addr);
 
+	/**
+	  * Instruct to remove listening on the address provided. This
+	  * may be called multiple times.
+	  */
+	int (*listen_addr_remove)(struct spdk_nvmf_listen_addr *listen_addr);
+
 	/**
 	 * Fill out a discovery log entry for a specific listen address.
 	 */