diff mbox series

[v5,06/11] net/virtio-user: add option to use packed queues

Message ID 20180906181947.20646-7-jfreimann@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series implement packed virtqueues |

Checks

Context Check Description
ci/Intel-compilation success Compilation OK

Commit Message

Jens Freimann Sept. 6, 2018, 6:19 p.m. UTC 
  From: Yuanhan Liu <yuanhan.liu@linux.intel.com>

Add option to enable packed queue support for virtio-user
devices.

Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 drivers/net/virtio/virtio_user/virtio_user_dev.c | 10 ++++++++--
 drivers/net/virtio/virtio_user/virtio_user_dev.h |  2 +-
 drivers/net/virtio/virtio_user_ethdev.c          | 14 +++++++++++++-
 3 files changed, 22 insertions(+), 4 deletions(-)

Comments

Gavin Hu Sept. 10, 2018, 6:32 a.m. UTC | #1 
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Jens Freimann
> Sent: Friday, September 7, 2018 2:20 AM
> To: dev@dpdk.org
> Cc: tiwei.bie@intel.com; maxime.coquelin@redhat.com
> Subject: [dpdk-dev] [PATCH v5 06/11] net/virtio-user: add option to use
> packed queues
>
> From: Yuanhan Liu <yuanhan.liu@linux.intel.com>
>
> Add option to enable packed queue support for virtio-user devices.
>
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> ---
>  drivers/net/virtio/virtio_user/virtio_user_dev.c | 10 ++++++++--
> drivers/net/virtio/virtio_user/virtio_user_dev.h |  2 +-
>  drivers/net/virtio/virtio_user_ethdev.c          | 14 +++++++++++++-
>  3 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index 7df600b02..9979bea0d 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -372,12 +372,13 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>   1ULL << VIRTIO_NET_F_GUEST_TSO4|\
>   1ULL << VIRTIO_NET_F_GUEST_TSO6|\
>   1ULL << VIRTIO_F_IN_ORDER|\
> - 1ULL << VIRTIO_F_VERSION_1)
> + 1ULL << VIRTIO_F_VERSION_1|\
> + 1ULL << VIRTIO_F_RING_PACKED)
>
>  int
>  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>       int cq, int queue_size, const char *mac, char **ifname,
> -     int mrg_rxbuf, int in_order)
> +     int mrg_rxbuf, int in_order, int packed_vq)
>  {
>  pthread_mutex_init(&dev->mutex, NULL);
>  snprintf(dev->path, PATH_MAX, "%s", path); @@ -432,6 +433,11
> @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int
> queues,
>  dev->unsupported_features |= (1ull <<
> VIRTIO_F_IN_ORDER);
>  }
>
> +if (packed_vq)
> +dev->device_features |= (1ull << VIRTIO_F_RING_PACKED);
> +else
> +dev->device_features &= ~(1ull <<
> VIRTIO_F_RING_PACKED);
> +

unsupported_features should be initialized also like following F_MAC.

