Skip to content

Commit 2b92db4

Browse files
authored
feat(iroh): Allow customising the TransportConfig for connections (#3111)
## Description Previously the TransportConfig was hardcoded for the client (client/server in QUIC terminology: client initiates the connection, server accepts the connection). This rendered some customisations on the server-side, as supported by the Builder, impossible since the client would negotiate different limits. Specifically the QUIC keep-alives were at a 1s or faster interval while the connection timeout was at 30s or less. This adds a new API marked as experimental which allows customising the transport config for clients. Secondly it also respects any custom TransportConfig set in the Builder for clients as well as servers. This is not a behaviour change since the parameters set by default are now moved to the Builder and due to how these are negotiated it does not matter that the server will now also have the same values set by default. Closes #2872. ## Breaking Changes ### iroh If Builder::transport_config is used it will now also affect initiated connections, while before it only affected the accepted connections. ## Notes & open questions <!-- Any notes, remarks or open questions you have to make about the PR. --> ## Change checklist - [x] Self-review. - [x] Documentation updates following the [style guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text), if relevant. - [x] Tests if relevant. - [x] All breaking changes documented.
1 parent c532de3 commit 2b92db4

File tree

1 file changed

+37
-10
lines changed

1 file changed

+37
-10
lines changed

iroh/src/endpoint.rs

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ pub struct Builder {
9595
secret_key: Option<SecretKey>,
9696
relay_mode: RelayMode,
9797
alpn_protocols: Vec<Vec<u8>>,
98-
transport_config: Option<quinn::TransportConfig>,
98+
transport_config: quinn::TransportConfig,
9999
keylog: bool,
100100
#[debug(skip)]
101101
discovery: Vec<DiscoveryBuilder>,
@@ -113,11 +113,13 @@ pub struct Builder {
113113

114114
impl Default for Builder {
115115
fn default() -> Self {
116+
let mut transport_config = quinn::TransportConfig::default();
117+
transport_config.keep_alive_interval(Some(Duration::from_secs(1)));
116118
Self {
117119
secret_key: Default::default(),
118120
relay_mode: default_relay_mode(),
119121
alpn_protocols: Default::default(),
120-
transport_config: Default::default(),
122+
transport_config,
121123
keylog: Default::default(),
122124
discovery: Default::default(),
123125
proxy_url: None,
@@ -146,7 +148,7 @@ impl Builder {
146148
.secret_key
147149
.unwrap_or_else(|| SecretKey::generate(rand::rngs::OsRng));
148150
let static_config = StaticConfig {
149-
transport_config: Arc::new(self.transport_config.unwrap_or_default()),
151+
transport_config: Arc::new(self.transport_config),
150152
keylog: self.keylog,
151153
secret_key: secret_key.clone(),
152154
};
@@ -378,8 +380,11 @@ impl Builder {
378380
/// internet applications. Applications protocols which forbid remotely-initiated
379381
/// streams should set `max_concurrent_bidi_streams` and `max_concurrent_uni_streams` to
380382
/// zero.
383+
///
384+
/// Please be aware that changing some settings may have adverse effects on establishing
385+
/// and maintaining direct connections.
381386
pub fn transport_config(mut self, transport_config: quinn::TransportConfig) -> Self {
382-
self.transport_config = Some(transport_config);
387+
self.transport_config = transport_config;
383388
self
384389
}
385390

@@ -599,8 +604,25 @@ impl Endpoint {
599604
/// The `alpn`, or application-level protocol identifier, is also required. The remote
600605
/// endpoint must support this `alpn`, otherwise the connection attempt will fail with
601606
/// an error.
602-
#[instrument(skip_all, fields(me = %self.node_id().fmt_short(), alpn = ?String::from_utf8_lossy(alpn)))]
603607
pub async fn connect(&self, node_addr: impl Into<NodeAddr>, alpn: &[u8]) -> Result<Connection> {
608+
self.connect_with(node_addr, alpn, self.static_config.transport_config.clone())
609+
.await
610+
}
611+
612+
/// Connects to a remote [`Endpoint`] using a custom [`TransportConfig`].
613+
///
614+
/// Like [`Endpoint::connect`], but allows providing a custom for the connection. See
615+
/// the docs of [`Endpoint::connect`] for details.
616+
///
617+
/// Please be aware that changing some settings may have adverse effects on establishing
618+
/// and maintaining direct connections. Carefully test settings you use and consider
619+
/// this currently as still rather experimental.
620+
pub async fn connect_with(
621+
&self,
622+
node_addr: impl Into<NodeAddr>,
623+
alpn: &[u8],
624+
transport_config: Arc<TransportConfig>,
625+
) -> Result<Connection> {
604626
let node_addr: NodeAddr = node_addr.into();
605627
tracing::Span::current().record("remote", node_addr.node_id.fmt_short());
606628
// Connecting to ourselves is not supported.
@@ -638,18 +660,25 @@ impl Endpoint {
638660

639661
// Start connecting via quinn. This will time out after 10 seconds if no reachable
640662
// address is available.
641-
self.connect_quinn(node_id, alpn, addr).await
663+
self.connect_quinn(node_id, alpn, addr, transport_config)
664+
.await
642665
}
643666

644667
#[instrument(
668+
name = "connect",
645669
skip_all,
646-
fields(remote_node = node_id.fmt_short(), alpn = %String::from_utf8_lossy(alpn))
670+
fields(
671+
me = %self.node_id().fmt_short(),
672+
remote_node = node_id.fmt_short(),
673+
alpn = %String::from_utf8_lossy(alpn),
674+
)
647675
)]
648676
async fn connect_quinn(
649677
&self,
650678
node_id: NodeId,
651679
alpn: &[u8],
652680
addr: QuicMappedAddr,
681+
transport_config: Arc<TransportConfig>,
653682
) -> Result<Connection> {
654683
debug!("Attempting connection...");
655684
let client_config = {
@@ -661,9 +690,7 @@ impl Endpoint {
661690
self.static_config.keylog,
662691
)?;
663692
let mut client_config = quinn::ClientConfig::new(Arc::new(quic_client_config));
664-
let mut transport_config = quinn::TransportConfig::default();
665-
transport_config.keep_alive_interval(Some(Duration::from_secs(1)));
666-
client_config.transport_config(Arc::new(transport_config));
693+
client_config.transport_config(transport_config);
667694
client_config
668695
};
669696

0 commit comments

Comments
 (0)