RE: [patch] ne2000.patch

RE: [patch] ne2000.patch

[Ian Pratt]

> > This small could make ne2000 net card on HVM work stable.
> > I have submitted the patch to qemu mail list. Actually, they only 
> > check the second part of it!
> 
> Does this actually fix any reported bugs or measurably 
> improve performance? If not then it isn't worth us diverging 
> from qemu code: 
> it's up to qemu maintainers whether or not to accept the patch.

Does it solve the issue that OpenBSD had with the ne2k emulation under
high load?

Ian

Re: [patch] ne2000.patch

[han]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=gb18030; format=flowed, Size: 1430 bytes --]

Actually, I didn't test against OpenBsd.
Without this patch, ne2K driver in Linux will complain about "bogus 
packet size" under high load(copy a large file larger than 100M among 
Domain). At the same time, the driver may also drop a large number of 
error packets. It also improve the performance 2 or 3 times in my test. 
The original result is about 700 KB/s. After patching it, the result is 
about 1.8M~2M /s.
What's more important is it could prevent the emulated net card from 
being halted caused by IRQ missing problem. After the device-model 
transfering to event driven model, ne2000 card cannot work stable in 
VMX. Hope this patch could solve this problem!

_______________________________________________________
Best Regards,
hanzhu


Ian Pratt дµÀ:
>>> This small could make ne2000 net card on HVM work stable.
>>> I have submitted the patch to qemu mail list. Actually, they only 
>>> check the second part of it!
>> Does this actually fix any reported bugs or measurably 
>> improve performance? If not then it isn't worth us diverging 
>> from qemu code: 
>> it's up to qemu maintainers whether or not to accept the patch.
> 
> Does it solve the issue that OpenBSD had with the ne2k emulation under
> high load?
> 
> Ian
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
> 

RE: [patch] ne2000.patch

[Han, Zhu]

Hi, Ian and Keir,
I'm convinced the ne2000 card cannot work under LINUX HVM without this patch even sync to the latest changeset. So, this patch makes the ne2000 card work and also makes it work stable in my test case.
BTW: Anyone could help me test it against openBSD VMX? I don't own such environment.

Best Regards, 
hanzhu

-----Original Message-----
From: xen-devel-bounces@lists.xensource.com [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Han, Zhu
Sent: 2006年5月30日 14:14
To: xen-devel@lists.xensource.com
Subject: [Xen-devel] [patch] ne2000.patch

This small could make ne2000 net card on HVM work stable.
I have submitted the patch to qemu mail list. Actually, they only check
the second part of it!

Several note:
1) Because ne2000.c in device model don't implement all the features
required by ne2000 hardware specs, I add some simple workaround to it!
When the buffer is full, resend the IRQ singal to the receiver! The
hardware asks us to send overflow IRQ to receiver, However, we needn't
do that since the software implementation will not overflow but just
miss the irq signal and halt itself!
2) According to the hardware spec, I have changed the condition when the
buffer is full.

Signed-off-by: hanzhu <zhu.han@intel.com>
diff -r 6d476981e3a5 tools/ioemu/hw/ne2000.c
--- a/tools/ioemu/hw/ne2000.c	Sun May 28 15:49:17 2006 +0100
+++ b/tools/ioemu/hw/ne2000.c	Mon May 29 16:28:11 2006 +0800
@@ -147,9 +147,37 @@ static void ne2000_reset(NE2000State *s)
     }
 }
 
+static int ne2000_buffer_full(NE2000State *s)
+{
+    int avail, index, boundary;
+
+    index = s->curpag << 8;
+    boundary = s->boundary << 8;
+    if (index <= boundary)
+        /* when index == boundary, we should assume the
+         * buffer is full instead of empty!
+         */
+        avail = boundary - index;
+    else
+        avail = (s->stop - s->start) - (index - boundary);
+
+    if(avail < (MAX_ETH_FRAME_SIZE + 4))
+        return 1;
+    
+    return 0;
+}
+
 static void ne2000_update_irq(NE2000State *s)
 {
     int isr;
+
+
+    if(ne2000_buffer_full(s)){
+        /* The freeing space is not enough, tell the ne2k driver
+         * to fetch these packets!
+         */
+        s->isr |= ENISR_RX;
+    }
     isr = s->isr & s->imr;
 #if defined(DEBUG_NE2000)
     printf("NE2000: Set IRQ line %d to %d (%02x %02x)\n",
@@ -172,14 +200,10 @@ static int ne2000_can_receive(void *opaq
     
     if (s->cmd & E8390_STOP)
         return 0;
-    index = s->curpag << 8;
-    boundary = s->boundary << 8;
-    if (index < boundary)
-        avail = boundary - index;
-    else
-        avail = (s->stop - s->start) - (index - boundary);
-    if (avail < (MAX_ETH_FRAME_SIZE + 4))
+
+    if(ne2000_buffer_full(s)){
         return 0;
+    }
     return MAX_ETH_FRAME_SIZE;
 }

Best Regards, 
hanzhu

[patch] ne2000.patch

[Han, Zhu]

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

This small could make ne2000 net card on HVM work stable.
I have submitted the patch to qemu mail list. Actually, they only check
the second part of it!

Several note:
1) Because ne2000.c in device model don't implement all the features
required by ne2000 hardware specs, I add some simple workaround to it!
When the buffer is full, resend the IRQ singal to the receiver! The
hardware asks us to send overflow IRQ to receiver, However, we needn't
do that since the software implementation will not overflow but just
miss the irq signal and halt itself!
2) According to the hardware spec, I have changed the condition when the
buffer is full.

