Skip to content

Ibv-conduit: Verbs Work Request API#15

Open
PHHargrove wants to merge 9 commits into
BerkeleyLab:developfrom
PHHargrove:ibv-wr-api
Open

Ibv-conduit: Verbs Work Request API#15
PHHargrove wants to merge 9 commits into
BerkeleyLab:developfrom
PHHargrove:ibv-wr-api

Conversation

@PHHargrove

@PHHargrove PHHargrove commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR is the next step toward DC (Dynamic Connection) support in IBV conduit.
It also represents a "modernization" of how we use libibverbs.

See https://man.archlinux.org/man/ibv_wr_post.3.en for the man page describing the Verbs Work Request API.

Status

Well tested on the CGPU nodes, including FAST and EVERYTHING modes both with and without the new --disable-ibv-wr-api.

While vendor docs suggest the new API should be marginally faster (and less memory-bus intensive) than the "legacy" mechanism, my testing is inconclusive. Tests on CGPU are consistent with those claims, but the measured improvements are too small relative to the standard deviations to be statistically significant (thus "inconclusive"). Notably I did not see evidence of any performance degradation in the data I have collected so far.

Ready for review.
Concurrent with code review, I will continue performance evaluation and extend testing to JLSE and Wombat.

Commits (reverse chronological order)

  • ibv: document --disable-ibv-wr-api

  • ibv: add an ident string for work request API

  • ibv: use the Verbs Work Request API when available

This commit updates ibv-conduit to use the Verbs Work Request API
(`ibv_wr_start()`, et al.) if available and not disabled at configure
time.

This is a prerequisite for planned DC transport support, but is also
touted by Mellanox/NVIDIA as having lower latency than the original
`ibv_send_post()` API.
  • ibv: Verbs Work Request API support in qp_create
This commit adds the necessary logic in `gasnetc_create_qp()`, which
was previously just an argument-marshaling macro, to create queue
pairs capable of supporting the Verbs Work Request API.  Use of that
API will follow.
  • ibv: configure probe for the Verbs Work Request API
This commit adds a configure probe for the Verbs Work Request API
(`ibv_wr_start()`, et al.).  An implementation will follow.
  • ibv: op-specific gasnetc_post_* functions
This commit replaces `gasnetc_snd_post_common()` (and the two macros
`gasnetc_snd_post()` and `gasnetc_snd_post_inline()` which call it
with defaulted arguments) with a serious of per-operation functions:

+ `gasnetc_post_send_imm()`
+ `gasnetc_post_write()`
+ `gasnetc_post_read()`
+ `gasnetc_post_fetch_add()`
+ `gasnetc_post_cmp_swp()`

Note that currently, these all call the (slightly modified, and now
file-scoped) `gasnetc_snd_post_common()` function.  However, the new
op-specific layer leads to improvements in stats/trace support.
Additionally, there is now better opportunity for constant propagation
if the compiler chooses to specialize `gasnetc_snd_post_common()`.

There are a few non-trivial changes to the callers to marshal
arguments differently.  While this may appear "arbitrary" at this
stage, these changes will enable support (soon) for the Verbs Work
Request API.
  • ibv: move some validation checks
This commit moves checks on valid `opcode` and a callback invariant
from `gasnetc_snd_post_common()` to `gasnetc_snd_validate()`, as well
as removing a redundant check for non-NULL `sreq->cep`.  Since
`gasnetc_snd_validate()` is called by `gasnetc_snd_post_common()`,
there is no loss of error checking.

This is preparation for multiple callers of `gasnetc_snd_validate()`,
all of which should include the checks which this commit moves.
  • ibv: cleanup gasnetc_signal_flag
This commit adds a `gasnetc_signal_flag` file-scope variable to
replace repeated computation (based on the `GASNET_USE_FENCED_PUTS`
environment var) when fenced puts are enabled at configure time.  When
fenced puts are disabled at configure time, the identifier
`gasnetc_signal_flag` is instead `#defined` to 0.

Additionally, `gasnetc_op_needs_fence_mask` is also now `#defined` to
0 when fenced puts are disabled at configure time.
  • ibv: minor cleanup in gasnetc_snd_post_common
Use struct assignment for marginally better clarity:
the intent is to clone the completion.

Use struct assignment for marginally better clarity:
the intent is to clone the completion.
This commit adds a `gasnetc_signal_flag` file-scope variable to
replace repeated computation (based on the `GASNET_USE_FENCED_PUTS`
environment var) when fenced puts are enabled at configure time.  When
fenced puts are disabled at configure time, the identifier
`gasnetc_signal_flag` is instead `#defined` to 0.

