From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LuOYY-00046W-E6 for qemu-devel@nongnu.org; Thu, 16 Apr 2009 06:10:54 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LuOYU-00044q-Ew for qemu-devel@nongnu.org; Thu, 16 Apr 2009 06:10:53 -0400 Received: from [199.232.76.173] (port=43238 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LuOYT-00044c-Qp for qemu-devel@nongnu.org; Thu, 16 Apr 2009 06:10:50 -0400 Received: from mx1.redhat.com ([66.187.233.31]:50482) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LuOYT-0005bM-El for qemu-devel@nongnu.org; Thu, 16 Apr 2009 06:10:49 -0400 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n3GAAmFl018305 for ; Thu, 16 Apr 2009 06:10:48 -0400 From: "Yan Vugenfirer" References: <009201c9bba3$5addabb0$10990310$@com> <1239697089.18289.64.camel@blaa> In-Reply-To: <1239697089.18289.64.camel@blaa> Subject: RE: [Qemu-devel] e1000, virtio_net: Check link status in can_receive Date: Thu, 16 Apr 2009 06:10:47 -0400 (EDT) Message-ID: <008801c9becf$6c5187c0$44f49740$@com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Language: en-us Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: 'Mark McLoughlin' , qemu-devel@nongnu.org Cc: dlaor@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 > 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 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; }