Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
candrews opened this issue Mar 10, 2019 · 10 comments

Comments

@candrews
Copy link

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
Copy link

vcunat commented Mar 11, 2019

For reference, the browser support claim doesn't seem true from what I can see; curl/curl#3662

@candrews
Copy link
Author

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

@saradickinson
Copy link
Contributor

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: 9d7d999
  • TFO was made the default in Dec 2015, and support was added for OSX that uses a different API to support TF) than linux: 736d9f2. 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
Copy link
Author

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 saradickinson transferred this issue from getdnsapi/stubby Mar 12, 2019
@saradickinson
Copy link
Contributor

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

@saradickinson saradickinson changed the title Use TCP Fast Open Use TCP Fast Open in server.c and possibly update client side TFO implementation? Mar 12, 2019
@odkrys
Copy link

odkrys commented Mar 12, 2019

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

@odkrys
Copy link

odkrys commented Mar 13, 2019

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
Copy link
Contributor

wtoorop commented Mar 15, 2019

@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).

wtoorop added a commit that referenced this issue Mar 15, 2019
Seems to work for TLS now too.
At least on Linux.
Thanks Craig Andrews
@odkrys
Copy link

odkrys commented Mar 15, 2019

sadly, it needs kernel 4.11+...

@wtoorop
Copy link
Contributor

wtoorop commented Feb 14, 2020

Fixed in 5.2.2

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

No branches or pull requests

5 participants