Repository navigation
feat(ernic): enable minio-cpp v1 CI/CD - #38
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Critical concurrency risk and additional configuration, fallback, and V2 implementation issues remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR enables ERNIC/MinIO v1 CI with HTTP fallback, removes BNXT support, and updates ionic-focused tooling and documentation.
Changes:
- Adds threaded HTTP-only MinIO testing and CI orchestration.
- Removes BNXT backend, configuration, tests, and documentation.
- Updates ERNIC images, build defaults, and integration workflows.
File summaries
| File | Summary |
|---|---|
test/unit/test-vendor-ops.cpp |
Removes BNXT tests. |
test/unit/CMakeLists.txt |
Removes BNXT test configuration. |
test/integration/rdma-test-server/main.cpp |
Adds HTTP-only and threaded serving. |
test/integration/rdma-test-server/CMakeLists.txt |
Removes BNXT configuration. |
src/rdma/vendor-ops.h |
Removes BNXT definitions. |
src/rdma/v2-transport.cpp |
Removes BNXT handling. |
src/rdma/transport.cpp |
Removes BNXT handling. |
src/rdma/bnxt/bnxt-backend.cpp |
Deletes BNXT backend. |
src/hipobj.cpp |
Adds V2 API stubs. |
README.md |
Updates supported hardware. |
integrations/minio-cpp/TESTING.md |
Updates testing instructions. |
integrations/minio-cpp/src/rdma.cpp |
Fixes MinIO namespace handling. |
integrations/minio-cpp/src/client.cpp |
Adds HTTP fallback paths. |
integrations/minio-cpp/include/hipobj_minio/context.h |
Qualifies MinIO types. |
integrations/minio-cpp/include/hipobj_minio/client.h |
Qualifies MinIO types. |
integrations/minio-cpp/examples/getput-rdma.cpp |
Adds Unix API include. |
INSTALL.md |
Updates build options. |
docs/interop.rst |
Removes BNXT references. |
docs/building.rst |
Updates build options. |
CMakeLists.txt |
Removes BNXT configuration. |
ci/ernic/server-v1-entrypoint.sh |
Starts the ERNIC v1 server. |
ci/ernic/run-local.sh |
Adds v1/v2 local flows. |
ci/ernic/minio-v1-client-entrypoint.sh |
Runs MinIO v1 tests. |
ci/ernic/docker-compose.yml |
Adds the MinIO v1 compose profile. |
ansible/playbooks/hipobj_client.yml |
Removes BNXT configuration. |
ansible/group_vars/hipobj_clients.yml |
Enables ionic by default. |
AGENTS.md |
Updates build guidance. |
.github/workflows/minio-integration-check.yml |
Removes BNXT flags. |
.github/workflows/hipobject-build.yml |
Removes BNXT flags. |
.github/workflows/ernic-integration-check.yml |
Re-enables ERNIC v1 CI. |
.cursor/environment.json |
Updates build configuration. |
.codespellrc |
Removes the BNXT exception. |
Review details
Suppressed comments (5)
AGENTS.md:26
- This change removes the explicit ionic flag while the repository guidance below still says
HIPOBJ_IONICdefaults to OFF. That leaves the agent/build instructions contradictory to the updated INSTALL/docs (and the PR's stated default); update the stale guidance together with the option change.
-DHIPOBJ_IONIC=ON
INSTALL.md:42
- This documents
HIPOBJ_IONICas ON, but the actualoption(HIPOBJ_IONIC ...)inCMakeLists.txt:66-67still defaults to OFF. A default configure therefore does not build the only remaining RDMA backend, contradicting the documented/default CI configuration; update the CMake default (or correct all documentation and setup instructions consistently).
| `HIPOBJ_IONIC` | ON | Build ionic RDMA backend |
ci/ernic/docker-compose.yml:107
- The minio-v1 client compose service repeats the old August ERNIC image default, so the documented v1 compose invocation can fail even though CI uses the new ionic image. Keep this service on the same updated default as the v1 server and client script.
image: ${ERNIC_IMAGE:-sbates130272/batesste-ci-images-ubuntu-rocm-ernic:august-27-2026}
integrations/minio-cpp/src/client.cpp:111
- This fallback treats
args.bufas host memory, but the same API accepts GPU pointers (the normal RDMA path registers GPU buffers). If initialization fails for a GPU-backed request, constructing astd::stringfrom that device pointer dereferences it on the CPU and can crash instead of falling back; use an explicit device-to-host copy or return an error for non-host buffers.
// RDMA not available (no NIC or no GPU topology) — fall back to HTTP PUT.
// Copy the buffer into a std::stringstream so minio-cpp can seek in it.
const char* src = static_cast<const char*>(args.buf);
std::string content(src, size);
src/hipobj.cpp:405
- These entry points unconditionally return
hipObjNotSupported, so every caller of the newly wired V2 API immediately falls back to V1 and no PREPARE/READY/CANCEL transfer can ever occur. This also means the v2 integration job cannot exercise the V2 data path despite the API contract and CI description; either provide the real V2 implementation or do not expose/advertise this path as implemented.
// hipObjPutV2 and hipObjGetV2 are not yet implemented; return hipObjNotSupported
// so callers fall back to the v1 RDMA path.
hipObjError_t hipObjPutV2(const char*, const char*, const void*, uint64_t,
uint64_t, const char*, hipObjOpsV2_t*, void*) {
return {hipObjNotSupported, 0};
- Files reviewed: 32/32 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| objects[key] = std::vector<uint8_t>(req.body.begin(), req.body.end()); | ||
| resp.status = 200; | ||
| resp.headers["etag"] = "\"test\""; | ||
| return resp; | ||
| } | ||
| if (req.method == "GET") { | ||
| auto it = objects.find(key); |
|
|
||
| ernic-server-v1: | ||
| profiles: [minio-v1] | ||
| image: ${ERNIC_IMAGE:-sbates130272/batesste-ci-images-ubuntu-rocm-ernic:august-27-2026} |
| # rocjitsu: rocjitsu GPU emulator serving a vfio-user socket via a | ||
| # shared tmpfs volume. The client picks up the socket and | ||
| # runs minio-getput-rdma in gpu mode (hipMalloc → RDMA PUT/GET). | ||
| # ernic-minio-client: minio-getput-rdma; payload integrity verified. |
|
@copilot resolve the merge conflicts in this pull request |
Resolved by merging |
e6664b7 to
1f0e9cf
Compare
88bb90c to
f456c48
Compare
Replaces the single ernic-integration-check lane with five hardware lanes
that run hipObject against emulated hardware -- rocm-ernic's ionic NIC and,
for the GPU lanes, rocjitsu's gfx1250 -- over vfio-user, under QEMU, with
nested KVM on the runner.
The lanes, all on pinned image tags:
S3 Backend one guest, NIC only, rocm-ernic's in-process S3
MinIO Bridge two guests, the minio-cpp bridge over the fabric
Two-VM Protocol two guests, v1 protocol, NIC on both
GPU Direct one guest, NIC + emulated GPU, hipMalloc'd buffer
Two-VM GPU Direct two guests, GPU on the client only, none on the server
Every lane asserts the bytes moved. A transfer that silently degrades to
HTTP, or reports success without touching the wire, now fails the job --
assert-transfer.sh checks the rdma_put/rdma_get/rdma_bytes counters rather
than the exit status, because the exit status was green for a data plane
that was doing nothing. Two-VM GPU Direct additionally asserts its own
asymmetry (VM 1 must show no 1002: function), so a guest that quietly grew
a GPU fails instead of passing for the wrong reason.
Supporting changes:
- Guest images are fetched by pinned artifact tag through a new composite
action, and ip_discovery.bin is generated at job time from whichever
rocjitsu build serves the socket, since it describes that device.
- Every workflow gets the same name, filename and trigger shape, which is
what makes the badge list and the required-checks list legible. README
badges follow the renames.
- PCI functions are counted in sysfs. lspci needs a pci.ids database the
guest does not have, and bailed with a parse error after one device.
- Client steps are bounded and line-buffered. A wedged client used to cost
the full 120-minute budget and print nothing; it now exits on a guest-side
timeout and speaks while it runs.
- HIPOBJ_IONIC defaults ON, so the provider the lanes exercise is built.
- STUFF.md retires into the files that needed it: the AIStor package URLs
move to ansible/group_vars, where the roles actually read them.
Guest memory: the two-VM lanes sized both guests at
HOST_MEM_MB * 3 / 4 / 2 clamped to 4096. A GitHub-hosted runner has ~16 GB,
so the formula asks for ~5995 and the clamp cut it to exactly 4096 -- the
size where a 1 MiB device-buffer transfer is a coin flip on the local lab
(6 pass / 7 fail, against 8/8 at 5996 and 9/9 at 32768). The ceiling is now
6144, matching the single-VM GPU lane, and the lane echoes its chosen
sizing, which it never did.
KNOWN FAILING, and temporarily non-fatal:
Two-VM GPU Direct has never passed
GPU Direct passed until the dmabuf fix stopped it faking
GPU Direct is red because it is now honest: it used to pass by staging
through host memory, and HIPOBJ_REQUIRE_GPU_DIRECT=1 refuses that. It
fails at 65536 bytes with "RDMA PUT failed (-1)".
Two-VM GPU Direct fails at 1 MiB, deterministically and in under a second,
with the remote read never resolving the client's dmabuf address:
rocm-ernic: WARN: ionic_datapath: remote read rkey=0x101
va=0x75823b600000 of 1048576 bytes failed
hipObj: work completion failed: status=10 opcode=0 vendor_err=0x9
Root cause is open. Ruled out by measurement on a two-guest lab, each at
8/8 where CI is 0-for-every-run: rocjitsu build (CI's gad7a357 and the
lab's gb3399b3 both pass), QEMU build (CI's stock 11.1.1 and a patched
one both pass), guest memory, vCPU count, accelerator (both kvm),
ip_discovery.bin (byte-identical), the 16 MiB IONIC_MESH_MAX_MSG cap (the
lane asks for 1 MiB), and BAR placement (identical at every guest size).
The one component not yet swapped is rocm-ernic itself: CI runs the pinned
ernic.0b48aa1, the lab a locally built binary, and they differ.
Both lanes carry continue-on-error on the client step only, so the rest of
this work is not held behind an open bug. Scoped to that one step on
purpose: everything before it -- the build, the guest boot, the NIC and GPU
enumeration, PORT_ACTIVE, the KFD node -- stays fatal, so a regression in
any of it still fails the job rather than hiding behind the known failure.
A green job that did not transfer anything is the exact thing assert-transfer
exists to prevent, so the green says so. When the client step fails, the job
emits a ::warning annotation and a step summary headed KNOWN FAILING,
stating that the job reports success and did not pass, with the error and
the ruled-out list. Remove the continue-on-error, and these steps, the
moment the transfer works.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Stephen Bates <sbates@raithlin.com>
The RDMA data plane reported success for transfers that moved nothing. Four defects, each of which independently made a green run meaningless. buildRdmaToken advertised mr->addr. ibv_reg_dmabuf_mr leaves that NULL, and on the bounce path it is the staging buffer rather than the caller's pointer, so the token described memory the peer could not use. BufferMap now records the address to advertise at registration time. Nothing copied between the caller's buffer and the staging buffer. When the NIC cannot reach GPU memory directly, hipObject registers host staging instead of the caller's pointer -- and then left it empty. Adds hipObjBufSync: callers driving a transfer through hipObjGetRdmaToken call it with HIPOBJ_SYNC_TO_HOST before a PUT and HIPOBJ_SYNC_TO_DEVICE after a GET. It is a no-op for a directly registered buffer. The minio-cpp bridge sent the caller's raw pointer in the token's ":<addr>:<len>" suffix, which rocm-ernic treats as an override of the address in the token. On the staging path that address is not covered by the MR, giving LOCAL_PROT_ERR and a zero-byte transfer that the bridge reported as success. Send zeros -- "no override" -- and let the token speak. rdmaPutWithRetry now stages to host before the request and rdmaGetWithRetry stages back to device after a successful reply. dmabuf support was predicted rather than attempted. init_dmabuf_support_flag() grepped /boot/config-$(uname -r) for both CONFIG_DMABUF_MOVE_NOTIFY=y and CONFIG_PCI_P2PDMA=y, and switched the whole GPU-direct path off when either was missing. Measured on 7.2.4-070204-generic: PCI_P2PDMA is set, ibv_reg_dmabuf_mr resolves in rdma-core 61.0, hsa_amd_portable_export_dmabuf succeeds -- and DMABUF_MOVE_NOTIFY is simply absent from the config. One missing line turned off a path that works. The gate is gone; the call is tried and its result believed. With that, a GPU-direct transfer that cannot be done directly must not be quietly done some other way. HIPOBJ_REQUIRE_GPU_DIRECT=1 makes the client refuse to fall back to host staging for a device pointer, and say so, rather than return success for a transfer the caller did not ask for. This is what turns the GPU Direct CI lane red: it was passing by faking. Failures also say which failure. Every RDMA error used to surface as one anonymous -1, which the HTTP layer rendered as "500 RDMA PUT failed" -- six distinct faults, from an unparseable token to a completion error, indistinguishable from a server with no RDMA at all. pollCompletion now reports the work-completion status, opcode and vendor_err, and each failure path names itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Stephen Bates <sbates@raithlin.com>
…wedging
Three bugs in the v1 integration test server, all found while chasing the
Two-VM GPU Direct lane, all of which cost time by describing themselves as
something else.
runThreaded() called FD_SET(listen_fd_, &fds) with no guard. runOnce() has
always checked listen_fd_ < 0; the threaded loop did not, and FD_SET(-1) is
a glibc fortify abort. So a port already in use did not report a busy port
-- it killed the process with "bit out of range 0 - FD_SETSIZE on fd_set",
which names no port and suggests memory corruption. Under systemd that
arrived as code=dumped, status=6/ABRT. startThreaded() now refuses to spawn
the thread without a socket, and the loop holds the invariant itself since
it is reachable directly.
main() never checked that start returned. It printed "listening on port N"
unconditionally -- the line every caller waits for before it starts sending
-- so a server that had already failed to bind announced itself ready and
then died. startThreaded() returns bool; both call sites check it and exit 1
with the port in the message. The constructor also says why the bind failed,
where it failed, rather than leaving a half-built object to abort later.
The RC QP was only reset on the success paths. One QP serves every request
in turn, and an RC QP already in RTS cannot be moved to RTR for the next
client, so a single failed transfer wedged the server for everyone after it:
each subsequent request returned "could not connect the RC QP to the
client", naming the wrong fault and burying the real one. Not hypothetical
-- it invalidated a size sweep on the lab, where four sizes appeared to fail
on their merits and had in fact never reached the wire. Reset moves into
Impl::resetQp(), runs on every path that touched the QP, and clears `ready`
if the rebuild itself fails rather than serving a QP it no longer has.
Verified on the two-guest lab: a transfer that fails is followed by two that
succeed against the same server instance, with one failure line in the log
and none after it. Two servers on one port give
hipobj-rdma-test-server: cannot bind port 9321: Address already in use
hipobj-rdma-test-server: not listening on port 9321
and exit 1, where the second used to abort. ctest is 12/12.
The QP reset is confirmed working in CI: the Two-VM GPU Direct client now
retries and gets the same error both times, where before the retry reported
a connect failure and hid the original fault.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Stephen Bates <sbates@raithlin.com>
660f523 to
76a300f
Compare
… eaten The GPU Direct lane never got the bounding that the other lanes were given: its client step had no step timeout, no guest-side timeout, and no ssh keepalives. At 76a300f that cost the full 120-minute job budget. The client printed its banner and then produced nothing for 110 minutes -- guest dmesg stops at t=146s, right after the process acquires a KFD VM, and never resumes. The expensive part is not the wasted runner time, it is that the lane went red silently. A job-level timeout CANCELS the running step rather than failing it, so continue-on-error did not apply, steps.client.outcome was 'cancelled' rather than 'failure', and the reporting step added in the previous commit was skipped: no warning annotation, no KNOWN FAILING summary, just a red lane with no explanation. continue-on-error rewrites a non-zero exit; it does nothing for a step that never exits. So the three bounds the two-VM lane already has: timeout-minutes: 10 on the step ConnectTimeout, ServerAliveInterval, on the ssh invocation ServerAliveCountMax sudo timeout 300 around the client binary The guest-side timeout is the one that matters for reporting. A 65536-byte transfer takes seconds, so five minutes means a hang, and timeout turns that hang into exit 124, which the existing `|| rc=$?` captures and `exit "$rc"` propagates. The step then fails rather than being cancelled, which is what continue-on-error rewrites and what steps.client.outcome == 'failure' fires on. Verified by simulating the path: exit 124, diagnostics run, reporting step reachable. The log also names the case -- "client hung and was killed after 300s" -- because the lane now has two failure signatures and they may not share a cause: an immediate `RDMA PUT failed (-1)` at 65536 bytes, and this silent hang after KFD VM acquisition. The KNOWN FAILING summary describes both and says how to tell them apart, rather than asserting the one we happened to see first. Written as `if [ "$rc" -eq 124 ]; then ...; fi` and not `[ ... ] && echo`: the guest heredoc runs under set -e, where a failing && list would abort the script on every run that is not a hang -- that is, on the normal failure path this lane is expected to take. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Stephen Bates <sbates@raithlin.com>
Summary
Replaces the single
ernic-integration-checklane with five hardware lanes thatrun hipObject against emulated hardware — rocm-ernic's ionic NIC and, for the GPU
lanes, rocjitsu's gfx1250 — over vfio-user, under QEMU, with nested KVM on the
runner. Along the way it fixes the RDMA data-plane defects that those lanes
exposed, several of which were making previously-green runs meaningless.
Important
Two lanes report
successwithout having transferred any bytes.GPU DirectandTwo-VM GPU Directcarrycontinue-on-erroron their client stepso an open bug does not block the rest of this work. Both emit a
Known failurewarning annotation and aKNOWN FAILINGjob summary when they fail.See Known failing before reading the green checks as
working GPU-direct RDMA.
The lanes
hipMalloc'd device bufferhipMalloc'd device bufferEvery lane asserts the bytes moved.
assert-transfer.shchecks therdma_put/rdma_get/rdma_bytescounters rather than the exit status,because the exit status was green for a data plane that was doing nothing. A
transfer that silently degrades to HTTP now fails the job.
Two-VM GPU Directadditionally asserts its own asymmetry — VM 1 must show no
1002:PCI function —so a guest that quietly grows a GPU fails rather than passing for the wrong
reason.
Data-plane fixes
Four defects, each of which independently made a green run meaningless:
buildRdmaTokenadvertisedmr->addr.ibv_reg_dmabuf_mrleaves thatNULL, and on the bounce path it is the staging buffer rather than thecaller's pointer, so the token described memory the peer could not use.
NIC cannot reach GPU memory directly, hipObject registers host staging instead
of the caller's pointer — and then left it empty. Adds
hipObjBufSync(
HIPOBJ_SYNC_TO_HOSTbefore a PUT,HIPOBJ_SYNC_TO_DEVICEafter a GET), ano-op for a directly registered buffer.
pointer in the
":<addr>:<len>"suffix, which rocm-ernic treats as an override.On the staging path that address is not covered by the MR, giving
LOCAL_PROT_ERRand a zero-byte transfer the bridge reported as success.init_dmabuf_support_flag()grepped/boot/config-$(uname -r)and disabled thewhole GPU-direct path when
CONFIG_DMABUF_MOVE_NOTIFY=ywas absent. Measured on7.2.4-070204-generic:
ibv_reg_dmabuf_mrresolves,hsa_amd_portable_export_dmabufsucceeds, and that config line is simply not present. One missing line was
switching off a path that works.
With those fixed,
HIPOBJ_REQUIRE_GPU_DIRECT=1makes the client refuse to fallback to host staging for a device pointer rather than return success for a
transfer the caller did not ask for. This is what turns the
GPU Directlanered — it was previously passing by faking.
Failures also say which failure. Every RDMA error used to surface as one anonymous
-1, rendered as500 RDMA PUT failed— six distinct faults, from an unparseabletoken to a completion error, indistinguishable from a server with no RDMA at all.
pollCompletionnow reports work-completion status, opcode andvendor_err.Test-server fixes
Three bugs, all found while chasing the GPU lanes, all of which cost time by
describing themselves as something else:
runThreaded()calledFD_SET(listen_fd_, &fds)unguarded, so a port alreadyin use did not report a busy port — it killed the process with a glibc fortify
abort naming no port.
main()printedlistening on port N— the line every caller waits for —without checking that the bind succeeded.
server for every client after it, each reporting
could not connect the RC QPand burying the real fault. This invalidated a size sweep before it was found.
Known failing
Both GPU lanes fail the actual transfer. They are non-fatal on the client step
only — every step before it stays fatal, so a regression in the build, the guest
boot, the NIC or the GPU enumeration still fails the job rather than hiding behind
the known bug.
Two-VM GPU Direct— the remote read of the client's dmabuf address failsimmediately at 1 MiB, well under the 16 MiB
IONIC_MESH_MAX_MSGcap:Deterministic in CI, and not reproducible on a two-guest lab that passes 8/8
with the same pinned binaries. Ruled out by measurement, not by argument:
rocjitsu build, QEMU build, guest memory, vCPU count, accelerator,
ip_discovery.bin(byte-identical), the message-size cap, and BAR placement. Theone component not yet swapped is
rocm-ernicitself — CI runs the pinnedernic.0b48aa1, the lab a locally built binary.GPU Direct—RDMA PUT failed (-1)at 65536 bytes, plus an intermittenthang with no output after the client acquires a KFD VM. It is not known whether
the two signatures share a cause, or whether either is the same bug as the
two-VM fault.
Neither is a regression:
Two-VM GPU Directhas never passed, andGPU Directwas green only because the client was staging through host memory and calling it
GPU-direct.
Commits
b3b2a4aci:the five lanes, assert-transfer, pinned image tags, guest sizing7175f8cfix(rdma):the four data-plane defects above76a300ffix(test):the three test-server bugs abovea1dee77ci:bound the GPU Direct client so a hang is reported, not eatenTest plan
a1dee77— with the caveat above that two ofthose greens are annotated known failures
S3 Backend,MinIO Bridge,Two-VM Protocolmove real bytes over theemulated NIC and pass
assert-transfer.shctest12/12 locally, including the test-server fixesReviewer notes
the measurement detail behind each change.
HIPOBJ_IONICnow defaults ON so the provider these lanes exercise isbuilt.
INSTALL.mdstill documents OFF for local lab workflows.STUFF.mdis retired; its AIStor package URLs move toansible/group_vars,where the roles actually read them.
continue-on-erroron both client steps, and the reporting stepsbeside them, is the first thing to do once the GPU-direct path works.
🤖 Generated with Claude Code