Skip to content

fabtest/benchmarks: Process retcode with ft_exit_code for rma_pingpong#12161

Merged
alekswn merged 1 commit into
ofiwg:mainfrom
alekswn:wide-wqe-fabtest
Apr 27, 2026
Merged

fabtest/benchmarks: Process retcode with ft_exit_code for rma_pingpong#12161
alekswn merged 1 commit into
ofiwg:mainfrom
alekswn:wide-wqe-fabtest

Conversation

@alekswn
Copy link
Copy Markdown
Contributor

@alekswn alekswn commented Apr 20, 2026

No description provided.

@alekswn alekswn requested review from a team and j-xiong April 20, 2026 23:01
Comment thread fabtests/benchmarks/benchmark_shared.c Outdated
if (inject_size_set)
inject_size = opts.inject_size;
if (inject_size_set && inject_size < opts.inject_size) {
FT_ERR("Provider does not support inject size %zu (max size %zu)", opts.inject_size, inject_size);
Copy link
Copy Markdown
Contributor

@shijin-aws shijin-aws Apr 20, 2026

Choose a reason for hiding this comment

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

So if user call fi_rma_bw -o write -j 32, it will fail in the same way as fi_setopt(FI_OPT_INJECT_RMA_SIZE ... ) because such inject size cannot be satisfied anyway right? If so then the setopt call is not redundant... But I agree that this check is better than fi_setopt

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.

Turns out this setopt is needed to disable inject for test_fabtests_performance.
Will remove this commit.

shijin-aws
shijin-aws previously approved these changes Apr 20, 2026
Comment thread fabtests/common/shared.c Outdated
Comment on lines -1368 to -1377
ret = ft_getinfo(hints, &fi);
if (ret)
return ret;

if (oob_sock >= 0 && opts.dst_addr) {
ret = ft_sock_sync(oob_sock, 0);
if (ret)
return ret;
}

ret = ft_getinfo(hints, &fi);
if (ret)
return ret;

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.

This sync before fi_getinfo() is intentional. It makes sure that address resolution can work as expected for all providers. Some providers require the server be ready before the client can resolve the address via fi_getinfo(). See L1390 for when the server makes the sync call.

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.

removed that

Comment thread fabtests/benchmarks/rma_pingpong.c
@alekswn alekswn changed the title fabtest/common: Changes in common code needed for wide WQE fabtest fabtest/benchmarks: Changes in common code needed for wide WQE fabtest Apr 21, 2026
j-xiong
j-xiong previously approved these changes Apr 21, 2026
@alekswn alekswn requested a review from shijin-aws April 21, 2026 21:23
shijin-aws
shijin-aws previously approved these changes Apr 23, 2026
@alekswn alekswn dismissed stale reviews from shijin-aws and j-xiong via e93e678 April 24, 2026 19:50
@alekswn alekswn changed the title fabtest/benchmarks: Changes in common code needed for wide WQE fabtest fabtest/benchmarks: Process retcode with ft_exit_code for rma_pingpong Apr 24, 2026
@alekswn alekswn requested review from j-xiong and shijin-aws April 24, 2026 20:26
Signed-off-by: Alexey Novikov <nalexey@amazon.com>
@alekswn
Copy link
Copy Markdown
Contributor Author

alekswn commented Apr 25, 2026

bot:aws:retest

@alekswn
Copy link
Copy Markdown
Contributor Author

alekswn commented Apr 27, 2026

@j-xiong Could you restart Intel CI?

@j-xiong
Copy link
Copy Markdown
Contributor

j-xiong commented Apr 27, 2026

CI restarted. Still in the queue so it may take some time to get the status updated.

@alekswn alekswn merged commit c3df0eb into ofiwg:main Apr 27, 2026
22 checks passed
@alekswn alekswn deleted the wide-wqe-fabtest branch April 27, 2026 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants