Poor user-net performance.

PATCH In review Paul Brook 3 participants 5 messages Dec 18, 2005 – Dec 21, 2005

Share & tools

Paul Brook

author

2005-12-18 23:29 UTC #0 lore raw

PATCH 1/1 Poor user-net performance.

Dan mentioned he was seeing very poor network performance when using user-net. It turns out this is because packets were getting dropped (the NIC buffers were full), and the slirp code doesn't throttle properly when this occurs. You end up with a bunch of packets getting sent to the guest, half of which get dropped, then a delay until slirp retransmits.

This is much more noticeable on Arm because the NIC only has a small memory buffer.

Teaching the slirp code how to do tcp backoff/window scaling is probably a fair amount of work. The easy alternative is to throttle the slirp output when we know the card's buffers are full. slirp has a convenient hook for this, so it's a matter of adding the hooks to the hardware emulation.

This increases arm user-net performance from painfully slow (~200kbit) to guest CPU bound (~20Mbit).

Paul

Index: vl.c
===================================================================
RCS file: /sources/qemu/qemu/vl.c,v
retrieving revision 1.157

vl.c +23 -6

diff -u -p -r1.157 vl.c
@@ -1768,13 +1768,16 @@ VLANState *qemu_find_vlan(int id)
 }
 
 VLANClientState *qemu_new_vlan_client(VLANState *vlan,
-                                      IOReadHandler *fd_read, void *opaque)
+                                      IOReadHandler *fd_read,
+                                      IOCanRWHandler *fd_can_read,
+                                      void *opaque)
 {
     VLANClientState *vc, **pvc;
     vc = qemu_mallocz(sizeof(VLANClientState));
     if (!vc)
         return NULL;
     vc->fd_read = fd_read;
+    vc->fd_can_read = fd_can_read;
     vc->opaque = opaque;
     vc->vlan = vlan;

@@ -1786,6 +1789,20 @@ VLANClientState *qemu_new_vlan_client(VL
     return vc;
 }
 
+int qemu_can_send_packet(VLANClientState *vc1)
+{
+    VLANState *vlan = vc1->vlan;
+    VLANClientState *vc;
+
+    for(vc = vlan->first_client; vc != NULL; vc = vc->next) {
+        if (vc != vc1) {
+            if (vc->fd_can_read && !vc->fd_can_read(vc->opaque))
+                return 0;
+        }
+    }
+    return 1;
+}
+
 void qemu_send_packet(VLANClientState *vc1, const uint8_t *buf, int size)
 {
     VLANState *vlan = vc1->vlan;

@@ -1811,7 +1828,7 @@ static VLANClientState *slirp_vc;
 
 int slirp_can_output(void)
 {
-    return 1;
+    qemu_can_send_packet(slirp_vc);
 }
 
 void slirp_output(const uint8_t *pkt, int pkt_len)

@@ -1839,7 +1856,7 @@ static int net_slirp_init(VLANState *vla
         slirp_init();
     }
     slirp_vc = qemu_new_vlan_client(vlan, 
-                                    slirp_receive, NULL);
+                                    slirp_receive, NULL, NULL);
     snprintf(slirp_vc->info_str, sizeof(slirp_vc->info_str), "user redirector");
     return 0;
 }