>  if (dev->mac_specified) {
>  dev->device_features |= (1ull << VIRTIO_NET_F_MAC);
>  } else {
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> index d6e0e137b..7f46ba1d9 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> @@ -49,7 +49,7 @@ int virtio_user_start_device(struct virtio_user_dev
> *dev);  int virtio_user_stop_device(struct virtio_user_dev *dev);  int
> virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>   int cq, int queue_size, const char *mac, char
> **ifname,
> - int mrg_rxbuf, int in_order);
> + int mrg_rxbuf, int in_order, int packed_vq);
>  void virtio_user_dev_uninit(struct virtio_user_dev *dev);  void
> virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx);
> uint8_t virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t
> q_pairs); diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> b/drivers/net/virtio/virtio_user_ethdev.c
> index 525d16cab..72ac86186 100644
> --- a/drivers/net/virtio/virtio_user_ethdev.c
> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> @@ -364,6 +364,8 @@ static const char *valid_args[] = {
>  VIRTIO_USER_ARG_MRG_RXBUF,
>  #define VIRTIO_USER_ARG_IN_ORDER       "in_order"
>  VIRTIO_USER_ARG_IN_ORDER,
> +#define VIRTIO_USER_ARG_PACKED_VQ "packed_vq"
> +VIRTIO_USER_ARG_PACKED_VQ,
>  NULL
>  };
>
> @@ -473,6 +475,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>  char *ifname = NULL;
>  char *mac_addr = NULL;
>  int ret = -1;
> +uint64_t packed_vq = 0;
>
>  kvlist = rte_kvargs_parse(rte_vdev_device_args(dev), valid_args);
>  if (!kvlist) {
> @@ -556,6 +559,15 @@ virtio_user_pmd_probe(struct rte_vdev_device
> *dev)
>  cq = 1;
>  }
>
> +if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_PACKED_VQ) == 1) {
> +if (rte_kvargs_process(kvlist,
> VIRTIO_USER_ARG_PACKED_VQ,
> +       &get_integer_arg, &packed_vq) < 0) {
> +PMD_INIT_LOG(ERR, "error to parse %s",
> +     VIRTIO_USER_ARG_PACKED_VQ);
> +goto end;
> +}
> +}
> +
>  if (queues > 1 && cq == 0) {
>  PMD_INIT_LOG(ERR, "multi-q requires ctrl-q");
>  goto end;
> @@ -603,7 +615,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>  vu_dev->is_server = false;
>  if (virtio_user_dev_init(hw->virtio_user_dev, path, queues,
> cq,
>   queue_size, mac_addr, &ifname, mrg_rxbuf,
> - in_order) < 0) {
> + in_order, packed_vq) < 0) {
>  PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
>  virtio_user_eth_dev_free(eth_dev);
>  goto end;
> --
> 2.17.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Maxime Coquelin Sept. 12, 2018, 9:25 a.m. UTC | #2 
On 09/06/2018 08:19 PM, Jens Freimann wrote:
> From: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> 
> Add option to enable packed queue support for virtio-user
> devices.
> 
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> ---
>   drivers/net/virtio/virtio_user/virtio_user_dev.c | 10 ++++++++--
>   drivers/net/virtio/virtio_user/virtio_user_dev.h |  2 +-
>   drivers/net/virtio/virtio_user_ethdev.c          | 14 +++++++++++++-
>   3 files changed, 22 insertions(+), 4 deletions(-)


Shouldn't this patch be better placed at the end of the series to avoid
negotiating packed ring feature while the datapaths aren't implemented?

> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index 7df600b02..9979bea0d 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -372,12 +372,13 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>   	 1ULL << VIRTIO_NET_F_GUEST_TSO4	|	\
>   	 1ULL << VIRTIO_NET_F_GUEST_TSO6	|	\
>   	 1ULL << VIRTIO_F_IN_ORDER		|	\
> -	 1ULL << VIRTIO_F_VERSION_1)
> +	 1ULL << VIRTIO_F_VERSION_1		|	\
> +	 1ULL << VIRTIO_F_RING_PACKED)
>   
>   int
>   virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>   		     int cq, int queue_size, const char *mac, char **ifname,
> -		     int mrg_rxbuf, int in_order)
> +		     int mrg_rxbuf, int in_order, int packed_vq)
>   {
>   	pthread_mutex_init(&dev->mutex, NULL);
>   	snprintf(dev->path, PATH_MAX, "%s", path);
> @@ -432,6 +433,11 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>   		dev->unsupported_features |= (1ull << VIRTIO_F_IN_ORDER);
>   	}
>   
> +	if (packed_vq)
> +		dev->device_features |= (1ull << VIRTIO_F_RING_PACKED);
> +	else
> +		dev->device_features &= ~(1ull << VIRTIO_F_RING_PACKED);
> +
>   	if (dev->mac_specified) {
>   		dev->device_features |= (1ull << VIRTIO_NET_F_MAC);
>   	} else {
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> index d6e0e137b..7f46ba1d9 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> @@ -49,7 +49,7 @@ int virtio_user_start_device(struct virtio_user_dev *dev);
>   int virtio_user_stop_device(struct virtio_user_dev *dev);
>   int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>   			 int cq, int queue_size, const char *mac, char **ifname,
> -			 int mrg_rxbuf, int in_order);
> +			 int mrg_rxbuf, int in_order, int packed_vq);
>   void virtio_user_dev_uninit(struct virtio_user_dev *dev);
>   void virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx);
>   uint8_t virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t q_pairs);
> diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
> index 525d16cab..72ac86186 100644
> --- a/drivers/net/virtio/virtio_user_ethdev.c
> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> @@ -364,6 +364,8 @@ static const char *valid_args[] = {
>   	VIRTIO_USER_ARG_MRG_RXBUF,
>   #define VIRTIO_USER_ARG_IN_ORDER       "in_order"
>   	VIRTIO_USER_ARG_IN_ORDER,
> +#define VIRTIO_USER_ARG_PACKED_VQ "packed_vq"
> +	VIRTIO_USER_ARG_PACKED_VQ,
>   	NULL
>   };
>   
> @@ -473,6 +475,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>   	char *ifname = NULL;
>   	char *mac_addr = NULL;
>   	int ret = -1;
> +	uint64_t packed_vq = 0;
>   
>   	kvlist = rte_kvargs_parse(rte_vdev_device_args(dev), valid_args);
>   	if (!kvlist) {
> @@ -556,6 +559,15 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>   		cq = 1;
>   	}
>   
> +	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_PACKED_VQ) == 1) {
> +		if (rte_kvargs_process(kvlist, VIRTIO_USER_ARG_PACKED_VQ,
> +				       &get_integer_arg, &packed_vq) < 0) {
> +			PMD_INIT_LOG(ERR, "error to parse %s",
> +				     VIRTIO_USER_ARG_PACKED_VQ);
> +			goto end;
> +		}
> +	}
> +
>   	if (queues > 1 && cq == 0) {
>   		PMD_INIT_LOG(ERR, "multi-q requires ctrl-q");
>   		goto end;
> @@ -603,7 +615,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>   			vu_dev->is_server = false;
>   		if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq,
>   				 queue_size, mac_addr, &ifname, mrg_rxbuf,
> -				 in_order) < 0) {
> +				 in_order, packed_vq) < 0) {
>   			PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
>   			virtio_user_eth_dev_free(eth_dev);
>   			goto end;
>
Jens Freimann Sept. 21, 2018, 10:05 a.m. UTC | #3 
On Mon, Sep 10, 2018 at 06:32:09AM +0000, Gavin Hu (Arm Technology China) wrote:
>
>
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Jens Freimann
>> Sent: Friday, September 7, 2018 2:20 AM
>> To: dev@dpdk.org
>> Cc: tiwei.bie@intel.com; maxime.coquelin@redhat.com
>> Subject: [dpdk-dev] [PATCH v5 06/11] net/virtio-user: add option to use
>> packed queues
>>
>> From: Yuanhan Liu <yuanhan.liu@linux.intel.com>
>>
>> Add option to enable packed queue support for virtio-user devices.
>>
>> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
>> ---
>>  drivers/net/virtio/virtio_user/virtio_user_dev.c | 10 ++++++++--
>> drivers/net/virtio/virtio_user/virtio_user_dev.h |  2 +-
>>  drivers/net/virtio/virtio_user_ethdev.c          | 14 +++++++++++++-
>>  3 files changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>> index 7df600b02..9979bea0d 100644
>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>> @@ -372,12 +372,13 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>>   1ULL << VIRTIO_NET_F_GUEST_TSO4|\
>>   1ULL << VIRTIO_NET_F_GUEST_TSO6|\
>>   1ULL << VIRTIO_F_IN_ORDER|\
>> - 1ULL << VIRTIO_F_VERSION_1)
>> + 1ULL << VIRTIO_F_VERSION_1|\
>> + 1ULL << VIRTIO_F_RING_PACKED)
>>
>>  int
>>  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>>       int cq, int queue_size, const char *mac, char **ifname,
>> -     int mrg_rxbuf, int in_order)
>> +     int mrg_rxbuf, int in_order, int packed_vq)
>>  {
>>  pthread_mutex_init(&dev->mutex, NULL);
>>  snprintf(dev->path, PATH_MAX, "%s", path); @@ -432,6 +433,11
>> @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int
>> queues,
>>  dev->unsupported_features |= (1ull <<
>> VIRTIO_F_IN_ORDER);
>>  }
>>
>> +if (packed_vq)
>> +dev->device_features |= (1ull << VIRTIO_F_RING_PACKED);
>> +else
>> +dev->device_features &= ~(1ull <<
>> VIRTIO_F_RING_PACKED);
>> +
>
>unsupported_features should be initialized also like following F_MAC.

