On Fri, 2012-03-23 at 19:46 +0100, Henrik Nordström wrote:
> fre 2012-03-23 klockan 19:49 +0400 skrev Alexander Komyagin:
> 
> > It seems I finally figured out where the problem is. Squid 3.2.0.16
> > performs host verification for each request. And this verification
> > produces the call to libc getaddrinfo() function (converting IP address
> > from text to numeric in my case), but uClibc implementation of this
> > functions looks like not aware of AF_HOSTNUMERIC hint flag, thus
> > performing full lookup (I haven't checked details yet). With RSBAC-Net
> > turned on it's too much overhead. (Actually there are two calls for such
> > conversion per request - one more is in url.cc:urlParseFinish()
> > function)
> 
> Why is the overhead with RSBAC-Net that high in this case?
> 
> getaddrinfo() is a quite frequenty used call.
> 
> But I agree that getaddrinfo for converting textual ip to addrinfo to
> compare with resolved hostname may be a bit overkill. Not even sure why
> we are converting textual IP to to addrinfo there, we should already
> have it in IpAddress internal representaion form.
> 
> > After adding a special hack for numeric IP address conversion requests
> > in Squid (avoiding call to real getaddrinfo() in that case), the
> > performance problem has gone (workers work fine too!).
> 
> Good!
> 
> 
> 

And here is the mentioned patch. I guarded it with "#ifdef __UCLIBC__",
so it can be seamlessly integrated into upstream if needed. But still it
would be better to wait until uClibc is fixed.

