From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47289) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZeIFl-0005Aj-UU for qemu-devel@nongnu.org; Tue, 22 Sep 2015 03:44:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZeIFi-0002qs-K0 for qemu-devel@nongnu.org; Tue, 22 Sep 2015 03:44:41 -0400 Received: from [59.151.112.132] (port=2847 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZeIFg-0002nP-Sw for qemu-devel@nongnu.org; Tue, 22 Sep 2015 03:44:38 -0400 Message-ID: <560106C2.5080607@cn.fujitsu.com> Date: Tue, 22 Sep 2015 15:44:02 +0800 From: Yang Hongyang MIME-Version: 1.0 References: <1442405768-23019-1-git-send-email-yanghy@cn.fujitsu.com> <1442405768-23019-5-git-send-email-yanghy@cn.fujitsu.com> <56010391.4070806@redhat.com> In-Reply-To: <56010391.4070806@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v11 04/12] net: merge qemu_deliver_packet and qemu_deliver_packet_iov List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang , qemu-devel@nongnu.org Cc: thuth@redhat.com, stefanha@redhat.com, armbru@redhat.com, lizhijian@cn.fujitsu.com, zhang.zhanghailiang@huawei.com Hi Jason, Thanks for the review, On 09/22/2015 03:30 PM, Jason Wang wrote: > > > On 09/16/2015 08:16 PM, Yang Hongyang wrote: >> qemu_deliver_packet_iov already have the compat delivery, we >> can drop qemu_deliver_packet. >> >> Signed-off-by: Yang Hongyang >> --- >> include/net/net.h | 5 ----- >> net/net.c | 40 +++++++--------------------------------- >> net/queue.c | 6 +++++- >> 3 files changed, 12 insertions(+), 39 deletions(-) >> >> diff --git a/include/net/net.h b/include/net/net.h >> index 36e5fab..7af3e15 100644 >> --- a/include/net/net.h >> +++ b/include/net/net.h >> @@ -152,11 +152,6 @@ void qemu_check_nic_model(NICInfo *nd, const char *model); >> int qemu_find_nic_model(NICInfo *nd, const char * const *models, >> const char *default_model); >> >> -ssize_t qemu_deliver_packet(NetClientState *sender, >> - unsigned flags, >> - const uint8_t *data, >> - size_t size, >> - void *opaque); >> ssize_t qemu_deliver_packet_iov(NetClientState *sender, >> unsigned flags, >> const struct iovec *iov, >> diff --git a/net/net.c b/net/net.c >> index ad37419..ca35b27 100644 >> --- a/net/net.c >> +++ b/net/net.c >> @@ -597,36 +597,6 @@ static ssize_t filter_receive(NetClientState *nc, NetFilterChain chain, >> return filter_receive_iov(nc, chain, sender, flags, &iov, 1, sent_cb); >> } >> >> -ssize_t qemu_deliver_packet(NetClientState *sender, >> - unsigned flags, >> - const uint8_t *data, >> - size_t size, >> - void *opaque) >> -{ >> - NetClientState *nc = opaque; >> - ssize_t ret; >> - >> - if (nc->link_down) { >> - return size; >> - } >> - >> - if (nc->receive_disabled) { >> - return 0; >> - } >> - >> - if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) { >> - ret = nc->info->receive_raw(nc, data, size); >> - } else { >> - ret = nc->info->receive(nc, data, size); >> - } >> - >> - if (ret == 0) { >> - nc->receive_disabled = 1; >> - } >> - >> - return ret; >> -} >> - >> void qemu_purge_queued_packets(NetClientState *nc) >> { >> if (!nc->peer) { >> @@ -717,14 +687,18 @@ ssize_t qemu_send_packet_raw(NetClientState *nc, const uint8_t *buf, int size) >> } >> >> static ssize_t nc_sendv_compat(NetClientState *nc, const struct iovec *iov, >> - int iovcnt) >> + int iovcnt, unsigned flags) >> { >> uint8_t buffer[NET_BUFSIZE]; >> size_t offset; >> >> offset = iov_to_buf(iov, iovcnt, 0, buffer, sizeof(buffer)); >> >> - return nc->info->receive(nc, buffer, offset); >> + if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) { >> + return nc->info->receive_raw(nc, buffer, offset); >> + } else { >> + return nc->info->receive(nc, buffer, offset); >> + } > > Then for net clients that doesn't support receive_iov, an extra memcpy > is introduced. This seems unnecessary. And this patch looks independent, > so please drop this patch from the series (This can also help to reduce > the iteration). I think we may need this after all net clients support > receive_iov(). This patch is required by the next patch which introduce a +/* Returns: + * >0 - success + * 0 - queue packet for future redelivery + * <0 - failure (discard packet) + */ +typedef ssize_t (NetQueueDeliverFunc)(NetClientState *sender, + unsigned flags, + const struct iovec *iov, + int iovcnt, + void *opaque); If we drop this patch, we will need to introduce 2 deliver func, which seems not clean and simple. > > >> } >> >> ssize_t qemu_deliver_packet_iov(NetClientState *sender, >> @@ -747,7 +721,7 @@ ssize_t qemu_deliver_packet_iov(NetClientState *sender, >> if (nc->info->receive_iov) { >> ret = nc->info->receive_iov(nc, iov, iovcnt); >> } else { >> - ret = nc_sendv_compat(nc, iov, iovcnt); >> + ret = nc_sendv_compat(nc, iov, iovcnt, flags); >> } >> >> if (ret == 0) { >> diff --git a/net/queue.c b/net/queue.c >> index ebbe2bb..cf8db3a 100644 >> --- a/net/queue.c >> +++ b/net/queue.c >> @@ -152,9 +152,13 @@ static ssize_t qemu_net_queue_deliver(NetQueue *queue, >> size_t size) >> { >> ssize_t ret = -1; >> + struct iovec iov = { >> + .iov_base = (void *)data, >> + .iov_len = size >> + }; >> >> queue->delivering = 1; >> - ret = qemu_deliver_packet(sender, flags, data, size, queue->opaque); >> + ret = qemu_deliver_packet_iov(sender, flags, &iov, 1, queue->opaque); >> queue->delivering = 0; >> >> return ret; > > > . > -- Thanks, Yang.