From d8e14044d786a256630362d18fd25d830a4ddcf8 Mon Sep 17 00:00:00 2001 From: Shi Jin Date: Fri, 19 Sep 2025 22:28:05 +0000 Subject: [PATCH] prov/efa: Check rdma iov limit in data path direct Today data path direct doesn't check the rdma iov count, while it only supports 1 iov (due to the hard-coded data structures and efa device io defs). This patch adds the check to avoid unexpected crashing. Signed-off-by: Shi Jin --- prov/efa/Makefile.include | 3 +- prov/efa/src/efa.h | 9 +++ prov/efa/src/efa_data_path_direct_entry.h | 5 ++ .../efa/test/efa_unit_test_data_path_direct.c | 68 +++++++++++++++++++ prov/efa/test/efa_unit_tests.c | 2 + prov/efa/test/efa_unit_tests.h | 5 ++ 6 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 prov/efa/test/efa_unit_test_data_path_direct.c diff --git a/prov/efa/Makefile.include b/prov/efa/Makefile.include index a6c8cc142a2..40853f7122e 100644 --- a/prov/efa/Makefile.include +++ b/prov/efa/Makefile.include @@ -163,7 +163,8 @@ nodist_prov_efa_test_efa_unit_test_SOURCES = \ prov/efa/test/efa_unit_test_rdm_peer.c \ prov/efa/test/efa_unit_test_pke.c \ prov/efa/test/efa_unit_test_msg.c \ - prov/efa/test/efa_unit_test_rma.c + prov/efa/test/efa_unit_test_rma.c \ + prov/efa/test/efa_unit_test_data_path_direct.c efa_CPPFLAGS += -I$(top_srcdir)/include -I$(top_srcdir)/prov/efa/test $(cmocka_CPPFLAGS) diff --git a/prov/efa/src/efa.h b/prov/efa/src/efa.h index a71b9fde460..f936d8fb084 100644 --- a/prov/efa/src/efa.h +++ b/prov/efa/src/efa.h @@ -112,6 +112,15 @@ */ #define EFA_RDM_BUFPOOL_ALIGNMENT (64) + +/** + * The hard-coded rdma sge num limit + * due to the structs layout rdma wqes. + * See `struct efa_io_rdma_req` in efa_io_defs.h + */ +#define EFA_DEVICE_MAX_RDMA_SGE 1 + + struct efa_fabric { struct util_fabric util_fabric; struct fid_fabric *shm_fabric; diff --git a/prov/efa/src/efa_data_path_direct_entry.h b/prov/efa/src/efa_data_path_direct_entry.h index 6fde4f933cb..e03a1ee284a 100644 --- a/prov/efa/src/efa_data_path_direct_entry.h +++ b/prov/efa/src/efa_data_path_direct_entry.h @@ -318,6 +318,11 @@ static inline void efa_data_path_direct_wr_set_sge_list(struct efa_qp *efa_qp, break; case EFA_IO_RDMA_READ: case EFA_IO_RDMA_WRITE: + if (OFI_UNLIKELY(num_sge > EFA_DEVICE_MAX_RDMA_SGE)) { + EFA_WARN(FI_LOG_EP_DATA, "EFA device doesn't support > %d iov for rdma operations\n", EFA_DEVICE_MAX_RDMA_SGE); + qp->wr_session_err = EINVAL; + return; + } rdma_req = &tx_wqe->data.rdma_req; rdma_req->remote_mem.length = efa_sge_total_bytes(sg_list, num_sge); diff --git a/prov/efa/test/efa_unit_test_data_path_direct.c b/prov/efa/test/efa_unit_test_data_path_direct.c new file mode 100644 index 00000000000..7890db3c2cf --- /dev/null +++ b/prov/efa/test/efa_unit_test_data_path_direct.c @@ -0,0 +1,68 @@ +/* SPDX-License-Identifier: BSD-2-Clause OR GPL-2.0-only */ +/* SPDX-FileCopyrightText: Copyright Amazon.com, Inc. or its affiliates. All + * rights reserved. */ + +#include "efa_unit_tests.h" +#include "ofi_util.h" +#include "efa_io_defs.h" +#include "efa_data_path_direct_entry.h" + +static void test_efa_data_path_direct_multiple_sge_fail_impl(struct efa_resource **state, uint32_t fi_opcode) +{ +#if HAVE_EFA_DATA_PATH_DIRECT + struct efa_resource *resource = *state; + struct efa_unit_test_buff local_buff1, local_buff2; + struct ibv_sge sge_list[2]; + struct efa_qp *qp; + struct efa_cq *efa_cq; + bool data_path_direct_enabled_orig; + struct fid_ep *ep; + + efa_unit_test_resource_construct(resource, FI_EP_RDM, EFA_DIRECT_FABRIC_NAME); + efa_cq = container_of(resource->cq, struct efa_cq, util_cq.cq_fid); + data_path_direct_enabled_orig = efa_cq->ibv_cq.data_path_direct_enabled; + efa_cq->ibv_cq.data_path_direct_enabled = true; + + /* Open a test ep with data path direct enabled */ + assert_int_equal(fi_endpoint(resource->domain, resource->info, &ep, NULL), 0); + assert_int_equal(fi_ep_bind(ep, &resource->cq->fid, FI_SEND | FI_RECV), 0); + assert_int_equal(fi_enable(ep), 0); + + qp = container_of(ep, struct efa_base_ep, util_ep.ep_fid)->qp; + efa_unit_test_buff_construct(&local_buff1, resource, 2048); + efa_unit_test_buff_construct(&local_buff2, resource, 2048); + + /* Setup SGE list with 2 elements (exceeds EFA_DEVICE_MAX_RDMA_SGE=1) */ + sge_list[0].addr = (uintptr_t)local_buff1.buff; + sge_list[0].length = local_buff1.size; + sge_list[0].lkey = ((struct efa_mr *)fi_mr_desc(local_buff1.mr))->ibv_mr->lkey; + sge_list[1].addr = (uintptr_t)local_buff2.buff; + sge_list[1].length = local_buff2.size; + sge_list[1].lkey = ((struct efa_mr *)fi_mr_desc(local_buff2.mr))->ibv_mr->lkey; + + /* Start RDMA operation and test multiple SGE failure */ + if (fi_opcode == FI_READ) + efa_data_path_direct_wr_rdma_read(qp, 123456, 0x87654321); + else + efa_data_path_direct_wr_rdma_write(qp, 123456, 0x87654321); + efa_data_path_direct_wr_set_sge_list(qp, 2, sge_list); + assert_int_equal(qp->data_path_direct_qp.wr_session_err, EINVAL); + + assert_int_equal(fi_close(&ep->fid), 0); + efa_cq->ibv_cq.data_path_direct_enabled = data_path_direct_enabled_orig; + efa_unit_test_buff_destruct(&local_buff1); + efa_unit_test_buff_destruct(&local_buff2); +#else + skip(); +#endif +} + +void test_efa_data_path_direct_rdma_read_multiple_sge_fail(struct efa_resource **state) +{ + test_efa_data_path_direct_multiple_sge_fail_impl(state, FI_READ); +} + +void test_efa_data_path_direct_rdma_write_multiple_sge_fail(struct efa_resource **state) +{ + test_efa_data_path_direct_multiple_sge_fail_impl(state, FI_WRITE); +} \ No newline at end of file diff --git a/prov/efa/test/efa_unit_tests.c b/prov/efa/test/efa_unit_tests.c index 70e0792fcb1..e005adeda22 100644 --- a/prov/efa/test/efa_unit_tests.c +++ b/prov/efa/test/efa_unit_tests.c @@ -370,6 +370,8 @@ int main(void) cmocka_unit_test_setup_teardown(test_efa_rma_inject_write, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), cmocka_unit_test_setup_teardown(test_efa_rma_inject_writedata, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), cmocka_unit_test_setup_teardown(test_efa_rma_writemsg_with_inject, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), + cmocka_unit_test_setup_teardown(test_efa_data_path_direct_rdma_read_multiple_sge_fail, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), + cmocka_unit_test_setup_teardown(test_efa_data_path_direct_rdma_write_multiple_sge_fail, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), cmocka_unit_test_setup_teardown(test_efa_cq_read_no_completion, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), cmocka_unit_test_setup_teardown(test_efa_cq_read_send_success, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown), cmocka_unit_test_setup_teardown(test_efa_cq_read_senddata_success, 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 4ff878fd884..f36b3347b4b 100644 --- a/prov/efa/test/efa_unit_tests.h +++ b/prov/efa/test/efa_unit_tests.h @@ -374,6 +374,11 @@ void test_efa_rdm_ep_data_path_direct_disabled(); void test_efa_ep_lock_type_no_op(); void test_efa_ep_lock_type_mutex(); +/* begin efa_unit_test_data_path_direct.c */ +void test_efa_data_path_direct_rdma_read_multiple_sge_fail(); +void test_efa_data_path_direct_rdma_write_multiple_sge_fail(); +/* end efa_unit_test_data_path_direct.c */ + void test_efa_cntr_ibv_cq_poll_list_same_tx_rx_cq_single_ep(); void test_efa_cntr_ibv_cq_poll_list_separate_tx_rx_cq_single_ep();