WebRTC Bug Fixes, Diagnostics, and Backpressure#17
Open
timwu20 wants to merge 10 commits intohaiko-webrtc-multistream-nego-fix-3from
Open
WebRTC Bug Fixes, Diagnostics, and Backpressure#17timwu20 wants to merge 10 commits intohaiko-webrtc-multistream-nego-fix-3from
timwu20 wants to merge 10 commits intohaiko-webrtc-multistream-nego-fix-3from
Conversation
4 tasks
9db0cda to
d218c6a
Compare
2d2e257 to
dbc1e29
Compare
dbc1e29 to
3a074bb
Compare
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.
Stacked on top of paritytech/litep2p#554 (haiko's multistream-select negotiation fix). These changes address several issues discovered during testing with browser clients (smoldot) and improve reliability of the WebRTC transport.
Changes
Bug Fixes:
594520e) — Route all packets from existing opening connections to those connections instead of creating duplicate Rtc instances. Remove prematureaccepts()check that was rejecting valid packets before ICE candidate nomination completed.130a647) — When a request-response handler writes a response and drops the Substream, a subsequent FIN from the remote peer no longer causesEssentialTaskClosed. Addsfin_deliveredtracking to distinguish clean closes from unclean drops, emittingResetStreamonly when appropriate.d51dd01) — When a data channel closes during DCEP or multistream-select negotiation, report the failure to the protocol layer so it can retry instead of silently losing the substream.e72cd9d) — In request-response, callsubstream.close()before sending the feedback oneshot, ensuring the FIN is sent before the caller is notified of success.ce6d2ae) —channel.write()returnsOk(false)when the buffer is full; previously the bool was silently discarded, causing data loss. Now returnsError::ChannelClogged.Backpressure:
be4c14b) — Whenchannel.write()rejects data (global 128 KB SCTP buffer full), apply backpressure to the Substream'sAsyncWriteimpl so writers block instead of losing data. Pending writes are queued and retried.4cefe69) — Replace per-channelChannelBufferedAmountLowevent with retry onOutput::Transmit, which correctly handles the mismatch between str0m's global buffer limit and per-channel events.Diagnostics:
99498b8) — Trace-level logging for handshake negotiation flow: payload forwarding, handshake reads, and timeout reporting.Dependency:
8a7082f) — Migrate to ChainSafe's str0m fork with updated DTLS cert API andRtc::builder().build(Instant::now()).Test plan
cargo test --features webrtc --lib)