From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id F0F9FC3F4 for ; Thu, 28 Jan 2016 10:48:21 +0100 (CET) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga104.fm.intel.com with ESMTP; 28 Jan 2016 01:48:22 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,358,1449561600"; d="scan'208";a="899987705" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga002.jf.intel.com with ESMTP; 28 Jan 2016 01:48:20 -0800 Received: from fmsmsx120.amr.corp.intel.com (10.18.124.208) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 28 Jan 2016 01:48:20 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx120.amr.corp.intel.com (10.18.124.208) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 28 Jan 2016 01:48:19 -0800 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.215]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.231]) with mapi id 14.03.0248.002; Thu, 28 Jan 2016 17:48:18 +0800 From: "Xie, Huawei" To: Tetsuya Mukawa , "dev@dpdk.org" , "yuanhan.liu@linux.intel.com" , "Tan, Jianfeng" Thread-Topic: [RFC PATCH 5/5] virtio: Extend virtio-net PMD to support container environment Thread-Index: AdFZG5KPAunQ5LVPT++h9TW0MGQ/9A== Date: Thu, 28 Jan 2016 09:48:18 +0000 Message-ID: References: <1453108389-21006-2-git-send-email-mukawa@igel.co.jp> <1453374478-30996-6-git-send-email-mukawa@igel.co.jp> <56A98140.2000507@igel.co.jp> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.4.160] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [RFC PATCH 5/5] virtio: Extend virtio-net PMD to support container environment X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Jan 2016 09:48:22 -0000 On 1/28/2016 10:47 AM, Tetsuya Mukawa wrote:=0A= > On 2016/01/28 0:58, Xie, Huawei wrote:=0A= >> On 1/21/2016 7:09 PM, Tetsuya Mukawa wrote:=0A= >> [snip]=0A= >>> +=0A= >>> +static int=0A= >>> +qtest_raw_recv(int fd, char *buf, size_t count)=0A= >>> +{=0A= >>> + size_t len =3D count;=0A= >>> + size_t total_len =3D 0;=0A= >>> + int ret =3D 0;=0A= >>> +=0A= >>> + while (len > 0) {=0A= >>> + ret =3D read(fd, buf, len);=0A= >>> + if (ret =3D=3D (int)len)=0A= >>> + break;=0A= >>> + if (*(buf + ret - 1) =3D=3D '\n')=0A= >>> + break;=0A= >> The above two lines should be put after the below if block.=0A= > Yes, it should be so.=0A= >=0A= >>> + if (ret =3D=3D -1) {=0A= >>> + if (errno =3D=3D EINTR)=0A= >>> + continue;=0A= >>> + return ret;=0A= >>> + }=0A= >>> + total_len +=3D ret;=0A= >>> + buf +=3D ret;=0A= >>> + len -=3D ret;=0A= >>> + }=0A= >>> + return total_len + ret;=0A= >>> +}=0A= >>> +=0A= >> [snip]=0A= >>=0A= >>> +=0A= >>> +static void=0A= >>> +qtest_handle_one_message(struct qtest_session *s, char *buf)=0A= >>> +{=0A= >>> + int ret;=0A= >>> +=0A= >>> + if (strncmp(buf, interrupt_message, strlen(interrupt_message)) =3D=3D= 0) {=0A= >>> + if (rte_atomic16_read(&s->enable_intr) =3D=3D 0)=0A= >>> + return;=0A= >>> +=0A= >>> + /* relay interrupt to pipe */=0A= >>> + ret =3D write(s->irqfds.writefd, "1", 1);=0A= >>> + if (ret < 0)=0A= >>> + rte_panic("cannot relay interrupt\n");=0A= >>> + } else {=0A= >>> + /* relay normal message to pipe */=0A= >>> + ret =3D qtest_raw_send(s->msgfds.writefd, buf, strlen(buf));=0A= >>> + if (ret < 0)=0A= >>> + rte_panic("cannot relay normal message\n");=0A= >>> + }=0A= >>> +}=0A= >>> +=0A= >>> +static char *=0A= >>> +qtest_get_next_message(char *p)=0A= >>> +{=0A= >>> + p =3D strchr(p, '\n');=0A= >>> + if ((p =3D=3D NULL) || (*(p + 1) =3D=3D '\0'))=0A= >>> + return NULL;=0A= >>> + return p + 1;=0A= >>> +}=0A= >>> +=0A= >>> +static void=0A= >>> +qtest_close_one_socket(int *fd)=0A= >>> +{=0A= >>> + if (*fd > 0) {=0A= >>> + close(*fd);=0A= >>> + *fd =3D -1;=0A= >>> + }=0A= >>> +}=0A= >>> +=0A= >>> +static void=0A= >>> +qtest_close_sockets(struct qtest_session *s)=0A= >>> +{=0A= >>> + qtest_close_one_socket(&s->qtest_socket);=0A= >>> + qtest_close_one_socket(&s->msgfds.readfd);=0A= >>> + qtest_close_one_socket(&s->msgfds.writefd);=0A= >>> + qtest_close_one_socket(&s->irqfds.readfd);=0A= >>> + qtest_close_one_socket(&s->irqfds.writefd);=0A= >>> + qtest_close_one_socket(&s->ivshmem_socket);=0A= >>> +}=0A= >>> +=0A= >>> +/*=0A= >>> + * This thread relays QTest response using pipe.=0A= >>> + * The function is needed because we need to separate IRQ message from= others.=0A= >>> + */=0A= >>> +static void *=0A= >>> +qtest_event_handler(void *data) {=0A= >>> + struct qtest_session *s =3D (struct qtest_session *)data;=0A= >>> + char buf[1024];=0A= >>> + char *p;=0A= >>> + int ret;=0A= >>> +=0A= >>> + for (;;) {=0A= >>> + memset(buf, 0, sizeof(buf));=0A= >>> + ret =3D qtest_raw_recv(s->qtest_socket, buf, sizeof(buf));=0A= >>> + if (ret < 0) {=0A= >>> + qtest_close_sockets(s);=0A= >>> + return NULL;=0A= >>> + }=0A= >>> +=0A= >>> + /* may receive multiple messages at the same time */=0A= >> From the qtest_raw_recv implementation, if at some point one message is= =0A= >> received by two qtest_raw_recv calls, then is that message discarded?=0A= >> We could save the last incomplete message in buffer, and combine the=0A= >> message received next time together.=0A= > I guess we don't lose replies from QEMU.=0A= > Please let me describe more.=0A= >=0A= > According to the qtest specification, after sending a message, we need=0A= > to receive a reply like below.=0A= > APP: ---command---> QEMU=0A= > APP: <-----------OK---- QEMU=0A= >=0A= > But, to handle interrupt message, we need to take care below case.=0A= > APP: ---command---> QEMU=0A= > APP: <---interrupt---- QEMU=0A= > APP: <-----------OK---- QEMU=0A= >=0A= > Also, we need to handle a case like multiple threads tries to send a=0A= > qtest message.=0A= > Anyway, here is current implementation.=0A= >=0A= > So far, we have 3 types of sockets.=0A= > 1. socket for qtest messaging.=0A= > 2. socket for relaying normal message.=0A= > 3. socket for relaying interrupt message.=0A= >=0A= > About read direction:=0A= > The qtest socket is only read by "qtest_event_handler". The handler may= =0A= > receive multiple messages at once.=0A= =0A= I think there are two assumptions that all messages are ended with "\n"=0A= and the sizeof(buf) could hold the maximum length of sum of all multiple=0A= messages that QEMU could send at one time.=0A= Otherwise in the last read call of qtest_raw_receive, you might receive=0A= only part of the a message.=0A= =0A= > In the case, the handler split messages, and send it to normal message= =0A= > socket or interrupt message socket.=0A= >=0A= > About write direction:=0A= > The qtest socket will be written by below functions.=0A= > - qtest_raw_in/out=0A= > - qtest_raw_read/write=0A= > But all functions that use above functions need to have mutex before=0A= > sending messages.=0A= > So all messaging will not be overlapped, then only one thread will read= =0A= > the socket for relaying normal message.=0A= >=0A= > Tetsuya=0A= >=0A= =0A=