feat: implement rtt hints for nat traversal #3784
Conversation
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3784/docs/iroh/ Last updated: 2025-12-17T16:49:52Z |
dignifiedquire
force-pushed
the
feat-rtt-hints
branch
from
1400dbc to
b926b91
Compare
| transports::Addr::Relay(url, _id) => { | ||
| // Relay case, grab the latency from net_report | ||
| relay_latencies.as_ref().and_then(|l| l.get(&url)) | ||
| } |
There was a problem hiding this comment.
The relay latency is not the same thing as the RTT to the peer via the relay, because that depends on the (potentially two!) relays involved in going back and forth.
Besides, I don't think we need an RTT hint for any relay addresses, because they will/should never be used in holepunching.
| // IP case, grab the worst latency we currently have | ||
| all_rtts | ||
| .get(&addr) | ||
| .and_then(|rtts| rtts.iter().max().copied()) |
There was a problem hiding this comment.
I think this should be .min(Duration::from_millis(333)) additionally, just so we don't accidentally balloon the PTO in case there was some kind of weird bad RTT in our IP paths.
There was a problem hiding this comment.
Also, IIUC, this won't help broken paths, right? You only have a single connection in the RemoteState, and the remote address that doesn't work for holepunching won't have an RTT estimate. So you're back to having a 333ms initial RTT estimate for remote addresses that don't work. Am I missing something?
There was a problem hiding this comment.
the probes will timeout faster for broken paths, that’s the whole point of this PR
There was a problem hiding this comment.
But... the way the code is written, I don't see how that's going to be the case?
Depends on n0-computer/quinn#278