qemu-kvm-0.11 regression, crashes on older guests with virtio network

qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Dustin Kirkland]

I believe that we have identified a regression in qemu-kvm-0.11.0.

The kvm process crashes for older guests with virtio networking, when
the guest's incoming network connection is saturated.  The subject
guest is Ubuntu 8.04 Hardy, 2.6.24 kernel with virtio backports.

For your convenience, I have uploaded a stock, ~260MB Ubuntu 8.04 disk
image (user/pw = ubuntu) that I'm using to reproduce the problem at:
 * http://rookery.canonical.com/~kirkland/hardy.img.bz2

The command line is:
 * sudo /usr/bin/kvm -m 512 -net nic,model=virtio -net
tap,script=/home/kirkland/bin/bridge.sh -drive
file=hardy.img,if=virtio,index=0,boot=on

The bridge script is:
 * http://rookery.canonical.com/~kirkland/bridge.sh

I'm saturating the guest's incoming network connection, with:
  user@guest$ nc -lp 1234 > /dev/null
and
  user@host$ cat /dev/urandom | nc -w 3 guest_ip 1234

In less than a second of sending, the vm crashes with:
  virtio-net truncating packet

I have strace logs, if that's helpful.

I have not reproduced the problem:
 a) by saturating the guest's outgoing network
 b) with newer guests ( >= 2.6.27 )
 c) on kvm-84 on the host

We're tracking this issue at:
 * https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/458521

I'll gladly review and test patches, or take pointers on where I might
look to solve this issue.

-- 
:-Dustin

[Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Dustin Kirkland]

I believe that we have identified a regression in qemu-kvm-0.11.0.

The kvm process crashes for older guests with virtio networking, when
the guest's incoming network connection is saturated.  The subject
guest is Ubuntu 8.04 Hardy, 2.6.24 kernel with virtio backports.

For your convenience, I have uploaded a stock, ~260MB Ubuntu 8.04 disk
image (user/pw = ubuntu) that I'm using to reproduce the problem at:
 * http://rookery.canonical.com/~kirkland/hardy.img.bz2

The command line is:
 * sudo /usr/bin/kvm -m 512 -net nic,model=virtio -net
tap,script=/home/kirkland/bin/bridge.sh -drive
file=hardy.img,if=virtio,index=0,boot=on

The bridge script is:
 * http://rookery.canonical.com/~kirkland/bridge.sh

I'm saturating the guest's incoming network connection, with:
  user@guest$ nc -lp 1234 > /dev/null
and
  user@host$ cat /dev/urandom | nc -w 3 guest_ip 1234

In less than a second of sending, the vm crashes with:
  virtio-net truncating packet

I have strace logs, if that's helpful.

I have not reproduced the problem:
 a) by saturating the guest's outgoing network
 b) with newer guests ( >= 2.6.27 )
 c) on kvm-84 on the host

We're tracking this issue at:
 * https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/458521

I'll gladly review and test patches, or take pointers on where I might
look to solve this issue.

-- 
:-Dustin

Re: qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Dustin Kirkland]

On Wed, Oct 28, 2009 at 2:22 PM, Dustin Kirkland <kirkland@canonical.com> wrote:
> I have not reproduced the problem:
>  a) by saturating the guest's outgoing network
>  b) with newer guests ( >= 2.6.27 )
>  c) on kvm-84 on the host

 d) or by using e1000, or rtl8139 NIC models.

:-Dustin

[Qemu-devel] Re: qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Dustin Kirkland]

On Wed, Oct 28, 2009 at 2:22 PM, Dustin Kirkland <kirkland@canonical.com> wrote:
> I have not reproduced the problem:
>  a) by saturating the guest's outgoing network
>  b) with newer guests ( >= 2.6.27 )
>  c) on kvm-84 on the host

 d) or by using e1000, or rtl8139 NIC models.

:-Dustin

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Scott Tsai]

Excerpts from Dustin Kirkland's message of Thu Oct 29 03:22:43 +0800 2009:
> We're tracking this issue at:
>  * https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/458521
> 
> I'll gladly review and test patches, or take pointers on where I might
> look to solve this issue.

Try the following patch against the stable-0.11 branch.
I've only just started learning about the virtio-net code but hopefully this patch points you
to the right direction.

Note that this patch just drops the packets that would have caused virtio-net 
to call exit(1).

>From d48af0377f359983bff67eb9296ba040def401ec Mon Sep 17 00:00:00 2001
From: Scott Tsai <scottt.tw@gmail.com>
Date: Thu, 29 Oct 2009 10:56:12 +0800
Subject: [PATCH] virtio-net: drop large packets when no mergable_rx_bufs

see: https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/458521
---
 hw/virtio-net.c |    8 +++++++-
 hw/virtio.c     |   22 ++++++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index ce8e6cb..2e6725b 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -502,6 +502,8 @@ static int receive_filter(VirtIONet *n, const uint8_t *buf, int size)
     return 0;
 }
 
+int buffer_fits_in_virtqueue_top(VirtQueue *vq, int size);
+
 static ssize_t virtio_net_receive2(VLANClientState *vc, const uint8_t *buf, size_t size, int raw)
 {
     VirtIONet *n = vc->opaque;
@@ -518,6 +520,10 @@ static ssize_t virtio_net_receive2(VLANClientState *vc, const uint8_t *buf, size
     hdr_len = n->mergeable_rx_bufs ?
         sizeof(struct virtio_net_hdr_mrg_rxbuf) : sizeof(struct virtio_net_hdr);
 
+    /* drop packet instead of truncating it */
+    if (!n->mergeable_rx_bufs && !buffer_fits_in_virtqueue_top(n->rx_vq, hdr_len + size))
+        return;
+
     offset = i = 0;
 
     while (offset < size) {
@@ -531,7 +537,7 @@ static ssize_t virtio_net_receive2(VLANClientState *vc, const uint8_t *buf, size
             virtqueue_pop(n->rx_vq, &elem) == 0) {
             if (i == 0)
                 return -1;
-            fprintf(stderr, "virtio-net truncating packet\n");
+            fprintf(stderr, "virtio-net truncating packet: mergable_rx_bufs: %d\n", n->mergeable_rx_bufs);
             exit(1);
         }
 
diff --git a/hw/virtio.c b/hw/virtio.c
index 41e7ca2..d6f5a12 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -356,6 +356,28 @@ int virtqueue_avail_bytes(VirtQueue *vq, int in_bytes, int out_bytes)
     return 0;
 }
 
+/* buffer_fits_in_virtqueue_top: returns true if a 'size' byte buffer could fit in the
+ * input descriptors that virtqueue_pop() would have returned
+ */
+int buffer_fits_in_virtqueue_top(VirtQueue *vq, int size);
+
+int buffer_fits_in_virtqueue_top(VirtQueue *vq, int size)
+{
+    unsigned int i;
+    int input_iov_len_sum;
+
+    if (!virtqueue_num_heads(vq, vq->last_avail_idx))
+        return 0;
+
+    input_iov_len_sum = 0;
+    i = virtqueue_get_head(vq, vq->last_avail_idx);
+    do {
+        if (vring_desc_flags(vq, i) & VRING_DESC_F_WRITE)
+            input_iov_len_sum += vring_desc_len(vq, i);
+    } while ((i = virtqueue_next_desc(vq, i)) != vq->vring.num);
+    return input_iov_len_sum >= size;
+}
+
 int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
 {
     unsigned int i, head, max;
-- 
1.6.2.5

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Scott Tsai]

Excerpts from Dustin Kirkland's message of Thu Oct 29 03:22:43 +0800 2009:
> We're tracking this issue at:
>  * https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/458521
> 
> I'll gladly review and test patches, or take pointers on where I might
> look to solve this issue.

Try the following patch against the stable-0.11 branch.
I've only just started learning about the virtio-net code but hopefully this patch points you
to the right direction.

Note that this patch just drops the packets that would have caused virtio-net 
to call exit(1).

>From d48af0377f359983bff67eb9296ba040def401ec Mon Sep 17 00:00:00 2001
From: Scott Tsai <scottt.tw@gmail.com>
Date: Thu, 29 Oct 2009 10:56:12 +0800
Subject: [PATCH] virtio-net: drop large packets when no mergable_rx_bufs

see: https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/458521
---
 hw/virtio-net.c |    8 +++++++-
 hw/virtio.c     |   22 ++++++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index ce8e6cb..2e6725b 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -502,6 +502,8 @@ static int receive_filter(VirtIONet *n, const uint8_t *buf, int size)
     return 0;
 }
 
+int buffer_fits_in_virtqueue_top(VirtQueue *vq, int size);
+
 static ssize_t virtio_net_receive2(VLANClientState *vc, const uint8_t *buf, size_t size, int raw)
 {
     VirtIONet *n = vc->opaque;
@@ -518,6 +520,10 @@ static ssize_t virtio_net_receive2(VLANClientState *vc, const uint8_t *buf, size
     hdr_len = n->mergeable_rx_bufs ?
         sizeof(struct virtio_net_hdr_mrg_rxbuf) : sizeof(struct virtio_net_hdr);
 
+    /* drop packet instead of truncating it */
+    if (!n->mergeable_rx_bufs && !buffer_fits_in_virtqueue_top(n->rx_vq, hdr_len + size))
+        return;
+
     offset = i = 0;
 
     while (offset < size) {
@@ -531,7 +537,7 @@ static ssize_t virtio_net_receive2(VLANClientState *vc, const uint8_t *buf, size
             virtqueue_pop(n->rx_vq, &elem) == 0) {
             if (i == 0)
                 return -1;
-            fprintf(stderr, "virtio-net truncating packet\n");
+            fprintf(stderr, "virtio-net truncating packet: mergable_rx_bufs: %d\n", n->mergeable_rx_bufs);
             exit(1);
         }
 
diff --git a/hw/virtio.c b/hw/virtio.c
index 41e7ca2..d6f5a12 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -356,6 +356,28 @@ int virtqueue_avail_bytes(VirtQueue *vq, int in_bytes, int out_bytes)
     return 0;
 }
 
+/* buffer_fits_in_virtqueue_top: returns true if a 'size' byte buffer could fit in the
+ * input descriptors that virtqueue_pop() would have returned
+ */
+int buffer_fits_in_virtqueue_top(VirtQueue *vq, int size);
+
+int buffer_fits_in_virtqueue_top(VirtQueue *vq, int size)
+{
+    unsigned int i;
+    int input_iov_len_sum;
+
+    if (!virtqueue_num_heads(vq, vq->last_avail_idx))
+        return 0;
+
+    input_iov_len_sum = 0;
+    i = virtqueue_get_head(vq, vq->last_avail_idx);
+    do {
+        if (vring_desc_flags(vq, i) & VRING_DESC_F_WRITE)
+            input_iov_len_sum += vring_desc_len(vq, i);
+    } while ((i = virtqueue_next_desc(vq, i)) != vq->vring.num);
+    return input_iov_len_sum >= size;
+}
+
 int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
 {
     unsigned int i, head, max;
-- 
1.6.2.5

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Mark McLoughlin]

Hi Dustin,

On Wed, 2009-10-28 at 14:22 -0500, Dustin Kirkland wrote:
> I believe that we have identified a regression in qemu-kvm-0.11.0.

Regression versus which previous version of qemu-kvm?

> The kvm process crashes for older guests with virtio networking, when
> the guest's incoming network connection is saturated.  The subject
> guest is Ubuntu 8.04 Hardy, 2.6.24 kernel with virtio backports.

It'd be good to see your virtio_net code

e.g. it should not have the VIRTIO_NET_F_GUEST_TSO4 feature set if it
doesn't have the "big_packets" code in try_fill_recv()

> For your convenience, I have uploaded a stock, ~260MB Ubuntu 8.04 disk
> image (user/pw = ubuntu) that I'm using to reproduce the problem at:
>  * http://rookery.canonical.com/~kirkland/hardy.img.bz2
> 
> The command line is:
>  * sudo /usr/bin/kvm -m 512 -net nic,model=virtio -net
> tap,script=/home/kirkland/bin/bridge.sh -drive
> file=hardy.img,if=virtio,index=0,boot=on
> 
> The bridge script is:
>  * http://rookery.canonical.com/~kirkland/bridge.sh
> 
> I'm saturating the guest's incoming network connection, with:
>   user@guest$ nc -lp 1234 > /dev/null
> and
>   user@host$ cat /dev/urandom | nc -w 3 guest_ip 1234
> 
> In less than a second of sending, the vm crashes with:
>   virtio-net truncating packet

Assuming this is something like the virtio-net in 2.6.26, there was no
receivable buffers support so (as Scott points out) it must be that
we've read a packet from the tap device which is >1514 bytes (or >1524
bytes with IFF_VNET_HDR) but the guest has not supplied buffers which
are large enough to take it

One thing to check is that the tap device is being initialized by
qemu-kvm using TUNSETOFFLOAD with either zero or TUN_F_CSUM - i.e. GSO
should not be enabled, because the guest cannot handle large GSO packets

Another possibility is that the MTU on the bridge in the host is too
large and that's what's causing the large packets to be sent

I agree we shouldn't exit in this scenario, but let's figure out what's
causing it to occur

Cheers,
Mark.

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Mark McLoughlin]

Hi Dustin,

On Wed, 2009-10-28 at 14:22 -0500, Dustin Kirkland wrote:
> I believe that we have identified a regression in qemu-kvm-0.11.0.

Regression versus which previous version of qemu-kvm?

> The kvm process crashes for older guests with virtio networking, when
> the guest's incoming network connection is saturated.  The subject
> guest is Ubuntu 8.04 Hardy, 2.6.24 kernel with virtio backports.

It'd be good to see your virtio_net code

e.g. it should not have the VIRTIO_NET_F_GUEST_TSO4 feature set if it
doesn't have the "big_packets" code in try_fill_recv()

> For your convenience, I have uploaded a stock, ~260MB Ubuntu 8.04 disk
> image (user/pw = ubuntu) that I'm using to reproduce the problem at:
>  * http://rookery.canonical.com/~kirkland/hardy.img.bz2
> 
> The command line is:
>  * sudo /usr/bin/kvm -m 512 -net nic,model=virtio -net
> tap,script=/home/kirkland/bin/bridge.sh -drive
> file=hardy.img,if=virtio,index=0,boot=on
> 
> The bridge script is:
>  * http://rookery.canonical.com/~kirkland/bridge.sh
> 
> I'm saturating the guest's incoming network connection, with:
>   user@guest$ nc -lp 1234 > /dev/null
> and
>   user@host$ cat /dev/urandom | nc -w 3 guest_ip 1234
> 
> In less than a second of sending, the vm crashes with:
>   virtio-net truncating packet

Assuming this is something like the virtio-net in 2.6.26, there was no
receivable buffers support so (as Scott points out) it must be that
we've read a packet from the tap device which is >1514 bytes (or >1524
bytes with IFF_VNET_HDR) but the guest has not supplied buffers which
are large enough to take it

One thing to check is that the tap device is being initialized by
qemu-kvm using TUNSETOFFLOAD with either zero or TUN_F_CSUM - i.e. GSO
should not be enabled, because the guest cannot handle large GSO packets

Another possibility is that the MTU on the bridge in the host is too
large and that's what's causing the large packets to be sent

I agree we shouldn't exit in this scenario, but let's figure out what's
causing it to occur

Cheers,
Mark.

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Scott Tsai]

Excerpts from Mark McLoughlin's message of Thu Oct 29 17:16:43 +0800 2009:
> Assuming this is something like the virtio-net in 2.6.26, there was no
> receivable buffers support so (as Scott points out) it must be that
> we've read a packet from the tap device which is >1514 bytes (or >1524
> bytes with IFF_VNET_HDR) but the guest has not supplied buffers which
> are large enough to take it

> One thing to check is that the tap device is being initialized by
> qemu-kvm using TUNSETOFFLOAD with either zero or TUN_F_CSUM - i.e. GSO
> should not be enabled, because the guest cannot handle large GSO packets

> Another possibility is that the MTU on the bridge in the host is too
> large and that's what's causing the large packets to be sent

Using Dustin's image, I see:
	virtio_net_set_features(features: 0x00000930)
	tap_set_offload(csum: 1, tso4: 1, tso6: 1, ecn: 1)
being called and get an mtu of 1500 on virbr0 using his birdge.sh script.

virtio_net_receive2 was trying to transfer a 1534 byte packet (1524 'size' + 10 'virtio_net_hdr')
and the guest only had 1524 bytes of space in its input descriptors.

BTW, I can also reproduce this running Dustin's image inside Fedora 11's qemu-0.10.6-9.fc11.x86_64.

The patch I posted earlier actually only applies to the 0.10 branch, here's a patch that compiles for 0.11:

>From 06aa7db0705cf747c35cbcbd09d0e37713f16fe4 Mon Sep 17 00:00:00 2001
From: Scott Tsai <scottt.tw@gmail.com>
Date: Thu, 29 Oct 2009 10:56:12 +0800
Subject: [PATCH] virtio-net: drop large packets when no mergable_rx_bufs

Currently virtio-net calls exit(1) when it receives a large packet and
the VIRTIO_NET_F_MRG_RXBUF feature isn't set.
Change it to drop the packet instead.

see: https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/458521
---
 hw/virtio-net.c |    8 +++++++-
 hw/virtio.c     |   33 +++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index ce8e6cb..2e6725b 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -502,6 +502,8 @@ static int receive_filter(VirtIONet *n, const uint8_t *buf, int size)
     return 0;
 }
 
+int buffer_fits_in_virtqueue_top(VirtQueue *vq, int size);
+
 static ssize_t virtio_net_receive2(VLANClientState *vc, const uint8_t *buf, size_t size, int raw)
 {
     VirtIONet *n = vc->opaque;
@@ -518,6 +520,10 @@ static ssize_t virtio_net_receive2(VLANClientState *vc, const uint8_t *buf, size
     hdr_len = n->mergeable_rx_bufs ?
         sizeof(struct virtio_net_hdr_mrg_rxbuf) : sizeof(struct virtio_net_hdr);
 
+    /* drop packet instead of truncating it */
+    if (!n->mergeable_rx_bufs && !buffer_fits_in_virtqueue_top(n->rx_vq, hdr_len + size))
+        return;
+
     offset = i = 0;
 
     while (offset < size) {
@@ -531,7 +537,7 @@ static ssize_t virtio_net_receive2(VLANClientState *vc, const uint8_t *buf, size
             virtqueue_pop(n->rx_vq, &elem) == 0) {
             if (i == 0)
                 return -1;
-            fprintf(stderr, "virtio-net truncating packet\n");
+            fprintf(stderr, "virtio-net truncating packet: mergable_rx_bufs: %d\n", n->mergeable_rx_bufs);
             exit(1);
         }
 
diff --git a/hw/virtio.c b/hw/virtio.c
index 41e7ca2..d9e0353 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -356,6 +356,39 @@ int virtqueue_avail_bytes(VirtQueue *vq, int in_bytes, int out_bytes)
     return 0;
 }
 
+/* buffer_fits_in_virtqueue_top: returns true if a 'size' byte buffer could fit in the
+ * input descriptors that virtqueue_pop() would have returned
+ */
+int buffer_fits_in_virtqueue_top(VirtQueue *vq, int size);
+
+int buffer_fits_in_virtqueue_top(VirtQueue *vq, int size)
+{
+    unsigned int i, max;
+    int input_iov_len_sum;
+    target_phys_addr_t desc_pa;
+
+    if (!virtqueue_num_heads(vq, vq->last_avail_idx))
+        return 0;
+
+    desc_pa = vq->vring.desc;
+    max = vq->vring.num;
+    i = virtqueue_get_head(vq, vq->last_avail_idx);
+
+    if (vring_desc_flags(desc_pa, i) & VRING_DESC_F_INDIRECT) {
+        /* loop over the indirect descriptor table */
+        max = vring_desc_len(desc_pa, i) / sizeof(VRingDesc);
+        desc_pa = vring_desc_addr(desc_pa, i);
+        i = 0;
+    }
+
+    input_iov_len_sum = 0;
+    do {
+        if (vring_desc_flags(desc_pa, i) & VRING_DESC_F_WRITE)
+            input_iov_len_sum += vring_desc_len(desc_pa, i);
+    } while ((i = virtqueue_next_desc(desc_pa, i, max)) != vq->vring.num);
+    return input_iov_len_sum >= size;
+}
+
 int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
 {
     unsigned int i, head, max;
-- 
1.6.2.5

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Scott Tsai]

Excerpts from Mark McLoughlin's message of Thu Oct 29 17:16:43 +0800 2009:
> Assuming this is something like the virtio-net in 2.6.26, there was no
> receivable buffers support so (as Scott points out) it must be that
> we've read a packet from the tap device which is >1514 bytes (or >1524
> bytes with IFF_VNET_HDR) but the guest has not supplied buffers which
> are large enough to take it

> One thing to check is that the tap device is being initialized by
> qemu-kvm using TUNSETOFFLOAD with either zero or TUN_F_CSUM - i.e. GSO
> should not be enabled, because the guest cannot handle large GSO packets

> Another possibility is that the MTU on the bridge in the host is too
> large and that's what's causing the large packets to be sent

Using Dustin's image, I see:
	virtio_net_set_features(features: 0x00000930)
	tap_set_offload(csum: 1, tso4: 1, tso6: 1, ecn: 1)
being called and get an mtu of 1500 on virbr0 using his birdge.sh script.

virtio_net_receive2 was trying to transfer a 1534 byte packet (1524 'size' + 10 'virtio_net_hdr')
and the guest only had 1524 bytes of space in its input descriptors.

BTW, I can also reproduce this running Dustin's image inside Fedora 11's qemu-0.10.6-9.fc11.x86_64.

The patch I posted earlier actually only applies to the 0.10 branch, here's a patch that compiles for 0.11:

>From 06aa7db0705cf747c35cbcbd09d0e37713f16fe4 Mon Sep 17 00:00:00 2001
From: Scott Tsai <scottt.tw@gmail.com>
Date: Thu, 29 Oct 2009 10:56:12 +0800
Subject: [PATCH] virtio-net: drop large packets when no mergable_rx_bufs

Currently virtio-net calls exit(1) when it receives a large packet and
the VIRTIO_NET_F_MRG_RXBUF feature isn't set.
Change it to drop the packet instead.

see: https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/458521
---
 hw/virtio-net.c |    8 +++++++-
 hw/virtio.c     |   33 +++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index ce8e6cb..2e6725b 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -502,6 +502,8 @@ static int receive_filter(VirtIONet *n, const uint8_t *buf, int size)
     return 0;
 }
 
+int buffer_fits_in_virtqueue_top(VirtQueue *vq, int size);
+
 static ssize_t virtio_net_receive2(VLANClientState *vc, const uint8_t *buf, size_t size, int raw)
 {
     VirtIONet *n = vc->opaque;
@@ -518,6 +520,10 @@ static ssize_t virtio_net_receive2(VLANClientState *vc, const uint8_t *buf, size
     hdr_len = n->mergeable_rx_bufs ?
         sizeof(struct virtio_net_hdr_mrg_rxbuf) : sizeof(struct virtio_net_hdr);
 
+    /* drop packet instead of truncating it */
+    if (!n->mergeable_rx_bufs && !buffer_fits_in_virtqueue_top(n->rx_vq, hdr_len + size))
+        return;
+
     offset = i = 0;
 
     while (offset < size) {
@@ -531,7 +537,7 @@ static ssize_t virtio_net_receive2(VLANClientState *vc, const uint8_t *buf, size
             virtqueue_pop(n->rx_vq, &elem) == 0) {
             if (i == 0)
                 return -1;
-            fprintf(stderr, "virtio-net truncating packet\n");
+            fprintf(stderr, "virtio-net truncating packet: mergable_rx_bufs: %d\n", n->mergeable_rx_bufs);
             exit(1);
         }
 
diff --git a/hw/virtio.c b/hw/virtio.c
index 41e7ca2..d9e0353 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -356,6 +356,39 @@ int virtqueue_avail_bytes(VirtQueue *vq, int in_bytes, int out_bytes)
     return 0;
 }
 
+/* buffer_fits_in_virtqueue_top: returns true if a 'size' byte buffer could fit in the
+ * input descriptors that virtqueue_pop() would have returned
+ */
+int buffer_fits_in_virtqueue_top(VirtQueue *vq, int size);
+
+int buffer_fits_in_virtqueue_top(VirtQueue *vq, int size)
+{
+    unsigned int i, max;
+    int input_iov_len_sum;
+    target_phys_addr_t desc_pa;
+
+    if (!virtqueue_num_heads(vq, vq->last_avail_idx))
+        return 0;
+
+    desc_pa = vq->vring.desc;
+    max = vq->vring.num;
+    i = virtqueue_get_head(vq, vq->last_avail_idx);
+
+    if (vring_desc_flags(desc_pa, i) & VRING_DESC_F_INDIRECT) {
+        /* loop over the indirect descriptor table */
+        max = vring_desc_len(desc_pa, i) / sizeof(VRingDesc);
+        desc_pa = vring_desc_addr(desc_pa, i);
+        i = 0;
+    }
+
+    input_iov_len_sum = 0;
+    do {
+        if (vring_desc_flags(desc_pa, i) & VRING_DESC_F_WRITE)
+            input_iov_len_sum += vring_desc_len(desc_pa, i);
+    } while ((i = virtqueue_next_desc(desc_pa, i, max)) != vq->vring.num);
+    return input_iov_len_sum >= size;
+}
+
 int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
 {
     unsigned int i, head, max;
-- 
1.6.2.5

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Mark McLoughlin]

On Thu, 2009-10-29 at 20:00 +0800, Scott Tsai wrote:
> Excerpts from Mark McLoughlin's message of Thu Oct 29 17:16:43 +0800 2009:
> > Assuming this is something like the virtio-net in 2.6.26, there was no
> > receivable buffers support so (as Scott points out) it must be that
> > we've read a packet from the tap device which is >1514 bytes (or >1524
> > bytes with IFF_VNET_HDR) but the guest has not supplied buffers which
> > are large enough to take it
> 
> > One thing to check is that the tap device is being initialized by
> > qemu-kvm using TUNSETOFFLOAD with either zero or TUN_F_CSUM - i.e. GSO
> > should not be enabled, because the guest cannot handle large GSO packets
> 
> > Another possibility is that the MTU on the bridge in the host is too
> > large and that's what's causing the large packets to be sent
> 
> Using Dustin's image, I see:
> 	virtio_net_set_features(features: 0x00000930)

Hmm - 0x930 doesn't seem right. Is that 930 decimal, 0x3a2 hex?

> 	tap_set_offload(csum: 1, tso4: 1, tso6: 1, ecn: 1)
> being called and get an mtu of 1500 on virbr0 using his birdge.sh script.
> 
> virtio_net_receive2 was trying to transfer a 1534 byte packet (1524 'size' + 10 'virtio_net_hdr')
> and the guest only had 1524 bytes of space in its input descriptors.

Okay, that sounds like a bug in Dustin's version of the guest virtio-net
driver - if it is only supplying 1524 byte buffers, it should not be
saying it supports the VIRTIO_NET_F_GUEST_TSO4 feature

Cheers,
Mark.

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Mark McLoughlin]

