Commit 4cd6544d authored by Ziye Yang's avatar Ziye Yang Committed by Ben Walker
Browse files

nvmf: solve the memory leak issue caused by subsystem listerner port 



The possible issue could be following if you shutdown NVMe-oF target
with TCP transport as an example,

=================================================================
==61022==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 560 byte(s) in 1 object(s) allocated from:
    #0 0x7ffff6efcfe0 in calloc (/lib64/libasan.so.3+0xc6fe0)
    #1 0x4c6216 in spdk_nvmf_tcp_listen /home/ziyeyang/spdk/lib/nvmf/tcp.c:680

Indirect leak of 48 byte(s) in 1 object(s) allocated from:
    #0 0x7ffff6efcfe0 in calloc (/lib64/libasan.so.3+0xc6fe0)
    #1 0x4a77b8 in spdk_posix_sock_create /home/ziyeyang/spdk/lib/sock/posix/posix.c:291

After checking the issue, it seems that we did not call
spdk_nvmf_transport_stop_listen when removing the subsystem listener.
And this patch can solve this issue.

Change-Id: Ic75d99cb0c6a3ba1c47ac79a2d8e3887b0f6b012
Signed-off-by: default avatarZiye Yang <ziye.yang@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/447020


Reviewed-by: default avatarDarek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avataryidong0635 <dongx.yi@intel.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarSeth Howell <seth.howell5141@gmail.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent d6c4518c

lib/nvmf/subsystem.c

+17 −4
Original line number Diff line number Diff line
@@ -308,6 +308,21 @@ _spdk_nvmf_subsystem_remove_host(struct spdk_nvmf_subsystem *subsystem, struct s 


static int _spdk_nvmf_subsystem_remove_ns(struct spdk_nvmf_subsystem *subsystem, uint32_t nsid);



static void

_nvmf_subsystem_remove_listener(struct spdk_nvmf_subsystem *subsystem,

				struct spdk_nvmf_listener *listener)

{

	struct spdk_nvmf_transport *transport;



	transport = spdk_nvmf_tgt_get_transport(subsystem->tgt, listener->trid.trtype);

	if (transport != NULL) {

		spdk_nvmf_transport_stop_listen(transport, &listener->trid);

	}



	TAILQ_REMOVE(&subsystem->listeners, listener, link);

	free(listener);

}



void

spdk_nvmf_subsystem_destroy(struct spdk_nvmf_subsystem *subsystem)

{
@@ -325,8 +340,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) {

		TAILQ_REMOVE(&subsystem->listeners, listener, link);

		free(listener);

		_nvmf_subsystem_remove_listener(subsystem, listener);

	}



	TAILQ_FOREACH_SAFE(host, &subsystem->hosts, link, host_tmp) {
@@ -779,8 +793,7 @@ spdk_nvmf_subsystem_remove_listener(struct spdk_nvmf_subsystem *subsystem, 
		return -ENOENT;

	}



	TAILQ_REMOVE(&subsystem->listeners, listener, link);

	free(listener);

	_nvmf_subsystem_remove_listener(subsystem, listener);



	return 0;

}

test/unit/lib/nvmf/ctrlr_discovery.c/ctrlr_discovery_ut.c

+5 −0
Original line number Diff line number Diff line
@@ -48,6 +48,11 @@ DEFINE_STUB_V(spdk_bdev_module_release_bdev, 
DEFINE_STUB(spdk_bdev_get_block_size, uint32_t,

	    (const struct spdk_bdev *bdev), 512);



DEFINE_STUB(spdk_nvmf_transport_stop_listen,

	    int,

	    (struct spdk_nvmf_transport *transport,

	     const struct spdk_nvme_transport_id *trid), 0);



uint32_t

spdk_env_get_current_core(void)

{

test/unit/lib/nvmf/subsystem.c/subsystem_ut.c

+5 −0
Original line number Diff line number Diff line
@@ -53,6 +53,11 @@ DEFINE_STUB_V(spdk_bdev_module_release_bdev, 
DEFINE_STUB(spdk_bdev_get_block_size, uint32_t,

	    (const struct spdk_bdev *bdev), 512);



DEFINE_STUB(spdk_nvmf_transport_stop_listen,

	    int,

	    (struct spdk_nvmf_transport *transport,

	     const struct spdk_nvme_transport_id *trid), 0);



static void

subsystem_ns_remove_cb(struct spdk_nvmf_subsystem *subsystem, void *cb_arg, int status)

{