Quic attempt 4 #2801

kpp · 2022-08-05T12:16:39Z

Description

Yep... This time it's written over quinn, not quinn-proto. Feature: implemented with 600 lines of code in quic/src/lib.rs.

TODO:

test endpoint_reuse was broken in eca5328.

Links to any relevant issues

Attempt N3: #2289

Perf: https://github.com/kpp/libp2p-perf/tree/quic-attempt-4

Open Questions

It uses quin#named-futures, which is outdated. So right now only tokio runtime is supported.
We need to deal with AddressChange somehow. I have no idea how to do that.
There are a lot of unwraps. I need to clean up the code a little bit.

Benchmarks:

QUIC with libp2p-perf

$ ./run.sh 
# Start Rust and Golang servers.
Local peer id: PeerId("Qmcqq9TFaYbb94uwdER1BXyGfCFY4Bb1gKozxNyVvLvTSw")
about to listen on "/ip4/127.0.0.1/udp/9992/quic"
Listening on "/ip4/127.0.0.1/udp/9992/quic".

# Rust -> Rust

Local peer id: PeerId("12D3KooWH1oPSwuvp5v5AUqvGT3HR64YZRC7VZmhVZkewiLcPDpa")
IncomingConnection { local_addr: "/ip4/127.0.0.1/udp/9992/quic", send_back_addr: "/ip4/127.0.0.1/udp/54502/quic" }
ConnectionEstablished { peer_id: PeerId("12D3KooWH1oPSwuvp5v5AUqvGT3HR64YZRC7VZmhVZkewiLcPDpa"), endpoint: Listener { local_addr: "/ip4/127.0.0.1/udp/9992/quic", send_back_addr: "/ip4/127.0.0.1/udp/54502/quic" }, num_established: 1, concurrent_dial_errors: None }
Behaviour(PerfRunDone(10.001692342s, 4396669396))
Interval	Transfer	Bandwidth
0 s - 10.00 s	4396 MBytes	3516.68 MBit/s
ConnectionClosed { peer_id: PeerId("Qmcqq9TFaYbb94uwdER1BXyGfCFY4Bb1gKozxNyVvLvTSw"), endpoint: Dialer { address: "/ip4/127.0.0.1/udp/9992/quic", role_override: Dialer }, num_established: 0, cause: None }
ConnectionClosed { peer_id: PeerId("12D3KooWH1oPSwuvp5v5AUqvGT3HR64YZRC7VZmhVZkewiLcPDpa"), endpoint: Listener { local_addr: "/ip4/127.0.0.1/udp/9992/quic", send_back_addr: "/ip4/127.0.0.1/udp/54502/quic" }, num_established: 0, cause: Some(IO(Custom { kind: Other, error: ApplicationClosed(ApplicationClose { error_code: 0, reason: b"" }) })) }

# Rust -> Golang

Local peer id: PeerId("12D3KooWLy9RkY4uW26ryp3seZ6QXuiJbmBywVE359MnQopnYEj2")
Interval	Transfer	Bandwidth
0 s - 10.00 s	3062 MBytes	2449.60 MBit/s
ConnectionClosed { peer_id: PeerId("12D3KooWL3XJ9EMCyZvmmGXL2LMiVBtrVa2BuESsJiXkSj7333Jw"), endpoint: Dialer { address: "/ip4/127.0.0.1/udp/9993/quic", role_override: Dialer }, num_established: 0, cause: None }


# Golang -> Rust

