Repository navigation
Encode RDMA tokens little-endian explicitly and test the wire layout - #184
Merged
Merged
Conversation
encodeRdmaTokenHex() and decodeRdmaTokenHex() copied each field with memcpy at a hand-tracked offset, so integers went on the wire in the host's byte order, and the sizes 4, 8, and 2 were written out by hand. docs/interop.rst defines the integers as little-endian, which only matched because every supported host is little-endian. - Add TokenWriter and TokenReader, which write and read the token's binary form front to back over a std::span. Integers are encoded little-endian with shifts, so the bytes don't depend on the host, and each call advances the span, which replaces the offset bookkeeping. With _GLIBCXX_ASSERTIONS, an overrun aborts. - Derive kTokenBinaryLen from the sizes of RdmaToken's fields, so a field type change fails the static_assert against kRdmaTokenHexLen instead of silently changing the wire format. - Add golden-vector tests that encode and decode a token with a different value in every byte against the full 88-character string from the layout in docs/interop.rst. The existing round-trip tests pass with any self-consistent layout, so they couldn't catch swapped fields, changed widths, or the wrong byte order. - docs/interop.rst now says the fields are packed with no padding and the integers are little-endian whatever the host's byte order. RdmaToken's comment points at that layout. The bytes on the wire are unchanged on every supported platform, so there's no changelog entry. Co-authored-by: Claude <noreply@anthropic.com>
derobins
requested review from
akolliasAMD,
gaoikawa,
jordan-turbofish,
kurtmcmillan,
riley-dixon,
sbates130272 and
zbyrne
as code owners
October 13, 2026 10:24
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The implementation matches the documented layout and is covered by encoding and decoding golden-vector tests.
0 open findings
What changed in this PR
This PR makes RDMA token serialization explicitly little-endian and verifies the documented wire format.
Changes:
- Adds span-based token readers and writers.
- Adds golden-vector encoding and decoding tests.
- Clarifies wire-layout documentation.
| File | Description |
|---|---|
src/rdma/token.cpp |
Implements explicit little-endian serialization. |
src/rdma/token.h |
Documents the token’s wire-layout relationship. |
test/unit/test-rdma-token.cpp |
Adds golden wire-format tests. |
docs/interop.rst |
Clarifies packing and byte order. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
encodeRdmaTokenHex() and decodeRdmaTokenHex() copied each field with memcpy at a hand-tracked offset, so integers went on the wire in the host's byte order, and the sizes 4, 8, and 2 were written out by hand. docs/interop.rst defines the integers as little-endian, which only matched because every supported host is little-endian.
The bytes on the wire are unchanged on every supported platform, so there's no changelog entry.