Skip to content
This repository has been archived by the owner on Jun 21, 2019. It is now read-only.

Switch to futures-based DNS resolution #3

Merged
merged 9 commits into from
Dec 27, 2016

Conversation

inejge
Copy link
Contributor

@inejge inejge commented Dec 16, 2016

Re #2: use the TRust-DNS library for DNS lookups.

@inejge
Copy link
Contributor Author

inejge commented Dec 16, 2016

Appveyor failed to build libsqlite3-sys, which is a transitive dependency of trust-dns.

@alexcrichton
Copy link
Contributor

Oh interesting! In the interest of keeping this example lean and compiling by default on most platforms, would it be possible to make the libsqlite3 dependency optional @bluejekyll?

@inejge
Copy link
Contributor Author

inejge commented Dec 16, 2016

would it be possible to make the libsqlite3 dependency optional

Not presuming to speak in @bluejekyll's name, but I happen to have used a recent, post-0.8.1 TRust-DNS git revision in another project; there, the client and server parts are separate. The client, which is needed here, doesn't depend on libsqlite3. I'll push a commit which uses that revision.

@inejge
Copy link
Contributor Author

inejge commented Dec 16, 2016

Heh, now it's the old nemesis of Windows builds, OpenSSL (for DNSSEC support). I can try to wrap its use in a feature, but I don't have the time right now.

@bluejekyll
Copy link
Member

bluejekyll commented Dec 16, 2016 via email

@bluejekyll
Copy link
Member

Ok, the 0.9.0 release is up. This will remove the sqlite dependency on the client.

OpenSSL will be little longer.

@inejge
Copy link
Contributor Author

inejge commented Dec 17, 2016

So I fetched 0.9.0 and made a slash-and-burn, wouldn't-wish-that-on-your-worst-enemy patch to segregate OpenSSL behind the "dnssec" feature. The good thing is that the server finally passes all checks. However, what's being used is not representative of the true shape of TRust-DNS code. Encouraged by the ease of running tokio-socks5, a Windows user might expect TRust-DNS to be immediately usable in their projects, whereas there's still a big caveat of OpenSSL requirement for the mainline library.

@alexcrichton, in your opinion, is this usable, or it might be more prudent to wait until TRust-DNS drops its dependency on OpenSSL?

@alexcrichton
Copy link
Contributor

@inejge ah yeah I think we'll want to wait for the example to not require OpenSSL by default due to the portability-by-default problems there

@bluejekyll
Copy link
Member

I've got a change for the client locally that I'll publish as 0.9.1 to the client when I get a minute, that will make OpenSSL a default but optional feature.

It's not wasted work as I'll be reusing some of these changes for the port to ring.

@alexcrichton
Copy link
Contributor

Nice! With that sounds good to me to switch over.

@bluejekyll
Copy link
Member

Ok, I found a few minutes to publish 0.9.1 to Crates.io. Let me know if there are any issues.

@alexcrichton
Copy link
Contributor

@bluejekyll it looks like trust-dns is pulling in mio 0.5, but was that meant to get upgraded to 0.6?

@bluejekyll
Copy link
Member

Gah. That's the old deprecated client. I should make that optional too, I can look into that tonight...

@inejge
Copy link
Contributor Author

inejge commented Dec 21, 2016

0.9.1 builds cleanly without OpenSSL, but I'll wait for 0.9.2 before pushing a commit. @alexcrichton, would you prefer if I squashed the last few commits (which just switched trust-dns versions), or should I leave the history as is?

@bluejekyll
Copy link
Member

bluejekyll commented Dec 21, 2016

0.9.2 now published, the direct dependency on MIO can be disabled, which will also remove the non-futures based client.

This revision separates the client and server parts (we only
need the client), and makes OpenSSL optional, making Windows
builds easier in turn. The interface of ClientFuture now
requires a mutable binding for query().
@inejge
Copy link
Contributor Author

inejge commented Dec 23, 2016

Ok, I went ahead and squashed the last few commits into one to avoid repetition, the server now uses trust-dns 0.9.2 without OpenSSL and the older client which pulled in a separate copy of mio.

@alexcrichton alexcrichton merged commit de4e964 into tokio-rs:master Dec 27, 2016
@alexcrichton
Copy link
Contributor

Awesome, thanks again @inejge and @bluejekyll!

@bluejekyll
Copy link
Member

Cool. btw, I'll be working on a resolver soon, which we should probably replace the client with when I get that ready.

@alexcrichton
Copy link
Contributor

Sounds good to me!

I changed the implementation here slightly to only create one instance of BasicClientFuture rather than one-per-connection. Other than that I left as-is. I'm still not entirely sure what's the best way to handle IPv4 and IPv6 or multiple addresses within each. In the sense of a proxy server I guess we should send out TCP connections to a couple in parallel and just pick the first successful one? In any case that seems orthogonal to trust-dns, and that'll just give us a bag of IP addresses and it's up to the proxy what to do!

@inejge inejge deleted the futures-dns branch December 28, 2016 12:40
@inejge
Copy link
Contributor Author

inejge commented Dec 28, 2016

I'm still not entirely sure what's the best way to handle IPv4 and IPv6 or multiple addresses within each.

The server could try to follow RFC 6555 (Happy Eyeballs: Success with Dual-Stack Hosts), but that would need a resolver which implements RFC 6724 sorting to avoid hardcoded assumptions and unnecessary connection attempts for situations when there's no IPv6 connectivity. For demonstration purposes though, one could have a fixed preference for IPv6 and parallel attempts with a single address from each family. As you said, that's orthogonal to trust-dns, and could wait for a more complete resolver.

@bluejekyll
Copy link
Member

a resolver which implements RFC 6724 sorting to avoid hardcoded assumptions

I'll take a look at supporting that directly.

Also, will this want to support 6to4 nat or vise versa, 4to6? Just curious.

@alexcrichton
Copy link
Contributor

I barely know how 4to6 or 6to4 operates, much less how a client would work with that :(

@inejge
Copy link
Contributor Author

inejge commented Dec 29, 2016

I'll take a look at supporting [RFC 6724] directly.

The tricky part is the necessity of enumerating the host's active interfaces and configured IP addresses to run the algorithm, which means platform-specific code.

Also, will this want to support 6to4 nat or vise versa, 4to6?

The algorithm doesn't concern itself directly with network-level policies or mechanisms. It does have preference for choosing addresses of the same scope and type in the default policy table, but that's orthogonal to the actual use of those kinds of addresses on the host, or any translation which might have been used to supply an address.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants