prov/efa: Fix the max_msg_size reporting for efa-direct

Currently, efa-direct always return the max of rdma size and mtu size when device rdma is supported. This is wrong because when FI_RMA is not requested, only send/recv will be used, and the max_msg_size should be returned as the mtu size only. This patch fixes this issue by moving the max_msg_size change to alter_direct_info based on hints->caps. Also adjust and introduces more unit tests. Signed-off-by: Shi Jin <sjina@amazon.com>
ofiwg · Feb 20, 2025 · f3e26d6 · f3e26d6
1 parent a07fe39
commit f3e26d6
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 18 deletions.
diff --git a/prov/efa/src/efa_prov_info.c b/prov/efa/src/efa_prov_info.c
@@ -153,14 +153,8 @@ void efa_prov_info_set_ep_attr(struct fi_info *prov_info,
 	prov_info->ep_attr->max_msg_size = device->ibv_port_attr.max_msg_sz;
 	prov_info->ep_attr->type = ep_type;
 
-	if (ep_type == FI_EP_RDM) {
-		/* ep_attr->max_msg_size is the maximum of both MSG and RMA operations */
-		if (prov_info->caps & FI_RMA)
-			prov_info->ep_attr->max_msg_size = MAX(device->ibv_port_attr.max_msg_sz, device->max_rdma_size);
-	} else {
-		assert(ep_type == FI_EP_DGRAM);
+	if (ep_type == FI_EP_DGRAM)
 		prov_info->ep_attr->msg_prefix_size = 40;
-	}
 }
 
 /**

diff --git a/prov/efa/src/efa_user_info.c b/prov/efa/src/efa_user_info.c
@@ -380,6 +380,11 @@ int efa_user_info_alter_direct(int version, struct fi_info *info, const struct f
 			EFA_INFO(FI_LOG_CORE,
 				"FI_MSG_PREFIX size = %ld\n", info->ep_attr->msg_prefix_size);
 		}
+		/* When user requests FI_RMA and it's supported, the max_msg_size should be returned
+		 * as the maximum of both MSG and RMA operations
+		 */
+		if (hints->caps & FI_RMA)
+			info->ep_attr->max_msg_size = MAX(g_device_list[0].ibv_port_attr.max_msg_sz, g_device_list[0].max_rdma_size);
 	}
 
 	/* Print a warning and use FI_AV_TABLE if the app requests FI_AV_MAP */

diff --git a/prov/efa/test/efa_unit_test_info.c b/prov/efa/test/efa_unit_test_info.c
@@ -96,24 +96,62 @@ void test_info_dgram_attributes()
 /**
  * @brief Verify that efa direct path fi_info objects have some expected values
  */
-void test_info_direct_attributes()
+static void test_info_direct_attributes_impl(struct fi_info *hints,
+					     int expected_return)
 {
-	struct fi_info *hints, *info = NULL, *info_head = NULL;
+	struct fi_info *info = NULL, *info_head = NULL;
 	int err;
 
-	hints = efa_unit_test_alloc_hints(FI_EP_RDM, EFA_DIRECT_FABRIC_NAME);
-	assert_non_null(hints);
+	err = fi_getinfo(FI_VERSION(1, 6), NULL, NULL, 0, hints, &info_head);
+	assert_int_equal(err, expected_return);
+	if (expected_return)
+		return;
 
-	err = fi_getinfo(FI_VERSION(1,6), NULL, NULL, 0, hints, &info);
-	assert_int_equal(err, 0);
-	assert_non_null(info);
+	assert_non_null(info_head);
 	for (info = info_head; info; info = info->next) {
-		assert_true(!strcmp(info->fabric_attr->name, EFA_DIRECT_FABRIC_NAME));
+		assert_true(!strcmp(info->fabric_attr->name,
+				    EFA_DIRECT_FABRIC_NAME));
 		assert_true(strstr(info->domain_attr->name, "rdm"));
 		assert_false(info->caps & (FI_ATOMIC | FI_TAGGED));
 		assert_false(info->tx_attr->msg_order & FI_ORDER_SAS);
-		assert_int_equal(info->ep_attr->max_msg_size, g_device_list[0].max_rdma_size);
+		assert_int_equal(
+			info->ep_attr->max_msg_size,
+			(hints->caps & FI_RMA) ?
+				g_device_list[0].max_rdma_size :
+				g_device_list[0].ibv_port_attr.max_msg_sz);
 	}
+
+	fi_freeinfo(info_head);
+}
+
+void test_info_direct_attributes_no_rma()
+{
+	struct fi_info *hints;
+
+	hints = efa_unit_test_alloc_hints(FI_EP_RDM, EFA_DIRECT_FABRIC_NAME);
+	assert_non_null(hints);
+
+	/* The default hints for efa-direct only has FI_MSG cap, so it should
+	 * succeed anyway */
+	test_info_direct_attributes_impl(hints, FI_SUCCESS);
+	fi_freeinfo(hints);
+}
+
+void test_info_direct_attributes_rma()
+{
+	struct fi_info *hints;
+	bool support_fi_rma = (efa_device_support_rdma_read() &&
+			       efa_device_support_rdma_write() &&
+			       efa_device_support_unsolicited_write_recv());
+
+	int expected_return = support_fi_rma ? FI_SUCCESS : -FI_ENODATA;
+
+	hints = efa_unit_test_alloc_hints(FI_EP_RDM, EFA_DIRECT_FABRIC_NAME);
+	assert_non_null(hints);
+
+	hints->caps |= FI_RMA;
+	test_info_direct_attributes_impl(hints, expected_return);
+	fi_freeinfo(hints);
 }
 
 /**

diff --git a/prov/efa/test/efa_unit_tests.c b/prov/efa/test/efa_unit_tests.c
@@ -156,7 +156,8 @@ int main(void)
 		cmocka_unit_test_setup_teardown(test_info_open_ep_with_wrong_info, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
 		cmocka_unit_test_setup_teardown(test_info_rdm_attributes, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
 		cmocka_unit_test_setup_teardown(test_info_dgram_attributes, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
-		cmocka_unit_test_setup_teardown(test_info_direct_attributes, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
+		cmocka_unit_test_setup_teardown(test_info_direct_attributes_no_rma, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
+		cmocka_unit_test_setup_teardown(test_info_direct_attributes_rma, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
 		cmocka_unit_test_setup_teardown(test_info_direct_hmem_support_p2p, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
 		cmocka_unit_test_setup_teardown(test_info_tx_rx_msg_order_rdm_order_none, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
 		cmocka_unit_test_setup_teardown(test_info_tx_rx_msg_order_rdm_order_sas, 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
@@ -171,7 +171,8 @@ void test_ibv_cq_ex_read_ignore_removed_peer();
 void test_info_open_ep_with_wrong_info();
 void test_info_rdm_attributes();
 void test_info_dgram_attributes();
-void test_info_direct_attributes();
+void test_info_direct_attributes_no_rma();
+void test_info_direct_attributes_rma();
 void test_info_direct_hmem_support_p2p();
 void test_info_tx_rx_msg_order_rdm_order_none();
 void test_info_tx_rx_msg_order_rdm_order_sas();