diff mbox

[dpdk-dev,v3,17/21] net/virtio: disable ctrl virtqueue for packed rings

Message ID 20180405101031.26468-18-jfreimann@redhat.com (mailing list archive)
State Changes Requested, archived
Delegated to: Maxime Coquelin
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Jens Freimann April 5, 2018, 10:10 a.m. UTC 
  Signed-off-by: Jens Freiman <jfreimann@redhat.com>
---
 drivers/net/virtio/virtio_ethdev.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Maxime Coquelin April 6, 2018, 9:38 a.m. UTC | #1 
On 04/05/2018 12:10 PM, Jens Freimann wrote:
> Signed-off-by: Jens Freiman <jfreimann@redhat.com>
> ---
>   drivers/net/virtio/virtio_ethdev.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index dc220c743..7367d9c5d 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -1157,6 +1157,13 @@ virtio_negotiate_features(struct virtio_hw *hw, uint64_t req_features)
>   	req_features &= ~(1ull << VIRTIO_F_RING_PACKED);
>   #endif
>   
> +	if (req_features & (1ULL << VIRTIO_F_RING_PACKED)) {
> +		req_features &= ~(1ull << VIRTIO_NET_F_CTRL_MAC_ADDR);
> +		req_features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ);
> +		req_features &= ~(1ull << VIRTIO_NET_F_CTRL_RX);
> +		req_features &= ~(1ull << VIRTIO_NET_F_CTRL_VLAN);
> +	}
> +

Does packed ring not support ctrl vqs, or is it just a workaround while
it is implemented?

>   	/*
>   	 * Negotiate features: Subset of device feature bits are written back
>   	 * guest feature bits.
>
Jens Freimann April 6, 2018, 9:43 a.m. UTC | #2 
On Fri, Apr 06, 2018 at 11:38:50AM +0200, Maxime Coquelin wrote:
>
>
>On 04/05/2018 12:10 PM, Jens Freimann wrote:
>>Signed-off-by: Jens Freiman <jfreimann@redhat.com>
>>---
>>  drivers/net/virtio/virtio_ethdev.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>>diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
>>index dc220c743..7367d9c5d 100644
>>--- a/drivers/net/virtio/virtio_ethdev.c
>>+++ b/drivers/net/virtio/virtio_ethdev.c
>>@@ -1157,6 +1157,13 @@ virtio_negotiate_features(struct virtio_hw *hw, uint64_t req_features)
>>  	req_features &= ~(1ull << VIRTIO_F_RING_PACKED);
>>  #endif
>>+	if (req_features & (1ULL << VIRTIO_F_RING_PACKED)) {
>>+		req_features &= ~(1ull << VIRTIO_NET_F_CTRL_MAC_ADDR);
>>+		req_features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ);
>>+		req_features &= ~(1ull << VIRTIO_NET_F_CTRL_RX);
>>+		req_features &= ~(1ull << VIRTIO_NET_F_CTRL_VLAN);
>>+	}
>>+
>
>Does packed ring not support ctrl vqs, or is it just a workaround while
>it is implemented?

packed queues support virtqueues, but I had not implemented it yet. I
have a patch for it though and will include it in v4.

regards,
Jens 
>
>>  	/*
>>  	 * Negotiate features: Subset of device feature bits are written back
>>  	 * guest feature bits.
>>
Maxime Coquelin April 6, 2018, 9:48 a.m. UTC | #3 
On 04/06/2018 11:43 AM, Jens Freimann wrote:
> On Fri, Apr 06, 2018 at 11:38:50AM +0200, Maxime Coquelin wrote:
>>
>>
>> On 04/05/2018 12:10 PM, Jens Freimann wrote:
>>> Signed-off-by: Jens Freiman <jfreimann@redhat.com>
>>> ---
>>>  drivers/net/virtio/virtio_ethdev.c | 7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/net/virtio/virtio_ethdev.c 
>>> b/drivers/net/virtio/virtio_ethdev.c
>>> index dc220c743..7367d9c5d 100644
>>> --- a/drivers/net/virtio/virtio_ethdev.c
>>> +++ b/drivers/net/virtio/virtio_ethdev.c
>>> @@ -1157,6 +1157,13 @@ virtio_negotiate_features(struct virtio_hw 
>>> *hw, uint64_t req_features)
>>>      req_features &= ~(1ull << VIRTIO_F_RING_PACKED);
>>>  #endif
>>> +    if (req_features & (1ULL << VIRTIO_F_RING_PACKED)) {
>>> +        req_features &= ~(1ull << VIRTIO_NET_F_CTRL_MAC_ADDR);
>>> +        req_features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ);
>>> +        req_features &= ~(1ull << VIRTIO_NET_F_CTRL_RX);
>>> +        req_features &= ~(1ull << VIRTIO_NET_F_CTRL_VLAN);
>>> +    }
>>> +
>>
>> Does packed ring not support ctrl vqs, or is it just a workaround while
>> it is implemented?
> 
> packed queues support virtqueues, but I had not implemented it yet. I
> have a patch for it though and will include it in v4.

Great, so if you hadn't the patch, I think that a comment would be
welcome. But it does not apply as you have the patch in your queue!

Thanks,
Maxime

> 
> regards,
> Jens
>>
>>>      /*
>>>       * Negotiate features: Subset of device feature bits are written 
>>> back
>>>       * guest feature bits.
>>>
diff mbox

Patch

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index dc220c743..7367d9c5d 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1157,6 +1157,13 @@  virtio_negotiate_features(struct virtio_hw *hw, uint64_t req_features)
 	req_features &= ~(1ull << VIRTIO_F_RING_PACKED);
 #endif
 
+	if (req_features & (1ULL << VIRTIO_F_RING_PACKED)) {
+		req_features &= ~(1ull << VIRTIO_NET_F_CTRL_MAC_ADDR);
+		req_features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ);
+		req_features &= ~(1ull << VIRTIO_NET_F_CTRL_RX);
+		req_features &= ~(1ull << VIRTIO_NET_F_CTRL_VLAN);
+	}
+
 	/*
 	 * Negotiate features: Subset of device feature bits are written back
 	 * guest feature bits.