httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r..@covalent.net
Subject Re: [PATCH] sockaddr games in Unix network_io implementation
Date Tue, 27 Jun 2000 15:29:00 GMT

+1.  If there is more to do, please go ahead.  :-)  I am glad this was
done finally.  This has been a thorn in APR's side for too long.  :-)

Ryan

On Tue, 27 Jun 2000, Jeff Trawick wrote:

> This patch attempts to avoid getsockname() wherever possible
> (including the Apache configuration where the listening socket is
> bound to a particular interface, such that we don't need to call
> getsockname() for the connected socket).  I started out with a smaller
> set of changes because I felt Ryan was unconfortable with the whole
> idea, but I left that behind because I wanted to know how much code it
> would take for APR to really optimize this.  I would guess that there
> are 5 extra lines of code to keep up with port and ip address
> separately and 7 extra lines of code that check for conditions where we
> already know a piece of information.
> 
> Benefits:
> 
> . getsockname() avoided with Apache when the listening socket on which
>   the new connection arrived is bound to a specific interface
> . getsockname() avoided in various other conditions (but none which
>   are important to Apache as far as I know)
> . changes fix a minor glitch where in the current CVS code
>   getsockname() wasn't called after bind() (only a problem if we got
>   an ephemeral port)
> 
> Detriments:
> 
> . if you don't know sockets, some of this code may be unclear at
>   first (but if "/* kernel got us an ephemeral port */" is a new
>   concept then it is time for a little homework before playing in this
>   code anyway)
> 
> Note: I'm not about to claim that getsockname() is a hog, but there is
> special overhead associated with syscalls and on some systems that
> overhead is signficant.
> 
> Have fun...
> 
> Index: src/lib/apr/network_io/unix/networkio.h
> ===================================================================
> RCS file: /cvs/apache/apache-2.0/src/lib/apr/network_io/unix/networkio.h,v
> retrieving revision 1.29
> diff -u -r1.29 networkio.h
> --- networkio.h	2000/06/22 00:36:04	1.29
> +++ networkio.h	2000/06/27 12:45:32
> @@ -126,6 +126,8 @@
>  #ifndef HAVE_POLL
>      int connected;
>  #endif
> +    int local_port_unknown;
> +    int local_interface_unknown;
>  };
>  
>  struct ap_pollfd_t {
> Index: src/lib/apr/network_io/unix/sockaddr.c
> ===================================================================
> RCS file: /cvs/apache/apache-2.0/src/lib/apr/network_io/unix/sockaddr.c,v
> retrieving revision 1.12
> diff -u -r1.12 sockaddr.c
> --- sockaddr.c	2000/06/26 20:37:42	1.12
> +++ sockaddr.c	2000/06/27 12:45:33
> @@ -70,8 +70,32 @@
>  
>  
>  
> +static ap_status_t get_local_addr(ap_socket_t *sock)
> +{
> +    socklen_t namelen = sizeof(*sock->local_addr);
> +
> +    if (getsockname(sock->socketdes, (struct sockaddr *)sock->local_addr, 
> +                    &namelen) < 0) {
> +        return errno;
> +    }
> +    else {
> +        sock->local_port_unknown = sock->local_interface_unknown = 0;
> +        return APR_SUCCESS;
> +    }
> +}
> +
> +
> +
>  ap_status_t ap_get_local_port(ap_uint32_t *port, ap_socket_t *sock)
>  {
> +    if (sock->local_port_unknown) {
> +        ap_status_t rv = get_local_addr(sock);
> +
> +        if (rv != APR_SUCCESS) {
> +            return rv;
> +        }
> +    }
> +
>      *port = ntohs(sock->local_addr->sin_port);
>      return APR_SUCCESS;
>  }
> @@ -130,6 +154,14 @@
>  
>  ap_status_t ap_get_local_ipaddr(char **addr, ap_socket_t *sock)
>  {
> +    if (sock->local_interface_unknown) {
> +        ap_status_t rv = get_local_addr(sock);
> +
> +        if (rv != APR_SUCCESS) {
> +            return rv;
> +        }
> +    }
> +
>      *addr = ap_pstrdup(sock->cntxt, inet_ntoa(sock->local_addr->sin_addr));
>      return APR_SUCCESS;
>  }
> @@ -147,6 +179,14 @@
>  #if APR_HAVE_NETINET_IN_H
>  ap_status_t ap_get_local_name(struct sockaddr_in **name, ap_socket_t *sock)
>  {
> +    if (sock->local_port_unknown || sock->local_interface_unknown) {
> +        ap_status_t rv = get_local_addr(sock);
> +
> +        if (rv != APR_SUCCESS) {
> +            return rv;
> +        }
> +    }
> +
>      *name = sock->local_addr;
>      return APR_SUCCESS;
>  }
> Index: src/lib/apr/network_io/unix/sockets.c
> ===================================================================
> RCS file: /cvs/apache/apache-2.0/src/lib/apr/network_io/unix/sockets.c,v
> retrieving revision 1.49
> diff -u -r1.49 sockets.c
> --- sockets.c	2000/06/26 20:37:42	1.49
> +++ sockets.c	2000/06/27 12:45:33
> @@ -115,8 +115,12 @@
>  {
>      if (bind(sock->socketdes, (struct sockaddr *)sock->local_addr, sock->addr_len)
== -1)
>          return errno;
> -    else
> +    else {
> +        if (sock->local_addr->sin_port == 0) { /* no need for ntohs() when comparing
w/ 0 */
> +            sock->local_port_unknown = 1; /* kernel got us an ephemeral port */
> +        }
>          return APR_SUCCESS;
> +    }
>  }
>  
>  ap_status_t ap_listen(ap_socket_t *sock, ap_int32_t backlog)
> @@ -151,9 +155,22 @@
>          return errno;
>      }
>  
> -    if (getsockname((*new)->socketdes, (struct sockaddr *)(*new)->local_addr,

> -                    &((*new)->addr_len)) < 0) {
> -	return errno;
> +    *(*new)->local_addr = *sock->local_addr;
> +
> +    if (sock->local_port_unknown) {
> +        /* not likely for a listening socket, but theoretically possible :) */
> +        (*new)->local_port_unknown = 1;
> +    }
> +
> +    if (sock->local_interface_unknown ||
> +        sock->local_addr->sin_addr.s_addr == 0) {
> +        /* If the interface address inside the listening socket's local_addr wasn't

> +         * up-to-date, we don't know local interface of the connected socket either.
> +         *
> +         * If the listening socket was not bound to a specific interface, we
> +         * don't know the local_addr of the connected socket.
> +         */
> +        (*new)->local_interface_unknown = 1;
>      }
>  
>      ap_register_cleanup((*new)->cntxt, (void *)(*new), 
> @@ -197,8 +214,16 @@
>          return errno;
>      }
>      else {
> -        socklen_t namelen = sizeof(*sock->local_addr);
> -        getsockname(sock->socketdes, (struct sockaddr *)sock->local_addr, &namelen);
> +        if (sock->local_addr->sin_port == 0) {
> +            /* connect() got us an ephemeral port */
> +            sock->local_port_unknown = 1;
> +        }
> +        if (sock->local_addr->sin_addr.s_addr == 0) {
> +            /* not bound to specific local interface; connect() had to assign
> +             * one for the socket
> +             */
> +            sock->local_interface_unknown = 1;
> +        }
>  #ifndef HAVE_POLL
>  	sock->connected=1;
>  #endif
> @@ -240,11 +265,8 @@
>       
>          (*sock)->addr_len = sizeof(*(*sock)->local_addr);
>          (*sock)->timeout = -1;
> -        if (getsockname(*thesock, (struct sockaddr *)(*sock)->local_addr, 
> -                        &((*sock)->addr_len)) < 0) {
> -            return errno;
> -        }
>      }
> +    (*sock)->local_port_unknown = (*sock)->local_interface_unknown = 1;
>      (*sock)->socketdes = *thesock;
>      return APR_SUCCESS;
>  }
> 
> -- 
> Jeff Trawick | trawick@ibm.net | PGP public key at web site:
>      http://www.geocities.com/SiliconValley/Park/9289/
>           Born in Roswell... married an alien...
> 


_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Mime
View raw message