Re: [Qemu-devel] e1000, virtio_net: Check link status in can_receive

[Mark McLoughlin]

On Sun, 2009-04-12 at 05:17 -0400, Yan Vugenfirer wrote:

> From: Yan Vugenfirer <address@hidden>
> Date: Sun, 5 Apr 2009 10:20:53 -0400
> Subject: [PATCH 1/1] e1000, virtio_net: Check link status in can_receive
> 
> Fixing the bug of 100% cpu usage by qemu after using "set_link <NIC> down" 
> monitor command. The fix is for virtio_net and for e1000 emulations.
> 
> ---
>  hw/e1000.c      |    3 ++-
>  hw/virtio-net.c |    3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/e1000.c b/hw/e1000.c
> index 1644201..36878d9 100644
> --- a/hw/e1000.c
> +++ b/hw/e1000.c
> @@ -590,7 +590,8 @@ e1000_can_receive(void *opaque)
>  {
>      E1000State *s = opaque;
>  
> -    return (s->mac_reg[RCTL] & E1000_RCTL_EN);
> +    return (s->mac_reg[RCTL] & E1000_RCTL_EN) &&
> +           (s->mac_reg[STATUS] & E1000_STATUS_LU);
>  }
>  
>  static void
> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> index ad55bb7..dcd18c1 100644
> --- a/hw/virtio-net.c
> +++ b/hw/virtio-net.c
> @@ -259,7 +259,8 @@ static void virtio_net_handle_rx(VirtIODevice *vdev, 
> VirtQueue *vq)
>  static int do_virtio_net_can_receive(VirtIONet *n, int bufsize)
>  {
>      if (!virtio_queue_ready(n->rx_vq) ||
> -        !(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK))
> +        !(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK) ||
> +        !(n->status & VIRTIO_NET_S_LINK_UP))
>          return 0;
>  
>      if (virtio_queue_empty(n->rx_vq) ||

Firstly, I think this "100% CPU" bug is specific to kvm - we have some
code in kvm's tree to buffer tap packets which causes this, I think. I
have some qemu patches in the works to remove this difference between
the trees.

Your patch does make some sense, but I think what we really want is for
qemu_send_packet() to drop the tap packet when virtio/e1000 is down.
This is the behaviour we have in the other direction. Does the patch
below fix the problem for you?

Thanks,
Mark.

diff --git a/qemu/net.c b/qemu/net.c
index 4d07905..95fd808 100644
--- a/qemu/net.c
+++ b/qemu/net.c
@@ -409,8 +409,10 @@ int qemu_send_packet(VLANClientState *vc1, const uint8_t 
*buf, int size)
     hex_dump(stdout, buf, size);
 #endif
     for(vc = vlan->first_client; vc != NULL; vc = vc->next) {
-        if (vc != vc1 && !vc->link_down) {
-            if (!vc->fd_can_read || vc->fd_can_read(vc->opaque)) {
+        if (vc != vc1) {
+            if (vc->link_down)
+                ret = 0;
+            else if (!vc->fd_can_read || vc->fd_can_read(vc->opaque)) {
                 vc->fd_read(vc->opaque, buf, size);
                 ret = 0;
             }