From patchwork Fri Aug 24 07:56:23 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: li qongqing X-Patchwork-Id: 179778 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 6F2B42C0102 for ; Fri, 24 Aug 2012 17:56:52 +1000 (EST) Received: from localhost ([::1]:33736 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T4ol8-0008Sv-Ax for incoming@patchwork.ozlabs.org; Fri, 24 Aug 2012 03:56:50 -0400 Received: from eggs.gnu.org ([208.118.235.92]:37876) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T4oks-0008IC-Ny for qemu-devel@nongnu.org; Fri, 24 Aug 2012 03:56:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T4oko-000226-Op for qemu-devel@nongnu.org; Fri, 24 Aug 2012 03:56:34 -0400 Received: from mail.windriver.com ([147.11.1.11]:64709) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T4oko-00021e-G1 for qemu-devel@nongnu.org; Fri, 24 Aug 2012 03:56:30 -0400 Received: from ALA-HCA.corp.ad.wrs.com (ala-hca [147.11.189.40]) by mail.windriver.com (8.14.5/8.14.3) with ESMTP id q7O7uQ8T014692 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=FAIL); Fri, 24 Aug 2012 00:56:26 -0700 (PDT) Received: from lirq-OptiPlex-780.corp.ad.wrs.com (128.224.162.163) by ALA-HCA.corp.ad.wrs.com (147.11.189.50) with Microsoft SMTP Server id 14.2.309.2; Fri, 24 Aug 2012 00:56:26 -0700 From: To: Date: Fri, 24 Aug 2012 15:56:23 +0800 Message-ID: <1345794984-10337-1-git-send-email-rongqing.li@windriver.com> X-Mailer: git-send-email 1.7.4.1 MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: Solaris 9 X-Received-From: 147.11.1.11 Cc: stefanha@linux.vnet.ibm.com Subject: [Qemu-devel] [PATCH 1/2] Usb: create the receive queue for the virtual USB NIC X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org From: Roy.Li 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 --- 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,