On Thu, 2009-10-29 at 20:00 +0800, Scott Tsai wrote:
> Excerpts from Mark McLoughlin's message of Thu Oct 29 17:16:43 +0800 2009:
> > Assuming this is something like the virtio-net in 2.6.26, there was no
> > receivable buffers support so (as Scott points out) it must be that
> > we've read a packet from the tap device which is >1514 bytes (or >1524
> > bytes with IFF_VNET_HDR) but the guest has not supplied buffers which
> > are large enough to take it
> 
> > One thing to check is that the tap device is being initialized by
> > qemu-kvm using TUNSETOFFLOAD with either zero or TUN_F_CSUM - i.e. GSO
> > should not be enabled, because the guest cannot handle large GSO packets
> 
> > Another possibility is that the MTU on the bridge in the host is too
> > large and that's what's causing the large packets to be sent
> 
> Using Dustin's image, I see:
> 	virtio_net_set_features(features: 0x00000930)

Hmm - 0x930 doesn't seem right. Is that 930 decimal, 0x3a2 hex?

> 	tap_set_offload(csum: 1, tso4: 1, tso6: 1, ecn: 1)
> being called and get an mtu of 1500 on virbr0 using his birdge.sh script.
> 
> virtio_net_receive2 was trying to transfer a 1534 byte packet (1524 'size' + 10 'virtio_net_hdr')
> and the guest only had 1524 bytes of space in its input descriptors.

Okay, that sounds like a bug in Dustin's version of the guest virtio-net
driver - if it is only supplying 1524 byte buffers, it should not be
saying it supports the VIRTIO_NET_F_GUEST_TSO4 feature

Cheers,
Mark.

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Scott Tsai]

> Hmm - 0x930 doesn't seem right. Is that 930 decimal, 0x3a2 hex?

yup. printf format string typo.

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Scott Tsai]

> Hmm - 0x930 doesn't seem right. Is that 930 decimal, 0x3a2 hex?

yup. printf format string typo.

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Anthony Liguori]

Mark McLoughlin wrote:
>
>> 	tap_set_offload(csum: 1, tso4: 1, tso6: 1, ecn: 1)
>> being called and get an mtu of 1500 on virbr0 using his birdge.sh script.
>>
>> virtio_net_receive2 was trying to transfer a 1534 byte packet (1524 'size' + 10 'virtio_net_hdr')
>> and the guest only had 1524 bytes of space in its input descriptors.
>>     
>
> Okay, that sounds like a bug in Dustin's version of the guest virtio-net
> driver - if it is only supplying 1524 byte buffers, it should not be
> saying it supports the VIRTIO_NET_F_GUEST_TSO4 feature
>   

See:

commit 8eca6b1bc770982595db2f7207c65051572436cb
Author: aliguori <aliguori@c046a42c-6fe2-441c-8c8c-71466251a162>
Date:   Sun Apr 5 17:40:08 2009 +0000

    Fix oops on 2.6.25 guest (Rusty Russell)
   
    I believe this is behind the following:
    https://bugs.edge.launchpad.net/ubuntu/jaunty/+source/linux/+bug/331128
   
    virtio_pci in 2.6.25 didn't do feature negotiation correctly: it 
acked every
    bit.  Fortunately, we can detect this.
   
    Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

It looks like Rusty's fix wasn't enough.  If I change virtio-net to only 
advertise F_MAC, we don't run into this problem.

Regards,

Anthony Liguori

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Anthony Liguori]

Mark McLoughlin wrote:
>
>> 	tap_set_offload(csum: 1, tso4: 1, tso6: 1, ecn: 1)
>> being called and get an mtu of 1500 on virbr0 using his birdge.sh script.
>>
>> virtio_net_receive2 was trying to transfer a 1534 byte packet (1524 'size' + 10 'virtio_net_hdr')
>> and the guest only had 1524 bytes of space in its input descriptors.
>>     
>
> Okay, that sounds like a bug in Dustin's version of the guest virtio-net
> driver - if it is only supplying 1524 byte buffers, it should not be
> saying it supports the VIRTIO_NET_F_GUEST_TSO4 feature
>   

See:

commit 8eca6b1bc770982595db2f7207c65051572436cb
Author: aliguori <aliguori@c046a42c-6fe2-441c-8c8c-71466251a162>
Date:   Sun Apr 5 17:40:08 2009 +0000

    Fix oops on 2.6.25 guest (Rusty Russell)
   
    I believe this is behind the following:
    https://bugs.edge.launchpad.net/ubuntu/jaunty/+source/linux/+bug/331128
   
    virtio_pci in 2.6.25 didn't do feature negotiation correctly: it 
acked every
    bit.  Fortunately, we can detect this.
   
    Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

It looks like Rusty's fix wasn't enough.  If I change virtio-net to only 
advertise F_MAC, we don't run into this problem.

Regards,

Anthony Liguori

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Mark McLoughlin]

On Thu, 2009-10-29 at 09:11 -0500, Anthony Liguori wrote:
> Mark McLoughlin wrote:
> >
> >> 	tap_set_offload(csum: 1, tso4: 1, tso6: 1, ecn: 1)
> >> being called and get an mtu of 1500 on virbr0 using his birdge.sh script.
> >>
> >> virtio_net_receive2 was trying to transfer a 1534 byte packet (1524 'size' + 10 'virtio_net_hdr')
> >> and the guest only had 1524 bytes of space in its input descriptors.
> >>     
> >
> > Okay, that sounds like a bug in Dustin's version of the guest virtio-net
> > driver - if it is only supplying 1524 byte buffers, it should not be
> > saying it supports the VIRTIO_NET_F_GUEST_TSO4 feature
> >   
> 
> See:
> 
> commit 8eca6b1bc770982595db2f7207c65051572436cb
> Author: aliguori <aliguori@c046a42c-6fe2-441c-8c8c-71466251a162>
> Date:   Sun Apr 5 17:40:08 2009 +0000
> 
>     Fix oops on 2.6.25 guest (Rusty Russell)
>    
>     I believe this is behind the following:
>     https://bugs.edge.launchpad.net/ubuntu/jaunty/+source/linux/+bug/331128
>    
>     virtio_pci in 2.6.25 didn't do feature negotiation correctly: it 
> acked every
>     bit.  Fortunately, we can detect this.
>    
>     Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>     Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> 
> It looks like Rusty's fix wasn't enough.  If I change virtio-net to only 
> advertise F_MAC, we don't run into this problem.

If it's not acking VBAD_FEATURE, then it doesn't sound like the same
issue

It's also not acking e.g. MRG_RXBUF, which suggests that it is
selectively acking features, and choosing to ack TSO4

A quick look through the guest driver code should clear up the
confusion. Dustion, got a pointer?

Thanks,
Mark.

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Mark McLoughlin]

On Thu, 2009-10-29 at 09:11 -0500, Anthony Liguori wrote:
> Mark McLoughlin wrote:
> >
> >> 	tap_set_offload(csum: 1, tso4: 1, tso6: 1, ecn: 1)
> >> being called and get an mtu of 1500 on virbr0 using his birdge.sh script.
> >>
> >> virtio_net_receive2 was trying to transfer a 1534 byte packet (1524 'size' + 10 'virtio_net_hdr')
> >> and the guest only had 1524 bytes of space in its input descriptors.
> >>     
> >
> > Okay, that sounds like a bug in Dustin's version of the guest virtio-net
> > driver - if it is only supplying 1524 byte buffers, it should not be
> > saying it supports the VIRTIO_NET_F_GUEST_TSO4 feature
> >   
> 
> See:
> 
> commit 8eca6b1bc770982595db2f7207c65051572436cb
> Author: aliguori <aliguori@c046a42c-6fe2-441c-8c8c-71466251a162>
> Date:   Sun Apr 5 17:40:08 2009 +0000
> 
>     Fix oops on 2.6.25 guest (Rusty Russell)
>    
>     I believe this is behind the following:
>     https://bugs.edge.launchpad.net/ubuntu/jaunty/+source/linux/+bug/331128
>    
>     virtio_pci in 2.6.25 didn't do feature negotiation correctly: it 
> acked every
>     bit.  Fortunately, we can detect this.
>    
>     Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>     Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> 
> It looks like Rusty's fix wasn't enough.  If I change virtio-net to only 
> advertise F_MAC, we don't run into this problem.

If it's not acking VBAD_FEATURE, then it doesn't sound like the same
issue

It's also not acking e.g. MRG_RXBUF, which suggests that it is
selectively acking features, and choosing to ack TSO4

A quick look through the guest driver code should clear up the
confusion. Dustion, got a pointer?

Thanks,
Mark.

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Dustin Kirkland]

[-- Attachment #1: Type: text/plain, Size: 2059 bytes --]

On Thu, 2009-10-29 at 14:25 +0000, Mark McLoughlin wrote:
> On Thu, 2009-10-29 at 09:11 -0500, Anthony Liguori wrote:
> > Mark McLoughlin wrote:
> > >
> > >> 	tap_set_offload(csum: 1, tso4: 1, tso6: 1, ecn: 1)
> > >> being called and get an mtu of 1500 on virbr0 using his birdge.sh script.
> > >>
> > >> virtio_net_receive2 was trying to transfer a 1534 byte packet (1524 'size' + 10 'virtio_net_hdr')
> > >> and the guest only had 1524 bytes of space in its input descriptors.
> > >>     
> > >
> > > Okay, that sounds like a bug in Dustin's version of the guest virtio-net
> > > driver - if it is only supplying 1524 byte buffers, it should not be
> > > saying it supports the VIRTIO_NET_F_GUEST_TSO4 feature
> > >   
> > 
> > See:
> > 
> > commit 8eca6b1bc770982595db2f7207c65051572436cb
> > Author: aliguori <aliguori@c046a42c-6fe2-441c-8c8c-71466251a162>
> > Date:   Sun Apr 5 17:40:08 2009 +0000
> > 
> >     Fix oops on 2.6.25 guest (Rusty Russell)
> >    
> >     I believe this is behind the following:
> >     https://bugs.edge.launchpad.net/ubuntu/jaunty/+source/linux/+bug/331128
> >    
> >     virtio_pci in 2.6.25 didn't do feature negotiation correctly: it 
> > acked every
> >     bit.  Fortunately, we can detect this.
> >    
> >     Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> >     Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> > 
> > It looks like Rusty's fix wasn't enough.  If I change virtio-net to only 
> > advertise F_MAC, we don't run into this problem.
> 
> If it's not acking VBAD_FEATURE, then it doesn't sound like the same
> issue
> 
> It's also not acking e.g. MRG_RXBUF, which suggests that it is
> selectively acking features, and choosing to ack TSO4
> 
> A quick look through the guest driver code should clear up the
> confusion. Dustion, got a pointer?

Hi Mark,

I'm currently testing Scott's patch above.

In the mean time, Hardy's kernel is in git here:

http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-hardy.git;a=summary

Thanks,
:-Dustin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Dustin Kirkland]

[-- Attachment #1: Type: text/plain, Size: 2059 bytes --]

On Thu, 2009-10-29 at 14:25 +0000, Mark McLoughlin wrote:
> On Thu, 2009-10-29 at 09:11 -0500, Anthony Liguori wrote:
> > Mark McLoughlin wrote:
> > >
> > >> 	tap_set_offload(csum: 1, tso4: 1, tso6: 1, ecn: 1)
> > >> being called and get an mtu of 1500 on virbr0 using his birdge.sh script.
> > >>
> > >> virtio_net_receive2 was trying to transfer a 1534 byte packet (1524 'size' + 10 'virtio_net_hdr')
> > >> and the guest only had 1524 bytes of space in its input descriptors.
> > >>     
> > >
> > > Okay, that sounds like a bug in Dustin's version of the guest virtio-net
> > > driver - if it is only supplying 1524 byte buffers, it should not be
> > > saying it supports the VIRTIO_NET_F_GUEST_TSO4 feature
> > >   
> > 
> > See:
> > 
> > commit 8eca6b1bc770982595db2f7207c65051572436cb
> > Author: aliguori <aliguori@c046a42c-6fe2-441c-8c8c-71466251a162>
> > Date:   Sun Apr 5 17:40:08 2009 +0000
> > 
> >     Fix oops on 2.6.25 guest (Rusty Russell)
> >    
> >     I believe this is behind the following:
> >     https://bugs.edge.launchpad.net/ubuntu/jaunty/+source/linux/+bug/331128
> >    
> >     virtio_pci in 2.6.25 didn't do feature negotiation correctly: it 
> > acked every
> >     bit.  Fortunately, we can detect this.
> >    
> >     Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> >     Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> > 
> > It looks like Rusty's fix wasn't enough.  If I change virtio-net to only 
> > advertise F_MAC, we don't run into this problem.
> 
> If it's not acking VBAD_FEATURE, then it doesn't sound like the same
> issue
> 
> It's also not acking e.g. MRG_RXBUF, which suggests that it is
> selectively acking features, and choosing to ack TSO4
> 
> A quick look through the guest driver code should clear up the
> confusion. Dustion, got a pointer?

Hi Mark,

I'm currently testing Scott's patch above.

In the mean time, Hardy's kernel is in git here:

http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-hardy.git;a=summary

Thanks,
:-Dustin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Dustin Kirkland]