@@ -2024,7 +2041,7 @@ static TAPState *net_tap_fd_init(VLANSta
     if (!s)
         return NULL;
     s->fd = fd;
-    s->vc = qemu_new_vlan_client(vlan, tap_receive, s);
+    s->vc = qemu_new_vlan_client(vlan, tap_receive, NULL, s);
     qemu_set_fd_handler(s->fd, tap_send, NULL, s);
     snprintf(s->vc->info_str, sizeof(s->vc->info_str), "tap: fd=%d", fd);
     return s;

@@ -2327,7 +2344,7 @@ static NetSocketState *net_socket_fd_ini
         return NULL;
     s->fd = fd;
 
-    s->vc = qemu_new_vlan_client(vlan, net_socket_receive_dgram, s);
+    s->vc = qemu_new_vlan_client(vlan, net_socket_receive_dgram, NULL, s);
     qemu_set_fd_handler(s->fd, net_socket_send_dgram, NULL, s);
 
     /* mcast: save bound address as dst */

@@ -2355,7 +2372,7 @@ static NetSocketState *net_socket_fd_ini
         return NULL;
     s->fd = fd;
     s->vc = qemu_new_vlan_client(vlan, 
-                                 net_socket_receive, s);
+                                 net_socket_receive, NULL, s);
     snprintf(s->vc->info_str, sizeof(s->vc->info_str),
              "socket: fd=%d", fd);
     if (is_connected) {

Index: vl.h
===================================================================
RCS file: /sources/qemu/qemu/vl.h,v
retrieving revision 1.99

vl.h +8 -1

diff -u -p -r1.99 vl.h
@@ -279,6 +279,9 @@ typedef struct VLANClientState VLANClien
 
 struct VLANClientState {
     IOReadHandler *fd_read;
+    /* Packets may still be sent if this returns zero.  It's used to
+       rate-limit the slirp code.  */
+    IOCanRWHandler *fd_can_read;
     void *opaque;
     struct VLANClientState *next;
     struct VLANState *vlan;

@@ -293,8 +296,12 @@ typedef struct VLANState {
 
 VLANState *qemu_find_vlan(int id);
 VLANClientState *qemu_new_vlan_client(VLANState *vlan,
-                                      IOReadHandler *fd_read, void *opaque);
+                                      IOReadHandler *fd_read,
+                                      IOCanRWHandler *fd_can_read,
+                                      void *opaque);
+int qemu_can_send_packet(VLANClientState *vc);
 void qemu_send_packet(VLANClientState *vc, const uint8_t *buf, int size);
+void qemu_handler_true(void *opaque);
 
 void do_info_network(void);

Index: hw/lance.c
===================================================================
RCS file: /sources/qemu/qemu/hw/lance.c,v
retrieving revision 1.5

lance.c +6 -1

diff -u -p -r1.5 lance.c
@@ -283,6 +283,11 @@ static CPUWriteMemoryFunc *lance_mem_wri
 
 #define MIN_BUF_SIZE 60
 
+static void lance_can_receive(void *opaque)
+{
+    return 1;
+}
+
 static void lance_receive(void *opaque, const uint8_t *buf, int size)
 {
     LANCEState *s = opaque;

@@ -440,7 +445,7 @@ void lance_init(NICInfo *nd, int irq, ui
 
     lance_reset(s);
 
-    s->vc = qemu_new_vlan_client(nd->vlan, lance_receive, s);
+    s->vc = qemu_new_vlan_client(nd->vlan, lance_receive, lance_can_receive, s);
 
     snprintf(s->vc->info_str, sizeof(s->vc->info_str),
              "lance macaddr=%02x:%02x:%02x:%02x:%02x:%02x",

Index: hw/ne2000.c
===================================================================
RCS file: /sources/qemu/qemu/hw/ne2000.c,v
retrieving revision 1.17

ne2000.c +16 -9

diff -u -p -r1.17 ne2000.c
@@ -200,14 +200,10 @@ static int compute_mcast_idx(const uint8
     return (crc >> 26);
 }
 
-/* return the max buffer size if the NE2000 can receive more data */
-static int ne2000_can_receive(void *opaque)
+static int ne2000_buffer_full(NE2000State *s)
 {
-    NE2000State *s = opaque;
     int avail, index, boundary;
-    
-    if (s->cmd & E8390_STOP)
-        return 0;
+
     index = s->curpag << 8;
     boundary = s->boundary << 8;
     if (index < boundary)

@@ -219,6 +215,15 @@ static int ne2000_can_receive(void *opaq
     return MAX_ETH_FRAME_SIZE;
 }
 
+static int ne2000_can_receive(void *opaque)
+{
+    NE2000State *s = opaque;
+    
+    if (s->cmd & E8390_STOP)
+        return 1;
+    return !ne2000_buffer_full(s);
+}
+
 #define MIN_BUF_SIZE 60
 
 static void ne2000_receive(void *opaque, const uint8_t *buf, int size)

@@ -234,7 +239,7 @@ static void ne2000_receive(void *opaque,
     printf("NE2000: received len=%d\n", size);
 #endif
 
-    if (!ne2000_can_receive(s))
+    if (s->cmd & E8390_STOP || ne2000_buffer_full(s))
         return;
     
     /* XXX: check this */

@@ -715,7 +720,8 @@ void isa_ne2000_init(int base, int irq, 
 
     ne2000_reset(s);
 
-    s->vc = qemu_new_vlan_client(nd->vlan, ne2000_receive, s);
+    s->vc = qemu_new_vlan_client(nd->vlan, ne2000_receive,
+                                 ne2000_can_receive, s);
 
     snprintf(s->vc->info_str, sizeof(s->vc->info_str),
              "ne2000 macaddr=%02x:%02x:%02x:%02x:%02x:%02x",

@@ -784,7 +790,8 @@ void pci_ne2000_init(PCIBus *bus, NICInf
     s->pci_dev = (PCIDevice *)d;
     memcpy(s->macaddr, nd->macaddr, 6);
     ne2000_reset(s);
-    s->vc = qemu_new_vlan_client(nd->vlan, ne2000_receive, s);
+    s->vc = qemu_new_vlan_client(nd->vlan, ne2000_receive,
+                                 ne2000_can_receive, s);
 
     snprintf(s->vc->info_str, sizeof(s->vc->info_str),
              "ne2000 pci macaddr=%02x:%02x:%02x:%02x:%02x:%02x",

Index: hw/smc91c111.c
===================================================================
RCS file: /sources/qemu/qemu/hw/smc91c111.c,v
retrieving revision 1.2

smc91c111.c +13 -1

diff -u -p -r1.2 smc91c111.c
@@ -593,6 +593,17 @@ static uint32_t smc91c111_readl(void *op
     return val;
 }
 
+static int smc91c111_can_receive(void *opaque)
+{
+    smc91c111_state *s = (smc91c111_state *)opaque;
+
+    if ((s->rcr & RCR_RXEN) == 0 || (s->rcr & RCR_SOFT_RST))
+        return 1;
+    if (s->allocated == (1 << NUM_PACKETS) - 1)
+        return 0;
+    return 1;
+}
+
 static void smc91c111_receive(void *opaque, const uint8_t *buf, int size)
 {
     smc91c111_state *s = (smc91c111_state *)opaque;

@@ -697,6 +708,7 @@ void smc91c111_init(NICInfo *nd, uint32_
 
     smc91c111_reset(s);
 
-    s->vc = qemu_new_vlan_client(nd->vlan, smc91c111_receive, s);
+    s->vc = qemu_new_vlan_client(nd->vlan, smc91c111_receive,
+                                 smc91c111_can_receive, s);
     /* ??? Save/restore.  */
 }

Juergen Lock

2005-12-20 20:59 UTC #1 lore raw

Quoted 9 lines · Paul Brook

Dan mentioned he was seeing very poor network performance when using user-net. It turns out this is because packets were getting dropped (the NIC buffers were full), and the slirp code doesn't throttle properly when this occurs. You end up with a bunch of packets getting sent to the guest, half of which get dropped, then a delay until slirp retransmits.
This is much more noticeable on Arm because the NIC only has a small memory buffer.

Quoted 8 lines

Teaching the slirp code how to do tcp backoff/window scaling is probably a fair amount of work. The easy alternative is to throttle the slirp output when we know the card's buffers are full. slirp has a convenient hook for this, so it's a matter of adding the hooks to the hardware emulation.
This increases arm user-net performance from painfully slow (~200kbit) to guest CPU bound (~20Mbit).

Hmm is the patch supposed to work for i386 guests too? I get no dhcp response and network unreachable when i try to ssh to the host from the guest after manually ifconfig'ing, seem like the ne2k doesn't receive any packets. (FreeBSD/i386 host, linux i386 guest - kanotix livecd iso). The same works normally without the patch.

Paul Brook

author

2005-12-20 21:15 UTC #2 lore raw

Quoted 7 lines · Juergen Lock

Quoted 5 lines · Paul Brook
Teaching the slirp code how to do tcp backoff/window scaling is probably a fair amount of work. The easy alternative is to throttle the slirp output when we know the card's buffers are full. slirp has a convenient hook for this, so it's a matter of adding the hooks to the hardware emulation.

Quoted 5 lines

Hmm is the patch supposed to work for i386 guests too? I get no dhcp response and network unreachable when i try to ssh to the host from the guest after manually ifconfig'ing, seem like the ne2k doesn't receive any packets. (FreeBSD/i386 host, linux i386 guest - kanotix livecd iso). The same works normally without the patch.

Oops. Updated patch should fix this. It was a dumb error in the ne2k code.

Paul

Index: vl.c
===================================================================
RCS file: /sources/qemu/qemu/vl.c,v
retrieving revision 1.157

vl.c +23 -6

diff -u -p -r1.157 vl.c
@@ -1768,13 +1768,16 @@ VLANState *qemu_find_vlan(int id)
 }
 
 VLANClientState *qemu_new_vlan_client(VLANState *vlan,
-                                      IOReadHandler *fd_read, void *opaque)
+                                      IOReadHandler *fd_read,
+                                      IOCanRWHandler *fd_can_read,
+                                      void *opaque)
 {
     VLANClientState *vc, **pvc;
     vc = qemu_mallocz(sizeof(VLANClientState));
     if (!vc)
         return NULL;
     vc->fd_read = fd_read;
+    vc->fd_can_read = fd_can_read;
     vc->opaque = opaque;
     vc->vlan = vlan;

@@ -1786,6 +1789,20 @@ VLANClientState *qemu_new_vlan_client(VL
     return vc;
 }
 
+int qemu_can_send_packet(VLANClientState *vc1)
+{
+    VLANState *vlan = vc1->vlan;
+    VLANClientState *vc;
+
+    for(vc = vlan->first_client; vc != NULL; vc = vc->next) {
+        if (vc != vc1) {
+            if (vc->fd_can_read && !vc->fd_can_read(vc->opaque))
+                return 0;
+        }
+    }
+    return 1;
+}
+
 void qemu_send_packet(VLANClientState *vc1, const uint8_t *buf, int size)
 {
     VLANState *vlan = vc1->vlan;

@@ -1811,7 +1828,7 @@ static VLANClientState *slirp_vc;
 
 int slirp_can_output(void)
 {
-    return 1;
+    qemu_can_send_packet(slirp_vc);
 }
 
 void slirp_output(const uint8_t *pkt, int pkt_len)

@@ -1839,7 +1856,7 @@ static int net_slirp_init(VLANState *vla
         slirp_init();
     }
     slirp_vc = qemu_new_vlan_client(vlan, 
-                                    slirp_receive, NULL);
+                                    slirp_receive, NULL, NULL);
     snprintf(slirp_vc->info_str, sizeof(slirp_vc->info_str), "user redirector");
     return 0;
 }

@@ -2024,7 +2041,7 @@ static TAPState *net_tap_fd_init(VLANSta
     if (!s)
         return NULL;
     s->fd = fd;
-    s->vc = qemu_new_vlan_client(vlan, tap_receive, s);
+    s->vc = qemu_new_vlan_client(vlan, tap_receive, NULL, s);
     qemu_set_fd_handler(s->fd, tap_send, NULL, s);
     snprintf(s->vc->info_str, sizeof(s->vc->info_str), "tap: fd=%d", fd);
     return s;

@@ -2327,7 +2344,7 @@ static NetSocketState *net_socket_fd_ini
         return NULL;
     s->fd = fd;
 
-    s->vc = qemu_new_vlan_client(vlan, net_socket_receive_dgram, s);
+    s->vc = qemu_new_vlan_client(vlan, net_socket_receive_dgram, NULL, s);
     qemu_set_fd_handler(s->fd, net_socket_send_dgram, NULL, s);
 
     /* mcast: save bound address as dst */

@@ -2355,7 +2372,7 @@ static NetSocketState *net_socket_fd_ini
         return NULL;
     s->fd = fd;
     s->vc = qemu_new_vlan_client(vlan, 
-                                 net_socket_receive, s);
+                                 net_socket_receive, NULL, s);
     snprintf(s->vc->info_str, sizeof(s->vc->info_str),
              "socket: fd=%d", fd);
     if (is_connected) {

Index: vl.h
===================================================================
RCS file: /sources/qemu/qemu/vl.h,v
retrieving revision 1.99

vl.h +8 -1

diff -u -p -r1.99 vl.h
@@ -279,6 +279,9 @@ typedef struct VLANClientState VLANClien
 
 struct VLANClientState {
     IOReadHandler *fd_read;
+    /* Packets may still be sent if this returns zero.  It's used to
+       rate-limit the slirp code.  */
+    IOCanRWHandler *fd_can_read;
     void *opaque;
     struct VLANClientState *next;
     struct VLANState *vlan;

@@ -293,8 +296,12 @@ typedef struct VLANState {
 
 VLANState *qemu_find_vlan(int id);
 VLANClientState *qemu_new_vlan_client(VLANState *vlan,
-                                      IOReadHandler *fd_read, void *opaque);
+                                      IOReadHandler *fd_read,
+                                      IOCanRWHandler *fd_can_read,
+                                      void *opaque);
+int qemu_can_send_packet(VLANClientState *vc);
 void qemu_send_packet(VLANClientState *vc, const uint8_t *buf, int size);
+void qemu_handler_true(void *opaque);
 
 void do_info_network(void);

Index: hw/lance.c
===================================================================
RCS file: /sources/qemu/qemu/hw/lance.c,v
retrieving revision 1.5

lance.c +6 -1

diff -u -p -r1.5 lance.c
@@ -283,6 +283,11 @@ static CPUWriteMemoryFunc *lance_mem_wri
 
 #define MIN_BUF_SIZE 60
 
+static void lance_can_receive(void *opaque)
+{
+    return 1;
+}
+
 static void lance_receive(void *opaque, const uint8_t *buf, int size)
 {
     LANCEState *s = opaque;

@@ -440,7 +445,7 @@ void lance_init(NICInfo *nd, int irq, ui
 
     lance_reset(s);
 
-    s->vc = qemu_new_vlan_client(nd->vlan, lance_receive, s);
+    s->vc = qemu_new_vlan_client(nd->vlan, lance_receive, lance_can_receive, s);
 
     snprintf(s->vc->info_str, sizeof(s->vc->info_str),
              "lance macaddr=%02x:%02x:%02x:%02x:%02x:%02x",

Index: hw/ne2000.c
===================================================================
RCS file: /sources/qemu/qemu/hw/ne2000.c,v
retrieving revision 1.17

ne2000.c +18 ~~-11~~

diff -u -p -r1.17 ne2000.c
@@ -200,14 +200,10 @@ static int compute_mcast_idx(const uint8
     return (crc >> 26);
 }
 
-/* return the max buffer size if the NE2000 can receive more data */
-static int ne2000_can_receive(void *opaque)
+static int ne2000_buffer_full(NE2000State *s)
 {
-    NE2000State *s = opaque;
     int avail, index, boundary;
-    
-    if (s->cmd & E8390_STOP)
-        return 0;
+
     index = s->curpag << 8;
     boundary = s->boundary << 8;
     if (index < boundary)

@@ -215,8 +211,17 @@ static int ne2000_can_receive(void *opaq
     else
         avail = (s->stop - s->start) - (index - boundary);
     if (avail < (MAX_ETH_FRAME_SIZE + 4))
-        return 0;
-    return MAX_ETH_FRAME_SIZE;
+        return 1;
+    return 0;
+}
+
+static int ne2000_can_receive(void *opaque)
+{
+    NE2000State *s = opaque;
+    
+    if (s->cmd & E8390_STOP)
+        return 1;
+    return !ne2000_buffer_full(s);
 }
 
 #define MIN_BUF_SIZE 60

@@ -234,7 +239,7 @@ static void ne2000_receive(void *opaque,
     printf("NE2000: received len=%d\n", size);
 #endif
 
-    if (!ne2000_can_receive(s))
+    if (s->cmd & E8390_STOP || ne2000_buffer_full(s))
         return;
     
     /* XXX: check this */

@@ -715,7 +720,8 @@ void isa_ne2000_init(int base, int irq, 
 
     ne2000_reset(s);
 
-    s->vc = qemu_new_vlan_client(nd->vlan, ne2000_receive, s);
+    s->vc = qemu_new_vlan_client(nd->vlan, ne2000_receive,
+                                 ne2000_can_receive, s);
 
     snprintf(s->vc->info_str, sizeof(s->vc->info_str),
              "ne2000 macaddr=%02x:%02x:%02x:%02x:%02x:%02x",

@@ -784,7 +790,8 @@ void pci_ne2000_init(PCIBus *bus, NICInf
     s->pci_dev = (PCIDevice *)d;
     memcpy(s->macaddr, nd->macaddr, 6);
     ne2000_reset(s);
-    s->vc = qemu_new_vlan_client(nd->vlan, ne2000_receive, s);
+    s->vc = qemu_new_vlan_client(nd->vlan, ne2000_receive,
+                                 ne2000_can_receive, s);
 
     snprintf(s->vc->info_str, sizeof(s->vc->info_str),
              "ne2000 pci macaddr=%02x:%02x:%02x:%02x:%02x:%02x",

Index: hw/smc91c111.c
===================================================================
RCS file: /sources/qemu/qemu/hw/smc91c111.c,v
retrieving revision 1.2

smc91c111.c +13 -1

diff -u -p -r1.2 smc91c111.c
@@ -593,6 +593,17 @@ static uint32_t smc91c111_readl(void *op
     return val;
 }
 
+static int smc91c111_can_receive(void *opaque)
+{
+    smc91c111_state *s = (smc91c111_state *)opaque;
+
+    if ((s->rcr & RCR_RXEN) == 0 || (s->rcr & RCR_SOFT_RST))
+        return 1;
+    if (s->allocated == (1 << NUM_PACKETS) - 1)
+        return 0;
+    return 1;
+}
+
 static void smc91c111_receive(void *opaque, const uint8_t *buf, int size)
 {
     smc91c111_state *s = (smc91c111_state *)opaque;

@@ -697,6 +708,7 @@ void smc91c111_init(NICInfo *nd, uint32_
 
     smc91c111_reset(s);
 
-    s->vc = qemu_new_vlan_client(nd->vlan, smc91c111_receive, s);
+    s->vc = qemu_new_vlan_client(nd->vlan, smc91c111_receive,
+                                 smc91c111_can_receive, s);
     /* ??? Save/restore.  */
 }

Juergen Lock

2005-12-21 21:20 UTC #3 lore raw

Quoted 8 lines · Paul Brook

Quoted 7 lines · Juergen Lock
Quoted 5 lines · Paul Brook
Teaching the slirp code how to do tcp backoff/window scaling is probably a fair amount of work. The easy alternative is to throttle the slirp output when we know the card's buffers are full. slirp has a convenient hook for this, so it's a matter of adding the hooks to the hardware emulation.

Quoted 7 lines

Quoted 5 lines
Hmm is the patch supposed to work for i386 guests too? I get no dhcp response and network unreachable when i try to ssh to the host from the guest after manually ifconfig'ing, seem like the ne2k doesn't receive any packets. (FreeBSD/i386 host, linux i386 guest - kanotix livecd iso). The same works normally without the patch.
Oops. Updated patch should fix this. It was a dumb error in the ne2k code.

Updated patch works, thanx, I get nearly 2 MB/sec host to guest over smb
now. (And I'm adding the patch to the FreeBSD port.)

Filip Navara

2005-12-21 21:43 UTC #4 lore raw

In reply to Paul Brook

Paul Brook wrote:
[snip]

Quoted 6 lines

int slirp_can_output(void)
{
vl.c +1 -1
-    return 1;
+    qemu_can_send_packet(slirp_vc);
}

It would be wise to return the value here.

- Filip