Re: [Qemu-devel] [RFC PATCH 6/8] tap: Drop tap_can_send

[Jason Wang <jasowang@redhat.com>]

On Wed, Apr 29, 2015 at 6:37 PM, Fam Zheng <famz@redhat.com> 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 <famz@redhat.com>
> ---
>  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
>