[-- Attachment #1: Type: text/plain, Size: 371 bytes --]

On Thu, 2009-10-29 at 09:34 -0500, Dustin Kirkland wrote:
> In the mean time, Hardy's kernel is in git here:
> 
> http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-hardy.git;a=summary

I'll save you a few clicks...

http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-hardy.git;a=blob;f=drivers/net/virtio_net.c;h=d1a200ff5fd266c05e9a876e5e4e550737f77d84;hb=HEAD

:-dustin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Dustin Kirkland]

[-- Attachment #1: Type: text/plain, Size: 371 bytes --]

On Thu, 2009-10-29 at 09:34 -0500, Dustin Kirkland wrote:
> In the mean time, Hardy's kernel is in git here:
> 
> http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-hardy.git;a=summary

I'll save you a few clicks...

http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-hardy.git;a=blob;f=drivers/net/virtio_net.c;h=d1a200ff5fd266c05e9a876e5e4e550737f77d84;hb=HEAD

:-dustin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Mark McLoughlin]

On Thu, 2009-10-29 at 09:46 -0500, Dustin Kirkland wrote:
> On Thu, 2009-10-29 at 09:34 -0500, Dustin Kirkland wrote:
> > In the mean time, Hardy's kernel is in git here:
> > 
> > http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-hardy.git;a=summary
> 
> I'll save you a few clicks...
> 
> http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-hardy.git;a=blob;f=drivers/net/virtio_net.c;h=d1a200ff5fd266c05e9a876e5e4e550737f77d84;hb=HEAD

Actually, what would save more clicks is if:

  git://kernel.ubuntu.com/ubuntu/ubuntu-hardy.git

was listed in web interface. Took me a while to find:

  https://wiki.ubuntu.com/KernelTeam/KernelGitGuide

:-)

Thanks,
Mark.

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Mark McLoughlin]

On Thu, 2009-10-29 at 09:46 -0500, Dustin Kirkland wrote:
> On Thu, 2009-10-29 at 09:34 -0500, Dustin Kirkland wrote:
> > In the mean time, Hardy's kernel is in git here:
> > 
> > http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-hardy.git;a=summary
> 
> I'll save you a few clicks...
> 
> http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-hardy.git;a=blob;f=drivers/net/virtio_net.c;h=d1a200ff5fd266c05e9a876e5e4e550737f77d84;hb=HEAD

Actually, what would save more clicks is if:

  git://kernel.ubuntu.com/ubuntu/ubuntu-hardy.git

was listed in web interface. Took me a while to find:

  https://wiki.ubuntu.com/KernelTeam/KernelGitGuide

:-)

Thanks,
Mark.

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Anthony Liguori]

Mark McLoughlin wrote:
> On Thu, 2009-10-29 at 09:11 -0500, Anthony Liguori wrote:
>   
>> Mark McLoughlin wrote:
>>     
>>>> 	tap_set_offload(csum: 1, tso4: 1, tso6: 1, ecn: 1)
>>>> being called and get an mtu of 1500 on virbr0 using his birdge.sh script.
>>>>
>>>> virtio_net_receive2 was trying to transfer a 1534 byte packet (1524 'size' + 10 'virtio_net_hdr')
>>>> and the guest only had 1524 bytes of space in its input descriptors.
>>>>     
>>>>         
>>> Okay, that sounds like a bug in Dustin's version of the guest virtio-net
>>> driver - if it is only supplying 1524 byte buffers, it should not be
>>> saying it supports the VIRTIO_NET_F_GUEST_TSO4 feature
>>>   
>>>       
>> See:
>>
>> commit 8eca6b1bc770982595db2f7207c65051572436cb
>> Author: aliguori <aliguori@c046a42c-6fe2-441c-8c8c-71466251a162>
>> Date:   Sun Apr 5 17:40:08 2009 +0000
>>
>>     Fix oops on 2.6.25 guest (Rusty Russell)
>>    
>>     I believe this is behind the following:
>>     https://bugs.edge.launchpad.net/ubuntu/jaunty/+source/linux/+bug/331128
>>    
>>     virtio_pci in 2.6.25 didn't do feature negotiation correctly: it 
>> acked every
>>     bit.  Fortunately, we can detect this.
>>    
>>     Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>>     Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>>
>> It looks like Rusty's fix wasn't enough.  If I change virtio-net to only 
>> advertise F_MAC, we don't run into this problem.
>>     
>
> If it's not acking VBAD_FEATURE, then it doesn't sound like the same
> issue
>   

It was acking VBAD_FEATURE when I tested it.

But if you look at the patch, it whitelists the following features:

    features |= (1 << VIRTIO_NET_F_MAC);
    features |= (1 << VIRTIO_NET_F_GUEST_CSUM);
    features |= (1 << VIRTIO_NET_F_GUEST_TSO4);
    features |= (1 << VIRTIO_NET_F_GUEST_TSO6);
    features |= (1 << VIRTIO_NET_F_GUEST_ECN);

Which is why it's ack'ing TSO4.   Removing TSO4 didn't seem to fix it 
for me.

Regards,

Anthony Liguori

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Anthony Liguori]

Mark McLoughlin wrote:
> On Thu, 2009-10-29 at 09:11 -0500, Anthony Liguori wrote:
>   
>> Mark McLoughlin wrote:
>>     
>>>> 	tap_set_offload(csum: 1, tso4: 1, tso6: 1, ecn: 1)
>>>> being called and get an mtu of 1500 on virbr0 using his birdge.sh script.
>>>>
>>>> virtio_net_receive2 was trying to transfer a 1534 byte packet (1524 'size' + 10 'virtio_net_hdr')
>>>> and the guest only had 1524 bytes of space in its input descriptors.
>>>>     
>>>>         
>>> Okay, that sounds like a bug in Dustin's version of the guest virtio-net
>>> driver - if it is only supplying 1524 byte buffers, it should not be
>>> saying it supports the VIRTIO_NET_F_GUEST_TSO4 feature
>>>   
>>>       
>> See:
>>
>> commit 8eca6b1bc770982595db2f7207c65051572436cb
>> Author: aliguori <aliguori@c046a42c-6fe2-441c-8c8c-71466251a162>
>> Date:   Sun Apr 5 17:40:08 2009 +0000
>>
>>     Fix oops on 2.6.25 guest (Rusty Russell)
>>    
>>     I believe this is behind the following:
>>     https://bugs.edge.launchpad.net/ubuntu/jaunty/+source/linux/+bug/331128
>>    
>>     virtio_pci in 2.6.25 didn't do feature negotiation correctly: it 
>> acked every
>>     bit.  Fortunately, we can detect this.
>>    
>>     Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>>     Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>>
>> It looks like Rusty's fix wasn't enough.  If I change virtio-net to only 
>> advertise F_MAC, we don't run into this problem.
>>     
>
> If it's not acking VBAD_FEATURE, then it doesn't sound like the same
> issue
>   

It was acking VBAD_FEATURE when I tested it.

But if you look at the patch, it whitelists the following features:

    features |= (1 << VIRTIO_NET_F_MAC);
    features |= (1 << VIRTIO_NET_F_GUEST_CSUM);
    features |= (1 << VIRTIO_NET_F_GUEST_TSO4);
    features |= (1 << VIRTIO_NET_F_GUEST_TSO6);
    features |= (1 << VIRTIO_NET_F_GUEST_ECN);

Which is why it's ack'ing TSO4.   Removing TSO4 didn't seem to fix it 
for me.

Regards,

Anthony Liguori

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Mark McLoughlin]

On Thu, 2009-10-29 at 09:39 -0500, Anthony Liguori wrote:
> Mark McLoughlin wrote:
> > On Thu, 2009-10-29 at 09:11 -0500, Anthony Liguori wrote:
> >   
> >> Mark McLoughlin wrote:
> >>     
> >>>> 	tap_set_offload(csum: 1, tso4: 1, tso6: 1, ecn: 1)
> >>>> being called and get an mtu of 1500 on virbr0 using his birdge.sh script.
> >>>>
> >>>> virtio_net_receive2 was trying to transfer a 1534 byte packet (1524 'size' + 10 'virtio_net_hdr')
> >>>> and the guest only had 1524 bytes of space in its input descriptors.
> >>>>     
> >>>>         
> >>> Okay, that sounds like a bug in Dustin's version of the guest virtio-net
> >>> driver - if it is only supplying 1524 byte buffers, it should not be
> >>> saying it supports the VIRTIO_NET_F_GUEST_TSO4 feature
> >>>   
> >>>       
> >> See:
> >>
> >> commit 8eca6b1bc770982595db2f7207c65051572436cb
> >> Author: aliguori <aliguori@c046a42c-6fe2-441c-8c8c-71466251a162>
> >> Date:   Sun Apr 5 17:40:08 2009 +0000
> >>
> >>     Fix oops on 2.6.25 guest (Rusty Russell)
> >>    
> >>     I believe this is behind the following:
> >>     https://bugs.edge.launchpad.net/ubuntu/jaunty/+source/linux/+bug/331128
> >>    
> >>     virtio_pci in 2.6.25 didn't do feature negotiation correctly: it 
> >> acked every
> >>     bit.  Fortunately, we can detect this.
> >>    
> >>     Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> >>     Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> >>
> >> It looks like Rusty's fix wasn't enough.  If I change virtio-net to only 
> >> advertise F_MAC, we don't run into this problem.
> >>     
> >
> > If it's not acking VBAD_FEATURE, then it doesn't sound like the same
> > issue
> >   
> 
> It was acking VBAD_FEATURE when I tested it.
> 
> But if you look at the patch, it whitelists the following features:
> 
>     features |= (1 << VIRTIO_NET_F_MAC);
>     features |= (1 << VIRTIO_NET_F_GUEST_CSUM);
>     features |= (1 << VIRTIO_NET_F_GUEST_TSO4);
>     features |= (1 << VIRTIO_NET_F_GUEST_TSO6);
>     features |= (1 << VIRTIO_NET_F_GUEST_ECN);

Ah, it all makes sense now.

I was getting confused between HOST_* and GUEST_*

this should have been:

    features |= (1 << VIRTIO_NET_F_MAC);
    features |= (1 << VIRTIO_NET_F_HOST_CSUM);
    features |= (1 << VIRTIO_NET_F_HOST_TSO4);
    features |= (1 << VIRTIO_NET_F_HOST_TSO6);
    features |= (1 << VIRTIO_NET_F_HOST_ECN);

Could you try that Dustin?

> Which is why it's ack'ing TSO4.   Removing TSO4 didn't seem to fix it 
> for me.

Odd.

Cheers,
Mark.

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Mark McLoughlin]

On Thu, 2009-10-29 at 09:39 -0500, Anthony Liguori wrote:
> Mark McLoughlin wrote:
> > On Thu, 2009-10-29 at 09:11 -0500, Anthony Liguori wrote:
> >   
> >> Mark McLoughlin wrote:
> >>     
> >>>> 	tap_set_offload(csum: 1, tso4: 1, tso6: 1, ecn: 1)
> >>>> being called and get an mtu of 1500 on virbr0 using his birdge.sh script.
> >>>>
> >>>> virtio_net_receive2 was trying to transfer a 1534 byte packet (1524 'size' + 10 'virtio_net_hdr')
> >>>> and the guest only had 1524 bytes of space in its input descriptors.
> >>>>     
> >>>>         
> >>> Okay, that sounds like a bug in Dustin's version of the guest virtio-net
> >>> driver - if it is only supplying 1524 byte buffers, it should not be
> >>> saying it supports the VIRTIO_NET_F_GUEST_TSO4 feature
> >>>   
> >>>       
> >> See:
> >>
> >> commit 8eca6b1bc770982595db2f7207c65051572436cb
> >> Author: aliguori <aliguori@c046a42c-6fe2-441c-8c8c-71466251a162>
> >> Date:   Sun Apr 5 17:40:08 2009 +0000
> >>
> >>     Fix oops on 2.6.25 guest (Rusty Russell)
> >>    
> >>     I believe this is behind the following:
> >>     https://bugs.edge.launchpad.net/ubuntu/jaunty/+source/linux/+bug/331128
> >>    
> >>     virtio_pci in 2.6.25 didn't do feature negotiation correctly: it 
> >> acked every
> >>     bit.  Fortunately, we can detect this.
> >>    
> >>     Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> >>     Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> >>
> >> It looks like Rusty's fix wasn't enough.  If I change virtio-net to only 
> >> advertise F_MAC, we don't run into this problem.
> >>     
> >
> > If it's not acking VBAD_FEATURE, then it doesn't sound like the same
> > issue
> >   
> 
> It was acking VBAD_FEATURE when I tested it.
> 
> But if you look at the patch, it whitelists the following features:
> 
>     features |= (1 << VIRTIO_NET_F_MAC);
>     features |= (1 << VIRTIO_NET_F_GUEST_CSUM);
>     features |= (1 << VIRTIO_NET_F_GUEST_TSO4);
>     features |= (1 << VIRTIO_NET_F_GUEST_TSO6);
>     features |= (1 << VIRTIO_NET_F_GUEST_ECN);