Fixed it. Thanks!

regards,
Jens
diff mbox series

Patch

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 7df600b02..9979bea0d 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -372,12 +372,13 @@  virtio_user_dev_setup(struct virtio_user_dev *dev)
 	 1ULL << VIRTIO_NET_F_GUEST_TSO4	|	\
 	 1ULL << VIRTIO_NET_F_GUEST_TSO6	|	\
 	 1ULL << VIRTIO_F_IN_ORDER		|	\
-	 1ULL << VIRTIO_F_VERSION_1)
+	 1ULL << VIRTIO_F_VERSION_1		|	\
+	 1ULL << VIRTIO_F_RING_PACKED)
 
 int
 virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 		     int cq, int queue_size, const char *mac, char **ifname,
-		     int mrg_rxbuf, int in_order)
+		     int mrg_rxbuf, int in_order, int packed_vq)
 {
 	pthread_mutex_init(&dev->mutex, NULL);
 	snprintf(dev->path, PATH_MAX, "%s", path);
@@ -432,6 +433,11 @@  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 		dev->unsupported_features |= (1ull << VIRTIO_F_IN_ORDER);
 	}
 
+	if (packed_vq)
+		dev->device_features |= (1ull << VIRTIO_F_RING_PACKED);
+	else
+		dev->device_features &= ~(1ull << VIRTIO_F_RING_PACKED);
+
 	if (dev->mac_specified) {
 		dev->device_features |= (1ull << VIRTIO_NET_F_MAC);
 	} else {
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index d6e0e137b..7f46ba1d9 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -49,7 +49,7 @@  int virtio_user_start_device(struct virtio_user_dev *dev);
 int virtio_user_stop_device(struct virtio_user_dev *dev);
 int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 			 int cq, int queue_size, const char *mac, char **ifname,
-			 int mrg_rxbuf, int in_order);
+			 int mrg_rxbuf, int in_order, int packed_vq);
 void virtio_user_dev_uninit(struct virtio_user_dev *dev);
 void virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx);
 uint8_t virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t q_pairs);
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 525d16cab..72ac86186 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -364,6 +364,8 @@  static const char *valid_args[] = {
 	VIRTIO_USER_ARG_MRG_RXBUF,
 #define VIRTIO_USER_ARG_IN_ORDER       "in_order"
 	VIRTIO_USER_ARG_IN_ORDER,
+#define VIRTIO_USER_ARG_PACKED_VQ "packed_vq"
+	VIRTIO_USER_ARG_PACKED_VQ,
 	NULL
 };
 
