Implement WebRTC messages framing #2896

tomaka · 2022-10-18T12:22:16Z

This PR finishes implementing the WebRTC spec by adding the last remaining item: the messages framing.

Implementing this messages framing while minimizing the amount of data copies is rather challenging.
Instead of going for the complicated solution, I went for the more easy solution of having an intermediate read buffer where data is first copied.
Going for the simple solution decreases the chances of bugs and increases the ease of debugging, so it's preferable at the moment.

In the future, once WebRTC is fully working, we can rewrite this messages framing code in a more optimized way.

mergify

Automatically approving tomaka's pull requests. This auto-approval will be removed once more maintainers are active.

github-actions · 2022-10-18T12:43:01Z

twiggy diff report

Difference in .wasm size before and after this pull request.

 Delta Bytes │ Item
─────────────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
       -8588 ┊ <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::h0487131d4c491b93
       +8588 ┊ <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::h801956d7493f1941
       -3556 ┊ <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::h3107aa82f3270ecf
       +3556 ┊ <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::hc5a072cb1d299ae1
       -3305 ┊ <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::h9c82c520cf398c6d
       +3227 ┊ <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::hb52fe97072de8226
       +2219 ┊ smoldot::libp2p::connection::established::multi_stream::MultiStream<TNow,TSubId,TRqUd,TNotifUd>::substream_read_write::h1bff5e6e66178745
       +1290 ┊ smoldot::libp2p::collection::multi_stream::MultiStreamConnectionTask<TNow,TSubId>::substream_read_write::h588b905c4bd92eae
        -983 ┊ hashbrown::raw::RawTable<T,A>::reserve_rehash::ha927364891a8e5ba
        +983 ┊ hashbrown::raw::RawTable<T,A>::reserve_rehash::hb54d82e16dda9027
        +975 ┊ <F as nom::internal::Parser<I,O,E>>::parse::h55db268c0db3260e
        +975 ┊ <F as nom::internal::Parser<I,O,E>>::parse::hee9629e946c75926
        +967 ┊ hashbrown::raw::RawTable<T,A>::reserve_rehash::h567f0b8a2cb017e5
        -655 ┊ smoldot::libp2p::connection::established::multi_stream::MultiStream<TNow,TSubId,TRqUd,TNotifUd>::on_substream_event::h92465f384b4efc0f
        +647 ┊ smoldot::libp2p::connection::established::multi_stream::MultiStream<TNow,TSubId,TRqUd,TNotifUd>::on_substream_event::h5ba4e91323587d5f
        +528 ┊ data[0]
        +429 ┊ <F as nom::internal::Parser<I,O,E>>::parse::h59c484db9d5243f2
        +429 ┊ <F as nom::internal::Parser<I,O,E>>::parse::h7e5d4ea4b1140b21
        +414 ┊ core::ptr::drop_in_place<core::future::from_generator::GenFuture<smoldot_light::network_service::tasks::connection_task<smoldot_light_wasm::platform::Platform>::{{closure}}>>::h5025e80c68d0931b
        -414 ┊ core::ptr::drop_in_place<core::future::from_generator::GenFuture<smoldot_light::network_service::tasks::connection_task<smoldot_light_wasm::platform::Platform>::{{closure}}>>::hbc454602a8b2d1ad
       +7098 ┊ ... and 479 more.
      +23156 ┊ Σ [499 Total Rows]

tomaka · 2022-10-18T18:03:28Z

This PR turned into a bit of a "fix every WebRTC issue" PR, but it is much more productive to just push everything onto a branch instead of analyze each bug individually.

melekes · 2022-10-19T08:21:59Z

bin/wasm-node/javascript/src/index-browser.ts

              "o=- 0 0 IN IP" + ipVersion  + " " + targetIp + "\n" +
              // Name for the session. We are allowed to pass a dummy `-`. (RFC8866)
              "s=-" + "\n" +
              // Start and end of the validity of the session. `0 0` means that the session never
              // expires. (RFC8866)
              "t=0 0" + "\n" +
-              // A lite implementation is only appropriate for devices that will
-              // *always* be connected to the public Internet and have a public
+              // A non-lite implementation is only appropriate for devices that will


??? This was a copy from the specification. Did something change?

