* [Qemu-devel] [PATCH] slirp: Read host DNS config on demand
@ 2009-07-22 17:57 Ed Swierk
0 siblings, 0 replies; 7+ messages in thread
From: Ed Swierk @ 2009-07-22 17:57 UTC (permalink / raw)
To: qemu-devel
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>
---
Index: qemu-kvm-0.10.5/slirp/ip_icmp.c
===================================================================
--- qemu-kvm-0.10.5.orig/slirp/ip_icmp.c
+++ qemu-kvm-0.10.5/slirp/ip_icmp.c
@@ -140,7 +140,8 @@ icmp_input(m, hlen)
/* It's an alias */
switch(ntohl(so->so_faddr.s_addr) & 0xff) {
case CTL_DNS:
- addr.sin_addr = dns_addr;
+ if (get_dns_addr(&addr.sin_addr) < 0)
+ addr.sin_addr = loopback_addr;
break;
case CTL_ALIAS:
default:
Index: qemu-kvm-0.10.5/slirp/libslirp.h
===================================================================
--- qemu-kvm-0.10.5.orig/slirp/libslirp.h
+++ qemu-kvm-0.10.5/slirp/libslirp.h
@@ -5,6 +5,8 @@
extern "C" {
#endif
+int get_dns_addr(struct in_addr *pdns_addr);
+
void slirp_init(int restrict, char *special_ip);
void slirp_select_fill(int *pnfds,
Index: qemu-kvm-0.10.5/slirp/main.h
===================================================================
--- qemu-kvm-0.10.5.orig/slirp/main.h
+++ qemu-kvm-0.10.5/slirp/main.h
@@ -37,7 +37,6 @@ extern struct in_addr special_addr;
extern struct in_addr alias_addr;
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;
Index: qemu-kvm-0.10.5/slirp/slirp.c
===================================================================
--- qemu-kvm-0.10.5.orig/slirp/slirp.c
+++ qemu-kvm-0.10.5/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;
@@ -61,9 +59,12 @@ fd_set *global_readfds, *global_writefds
char slirp_hostname[33];
+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;
@@ -71,6 +72,11 @@ static int get_dns_addr(struct in_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);
@@ -94,6 +100,8 @@ static int get_dns_addr(struct in_addr *
pIPAddr = &(FixedInfo->DnsServerList);
inet_aton(pIPAddr->IpAddress.String, &tmp_addr);
*pdns_addr = tmp_addr;
+ dns_addr = tmp_addr;
+ dns_addr_time = curtime;
#if 0
printf( "DNS Servers:\n" );
printf( "DNS Addr:%s\n", pIPAddr->IpAddress.String );
@@ -113,7 +121,7 @@ static int get_dns_addr(struct in_addr *
#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];
@@ -121,6 +129,11 @@ static int get_dns_addr(struct in_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;
@@ -135,8 +148,11 @@ static int get_dns_addr(struct in_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(", ");
@@ -195,11 +211,6 @@ void slirp_init(int restrict, char *spec
/* set default addresses */
inet_aton("127.0.0.1", &loopback_addr);
- if (get_dns_addr(&dns_addr) < 0) {
- dns_addr = loopback_addr;
- fprintf (stderr, "Warning: No DNS servers found\n");
- }
-
if (special_ip)
slirp_special_ip = special_ip;
Index: qemu-kvm-0.10.5/slirp/socket.c
===================================================================
--- qemu-kvm-0.10.5.orig/slirp/socket.c
+++ qemu-kvm-0.10.5/slirp/socket.c
@@ -567,7 +567,8 @@ sosendto(so, m)
/* It's an alias */
switch(ntohl(so->so_faddr.s_addr) & 0xff) {
case CTL_DNS:
- addr.sin_addr = dns_addr;
+ if (get_dns_addr(&addr.sin_addr) < 0)
+ addr.sin_addr = loopback_addr;
break;
case CTL_ALIAS:
default:
Index: qemu-kvm-0.10.5/slirp/tcp_subr.c
===================================================================
--- qemu-kvm-0.10.5.orig/slirp/tcp_subr.c
+++ qemu-kvm-0.10.5/slirp/tcp_subr.c
@@ -398,7 +398,8 @@ int tcp_fconnect(so)
/* It's an alias */
switch(ntohl(so->so_faddr.s_addr) & 0xff) {
case CTL_DNS:
- addr.sin_addr = dns_addr;
+ if (get_dns_addr(&addr.sin_addr) < 0)
+ addr.sin_addr = loopback_addr;
break;
case CTL_ALIAS:
default:
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH] slirp: Read host DNS config on demand
@ 2009-08-01 1:10 Ed Swierk
2009-08-02 7:43 ` Michael S. Tsirkin
0 siblings, 1 reply; 7+ messages in thread
From: Ed Swierk @ 2009-08-01 1:10 UTC (permalink / raw)
To: qemu-devel; +Cc: jan.kiszka
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>
---
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;
}
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] slirp: Read host DNS config on demand
2009-08-01 1:10 [Qemu-devel] [PATCH] slirp: Read host DNS config on demand Ed Swierk
@ 2009-08-02 7:43 ` Michael S. Tsirkin
2009-08-02 8:03 ` Jan Kiszka
0 siblings, 1 reply; 7+ messages in thread
From: Michael S. Tsirkin @ 2009-08-02 7:43 UTC (permalink / raw)
To: Ed Swierk; +Cc: jan.kiszka, qemu-devel
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?
> ---
> 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;
> }
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] slirp: Read host DNS config on demand
2009-08-02 7:43 ` Michael S. Tsirkin
@ 2009-08-02 8:03 ` Jan Kiszka
2009-08-02 8:20 ` Michael S. Tsirkin
2009-08-03 2:08 ` Ed Swierk
0 siblings, 2 replies; 7+ messages in thread
From: Jan Kiszka @ 2009-08-02 8:03 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel, Ed Swierk
[-- 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 --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] slirp: Read host DNS config on demand
2009-08-02 8:03 ` Jan Kiszka
@ 2009-08-02 8:20 ` Michael S. Tsirkin
2009-08-03 2:08 ` Ed Swierk
1 sibling, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2009-08-02 8:20 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-devel, Ed Swierk
On Sun, Aug 02, 2009 at 10:03:10AM +0200, Jan Kiszka wrote:
> 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
Maybe you are right ... OTOH it is at least consistently infinite :)
> >
> >> ---
> >> 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;
> >> }
> >>
> >>
> >>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] slirp: Read host DNS config on demand
2009-08-02 8:03 ` Jan Kiszka
2009-08-02 8:20 ` Michael S. Tsirkin
@ 2009-08-03 2:08 ` Ed Swierk
2009-08-03 13:30 ` Jamie Lokier
1 sibling, 1 reply; 7+ messages in thread
From: Ed Swierk @ 2009-08-03 2:08 UTC (permalink / raw)
To: Jan Kiszka, Michael S. Tsirkin, qemu-devel
On Sun, Aug 2, 2009 at 8:03 AM, Jan Kiszka<jan.kiszka@web.de> wrote:
> 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.
Right. The most common situation in which the DNS server address
changes is after you wake up your laptop and wait for it to connect to
a new network. DHCP itself can take several seconds to do its thing,
so you're unlikely to notice the up-to-10-second delay before full
network connectivity is restored in a VM.
Of course there's nothing magical about 10 seconds. A shorter timeout
would be even nicer, and I doubt anyone would really miss the CPU
cycles lost to re-reading a tiny text file, but I don't think
instantaneous response is required here.
--Ed
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] slirp: Read host DNS config on demand
2009-08-03 2:08 ` Ed Swierk
@ 2009-08-03 13:30 ` Jamie Lokier
0 siblings, 0 replies; 7+ messages in thread
From: Jamie Lokier @ 2009-08-03 13:30 UTC (permalink / raw)
To: Ed Swierk; +Cc: Jan Kiszka, qemu-devel, Michael S. Tsirkin
Ed Swierk wrote:
> On Sun, Aug 2, 2009 at 8:03 AM, Jan Kiszka<jan.kiszka@web.de> wrote:
> > 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.
>
> Right. The most common situation in which the DNS server address
> changes is after you wake up your laptop and wait for it to connect to
> a new network. DHCP itself can take several seconds to do its thing,
> so you're unlikely to notice the up-to-10-second delay before full
> network connectivity is restored in a VM.
I strongly disagree. Usual practice is to watch the NetworkManager
icon until it indicates that DHCP is complete with a green light,
finger poised over the clicking button, and then expect web browsing
and SSH to work immediately after the green light appears. A 10
second delay at that point would be most irritating.
Ideally, that's what RTNETLINK events are for: you can register for
asynchronous events whenever a network interface is reconfigured, and
trigger things like rereading /etc/resolv.conf from that. Windows has
a similar mechanism for watching interface changes.
But a 1/2 second delay would be fine.
> Of course there's nothing magical about 10 seconds. A shorter timeout
> would be even nicer, and I doubt anyone would really miss the CPU
> cycles lost to re-reading a tiny text file, but I don't think
> instantaneous response is required here.
All you need is to call stat() to confirm that the file hasn't
changed. Just check the dev/ino/mtime/size.
If that were a periodic 1 second timer, the biggest cost on a laptop
might be power drain due to CPU wakeups. But since the resolv.conf
check should only should be done in reaction to the VM actively doing
a DNS query (plus 1 second has passed since the last resolve.conf
check), the CPU is already woken up, so it's cheap.
-- Jamie
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-08-03 13:30 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-08-01 1:10 [Qemu-devel] [PATCH] slirp: Read host DNS config on demand Ed Swierk
2009-08-02 7:43 ` Michael S. Tsirkin
2009-08-02 8:03 ` Jan Kiszka
2009-08-02 8:20 ` Michael S. Tsirkin
2009-08-03 2:08 ` Ed Swierk
2009-08-03 13:30 ` Jamie Lokier
-- strict thread matches above, loose matches on Subject: below --
2009-07-22 17:57 Ed Swierk
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).