[Qemu-devel] [PULL 0/3] Net patches

[Qemu-devel] [PULL 0/3] Net patches

[Stefan Hajnoczi]

Paolo's fixes to resume rx when buffers have been replenished on e1000 and
xen_nic.  virtio-net does not have this bug.

The following changes since commit 3d1d9652978ac5a32a0beb4bdf6065ca39440d89:

  handle device help before accelerator set up (2012-08-09 19:53:01 +0000)

are available in the git repository at:

  git://github.com/stefanha/qemu.git net

for you to fetch changes up to 4247561bbeacb5ece8b90d0d2439efefc0e7283a:

  xen: flush queue when getting an event (2012-08-10 11:43:54 +0100)

----------------------------------------------------------------
Paolo Bonzini (3):
      net: notify iothread after flushing queue
      e1000: flush queue whenever can_receive can go from false to true
      xen: flush queue when getting an event

 hw/e1000.c      |    4 ++++
 hw/virtio-net.c |    4 ----
 hw/xen_nic.c    |    1 +
 net.c           |    7 ++++++-
 net/queue.c     |    5 +++--
 net/queue.h     |    2 +-
 6 files changed, 15 insertions(+), 8 deletions(-)

-- 
1.7.10.4

[Qemu-devel] [PATCH 1/3] net: notify iothread after flushing queue

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

virtio-net has code to flush the queue and notify the iothread
whenever new receive buffers are added by the guest.  That is
fine, and indeed we need to do the same in all other drivers.
However, notifying the iothread should be work for the network
subsystem.  And since we are at it we can add a little smartness:
if some of the queued packets already could not be delivered,
there is no need to notify the iothread.

Reported-by: Luigi Rizzo <rizzo@iet.unipi.it>
Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Cc: Jan Kiszka <jan.kiszka@siemens.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 hw/virtio-net.c |    4 ----
 net.c           |    7 ++++++-
 net/queue.c     |    5 +++--
 net/queue.h     |    2 +-
 4 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index b1998b2..6490743 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -447,10 +447,6 @@ static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
     VirtIONet *n = to_virtio_net(vdev);
 
     qemu_flush_queued_packets(&n->nic->nc);
-
-    /* We now have RX buffers, signal to the IO thread to break out of the
-     * select to re-poll the tap file descriptor */
-    qemu_notify_event();
 }
 
 static int virtio_net_can_receive(NetClientState *nc)
diff --git a/net.c b/net.c
index 60043dd..76a8336 100644
--- a/net.c
+++ b/net.c
@@ -357,7 +357,12 @@ void qemu_flush_queued_packets(NetClientState *nc)
 {
     nc->receive_disabled = 0;
 
-    qemu_net_queue_flush(nc->send_queue);
+    if (qemu_net_queue_flush(nc->send_queue)) {
+        /* We emptied the queue successfully, signal to the IO thread to repoll
+         * the file descriptor (for tap, for example).
+         */
+        qemu_notify_event();
+    }
 }
 
 static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender,
diff --git a/net/queue.c b/net/queue.c
index e8030aa..6e64091 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -228,7 +228,7 @@ void qemu_net_queue_purge(NetQueue *queue, NetClientState *from)
     }
 }
 
-void qemu_net_queue_flush(NetQueue *queue)
+bool qemu_net_queue_flush(NetQueue *queue)
 {
     while (!QTAILQ_EMPTY(&queue->packets)) {
         NetPacket *packet;
@@ -244,7 +244,7 @@ void qemu_net_queue_flush(NetQueue *queue)
                                      packet->size);
         if (ret == 0) {
             QTAILQ_INSERT_HEAD(&queue->packets, packet, entry);
-            break;
+            return false;
         }
 
         if (packet->sent_cb) {
@@ -253,4 +253,5 @@ void qemu_net_queue_flush(NetQueue *queue)
 
         g_free(packet);
     }
+    return true;
 }
