All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 1/2] Usb: create the receive queue for the virtual USB NIC
@ 2012-08-24  7:56 rongqing.li
  2012-08-24  7:56 ` [Qemu-devel] [PATCH 2/2] hub: change hub can_receive() strategy rongqing.li
  2012-08-24 12:25 ` [Qemu-devel] [PATCH 1/2] Usb: create the receive queue for the virtual USB NIC Stefan Hajnoczi
  0 siblings, 2 replies; 6+ messages in thread
From: rongqing.li @ 2012-08-24  7:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: stefanha

From: Roy.Li <rongqing.li@windriver.com>

The virtual USB NIC originally used a fixed buffer to receive packets which
only store 1 packet at a time, which is easy to overrun with packets if the
guest does not consume it quickly, and always lost packets at the below case:

	The emulated machine is using an USB-ethernet adapter,
	it is connected to the network using SLIRP and I'm
	dumping the traffic in a .pcap file.  As per the following
	command line :
	-net nic,model=usb,vlan=1 -net user,vlan=1 -net dump,vlan=1,file=/tmp/pkt.pcap
	Every time that two packets are coming in a row from
	the host, the usb-net code will receive the first one,
	then returns 0 to can_receive call since it has a
	1 packet long queue. But as the dump code is always
	ready to receive, qemu_can_send_packet will return
	true and the next packet will discard the previous
	one in the usb-net code.

Commit 60c07d933c(net: fix qemu_can_send_packet logic) is trying to fix,
but the logic is wrong and introduce other bug. Now replace the fixed buffer
with a receive queue which can accept a variable number of packets to fix
this bug.

Signed-off-by: Roy.Li <rongqing.li@windriver.com>
---
 hw/usb/dev-network.c |   87 ++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 63 insertions(+), 24 deletions(-)

diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c
index c84892c..0c867b7 100644
--- a/hw/usb/dev-network.c
+++ b/hw/usb/dev-network.c
@@ -87,6 +87,7 @@ enum usbstring_idx {
 #define STATUS_BYTECOUNT		16   /* 8 byte header + data */
 
 #define ETH_FRAME_LEN			1514 /* Max. octets in frame sans FCS */
+#define MAX_RCV_QUEUE			45 /* Max length of receiving queue */
 
 static const USBDescStrings usb_net_stringtable = {
     [STRING_MANUFACTURER]       = "QEMU",
@@ -622,6 +623,12 @@ struct rndis_response {
     uint8_t buf[0];
 };
 
+struct USBNet_buffer {
+    QTAILQ_ENTRY(USBNet_buffer) entries;
+    unsigned int in_ptr, in_len;
+    uint8_t in_buf[0];
+};
+
 typedef struct USBNetState {
     USBDevice dev;
 
@@ -636,8 +643,8 @@ typedef struct USBNetState {
     uint8_t out_buf[2048];
 
     USBPacket *inpkt;
-    unsigned int in_ptr, in_len;
-    uint8_t in_buf[2048];
+    QTAILQ_HEAD(USBNet_buffer_head, USBNet_buffer) rndis_netbuf;
+    uint32_t buf_len;
 
     char usbstring_mac[13];
     NICState *nic;
@@ -867,6 +874,17 @@ static void rndis_clear_responsequeue(USBNetState *s)
     }
 }
 
+static void rndis_clear_netbuffer(USBNetState *s)
+{
+    struct USBNet_buffer *r;
+
+    while ((r = s->rndis_netbuf.tqh_first)) {
+        QTAILQ_REMOVE(&s->rndis_netbuf, r, entries);
+        g_free(r);
+    }
+    s->buf_len = 0;
+}
+
 static int rndis_init_response(USBNetState *s, rndis_init_msg_type *buf)
 {
     rndis_init_cmplt_type *resp =
@@ -1025,7 +1043,8 @@ static int rndis_parse(USBNetState *s, uint8_t *data, int length)
 
     case RNDIS_RESET_MSG:
         rndis_clear_responsequeue(s);
-        s->out_ptr = s->in_ptr = s->in_len = 0;
+        rndis_clear_netbuffer(s);
+        s->out_ptr = 0;
         return rndis_reset_response(s, (rndis_reset_msg_type *) data);
 
     case RNDIS_KEEPALIVE_MSG:
@@ -1134,25 +1153,39 @@ static int usb_net_handle_datain(USBNetState *s, USBPacket *p)
 {
     int ret = USB_RET_NAK;
 
-    if (s->in_ptr > s->in_len) {
-        s->in_ptr = s->in_len = 0;
-        ret = USB_RET_NAK;
+    struct USBNet_buffer *r = s->rndis_netbuf.tqh_first;
+
+    if (!r) {
         return ret;
     }
-    if (!s->in_len) {
-        ret = USB_RET_NAK;
+
+    if (r->in_ptr > r->in_len) {
+        s->buf_len--;
+        QTAILQ_REMOVE(&s->rndis_netbuf, r, entries);
+        g_free(r);
+        return ret;
+    }
+
+    if (!r->in_len) {
+        s->buf_len--;
+        QTAILQ_REMOVE(&s->rndis_netbuf, r, entries);
+        g_free(r);
         return ret;
     }
-    ret = s->in_len - s->in_ptr;
+
+    ret = r->in_len - r->in_ptr;
     if (ret > p->iov.size) {
         ret = p->iov.size;
     }
-    usb_packet_copy(p, &s->in_buf[s->in_ptr], ret);
-    s->in_ptr += ret;
-    if (s->in_ptr >= s->in_len &&
-                    (is_rndis(s) || (s->in_len & (64 - 1)) || !ret)) {
+
+    usb_packet_copy(p, &r->in_buf[r->in_ptr], ret);
+    r->in_ptr += ret;
+    if (r->in_ptr >= r->in_len &&
+                    (is_rndis(s) || (r->in_len & (64 - 1)) || !ret)) {
         /* no short packet necessary */
-        s->in_ptr = s->in_len = 0;
+        s->buf_len--;
+        QTAILQ_REMOVE(&s->rndis_netbuf, r, entries);
+        g_free(r);
     }
 
 #ifdef TRAFFIC_DEBUG
@@ -1251,15 +1284,16 @@ static ssize_t usbnet_receive(NetClientState *nc, const uint8_t *buf, size_t siz
 {
     USBNetState *s = DO_UPCAST(NICState, nc, nc)->opaque;
     struct rndis_packet_msg_type *msg;
+    struct USBNet_buffer *r;
 
     if (is_rndis(s)) {
-        msg = (struct rndis_packet_msg_type *) s->in_buf;
         if (s->rndis_state != RNDIS_DATA_INITIALIZED) {
             return -1;
         }
-        if (size + sizeof(struct rndis_packet_msg_type) > sizeof(s->in_buf))
-            return -1;
 
+        r = g_malloc0(sizeof(struct USBNet_buffer) + \
+                      sizeof(struct rndis_packet_msg_type) + size);
+        msg = (struct rndis_packet_msg_type *) r->in_buf;
         memset(msg, 0, sizeof(struct rndis_packet_msg_type));
         msg->MessageType = cpu_to_le32(RNDIS_PACKET_MSG);
         msg->MessageLength = cpu_to_le32(size + sizeof(struct rndis_packet_msg_type));
@@ -1274,14 +1308,16 @@ static ssize_t usbnet_receive(NetClientState *nc, const uint8_t *buf, size_t siz
          * msg->Reserved;
          */
         memcpy(msg + 1, buf, size);
-        s->in_len = size + sizeof(struct rndis_packet_msg_type);
+        r->in_len = size + sizeof(struct rndis_packet_msg_type);
     } else {
-        if (size > sizeof(s->in_buf))
-            return -1;
-        memcpy(s->in_buf, buf, size);
-        s->in_len = size;
+        r = g_malloc0(sizeof(struct USBNet_buffer) + size);
+        memcpy(r->in_buf, buf, size);
+        r->in_len = size;
     }
-    s->in_ptr = 0;
+    r->in_ptr = 0;
+    s->buf_len++;
+    QTAILQ_INSERT_TAIL(&s->rndis_netbuf, r, entries);
+
     return size;
 }
 
@@ -1293,7 +1329,7 @@ static int usbnet_can_receive(NetClientState *nc)
         return 1;
     }
 
-    return !s->in_len;
+    return s->buf_len < MAX_RCV_QUEUE;
 }
 
 static void usbnet_cleanup(NetClientState *nc)
@@ -1309,6 +1345,7 @@ static void usb_net_handle_destroy(USBDevice *dev)
 
     /* TODO: remove the nd_table[] entry */
     rndis_clear_responsequeue(s);
+    rndis_clear_netbuffer(s);
     qemu_del_net_client(&s->nic->nc);
 }
 
@@ -1329,12 +1366,14 @@ static int usb_net_initfn(USBDevice *dev)
 
     s->rndis_state = RNDIS_UNINITIALIZED;
     QTAILQ_INIT(&s->rndis_resp);
+    QTAILQ_INIT(&s->rndis_netbuf);
 
     s->medium = 0;	/* NDIS_MEDIUM_802_3 */
     s->speed = 1000000; /* 100MBps, in 100Bps units */
     s->media_state = 0;	/* NDIS_MEDIA_STATE_CONNECTED */;
     s->filter = 0;
     s->vendorid = 0x1234;
+    s->buf_len = 0;
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
     s->nic = qemu_new_nic(&net_usbnet_info, &s->conf,
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-08-24 12:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-24  7:56 [Qemu-devel] [PATCH 1/2] Usb: create the receive queue for the virtual USB NIC rongqing.li
2012-08-24  7:56 ` [Qemu-devel] [PATCH 2/2] hub: change hub can_receive() strategy rongqing.li
2012-08-24  8:20   ` Paolo Bonzini
2012-08-24  8:33     ` Rongqing Li
2012-08-24 10:11       ` Stefan Hajnoczi
2012-08-24 12:25 ` [Qemu-devel] [PATCH 1/2] Usb: create the receive queue for the virtual USB NIC Stefan Hajnoczi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.