Ah, it all makes sense now.

I was getting confused between HOST_* and GUEST_*

this should have been:

    features |= (1 << VIRTIO_NET_F_MAC);
    features |= (1 << VIRTIO_NET_F_HOST_CSUM);
    features |= (1 << VIRTIO_NET_F_HOST_TSO4);
    features |= (1 << VIRTIO_NET_F_HOST_TSO6);
    features |= (1 << VIRTIO_NET_F_HOST_ECN);

Could you try that Dustin?

> Which is why it's ack'ing TSO4.   Removing TSO4 didn't seem to fix it 
> for me.

Odd.

Cheers,
Mark.

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Dustin Kirkland]

[-- Attachment #1: Type: text/plain, Size: 1537 bytes --]

On Thu, 2009-10-29 at 14:48 +0000, Mark McLoughlin wrote:
> Ah, it all makes sense now.
> 
> I was getting confused between HOST_* and GUEST_*
> 
> this should have been:
> 
>     features |= (1 << VIRTIO_NET_F_MAC);
>     features |= (1 << VIRTIO_NET_F_HOST_CSUM);
>     features |= (1 << VIRTIO_NET_F_HOST_TSO4);
>     features |= (1 << VIRTIO_NET_F_HOST_TSO6);
>     features |= (1 << VIRTIO_NET_F_HOST_ECN);
> 
> Could you try that Dustin?

Hmm, not sure I'm doing this correctly...  I tried changing the
following, but looks like I might also have to define these as well,
since:

/tmp/qemu-kvm/qemu-kvm/hw/virtio-net.c:167: error:
‘VIRTIO_NET_F_HOST_CSUM’ undeclared (first use in this function)


diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index ce8e6cb..6582e69 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -164,10 +164,10 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
     /* Linux kernel 2.6.25.  It understood MAC (as everyone must),
      * but also these: */
     features |= (1 << VIRTIO_NET_F_MAC);
-    features |= (1 << VIRTIO_NET_F_GUEST_CSUM);
-    features |= (1 << VIRTIO_NET_F_GUEST_TSO4);
-    features |= (1 << VIRTIO_NET_F_GUEST_TSO6);
-    features |= (1 << VIRTIO_NET_F_GUEST_ECN);
+    features |= (1 << VIRTIO_NET_F_HOST_CSUM);
+    features |= (1 << VIRTIO_NET_F_HOST_TSO4);
+    features |= (1 << VIRTIO_NET_F_HOST_TSO6);
+    features |= (1 << VIRTIO_NET_F_HOST_ECN);
 
     return features & virtio_net_get_features(vdev);
 }


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Dustin Kirkland]

[-- Attachment #1: Type: text/plain, Size: 1537 bytes --]

On Thu, 2009-10-29 at 14:48 +0000, Mark McLoughlin wrote:
> Ah, it all makes sense now.
> 
> I was getting confused between HOST_* and GUEST_*
> 
> this should have been:
> 
>     features |= (1 << VIRTIO_NET_F_MAC);
>     features |= (1 << VIRTIO_NET_F_HOST_CSUM);
>     features |= (1 << VIRTIO_NET_F_HOST_TSO4);
>     features |= (1 << VIRTIO_NET_F_HOST_TSO6);
>     features |= (1 << VIRTIO_NET_F_HOST_ECN);
> 
> Could you try that Dustin?

Hmm, not sure I'm doing this correctly...  I tried changing the
following, but looks like I might also have to define these as well,
since:

/tmp/qemu-kvm/qemu-kvm/hw/virtio-net.c:167: error:
‘VIRTIO_NET_F_HOST_CSUM’ undeclared (first use in this function)


diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index ce8e6cb..6582e69 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -164,10 +164,10 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
     /* Linux kernel 2.6.25.  It understood MAC (as everyone must),
      * but also these: */
     features |= (1 << VIRTIO_NET_F_MAC);
-    features |= (1 << VIRTIO_NET_F_GUEST_CSUM);
-    features |= (1 << VIRTIO_NET_F_GUEST_TSO4);
-    features |= (1 << VIRTIO_NET_F_GUEST_TSO6);
-    features |= (1 << VIRTIO_NET_F_GUEST_ECN);
+    features |= (1 << VIRTIO_NET_F_HOST_CSUM);
+    features |= (1 << VIRTIO_NET_F_HOST_TSO4);
+    features |= (1 << VIRTIO_NET_F_HOST_TSO6);
+    features |= (1 << VIRTIO_NET_F_HOST_ECN);
 
     return features & virtio_net_get_features(vdev);
 }


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Mark McLoughlin]

On Thu, 2009-10-29 at 10:01 -0500, Dustin Kirkland wrote:
> On Thu, 2009-10-29 at 14:48 +0000, Mark McLoughlin wrote:
> > Ah, it all makes sense now.
> > 
> > I was getting confused between HOST_* and GUEST_*
> > 
> > this should have been:
> > 
> >     features |= (1 << VIRTIO_NET_F_MAC);
> >     features |= (1 << VIRTIO_NET_F_HOST_CSUM);
> >     features |= (1 << VIRTIO_NET_F_HOST_TSO4);
> >     features |= (1 << VIRTIO_NET_F_HOST_TSO6);
> >     features |= (1 << VIRTIO_NET_F_HOST_ECN);
> > 
> > Could you try that Dustin?
> 
> Hmm, not sure I'm doing this correctly...  I tried changing the
> following, but looks like I might also have to define these as well,
> since:
> 
> /tmp/qemu-kvm/qemu-kvm/hw/virtio-net.c:167: error:
> ‘VIRTIO_NET_F_HOST_CSUM’ undeclared (first use in this function)

Sorry, should be VIRTIO_NET_F_CSUM ... the rest is correct

Thanks,
mark.

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Mark McLoughlin]

On Thu, 2009-10-29 at 10:01 -0500, Dustin Kirkland wrote:
> On Thu, 2009-10-29 at 14:48 +0000, Mark McLoughlin wrote:
> > Ah, it all makes sense now.
> > 
> > I was getting confused between HOST_* and GUEST_*
> > 
> > this should have been:
> > 
> >     features |= (1 << VIRTIO_NET_F_MAC);
> >     features |= (1 << VIRTIO_NET_F_HOST_CSUM);
> >     features |= (1 << VIRTIO_NET_F_HOST_TSO4);
> >     features |= (1 << VIRTIO_NET_F_HOST_TSO6);
> >     features |= (1 << VIRTIO_NET_F_HOST_ECN);
> > 
> > Could you try that Dustin?
> 
> Hmm, not sure I'm doing this correctly...  I tried changing the
> following, but looks like I might also have to define these as well,
> since:
> 
> /tmp/qemu-kvm/qemu-kvm/hw/virtio-net.c:167: error:
> ‘VIRTIO_NET_F_HOST_CSUM’ undeclared (first use in this function)

Sorry, should be VIRTIO_NET_F_CSUM ... the rest is correct

Thanks,
mark.

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Dustin Kirkland]

[-- Attachment #1: Type: text/plain, Size: 265 bytes --]

On Thu, 2009-10-29 at 15:01 +0000, Mark McLoughlin wrote:
> Sorry, should be VIRTIO_NET_F_CSUM ... the rest is correct

Brilliant!

Works like a champ.  I'll send a patch in a subsequent email.  Would you
add a signed-off-by (or whatever), Mark?

:-Dustin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Dustin Kirkland]

[-- Attachment #1: Type: text/plain, Size: 265 bytes --]

On Thu, 2009-10-29 at 15:01 +0000, Mark McLoughlin wrote:
> Sorry, should be VIRTIO_NET_F_CSUM ... the rest is correct

Brilliant!

Works like a champ.  I'll send a patch in a subsequent email.  Would you
add a signed-off-by (or whatever), Mark?

:-Dustin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Mark McLoughlin]

On Thu, 2009-10-29 at 10:13 -0500, Dustin Kirkland wrote:
> On Thu, 2009-10-29 at 15:01 +0000, Mark McLoughlin wrote:
> > Sorry, should be VIRTIO_NET_F_CSUM ... the rest is correct
> 
> Brilliant!
> 
> Works like a champ.  I'll send a patch in a subsequent email.  Would you
> add a signed-off-by (or whatever), Mark?

Sure,

Signed-off-by: Mark McLoughlin <markmc@redhat.com>

or:

Acked-by: Mark McLoughlin <markmc@redhat.com>

whichever works :)

Cheers,
Mark.

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Mark McLoughlin]

On Thu, 2009-10-29 at 10:13 -0500, Dustin Kirkland wrote:
> On Thu, 2009-10-29 at 15:01 +0000, Mark McLoughlin wrote:
> > Sorry, should be VIRTIO_NET_F_CSUM ... the rest is correct
> 
> Brilliant!
> 
> Works like a champ.  I'll send a patch in a subsequent email.  Would you
> add a signed-off-by (or whatever), Mark?

Sure,

Signed-off-by: Mark McLoughlin <markmc@redhat.com>

or:

Acked-by: Mark McLoughlin <markmc@redhat.com>

whichever works :)

Cheers,
Mark.

[PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Dustin Kirkland]

[-- Attachment #1: Type: text/plain, Size: 1713 bytes --]

whitelist host virtio networking features

This patch is a followup to 8eca6b1bc770982595db2f7207c65051572436cb,
fixing crashes when guests with 2.6.25 virtio drivers have saturated
virtio network connections.

https://bugs.edge.launchpad.net/ubuntu/+source/qemu-kvm/+bug/458521

That patch should have been whitelisting *_HOST_* rather than the the
*_GUEST_* features.

I tested this by running an Ubuntu 8.04 Hardy guest (2.6.24 kernel +
2.6.25-virtio driver).  I saturated both the incoming, and outgoing
network connection with nc, seeing sustained 6MB/s up and 6MB/s down
bitrates for ~20 minutes.  Previously, this crashed immediately.  Now,
the guest does not crash and maintains network connectivity throughout
the test.

Signed-off-by: Dustin Kirkland <kirkland@canonical.com>
Signed-off-by: Mark McLoughlin <markmc@redhat.com>
Tested-by: Dustin Kirkland <kirkland@canonical.com>

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index ce8e6cb..27834fa 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -164,10 +164,10 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
     /* Linux kernel 2.6.25.  It understood MAC (as everyone must),
      * but also these: */
     features |= (1 << VIRTIO_NET_F_MAC);
-    features |= (1 << VIRTIO_NET_F_GUEST_CSUM);
-    features |= (1 << VIRTIO_NET_F_GUEST_TSO4);
-    features |= (1 << VIRTIO_NET_F_GUEST_TSO6);
-    features |= (1 << VIRTIO_NET_F_GUEST_ECN);
+    features |= (1 << VIRTIO_NET_F_CSUM);
+    features |= (1 << VIRTIO_NET_F_HOST_TSO4);
+    features |= (1 << VIRTIO_NET_F_HOST_TSO6);
+    features |= (1 << VIRTIO_NET_F_HOST_ECN);
 
     return features & virtio_net_get_features(vdev);
 }


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[Qemu-devel] [PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Dustin Kirkland]

[-- Attachment #1: Type: text/plain, Size: 1713 bytes --]

whitelist host virtio networking features

This patch is a followup to 8eca6b1bc770982595db2f7207c65051572436cb,
fixing crashes when guests with 2.6.25 virtio drivers have saturated
virtio network connections.

https://bugs.edge.launchpad.net/ubuntu/+source/qemu-kvm/+bug/458521

That patch should have been whitelisting *_HOST_* rather than the the
*_GUEST_* features.

I tested this by running an Ubuntu 8.04 Hardy guest (2.6.24 kernel +
2.6.25-virtio driver).  I saturated both the incoming, and outgoing
network connection with nc, seeing sustained 6MB/s up and 6MB/s down
bitrates for ~20 minutes.  Previously, this crashed immediately.  Now,
the guest does not crash and maintains network connectivity throughout
the test.

Signed-off-by: Dustin Kirkland <kirkland@canonical.com>
Signed-off-by: Mark McLoughlin <markmc@redhat.com>
Tested-by: Dustin Kirkland <kirkland@canonical.com>

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index ce8e6cb..27834fa 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -164,10 +164,10 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
     /* Linux kernel 2.6.25.  It understood MAC (as everyone must),
      * but also these: */
     features |= (1 << VIRTIO_NET_F_MAC);
-    features |= (1 << VIRTIO_NET_F_GUEST_CSUM);
-    features |= (1 << VIRTIO_NET_F_GUEST_TSO4);
-    features |= (1 << VIRTIO_NET_F_GUEST_TSO6);
-    features |= (1 << VIRTIO_NET_F_GUEST_ECN);
+    features |= (1 << VIRTIO_NET_F_CSUM);
+    features |= (1 << VIRTIO_NET_F_HOST_TSO4);
+    features |= (1 << VIRTIO_NET_F_HOST_TSO6);
+    features |= (1 << VIRTIO_NET_F_HOST_ECN);
 
     return features & virtio_net_get_features(vdev);
 }


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Dustin Kirkland]