diff --git a/net/queue.h b/net/queue.h
index 9d44a9b..fc02b33 100644
--- a/net/queue.h
+++ b/net/queue.h
@@ -53,6 +53,6 @@ ssize_t qemu_net_queue_send_iov(NetQueue *queue,
                                 NetPacketSent *sent_cb);
 
 void qemu_net_queue_purge(NetQueue *queue, NetClientState *from);
-void qemu_net_queue_flush(NetQueue *queue);
+bool qemu_net_queue_flush(NetQueue *queue);
 
 #endif /* QEMU_NET_QUEUE_H */
-- 
1.7.10.4

[Qemu-devel] [PATCH 2/3] e1000: flush queue whenever can_receive can go from false to true

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

When the guests replenish the receive ring buffer, the network device
should flush its queue of pending packets.  This is done with
qemu_flush_queued_packets.

e1000's can_receive can go from false to true when RCTL or RDT are
modified.

Reported-by: Luigi Rizzo <rizzo@iet.unipi.it>
Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Cc: Jan Kiszka <jan.kiszka@siemens.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 hw/e1000.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/e1000.c b/hw/e1000.c
index ae8a6c5..ec3a7c4 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -295,6 +295,7 @@ set_rx_control(E1000State *s, int index, uint32_t val)
     s->rxbuf_min_shift = ((val / E1000_RCTL_RDMTS_QUAT) & 3) + 1;
     DBGOUT(RX, "RCTL: %d, mac_reg[RCTL] = 0x%x\n", s->mac_reg[RDT],
            s->mac_reg[RCTL]);
+    qemu_flush_queued_packets(&s->nic->nc);
 }
 
 static void
@@ -926,6 +927,9 @@ set_rdt(E1000State *s, int index, uint32_t val)
 {
     s->check_rxov = 0;
     s->mac_reg[index] = val & 0xffff;
+    if (e1000_has_rxbufs(s, 1)) {
+        qemu_flush_queued_packets(&s->nic->nc);
+    }
 }
 
 static void
-- 
1.7.10.4

[Qemu-devel] [PATCH 3/3] xen: flush queue when getting an event

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

xen does not have a register that, when written, will cause can_receive
to go from false to true.  However, flushing the queue can be attempted
whenever the front-end raises its side of the Xen event channel.  There
is a single event channel for tx and rx.

Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 hw/xen_nic.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/xen_nic.c b/hw/xen_nic.c
index 8b79bfb..cf7d559 100644
--- a/hw/xen_nic.c
+++ b/hw/xen_nic.c
@@ -415,6 +415,7 @@ static void net_event(struct XenDevice *xendev)
 {
     struct XenNetDev *netdev = container_of(xendev, struct XenNetDev, xendev);
     net_tx_packets(netdev);
+    qemu_flush_queued_packets(&netdev->nic->nc);
 }
 
 static int net_free(struct XenDevice *xendev)
-- 
1.7.10.4

[Qemu-devel] [PATCH 0/3] net: add missing queue flush for e1000 and xen

[Paolo Bonzini]

Luigi reminded me of these patches...

When the guests replenish the receive ring buffer, the network device
should flush its queue of pending packets.  This is done with
qemu_flush_queued_packets, and patches 2+3 add the missing call to
two drivers, e1000 and xen.  More may come later---no time to test
them now.

However, the device should not just retry delivery of packets that were
already read from the tap device, it should also try to read more packets
from the tap device.  The latter requires a qemu_notify_event to force
recomputation of the fd_sets.  virtio already does this, but it is a
layering violation; patch 1 moves the call from virtio to the network
subsystem, so that e1000 and xen will then get it for free.

Paolo Bonzini (3):
  net: notify iothread after flushing queue
  e1000: flush queue whenever can_receive can go from false to true
  xen: flush queue when getting an event

 hw/e1000.c              |  4 ++++
 hw/virtio-net.c         |  4 ----
 hw/xen_nic.c            |  1 +
 net.c                   |  7 ++++++-
 net/queue.c             |  5 +++--
 net/queue.h             |  2 +-
 6 file modificati, 15 inserzioni(+), 8 rimozioni(-)

-- 
1.7.11.2