2022/08/05 15:10:43 failed to sufficiently increase receive buffer size (was: 208 kiB, wanted: 2048 kiB, got: 416 kiB). See https://github.com/lucas-clemente/quic-go/wiki/UDP-Receive-Buffer-Size for details.
IncomingConnection { local_addr: "/ip4/127.0.0.1/udp/9992/quic", send_back_addr: "/ip4/127.0.0.1/udp/54359/quic" }
ConnectionEstablished { peer_id: PeerId("12D3KooWKGLyqMFxSXgqv8n1mwX3Ljxg18ENFTtuiHTRYE8DHxrv"), endpoint: Listener { local_addr: "/ip4/127.0.0.1/udp/9992/quic", send_back_addr: "/ip4/127.0.0.1/udp/54359/quic" }, num_established: 1, concurrent_dial_errors: None }
Interval 	Transfer	Bandwidth
0s - 10.00 s 	2160 MBytes	1727.97 MBit/s
Behaviour(PerfRunDone(9.999945043s, 2160640000))
ConnectionClosed { peer_id: PeerId("12D3KooWKGLyqMFxSXgqv8n1mwX3Ljxg18ENFTtuiHTRYE8DHxrv"), endpoint: Listener { local_addr: "/ip4/127.0.0.1/udp/9992/quic", send_back_addr: "/ip4/127.0.0.1/udp/54359/quic" }, num_established: 0, cause: Some(IO(Custom { kind: Other, error: ApplicationClosed(ApplicationClose { error_code: 0, reason: b"" }) })) }

# Golang -> Golang

2022/08/05 15:10:53 failed to sufficiently increase receive buffer size (was: 208 kiB, wanted: 2048 kiB, got: 416 kiB). See https://github.com/lucas-clemente/quic-go/wiki/UDP-Receive-Buffer-Size for details.
Interval 	Transfer	Bandwidth
0s - 10.00 s 	2085 MBytes	1667.91 MBit/s

TCP with libp2p-perf

   Compiling libp2p-core v0.32.1
   Compiling libp2p-noise v0.35.0
   Compiling libp2p-plaintext v0.32.0
   Compiling netlink-proto v0.10.0
   Compiling rtnetlink v0.10.1
   Compiling if-watch v1.1.1
   Compiling ed25519-dalek v1.0.1
   Compiling toml v0.5.9
   Compiling proc-macro-crate v1.1.3
   Compiling multihash-derive v0.8.0
   Compiling multihash v0.16.2
   Compiling multiaddr v0.14.0
   Compiling libp2p-swarm v0.35.0
   Compiling libp2p-dns v0.32.1
   Compiling libp2p-tcp v0.32.0
   Compiling libp2p-yamux v0.36.0
   Compiling libp2p v0.44.0
   Compiling libp2p-perf v0.1.0 (/home/kpp/parity/libp2p-perf/rust)
    Finished release [optimized + debuginfo] target(s) in 37.42s
kpp@xps:~/parity/libp2p-perf$ ./run.sh 
# Start Rust and Golang servers.

# Rust -> Rust

## Transport security noise
Interval	Transfer	Bandwidth
0 s - 10.00 s	8201 MBytes	6560.56 MBit/s

## Transport security plaintext
Interval	Transfer	Bandwidth
0 s - 10.00 s	14598 MBytes	11678.23 MBit/s

# Rust -> Golang

## Transport security noise
Interval	Transfer	Bandwidth
0 s - 10.00 s	6832 MBytes	5465.37 MBit/s

## Transport security plaintext
Interval	Transfer	Bandwidth
0 s - 10.00 s	12462 MBytes	9969.54 MBit/s

# Golang -> Rust

## Transport security noise
Interval 	Transfer	Bandwidth
0s - 10.00 s 	9636 MBytes	7707.60 MBit/s

## Transport security plaintext
Interval 	Transfer	Bandwidth
0s - 10.00 s 	20705 MBytes	16563.89 MBit/s

# Golang -> Golang

## Transport security noise
Interval 	Transfer	Bandwidth
0s - 10.00 s 	5741 MBytes	4592.47 MBit/s

## Transport security plaintext
Interval 	Transfer	Bandwidth
0s - 10.00 s 	21973 MBytes	17578.36 MBit/s

dignifiedquire · 2022-08-05T12:26:25Z

