From 37f4902cda5e45f4ed59a64f689c9f8abce366a2 Mon Sep 17 00:00:00 2001 From: Sai Sunku Date: Mon, 24 Feb 2025 23:23:02 +0000 Subject: [PATCH] prov/efa: Remove efa_av->ep_type in favor of efa_domain->info_type efa_av->ep_type was used to distinguish between the RDM path and the DGRAM path. The efa-direct path has FI_EP_RDM ep type but it does not require a efa_conn->rdm_peer. But the efa-direct path still uses unique connid based on timestamp. Use efa_domain->info_type to distinguish the three code paths. Signed-off-by: Sai Sunku --- prov/efa/src/efa_av.c | 22 ++++++++----------- prov/efa/src/efa_av.h | 1 - prov/efa/test/efa_unit_test_av.c | 32 ---------------------------- prov/efa/test/efa_unit_test_domain.c | 30 ++++++++++++++++++++++++++ prov/efa/test/efa_unit_tests.c | 8 +++++-- prov/efa/test/efa_unit_tests.h | 8 +++++-- 6 files changed, 51 insertions(+), 50 deletions(-) diff --git a/prov/efa/src/efa_av.c b/prov/efa/src/efa_av.c index df2361c3928..75caac754bd 100644 --- a/prov/efa/src/efa_av.c +++ b/prov/efa/src/efa_av.c @@ -265,7 +265,7 @@ int efa_conn_rdm_init(struct efa_av *av, struct efa_conn *conn, bool insert_shm_ struct efa_rdm_ep *efa_rdm_ep; struct efa_rdm_peer *peer; - assert(av->ep_type == FI_EP_RDM); + assert(av->domain->info_type == EFA_INFO_RDM); assert(conn->ep_addr); /* currently multiple EP bind to same av is not supported */ @@ -347,7 +347,7 @@ void efa_conn_rdm_deinit(struct efa_av *av, struct efa_conn *conn) struct efa_rdm_peer *peer; struct efa_rdm_ep *ep; - assert(av->ep_type == FI_EP_RDM); + assert(av->domain->info_type == EFA_INFO_RDM); peer = &conn->rdm_peer; if (peer->is_local && av->shm_rdm_av) { @@ -408,7 +408,7 @@ int efa_av_update_reverse_av(struct efa_av *av, struct efa_ep_addr *raw_addr, /* We used a static connid for all dgram endpoints, therefore cur_entry should always be NULL, * and only RDM endpoint can reach here. hence the following assertion */ - assert(av->ep_type == FI_EP_RDM); + assert(av->domain->info_type == EFA_INFO_RDM); prv_entry = malloc(sizeof(*prv_entry)); if (!prv_entry) { EFA_WARN(FI_LOG_AV, "Cannot allocate memory for prv_reverse_av entry\n"); @@ -478,7 +478,7 @@ struct efa_conn *efa_conn_alloc(struct efa_av *av, struct efa_ep_addr *raw_addr, if (!conn->ah) goto err_release; - if (av->ep_type == FI_EP_RDM) { + if (av->domain->info_type == EFA_INFO_RDM) { err = efa_conn_rdm_init(av, conn, insert_shm_av); if (err) { errno = -err; @@ -488,7 +488,7 @@ struct efa_conn *efa_conn_alloc(struct efa_av *av, struct efa_ep_addr *raw_addr, err = efa_av_update_reverse_av(av, raw_addr, conn); if (err) { - if (av->ep_type == FI_EP_RDM) + if (av->domain->info_type == EFA_INFO_RDM) efa_conn_rdm_deinit(av, conn); goto err_release; } @@ -546,7 +546,7 @@ void efa_conn_release(struct efa_av *av, struct efa_conn *conn) free(prv_reverse_av_entry); } - if (av->ep_type == FI_EP_RDM) + if (av->domain->info_type == EFA_INFO_RDM) efa_conn_rdm_deinit(av, conn); efa_ah_release(av, conn->ah); @@ -590,7 +590,7 @@ int efa_av_insert_one(struct efa_av *av, struct efa_ep_addr *addr, fi_addr_t efa_fiaddr; int ret = 0; - if (av->ep_type == FI_EP_DGRAM) + if (av->domain->info_type == EFA_INFO_DGRAM) addr->qkey = EFA_DGRAM_CONNID; ofi_genlock_lock(&av->util_av.lock); @@ -811,7 +811,7 @@ static int efa_av_close(struct fid *fid) fi_strerror(err)); } - if (av->ep_type == FI_EP_RDM) { + if (av->domain->info_type == EFA_INFO_RDM) { if (av->shm_rdm_av) { err = fi_close(&av->shm_rdm_av->fid); if (OFI_UNLIKELY(err)) { @@ -904,9 +904,7 @@ int efa_av_open(struct fid_domain *domain_fid, struct fi_av_attr *attr, if (ret) goto err; - if (EFA_EP_TYPE_IS_RDM(efa_domain->info)) { - av->ep_type = FI_EP_RDM; - + if (efa_domain->info_type == EFA_INFO_RDM) { av_attr = *attr; if (efa_domain->fabric && efa_domain->fabric->shm_fabric) { /* @@ -928,8 +926,6 @@ int efa_av_open(struct fid_domain *domain_fid, struct fi_av_attr *attr, if (ret) goto err_close_util_av; } - } else { - av->ep_type = FI_EP_DGRAM; } EFA_INFO(FI_LOG_AV, "fi_av_attr:%" PRId64 "\n", diff --git a/prov/efa/src/efa_av.h b/prov/efa/src/efa_av.h index bd4d4a2d74e..945a644ec76 100644 --- a/prov/efa/src/efa_av.h +++ b/prov/efa/src/efa_av.h @@ -69,7 +69,6 @@ struct efa_av { struct efa_prv_reverse_av *prv_reverse_av; struct efa_ah *ah_map; struct util_av util_av; - enum fi_ep_type ep_type; }; int efa_av_open(struct fid_domain *domain_fid, struct fi_av_attr *attr, diff --git a/prov/efa/test/efa_unit_test_av.c b/prov/efa/test/efa_unit_test_av.c index 362b83c8207..3d925c9e354 100644 --- a/prov/efa/test/efa_unit_test_av.c +++ b/prov/efa/test/efa_unit_test_av.c @@ -4,38 +4,6 @@ #include "efa_unit_tests.h" #include "efa_av.h" -/** - * @brief Verify the ep type in struct efa_av for efa RDM path - * - * @param[in] state struct efa_resource that is managed by the framework - */ -void test_av_ep_type_efa_rdm(struct efa_resource **state) -{ - struct efa_resource *resource = *state; - struct efa_av *efa_av; - - efa_unit_test_resource_construct(resource, FI_EP_RDM, EFA_FABRIC_NAME); - g_efa_unit_test_mocks.ibv_create_ah = &efa_mock_ibv_create_ah_check_mock; - efa_av = container_of(resource->av, struct efa_av, util_av.av_fid); - assert(efa_av->ep_type == FI_EP_RDM); -} - -/** - * @brief Verify the ep type in struct efa_av for efa direct path - * - * @param[in] state struct efa_resource that is managed by the framework - */ -void test_av_ep_type_efa_direct(struct efa_resource **state) -{ - struct efa_resource *resource = *state; - struct efa_av *efa_av; - - efa_unit_test_resource_construct(resource, FI_EP_RDM, EFA_DIRECT_FABRIC_NAME); - g_efa_unit_test_mocks.ibv_create_ah = &efa_mock_ibv_create_ah_check_mock; - efa_av = container_of(resource->av, struct efa_av, util_av.av_fid); - assert(efa_av->ep_type == FI_EP_RDM); -} - /** * @brief Only works on nodes with EFA devices * This test calls fi_av_insert() twice with the same raw address, diff --git a/prov/efa/test/efa_unit_test_domain.c b/prov/efa/test/efa_unit_test_domain.c index 5152c63ca27..14659642f1a 100644 --- a/prov/efa/test/efa_unit_test_domain.c +++ b/prov/efa/test/efa_unit_test_domain.c @@ -3,6 +3,36 @@ #include "efa_unit_tests.h" +/** + * @brief Verify the info type in struct efa_domain for efa RDM path + * + * @param[in] state struct efa_resource that is managed by the framework + */ +void test_efa_domain_info_type_efa_rdm(struct efa_resource **state) +{ + struct efa_resource *resource = *state; + struct efa_domain *efa_domain; + + efa_unit_test_resource_construct(resource, FI_EP_RDM, EFA_FABRIC_NAME); + efa_domain = container_of(resource->domain, struct efa_domain, util_domain.domain_fid); + assert(efa_domain->info_type == EFA_INFO_RDM); +} + +/** + * @brief Verify the info type in struct efa_domain for efa direct path + * + * @param[in] state struct efa_resource that is managed by the framework + */ +void test_efa_domain_info_type_efa_direct(struct efa_resource **state) +{ + struct efa_resource *resource = *state; + struct efa_domain *efa_domain; + + efa_unit_test_resource_construct(resource, FI_EP_RDM, EFA_DIRECT_FABRIC_NAME); + efa_domain = container_of(resource->domain, struct efa_domain, util_domain.domain_fid); + assert(efa_domain->info_type == EFA_INFO_DIRECT); +} + /* test fi_open_ops with a wrong name */ void test_efa_domain_open_ops_wrong_name(struct efa_resource **state) { diff --git a/prov/efa/test/efa_unit_tests.c b/prov/efa/test/efa_unit_tests.c index 52dafdf4d8b..f445bf26aea 100644 --- a/prov/efa/test/efa_unit_tests.c +++ b/prov/efa/test/efa_unit_tests.c @@ -90,8 +90,6 @@ int main(void) /* Requires an EFA device to work */ const struct CMUnitTest efa_unit_tests[] = { /* begin efa_unit_test_av.c */ - cmocka_unit_test_setup_teardown(test_av_ep_type_efa_rdm, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), - cmocka_unit_test_setup_teardown(test_av_ep_type_efa_direct, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), 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), /* end efa_unit_test_av.c */ @@ -234,8 +232,14 @@ int main(void) cmocka_unit_test_setup_teardown(test_efa_rdm_pke_alloc_rta_rxe, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), cmocka_unit_test_setup_teardown(test_efa_rdm_pke_alloc_rtw_rxe, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), cmocka_unit_test_setup_teardown(test_efa_rdm_pke_alloc_rtr_rxe, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), + + /* begin efa_unit_test_domain.c */ + cmocka_unit_test_setup_teardown(test_efa_domain_info_type_efa_direct, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), + cmocka_unit_test_setup_teardown(test_efa_domain_info_type_efa_rdm, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), cmocka_unit_test_setup_teardown(test_efa_domain_open_ops_wrong_name, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), cmocka_unit_test_setup_teardown(test_efa_domain_open_ops_mr_query, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), + /* end efa_unit_test_domain.c */ + cmocka_unit_test_setup_teardown(test_efa_rdm_cq_ibv_cq_poll_list_same_tx_rx_cq_single_ep, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), cmocka_unit_test_setup_teardown(test_efa_rdm_cq_ibv_cq_poll_list_separate_tx_rx_cq_single_ep, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), cmocka_unit_test_setup_teardown(test_efa_rdm_cq_post_initial_rx_pkts, 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 914487b0499..de736760e81 100644 --- a/prov/efa/test/efa_unit_tests.h +++ b/prov/efa/test/efa_unit_tests.h @@ -105,8 +105,6 @@ struct efa_rdm_ope *efa_unit_test_alloc_rxe(struct efa_resource *resource, uint3 /* test cases */ /* begin efa_unit_test_av.c */ -void test_av_ep_type_efa_rdm(); -void test_av_ep_type_efa_direct(); void test_av_insert_duplicate_raw_addr(); void test_av_insert_duplicate_gid(); /* end efa_unit_test_av.c */ @@ -245,8 +243,14 @@ void test_efa_rdm_peer_get_runt_size_cuda_memory_exceeding_total_len_128_alignme void test_efa_rdm_peer_select_readbase_rtm_no_runt(); void test_efa_rdm_peer_select_readbase_rtm_do_runt(); void test_efa_rdm_pke_get_available_copy_methods_align128(); + +/* begin efa_unit_test_domain.c */ +void test_efa_domain_info_type_efa_direct(); +void test_efa_domain_info_type_efa_rdm(); void test_efa_domain_open_ops_wrong_name(); void test_efa_domain_open_ops_mr_query(); +/* end efa_unit_test_domain.c */ + void test_efa_rdm_cq_ibv_cq_poll_list_same_tx_rx_cq_single_ep(); void test_efa_rdm_cq_ibv_cq_poll_list_separate_tx_rx_cq_single_ep(); void test_efa_rdm_cq_post_initial_rx_pkts();