Skip to content

prov/efa: Add FI_MORE unit tests - #12787

Open
sunkuamzn wants to merge 8 commits into
ofiwg:mainfrom
sunkuamzn:fi-more-unit-tests
Open

prov/efa: Add FI_MORE unit tests#12787
sunkuamzn wants to merge 8 commits into
ofiwg:mainfrom
sunkuamzn:fi-more-unit-tests

Conversation

@sunkuamzn

@sunkuamzn sunkuamzn commented Sep 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR introduces comprehensive unit tests for the FI_MORE path

The first commit introduces the macro EFA_PROD_STATIC_INLINE which allows mocking of static inline functions in unit test builds

The next two commits fix the cmocka and gtest suite to use no-op mocks for the data path functions. These mocks were previously hardcoded in efa_data_path_ops.h

The next commit adds tests for efa-direct. The commit after that adds tests for efa-rdm protocol path.

The final commit is a doc update

@sunkuamzn
sunkuamzn force-pushed the fi-more-unit-tests branch 2 times, most recently from 8a9fb16 to 6a01fd2 Compare September 4, 2026 07:18
@sunkuamzn

sunkuamzn commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

bot:aws:retest

@sunkuamzn
sunkuamzn marked this pull request as ready for review September 4, 2026 16:47
@sunkuamzn
sunkuamzn requested a review from a team September 4, 2026 16:49
@sunkuamzn
sunkuamzn force-pushed the fi-more-unit-tests branch 2 times, most recently from 5ed5525 to 4c972ed Compare September 4, 2026 17:44
yinliaws
yinliaws reviewed Sep 4, 2026
View reviewed changes
Comment thread prov/efa/test/gtest/efa_gtest_fi_more_helpers.c
* a later real post takes the pending branch and rings the real doorbell
* for a producer counter the device never saw.
*/
sq->num_wqe_pending = 0;

@yinliaws yinliaws Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The restore helper only clears num_wqe_pending, but test posts also advance the producer counter, phase, available-entry count, and WR-ID state. If the endpoint is reused after restoring the probe, the next real post may reference queue entries the device never received. Could the helper save and restore all modified SQ bookkeeping as well?

@sunkuamzn sunkuamzn Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The endpoint is never reused, so I think it's fine. It's not worth storing and restoring even more state.

I edited the comment which claimed all state was restored.

@sunkuamzn
prov/efa: Introduce macro to allow unit tests on static inline functions
f753f79 
This commit introduces EFA_PROD_STATIC_INLINE which expands to
"static inline" in a production build and to nothing in a test build.
All functions that use EFA_PROD_STATIC_INLINE can be mocked in unit
tests.

Without static inline the functions have external linkage, so exactly one
translation unit can define them. The bodies are therefore guarded by
EFA_DATA_PATH_OPS_EMIT_BODIES, which only
prov/efa/test/efa_unit_test_data_path_ops.c defines

EFA_DATA_PATH_OPS_EMIT_BODIES has to be defined before any include
that could reach efa_data_path_ops.h. That single translation unit emits
the definitions and doesn't call any of them. Every other TU sees the
declarations alone and the calls can be mocked.

Production build is unaffected: with identical configure flags, 108 of
the 112 EFA provider objects are byte identical to the parent commit.
The 4 that differ do so only in a __LINE__ constant passed to one
EFA_INFO call, which moved because efa_data_path_ops.h is longer.

Signed-off-by: Sai Sunku <sunkusa@amazon.com>
@sunkuamzn
prov/efa/test: Set the default cmocka data path ops to no-op functions
8826259 
The data path functions were previously hard coded to no-op functions in
the unit test build in efa_data_path_ops.h. This commit explicitly sets
the mocks in prov/efa/test to no-op functions.

This commit is needed to avoid breaking all current cmocka tests which
rely on the data path ops functions being no-op functions.

