From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33492) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z8Akf-0000yW-Vj for qemu-devel@nongnu.org; Thu, 25 Jun 2015 13:15:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z8Aka-0001N5-3o for qemu-devel@nongnu.org; Thu, 25 Jun 2015 13:15:49 -0400 Received: from mail-wi0-x233.google.com ([2a00:1450:400c:c05::233]:34073) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z8AkZ-0001Ml-Qj for qemu-devel@nongnu.org; Thu, 25 Jun 2015 13:15:44 -0400 Received: by wicnd19 with SMTP id nd19so24116470wic.1 for ; Thu, 25 Jun 2015 10:15:43 -0700 (PDT) Message-ID: <558C373B.20609@gmail.com> Date: Thu, 25 Jun 2015 20:15:39 +0300 From: Marcel Apfelbaum MIME-Version: 1.0 References: <1434650744-6585-1-git-send-email-marcel@redhat.com> <20150623115705-mutt-send-email-mst@redhat.com> <5589338E.80304@redhat.com> <20150623125126-mutt-send-email-mst@redhat.com> In-Reply-To: <20150623125126-mutt-send-email-mst@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] net/virtio: fix multi-queue negotiation Reply-To: marcel@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" , Marcel Apfelbaum Cc: famz@redhat.com, qemu-devel@nongnu.org, n.nikolaev@virtualopensystems.com On 06/23/2015 01:56 PM, Michael S. Tsirkin wrote: > On Tue, Jun 23, 2015 at 01:23:10PM +0300, Marcel Apfelbaum wrote: >> On 06/23/2015 12:57 PM, Michael S. Tsirkin wrote: >>> On Thu, Jun 18, 2015 at 09:05:44PM +0300, Marcel Apfelbaum wrote: >>>> Clear host multi-queue related features if the peer >>>> doesn't support it. >>>> >>>> Signed-off-by: Marcel Apfelbaum >>>> --- >>>> Notes: >>>> This fixes a guest CPU soft lock, however the virtio-net >>>> device will not work correctly. It seems that is >>>> peer's "fault", not knowing how to handle the situation. >>>> However, I submit this patch since it corrects the negotiation >>>> and saves us from a guest crash (!). >>>> >>>> Any ideas from the virtio/multi-queue developers on how to debug >>>> this further are welcomed. >>>> >>>> hw/net/virtio-net.c | 13 ++++++++++++- >>>> 1 file changed, 12 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >>>> index 9281aa1..63e59e8 100644 >>>> --- a/hw/net/virtio-net.c >>>> +++ b/hw/net/virtio-net.c >>>> @@ -367,6 +367,11 @@ static int peer_has_ufo(VirtIONet *n) >>>> return n->has_ufo; >>>> } >>>> >>>> +static int peer_has_multiqueue(VirtIONet *n) >>>> +{ >>>> + return n->multiqueue; >>>> +} >>>> + >>>> static void virtio_net_set_mrg_rx_bufs(VirtIONet *n, int mergeable_rx_bufs, >>>> int version_1) >>>> { >>>> @@ -469,6 +474,13 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features) >>>> virtio_clear_feature(&features, VIRTIO_NET_F_HOST_UFO); >>>> } >>>> >>>> + if (!peer_has_multiqueue(n)) { >>>> + virtio_clear_feature(&features, VIRTIO_NET_F_MQ); >>>> + virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_ANNOUNCE); >>>> + virtio_clear_feature(&features, VIRTIO_NET_F_CTRL_VQ); >>>> + virtio_clear_feature(&features, VIRTIO_NET_F_CTRL_RX); >>>> + } >>>> + >>>> if (!get_vhost_net(nc->peer)) { >>>> virtio_add_feature(&features, VIRTIO_F_VERSION_1); >>>> return features; >>> >>> _MQ makes sense but what about the rest? Why clear them? >> Guest's virtio driver doesn't like if MQ is off and any of the other flags remains ON. >> It has a specific check for each of them and fails the loading if not all are off. >> >> Thanks, >> Marcel > > > I think you are mistaken. > > if (!virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ) && > (VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_RX, > "VIRTIO_NET_F_CTRL_VQ") || > VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_VLAN, > "VIRTIO_NET_F_CTRL_VQ") || > VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_GUEST_ANNOUNCE, > "VIRTIO_NET_F_CTRL_VQ") || > VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") || > VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR, > "VIRTIO_NET_F_CTRL_VQ"))) { > return false; > } > > return true; > > So if CTRL VQ is not set, you can not have MQ or any of the others. > Hmm, I know what saw, but I don't fully understand why is happening. I'll give another try and I'll return with more info. Thanks, Marcel > > >>> >>>> @@ -1314,7 +1326,6 @@ static void virtio_net_tx_bh(void *opaque) >>>> static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue) >>>> { >>>> n->multiqueue = multiqueue; >>>> - >>>> virtio_net_set_queues(n); >>>> } >>>> >>>> -- >>>> 2.1.0 >