feat: canonical ODP wire format in mctp-rs + odp-client crate#868
Closed
dymk wants to merge 16 commits into
Closed
feat: canonical ODP wire format in mctp-rs + odp-client crate#868dymk wants to merge 16 commits into
dymk wants to merge 16 commits into
Conversation
Eliminates the method-resolution footgun where callers writing msg.serialize(buf) on an owned OdpMessage would silently invoke the body-only MctpMessageTrait::serialize instead of the inherent header+body serializer. The inherent method is now serialize_with_header, making caller intent explicit at every call site. Also drops the leading underscore on MctpMessageTrait::deserialize's header parameter, which was misleading because the parameter is used.
The new odp-client/ crate wraps mctp-rs and presents an OdpTransport-based API. This commit creates the package, registers it in workspace.members, and adds empty stub files for the modules that subsequent tasks will fill in (transport, client, error, mctp_serial, loopback, server). The lib.rs re-exports the canonical ODP wire-format types from mctp_rs::odp so consumers don't import mctp-rs directly.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR moves the ODP MCTP wire format into mctp-rs and introduces odp-client as the new no-std client/relay abstraction, while migrating in-tree relay users away from embedded_services::relay.
Changes:
- Adds
mctp-rsODP message/header support behind anodpfeature. - Adds the
odp-clientcrate with transport, client, loopback, serial transport, serialization traits, and relay macro support. - Migrates battery, thermal, time-alarm, debug, UART, eSPI, and example code to
odp-client, then removes the old relay module fromembedded-service.
Reviewed changes
Copilot reviewed 42 out of 44 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
Cargo.toml |
Adds odp-client and embedded-io to workspace configuration. |
Cargo.lock |
Updates dependency graph for the new crate and migrations. |
.github/copilot-instructions.md |
Updates relay pattern docs to reference odp_client::server. |
mctp-rs/Cargo.toml |
Adds the odp feature. |
mctp-rs/src/message_type/mod.rs |
Exposes the new ODP message module behind the feature. |
mctp-rs/src/message_type/odp.rs |
Defines canonical ODP wire-format types and tests. |
odp-client/Cargo.toml |
Defines the new client crate and dependencies. |
odp-client/src/lib.rs |
Re-exports client, transport, wire-format, and relay APIs. |
odp-client/src/client.rs |
Implements sync OdpClient request/response invocation. |
odp-client/src/error.rs |
Defines the unified OdpError surface. |
odp-client/src/loopback.rs |
Adds an in-memory test transport. |
odp-client/src/mctp_serial.rs |
Adds MCTP-over-serial transport implementation. |
odp-client/src/serializable.rs |
Moves serializable message/result traits. |
odp-client/src/server.rs |
Moves/renames relay traits and macro. |
odp-client/src/transport.rs |
Defines the byte-oriented transport trait. |
odp-client/tests/* |
Adds integration coverage for client, transport, loopback, serial, and errors. |
embedded-service/* |
Removes the old relay module and dropped dependencies. |
battery-service-relay/* |
Migrates relay serialization and handler traits to odp-client. |
thermal-service-relay/* |
Migrates relay serialization and handler traits to odp-client. |
time-alarm-service-relay/* |
Migrates relay serialization and handler traits to odp-client. |
debug-service/* |
Migrates debug relay handler traits to odp-client. |
debug-service-messages/* |
Migrates message serialization traits to odp-client. |
uart-service/* |
Migrates relay handler imports and dependency wiring. |
espi-service/* |
Migrates relay handler bounds/imports and dependency wiring. |
examples/rt685s-evk/* |
Migrates example macro usage and dependencies. |
Comment on lines
+82
to
+87
| if resp_header.message_id != header.message_id { | ||
| return Err(OdpError::UnexpectedMessageId { | ||
| expected: header.message_id, | ||
| got: resp_header.message_id, | ||
| }); | ||
| } |
Comment on lines
+89
to
+92
| parse(OdpResponse { | ||
| header: resp_header, | ||
| body: &self.buf[4..n], | ||
| }) |
Comment on lines
+131
to
+163
| // 1. Read serial frame (bytes until 0x7E) into rx_frame_buf. | ||
| let mut filled = 0usize; | ||
| loop { | ||
| if filled >= self.rx_frame_buf.len() { | ||
| return Err(OdpError::BufferTooSmall); | ||
| } | ||
| let mut byte = [0u8; 1]; | ||
| match self.uart.read_exact(&mut byte) { | ||
| Ok(()) => {} | ||
| Err(ReadExactError::UnexpectedEof) => return Err(OdpError::Transport), | ||
| Err(ReadExactError::Other(_)) => return Err(OdpError::Transport), | ||
| } | ||
| self.rx_frame_buf[filled] = byte[0]; | ||
| filled += 1; | ||
| if byte[0] == SERIAL_END_FLAG { | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // 2. MCTP-strip via a fresh PacketContext borrowing assembly_buf. | ||
| let mut rx_ctx = MctpPacketContext::<MctpSerialMedium>::new(MctpSerialMedium, &mut self.assembly_buf); | ||
| let message = rx_ctx | ||
| .deserialize_packet(&self.rx_frame_buf[..filled]) | ||
| .map_err(|_| OdpError::Decode)? | ||
| .ok_or(OdpError::Decode)?; | ||
|
|
||
| // 3. Copy out the MCTP body (= ODP header+body wire bytes). | ||
| let body = message.message_buffer.body(); | ||
| if buf.len() < body.len() { | ||
| return Err(OdpError::BufferTooSmall); | ||
| } | ||
| buf[..body.len()].copy_from_slice(body); | ||
| Ok(body.len()) |
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.
Consolidates three duplicate copies of the ODP MCTP wire format and relay scaffolding into two new homes:
mctp-rsgains anodpfeature exposingmctp::message_type::odpwith the canonical wire format:ODP_MESSAGE_TYPE = 0x7D,OdpServiceenum,OdpHeaderBE bitfield,OdpMessage<'a>MctpMessageHeaderTraitandMctpMessageTrait<'buf>New
odp-clientcrate (no_std) wraps the wire format so consumers depend only onodp-client, never onmctp-rsdirectly:OdpTransport— pure byte-message trait (send_message/recv_message); noMctpMediumleakageMctpSerialTransport<U: embedded_io::Read + Write>— concrete impl that encapsulates MCTP serial framing internallyLoopbackTransport— in-memory test double backed byheapless::VecOdpClient<T: OdpTransport>+Relaytrait — sync client with closure-basedinvokethat hands the caller a borrowed response sliceOdpError—Copy/Eqerror type with structuredUnexpectedMessageId { expected, got }variantimpl_odp_relay_handler!macro — async server-side relay handler (moved verbatim fromembedded-service::relay, renamed fromimpl_odp_mctp_relay_handler!)SerializableMessage/SerializableResulttraits — moved fromembedded-service::relayDeletes
embedded-service/src/relay/entirely and dropsbitfield,paste, andmctp-rsfromembedded-service/Cargo.toml. The macroimpl_odp_mctp_relay_handler!is renamed toimpl_odp_relay_handler!and lives atodp_client::impl_odp_relay_handler. All in-tree consumers (espi-service, uart-service, thermal-service-relay, battery-service-relay, time-alarm-service-relay, debug-service, debug-service-messages, examples/rt685s-evk) are migrated in this PR.This is a clean break: no shims, no
#[deprecated], no backwards-compat layer. Consumers (SP, ec-test-cli, EC firmware) will adoptodp-clientin follow-up PRs in odp-secure-services, odp-platform-common, and odp-platform-qemu-sbsa.Verification:
cargo build --workspace✅cargo test --workspace✅ (all suites green, includes 15 new odp-client integration tests + 13 mctp-rs odp wire-format tests)cargo clippy --workspace --all-targets -- -D warnings✅cargo fmt --all -- --check✅cd examples/rt685s-evk && cargo build --bin time_alarm✅ (exercises the relocated macro end-to-end)