Also falling back to bundled functions as Amos suggests, in case uClibc
is used, can't be done without some additional modifications to them
(for example, their declarations conflict with ones in netdb.h file,
which can't be dropped).

-- 
Best wishes,
Alexander Komyagin
--- squid-3.2.0.16.orig/src/ip//Address.cc	2012-03-12 17:26:53.395525059 +0400
+++ squid-3.2.0.16/src/ip//Address.cc	2012-03-26 17:39:16.234967644 +0400
@@ -53,6 +53,12 @@
 #include <arpa/inet.h>
 #endif
 
+//// Temporary hack for uclibc getaddrinfo problem
+#ifdef __UCLIBC__
+#include "AddressLookupHack.cc"
+#endif /* __UCLIBC__ */
+////
+
 /* Debugging only. Dump the address content when a fatal assert is encountered. */
 #define IASSERT(a,b)  \
 	if(!(b)){	printf("assert \"%s\" at line %d\n", a, __LINE__); \
@@ -395,21 +401,34 @@
     struct addrinfo *res = NULL;
 
     struct addrinfo want;
+    
+    int (* __getaddrinfo) (const char *nodename, const char *servname,
+                               const struct addrinfo *hints, struct addrinfo **res) = &getaddrinfo;
+                               
+    void (* __freeaddrinfo) (struct addrinfo *ai) = &freeaddrinfo;
 
     memset(&want, 0, sizeof(struct addrinfo));
     if (nodns) {
         want.ai_flags = AI_NUMERICHOST; // prevent actual DNS lookups!
+       
+#ifdef __UCLIBC__ 
+	//HACK: set __getaddrinfo dynamically since uClibc function
+	// doesn't really care about AI_NUMERICHOST, which can lead
+	// to performance downgrade! [AK]
+	__getaddrinfo = &xgetaddrinfo;
+	__freeaddrinfo = &xfreeaddrinfo;
+#endif /* __UCLIBC__ */
     }
 #if 0
     else if (!Ip::EnableIpv6)
         want.ai_family = AF_INET;  // maybe prevent IPv6 DNS lookups.
 #endif
 
-    if ( (err = getaddrinfo(s, NULL, &want, &res)) != 0) {
+    if ( (err = __getaddrinfo(s, NULL, &want, &res)) != 0) {
         debugs(14,3, HERE << "Given Non-IP '" << s << "': " << gai_strerror(err) );
         /* free the memory getaddrinfo() dynamically allocated. */
         if (res) {
-            freeaddrinfo(res);
+            __freeaddrinfo(res);
             res = NULL;
         }
         return false;
@@ -424,7 +443,7 @@
     SetPort(port);
 
     /* free the memory getaddrinfo() dynamically allocated. */
-    freeaddrinfo(res);
+    __freeaddrinfo(res);
 
     res = NULL;
 
--- squid-3.2.0.16.orig/src/ip//AddressLookupHack.cc	1970-01-01 03:00:00.000000000 +0300
+++ squid-3.2.0.16/src/ip//AddressLookupHack.cc	2012-03-26 16:56:30.090828275 +0400
@@ -0,0 +1,116 @@
+/*
+ * Hack for uClibc getaddrinfo() problems in Ip::Address
+ * AUTHOR: Alexander Komyagin
+ *
+ * By now it seems that uClibc getaddrinfo() totally ingnores
+ * AF_NUMERICHOST hint flag. That leads to a very huge 
+ * performance downgrade (especially with additional overhead
+ * provided by RSBAC Networking when enabled).
+ *
+ */
+
+//all these were gracefully taken from
+// squid compat and reduced when necessary
+// to fit their particular purpose in our
+// situation
+
+static struct addrinfo *
+dup_addrinfo (struct addrinfo *info, void *addr, size_t addrlen) {
+    struct addrinfo *ret;
+
+    ret = (struct addrinfo *)malloc (sizeof (struct addrinfo));
+    if (ret == NULL)
+        return NULL;
+    memcpy (ret, info, sizeof (struct addrinfo));
+    ret->ai_addr = (struct sockaddr *)malloc (addrlen);
+    if (ret->ai_addr == NULL) {
+        free (ret);
+        return NULL;
+    }
+    memcpy (ret->ai_addr, addr, addrlen);
+    ret->ai_addrlen = addrlen;
+    return ret;
+}
+
+static int
+xgetaddrinfo (const char *nodename, const char *servname,
+              const struct addrinfo *hints, struct addrinfo **res)
+{
+    int port = 0;
+    struct addrinfo result;
+    int ret;
+
+    if (servname == NULL && nodename == NULL)
+        return EAI_NONAME;
+
+    memset (&result, 0, sizeof result);
+
+    assert(hints != NULL);
+    assert(servname == NULL);
+    
+    /* if nodename == NULL refer to the local host for a client or any
+       for a server */
+    if (nodename == NULL) {
+        struct sockaddr_in sin;
+
+        /* check protocol family is PF_UNSPEC or PF_INET - could try harder
+           for IPv6 but that's more code than I'm prepared to write */
+        if (hints->ai_family == PF_UNSPEC || hints->ai_family == PF_INET)
+            result.ai_family = AF_INET;
+        else
+            return EAI_FAMILY;
+
+        sin.sin_family = result.ai_family;
+        sin.sin_port = htons (port);
+        if (hints->ai_flags & AI_PASSIVE)
+            sin.sin_addr.s_addr = htonl (INADDR_ANY);
+        else
+            sin.sin_addr.s_addr = htonl (INADDR_LOOPBACK);
+        /* Duplicate result and addr and return */
+        *res = dup_addrinfo (&result, &sin, sizeof sin);
+        return (*res == NULL) ? EAI_MEMORY : 0;
+    }
+
+    /* If AI_NUMERIC is specified, use xinet_pton to translate numbers and
+       dots notation. */
+    assert(hints->ai_flags & AI_NUMERICHOST);
+    
+	struct sockaddr_in sin;
+
+	/* check protocol family is PF_UNSPEC or PF_INET */
+	if (hints->ai_family == PF_UNSPEC || hints->ai_family == PF_INET)
+		result.ai_family = AF_INET;
+	else
+		return EAI_FAMILY;
+
+	sin.sin_family = result.ai_family;
+	sin.sin_port = htons (port);
+
+        ret = inet_pton(result.ai_family, nodename, &sin.sin_addr);
+	if (ret != 1)
+        {
+                debugs(14,3, HERE << "inet_pton: " << ret);
+		return EAI_NONAME;
+        }
+	sin.sin_addr.s_addr = inet_addr (nodename);
+	/* Duplicate result and addr and return */
+	*res = dup_addrinfo (&result, &sin, sizeof sin);
+	return (*res == NULL) ? EAI_MEMORY : 0;
+}
+
+static void
+xfreeaddrinfo (struct addrinfo *ai)
+{
+    struct addrinfo *next;
+
+    while (ai != NULL) {
+        next = ai->ai_next;
+        if (ai->ai_canonname != NULL)
+            free (ai->ai_canonname);
+        if (ai->ai_addr != NULL)
+            free (ai->ai_addr);
+        free (ai);
+        ai = next;
+    }
+}
+

Reply via email to