Commit b50c6bc2 authored by Naresh Gottumukkala's avatar Naresh Gottumukkala Committed by Jim Harris
Browse files

nvmf/fc: Use connection list hash table. 



Currently we are iterating over a hwqp connection list for every
IO command received. With high load of connections, this is causing
penalty. Use hash table for connection lookup based on connection ID
and also RPI identifier.

Signed-off-by: default avatarNaresh Gottumukkala <raju.gottumukkala@broadcom.com>
Change-Id: I857e299722a0b72b25b0dbfe646d446ad98b7c76
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/5688


Community-CI: Broadcom CI
Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
parent 7c9a81a2

lib/nvmf/Makefile

+1 −1
Original line number Diff line number Diff line
@@ -64,7 +64,7 @@ endif 


ifeq ($(CONFIG_FC),y)

C_SRCS += fc.c fc_ls.c

CFLAGS += -I$(CURDIR)

CFLAGS += -I$(CURDIR) $(ENV_CFLAGS)

ifneq ($(strip $(CONFIG_FC_PATH)),)

CFLAGS += -I$(CONFIG_FC_PATH)

endif

lib/nvmf/fc.c

+52 −21
Original line number Diff line number Diff line
@@ -106,6 +106,9 @@ static char *fc_req_state_strs[] = { 
#define TRACE_FC_REQ_BDEV_ABORTED               SPDK_TPOINT_ID(TRACE_GROUP_NVMF_FC, 0x0E)

#define TRACE_FC_REQ_PENDING                    SPDK_TPOINT_ID(TRACE_GROUP_NVMF_FC, 0x0F)



#define HWQP_CONN_TABLE_SIZE			8192

#define HWQP_RPI_TABLE_SIZE			4096



SPDK_TRACE_REGISTER_FN(nvmf_fc_trace, "nvmf_fc", TRACE_GROUP_NVMF_FC)

{

	spdk_trace_register_object(OBJECT_NVMF_FC_IO, 'r');
@@ -306,6 +309,18 @@ nvmf_fc_record_req_trace_point(struct spdk_nvmf_fc_request *fc_req, 
	}

}



static struct rte_hash *

nvmf_fc_create_hash_table(const char *name, size_t num_entries, size_t key_len)

{

	struct rte_hash_parameters hash_params = { 0 };



	hash_params.entries = num_entries;

	hash_params.key_len = key_len;

	hash_params.name = name;



	return rte_hash_create(&hash_params);

}



static void

nvmf_fc_handle_connection_failure(void *arg)

{
@@ -420,20 +435,6 @@ nvmf_fc_conn_free_fc_request(struct spdk_nvmf_fc_conn *fc_conn, struct spdk_nvmf 
	fc_conn->pool_free_elems += 1;

}



struct spdk_nvmf_fc_conn *

nvmf_fc_hwqp_find_fc_conn(struct spdk_nvmf_fc_hwqp *hwqp, uint64_t conn_id)

{

	struct spdk_nvmf_fc_conn *fc_conn;



	TAILQ_FOREACH(fc_conn, &hwqp->connection_list, link) {

		if (fc_conn->conn_id == conn_id) {

			return fc_conn;

		}

	}



	return NULL;

}



static inline void

nvmf_fc_request_remove_from_pending(struct spdk_nvmf_fc_request *fc_req)

{
@@ -441,21 +442,39 @@ nvmf_fc_request_remove_from_pending(struct spdk_nvmf_fc_request *fc_req) 
		      spdk_nvmf_request, buf_link);

}



void

int

nvmf_fc_init_hwqp(struct spdk_nvmf_fc_port *fc_port, struct spdk_nvmf_fc_hwqp *hwqp)

{

	char name[64];



	hwqp->fc_port = fc_port;



	/* clear counters */

	memset(&hwqp->counters, 0, sizeof(struct spdk_nvmf_fc_errors));



	TAILQ_INIT(&hwqp->in_use_reqs);

	TAILQ_INIT(&hwqp->connection_list);

	TAILQ_INIT(&hwqp->sync_cbs);

	TAILQ_INIT(&hwqp->ls_pending_queue);



	snprintf(name, sizeof(name), "nvmf_fc_conn_hash:%d-%d", fc_port->port_hdl, hwqp->hwqp_id);

	hwqp->connection_list_hash = nvmf_fc_create_hash_table(name, HWQP_CONN_TABLE_SIZE,

				     sizeof(uint64_t));

	if (!hwqp->connection_list_hash) {

		SPDK_ERRLOG("Failed to create connection hash table.\n");

		return -ENOMEM;

	}



	snprintf(name, sizeof(name), "nvmf_fc_rpi_hash:%d-%d", fc_port->port_hdl, hwqp->hwqp_id);

	hwqp->rport_list_hash = nvmf_fc_create_hash_table(name, HWQP_RPI_TABLE_SIZE, sizeof(uint16_t));

	if (!hwqp->rport_list_hash) {

		SPDK_ERRLOG("Failed to create rpi hash table.\n");

		rte_hash_free(hwqp->connection_list_hash);

		return -ENOMEM;

	}



	/* Init low level driver queues */

	nvmf_fc_init_q(hwqp);

	return 0;

}



static struct spdk_nvmf_fc_poll_group *
@@ -1339,9 +1358,8 @@ nvmf_fc_hwqp_handle_request(struct spdk_nvmf_fc_hwqp *hwqp, struct spdk_nvmf_fc_ 


	rqst_conn_id = from_be64(&cmd_iu->conn_id);



	/* Check if conn id is valid */

	fc_conn = nvmf_fc_hwqp_find_fc_conn(hwqp, rqst_conn_id);

	if (!fc_conn) {

	if (rte_hash_lookup_data(hwqp->connection_list_hash,

				 (void *)&rqst_conn_id, (void **)&fc_conn) < 0) {

		SPDK_ERRLOG("IU CMD conn(%ld) invalid\n", rqst_conn_id);

		hwqp->counters.invalid_conn_err++;

		return -ENODEV;
@@ -2144,6 +2162,7 @@ static int 
nvmf_fc_adm_hw_port_data_init(struct spdk_nvmf_fc_port *fc_port,

			      struct spdk_nvmf_fc_hw_port_init_args *args)

{

	int rc = 0;

	/* Used a high number for the LS HWQP so that it does not clash with the

	 * IO HWQP's and immediately shows a LS queue during tracing.

	 */
@@ -2169,7 +2188,10 @@ nvmf_fc_adm_hw_port_data_init(struct spdk_nvmf_fc_port *fc_port, 
	/*

	 * Initialize the LS queue.

	 */

	nvmf_fc_init_hwqp(fc_port, &fc_port->ls_queue);

	rc = nvmf_fc_init_hwqp(fc_port, &fc_port->ls_queue);

	if (rc) {

		return rc;

	}



	/*

	 * Initialize the IO queues.
@@ -2178,7 +2200,16 @@ nvmf_fc_adm_hw_port_data_init(struct spdk_nvmf_fc_port *fc_port, 
		struct spdk_nvmf_fc_hwqp *hwqp = &fc_port->io_queues[i];

		hwqp->hwqp_id = i;

		hwqp->queues = args->io_queues[i];

		nvmf_fc_init_hwqp(fc_port, hwqp);

		rc = nvmf_fc_init_hwqp(fc_port, hwqp);

		if (rc) {

			for (; i > 0; --i) {

				rte_hash_free(fc_port->io_queues[i - 1].connection_list_hash);

				rte_hash_free(fc_port->io_queues[i - 1].rport_list_hash);

			}

			rte_hash_free(fc_port->ls_queue.connection_list_hash);

			rte_hash_free(fc_port->ls_queue.rport_list_hash);

			return rc;

		}

	}



	/*

lib/nvmf/fc_ls.c

+130 −40
Original line number Diff line number Diff line
@@ -1300,29 +1300,122 @@ nvmf_fc_poller_api_perform_cb(struct spdk_nvmf_fc_poller_api_cb_info *cb_info, 
	}

}



static int

nvmf_fc_poller_add_conn_lookup_data(struct spdk_nvmf_fc_hwqp *hwqp,

				    struct spdk_nvmf_fc_conn *fc_conn)

{

	int rc = -1;

	struct spdk_nvmf_fc_hwqp_rport *rport = NULL;



	/* Add connection based lookup entry. */

	rc = rte_hash_add_key_data(hwqp->connection_list_hash,

				   (void *)&fc_conn->conn_id, (void *)fc_conn);



	if (rc < 0) {

		SPDK_ERRLOG("Failed to add connection hash entry\n");

		return rc;

	}



	/* RPI based lookup */

	if (rte_hash_lookup_data(hwqp->rport_list_hash, (void *)&fc_conn->rpi, (void **)&rport) < 0) {

		rport = calloc(1, sizeof(struct spdk_nvmf_fc_hwqp_rport));

		if (!rport) {

			SPDK_ERRLOG("Failed to allocate rport entry\n");

			rc = -ENOMEM;

			goto del_conn_hash;

		}



		/* Add rport table entry */

		rc = rte_hash_add_key_data(hwqp->rport_list_hash,

					   (void *)&fc_conn->rpi, (void *)rport);

		if (rc < 0) {

			SPDK_ERRLOG("Failed to add rport hash entry\n");

			goto del_rport;

		}

		TAILQ_INIT(&rport->conn_list);

	}



	/* Add to rport conn list */

	TAILQ_INSERT_TAIL(&rport->conn_list, fc_conn, rport_link);

	return 0;



del_rport:

	free(rport);

del_conn_hash:

	rte_hash_del_key(hwqp->connection_list_hash, (void *)&fc_conn->conn_id);

	return rc;

}



static void

nvmf_fc_poller_del_conn_lookup_data(struct spdk_nvmf_fc_hwqp *hwqp,

				    struct spdk_nvmf_fc_conn *fc_conn)

{

	struct spdk_nvmf_fc_hwqp_rport *rport = NULL;



	if (rte_hash_del_key(hwqp->connection_list_hash, (void *)&fc_conn->conn_id) < 0) {

		SPDK_ERRLOG("Failed to del connection(%lx) hash entry\n",

			    fc_conn->conn_id);

	}



	if (rte_hash_lookup_data(hwqp->rport_list_hash, (void *)&fc_conn->rpi, (void **)&rport) >= 0) {

		TAILQ_REMOVE(&rport->conn_list, fc_conn, rport_link);



		/* If last conn del rpi hash */

		if (TAILQ_EMPTY(&rport->conn_list)) {

			if (rte_hash_del_key(hwqp->rport_list_hash, (void *)&fc_conn->rpi) < 0) {

				SPDK_ERRLOG("Failed to del rpi(%lx) hash entry\n",

					    fc_conn->conn_id);

			}

			free(rport);

		}

	} else {

		SPDK_ERRLOG("RPI(%d) hash entry not found\n", fc_conn->rpi);

	}

}



static struct spdk_nvmf_fc_request *

nvmf_fc_poller_rpi_find_req(struct spdk_nvmf_fc_hwqp *hwqp, uint16_t rpi, uint16_t oxid)

{

	struct spdk_nvmf_fc_request *fc_req = NULL;

	struct spdk_nvmf_fc_conn *fc_conn;

	struct spdk_nvmf_fc_hwqp_rport *rport = NULL;



	if (rte_hash_lookup_data(hwqp->rport_list_hash, (void *)&rpi, (void **)&rport) >= 0) {

		TAILQ_FOREACH(fc_conn, &rport->conn_list, rport_link) {

			TAILQ_FOREACH(fc_req, &fc_conn->in_use_reqs, conn_link) {

				if (fc_req->oxid == oxid) {

					return fc_req;

				}

			}

		}

	}

	return NULL;

}



static void

nvmf_fc_poller_api_add_connection(void *arg)

{

	enum spdk_nvmf_fc_poller_api_ret ret = SPDK_NVMF_FC_POLLER_API_SUCCESS;

	struct spdk_nvmf_fc_poller_api_add_connection_args *conn_args =

		(struct spdk_nvmf_fc_poller_api_add_connection_args *)arg;

	struct spdk_nvmf_fc_conn *fc_conn;

	struct spdk_nvmf_fc_conn *fc_conn = conn_args->fc_conn, *tmp;



	SPDK_DEBUGLOG(nvmf_fc_poller_api, "Poller add connection, conn_id 0x%lx\n",

		      conn_args->fc_conn->conn_id);

		      fc_conn->conn_id);



	/* make sure connection is not already in poller's list */

	fc_conn = nvmf_fc_hwqp_find_fc_conn(conn_args->fc_conn->hwqp,

					    conn_args->fc_conn->conn_id);

	if (fc_conn) {

	if (rte_hash_lookup_data(fc_conn->hwqp->connection_list_hash,

				 (void *)&fc_conn->conn_id, (void **)&tmp) >= 0) {

		SPDK_ERRLOG("duplicate connection found");

		ret = SPDK_NVMF_FC_POLLER_API_DUP_CONN_ID;

	} else {

		SPDK_DEBUGLOG(nvmf_fc_poller_api,

			      "conn_id=%lx", fc_conn->conn_id);

		TAILQ_INSERT_TAIL(&conn_args->fc_conn->hwqp->connection_list,

				  conn_args->fc_conn, link);

		conn_args->fc_conn->hwqp->num_conns++;

		if (nvmf_fc_poller_add_conn_lookup_data(fc_conn->hwqp, fc_conn)) {

			SPDK_ERRLOG("Failed to add connection 0x%lx\n", fc_conn->conn_id);

			ret = SPDK_NVMF_FC_POLLER_API_ERROR;

		} else {

			SPDK_DEBUGLOG(nvmf_fc_poller_api, "conn_id=%lx", fc_conn->conn_id);

			fc_conn->hwqp->num_conns++;

		}

	}



	/* perform callback */
@@ -1386,17 +1479,19 @@ nvmf_fc_poller_conn_abort_done(void *hwqp, int32_t status, void *cb_args) 
	}



	if (!conn_args->fc_request_cnt) {

		if (!TAILQ_EMPTY(&conn_args->hwqp->connection_list)) {

		struct spdk_nvmf_fc_conn *fc_conn = conn_args->fc_conn, *tmp;



		if (rte_hash_lookup_data(conn_args->hwqp->connection_list_hash,

					 (void *)&fc_conn->conn_id, (void *)&tmp) >= 0) {

			/* All the requests for this connection are aborted. */

			TAILQ_REMOVE(&conn_args->hwqp->connection_list,	conn_args->fc_conn, link);

			conn_args->fc_conn->hwqp->num_conns--;

			nvmf_fc_poller_del_conn_lookup_data(conn_args->hwqp, fc_conn);

			fc_conn->hwqp->num_conns--;



			SPDK_DEBUGLOG(nvmf_fc_poller_api, "Connection deleted, conn_id 0x%lx\n",

				      conn_args->fc_conn->conn_id);

			SPDK_DEBUGLOG(nvmf_fc_poller_api, "Connection deleted, conn_id 0x%lx\n", fc_conn->conn_id);



			if (!conn_args->backend_initiated) {

				/* disconnect qpair from nvmf controller */

				spdk_nvmf_qpair_disconnect(&conn_args->fc_conn->qpair,

				spdk_nvmf_qpair_disconnect(&fc_conn->qpair,

							   nvmf_fc_disconnect_qpair_cb, &conn_args->cb_info);

			}

		} else {
@@ -1417,7 +1512,7 @@ nvmf_fc_poller_api_del_connection(void *arg) 
{

	struct spdk_nvmf_fc_poller_api_del_connection_args *conn_args =

		(struct spdk_nvmf_fc_poller_api_del_connection_args *)arg;

	struct spdk_nvmf_fc_conn *fc_conn = conn_args->fc_conn;

	struct spdk_nvmf_fc_conn *fc_conn = NULL;

	struct spdk_nvmf_fc_request *fc_req = NULL, *tmp;

	struct spdk_nvmf_fc_hwqp *hwqp = conn_args->hwqp;
@@ -1425,7 +1520,8 @@ nvmf_fc_poller_api_del_connection(void *arg) 
		      fc_conn->conn_id);



	/* Make sure connection is valid */

	if (!nvmf_fc_hwqp_find_fc_conn(hwqp, fc_conn->conn_id)) {

	if (rte_hash_lookup_data(hwqp->connection_list_hash,

				 (void *)&conn_args->fc_conn->conn_id, (void **)&fc_conn) < 0) {

		/* perform callback */

		nvmf_fc_poller_api_perform_cb(&conn_args->cb_info, SPDK_NVMF_FC_POLLER_API_NO_CONN_ID);

		return;
@@ -1433,8 +1529,7 @@ nvmf_fc_poller_api_del_connection(void *arg) 


	conn_args->fc_request_cnt = 0;



	TAILQ_FOREACH_SAFE(fc_req, &hwqp->in_use_reqs, link, tmp) {

		if (fc_req->fc_conn->conn_id == fc_conn->conn_id) {

	TAILQ_FOREACH_SAFE(fc_req, &fc_conn->in_use_reqs, conn_link, tmp) {

		if (nvmf_qpair_is_admin_queue(&fc_conn->qpair) &&

		    (fc_req->req.cmd->nvme_cmd.opc == SPDK_NVME_OPC_ASYNC_EVENT_REQUEST)) {

			/* AER will be cleaned by spdk_nvmf_qpair_disconnect. */
@@ -1446,11 +1541,10 @@ nvmf_fc_poller_api_del_connection(void *arg) 
				      nvmf_fc_poller_conn_abort_done,

				      conn_args);

	}

	}



	if (!conn_args->fc_request_cnt) {

		SPDK_DEBUGLOG(nvmf_fc_poller_api, "Connection deleted.\n");

		TAILQ_REMOVE(&hwqp->connection_list, fc_conn, link);

		nvmf_fc_poller_del_conn_lookup_data(conn_args->hwqp, conn_args->fc_conn);

		hwqp->num_conns--;



		if (!conn_args->backend_initiated) {
@@ -1478,17 +1572,13 @@ static void 
nvmf_fc_poller_api_abts_received(void *arg)

{

	struct spdk_nvmf_fc_poller_api_abts_recvd_args *args = arg;

	struct spdk_nvmf_fc_request *fc_req = NULL;

	struct spdk_nvmf_fc_hwqp *hwqp = args->hwqp;

	struct spdk_nvmf_fc_request *fc_req;



	TAILQ_FOREACH(fc_req, &hwqp->in_use_reqs, link) {

		if ((fc_req->rpi == args->ctx->rpi) &&

		    (fc_req->oxid == args->ctx->oxid)) {

			nvmf_fc_request_abort(fc_req, false,

					      nvmf_fc_poller_abts_done, args);

	fc_req = nvmf_fc_poller_rpi_find_req(args->hwqp, args->ctx->rpi, args->ctx->oxid);

	if (fc_req) {

		nvmf_fc_request_abort(fc_req, false, nvmf_fc_poller_abts_done, args);

		return;

	}

	}



	nvmf_fc_poller_api_perform_cb(&args->cb_info,

				      SPDK_NVMF_FC_POLLER_API_OXID_NOT_FOUND);

lib/nvmf/nvmf_fc.h

+9 −5
Original line number Diff line number Diff line
@@ -41,6 +41,7 @@ 
#include "spdk/nvmf_fc_spec.h"

#include "spdk/thread.h"

#include "nvmf_internal.h"

#include <rte_hash.h>



#define SPDK_NVMF_FC_TR_ADDR_LEN 64

#define NVMF_FC_INVALID_CONN_ID UINT64_MAX
@@ -237,9 +238,6 @@ struct spdk_nvmf_fc_conn { 
	/* for assocations's available connection list */

	TAILQ_ENTRY(spdk_nvmf_fc_conn) assoc_avail_link;



	/* for hwqp's connection list */

	TAILQ_ENTRY(spdk_nvmf_fc_conn) link;



	/* for hwqp's rport connection list link  */

	TAILQ_ENTRY(spdk_nvmf_fc_conn) rport_link;
@@ -304,8 +302,9 @@ struct spdk_nvmf_fc_hwqp { 
	struct spdk_nvmf_fc_poll_group *fgroup;



	/* qpair (fc_connection) list */

	TAILQ_HEAD(, spdk_nvmf_fc_conn) connection_list;

	uint32_t num_conns; /* number of connections to queue */

	struct rte_hash *connection_list_hash;

	struct rte_hash *rport_list_hash;



	TAILQ_HEAD(, spdk_nvmf_fc_request) in_use_reqs;
@@ -510,6 +509,11 @@ struct spdk_nvmf_fc_poller_api_queue_sync_done_args { 
	uint64_t tag;

};



struct spdk_nvmf_fc_hwqp_rport {

	uint16_t rpi;

	TAILQ_HEAD(, spdk_nvmf_fc_conn) conn_list;

};



/*

 * NVMF LS request structure

 */
@@ -871,7 +875,7 @@ void nvmf_fc_ls_add_conn_failure( 
	struct spdk_nvmf_fc_conn *fc_conn,

	bool aq_conn);



void nvmf_fc_init_hwqp(struct spdk_nvmf_fc_port *fc_port, struct spdk_nvmf_fc_hwqp *hwqp);

int nvmf_fc_init_hwqp(struct spdk_nvmf_fc_port *fc_port, struct spdk_nvmf_fc_hwqp *hwqp);



struct spdk_nvmf_fc_conn *nvmf_fc_hwqp_find_fc_conn(struct spdk_nvmf_fc_hwqp *hwqp,

		uint64_t conn_id);

test/unit/lib/nvmf/fc.c/Makefile

+1 −1
Original line number Diff line number Diff line
@@ -35,7 +35,7 @@ SPDK_ROOT_DIR := $(abspath $(CURDIR)/../../../../../) 
include $(SPDK_ROOT_DIR)/mk/config.mk



CFLAGS += -I$(SPDK_ROOT_DIR)/test/common/lib -I$(SPDK_ROOT_DIR)/lib \

-I$(SPDK_ROOT_DIR)/lib/nvmf

-I$(SPDK_ROOT_DIR)/lib/nvmf $(ENV_CFLAGS)



ifneq ($(strip $(CONFIG_FC_PATH)),)

CFLAGS += -I$(CONFIG_FC_PATH)