Copilot commented on code in PR #3569:
URL: https://github.com/apache/brpc/pull/3569#discussion_r4171615468
##########
src/brpc/rdma/rdma_endpoint.cpp:
##########
@@ -1137,8 +1151,20 @@ static ibv_qp* AllocateQp(ibv_cq* send_cq, ibv_cq*
recv_cq, uint32_t sq_size, ui
attr.cap.max_recv_wr = rq_size;
attr.cap.max_send_sge = GetRdmaMaxSge();
attr.cap.max_recv_sge = 1;
+ attr.cap.max_inline_data = BF_MAX_INLINE_DATA;
attr.qp_type = IBV_QPT_RC;
- return IbvCreateQp(GetRdmaPd(), &attr);
+ ibv_qp* qp = IbvCreateQp(GetRdmaPd(), &attr);
+ if (qp == nullptr) {
+ // The device may not support inline data, try again without it
+ attr.cap.max_inline_data = 0;
+ qp = IbvCreateQp(GetRdmaPd(), &attr);
Review Comment:
The QP capability negotiation and fallback are untested. Please add
automated coverage for a granted nonzero inline size and for a first
`IbvCreateQp` failure followed by a zero-inline retry, asserting the retained
`max_inline_data`; these paths determine whether every RDMA connection can be
created safely on devices without the requested capability.
##########
src/brpc/rdma/rdma_endpoint.cpp:
##########
@@ -894,6 +901,12 @@ ssize_t RdmaEndpoint::CutFromIOBufList(butil::IOBuf**
from, size_t ndata) {
}
wr.num_sge = sge_index;
+ // A small message from the block pool (host memory) is copied into
+ // the WQE. User registered memory may be device memory, which the
+ // CPU cannot copy from, so it is never inlined.
+ if (in_pool && this_len <= _resource->max_inline_data) {
+ wr.send_flags |= IBV_SEND_INLINE;
Review Comment:
This new send-path branch has no automated regression coverage. Please add
tests that verify pool-backed payloads at the granted limit receive
`IBV_SEND_INLINE`, payloads just above it do not, and user-registered payloads
never do; otherwise a threshold or memory-origin regression could make
GPU-backed sends unsafe. The existing RDMA test suite already exercises
`RdmaEndpoint` and user-defined IOBufs
(`test/brpc_rdma_unittest.cpp:3020-3082`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
