* [Qemu-devel] [PATCH v3 0/3] slirp: fill mainloop with more precise timeout value
@ 2013-08-21 2:15 Liu Ping Fan
2013-08-21 2:15 ` [Qemu-devel] [PATCH v3 1/3] slirp: make timeout local Liu Ping Fan
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Liu Ping Fan @ 2013-08-21 2:15 UTC (permalink / raw)
To: qemu-devel; +Cc: Jan Kiszka, Stefan Hajnoczi, Paolo Bonzini
With this series, we can set the mainloop timeout more precisely when slirp has
to emulate tcp timeout problem.
v3:
fix comment: document timeout unit "milliseconds"
fix logic: no slirps, no timeout modifications in slirp_pollfds_fill()
v2:
fold slirp_update_timeout logic into slirp_pollfds_fill.
Liu Ping Fan (3):
slirp: make timeout local
slirp: define timeout as macro
slirp: set mainloop timeout with more precise value
main-loop.c | 3 +--
slirp/libslirp.h | 3 +--
slirp/slirp.c | 55 +++++++++++++++++++++++++++++++++++++++----------------
slirp/slirp.h | 3 +++
stubs/slirp.c | 6 +-----
5 files changed, 45 insertions(+), 25 deletions(-)
--
1.8.1.4
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v3 1/3] slirp: make timeout local
2013-08-21 2:15 [Qemu-devel] [PATCH v3 0/3] slirp: fill mainloop with more precise timeout value Liu Ping Fan
@ 2013-08-21 2:15 ` Liu Ping Fan
2013-08-21 2:15 ` [Qemu-devel] [PATCH v3 2/3] slirp: define timeout as macro Liu Ping Fan
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Liu Ping Fan @ 2013-08-21 2:15 UTC (permalink / raw)
To: qemu-devel; +Cc: Jan Kiszka, Stefan Hajnoczi, Paolo Bonzini
Each slirp has its own time to caculate timeout.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
slirp/slirp.c | 22 ++++++++++------------
slirp/slirp.h | 3 +++
2 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 80b28ea..b71c617 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -40,8 +40,6 @@ static const uint8_t special_ethaddr[ETH_ALEN] = {
static const uint8_t zero_ethaddr[ETH_ALEN] = { 0, 0, 0, 0, 0, 0 };
u_int curtime;
-static u_int time_fasttimo, last_slowtimo;
-static int do_slowtimo;
static QTAILQ_HEAD(slirp_instances, Slirp) slirp_instances =
QTAILQ_HEAD_INITIALIZER(slirp_instances);
@@ -278,14 +276,13 @@ void slirp_pollfds_fill(GArray *pollfds)
/*
* First, TCP sockets
*/
- do_slowtimo = 0;
QTAILQ_FOREACH(slirp, &slirp_instances, entry) {
/*
* *_slowtimo needs calling if there are IP fragments
* in the fragment queue, or there are TCP connections active
*/
- do_slowtimo |= ((slirp->tcb.so_next != &slirp->tcb) ||
+ slirp->do_slowtimo = ((slirp->tcb.so_next != &slirp->tcb) ||
(&slirp->ipq.ip_link != slirp->ipq.ip_link.next));
for (so = slirp->tcb.so_next; so != &slirp->tcb;
@@ -299,8 +296,9 @@ void slirp_pollfds_fill(GArray *pollfds)
/*
* See if we need a tcp_fasttimo
*/
- if (time_fasttimo == 0 && so->so_tcpcb->t_flags & TF_DELACK) {
- time_fasttimo = curtime; /* Flag when we want a fasttimo */
+ if (slirp->time_fasttimo == 0 &&
+ so->so_tcpcb->t_flags & TF_DELACK) {
+ slirp->time_fasttimo = curtime; /* Flag when want a fasttimo */
}
/*
@@ -381,7 +379,7 @@ void slirp_pollfds_fill(GArray *pollfds)
udp_detach(so);
continue;
} else {
- do_slowtimo = 1; /* Let socket expire */
+ slirp->do_slowtimo = true; /* Let socket expire */
}
}
@@ -422,7 +420,7 @@ void slirp_pollfds_fill(GArray *pollfds)
icmp_detach(so);
continue;
} else {
- do_slowtimo = 1; /* Let socket expire */
+ slirp->do_slowtimo = true; /* Let socket expire */
}
}
@@ -454,14 +452,14 @@ void slirp_pollfds_poll(GArray *pollfds, int select_error)
/*
* See if anything has timed out
*/
- if (time_fasttimo && ((curtime - time_fasttimo) >= 2)) {
+ if (slirp->time_fasttimo && ((curtime - slirp->time_fasttimo) >= 2)) {
tcp_fasttimo(slirp);
- time_fasttimo = 0;
+ slirp->time_fasttimo = 0;
}
- if (do_slowtimo && ((curtime - last_slowtimo) >= 499)) {
+ if (slirp->do_slowtimo && ((curtime - slirp->last_slowtimo) >= 499)) {
ip_slowtimo(slirp);
tcp_slowtimo(slirp);
- last_slowtimo = curtime;
+ slirp->last_slowtimo = curtime;
}
/*
diff --git a/slirp/slirp.h b/slirp/slirp.h
index fe0e65d..e4a1bd4 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -203,6 +203,9 @@ bool arp_table_search(Slirp *slirp, uint32_t ip_addr,
struct Slirp {
QTAILQ_ENTRY(Slirp) entry;
+ u_int time_fasttimo;
+ u_int last_slowtimo;
+ bool do_slowtimo;
/* virtual network configuration */
struct in_addr vnetwork_addr;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v3 2/3] slirp: define timeout as macro
2013-08-21 2:15 [Qemu-devel] [PATCH v3 0/3] slirp: fill mainloop with more precise timeout value Liu Ping Fan
2013-08-21 2:15 ` [Qemu-devel] [PATCH v3 1/3] slirp: make timeout local Liu Ping Fan
@ 2013-08-21 2:15 ` Liu Ping Fan
2013-08-21 2:15 ` [Qemu-devel] [PATCH v3 3/3] slirp: set mainloop timeout with more precise value Liu Ping Fan
2013-08-22 12:07 ` [Qemu-devel] [PATCH v3 0/3] slirp: fill mainloop with more precise timeout value Stefan Hajnoczi
3 siblings, 0 replies; 11+ messages in thread
From: Liu Ping Fan @ 2013-08-21 2:15 UTC (permalink / raw)
To: qemu-devel; +Cc: Jan Kiszka, Stefan Hajnoczi, Paolo Bonzini
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
slirp/slirp.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/slirp/slirp.c b/slirp/slirp.c
index b71c617..1e8983e 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -47,6 +47,9 @@ static QTAILQ_HEAD(slirp_instances, Slirp) slirp_instances =
static struct in_addr dns_addr;
static u_int dns_addr_time;
+#define TIMEOUT_FAST 2 /* milliseconds */
+#define TIMEOUT_SLOW 499 /* milliseconds */
+
#ifdef _WIN32
int get_dns_addr(struct in_addr *pdns_addr)
@@ -452,11 +455,13 @@ void slirp_pollfds_poll(GArray *pollfds, int select_error)
/*
* See if anything has timed out
*/
- if (slirp->time_fasttimo && ((curtime - slirp->time_fasttimo) >= 2)) {
+ if (slirp->time_fasttimo &&
+ ((curtime - slirp->time_fasttimo) >= TIMEOUT_FAST)) {
tcp_fasttimo(slirp);
slirp->time_fasttimo = 0;
}
- if (slirp->do_slowtimo && ((curtime - slirp->last_slowtimo) >= 499)) {
+ if (slirp->do_slowtimo &&
+ ((curtime - slirp->last_slowtimo) >= TIMEOUT_SLOW)) {
ip_slowtimo(slirp);
tcp_slowtimo(slirp);
slirp->last_slowtimo = curtime;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v3 3/3] slirp: set mainloop timeout with more precise value
2013-08-21 2:15 [Qemu-devel] [PATCH v3 0/3] slirp: fill mainloop with more precise timeout value Liu Ping Fan
2013-08-21 2:15 ` [Qemu-devel] [PATCH v3 1/3] slirp: make timeout local Liu Ping Fan
2013-08-21 2:15 ` [Qemu-devel] [PATCH v3 2/3] slirp: define timeout as macro Liu Ping Fan
@ 2013-08-21 2:15 ` Liu Ping Fan
2013-08-21 7:36 ` Alex Bligh
2013-08-23 16:54 ` Jan Kiszka
2013-08-22 12:07 ` [Qemu-devel] [PATCH v3 0/3] slirp: fill mainloop with more precise timeout value Stefan Hajnoczi
3 siblings, 2 replies; 11+ messages in thread
From: Liu Ping Fan @ 2013-08-21 2:15 UTC (permalink / raw)
To: qemu-devel; +Cc: Jan Kiszka, Stefan Hajnoczi, Paolo Bonzini
If slirp needs to emulate tcp timeout, then the timeout value
for mainloop should be more precise, which is determined by
slirp's fasttimo or slowtimo. Achieve this by swap the logic
sequence of slirp_pollfds_fill and slirp_update_timeout.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
main-loop.c | 3 +--
slirp/libslirp.h | 3 +--
slirp/slirp.c | 28 ++++++++++++++++++++++++----
stubs/slirp.c | 6 +-----
4 files changed, 27 insertions(+), 13 deletions(-)
diff --git a/main-loop.c b/main-loop.c
index a44fff6..e258567 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -458,8 +458,7 @@ int main_loop_wait(int nonblocking)
g_array_set_size(gpollfds, 0); /* reset for new iteration */
/* XXX: separate device handlers from system ones */
#ifdef CONFIG_SLIRP
- slirp_update_timeout(&timeout);
- slirp_pollfds_fill(gpollfds);
+ slirp_pollfds_fill(gpollfds, &timeout);
#endif
qemu_iohandler_fill(gpollfds);
ret = os_host_main_loop_wait(timeout);
diff --git a/slirp/libslirp.h b/slirp/libslirp.h
index ceabff8..5bdcbd5 100644
--- a/slirp/libslirp.h
+++ b/slirp/libslirp.h
@@ -16,8 +16,7 @@ Slirp *slirp_init(int restricted, struct in_addr vnetwork,
void *opaque);
void slirp_cleanup(Slirp *slirp);
-void slirp_update_timeout(uint32_t *timeout);
-void slirp_pollfds_fill(GArray *pollfds);
+void slirp_pollfds_fill(GArray *pollfds, uint32_t *timeout);
void slirp_pollfds_poll(GArray *pollfds, int select_error);
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 1e8983e..f312a7d 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -260,14 +260,33 @@ void slirp_cleanup(Slirp *slirp)
#define CONN_CANFSEND(so) (((so)->so_state & (SS_FCANTSENDMORE|SS_ISFCONNECTED)) == SS_ISFCONNECTED)
#define CONN_CANFRCV(so) (((so)->so_state & (SS_FCANTRCVMORE|SS_ISFCONNECTED)) == SS_ISFCONNECTED)
-void slirp_update_timeout(uint32_t *timeout)
+static void slirp_update_timeout(uint32_t *timeout)
{
- if (!QTAILQ_EMPTY(&slirp_instances)) {
- *timeout = MIN(1000, *timeout);
+ Slirp *slirp;
+ uint32_t t;
+
+ *timeout = MIN(1000, *timeout);
+ if (*timeout <= TIMEOUT_FAST) {
+ return;
+ }
+ t = *timeout;
+
+ /* If we have tcp timeout with slirp, then we will fill @timeout with
+ * more precise value.
+ */
+ QTAILQ_FOREACH(slirp, &slirp_instances, entry) {
+ if (slirp->time_fasttimo) {
+ *timeout = TIMEOUT_FAST;
+ return;
+ }
+ if (slirp->do_slowtimo) {
+ t = MIN(TIMEOUT_SLOW, t);
+ }
}
+ *timeout = t;
}
-void slirp_pollfds_fill(GArray *pollfds)
+void slirp_pollfds_fill(GArray *pollfds, uint32_t *timeout)
{
Slirp *slirp;
struct socket *so, *so_next;
@@ -437,6 +456,7 @@ void slirp_pollfds_fill(GArray *pollfds)
}
}
}
+ slirp_update_timeout(timeout);
}
void slirp_pollfds_poll(GArray *pollfds, int select_error)
diff --git a/stubs/slirp.c b/stubs/slirp.c
index f1fc833..bd0ac7f 100644
--- a/stubs/slirp.c
+++ b/stubs/slirp.c
@@ -1,11 +1,7 @@
#include "qemu-common.h"
#include "slirp/slirp.h"
-void slirp_update_timeout(uint32_t *timeout)
-{
-}
-
-void slirp_pollfds_fill(GArray *pollfds)
+void slirp_pollfds_fill(GArray *pollfds, uint32_t *timeout)
{
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/3] slirp: set mainloop timeout with more precise value
2013-08-21 2:15 ` [Qemu-devel] [PATCH v3 3/3] slirp: set mainloop timeout with more precise value Liu Ping Fan
@ 2013-08-21 7:36 ` Alex Bligh
2013-08-21 8:07 ` liu ping fan
2013-08-23 16:54 ` Jan Kiszka
1 sibling, 1 reply; 11+ messages in thread
From: Alex Bligh @ 2013-08-21 7:36 UTC (permalink / raw)
To: Liu Ping Fan, qemu-devel
Cc: Jan Kiszka, Alex Bligh, Stefan Hajnoczi, Paolo Bonzini
--On 21 August 2013 10:15:52 +0800 Liu Ping Fan <qemulist@gmail.com> wrote:
> -void slirp_update_timeout(uint32_t *timeout)
> +static void slirp_update_timeout(uint32_t *timeout)
> {
> - if (!QTAILQ_EMPTY(&slirp_instances)) {
> - *timeout = MIN(1000, *timeout);
If you are putting things in macros, you might as well change that
1000 as well, and hopefully comment why that particular magic value
is there.
> + Slirp *slirp;
> + uint32_t t;
> +
> + *timeout = MIN(1000, *timeout);
> + if (*timeout <= TIMEOUT_FAST) {
> + return;
> + }
> + t = *timeout;
--
Alex Bligh
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/3] slirp: set mainloop timeout with more precise value
2013-08-21 7:36 ` Alex Bligh
@ 2013-08-21 8:07 ` liu ping fan
2013-08-23 16:49 ` Jan Kiszka
0 siblings, 1 reply; 11+ messages in thread
From: liu ping fan @ 2013-08-21 8:07 UTC (permalink / raw)
To: Alex Bligh; +Cc: Jan Kiszka, qemu-devel, Stefan Hajnoczi, Paolo Bonzini
On Wed, Aug 21, 2013 at 3:36 PM, Alex Bligh <alex@alex.org.uk> wrote:
>
>
> --On 21 August 2013 10:15:52 +0800 Liu Ping Fan <qemulist@gmail.com> wrote:
>
>> -void slirp_update_timeout(uint32_t *timeout)
>> +static void slirp_update_timeout(uint32_t *timeout)
>> {
>> - if (!QTAILQ_EMPTY(&slirp_instances)) {
>> - *timeout = MIN(1000, *timeout);
>
>
> If you are putting things in macros, you might as well change that
TIMEOUT_FAST/SLOW have definite meaning, and used more than one place
in the code. For 1000ms, I do not know this magic value's meaning, but
whatever, it just occurs once. So there is no trouble to read the
code.
> 1000 as well, and hopefully comment why that particular magic value
> is there.
>
>
>> + Slirp *slirp;
>> + uint32_t t;
>> +
>> + *timeout = MIN(1000, *timeout);
>> + if (*timeout <= TIMEOUT_FAST) {
>> + return;
>> + }
>> + t = *timeout;
>
>
>
>
> --
> Alex Bligh
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/3] slirp: fill mainloop with more precise timeout value
2013-08-21 2:15 [Qemu-devel] [PATCH v3 0/3] slirp: fill mainloop with more precise timeout value Liu Ping Fan
` (2 preceding siblings ...)
2013-08-21 2:15 ` [Qemu-devel] [PATCH v3 3/3] slirp: set mainloop timeout with more precise value Liu Ping Fan
@ 2013-08-22 12:07 ` Stefan Hajnoczi
3 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2013-08-22 12:07 UTC (permalink / raw)
To: Liu Ping Fan; +Cc: Jan Kiszka, qemu-devel, Stefan Hajnoczi, Paolo Bonzini
On Wed, Aug 21, 2013 at 10:15:49AM +0800, Liu Ping Fan wrote:
> With this series, we can set the mainloop timeout more precisely when slirp has
> to emulate tcp timeout problem.
>
> v3:
> fix comment: document timeout unit "milliseconds"
> fix logic: no slirps, no timeout modifications in slirp_pollfds_fill()
> v2:
> fold slirp_update_timeout logic into slirp_pollfds_fill.
>
>
> Liu Ping Fan (3):
> slirp: make timeout local
> slirp: define timeout as macro
> slirp: set mainloop timeout with more precise value
>
> main-loop.c | 3 +--
> slirp/libslirp.h | 3 +--
> slirp/slirp.c | 55 +++++++++++++++++++++++++++++++++++++++----------------
> slirp/slirp.h | 3 +++
> stubs/slirp.c | 6 +-----
> 5 files changed, 45 insertions(+), 25 deletions(-)
These patches look okay, I've skimmed them. Leaving detailed slirp review to Jan.
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/3] slirp: set mainloop timeout with more precise value
2013-08-21 8:07 ` liu ping fan
@ 2013-08-23 16:49 ` Jan Kiszka
2013-08-25 1:52 ` liu ping fan
0 siblings, 1 reply; 11+ messages in thread
From: Jan Kiszka @ 2013-08-23 16:49 UTC (permalink / raw)
To: liu ping fan; +Cc: Paolo Bonzini, Stefan Hajnoczi, qemu-devel, Alex Bligh
On 2013-08-21 10:07, liu ping fan wrote:
> On Wed, Aug 21, 2013 at 3:36 PM, Alex Bligh <alex@alex.org.uk> wrote:
>>
>>
>> --On 21 August 2013 10:15:52 +0800 Liu Ping Fan <qemulist@gmail.com> wrote:
>>
>>> -void slirp_update_timeout(uint32_t *timeout)
>>> +static void slirp_update_timeout(uint32_t *timeout)
>>> {
>>> - if (!QTAILQ_EMPTY(&slirp_instances)) {
>>> - *timeout = MIN(1000, *timeout);
>>
>>
>> If you are putting things in macros, you might as well change that
>
> TIMEOUT_FAST/SLOW have definite meaning, and used more than one place
> in the code. For 1000ms, I do not know this magic value's meaning, but
> whatever, it just occurs once. So there is no trouble to read the
> code.
You could name it ONE_SEC or so. Can be done as trivial patch on top.
IIRC, slirp requires regular polling for the aging of certain requests
like DNS.
Jan
>
>> 1000 as well, and hopefully comment why that particular magic value
>> is there.
>>
>>
>>> + Slirp *slirp;
>>> + uint32_t t;
>>> +
>>> + *timeout = MIN(1000, *timeout);
>>> + if (*timeout <= TIMEOUT_FAST) {
>>> + return;
>>> + }
>>> + t = *timeout;
>>
>>
>>
>>
>> --
>> Alex Bligh
--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/3] slirp: set mainloop timeout with more precise value
2013-08-21 2:15 ` [Qemu-devel] [PATCH v3 3/3] slirp: set mainloop timeout with more precise value Liu Ping Fan
2013-08-21 7:36 ` Alex Bligh
@ 2013-08-23 16:54 ` Jan Kiszka
2013-08-25 1:53 ` liu ping fan
1 sibling, 1 reply; 11+ messages in thread
From: Jan Kiszka @ 2013-08-23 16:54 UTC (permalink / raw)
To: Liu Ping Fan; +Cc: Paolo Bonzini, qemu-devel, Stefan Hajnoczi
On 2013-08-21 04:15, Liu Ping Fan wrote:
> If slirp needs to emulate tcp timeout, then the timeout value
> for mainloop should be more precise, which is determined by
> slirp's fasttimo or slowtimo. Achieve this by swap the logic
> sequence of slirp_pollfds_fill and slirp_update_timeout.
>
> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
> ---
> main-loop.c | 3 +--
> slirp/libslirp.h | 3 +--
> slirp/slirp.c | 28 ++++++++++++++++++++++++----
> stubs/slirp.c | 6 +-----
> 4 files changed, 27 insertions(+), 13 deletions(-)
>
> diff --git a/main-loop.c b/main-loop.c
> index a44fff6..e258567 100644
> --- a/main-loop.c
> +++ b/main-loop.c
> @@ -458,8 +458,7 @@ int main_loop_wait(int nonblocking)
> g_array_set_size(gpollfds, 0); /* reset for new iteration */
> /* XXX: separate device handlers from system ones */
> #ifdef CONFIG_SLIRP
> - slirp_update_timeout(&timeout);
> - slirp_pollfds_fill(gpollfds);
> + slirp_pollfds_fill(gpollfds, &timeout);
> #endif
> qemu_iohandler_fill(gpollfds);
> ret = os_host_main_loop_wait(timeout);
> diff --git a/slirp/libslirp.h b/slirp/libslirp.h
> index ceabff8..5bdcbd5 100644
> --- a/slirp/libslirp.h
> +++ b/slirp/libslirp.h
> @@ -16,8 +16,7 @@ Slirp *slirp_init(int restricted, struct in_addr vnetwork,
> void *opaque);
> void slirp_cleanup(Slirp *slirp);
>
> -void slirp_update_timeout(uint32_t *timeout);
> -void slirp_pollfds_fill(GArray *pollfds);
> +void slirp_pollfds_fill(GArray *pollfds, uint32_t *timeout);
>
> void slirp_pollfds_poll(GArray *pollfds, int select_error);
>
> diff --git a/slirp/slirp.c b/slirp/slirp.c
> index 1e8983e..f312a7d 100644
> --- a/slirp/slirp.c
> +++ b/slirp/slirp.c
> @@ -260,14 +260,33 @@ void slirp_cleanup(Slirp *slirp)
> #define CONN_CANFSEND(so) (((so)->so_state & (SS_FCANTSENDMORE|SS_ISFCONNECTED)) == SS_ISFCONNECTED)
> #define CONN_CANFRCV(so) (((so)->so_state & (SS_FCANTRCVMORE|SS_ISFCONNECTED)) == SS_ISFCONNECTED)
>
> -void slirp_update_timeout(uint32_t *timeout)
> +static void slirp_update_timeout(uint32_t *timeout)
> {
> - if (!QTAILQ_EMPTY(&slirp_instances)) {
> - *timeout = MIN(1000, *timeout);
> + Slirp *slirp;
> + uint32_t t;
> +
> + *timeout = MIN(1000, *timeout);
> + if (*timeout <= TIMEOUT_FAST) {
Nitpicking, sorry, but TIMEOUT_FAST is always smaller than 1000. So this
check should come first, and then the MIN assignment (to t).
Jan
> + return;
> + }
> + t = *timeout;
> +
> + /* If we have tcp timeout with slirp, then we will fill @timeout with
> + * more precise value.
> + */
> + QTAILQ_FOREACH(slirp, &slirp_instances, entry) {
> + if (slirp->time_fasttimo) {
> + *timeout = TIMEOUT_FAST;
> + return;
> + }
> + if (slirp->do_slowtimo) {
> + t = MIN(TIMEOUT_SLOW, t);
> + }
> }
> + *timeout = t;
> }
>
> -void slirp_pollfds_fill(GArray *pollfds)
> +void slirp_pollfds_fill(GArray *pollfds, uint32_t *timeout)
> {
> Slirp *slirp;
> struct socket *so, *so_next;
> @@ -437,6 +456,7 @@ void slirp_pollfds_fill(GArray *pollfds)
> }
> }
> }
> + slirp_update_timeout(timeout);
> }
>
> void slirp_pollfds_poll(GArray *pollfds, int select_error)
> diff --git a/stubs/slirp.c b/stubs/slirp.c
> index f1fc833..bd0ac7f 100644
> --- a/stubs/slirp.c
> +++ b/stubs/slirp.c
> @@ -1,11 +1,7 @@
> #include "qemu-common.h"
> #include "slirp/slirp.h"
>
> -void slirp_update_timeout(uint32_t *timeout)
> -{
> -}
> -
> -void slirp_pollfds_fill(GArray *pollfds)
> +void slirp_pollfds_fill(GArray *pollfds, uint32_t *timeout)
> {
> }
>
>
--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/3] slirp: set mainloop timeout with more precise value
2013-08-23 16:49 ` Jan Kiszka
@ 2013-08-25 1:52 ` liu ping fan
0 siblings, 0 replies; 11+ messages in thread
From: liu ping fan @ 2013-08-25 1:52 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Paolo Bonzini, Stefan Hajnoczi, qemu-devel, Alex Bligh
On Sat, Aug 24, 2013 at 12:49 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2013-08-21 10:07, liu ping fan wrote:
>> On Wed, Aug 21, 2013 at 3:36 PM, Alex Bligh <alex@alex.org.uk> wrote:
>>>
>>>
>>> --On 21 August 2013 10:15:52 +0800 Liu Ping Fan <qemulist@gmail.com> wrote:
>>>
>>>> -void slirp_update_timeout(uint32_t *timeout)
>>>> +static void slirp_update_timeout(uint32_t *timeout)
>>>> {
>>>> - if (!QTAILQ_EMPTY(&slirp_instances)) {
>>>> - *timeout = MIN(1000, *timeout);
>>>
>>>
>>> If you are putting things in macros, you might as well change that
>>
>> TIMEOUT_FAST/SLOW have definite meaning, and used more than one place
>> in the code. For 1000ms, I do not know this magic value's meaning, but
>> whatever, it just occurs once. So there is no trouble to read the
>> code.
>
> You could name it ONE_SEC or so. Can be done as trivial patch on top.
>
> IIRC, slirp requires regular polling for the aging of certain requests
> like DNS.
>
Thanks for the explanation. Will fix and document it.
Regards,
Pingfan
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/3] slirp: set mainloop timeout with more precise value
2013-08-23 16:54 ` Jan Kiszka
@ 2013-08-25 1:53 ` liu ping fan
0 siblings, 0 replies; 11+ messages in thread
From: liu ping fan @ 2013-08-25 1:53 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Paolo Bonzini, qemu-devel, Stefan Hajnoczi
On Sat, Aug 24, 2013 at 12:54 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2013-08-21 04:15, Liu Ping Fan wrote:
>> If slirp needs to emulate tcp timeout, then the timeout value
>> for mainloop should be more precise, which is determined by
>> slirp's fasttimo or slowtimo. Achieve this by swap the logic
>> sequence of slirp_pollfds_fill and slirp_update_timeout.
>>
>> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
>> ---
>> main-loop.c | 3 +--
>> slirp/libslirp.h | 3 +--
>> slirp/slirp.c | 28 ++++++++++++++++++++++++----
>> stubs/slirp.c | 6 +-----
>> 4 files changed, 27 insertions(+), 13 deletions(-)
>>
>> diff --git a/main-loop.c b/main-loop.c
>> index a44fff6..e258567 100644
>> --- a/main-loop.c
>> +++ b/main-loop.c
>> @@ -458,8 +458,7 @@ int main_loop_wait(int nonblocking)
>> g_array_set_size(gpollfds, 0); /* reset for new iteration */
>> /* XXX: separate device handlers from system ones */
>> #ifdef CONFIG_SLIRP
>> - slirp_update_timeout(&timeout);
>> - slirp_pollfds_fill(gpollfds);
>> + slirp_pollfds_fill(gpollfds, &timeout);
>> #endif
>> qemu_iohandler_fill(gpollfds);
>> ret = os_host_main_loop_wait(timeout);
>> diff --git a/slirp/libslirp.h b/slirp/libslirp.h
>> index ceabff8..5bdcbd5 100644
>> --- a/slirp/libslirp.h
>> +++ b/slirp/libslirp.h
>> @@ -16,8 +16,7 @@ Slirp *slirp_init(int restricted, struct in_addr vnetwork,
>> void *opaque);
>> void slirp_cleanup(Slirp *slirp);
>>
>> -void slirp_update_timeout(uint32_t *timeout);
>> -void slirp_pollfds_fill(GArray *pollfds);
>> +void slirp_pollfds_fill(GArray *pollfds, uint32_t *timeout);
>>
>> void slirp_pollfds_poll(GArray *pollfds, int select_error);
>>
>> diff --git a/slirp/slirp.c b/slirp/slirp.c
>> index 1e8983e..f312a7d 100644
>> --- a/slirp/slirp.c
>> +++ b/slirp/slirp.c
>> @@ -260,14 +260,33 @@ void slirp_cleanup(Slirp *slirp)
>> #define CONN_CANFSEND(so) (((so)->so_state & (SS_FCANTSENDMORE|SS_ISFCONNECTED)) == SS_ISFCONNECTED)
>> #define CONN_CANFRCV(so) (((so)->so_state & (SS_FCANTRCVMORE|SS_ISFCONNECTED)) == SS_ISFCONNECTED)
>>
>> -void slirp_update_timeout(uint32_t *timeout)
>> +static void slirp_update_timeout(uint32_t *timeout)
>> {
>> - if (!QTAILQ_EMPTY(&slirp_instances)) {
>> - *timeout = MIN(1000, *timeout);
>> + Slirp *slirp;
>> + uint32_t t;
>> +
>> + *timeout = MIN(1000, *timeout);
>> + if (*timeout <= TIMEOUT_FAST) {
>
> Nitpicking, sorry, but TIMEOUT_FAST is always smaller than 1000. So this
> check should come first, and then the MIN assignment (to t).
>
Will fix.
Regards,
Pingfan
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-08-25 1:53 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-21 2:15 [Qemu-devel] [PATCH v3 0/3] slirp: fill mainloop with more precise timeout value Liu Ping Fan
2013-08-21 2:15 ` [Qemu-devel] [PATCH v3 1/3] slirp: make timeout local Liu Ping Fan
2013-08-21 2:15 ` [Qemu-devel] [PATCH v3 2/3] slirp: define timeout as macro Liu Ping Fan
2013-08-21 2:15 ` [Qemu-devel] [PATCH v3 3/3] slirp: set mainloop timeout with more precise value Liu Ping Fan
2013-08-21 7:36 ` Alex Bligh
2013-08-21 8:07 ` liu ping fan
2013-08-23 16:49 ` Jan Kiszka
2013-08-25 1:52 ` liu ping fan
2013-08-23 16:54 ` Jan Kiszka
2013-08-25 1:53 ` liu ping fan
2013-08-22 12:07 ` [Qemu-devel] [PATCH v3 0/3] slirp: fill mainloop with more precise timeout value Stefan Hajnoczi
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).