out of order handling of tcp queries

[miekg]

Right now this is serial because: https://github.com/miekg/dns/blob/master/server.go#L567
If we put go in front of that it is out of order.

However I'm not fully convinced the response.Writer deals ok with this; should there be a mutex there?

[miekg]

A simple diff works:

diff --git a/server.go b/server.go
index 685753f..8248508 100644
--- a/server.go
+++ b/server.go
@@ -564,7 +564,7 @@ Redo:
                        w.tsigRequestMAC = req.Extra[len(req.Extra)-1].(*TSIG).MAC
                }
        }
-       h.ServeDNS(w, req) // Writes back to the client
+       go h.ServeDNS(w, req) // Writes back to the client
 
 Exit:
        if w.tcp == nil {
@@ -712,7 +712,6 @@ func (w *response) Close() error {
        // Can't close the udp conn, as that is actually the listener.
        if w.tcp != nil {
                e := w.tcp.Close()
-               w.tcp = nil
                return e
        }
        return nil

but shows up in a test failure in CoreDNS (data-race on axfr), on setting specific dns.ResponseWriter fields; so that thing would indeed need a mutex, also for writing the reply back to the client.

Now this only comes up when using TCP and DNS over TLS and responding to out-of-order request, for UDP we dont' care.

[tmthrgd]

@miekg This change is far more complex than adding a go and a mutex. I can take a closer look in the morning.

(Has this come up as a bottleneck somewhere?)

[miekg]

[ Quoting <notifications@github.com> in "Re: [miekg/dns] out of order handli..." ]

@miekg This change is far more complex than adding a `go ` and a mutex. I can take a closer look in the morning. (Has this come up as a bottleneck somewhere?)

No, not directly, but with DNS-over-TLS this might speed up things. I'm pretty worried that making it faster for the DNS-over-TLS will make it slower for UDP. Thanks for taking a look!

[tmthrgd]

@miekg I took a look at rewriting the serve method with the intention of allowing this change.

It's looking like it's impossible to make this work with the Hijack method. I'm not sure how useful the Hijack method is in general, or what the intention behind it was, but I doubt it's possible to remove it while keeping it backward compatible.

[miekg]

Can't be done in current architecture; also DoT is quite bad for performance etc. etc.

Closing.