Skip to content

Commit da794db

Browse files
shijin-awsshijin-aws
committed
prov/efa: Fail early in rma when iov count exceeds device support
efa-direct offloads the rma request to rdma wqes in efa device directly. But today efa device doesn't support > 1 iov (queried as ibv_device_attr.max_sge_rd). This patch makes efa_post_read and efa_post_write fail early with clear error message for this scenario to avoid unexpected crashing. Signed-off-by: Shi Jin <[email protected]>
1 parent 46d451b commit da794db

File tree

5 files changed

+114
-2
lines changed

5 files changed

+114
-2
lines changed

‎prov/efa/src/efa.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,15 @@
112112
*/
113113
#define EFA_RDM_BUFPOOL_ALIGNMENT (64)
114114

115+
116+
/**
117+
* The hard-coded rdma sge num limit
118+
* due to the structs layout rdma wqes.
119+
* See `struct efa_io_rdma_req` in efa_io_defs.h
120+
*/
121+
#define EFA_DEVICE_MAX_RDMA_SGE 1
122+
123+
115124
struct efa_fabric {
116125
struct util_fabric util_fabric;
117126
struct fid_fabric *shm_fabric;

‎prov/efa/src/efa_rma.c‎

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,24 @@ static inline int efa_rma_check_cap(struct efa_base_ep *base_ep) {
2121
return -FI_EOPNOTSUPP;
2222
}
2323

24+
/**
25+
* @brief Check whether the device supports the given iov_count for rdma work request
26+
*
27+
* @param base_ep pointer of efa_base_ep
28+
* @param count the iov count of the msg
29+
* @return int 0 on success, -FI_EOPNOTSUPP on failure
30+
*/
31+
static inline int efa_rma_check_iov_count(struct efa_base_ep *base_ep, size_t count)
32+
{
33+
size_t max_sge = MIN(EFA_DEVICE_MAX_RDMA_SGE, base_ep->domain->device->ibv_attr.max_sge_rd);
34+
35+
if (OFI_UNLIKELY(count > max_sge)) {
36+
EFA_WARN(FI_LOG_EP_DATA, "EFA device currently doesn't support > %zu iov for rdma work request\n", max_sge);
37+
return -FI_EOPNOTSUPP;
38+
}
39+
return 0;
40+
}
41+
2442
/*
2543
* efa_rma_post_read() will post a read request.
2644
*
@@ -56,8 +74,6 @@ static inline ssize_t efa_rma_post_read(struct efa_base_ep *base_ep,
5674
ofi_total_iov_len(msg->msg_iov, msg->iov_count),
5775
msg->addr, (size_t) msg->context, flags);
5876

59-
assert(msg->iov_count > 0 &&
60-
msg->iov_count <= base_ep->domain->info->tx_attr->iov_limit);
6177
assert(msg->rma_iov_count > 0 &&
6278
msg->rma_iov_count <= base_ep->domain->info->tx_attr->rma_iov_limit);
6379
assert(ofi_total_iov_len(msg->msg_iov, msg->iov_count) <=
@@ -124,6 +140,10 @@ ssize_t efa_rma_readmsg(struct fid_ep *ep_fid, const struct fi_msg_rma *msg, uin
124140
if (err)
125141
return err;
126142

143+
err = efa_rma_check_iov_count(base_ep, msg->iov_count);
144+
if (err)
145+
return err;
146+
127147
return efa_rma_post_read(base_ep, msg, flags | base_ep->util_ep.tx_msg_flags);
128148
}
129149

@@ -143,6 +163,10 @@ ssize_t efa_rma_readv(struct fid_ep *ep_fid, const struct iovec *iov, void **des
143163
if (err)
144164
return err;
145165

166+
err = efa_rma_check_iov_count(base_ep, iov_count);
167+
if (err)
168+
return err;
169+
146170
len = ofi_total_iov_len(iov, iov_count);
147171
EFA_SETUP_RMA_IOV(rma_iov, addr, len, key);
148172
EFA_SETUP_MSG_RMA(msg, iov, desc, iov_count, src_addr, &rma_iov, 1,
@@ -207,6 +231,11 @@ static inline ssize_t efa_rma_post_write(struct efa_base_ep *base_ep,
207231
return -FI_ENOSYS;
208232
}
209233

234+
if (OFI_UNLIKELY(msg->iov_count > base_ep->domain->device->ibv_attr.max_sge_rd)) {
235+
EFA_WARN(FI_LOG_EP_DATA, "EFA device currently doesn't support > %d iov for rdma work request\n", base_ep->domain->device->ibv_attr.max_sge_rd);
236+
return -FI_EINVAL;
237+
}
238+
210239
efa_tracepoint(write_begin_msg_context, (size_t) msg->context, (size_t) msg->addr);
211240

212241
EFA_DBG(FI_LOG_EP_DATA,
@@ -277,6 +306,10 @@ ssize_t efa_rma_writemsg(struct fid_ep *ep_fid, const struct fi_msg_rma *msg,
277306
if (err)
278307
return err;
279308

309+
err = efa_rma_check_iov_count(base_ep, msg->iov_count);
310+
if (err)
311+
return err;
312+
280313
return efa_rma_post_write(base_ep, msg, flags | base_ep->util_ep.tx_msg_flags);
281314
}
282315

@@ -295,6 +328,10 @@ ssize_t efa_rma_writev(struct fid_ep *ep_fid, const struct iovec *iov,
295328
if (err)
296329
return err;
297330

331+
err = efa_rma_check_iov_count(base_ep, iov_count);
332+
if (err)
333+
return err;
334+
298335
len = ofi_total_iov_len(iov, iov_count);
299336
EFA_SETUP_RMA_IOV(rma_iov, addr, len, key);
300337
EFA_SETUP_MSG_RMA(msg, iov, desc, iov_count, dest_addr, &rma_iov, 1,

‎prov/efa/test/efa_unit_test_rma.c‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ static void test_efa_rma_prep(struct efa_resource *resource, fi_addr_t *addr)
1919
base_ep = container_of(resource->ep, struct efa_base_ep, util_ep.ep_fid);
2020
/* Add rma caps explicitly to ep->info to allow local test */
2121
base_ep->info->caps |= FI_RMA;
22+
/* Mock the max rdma sge to allow local test */
23+
base_ep->domain->device->ibv_attr.max_sge_rd = EFA_DEVICE_MAX_RDMA_SGE;
2224
/* Set up the mock operations */
2325
g_efa_unit_test_mocks.efa_qp_post_recv = &efa_mock_efa_qp_post_recv_return_mock;
2426
g_efa_unit_test_mocks.efa_qp_wr_complete = &efa_mock_efa_qp_wr_complete_no_op;
@@ -299,3 +301,63 @@ void test_efa_rma_writemsg_with_inject(struct efa_resource **state)
299301

300302
efa_unit_test_buff_destruct(&local_buff);
301303
}
304+
305+
void test_efa_rma_readv_multiple_iov_fail(struct efa_resource **state)
306+
{
307+
struct efa_resource *resource = *state;
308+
struct efa_unit_test_buff local_buff1, local_buff2;
309+
struct iovec iov[2];
310+
fi_addr_t src_addr;
311+
void *desc[2];
312+
int ret;
313+
uint64_t remote_addr = 0x87654321;
314+
uint64_t remote_key = 123456;
315+
316+
test_efa_rma_prep(resource, &src_addr);
317+
efa_unit_test_buff_construct(&local_buff1, resource, 2048);
318+
efa_unit_test_buff_construct(&local_buff2, resource, 2048);
319+
320+
iov[0].iov_base = local_buff1.buff;
321+
iov[0].iov_len = local_buff1.size;
322+
iov[1].iov_base = local_buff2.buff;
323+
iov[1].iov_len = local_buff2.size;
324+
desc[0] = fi_mr_desc(local_buff1.mr);
325+
desc[1] = fi_mr_desc(local_buff2.mr);
326+
327+
ret = fi_readv(resource->ep, iov, desc, 2, src_addr, remote_addr,
328+
remote_key, NULL);
329+
assert_int_equal(ret, -FI_EOPNOTSUPP);
330+
331+
efa_unit_test_buff_destruct(&local_buff1);
332+
efa_unit_test_buff_destruct(&local_buff2);
333+
}
334+
335+
void test_efa_rma_writev_multiple_iov_einval(struct efa_resource **state)
336+
{
337+
struct efa_resource *resource = *state;
338+
struct efa_unit_test_buff local_buff1, local_buff2;
339+
struct iovec iov[2];
340+
fi_addr_t dest_addr;
341+
void *desc[2];
342+
int ret;
343+
uint64_t remote_addr = 0x87654321;
344+
uint64_t remote_key = 123456;
345+
346+
test_efa_rma_prep(resource, &dest_addr);
347+
efa_unit_test_buff_construct(&local_buff1, resource, 2048);
348+
efa_unit_test_buff_construct(&local_buff2, resource, 2048);
349+
350+
iov[0].iov_base = local_buff1.buff;
351+
iov[0].iov_len = local_buff1.size;
352+
iov[1].iov_base = local_buff2.buff;
353+
iov[1].iov_len = local_buff2.size;
354+
desc[0] = fi_mr_desc(local_buff1.mr);
355+
desc[1] = fi_mr_desc(local_buff2.mr);
356+
357+
ret = fi_writev(resource->ep, iov, desc, 2, dest_addr, remote_addr,
358+
remote_key, NULL);
359+
assert_int_equal(ret, -FI_EINVAL);
360+
361+
efa_unit_test_buff_destruct(&local_buff1);
362+
efa_unit_test_buff_destruct(&local_buff2);
363+
}