@kpp any chance you can also post benchmarks for tcp on the same setup? would be interesting to see how things compare

kpp · 2022-08-05T12:48:46Z

Sure. Posted in the PR description.

elenaf9 · 2022-08-05T13:12:16Z

@kpp any chance you can also post benchmarks for tcp on the same setup? would be interesting to see how things compare

Adding to this: do you have benchmarks from the latest implementation in #2289 for comparison?

As discussed in #2289 (comment), we have thus far not switched to using quinn (instead of quinn-proto) because of their usage of unbounded channels and overhead at spawning tasks. Both will be fixed with quinn-rs/quinn#1219, but this PR is not merged yet and has actually been stale for a couple of months now.
I am generally in favor of using quinn, but until they publish a new version that includes that change we won't be able to merge an implementation that relies on this (and this PR directly depends on quinn-rs/quinn#1357, which is not merged yet as well).
I am undecided here. I don't want to block on quinn-rs/quinn#1219 or quinn-rs/quinn#1357, which is why imo it makes more sense to first focus on finishing #2289, and switch over at a later point. That being said, I definitely like how using quinn saves a lot of complexity and simplifies the integration. Maybe we could try to collaborate with the quinn maintainers to get the mentioned PRs merged and a new version published (provided that the benchmarks do not indicate any performance hit when using quinn).

Ralith · 2022-08-28T02:13:55Z

The quinn PRs cited above are mainly blocked on feedback. If they're useful to you, please post on them! I'm particularly interested in hearing which futures are useful to have nameable, and which ones should be 'static. If you have measured cases where quinn-rs/quinn#1219 significantly improves performance/reduces resource usage, that would also be very helpful to know about.

In the meantime, I've rebased quinn-rs/quinn#1357 to expose the latest features from main.

transports/quic/Cargo.toml

xpepermint · 2022-09-27T12:16:11Z

Hey, let me drop some related info that might help. So, after months working on my own implementation of QUIC/TLS13 for Rust I came across this https://github.com/aws/s2n-quic repository. The design looks pretty much identical to my own concept thus I believe they took the correct path and it's worth checking. The overall interface approache is pretty much how things should be designed in Rust.

kpp · 2022-09-27T12:36:16Z

Hey, thanks. I saw their project and it worth trying to implement libp2p-quic over s2n-quic to benchmark the solutions.

Ralith · 2022-09-27T17:56:28Z

A comparative analysis should probably also include Cloudflare's and Mozilla's implementations. Rust has a lot of QUICs these days!

xpepermint · 2022-09-27T20:01:58Z

Yeah, Mozilla's implementation (Neqo) looks great and the source code is probably the most optimal, but unfortunately they don't have any plan of releasing the crates mozilla/neqo#1299 and I think they pretty much decided what async runtime they want to support mozilla/neqo#530. Maybe this it's a big deal after all, but I just share it here.

camshaft · 2022-10-15T19:13:06Z

Let me know if you need any assistance in using or benchmarking s2n-quic.

thomaseizinger

It is nice too see how small the implementation could be if we used quinn.

Left some thoughts as I read through the code!

transports/quic/src/lib.rs

transports/quic/Cargo.toml

thomaseizinger · 2022-11-01T02:20:28Z

transports/quic/src/lib.rs

