fix(iroh-net): Work around broken windows DNS configuration #2075
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
flub
changed the title
| @@ -41,7 +41,7 @@ fn get_resolver() -> Result<TokioAsyncResolver> { | |||
| config.add_search(name.clone()); | |||
| } | |||
| for nameserver_cfg in system_config.name_servers() { | |||
| if WINDOWS_BAD_SITE_LOCAL_DNS_SERVERS.contains(&nameserver_cfg.socket_addr.ip()) { | |||
| if !WINDOWS_BAD_SITE_LOCAL_DNS_SERVERS.contains(&nameserver_cfg.socket_addr.ip()) { | |||
1 task
github-merge-queue bot
pushed a commit
that referenced
this pull request
Apr 23, 2024
## Description They have been stable on CI for a while now. Maybe the DNS config tweak we did in #2075 made a difference. ## Breaking Changes <!-- Optional, if there are any breaking changes document them, including how to migrate older code. --> ## Notes & open questions Closes #2086 ## Change checklist - [x] Self-review. - ~~[ ] Documentation updates if relevant.~~ - ~~[ ] Tests if relevant.~~ - ~~[ ] All breaking changes documented.~~
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This actively refuses to use the
fec0:0:0:ffff::1,fec0:0:0:ffff::2andfec0:0:0:ffff::3DNS servers if the system has them configured.Windows by default adds 3 IPv6 site-local anycast addresses to the DNS servers:
fec0:0:0:ffff::1,fec0:0:0:ffff::2andfec0:0:0:ffff::3. Supposedly Microsoft DNS servers by default listen on those. These are present as soon as there's an IPv6 interface configured it seems, even for a loopback interface which is extremely common if not the default.Our hickory-resolver loads the system configuration, which includes these 3 IPv6 DNS servers. When it needs to make a DNS query it selects a random nameserver and tries this. If that fails it will try another one. For the next query there is bias, it will remember which servers to avoid or use. So if you get lucky and your first query falls on an actual DNS server then you are good. If you get unlucky recovering is a bit of a tussle because:
Inside netcheck we do DNS queries with a 1s timeout, this because all the probes have a 3s timeout. However hickory-resolver has a 5s timeout configured, so it's queries stay alive longer than ours. This means almost all subsequent DNS queries will end up reusing an existing connection to one of those bad servers if you are unlucky to land on one. The interplay of these timeouts and the connection reuse make recovering to a good DNS server a rather tough prospect for netcheck. It probably would eventually, given enough netcheck runs (which run at intervals of ~30s).
The odds of these nameservers being the sole way of having working DNS is basically zero. The odds of these nameservers breaking the resolver are about 50%. So remove these deprecated things.
Notes & open questions
Unfortunately the resolver returned by
get_resolver()does not have an API that allows to test it. But the test would basically be the inverse logic of the logic that removes the bad servers so perhaps not that useful anyway.Closes #2069
Closes n0-computer/dumbpipe#17
Change checklist