[v2,2/2] Revert "net/virtio-user: fail if cq used with packed vq"
Checks
Commit Message
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(-)
Comments
On 1/10/19 2:17 PM, 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
Maybe good to explain why it can be reverted now.
>
> 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(-)
>
With the clarification in commit message:
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
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.
> - } 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:
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
>
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.
>
>> - } 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
>>
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
>>>
@@ -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;
- }
- } else {
+ if (packed_vq)
+ dev->device_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);