OpenDevicePartnership / OpenDevicePartnership/embedded-services
[mctp-rs] MctpSerialMedium: stuff header bytes (revision + byte_count) for strict DSP0253 conformance
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 23
- Forks
- 53
- Avg merge
- 2d 20h
- Merged PRs (30d)
- 13
Description
From PR #844 review (Copilot):
Serial
serializewritesbyte_countinto the header without applyingSerialEncodingstuffing. Sincebyte_countcan legitimately be 0x7D or 0x7E (125/126 decoded bytes), this can inject a raw escape/flag byte into the header on the wire, which is likely to break framing for receivers that use 0x7E delimiters. Consider stuffing the header bytes too (and decoding them correspondingly) or otherwise guaranteeing these values cannot occur on the wire.
Location: mctp-rs/src/medium/serial.rs:278
Deliberate design (today): the in-tree code carries the upstream author's decision to mirror SmbusEspiMedium's pattern of emitting the 2-byte header (SERIAL_REVISION, byte_count) raw. This is captured in an inline source comment:
Header: revision + byte_count emitted directly (NOT stuffed), matching SmbusEspiMedium's header pattern. ... when byte_count happens to equal 0x7E or 0x7D — round-trips cleanly through this implementation's deserialize.
Strict-conformance gap: DSP0253 §7.1 specifies that ALL bytes between flag delimiters (0x7E) must be byte-stuffed. The current implementation:
- Self-roundtrips correctly (the in-tree
MctpSerialMedium::deserializereads the 2 header bytes raw, matching serialize). - Mis-frames against strict-DSP0253 receivers when
body_wire_len ∈ {125, 126}— a raw0x7Dor0x7Ewould be interpreted as escape or end-flag respectively.
Practical exposure: the serial feature is opt-in and no embedded-services consumer enables it today. The exposure surfaces only when (a) serial is enabled AND (b) the peer is a strict-DSP0253 implementation other than this one.
Fix sketch: byte-stuff the 2-byte header via SerialEncoding::write_byte (matching the FCS pattern at the bottom of serialize) and un-stuff in deserialize. Update the inline source comment + golden fixtures.
Scope: out of scope for the source-bump PR #844 (verbatim port of upstream design); follow-up against the in-tree source.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in mctp-rs/src/medium/serial.rs around line 278 and inspect MctpSerialMedium::serialize and deserialize, including the existing SerialEncoding::write_byte use for the FCS. Update header encoding and decoding so all header bytes are stuffed consistently, then update the inline comment and golden fixtures; done means strict DSP0253 framing works for byte counts 125 and 126.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 65/100