[Qemu-devel] Re: [PATCH] slirp: Read host DNS config on demand

[Jan Kiszka]

Ed Swierk wrote:
> On Wed, 2009-07-22 at 20:57 +0200, Jan Kiszka wrote:
>> Nice. I noticed this, too, and shortly thought about some possible
>> fixes. Don't know why I dropped it again. Anyway, your approach looks
>> straightforward. Please rebase over git head, and I will happily test
>> and ack it!
> 
> FYI there are two other things that tend to break on laptops especially:
> 
> - If the system clock jumps backwards, functions that rely on
> gettimeofday() for computing a time interval get confused.
> gettimeofday() should be replaced with clock_gettime(CLOCK_MONOTONIC).
> Hopefully all Unixy platforms support it?
> 
> - Storing the address of some random host interface in our_addr is bad
> for the same reason as storing the address of the DNS server.
> Fortunately most references to our_addr are in dead code, and the
> remaining ones are questionable.
> 
> Anyway, here's the updated patch for the DNS issue.
> 
> -
> 
> Currently the qemu user-mode networking stack reads the host DNS
> configuration (/etc/resolv.conf or the Windows equivalent) only once
> when qemu starts.  This causes name lookups in the guest to fail if the
> host is moved to a different network from which the original DNS servers
> are unreachable, a common occurrence when the host is a laptop.
> 
> This patch changes the slirp code to read the host DNS configuration on
> demand, caching the results for 10 seconds to avoid unnecessary overhead
> if name lookups occur in rapid succession.
> 
> Signed-off-by: Ed Swierk <address@hidden>

Works as expected.

Acked-by: Jan Kiszka <address@hidden>

