* [Qemu-devel] [PATCH] virtio-net: unbreak the minix guest
@ 2013-04-25 6:19 Jason Wang
2013-04-25 6:57 ` Michael S. Tsirkin
0 siblings, 1 reply; 3+ messages in thread
From: Jason Wang @ 2013-04-25 6:19 UTC (permalink / raw)
To: aliguori, qemu-devel; +Cc: Jason Wang, Aurelien Jarno, mst
Multiqueue patchset conditionally add control vq only when guest negotiate the
feature. Though the spec is not clear on this but it breaks the minix guest
since it will identify the ctrl vq even if it does not support it. Though this
behavior seems a violation on the spec "If the VIRTIO_NET_F_CTRL_VQ feature bit
is negotiated, identify the control virtqueue.", to keep the backward
compatibility, always add the ctrl vq at end of the queues.
Also remove the meaningless ctrl_vq initialization and vq deletion.
Reported-by: Aurelien Jarno <aurelien@aurel32.net>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/virtio-net.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 4d2cdd2..1068f4e 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1045,7 +1045,7 @@ static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl)
n->multiqueue = multiqueue;
- for (i = 2; i <= n->max_queues * 2 + 1; i++) {
+ for (i = 2; i <= n->max_queues * 2; i++) {
virtio_del_queue(vdev, i);
}
@@ -1067,9 +1067,9 @@ static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl)
n->vqs[i].n = n;
}
- if (ctrl) {
- n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl);
- }
+ /* Some guest (e.g minix) may identifiy ctrl vq even if it does not
+ * support. */
+ n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl);
virtio_net_set_queues(n);
}
@@ -1317,7 +1317,6 @@ static int virtio_net_device_init(VirtIODevice *vdev)
virtio_net_handle_tx_bh);
n->vqs[0].tx_bh = qemu_bh_new(virtio_net_tx_bh, &n->vqs[0]);
}
- n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl);
qemu_macaddr_default_if_unset(&n->nic_conf.macaddr);
memcpy(&n->mac[0], &n->nic_conf.macaddr, sizeof(n->mac));
n->status = VIRTIO_NET_S_LINK_UP;
--
1.7.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] virtio-net: unbreak the minix guest
2013-04-25 6:19 [Qemu-devel] [PATCH] virtio-net: unbreak the minix guest Jason Wang
@ 2013-04-25 6:57 ` Michael S. Tsirkin
2013-04-25 7:02 ` Jason Wang
0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2013-04-25 6:57 UTC (permalink / raw)
To: Jason Wang; +Cc: aliguori, qemu-devel, Aurelien Jarno
On Thu, Apr 25, 2013 at 02:19:53PM +0800, Jason Wang wrote:
> Multiqueue patchset conditionally add control vq only when guest negotiate the
> feature. Though the spec is not clear on this but it breaks the minix guest
> since it will identify the ctrl vq even if it does not support it. Though this
> behavior seems a violation on the spec "If the VIRTIO_NET_F_CTRL_VQ feature bit
> is negotiated, identify the control virtqueue.", to keep the backward
> compatibility, always add the ctrl vq at end of the queues.
>
> Also remove the meaningless ctrl_vq initialization and vq deletion.
>
> Reported-by: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> hw/net/virtio-net.c | 9 ++++-----
> 1 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 4d2cdd2..1068f4e 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1045,7 +1045,7 @@ static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl)
>
> n->multiqueue = multiqueue;
>
> - for (i = 2; i <= n->max_queues * 2 + 1; i++) {
> + for (i = 2; i <= n->max_queues * 2; i++) {
> virtio_del_queue(vdev, i);
> }
>
Unrelated cleanup? Separate patch pls.
> @@ -1067,9 +1067,9 @@ static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl)
> n->vqs[i].n = n;
> }
>
> - if (ctrl) {
> - n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl);
> - }
> + /* Some guest (e.g minix) may identifiy ctrl vq even if it does not
> + * support. */
/*
* Note: Minux Guests (version XYZ) use ctrl vq but don't ack
* VIRTIO_NET_F_CTRL_VQ. Create ctrl vq unconditionally to avoid
* breaking them.
*/
> + n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl);
>
> virtio_net_set_queues(n);
> }
> @@ -1317,7 +1317,6 @@ static int virtio_net_device_init(VirtIODevice *vdev)
> virtio_net_handle_tx_bh);
> n->vqs[0].tx_bh = qemu_bh_new(virtio_net_tx_bh, &n->vqs[0]);
> }
> - n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl);
Why drop this here? Unrelated cleanup? Separate patch pls.
> qemu_macaddr_default_if_unset(&n->nic_conf.macaddr);
> memcpy(&n->mac[0], &n->nic_conf.macaddr, sizeof(n->mac));
> n->status = VIRTIO_NET_S_LINK_UP;
> --
> 1.7.1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] virtio-net: unbreak the minix guest
2013-04-25 6:57 ` Michael S. Tsirkin
@ 2013-04-25 7:02 ` Jason Wang
0 siblings, 0 replies; 3+ messages in thread
From: Jason Wang @ 2013-04-25 7:02 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: aliguori, qemu-devel, Aurelien Jarno
On 04/25/2013 02:57 PM, Michael S. Tsirkin wrote:
> On Thu, Apr 25, 2013 at 02:19:53PM +0800, Jason Wang wrote:
>> Multiqueue patchset conditionally add control vq only when guest negotiate the
>> feature. Though the spec is not clear on this but it breaks the minix guest
>> since it will identify the ctrl vq even if it does not support it. Though this
>> behavior seems a violation on the spec "If the VIRTIO_NET_F_CTRL_VQ feature bit
>> is negotiated, identify the control virtqueue.", to keep the backward
>> compatibility, always add the ctrl vq at end of the queues.
>>
>> Also remove the meaningless ctrl_vq initialization and vq deletion.
>>
>> Reported-by: Aurelien Jarno <aurelien@aurel32.net>
>> Cc: Aurelien Jarno <aurelien@aurel32.net>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> hw/net/virtio-net.c | 9 ++++-----
>> 1 files changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index 4d2cdd2..1068f4e 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -1045,7 +1045,7 @@ static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl)
>>
>> n->multiqueue = multiqueue;
>>
>> - for (i = 2; i <= n->max_queues * 2 + 1; i++) {
>> + for (i = 2; i <= n->max_queues * 2; i++) {
>> virtio_del_queue(vdev, i);
>> }
>>
> Unrelated cleanup? Separate patch pls.
Ok.
>
>> @@ -1067,9 +1067,9 @@ static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl)
>> n->vqs[i].n = n;
>> }
>>
>> - if (ctrl) {
>> - n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl);
>> - }
>> + /* Some guest (e.g minix) may identifiy ctrl vq even if it does not
>> + * support. */
> /*
> * Note: Minux Guests (version XYZ) use ctrl vq but don't ack
> * VIRTIO_NET_F_CTRL_VQ. Create ctrl vq unconditionally to avoid
> * breaking them.
> */
Sure.
>> + n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl);
>>
>> virtio_net_set_queues(n);
>> }
>> @@ -1317,7 +1317,6 @@ static int virtio_net_device_init(VirtIODevice *vdev)
>> virtio_net_handle_tx_bh);
>> n->vqs[0].tx_bh = qemu_bh_new(virtio_net_tx_bh, &n->vqs[0]);
>> }
>> - n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl);
> Why drop this here? Unrelated cleanup? Separate patch pls.
Ok, will send v2. Thanks
>> qemu_macaddr_default_if_unset(&n->nic_conf.macaddr);
>> memcpy(&n->mac[0], &n->nic_conf.macaddr, sizeof(n->mac));
>> n->status = VIRTIO_NET_S_LINK_UP;
>> --
>> 1.7.1
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-04-25 7:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-25 6:19 [Qemu-devel] [PATCH] virtio-net: unbreak the minix guest Jason Wang
2013-04-25 6:57 ` Michael S. Tsirkin
2013-04-25 7:02 ` Jason Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).