Skip to content

Commit 75c64e4

Browse files
Revert "fix(s2n-quic-dc): derive crypto before opening TCP stream (#2451)" (#2459)
In further testing, this introduces too much reordering under high concurrency for our dedup tracking to mitigate. connect() can "complete" in reverse order of starts (e.g., due to queueing of the wake events) which leads to huge gaps (up to concurrency) in the chosen key IDs. We can likely get the best of both worlds by deriving crypto beforehand but only associating it with a stream just-in-time as connect()s complete (keeping a per-peer pool just for opening connections) but that's a more invasive change, for now just revert this. This reverts commit 71e56fe.
1 parent 4500593 commit 75c64e4

File tree

2 files changed

+17
-28
lines changed

2 files changed

+17
-28
lines changed

dc/s2n-quic-dc/src/stream/client/tokio.rs

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ use crate::{
99
endpoint,
1010
environment::tokio::{self as env, Environment},
1111
socket::Protocol,
12-
TransportFeatures,
1312
},
1413
};
1514
use std::{io, net::SocketAddr};
@@ -30,15 +29,12 @@ where
3029
// ensure we have a secret for the peer
3130
let peer = handshake.await?;
3231

33-
let (crypto, parameters) = peer.pair(&TransportFeatures::UDP);
34-
3532
let stream = endpoint::open_stream(
3633
env,
37-
peer.map(),
38-
crypto,
39-
parameters,
34+
peer,
4035
env::UdpUnbound(acceptor_addr.into()),
4136
subscriber,
37+
None,
4238
)?;
4339

4440
// build the stream inside the application context
@@ -64,14 +60,7 @@ where
6460
Sub: event::Subscriber,
6561
{
6662
// Race TCP handshake with the TLS handshake
67-
let handshake = async {
68-
let peer = handshake.await?;
69-
let (crypto, parameters) = peer.pair(&TransportFeatures::TCP);
70-
Ok((peer, crypto, parameters))
71-
};
72-
// poll the crypto first so the server can read the first packet on accept in the happy path
73-
let ((peer, crypto, parameters), socket) =
74-
tokio::try_join!(handshake, TcpStream::connect(acceptor_addr))?;
63+
let (socket, peer) = tokio::try_join!(TcpStream::connect(acceptor_addr), handshake,)?;
7564

7665
// Make sure TCP_NODELAY is set
7766
let _ = socket.set_nodelay(true);
@@ -88,15 +77,14 @@ where
8877

8978
let stream = endpoint::open_stream(
9079
env,
91-
peer.map(),
92-
crypto,
93-
parameters,
80+
peer,
9481
env::TcpRegistered {
9582
socket,
9683
peer_addr,
9784
local_port,
9885
},
9986
subscriber,
87+
None,
10088
)?;
10189

10290
// build the stream inside the application context
@@ -126,20 +114,16 @@ where
126114
{
127115
let local_port = socket.local_addr()?.port();
128116
let peer_addr = socket.peer_addr()?.into();
129-
130-
let (crypto, parameters) = peer.pair(&TransportFeatures::TCP);
131-
132117
let stream = endpoint::open_stream(
133118
env,
134-
peer.map(),
135-
crypto,
136-
parameters,
119+
peer,
137120
env::TcpRegistered {
138121
socket,
139122
peer_addr,
140123
local_port,
141124
},
142125
subscriber,
126+
None,
143127
)?;
144128

145129
// build the stream inside the application context

dc/s2n-quic-dc/src/stream/endpoint.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
use crate::{
55
event::{self, api::Subscriber as _, IntoEvent as _},
66
msg, packet,
7-
path::secret::{self, Map},
7+
path::secret::{self, map, Map},
88
random::Random,
99
stream::{
1010
application,
@@ -35,16 +35,21 @@ pub struct AcceptError<Peer> {
3535
#[inline]
3636
pub fn open_stream<Env, P>(
3737
env: &Env,
38-
map: &Map,
39-
crypto: secret::map::Bidirectional,
40-
parameters: dc::ApplicationParams,
38+
entry: map::Peer,
4139
peer: P,
4240
subscriber: Env::Subscriber,
41+
parameter_override: Option<&dyn Fn(dc::ApplicationParams) -> dc::ApplicationParams>,
4342
) -> Result<application::Builder<Env::Subscriber>>
4443
where
4544
Env: Environment,
4645
P: Peer<Env>,
4746
{
47+
let (crypto, mut parameters) = entry.pair(&peer.features());
48+
49+
if let Some(o) = parameter_override {
50+
parameters = o(parameters);
51+
}
52+
4853
let key_id = crypto.credentials.key_id;
4954
let stream_id = packet::stream::Id {
5055
key_id,
@@ -69,7 +74,7 @@ where
6974
stream_id,
7075
None,
7176
crypto,
72-
map,
77+
entry.map(),
7378
parameters,
7479
None,
7580
None,

0 commit comments

Comments
 (0)