[squid-dev] [PATCH] Remove cache_peer_domain

Amos Jeffries squid3 at treenet.co.nz
Sun Feb 1 18:22:45 UTC 2015


The cache_peer_domain directive functionality is also provided through
cache_peer_access.

While this check appears at face value to be simpler than ACLs, the
reality is that:
* the difference is simply the time it takes to initialize and destruct
an on-stack Checklist,
* processing the checks may take longer than ACLs (linked-list of string
comparisons vs single tree lookup)
* ACLs are the common case due to their extra flexibility, and
* extra work is being done per-transaction just to check which of the
two features is in use.

Overall I just dont believe it to be worth keeping. By removing we gain
less code and configuration directives to work around in the long term.

Amos
-------------- next part --------------
=== modified file 'src/CachePeer.h'
--- src/CachePeer.h	2015-01-13 07:25:36 +0000
+++ src/CachePeer.h	2015-02-01 17:07:09 +0000
@@ -5,41 +5,40 @@
  * contributions from numerous individuals and organizations.
  * Please see the COPYING and CONTRIBUTORS files for details.
  */
 
 #ifndef SQUID_CACHEPEER_H_
 #define SQUID_CACHEPEER_H_
 
 #include "acl/forward.h"
 #include "base/CbcPointer.h"
 #include "enums.h"
 #include "icp_opcode.h"
 #include "ip/Address.h"
 
 //TODO: remove, it is unconditionally defined and always used.
 #define PEER_MULTICAST_SIBLINGS 1
 
 #if HAVE_OPENSSL_SSL_H
 #include <openssl/ssl.h>
 #endif
 
-class CachePeerDomainList;
 class NeighborTypeDomainList;
 class PconnPool;
 class PeerDigest;
 class PeerPoolMgr;
 
 // currently a POD
 class CachePeer
 {
 public:
     u_int index;
     char *name;
     char *host;
     peer_t type;
 
     Ip::Address in_addr;
 
     struct {
         int pings_sent;
         int pings_acked;
         int fetches;
@@ -54,41 +53,40 @@
         time_t last_connect_probe;
         int logged_state;   /* so we can print dead/revived msgs */
         int conn_open;      /* current opened connections */
     } stats;
 
     struct {
         int version;
         int counts[ICP_END+1];
         unsigned short port;
     } icp;
 
 #if USE_HTCP
     struct {
         double version;
         int counts[2];
         unsigned short port;
     } htcp;
 #endif
 
     unsigned short http_port;
-    CachePeerDomainList *peer_domain;
     NeighborTypeDomainList *typelist;
     acl_access *access;
 
     struct {
         bool proxy_only;
         bool no_query;
         bool background_ping;
         bool no_digest;
         bool default_parent;
         bool roundrobin;
         bool weighted_roundrobin;
         bool mcast_responder;
         bool closest_only;
 #if USE_HTCP
         bool htcp;
         bool htcp_oldsquid;
         bool htcp_no_clr;
         bool htcp_no_purge_clr;
         bool htcp_only_clr;
         bool htcp_forward_clr;

=== removed file 'src/CachePeerDomainList.h'
--- src/CachePeerDomainList.h	2015-01-13 07:25:36 +0000
+++ src/CachePeerDomainList.h	1970-01-01 00:00:00 +0000
@@ -1,22 +0,0 @@
-/*
- * Copyright (C) 1996-2015 The Squid Software Foundation and contributors
- *
- * Squid software is distributed under GPLv2+ license and includes
- * contributions from numerous individuals and organizations.
- * Please see the COPYING and CONTRIBUTORS files for details.
- */
-
-#ifndef SQUID_CACHEPEERDOMAINLIST_H_
-#define SQUID_CACHEPEERDOMAINLIST_H_
-
-/// representation of the cache_peer_domain list. POD.
-class CachePeerDomainList
-{
-public:
-    char *domain;
-    bool do_ping;
-    CachePeerDomainList *next;
-};
-
-#endif /* SQUID_CACHEPEERDOMAINLIST_H_ */
-

=== modified file 'src/Makefile.am'
--- src/Makefile.am	2015-01-29 16:09:11 +0000
+++ src/Makefile.am	2015-02-01 16:41:28 +0000
@@ -268,41 +268,40 @@
 	DescriptorSet.h \
 	SquidConfig.h \
 	SquidConfig.cc
 
 squid_SOURCES = \
 	$(ACL_REGISTRATION_SOURCES) \
 	AccessLogEntry.cc \
 	AccessLogEntry.h \
 	AsyncEngine.cc \
 	AsyncEngine.h \
 	cache_cf.h \
 	AuthReg.h \
 	YesNoNone.h \
 	YesNoNone.cc \
 	RefreshPattern.h \
 	cache_cf.cc \
 	CacheDigest.h \
 	CacheDigest.cc \
 	cache_manager.cc \
 	NeighborTypeDomainList.h \
-	CachePeerDomainList.h \
 	CachePeer.h \
 	CacheManager.h \
 	carp.h \
 	carp.cc \
 	cbdata.cc \
 	cbdata.h \
 	ChunkedCodingParser.cc \
 	ChunkedCodingParser.h \
 	client_db.h \
 	client_db.cc \
 	client_side.h \
 	client_side.cc \
 	client_side.h \
 	client_side_reply.cc \
 	client_side_reply.h \
 	client_side_request.cc \
 	client_side_request.h \
 	ClientInfo.h \
 	BodyPipe.cc \
 	BodyPipe.h \

=== modified file 'src/cache_cf.cc'
--- src/cache_cf.cc	2015-01-29 19:05:24 +0000
+++ src/cache_cf.cc	2015-02-01 17:08:05 +0000
@@ -6,41 +6,40 @@
  * Please see the COPYING and CONTRIBUTORS files for details.
  */
 
 /* DEBUG: section 03    Configuration File Parsing */
 
 #include "squid.h"
 #include "acl/Acl.h"
 #include "acl/AclAddress.h"
 #include "acl/AclDenyInfoList.h"
 #include "acl/AclNameList.h"
 #include "acl/AclSizeLimit.h"
 #include "acl/Gadgets.h"
 #include "acl/MethodData.h"
 #include "acl/Tree.h"
 #include "anyp/PortCfg.h"
 #include "anyp/UriScheme.h"
 #include "AuthReg.h"
 #include "base/RunnersRegistry.h"
 #include "cache_cf.h"
 #include "CachePeer.h"
-#include "CachePeerDomainList.h"
 #include "ConfigParser.h"
 #include "CpuAffinityMap.h"
 #include "DiskIO/DiskIOModule.h"
 #include "eui/Config.h"
 #include "ExternalACL.h"
 #include "format/Format.h"
 #include "ftp/Elements.h"
 #include "globals.h"
 #include "HttpHeaderTools.h"
 #include "ident/Config.h"
 #include "ip/Intercept.h"
 #include "ip/QosConfig.h"
 #include "ip/tools.h"
 #include "ipc/Kids.h"
 #include "log/Config.h"
 #include "log/CustomLog.h"
 #include "MemBuf.h"
 #include "mgr/ActionPasswordList.h"
 #include "mgr/Registration.h"
 #include "neighbors.h"
@@ -1951,61 +1950,53 @@
 
     case PEER_SIBLING:
         result = "sibling";
         break;
 
     case PEER_MULTICAST:
         result = "multicast";
         break;
 
     default:
         result = "unknown";
         break;
     }
 
     return result;
 }
 
 static void
 dump_peer(StoreEntry * entry, const char *name, CachePeer * p)
 {
-    CachePeerDomainList *d;
     NeighborTypeDomainList *t;
     LOCAL_ARRAY(char, xname, 128);
 
     while (p != NULL) {
         storeAppendPrintf(entry, "%s %s %s %d %d name=%s",
                           name,
                           p->host,
                           neighborTypeStr(p),
                           p->http_port,
                           p->icp.port,
                           p->name);
         dump_peer_options(entry, p);
 
-        for (d = p->peer_domain; d; d = d->next) {
-            storeAppendPrintf(entry, "cache_peer_domain %s %s%s\n",
-                              p->host,
-                              d->do_ping ? null_string : "!",
-                              d->domain);
-        }
-
         if (p->access) {
             snprintf(xname, 128, "cache_peer_access %s", p->name);
             dump_acl_access(entry, xname, p->access);
         }
 
         for (t = p->typelist; t; t = t->next) {
             storeAppendPrintf(entry, "neighbor_type_domain %s %s %s\n",
                               p->host,
                               peer_type_str(t->type),
                               t->domain);
         }
 
         p = p->next;
     }
 }
 
 /**
  * utility function to prevent getservbyname() being called with a numeric value
  * on Windows at least it returns garage results.
  */
