[Qemu-devel] 0.9.0 Win32 Tap inferface PPC Guest issue

[Qemu-devel] 0.9.0 Win32 Tap inferface PPC Guest issue

[Ely Soto]

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

I'm having an issue getting the 0.9.0 build ppc to work with tap.

OpenVPN 2.09

Relevant Parameters
-net nic -net tap,ifname=tap-lan

0.8.2 works quite well.

I've narrowed the break  to the change described here.
http://lists.gnu.org/archive/html/qemu-devel/2006-08/msg00243.html

Backing this out of the 0.9.0 gets tap networking working again.

I'll keep investigating.

Ely Soto, Flight Software Engineer
Orbital Sciences Corp. Dulles, VA.
soto.ely@orbital.com
Office No. 703-406-5341
Mobile No. 703-403-7077

-----------------------------------------
Notice:  This e-mail is intended solely for use of the individual
or entity to which it is addressed and may contain information that
is proprietary, privileged and exempt from disclosure under
applicable law.  If the reader is not the intended recipient or
agent responsible for delivering the message to the intended
recipient, you are hereby notified that any dissemination,
distribution or copying of this communication is strictly
prohibited.  This communication may also contain data subject to
U.S. export laws.  If so, that data subject to the International
Traffic in Arms Regulation cannot be disseminated, distributed or
copied to foreign nationals, residing in the U.S. or abroad, absent
the express prior approval of the U.S. Department of State.   If
you have received this communication in error, please notify the
sender by reply e-mail and destroy the e-mail message and any
physical copies made of the communication.  Thank you.

