Use TCP Fast Open in server.c and possibly update client side TFO implementation?

getdnsapi / getdns

A modern asynchronous DNS API https://getdnsapi.net/

Other

468 stars 126 forks source link

Use TCP Fast Open in server.c and possibly update client side TFO implementation? #422

Closed candrews closed 4 years ago

candrews commented 5 years ago

TCP Fast Open eliminates a round trip for TCP connections. Since stubby is performance sensitive and makes many TCP connections to the DNS-over-TLS server, using TCP Fast Open would be a nice improvement. See https://lwn.net/Articles/508865/ for background.

On the client side, it's as simple as setting the TCP_FASTOPEN_CONNECT option on the socket.

On the server side, stubby would do something like this on the listening socket:

int qlen = 5;
setsockopt(fd, SOL_TCP, TCP_FASTOPEN, &qlen, sizeof(qlen));

Chrome and Firefox have supported TCP Fast Open for clients for over a year, and other DNS servers (ex unbound) use it for client and sever connections too.

vcunat commented 5 years ago

For reference, the browser support claim doesn't seem true from what I can see; https://github.com/curl/curl/issues/3662

candrews commented 5 years ago

Indeed, that was news to me too (I opened that issue in curl too) :)

saradickinson commented 5 years ago

getdns (the underlying library for stubby) was actually an early implementor of TFO...!

A quick peruse of the getdns source code/compile time options would show on the client side:

support for TCP Fast open was first adding in Oct 2014 before that socket option existed: https://github.com/getdnsapi/getdns/commit/9d7d9997dfcb2de3b523228602983ed12dbf7e1a
TFO was made the default in Dec 2015, and support was added for OSX that uses a different API to support TF) than linux: https://github.com/getdnsapi/getdns/commit/736d9f20bfe3d96f6a484356c1e99ccff1907b62. I pinged the linux API folks at that time to point out how much easier the OSX API was and they subsequently added the TCP_FASTOPEN_CONNECT, the getdns code just hasn't been updated to use it....
there is a configure option to control if TFO is used or not

It isn't currently implemented in the server code but that is normally listening to a system library which wouldn't use TCP by default but it should be there for completeness.

candrews commented 5 years ago

I clearly didn't check the getdns library - Thank you very much for pointing all of this out! :)

Would it be helpful for me to open an issue in getdns for your last point?

It isn't currently implemented in the server code but that is normally listening to a system library which wouldn't use TCP by default but it should be there for completeness.

saradickinson commented 5 years ago

@wtoorop @candrews Just played with the 'Transfer issue' beta feature in GitHub to move this to the getdns repo.... :-)

odkrys commented 5 years ago

Is TFO only available with GETDNS_TRANSPORT_TCP ? I can't see any TFO flag when I set it to GETDNS_TRANSPORT_TLS.

odkrys commented 5 years ago

I see. Unbound TFO implement only support for tcp not tls. And openssl library doesn't looks like supporting TFO yet.

So I tried to compile with gnutls in develop branch but it required gnutls-dane. gnutls-dane requires libunbound and unbound require libopenssl.

Huh..?

wtoorop commented 5 years ago

@odkrys I'm about to do a commit using TCP_FASTOPEN_CONNECT option, which will do TFO for TLS on Linux (I tested 8.8.8.8 and it seems to work). And yes the circular dependencies are a bit crazy... Also, even if you compile getdns with GnuTLS, we still need OpenSSL for Zero configuration DNSSEC, because of the S/MIME verification of trust-anchors.xml which we cannot do with GnuTLS (yet).

odkrys commented 5 years ago

sadly, it needs kernel 4.11+...

wtoorop commented 4 years ago

Fixed in 5.2.2