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 e6ec2ae79d2fe8aa08f549067c0a5f123b433877..0ce9786b0ba8990568bffa0cfb39c197c26f8451 100644 (file)
--- 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 be350069b87d274a5d23736e825a6857b492f7fb..5c935e94409b6c567819bc1ed5ad5658dda4e751 100644 (file)
--- a/src/HttpRequest.h
+++ b/src/HttpRequest.h
@@ -98,12 +98,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
@@ -130,6 +133,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 4e674300e8b7a0b7600a0290ff222a49ca4e4e65..30d2af88472fa0a755d674a342d0f166d05e94ad 100644 (file)
--- 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 e666a850157532af4663df92725550ca7820105b..9b2c5005494bcb28623601423e192c6653384096 100644 (file)
--- 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 84851a2b7d139cb6ae772baea138feb91fdce1b4..dd9572499b191d9f3bbbdd4c76fdff769ffecd8d 100644 (file)
--- 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;