Skip to content

Commit b3f9890

Browse files
committed
prov/efa: Fix the unsolicited write recv check
Currently, efa provider uses the device & env to check the unsolicited write recv. This check is wrong and can cause false positive when the efadv_cq creation failed. This patch fixes this issue by tracking the unsolicited write recv status in both efa_ibv_cq and efa_qp. Signed-off-by: Shi Jin <[email protected]>
1 parent 76494e4 commit b3f9890

File tree

10 files changed

+55
-17
lines changed

10 files changed

+55
-17
lines changed

prov/efa/src/efa_base_ep.c

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -195,9 +195,12 @@ static int efa_base_ep_modify_qp_rst2rts(struct efa_base_ep *base_ep,
195195
*
196196
* @param qp double pointer of struct efa_qp
197197
* @param init_attr_ex ibv_qp_init_attr_ex
198+
* @param tclass traffic class (QoS) of the qp
199+
* @param use_unsolicited_write_recv whether to use unsolicited write recv in the qp
198200
* @return int 0 on success, negative integer on failure
199201
*/
200-
int efa_qp_create(struct efa_qp **qp, struct ibv_qp_init_attr_ex *init_attr_ex, uint32_t tclass)
202+
int efa_qp_create(struct efa_qp **qp, struct ibv_qp_init_attr_ex *init_attr_ex,
203+
uint32_t tclass, bool use_unsolicited_write_recv)
201204
{
202205
struct efadv_qp_init_attr efa_attr = { 0 };
203206

@@ -219,8 +222,9 @@ int efa_qp_create(struct efa_qp **qp, struct ibv_qp_init_attr_ex *init_attr_ex,
219222
init_attr_ex->send_ops_flags |= IBV_QP_EX_WITH_RDMA_WRITE;
220223
init_attr_ex->send_ops_flags |= IBV_QP_EX_WITH_RDMA_WRITE_WITH_IMM;
221224
}
225+
(*qp)->unsolicited_write_recv_enabled = use_unsolicited_write_recv;
222226
#if HAVE_CAPS_UNSOLICITED_WRITE_RECV
223-
if (efa_use_unsolicited_write_recv())
227+
if (use_unsolicited_write_recv)
224228
efa_attr.flags |= EFADV_QP_FLAGS_UNSOLICITED_WRITE_RECV;
225229
#endif
226230
efa_attr.driver_qp_type = EFADV_QP_DRIVER_TYPE_SRD;
@@ -313,7 +317,10 @@ static int efa_base_ep_create_qp(struct efa_base_ep *base_ep,
313317

314318
efa_base_ep_construct_ibv_qp_init_attr_ex(base_ep, &attr_ex, tx_cq->ibv_cq_ex, rx_cq->ibv_cq_ex);
315319

316-
ret = efa_qp_create(&base_ep->qp, &attr_ex, base_ep->info->tx_attr->tclass);
320+
assert(tx_cq->unsolicited_write_recv_enabled == rx_cq->unsolicited_write_recv_enabled);
321+
322+
ret = efa_qp_create(&base_ep->qp, &attr_ex, base_ep->info->tx_attr->tclass,
323+
tx_cq->unsolicited_write_recv_enabled);
317324
if (ret)
318325
return ret;
319326

@@ -332,7 +339,7 @@ static int efa_base_ep_create_qp(struct efa_base_ep *base_ep,
332339
#endif
333340

334341
if (create_user_recv_qp) {
335-
ret = efa_qp_create(&base_ep->user_recv_qp, &attr_ex, base_ep->info->tx_attr->tclass);
342+
ret = efa_qp_create(&base_ep->user_recv_qp, &attr_ex, base_ep->info->tx_attr->tclass, tx_cq->unsolicited_write_recv_enabled);
336343
if (ret) {
337344
efa_base_ep_destruct_qp(base_ep);
338345
return ret;
@@ -669,7 +676,7 @@ int efa_base_ep_check_qp_in_order_aligned_128_bytes(struct efa_base_ep *ep,
669676
/* Create a dummy qp for query only */
670677
efa_base_ep_construct_ibv_qp_init_attr_ex(ep, &attr_ex, ibv_cq.ibv_cq_ex, ibv_cq.ibv_cq_ex);
671678

672-
ret = efa_qp_create(&qp, &attr_ex, FI_TC_UNSPEC);
679+
ret = efa_qp_create(&qp, &attr_ex, FI_TC_UNSPEC, ibv_cq.unsolicited_write_recv_enabled);
673680
if (ret)
674681
goto out;
675682

prov/efa/src/efa_base_ep.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ struct efa_qp {
5151
#if HAVE_EFA_DATA_PATH_DIRECT
5252
struct efa_data_path_direct_qp data_path_direct_qp;
5353
#endif
54+
bool unsolicited_write_recv_enabled;
5455
};
5556

5657
struct efa_av;
@@ -111,7 +112,8 @@ int efa_base_ep_getname(fid_t fid, void *addr, size_t *addrlen);
111112
int efa_ep_open(struct fid_domain *domain_fid, struct fi_info *user_info,
112113
struct fid_ep **ep_fid, void *context);
113114

114-
int efa_qp_create(struct efa_qp **qp, struct ibv_qp_init_attr_ex *init_attr_ex, uint32_t tclass);
115+
int efa_qp_create(struct efa_qp **qp, struct ibv_qp_init_attr_ex *init_attr_ex,
116+
uint32_t tclass, bool enable_unsolicited_write_recv);
115117

116118
void efa_qp_destruct(struct efa_qp *qp);
117119

prov/efa/src/efa_cq.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ struct efa_ibv_cq {
2222
struct efa_data_path_direct_cq data_path_direct;
2323
#endif
2424
struct ibv_comp_channel *channel;
25+
bool unsolicited_write_recv_enabled;
2526
};
2627

2728
struct efa_ibv_cq_poll_list_entry {
@@ -216,6 +217,7 @@ int efa_cq_open_ibv_cq(struct fi_cq_attr *attr,
216217
.wc_flags = EFADV_WC_EX_WITH_SGID,
217218
};
218219

220+
ibv_cq->unsolicited_write_recv_enabled = false;
219221
#if HAVE_CAPS_UNSOLICITED_WRITE_RECV
220222
if (efa_use_unsolicited_write_recv())
221223
efadv_cq_init_attr.wc_flags |= EFADV_WC_EX_WITH_IS_UNSOLICITED;
@@ -251,6 +253,11 @@ int efa_cq_open_ibv_cq(struct fi_cq_attr *attr,
251253
&init_attr_ex, ibv_ctx, &ibv_cq->ibv_cq_ex, &ibv_cq->ibv_cq_ex_type);
252254
}
253255

256+
#if HAVE_CAPS_UNSOLICITED_WRITE_RECV
257+
if (efadv_cq_init_attr.wc_flags & EFADV_WC_EX_WITH_IS_UNSOLICITED)
258+
ibv_cq->unsolicited_write_recv_enabled = true;
259+
#endif
260+
254261
ibv_cq->ibv_cq_ex_type = EFADV_CQ;
255262
return 0;
256263
}
@@ -283,6 +290,7 @@ int efa_cq_open_ibv_cq(struct fi_cq_attr *attr,
283290
};
284291

285292
ibv_cq->data_path_direct_enabled = false;
293+
ibv_cq->unsolicited_write_recv_enabled = false;
286294
return efa_cq_open_ibv_cq_with_ibv_create_cq_ex(
287295
&init_attr_ex, ibv_ctx, &ibv_cq->ibv_cq_ex, &ibv_cq->ibv_cq_ex_type);
288296
}

prov/efa/src/efa_data_path_ops.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ static inline int efa_ibv_cq_wc_read_sgid(struct efa_ibv_cq *ibv_cq, union ibv_g
320320
*/
321321
static inline bool efa_cq_wc_is_unsolicited(struct efa_ibv_cq *ibv_cq)
322322
{
323-
return efa_use_unsolicited_write_recv() && efa_ibv_cq_wc_is_unsolicited(ibv_cq);
323+
return ibv_cq->unsolicited_write_recv_enabled && efa_ibv_cq_wc_is_unsolicited(ibv_cq);
324324
}
325325

326326
static inline bool efa_cq_wc_available(struct efa_ibv_cq *cq)

prov/efa/src/rdm/efa_rdm_ep_fiops.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1074,7 +1074,7 @@ void efa_rdm_ep_set_extra_info(struct efa_rdm_ep *ep)
10741074

10751075
ep->extra_info[0] |= EFA_RDM_EXTRA_FEATURE_DELIVERY_COMPLETE;
10761076

1077-
if (efa_use_unsolicited_write_recv())
1077+
if (ep->base_ep.qp->unsolicited_write_recv_enabled)
10781078
ep->extra_info[0] |= EFA_RDM_EXTRA_FEATURE_UNSOLICITED_WRITE_RECV;
10791079

10801080
if (ep->use_zcpy_rx) {

prov/efa/src/rdm/efa_rdm_rma.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,7 @@ ssize_t efa_rdm_rma_post_write(struct efa_rdm_ep *ep, struct efa_rdm_ope *txe)
395395
"This is usually caused by inconsistent efa driver, libfabric, or rdma-core versions.\n"
396396
"Please use consistent software versions on both hosts, or disable the unsolicited write "
397397
"recv feature by setting environment variable FI_EFA_USE_UNSOLICITED_WRITE_RECV=0\n",
398-
efa_use_unsolicited_write_recv(), efa_rdm_peer_support_unsolicited_write_recv(txe->peer),
398+
ep->base_ep.qp->unsolicited_write_recv_enabled, efa_rdm_peer_support_unsolicited_write_recv(txe->peer),
399399
err_msg);
400400
return -FI_EOPNOTSUPP;
401401
}

prov/efa/test/efa_unit_test_cq.c

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,24 @@ void test_rdm_cq_handshake_bad_send_status_unsupported_op(struct efa_resource **
388388
test_rdm_cq_handshake_bad_send_status_impl(state, EFA_IO_COMP_STATUS_LOCAL_ERROR_UNSUPPORTED_OP, true);
389389
}
390390

391+
392+
/**
393+
* @brief Verify that unsolicited write recv status is tracked in efa_ibv_cq correctly
394+
*
395+
* @param[in] state struct efa_resource that is managed by the framework
396+
*/
397+
void test_ibv_cq_unsolicited_write_recv_status(struct efa_resource **state)
398+
{
399+
struct efa_resource *resource = *state;
400+
struct efa_cq *efa_cq;
401+
402+
403+
efa_unit_test_resource_construct(resource, FI_EP_RDM, EFA_FABRIC_NAME);
404+
efa_cq = container_of(resource->cq, struct efa_cq, util_cq.cq_fid);
405+
406+
assert_true(efa_use_unsolicited_write_recv() == efa_cq->ibv_cq.unsolicited_write_recv_enabled);
407+
}
408+
391409
/**
392410
* @brief verify that fi_cq_read/fi_cq_readerr works properly when rdma-core return bad status for recv.
393411
*
@@ -460,7 +478,7 @@ void test_ibv_cq_ex_read_bad_recv_status(struct efa_resource **state)
460478
ibv_cq->ibv_cq_ex->status = IBV_WC_GENERAL_ERR;
461479

462480
#if HAVE_CAPS_UNSOLICITED_WRITE_RECV
463-
if (efa_use_unsolicited_write_recv()) {
481+
if (ibv_cq->unsolicited_write_recv_enabled) {
464482
g_efa_unit_test_mocks.efa_ibv_cq_wc_is_unsolicited = &efa_mock_efa_ibv_cq_wc_is_unsolicited_return_mock;
465483
will_return(efa_mock_efa_ibv_cq_wc_is_unsolicited_return_mock, false);
466484
}
@@ -536,19 +554,18 @@ void test_ibv_cq_ex_read_bad_recv_rdma_with_imm_status_impl(struct efa_resource
536554
will_return_always(efa_mock_efa_ibv_cq_wc_read_qp_num_return_mock, efa_rdm_ep->base_ep.qp->qp_num);
537555
will_return(efa_mock_efa_ibv_cq_wc_read_vendor_err_return_mock, EFA_IO_COMP_STATUS_FLUSHED);
538556

539-
g_efa_unit_test_mocks.efa_device_support_unsolicited_write_recv = &efa_mock_efa_device_support_unsolicited_write_recv;
540557

541558
#if HAVE_CAPS_UNSOLICITED_WRITE_RECV
542559
if (use_unsolicited_recv) {
543560
g_efa_unit_test_mocks.efa_ibv_cq_wc_is_unsolicited = &efa_mock_efa_ibv_cq_wc_is_unsolicited_return_mock;
544-
will_return(efa_mock_efa_device_support_unsolicited_write_recv, true);
561+
ibv_cq->unsolicited_write_recv_enabled = true;
545562
will_return(efa_mock_efa_ibv_cq_wc_is_unsolicited_return_mock, true);
546563
ibv_cq->ibv_cq_ex->wr_id = 0;
547564
} else {
548565
/*
549566
* For solicited write recv, it will consume an internal rx pkt
550567
*/
551-
will_return(efa_mock_efa_device_support_unsolicited_write_recv, false);
568+
ibv_cq->unsolicited_write_recv_enabled = false;
552569
struct efa_rdm_pke *pkt_entry = efa_rdm_pke_alloc(efa_rdm_ep, efa_rdm_ep->efa_rx_pkt_pool, EFA_RDM_PKE_FROM_EFA_RX_POOL);
553570
assert_non_null(pkt_entry);
554571
efa_rdm_ep->efa_rx_pkts_posted = efa_rdm_ep_get_rx_pool_size(efa_rdm_ep);
@@ -558,7 +575,7 @@ void test_ibv_cq_ex_read_bad_recv_rdma_with_imm_status_impl(struct efa_resource
558575
/*
559576
* Always test with solicited recv
560577
*/
561-
will_return(efa_mock_efa_device_support_unsolicited_write_recv, false);
578+
ibv_cq->unsolicited_write_recv_enabled = false;
562579
struct efa_rdm_pke *pkt_entry = efa_rdm_pke_alloc(efa_rdm_ep, efa_rdm_ep->efa_rx_pkt_pool, EFA_RDM_PKE_FROM_EFA_RX_POOL);
563580
assert_non_null(pkt_entry);
564581
efa_rdm_ep->efa_rx_pkts_posted = efa_rdm_ep_get_rx_pool_size(efa_rdm_ep);
@@ -1065,7 +1082,7 @@ static void test_efa_cq_read_prep(struct efa_resource *resource,
10651082

10661083

10671084
#if HAVE_CAPS_UNSOLICITED_WRITE_RECV
1068-
if (efa_use_unsolicited_write_recv()) {
1085+
if (ibv_cq->unsolicited_write_recv_enabled) {
10691086
g_efa_unit_test_mocks.efa_ibv_cq_wc_is_unsolicited = &efa_mock_efa_ibv_cq_wc_is_unsolicited_return_mock;
10701087
will_return_maybe(efa_mock_efa_ibv_cq_wc_is_unsolicited_return_mock, is_unsolicited_write_recv);
10711088
}

prov/efa/test/efa_unit_test_ep.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1461,7 +1461,7 @@ void test_efa_rdm_ep_rx_refill_threshold_larger_than_rx_size(struct efa_resource
14611461
}
14621462

14631463
/**
1464-
* @brief when unsolicited write recv is supported (by device + env),
1464+
* @brief when unsolicited write recv is supported (checked by cq),
14651465
* efa_rdm_ep_support_unsolicited_write_recv
14661466
* should return true, otherwise it should return false
14671467
*
@@ -1471,13 +1471,15 @@ void test_efa_rdm_ep_rx_refill_threshold_larger_than_rx_size(struct efa_resource
14711471
void test_efa_rdm_ep_support_unsolicited_write_recv(struct efa_resource **state)
14721472
{
14731473
struct efa_rdm_ep *efa_rdm_ep;
1474+
struct efa_cq *efa_cq;
14741475
struct efa_resource *resource = *state;
14751476

14761477
efa_unit_test_resource_construct(resource, FI_EP_RDM, EFA_FABRIC_NAME);
14771478

14781479
efa_rdm_ep = container_of(resource->ep, struct efa_rdm_ep, base_ep.util_ep.ep_fid);
1480+
efa_cq = container_of(resource->cq, struct efa_cq, util_cq.cq_fid);
14791481

1480-
assert_int_equal(efa_use_unsolicited_write_recv(),
1482+
assert_int_equal(efa_cq->ibv_cq.unsolicited_write_recv_enabled,
14811483
efa_rdm_ep_support_unsolicited_write_recv(efa_rdm_ep));
14821484
}
14831485

prov/efa/test/efa_unit_tests.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ int main(void)
200200
cmocka_unit_test_setup_teardown(test_rdm_cq_handshake_bad_send_status_unreach_remote, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
201201
cmocka_unit_test_setup_teardown(test_rdm_cq_handshake_bad_send_status_remote_abort, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
202202
cmocka_unit_test_setup_teardown(test_rdm_cq_handshake_bad_send_status_unsupported_op, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
203+
cmocka_unit_test_setup_teardown(test_ibv_cq_unsolicited_write_recv_status, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
203204
cmocka_unit_test_setup_teardown(test_ibv_cq_ex_read_bad_recv_status, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
204205
cmocka_unit_test_setup_teardown(test_ibv_cq_ex_read_bad_recv_rdma_with_imm_status_use_unsolicited_recv, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
205206
cmocka_unit_test_setup_teardown(test_ibv_cq_ex_read_bad_recv_rdma_with_imm_status_use_solicited_recv, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),

prov/efa/test/efa_unit_tests.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ void test_rdm_cq_handshake_bad_send_status_unresp_remote();
173173
void test_rdm_cq_handshake_bad_send_status_unreach_remote();
174174
void test_rdm_cq_handshake_bad_send_status_remote_abort();
175175
void test_rdm_cq_handshake_bad_send_status_unsupported_op();
176+
void test_ibv_cq_unsolicited_write_recv_status();
176177
void test_ibv_cq_ex_read_bad_recv_status();
177178
void test_ibv_cq_ex_read_bad_recv_rdma_with_imm_status_use_unsolicited_recv();
178179
void test_ibv_cq_ex_read_bad_recv_rdma_with_imm_status_use_solicited_recv();

0 commit comments

Comments
 (0)