You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Jeff Trawick <tr...@attglobal.net> on 2002/09/20 20:17:25 UTC

[pre-PATCH] short-circuit logic for apr_sockaddr_info_get()

With the following patch to mod_proxy and APR, I saw drastic
improvements to proxy throughput because mod_proxy was able to tell
APR that we should only look for IPv6 addresses if the target has no
IPv4 address.  (Currently, the way we maintain the capability of
talking to IPv6 nodes is to ask getaddrinfo() to look for all
addresses, but that results in two lookups by the resolver library.)

See related discussion on new-httpd earlier this month:
http://marc.theaimsgroup.com/?l=apache-httpd-dev&m=103124053118349&w=2

Does anyone have concerns about the API?  There is still some logic
cleanup to be done before committing anyway.

Index: srclib/apr/include/apr_network_io.h
===================================================================
RCS file: /home/cvs/apr/include/apr_network_io.h,v
retrieving revision 1.128
diff -u -r1.128 apr_network_io.h
--- srclib/apr/include/apr_network_io.h	15 Aug 2002 03:23:36 -0000	1.128
+++ srclib/apr/include/apr_network_io.h	20 Sep 2002 18:00:00 -0000
@@ -136,6 +136,9 @@
 typedef enum {APR_SHUTDOWN_READ, APR_SHUTDOWN_WRITE, 
 	      APR_SHUTDOWN_READWRITE} apr_shutdown_how_e;
 
+#define APR_IPV4_ADDR_OK  0x01  /* see doc for apr_sockaddr_info_get() */
+#define APR_IPV6_ADDR_OK  0x02  /* see doc for apr_sockaddr_info_get() */
+
 #if (!APR_HAVE_IN_ADDR)
 /**
  * We need to make sure we always have an in_addr type, so APR will just
@@ -339,7 +342,15 @@
  * @param family The address family to use, or APR_UNSPEC if the system should 
  *               decide.
  * @param port The port number.
- * @param flags Special processing flags.
+ * @param flags Special processing flags:
+ * <PRE>
+ *       APR_IPV4_ADDR_OK          first query for IPv4 addresses; only look
+ *                                 for IPv6 addresses if the first query failed
+ *                                 (flag only valid if family is APR_UNSPEC)
+ *       APR_IPV6_ADDR_OK          first query for IPv6 addresses; only look
+ *                                 for IPv4 addresses if the first query failed
+ *                                 (flag only valid if family is APR_UNSPEC)
+ * </PRE>
  * @param p The pool for the apr_sockaddr_t and associated storage.
  */
 APR_DECLARE(apr_status_t) apr_sockaddr_info_get(apr_sockaddr_t **sa,
Index: srclib/apr/network_io/unix/sa_common.c
===================================================================
RCS file: /home/cvs/apr/network_io/unix/sa_common.c,v
retrieving revision 1.57
diff -u -r1.57 sa_common.c
--- srclib/apr/network_io/unix/sa_common.c	10 Sep 2002 16:40:13 -0000	1.57
+++ srclib/apr/network_io/unix/sa_common.c	20 Sep 2002 18:00:00 -0000
@@ -338,6 +338,50 @@
 }
 #endif
 
