diff mbox

[dpdk-dev,v3,01/21] net/virtio: by default disable packed virtqueues

Message ID 20180405101031.26468-2-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 
  Disable packed virtqueues for now and make it dependend on a build-time
config option. This can be reverted once we have missing features like
indirect descriptors implemented.

Signed-off-by: Jens Freimann <jfreimann@redhat.com>
---
 config/common_base                 | 1 +
 drivers/net/virtio/virtio_ethdev.c | 4 ++++
 2 files changed, 5 insertions(+)

Comments

Maxime Coquelin April 5, 2018, 1:42 p.m. UTC | #1 
On 04/05/2018 12:10 PM, Jens Freimann wrote:
> Disable packed virtqueues for now and make it dependend on a build-time
> config option. This can be reverted once we have missing features like
> indirect descriptors implemented.
> 
> Signed-off-by: Jens Freimann <jfreimann@redhat.com>
> ---
>   config/common_base                 | 1 +
>   drivers/net/virtio/virtio_ethdev.c | 4 ++++
>   2 files changed, 5 insertions(+)
> 
> diff --git a/config/common_base b/config/common_base
> index c09c7cf88..cd4b419b4 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -346,6 +346,7 @@ CONFIG_RTE_LIBRTE_VIRTIO_PMD=y
>   CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_RX=n
>   CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_TX=n
>   CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DUMP=n
> +CONFIG_RTE_LIBRTE_VIRTIO_PQ=n
>   
>   #
>   # Compile virtio device emulation inside virtio PMD driver
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index 11f758929..06fbf7311 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -1149,6 +1149,10 @@ virtio_negotiate_features(struct virtio_hw *hw, uint64_t req_features)
>   			req_features &= ~(1ULL << VIRTIO_NET_F_MTU);
>   	}
>   
> +#ifndef RTE_LIBRTE_VIRTIO_PQ
> +	req_features &= ~(1ull << VIRTIO_F_RING_PACKED);
Is VIRTIO_F_RING_PACKED already declared here?
I don't think it is, so would break build when bisecting.

Maybe the thing to do is to not have VIRTIO_F_RING_PACKED in the feature 
set by default. And when RTE_LIBRTE_VIRTIO_PQ is set, enable it and
explicitly disable indirect descs.

> +#endif
> +
>   	/*
>   	 * Negotiate features: Subset of device feature bits are written back
>   	 * guest feature bits.
>
Jens Freimann April 5, 2018, 2:19 p.m. UTC | #2 
On Thu, Apr 05, 2018 at 03:42:47PM +0200, Maxime Coquelin wrote:
>
>
>On 04/05/2018 12:10 PM, Jens Freimann wrote:
>>Disable packed virtqueues for now and make it dependend on a build-time
>>config option. This can be reverted once we have missing features like
>>indirect descriptors implemented.
>>
>>Signed-off-by: Jens Freimann <jfreimann@redhat.com>
>>---
>>  config/common_base                 | 1 +
>>  drivers/net/virtio/virtio_ethdev.c | 4 ++++
>>  2 files changed, 5 insertions(+)
>>
>>diff --git a/config/common_base b/config/common_base
>>index c09c7cf88..cd4b419b4 100644
>>--- a/config/common_base
>>+++ b/config/common_base
>>@@ -346,6 +346,7 @@ CONFIG_RTE_LIBRTE_VIRTIO_PMD=y
>>  CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_RX=n
>>  CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_TX=n
>>  CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DUMP=n
>>+CONFIG_RTE_LIBRTE_VIRTIO_PQ=n
>>  #
>>  # Compile virtio device emulation inside virtio PMD driver
>>diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
>>index 11f758929..06fbf7311 100644
>>--- a/drivers/net/virtio/virtio_ethdev.c
>>+++ b/drivers/net/virtio/virtio_ethdev.c
>>@@ -1149,6 +1149,10 @@ virtio_negotiate_features(struct virtio_hw *hw, uint64_t req_features)
>>  			req_features &= ~(1ULL << VIRTIO_NET_F_MTU);
>>  	}
>>+#ifndef RTE_LIBRTE_VIRTIO_PQ
>>+	req_features &= ~(1ull << VIRTIO_F_RING_PACKED);
>Is VIRTIO_F_RING_PACKED already declared here?
>I don't think it is, so would break build when bisecting.

yes, you are right it is not. I pushed this patch to far up during git
rebase. 
>
>Maybe the thing to do is to not have VIRTIO_F_RING_PACKED in the 
>feature set by default. And when RTE_LIBRTE_VIRTIO_PQ is set, enable 
>it and
>explicitly disable indirect descs.

Yes, both would work. I'll go with your suggestion.
Thanks!

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

Patch

diff --git a/config/common_base b/config/common_base
index c09c7cf88..cd4b419b4 100644
--- a/config/common_base
+++ b/config/common_base
@@ -346,6 +346,7 @@  CONFIG_RTE_LIBRTE_VIRTIO_PMD=y
 CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_RX=n
 CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_TX=n
 CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DUMP=n
+CONFIG_RTE_LIBRTE_VIRTIO_PQ=n
 
 #
 # Compile virtio device emulation inside virtio PMD driver
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 11f758929..06fbf7311 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1149,6 +1149,10 @@  virtio_negotiate_features(struct virtio_hw *hw, uint64_t req_features)
 			req_features &= ~(1ULL << VIRTIO_NET_F_MTU);
 	}
 
+#ifndef RTE_LIBRTE_VIRTIO_PQ
+	req_features &= ~(1ull << VIRTIO_F_RING_PACKED);
+#endif
+
 	/*
 	 * Negotiate features: Subset of device feature bits are written back
 	 * guest feature bits.