On Thu, Oct 29, 2009 at 10:34 AM, Dustin Kirkland
<kirkland@canonical.com> wrote:
> whitelist host virtio networking features
>
> This patch is a followup to 8eca6b1bc770982595db2f7207c65051572436cb,
> fixing crashes when guests with 2.6.25 virtio drivers have saturated
> virtio network connections.
>
> https://bugs.edge.launchpad.net/ubuntu/+source/qemu-kvm/+bug/458521
>
> That patch should have been whitelisting *_HOST_* rather than the the
> *_GUEST_* features.
>
> I tested this by running an Ubuntu 8.04 Hardy guest (2.6.24 kernel +
> 2.6.25-virtio driver).  I saturated both the incoming, and outgoing
> network connection with nc, seeing sustained 6MB/s up and 6MB/s down
> bitrates for ~20 minutes.  Previously, this crashed immediately.  Now,
> the guest does not crash and maintains network connectivity throughout
> the test.
<snip>

FYI...

Canonical's Ubuntu Security Team will be filing a CVE on this issue,
since there is a bit of an attack vector here, and since
qemu-kvm-0.11.0 is generally available as an official release (and now
part of Ubuntu 9.10).

Guests running linux <= 2.6.25 virtio-net (e.g Ubuntu 8.04 hardy) on
top of qemu-kvm-0.11.0 can be remotely crashed by a non-privileged
network user flooding an open port on the guest.  The crash happens in
a manner that abruptly terminates the guest's execution (ie, without
shutting down cleanly).  This may affect the guest filesystem's
general happiness.

:-Dustin

[Qemu-devel] Re: [PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Dustin Kirkland]

On Thu, Oct 29, 2009 at 10:34 AM, Dustin Kirkland
<kirkland@canonical.com> wrote:
> whitelist host virtio networking features
>
> This patch is a followup to 8eca6b1bc770982595db2f7207c65051572436cb,
> fixing crashes when guests with 2.6.25 virtio drivers have saturated
> virtio network connections.
>
> https://bugs.edge.launchpad.net/ubuntu/+source/qemu-kvm/+bug/458521
>
> That patch should have been whitelisting *_HOST_* rather than the the
> *_GUEST_* features.
>
> I tested this by running an Ubuntu 8.04 Hardy guest (2.6.24 kernel +
> 2.6.25-virtio driver).  I saturated both the incoming, and outgoing
> network connection with nc, seeing sustained 6MB/s up and 6MB/s down
> bitrates for ~20 minutes.  Previously, this crashed immediately.  Now,
> the guest does not crash and maintains network connectivity throughout
> the test.
<snip>

FYI...

Canonical's Ubuntu Security Team will be filing a CVE on this issue,
since there is a bit of an attack vector here, and since
qemu-kvm-0.11.0 is generally available as an official release (and now
part of Ubuntu 9.10).

Guests running linux <= 2.6.25 virtio-net (e.g Ubuntu 8.04 hardy) on
top of qemu-kvm-0.11.0 can be remotely crashed by a non-privileged
network user flooding an open port on the guest.  The crash happens in
a manner that abruptly terminates the guest's execution (ie, without
shutting down cleanly).  This may affect the guest filesystem's
general happiness.

:-Dustin

Re: [PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Mark McLoughlin]

On Fri, 2009-10-30 at 16:15 -0500, Dustin Kirkland wrote:
> On Thu, Oct 29, 2009 at 10:34 AM, Dustin Kirkland
> <kirkland@canonical.com> wrote:
> > whitelist host virtio networking features
> >
> > This patch is a followup to 8eca6b1bc770982595db2f7207c65051572436cb,
> > fixing crashes when guests with 2.6.25 virtio drivers have saturated
> > virtio network connections.
> >
> > https://bugs.edge.launchpad.net/ubuntu/+source/qemu-kvm/+bug/458521
> >
> > That patch should have been whitelisting *_HOST_* rather than the the
> > *_GUEST_* features.
> >
> > I tested this by running an Ubuntu 8.04 Hardy guest (2.6.24 kernel +
> > 2.6.25-virtio driver).  I saturated both the incoming, and outgoing
> > network connection with nc, seeing sustained 6MB/s up and 6MB/s down
> > bitrates for ~20 minutes.  Previously, this crashed immediately.  Now,
> > the guest does not crash and maintains network connectivity throughout
> > the test.
> <snip>
> 
> FYI...

Thanks for the notice

> Canonical's Ubuntu Security Team will be filing a CVE on this issue,
> since there is a bit of an attack vector here, and since
> qemu-kvm-0.11.0 is generally available as an official release (and now
> part of Ubuntu 9.10).
> 
> Guests running linux <= 2.6.25 virtio-net (e.g Ubuntu 8.04 hardy) on
> top of qemu-kvm-0.11.0 can be remotely crashed by a non-privileged
> network user flooding an open port on the guest.  The crash happens in
> a manner that abruptly terminates the guest's execution (ie, without
> shutting down cleanly).  This may affect the guest filesystem's
> general happiness.

IMHO, the CVE should be against the 2.6.25 virtio drivers - the bug is
in the guest and the issue we're discussing here is just a hacky
workaround for the guest bug.

Cheers,
Mark.

[Qemu-devel] Re: [PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Mark McLoughlin]

On Fri, 2009-10-30 at 16:15 -0500, Dustin Kirkland wrote:
> On Thu, Oct 29, 2009 at 10:34 AM, Dustin Kirkland
> <kirkland@canonical.com> wrote:
> > whitelist host virtio networking features
> >
> > This patch is a followup to 8eca6b1bc770982595db2f7207c65051572436cb,
> > fixing crashes when guests with 2.6.25 virtio drivers have saturated
> > virtio network connections.
> >
> > https://bugs.edge.launchpad.net/ubuntu/+source/qemu-kvm/+bug/458521
> >
> > That patch should have been whitelisting *_HOST_* rather than the the
> > *_GUEST_* features.
> >
> > I tested this by running an Ubuntu 8.04 Hardy guest (2.6.24 kernel +
> > 2.6.25-virtio driver).  I saturated both the incoming, and outgoing
> > network connection with nc, seeing sustained 6MB/s up and 6MB/s down
> > bitrates for ~20 minutes.  Previously, this crashed immediately.  Now,
> > the guest does not crash and maintains network connectivity throughout
> > the test.
> <snip>
> 
> FYI...

Thanks for the notice

> Canonical's Ubuntu Security Team will be filing a CVE on this issue,
> since there is a bit of an attack vector here, and since
> qemu-kvm-0.11.0 is generally available as an official release (and now
> part of Ubuntu 9.10).
> 
> Guests running linux <= 2.6.25 virtio-net (e.g Ubuntu 8.04 hardy) on
> top of qemu-kvm-0.11.0 can be remotely crashed by a non-privileged
> network user flooding an open port on the guest.  The crash happens in
> a manner that abruptly terminates the guest's execution (ie, without
> shutting down cleanly).  This may affect the guest filesystem's
> general happiness.

IMHO, the CVE should be against the 2.6.25 virtio drivers - the bug is
in the guest and the issue we're discussing here is just a hacky
workaround for the guest bug.

Cheers,
Mark.

Re: [PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Anthony Liguori]

Mark McLoughlin wrote:
>> Canonical's Ubuntu Security Team will be filing a CVE on this issue,
>> since there is a bit of an attack vector here, and since
>> qemu-kvm-0.11.0 is generally available as an official release (and now
>> part of Ubuntu 9.10).
>>
>> Guests running linux <= 2.6.25 virtio-net (e.g Ubuntu 8.04 hardy) on
>> top of qemu-kvm-0.11.0 can be remotely crashed by a non-privileged
>> network user flooding an open port on the guest.  The crash happens in
>> a manner that abruptly terminates the guest's execution (ie, without
>> shutting down cleanly).  This may affect the guest filesystem's
>> general happiness.
>>     
>
> IMHO, the CVE should be against the 2.6.25 virtio drivers - the bug is
> in the guest and the issue we're discussing here is just a hacky
> workaround for the guest bug.
>   

Yeah, I'm inclined to agree.  The guest generates bad data and we exit.  
exit()ing is probably not wonderful but it's a well understood behavior.

The fundamental bug here is in the guest, not in qemu.

Regards,

Anthony Liguori

> Cheers,
> Mark.
>
>   

[Qemu-devel] Re: [PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Anthony Liguori]

Mark McLoughlin wrote:
>> Canonical's Ubuntu Security Team will be filing a CVE on this issue,
>> since there is a bit of an attack vector here, and since
>> qemu-kvm-0.11.0 is generally available as an official release (and now
>> part of Ubuntu 9.10).
>>
>> Guests running linux <= 2.6.25 virtio-net (e.g Ubuntu 8.04 hardy) on
>> top of qemu-kvm-0.11.0 can be remotely crashed by a non-privileged
>> network user flooding an open port on the guest.  The crash happens in
>> a manner that abruptly terminates the guest's execution (ie, without
>> shutting down cleanly).  This may affect the guest filesystem's
>> general happiness.
>>     
>
> IMHO, the CVE should be against the 2.6.25 virtio drivers - the bug is
> in the guest and the issue we're discussing here is just a hacky
> workaround for the guest bug.
>   

Yeah, I'm inclined to agree.  The guest generates bad data and we exit.  
exit()ing is probably not wonderful but it's a well understood behavior.

The fundamental bug here is in the guest, not in qemu.

Regards,

Anthony Liguori

> Cheers,
> Mark.
>
>   

Re: [Qemu-devel] Re: [PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Jamie Lokier]

Anthony Liguori wrote:
> Mark McLoughlin wrote:
> >>Canonical's Ubuntu Security Team will be filing a CVE on this issue,
> >>since there is a bit of an attack vector here, and since
> >>qemu-kvm-0.11.0 is generally available as an official release (and now
> >>part of Ubuntu 9.10).
> >>
> >>Guests running linux <= 2.6.25 virtio-net (e.g Ubuntu 8.04 hardy) on
> >>top of qemu-kvm-0.11.0 can be remotely crashed by a non-privileged
> >>network user flooding an open port on the guest.  The crash happens in
> >>a manner that abruptly terminates the guest's execution (ie, without
> >>shutting down cleanly).  This may affect the guest filesystem's
> >>general happiness.
> >>    
> >
> >IMHO, the CVE should be against the 2.6.25 virtio drivers - the bug is
> >in the guest and the issue we're discussing here is just a hacky
> >workaround for the guest bug.
> >  
> 
> Yeah, I'm inclined to agree.  The guest generates bad data and we exit.  
> exit()ing is probably not wonderful but it's a well understood behavior.
> 
> The fundamental bug here is in the guest, not in qemu.

Guests should never be able to crash or terminate qemu, unless they
call something that is intentionally an "exit qemu" hook for the
guest.  And even that should be possible to disable.

What happens if the guest is running malicious code?  What if it's
been hacked?  The worst that should happen is the guest behaves like a
hacked machine.

What about running old machine images?  One of the major uses I've
seen of KVM is for running old machine images - images which are not
to be updated, so that they continue to have the same behaviour.

I agree that the 2.6.25 virtio drivers have a bug and ought to be
fixed, but a qemu which abruptly terminates is never good.

-- Jamie

Re: [Qemu-devel] Re: [PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Michael Tokarev]

Jamie Lokier wrote:
> Anthony Liguori wrote:
>> Mark McLoughlin wrote:
>>>> Canonical's Ubuntu Security Team will be filing a CVE on this issue,
>>>> since there is a bit of an attack vector here, and since
>>>> qemu-kvm-0.11.0 is generally available as an official release (and now
>>>> part of Ubuntu 9.10).
>>>>
>>>> Guests running linux <= 2.6.25 virtio-net (e.g Ubuntu 8.04 hardy) on
>>>> top of qemu-kvm-0.11.0 can be remotely crashed by a non-privileged
>>>> network user flooding an open port on the guest.  The crash happens in
>>>> a manner that abruptly terminates the guest's execution (ie, without
>>>> shutting down cleanly).  This may affect the guest filesystem's
>>>> general happiness.
>>>>    
>>> IMHO, the CVE should be against the 2.6.25 virtio drivers - the bug is
>>> in the guest and the issue we're discussing here is just a hacky
>>> workaround for the guest bug.
>>>  
>> Yeah, I'm inclined to agree.  The guest generates bad data and we exit.  
>> exit()ing is probably not wonderful but it's a well understood behavior.
>>
>> The fundamental bug here is in the guest, not in qemu.
> 
> Guests should never be able to crash or terminate qemu, unless they
> call something that is intentionally an "exit qemu" hook for the
> guest.  And even that should be possible to disable.

Well, if your buggy NIC driver does something wrong programming
the hardware (like the famous r8169 did - it allocated less
buffer space than telling to the card, so the card were happily
overwriting unrelated kernel memory with content received from
network), you will most likely get a machine which does not
respond to external events, a stuck machine, until you hit
"reset" button (provided there is one) or toggle power.
Or just a reboot, depending on what exactly you've hit.

If you want kvm to behave like this, wrap it into a trivial
shell script that restarts the guest.

/mjt

Re: [Qemu-devel] Re: [PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Michael Tokarev]

Jamie Lokier wrote:
> Anthony Liguori wrote:
>> Mark McLoughlin wrote:
>>>> Canonical's Ubuntu Security Team will be filing a CVE on this issue,
>>>> since there is a bit of an attack vector here, and since
>>>> qemu-kvm-0.11.0 is generally available as an official release (and now
>>>> part of Ubuntu 9.10).
>>>>
>>>> Guests running linux <= 2.6.25 virtio-net (e.g Ubuntu 8.04 hardy) on
>>>> top of qemu-kvm-0.11.0 can be remotely crashed by a non-privileged
>>>> network user flooding an open port on the guest.  The crash happens in
>>>> a manner that abruptly terminates the guest's execution (ie, without
>>>> shutting down cleanly).  This may affect the guest filesystem's
>>>> general happiness.
>>>>    
>>> IMHO, the CVE should be against the 2.6.25 virtio drivers - the bug is
>>> in the guest and the issue we're discussing here is just a hacky
>>> workaround for the guest bug.
>>>  
>> Yeah, I'm inclined to agree.  The guest generates bad data and we exit.  
>> exit()ing is probably not wonderful but it's a well understood behavior.
>>
>> The fundamental bug here is in the guest, not in qemu.
> 
> Guests should never be able to crash or terminate qemu, unless they
> call something that is intentionally an "exit qemu" hook for the
> guest.  And even that should be possible to disable.