@@ -473,6 +475,7 @@  virtio_user_pmd_probe(struct rte_vdev_device *dev)
 	char *ifname = NULL;
 	char *mac_addr = NULL;
 	int ret = -1;
+	uint64_t packed_vq = 0;
 
 	kvlist = rte_kvargs_parse(rte_vdev_device_args(dev), valid_args);
 	if (!kvlist) {
@@ -556,6 +559,15 @@  virtio_user_pmd_probe(struct rte_vdev_device *dev)
 		cq = 1;
 	}
 
+	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_PACKED_VQ) == 1) {
+		if (rte_kvargs_process(kvlist, VIRTIO_USER_ARG_PACKED_VQ,
+				       &get_integer_arg, &packed_vq) < 0) {
+			PMD_INIT_LOG(ERR, "error to parse %s",
+				     VIRTIO_USER_ARG_PACKED_VQ);
+			goto end;
+		}
+	}
+
 	if (queues > 1 && cq == 0) {
 		PMD_INIT_LOG(ERR, "multi-q requires ctrl-q");
 		goto end;
@@ -603,7 +615,7 @@  virtio_user_pmd_probe(struct rte_vdev_device *dev)
 			vu_dev->is_server = false;
 		if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq,
 				 queue_size, mac_addr, &ifname, mrg_rxbuf,
-				 in_order) < 0) {
+				 in_order, packed_vq) < 0) {
 			PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
 			virtio_user_eth_dev_free(eth_dev);
 			goto end;