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

["Yan Vugenfirer" <yvugenfi@redhat.com>]

Hi Mark,

The patch for qemu_send_packet() solves the problem too. 
It looks like a better place for a fix.

Thanks,
Yan.

-----Original Message-----
From: qemu-devel-bounces+yvugenfi=redhat.com@nongnu.org
[mailto:qemu-devel-bounces+yvugenfi=redhat.com@nongnu.org] On Behalf Of
Mark McLoughlin
Sent: Tuesday, April 14, 2009 1:18 AM
To: qemu-devel@nongnu.org
Cc: dlaor@redhat.com
Subject: Re: [Qemu-devel] e1000, virtio_net: Check link status in
can_receive

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

> From: Yan Vugenfirer <yvugenfi@redhat.com>
> 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;
             }