From: Amos Jeffries <squid3@treenet.co.nz>
Date: Thu, 12 Nov 2009 12:04:08 +0000 (+1300)
Subject: Bug 2617: Performance degradation during processing list of dstdomain ACL's
X-Git-Tag: SQUID_3_1_0_15~33
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=bcf0ef895ea753696557cc8566dd235f3cd777be;p=thirdparty%2Fsquid.git

Bug 2617: Performance degradation during processing list of dstdomain ACL's

Previously a raw-IP needed looking up every time a dstdomain entry was to
be checked against it. This caches the rDNS value across a checklist series.

Such that constructs like the following only do a single DNS check:

   acl foo dstdomain localhost local domain
   http_access allow foo foo !foo
   http_access deny foo
---

diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc
index 11ed802118..9469d721b8 100644
--- a/src/HttpRequest.cc
+++ b/src/HttpRequest.cc
@@ -76,6 +76,7 @@ HttpRequest::init()
     urlpath = NULL;
     login[0] = '\0';
     host[0] = '\0';
+    host_is_numeric = -1;
     auth_user_request = NULL;
     pinned_connection = NULL;
     port = 0;
diff --git a/src/HttpRequest.h b/src/HttpRequest.h
index 98d98716aa..e8b031ff44 100644
--- a/src/HttpRequest.h
+++ b/src/HttpRequest.h
@@ -92,12 +92,15 @@ public:
         host_addr = src;
         if ( host_addr.IsAnyAddr() ) {
             xstrncpy(host, src, SQUIDHOSTNAMELEN);
+            host_is_numeric = 0;
         } else {
             host_addr.ToHostname(host, SQUIDHOSTNAMELEN);
             debugs(23, 3, "HttpRequest::SetHost() given IP: " << host_addr);
+            host_is_numeric = 1;
         }
     };
     inline const char* GetHost(void) const { return host; };
+    inline const int GetHostIsNumeric(void) const { return host_is_numeric; };
 
 #if USE_ADAPTATION
     /// Returns possibly nil history, creating it if adapt. logging is enabled
@@ -124,6 +127,7 @@ public:
 
 private:
     char host[SQUIDHOSTNAMELEN];
+    int host_is_numeric;
 
     /***
      * The client side connection data of pinned connections for the client side
diff --git a/src/acl/DestinationDomain.cc b/src/acl/DestinationDomain.cc
index 4e674300e8..5d765343d1 100644
--- a/src/acl/DestinationDomain.cc
+++ b/src/acl/DestinationDomain.cc
@@ -76,20 +76,34 @@ ACLDestinationDomainStrategy::match (ACLData<MatchType> * &data, ACLFilledCheckl
 {
     assert(checklist != NULL && checklist->request != NULL);
 
-    const ipcache_addrs *ia = NULL;
-    const char *fqdn = NULL;
-
-    if (data->match(checklist->request->GetHost()))
+    if (data->match(checklist->request->GetHost())) {
         return 1;
+    }
+
+    /* numeric IPA? no, trust the above result. */
+    if (checklist->request->GetHostIsNumeric() == 0) {
+        return 0;
+    }
+
+    /* do we already have the rDNS? match on it if we do. */
+    if (checklist->dst_rdns) {
+        debugs(28, 3, "aclMatchAcl: '" << AclMatchedName << "' match with stored rDNS '" << checklist->dst_rdns << "' for '" << checklist->request->GetHost() << "'");
+        return data->match(checklist->dst_rdns);
+    }
 
-    /* numeric IPA? */
-    if ((ia = ipcacheCheckNumeric(checklist->request->GetHost())) == NULL)
+    /* raw IP without rDNS? look it up and wait for the result */
+    const ipcache_addrs *ia = ipcacheCheckNumeric(checklist->request->GetHost());
+    if (!ia) {
+        /* not a valid IPA */
+        checklist->dst_rdns = xstrdup("invalid");
         return 0;
+    }
 
     checklist->dst_addr = ia->in_addrs[0];
-    fqdn = fqdncache_gethostbyaddr(checklist->dst_addr, FQDN_LOOKUP_IF_MISS);
+    const char *fqdn = fqdncache_gethostbyaddr(checklist->dst_addr, FQDN_LOOKUP_IF_MISS);
 
     if (fqdn) {
+        checklist->dst_rdns = xstrdup(fqdn);
         return data->match(fqdn);
     } else if (!checklist->destinationDomainChecked()) {
         /* FIXME: Using AclMatchedName here is not OO correct. Should find a way to the current acl */
diff --git a/src/acl/FilledChecklist.cc b/src/acl/FilledChecklist.cc
index e666a85015..9b2c500549 100644
--- a/src/acl/FilledChecklist.cc
+++ b/src/acl/FilledChecklist.cc
@@ -120,6 +120,7 @@ ACLFilledChecklist::operator delete (void *address)
 
 ACLFilledChecklist::ACLFilledChecklist() :
         dst_peer(NULL),
+        dst_rdns(NULL),
         request (NULL),
         reply (NULL),
         auth_user_request (NULL),
@@ -146,6 +147,8 @@ ACLFilledChecklist::~ACLFilledChecklist()
 {
     assert (!asyncInProgress());
 
+    safe_free(dst_rdns); // created by xstrdup().
+
     if (extacl_entry)
         cbdataReferenceDone(extacl_entry);
 
@@ -231,6 +234,7 @@ ACLFilledChecklist::markSourceDomainChecked()
  */
 ACLFilledChecklist::ACLFilledChecklist(const acl_access *A, HttpRequest *http_request, const char *ident):
         dst_peer(NULL),
+        dst_rdns(NULL),
         request(NULL),
         reply(NULL),
         auth_user_request(NULL),
diff --git a/src/acl/FilledChecklist.h b/src/acl/FilledChecklist.h
index 84851a2b7d..dd9572499b 100644
--- a/src/acl/FilledChecklist.h
+++ b/src/acl/FilledChecklist.h
@@ -47,6 +47,7 @@ public:
     IpAddress dst_addr;
     IpAddress my_addr;
     struct peer *dst_peer;
+    char *dst_rdns;
 
     HttpRequest *request;
     HttpReply *reply;