Harden RSSI RX FSM filtering and payload timing#1456
Open
bengineerd wants to merge 2 commits into
Open
Conversation
… rssi-rx-fsm-fixes
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.
Description
Fix RSSI receive-side header filtering, SYN parameter staging, and payload-buffer timing.
RssiRxFsmpreviously accepted some malformed or unsupported receive-side combinations too late, or after partially updating local state. In particular, SYN parameter fields could be copied into the visible peer-parameter record before the full SYN header passed checksum and frame-boundary validation. DATA legality also depended partly on registered state from the previous segment, and duplicate DATA could enter the payload-buffer path before being rejected later.The receive application path also assumed payload RAM read data was available in the same cycle that a buffered DATA entry was selected. That was fragile with the registered RAM path used in
RssiCore, especially for one-word DATA frames and final-beat pause handling.This changes
RssiRxFsmto stage SYN parameters until the full SYN is validated, reject unsupported SYN/EACK/DATA flag combinations before payload buffering, require DATA to be the next in-order sequence before writing payload RAM, and add an explicit applicationREAD_Sstate for registered RAM read latency. The RX FSM regression is ungated now that these cases pass.Details
This fix is necessary because RSSI receive validation has to protect both protocol state and payload RAM contents. A malformed SYN should not alter the negotiated-parameter inputs seen by the connection FSM. DATA must carry the expected RSSI data semantics, not unsupported EACK/BUSY/NULL/RST combinations, and duplicate DATA should not be allowed to disturb receive-buffer side state after the original segment has already been delivered.
This usually worked until now because common traffic uses well-formed SYN/DATA headers, ordered delivery, and enough idle cycles that the registered RAM latency was not obvious. The bugs show up in directed regressions that intentionally send malformed SYNs, unsupported EACK/DATA flag combinations, checksum-failed payloads followed by retransmission, duplicate DATA after delivery, and one-word DATA through the same registered RAM timing used by
RssiCore.The relevant protocol intent is:
The Rogue RSSI controller follows the same broad receive model:
Header::verify()parses flags, checks the header length and checksum, and only then exposes sequence, acknowledge, and SYN parameter fields inHeader.cpp;Controller.cpp;Controller.cpp;nextSeqRx_, while duplicates are dropped inController.cpp;Controller.cpp.Rogue's software controller can queue out-of-order frames, while this SURF RSSI v1 hardware path keeps the simpler in-order receive-buffer contract. This PR keeps that hardware contract, but makes the RTL enforce it before payload RAM writes and before visible SYN parameter updates.
Validation:
Related
Stacked on #1453.