From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59283) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YnkK3-0003hE-1N for qemu-devel@nongnu.org; Thu, 30 Apr 2015 04:59:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YnkJy-0000Ov-37 for qemu-devel@nongnu.org; Thu, 30 Apr 2015 04:59:54 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41473) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YnkJx-0000Of-RT for qemu-devel@nongnu.org; Thu, 30 Apr 2015 04:59:50 -0400 Date: Thu, 30 Apr 2015 16:59:13 +0800 From: Jason Wang Message-Id: <1430384353.20407.3@smtp.corp.redhat.com> In-Reply-To: <1430303875-31647-7-git-send-email-famz@redhat.com> References: <1430303875-31647-1-git-send-email-famz@redhat.com> <1430303875-31647-7-git-send-email-famz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Subject: Re: [Qemu-devel] [RFC PATCH 6/8] tap: Drop tap_can_send List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: Kevin Wolf , qemu-devel@nongnu.org, Vincenzo Maffione , Stefan Hajnoczi , Paolo Bonzini , Giuseppe Lettieri , Luigi Rizzo On Wed, Apr 29, 2015 at 6:37 PM, Fam Zheng wrote: > This callback is called by main loop before polling s->fd, if it > returns > false, the fd will not be polled in this iteration. > > This is redundant with checks inside read callback. After this patch, > the data will be sent to peer when it arrives. If the device can't > receive, it will be queued to incoming_queue, and when the device > status > changes, this queue will be flushed. > > Signed-off-by: Fam Zheng > --- > net/tap.c | 23 +++++++++++------------ > 1 file changed, 11 insertions(+), 12 deletions(-) > > diff --git a/net/tap.c b/net/tap.c > index 968df46..2ddf570 100644 > --- a/net/tap.c > +++ b/net/tap.c > @@ -61,14 +61,12 @@ typedef struct TAPState { > > static int launch_script(const char *setup_script, const char > *ifname, int fd); > > -static int tap_can_send(void *opaque); > static void tap_send(void *opaque); > static void tap_writable(void *opaque); > > static void tap_update_fd_handler(TAPState *s) > { > - qemu_set_fd_handler2(s->fd, > - s->read_poll && s->enabled ? tap_can_send : > NULL, > + qemu_set_fd_handler2(s->fd, NULL, > s->read_poll && s->enabled ? tap_send : > NULL, > s->write_poll && s->enabled ? tap_writable > : NULL, > s); > @@ -165,13 +163,6 @@ static ssize_t tap_receive(NetClientState *nc, > const uint8_t *buf, size_t size) > return tap_write_packet(s, iov, 1); > } > > -static int tap_can_send(void *opaque) > -{ > - TAPState *s = opaque; > - > - return qemu_can_send_packet(&s->nc); > -} > - > #ifndef __sun__ > ssize_t tap_read_packet(int tapfd, uint8_t *buf, int maxlen) > { > @@ -190,10 +181,13 @@ static void tap_send(void *opaque) > TAPState *s = opaque; > int size; > int packets = 0; > + bool can_send = true; > > - while (qemu_can_send_packet(&s->nc)) { > + while (can_send) { > uint8_t *buf = s->buf; > > + can_send = qemu_can_send_packet(&s->nc); > + > size = tap_read_packet(s->fd, s->buf, sizeof(s->buf)); > if (size <= 0) { > break; > @@ -204,8 +198,13 @@ static void tap_send(void *opaque) > size -= s->host_vnet_hdr_len; > } > > + /* If !can_send, we will want to disable the read poll, but > we still > + * need the send completion callback to enable it again, > which is a > + * sign of peer becoming ready. So call the send function > + * regardlessly of can_send. > + */ It was probably not safe to depend on sent_cb to re-enable the polling. Since the packet could be purged in some conditions (e.g net_vm_change_state_handler()). So tap_send_completed won't be called in this case. > > size = qemu_send_packet_async(&s->nc, buf, size, > tap_send_completed); > - if (size == 0) { > + if (size == 0 || !can_send) { > tap_read_poll(s, false); > break; > } else if (size < 0) { > -- > 1.9.3 >