[-- Attachment #2: Type: text/html, Size: 2001 bytes --]

Re: [Qemu-devel] 0.9.0 Win32 Tap inferface PPC Guest issue

[Ely Soto]

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

Further details:

This call functions correctly and returns true.

tap-win32.c, line 534
        ReleaseSemaphore(overlapped->tap_semaphore, 1, NULL)

However, WaitForMultipleObjects never returns successfully to execute the 
callback function.

vl.c, line 5859

main_loop_wait() 
        ...
        ret = WaitForMultipleObjects(w->num, w->events, FALSE, timeout);
        if (WAIT_OBJECT_0 + 0 <= ret && ret <= WAIT_OBJECT_0 + w->num - 1) 
{
        ...

Ely Soto, Flight Software Engineer
Orbital Sciences Corp. Dulles, VA.
soto.ely@orbital.com
Office No. 703-406-5341
Mobile No. 703-403-7077

-----------------------------------------
Notice:  This e-mail is intended solely for use of the individual
or entity to which it is addressed and may contain information that
is proprietary, privileged and exempt from disclosure under
applicable law.  If the reader is not the intended recipient or
agent responsible for delivering the message to the intended
recipient, you are hereby notified that any dissemination,
distribution or copying of this communication is strictly
prohibited.  This communication may also contain data subject to
U.S. export laws.  If so, that data subject to the International
Traffic in Arms Regulation cannot be disseminated, distributed or
copied to foreign nationals, residing in the U.S. or abroad, absent
the express prior approval of the U.S. Department of State.   If
you have received this communication in error, please notify the
sender by reply e-mail and destroy the e-mail message and any
physical copies made of the communication.  Thank you.

[-- Attachment #2: Type: text/html, Size: 2282 bytes --]

Re: [Qemu-devel] 0.9.0 Win32 Tap inferface PPC Guest issue

[Ely Soto]

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

Initially it looks like it may be some sort of timing issue.

It happened to start working a bit without any code changes after I was 
stepping through the code.
Can't reproduce that yet.

Ely Soto

-----------------------------------------
Notice:  This e-mail is intended solely for use of the individual
or entity to which it is addressed and may contain information that
is proprietary, privileged and exempt from disclosure under
applicable law.  If the reader is not the intended recipient or
agent responsible for delivering the message to the intended
recipient, you are hereby notified that any dissemination,
distribution or copying of this communication is strictly
prohibited.  This communication may also contain data subject to
U.S. export laws.  If so, that data subject to the International
Traffic in Arms Regulation cannot be disseminated, distributed or
copied to foreign nationals, residing in the U.S. or abroad, absent
the express prior approval of the U.S. Department of State.   If
you have received this communication in error, please notify the
sender by reply e-mail and destroy the e-mail message and any
physical copies made of the communication.  Thank you.

[-- Attachment #2: Type: text/html, Size: 1391 bytes --]

Re: [Qemu-devel] 0.9.0 Win32 Tap inferface PPC Guest issue

[Ely Soto]

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

Ok I located the source of the problem.

The code in main_loop_wait() that uses WaitForMultipleObjects will only 
service one object that woke it up.

What is occuring is that the host_alarm timer is being serviced anytime 
the tap_semaphore is also signaled, therefore the tap callback function is
never called while at the same time the tap_semaphore count is 
decremented. This guarantees that the callback function will never get 
services if that particular timing continues.

This is likely something that won't occur on every system because of the 
nature of the timing, but it is a problem.

I'm working on a better fix as right now my solution is to poll all the 
objects after a wakeup to see who really is signaled.
In order to do that you must use an Event Object in Manual Reset Mode.

vl.c

void main_loop_wait(int timeout)
...
        WaitObjects *w = &wait_objects;
 
        ret = WaitForMultipleObjects(w->num, w->events, FALSE, timeout);
        if (WAIT_OBJECT_0 + 0 <= ret && ret <= WAIT_OBJECT_0 + w->num - 1) 
{
            if (w->func[ret - WAIT_OBJECT_0])
                w->func[ret - WAIT_OBJECT_0](w->opaque[ret - 
WAIT_OBJECT_0]);
        } else if (ret == WAIT_TIMEOUT) {
        } else {
            err = GetLastError();
            fprintf(stderr, "Wait error %d %d\n", ret, err);
        }
...



Ely Soto


-----------------------------------------
Notice:  This e-mail is intended solely for use of the individual
or entity to which it is addressed and may contain information that
is proprietary, privileged and exempt from disclosure under
applicable law.  If the reader is not the intended recipient or
agent responsible for delivering the message to the intended
recipient, you are hereby notified that any dissemination,
distribution or copying of this communication is strictly
prohibited.  This communication may also contain data subject to
U.S. export laws.  If so, that data subject to the International
Traffic in Arms Regulation cannot be disseminated, distributed or
copied to foreign nationals, residing in the U.S. or abroad, absent
the express prior approval of the U.S. Department of State.   If
you have received this communication in error, please notify the
sender by reply e-mail and destroy the e-mail message and any
physical copies made of the communication.  Thank you.

[-- Attachment #2: Type: text/html, Size: 4005 bytes --]

Re: [Qemu-devel] 0.9.0 Win32 Tap inferface PPC Guest issue

[Ely Soto]

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

Ok this is the fix I came up with.

Basically it will check all the other objects that could potentially be 
waiting for service. This will prevent one high priority object from 
starving the rest.

I'm not certain what the intention for the timeout > 0 check but this 
completely prevented any wait object servicing on my machine.

The Tap interface now works smoothly without issue.


Diff/patch for 0.9.0 release version of vl.c 
==================================
5840c5840
<     int ret, nfds;
---
>     int ret, ret2, nfds, i;
5851c5851
<     if (ret == 0 && timeout > 0) {
---
>     if (ret == 0) {
5858a5859,5873
> 
>             /* Check for additional signaled events */ 
>             for(i = (ret - WAIT_OBJECT_0 + 1); i < w->num; i++) {
> 
>                 /* Check if event is signaled */
>                 ret2 = WaitForSingleObject(w->events[i], 0);
>                 if(ret2 == WAIT_OBJECT_0) {
>                     if (w->func[i])
>                         w->func[i](w->opaque[i]);
>                 } else if (ret2 == WAIT_TIMEOUT) {
>                 } else {
>                     err = GetLastError();
>                     fprintf(stderr, "WaitForSingleObject error %d %d\n", 
i, err);
>                 } 
>             } 
5862c5877
<             fprintf(stderr, "Wait error %d %d\n", ret, err);
---
>             fprintf(stderr, "WaitForMultipleObjects error %d %d\n", ret, 
err);
==================================


Diff/patch for version 1.279 of vl.c 
==================================
6146c6146
<     int ret, nfds;
---
>     int ret, ret2, nfds, i;
6157c6157
<     if (ret == 0 && timeout > 0) {
---
>     if (ret == 0) {
6164a6165,6179
> 
>             /* Check for additional signaled events */ 
>             for(i = (ret - WAIT_OBJECT_0 + 1); i < w->num; i++) {
> 
>                 /* Check if event is signaled */
>                 ret2 = WaitForSingleObject(w->events[i], 0);
>                 if(ret2 == WAIT_OBJECT_0) {
>                     if (w->func[i])
>                         w->func[i](w->opaque[i]);
>                 } else if (ret2 == WAIT_TIMEOUT) {
>                 } else {
>                     err = GetLastError();
>                     fprintf(stderr, "WaitForSingleObject error %d %d\n", 
i, err);
>                 } 
>             } 
6168c6183
<             fprintf(stderr, "Wait error %d %d\n", ret, err);
---
>             fprintf(stderr, "WaitForMultipleObjects error %d %d\n", ret, 
err);
==================================

Ely Soto




Ely Soto <Soto.Ely@orbital.com> 
Sent by: qemu-devel-bounces+soto.ely=orbital.com@nongnu.org
04/09/2007 01:54 PM
Please respond to
qemu-devel@nongnu.org


To
qemu-devel@nongnu.org
cc

Subject
Re: [Qemu-devel] 0.9.0 Win32 Tap inferface PPC Guest issue







Ok I located the source of the problem. 

The code in main_loop_wait() that uses WaitForMultipleObjects will only 
service one object that woke it up. 

What is occuring is that the host_alarm timer is being serviced anytime 
the tap_semaphore is also signaled, therefore the tap callback function is 

never called while at the same time the tap_semaphore count is 
decremented. This guarantees that the callback function will never get 
services if that particular timing continues. 

This is likely something that won't occur on every system because of the 
nature of the timing, but it is a problem. 

I'm working on a better fix as right now my solution is to poll all the 
objects after a wakeup to see who really is signaled. 
In order to do that you must use an Event Object in Manual Reset Mode. 

vl.c 

void main_loop_wait(int timeout) 
... 
        WaitObjects *w = &wait_objects; 
 
        ret = WaitForMultipleObjects(w->num, w->events, FALSE, timeout); 
        if (WAIT_OBJECT_0 + 0 <= ret && ret <= WAIT_OBJECT_0 + w->num - 1) 
{ 
            if (w->func[ret - WAIT_OBJECT_0]) 
                w->func[ret - WAIT_OBJECT_0](w->opaque[ret - 
WAIT_OBJECT_0]); 
        } else if (ret == WAIT_TIMEOUT) { 
        } else { 
            err = GetLastError(); 
            fprintf(stderr, "Wait error %d %d\n", ret, err); 
        } 
... 



Ely Soto


-----------------------------------------
Notice:  This e-mail is intended solely for use of the individual
or entity to which it is addressed and may contain information that
is proprietary, privileged and exempt from disclosure under
applicable law.  If the reader is not the intended recipient or
agent responsible for delivering the message to the intended
recipient, you are hereby notified that any dissemination,
distribution or copying of this communication is strictly
prohibited.  This communication may also contain data subject to
U.S. export laws.  If so, that data subject to the International
Traffic in Arms Regulation cannot be disseminated, distributed or
copied to foreign nationals, residing in the U.S. or abroad, absent
the express prior approval of the U.S. Department of State.   If
you have received this communication in error, please notify the
sender by reply e-mail and destroy the e-mail message and any
physical copies made of the communication.  Thank you.

[-- Attachment #2: Type: text/html, Size: 12873 bytes --]

Re: [Qemu-devel] 0.9.0 Win32 Tap inferface PPC Guest issue

[Thiemo Seufer]

Ely Soto wrote:
> Ok this is the fix I came up with.
> 
> Basically it will check all the other objects that could potentially be 
> waiting for service. This will prevent one high priority object from 
> starving the rest.
> 
> I'm not certain what the intention for the timeout > 0 check but this 
> completely prevented any wait object servicing on my machine.
> 
> The Tap interface now works smoothly without issue.

Please resend the version for current CVS as unified diff (diff -u)
without breaking the line formatting (an attachment should do).


Thiemo

Re: [Qemu-devel] 0.9.0 Win32 Tap inferface PPC Guest issue

[Ely Soto]

[-- Attachment #1.1: Type: text/plain, Size: 1765 bytes --]

Done.



-Ely Soto




Thiemo Seufer <ths@networkno.de> 
04/17/2007 07:22 PM

To
Ely Soto <Soto.Ely@orbital.com>
cc
qemu-devel@nongnu.org
Subject
Re: [Qemu-devel] 0.9.0 Win32 Tap inferface PPC Guest issue






Ely Soto wrote:
> Ok this is the fix I came up with.
> 
> Basically it will check all the other objects that could potentially be 
> waiting for service. This will prevent one high priority object from 
> starving the rest.
> 
> I'm not certain what the intention for the timeout > 0 check but this 
> completely prevented any wait object servicing on my machine.
> 
> The Tap interface now works smoothly without issue.

Please resend the version for current CVS as unified diff (diff -u)
without breaking the line formatting (an attachment should do).


Thiemo



-----------------------------------------
Notice:  This e-mail is intended solely for use of the individual
or entity to which it is addressed and may contain information that
is proprietary, privileged and exempt from disclosure under
applicable law.  If the reader is not the intended recipient or
agent responsible for delivering the message to the intended
recipient, you are hereby notified that any dissemination,
distribution or copying of this communication is strictly
prohibited.  This communication may also contain data subject to
U.S. export laws.  If so, that data subject to the International
Traffic in Arms Regulation cannot be disseminated, distributed or
copied to foreign nationals, residing in the U.S. or abroad, absent
the express prior approval of the U.S. Department of State.   If
you have received this communication in error, please notify the
sender by reply e-mail and destroy the e-mail message and any
physical copies made of the communication.  Thank you.

[-- Attachment #1.2: Type: text/html, Size: 2668 bytes --]

[-- Attachment #2: vl.c_1.280.diff --]
[-- Type: application/octet-stream, Size: 1656 bytes --]

--- vl_1.280.c	Wed Apr 18 13:15:02 2007
+++ vl_1.280_fix.c	Wed Apr 18 13:21:05 2007
@@ -6143,7 +6143,7 @@
 {
     IOHandlerRecord *ioh;
     fd_set rfds, wfds, xfds;
-    int ret, nfds;
+    int ret, ret2, nfds, i;
     struct timeval tv;
     PollingEntry *pe;
 
@@ -6154,7 +6154,7 @@
         ret |= pe->func(pe->opaque);
     }
 #ifdef _WIN32
-    if (ret == 0 && timeout > 0) {
+    if (ret == 0) {
         int err;
         WaitObjects *w = &wait_objects;
         
@@ -6162,10 +6162,25 @@
         if (WAIT_OBJECT_0 + 0 <= ret && ret <= WAIT_OBJECT_0 + w->num - 1) {
             if (w->func[ret - WAIT_OBJECT_0])
                 w->func[ret - WAIT_OBJECT_0](w->opaque[ret - WAIT_OBJECT_0]);
+                
+            /* Check for additional signaled events */ 
+            for(i = (ret - WAIT_OBJECT_0 + 1); i < w->num; i++) {
+                                
+                /* Check if event is signaled */
+                ret2 = WaitForSingleObject(w->events[i], 0);
+                if(ret2 == WAIT_OBJECT_0) {
+                    if (w->func[i])
+                        w->func[i](w->opaque[i]);
+                } else if (ret2 == WAIT_TIMEOUT) {
+                } else {
+                    err = GetLastError();
+                    fprintf(stderr, "WaitForSingleObject error %d %d\n", i, err);
+                }                
+            }                 
         } else if (ret == WAIT_TIMEOUT) {
         } else {
             err = GetLastError();
-            fprintf(stderr, "Wait error %d %d\n", ret, err);
+            fprintf(stderr, "WaitForMultipleObjects error %d %d\n", ret, err);
         }
     }
 #endif