[Qemu-devel] [PATCH 1/3] net: notify iothread after flushing queue

[Paolo Bonzini]

virtio-net has code to flush the queue and notify the iothread
whenever new receive buffers are added by the guest.  That is
fine, and indeed we need to do the same in all other drivers.
However, notifying the iothread should be work for the network
subsystem.  And since we are at it we can add a little smartness:
if some of the queued packets already could not be delivered,
there is no need to notify the iothread.

Reported-by: Luigi Rizzo <rizzo@iet.unipi.it>
Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Cc: Jan Kiszka <jan.kiszka@siemens.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio-net.c | 4 ----
 net.c           | 7 ++++++-
 net/queue.c     | 5 +++--
 net/queue.h     | 2 +-
 4 file modificati, 10 inserzioni(+), 8 rimozioni(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index b1998b2..6490743 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -447,10 +447,6 @@ static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
     VirtIONet *n = to_virtio_net(vdev);
 
     qemu_flush_queued_packets(&n->nic->nc);
-
-    /* We now have RX buffers, signal to the IO thread to break out of the
-     * select to re-poll the tap file descriptor */
-    qemu_notify_event();
 }
 
 static int virtio_net_can_receive(NetClientState *nc)
diff --git a/net.c b/net.c
index 60043dd..76a8336 100644
--- a/net.c
+++ b/net.c
@@ -357,7 +357,12 @@ void qemu_flush_queued_packets(NetClientState *nc)
 {
     nc->receive_disabled = 0;
 
-    qemu_net_queue_flush(nc->send_queue);
+    if (qemu_net_queue_flush(nc->send_queue)) {
+        /* We emptied the queue successfully, signal to the IO thread to repoll
+         * the file descriptor (for tap, for example).
+         */
+        qemu_notify_event();
+    }
 }
 
 static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender,
diff --git a/net/queue.c b/net/queue.c
index e8030aa..f3f0098 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -228,7 +228,7 @@ void qemu_net_queue_purge(NetQueue *queue, NetClientState *from)
     }
 }
 
-void qemu_net_queue_flush(NetQueue *queue)
+bool qemu_net_queue_flush(NetQueue *queue)
 {
     while (!QTAILQ_EMPTY(&queue->packets)) {
         NetPacket *packet;
@@ -244,7 +244,7 @@ void qemu_net_queue_flush(NetQueue *queue)
                                      packet->size);
         if (ret == 0) {
             QTAILQ_INSERT_HEAD(&queue->packets, packet, entry);
-            break;
+            return 0;
         }
 
         if (packet->sent_cb) {
@@ -253,4 +253,5 @@ void qemu_net_queue_flush(NetQueue *queue)
 
         g_free(packet);
     }
+    return 1;
 }
diff --git a/net/queue.h b/net/queue.h
index 9d44a9b..fc02b33 100644
--- a/net/queue.h
+++ b/net/queue.h
@@ -53,6 +53,6 @@ ssize_t qemu_net_queue_send_iov(NetQueue *queue,
                                 NetPacketSent *sent_cb);
 
 void qemu_net_queue_purge(NetQueue *queue, NetClientState *from);
-void qemu_net_queue_flush(NetQueue *queue);
+bool qemu_net_queue_flush(NetQueue *queue);
 
 #endif /* QEMU_NET_QUEUE_H */
-- 
1.7.11.2

Re: [Qemu-devel] [PATCH 1/3] net: notify iothread after flushing queue

[Blue Swirl]

