[Pdns-dev] Patch for handling of TCP queries to authoritative server.

[Augie Schwer]

http://wiki.powerdns.com/cgi-bin/trac.fcgi/ticket/118

Patch below and attached to the above ticket.

The changes here include:

1) Change the order that a query gets processed; it used to be:
Recursion -> Packet Cache -> Backend ; now it is:
Packet Cache -> Backend -> Recursion .

2) Do the above order even if "Recursion Desired" (rd) is specified by
the client.

3) Create a copy of the DNSPacket object used since the PacketHandler
seems to mess with the existing packet so if recursion is attempted it
will fail.

It's been a while since I have done any C/C++ coding, but I think this
should be bug free; I have been throwing quite a bit of non-unique
traffic at a build with this patch on a lab box and it hasn't fallen
over yet. :)

Index: tcpreceiver.cc
===================================================================
--- tcpreceiver.cc      (revision 940)
+++ tcpreceiver.cc      (working copy)
@@ -169,12 +169,55 @@
        continue;
       }

+       // Check Packet Cache.
+       DNSPacket* cached=new DNSPacket;
+       DLOG(L<<"About to check Packet Cache for query answer."<<endl);
+       if(PC.get(packet, cached)) { // short circuit - does the
PacketCache recognize this question?
+               cached->setRemote(&packet->remote);
+               cached->spoofID(packet->d.id);
+               DLOG(L<<"got an answer from Packet Cache."<<endl);
+               if(sendDelPacket(cached, fd)<0)
+                 goto out;

+               S.inc("tcp-answers");
+               continue;
+      }
+      else
+       delete cached;
+
+      // Check Backend.
+      // The PacketHandler question method screws the packet up for future use.
+      //   so I make a copy here.
+      DNSPacket *packet2 = NULL;
+      packet2 = new DNSPacket;
+      packet2->setRemote(&remote);
+      packet2->d_tcp=true;
+      packet2->parse(mesg, pktlen);
+
+      DNSPacket *reply;
+      {
+       Lock l(&s_plock);
+       if(!s_P) {
+         L<<Logger::Error<<"TCP server is without backend
connections, launching"<<endl;
+         s_P=new PacketHandler;
+       }
+       DLOG(L<<"About to check backend for query answer."<<endl);
+       reply=s_P->question(packet2); // we really need to ask the backend :-)
+      }
+
+      if(packet2 != NULL)
+       delete packet2;
+
+      if(reply) { // able to write an answer?
+       DLOG(L<<"got answer from backend."<<endl);
+       S.inc("tcp-answers");
+       sendDelPacket(reply, fd);
+       continue;
+      }
+
       if(packet->d.rd && arg().mustDo("recursor")) {
        // now what
        // this is a pretty rare event all in all, so we can afford to be slow
-
-       // this code SHOULD attempt to answer from the local cache first!
        S.inc("recursing-questions");
        Resolver res;
        unsigned int len;
@@ -194,38 +237,6 @@
        }
        continue;
       }
-
-      DNSPacket* cached=new DNSPacket;
-      if(!packet->d.rd && (PC.get(packet, cached))) { // short
circuit - does the PacketCache recognize this question?
-       cached->setRemote(&packet->remote);
-       cached->spoofID(packet->d.id);
-       if(sendDelPacket(cached, fd)<0)
-         goto out;
-
-       S.inc("tcp-answers");
-       continue;
-      }
-      else
-       delete cached;
-
-      DNSPacket *reply;
-      {
-       Lock l(&s_plock);
-       if(!s_P) {
-         L<<Logger::Error<<"TCP server is without backend
connections, launching"<<endl;
-         s_P=new PacketHandler;
-       }
-       reply=s_P->question(packet); // we really need to ask the backend :-)
-      }
-
-      delete packet;
-      packet = NULL;
-
-      if(!reply)  // unable to write an answer?
-       break;
-
-      S.inc("tcp-answers");
-      sendDelPacket(reply, fd);
     }

   out:


-- 
Augie Schwer    -    Augie at Schwer.us    -    http://schwer.us
Key fingerprint = 9815 AE19 AFD1 1FE7 5DEE 2AC3 CB99 2784 27B0 C072