Signed-off-by: Sai Sunku <sunkusa@amazon.com>
@sunkuamzn
prov/efa: let gtest fixtures declare they want no device access
e5cf2a5 
This commit does the same as the previous commit but for gtest. Some
existing gtest unit tests also rely on the data path functions being
no-op functions.

Signed-off-by: Sai Sunku <sunkusa@amazon.com>
@sunkuamzn
prov/efa: add utility functions to test EFA data path submissions
f022a31 
This commit introduces utility functions that can intercept and verify
EFA data path submissions

For the rdma-core path it modifies the qp ops table so that each of the
functions can be mocked independently. In this commit they're no-ops. It
also clears data_path_direct_enabled so that path is the one taken.

For the direct path it allocates a host buffer for the send queue and the
doorbell. It overwrites the pointers in efa_data_path_direct_sq so that
any writes to the SQ and DB go to the host buffer, which can be inspected
after the test.

The restore function undoes the setup and clears the counters, including
the unsubmitted work a FI_MORE test deliberately leaves behind.

Signed-off-by: Sai Sunku <sunkusa@amazon.com>
@sunkuamzn
prov/efa: Add efa-direct FI_MORE unit tests
4248a6b 
Add tests that call fi_sendmsg, fi_readmsg and fi_writemsg on efa-direct
and verify that the FI_MORE flag is set appropriately. The tests run on
both the rdma-core path and the data path direct path.

The batch test asserts nothing has been submitted after each post that carries
FI_MORE, not just at the end.

The rdma-core case has an extra failed submission test. Because a failed
submission must still call ibv_wr_complete.

The memory registration asks for RMA access only in the read and write
cases. A device that does not support RDMA read or RDMA write rejects such
a registration outright, which would fail the send case on hardware it has
no reason to depend on.

Signed-off-by: Sai Sunku <sunkusa@amazon.com>
@sunkuamzn
prov/efa: Honor FI_MORE for an application issued read
3087ac4 
efa_rdm_pke_read hardcoded the flags to 0 and ignored FI_MORE. This
commit fixes that.

This commit also unsets FI_MORE in the RXE. The efa-rdm protocol path
uses bounce buffers, so FI_MORE does not make sense in the receive path.
Leaving it set could enable reads posted for the longread protocol to
have FI_MORE set and cause a hang.

Signed-off-by: Sai Sunku <sunkusa@amazon.com>
@sunkuamzn
prov/efa: Add FI_MORE unit tests for efa protocol path
e3eac26 
The efa protocol path only sets FI_MORE for eager protocol. A tagged
send, a device RDMA write and, after the previous commit, a read all
honor it. A message large enough to need a medium RTM does not, because
honoring it there would leave the last packet of the message carrying it
and nothing would flush the message at all.

Add tests to verify this behavior

Gate the RMA cases on the specific capability each one needs rather than
skipping the whole fixture, so hardware missing RDMA read or RDMA write
still runs the send cases. The memory registration likewise asks only for
the access the device advertises, since asking for more fails the
registration outright.

Signed-off-by: Sai Sunku <sunkusa@amazon.com>
@sunkuamzn
prov/efa: document the FI_MORE test seams in gtest AGENTS.md
e64b53a 
Agent generated documentation for future agents ¯\_(ツ)_/¯

Record what this coverage needed and why, since none of it is guessable from
the surrounding code: that ld --wrap cannot intercept a call within one object
file, which is why the efa_cq_* helpers stay static inline and the one file
that defines the data path ops calls none of them; that an unmocked data path
op now reaches the device and a fixture opts out with
efa_test_arm_inert_data_path; that mocking only part of the completion queue is
the dangerous case, because an unarmed accessor then reads a queue that was
never really polled; that the device sink has to be redirected because neither
post implementation can be wrapped; that restore must also clear the
unsubmitted-work state; and that a capability gate reading the device list has
to probe with a fi_getinfo first.

Signed-off-by: Sai Sunku <sunkusa@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yinliaws yinliaws yinliaws approved these changes

Assignees

No one assigned

Labels

⚠️ Do not merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sunkuamzn @yinliaws