Well, if your buggy NIC driver does something wrong programming
the hardware (like the famous r8169 did - it allocated less
buffer space than telling to the card, so the card were happily
overwriting unrelated kernel memory with content received from
network), you will most likely get a machine which does not
respond to external events, a stuck machine, until you hit
"reset" button (provided there is one) or toggle power.
Or just a reboot, depending on what exactly you've hit.

If you want kvm to behave like this, wrap it into a trivial
shell script that restarts the guest.

/mjt

Re: [Qemu-devel] Re: [PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Jamie Lokier]

Michael Tokarev wrote:
> If you want kvm to behave like this, wrap it into a trivial
> shell script that restarts the guest.

True, kvm has enough crash-bugs elsewhere that I already have to deal
with that.  It'd be nice to distinguish kvm/qemu bugs from guest
bugs, though :-)

kvm/qemu also has lock-up bugs, where it's spinning at 100% and the
guest seems to be stuck (even though the VNC server continues to
work).  That's a bit harder to fix with a wrapper script.

-- Jamie

Re: [Qemu-devel] Re: [PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Jamie Lokier]

Michael Tokarev wrote:
> If you want kvm to behave like this, wrap it into a trivial
> shell script that restarts the guest.

True, kvm has enough crash-bugs elsewhere that I already have to deal
with that.  It'd be nice to distinguish kvm/qemu bugs from guest
bugs, though :-)

kvm/qemu also has lock-up bugs, where it's spinning at 100% and the
guest seems to be stuck (even though the VNC server continues to
work).  That's a bit harder to fix with a wrapper script.

-- Jamie

Re: [Qemu-devel] Re: [PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Anthony Liguori]

Jamie Lokier wrote:
> Anthony Liguori wrote:
>   
>> Mark McLoughlin wrote:
>>     
>>>> Canonical's Ubuntu Security Team will be filing a CVE on this issue,
>>>> since there is a bit of an attack vector here, and since
>>>> qemu-kvm-0.11.0 is generally available as an official release (and now
>>>> part of Ubuntu 9.10).
>>>>
>>>> Guests running linux <= 2.6.25 virtio-net (e.g Ubuntu 8.04 hardy) on
>>>> top of qemu-kvm-0.11.0 can be remotely crashed by a non-privileged
>>>> network user flooding an open port on the guest.  The crash happens in
>>>> a manner that abruptly terminates the guest's execution (ie, without
>>>> shutting down cleanly).  This may affect the guest filesystem's
>>>> general happiness.
>>>>    
>>>>         
>>> IMHO, the CVE should be against the 2.6.25 virtio drivers - the bug is
>>> in the guest and the issue we're discussing here is just a hacky
>>> workaround for the guest bug.
>>>  
>>>       
>> Yeah, I'm inclined to agree.  The guest generates bad data and we exit.  
>> exit()ing is probably not wonderful but it's a well understood behavior.
>>
>> The fundamental bug here is in the guest, not in qemu.
>>     
>
> Guests should never be able to crash or terminate qemu, unless they
> call something that is intentionally an "exit qemu" hook for the
> guest.  And even that should be possible to disable.
>   

They can exit qemu via an ACPI shutdown.  I don't see the difference.

Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Dustin Kirkland]

[-- Attachment #1: Type: text/plain, Size: 501 bytes --]

On Mon, 2009-11-02 at 12:55 -0600, Anthony Liguori wrote:
> They can exit qemu via an ACPI shutdown.  I don't see the difference.

An ACPI shutdown is triggered by an authenticated user inside of the
guest.

The present exit is triggered by any other anonymous user on the
network, with the ability to send a lot of packets very quickly to the
VM guest.  The guest isn't able to handle this properly (and rightly
that guest's kernel should be fixed).  But I do see a difference.

:-Dustin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] Re: [PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Dustin Kirkland]

[-- Attachment #1: Type: text/plain, Size: 501 bytes --]

On Mon, 2009-11-02 at 12:55 -0600, Anthony Liguori wrote:
> They can exit qemu via an ACPI shutdown.  I don't see the difference.

An ACPI shutdown is triggered by an authenticated user inside of the
guest.

The present exit is triggered by any other anonymous user on the
network, with the ability to send a lot of packets very quickly to the
VM guest.  The guest isn't able to handle this properly (and rightly
that guest's kernel should be fixed).  But I do see a difference.

:-Dustin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] Re: [PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Anthony Liguori]

Dustin Kirkland wrote:
> On Mon, 2009-11-02 at 12:55 -0600, Anthony Liguori wrote:
>   
>> They can exit qemu via an ACPI shutdown.  I don't see the difference.
>>     
>
> An ACPI shutdown is triggered by an authenticated user inside of the
> guest.
>
> The present exit is triggered by any other anonymous user on the
> network, with the ability to send a lot of packets very quickly to the
> VM guest.  The guest isn't able to handle this properly (and rightly
> that guest's kernel should be fixed).  But I do see a difference.
>   

Well the problem is triggered by the guest kernel writing garbage to 
virtio-net's backend.  That's why we're suggesting it's really a guest 
kernel issue.

If the guest kernel writes something bad to qemu, we're may kill the 
guest.  That's not a qemu bug, it's the designed behavior.

Regards,

Anthony Liguori

> :-Dustin
>   

Re: [Qemu-devel] Re: [PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Anthony Liguori]

Dustin Kirkland wrote:
> On Mon, 2009-11-02 at 12:55 -0600, Anthony Liguori wrote:
>   
>> They can exit qemu via an ACPI shutdown.  I don't see the difference.
>>     
>
> An ACPI shutdown is triggered by an authenticated user inside of the
> guest.
>
> The present exit is triggered by any other anonymous user on the
> network, with the ability to send a lot of packets very quickly to the
> VM guest.  The guest isn't able to handle this properly (and rightly
> that guest's kernel should be fixed).  But I do see a difference.
>   

Well the problem is triggered by the guest kernel writing garbage to 
virtio-net's backend.  That's why we're suggesting it's really a guest 
kernel issue.

If the guest kernel writes something bad to qemu, we're may kill the 
guest.  That's not a qemu bug, it's the designed behavior.

Regards,

Anthony Liguori

> :-Dustin
>   

Re: [Qemu-devel] Re: [PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Jamie Lokier]

Anthony Liguori wrote:
> Dustin Kirkland wrote:
> >On Mon, 2009-11-02 at 12:55 -0600, Anthony Liguori wrote:
> >  
> >>They can exit qemu via an ACPI shutdown.  I don't see the difference.
> >>    
> >
> >An ACPI shutdown is triggered by an authenticated user inside of the
> >guest.
> >
> >The present exit is triggered by any other anonymous user on the
> >network, with the ability to send a lot of packets very quickly to the
> >VM guest.  The guest isn't able to handle this properly (and rightly
> >that guest's kernel should be fixed).  But I do see a difference.
> >  
> 
> Well the problem is triggered by the guest kernel writing garbage to 
> virtio-net's backend.  That's why we're suggesting it's really a guest 
> kernel issue.
> 
> If the guest kernel writes something bad to qemu, we're may kill the 
> guest.  That's not a qemu bug, it's the designed behavior.

I'm ok with killing the guest, but think the right behaviour is
whatever the user's configured to happen when the guest is killed, in
the same way that ACPI shutdown does not shutdown if -no-shutdown is
used, and you can debug a guest which does that.

A guest writing garbage to virtio-net should not be able to use it to
escape being debugged :-)

How can a manager program determine if the exit is due to intentional
shutdown or crashing guest?

I'd be inclined to make those "exit() called from the device
emulation" cases do whatever -watchdog-action says, if the watchdog is
enabled.  With a great big log message.

-- Jamie

Re: [Qemu-devel] Re: [PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Jamie Lokier]

Anthony Liguori wrote:
> Dustin Kirkland wrote:
> >On Mon, 2009-11-02 at 12:55 -0600, Anthony Liguori wrote:
> >  
> >>They can exit qemu via an ACPI shutdown.  I don't see the difference.
> >>    
> >
> >An ACPI shutdown is triggered by an authenticated user inside of the
> >guest.
> >
> >The present exit is triggered by any other anonymous user on the
> >network, with the ability to send a lot of packets very quickly to the
> >VM guest.  The guest isn't able to handle this properly (and rightly
> >that guest's kernel should be fixed).  But I do see a difference.
> >  
> 
> Well the problem is triggered by the guest kernel writing garbage to 
> virtio-net's backend.  That's why we're suggesting it's really a guest 
> kernel issue.
> 
> If the guest kernel writes something bad to qemu, we're may kill the 
> guest.  That's not a qemu bug, it's the designed behavior.

I'm ok with killing the guest, but think the right behaviour is
whatever the user's configured to happen when the guest is killed, in
the same way that ACPI shutdown does not shutdown if -no-shutdown is
used, and you can debug a guest which does that.

A guest writing garbage to virtio-net should not be able to use it to
escape being debugged :-)

How can a manager program determine if the exit is due to intentional
shutdown or crashing guest?

I'd be inclined to make those "exit() called from the device
emulation" cases do whatever -watchdog-action says, if the watchdog is
enabled.  With a great big log message.

-- Jamie

Re: [PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Dustin Kirkland]

On Mon, Nov 2, 2009 at 8:38 AM, Mark McLoughlin <markmc@redhat.com> wrote:
> On Fri, 2009-10-30 at 16:15 -0500, Dustin Kirkland wrote:
>> Canonical's Ubuntu Security Team will be filing a CVE on this issue,
>> since there is a bit of an attack vector here, and since
>> qemu-kvm-0.11.0 is generally available as an official release (and now
>> part of Ubuntu 9.10).
>>
>> Guests running linux <= 2.6.25 virtio-net (e.g Ubuntu 8.04 hardy) on
>> top of qemu-kvm-0.11.0 can be remotely crashed by a non-privileged
>> network user flooding an open port on the guest.  The crash happens in
>> a manner that abruptly terminates the guest's execution (ie, without
>> shutting down cleanly).  This may affect the guest filesystem's
>> general happiness.
>
> IMHO, the CVE should be against the 2.6.25 virtio drivers - the bug is
> in the guest and the issue we're discussing here is just a hacky
> workaround for the guest bug.

Kees/Jamie/Marc-

I think Mark has a good point.  This bug has two parts.  Ultimately,
it's triggered by a buggy virtio-net implementation in the Ubuntu 8.04
kernel (as well as any others using the circa 2.6.25 virtio net code).
 The CVE should probably mention (or focus on) this too.

The qemu-kvm patch is still a good thing to do, as it shouldn't just
exit and terminate the VM, so that's needed as well.

:-Dustin

[Qemu-devel] Re: [PATCH] whitelist host virtio networking features [was Re: qemu-kvm-0.11 regression, crashes on older ...]

[Dustin Kirkland]

On Mon, Nov 2, 2009 at 8:38 AM, Mark McLoughlin <markmc@redhat.com> wrote:
> On Fri, 2009-10-30 at 16:15 -0500, Dustin Kirkland wrote:
>> Canonical's Ubuntu Security Team will be filing a CVE on this issue,
>> since there is a bit of an attack vector here, and since
>> qemu-kvm-0.11.0 is generally available as an official release (and now
>> part of Ubuntu 9.10).
>>
>> Guests running linux <= 2.6.25 virtio-net (e.g Ubuntu 8.04 hardy) on
>> top of qemu-kvm-0.11.0 can be remotely crashed by a non-privileged
>> network user flooding an open port on the guest.  The crash happens in
>> a manner that abruptly terminates the guest's execution (ie, without
>> shutting down cleanly).  This may affect the guest filesystem's
>> general happiness.
>
> IMHO, the CVE should be against the 2.6.25 virtio drivers - the bug is
> in the guest and the issue we're discussing here is just a hacky
> workaround for the guest bug.

Kees/Jamie/Marc-

I think Mark has a good point.  This bug has two parts.  Ultimately,
it's triggered by a buggy virtio-net implementation in the Ubuntu 8.04
kernel (as well as any others using the circa 2.6.25 virtio net code).
 The CVE should probably mention (or focus on) this too.

The qemu-kvm patch is still a good thing to do, as it shouldn't just
exit and terminate the VM, so that's needed as well.

:-Dustin

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Dustin Kirkland]