? The specification doesn't contain this text. It's me who wrote it. It has a mistake in it, it says that lite implementations are only for publicly-accessible devices. But no, it's non-lite implementations that are for public devices. We're not a public device, so we use the lite implementation.

"Certain ICE agents will always be connected to the public Internet
and have a public IP address at which it can receive packets from any
correspondent. To make it easier for these devices to support ICE,
ICE defines a special type of implementation called "lite" (in
contrast to the normal full implementation). Lite agents only use
host candidates and do not generate connectivity checks or run state
machines, though they need to be able to respond to connectivity
checks." https://datatracker.ietf.org/doc/rfc8445/

By specifying ice-lite in server's SDP (that we're pretending we've received through some channel, but instead construct manually), we're saying that server is publicly accessible.

To be honest this text doesn't make sense to me no matter whether it says "lite" or "not lite", so I've just reverted it 🤷

melekes · 2022-10-19T08:23:04Z

bin/wasm-node/javascript/src/index-browser.ts

@@ -303,8 +304,9 @@ export function start(options?: ClientOptions): Client {
              // (UDP or TCP)
              "a=sctp-port:5000" + "\n" +
              // The maximum SCTP user message size (in bytes) (RFC8841)
-              "a=max-message-size:100000" + "\n" +
-              // A transport address for a candidate that can be used for connectivity checks (RFC8839).
+              "a=max-message-size:16384" + "\n" +  // TODO: should this be part of the spec?


subject to a change, but yes!

libp2p/specs#412 (comment)

tomaka added 6 commits October 18, 2022 10:11

Wrap substreams within a struct

30ee30b

Move the substream id inside of the struct

b9e00a1

Make the substream extractable

0106fb8

Implement the framing

bb7ab99

Missing a continue_looping

a6685d3

Some tweaks

e10047b

mergify bot approved these changes Oct 18, 2022

View reviewed changes

tomaka added 2 commits October 18, 2022 14:24

CHANGELOG

2e010f4

Fix build

a1775c3

tomaka added 10 commits October 18, 2022 15:03

Bump write buffer to 16kiB

b892594

Add framing around handshake

bca2011

Improvements to the networking task

77aa3f2

Fix wrong decoder ugh

b5f9ef1

Don't use all_consuming

72c8175

Wrong encoding too 🤦

091a177

Fix wrong Noise initiator

5624158

Fix not inserting substreams in out_in_substreams_map

2fb0b14

Don't loop if the substream has been reset

07b4e06

Move wake_up_after inside of the loop

27a9be1

tomaka added 3 commits October 19, 2022 07:55

Change max message size in SDP

8a0dc27

Small comments fix

ad6a083

More fixing

ba593a9

melekes reviewed Oct 19, 2022

View reviewed changes

Revert the lite thing

6ad4bf1

melekes approved these changes Oct 19, 2022

View reviewed changes

tomaka added the automerge Automatically merge pull request as soon as possible label Oct 19, 2022

Merge branch 'main' into messages-framing

6c1b162

mergify bot merged commit b7d13d7 into paritytech:main Oct 19, 2022

tomaka deleted the messages-framing branch October 19, 2022 09:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement WebRTC messages framing #2896

Implement WebRTC messages framing #2896

tomaka commented Oct 18, 2022

mergify bot left a comment

github-actions bot commented Oct 18, 2022 •

edited

Loading

tomaka commented Oct 18, 2022

melekes Oct 19, 2022

tomaka Oct 19, 2022

melekes Oct 19, 2022

melekes Oct 19, 2022

tomaka Oct 19, 2022

melekes Oct 19, 2022

melekes Oct 19, 2022

Implement WebRTC messages framing #2896

Implement WebRTC messages framing #2896

Conversation

tomaka commented Oct 18, 2022

mergify bot left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 18, 2022 • edited Loading

twiggy diff report

tomaka commented Oct 18, 2022

melekes Oct 19, 2022

Choose a reason for hiding this comment

tomaka Oct 19, 2022

Choose a reason for hiding this comment

melekes Oct 19, 2022

Choose a reason for hiding this comment

melekes Oct 19, 2022

Choose a reason for hiding this comment

tomaka Oct 19, 2022

Choose a reason for hiding this comment

melekes Oct 19, 2022

Choose a reason for hiding this comment

melekes Oct 19, 2022

Choose a reason for hiding this comment

github-actions bot commented Oct 18, 2022 •

edited

Loading