From mboxrd@z Thu Jan  1 00:00:00 1970
From: Maxime Coquelin <maxime.coquelin@redhat.com>
Subject: Re: [PATCH v2 2/2] Revert "net/virtio-user: fail if cq
 used with packed vq"
Date: Thu, 10 Jan 2019 16:01:09 +0100
Message-ID: <8dcb3ae2-c2dc-3bb4-e7d0-9ec0948ec061@redhat.com>
References: <20190110131751.32670-1-jfreimann@redhat.com>
 <20190110131751.32670-3-jfreimann@redhat.com>
 <20190110144024.GA27794@dpdk-tbie.sh.intel.com>
 <20190110145857.6ofzht7npb3zheun@jenstp.localdomain>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Cc: dev@dpdk.org
To: Jens Freimann <jfreimann@redhat.com>, Tiwei Bie <tiwei.bie@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28])
 by dpdk.org (Postfix) with ESMTP id 6C17C1B87F
 for <dev@dpdk.org>; Thu, 10 Jan 2019 16:01:33 +0100 (CET)
In-Reply-To: <20190110145857.6ofzht7npb3zheun@jenstp.localdomain>
Content-Language: en-US
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>



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 <jfreimann@redhat.com>
>>> ---
>>>  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
>>>