Additionally, `gasnetc_op_needs_fence_mask` is also now `#defined` to
0 when fenced puts are disabled at configure time.
This commit moves checks on valid `opcode` and a callback invariant
from `gasnetc_snd_post_common()` to `gasnetc_snd_validate()`, as well
as removing a redundant check for non-NULL `sreq->cep`.  Since
`gasnetc_snd_validate()` is called by `gasnetc_snd_post_common()`,
there is no loss of error checking.

This is preparation for multiple callers of `gasnetc_snd_validate()`,
all of which should include the checks which this commit moves.
This commit replaces `gasnetc_snd_post_common()` (and the two macros
`gasnetc_snd_post()` and `gasnetc_snd_post_inline()` which call it
with defaulted arguments) with a serious of per-operation functions:

+ `gasnetc_post_send_imm()`
+ `gasnetc_post_write()`
+ `gasnetc_post_read()`
+ `gasnetc_post_fetch_add()`
+ `gasnetc_post_cmp_swp()`

Note that currently, these all call the (slightly modified, and now
file-scoped) `gasnetc_snd_post_common()` function.  However, the new
op-specific layer leads to improvements in stats/trace support.
Additionally, there is now better opportunity for constant propagation
if the compiler chooses to specialize `gasnetc_snd_post_common()`.

There are a few non-trivial changes to the callers to marshal
arguments differently.  While this may appear "arbitrary" at this
stage, these changes will enable support (soon) for the Verbs Work
Request API.
This commit adds a configure probe for the Verbs Work Request API
(`ibv_wr_start()`, et al.).  An implementation will follow.
This commit adds the necessary logic in `gasnetc_create_qp()`, which
was previously just an argument-marshaling macro, to create queue
pairs capable of supporting the Verbs Work Request API.  Use of that
API will follow.
This commit updates ibv-conduit to use the Verbs Work Request API
(`ibv_wr_start()`, et al.) if available and not disabled at configure
time.

This is a prerequisite for planned DC transport support, but is also
touted by Mellanox/NVIDIA as having lower latency than the original
`ibv_send_post()` API.
@PHHargrove

Copy link
Copy Markdown
Collaborator Author

TL;DR:
My testing across seven platforms (including CX-5, 6 and 7 HCAs) shows no statistically significant performance difference for the old versus new communications injection code.


I have attached summary data for performance tests rune on seven distinct systems.

Ampere.txt
arcticus.txt
cascade.txt
cgpu.txt
gh.txt
gomez_dual_hca.txt
gpu_amd_mi300x.txt

Here is the text of one of the files (with line numbers added) for some explanation:

     1	CGPU
     2	1c:00.0 Infiniband controller: Mellanox Technologies MT27800 Family [ConnectX-5]
     3
     4	@ PUT: 8-byte flood b/w (UP is good)
     5		100 iterations
     6		10.0907	+/-	0.1495	(1.4820%)
     7		10.0977	+/-	0.1516	(1.5010%)
     8		Abs diff	0.0070	(0.0329 sigma)
     9	@ GET: 8-byte flood b/w (UP is good)
    10		100 iterations
    11		28.8835	+/-	1.9259	(6.6679%)
    12		28.8737	+/-	1.3903	(4.8149%)
    13		Abs diff	0.0098	(0.0041 sigma)
    14	@ PUT: 8-byte o_i (DOWN is good)
    15		100 iterations
    16		0.4048	+/-	0.0675	(16.6853%)
    17		0.4035	+/-	0.0847	(20.9909%)
    18		Abs diff	0.0013	(0.0120 sigma)
    19	@ GET: 8-byte o_i (DOWN is good)
    20		100 iterations
    21		0.3792	+/-	0.0601	(15.8459%)
    22		0.3622	+/-	0.0623	(17.2040%)
    23		Abs diff	0.0170	(0.1964 sigma)
    24	@ AM: Short ReqRep ping-pong latency (DOWN is good)
    25		100 iterations
    26		2.9270	+/-	0.0769	(2.6266%)
    27		2.9507	+/-	0.0525	(1.7790%)
    28		Abs diff	0.0237	(0.2545 sigma)

Line 1: names the system
Line 2: shows lspci output for the HCA(s) used in testing - can be multiple lines

Line 4: Description of one test of five, taken from testsmall, testlogGP and testam
Line 5: count of iterations of this test
Line 6: mean and standard deviation of the pre-PR performance, with std dev as a percent of mean in parens
Line 7: same as line 6, for the PRed code
Line 8: absolute difference between the means, and again in units of sigma (one std dev) given in parens.

If one looks over the entirety of the summary files, all absolute differences are below 1.0 sigma, and only two of those (out of 35) are above 0.5 sigma. Meanwhile, 2.0 sigma is the generally accepted threshold for a "statistically significant" difference.

@bonachea bonachea self-requested a review July 15, 2026 18:50
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.

2 participants