Signed-off-by: hanzhu <zhu.han@intel.com>
diff -r 6d476981e3a5 tools/ioemu/hw/ne2000.c
--- a/tools/ioemu/hw/ne2000.c	Sun May 28 15:49:17 2006 +0100
+++ b/tools/ioemu/hw/ne2000.c	Mon May 29 16:28:11 2006 +0800
@@ -147,9 +147,37 @@ static void ne2000_reset(NE2000State *s)
     }
 }
 
+static int ne2000_buffer_full(NE2000State *s)
+{
+    int avail, index, boundary;
+
+    index = s->curpag << 8;
+    boundary = s->boundary << 8;
+    if (index <= boundary)
+        /* when index == boundary, we should assume the
+         * buffer is full instead of empty!
+         */
+        avail = boundary - index;
+    else
+        avail = (s->stop - s->start) - (index - boundary);
+
+    if(avail < (MAX_ETH_FRAME_SIZE + 4))
+        return 1;
+    
+    return 0;
+}
+
 static void ne2000_update_irq(NE2000State *s)
 {
     int isr;
+
+
+    if(ne2000_buffer_full(s)){
+        /* The freeing space is not enough, tell the ne2k driver
+         * to fetch these packets!
+         */
+        s->isr |= ENISR_RX;
+    }
     isr = s->isr & s->imr;
 #if defined(DEBUG_NE2000)
     printf("NE2000: Set IRQ line %d to %d (%02x %02x)\n",
@@ -172,14 +200,10 @@ static int ne2000_can_receive(void *opaq
     
     if (s->cmd & E8390_STOP)
         return 0;
-    index = s->curpag << 8;
-    boundary = s->boundary << 8;
-    if (index < boundary)
-        avail = boundary - index;
-    else
-        avail = (s->stop - s->start) - (index - boundary);
-    if (avail < (MAX_ETH_FRAME_SIZE + 4))
+
+    if(ne2000_buffer_full(s)){
         return 0;
+    }
     return MAX_ETH_FRAME_SIZE;
 }

Best Regards, 
hanzhu


[-- Attachment #2: ne2000.patch --]
[-- Type: application/octet-stream, Size: 1544 bytes --]

diff -r 6d476981e3a5 tools/ioemu/hw/ne2000.c
--- a/tools/ioemu/hw/ne2000.c	Sun May 28 15:49:17 2006 +0100
+++ b/tools/ioemu/hw/ne2000.c	Mon May 29 16:28:11 2006 +0800
@@ -147,9 +147,37 @@ static void ne2000_reset(NE2000State *s)
     }
 }
 
+static int ne2000_buffer_full(NE2000State *s)
+{
+    int avail, index, boundary;
+
+    index = s->curpag << 8;
+    boundary = s->boundary << 8;
+    if (index <= boundary)
+        /* when index == boundary, we should assume the
+         * buffer is full instead of empty!
+         */
+        avail = boundary - index;
+    else
+        avail = (s->stop - s->start) - (index - boundary);
+
+    if(avail < (MAX_ETH_FRAME_SIZE + 4))
+        return 1;
+    
+    return 0;
+}
+
 static void ne2000_update_irq(NE2000State *s)
 {
     int isr;
+
+
+    if(ne2000_buffer_full(s)){
+        /* The freeing space is not enough, tell the ne2k driver
+         * to fetch these packets!
+         */
+        s->isr |= ENISR_RX;
+    }
     isr = s->isr & s->imr;
 #if defined(DEBUG_NE2000)
     printf("NE2000: Set IRQ line %d to %d (%02x %02x)\n",
@@ -172,14 +200,10 @@ static int ne2000_can_receive(void *opaq
     
     if (s->cmd & E8390_STOP)
         return 0;
-    index = s->curpag << 8;
-    boundary = s->boundary << 8;
-    if (index < boundary)
-        avail = boundary - index;
-    else
-        avail = (s->stop - s->start) - (index - boundary);
-    if (avail < (MAX_ETH_FRAME_SIZE + 4))
+
+    if(ne2000_buffer_full(s)){
         return 0;
+    }
     return MAX_ETH_FRAME_SIZE;
 }
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [patch] ne2000.patch

[Keir Fraser]

On 30 May 2006, at 07:14, Han, Zhu wrote:

> This small could make ne2000 net card on HVM work stable.
> I have submitted the patch to qemu mail list. Actually, they only check
> the second part of it!

Does this actually fix any reported bugs or measurably improve 
performance? If not then it isn't worth us diverging from qemu code: 
it's up to qemu maintainers whether or not to accept the patch.

  -- Keir