From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tetsuya Mukawa Subject: Re: [RFC PATCH 5/5] virtio: Extend virtio-net PMD to support container environment Date: Thu, 28 Jan 2016 18:53:14 +0900 Message-ID: <56A9E50A.9050203@igel.co.jp> 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> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit To: "Xie, Huawei" , "dev@dpdk.org" , "yuanhan.liu@linux.intel.com" , "Tan, Jianfeng" Return-path: Received: from mail-pf0-f173.google.com (mail-pf0-f173.google.com [209.85.192.173]) by dpdk.org (Postfix) with ESMTP id D9E28C386 for ; Thu, 28 Jan 2016 10:53:18 +0100 (CET) Received: by mail-pf0-f173.google.com with SMTP id o185so16444440pfb.1 for ; Thu, 28 Jan 2016 01:53:18 -0800 (PST) In-Reply-To: List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 2016/01/28 18:48, Xie, Huawei wrote: > On 1/28/2016 10:47 AM, Tetsuya Mukawa wrote: >> On 2016/01/28 0:58, Xie, Huawei wrote: >>> On 1/21/2016 7:09 PM, Tetsuya Mukawa wrote: >>> [snip] >>>> + >>>> +static int >>>> +qtest_raw_recv(int fd, char *buf, size_t count) >>>> +{ >>>> + size_t len = count; >>>> + size_t total_len = 0; >>>> + int ret = 0; >>>> + >>>> + while (len > 0) { >>>> + ret = read(fd, buf, len); >>>> + if (ret == (int)len) >>>> + break; >>>> + if (*(buf + ret - 1) == '\n') >>>> + break; >>> The above two lines should be put after the below if block. >> Yes, it should be so. >> >>>> + if (ret == -1) { >>>> + if (errno == EINTR) >>>> + continue; >>>> + return ret; >>>> + } >>>> + total_len += ret; >>>> + buf += ret; >>>> + len -= ret; >>>> + } >>>> + return total_len + ret; >>>> +} >>>> + >>> [snip] >>> >>>> + >>>> +static void >>>> +qtest_handle_one_message(struct qtest_session *s, char *buf) >>>> +{ >>>> + int ret; >>>> + >>>> + if (strncmp(buf, interrupt_message, strlen(interrupt_message)) == 0) { >>>> + if (rte_atomic16_read(&s->enable_intr) == 0) >>>> + return; >>>> + >>>> + /* relay interrupt to pipe */ >>>> + ret = write(s->irqfds.writefd, "1", 1); >>>> + if (ret < 0) >>>> + rte_panic("cannot relay interrupt\n"); >>>> + } else { >>>> + /* relay normal message to pipe */ >>>> + ret = qtest_raw_send(s->msgfds.writefd, buf, strlen(buf)); >>>> + if (ret < 0) >>>> + rte_panic("cannot relay normal message\n"); >>>> + } >>>> +} >>>> + >>>> +static char * >>>> +qtest_get_next_message(char *p) >>>> +{ >>>> + p = strchr(p, '\n'); >>>> + if ((p == NULL) || (*(p + 1) == '\0')) >>>> + return NULL; >>>> + return p + 1; >>>> +} >>>> + >>>> +static void >>>> +qtest_close_one_socket(int *fd) >>>> +{ >>>> + if (*fd > 0) { >>>> + close(*fd); >>>> + *fd = -1; >>>> + } >>>> +} >>>> + >>>> +static void >>>> +qtest_close_sockets(struct qtest_session *s) >>>> +{ >>>> + qtest_close_one_socket(&s->qtest_socket); >>>> + qtest_close_one_socket(&s->msgfds.readfd); >>>> + qtest_close_one_socket(&s->msgfds.writefd); >>>> + qtest_close_one_socket(&s->irqfds.readfd); >>>> + qtest_close_one_socket(&s->irqfds.writefd); >>>> + qtest_close_one_socket(&s->ivshmem_socket); >>>> +} >>>> + >>>> +/* >>>> + * This thread relays QTest response using pipe. >>>> + * The function is needed because we need to separate IRQ message from others. >>>> + */ >>>> +static void * >>>> +qtest_event_handler(void *data) { >>>> + struct qtest_session *s = (struct qtest_session *)data; >>>> + char buf[1024]; >>>> + char *p; >>>> + int ret; >>>> + >>>> + for (;;) { >>>> + memset(buf, 0, sizeof(buf)); >>>> + ret = qtest_raw_recv(s->qtest_socket, buf, sizeof(buf)); >>>> + if (ret < 0) { >>>> + qtest_close_sockets(s); >>>> + return NULL; >>>> + } >>>> + >>>> + /* may receive multiple messages at the same time */ >>> From the qtest_raw_recv implementation, if at some point one message is >>> received by two qtest_raw_recv calls, then is that message discarded? >>> We could save the last incomplete message in buffer, and combine the >>> message received next time together. >> I guess we don't lose replies from QEMU. >> Please let me describe more. >> >> According to the qtest specification, after sending a message, we need >> to receive a reply like below. >> APP: ---command---> QEMU >> APP: <-----------OK---- QEMU >> >> But, to handle interrupt message, we need to take care below case. >> APP: ---command---> QEMU >> APP: <---interrupt---- QEMU >> APP: <-----------OK---- QEMU >> >> Also, we need to handle a case like multiple threads tries to send a >> qtest message. >> Anyway, here is current implementation. >> >> So far, we have 3 types of sockets. >> 1. socket for qtest messaging. >> 2. socket for relaying normal message. >> 3. socket for relaying interrupt message. >> >> About read direction: >> The qtest socket is only read by "qtest_event_handler". The handler may >> receive multiple messages at once. > I think there are two assumptions that all messages are ended with "\n" > and the sizeof(buf) could hold the maximum length of sum of all multiple > messages that QEMU could send at one time. > Otherwise in the last read call of qtest_raw_receive, you might receive > only part of the a message. I've got your point. I will fix above case. Thanks, Tetsuya