mgmt/mcumgr/transport: uart: assert UART MTU fits the netbuf#2
Draft
JPHutchins wants to merge 2 commits into
Draft
mgmt/mcumgr/transport: uart: assert UART MTU fits the netbuf#2JPHutchins wants to merge 2 commits into
JPHutchins wants to merge 2 commits into
Conversation
The mcumgr UART transport reassembles the base64-decoded SMP frame into a single CONFIG_MCUMGR_TRANSPORT_NETBUF_SIZE buffer, so CONFIG_MCUMGR_TRANSPORT_UART_MTU cannot exceed it. Kconfig.uart documents this relation (MCUMGR_TRANSPORT_UART_MTU <= MCUMGR_TRANSPORT_NETBUF_SIZE + 2) but it was only prose; a violation was silently broken at runtime. Enforce it with a BUILD_ASSERT, mirroring the existing UART_MTU != 0 check. Default configs satisfy it (256 <= 386). Signed-off-by: JP Hutchins <jp@intercreate.io> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Design/analysis doc for review; drop before any upstream submission. Signed-off-by: JP Hutchins <jp@intercreate.io> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Summary
The mcumgr UART SMP transport documents
MCUMGR_TRANSPORT_UART_MTU <= MCUMGR_TRANSPORT_NETBUF_SIZE + 2inKconfig.uartas a "must satisfy" relation, but onlyBUILD_ASSERT(UART_MTU != 0)is actually enforced. Since the transport reassembles the base64-decoded SMP frame into oneNETBUF_SIZEbuffer, aUART_MTUlarger than the netbuf can never be used — yet a misconfiguration is silent until runtime. This adds the missingBUILD_ASSERT, mirroring the existing one.smp_uart.cdepends on UART_MCUMGR && !UART_MCUMGR_RAW_PROTOCOL, so all referenced symbols are always defined here.Context
Found while investigating what an SMP serial client may send (mcu-tools/mcuboot#2746, intercreate/smpclient#81). The advertised/
buf_sizevalue is the decoded netbuf; the on-wire frame is ~1.37× larger (base64 + line framing), decoded fragment-by-fragment into the netbuf — so the netbuf is the real per-transaction ceiling. Confirmed empirically against native_sim/QEMU/mps2 servers (aNETBUF_SIZE − 4message round-trips while the encoded frame exceedsNETBUF_SIZE).Open questions (intentionally out of this PR — see the plan doc commit)
RX_BUF_COUNT * RX_BUF_SIZE >= UART_MTU, looks like a throughput heuristic rather than a hard requirement (fragments are decoded + recycled incrementally), so asserting it could reject configs that work today. Left for discussion.UART_MTU'sget_mtuis never called on the UART path; the help text ("sent and received") may warrant clarification.