@@ -2492,74 +2483,40 @@
 static void
 parse_peer_access(void)
 {
     char *host = NULL;
     CachePeer *p;
 
     if (!(host = ConfigParser::NextToken()))
         self_destruct();
 
     if ((p = peerFindByName(host)) == NULL) {
         debugs(15, DBG_CRITICAL, "" << cfg_filename << ", line " << config_lineno << ": No cache_peer '" << host << "'");
         return;
     }
 
     std::string directive = "peer_access ";
     directive += host;
     aclParseAccessLine(directive.c_str(), LegacyParser, &p->access);
 }
 
 static void
-parse_hostdomain(void)
-{
-    char *host = NULL;
-    char *domain = NULL;
-
-    if (!(host = ConfigParser::NextToken()))
-        self_destruct();
-
-    while ((domain = ConfigParser::NextToken())) {
-        CachePeerDomainList *l = NULL;
-        CachePeerDomainList **L = NULL;
-        CachePeer *p;
-
-        if ((p = peerFindByName(host)) == NULL) {
-            debugs(15, DBG_CRITICAL, "" << cfg_filename << ", line " << config_lineno << ": No cache_peer '" << host << "'");
-            continue;
-        }
-
-        l = static_cast<CachePeerDomainList *>(xcalloc(1, sizeof(CachePeerDomainList)));
-        l->do_ping = true;
-
-        if (*domain == '!') {   /* check for !.edu */
-            l->do_ping = false;
-            ++domain;
-        }
-
-        l->domain = xstrdup(domain);
-
-        for (L = &(p->peer_domain); *L; L = &((*L)->next));
-        *L = l;
-    }
-}
-
-static void
 parse_hostdomaintype(void)
 {
     char *host = NULL;
     char *type = NULL;
     char *domain = NULL;
 
     if (!(host = ConfigParser::NextToken()))
         self_destruct();
 
     if (!(type = ConfigParser::NextToken()))
         self_destruct();
 
     while ((domain = ConfigParser::NextToken())) {
         NeighborTypeDomainList *l = NULL;
         NeighborTypeDomainList **L = NULL;
         CachePeer *p;
 
         if ((p = peerFindByName(host)) == NULL) {
             debugs(15, DBG_CRITICAL, "" << cfg_filename << ", line " << config_lineno << ": No cache_peer '" << host << "'");
             return;

=== modified file 'src/cf.data.pre'
--- src/cf.data.pre	2015-01-31 16:34:08 +0000
+++ src/cf.data.pre	2015-02-01 17:14:20 +0000
@@ -131,40 +131,47 @@
 DOC_END
 
 NAME: external_refresh_check
 TYPE: obsolete
 DOC_START
 	This option is not yet supported by Squid-3.
 DOC_END
 
 NAME: location_rewrite_program location_rewrite_access location_rewrite_children location_rewrite_concurrency
 TYPE: obsolete
 DOC_START
 	This option is not yet supported by Squid-3.
 DOC_END
 
 NAME: refresh_stale_hit
 TYPE: obsolete
 DOC_START
 	This option is not yet supported by Squid-3.
 DOC_END
 
+# Options removed in 3.6
+NAME: cache_peer_domain cache_host_domain
+TYPE: obsolete
+DOC_START
+	Replace with dstdomain ACLs and cache_peer_access.
+DOC_END
+
 # Options removed in 3.5
 NAME: hierarchy_stoplist
 TYPE: obsolete
 DOC_START
 	Remove this line. Use always_direct or cache_peer_access ACLs instead if you need to prevent cache_peer use.
 DOC_END
 
 # Options removed in 3.4
 NAME: log_access
 TYPE: obsolete
 DOC_START
 	Remove this line. Use acls with access_log directives to control access logging
 DOC_END
 
 NAME: log_icap
 TYPE: obsolete
 DOC_START
 	Remove this line. Use acls with icap_log directives to control icap logging
 DOC_END
 
@@ -3325,87 +3332,54 @@
 			connections. Default request_timeout and
 			server_idle_pconn_timeout values ensure such a
 			configuration.
 	
 	name=xxx	Unique name for the peer.
 			Required if you have multiple peers on the same host
 			but different ports.
 			This name can be used in cache_peer_access and similar
 			directives to dentify the peer.
 			Can be used by outgoing access controls through the
 			peername ACL type.
 	
 	no-tproxy	Do not use the client-spoof TPROXY support when forwarding
 			requests to this peer. Use normal address selection instead.
 			This overrides the spoof_client_ip ACL.
 	
 	proxy-only	objects fetched from the peer will not be stored locally.
 	
 DOC_END
 
-NAME: cache_peer_domain cache_host_domain
-TYPE: hostdomain
-DEFAULT: none
-LOC: none
-DOC_START
-	Use to limit the domains for which a neighbor cache will be
-	queried.
-
-	Usage:
-		cache_peer_domain cache-host domain [domain ...]
-		cache_peer_domain cache-host !domain
-
-	For example, specifying
-
-		cache_peer_domain parent.foo.net	.edu
-
-	has the effect such that UDP query packets are sent to
-	'bigserver' only when the requested object exists on a
-	server in the .edu domain.  Prefixing the domainname
-	with '!' means the cache will be queried for objects
-	NOT in that domain.
-
-	NOTE:	* Any number of domains may be given for a cache-host,
-		  either on the same or separate lines.
-		* When multiple domains are given for a particular
-		  cache-host, the first matched domain is applied.
-		* Cache hosts with no domain restrictions are queried
-		  for all requests.
-		* There are no defaults.
-		* There is also a 'cache_peer_access' tag in the ACL
-		  section.
-DOC_END
-
 NAME: cache_peer_access
 TYPE: peer_access
 DEFAULT: none
 LOC: none
 DOC_START
-	Similar to 'cache_peer_domain' but provides more flexibility by
-	using ACL elements.
+	Use to limit the requests for which a neighbor proxy will be
+	queried. Peers with no restrictions are queried for all requests.
 
 	Usage:
 		cache_peer_access cache-host allow|deny [!]aclname ...
 
 	The syntax is identical to 'http_access' and the other lists of
-	ACL elements.  See the comments for 'http_access' below, or
-	the Squid FAQ (http://wiki.squid-cache.org/SquidFaq/SquidAcl).
+	ACL elements.  See the comments for 'http_access', or the
+	Squid FAQ (http://wiki.squid-cache.org/SquidFaq/SquidAcl).
 DOC_END
 
 NAME: neighbor_type_domain
 TYPE: hostdomaintype
 DEFAULT: none
 DEFAULT_DOC: The peer type from cache_peer directive is used for all requests to that peer.
 LOC: none
 DOC_START
 	Modify the cache_peer neighbor type when passing requests
 	about specific domains to the peer.
 
 	Usage:
 		 neighbor_type_domain neighbor parent|sibling domain domain ...
 
 	For example:
 		cache_peer foo.example.com parent 3128 3130
 		neighbor_type_domain foo.example.com sibling .au .de
 
 	The above configuration treats all requests to foo.example.com as a
 	parent proxy unless the request is for a .au or .de ccTLD domain name.

=== modified file 'src/neighbors.cc'
--- src/neighbors.cc	2015-01-29 16:09:11 +0000
+++ src/neighbors.cc	2015-02-01 17:06:32 +0000
@@ -1,36 +1,35 @@
 /*
  * Copyright (C) 1996-2015 The Squid Software Foundation and contributors
  *
  * Squid software is distributed under GPLv2+ license and includes
  * contributions from numerous individuals and organizations.
  * Please see the COPYING and CONTRIBUTORS files for details.
  */
 
 /* DEBUG: section 15    Neighbor Routines */
 
 #include "squid.h"
 #include "acl/FilledChecklist.h"
 #include "anyp/PortCfg.h"
 #include "CacheDigest.h"
 #include "CachePeer.h"
-#include "CachePeerDomainList.h"
 #include "comm/Connection.h"
 #include "comm/ConnOpener.h"
 #include "event.h"
 #include "FwdState.h"
 #include "globals.h"
 #include "htcp.h"
 #include "HttpRequest.h"
 #include "icmp/net_db.h"
 #include "ICP.h"
 #include "int.h"
 #include "ip/Address.h"
 #include "ip/tools.h"
 #include "ipcache.h"
 #include "MemObject.h"
 #include "mgr/Registration.h"
 #include "multicast.h"
 #include "neighbors.h"
 #include "NeighborTypeDomainList.h"
 #include "pconn.h"
 #include "PeerDigest.h"
@@ -120,85 +119,68 @@
         if (0 == matchDomainName(request->GetHost(), d->domain))
             if (d->type != PEER_NONE)
                 return d->type;
     }
 #if PEER_MULTICAST_SIBLINGS
     if (p->type == PEER_MULTICAST)
         if (p->options.mcast_siblings)
             return PEER_SIBLING;
 #endif
 
     return p->type;
 }
 
 /**
  * \return Whether it is appropriate to fetch REQUEST from PEER.
  */
 bool
 peerAllowedToUse(const CachePeer * p, HttpRequest * request)
 {
 
-    const CachePeerDomainList *d = NULL;
     assert(request != NULL);
 
     if (neighborType(p, request) == PEER_SIBLING) {
 #if PEER_MULTICAST_SIBLINGS
         if (p->type == PEER_MULTICAST && p->options.mcast_siblings &&
                 (request->flags.noCache || request->flags.refresh || request->flags.loopDetected || request->flags.needValidation))
             debugs(15, 2, "peerAllowedToUse(" << p->name << ", " << request->GetHost() << ") : multicast-siblings optimization match");
 #endif
         if (request->flags.noCache)
             return false;
 
         if (request->flags.refresh)
             return false;
 
         if (request->flags.loopDetected)
             return false;
 
         if (request->flags.needValidation)
             return false;
     }
 
     // CONNECT requests are proxy requests. Not to be forwarded to origin servers.
     // Unless the destination port matches, in which case we MAY perform a 'DIRECT' to this CachePeer.
     if (p->options.originserver && request->method == Http::METHOD_CONNECT && request->port != p->in_addr.port())
         return false;
 
-    if (p->peer_domain == NULL && p->access == NULL)
-        return true;
-
-    bool do_ping = false;
-    for (d = p->peer_domain; d; d = d->next) {
-        if (0 == matchDomainName(request->GetHost(), d->domain)) {
-            do_ping = d->do_ping;
-            break;
-        }
-
-        do_ping = !d->do_ping;
-    }
-
-    if (p->peer_domain && !do_ping)
-        return false;
-
     if (p->access == NULL)
-        return do_ping;
+        return true;
 
     ACLFilledChecklist checklist(p->access, request, NULL);
 
     return (checklist.fastCheck() == ACCESS_ALLOWED);
 }
 
 /* Return TRUE if it is okay to send an ICP request to this CachePeer.   */
 static int
 peerWouldBePinged(const CachePeer * p, HttpRequest * request)
 {
     if (p->icp.port == 0)
         return 0;
 
     if (p->options.no_query)
         return 0;
 
     if (p->options.mcast_responder)
         return 0;
 
     if (p->n_addresses == 0)
@@ -1160,48 +1142,40 @@
     }
 
     if (p->stats.probe_start != 0 &&
             squid_curtime - p->stats.probe_start > Config.Timeout.deadPeer) {
         debugs(15, 8, "neighborUp: DOWN (dead): " << p->host << " (" << p->in_addr << ")");
         return 0;
     }
 
     debugs(15, 8, "neighborUp: UP: " << p->host << " (" << p->in_addr << ")");
     return 1;
 }
 
 void
 peerDestroy(void *data)
 {
     CachePeer *p = (CachePeer *)data;
 
     if (p == NULL)
         return;
 
-    CachePeerDomainList *nl = NULL;
-
-    for (CachePeerDomainList *l = p->peer_domain; l; l = nl) {
-        nl = l->next;
-        safe_free(l->domain);
-        xfree(l);
-    }
-
     safe_free(p->host);
     safe_free(p->name);
     safe_free(p->domain);
 #if USE_CACHE_DIGESTS
 
     cbdataReferenceDone(p->digest);
 #endif
 }
 
 void
 peerNoteDigestGone(CachePeer * p)
 {
 #if USE_CACHE_DIGESTS
     cbdataReferenceDone(p->digest);
 #endif
 }
 
 static void
 peerDNSConfigure(const ipcache_addrs *ia, const Dns::LookupDetails &, void *data)
 {
@@ -1588,50 +1562,48 @@
 
     if (p->options.originserver)
         storeAppendPrintf(sentry, " originserver");
 
     if (p->domain)
         storeAppendPrintf(sentry, " forceddomain=%s", p->domain);
 
     if (p->connection_auth == 0)
         storeAppendPrintf(sentry, " connection-auth=off");
     else if (p->connection_auth == 1)
         storeAppendPrintf(sentry, " connection-auth=on");
     else if (p->connection_auth == 2)
         storeAppendPrintf(sentry, " connection-auth=auto");
 
     storeAppendPrintf(sentry, "\n");
 }
 
 static void
 dump_peers(StoreEntry * sentry, CachePeer * peers)
 {
-    CachePeer *e = NULL;
     char ntoabuf[MAX_IPSTRLEN];
-    CachePeerDomainList *d = NULL;
     icp_opcode op;
     int i;
 
     if (peers == NULL)
         storeAppendPrintf(sentry, "There are no neighbors installed.\n");
 
-    for (e = peers; e; e = e->next) {
+    for (CachePeer *e = peers; e; e = e->next) {
         assert(e->host != NULL);
         storeAppendPrintf(sentry, "\n%-11.11s: %s\n",
                           neighborTypeStr(e),
                           e->name);
         storeAppendPrintf(sentry, "Host       : %s/%d/%d\n",
                           e->host,
                           e->http_port,
                           e->icp.port);
         storeAppendPrintf(sentry, "Flags      :");
         dump_peer_options(sentry, e);
 
         for (i = 0; i < e->n_addresses; ++i) {
             storeAppendPrintf(sentry, "Address[%d] : %s\n", i,
                               e->addresses[i].toStr(ntoabuf,MAX_IPSTRLEN) );
         }
 
         storeAppendPrintf(sentry, "Status     : %s\n",
                           neighborUp(e) ? "Up" : "Down");
         storeAppendPrintf(sentry, "FETCHES    : %d\n", e->stats.fetches);
         storeAppendPrintf(sentry, "OPEN CONNS : %d\n", e->stats.conn_open);
@@ -1676,51 +1648,40 @@
 
                     storeAppendPrintf(sentry, "    %12.12s : %8d %3d%%\n",
                                       icp_opcode_str[op],
                                       e->icp.counts[op],
                                       Math::intPercent(e->icp.counts[op], e->stats.pings_acked));
                 }
 
 #if USE_HTCP
 
             }
 
 #endif
 
         }
 
         if (e->stats.last_connect_failure) {
             storeAppendPrintf(sentry, "Last failed connect() at: %s\n",
                               Time::FormatHttpd(e->stats.last_connect_failure));
         }
 
-        if (e->peer_domain != NULL) {
-            storeAppendPrintf(sentry, "DOMAIN LIST: ");
-
-            for (d = e->peer_domain; d; d = d->next) {
-                storeAppendPrintf(sentry, "%s%s ",
-                                  d->do_ping ? null_string : "!", d->domain);
-            }
-
-            storeAppendPrintf(sentry, "\n");
-        }
-
         storeAppendPrintf(sentry, "keep-alive ratio: %d%%\n", Math::intPercent(e->stats.n_keepalives_recv, e->stats.n_keepalives_sent));
     }
 }
 
 #if USE_HTCP
 void
 neighborsHtcpReply(const cache_key * key, HtcpReplyData * htcp, const Ip::Address &from)
 {
     StoreEntry *e = Store::Root().get(key);
     MemObject *mem = NULL;
     CachePeer *p;
     peer_t ntype = PEER_NONE;
     debugs(15, 6, "neighborsHtcpReply: " <<
            (htcp->hit ? "HIT" : "MISS") << " " <<
            storeKeyText(key)  );
 
     if (NULL != e)
         mem = e->mem_obj;
 
     if ((p = whichPeer(from)))



More information about the squid-dev mailing list