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

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 6841 bytes --]

Michael S. Tsirkin wrote:
> On Fri, Jul 31, 2009 at 06:10:22PM -0700, Ed Swierk wrote:
>> 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 <eswierk@aristanetworks.com>
>> Acked-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Introducing a random delay until update might surprise users.
> Would it make sense to use something like inotify instead?

IMHO, 10 s is below the user surprise threshold for a dynamically
changing network link. Moreover, inotify does not exist on all platforms.

Let's keep things simple for the first step, we could still further
improve it later on. 10 s are already an improvement over the current
infinite timeout.

Jan

> 
>> ---
>> 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 93087ed..67c70e3 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 e883f82..987aad9 100644
>> --- a/slirp/slirp.c
>> +++ b/slirp/slirp.c
>> @@ -29,8 +29,6 @@
>>  
>>  /* host address */
>>  struct in_addr our_addr;
>> -/* host dns address */
>> -struct in_addr dns_addr;
>>  /* host loopback address */
>>  struct in_addr loopback_addr;
>>  
>> @@ -51,9 +49,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;
>> @@ -61,6 +62,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);
>>  
>> @@ -84,6 +90,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;
>> @@ -98,7 +106,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];
>> @@ -106,6 +114,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;
>> @@ -120,8 +133,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(", ");
>> @@ -177,11 +193,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;
>>        }
>>
>>
>>



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]