+#if defined(HAVE_GETADDRINFO)
+static apr_status_t get_addrs_by_family(apr_sockaddr_t *sa, const char *hostname,
+                                        apr_int32_t family, apr_port_t port,
+                                        apr_pool_t *p)
+{
+    int error;
+    struct addrinfo hints, *ai, *ai_list;
+    apr_sockaddr_t *cursa;
+
+    memset(&hints, 0, sizeof(hints));
+    hints.ai_family = family;
+    hints.ai_socktype = SOCK_STREAM;
+    error = getaddrinfo(hostname, NULL, &hints, &ai_list);
+    if (error) {
+        if (error == EAI_SYSTEM) {
+            return errno;
+        }
+        else {
+            /* issues with representing this with APR's error scheme:
+             * glibc uses negative values for these numbers, perhaps so 
+             * they don't conflict with h_errno values...  Tru64 uses 
+             * positive values which conflict with h_errno values
+             */
+#if defined(NEGATIVE_EAI)
+            error = -error;
+#endif
+            return error + APR_OS_START_EAIERR;
+        }
+    }
+
+    cursa = sa;
+    ai = ai_list;
+    save_addrinfo(p, cursa, ai, port);
+    while (ai->ai_next) { /* while more addresses to report */
+        cursa->next = apr_pcalloc(p, sizeof(apr_sockaddr_t));
+        ai = ai->ai_next;
+        cursa = cursa->next;
+        save_addrinfo(p, cursa, ai, port);
+    }
+    freeaddrinfo(ai_list);
+    return APR_SUCCESS;
+}
+#endif
+
 APR_DECLARE(apr_status_t) apr_sockaddr_info_get(apr_sockaddr_t **sa,
                                           const char *hostname, 
                                           apr_int32_t family, apr_port_t port,
@@ -350,48 +394,35 @@
 
 #if defined(HAVE_GETADDRINFO)
     if (hostname != NULL) {
-        struct addrinfo hints, *ai, *ai_list;
-        apr_sockaddr_t *cursa;
         int error;
 
-        memset(&hints, 0, sizeof(hints));
-#if !APR_HAVE_IPV6
-        /* we can't talk IPv6 so we might as well not search for IPv6
-         * addresses
-         */
-        if (family == AF_UNSPEC)
-            hints.ai_family = AF_INET;
-        else
-#endif
-            hints.ai_family = family;
-        hints.ai_socktype = SOCK_STREAM;
-        error = getaddrinfo(hostname, NULL, &hints, &ai_list);
-        if (error) {
-            if (error == EAI_SYSTEM) {
-                return errno;
+#if APR_HAVE_IPV6
+        if (family == APR_UNSPEC && (flags & APR_IPV4_ADDR_OK)) {
+            error = get_addrs_by_family(*sa, hostname, APR_INET, port, p);
+            if (error) {
+                error = get_addrs_by_family(*sa, hostname, APR_INET6, port, p);
             }
-            else {
-                /* issues with representing this with APR's error scheme:
-                 * glibc uses negative values for these numbers, perhaps so 
-                 * they don't conflict with h_errno values...  Tru64 uses 
-                 * positive values which conflict with h_errno values
-                 */
-#if defined(NEGATIVE_EAI)
-                error = -error;
-#endif
-                return error + APR_OS_START_EAIERR;
+        }
+        else if (family == APR_UNSPEC && (flags & APR_IPV6_ADDR_OK)) {
+            error = get_addrs_by_family(*sa, hostname, APR_INET6, port, p);
+            if (error) {
+                error = get_addrs_by_family(*sa, hostname, APR_INET, port, p);
             }
         }
-        cursa = *sa;
-        ai = ai_list;
-        save_addrinfo(p, cursa, ai, port);
-        while (ai->ai_next) { /* while more addresses to report */
-            cursa->next = apr_pcalloc(p, sizeof(apr_sockaddr_t));
-            ai = ai->ai_next;
-            cursa = cursa->next;
-            save_addrinfo(p, cursa, ai, port);
+        else {
+            error = get_addrs_by_family(*sa, hostname, family, port, p);
         }
-        freeaddrinfo(ai_list);
+#else
+        if (family == APR_INET6) {
+            error = APR_ENOTIMPL;
+        }
+        else {
+            error = get_addrs_by_family(*sa, hostname, APR_INET, port, p);
+        }
+        if (error) {
+            return error;
+        }
+#endif
     }
     else {
         (*sa)->pool = p;
Index: modules/proxy/proxy_http.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/proxy/proxy_http.c,v
retrieving revision 1.160
diff -u -r1.160 proxy_http.c
--- modules/proxy/proxy_http.c	22 Aug 2002 15:02:49 -0000	1.160
+++ modules/proxy/proxy_http.c	20 Sep 2002 18:00:00 -0000
@@ -225,7 +225,7 @@
     /* do a DNS lookup for the destination host */
     /* see memory note above */
     err = apr_sockaddr_info_get(&uri_addr, apr_pstrdup(c->pool, uri->hostname),
-                                APR_UNSPEC, uri->port, 0, c->pool);
+                                APR_UNSPEC, uri->port, /* 0 */ APR_IPV4_ADDR_OK, c->pool);
 
     /* allocate these out of the connection pool - the check on
      * r->connection->id makes sure that this string does not get accessed
@@ -236,7 +236,7 @@
         p_conn->port = proxyport;
         /* see memory note above */
         err = apr_sockaddr_info_get(&p_conn->addr, p_conn->name, APR_UNSPEC,
-                                    p_conn->port, 0, c->pool);
+                                    p_conn->port, /* 0 */ APR_IPV4_ADDR_OK, c->pool);
     } else {
         p_conn->name = apr_pstrdup(c->pool, uri->hostname);
         p_conn->port = uri->port;

-- 
Jeff Trawick | trawick@attglobal.net
Born in Roswell... married an alien...

Re: [pre-PATCH] short-circuit logic for apr_sockaddr_info_get()

Posted by Jeff Trawick <tr...@attglobal.net>.
Greg Marr <gr...@alum.wpi.edu> writes:

> It isn't clear from the docs what the effect would be of passing
> APR_IPV4_ADDR_OK | APR_IPV6_ADDR_OK for the flags parameter.

I'll make it clear in the docs that they are mutually exclusive.

Thanks,

-- 
Jeff Trawick | trawick@attglobal.net
Born in Roswell... married an alien...

Re: [pre-PATCH] short-circuit logic for apr_sockaddr_info_get()

Posted by Greg Marr <gr...@alum.wpi.edu>.
At 02:17 PM 9/20/2002, Jeff Trawick wrote:
>Does anyone have concerns about the API?  There is still some logic 
>cleanup to be done before committing anyway.
>
>+ * @param flags Special processing flags:
>+ * <PRE>
>+ *       APR_IPV4_ADDR_OK          first query for IPv4 addresses; 
>only look
>+ *                                 for IPv6 addresses if the first 
>query failed
>+ *                                 (flag only valid if family is 
>APR_UNSPEC)
>+ *       APR_IPV6_ADDR_OK          first query for IPv6 addresses; 
>only look
>+ *                                 for IPv4 addresses if the first 
>query failed
>+ *                                 (flag only valid if family is 
>APR_UNSPEC)
>+ * </PRE>

It isn't clear from the docs what the effect would be of passing
APR_IPV4_ADDR_OK | APR_IPV6_ADDR_OK for the flags parameter.

According to the code, APR_IPV6_ADDR_OK is ignored if 
APR_IPV4_ADDR_OK is specified.  This should probably be explicit in 
the docs.

-- 
Greg Marr
gregm@alum.wpi.edu