Repository navigation
Conversation
Collaborator
|
This won't automatically run in continuous integration without approval. A member of the Versity organization must allow it. |
potatogim
force-pushed
the
rc-v2-wrapper
branch
6 times, most recently
from
September 16, 2026 17:35
5c45806 to
1ef6c0c
Compare
Member
|
PR #32 only includes X-Amz-Rdma-Offset when offset is nonzero. Its normal GET/PUT calls use offset 0, so the header is absent. The local PREPARE route currently requires and parses that header unconditionally in routes_linux.go. Those requests will receive a 400. The local C++ reference parser in v2_request.cpp already treats offset as optional, defaulting to zero. The Go route should do the same. |
Member
|
Add the rdma build tags requirements to these if we need to bypass these in github CI: some staticcheck errors: And I guess we need to make sure ROCm/hipObject#32 is accepted to solidify this wire protocol contract. |
Contributor
Author
|
Sorry for the slow reply, and thank you for the detailed analysis. routes_linux.go now parses x-amz-rdma-offset as optional and defaults the offset to zero, matching the C++ reference parser in v2_request.cpp. I also added a regression test covering the omitted, explicit-zero, and malformed cases: the first two reach the session server with the same zero offset, and only a malformed non-empty value rejects with 400. Pushed in a8f6fdf. |
Contributor
Author
|
Thank you for the CI output as well, and sorry for the slow reply here too. The rcroutes test files and semantic_linux.go now carry the rdma build tag, so the github CI job skips them instead of failing on undefined symbols. For the staticcheck findings, the stub build of runV2Mode no longer returns an error. main now gates the -v2 mode behind a hasV2Mode constant and exits directly when the hipobj backend is absent, and the error handling moved into the hipobj-backed implementation (runV2ModeErr). This removes the always-true comparison itself rather than suppressing it, and staticcheck comes back clean on both the default and the github,rdma tag sets. go vet -tags=github ./... and the full rcroutes suite (with the rdma tag) both pass. And I agree we need ROCm/hipObject#32 accepted to solidify the wire protocol contract. |
potatogim
force-pushed
the
rc-v2-wrapper
branch
from
September 23, 2026 11:43
a8f6fdf to
cf402cb
Compare
hgichon
reviewed
Sep 27, 2026
hgichon
left a comment
hgichon
left a comment
Contributor
There was a problem hiding this comment.
Reviewed the #2391..#2394 range (13 files, +4195/-6) while validating the RC v2 data path against a native InfiniBand fabric.
Overall the design holds up well — fail-closed admission behind a real one-byte GET probe, the callback contract honoured, and the hipobj build tag keeping the default images buildable. The notes below are two correctness bugs plus a set of robustness and coverage points.
The two I would not merge without:
Makefile:133—-tags rdma -tags hipobjdropsrdma(Go resets the tag slice per flag; reproduced on go1.22.5), soHIPOBJ_ENABLED=1builds thecuobjclientstub and the default benchmark fails at runtime.control_linux.go:1150—targetOfconcatenates raw while the serverurl.PathUnescapes, so a key containing%addresses a different object and one containing?is misrouted.escapeKeyPathalready exists in the package for exactly this reason.
One more worth weighing: rdma/rcobj/control_test.go is behind cgo && hipobj, so the 1049-line control-plane suite never compiles on the CI images this PR is designed to keep green — go test ./rdma/rcobj/ currently runs 2 tests. That leaves the core logic of this PR untested by default.
Context for the environment these were found in: gateway on Rocky 8.10 with inbox rdma-core (no MOFED), ConnectX-6 native IB 200G HDR next to a Broadcom RoCE NIC on the same host, client on Rocky 9. Happy to re-test any of this here once the fixes land.
potatogim
force-pushed
the
rc-v2-wrapper
branch
from
September 27, 2026 15:27
cf402cb to
79cda3b
Compare
Contributor
Author
|
Thank you for the careful review, and for running this on native InfiniBand. I have exercised the data path on ConnectX-5 as well. The two merge concerns (the This wrapper links libhipobj's v2 API ( |
potatogim
marked this pull request as draft
September 27, 2026 16:11
The RC route handler dropped the target's query string when it split the request target, so part-upload and part-read requests had nowhere to carry uploadId and partNumber. Target parsing now preserves the raw query alongside the decoded bucket and key. On top of that, the routes learn the multipart semantics they were missing: semantic validation requires uploadId and partNumber for part uploads and only partNumber for part reads, rejecting a stray uploadId on a read; dispatch passes uploadId and partNumber to UploadPart and dispatches part reads through GetObject with PartNumber; the configured multipart part limit flows from the vgwrdma flag through the route constructor (stub kept in sync) with the standard 10000 fallback; and successful READY responses, plus error responses for requests where a native session was created, advertise multipart part-transfer support with the mp capability token, defined alongside the other wire constants. Operational records keep the raw query and distinguish UploadPart metrics. Part of versity#2388 Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
Static analysis flagged an unused test fixture field, and the part number narrowed to int32 without a bound check at the two dispatch sites; the flag has no upper-bound validation, so a configured limit above MaxInt32 could overflow the conversion. Check the range before both conversions. The overwrite/object-lock check ran for part uploads too, but a part writes into an open multipart upload rather than the object; the REST UploadPart controller performs no such check, so restrict the check to plain PUTs. A part read whose backend length exceeded the announced size was silently truncated: the staging read stops at the buffer and the native staged-length equality then matches the truncated count. Reject the mismatch against the response content length, and correct the stageGet contract comment, which described a short-part behavior the native equality check never implemented. The audit synthesis glued the target query onto the synthesized path, which the access logger splits into bucket and key, so a part record showed an object key like obj?partNumber=3. Keep the path canonical and carry the query on the request URI instead. PREPARE failures after semantic parsing also published the bare transport op, recording an UploadPart denial as a plain PutObject; publish the parsed semantic identity on those paths. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
The part-length guard only compared the declared content length, so a backend response without one could still overflow the staging buffer unnoticed: ReadFull stops at the buffer, the native staged-length equality then matches the truncated count, and the client receives fewer bytes than the part holds. Probe one byte past a completely filled buffer with io.ReadFull semantics, so a transient zero-byte read does not hide the extra byte and a probe error other than EOF is returned. A short part with a mismatching declared length is rejected up front. Cover both paths in the dispatch tests: declared-length mismatch, probe-detected overrun, a transient zero read before the extra byte, and an exact part without a declared length. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
The rcroutes tests and semantic_linux.go were tagged cgo-only but reference symbols defined behind the rdma tag (errRouteBadRequest, opsEmitter, Handler), so the staticcheck CI job failed with undefined-symbol errors when it analyzed the package without the rdma tag. Add the rdma requirement to those files so the job skips them. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
COMPOSITE multipart uploads require a checksum on UploadPart. The RC session does not carry client checksum headers, so the part hash is computed from the received bytes using the algorithm ListParts reports. A part GET whose backend size does not match the announced length is a client/object error and now maps to 400 InvalidRdmaRequest instead of 500 InternalError. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
The Versity copyright line has no corporate suffix, so these notices should not add one either. No functional change. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
The Versity copyright line names the project, not an individual. The personal name stays in Signed-off-by. No functional change. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
potatogim
force-pushed
the
rc-v2-wrapper
branch
from
October 21, 2026 08:37
900441a to
fc3ec25
Compare
COMPOSITE multipart uploads may use XXHASH64, XXHASH3, or XXHASH128. The RC part path left those checksum fields empty, so UploadPart rejected the part after the bytes had already moved. Map the three algorithms and set the matching UploadPart fields. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
…llbacks Bind libhipobj through cgo: init/shutdown, buffer registration, and the GET/PUT entry points hand the library a callback table whose PREPARE, READY (split request/response), and CANCEL exchanges are implemented here with net/http and the gateway SigV4 signer. The READY exchange keeps a dedicated connection whose request bytes are flushed by sendReadyRequest while the RDMA data phase runs; the response is read by finishReady under the remaining budget, and an aborted exchange closes the connection instead of pooling it. Control dials bind the netdev backing the data plane's selected GID so multi-port devices never split planes across interfaces. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
…acking The eligibility layer probes the endpoint through the real API, the PREPARE callback records the capability advertisement for callers, and every transport replacement (pool rotation, redirect, redial) turns admission negative until a fresh probe succeeds, so no admitted bytes flow on an unproven connection. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
The v2 flow now runs on libhipobj: buffer registration, PUT/GET, Range GET, and the multipart flow go through rcobj, with the REST fallback preserved for endpoints that decline v2. The previous in-tree client completion-slot plumbing and its admission-window test leave with it. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
The suite pins the wire contract on a scripted fake endpoint: the PREPARE headers and SigV4 signing, the split READY exchange whose request is flushed before the response exists, abort closing a pending exchange instead of pooling it, zero-budget fail-fast, the CANCEL round trip, https rejection, and the fail-closed admission transitions on connection replacement. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
Bound every copy into the fixed C reply arrays by capacity and reject oversized protocol fields instead of spilling into adjacent struct memory; the CRC64NVME checksum is validated and only its 12-character base64 payload is stored. Parse the PREPARE token after the colon the bridge and gateway emit. Abort the pending READY exchange when CANCEL runs without a prior finish read, so its socket is closed rather than pooled. Report body-read failures instead of discarding them. Sign and transmit the Host authority named by the endpoint rather than the resolved IP. Hold an in-flight reference across Get/Put/register/deregister so Shutdown stops admission, waits for the calls inside C, and only then frees the callback slot; a failed shutdown stays retryable because the library kept its state. The CLI performs the real-API admission probe before any transfer, addresses multipart parts on the upload's own object, submits server ETags from the parts listing, verifies the exact assembled bytes, keeps buffers alive when deregistration fails, and runs a real REST operation in the NotSupported fallback. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
Move the multipart and Range verification compares before any buffer release, so a mismatch is never read out of freed memory. Gate every entry point on the closed check and inflight count under one lock, and serialize Shutdown with a condition variable so concurrent callers observe the actual teardown outcome. Wire the admission layer into the control plane: the probe is now a signed one-byte GET of a configured object, redirects are not followed, and PREPARE refuses to touch the wire while admission is negative. Tighten the checksum and reply-token parsing to the bridge contract, and pin both with tests. Allocate the CLI transfer buffers as device memory through the HIP runtime the library already links, with explicit H2D/D2H staging, since v2 transfers require a device-backed MR. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
Publish closed and the shutdown outcome together under the client mutex, and hand waiters the recorded result of the attempt they joined instead of letting them retry a failed teardown. Install the configured probe object into the control plane, issue the probe as a signed ranged GET of /<bucket>/<key> with the method carried through the SigV4 signing and request line, and treat a plain 2xx object answer as positive evidence: protocol negotiation stays with PREPARE so a legacy gateway can still decline with NotSupported and fall back. Bind admission to the callback budget: the probe shares the one absolute deadline, an exhausted budget fails before any probe, and redirected control exchanges invalidate the evidence so the next PREPARE re-probes. Keep the multipart and Range REST fallbacks out of the expected payload: the fallback result is verified on its own, and the device buffer is staged down only when the v2 path produced the bytes. Release early-exit device allocations through the checked cleanup in reverse order, and reject checksum padding bits the bridge would not have produced. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
Run the admission probe under the callback's absolute deadline: the probe context carries the same cutoff PREPARE uses, and a probe invoked without one fails closed instead of granting itself time the transfer does not have. Give every Shutdown waiter the outcome of the exact attempt it joined: waiters pin the in-progress attempt object and read its immutable result, so a retry started after a failure cannot substitute its outcome for theirs. Carry the selected NIC, port, and GID into the admission probe so its evidence reflects the interface that will carry the transfer, and make dialControl honor the bridge contract: fail the dial when the selected netdev or its address cannot be resolved, and build the per-exchange dialer so a failed lookup cannot inherit a stale binding. Invalidate admission evidence where the transport breaks: redirect invalidation now follows the status line before any fallible body work, and a refused dial on any control exchange turns the layer negative until a fresh probe. Move the hipobj error-code constants to a build-tag-free file so external callers can compare OpError codes under the stub build, correct the probe-object comment to say fail-closed rather than disabled, join cleanup errors into early-exit diagnostics instead of discarding them, and pin the checksum padding-bit rule with valid and invalid fixtures. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
Wait on the attempt-local completion channel instead of the global shutdown flag: a waiter whose teardown attempt has finished returns its outcome immediately, and retries by other callers can no longer delay it. The channel close orders the recorded result before every waiter's read, and the now-unused condition variable is gone. Invalidate admission on real connection loss during an admitted exchange: write, response-read, and body-drain failures on PREPARE, READY, and CANCEL now drop the cached evidence, while deadline expiry stays an intentional local abort that keeps the evidence. CANCEL additionally checks the response status, so a redirect on the cleanup exchange invalidates like any other. Snapshot the probe object and transport selection together under the probe mutex so the request the probe sends is exactly the one the triggering PREPARE configured, and document the apiLock serialization dependency the wrapper rides on. Join cleanup errors everywhere the transfer error could previously hide a failed release: part deregistration and free, verify registration, and the final release sequence all surface both failures. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
Classify transport loss by error identity rather than by when it is observed: connLost treats the typed deadline errors as intentional local aborts and everything else - resets, EOFs, broken pipes, truncated bodies - as a lost connection, so an ordinary failure with budget remaining no longer keeps admission positive. Dial failures follow the same rule instead of invalidating unconditionally, and the redirect checks share the invalidate helper. Pin admission evidence to the peer that answered the probe: the probe records the remote address of its connection, and READY compares its freshly dialed peer against that pin before sending, failing closed and re-probing when a re-resolution landed elsewhere; the PREPARE dial gains the same check in the commit that follows. Add a truth-table test for the loss predicate and pin the test control plane's peer the way a successful probe would, so the wire tests exercise the pinned-peer path. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
Enforce the pinned peer on PREPARE as well: the freshly dialed socket is compared against the probe's pin before any request is written, so a re-resolution that landed on a different server fails closed and re-probes instead of sending an admitted request to the replacement. A regression test asserts the replacement never receives PREPARE bytes and admission goes negative. Carry IPv6 endpoints through the CLI and wrapper dial path: the control endpoint is rebuilt with net.JoinHostPort so bracketed literals survive, and the selected interface binds a source address whose family matches the destination (zone attached for link-local IPv6), where previously any non-IPv4 destination with a selected NIC failed the dial outright. Extend the loss-classification coverage with wrapped deadline errors, net.OpError chains, and the EOF sentinels. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
Resolve the control authority the way a normal dial would: a hostname or scoped literal expands into destination candidates under the callback deadline, and the NIC-bound dial binds a same-family source for each candidate, retrying across families so an unreachable route falls back instead of failing outright. Give the standalone HTTPProbe a real identity: the probe path now takes the bucket and key it should read, refuses redirects itself rather than trusting the caller's client, and stamps each successful probe with a fresh generation so a later connection change still invalidates the evidence. Pin the REST fallback verification to exact lengths: the ranged read asserts the declared content length matches the expectation and the full GET rejects a body longer than expected, where both previously compared only a prefix and would pass against a longer object. Rebuild the mismatched-peer regression test around the production valve: admission is established by a real signed probe, the mismatch sends zero PREPARE bytes and turns the layer negative, and the next transfer re-probes through the valve and recovers, with the fake object API now answering the probe object. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
Divide the remaining dial budget evenly across the resolved destination candidates, so a blackholed route cannot consume the whole callback deadline before the other family is tried; each attempt carries its own share as both the dial timeout and the per-attempt context, and the loop stops once the shared budget is gone. Select the IPv6 source by destination scope: a routable destination binds a routable (global) source and only falls back to the link-local address when the interface offers nothing else, where previously the first family match (often link-local) was bound regardless, which cannot carry a routed connection. Escape the public probe path like the signed probe already did: object keys are names, not URL syntax, so the shared per-segment escaping now lives in the tag-independent file and both probe builders use it, keeping ? # and literal percent sequences from addressing a different object. Order probe evidence against invalidation: the eligibility layer counts connection changes, snapshots the count before probing, and discards a successful result whose count moved, so an in-flight probe can no longer re-admit a transport that was replaced while the probe was running. Count raw bytes per connection in the fake object API and assert the mismatched-peer rejection from a byte-delta snapshot, so even an unparseable partial write would fail the test. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
Bind the control socket to the selected interface with SO_BINDTODEVICE and let the kernel pick the source address, the same policy the reference bridge uses. The previous first-match source selection cannot see routing domains and would bind a ULA or link-local source for a globally routed destination, which the return route cannot reach; the kernel applies source selection per candidate while the device pin keeps the traffic on the selected interface. The unused address-picker is gone. Skip the empty trailing multipart part: the rounding in the two-part split produced a zero-length second part for size 1, whose empty device buffer and zero-size transfer the core rejects before PREPARE, leaving a supported CLI input unable to complete. The part loop now follows the actual part list and computes the last part's end from it. Reject a stale probe result before it touches any state: the invalidation-epoch check moved ahead of the failure publication, so an old probe that finishes after a newer probe published (or after a connection change) returns an error instead of overwriting the current evidence. Synchronize the mismatched-peer zero-byte assertion with the fake's connection handlers: each handler exposes a completion channel and the test waits for every connection opened after the snapshot to finish reading before asserting, so bytes still in flight when the valve drops the socket can no longer hide. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
Fall back to the interface's IPv4 address binding when the kernel refuses SO_BINDTODEVICE for lack of CAP_NET_RAW, the same degradation the reference bridge ships: kernels before 5.7 deny the option on every socket for unprivileged processes, and the previous revision returned the error on every candidate, so a working IPv4 control path failed its first admission probe. The device binding remains the primary path and kernel source selection stays in effect wherever the option is permitted. Register accepted connections in the fake object API before dispatching their handlers, so the mismatched-peer test waits for the post-snapshot connection to be accepted and its handler to finish reading before asserting zero bytes; a dial completing before Accept can no longer let the assertion pass while the rejected connection's bytes are still in flight. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
Separate fallback source discovery from per-candidate retry eligibility: the interface's IPv4 address is looked up at most once per dial (a failed lookup is remembered too), but every IPv4 candidate whose device-bound attempt is denied may still use the fallback, where the previous one-shot guard let a refused first destination disable the fallback for a later reachable one. Track observed connections in the mismatched-peer test by identity: the assertion now waits for the post-snapshot connection to be accepted and its handler to finish reading before checking its final byte count, so a dial completing before Accept can no longer let the zero-byte claim pass unobserved. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
…istically Correct three comments that described behavior the code does not provide: the control callback returns EPERM for the dial loop instead of swallowing it, the probe object is configured at client construction rather than through a later setter, and only size 1 produces a single padded multipart part. Cover the device-binding fallback deterministically: bind the netdev lookup, the control callback, and the source lookup to test-local replacements, then exercise the EPERM fallback for an IPv4 destination, a refused destination followed by a reachable one, an interface without an IPv4 source, and the shared-deadline budget on a refused destination. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
Rebuild the device-binding fallback tests around a single dial with ordered candidates: the resolver hook returns a refused first candidate and a reachable second one inside one invocation, so the per-candidate fallback eligibility and the exactly-once source discovery are observable directly, instead of across two independent dials whose local state reset hid the difference. Pin the source binding with an exact-address assertion on the returned connection's local address and a source-lookup counter, assert the EPERM identity in the no-source path together with the remembered failed discovery, and gate the IPv6 case on a zero lookup count so cached IPv4 state cannot silently produce an IPv4-source retry. Replace the fixed port-1 destination with a reserved-then-closed ephemeral port, removing the environmental assumption, and drop the stale even-division sentence from the multipart comment. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
…inating Make the source-binding assertion discriminating: the injected discovery now returns 127.0.0.2, a loopback address an unbound dial to 127.0.0.1 never selects, so equality on the returned connection's local address proves the explicit LocalAddr binding rather than ordinary route selection. Record the destination addresses the injected control callback saw, and assert candidate order and count in the ordered-candidate tests, so skipping or reordering the first candidate fails instead of passing silently. Consume the live-accept observation with a short poll in the refused-then-reachable test as well, matching the budget test, and create that test's live listener before reserving the dead port so the reservation cannot return the port the listener just took, collapsing both candidates into one. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
…rvation Create the budget test's live listener before reserving the dead port, so the reservation cannot return the port the listener just released and collapse both candidates into one; with the recorded attempt-count assertion that collapse turned into a false failure instead of a silent pass. Restrict the test's comment to multi-candidate prompt completion and state that deadline expiry and per-share cancellation remain unverified on loopback. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
The cgo directives hard-coded a development include path for the hipobj headers and linked -lhipobj with no library search path. cgo does not expand custom variables in #cgo lines, so the paths cannot be parameterized there. The binding relies on the standard CGO_CFLAGS/CGO_LDFLAGS environment injection instead, and the Makefile passes HIPOBJ_INC_DIR/HIPOBJ_LIB_DIR through the cuobjtest-gpu target the same way the CUOBJ_* paths flow. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
The rebase onto the routes branch dropped the -v2 flag and the runV2Mode dispatch, leaving the rcobj-driven mode file without an entry point. Restore both so the flag reaches the wrapper flow. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
The cgo binding requires libhipobj, which is not packaged for the CUDA-based cuobjtest builder image. Gate the cgo files, the v2 CLI mode, and the Makefile wiring behind a hipobj build tag selected by HIPOBJ_ENABLED: the default build compiles the non-cgo stub and the v2 flag reports the missing binding, so the existing CI images keep building, while a hipobj-provisioned environment links the real client with HIPOBJ_INC_DIR/HIPOBJ_LIB_DIR. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
…ck findings The PREPARE route rejected requests without x-amz-rdma-offset even though clients omit the header when the offset is zero, so plain GET/PUT requests from those clients failed header validation. Parse the header as optional and default the offset to zero, matching the C++ reference parser in v2_request.cpp, and cover the omitted, explicit-zero, and malformed cases with a regression test. The cuobjtest -v2 flag reported its stub-build error through a return value staticcheck proves is never nil on stub builds. Gate the mode behind a hasV2Mode constant and move the error handling into the hipobj-backed implementation so the stub path exits directly. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
Keep both the rdma and hipobj build tags on the HIPOBJ_ENABLED cuobjtest target so Go does not drop rdma when a second -tags flag appears. Escape object keys in the PREPARE/READY target the same way the admission probe already does. Narrow transport-loss detection so local failures (HTTPS mismatch, deadlines) do not drop admission evidence, pin multi-address endpoints to the resolved candidate set, reject an empty resolver result instead of dividing by zero, and refuse a second READY before the request is written. Malformed cookie echoes are reported as absent rather than a mismatch. Config.Region is used for SigV4 when Credentials.Region is empty. C string copies take the destination array size from the struct. The v2 CLI no longer labels a failed transfer as a byte mismatch. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
The Versity copyright line has no corporate suffix, so these notices should not add one either. No functional change. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
The Versity copyright line names the project, not an individual. The personal name stays in Signed-off-by. No functional change. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
A multi-address authority stored every resolved address as admission evidence, so a later dial could send a control request to a peer the probe never reached. Keep the answering address as the only pin, and drop admission when a control dial lands elsewhere. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
The binding calls hipMalloc, hipFree, and hipMemcpy itself. libhipobj does not re-export those symbols, and a static archive does not carry its HIP and HSA dependencies, so the link line names them. HIP_LIB_DIR selects the runtime search path. Signed-off-by: Jihyeon Gim <potatogim@potatogim.net>
potatogim
force-pushed
the
rc-v2-wrapper
branch
from
October 21, 2026 09:28
fc3ec25 to
bc230e5
Compare
Member
|
@potatogim v3 protocol proposal out in PR in hipObject repo: ROCm/hipObject#207 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2388. Builds on #2391.
Adds the Go side of the RC v2 data path: a cgo binding that links libhipobj (the hipobj-rc-v2 client library, ROCm/hipObject#32) and drives it from Go. The library owns the RDMA data plane and the transfer state machine; this wrapper implements the four control-plane callbacks (PREPARE, READY request/response, CANCEL) with net/http and the gateway SigV4 signing, per the hipobj.h callback contract, so control traffic reaches the RC routes from #2391 through the standard request path.
Admission is fail-closed: a one-byte GET probe through the real API must succeed before any v2 transfer, capability headers are captured by the PREPARE callback, and admission turns negative on connection replacement or redirect until a fresh probe succeeds. The control dials pin the peer that answered the probe and classify transport loss by error identity, not timing.
cuobjtest gains a -v2 mode that exercises the wrapper end to end. The cgo binding is opt-in behind a hipobj build tag wired through HIPOBJ_ENABLED in the Makefile: the default build compiles a non-cgo stub whose entry points return a not-linked error, so the existing CI images keep building without libhipobj, and a hipobj-provisioned environment links the real client with HIPOBJ_INC_DIR/HIPOBJ_LIB_DIR.
Tests cover the control-plane wire exchanges against an in-process fake (pinned-peer enforcement, transport-loss classification, dial fallback ordering, budget split), the admission layer (probe fail-closed, connection-change and redirect invalidation), and the stub contract.
Note: stacked on #2391; the changes added by this PR are the commits after the #2391 head.