On Thu, Aug 9, 2012 at 2:45 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> virtio-net has code to flush the queue and notify the iothread
> whenever new receive buffers are added by the guest.  That is
> fine, and indeed we need to do the same in all other drivers.
> However, notifying the iothread should be work for the network
> subsystem.  And since we are at it we can add a little smartness:
> if some of the queued packets already could not be delivered,
> there is no need to notify the iothread.
>
> Reported-by: Luigi Rizzo <rizzo@iet.unipi.it>
> Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> Cc: Jan Kiszka <jan.kiszka@siemens.de>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/virtio-net.c | 4 ----
>  net.c           | 7 ++++++-
>  net/queue.c     | 5 +++--
>  net/queue.h     | 2 +-
>  4 file modificati, 10 inserzioni(+), 8 rimozioni(-)
>
> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> index b1998b2..6490743 100644
> --- a/hw/virtio-net.c
> +++ b/hw/virtio-net.c
> @@ -447,10 +447,6 @@ static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
>      VirtIONet *n = to_virtio_net(vdev);
>
>      qemu_flush_queued_packets(&n->nic->nc);
> -
> -    /* We now have RX buffers, signal to the IO thread to break out of the
> -     * select to re-poll the tap file descriptor */
> -    qemu_notify_event();
>  }
>
>  static int virtio_net_can_receive(NetClientState *nc)
> diff --git a/net.c b/net.c
> index 60043dd..76a8336 100644
> --- a/net.c
> +++ b/net.c
> @@ -357,7 +357,12 @@ void qemu_flush_queued_packets(NetClientState *nc)
>  {
>      nc->receive_disabled = 0;
>
> -    qemu_net_queue_flush(nc->send_queue);
> +    if (qemu_net_queue_flush(nc->send_queue)) {
> +        /* We emptied the queue successfully, signal to the IO thread to repoll
> +         * the file descriptor (for tap, for example).
> +         */
> +        qemu_notify_event();
> +    }
>  }
>
>  static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender,
> diff --git a/net/queue.c b/net/queue.c
> index e8030aa..f3f0098 100644
> --- a/net/queue.c
> +++ b/net/queue.c
> @@ -228,7 +228,7 @@ void qemu_net_queue_purge(NetQueue *queue, NetClientState *from)
>      }
>  }
>
> -void qemu_net_queue_flush(NetQueue *queue)
> +bool qemu_net_queue_flush(NetQueue *queue)
>  {
>      while (!QTAILQ_EMPTY(&queue->packets)) {
>          NetPacket *packet;
> @@ -244,7 +244,7 @@ void qemu_net_queue_flush(NetQueue *queue)
>                                       packet->size);
>          if (ret == 0) {
>              QTAILQ_INSERT_HEAD(&queue->packets, packet, entry);
> -            break;
> +            return 0;

return false;

>          }
>
>          if (packet->sent_cb) {
> @@ -253,4 +253,5 @@ void qemu_net_queue_flush(NetQueue *queue)
>
>          g_free(packet);
>      }
> +    return 1;

return true;

>  }
> diff --git a/net/queue.h b/net/queue.h
> index 9d44a9b..fc02b33 100644
> --- a/net/queue.h
> +++ b/net/queue.h
> @@ -53,6 +53,6 @@ ssize_t qemu_net_queue_send_iov(NetQueue *queue,
>                                  NetPacketSent *sent_cb);
>
>  void qemu_net_queue_purge(NetQueue *queue, NetClientState *from);
> -void qemu_net_queue_flush(NetQueue *queue);
> +bool qemu_net_queue_flush(NetQueue *queue);
>
>  #endif /* QEMU_NET_QUEUE_H */
> --
> 1.7.11.2
>
>
>

Re: [Qemu-devel] [PATCH 1/3] net: notify iothread after flushing queue

[Stefan Hajnoczi]

On Thu, Aug 09, 2012 at 05:55:50PM +0000, Blue Swirl wrote:
> On Thu, Aug 9, 2012 at 2:45 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > @@ -244,7 +244,7 @@ void qemu_net_queue_flush(NetQueue *queue)
> >                                       packet->size);
> >          if (ret == 0) {
> >              QTAILQ_INSERT_HEAD(&queue->packets, packet, entry);
> > -            break;
> > +            return 0;
> 
> return false;
> 
> >          }
> >
> >          if (packet->sent_cb) {
> > @@ -253,4 +253,5 @@ void qemu_net_queue_flush(NetQueue *queue)
> >
> >          g_free(packet);
> >      }
> > +    return 1;
> 
> return true;

Will fix these up when merging.  I think it's clearer to use bool too.

Stefan