diff mbox

[dpdk-dev,06/17] net/virtio-user: add option to use packed queues

Message ID 20180316152120.13199-7-jfreimann@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Jens Freimann March 16, 2018, 3:21 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 | 12 ++++++++++--
 drivers/net/virtio/virtio_user/virtio_user_dev.h |  3 ++-
 drivers/net/virtio/virtio_user_ethdev.c          | 15 ++++++++++++++-
 3 files changed, 26 insertions(+), 4 deletions(-)

Comments

Tiwei Bie March 19, 2018, 8:33 a.m. UTC | #1 
On Fri, Mar 16, 2018 at 04:21:09PM +0100, Jens Freimann wrote:
[...]
> diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
> index 2636490..ee291b3 100644
> --- a/drivers/net/virtio/virtio_user_ethdev.c
> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> @@ -278,6 +278,8 @@
>  	VIRTIO_USER_ARG_QUEUE_SIZE,
>  #define VIRTIO_USER_ARG_INTERFACE_NAME "iface"
>  	VIRTIO_USER_ARG_INTERFACE_NAME,
> +#define VIRTIO_USER_ARG_VERSION_1_1     "version_1_1"
> +	VIRTIO_USER_ARG_VERSION_1_1,

Maybe we can enable packed-ring by default for virtio-user.
If we really need a flag to enable it, the devarg name should
be packed_ring instead of version_1_1.

Thanks
Jens Freimann March 26, 2018, 10:12 a.m. UTC | #2 
On Mon, Mar 19, 2018 at 04:33:27PM +0800, Tiwei Bie wrote:
>On Fri, Mar 16, 2018 at 04:21:09PM +0100, Jens Freimann wrote:
>[...]
>> diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
>> index 2636490..ee291b3 100644
>> --- a/drivers/net/virtio/virtio_user_ethdev.c
>> +++ b/drivers/net/virtio/virtio_user_ethdev.c
>> @@ -278,6 +278,8 @@
>>  	VIRTIO_USER_ARG_QUEUE_SIZE,
>>  #define VIRTIO_USER_ARG_INTERFACE_NAME "iface"
>>  	VIRTIO_USER_ARG_INTERFACE_NAME,
>> +#define VIRTIO_USER_ARG_VERSION_1_1     "version_1_1"
>> +	VIRTIO_USER_ARG_VERSION_1_1,
>
>Maybe we can enable packed-ring by default for virtio-user.
>If we really need a flag to enable it, the devarg name should
>be packed_ring instead of version_1_1.

Thinking about it, we should probably just get rid of this patch and
let feature negotiation do its thing. 

Thanks!

regards,
Jens
diff mbox

Patch

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index f90fee9..5f2ff09 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -314,11 +314,13 @@  int virtio_user_stop_device(struct virtio_user_dev *dev)
 	 1ULL << VIRTIO_NET_F_GUEST_CSUM	|	\
 	 1ULL << VIRTIO_NET_F_GUEST_TSO4	|	\
 	 1ULL << VIRTIO_NET_F_GUEST_TSO6	|	\
-	 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 cq, int queue_size, const char *mac, char **ifname,
+		     int version_1_1)
 {
 	snprintf(dev->path, PATH_MAX, "%s", path);
 	dev->max_queue_pairs = queues;
@@ -347,6 +349,12 @@  int virtio_user_stop_device(struct virtio_user_dev *dev)
 		PMD_INIT_LOG(ERR, "get_features failed: %s", strerror(errno));
 		return -1;
 	}
+
+	if (version_1_1)
+		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);
 
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 64467b4..ce1e103 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -42,7 +42,8 @@  struct virtio_user_dev {
 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 cq, int queue_size, const char *mac, char **ifname,
+			 int version_1_1);
 void virtio_user_dev_uninit(struct virtio_user_dev *dev);
 void virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx);
 #endif
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 2636490..ee291b3 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -278,6 +278,8 @@ 
 	VIRTIO_USER_ARG_QUEUE_SIZE,
 #define VIRTIO_USER_ARG_INTERFACE_NAME "iface"
 	VIRTIO_USER_ARG_INTERFACE_NAME,
+#define VIRTIO_USER_ARG_VERSION_1_1     "version_1_1"
+	VIRTIO_USER_ARG_VERSION_1_1,
 	NULL
 };
 
@@ -382,6 +384,7 @@ 
 	char *ifname = NULL;
 	char *mac_addr = NULL;
 	int ret = -1;
+	uint64_t version_1_1 = 0;
 
 	kvlist = rte_kvargs_parse(rte_vdev_device_args(dev), valid_args);
 	if (!kvlist) {
@@ -456,6 +459,15 @@ 
 		cq = 1;
 	}
 
+	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_VERSION_1_1) == 1) {
+		if (rte_kvargs_process(kvlist, VIRTIO_USER_ARG_VERSION_1_1,
+				       &get_integer_arg, &version_1_1) < 0) {
+			PMD_INIT_LOG(ERR, "error to parse %s",
+				     VIRTIO_USER_ARG_VERSION_1_1);
+			goto end;
+		}
+	}
+
 	if (queues > 1 && cq == 0) {
 		PMD_INIT_LOG(ERR, "multi-q requires ctrl-q");
 		goto end;
@@ -477,7 +489,8 @@ 
 
 		hw = eth_dev->data->dev_private;
 		if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq,
-				 queue_size, mac_addr, &ifname) < 0) {
+				 queue_size, mac_addr, &ifname,
+				 version_1_1) < 0) {
 			PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
 			virtio_user_eth_dev_free(eth_dev);
 			goto end;