From a4e28342a84f25b21a8b5d384c17e40bf013ef1d Mon Sep 17 00:00:00 2001 From: Ben Walker Date: Tue, 25 Jul 2017 13:47:41 -0700 Subject: [PATCH] nvmf: Add wrappers for transport calls Instead of scattering direct calls to the function callbacks throughout the code, add some wrappers. This will make some later refactoring marginally easier. Change-Id: If735089967e3ce828dcff68f2430e7810bf2f123 Signed-off-by: Ben Walker Reviewed-on: https://review.gerrithub.io/371749 Reviewed-by: Daniel Verkamp Tested-by: SPDK Automated Test System --- lib/nvmf/ctrlr.c | 18 ++-- lib/nvmf/ctrlr_discovery.c | 2 +- lib/nvmf/nvmf.c | 4 +- lib/nvmf/request.c | 2 +- lib/nvmf/subsystem.c | 4 +- lib/nvmf/transport.c | 90 ++++++++++++++++++- lib/nvmf/transport.h | 30 ++++++- test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c | 36 ++++++++ .../ctrlr_discovery.c/ctrlr_discovery_ut.c | 38 ++++---- test/unit/lib/nvmf/request.c/request_ut.c | 6 ++ test/unit/lib/nvmf/subsystem.c/subsystem_ut.c | 34 ++++--- 11 files changed, 207 insertions(+), 57 deletions(-) diff --git a/lib/nvmf/ctrlr.c b/lib/nvmf/ctrlr.c index efb23442d..476238b06 100644 --- a/lib/nvmf/ctrlr.c +++ b/lib/nvmf/ctrlr.c @@ -163,7 +163,7 @@ nvmf_init_nvme_ctrlr_properties(struct spdk_nvmf_ctrlr *ctrlr) static void ctrlr_destruct(struct spdk_nvmf_ctrlr *ctrlr) { TAILQ_REMOVE(&ctrlr->subsys->ctrlrs, ctrlr, link); - ctrlr->transport->ops->ctrlr_fini(ctrlr); + spdk_nvmf_transport_ctrlr_fini(ctrlr); } void @@ -174,7 +174,7 @@ spdk_nvmf_ctrlr_destruct(struct spdk_nvmf_ctrlr *ctrlr) TAILQ_REMOVE(&ctrlr->qpairs, qpair, link); ctrlr->num_qpairs--; - qpair->transport->ops->qpair_fini(qpair); + spdk_nvmf_transport_qpair_fini(qpair); } ctrlr_destruct(ctrlr); @@ -288,7 +288,7 @@ spdk_nvmf_ctrlr_connect(struct spdk_nvmf_qpair *qpair, } /* Establish a new ctrlr */ - ctrlr = qpair->transport->ops->ctrlr_init(qpair->transport); + ctrlr = spdk_nvmf_transport_ctrlr_init(qpair->transport); if (ctrlr == NULL) { SPDK_ERRLOG("Memory allocation failure\n"); rsp->status.sc = SPDK_NVME_SC_INTERNAL_DEVICE_ERROR; @@ -313,9 +313,9 @@ spdk_nvmf_ctrlr_connect(struct spdk_nvmf_qpair *qpair, memcpy(ctrlr->hostid, data->hostid, sizeof(ctrlr->hostid)); - if (qpair->transport->ops->ctrlr_add_qpair(ctrlr, qpair)) { + if (spdk_nvmf_transport_ctrlr_add_qpair(ctrlr, qpair)) { rsp->status.sc = SPDK_NVME_SC_INTERNAL_DEVICE_ERROR; - qpair->transport->ops->ctrlr_fini(ctrlr); + spdk_nvmf_transport_ctrlr_fini(ctrlr); free(ctrlr); return; } @@ -374,7 +374,7 @@ spdk_nvmf_ctrlr_connect(struct spdk_nvmf_qpair *qpair, return; } - if (qpair->transport->ops->ctrlr_add_qpair(ctrlr, qpair)) { + if (spdk_nvmf_transport_ctrlr_add_qpair(ctrlr, qpair)) { INVALID_CONNECT_CMD(qid); return; } @@ -399,8 +399,8 @@ spdk_nvmf_ctrlr_disconnect(struct spdk_nvmf_qpair *qpair) ctrlr->num_qpairs--; TAILQ_REMOVE(&ctrlr->qpairs, qpair, link); - qpair->transport->ops->ctrlr_remove_qpair(ctrlr, qpair); - qpair->transport->ops->qpair_fini(qpair); + spdk_nvmf_transport_ctrlr_remove_qpair(ctrlr, qpair); + spdk_nvmf_transport_qpair_fini(qpair); if (ctrlr->num_qpairs == 0) { ctrlr_destruct(ctrlr); @@ -654,7 +654,7 @@ spdk_nvmf_ctrlr_poll(struct spdk_nvmf_ctrlr *ctrlr) } TAILQ_FOREACH_SAFE(qpair, &ctrlr->qpairs, link, tmp) { - if (qpair->transport->ops->qpair_poll(qpair) < 0) { + if (spdk_nvmf_transport_qpair_poll(qpair) < 0) { SPDK_ERRLOG("Transport poll failed for qpair %p; closing connection\n", qpair); spdk_nvmf_ctrlr_disconnect(qpair); } diff --git a/lib/nvmf/ctrlr_discovery.c b/lib/nvmf/ctrlr_discovery.c index e0eda9fb3..2b2f670ef 100644 --- a/lib/nvmf/ctrlr_discovery.c +++ b/lib/nvmf/ctrlr_discovery.c @@ -102,7 +102,7 @@ nvmf_update_discovery_log(void) transport = spdk_nvmf_tgt_get_transport(&g_nvmf_tgt, listen_addr->trid.trtype); assert(transport != NULL); - transport->ops->listen_addr_discover(transport, listen_addr, entry); + spdk_nvmf_transport_listen_addr_discover(transport, listen_addr, entry); numrec++; } diff --git a/lib/nvmf/nvmf.c b/lib/nvmf/nvmf.c index 3ff6775e7..5412490f8 100644 --- a/lib/nvmf/nvmf.c +++ b/lib/nvmf/nvmf.c @@ -133,7 +133,7 @@ spdk_nvmf_listen_addr_destroy(struct spdk_nvmf_listen_addr *addr) return; } - transport->ops->listen_addr_remove(transport, addr); + spdk_nvmf_transport_listen_addr_remove(transport, addr); free(addr); } @@ -143,7 +143,7 @@ spdk_nvmf_tgt_poll(void) struct spdk_nvmf_transport *transport, *tmp; TAILQ_FOREACH_SAFE(transport, &g_nvmf_tgt.transports, link, tmp) { - transport->ops->acceptor_poll(transport); + spdk_nvmf_transport_acceptor_poll(transport); } } diff --git a/lib/nvmf/request.c b/lib/nvmf/request.c index 8c74c6522..9e21bfbe0 100644 --- a/lib/nvmf/request.c +++ b/lib/nvmf/request.c @@ -59,7 +59,7 @@ spdk_nvmf_request_complete(struct spdk_nvmf_request *req) response->cid, response->cdw0, response->rsvd1, *(uint16_t *)&response->status); - if (req->qpair->transport->ops->req_complete(req)) { + if (spdk_nvmf_transport_req_complete(req)) { SPDK_ERRLOG("Transport request completion error!\n"); return -1; } diff --git a/lib/nvmf/subsystem.c b/lib/nvmf/subsystem.c index 3a62884f4..68e8860c1 100644 --- a/lib/nvmf/subsystem.c +++ b/lib/nvmf/subsystem.c @@ -118,7 +118,7 @@ nvmf_subsystem_removable(struct spdk_nvmf_subsystem *subsystem) if (subsystem->is_removed) { TAILQ_FOREACH(ctrlr, &subsystem->ctrlrs, link) { TAILQ_FOREACH(qpair, &ctrlr->qpairs, link) { - if (!qpair->transport->ops->qpair_is_idle(qpair)) { + if (!spdk_nvmf_transport_qpair_is_idle(qpair)) { return false; } } @@ -287,7 +287,7 @@ spdk_nvmf_tgt_listen(struct spdk_nvme_transport_id *trid) return NULL; } - rc = transport->ops->listen_addr_add(transport, listen_addr); + rc = spdk_nvmf_transport_listen_addr_add(transport, listen_addr); if (rc < 0) { free(listen_addr); SPDK_ERRLOG("Unable to listen on address '%s'\n", trid->traddr); diff --git a/lib/nvmf/transport.c b/lib/nvmf/transport.c index ab8862111..4fed37da5 100644 --- a/lib/nvmf/transport.c +++ b/lib/nvmf/transport.c @@ -40,7 +40,9 @@ #include "spdk/queue.h" #include "spdk/util.h" +#include "ctrlr.h" #include "nvmf_internal.h" +#include "request.h" static const struct spdk_nvmf_transport_ops *const g_transport_ops[] = { #ifdef SPDK_CONFIG_RDMA @@ -91,7 +93,93 @@ spdk_nvmf_transport_destroy(struct spdk_nvmf_transport *transport) } void -spdk_nvmf_transport_poll(struct spdk_nvmf_transport *transport) +spdk_nvmf_transport_acceptor_poll(struct spdk_nvmf_transport *transport) { transport->ops->acceptor_poll(transport); } + +int +spdk_nvmf_transport_listen_addr_add(struct spdk_nvmf_transport *transport, + struct spdk_nvmf_listen_addr *listen_addr) +{ + return transport->ops->listen_addr_add(transport, listen_addr); +} + +int +spdk_nvmf_transport_listen_addr_remove(struct spdk_nvmf_transport *transport, + struct spdk_nvmf_listen_addr *listen_addr) +{ + return transport->ops->listen_addr_remove(transport, listen_addr); +} + +void +spdk_nvmf_transport_listen_addr_discover(struct spdk_nvmf_transport *transport, + struct spdk_nvmf_listen_addr *listen_addr, + struct spdk_nvmf_discovery_log_page_entry *entry) +{ + transport->ops->listen_addr_discover(transport, listen_addr, entry); +} + +struct spdk_nvmf_ctrlr * +spdk_nvmf_transport_ctrlr_init(struct spdk_nvmf_transport *transport) +{ + struct spdk_nvmf_ctrlr *ctrlr; + + ctrlr = transport->ops->ctrlr_init(transport); + ctrlr->transport = transport; + + return ctrlr; +} + +void +spdk_nvmf_transport_ctrlr_fini(struct spdk_nvmf_ctrlr *ctrlr) +{ + ctrlr->transport->ops->ctrlr_fini(ctrlr); +} + +int +spdk_nvmf_transport_ctrlr_add_qpair(struct spdk_nvmf_ctrlr *ctrlr, + struct spdk_nvmf_qpair *qpair) +{ + if (qpair->transport) { + assert(qpair->transport == ctrlr->transport); + if (qpair->transport != ctrlr->transport) { + return -1; + } + } else { + qpair->transport = ctrlr->transport; + } + + return ctrlr->transport->ops->ctrlr_add_qpair(ctrlr, qpair); +} + +int +spdk_nvmf_transport_ctrlr_remove_qpair(struct spdk_nvmf_ctrlr *ctrlr, + struct spdk_nvmf_qpair *qpair) +{ + return ctrlr->transport->ops->ctrlr_remove_qpair(ctrlr, qpair); +} + +int +spdk_nvmf_transport_req_complete(struct spdk_nvmf_request *req) +{ + return req->qpair->transport->ops->req_complete(req); +} + +void +spdk_nvmf_transport_qpair_fini(struct spdk_nvmf_qpair *qpair) +{ + qpair->transport->ops->qpair_fini(qpair); +} + +int +spdk_nvmf_transport_qpair_poll(struct spdk_nvmf_qpair *qpair) +{ + return qpair->transport->ops->qpair_poll(qpair); +} + +bool +spdk_nvmf_transport_qpair_is_idle(struct spdk_nvmf_qpair *qpair) +{ + return qpair->transport->ops->qpair_is_idle(qpair); +} diff --git a/lib/nvmf/transport.h b/lib/nvmf/transport.h index bad623b42..a831c7be6 100644 --- a/lib/nvmf/transport.h +++ b/lib/nvmf/transport.h @@ -136,7 +136,35 @@ struct spdk_nvmf_transport *spdk_nvmf_transport_create(struct spdk_nvmf_tgt *tgt enum spdk_nvme_transport_type type); int spdk_nvmf_transport_destroy(struct spdk_nvmf_transport *transport); -void spdk_nvmf_transport_poll(struct spdk_nvmf_transport *transport); +void spdk_nvmf_transport_acceptor_poll(struct spdk_nvmf_transport *transport); + +int spdk_nvmf_transport_listen_addr_add(struct spdk_nvmf_transport *transport, + struct spdk_nvmf_listen_addr *listen_addr); + +int spdk_nvmf_transport_listen_addr_remove(struct spdk_nvmf_transport *transport, + struct spdk_nvmf_listen_addr *listen_addr); + +void spdk_nvmf_transport_listen_addr_discover(struct spdk_nvmf_transport *transport, + struct spdk_nvmf_listen_addr *listen_addr, + struct spdk_nvmf_discovery_log_page_entry *entry); + +struct spdk_nvmf_ctrlr *spdk_nvmf_transport_ctrlr_init(struct spdk_nvmf_transport *transport); + +void spdk_nvmf_transport_ctrlr_fini(struct spdk_nvmf_ctrlr *ctrlr); + +int spdk_nvmf_transport_ctrlr_add_qpair(struct spdk_nvmf_ctrlr *ctrlr, + struct spdk_nvmf_qpair *qpair); + +int spdk_nvmf_transport_ctrlr_remove_qpair(struct spdk_nvmf_ctrlr *ctrlr, + struct spdk_nvmf_qpair *qpair); + +int spdk_nvmf_transport_req_complete(struct spdk_nvmf_request *req); + +void spdk_nvmf_transport_qpair_fini(struct spdk_nvmf_qpair *qpair); + +int spdk_nvmf_transport_qpair_poll(struct spdk_nvmf_qpair *qpair); + +bool spdk_nvmf_transport_qpair_is_idle(struct spdk_nvmf_qpair *qpair); extern const struct spdk_nvmf_transport_ops spdk_nvmf_transport_rdma; diff --git a/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c b/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c index 058c6a261..018c8e6c0 100644 --- a/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c +++ b/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c @@ -59,6 +59,42 @@ spdk_nvme_ctrlr_get_data(struct spdk_nvme_ctrlr *ctrlr) return NULL; } +struct spdk_nvmf_ctrlr * +spdk_nvmf_transport_ctrlr_init(struct spdk_nvmf_transport *transport) +{ + return NULL; +} + +void +spdk_nvmf_transport_ctrlr_fini(struct spdk_nvmf_ctrlr *ctrlr) +{ +} + +void +spdk_nvmf_transport_qpair_fini(struct spdk_nvmf_qpair *qpair) +{ +} + +int +spdk_nvmf_transport_ctrlr_add_qpair(struct spdk_nvmf_ctrlr *ctrlr, + struct spdk_nvmf_qpair *qpair) +{ + return 0; +} + +int +spdk_nvmf_transport_ctrlr_remove_qpair(struct spdk_nvmf_ctrlr *ctrlr, + struct spdk_nvmf_qpair *qpair) +{ + return 0; +} + +int +spdk_nvmf_transport_qpair_poll(struct spdk_nvmf_qpair *qpair) +{ + return 0; +} + static void test_foobar(void) { 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 caf089f4a..6567f62e4 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 @@ -79,37 +79,30 @@ spdk_bdev_get_name(const struct spdk_bdev *bdev) return "test"; } -static int -test_transport1_listen_addr_add(struct spdk_nvmf_transport *transport, - struct spdk_nvmf_listen_addr *listen_addr) +int +spdk_nvmf_transport_listen_addr_add(struct spdk_nvmf_transport *transport, + struct spdk_nvmf_listen_addr *listen_addr) { return 0; } -static void -test_transport1_listen_addr_discover(struct spdk_nvmf_transport *transport, - struct spdk_nvmf_listen_addr *listen_addr, - struct spdk_nvmf_discovery_log_page_entry *entry) +void +spdk_nvmf_transport_listen_addr_discover(struct spdk_nvmf_transport *transport, + struct spdk_nvmf_listen_addr *listen_addr, + struct spdk_nvmf_discovery_log_page_entry *entry) { entry->trtype = 42; } -static const struct spdk_nvmf_transport_ops test_transport1_ops = { - .listen_addr_add = test_transport1_listen_addr_add, - .listen_addr_discover = test_transport1_listen_addr_discover, -}; - -static struct spdk_nvmf_transport test_transport1 = { - .ops = &test_transport1_ops, -}; +static struct spdk_nvmf_transport g_transport = {}; struct spdk_nvmf_transport * spdk_nvmf_transport_create(struct spdk_nvmf_tgt *tgt, enum spdk_nvme_transport_type type) { if (type == SPDK_NVME_TRANSPORT_RDMA) { - test_transport1.tgt = tgt; - return &test_transport1; + g_transport.tgt = tgt; + return &g_transport; } return NULL; @@ -118,12 +111,13 @@ spdk_nvmf_transport_create(struct spdk_nvmf_tgt *tgt, struct spdk_nvmf_transport * spdk_nvmf_tgt_get_transport(struct spdk_nvmf_tgt *tgt, enum spdk_nvme_transport_type trtype) { - if (trtype == SPDK_NVME_TRANSPORT_RDMA) { - test_transport1.tgt = tgt; - return &test_transport1; - } + return &g_transport; +} - return NULL; +bool +spdk_nvmf_transport_qpair_is_idle(struct spdk_nvmf_qpair *qpair) +{ + return false; } int diff --git a/test/unit/lib/nvmf/request.c/request_ut.c b/test/unit/lib/nvmf/request.c/request_ut.c index eeb13db28..785c5dadf 100644 --- a/test/unit/lib/nvmf/request.c/request_ut.c +++ b/test/unit/lib/nvmf/request.c/request_ut.c @@ -44,6 +44,12 @@ void spdk_trace_record(uint16_t tpoint_id, uint16_t poller_id, uint32_t size, { } +int +spdk_nvmf_transport_req_complete(struct spdk_nvmf_request *req) +{ + return 0; +} + void spdk_nvmf_ctrlr_connect(struct spdk_nvmf_qpair *qpair, struct spdk_nvmf_fabric_connect_cmd *cmd, diff --git a/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c b/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c index 0efdb5216..1b8623470 100644 --- a/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c +++ b/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c @@ -66,37 +66,36 @@ spdk_nvmf_listen_addr_destroy(struct spdk_nvmf_listen_addr *addr) free(addr); } -static int -test_transport1_listen_addr_add(struct spdk_nvmf_transport *transport, - struct spdk_nvmf_listen_addr *listen_addr) +int +spdk_nvmf_transport_listen_addr_add(struct spdk_nvmf_transport *transport, + struct spdk_nvmf_listen_addr *listen_addr) { return 0; } -static void -test_transport1_listen_addr_discover(struct spdk_nvmf_transport *transport, - struct spdk_nvmf_listen_addr *listen_addr, - struct spdk_nvmf_discovery_log_page_entry *entry) +void +spdk_nvmf_transport_listen_addr_discover(struct spdk_nvmf_transport *transport, + struct spdk_nvmf_listen_addr *listen_addr, + struct spdk_nvmf_discovery_log_page_entry *entry) { entry->trtype = 42; } -static const struct spdk_nvmf_transport_ops test_transport1_ops = { - .listen_addr_add = test_transport1_listen_addr_add, - .listen_addr_discover = test_transport1_listen_addr_discover, -}; +bool +spdk_nvmf_transport_qpair_is_idle(struct spdk_nvmf_qpair *qpair) +{ + return false; +} -static struct spdk_nvmf_transport test_transport1 = { - .ops = &test_transport1_ops, -}; +static struct spdk_nvmf_transport g_transport = {}; struct spdk_nvmf_transport * spdk_nvmf_transport_create(struct spdk_nvmf_tgt *tgt, enum spdk_nvme_transport_type type) { if (type == SPDK_NVME_TRANSPORT_RDMA) { - test_transport1.tgt = tgt; - return &test_transport1; + g_transport.tgt = tgt; + return &g_transport; } return NULL; @@ -106,8 +105,7 @@ struct spdk_nvmf_transport * spdk_nvmf_tgt_get_transport(struct spdk_nvmf_tgt *tgt, enum spdk_nvme_transport_type trtype) { if (trtype == SPDK_NVME_TRANSPORT_RDMA) { - test_transport1.tgt = tgt; - return &test_transport1; + return &g_transport; } return NULL;