+    }
+
+    /// Poll for a next If Event.
+    fn poll_if_addr(&mut self, cx: &mut Context<'_>) -> Option<<Self as Stream>::Item> {


Any reason this can't return a Poll? You could use ready! then.

I don't know =) I will port the code from attempt N3.

transports/quic/src/lib.rs

Ralith · 2022-11-04T04:04:03Z

Re: boxing and cancellation-safety: now that GATs are possible, traits can include lifetime-bearing associated futures, so it's possible to abstract over futures like Accept without boxing. That said, a single malloc there isn't likely to be measurable, so you're probably fine either way.

thomaseizinger · 2022-11-04T13:07:29Z

Re: boxing and cancellation-safety: now that GATs are possible, traits can include lifetime-bearing associated futures, so it's possible to abstract over futures like Accept without boxing. That said, a single malloc there isn't likely to be measurable, so you're probably fine either way.

I think the issue here is more that we would need self-referential structs because we would store the future in the same type as Endpoint. I don't think GATs can help us there?

Ralith · 2022-11-04T17:18:43Z

The associated types can be !Unpin! pin-project-lite makes working with this type of thing very easy, as demonstrated by quinn's own wrapping of Notified.

thomaseizinger · 2022-11-07T23:05:51Z

The associated types can be Unpin! pin-project-lite makes working with this type of thing very easy, as demonstrated by quinn's own wrapping of Notified.

Unless I am missing something, I don't think that would work in our case? Listener can't have a lifetime because it owns Endpoint, yet we would have to store Accept in the same struct to poll for the next connection. Ideally, we would have an Endpoint::poll_accept function that we can call from the Stream implementation.

Ralith · 2022-11-07T23:13:36Z

You could refactor the traits that must be implemented to propagate !Unpin futures upwards rather than requiring implementations to erase them behind a poll-style API. The extra indirection can't be avoided if you want to preserve exactly the same API, but it's worth considering an API that constrains implementations less, though probably as future work.

thomaseizinger · 2022-11-08T02:55:59Z

You could refactor the traits that must be implemented to propagate !Unpin futures upwards rather than requiring implementations to erase them behind a poll-style API. The extra indirection can't be avoided if you want to preserve exactly the same API, but it's worth considering an API that constrains implementations less, though probably as future work.

Thanks! Most of our APIs are designed around poll interfaces for various reasons so that is unlikely to change anytime soon.

kpp and others added 17 commits August 4, 2022 18:46

Init impl with quinn

c2a4468

Copy tls code

c1a3137

Use tls-quic crypto

853e7fd

Handle Listeners

709b199

Copy IfAddr code

df5c1cc

Call poll_if_addr

16441e6

Get peer id in QuicUpgrade

2f69771

Add tests

14b4151

Make QuicSubstream::poll_close fuse'able

39585f9

Add quic to libp2p

e7c71f5

Fix tracing dep version

2e73f06

Add to/from udp addr tests

3fce558

Add test close_listener

e51072b

Fix warnings in tests

0a3fca1

Dial with an appropriate local addr

629b158

cargo clippy

45018da

cargo fmt

2dea096

kpp added 3 commits August 25, 2022 16:05

Merge branch 'master' into quic-attempt-4

945fd1f

Fix poll_address_change

d8ef5c1

Move to async_std with a custom quinn

645b0ba

kpp mentioned this pull request Aug 31, 2022

Add experimental support for QUIC paritytech/substrate#11514

Open

3 tasks

GamePad64 reviewed Sep 1, 2022

View reviewed changes

transports/quic/Cargo.toml Outdated Show resolved Hide resolved

GamePad64 reviewed Sep 4, 2022

View reviewed changes

transports/quic/Cargo.toml Outdated Show resolved Hide resolved

elenaf9 mentioned this pull request Sep 9, 2022

[Tracking Issue] transports/quic: Add QUIC Transport #2883

Closed

15 tasks

kpp added 2 commits September 12, 2022 15:15

Merge branch 'master' into quic-attempt-4

526e065

Upgrade if-watch to 2.0.0

03ad457

Merge branch 'master' into quic-attempt-4

0a6915b

Ralith mentioned this pull request Sep 22, 2022

Named futures + replace NewConnection with Connection methods quinn-rs/quinn#1357

Merged

Use cipher suite order from rustls

0e86fb2

thomaseizinger mentioned this pull request Sep 23, 2022

transports/quic: Add implementation based on quinn-proto #2289

Merged

7 tasks

xpepermint mentioned this pull request Sep 27, 2022

protocols/stream: Add a streaming-response protocol to rust-libp2p #2657

Closed

kpp added 4 commits October 28, 2022 14:21

Merge branch 'master' into quic-attempt-4

7569b16

Move crypto to libp2p-tls crate

197b270

Remove usused tls mod

e9dcec3

Update quinn crate

eca5328

kpp mentioned this pull request Oct 28, 2022

Tracking issue for QUIC support paritytech/polkadot-sdk#536

Open

10 tasks

dariusc93 mentioned this pull request Oct 31, 2022

Perform test with quic transport dariusc93/rust-ipfs#10

Closed

thomaseizinger reviewed Nov 1, 2022

View reviewed changes

kpp added 4 commits November 1, 2022 18:06

Ignore test endpoint_reuse for now

46902ab

Remove unused deps

568ede9

Use ops::Try whenever possible

150297e

Use quinn from crates.io

11ef3a5

p-shahi mentioned this pull request Nov 10, 2022

NAT traversal #2052

Open

14 tasks

kpp closed this Feb 11, 2023

mxinden mentioned this pull request Feb 13, 2023

refactor(quic): rewrite quic using quinn #3454

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Quic attempt 4 #2801

Quic attempt 4 #2801

kpp commented Aug 5, 2022 •

edited

Loading

dignifiedquire commented Aug 5, 2022

kpp commented Aug 5, 2022

elenaf9 commented Aug 5, 2022 •

edited

Loading

Ralith commented Aug 28, 2022 •

edited

Loading

xpepermint commented Sep 27, 2022 •

edited

Loading

kpp commented Sep 27, 2022

Ralith commented Sep 27, 2022

xpepermint commented Sep 27, 2022 •

edited

Loading

camshaft commented Oct 15, 2022

thomaseizinger left a comment

thomaseizinger Nov 1, 2022

kpp Nov 1, 2022

Ralith commented Nov 4, 2022 •

edited

Loading

thomaseizinger commented Nov 4, 2022

Ralith commented Nov 4, 2022 •

edited

Loading

thomaseizinger commented Nov 7, 2022

Ralith commented Nov 7, 2022 •

edited

Loading

thomaseizinger commented Nov 8, 2022

Quic attempt 4 #2801

Quic attempt 4 #2801

Conversation

kpp commented Aug 5, 2022 • edited Loading

Description

TODO:

Links to any relevant issues

Open Questions

Benchmarks:

dignifiedquire commented Aug 5, 2022

kpp commented Aug 5, 2022

elenaf9 commented Aug 5, 2022 • edited Loading

Ralith commented Aug 28, 2022 • edited Loading

xpepermint commented Sep 27, 2022 • edited Loading

kpp commented Sep 27, 2022

Ralith commented Sep 27, 2022

xpepermint commented Sep 27, 2022 • edited Loading

camshaft commented Oct 15, 2022

thomaseizinger left a comment

Choose a reason for hiding this comment

thomaseizinger Nov 1, 2022

Choose a reason for hiding this comment

kpp Nov 1, 2022

Choose a reason for hiding this comment

Ralith commented Nov 4, 2022 • edited Loading

thomaseizinger commented Nov 4, 2022

Ralith commented Nov 4, 2022 • edited Loading

thomaseizinger commented Nov 7, 2022

Ralith commented Nov 7, 2022 • edited Loading

thomaseizinger commented Nov 8, 2022

kpp commented Aug 5, 2022 •

edited

Loading

elenaf9 commented Aug 5, 2022 •

edited

Loading

Ralith commented Aug 28, 2022 •

edited

Loading

xpepermint commented Sep 27, 2022 •

edited

Loading

xpepermint commented Sep 27, 2022 •

edited

Loading

Ralith commented Nov 4, 2022 •

edited

Loading

Ralith commented Nov 4, 2022 •

edited

Loading

Ralith commented Nov 7, 2022 •

edited

Loading