[-- Attachment #1: Type: text/plain, Size: 1741 bytes --]

On Thu, 2009-10-29 at 20:00 +0800, Scott Tsai wrote:
> Excerpts from Mark McLoughlin's message of Thu Oct 29 17:16:43 +0800 2009:
> > Assuming this is something like the virtio-net in 2.6.26, there was no
> > receivable buffers support so (as Scott points out) it must be that
> > we've read a packet from the tap device which is >1514 bytes (or >1524
> > bytes with IFF_VNET_HDR) but the guest has not supplied buffers which
> > are large enough to take it
> 
> > One thing to check is that the tap device is being initialized by
> > qemu-kvm using TUNSETOFFLOAD with either zero or TUN_F_CSUM - i.e. GSO
> > should not be enabled, because the guest cannot handle large GSO packets
> 
> > Another possibility is that the MTU on the bridge in the host is too
> > large and that's what's causing the large packets to be sent
> 
> Using Dustin's image, I see:
> 	virtio_net_set_features(features: 0x00000930)
> 	tap_set_offload(csum: 1, tso4: 1, tso6: 1, ecn: 1)
> being called and get an mtu of 1500 on virbr0 using his birdge.sh script.
> 
> virtio_net_receive2 was trying to transfer a 1534 byte packet (1524 'size' + 10 'virtio_net_hdr')
> and the guest only had 1524 bytes of space in its input descriptors.
> 
> BTW, I can also reproduce this running Dustin's image inside Fedora 11's qemu-0.10.6-9.fc11.x86_64.
> 
> The patch I posted earlier actually only applies to the 0.10 branch, here's a patch that compiles for 0.11:


Hi Scott,

Thanks for this.  Testing this, kvm doesn't crash.  And the guest has
working network connectivity, until I saturate the network connection
with nc.  At that point, the guest loses network connectivity all
together.  So the fix is not quite ideal, yet.

:-Dustin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Dustin Kirkland]

[-- Attachment #1: Type: text/plain, Size: 1741 bytes --]

On Thu, 2009-10-29 at 20:00 +0800, Scott Tsai wrote:
> Excerpts from Mark McLoughlin's message of Thu Oct 29 17:16:43 +0800 2009:
> > Assuming this is something like the virtio-net in 2.6.26, there was no
> > receivable buffers support so (as Scott points out) it must be that
> > we've read a packet from the tap device which is >1514 bytes (or >1524
> > bytes with IFF_VNET_HDR) but the guest has not supplied buffers which
> > are large enough to take it
> 
> > One thing to check is that the tap device is being initialized by
> > qemu-kvm using TUNSETOFFLOAD with either zero or TUN_F_CSUM - i.e. GSO
> > should not be enabled, because the guest cannot handle large GSO packets
> 
> > Another possibility is that the MTU on the bridge in the host is too
> > large and that's what's causing the large packets to be sent
> 
> Using Dustin's image, I see:
> 	virtio_net_set_features(features: 0x00000930)
> 	tap_set_offload(csum: 1, tso4: 1, tso6: 1, ecn: 1)
> being called and get an mtu of 1500 on virbr0 using his birdge.sh script.
> 
> virtio_net_receive2 was trying to transfer a 1534 byte packet (1524 'size' + 10 'virtio_net_hdr')
> and the guest only had 1524 bytes of space in its input descriptors.
> 
> BTW, I can also reproduce this running Dustin's image inside Fedora 11's qemu-0.10.6-9.fc11.x86_64.
> 
> The patch I posted earlier actually only applies to the 0.10 branch, here's a patch that compiles for 0.11:


Hi Scott,

Thanks for this.  Testing this, kvm doesn't crash.  And the guest has
working network connectivity, until I saturate the network connection
with nc.  At that point, the guest loses network connectivity all
together.  So the fix is not quite ideal, yet.

:-Dustin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Scott Tsai]

Hi, Dustin,
What's the easiest way to see the patches to qemu that Canonical
carries for the different Ubuntu releases?
(I think http://patches.ubuntu.com/ only diffs against Debian for the
last stable Ubuntu release?)

Also, is there a way for an outside developer to get email
notifications when a patch is added to Ubuntu's qemu package?

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Scott Tsai]

Hi, Dustin,
What's the easiest way to see the patches to qemu that Canonical
carries for the different Ubuntu releases?
(I think http://patches.ubuntu.com/ only diffs against Debian for the
last stable Ubuntu release?)

Also, is there a way for an outside developer to get email
notifications when a patch is added to Ubuntu's qemu package?

Re: qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Michael S. Tsirkin]

On Thu, Oct 29, 2009 at 09:16:43AM +0000, Mark McLoughlin wrote:
> I agree we shouldn't exit in this scenario

virtio in qemu generally seems to handle guest errors
by calling exit(2). This probably makes it easier to notice
the problems, but is likely not the right thing to do.

A simple way to tell guest there's a problem is by
writing a value > ring size into used ring.

-- 
MST

[Qemu-devel] Re: qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Michael S. Tsirkin]

On Thu, Oct 29, 2009 at 09:16:43AM +0000, Mark McLoughlin wrote:
> I agree we shouldn't exit in this scenario

virtio in qemu generally seems to handle guest errors
by calling exit(2). This probably makes it easier to notice
the problems, but is likely not the right thing to do.

A simple way to tell guest there's a problem is by
writing a value > ring size into used ring.

-- 
MST

Re: qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Avi Kivity]

On 10/29/2009 02:23 PM, Michael S. Tsirkin wrote:
> On Thu, Oct 29, 2009 at 09:16:43AM +0000, Mark McLoughlin wrote:
>    
>> I agree we shouldn't exit in this scenario
>>      
> virtio in qemu generally seems to handle guest errors
> by calling exit(2). This probably makes it easier to notice
> the problems, but is likely not the right thing to do.
>    

Right, the thinking was the guest is shooting itself in the foot and 
hitting, but a guest can delegate control of a device to unprivileged 
code (for example device assignment in kvm), which would allow this 
unprivileged code to kill the guest.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Avi Kivity]

On 10/29/2009 02:23 PM, Michael S. Tsirkin wrote:
> On Thu, Oct 29, 2009 at 09:16:43AM +0000, Mark McLoughlin wrote:
>    
>> I agree we shouldn't exit in this scenario
>>      
> virtio in qemu generally seems to handle guest errors
> by calling exit(2). This probably makes it easier to notice
> the problems, but is likely not the right thing to do.
>    

Right, the thinking was the guest is shooting itself in the foot and 
hitting, but a guest can delegate control of a device to unprivileged 
code (for example device assignment in kvm), which would allow this 
unprivileged code to kill the guest.

-- 
error compiling committee.c: too many arguments to function

Re: qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Michael S. Tsirkin]

On Thu, Oct 29, 2009 at 04:38:22PM +0200, Avi Kivity wrote:
> On 10/29/2009 02:23 PM, Michael S. Tsirkin wrote:
>> On Thu, Oct 29, 2009 at 09:16:43AM +0000, Mark McLoughlin wrote:
>>    
>>> I agree we shouldn't exit in this scenario
>>>      
>> virtio in qemu generally seems to handle guest errors
>> by calling exit(2). This probably makes it easier to notice
>> the problems, but is likely not the right thing to do.
>>    
>
> Right, the thinking was the guest is shooting itself in the foot and  
> hitting, but a guest can delegate control of a device to unprivileged  
> code (for example device assignment in kvm),

When we emulate iommu, yes.

> which would allow this unprivileged code to kill the guest.

With usb emulation, we can have:
drivers/usb/class/usblp.c:343:static const char *usblp_messages[] = {
"ok", "out of paper", "off-line", "on fire" };

> -- 
> error compiling committee.c: too many arguments to function

[Qemu-devel] Re: qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Michael S. Tsirkin]

On Thu, Oct 29, 2009 at 04:38:22PM +0200, Avi Kivity wrote:
> On 10/29/2009 02:23 PM, Michael S. Tsirkin wrote:
>> On Thu, Oct 29, 2009 at 09:16:43AM +0000, Mark McLoughlin wrote:
>>    
>>> I agree we shouldn't exit in this scenario
>>>      
>> virtio in qemu generally seems to handle guest errors
>> by calling exit(2). This probably makes it easier to notice
>> the problems, but is likely not the right thing to do.
>>    
>
> Right, the thinking was the guest is shooting itself in the foot and  
> hitting, but a guest can delegate control of a device to unprivileged  
> code (for example device assignment in kvm),

When we emulate iommu, yes.

> which would allow this unprivileged code to kill the guest.

With usb emulation, we can have:
drivers/usb/class/usblp.c:343:static const char *usblp_messages[] = {
"ok", "out of paper", "off-line", "on fire" };

> -- 
> error compiling committee.c: too many arguments to function

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Dustin Kirkland]

[-- Attachment #1.1: Type: text/plain, Size: 820 bytes --]

On Thu, 2009-10-29 at 09:16 +0000, Mark McLoughlin wrote:
> Hi Dustin,
> 
> On Wed, 2009-10-28 at 14:22 -0500, Dustin Kirkland wrote:
> > I believe that we have identified a regression in qemu-kvm-0.11.0.
> 
> Regression versus which previous version of qemu-kvm?

Okay, sorry for the ambiguity.  I actually had to clarify this for
myself.  The regression is as compared to the kvm-84 package that the
previous version of Ubuntu (9.04 Jaunty) carried.

In this package, we carried the attached patch from Anthony Liguori.

I had incorrectly assumed that this patch made it into the upstream
tree.  As Anthony stated in his earlier email, a different
implementation of the fix from Rusty was committed instead.  The fix as
committed doesn't quite solve the problem as we're experiencing it.

:-Dustin

[-- Attachment #1.2: virtio-net_disable_gso.patch --]
[-- Type: text/x-patch, Size: 1472 bytes --]

Work around broken virtio drivers in 2.6.26

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c
index 9bce3a0..5b615f9 100644
--- a/qemu/hw/virtio-net.c
+++ b/qemu/hw/virtio-net.c
@@ -120,6 +120,9 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev)
 
     if (tap_has_vnet_hdr(host)) {
         tap_using_vnet_hdr(host, 1);
+#if 0
+        /* Stop advertising advanced features until we work around the fact
+         * that this is totally broken in 2.6.26 kernels */
         features |= (1 << VIRTIO_NET_F_CSUM);
         features |= (1 << VIRTIO_NET_F_GUEST_CSUM);
         features |= (1 << VIRTIO_NET_F_GUEST_TSO4);
@@ -130,6 +133,7 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev)
         features |= (1 << VIRTIO_NET_F_HOST_ECN);
         features |= (1 << VIRTIO_NET_F_MRG_RXBUF);
         /* Kernel can't actually handle UFO in software currently. */
+#endif
     }
 #endif
 
@@ -374,8 +378,14 @@ static int receive_header(VirtIONet *n, struct iovec *iov, int iovcnt,
     struct virtio_net_hdr *hdr = iov[0].iov_base;
     int offset = 0;
 
+#if 0
     hdr->flags = 0;
     hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
+#else
+    /* we need to clear out the whole header, including any garbage that may be
+     */
+    memset(hdr, 0, sizeof(*hdr));
+#endif
 
 #ifdef TAP_VNET_HDR
     if (tap_has_vnet_hdr(n->vc->vlan->first_client)) {

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] qemu-kvm-0.11 regression, crashes on older guests with virtio network

[Dustin Kirkland]

[-- Attachment #1.1: Type: text/plain, Size: 820 bytes --]

On Thu, 2009-10-29 at 09:16 +0000, Mark McLoughlin wrote:
> Hi Dustin,
> 
> On Wed, 2009-10-28 at 14:22 -0500, Dustin Kirkland wrote:
> > I believe that we have identified a regression in qemu-kvm-0.11.0.
> 
> Regression versus which previous version of qemu-kvm?

Okay, sorry for the ambiguity.  I actually had to clarify this for
myself.  The regression is as compared to the kvm-84 package that the
previous version of Ubuntu (9.04 Jaunty) carried.

In this package, we carried the attached patch from Anthony Liguori.

I had incorrectly assumed that this patch made it into the upstream
tree.  As Anthony stated in his earlier email, a different
implementation of the fix from Rusty was committed instead.  The fix as
committed doesn't quite solve the problem as we're experiencing it.

:-Dustin

[-- Attachment #1.2: virtio-net_disable_gso.patch --]
[-- Type: text/x-patch, Size: 1472 bytes --]

Work around broken virtio drivers in 2.6.26

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c
index 9bce3a0..5b615f9 100644
--- a/qemu/hw/virtio-net.c
+++ b/qemu/hw/virtio-net.c
@@ -120,6 +120,9 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev)
 
     if (tap_has_vnet_hdr(host)) {
         tap_using_vnet_hdr(host, 1);
+#if 0
+        /* Stop advertising advanced features until we work around the fact
+         * that this is totally broken in 2.6.26 kernels */
         features |= (1 << VIRTIO_NET_F_CSUM);
         features |= (1 << VIRTIO_NET_F_GUEST_CSUM);
         features |= (1 << VIRTIO_NET_F_GUEST_TSO4);
@@ -130,6 +133,7 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev)
         features |= (1 << VIRTIO_NET_F_HOST_ECN);
         features |= (1 << VIRTIO_NET_F_MRG_RXBUF);
         /* Kernel can't actually handle UFO in software currently. */
+#endif
     }
 #endif
 
@@ -374,8 +378,14 @@ static int receive_header(VirtIONet *n, struct iovec *iov, int iovcnt,
     struct virtio_net_hdr *hdr = iov[0].iov_base;
     int offset = 0;
 
+#if 0
     hdr->flags = 0;
     hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
+#else
+    /* we need to clear out the whole header, including any garbage that may be
+     */
+    memset(hdr, 0, sizeof(*hdr));
+#endif
 
 #ifdef TAP_VNET_HDR
     if (tap_has_vnet_hdr(n->vc->vlan->first_client)) {

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]