From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 6C17C1B87F for ; Thu, 10 Jan 2019 16:01:33 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7ACCD8E67D; Thu, 10 Jan 2019 15:01:32 +0000 (UTC) Received: from [10.36.112.17] (ovpn-112-17.ams2.redhat.com [10.36.112.17]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 083257B8E2; Thu, 10 Jan 2019 15:01:11 +0000 (UTC) To: Jens Freimann , Tiwei Bie Cc: dev@dpdk.org References: <20190110131751.32670-1-jfreimann@redhat.com> <20190110131751.32670-3-jfreimann@redhat.com> <20190110144024.GA27794@dpdk-tbie.sh.intel.com> <20190110145857.6ofzht7npb3zheun@jenstp.localdomain> From: Maxime Coquelin Message-ID: <8dcb3ae2-c2dc-3bb4-e7d0-9ec0948ec061@redhat.com> Date: Thu, 10 Jan 2019 16:01:09 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 MIME-Version: 1.0 In-Reply-To: <20190110145857.6ofzht7npb3zheun@jenstp.localdomain> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Thu, 10 Jan 2019 15:01:32 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH v2 2/2] Revert "net/virtio-user: fail if cq used with packed vq" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Jan 2019 15:01:33 -0000 On 1/10/19 3:58 PM, Jens Freimann wrote: > On Thu, Jan 10, 2019 at 10:40:25PM +0800, Tiwei Bie wrote: >> On Thu, Jan 10, 2019 at 02:17:51PM +0100, Jens Freimann wrote: >>> This reverts commit 5e4e7a7524a30c176bd6b1789ab30963f27f2681. >>> >>> Not a clean revert, I had to resolve a conflict due to >>>   616ea5519 net/virtio-user: fix packed vq option parsing >>> >>> Signed-off-by: Jens Freimann >>> --- >>>  drivers/net/virtio/virtio_user/virtio_user_dev.c | 11 +++-------- >>>  1 file changed, 3 insertions(+), 8 deletions(-) >>> >>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c >>> b/drivers/net/virtio/virtio_user/virtio_user_dev.c >>> index 49fcf48b9..b7059cb1e 100644 >>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c >>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c >>> @@ -478,15 +478,10 @@ virtio_user_dev_init(struct virtio_user_dev >>> *dev, char *path, int queues, >>>      if (!in_order) >>>          dev->unsupported_features |= (1ull << VIRTIO_F_IN_ORDER); >>> >>> -    if (packed_vq) { >>> -        if (cq) { >>> -            PMD_INIT_LOG(ERR, "control vq not supported yet with " >>> -                      "packed virtqueues\n"); >>> -            return -1; >>> -        } >> >> I think it's natural to drop above code in the same patch that >> introduces the control vq support. > > I just split it out because Maxime requested it. Yeah, disagreements happen. :) I understand this is not a clean revert anyway, so feel free to keep it in a single patch. Thanks, Maxime >> >>> -    } else { >>> +    if (packed_vq) >>> +        dev->device_features |= (1ull << VIRTIO_F_RING_PACKED); >> >> We shouldn't add this bit into device_features like this. >> We just need this: > > ok > > regards, > Jens >> >>     if (!packed_vq) >>         dev->unsupported_features |= (1ull << VIRTIO_F_RING_PACKED); >> >> >>> +    else >>>          dev->unsupported_features |= (1ull << VIRTIO_F_RING_PACKED); >>> -    } >>> >>>      if (dev->mac_specified) >>>          dev->frontend_features |= (1ull << VIRTIO_NET_F_MAC); >>> -- >>> 2.17.2 >>>