‎prov/efa/test/efa_unit_tests.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,8 @@ int main(void)
370370
cmocka_unit_test_setup_teardown(test_efa_rma_inject_write, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
371371
cmocka_unit_test_setup_teardown(test_efa_rma_inject_writedata, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
372372
cmocka_unit_test_setup_teardown(test_efa_rma_writemsg_with_inject, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
373+
cmocka_unit_test_setup_teardown(test_efa_rma_readv_multiple_iov_fail, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
374+
cmocka_unit_test_setup_teardown(test_efa_rma_writev_multiple_iov_einval, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
373375
cmocka_unit_test_setup_teardown(test_efa_cq_read_no_completion, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
374376
cmocka_unit_test_setup_teardown(test_efa_cq_read_send_success, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
375377
cmocka_unit_test_setup_teardown(test_efa_cq_read_senddata_success, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),

‎prov/efa/test/efa_unit_tests.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,8 @@ void test_efa_rma_writedata();
332332
void test_efa_rma_inject_write();
333333
void test_efa_rma_inject_writedata();
334334
void test_efa_rma_writemsg_with_inject();
335+
void test_efa_rma_readv_multiple_iov_fail();
336+
void test_efa_rma_writev_multiple_iov_einval();
335337
void test_efa_cq_read_no_completion();
336338
void test_efa_cq_read_send_success();
337339
void test_efa_cq_read_senddata_success();

0 commit comments

Comments
 (0)