> 
> ---
> diff --git a/slirp/ip_icmp.c b/slirp/ip_icmp.c
> index 95a4b39..751a8e2 100644
> --- a/slirp/ip_icmp.c
> +++ b/slirp/ip_icmp.c
> @@ -127,7 +127,8 @@ icmp_input(struct mbuf *m, int hlen)
>            slirp->vnetwork_addr.s_addr) {
>       /* It's an alias */
>       if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) {
> -       addr.sin_addr = dns_addr;
> +       if (get_dns_addr(&addr.sin_addr) < 0)
> +         addr.sin_addr = loopback_addr;
>       } else {
>         addr.sin_addr = loopback_addr;
>       }
> diff --git a/slirp/libslirp.h b/slirp/libslirp.h
> index 3bcc392..4b00a97 100644
> --- a/slirp/libslirp.h
> +++ b/slirp/libslirp.h
> @@ -8,6 +8,8 @@
>  struct Slirp;
>  typedef struct Slirp Slirp;
>  
> +int get_dns_addr(struct in_addr *pdns_addr);
> +
>  Slirp *slirp_init(int restricted, struct in_addr vnetwork,
>                    struct in_addr vnetmask, struct in_addr vhost,
>                    const char *vhostname, const char *tftp_path,
> diff --git a/slirp/main.h b/slirp/main.h
> index 28d92d8..e87b068 100644
> --- a/slirp/main.h
> +++ b/slirp/main.h
> @@ -32,7 +32,6 @@ extern u_int curtime;
>  extern fd_set *global_readfds, *global_writefds, *global_xfds;
>  extern struct in_addr our_addr;
>  extern struct in_addr loopback_addr;
> -extern struct in_addr dns_addr;
>  extern char *username;
>  extern char *socket_path;
>  extern int towrite_max;
> diff --git a/slirp/slirp.c b/slirp/slirp.c
> index 0ce62a3..4bc8a9d 100644
> --- a/slirp/slirp.c
> +++ b/slirp/slirp.c
> @@ -28,8 +28,6 @@
>  
>  /* host address */
>  struct in_addr our_addr;
> -/* host dns address */
> -struct in_addr dns_addr;
>  /* host loopback address */
>  struct in_addr loopback_addr;
>  
> @@ -50,9 +48,12 @@ static int do_slowtimo;
>  TAILQ_HEAD(slirp_instances, Slirp) slirp_instances =
>      TAILQ_HEAD_INITIALIZER(slirp_instances);
>  
> +struct in_addr dns_addr = { 0 };
> +u_int dns_addr_time = 0;
> +
>  #ifdef _WIN32
>  
> -static int get_dns_addr(struct in_addr *pdns_addr)
> +int get_dns_addr(struct in_addr *pdns_addr)
>  {
>      FIXED_INFO *FixedInfo=NULL;
>      ULONG    BufLen;
> @@ -60,6 +61,11 @@ static int get_dns_addr(struct in_addr *pdns_addr)
>      IP_ADDR_STRING *pIPAddr;
>      struct in_addr tmp_addr;
>  
> +    if (dns_addr.s_addr != 0 && (curtime - dns_addr_time) < 10000) {
> +        *pdns_addr = dns_addr;
> +        return 0;
> +    }
> +
>      FixedInfo = (FIXED_INFO *)GlobalAlloc(GPTR, sizeof(FIXED_INFO));
>      BufLen = sizeof(FIXED_INFO);
>  
> @@ -83,6 +89,8 @@ static int get_dns_addr(struct in_addr *pdns_addr)
>      pIPAddr = &(FixedInfo->DnsServerList);
>      inet_aton(pIPAddr->IpAddress.String, &tmp_addr);
>      *pdns_addr = tmp_addr;
> +    dns_addr = tmp_addr;
> +    dns_addr_time = curtime;
>      if (FixedInfo) {
>          GlobalFree(FixedInfo);
>          FixedInfo = NULL;
> @@ -97,7 +105,7 @@ static void winsock_cleanup(void)
>  
>  #else
>  
> -static int get_dns_addr(struct in_addr *pdns_addr)
> +int get_dns_addr(struct in_addr *pdns_addr)
>  {
>      char buff[512];
>      char buff2[257];
> @@ -105,6 +113,11 @@ static int get_dns_addr(struct in_addr *pdns_addr)
>      int found = 0;
>      struct in_addr tmp_addr;
>  
> +    if (dns_addr.s_addr != 0 && (curtime - dns_addr_time) < 10000) {
> +        *pdns_addr = dns_addr;
> +        return 0;
> +    }
> +
>      f = fopen("/etc/resolv.conf", "r");
>      if (!f)
>          return -1;
> @@ -119,8 +132,11 @@ static int get_dns_addr(struct in_addr *pdns_addr)
>              if (tmp_addr.s_addr == loopback_addr.s_addr)
>                  tmp_addr = our_addr;
>              /* If it's the first one, set it to dns_addr */
> -            if (!found)
> +            if (!found) {
>                  *pdns_addr = tmp_addr;
> +                dns_addr = tmp_addr;
> +                dns_addr_time = curtime;
> +            }
>  #ifdef DEBUG
>              else
>                  lprint(", ");
> @@ -176,11 +192,6 @@ static void slirp_init_once(void)
>      if (our_addr.s_addr == 0) {
>          our_addr = loopback_addr;
>      }
> -
> -    /* FIXME: This address may change during runtime */
> -    if (get_dns_addr(&dns_addr) < 0) {
> -        dns_addr = loopback_addr;
> -    }
>  }
>  
>  static void slirp_state_save(QEMUFile *f, void *opaque);
> diff --git a/slirp/socket.c b/slirp/socket.c
> index d8fbe89..207109c 100644
> --- a/slirp/socket.c
> +++ b/slirp/socket.c
> @@ -552,7 +552,8 @@ sosendto(struct socket *so, struct mbuf *m)
>           slirp->vnetwork_addr.s_addr) {
>         /* It's an alias */
>         if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) {
> -         addr.sin_addr = dns_addr;
> +         if (get_dns_addr(&addr.sin_addr) < 0)
> +           addr.sin_addr = loopback_addr;
>         } else {
>           addr.sin_addr = loopback_addr;
>         }
> diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
> index 51b3834..0417345 100644
> --- a/slirp/tcp_subr.c
> +++ b/slirp/tcp_subr.c
> @@ -340,7 +340,8 @@ int tcp_fconnect(struct socket *so)
>          slirp->vnetwork_addr.s_addr) {
>        /* It's an alias */
>        if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) {
> -     addr.sin_addr = dns_addr;
> +     if (get_dns_addr(&addr.sin_addr) < 0)
> +       addr.sin_addr = loopback_addr;
>        } else {
>       addr.sin_addr = loopback_addr;
>        }
> 
>

signature.asc
Description: OpenPGP digital signature