Deduplicate incoming RE-CONFIG requests to prevent new stream destruction#30
Open
timwu20 wants to merge 3 commits intoalgesten:mainfrom
Open
Deduplicate incoming RE-CONFIG requests to prevent new stream destruction#30timwu20 wants to merge 3 commits intoalgesten:mainfrom
timwu20 wants to merge 3 commits intoalgesten:mainfrom
Conversation
3 tasks
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.
Problem
handle_reconfig_param()insrc/association/mod.rsdoes not deduplicate incomingParamOutgoingResetRequestchunks. When the remote peer retransmits a RE-CONFIG request (because its SCTP RTO expired before our response arrived), we process it a second time. If the stream ID was reused between the first and second processing,reset_streams_if_any()destroys the new stream.Reproduction sequence
unregister_stream(1)removes stream 1. We send a RE-CONFIG responseopen_stream(1)— creates a new stream 1 inself.streamsself.streams.contains_key(&1)returnstrue(the new stream), sounregister_stream(1)removes the new streamstream(1)->ErrStreamNotExisted. The new channel is deadImpact
This bug causes WebRTC data channel failures in production when a data channel is opened shortly after another channel on the same SCTP stream ID is closed. It kills the block-announces notification protocol in Polkadot light client WebRTC connections, leaving the peer permanently unable to sync.
Fix
Add a
FxHashSet<u32>field (reconfig_requests_seen) to theAssociationstruct that tracks all incoming reconfig request sequence numbers. Before processing aParamOutgoingResetRequest, check if we have already seen this sequence number. If so, resend aSuccessPerformedresponse without reprocessing the stream resets.A separate set is needed because the existing
reconfig_requestsmap cannot serve as a dedup mechanism — entries are removed after successful processing inreset_streams_if_any(), and that removal is required for theInProgressretry loop that re-evaluates pending requests as TSN advances.Changes
src/association/mod.rs: Addedreconfig_requests_seen: FxHashSet<u32>field and dedup guard inhandle_reconfig_param()src/endpoint/endpoint_test.rs: Addedtest_assoc_reset_duplicate_reconfig_requestthat verifies a replayed reconfig packet does not destroy a newly opened streamTest plan
test_assoc_reset_duplicate_reconfig_requestvalidates the dedup behavior by capturing reconfig packet bytes, completing the reset, reopening the stream, replaying the captured packets, and asserting the new stream survivescargo clippyreports no new warnings