diff --git a/fabtests/pytest/efa/test_multi_ep.py b/fabtests/pytest/efa/test_multi_ep.py index bf34a5cca28..634529f0067 100644 --- a/fabtests/pytest/efa/test_multi_ep.py +++ b/fabtests/pytest/efa/test_multi_ep.py @@ -2,17 +2,10 @@ @pytest.mark.functional @pytest.mark.parametrize("shared_cq", [True, False]) -def test_multi_ep_cq(cmdline_args, shared_cq): +def test_multi_ep(cmdline_args, shared_cq): from common import ClientServerTest cmd = "fi_multi_ep -e rdm" if shared_cq: cmd += " -Q" test = ClientServerTest(cmdline_args, cmd) test.run() - -@pytest.mark.functional -def test_multi_ep_av(cmdline_args): - from common import ClientServerTest - cmd = "fi_multi_ep -e rdm -A" - test = ClientServerTest(cmdline_args, cmd) - test.run() diff --git a/prov/efa/src/efa_av.c b/prov/efa/src/efa_av.c index 4b1d2f70442..7fef9d5b41c 100644 --- a/prov/efa/src/efa_av.c +++ b/prov/efa/src/efa_av.c @@ -243,6 +243,9 @@ void efa_ah_release(struct efa_av *av, struct efa_ah *ah) } } +static +void efa_conn_release(struct efa_av *av, struct efa_conn *conn); + /** * @brief initialize the rdm related resources of an efa_conn object * @@ -263,11 +266,18 @@ int efa_conn_rdm_init(struct efa_av *av, struct efa_conn *conn, bool insert_shm_ int err, ret; char smr_name[EFA_SHM_NAME_MAX]; size_t smr_name_len; + struct efa_rdm_ep *efa_rdm_ep; + struct efa_rdm_peer *peer; assert(av->ep_type == FI_EP_RDM); assert(conn->ep_addr); - conn->shm_fi_addr = FI_ADDR_NOTAVAIL; + /* currently multiple EP bind to same av is not supported */ + assert(!dlist_empty(&av->util_av.ep_list)); + efa_rdm_ep = container_of(av->util_av.ep_list.next, struct efa_rdm_ep, base_ep.util_ep.av_entry); + + peer = &conn->rdm_peer; + efa_rdm_peer_construct(peer, efa_rdm_ep, conn); /* * The efa_conn_rdm_init() call can be made in two situations: @@ -305,8 +315,8 @@ int efa_conn_rdm_init(struct efa_av *av, struct efa_conn *conn, bool insert_shm_ * av. The efa provider should still use peer->shm_fiaddr for transmissions * through shm ep. */ - conn->shm_fi_addr = conn->fi_addr; - ret = fi_av_insert(av->shm_rdm_av, smr_name, 1, &conn->shm_fi_addr, FI_AV_USER_ID, NULL); + peer->shm_fiaddr = conn->fi_addr; + ret = fi_av_insert(av->shm_rdm_av, smr_name, 1, &peer->shm_fiaddr, FI_AV_USER_ID, NULL); if (OFI_UNLIKELY(ret != 1)) { EFA_WARN(FI_LOG_AV, "Failed to insert address to shm provider's av: %s\n", @@ -316,10 +326,11 @@ int efa_conn_rdm_init(struct efa_av *av, struct efa_conn *conn, bool insert_shm_ EFA_INFO(FI_LOG_AV, "Successfully inserted %s to shm provider's av. efa_fiaddr: %ld shm_fiaddr = %ld\n", - smr_name, conn->fi_addr, conn->shm_fi_addr); + smr_name, conn->fi_addr, peer->shm_fiaddr); - assert(conn->shm_fi_addr < efa_env.shm_av_size); + assert(peer->shm_fiaddr < efa_env.shm_av_size); av->shm_used++; + peer->is_local = 1; } return 0; @@ -339,29 +350,26 @@ void efa_conn_rdm_deinit(struct efa_av *av, struct efa_conn *conn) int err; struct efa_rdm_peer *peer; struct efa_rdm_ep *ep; - struct dlist_entry *entry, *tmp; assert(av->ep_type == FI_EP_RDM); peer = &conn->rdm_peer; - if (conn->shm_fi_addr != FI_ADDR_NOTAVAIL && av->shm_rdm_av) { - err = fi_av_remove(av->shm_rdm_av, &conn->shm_fi_addr, 1, 0); + if (peer->is_local && av->shm_rdm_av) { + err = fi_av_remove(av->shm_rdm_av, &peer->shm_fiaddr, 1, 0); if (err) { EFA_WARN(FI_LOG_AV, "remove address from shm av failed! err=%d\n", err); } else { av->shm_used--; - assert(conn->shm_fi_addr < efa_env.shm_av_size); + assert(peer->shm_fiaddr < efa_env.shm_av_size); } } - dlist_foreach_safe(&av->util_av.ep_list, entry, tmp) { - ep = container_of(entry, struct efa_rdm_ep, base_ep.util_ep.av_entry); - peer = efa_rdm_peer_map_lookup(&ep->fi_addr_to_peer_map, conn->fi_addr); - if (peer) { - efa_rdm_peer_destruct(peer, ep); - efa_rdm_peer_map_remove(&ep->fi_addr_to_peer_map, conn->fi_addr, peer); - } - } + /* + * We need peer->shm_fiaddr to remove shm address from shm av table, + * so efa_rdm_peer_clear must be after removing shm av table. + */ + ep = dlist_empty(&av->util_av.ep_list) ? NULL : container_of(av->util_av.ep_list.next, struct efa_rdm_ep, base_ep.util_ep.av_entry); + efa_rdm_peer_destruct(peer, ep); } /* diff --git a/prov/efa/src/efa_av.h b/prov/efa/src/efa_av.h index acf7e58e320..5d885adbdca 100644 --- a/prov/efa/src/efa_av.h +++ b/prov/efa/src/efa_av.h @@ -27,7 +27,6 @@ struct efa_conn { fi_addr_t fi_addr; fi_addr_t util_av_fi_addr; struct efa_rdm_peer rdm_peer; - fi_addr_t shm_fi_addr; }; struct efa_av_entry { @@ -61,6 +60,7 @@ struct efa_prv_reverse_av { struct efa_av { struct fid_av *shm_rdm_av; struct efa_domain *domain; + struct efa_base_ep *base_ep; size_t used; size_t shm_used; enum fi_av_type type; diff --git a/prov/efa/src/efa_base_ep.c b/prov/efa/src/efa_base_ep.c index 56bd82bd87e..2abdee189dc 100644 --- a/prov/efa/src/efa_base_ep.c +++ b/prov/efa/src/efa_base_ep.c @@ -8,6 +8,15 @@ int efa_base_ep_bind_av(struct efa_base_ep *base_ep, struct efa_av *av) { + /* + * Binding multiple endpoints to a single AV is currently not + * supported. + */ + if (av->base_ep) { + EFA_WARN(FI_LOG_EP_CTRL, + "Address vector already has endpoint bound to it.\n"); + return -FI_ENOSYS; + } if (base_ep->domain != av->domain) { EFA_WARN(FI_LOG_EP_CTRL, "Address vector doesn't belong to same domain as EP.\n"); @@ -20,6 +29,7 @@ int efa_base_ep_bind_av(struct efa_base_ep *base_ep, struct efa_av *av) } base_ep->av = av; + base_ep->av->base_ep = base_ep; return 0; } diff --git a/prov/efa/src/efa_errno.h b/prov/efa/src/efa_errno.h index 5d3769d32d6..029c35d4a07 100644 --- a/prov/efa/src/efa_errno.h +++ b/prov/efa/src/efa_errno.h @@ -107,8 +107,7 @@ _(4123, WRITE_SHM_CQ_ENTRY, Failure to write CQ entry for SHM operation) \ _(4124, ESTABLISHED_RECV_UNRESP, Unresponsive receiver (connection previously established)) \ _(4125, INVALID_PKT_TYPE_ZCPY_RX, Invalid packet type received when zero copy recv mode is ON) \ - _(4126, UNESTABLISHED_RECV_UNRESP, Unresponsive receiver (reachable by EFA device but handshake failed)) \ - _(4127, PEER_MAP_ENTRY_POOL_EXHAUSTED, Peer map entry pool exhausted) + _(4126, UNESTABLISHED_RECV_UNRESP, Unresponsive receiver (reachable by EFA device but handshake failed)) /** @} */ diff --git a/prov/efa/src/rdm/efa_rdm_ep.h b/prov/efa/src/rdm/efa_rdm_ep.h index 1b888e182a4..fc298010249 100644 --- a/prov/efa/src/rdm/efa_rdm_ep.h +++ b/prov/efa/src/rdm/efa_rdm_ep.h @@ -40,10 +40,6 @@ struct efa_rdm_ep_queued_copy { #define EFA_RDM_EP_MAX_WR_PER_IBV_POST_SEND (4096) #define EFA_RDM_EP_MAX_WR_PER_IBV_POST_RECV (8192) -struct efa_rdm_peer_map { - struct efa_rdm_peer_map_entry *head; -}; - struct efa_rdm_ep { struct efa_base_ep base_ep; @@ -189,9 +185,6 @@ struct efa_rdm_ep { struct dlist_entry entry; /* the count of opes queued before handshake is made with their peers */ size_t ope_queued_before_handshake_cnt; - - struct ofi_bufpool *peer_map_entry_pool; /* bufpool to hold fi_addr->efa_rdm_peer key-value pairs */ - struct efa_rdm_peer_map fi_addr_to_peer_map; /* Hashmap to find efa_rdm_peer given fi_addr */ }; int efa_rdm_ep_flush_queued_blocking_copy_to_hmem(struct efa_rdm_ep *ep); diff --git a/prov/efa/src/rdm/efa_rdm_ep_fiops.c b/prov/efa/src/rdm/efa_rdm_ep_fiops.c index fbebfd93455..3508fe7ba75 100644 --- a/prov/efa/src/rdm/efa_rdm_ep_fiops.c +++ b/prov/efa/src/rdm/efa_rdm_ep_fiops.c @@ -307,18 +307,7 @@ int efa_rdm_ep_create_buffer_pools(struct efa_rdm_ep *ep) if (ret) goto err_free; - ret = ofi_bufpool_create(&ep->peer_map_entry_pool, - sizeof(struct efa_rdm_peer_map_entry), - EFA_RDM_BUFPOOL_ALIGNMENT, - 0, /* no limit to max cnt */ - /* Don't track usage, because endpoint can be closed without removing entries from AV */ - EFA_MIN_AV_SIZE, OFI_BUFPOOL_NO_TRACK); - if (ret) - goto err_free; - efa_rdm_rxe_map_construct(&ep->rxe_map); - efa_rdm_peer_map_construct(&ep->fi_addr_to_peer_map); - return 0; err_free: @@ -352,9 +341,6 @@ int efa_rdm_ep_create_buffer_pools(struct efa_rdm_ep *ep) if (ep->efa_tx_pkt_pool) ofi_bufpool_destroy(ep->efa_tx_pkt_pool); - if (ep->peer_map_entry_pool) - ofi_bufpool_destroy(ep->peer_map_entry_pool); - return ret; } @@ -842,9 +828,6 @@ static void efa_rdm_ep_destroy_buffer_pools(struct efa_rdm_ep *efa_rdm_ep) if (efa_rdm_ep->rx_atomrsp_pool) ofi_bufpool_destroy(efa_rdm_ep->rx_atomrsp_pool); - - if (efa_rdm_ep->peer_map_entry_pool) - ofi_bufpool_destroy(efa_rdm_ep->peer_map_entry_pool); } /* diff --git a/prov/efa/src/rdm/efa_rdm_ep_utils.c b/prov/efa/src/rdm/efa_rdm_ep_utils.c index 2d87b48911d..9cf297acfc2 100644 --- a/prov/efa/src/rdm/efa_rdm_ep_utils.c +++ b/prov/efa/src/rdm/efa_rdm_ep_utils.c @@ -56,26 +56,14 @@ struct efa_rdm_peer *efa_rdm_ep_get_peer(struct efa_rdm_ep *ep, fi_addr_t addr) { struct util_av_entry *util_av_entry; struct efa_av_entry *av_entry; - struct efa_rdm_peer *peer; if (OFI_UNLIKELY(addr == FI_ADDR_NOTAVAIL)) return NULL; - peer = efa_rdm_peer_map_lookup(&ep->fi_addr_to_peer_map, addr); - if (peer) - return peer; - util_av_entry = ofi_bufpool_get_ibuf(ep->base_ep.util_ep.av->av_entry_pool, addr); av_entry = (struct efa_av_entry *)util_av_entry->data; - - if (av_entry->conn.ep_addr) { - peer = efa_rdm_peer_map_insert(&ep->fi_addr_to_peer_map, addr, ep); - efa_rdm_peer_construct(peer, ep, &av_entry->conn); - return peer; - } - - return NULL; + return av_entry->conn.ep_addr ? &av_entry->conn.rdm_peer : NULL; } /** diff --git a/prov/efa/src/rdm/efa_rdm_peer.c b/prov/efa/src/rdm/efa_rdm_peer.c index 7c82c943835..3e8e3dff774 100644 --- a/prov/efa/src/rdm/efa_rdm_peer.c +++ b/prov/efa/src/rdm/efa_rdm_peer.c @@ -31,11 +31,6 @@ void efa_rdm_peer_construct(struct efa_rdm_peer *peer, struct efa_rdm_ep *ep, st dlist_init(&peer->txe_list); dlist_init(&peer->rxe_list); dlist_init(&peer->overflow_pke_list); - - if (conn->shm_fi_addr != FI_ADDR_NOTAVAIL) { - peer->shm_fiaddr = conn->shm_fi_addr; - peer->is_local = 1; - } } /** @@ -116,41 +111,6 @@ void efa_rdm_peer_destruct(struct efa_rdm_peer *peer, struct efa_rdm_ep *ep) #endif } -struct efa_rdm_peer *efa_rdm_peer_map_insert(struct efa_rdm_peer_map *peer_map, fi_addr_t addr, struct efa_rdm_ep *ep) { - struct efa_rdm_peer_map_entry *map_entry; - struct efa_rdm_peer *peer; - - map_entry = ofi_buf_alloc(ep->peer_map_entry_pool); - if (OFI_UNLIKELY(!map_entry)) { - EFA_WARN(FI_LOG_CQ, - "Map entries for EFA AV to peer mapping exhausted.\n"); - efa_base_ep_write_eq_error(&ep->base_ep, FI_ENOBUFS, FI_EFA_ERR_PEER_MAP_ENTRY_POOL_EXHAUSTED); - return NULL; - } - - map_entry->key = addr; - peer = &map_entry->efa_rdm_peer; - - HASH_ADD(hh, peer_map->head, key, sizeof(addr), map_entry); - - return peer; -} - -struct efa_rdm_peer *efa_rdm_peer_map_lookup(struct efa_rdm_peer_map *peer_map, fi_addr_t addr) { - struct efa_rdm_peer_map_entry *map_entry; - - HASH_FIND(hh, peer_map->head, &addr, sizeof(addr), map_entry); - return map_entry ? &map_entry->efa_rdm_peer : NULL; -} - -void efa_rdm_peer_map_remove(struct efa_rdm_peer_map *peer_map, fi_addr_t addr, struct efa_rdm_peer *peer) { - struct efa_rdm_peer_map_entry *map_entry; - - HASH_FIND(hh, peer_map->head, &addr, sizeof(addr), map_entry); - HASH_DEL(peer_map->head, map_entry); - ofi_buf_free(map_entry); -} - /** * @brief run incoming packet_entry through reorder buffer * queue the packet entry if msg_id is larger than expected. diff --git a/prov/efa/src/rdm/efa_rdm_peer.h b/prov/efa/src/rdm/efa_rdm_peer.h index 21585051921..fe2f79ead61 100644 --- a/prov/efa/src/rdm/efa_rdm_peer.h +++ b/prov/efa/src/rdm/efa_rdm_peer.h @@ -75,12 +75,6 @@ struct efa_rdm_peer { struct efa_rdm_peer_user_recv_qp user_recv_qp; }; -struct efa_rdm_peer_map_entry { - uint64_t key; - struct efa_rdm_peer efa_rdm_peer; - UT_hash_handle hh; -}; - /** * @brief check for peer's RDMA_READ support, assuming HANDSHAKE has already occurred * @@ -292,12 +286,6 @@ bool efa_both_support_zero_hdr_data_transfer(struct efa_rdm_ep *ep, struct efa_r (peer->extra_info[0] & EFA_RDM_EXTRA_FEATURE_REQUEST_USER_RECV_QP)); } -static inline -void efa_rdm_peer_map_construct(struct efa_rdm_peer_map *peer_map) -{ - peer_map->head = NULL; -} - struct efa_conn; void efa_rdm_peer_construct(struct efa_rdm_peer *peer, struct efa_rdm_ep *ep, struct efa_conn *conn); @@ -316,10 +304,4 @@ size_t efa_rdm_peer_get_runt_size(struct efa_rdm_peer *peer, struct efa_rdm_ep * int efa_rdm_peer_select_readbase_rtm(struct efa_rdm_peer *peer, struct efa_rdm_ep *ep, struct efa_rdm_ope *ope); -struct efa_rdm_peer *efa_rdm_peer_map_insert(struct efa_rdm_peer_map *peer_map, fi_addr_t addr, struct efa_rdm_ep *ep); - -struct efa_rdm_peer *efa_rdm_peer_map_lookup(struct efa_rdm_peer_map *peer_map, fi_addr_t addr); - -void efa_rdm_peer_map_remove(struct efa_rdm_peer_map *peer_map, fi_addr_t addr, struct efa_rdm_peer *peer); - #endif /* EFA_RDM_PEER_H */ diff --git a/prov/efa/test/efa_unit_test_av.c b/prov/efa/test/efa_unit_test_av.c index 6e11ee5c177..9ca730d0b6e 100644 --- a/prov/efa/test/efa_unit_test_av.c +++ b/prov/efa/test/efa_unit_test_av.c @@ -74,39 +74,3 @@ void test_av_insert_duplicate_gid(struct efa_resource **state) assert_int_equal(num_addr, 1); assert_int_not_equal(addr1, addr2); } - -/** - * @brief This test verifies that multiple endpoints can bind to the same AV - * - * @param[in] state struct efa_resource that is managed by the framework - */ -void test_av_multiple_ep(struct efa_resource **state) -{ - struct efa_resource *resource = *state; - struct fid_ep *ep2, *ep3; - int ret; - - /* Resource construct function creates and binds 1 EP to the AV */ - efa_unit_test_resource_construct(resource, FI_EP_RDM); - - /* Create and bind two new endpoints to the same AV */ - fi_endpoint(resource->domain, resource->info, &ep2, NULL); - ret = fi_ep_bind(ep2, &resource->av->fid, 0); - assert_int_equal(ret, 0); - - fi_endpoint(resource->domain, resource->info, &ep3, NULL); - ret = fi_ep_bind(ep3, &resource->av->fid, 0); - assert_int_equal(ret, 0); - - /* Bind the two new endpoints to the same CQ and enable them */ - fi_ep_bind(ep2, &resource->cq->fid, FI_SEND | FI_RECV); - ret = fi_enable(ep2); - assert_int_equal(ret, 0); - - fi_ep_bind(ep3, &resource->cq->fid, FI_SEND | FI_RECV); - ret = fi_enable(ep3); - assert_int_equal(ret, 0); - - fi_close(&ep2->fid); - fi_close(&ep3->fid); -} diff --git a/prov/efa/test/efa_unit_tests.c b/prov/efa/test/efa_unit_tests.c index 293e080c0dd..63316838a21 100644 --- a/prov/efa/test/efa_unit_tests.c +++ b/prov/efa/test/efa_unit_tests.c @@ -80,7 +80,6 @@ int main(void) const struct CMUnitTest efa_unit_tests[] = { cmocka_unit_test_setup_teardown(test_av_insert_duplicate_raw_addr, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), cmocka_unit_test_setup_teardown(test_av_insert_duplicate_gid, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), - cmocka_unit_test_setup_teardown(test_av_multiple_ep, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), cmocka_unit_test_setup_teardown(test_efa_device_construct_error_handling, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), cmocka_unit_test_setup_teardown(test_efa_rdm_ep_ignore_missing_host_id_file, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), cmocka_unit_test_setup_teardown(test_efa_rdm_ep_has_valid_host_id, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), diff --git a/prov/efa/test/efa_unit_tests.h b/prov/efa/test/efa_unit_tests.h index 689fd4fa3a8..a13033e6f8b 100644 --- a/prov/efa/test/efa_unit_tests.h +++ b/prov/efa/test/efa_unit_tests.h @@ -40,8 +40,6 @@ void efa_unit_test_resource_construct_ep_not_enabled( struct efa_resource *resource, enum fi_ep_type ep_type); void efa_unit_test_resource_construct_no_cq_and_ep_not_enabled( struct efa_resource *resource, enum fi_ep_type ep_type); -void efa_unit_test_resource_construct_no_av_no_cq_and_ep_not_enabled( - struct efa_resource *resource, enum fi_ep_type ep_type); void efa_unit_test_resource_construct_with_hints(struct efa_resource *resource, enum fi_ep_type ep_type, uint32_t fi_version, struct fi_info *hints, @@ -102,7 +100,6 @@ void efa_unit_test_handshake_pkt_construct(struct efa_rdm_pke *pkt_entry, struct /* test cases */ void test_av_insert_duplicate_raw_addr(); void test_av_insert_duplicate_gid(); -void test_av_multiple_ep(); void test_efa_device_construct_error_handling(); void test_efa_rdm_ep_ignore_missing_host_id_file(); void test_efa_rdm_ep_has_valid_host_id();