All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 00/13] vhost-user multiple queues enabling
@ 2015-10-09  5:45 Yuanhan Liu
  2015-10-09  5:46 ` [PATCH v6 01/13] vhost-user: add protocol features support Yuanhan Liu
                   ` (15 more replies)
  0 siblings, 16 replies; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-09  5:45 UTC (permalink / raw)
  To: dev; +Cc: marcel, Michael S. Tsirkin

This patch set enables vhost-user multiple queues.

Overview
========

It depends on some QEMU patches that has already been merged to upstream.
Those qemu patches introduce some new vhost-user messages, for vhost-user
mq enabling negotiation. Here is the main negotiation steps (Qemu
as master, and DPDK vhost-user as slave):

- Master queries features by VHOST_USER_GET_FEATURES from slave

- Check if VHOST_USER_F_PROTOCOL_FEATURES exist. If not, mq is not
  supported. (check patch 1 for why VHOST_USER_F_PROTOCOL_FEATURES
  is introduced)

- Master then sends another command, VHOST_USER_GET_QUEUE_NUM, for
  querying how many queues the slave supports.

  Master will compare the result with the requested queue number.
  Qemu exits if the former is smaller.

- Master then tries to initiate all queue pairs by sending some vhost
  user commands, including VHOST_USER_SET_VRING_CALL, which will
  trigger the slave to do related vring setup, such as vring allocation.


Till now, all necessary initiation and negotiation are done. And master
could send another message, VHOST_USER_SET_VRING_ENABLE, to enable/disable
a specific queue dynamically later.


Patchset
========

Patch 1-6 are all prepare works for enabling mq; they are all atomic
changes, with "do not breaking anything" beared in mind while making
them.

Patch 7 acutally enables mq feature, by setting two key feature flags.

Patch 8 handles VHOST_USER_SET_VRING_ENABLE message, which is for enabling
disabling a specific virt queue pair, and there is only one queue pair is
enabled by default.

Patch 9-12 is for demostrating the mq feature.

Patch 13 udpates the doc release note.


Testing
=======

Host side
----------

- # Start vhost-switch

  sudo mount -t hugetlbfs nodev /mnt/huge
  sudo modprobe uio
  sudo insmod $RTE_SDK/$RTE_TARGET/kmod/igb_uio.ko

  sudo $RTE_SDK/tools/dpdk_nic_bind.py --bind igb_uio 0000:08:00.0

  sudo $RTE_SDK/examples/vhost/build/vhost-switch -c 0xf0 -n 4     \
       --huge-dir /mnt/huge --socket-mem 2048,0 -- -p 1 --vm2vm 0  \
       --dev-basename usvhost --rxq 2

  # Above common generates a usvhost socket file at PWD. You could also
  # specify "--stats 1" option to enable stats dumping.



- # start qemu


  sudo sudo mount -t hugetlbfs nodev $HOME/hugetlbfs
  $QEMU_DIR/x86_64-softmmu/qemu-system-x86_64 -machine accel=kvm -m 4G \
        -object memory-backend-file,id=mem,size=4G,mem-path=$HOME/hugetlbfs,share=on \
	-numa node,memdev=mem -chardev socket,id=chr0,path=/path/to/usvhost \
	-netdev vhost-user,id=net0,chardev=chr0,vhostforce,queues=2     \
	-device virtio-net-pci,netdev=net0,mq=on,vectors=6,mac=52:54:00:12:34:58,csum=off,gso=off,guest_tso4=off,guest_tso6=off,guest_ecn=off \
	-hda $HOME/iso/fc-22-x86_64.img -smp 10 -cpu core2duo,+sse3,+sse4.1,+sse4.2


Guest side
----------

   modprobe uio
   insmod $RTE_SDK/$RTE_TARGET/kmod/igb_uio.ko
   echo 1024 > /sys/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages
   ./tools/dpdk_nic_bind.py --bind igb_uio 00:03.0
    
   $RTE_SDK/$RTE_TARGET/app/testpmd -c 1f -n 4 -- --rxq=2 --txq=2 \
        --nb-cores=4 -i --disable-hw-vlan --txqflags 0xf00

   > set fwd mac
   > start tx_first
 

After those setups, you then could use packet generator for packet tx/rx testing.



Test with OVS
=============

Marcel also created a simple yet quite clear test guide with OVS at:

   http://wiki.qemu.org/Features/vhost-user-ovs-dpdk

BTW, Marcel, would you please complete the page on mq testing?


---
Changchun Ouyang (7):
  vhost: rxtx: prepare work for multiple queue support
  virtio: read virtio_net_config correctly
  vhost: add VHOST_USER_SET_VRING_ENABLE message
  vhost: add API bind a virtq to a specific core
  ixgbe: support VMDq RSS in non-SRIOV environment
  examples/vhost: demonstrate the usage of vhost mq feature
  examples/vhost: add per queue stats

Yuanhan Liu (6):
  vhost-user: add protocol features support
  vhost-user: add VHOST_USER_GET_QUEUE_NUM message
  vhost: vring queue setup for multiple queue support
  vhost-user: handle VHOST_USER_RESET_OWNER correctly
  vhost-user: enable vhost-user multiple queue
  doc: update release note for vhost-user mq support

 doc/guides/rel_notes/release_2_2.rst          |   5 +
 drivers/net/ixgbe/ixgbe_rxtx.c                |  86 +++++-
 drivers/net/virtio/virtio_ethdev.c            |  16 +-
 examples/vhost/main.c                         | 420 +++++++++++++++++---------
 examples/vhost/main.h                         |   3 +-
 lib/librte_ether/rte_ethdev.c                 |  11 +
 lib/librte_vhost/rte_vhost_version.map        |   7 +
 lib/librte_vhost/rte_virtio_net.h             |  38 ++-
 lib/librte_vhost/vhost_rxtx.c                 |  56 +++-
 lib/librte_vhost/vhost_user/vhost-net-user.c  |  27 +-
 lib/librte_vhost/vhost_user/vhost-net-user.h  |   4 +
 lib/librte_vhost/vhost_user/virtio-net-user.c |  83 +++--
 lib/librte_vhost/vhost_user/virtio-net-user.h |  10 +
 lib/librte_vhost/virtio-net.c                 | 181 +++++++----
 14 files changed, 692 insertions(+), 255 deletions(-)

-- 
1.9.0

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PATCH v6 01/13] vhost-user: add protocol features support
  2015-10-09  5:45 [PATCH v6 00/13] vhost-user multiple queues enabling Yuanhan Liu
@ 2015-10-09  5:46 ` Yuanhan Liu
  2015-10-09  5:46 ` [PATCH v6 02/13] vhost-user: add VHOST_USER_GET_QUEUE_NUM message Yuanhan Liu
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-09  5:46 UTC (permalink / raw)
  To: dev; +Cc: marcel, Michael S. Tsirkin

The two protocol features messages are introduced by qemu vhost
maintainer(Michael) for extendting vhost-user interface. Here is
an excerpta from the vhost-user spec:

    Any protocol extensions are gated by protocol feature bits,
    which allows full backwards compatibility on both master
    and slave.

The vhost-user multiple queue features will be treated as a vhost-user
extension, hence, we have to implement the two messages first.

VHOST_USER_PROTOCOL_FEATURES is initated to 0, as we don't support
any yet.

Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 lib/librte_vhost/rte_virtio_net.h             |  1 +
 lib/librte_vhost/vhost_user/vhost-net-user.c  | 13 ++++++++++++-
 lib/librte_vhost/vhost_user/vhost-net-user.h  |  2 ++
 lib/librte_vhost/vhost_user/virtio-net-user.c | 13 +++++++++++++
 lib/librte_vhost/vhost_user/virtio-net-user.h |  5 +++++
 lib/librte_vhost/virtio-net.c                 |  5 ++++-
 6 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/lib/librte_vhost/rte_virtio_net.h b/lib/librte_vhost/rte_virtio_net.h
index a037c15..e3a21e5 100644
--- a/lib/librte_vhost/rte_virtio_net.h
+++ b/lib/librte_vhost/rte_virtio_net.h
@@ -99,6 +99,7 @@ struct virtio_net {
 	struct vhost_virtqueue	*virtqueue[VIRTIO_QNUM];	/**< Contains all virtqueue information. */
 	struct virtio_memory	*mem;		/**< QEMU memory and memory region information. */
 	uint64_t		features;	/**< Negotiated feature set. */
+	uint64_t		protocol_features;	/**< Negotiated protocol feature set. */
 	uint64_t		device_fh;	/**< device identifier. */
 	uint32_t		flags;		/**< Device flags. Only used to check if device is running on data core. */
 #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ)
diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.c b/lib/librte_vhost/vhost_user/vhost-net-user.c
index d1f8877..bc2ad24 100644
--- a/lib/librte_vhost/vhost_user/vhost-net-user.c
+++ b/lib/librte_vhost/vhost_user/vhost-net-user.c
@@ -95,7 +95,9 @@ static const char *vhost_message_str[VHOST_USER_MAX] = {
 	[VHOST_USER_GET_VRING_BASE] = "VHOST_USER_GET_VRING_BASE",
 	[VHOST_USER_SET_VRING_KICK] = "VHOST_USER_SET_VRING_KICK",
 	[VHOST_USER_SET_VRING_CALL] = "VHOST_USER_SET_VRING_CALL",
-	[VHOST_USER_SET_VRING_ERR]  = "VHOST_USER_SET_VRING_ERR"
+	[VHOST_USER_SET_VRING_ERR]  = "VHOST_USER_SET_VRING_ERR",
+	[VHOST_USER_GET_PROTOCOL_FEATURES]  = "VHOST_USER_GET_PROTOCOL_FEATURES",
+	[VHOST_USER_SET_PROTOCOL_FEATURES]  = "VHOST_USER_SET_PROTOCOL_FEATURES",
 };
 
 /**
@@ -363,6 +365,15 @@ vserver_message_handler(int connfd, void *dat, int *remove)
 		ops->set_features(ctx, &features);
 		break;
 
+	case VHOST_USER_GET_PROTOCOL_FEATURES:
+		msg.payload.u64 = VHOST_USER_PROTOCOL_FEATURES;
+		msg.size = sizeof(msg.payload.u64);
+		send_vhost_message(connfd, &msg);
+		break;
+	case VHOST_USER_SET_PROTOCOL_FEATURES:
+		user_set_protocol_features(ctx, msg.payload.u64);
+		break;
+
 	case VHOST_USER_SET_OWNER:
 		ops->set_owner(ctx);
 		break;
diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.h b/lib/librte_vhost/vhost_user/vhost-net-user.h
index 2e72f3c..4490d23 100644
--- a/lib/librte_vhost/vhost_user/vhost-net-user.h
+++ b/lib/librte_vhost/vhost_user/vhost-net-user.h
@@ -63,6 +63,8 @@ typedef enum VhostUserRequest {
 	VHOST_USER_SET_VRING_KICK = 12,
 	VHOST_USER_SET_VRING_CALL = 13,
 	VHOST_USER_SET_VRING_ERR = 14,
+	VHOST_USER_GET_PROTOCOL_FEATURES = 15,
+	VHOST_USER_SET_PROTOCOL_FEATURES = 16,
 	VHOST_USER_MAX
 } VhostUserRequest;
 
diff --git a/lib/librte_vhost/vhost_user/virtio-net-user.c b/lib/librte_vhost/vhost_user/virtio-net-user.c
index 4689927..360254e 100644
--- a/lib/librte_vhost/vhost_user/virtio-net-user.c
+++ b/lib/librte_vhost/vhost_user/virtio-net-user.c
@@ -316,3 +316,16 @@ user_destroy_device(struct vhost_device_ctx ctx)
 		dev->mem = NULL;
 	}
 }
+
+void
+user_set_protocol_features(struct vhost_device_ctx ctx,
+			   uint64_t protocol_features)
+{
+	struct virtio_net *dev;
+
+	dev = get_device(ctx);
+	if (dev == NULL || protocol_features & ~VHOST_USER_PROTOCOL_FEATURES)
+		return;
+
+	dev->protocol_features = protocol_features;
+}
diff --git a/lib/librte_vhost/vhost_user/virtio-net-user.h b/lib/librte_vhost/vhost_user/virtio-net-user.h
index df24860..e7a6ff4 100644
--- a/lib/librte_vhost/vhost_user/virtio-net-user.h
+++ b/lib/librte_vhost/vhost_user/virtio-net-user.h
@@ -37,12 +37,17 @@
 #include "vhost-net.h"
 #include "vhost-net-user.h"
 
+#define VHOST_USER_PROTOCOL_FEATURES	0ULL
+
 int user_set_mem_table(struct vhost_device_ctx, struct VhostUserMsg *);
 
 void user_set_vring_call(struct vhost_device_ctx, struct VhostUserMsg *);
 
 void user_set_vring_kick(struct vhost_device_ctx, struct VhostUserMsg *);
 
+void user_set_protocol_features(struct vhost_device_ctx ctx,
+				uint64_t protocol_features);
+
 int user_get_vring_base(struct vhost_device_ctx, struct vhost_vring_state *);
 
 void user_destroy_device(struct vhost_device_ctx);
diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
index d0f1764..deac6b9 100644
--- a/lib/librte_vhost/virtio-net.c
+++ b/lib/librte_vhost/virtio-net.c
@@ -67,11 +67,14 @@ struct virtio_net_device_ops const *notify_ops;
 /* root address of the linked list of managed virtio devices */
 static struct virtio_net_config_ll *ll_root;
 
+#define VHOST_USER_F_PROTOCOL_FEATURES	30
+
 /* Features supported by this lib. */
 #define VHOST_SUPPORTED_FEATURES ((1ULL << VIRTIO_NET_F_MRG_RXBUF) | \
 				(1ULL << VIRTIO_NET_F_CTRL_VQ) | \
 				(1ULL << VIRTIO_NET_F_CTRL_RX) | \
-				(1ULL << VHOST_F_LOG_ALL))
+				(1ULL << VHOST_F_LOG_ALL)      | \
+				(1ULL << VHOST_USER_F_PROTOCOL_FEATURES))
 static uint64_t VHOST_FEATURES = VHOST_SUPPORTED_FEATURES;
 
 
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v6 02/13] vhost-user: add VHOST_USER_GET_QUEUE_NUM message
  2015-10-09  5:45 [PATCH v6 00/13] vhost-user multiple queues enabling Yuanhan Liu
  2015-10-09  5:46 ` [PATCH v6 01/13] vhost-user: add protocol features support Yuanhan Liu
@ 2015-10-09  5:46 ` Yuanhan Liu
  2015-10-09  5:46 ` [PATCH v6 03/13] vhost: vring queue setup for multiple queue support Yuanhan Liu
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-09  5:46 UTC (permalink / raw)
  To: dev; +Cc: marcel, Michael S. Tsirkin

To tell the frontend (qemu) how many queue pairs we support.

And it is initiated to VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX.

Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 lib/librte_vhost/vhost_user/vhost-net-user.c | 7 +++++++
 lib/librte_vhost/vhost_user/vhost-net-user.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.c b/lib/librte_vhost/vhost_user/vhost-net-user.c
index bc2ad24..8675cd4 100644
--- a/lib/librte_vhost/vhost_user/vhost-net-user.c
+++ b/lib/librte_vhost/vhost_user/vhost-net-user.c
@@ -98,6 +98,7 @@ static const char *vhost_message_str[VHOST_USER_MAX] = {
 	[VHOST_USER_SET_VRING_ERR]  = "VHOST_USER_SET_VRING_ERR",
 	[VHOST_USER_GET_PROTOCOL_FEATURES]  = "VHOST_USER_GET_PROTOCOL_FEATURES",
 	[VHOST_USER_SET_PROTOCOL_FEATURES]  = "VHOST_USER_SET_PROTOCOL_FEATURES",
+	[VHOST_USER_GET_QUEUE_NUM]  = "VHOST_USER_GET_QUEUE_NUM",
 };
 
 /**
@@ -421,6 +422,12 @@ vserver_message_handler(int connfd, void *dat, int *remove)
 		RTE_LOG(INFO, VHOST_CONFIG, "not implemented\n");
 		break;
 
+	case VHOST_USER_GET_QUEUE_NUM:
+		msg.payload.u64 = VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX;
+		msg.size = sizeof(msg.payload.u64);
+		send_vhost_message(connfd, &msg);
+		break;
+
 	default:
 		break;
 
diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.h b/lib/librte_vhost/vhost_user/vhost-net-user.h
index 4490d23..389d21d 100644
--- a/lib/librte_vhost/vhost_user/vhost-net-user.h
+++ b/lib/librte_vhost/vhost_user/vhost-net-user.h
@@ -65,6 +65,7 @@ typedef enum VhostUserRequest {
 	VHOST_USER_SET_VRING_ERR = 14,
 	VHOST_USER_GET_PROTOCOL_FEATURES = 15,
 	VHOST_USER_SET_PROTOCOL_FEATURES = 16,
+	VHOST_USER_GET_QUEUE_NUM = 17,
 	VHOST_USER_MAX
 } VhostUserRequest;
 
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v6 03/13] vhost: vring queue setup for multiple queue support
  2015-10-09  5:45 [PATCH v6 00/13] vhost-user multiple queues enabling Yuanhan Liu
  2015-10-09  5:46 ` [PATCH v6 01/13] vhost-user: add protocol features support Yuanhan Liu
  2015-10-09  5:46 ` [PATCH v6 02/13] vhost-user: add VHOST_USER_GET_QUEUE_NUM message Yuanhan Liu
@ 2015-10-09  5:46 ` Yuanhan Liu
  2015-10-20  6:49   ` Thomas Monjalon
  2015-10-09  5:46 ` [PATCH v6 04/13] vhost: rxtx: prepare work " Yuanhan Liu
                   ` (12 subsequent siblings)
  15 siblings, 1 reply; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-09  5:46 UTC (permalink / raw)
  To: dev; +Cc: marcel, Michael S. Tsirkin

All queue pairs, including the default (the first) queue pair,
are allocated dynamically, when a vring_call message is received
first time for a specific queue pair.

This is a refactor work for enabling vhost-user multiple queue;
it should not break anything as it does no functional changes:
we don't support mq set, so there is only one mq at max.

This patch is based on Changchun's patch.

Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

---
v6: set vq->vhost_hlen correctly.
---
 lib/librte_vhost/rte_virtio_net.h             |   3 +-
 lib/librte_vhost/vhost_user/virtio-net-user.c |  44 ++++----
 lib/librte_vhost/virtio-net.c                 | 144 ++++++++++++++++----------
 3 files changed, 114 insertions(+), 77 deletions(-)

diff --git a/lib/librte_vhost/rte_virtio_net.h b/lib/librte_vhost/rte_virtio_net.h
index e3a21e5..5dd6493 100644
--- a/lib/librte_vhost/rte_virtio_net.h
+++ b/lib/librte_vhost/rte_virtio_net.h
@@ -96,7 +96,7 @@ struct vhost_virtqueue {
  * Device structure contains all configuration information relating to the device.
  */
 struct virtio_net {
-	struct vhost_virtqueue	*virtqueue[VIRTIO_QNUM];	/**< Contains all virtqueue information. */
+	struct vhost_virtqueue	*virtqueue[VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX];	/**< Contains all virtqueue information. */
 	struct virtio_memory	*mem;		/**< QEMU memory and memory region information. */
 	uint64_t		features;	/**< Negotiated feature set. */
 	uint64_t		protocol_features;	/**< Negotiated protocol feature set. */
@@ -104,6 +104,7 @@ struct virtio_net {
 	uint32_t		flags;		/**< Device flags. Only used to check if device is running on data core. */
 #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ)
 	char			ifname[IF_NAME_SZ];	/**< Name of the tap device or socket path. */
+	uint32_t		virt_qp_nb;	/**< number of queue pair we have allocated */
 	void			*priv;		/**< private context */
 } __rte_cache_aligned;
 
diff --git a/lib/librte_vhost/vhost_user/virtio-net-user.c b/lib/librte_vhost/vhost_user/virtio-net-user.c
index 360254e..e83d279 100644
--- a/lib/librte_vhost/vhost_user/virtio-net-user.c
+++ b/lib/librte_vhost/vhost_user/virtio-net-user.c
@@ -206,25 +206,33 @@ err_mmap:
 }
 
 static int
+vq_is_ready(struct vhost_virtqueue *vq)
+{
+	return vq && vq->desc   &&
+	       vq->kickfd != -1 &&
+	       vq->callfd != -1;
+}
+
+static int
 virtio_is_ready(struct virtio_net *dev)
 {
 	struct vhost_virtqueue *rvq, *tvq;
+	uint32_t i;
 
-	/* mq support in future.*/
-	rvq = dev->virtqueue[VIRTIO_RXQ];
-	tvq = dev->virtqueue[VIRTIO_TXQ];
-	if (rvq && tvq && rvq->desc && tvq->desc &&
-		(rvq->kickfd != -1) &&
-		(rvq->callfd != -1) &&
-		(tvq->kickfd != -1) &&
-		(tvq->callfd != -1)) {
-		RTE_LOG(INFO, VHOST_CONFIG,
-			"virtio is now ready for processing.\n");
-		return 1;
+	for (i = 0; i < dev->virt_qp_nb; i++) {
+		rvq = dev->virtqueue[i * VIRTIO_QNUM + VIRTIO_RXQ];
+		tvq = dev->virtqueue[i * VIRTIO_QNUM + VIRTIO_TXQ];
+
+		if (!vq_is_ready(rvq) || !vq_is_ready(tvq)) {
+			RTE_LOG(INFO, VHOST_CONFIG,
+				"virtio is not ready for processing.\n");
+			return 0;
+		}
 	}
+
 	RTE_LOG(INFO, VHOST_CONFIG,
-		"virtio isn't ready for processing.\n");
-	return 0;
+		"virtio is now ready for processing.\n");
+	return 1;
 }
 
 void
@@ -290,13 +298,9 @@ user_get_vring_base(struct vhost_device_ctx ctx,
 	 * sent and only sent in vhost_vring_stop.
 	 * TODO: cleanup the vring, it isn't usable since here.
 	 */
-	if ((dev->virtqueue[VIRTIO_RXQ]->kickfd) >= 0) {
-		close(dev->virtqueue[VIRTIO_RXQ]->kickfd);
-		dev->virtqueue[VIRTIO_RXQ]->kickfd = -1;
-	}
-	if ((dev->virtqueue[VIRTIO_TXQ]->kickfd) >= 0) {
-		close(dev->virtqueue[VIRTIO_TXQ]->kickfd);
-		dev->virtqueue[VIRTIO_TXQ]->kickfd = -1;
+	if ((dev->virtqueue[state->index]->kickfd) >= 0) {
+		close(dev->virtqueue[state->index]->kickfd);
+		dev->virtqueue[state->index]->kickfd = -1;
 	}
 
 	return 0;
diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
index deac6b9..57fb7b1 100644
--- a/lib/librte_vhost/virtio-net.c
+++ b/lib/librte_vhost/virtio-net.c
@@ -36,6 +36,7 @@
 #include <stddef.h>
 #include <stdint.h>
 #include <stdlib.h>
+#include <assert.h>
 #include <sys/mman.h>
 #include <unistd.h>
 #ifdef RTE_LIBRTE_VHOST_NUMA
@@ -178,6 +179,15 @@ add_config_ll_entry(struct virtio_net_config_ll *new_ll_dev)
 
 }
 
+static void
+cleanup_vq(struct vhost_virtqueue *vq)
+{
+	if (vq->callfd >= 0)
+		close(vq->callfd);
+	if (vq->kickfd >= 0)
+		close(vq->kickfd);
+}
+
 /*
  * Unmap any memory, close any file descriptors and
  * free any memory owned by a device.
@@ -185,6 +195,8 @@ add_config_ll_entry(struct virtio_net_config_ll *new_ll_dev)
 static void
 cleanup_device(struct virtio_net *dev)
 {
+	uint32_t i;
+
 	/* Unmap QEMU memory file if mapped. */
 	if (dev->mem) {
 		munmap((void *)(uintptr_t)dev->mem->mapped_address,
@@ -192,15 +204,10 @@ cleanup_device(struct virtio_net *dev)
 		free(dev->mem);
 	}
 
-	/* Close any event notifiers opened by device. */
-	if (dev->virtqueue[VIRTIO_RXQ]->callfd >= 0)
-		close(dev->virtqueue[VIRTIO_RXQ]->callfd);
-	if (dev->virtqueue[VIRTIO_RXQ]->kickfd >= 0)
-		close(dev->virtqueue[VIRTIO_RXQ]->kickfd);
-	if (dev->virtqueue[VIRTIO_TXQ]->callfd >= 0)
-		close(dev->virtqueue[VIRTIO_TXQ]->callfd);
-	if (dev->virtqueue[VIRTIO_TXQ]->kickfd >= 0)
-		close(dev->virtqueue[VIRTIO_TXQ]->kickfd);
+	for (i = 0; i < dev->virt_qp_nb; i++) {
+		cleanup_vq(dev->virtqueue[i * VIRTIO_QNUM + VIRTIO_RXQ]);
+		cleanup_vq(dev->virtqueue[i * VIRTIO_QNUM + VIRTIO_TXQ]);
+	}
 }
 
 /*
@@ -209,9 +216,11 @@ cleanup_device(struct virtio_net *dev)
 static void
 free_device(struct virtio_net_config_ll *ll_dev)
 {
-	/* Free any malloc'd memory */
-	rte_free(ll_dev->dev.virtqueue[VIRTIO_RXQ]);
-	rte_free(ll_dev->dev.virtqueue[VIRTIO_TXQ]);
+	uint32_t i;
+
+	for (i = 0; i < ll_dev->dev.virt_qp_nb; i++)
+		rte_free(ll_dev->dev.virtqueue[i * VIRTIO_QNUM]);
+
 	rte_free(ll_dev);
 }
 
@@ -244,6 +253,50 @@ rm_config_ll_entry(struct virtio_net_config_ll *ll_dev,
 	}
 }
 
+static void
+init_vring_queue(struct vhost_virtqueue *vq)
+{
+	memset(vq, 0, sizeof(struct vhost_virtqueue));
+
+	vq->kickfd = -1;
+	vq->callfd = -1;
+
+	/* Backends are set to -1 indicating an inactive device. */
+	vq->backend = -1;
+}
+
+static void
+init_vring_queue_pair(struct virtio_net *dev, uint32_t qp_idx)
+{
+	init_vring_queue(dev->virtqueue[qp_idx * VIRTIO_QNUM + VIRTIO_RXQ]);
+	init_vring_queue(dev->virtqueue[qp_idx * VIRTIO_QNUM + VIRTIO_TXQ]);
+}
+
+static int
+alloc_vring_queue_pair(struct virtio_net *dev, uint32_t qp_idx)
+{
+	struct vhost_virtqueue *virtqueue = NULL;
+	uint32_t virt_rx_q_idx = qp_idx * VIRTIO_QNUM + VIRTIO_RXQ;
+	uint32_t virt_tx_q_idx = qp_idx * VIRTIO_QNUM + VIRTIO_TXQ;
+
+	virtqueue = rte_malloc(NULL,
+			       sizeof(struct vhost_virtqueue) * VIRTIO_QNUM, 0);
+	if (virtqueue == NULL) {
+		RTE_LOG(ERR, VHOST_CONFIG,
+			"Failed to allocate memory for virt qp:%d.\n", qp_idx);
+		return -1;
+	}
+
+	dev->virtqueue[virt_rx_q_idx] = virtqueue;
+	dev->virtqueue[virt_tx_q_idx] = virtqueue + VIRTIO_TXQ;
+
+	init_vring_queue_pair(dev, qp_idx);
+
+	dev->virt_qp_nb += 1;
+
+	return 0;
+}
+
 /*
  *  Initialise all variables in device structure.
  */
@@ -251,6 +304,7 @@ static void
 init_device(struct virtio_net *dev)
 {
 	uint64_t vq_offset;
+	uint32_t i;
 
 	/*
 	 * Virtqueues have already been malloced so
@@ -261,17 +315,9 @@ init_device(struct virtio_net *dev)
 	/* Set everything to 0. */
 	memset((void *)(uintptr_t)((uint64_t)(uintptr_t)dev + vq_offset), 0,
 		(sizeof(struct virtio_net) - (size_t)vq_offset));
-	memset(dev->virtqueue[VIRTIO_RXQ], 0, sizeof(struct vhost_virtqueue));
-	memset(dev->virtqueue[VIRTIO_TXQ], 0, sizeof(struct vhost_virtqueue));
-
-	dev->virtqueue[VIRTIO_RXQ]->kickfd = -1;
-	dev->virtqueue[VIRTIO_RXQ]->callfd = -1;
-	dev->virtqueue[VIRTIO_TXQ]->kickfd = -1;
-	dev->virtqueue[VIRTIO_TXQ]->callfd = -1;
 
-	/* Backends are set to -1 indicating an inactive device. */
-	dev->virtqueue[VIRTIO_RXQ]->backend = VIRTIO_DEV_STOPPED;
-	dev->virtqueue[VIRTIO_TXQ]->backend = VIRTIO_DEV_STOPPED;
+	for (i = 0; i < dev->virt_qp_nb; i++)
+		init_vring_queue_pair(dev, i);
 }
 
 /*
@@ -283,7 +329,6 @@ static int
 new_device(struct vhost_device_ctx ctx)
 {
 	struct virtio_net_config_ll *new_ll_dev;
-	struct vhost_virtqueue *virtqueue_rx, *virtqueue_tx;
 
 	/* Setup device and virtqueues. */
 	new_ll_dev = rte_malloc(NULL, sizeof(struct virtio_net_config_ll), 0);
@@ -294,28 +339,6 @@ new_device(struct vhost_device_ctx ctx)
 		return -1;
 	}
 
-	virtqueue_rx = rte_malloc(NULL, sizeof(struct vhost_virtqueue), 0);
-	if (virtqueue_rx == NULL) {
-		rte_free(new_ll_dev);
-		RTE_LOG(ERR, VHOST_CONFIG,
-			"(%"PRIu64") Failed to allocate memory for rxq.\n",
-			ctx.fh);
-		return -1;
-	}
-
-	virtqueue_tx = rte_malloc(NULL, sizeof(struct vhost_virtqueue), 0);
-	if (virtqueue_tx == NULL) {
-		rte_free(virtqueue_rx);
-		rte_free(new_ll_dev);
-		RTE_LOG(ERR, VHOST_CONFIG,
-			"(%"PRIu64") Failed to allocate memory for txq.\n",
-			ctx.fh);
-		return -1;
-	}
-
-	new_ll_dev->dev.virtqueue[VIRTIO_RXQ] = virtqueue_rx;
-	new_ll_dev->dev.virtqueue[VIRTIO_TXQ] = virtqueue_tx;
-
 	/* Initialise device and virtqueues. */
 	init_device(&new_ll_dev->dev);
 
@@ -437,6 +460,8 @@ static int
 set_features(struct vhost_device_ctx ctx, uint64_t *pu)
 {
 	struct virtio_net *dev;
+	uint16_t vhost_hlen;
+	uint16_t i;
 
 	dev = get_device(ctx);
 	if (dev == NULL)
@@ -444,27 +469,26 @@ set_features(struct vhost_device_ctx ctx, uint64_t *pu)
 	if (*pu & ~VHOST_FEATURES)
 		return -1;
 
-	/* Store the negotiated feature list for the device. */
 	dev->features = *pu;
-
-	/* Set the vhost_hlen depending on if VIRTIO_NET_F_MRG_RXBUF is set. */
 	if (dev->features & (1 << VIRTIO_NET_F_MRG_RXBUF)) {
 		LOG_DEBUG(VHOST_CONFIG,
 			"(%"PRIu64") Mergeable RX buffers enabled\n",
 			dev->device_fh);
-		dev->virtqueue[VIRTIO_RXQ]->vhost_hlen =
-			sizeof(struct virtio_net_hdr_mrg_rxbuf);
-		dev->virtqueue[VIRTIO_TXQ]->vhost_hlen =
-			sizeof(struct virtio_net_hdr_mrg_rxbuf);
+		vhost_hlen = sizeof(struct virtio_net_hdr_mrg_rxbuf);
 	} else {
 		LOG_DEBUG(VHOST_CONFIG,
 			"(%"PRIu64") Mergeable RX buffers disabled\n",
 			dev->device_fh);
-		dev->virtqueue[VIRTIO_RXQ]->vhost_hlen =
-			sizeof(struct virtio_net_hdr);
-		dev->virtqueue[VIRTIO_TXQ]->vhost_hlen =
-			sizeof(struct virtio_net_hdr);
+		vhost_hlen = sizeof(struct virtio_net_hdr);
+	}
+
+	for (i = 0; i < dev->virt_qp_nb; i++) {
+		uint16_t base_idx = i * VIRTIO_QNUM;
+
+		dev->virtqueue[base_idx + VIRTIO_RXQ]->vhost_hlen = vhost_hlen;
+		dev->virtqueue[base_idx + VIRTIO_TXQ]->vhost_hlen = vhost_hlen;
 	}
+
 	return 0;
 }
 
@@ -680,13 +704,21 @@ set_vring_call(struct vhost_device_ctx ctx, struct vhost_vring_file *file)
 {
 	struct virtio_net *dev;
 	struct vhost_virtqueue *vq;
+	uint32_t cur_qp_idx = file->index / VIRTIO_QNUM;
 
 	dev = get_device(ctx);
 	if (dev == NULL)
 		return -1;
 
+	/* alloc vring queue pair if it is a new queue pair */
+	if (cur_qp_idx + 1 > dev->virt_qp_nb) {
+		if (alloc_vring_queue_pair(dev, cur_qp_idx) < 0)
+			return -1;
+	}
+
 	/* file->index refers to the queue index. The txq is 1, rxq is 0. */
 	vq = dev->virtqueue[file->index];
+	assert(vq != NULL);
 
 	if (vq->callfd >= 0)
 		close(vq->callfd);
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v6 04/13] vhost: rxtx: prepare work for multiple queue support
  2015-10-09  5:45 [PATCH v6 00/13] vhost-user multiple queues enabling Yuanhan Liu
                   ` (2 preceding siblings ...)
  2015-10-09  5:46 ` [PATCH v6 03/13] vhost: vring queue setup for multiple queue support Yuanhan Liu
@ 2015-10-09  5:46 ` Yuanhan Liu
  2015-10-20  6:57   ` Thomas Monjalon
  2015-10-09  5:46 ` [PATCH v6 05/13] vhost-user: handle VHOST_USER_RESET_OWNER correctly Yuanhan Liu
                   ` (11 subsequent siblings)
  15 siblings, 1 reply; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-09  5:46 UTC (permalink / raw)
  To: dev; +Cc: Michael S. Tsirkin, marcel

From: Changchun Ouyang <changchun.ouyang@intel.com>

Do not use VIRTIO_RXQ or VIRTIO_TXQ anymore; use the queue_id,
instead, which will be set to a proper value for a specific queue
when we have multiple queue support enabled.

For now, queue_id is still set with VIRTIO_RXQ or VIRTIO_TXQ,
so it should not break anything.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 lib/librte_vhost/vhost_rxtx.c | 46 ++++++++++++++++++++++++++++++-------------
 1 file changed, 32 insertions(+), 14 deletions(-)

diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c
index 7026bfa..14e00ef 100644
--- a/lib/librte_vhost/vhost_rxtx.c
+++ b/lib/librte_vhost/vhost_rxtx.c
@@ -42,6 +42,16 @@
 
 #define MAX_PKT_BURST 32
 
+static inline int __attribute__((always_inline))
+is_valid_virt_queue_idx(uint32_t virtq_idx, int is_tx, uint32_t max_qp_idx)
+{
+	if ((is_tx ^ (virtq_idx & 0x1)) ||
+	    (virtq_idx >= max_qp_idx * VIRTIO_QNUM))
+		return 0;
+
+	return 1;
+}
+
 /**
  * This function adds buffers to the virtio devices RX virtqueue. Buffers can
  * be received from the physical port or from another virtio device. A packet
@@ -68,12 +78,14 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id,
 	uint8_t success = 0;
 
 	LOG_DEBUG(VHOST_DATA, "(%"PRIu64") virtio_dev_rx()\n", dev->device_fh);
-	if (unlikely(queue_id != VIRTIO_RXQ)) {
-		LOG_DEBUG(VHOST_DATA, "mq isn't supported in this version.\n");
+	if (unlikely(!is_valid_virt_queue_idx(queue_id, 0, dev->virt_qp_nb))) {
+		RTE_LOG(ERR, VHOST_DATA,
+			"%s (%"PRIu64"): virtqueue idx:%d invalid.\n",
+			__func__, dev->device_fh, queue_id);
 		return 0;
 	}
 
-	vq = dev->virtqueue[VIRTIO_RXQ];
+	vq = dev->virtqueue[queue_id];
 	count = (count > MAX_PKT_BURST) ? MAX_PKT_BURST : count;
 
 	/*
@@ -235,8 +247,9 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id,
 }
 
 static inline uint32_t __attribute__((always_inline))
-copy_from_mbuf_to_vring(struct virtio_net *dev, uint16_t res_base_idx,
-	uint16_t res_end_idx, struct rte_mbuf *pkt)
+copy_from_mbuf_to_vring(struct virtio_net *dev, uint32_t queue_id,
+			uint16_t res_base_idx, uint16_t res_end_idx,
+			struct rte_mbuf *pkt)
 {
 	uint32_t vec_idx = 0;
 	uint32_t entry_success = 0;
@@ -264,7 +277,7 @@ copy_from_mbuf_to_vring(struct virtio_net *dev, uint16_t res_base_idx,
 	 * Convert from gpa to vva
 	 * (guest physical addr -> vhost virtual addr)
 	 */
-	vq = dev->virtqueue[VIRTIO_RXQ];
+	vq = dev->virtqueue[queue_id];
 	vb_addr = gpa_to_vva(dev, vq->buf_vec[vec_idx].buf_addr);
 	vb_hdr_addr = vb_addr;
 
@@ -464,11 +477,14 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id,
 
 	LOG_DEBUG(VHOST_DATA, "(%"PRIu64") virtio_dev_merge_rx()\n",
 		dev->device_fh);
-	if (unlikely(queue_id != VIRTIO_RXQ)) {
-		LOG_DEBUG(VHOST_DATA, "mq isn't supported in this version.\n");
+	if (unlikely(!is_valid_virt_queue_idx(queue_id, 0, dev->virt_qp_nb))) {
+		RTE_LOG(ERR, VHOST_DATA,
+			"%s (%"PRIu64"): virtqueue idx:%d invalid.\n",
+			__func__, dev->device_fh, queue_id);
+		return 0;
 	}
 
-	vq = dev->virtqueue[VIRTIO_RXQ];
+	vq = dev->virtqueue[queue_id];
 	count = RTE_MIN((uint32_t)MAX_PKT_BURST, count);
 
 	if (count == 0)
@@ -509,8 +525,8 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id,
 							res_cur_idx);
 		} while (success == 0);
 
-		entry_success = copy_from_mbuf_to_vring(dev, res_base_idx,
-			res_cur_idx, pkts[pkt_idx]);
+		entry_success = copy_from_mbuf_to_vring(dev, queue_id,
+			res_base_idx, res_cur_idx, pkts[pkt_idx]);
 
 		rte_compiler_barrier();
 
@@ -562,12 +578,14 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t queue_id,
 	uint16_t free_entries, entry_success = 0;
 	uint16_t avail_idx;
 
-	if (unlikely(queue_id != VIRTIO_TXQ)) {
-		LOG_DEBUG(VHOST_DATA, "mq isn't supported in this version.\n");
+	if (unlikely(!is_valid_virt_queue_idx(queue_id, 1, dev->virt_qp_nb))) {
+		RTE_LOG(ERR, VHOST_DATA,
+			"%s (%"PRIu64"): virtqueue idx:%d invalid.\n",
+			__func__, dev->device_fh, queue_id);
 		return 0;
 	}
 
-	vq = dev->virtqueue[VIRTIO_TXQ];
+	vq = dev->virtqueue[queue_id];
 	avail_idx =  *((volatile uint16_t *)&vq->avail->idx);
 
 	/* If there are no available buffers then return. */
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v6 05/13] vhost-user: handle VHOST_USER_RESET_OWNER correctly
  2015-10-09  5:45 [PATCH v6 00/13] vhost-user multiple queues enabling Yuanhan Liu
                   ` (3 preceding siblings ...)
  2015-10-09  5:46 ` [PATCH v6 04/13] vhost: rxtx: prepare work " Yuanhan Liu
@ 2015-10-09  5:46 ` Yuanhan Liu
  2015-10-20  7:03   ` Thomas Monjalon
  2015-10-09  5:46 ` [PATCH v6 06/13] virtio: read virtio_net_config correctly Yuanhan Liu
                   ` (10 subsequent siblings)
  15 siblings, 1 reply; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-09  5:46 UTC (permalink / raw)
  To: dev; +Cc: marcel, Michael S. Tsirkin

Destroy corresponding device when a VHOST_USER_RESET_OWNER message is
received, otherwise, the vhost-switch would still try to access vq
of that device, which results to SIGSEG fault, and let vhost-switch
crash in the end.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 lib/librte_vhost/vhost_user/vhost-net-user.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.c b/lib/librte_vhost/vhost_user/vhost-net-user.c
index 8675cd4..f802b77 100644
--- a/lib/librte_vhost/vhost_user/vhost-net-user.c
+++ b/lib/librte_vhost/vhost_user/vhost-net-user.c
@@ -379,7 +379,7 @@ vserver_message_handler(int connfd, void *dat, int *remove)
 		ops->set_owner(ctx);
 		break;
 	case VHOST_USER_RESET_OWNER:
-		ops->reset_owner(ctx);
+		user_destroy_device(ctx);
 		break;
 
 	case VHOST_USER_SET_MEM_TABLE:
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v6 06/13] virtio: read virtio_net_config correctly
  2015-10-09  5:45 [PATCH v6 00/13] vhost-user multiple queues enabling Yuanhan Liu
                   ` (4 preceding siblings ...)
  2015-10-09  5:46 ` [PATCH v6 05/13] vhost-user: handle VHOST_USER_RESET_OWNER correctly Yuanhan Liu
@ 2015-10-09  5:46 ` Yuanhan Liu
  2015-10-20  7:07   ` Thomas Monjalon
  2015-10-09  5:46 ` [PATCH v6 07/13] vhost-user: enable vhost-user multiple queue Yuanhan Liu
                   ` (9 subsequent siblings)
  15 siblings, 1 reply; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-09  5:46 UTC (permalink / raw)
  To: dev; +Cc: Michael S. Tsirkin, marcel

From: Changchun Ouyang <changchun.ouyang@intel.com>

The old code adjusts the config bytes we want to read depending on
what kind of features we have, but we later cast the entire buf we
read with "struct virtio_net_config", which is obviously wrong.

The right way to go is to read related config bytes when corresponding
feature is set, which is exactly what this patch does.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

---
v6: read mac unconditionally.
---
 drivers/net/virtio/virtio_ethdev.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 465d3cd..e6aa1f7 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1162,7 +1162,6 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 	struct virtio_hw *hw = eth_dev->data->dev_private;
 	struct virtio_net_config *config;
 	struct virtio_net_config local_config;
-	uint32_t offset_conf = sizeof(config->mac);
 	struct rte_pci_device *pci_dev;
 
 	RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM < sizeof(struct virtio_net_hdr));
@@ -1221,8 +1220,14 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 	if (vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VQ)) {
 		config = &local_config;
 
+		vtpci_read_dev_config(hw,
+			offsetof(struct virtio_net_config, mac),
+			&config->mac, sizeof(config->mac));
+
 		if (vtpci_with_feature(hw, VIRTIO_NET_F_STATUS)) {
-			offset_conf += sizeof(config->status);
+			vtpci_read_dev_config(hw,
+				offsetof(struct virtio_net_config, status),
+				&config->status, sizeof(config->status));
 		} else {
 			PMD_INIT_LOG(DEBUG,
 				     "VIRTIO_NET_F_STATUS is not supported");
@@ -1230,15 +1235,16 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 		}
 
 		if (vtpci_with_feature(hw, VIRTIO_NET_F_MQ)) {
-			offset_conf += sizeof(config->max_virtqueue_pairs);
+			vtpci_read_dev_config(hw,
+				offsetof(struct virtio_net_config, max_virtqueue_pairs),
+				&config->max_virtqueue_pairs,
+				sizeof(config->max_virtqueue_pairs));
 		} else {
 			PMD_INIT_LOG(DEBUG,
 				     "VIRTIO_NET_F_MQ is not supported");
 			config->max_virtqueue_pairs = 1;
 		}
 
-		vtpci_read_dev_config(hw, 0, (uint8_t *)config, offset_conf);
-
 		hw->max_rx_queues =
 			(VIRTIO_MAX_RX_QUEUES < config->max_virtqueue_pairs) ?
 			VIRTIO_MAX_RX_QUEUES : config->max_virtqueue_pairs;
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v6 07/13] vhost-user: enable vhost-user multiple queue
  2015-10-09  5:45 [PATCH v6 00/13] vhost-user multiple queues enabling Yuanhan Liu
                   ` (5 preceding siblings ...)
  2015-10-09  5:46 ` [PATCH v6 06/13] virtio: read virtio_net_config correctly Yuanhan Liu
@ 2015-10-09  5:46 ` Yuanhan Liu
  2015-10-09  5:46 ` [PATCH v6 08/13] vhost: add VHOST_USER_SET_VRING_ENABLE message Yuanhan Liu
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-09  5:46 UTC (permalink / raw)
  To: dev; +Cc: marcel, Michael S. Tsirkin

By setting VHOST_USER_PROTOCOL_F_MQ protocol feature bit, and
VIRTIO_NET_F_MQ feature bit.

Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 lib/librte_vhost/vhost_user/virtio-net-user.h | 4 +++-
 lib/librte_vhost/virtio-net.c                 | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/librte_vhost/vhost_user/virtio-net-user.h b/lib/librte_vhost/vhost_user/virtio-net-user.h
index e7a6ff4..5f6d667 100644
--- a/lib/librte_vhost/vhost_user/virtio-net-user.h
+++ b/lib/librte_vhost/vhost_user/virtio-net-user.h
@@ -37,7 +37,9 @@
 #include "vhost-net.h"
 #include "vhost-net-user.h"
 
-#define VHOST_USER_PROTOCOL_FEATURES	0ULL
+#define VHOST_USER_PROTOCOL_F_MQ	0
+
+#define VHOST_USER_PROTOCOL_FEATURES	(1ULL << VHOST_USER_PROTOCOL_F_MQ)
 
 int user_set_mem_table(struct vhost_device_ctx, struct VhostUserMsg *);
 
diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
index 57fb7b1..d644022 100644
--- a/lib/librte_vhost/virtio-net.c
+++ b/lib/librte_vhost/virtio-net.c
@@ -74,6 +74,7 @@ static struct virtio_net_config_ll *ll_root;
 #define VHOST_SUPPORTED_FEATURES ((1ULL << VIRTIO_NET_F_MRG_RXBUF) | \
 				(1ULL << VIRTIO_NET_F_CTRL_VQ) | \
 				(1ULL << VIRTIO_NET_F_CTRL_RX) | \
+				(1ULL << VIRTIO_NET_F_MQ)      | \
 				(1ULL << VHOST_F_LOG_ALL)      | \
 				(1ULL << VHOST_USER_F_PROTOCOL_FEATURES))
 static uint64_t VHOST_FEATURES = VHOST_SUPPORTED_FEATURES;
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v6 08/13] vhost: add VHOST_USER_SET_VRING_ENABLE message
  2015-10-09  5:45 [PATCH v6 00/13] vhost-user multiple queues enabling Yuanhan Liu
                   ` (6 preceding siblings ...)
  2015-10-09  5:46 ` [PATCH v6 07/13] vhost-user: enable vhost-user multiple queue Yuanhan Liu
@ 2015-10-09  5:46 ` Yuanhan Liu
  2015-10-09  5:46 ` [PATCH v6 09/13] vhost: add API bind a virtq to a specific core Yuanhan Liu
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-09  5:46 UTC (permalink / raw)
  To: dev; +Cc: Michael S. Tsirkin, marcel

From: Changchun Ouyang <changchun.ouyang@intel.com>

This message is used to enable/disable a specific vring queue pair.
The first queue pair is enabled by default.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

---
v6: add a vring state changed callback, for informing the application
    that a specific vring is enabled/disabled. You could either flush
    packets haven't been processed yet, or simply just drop them.
---
 lib/librte_vhost/rte_virtio_net.h             |  9 ++++++++-
 lib/librte_vhost/vhost_rxtx.c                 | 10 ++++++++++
 lib/librte_vhost/vhost_user/vhost-net-user.c  |  5 +++++
 lib/librte_vhost/vhost_user/vhost-net-user.h  |  1 +
 lib/librte_vhost/vhost_user/virtio-net-user.c | 28 +++++++++++++++++++++++++++
 lib/librte_vhost/vhost_user/virtio-net-user.h |  3 +++
 lib/librte_vhost/virtio-net.c                 | 12 +++++++++---
 7 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/lib/librte_vhost/rte_virtio_net.h b/lib/librte_vhost/rte_virtio_net.h
index 5dd6493..fd87f01 100644
--- a/lib/librte_vhost/rte_virtio_net.h
+++ b/lib/librte_vhost/rte_virtio_net.h
@@ -89,6 +89,7 @@ struct vhost_virtqueue {
 	volatile uint16_t	last_used_idx_res;	/**< Used for multiple devices reserving buffers. */
 	int			callfd;			/**< Used to notify the guest (trigger interrupt). */
 	int			kickfd;			/**< Currently unused as polling mode is enabled. */
+	int			enabled;
 	struct buf_vector	buf_vec[BUF_VECTOR_MAX];	/**< for scatter RX. */
 } __rte_cache_aligned;
 
@@ -132,7 +133,7 @@ struct virtio_memory {
 };
 
 /**
- * Device operations to add/remove device.
+ * Device and vring operations.
  *
  * Make sure to set VIRTIO_DEV_RUNNING to the device flags in new_device and
  * remove it in destroy_device.
@@ -141,12 +142,18 @@ struct virtio_memory {
 struct virtio_net_device_ops {
 	int (*new_device)(struct virtio_net *);	/**< Add device. */
 	void (*destroy_device)(volatile struct virtio_net *);	/**< Remove device. */
+
+	int (*vring_state_changed)(struct virtio_net *dev, uint16_t queue_id, int enable);	/**< triggered when a vring is enabled or disabled */
 };
 
 static inline uint16_t __attribute__((always_inline))
 rte_vring_available_entries(struct virtio_net *dev, uint16_t queue_id)
 {
 	struct vhost_virtqueue *vq = dev->virtqueue[queue_id];
+
+	if (vq->enabled)
+		return 0;
+
 	return *(volatile uint16_t *)&vq->avail->idx - vq->last_used_idx_res;
 }
 
diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c
index 14e00ef..400f263 100644
--- a/lib/librte_vhost/vhost_rxtx.c
+++ b/lib/librte_vhost/vhost_rxtx.c
@@ -86,6 +86,9 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id,
 	}
 
 	vq = dev->virtqueue[queue_id];
+	if (unlikely(vq->enabled == 0))
+		return 0;
+
 	count = (count > MAX_PKT_BURST) ? MAX_PKT_BURST : count;
 
 	/*
@@ -278,6 +281,7 @@ copy_from_mbuf_to_vring(struct virtio_net *dev, uint32_t queue_id,
 	 * (guest physical addr -> vhost virtual addr)
 	 */
 	vq = dev->virtqueue[queue_id];
+
 	vb_addr = gpa_to_vva(dev, vq->buf_vec[vec_idx].buf_addr);
 	vb_hdr_addr = vb_addr;
 
@@ -485,6 +489,9 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id,
 	}
 
 	vq = dev->virtqueue[queue_id];
+	if (unlikely(vq->enabled == 0))
+		return 0;
+
 	count = RTE_MIN((uint32_t)MAX_PKT_BURST, count);
 
 	if (count == 0)
@@ -586,6 +593,9 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t queue_id,
 	}
 
 	vq = dev->virtqueue[queue_id];
+	if (unlikely(vq->enabled == 0))
+		return 0;
+
 	avail_idx =  *((volatile uint16_t *)&vq->avail->idx);
 
 	/* If there are no available buffers then return. */
diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.c b/lib/librte_vhost/vhost_user/vhost-net-user.c
index f802b77..8fad385 100644
--- a/lib/librte_vhost/vhost_user/vhost-net-user.c
+++ b/lib/librte_vhost/vhost_user/vhost-net-user.c
@@ -99,6 +99,7 @@ static const char *vhost_message_str[VHOST_USER_MAX] = {
 	[VHOST_USER_GET_PROTOCOL_FEATURES]  = "VHOST_USER_GET_PROTOCOL_FEATURES",
 	[VHOST_USER_SET_PROTOCOL_FEATURES]  = "VHOST_USER_SET_PROTOCOL_FEATURES",
 	[VHOST_USER_GET_QUEUE_NUM]  = "VHOST_USER_GET_QUEUE_NUM",
+	[VHOST_USER_SET_VRING_ENABLE]  = "VHOST_USER_SET_VRING_ENABLE",
 };
 
 /**
@@ -428,6 +429,10 @@ vserver_message_handler(int connfd, void *dat, int *remove)
 		send_vhost_message(connfd, &msg);
 		break;
 
+	case VHOST_USER_SET_VRING_ENABLE:
+		user_set_vring_enable(ctx, &msg.payload.state);
+		break;
+
 	default:
 		break;
 
diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.h b/lib/librte_vhost/vhost_user/vhost-net-user.h
index 389d21d..38637cc 100644
--- a/lib/librte_vhost/vhost_user/vhost-net-user.h
+++ b/lib/librte_vhost/vhost_user/vhost-net-user.h
@@ -66,6 +66,7 @@ typedef enum VhostUserRequest {
 	VHOST_USER_GET_PROTOCOL_FEATURES = 15,
 	VHOST_USER_SET_PROTOCOL_FEATURES = 16,
 	VHOST_USER_GET_QUEUE_NUM = 17,
+	VHOST_USER_SET_VRING_ENABLE = 18,
 	VHOST_USER_MAX
 } VhostUserRequest;
 
diff --git a/lib/librte_vhost/vhost_user/virtio-net-user.c b/lib/librte_vhost/vhost_user/virtio-net-user.c
index e83d279..35e3ddc 100644
--- a/lib/librte_vhost/vhost_user/virtio-net-user.c
+++ b/lib/librte_vhost/vhost_user/virtio-net-user.c
@@ -306,6 +306,34 @@ user_get_vring_base(struct vhost_device_ctx ctx,
 	return 0;
 }
 
+/*
+ * when virtio queues are ready to work, qemu will send us to
+ * enable the virtio queue pair.
+ */
+int
+user_set_vring_enable(struct vhost_device_ctx ctx,
+		      struct vhost_vring_state *state)
+{
+	struct virtio_net *dev = get_device(ctx);
+	uint16_t base_idx = state->index;
+	int enable = (int)state->num;
+
+	RTE_LOG(INFO, VHOST_CONFIG,
+		"set queue enable: %d to qp idx: %d\n",
+		enable, state->index);
+
+	if (dev->protocol_features & (1 << VHOST_USER_PROTOCOL_F_MQ) &&
+	    notify_ops->vring_state_changed) {
+		notify_ops->vring_state_changed(dev, base_idx + VIRTIO_RXQ, enable);
+		notify_ops->vring_state_changed(dev, base_idx + VIRTIO_TXQ, enable);
+	}
+
+	dev->virtqueue[base_idx + VIRTIO_RXQ]->enabled = enable;
+	dev->virtqueue[base_idx + VIRTIO_TXQ]->enabled = enable;
+
+	return 0;
+}
+
 void
 user_destroy_device(struct vhost_device_ctx ctx)
 {
diff --git a/lib/librte_vhost/vhost_user/virtio-net-user.h b/lib/librte_vhost/vhost_user/virtio-net-user.h
index 5f6d667..b82108d 100644
--- a/lib/librte_vhost/vhost_user/virtio-net-user.h
+++ b/lib/librte_vhost/vhost_user/virtio-net-user.h
@@ -52,5 +52,8 @@ void user_set_protocol_features(struct vhost_device_ctx ctx,
 
 int user_get_vring_base(struct vhost_device_ctx, struct vhost_vring_state *);
 
+int user_set_vring_enable(struct vhost_device_ctx ctx,
+			  struct vhost_vring_state *state);
+
 void user_destroy_device(struct vhost_device_ctx);
 #endif
diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
index d644022..b11fd61 100644
--- a/lib/librte_vhost/virtio-net.c
+++ b/lib/librte_vhost/virtio-net.c
@@ -255,7 +255,7 @@ rm_config_ll_entry(struct virtio_net_config_ll *ll_dev,
 }
 
 static void
-init_vring_queue(struct vhost_virtqueue *vq)
+init_vring_queue(struct vhost_virtqueue *vq, int qp_idx)
 {
 	memset(vq, 0, sizeof(struct vhost_virtqueue));
 
@@ -264,13 +264,19 @@ init_vring_queue(struct vhost_virtqueue *vq)
 
 	/* Backends are set to -1 indicating an inactive device. */
 	vq->backend = -1;
+
+	/* always set the default vq pair to enabled */
+	if (qp_idx == 0)
+		vq->enabled = 1;
 }
 
 static void
 init_vring_queue_pair(struct virtio_net *dev, uint32_t qp_idx)
 {
-	init_vring_queue(dev->virtqueue[qp_idx * VIRTIO_QNUM + VIRTIO_RXQ]);
-	init_vring_queue(dev->virtqueue[qp_idx * VIRTIO_QNUM + VIRTIO_TXQ]);
+	uint32_t base_idx = qp_idx * VIRTIO_QNUM;
+
+	init_vring_queue(dev->virtqueue[base_idx + VIRTIO_RXQ], qp_idx);
+	init_vring_queue(dev->virtqueue[base_idx + VIRTIO_TXQ], qp_idx);
 }
 
 static int
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v6 09/13] vhost: add API bind a virtq to a specific core
  2015-10-09  5:45 [PATCH v6 00/13] vhost-user multiple queues enabling Yuanhan Liu
                   ` (7 preceding siblings ...)
  2015-10-09  5:46 ` [PATCH v6 08/13] vhost: add VHOST_USER_SET_VRING_ENABLE message Yuanhan Liu
@ 2015-10-09  5:46 ` Yuanhan Liu
  2015-10-20  6:16   ` Xie, Huawei
  2015-10-09  5:46 ` [PATCH v6 10/13] ixgbe: support VMDq RSS in non-SRIOV environment Yuanhan Liu
                   ` (6 subsequent siblings)
  15 siblings, 1 reply; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-09  5:46 UTC (permalink / raw)
  To: dev; +Cc: Michael S. Tsirkin, marcel

From: Changchun Ouyang <changchun.ouyang@intel.com>

The new API rte_vhost_core_id_set() is to bind a virtq to a specific
core, while the another API rte_vhost_core_id_get() is for getting
the bind core for a virtq.

The usage, which will be introduced soon, could be find at examles/vhost/main.c.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 lib/librte_vhost/rte_vhost_version.map |  7 +++++++
 lib/librte_vhost/rte_virtio_net.h      | 25 +++++++++++++++++++++++++
 lib/librte_vhost/virtio-net.c          | 25 +++++++++++++++++++++++++
 3 files changed, 57 insertions(+)

diff --git a/lib/librte_vhost/rte_vhost_version.map b/lib/librte_vhost/rte_vhost_version.map
index 3d8709e..2ce141c 100644
--- a/lib/librte_vhost/rte_vhost_version.map
+++ b/lib/librte_vhost/rte_vhost_version.map
@@ -18,5 +18,12 @@ DPDK_2.1 {
 	global:
 
 	rte_vhost_driver_unregister;
+} DPDK_2.0;
+
+
+DPDK_2.2 {
+	global:
 
+	rte_vhost_core_id_get;
+	rte_vhost_core_id_set;
 } DPDK_2.0;
diff --git a/lib/librte_vhost/rte_virtio_net.h b/lib/librte_vhost/rte_virtio_net.h
index fd87f01..3b75d18 100644
--- a/lib/librte_vhost/rte_virtio_net.h
+++ b/lib/librte_vhost/rte_virtio_net.h
@@ -90,6 +90,7 @@ struct vhost_virtqueue {
 	int			callfd;			/**< Used to notify the guest (trigger interrupt). */
 	int			kickfd;			/**< Currently unused as polling mode is enabled. */
 	int			enabled;
+	uint32_t		core_id;		/**< Data core that the vq is attached to */
 	struct buf_vector	buf_vec[BUF_VECTOR_MAX];	/**< for scatter RX. */
 } __rte_cache_aligned;
 
@@ -244,4 +245,28 @@ uint16_t rte_vhost_enqueue_burst(struct virtio_net *dev, uint16_t queue_id,
 uint16_t rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t queue_id,
 	struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count);
 
+/**
+ * This function get the data core id for queue pair in one vhost device.
+ * @param dev
+ *  virtio-net device
+ * @param queue_id
+ *  virtio queue index in mq case
+ * @return
+ *  core id of queue pair of specified virtio device.
+ */
+uint16_t rte_vhost_core_id_get(volatile struct virtio_net *dev,
+			       uint16_t queue_id);
+
+/**
+ * This function set the data core id for queue pair in one vhost device.
+ * @param dev
+ *  virtio-net device
+ * @param queue_id
+ *  virtio queue index in mq case
+ * @param core_id
+ *  data core id for virtio queue pair in mq case
+ */
+void rte_vhost_core_id_set(struct virtio_net *dev, uint16_t queue_id,
+			   uint16_t core_id);
+
 #endif /* _VIRTIO_NET_H_ */
diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
index b11fd61..d304ee6 100644
--- a/lib/librte_vhost/virtio-net.c
+++ b/lib/librte_vhost/virtio-net.c
@@ -868,6 +868,31 @@ int rte_vhost_feature_enable(uint64_t feature_mask)
 	return -1;
 }
 
+uint16_t
+rte_vhost_core_id_get(volatile struct virtio_net *dev, uint16_t queue_id)
+{
+	if (dev == NULL)
+		return 0;
+
+	if (dev->virtqueue == NULL || dev->virtqueue[queue_id] == NULL)
+		return 0;
+
+	return dev->virtqueue[queue_id]->core_id;
+}
+
+void
+rte_vhost_core_id_set(struct virtio_net *dev, uint16_t queue_id,
+		      uint16_t core_id)
+{
+	if (dev == NULL)
+		return;
+
+	if (dev->virtqueue == NULL || dev->virtqueue[queue_id] == NULL)
+		return;
+
+	dev->virtqueue[queue_id]->core_id = core_id;
+}
+
 /*
  * Register ops so that we can add/remove device to data core.
  */
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v6 10/13] ixgbe: support VMDq RSS in non-SRIOV environment
  2015-10-09  5:45 [PATCH v6 00/13] vhost-user multiple queues enabling Yuanhan Liu
                   ` (8 preceding siblings ...)
  2015-10-09  5:46 ` [PATCH v6 09/13] vhost: add API bind a virtq to a specific core Yuanhan Liu
@ 2015-10-09  5:46 ` Yuanhan Liu
  2015-10-20  7:46   ` Thomas Monjalon
  2015-10-21 10:05   ` Ananyev, Konstantin
  2015-10-09  5:46 ` [PATCH v6 11/13] examples/vhost: demonstrate the usage of vhost mq feature Yuanhan Liu
                   ` (5 subsequent siblings)
  15 siblings, 2 replies; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-09  5:46 UTC (permalink / raw)
  To: dev; +Cc: Michael S. Tsirkin, marcel

From: Changchun Ouyang <changchun.ouyang@intel.com>

In non-SRIOV environment, VMDq RSS could be enabled by MRQC register.
In theory, the queue number per pool could be 2 or 4, but only 2 queues
are available due to HW limitation, the same limit also exists in Linux
ixgbe driver.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 drivers/net/ixgbe/ixgbe_rxtx.c | 86 +++++++++++++++++++++++++++++++++++-------
 lib/librte_ether/rte_ethdev.c  | 11 ++++++
 2 files changed, 84 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index a598a72..e502fe8 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -3445,16 +3445,16 @@ void ixgbe_configure_dcb(struct rte_eth_dev *dev)
 	return;
 }
 
-/*
- * VMDq only support for 10 GbE NIC.
+/**
+ * Config pool for VMDq on 10 GbE NIC.
  */
 static void
-ixgbe_vmdq_rx_hw_configure(struct rte_eth_dev *dev)
+ixgbe_vmdq_pool_configure(struct rte_eth_dev *dev)
 {
 	struct rte_eth_vmdq_rx_conf *cfg;
 	struct ixgbe_hw *hw;
 	enum rte_eth_nb_pools num_pools;
-	uint32_t mrqc, vt_ctl, vlanctrl;
+	uint32_t vt_ctl, vlanctrl;
 	uint32_t vmolr = 0;
 	int i;
 
@@ -3463,12 +3463,6 @@ ixgbe_vmdq_rx_hw_configure(struct rte_eth_dev *dev)
 	cfg = &dev->data->dev_conf.rx_adv_conf.vmdq_rx_conf;
 	num_pools = cfg->nb_queue_pools;
 
-	ixgbe_rss_disable(dev);
-
-	/* MRQC: enable vmdq */
-	mrqc = IXGBE_MRQC_VMDQEN;
-	IXGBE_WRITE_REG(hw, IXGBE_MRQC, mrqc);
-
 	/* PFVTCTL: turn on virtualisation and set the default pool */
 	vt_ctl = IXGBE_VT_CTL_VT_ENABLE | IXGBE_VT_CTL_REPLEN;
 	if (cfg->enable_default_pool)
@@ -3534,7 +3528,29 @@ ixgbe_vmdq_rx_hw_configure(struct rte_eth_dev *dev)
 	IXGBE_WRITE_FLUSH(hw);
 }
 
-/*
+/**
+ * VMDq only support for 10 GbE NIC.
+ */
+static void
+ixgbe_vmdq_rx_hw_configure(struct rte_eth_dev *dev)
+{
+	struct ixgbe_hw *hw;
+	uint32_t mrqc;
+
+	PMD_INIT_FUNC_TRACE();
+	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	ixgbe_rss_disable(dev);
+
+	/* MRQC: enable vmdq */
+	mrqc = IXGBE_MRQC_VMDQEN;
+	IXGBE_WRITE_REG(hw, IXGBE_MRQC, mrqc);
+	IXGBE_WRITE_FLUSH(hw);
+
+	ixgbe_vmdq_pool_configure(dev);
+}
+
+/**
  * ixgbe_dcb_config_tx_hw_config - Configure general VMDq TX parameters
  * @hw: pointer to hardware structure
  */
@@ -3639,6 +3655,41 @@ ixgbe_config_vf_rss(struct rte_eth_dev *dev)
 }
 
 static int
+ixgbe_config_vmdq_rss(struct rte_eth_dev *dev)
+{
+	struct ixgbe_hw *hw;
+	uint32_t mrqc;
+
+	ixgbe_rss_configure(dev);
+
+	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	/* MRQC: enable VMDQ RSS */
+	mrqc = IXGBE_READ_REG(hw, IXGBE_MRQC);
+	mrqc &= ~IXGBE_MRQC_MRQE_MASK;
+
+	switch (RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool) {
+	case 2:
+		mrqc |= IXGBE_MRQC_VMDQRSS64EN;
+		break;
+
+	case 4:
+		mrqc |= IXGBE_MRQC_VMDQRSS32EN;
+		break;
+
+	default:
+		PMD_INIT_LOG(ERR, "Invalid pool number in non-IOV mode with VMDQ RSS");
+		return -EINVAL;
+	}
+
+	IXGBE_WRITE_REG(hw, IXGBE_MRQC, mrqc);
+
+	ixgbe_vmdq_pool_configure(dev);
+
+	return 0;
+}
+
+static int
 ixgbe_config_vf_default(struct rte_eth_dev *dev)
 {
 	struct ixgbe_hw *hw =
@@ -3694,6 +3745,10 @@ ixgbe_dev_mq_rx_configure(struct rte_eth_dev *dev)
 				ixgbe_vmdq_rx_hw_configure(dev);
 				break;
 
+			case ETH_MQ_RX_VMDQ_RSS:
+				ixgbe_config_vmdq_rss(dev);
+				break;
+
 			case ETH_MQ_RX_NONE:
 				/* if mq_mode is none, disable rss mode.*/
 			default: ixgbe_rss_disable(dev);
@@ -4186,6 +4241,8 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 
 	/* Setup RX queues */
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
+		uint32_t psrtype = 0;
+
 		rxq = dev->data->rx_queues[i];
 
 		/*
@@ -4213,12 +4270,10 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 		if (rx_conf->header_split) {
 			if (hw->mac.type == ixgbe_mac_82599EB) {
 				/* Must setup the PSRTYPE register */
-				uint32_t psrtype;
 				psrtype = IXGBE_PSRTYPE_TCPHDR |
 					IXGBE_PSRTYPE_UDPHDR   |
 					IXGBE_PSRTYPE_IPV4HDR  |
 					IXGBE_PSRTYPE_IPV6HDR;
-				IXGBE_WRITE_REG(hw, IXGBE_PSRTYPE(rxq->reg_idx), psrtype);
 			}
 			srrctl = ((rx_conf->split_hdr_size <<
 				IXGBE_SRRCTL_BSIZEHDRSIZE_SHIFT) &
@@ -4228,6 +4283,11 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 #endif
 			srrctl = IXGBE_SRRCTL_DESCTYPE_ADV_ONEBUF;
 
+		/* Set RQPL for VMDQ RSS according to max Rx queue */
+		psrtype |= (RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool >> 1) <<
+			IXGBE_PSRTYPE_RQPL_SHIFT;
+		IXGBE_WRITE_REG(hw, IXGBE_PSRTYPE(rxq->reg_idx), psrtype);
+
 		/* Set if packets are dropped when no descriptors available */
 		if (rxq->drop_en)
 			srrctl |= IXGBE_SRRCTL_DROP_EN;
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index f593f6e..fe9dc5c 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1067,6 +1067,17 @@ rte_eth_dev_check_mq_mode(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 				return -EINVAL;
 			}
 		}
+
+		if (dev_conf->rxmode.mq_mode == ETH_MQ_RX_VMDQ_RSS) {
+			uint32_t nb_queue_pools =
+				dev_conf->rx_adv_conf.vmdq_rx_conf.nb_queue_pools;
+			struct rte_eth_dev_info dev_info;
+
+			rte_eth_dev_info_get(port_id, &dev_info);
+			dev->data->dev_conf.rxmode.mq_mode = ETH_MQ_RX_VMDQ_RSS;
+			RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool =
+				dev_info.vmdq_queue_num / nb_queue_pools;
+		}
 	}
 	return 0;
 }
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v6 11/13] examples/vhost: demonstrate the usage of vhost mq feature
  2015-10-09  5:45 [PATCH v6 00/13] vhost-user multiple queues enabling Yuanhan Liu
                   ` (9 preceding siblings ...)
  2015-10-09  5:46 ` [PATCH v6 10/13] ixgbe: support VMDq RSS in non-SRIOV environment Yuanhan Liu
@ 2015-10-09  5:46 ` Yuanhan Liu
  2015-10-09  5:46 ` [PATCH v6 12/13] examples/vhost: add per queue stats Yuanhan Liu
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-09  5:46 UTC (permalink / raw)
  To: dev; +Cc: Michael S. Tsirkin, marcel

From: Changchun Ouyang <changchun.ouyang@intel.com>

This patch demonstrates the usage of vhost mq feature, by leveraging
the VMDq+RSS HW feature to receive packets and distribute them into
different queue in the pool according to 5 tuples.

Queue number is specified by the --rxq option.

HW queue numbers in pool is exactly same with the queue number in virtio
device, e.g. rxq = 4, the queue number is 4, it means 4 HW queues in
each VMDq pool, and 4 queues in each virtio device/port, one maps to
each.

=========================================
==================|   |==================|
       vport0     |   |      vport1      |
---  ---  ---  ---|   |---  ---  ---  ---|
q0 | q1 | q2 | q3 |   |q0 | q1 | q2 | q3 |
/\= =/\= =/\= =/\=|   |/\= =/\= =/\= =/\=|
||   ||   ||   ||      ||   ||   ||   ||
||   ||   ||   ||      ||   ||   ||   ||
||= =||= =||= =||=|   =||== ||== ||== ||=|
q0 | q1 | q2 | q3 |   |q0 | q1 | q2 | q3 |

------------------|   |------------------|
     VMDq pool0   |   |    VMDq pool1    |
==================|   |==================|

In RX side, it firstly polls each queue of the pool and gets the
packets from it and enqueue them into its corresponding queue in
virtio device/port.  In TX side, it dequeue packets from each queue
of virtio device/port and send them to either physical port or
another virtio device according to its destination MAC address.

We bind the virtq to a specific core by rte_vhost_core_id_set(),
and later we can retrieve it by rte_vhost_core_id_get().

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 examples/vhost/main.c | 325 ++++++++++++++++++++++++++++++++++----------------
 examples/vhost/main.h |   3 +-
 2 files changed, 225 insertions(+), 103 deletions(-)

diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 9eac2d0..23b7aa7 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -163,6 +163,9 @@ static int mergeable;
 /* Do vlan strip on host, enabled on default */
 static uint32_t vlan_strip = 1;
 
+/* Rx queue number per virtio device */
+static uint32_t rxq = 1;
+
 /* number of descriptors to apply*/
 static uint32_t num_rx_descriptor = RTE_TEST_RX_DESC_DEFAULT_ZCP;
 static uint32_t num_tx_descriptor = RTE_TEST_TX_DESC_DEFAULT_ZCP;
@@ -365,6 +368,37 @@ validate_num_devices(uint32_t max_nb_devices)
 	return 0;
 }
 
+static int
+get_dev_nb_for_82599(struct rte_eth_dev_info dev_info)
+{
+	int dev_nb = -1;
+	switch (rxq) {
+	case 1:
+	case 2:
+		/*
+		 * for 82599, dev_info.max_vmdq_pools always 64 dispite rx mode.
+		 */
+		dev_nb = (int)dev_info.max_vmdq_pools;
+		break;
+	case 4:
+		dev_nb = (int)dev_info.max_vmdq_pools / 2;
+		break;
+	default:
+		RTE_LOG(ERR, VHOST_CONFIG, "invalid rxq for VMDq.\n");
+	}
+	return dev_nb;
+}
+
+static int
+get_dev_nb_for_fvl(struct rte_eth_dev_info dev_info)
+{
+	/*
+	 * for FVL, dev_info.max_vmdq_pools is calculated according to
+	 * the configured value: CONFIG_RTE_LIBRTE_I40E_QUEUE_NUM_PER_VM.
+	 */
+	return (int)dev_info.max_vmdq_pools;
+}
+
 /*
  * Initialises a given port using global settings and with the rx buffers
  * coming from the mbuf_pool passed as parameter
@@ -380,6 +414,7 @@ port_init(uint8_t port)
 	uint16_t rx_ring_size, tx_ring_size;
 	int retval;
 	uint16_t q;
+	struct rte_eth_dev *eth_dev;
 
 	/* The max pool number from dev_info will be used to validate the pool number specified in cmd line */
 	rte_eth_dev_info_get (port, &dev_info);
@@ -408,8 +443,16 @@ port_init(uint8_t port)
 		txconf->tx_deferred_start = 1;
 	}
 
-	/*configure the number of supported virtio devices based on VMDQ limits */
-	num_devices = dev_info.max_vmdq_pools;
+	/* Configure the virtio devices num based on VMDQ limits */
+	if (dev_info.max_vmdq_pools == ETH_64_POOLS) {
+		num_devices = (uint32_t)get_dev_nb_for_82599(dev_info);
+		if (num_devices == (uint32_t)-1)
+			return -1;
+	} else {
+		num_devices = (uint32_t)get_dev_nb_for_fvl(dev_info);
+		if (num_devices == (uint32_t)-1)
+			return -1;
+	}
 
 	if (zero_copy) {
 		rx_ring_size = num_rx_descriptor;
@@ -431,7 +474,7 @@ port_init(uint8_t port)
 		return retval;
 	/* NIC queues are divided into pf queues and vmdq queues.  */
 	num_pf_queues = dev_info.max_rx_queues - dev_info.vmdq_queue_num;
-	queues_per_pool = dev_info.vmdq_queue_num / dev_info.max_vmdq_pools;
+	queues_per_pool = dev_info.vmdq_queue_num / num_devices;
 	num_vmdq_queues = num_devices * queues_per_pool;
 	num_queues = num_pf_queues + num_vmdq_queues;
 	vmdq_queue_base = dev_info.vmdq_queue_base;
@@ -447,6 +490,14 @@ port_init(uint8_t port)
 	if (retval != 0)
 		return retval;
 
+	eth_dev = &rte_eth_devices[port];
+	if (RTE_ETH_DEV_SRIOV(eth_dev).nb_q_per_pool > 4) {
+		RTE_LOG(ERR, VHOST_CONFIG, "ethdev port_id=%d SRIOV active, "
+			"invalid queue number for VMDQ RSS, allowed value "
+			"are 1, 2 or 4\n", port);
+		return -EINVAL;
+	}
+
 	/* Setup the queues. */
 	for (q = 0; q < rx_rings; q ++) {
 		retval = rte_eth_rx_queue_setup(port, q, rx_ring_size,
@@ -576,7 +627,8 @@ us_vhost_usage(const char *prgname)
 	"		--rx-desc-num [0-N]: the number of descriptors on rx, "
 			"used only when zero copy is enabled.\n"
 	"		--tx-desc-num [0-N]: the number of descriptors on tx, "
-			"used only when zero copy is enabled.\n",
+			"used only when zero copy is enabled.\n"
+	"		--rxq [1,2,4]: rx queue number for each vhost device\n",
 	       prgname);
 }
 
@@ -602,6 +654,7 @@ us_vhost_parse_args(int argc, char **argv)
 		{"zero-copy", required_argument, NULL, 0},
 		{"rx-desc-num", required_argument, NULL, 0},
 		{"tx-desc-num", required_argument, NULL, 0},
+		{"rxq", required_argument, NULL, 0},
 		{NULL, 0, 0, 0},
 	};
 
@@ -778,6 +831,18 @@ us_vhost_parse_args(int argc, char **argv)
 				}
 			}
 
+			/* Specify the Rx queue number for each vhost dev. */
+			if (!strncmp(long_option[option_index].name,
+				"rxq", MAX_LONG_OPT_SZ)) {
+				ret = parse_num_opt(optarg, 4);
+				if ((ret == -1) || (ret == 0) || (!POWEROF2(ret))) {
+					RTE_LOG(INFO, VHOST_CONFIG,
+					"Valid value for rxq is [1,2,4]\n");
+					us_vhost_usage(prgname);
+					return -1;
+				} else
+					rxq = ret;
+			}
 			break;
 
 			/* Invalid option - print options. */
@@ -813,6 +878,19 @@ us_vhost_parse_args(int argc, char **argv)
 		return -1;
 	}
 
+	if (rxq > 1) {
+		vmdq_conf_default.rxmode.mq_mode = ETH_MQ_RX_VMDQ_RSS;
+		vmdq_conf_default.rx_adv_conf.rss_conf.rss_hf = ETH_RSS_IP |
+				ETH_RSS_UDP | ETH_RSS_TCP | ETH_RSS_SCTP;
+	}
+
+	if ((zero_copy == 1) && (rxq > 1)) {
+		RTE_LOG(INFO, VHOST_PORT,
+			"Vhost zero copy doesn't support mq mode,"
+			"please specify '--rxq 1' to disable it.\n");
+		return -1;
+	}
+
 	return 0;
 }
 
@@ -959,9 +1037,11 @@ link_vmdq(struct vhost_dev *vdev, struct rte_mbuf *m)
 					dev->device_fh);
 
 	/* Enable stripping of the vlan tag as we handle routing. */
-	if (vlan_strip)
-		rte_eth_dev_set_vlan_strip_on_queue(ports[0],
-			(uint16_t)vdev->vmdq_rx_q, 1);
+	if (vlan_strip) {
+		for (i = 0; i < (int)rxq; i++)
+			rte_eth_dev_set_vlan_strip_on_queue(ports[0],
+				(uint16_t)(vdev->vmdq_rx_q + i), 1);
+	}
 
 	/* Set device as ready for RX. */
 	vdev->ready = DEVICE_RX;
@@ -976,7 +1056,7 @@ link_vmdq(struct vhost_dev *vdev, struct rte_mbuf *m)
 static inline void
 unlink_vmdq(struct vhost_dev *vdev)
 {
-	unsigned i = 0;
+	unsigned i = 0, j = 0;
 	unsigned rx_count;
 	struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
 
@@ -989,15 +1069,19 @@ unlink_vmdq(struct vhost_dev *vdev)
 		vdev->vlan_tag = 0;
 
 		/*Clear out the receive buffers*/
-		rx_count = rte_eth_rx_burst(ports[0],
-					(uint16_t)vdev->vmdq_rx_q, pkts_burst, MAX_PKT_BURST);
+		for (i = 0; i < rxq; i++) {
+			rx_count = rte_eth_rx_burst(ports[0],
+					(uint16_t)vdev->vmdq_rx_q + i,
+					pkts_burst, MAX_PKT_BURST);
 
-		while (rx_count) {
-			for (i = 0; i < rx_count; i++)
-				rte_pktmbuf_free(pkts_burst[i]);
+			while (rx_count) {
+				for (j = 0; j < rx_count; j++)
+					rte_pktmbuf_free(pkts_burst[j]);
 
-			rx_count = rte_eth_rx_burst(ports[0],
-					(uint16_t)vdev->vmdq_rx_q, pkts_burst, MAX_PKT_BURST);
+				rx_count = rte_eth_rx_burst(ports[0],
+					(uint16_t)vdev->vmdq_rx_q + i,
+					pkts_burst, MAX_PKT_BURST);
+			}
 		}
 
 		vdev->ready = DEVICE_MAC_LEARNING;
@@ -1009,7 +1093,7 @@ unlink_vmdq(struct vhost_dev *vdev)
  * the packet on that devices RX queue. If not then return.
  */
 static inline int __attribute__((always_inline))
-virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)
+virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m, uint32_t qp_idx)
 {
 	struct virtio_net_data_ll *dev_ll;
 	struct ether_hdr *pkt_hdr;
@@ -1024,7 +1108,7 @@ virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)
 
 	while (dev_ll != NULL) {
 		if ((dev_ll->vdev->ready == DEVICE_RX) && ether_addr_cmp(&(pkt_hdr->d_addr),
-				          &dev_ll->vdev->mac_address)) {
+					&dev_ll->vdev->mac_address)) {
 
 			/* Drop the packet if the TX packet is destined for the TX device. */
 			if (dev_ll->vdev->dev->device_fh == dev->device_fh) {
@@ -1042,7 +1126,9 @@ virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)
 				LOG_DEBUG(VHOST_DATA, "(%"PRIu64") Device is marked for removal\n", tdev->device_fh);
 			} else {
 				/*send the packet to the local virtio device*/
-				ret = rte_vhost_enqueue_burst(tdev, VIRTIO_RXQ, &m, 1);
+				ret = rte_vhost_enqueue_burst(tdev,
+					VIRTIO_RXQ + qp_idx * VIRTIO_QNUM,
+					&m, 1);
 				if (enable_stats) {
 					rte_atomic64_add(
 					&dev_statistics[tdev->device_fh].rx_total_atomic,
@@ -1119,7 +1205,8 @@ find_local_dest(struct virtio_net *dev, struct rte_mbuf *m,
  * or the physical port.
  */
 static inline void __attribute__((always_inline))
-virtio_tx_route(struct vhost_dev *vdev, struct rte_mbuf *m, uint16_t vlan_tag)
+virtio_tx_route(struct vhost_dev *vdev, struct rte_mbuf *m,
+		uint16_t vlan_tag, uint32_t qp_idx)
 {
 	struct mbuf_table *tx_q;
 	struct rte_mbuf **m_table;
@@ -1129,7 +1216,8 @@ virtio_tx_route(struct vhost_dev *vdev, struct rte_mbuf *m, uint16_t vlan_tag)
 	struct ether_hdr *nh;
 
 	/*check if destination is local VM*/
-	if ((vm2vm_mode == VM2VM_SOFTWARE) && (virtio_tx_local(vdev, m) == 0)) {
+	if ((vm2vm_mode == VM2VM_SOFTWARE) &&
+	    (virtio_tx_local(vdev, m, qp_idx) == 0)) {
 		rte_pktmbuf_free(m);
 		return;
 	}
@@ -1293,22 +1381,26 @@ switch_worker(__attribute__((unused)) void *arg)
 			}
 			if (likely(vdev->ready == DEVICE_RX)) {
 				/*Handle guest RX*/
+				uint16_t qp_idx = dev_ll->work_qp_idx;
 				rx_count = rte_eth_rx_burst(ports[0],
-					vdev->vmdq_rx_q, pkts_burst, MAX_PKT_BURST);
+					vdev->vmdq_rx_q + qp_idx, pkts_burst, MAX_PKT_BURST);
 
 				if (rx_count) {
 					/*
 					* Retry is enabled and the queue is full then we wait and retry to avoid packet loss
 					* Here MAX_PKT_BURST must be less than virtio queue size
 					*/
-					if (enable_retry && unlikely(rx_count > rte_vring_available_entries(dev, VIRTIO_RXQ))) {
+					if (enable_retry && unlikely(rx_count > rte_vring_available_entries(dev,
+										VIRTIO_RXQ + qp_idx * VIRTIO_QNUM))) {
 						for (retry = 0; retry < burst_rx_retry_num; retry++) {
 							rte_delay_us(burst_rx_delay_time);
-							if (rx_count <= rte_vring_available_entries(dev, VIRTIO_RXQ))
+							if (rx_count <= rte_vring_available_entries(dev,
+										VIRTIO_RXQ + qp_idx * VIRTIO_QNUM))
 								break;
 						}
 					}
-					ret_count = rte_vhost_enqueue_burst(dev, VIRTIO_RXQ, pkts_burst, rx_count);
+					ret_count = rte_vhost_enqueue_burst(dev, VIRTIO_RXQ + qp_idx * VIRTIO_QNUM,
+										pkts_burst, rx_count);
 					if (enable_stats) {
 						rte_atomic64_add(
 						&dev_statistics[dev_ll->vdev->dev->device_fh].rx_total_atomic,
@@ -1320,14 +1412,18 @@ switch_worker(__attribute__((unused)) void *arg)
 						rx_count--;
 						rte_pktmbuf_free(pkts_burst[rx_count]);
 					}
-
 				}
 			}
 
 			if (likely(!vdev->remove)) {
 				/* Handle guest TX*/
-				tx_count = rte_vhost_dequeue_burst(dev, VIRTIO_TXQ, mbuf_pool, pkts_burst, MAX_PKT_BURST);
-				/* If this is the first received packet we need to learn the MAC and setup VMDQ */
+				uint16_t qp_idx = dev_ll->work_qp_idx;
+				tx_count = rte_vhost_dequeue_burst(dev, VIRTIO_TXQ + qp_idx * VIRTIO_QNUM,
+						mbuf_pool, pkts_burst, MAX_PKT_BURST);
+				/*
+				 * If this is the first received packet we need to learn
+				 * the MAC and setup VMDQ
+				 */
 				if (unlikely(vdev->ready == DEVICE_MAC_LEARNING) && tx_count) {
 					if (vdev->remove || (link_vmdq(vdev, pkts_burst[0]) == -1)) {
 						while (tx_count)
@@ -1335,7 +1431,8 @@ switch_worker(__attribute__((unused)) void *arg)
 					}
 				}
 				while (tx_count)
-					virtio_tx_route(vdev, pkts_burst[--tx_count], (uint16_t)dev->device_fh);
+					virtio_tx_route(vdev, pkts_burst[--tx_count],
+						(uint16_t)dev->device_fh, qp_idx);
 			}
 
 			/*move to the next device in the list*/
@@ -2323,6 +2420,7 @@ destroy_device (volatile struct virtio_net *dev)
 	struct virtio_net_data_ll *ll_main_dev_last = NULL;
 	struct vhost_dev *vdev;
 	int lcore;
+	uint32_t i;
 
 	dev->flags &= ~VIRTIO_DEV_RUNNING;
 
@@ -2334,61 +2432,73 @@ destroy_device (volatile struct virtio_net *dev)
 	}
 
 	/* Search for entry to be removed from lcore ll */
-	ll_lcore_dev_cur = lcore_info[vdev->coreid].lcore_ll->ll_root_used;
-	while (ll_lcore_dev_cur != NULL) {
-		if (ll_lcore_dev_cur->vdev == vdev) {
-			break;
-		} else {
-			ll_lcore_dev_last = ll_lcore_dev_cur;
-			ll_lcore_dev_cur = ll_lcore_dev_cur->next;
+	for (i = 0; i < rxq; i++) {
+		uint16_t core_id = rte_vhost_core_id_get(dev, i);
+
+		ll_lcore_dev_cur = lcore_info[core_id].lcore_ll->ll_root_used;
+
+		while (ll_lcore_dev_cur != NULL) {
+			if (ll_lcore_dev_cur->vdev == vdev) {
+				break;
+			} else {
+				ll_lcore_dev_last = ll_lcore_dev_cur;
+				ll_lcore_dev_cur = ll_lcore_dev_cur->next;
+			}
 		}
-	}
 
-	if (ll_lcore_dev_cur == NULL) {
-		RTE_LOG(ERR, VHOST_CONFIG,
-			"(%"PRIu64") Failed to find the dev to be destroy.\n",
-			dev->device_fh);
-		return;
-	}
+		if (ll_lcore_dev_cur == NULL) {
+			RTE_LOG(ERR, VHOST_CONFIG,
+				"(%"PRIu64") Failed to find the dev to be destroy.\n",
+				dev->device_fh);
+			if (i == 0)
+				return;
+			else
+				break;
+		}
 
-	/* Search for entry to be removed from main ll */
-	ll_main_dev_cur = ll_root_used;
-	ll_main_dev_last = NULL;
-	while (ll_main_dev_cur != NULL) {
-		if (ll_main_dev_cur->vdev == vdev) {
-			break;
-		} else {
-			ll_main_dev_last = ll_main_dev_cur;
-			ll_main_dev_cur = ll_main_dev_cur->next;
+		/* Search for entry to be removed from main ll */
+		if (i == 0) {
+			ll_main_dev_cur = ll_root_used;
+			ll_main_dev_last = NULL;
+			while (ll_main_dev_cur != NULL) {
+				if (ll_main_dev_cur->vdev == vdev) {
+					break;
+				} else {
+					ll_main_dev_last = ll_main_dev_cur;
+					ll_main_dev_cur = ll_main_dev_cur->next;
+				}
+			}
 		}
-	}
 
-	/* Remove entries from the lcore and main ll. */
-	rm_data_ll_entry(&lcore_info[vdev->coreid].lcore_ll->ll_root_used, ll_lcore_dev_cur, ll_lcore_dev_last);
-	rm_data_ll_entry(&ll_root_used, ll_main_dev_cur, ll_main_dev_last);
+		/* Remove entries from the lcore and main ll. */
+		rm_data_ll_entry(&lcore_info[core_id].lcore_ll->ll_root_used, ll_lcore_dev_cur, ll_lcore_dev_last);
+		if (i == 0)
+			rm_data_ll_entry(&ll_root_used, ll_main_dev_cur, ll_main_dev_last);
 
-	/* Set the dev_removal_flag on each lcore. */
-	RTE_LCORE_FOREACH_SLAVE(lcore) {
-		lcore_info[lcore].lcore_ll->dev_removal_flag = REQUEST_DEV_REMOVAL;
-	}
+		/* Set the dev_removal_flag on each lcore. */
+		RTE_LCORE_FOREACH_SLAVE(lcore) {
+			lcore_info[lcore].lcore_ll->dev_removal_flag = REQUEST_DEV_REMOVAL;
+		}
 
-	/*
-	 * Once each core has set the dev_removal_flag to ACK_DEV_REMOVAL we can be sure that
-	 * they can no longer access the device removed from the linked lists and that the devices
-	 * are no longer in use.
-	 */
-	RTE_LCORE_FOREACH_SLAVE(lcore) {
-		while (lcore_info[lcore].lcore_ll->dev_removal_flag != ACK_DEV_REMOVAL) {
-			rte_pause();
+		/*
+		 * Once each core has set the dev_removal_flag to ACK_DEV_REMOVAL we can be sure that
+		 * they can no longer access the device removed from the linked lists and that the devices
+		 * are no longer in use.
+		 */
+		RTE_LCORE_FOREACH_SLAVE(lcore) {
+			while (lcore_info[lcore].lcore_ll->dev_removal_flag != ACK_DEV_REMOVAL)
+				rte_pause();
 		}
-	}
 
-	/* Add the entries back to the lcore and main free ll.*/
-	put_data_ll_free_entry(&lcore_info[vdev->coreid].lcore_ll->ll_root_free, ll_lcore_dev_cur);
-	put_data_ll_free_entry(&ll_root_free, ll_main_dev_cur);
+		/* Add the entries back to the lcore and main free ll.*/
+		put_data_ll_free_entry(&lcore_info[core_id].lcore_ll->ll_root_free, ll_lcore_dev_cur);
+
+		if (i == 0)
+			put_data_ll_free_entry(&ll_root_free, ll_main_dev_cur);
 
-	/* Decrement number of device on the lcore. */
-	lcore_info[vdev->coreid].lcore_ll->device_num--;
+		/* Decrement number of device on the lcore. */
+		lcore_info[core_id].lcore_ll->device_num--;
+	}
 
 	RTE_LOG(INFO, VHOST_DATA, "(%"PRIu64") Device has been removed from data core\n", dev->device_fh);
 
@@ -2593,6 +2703,14 @@ new_device (struct virtio_net *dev)
 	uint32_t device_num_min = num_devices;
 	struct vhost_dev *vdev;
 	uint32_t regionidx;
+	uint32_t i;
+
+	if ((rxq > 1) && (dev->virt_qp_nb != rxq)) {
+		RTE_LOG(ERR, VHOST_DATA, "(%"PRIu64") queue num in VMDq pool:"
+			"%d != queue pair num in vhost dev:%d\n",
+			dev->device_fh, rxq, dev->virt_qp_nb);
+		return -1;
+	}
 
 	vdev = rte_zmalloc("vhost device", sizeof(*vdev), RTE_CACHE_LINE_SIZE);
 	if (vdev == NULL) {
@@ -2638,12 +2756,12 @@ new_device (struct virtio_net *dev)
 		}
 	}
 
-
 	/* Add device to main ll */
 	ll_dev = get_data_ll_free_entry(&ll_root_free);
 	if (ll_dev == NULL) {
-		RTE_LOG(INFO, VHOST_DATA, "(%"PRIu64") No free entry found in linked list. Device limit "
-			"of %d devices per core has been reached\n",
+		RTE_LOG(INFO, VHOST_DATA,
+			"(%"PRIu64") No free entry found in linked list."
+			"Device limit of %d devices per core has been reached\n",
 			dev->device_fh, num_devices);
 		if (vdev->regions_hpa)
 			rte_free(vdev->regions_hpa);
@@ -2652,8 +2770,7 @@ new_device (struct virtio_net *dev)
 	}
 	ll_dev->vdev = vdev;
 	add_data_ll_entry(&ll_root_used, ll_dev);
-	vdev->vmdq_rx_q
-		= dev->device_fh * queues_per_pool + vmdq_queue_base;
+	vdev->vmdq_rx_q	= dev->device_fh * rxq + vmdq_queue_base;
 
 	if (zero_copy) {
 		uint32_t index = vdev->vmdq_rx_q;
@@ -2734,37 +2851,42 @@ new_device (struct virtio_net *dev)
 	vdev->remove = 0;
 
 	/* Find a suitable lcore to add the device. */
-	RTE_LCORE_FOREACH_SLAVE(lcore) {
-		if (lcore_info[lcore].lcore_ll->device_num < device_num_min) {
-			device_num_min = lcore_info[lcore].lcore_ll->device_num;
-			core_add = lcore;
+	for (i = 0; i < rxq; i++) {
+		device_num_min = num_devices;
+		RTE_LCORE_FOREACH_SLAVE(lcore) {
+			if (lcore_info[lcore].lcore_ll->device_num < device_num_min) {
+				device_num_min = lcore_info[lcore].lcore_ll->device_num;
+				core_add = lcore;
+			}
 		}
-	}
-	/* Add device to lcore ll */
-	ll_dev = get_data_ll_free_entry(&lcore_info[core_add].lcore_ll->ll_root_free);
-	if (ll_dev == NULL) {
-		RTE_LOG(INFO, VHOST_DATA, "(%"PRIu64") Failed to add device to data core\n", dev->device_fh);
-		vdev->ready = DEVICE_SAFE_REMOVE;
-		destroy_device(dev);
-		rte_free(vdev->regions_hpa);
-		rte_free(vdev);
-		return -1;
-	}
-	ll_dev->vdev = vdev;
-	vdev->coreid = core_add;
+		/* Add device to lcore ll */
+		ll_dev = get_data_ll_free_entry(&lcore_info[core_add].lcore_ll->ll_root_free);
+		if (ll_dev == NULL) {
+			RTE_LOG(INFO, VHOST_DATA, "(%"PRIu64") Failed to add device to data core\n", dev->device_fh);
+			vdev->ready = DEVICE_SAFE_REMOVE;
+			destroy_device(dev);
+			rte_free(vdev->regions_hpa);
+			rte_free(vdev);
+			return -1;
+		}
+		ll_dev->vdev = vdev;
+		ll_dev->work_qp_idx = i;
+		rte_vhost_core_id_set(dev, i, core_add);
+		add_data_ll_entry(&lcore_info[core_add].lcore_ll->ll_root_used, ll_dev);
 
-	add_data_ll_entry(&lcore_info[vdev->coreid].lcore_ll->ll_root_used, ll_dev);
+		/* Disable notifications. */
+		rte_vhost_enable_guest_notification(dev, i * VIRTIO_QNUM + VIRTIO_RXQ, 0);
+		rte_vhost_enable_guest_notification(dev, i * VIRTIO_QNUM + VIRTIO_TXQ, 0);
+		lcore_info[core_add].lcore_ll->device_num++;
+		RTE_LOG(INFO, VHOST_DATA, "(%"PRIu64") Device has been added to data core %d for vq: %d\n",
+			dev->device_fh, core_add, i);
+	}
 
 	/* Initialize device stats */
 	memset(&dev_statistics[dev->device_fh], 0, sizeof(struct device_statistics));
 
-	/* Disable notifications. */
-	rte_vhost_enable_guest_notification(dev, VIRTIO_RXQ, 0);
-	rte_vhost_enable_guest_notification(dev, VIRTIO_TXQ, 0);
-	lcore_info[vdev->coreid].lcore_ll->device_num++;
 	dev->flags |= VIRTIO_DEV_RUNNING;
 
-	RTE_LOG(INFO, VHOST_DATA, "(%"PRIu64") Device has been added to data core %d\n", dev->device_fh, vdev->coreid);
 
 	return 0;
 }
@@ -2833,6 +2955,7 @@ print_stats(void)
 					rx_dropped,
 					rx);
 
+
 			dev_ll = dev_ll->next;
 		}
 		printf("\n======================================================\n");
diff --git a/examples/vhost/main.h b/examples/vhost/main.h
index d04e2be..5561c82 100644
--- a/examples/vhost/main.h
+++ b/examples/vhost/main.h
@@ -82,8 +82,6 @@ struct vhost_dev {
 	uint16_t vmdq_rx_q;
 	/**< Vlan tag assigned to the pool */
 	uint32_t vlan_tag;
-	/**< Data core that the device is added to. */
-	uint16_t coreid;
 	/**< A device is set as ready if the MAC address has been set. */
 	volatile uint8_t ready;
 	/**< Device is marked for removal from the data core. */
@@ -94,6 +92,7 @@ struct virtio_net_data_ll
 {
 	struct vhost_dev		*vdev;	/* Pointer to device created by configuration core. */
 	struct virtio_net_data_ll	*next;  /* Pointer to next device in linked list. */
+	uint32_t work_qp_idx;
 };
 
 /*
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v6 12/13] examples/vhost: add per queue stats
  2015-10-09  5:45 [PATCH v6 00/13] vhost-user multiple queues enabling Yuanhan Liu
                   ` (10 preceding siblings ...)
  2015-10-09  5:46 ` [PATCH v6 11/13] examples/vhost: demonstrate the usage of vhost mq feature Yuanhan Liu
@ 2015-10-09  5:46 ` Yuanhan Liu
  2015-10-09  5:46 ` [PATCH v6 13/13] doc: update release note for vhost-user mq support Yuanhan Liu
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-09  5:46 UTC (permalink / raw)
  To: dev; +Cc: Michael S. Tsirkin, marcel

From: Changchun Ouyang <changchun.ouyang@intel.com>

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 examples/vhost/main.c | 97 +++++++++++++++++++++++++++++----------------------
 1 file changed, 56 insertions(+), 41 deletions(-)

diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 23b7aa7..06a3ac7 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -314,7 +314,7 @@ struct ipv4_hdr {
 #define VLAN_ETH_HLEN   18
 
 /* Per-device statistics struct */
-struct device_statistics {
+struct qp_statistics {
 	uint64_t tx_total;
 	rte_atomic64_t rx_total_atomic;
 	uint64_t rx_total;
@@ -322,6 +322,10 @@ struct device_statistics {
 	rte_atomic64_t rx_atomic;
 	uint64_t rx;
 } __rte_cache_aligned;
+
+struct device_statistics {
+	struct qp_statistics *qp_stats;
+};
 struct device_statistics dev_statistics[MAX_DEVICES];
 
 /*
@@ -775,6 +779,17 @@ us_vhost_parse_args(int argc, char **argv)
 					return -1;
 				} else {
 					enable_stats = ret;
+					if (enable_stats)
+						for (i = 0; i < MAX_DEVICES; i++) {
+							dev_statistics[i].qp_stats =
+								malloc(VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX * sizeof(struct qp_statistics));
+							if (dev_statistics[i].qp_stats == NULL) {
+								RTE_LOG(ERR, VHOST_CONFIG, "Failed to allocate memory for qp stats.\n");
+								return -1;
+							}
+							memset(dev_statistics[i].qp_stats, 0,
+								VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX * sizeof(struct qp_statistics));
+						}
 				}
 			}
 
@@ -1131,13 +1146,13 @@ virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m, uint32_t qp_idx)
 					&m, 1);
 				if (enable_stats) {
 					rte_atomic64_add(
-					&dev_statistics[tdev->device_fh].rx_total_atomic,
+					&dev_statistics[tdev->device_fh].qp_stats[qp_idx].rx_total_atomic,
 					1);
 					rte_atomic64_add(
-					&dev_statistics[tdev->device_fh].rx_atomic,
+					&dev_statistics[tdev->device_fh].qp_stats[qp_idx].rx_atomic,
 					ret);
-					dev_statistics[tdev->device_fh].tx_total++;
-					dev_statistics[tdev->device_fh].tx += ret;
+					dev_statistics[dev->device_fh].qp_stats[qp_idx].tx_total++;
+					dev_statistics[dev->device_fh].qp_stats[qp_idx].tx += ret;
 				}
 			}
 
@@ -1271,8 +1286,8 @@ virtio_tx_route(struct vhost_dev *vdev, struct rte_mbuf *m,
 	tx_q->m_table[len] = m;
 	len++;
 	if (enable_stats) {
-		dev_statistics[dev->device_fh].tx_total++;
-		dev_statistics[dev->device_fh].tx++;
+		dev_statistics[dev->device_fh].qp_stats[qp_idx].tx_total++;
+		dev_statistics[dev->device_fh].qp_stats[qp_idx].tx++;
 	}
 
 	if (unlikely(len == MAX_PKT_BURST)) {
@@ -1403,10 +1418,10 @@ switch_worker(__attribute__((unused)) void *arg)
 										pkts_burst, rx_count);
 					if (enable_stats) {
 						rte_atomic64_add(
-						&dev_statistics[dev_ll->vdev->dev->device_fh].rx_total_atomic,
+						&dev_statistics[dev_ll->vdev->dev->device_fh].qp_stats[qp_idx].rx_total_atomic,
 						rx_count);
 						rte_atomic64_add(
-						&dev_statistics[dev_ll->vdev->dev->device_fh].rx_atomic, ret_count);
+						&dev_statistics[dev_ll->vdev->dev->device_fh].qp_stats[qp_idx].rx_atomic, ret_count);
 					}
 					while (likely(rx_count)) {
 						rx_count--;
@@ -1954,8 +1969,8 @@ virtio_tx_route_zcp(struct virtio_net *dev, struct rte_mbuf *m,
 		(mbuf->next == NULL) ? "null" : "non-null");
 
 	if (enable_stats) {
-		dev_statistics[dev->device_fh].tx_total++;
-		dev_statistics[dev->device_fh].tx++;
+		dev_statistics[dev->device_fh].qp_stats[0].tx_total++;
+		dev_statistics[dev->device_fh].qp_stats[0].tx++;
 	}
 
 	if (unlikely(len == MAX_PKT_BURST)) {
@@ -2238,9 +2253,9 @@ switch_worker_zcp(__attribute__((unused)) void *arg)
 					ret_count = virtio_dev_rx_zcp(dev,
 							pkts_burst, rx_count);
 					if (enable_stats) {
-						dev_statistics[dev->device_fh].rx_total
+						dev_statistics[dev->device_fh].qp_stats[0].rx_total
 							+= rx_count;
-						dev_statistics[dev->device_fh].rx
+						dev_statistics[dev->device_fh].qp_stats[0].rx
 							+= ret_count;
 					}
 					while (likely(rx_count)) {
@@ -2883,7 +2898,9 @@ new_device (struct virtio_net *dev)
 	}
 
 	/* Initialize device stats */
-	memset(&dev_statistics[dev->device_fh], 0, sizeof(struct device_statistics));
+	if (enable_stats)
+		memset(dev_statistics[dev->device_fh].qp_stats, 0,
+			VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX * sizeof(struct qp_statistics));
 
 	dev->flags |= VIRTIO_DEV_RUNNING;
 
@@ -2911,7 +2928,7 @@ print_stats(void)
 	struct virtio_net_data_ll *dev_ll;
 	uint64_t tx_dropped, rx_dropped;
 	uint64_t tx, tx_total, rx, rx_total;
-	uint32_t device_fh;
+	uint32_t device_fh, i;
 	const char clr[] = { 27, '[', '2', 'J', '\0' };
 	const char top_left[] = { 27, '[', '1', ';', '1', 'H','\0' };
 
@@ -2926,36 +2943,37 @@ print_stats(void)
 		dev_ll = ll_root_used;
 		while (dev_ll != NULL) {
 			device_fh = (uint32_t)dev_ll->vdev->dev->device_fh;
-			tx_total = dev_statistics[device_fh].tx_total;
-			tx = dev_statistics[device_fh].tx;
-			tx_dropped = tx_total - tx;
-			if (zero_copy == 0) {
-				rx_total = rte_atomic64_read(
-					&dev_statistics[device_fh].rx_total_atomic);
-				rx = rte_atomic64_read(
-					&dev_statistics[device_fh].rx_atomic);
-			} else {
-				rx_total = dev_statistics[device_fh].rx_total;
-				rx = dev_statistics[device_fh].rx;
-			}
-			rx_dropped = rx_total - rx;
-
-			printf("\nStatistics for device %"PRIu32" ------------------------------"
-					"\nTX total: 		%"PRIu64""
-					"\nTX dropped: 		%"PRIu64""
-					"\nTX successful: 		%"PRIu64""
-					"\nRX total: 		%"PRIu64""
-					"\nRX dropped: 		%"PRIu64""
-					"\nRX successful: 		%"PRIu64"",
+			for (i = 0; i < rxq; i++) {
+				tx_total = dev_statistics[device_fh].qp_stats[i].tx_total;
+				tx = dev_statistics[device_fh].qp_stats[i].tx;
+				tx_dropped = tx_total - tx;
+				if (zero_copy == 0) {
+					rx_total = rte_atomic64_read(
+						&dev_statistics[device_fh].qp_stats[i].rx_total_atomic);
+					rx = rte_atomic64_read(
+						&dev_statistics[device_fh].qp_stats[i].rx_atomic);
+				} else {
+					rx_total = dev_statistics[device_fh].qp_stats[0].rx_total;
+					rx = dev_statistics[device_fh].qp_stats[0].rx;
+				}
+				rx_dropped = rx_total - rx;
+
+				printf("\nStatistics for device %"PRIu32" queue id: %d------------------"
+					"\nTX total:		%"PRIu64""
+					"\nTX dropped:		%"PRIu64""
+					"\nTX success:		%"PRIu64""
+					"\nRX total:		%"PRIu64""
+					"\nRX dropped:		%"PRIu64""
+					"\nRX success:		%"PRIu64"",
 					device_fh,
+					i,
 					tx_total,
 					tx_dropped,
 					tx,
 					rx_total,
 					rx_dropped,
 					rx);
-
-
+			}
 			dev_ll = dev_ll->next;
 		}
 		printf("\n======================================================\n");
@@ -3137,9 +3155,6 @@ main(int argc, char *argv[])
 	if (init_data_ll() == -1)
 		rte_exit(EXIT_FAILURE, "Failed to initialize linked list\n");
 
-	/* Initialize device stats */
-	memset(&dev_statistics, 0, sizeof(dev_statistics));
-
 	/* Enable stats if the user option is set. */
 	if (enable_stats)
 		pthread_create(&tid, NULL, (void*)print_stats, NULL );
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v6 13/13] doc: update release note for vhost-user mq support
  2015-10-09  5:45 [PATCH v6 00/13] vhost-user multiple queues enabling Yuanhan Liu
                   ` (11 preceding siblings ...)
  2015-10-09  5:46 ` [PATCH v6 12/13] examples/vhost: add per queue stats Yuanhan Liu
@ 2015-10-09  5:46 ` Yuanhan Liu
  2015-10-09 13:45 ` [PATCH v6 00/13] vhost-user multiple queues enabling Marcel Apfelbaum
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-09  5:46 UTC (permalink / raw)
  To: dev; +Cc: marcel, Michael S. Tsirkin

Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 doc/guides/rel_notes/release_2_2.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/guides/rel_notes/release_2_2.rst b/doc/guides/rel_notes/release_2_2.rst
index 5687676..34c910f 100644
--- a/doc/guides/rel_notes/release_2_2.rst
+++ b/doc/guides/rel_notes/release_2_2.rst
@@ -4,6 +4,11 @@ DPDK Release 2.2
 New Features
 ------------
 
+* **vhost: added vhost-user mulitple queue support.**
+
+  Added vhost-user multiple queue support, and it is demonstrated at
+  ``examples/vhost/vhost-switch``.
+
 
 Resolved Issues
 ---------------
-- 
1.9.0

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 00/13] vhost-user multiple queues enabling
  2015-10-09  5:45 [PATCH v6 00/13] vhost-user multiple queues enabling Yuanhan Liu
                   ` (12 preceding siblings ...)
  2015-10-09  5:46 ` [PATCH v6 13/13] doc: update release note for vhost-user mq support Yuanhan Liu
@ 2015-10-09 13:45 ` Marcel Apfelbaum
  2015-10-19 13:16 ` Flavio Leitner
  2015-10-20 10:39 ` Marcel Apfelbaum
  15 siblings, 0 replies; 37+ messages in thread
From: Marcel Apfelbaum @ 2015-10-09 13:45 UTC (permalink / raw)
  To: Yuanhan Liu, dev; +Cc: Michael S. Tsirkin

On 10/09/2015 08:45 AM, Yuanhan Liu wrote:
> This patch set enables vhost-user multiple queues.
>
> Overview
> ========
>
> It depends on some QEMU patches that has already been merged to upstream.
> Those qemu patches introduce some new vhost-user messages, for vhost-user
> mq enabling negotiation. Here is the main negotiation steps (Qemu
> as master, and DPDK vhost-user as slave):
>
> - Master queries features by VHOST_USER_GET_FEATURES from slave
>
> - Check if VHOST_USER_F_PROTOCOL_FEATURES exist. If not, mq is not
>    supported. (check patch 1 for why VHOST_USER_F_PROTOCOL_FEATURES
>    is introduced)
>
> - Master then sends another command, VHOST_USER_GET_QUEUE_NUM, for
>    querying how many queues the slave supports.
>
>    Master will compare the result with the requested queue number.
>    Qemu exits if the former is smaller.
>
> - Master then tries to initiate all queue pairs by sending some vhost
>    user commands, including VHOST_USER_SET_VRING_CALL, which will
>    trigger the slave to do related vring setup, such as vring allocation.
>
>
> Till now, all necessary initiation and negotiation are done. And master
> could send another message, VHOST_USER_SET_VRING_ENABLE, to enable/disable
> a specific queue dynamically later.
>
>
> Patchset
> ========
>
> Patch 1-6 are all prepare works for enabling mq; they are all atomic
> changes, with "do not breaking anything" beared in mind while making
> them.
>
> Patch 7 acutally enables mq feature, by setting two key feature flags.
>
> Patch 8 handles VHOST_USER_SET_VRING_ENABLE message, which is for enabling
> disabling a specific virt queue pair, and there is only one queue pair is
> enabled by default.
>
> Patch 9-12 is for demostrating the mq feature.
>
> Patch 13 udpates the doc release note.
>
>
> Testing
> =======
>
> Host side
> ----------
>
> - # Start vhost-switch
>
>    sudo mount -t hugetlbfs nodev /mnt/huge
>    sudo modprobe uio
>    sudo insmod $RTE_SDK/$RTE_TARGET/kmod/igb_uio.ko
>
>    sudo $RTE_SDK/tools/dpdk_nic_bind.py --bind igb_uio 0000:08:00.0
>
>    sudo $RTE_SDK/examples/vhost/build/vhost-switch -c 0xf0 -n 4     \
>         --huge-dir /mnt/huge --socket-mem 2048,0 -- -p 1 --vm2vm 0  \
>         --dev-basename usvhost --rxq 2
>
>    # Above common generates a usvhost socket file at PWD. You could also
>    # specify "--stats 1" option to enable stats dumping.
>
>
>
> - # start qemu
>
>
>    sudo sudo mount -t hugetlbfs nodev $HOME/hugetlbfs
>    $QEMU_DIR/x86_64-softmmu/qemu-system-x86_64 -machine accel=kvm -m 4G \
>          -object memory-backend-file,id=mem,size=4G,mem-path=$HOME/hugetlbfs,share=on \
> 	-numa node,memdev=mem -chardev socket,id=chr0,path=/path/to/usvhost \
> 	-netdev vhost-user,id=net0,chardev=chr0,vhostforce,queues=2     \
> 	-device virtio-net-pci,netdev=net0,mq=on,vectors=6,mac=52:54:00:12:34:58,csum=off,gso=off,guest_tso4=off,guest_tso6=off,guest_ecn=off \
> 	-hda $HOME/iso/fc-22-x86_64.img -smp 10 -cpu core2duo,+sse3,+sse4.1,+sse4.2
>
>
> Guest side
> ----------
>
>     modprobe uio
>     insmod $RTE_SDK/$RTE_TARGET/kmod/igb_uio.ko
>     echo 1024 > /sys/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages
>     ./tools/dpdk_nic_bind.py --bind igb_uio 00:03.0
>
>     $RTE_SDK/$RTE_TARGET/app/testpmd -c 1f -n 4 -- --rxq=2 --txq=2 \
>          --nb-cores=4 -i --disable-hw-vlan --txqflags 0xf00
>
>     > set fwd mac
>     > start tx_first
>
>
> After those setups, you then could use packet generator for packet tx/rx testing.
>
>
>
> Test with OVS
> =============
>
> Marcel also created a simple yet quite clear test guide with OVS at:
>
>     http://wiki.qemu.org/Features/vhost-user-ovs-dpdk
>
> BTW, Marcel, would you please complete the page on mq testing?

Hi,

No problem, I'll be happy to add it. I was waiting for this submission
and hoping for an OSV re-submission of the counter-patch.

I need at least a public link to all working series, otherwise
the WIKI would not be as simple and people will not use it :)

Thank you for the fantastic work!
Marcel

>
>
> ---
> Changchun Ouyang (7):
>    vhost: rxtx: prepare work for multiple queue support
>    virtio: read virtio_net_config correctly
>    vhost: add VHOST_USER_SET_VRING_ENABLE message
>    vhost: add API bind a virtq to a specific core
>    ixgbe: support VMDq RSS in non-SRIOV environment
>    examples/vhost: demonstrate the usage of vhost mq feature
>    examples/vhost: add per queue stats
>
> Yuanhan Liu (6):
>    vhost-user: add protocol features support
>    vhost-user: add VHOST_USER_GET_QUEUE_NUM message
>    vhost: vring queue setup for multiple queue support
>    vhost-user: handle VHOST_USER_RESET_OWNER correctly
>    vhost-user: enable vhost-user multiple queue
>    doc: update release note for vhost-user mq support
>
>   doc/guides/rel_notes/release_2_2.rst          |   5 +
>   drivers/net/ixgbe/ixgbe_rxtx.c                |  86 +++++-
>   drivers/net/virtio/virtio_ethdev.c            |  16 +-
>   examples/vhost/main.c                         | 420 +++++++++++++++++---------
>   examples/vhost/main.h                         |   3 +-
>   lib/librte_ether/rte_ethdev.c                 |  11 +
>   lib/librte_vhost/rte_vhost_version.map        |   7 +
>   lib/librte_vhost/rte_virtio_net.h             |  38 ++-
>   lib/librte_vhost/vhost_rxtx.c                 |  56 +++-
>   lib/librte_vhost/vhost_user/vhost-net-user.c  |  27 +-
>   lib/librte_vhost/vhost_user/vhost-net-user.h  |   4 +
>   lib/librte_vhost/vhost_user/virtio-net-user.c |  83 +++--
>   lib/librte_vhost/vhost_user/virtio-net-user.h |  10 +
>   lib/librte_vhost/virtio-net.c                 | 181 +++++++----
>   14 files changed, 692 insertions(+), 255 deletions(-)
>

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 00/13] vhost-user multiple queues enabling
  2015-10-09  5:45 [PATCH v6 00/13] vhost-user multiple queues enabling Yuanhan Liu
                   ` (13 preceding siblings ...)
  2015-10-09 13:45 ` [PATCH v6 00/13] vhost-user multiple queues enabling Marcel Apfelbaum
@ 2015-10-19 13:16 ` Flavio Leitner
  2015-10-20 10:39 ` Marcel Apfelbaum
  15 siblings, 0 replies; 37+ messages in thread
From: Flavio Leitner @ 2015-10-19 13:16 UTC (permalink / raw)
  To: Yuanhan Liu; +Cc: dev, marcel, Michael S. Tsirkin

On Fri, Oct 09, 2015 at 01:45:59PM +0800, Yuanhan Liu wrote:
> This patch set enables vhost-user multiple queues.
> 
> Overview
> ========
> 
> It depends on some QEMU patches that has already been merged to upstream.
> Those qemu patches introduce some new vhost-user messages, for vhost-user
> mq enabling negotiation. Here is the main negotiation steps (Qemu
> as master, and DPDK vhost-user as slave):
> 
> - Master queries features by VHOST_USER_GET_FEATURES from slave
> 
> - Check if VHOST_USER_F_PROTOCOL_FEATURES exist. If not, mq is not
>   supported. (check patch 1 for why VHOST_USER_F_PROTOCOL_FEATURES
>   is introduced)
> 
> - Master then sends another command, VHOST_USER_GET_QUEUE_NUM, for
>   querying how many queues the slave supports.
> 
>   Master will compare the result with the requested queue number.
>   Qemu exits if the former is smaller.
> 
> - Master then tries to initiate all queue pairs by sending some vhost
>   user commands, including VHOST_USER_SET_VRING_CALL, which will
>   trigger the slave to do related vring setup, such as vring allocation.
> 
> 
> Till now, all necessary initiation and negotiation are done. And master
> could send another message, VHOST_USER_SET_VRING_ENABLE, to enable/disable
> a specific queue dynamically later.
> 
> 
> Patchset
> ========
> 
> Patch 1-6 are all prepare works for enabling mq; they are all atomic
> changes, with "do not breaking anything" beared in mind while making
> them.
> 
> Patch 7 acutally enables mq feature, by setting two key feature flags.
> 
> Patch 8 handles VHOST_USER_SET_VRING_ENABLE message, which is for enabling
> disabling a specific virt queue pair, and there is only one queue pair is
> enabled by default.
> 
> Patch 9-12 is for demostrating the mq feature.
> 
> Patch 13 udpates the doc release note.
> 
> 
> Testing
> =======
> 
> Host side
> ----------
> 
> - # Start vhost-switch
> 
>   sudo mount -t hugetlbfs nodev /mnt/huge
>   sudo modprobe uio
>   sudo insmod $RTE_SDK/$RTE_TARGET/kmod/igb_uio.ko
> 
>   sudo $RTE_SDK/tools/dpdk_nic_bind.py --bind igb_uio 0000:08:00.0
> 
>   sudo $RTE_SDK/examples/vhost/build/vhost-switch -c 0xf0 -n 4     \
>        --huge-dir /mnt/huge --socket-mem 2048,0 -- -p 1 --vm2vm 0  \
>        --dev-basename usvhost --rxq 2
> 
>   # Above common generates a usvhost socket file at PWD. You could also
>   # specify "--stats 1" option to enable stats dumping.
> 
> 
> 
> - # start qemu
> 
> 
>   sudo sudo mount -t hugetlbfs nodev $HOME/hugetlbfs
>   $QEMU_DIR/x86_64-softmmu/qemu-system-x86_64 -machine accel=kvm -m 4G \
>         -object memory-backend-file,id=mem,size=4G,mem-path=$HOME/hugetlbfs,share=on \
> 	-numa node,memdev=mem -chardev socket,id=chr0,path=/path/to/usvhost \
> 	-netdev vhost-user,id=net0,chardev=chr0,vhostforce,queues=2     \
> 	-device virtio-net-pci,netdev=net0,mq=on,vectors=6,mac=52:54:00:12:34:58,csum=off,gso=off,guest_tso4=off,guest_tso6=off,guest_ecn=off \
> 	-hda $HOME/iso/fc-22-x86_64.img -smp 10 -cpu core2duo,+sse3,+sse4.1,+sse4.2
> 
> 
> Guest side
> ----------
> 
>    modprobe uio
>    insmod $RTE_SDK/$RTE_TARGET/kmod/igb_uio.ko
>    echo 1024 > /sys/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages
>    ./tools/dpdk_nic_bind.py --bind igb_uio 00:03.0
>     
>    $RTE_SDK/$RTE_TARGET/app/testpmd -c 1f -n 4 -- --rxq=2 --txq=2 \
>         --nb-cores=4 -i --disable-hw-vlan --txqflags 0xf00
> 
>    > set fwd mac
>    > start tx_first
>  
> 
> After those setups, you then could use packet generator for packet tx/rx testing.
> 
> 
> 
> Test with OVS
> =============
> 
> Marcel also created a simple yet quite clear test guide with OVS at:
> 
>    http://wiki.qemu.org/Features/vhost-user-ovs-dpdk
> 
> BTW, Marcel, would you please complete the page on mq testing?
> 
> 
> ---

I reviewed the patch and tested with success using Open vSwitch.
The Open vSwitch patch is posted here:
http://openvswitch.org/pipermail/dev/2015-October/061413.html

Acked-by: Flavio Leitner <fbl@sysclose.org>

Thanks,
fbl

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 09/13] vhost: add API bind a virtq to a specific core
  2015-10-09  5:46 ` [PATCH v6 09/13] vhost: add API bind a virtq to a specific core Yuanhan Liu
@ 2015-10-20  6:16   ` Xie, Huawei
  2015-10-20  6:25     ` Yuanhan Liu
  0 siblings, 1 reply; 37+ messages in thread
From: Xie, Huawei @ 2015-10-20  6:16 UTC (permalink / raw)
  To: Yuanhan Liu, dev; +Cc: marcel, Changchun Ouyang, Michael S. Tsirkin

On 10/9/2015 1:46 PM, Yuanhan Liu wrote:
> From: Changchun Ouyang <changchun.ouyang@intel.com>
>
> The new API rte_vhost_core_id_set() is to bind a virtq to a specific
> core, while the another API rte_vhost_core_id_get() is for getting
> the bind core for a virtq.
Where is the absolute necessity to put switching/application logic into
vhost virt queue structure?
Besides, suggest seperate vhost example patchset and vhost library
patchset so that if we have issue with one, it doesn't stop another
being merged.
>
> The usage, which will be introduced soon, could be find at examles/vhost/main.c.
>
> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> ---


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 09/13] vhost: add API bind a virtq to a specific core
  2015-10-20  6:16   ` Xie, Huawei
@ 2015-10-20  6:25     ` Yuanhan Liu
  2015-10-20  7:17       ` Thomas Monjalon
  0 siblings, 1 reply; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-20  6:25 UTC (permalink / raw)
  To: Xie, Huawei; +Cc: dev, marcel, Michael S. Tsirkin

On Tue, Oct 20, 2015 at 06:16:23AM +0000, Xie, Huawei wrote:
> On 10/9/2015 1:46 PM, Yuanhan Liu wrote:
> > From: Changchun Ouyang <changchun.ouyang@intel.com>
> >
> > The new API rte_vhost_core_id_set() is to bind a virtq to a specific
> > core, while the another API rte_vhost_core_id_get() is for getting
> > the bind core for a virtq.
> Where is the absolute necessity to put switching/application logic into
> vhost virt queue structure?

I don't think it's a must, and we can drop it.

> Besides, suggest seperate vhost example patchset and vhost library
> patchset so that if we have issue with one, it doesn't stop another
> being merged.

That's what I've done: patch 1-8 are for enabling vhost-user mq, while
other patches are for vhost-switch example.  Maybe I should send out 2
patch sets, one for each.

But, anyway, please help focus on reviewing path 1-8.

	--yliu
> >
> > The usage, which will be introduced soon, could be find at examles/vhost/main.c.
> >
> > Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> > ---
> 

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 03/13] vhost: vring queue setup for multiple queue support
  2015-10-09  5:46 ` [PATCH v6 03/13] vhost: vring queue setup for multiple queue support Yuanhan Liu
@ 2015-10-20  6:49   ` Thomas Monjalon
  2015-10-20  6:56     ` Yuanhan Liu
  0 siblings, 1 reply; 37+ messages in thread
From: Thomas Monjalon @ 2015-10-20  6:49 UTC (permalink / raw)
  To: Yuanhan Liu; +Cc: dev

2015-10-09 13:46, Yuanhan Liu:
> All queue pairs, including the default (the first) queue pair,
> are allocated dynamically, when a vring_call message is received
> first time for a specific queue pair.
> 
> This is a refactor work for enabling vhost-user multiple queue;
> it should not break anything as it does no functional changes:
> we don't support mq set, so there is only one mq at max.
> 
> This patch is based on Changchun's patch.
> 
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

Maybe you should keep Changchun's sign-off.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 03/13] vhost: vring queue setup for multiple queue support
  2015-10-20  6:49   ` Thomas Monjalon
@ 2015-10-20  6:56     ` Yuanhan Liu
  0 siblings, 0 replies; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-20  6:56 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On Tue, Oct 20, 2015 at 08:49:38AM +0200, Thomas Monjalon wrote:
> 2015-10-09 13:46, Yuanhan Liu:
> > All queue pairs, including the default (the first) queue pair,
> > are allocated dynamically, when a vring_call message is received
> > first time for a specific queue pair.
> > 
> > This is a refactor work for enabling vhost-user multiple queue;
> > it should not break anything as it does no functional changes:
> > we don't support mq set, so there is only one mq at max.
> > 
> > This patch is based on Changchun's patch.
> > 
> > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> 
> Maybe you should keep Changchun's sign-off.

Yeah, I should've done that.

	--yliu

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 04/13] vhost: rxtx: prepare work for multiple queue support
  2015-10-09  5:46 ` [PATCH v6 04/13] vhost: rxtx: prepare work " Yuanhan Liu
@ 2015-10-20  6:57   ` Thomas Monjalon
  2015-10-20  7:10     ` Yuanhan Liu
  0 siblings, 1 reply; 37+ messages in thread
From: Thomas Monjalon @ 2015-10-20  6:57 UTC (permalink / raw)
  To: Yuanhan Liu; +Cc: dev, marcel, Michael S. Tsirkin

2015-10-09 13:46, Yuanhan Liu:
> From: Changchun Ouyang <changchun.ouyang@intel.com>
> 
> Do not use VIRTIO_RXQ or VIRTIO_TXQ anymore; use the queue_id,
> instead, which will be set to a proper value for a specific queue
> when we have multiple queue support enabled.
> 
> For now, queue_id is still set with VIRTIO_RXQ or VIRTIO_TXQ,
> so it should not break anything.

The title may be more informative.
Example:
	vhost: use queue id instead of constant ring index

> -	if (unlikely(queue_id != VIRTIO_RXQ)) {
> -		LOG_DEBUG(VHOST_DATA, "mq isn't supported in this version.\n");
> +	if (unlikely(!is_valid_virt_queue_idx(queue_id, 0, dev->virt_qp_nb))) {
> +		RTE_LOG(ERR, VHOST_DATA,
> +			"%s (%"PRIu64"): virtqueue idx:%d invalid.\n",
> +			__func__, dev->device_fh, queue_id);
[...]
> -	if (unlikely(queue_id != VIRTIO_RXQ)) {
> -		LOG_DEBUG(VHOST_DATA, "mq isn't supported in this version.\n");
> +	if (unlikely(!is_valid_virt_queue_idx(queue_id, 0, dev->virt_qp_nb))) {
> +		RTE_LOG(ERR, VHOST_DATA,
> +			"%s (%"PRIu64"): virtqueue idx:%d invalid.\n",
> +			__func__, dev->device_fh, queue_id);
> +		return 0;
[...]
> -	if (unlikely(queue_id != VIRTIO_TXQ)) {
> -		LOG_DEBUG(VHOST_DATA, "mq isn't supported in this version.\n");
> +	if (unlikely(!is_valid_virt_queue_idx(queue_id, 1, dev->virt_qp_nb))) {
> +		RTE_LOG(ERR, VHOST_DATA,
> +			"%s (%"PRIu64"): virtqueue idx:%d invalid.\n",
> +			__func__, dev->device_fh, queue_id);

Why not use the log macros in drivers/net/virtio/virtio_logs.h?
If you don't like the current log format, it would deserve a separate patch
to change them consistently.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 05/13] vhost-user: handle VHOST_USER_RESET_OWNER correctly
  2015-10-09  5:46 ` [PATCH v6 05/13] vhost-user: handle VHOST_USER_RESET_OWNER correctly Yuanhan Liu
@ 2015-10-20  7:03   ` Thomas Monjalon
  2015-10-20  7:14     ` Yuanhan Liu
  0 siblings, 1 reply; 37+ messages in thread
From: Thomas Monjalon @ 2015-10-20  7:03 UTC (permalink / raw)
  To: Yuanhan Liu; +Cc: dev, marcel, Michael S. Tsirkin

2015-10-09 13:46, Yuanhan Liu:
> Destroy corresponding device when a VHOST_USER_RESET_OWNER message is
> received, otherwise, the vhost-switch would still try to access vq
> of that device, which results to SIGSEG fault, and let vhost-switch
> crash in the end.

It is a fix, so the title should look like:
	vhost: fix crash when receiving VHOST_USER_RESET_OWNER
and there should be a "Fixes:" tag.

Please could you also review the related patches from Jerome Jutteau?
	http://dpdk.org/dev/patchwork/project/dpdk/list/?submitter=354

Thanks

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 06/13] virtio: read virtio_net_config correctly
  2015-10-09  5:46 ` [PATCH v6 06/13] virtio: read virtio_net_config correctly Yuanhan Liu
@ 2015-10-20  7:07   ` Thomas Monjalon
  2015-10-20  7:23     ` Yuanhan Liu
  0 siblings, 1 reply; 37+ messages in thread
From: Thomas Monjalon @ 2015-10-20  7:07 UTC (permalink / raw)
  To: Yuanhan Liu; +Cc: dev, marcel, Michael S. Tsirkin

2015-10-09 13:46, Yuanhan Liu:
> From: Changchun Ouyang <changchun.ouyang@intel.com>
> 
> The old code adjusts the config bytes we want to read depending on
> what kind of features we have, but we later cast the entire buf we
> read with "struct virtio_net_config", which is obviously wrong.

When describing a bug, it is important to explain what is the consequence,
i.e. which use case is failing. If it is only to prepare the new feature,
it is better to clearly state that the bug had no impact until now.

And as usual, the "fix" word in the title and the "Fixes" tag are required.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 04/13] vhost: rxtx: prepare work for multiple queue support
  2015-10-20  6:57   ` Thomas Monjalon
@ 2015-10-20  7:10     ` Yuanhan Liu
  2015-10-20  7:24       ` Thomas Monjalon
  0 siblings, 1 reply; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-20  7:10 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, marcel, Michael S. Tsirkin

On Tue, Oct 20, 2015 at 08:57:52AM +0200, Thomas Monjalon wrote:
> 2015-10-09 13:46, Yuanhan Liu:
> > From: Changchun Ouyang <changchun.ouyang@intel.com>
> > 
> > Do not use VIRTIO_RXQ or VIRTIO_TXQ anymore; use the queue_id,
> > instead, which will be set to a proper value for a specific queue
> > when we have multiple queue support enabled.
> > 
> > For now, queue_id is still set with VIRTIO_RXQ or VIRTIO_TXQ,
> > so it should not break anything.
> 
> The title may be more informative.
> Example:
> 	vhost: use queue id instead of constant ring index

Thanks, that's indeed better.

> 
> > -	if (unlikely(queue_id != VIRTIO_RXQ)) {
> > -		LOG_DEBUG(VHOST_DATA, "mq isn't supported in this version.\n");
> > +	if (unlikely(!is_valid_virt_queue_idx(queue_id, 0, dev->virt_qp_nb))) {
> > +		RTE_LOG(ERR, VHOST_DATA,
> > +			"%s (%"PRIu64"): virtqueue idx:%d invalid.\n",
> > +			__func__, dev->device_fh, queue_id);
> [...]
> > -	if (unlikely(queue_id != VIRTIO_RXQ)) {
> > -		LOG_DEBUG(VHOST_DATA, "mq isn't supported in this version.\n");
> > +	if (unlikely(!is_valid_virt_queue_idx(queue_id, 0, dev->virt_qp_nb))) {
> > +		RTE_LOG(ERR, VHOST_DATA,
> > +			"%s (%"PRIu64"): virtqueue idx:%d invalid.\n",
> > +			__func__, dev->device_fh, queue_id);
> > +		return 0;
> [...]
> > -	if (unlikely(queue_id != VIRTIO_TXQ)) {
> > -		LOG_DEBUG(VHOST_DATA, "mq isn't supported in this version.\n");
> > +	if (unlikely(!is_valid_virt_queue_idx(queue_id, 1, dev->virt_qp_nb))) {
> > +		RTE_LOG(ERR, VHOST_DATA,
> > +			"%s (%"PRIu64"): virtqueue idx:%d invalid.\n",
> > +			__func__, dev->device_fh, queue_id);
> 
> Why not use the log macros in drivers/net/virtio/virtio_logs.h?

Do you mean I should keep using LOG_DEBUG instead?

> If you don't like the current log format, it would deserve a separate patch
> to change them consistently.

Changchun made the change, and I don't know why.

	--yliu

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 05/13] vhost-user: handle VHOST_USER_RESET_OWNER correctly
  2015-10-20  7:03   ` Thomas Monjalon
@ 2015-10-20  7:14     ` Yuanhan Liu
  0 siblings, 0 replies; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-20  7:14 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, marcel, Michael S. Tsirkin

On Tue, Oct 20, 2015 at 09:03:48AM +0200, Thomas Monjalon wrote:
> 2015-10-09 13:46, Yuanhan Liu:
> > Destroy corresponding device when a VHOST_USER_RESET_OWNER message is
> > received, otherwise, the vhost-switch would still try to access vq
> > of that device, which results to SIGSEG fault, and let vhost-switch
> > crash in the end.
> 
> It is a fix, so the title should look like:
> 	vhost: fix crash when receiving VHOST_USER_RESET_OWNER
> and there should be a "Fixes:" tag.

Got it.

> 
> Please could you also review the related patches from Jerome Jutteau?
> 	http://dpdk.org/dev/patchwork/project/dpdk/list/?submitter=354


I've already reviewed v1 in the first time, giving him a series ACK.
However, I had a minor comment, which will not affect my ACK, and he
updated his patch to v2.

Maybe I should make another ACK in this v2.

	--yliu

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 09/13] vhost: add API bind a virtq to a specific core
  2015-10-20  6:25     ` Yuanhan Liu
@ 2015-10-20  7:17       ` Thomas Monjalon
  0 siblings, 0 replies; 37+ messages in thread
From: Thomas Monjalon @ 2015-10-20  7:17 UTC (permalink / raw)
  To: Yuanhan Liu; +Cc: dev, marcel, Michael S. Tsirkin

2015-10-20 14:25, Yuanhan Liu:
> On Tue, Oct 20, 2015 at 06:16:23AM +0000, Xie, Huawei wrote:
> > On 10/9/2015 1:46 PM, Yuanhan Liu wrote:
> > > From: Changchun Ouyang <changchun.ouyang@intel.com>
> > >
> > > The new API rte_vhost_core_id_set() is to bind a virtq to a specific
> > > core, while the another API rte_vhost_core_id_get() is for getting
> > > the bind core for a virtq.
> > Where is the absolute necessity to put switching/application logic into
> > vhost virt queue structure?
> 
> I don't think it's a must, and we can drop it.

Yes please, it looks to be a layer violation. Holy rule ;)

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 06/13] virtio: read virtio_net_config correctly
  2015-10-20  7:07   ` Thomas Monjalon
@ 2015-10-20  7:23     ` Yuanhan Liu
  2015-10-20  7:27       ` Thomas Monjalon
  0 siblings, 1 reply; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-20  7:23 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, marcel, Michael S. Tsirkin

On Tue, Oct 20, 2015 at 09:07:49AM +0200, Thomas Monjalon wrote:
> 2015-10-09 13:46, Yuanhan Liu:
> > From: Changchun Ouyang <changchun.ouyang@intel.com>
> > 
> > The old code adjusts the config bytes we want to read depending on
> > what kind of features we have, but we later cast the entire buf we
> > read with "struct virtio_net_config", which is obviously wrong.
> 
> When describing a bug, it is important to explain what is the consequence,
> i.e. which use case is failing. If it is only to prepare the new feature,
> it is better to clearly state that the bug had no impact until now.
> 
> And as usual, the "fix" word in the title and the "Fixes" tag are required.

What's the right way supposed to use "Fixes" tag, BTW? Checking the git
hisotry, I saw something like:

	Fixes: $commit_hash ($commit_log).

Which basically means it's a regression fix. However, in this case, it's
more than like a bug, but not a regression.

	--yliu

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 04/13] vhost: rxtx: prepare work for multiple queue support
  2015-10-20  7:10     ` Yuanhan Liu
@ 2015-10-20  7:24       ` Thomas Monjalon
  0 siblings, 0 replies; 37+ messages in thread
From: Thomas Monjalon @ 2015-10-20  7:24 UTC (permalink / raw)
  To: Yuanhan Liu; +Cc: dev, marcel, Michael S. Tsirkin

2015-10-20 15:10, Yuanhan Liu:
> On Tue, Oct 20, 2015 at 08:57:52AM +0200, Thomas Monjalon wrote:
> > > -	if (unlikely(queue_id != VIRTIO_RXQ)) {
> > > -		LOG_DEBUG(VHOST_DATA, "mq isn't supported in this version.\n");
> > > +	if (unlikely(!is_valid_virt_queue_idx(queue_id, 0, dev->virt_qp_nb))) {
> > > +		RTE_LOG(ERR, VHOST_DATA,
> > > +			"%s (%"PRIu64"): virtqueue idx:%d invalid.\n",
> > > +			__func__, dev->device_fh, queue_id);
> > [...]
> > > -	if (unlikely(queue_id != VIRTIO_RXQ)) {
> > > -		LOG_DEBUG(VHOST_DATA, "mq isn't supported in this version.\n");
> > > +	if (unlikely(!is_valid_virt_queue_idx(queue_id, 0, dev->virt_qp_nb))) {
> > > +		RTE_LOG(ERR, VHOST_DATA,
> > > +			"%s (%"PRIu64"): virtqueue idx:%d invalid.\n",
> > > +			__func__, dev->device_fh, queue_id);
> > > +		return 0;
> > [...]
> > > -	if (unlikely(queue_id != VIRTIO_TXQ)) {
> > > -		LOG_DEBUG(VHOST_DATA, "mq isn't supported in this version.\n");
> > > +	if (unlikely(!is_valid_virt_queue_idx(queue_id, 1, dev->virt_qp_nb))) {
> > > +		RTE_LOG(ERR, VHOST_DATA,
> > > +			"%s (%"PRIu64"): virtqueue idx:%d invalid.\n",
> > > +			__func__, dev->device_fh, queue_id);
> > 
> > Why not use the log macros in drivers/net/virtio/virtio_logs.h?
> 
> Do you mean I should keep using LOG_DEBUG instead?

No you can use the error level.
Actually I was checking the logs in virtio which use some log wrapper to keep
some consistency. But the vhost code doesn't seem to have such policy.
So you are free :)
But I would say you should avoid changing the log format if does not comply
with the rest of the library. The __func__ macro is not really used.
Maybe Huawei could give a general policy for vhost logs (having a general
consistency in all DPDK libs is better).

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 06/13] virtio: read virtio_net_config correctly
  2015-10-20  7:23     ` Yuanhan Liu
@ 2015-10-20  7:27       ` Thomas Monjalon
  2015-10-20  8:00         ` Yuanhan Liu
  0 siblings, 1 reply; 37+ messages in thread
From: Thomas Monjalon @ 2015-10-20  7:27 UTC (permalink / raw)
  To: Yuanhan Liu; +Cc: dev, marcel, Michael S. Tsirkin

2015-10-20 15:23, Yuanhan Liu:
> On Tue, Oct 20, 2015 at 09:07:49AM +0200, Thomas Monjalon wrote:
> > 2015-10-09 13:46, Yuanhan Liu:
> > > From: Changchun Ouyang <changchun.ouyang@intel.com>
> > > 
> > > The old code adjusts the config bytes we want to read depending on
> > > what kind of features we have, but we later cast the entire buf we
> > > read with "struct virtio_net_config", which is obviously wrong.
> > 
> > When describing a bug, it is important to explain what is the consequence,
> > i.e. which use case is failing. If it is only to prepare the new feature,
> > it is better to clearly state that the bug had no impact until now.
> > 
> > And as usual, the "fix" word in the title and the "Fixes" tag are required.
> 
> What's the right way supposed to use "Fixes" tag, BTW? Checking the git
> hisotry, I saw something like:
> 
> 	Fixes: $commit_hash ($commit_log).

In http://dpdk.org/dev, this git alias is recommended:
	fixline = log -1 --abbrev=12 --format='Fixes: %h (\"%s\")'

> Which basically means it's a regression fix. However, in this case, it's
> more than like a bug, but not a regression.

Referencing the original commit (introducing the bug) makes it clear.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 10/13] ixgbe: support VMDq RSS in non-SRIOV environment
  2015-10-09  5:46 ` [PATCH v6 10/13] ixgbe: support VMDq RSS in non-SRIOV environment Yuanhan Liu
@ 2015-10-20  7:46   ` Thomas Monjalon
  2015-10-21 10:05   ` Ananyev, Konstantin
  1 sibling, 0 replies; 37+ messages in thread
From: Thomas Monjalon @ 2015-10-20  7:46 UTC (permalink / raw)
  To: Helin Zhang, Konstantin Ananyev; +Cc: dev

Helin, Konstantin,
could you review please?

2015-10-09 13:46, Yuanhan Liu:
> From: Changchun Ouyang <changchun.ouyang@intel.com>
> 
> In non-SRIOV environment, VMDq RSS could be enabled by MRQC register.
> In theory, the queue number per pool could be 2 or 4, but only 2 queues
> are available due to HW limitation, the same limit also exists in Linux
> ixgbe driver.
> 
> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 06/13] virtio: read virtio_net_config correctly
  2015-10-20  7:27       ` Thomas Monjalon
@ 2015-10-20  8:00         ` Yuanhan Liu
  0 siblings, 0 replies; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-20  8:00 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, marcel, Michael S. Tsirkin

On Tue, Oct 20, 2015 at 09:27:37AM +0200, Thomas Monjalon wrote:
> 2015-10-20 15:23, Yuanhan Liu:
> > On Tue, Oct 20, 2015 at 09:07:49AM +0200, Thomas Monjalon wrote:
> > > 2015-10-09 13:46, Yuanhan Liu:
> > > > From: Changchun Ouyang <changchun.ouyang@intel.com>
> > > > 
> > > > The old code adjusts the config bytes we want to read depending on
> > > > what kind of features we have, but we later cast the entire buf we
> > > > read with "struct virtio_net_config", which is obviously wrong.
> > > 
> > > When describing a bug, it is important to explain what is the consequence,
> > > i.e. which use case is failing. If it is only to prepare the new feature,
> > > it is better to clearly state that the bug had no impact until now.
> > > 
> > > And as usual, the "fix" word in the title and the "Fixes" tag are required.
> > 
> > What's the right way supposed to use "Fixes" tag, BTW? Checking the git
> > hisotry, I saw something like:
> > 
> > 	Fixes: $commit_hash ($commit_log).
> 
> In http://dpdk.org/dev, this git alias is recommended:
> 	fixline = log -1 --abbrev=12 --format='Fixes: %h (\"%s\")'

Thanks for the tip.

> 
> > Which basically means it's a regression fix. However, in this case, it's
> > more than like a bug, but not a regression.
> 
> Referencing the original commit (introducing the bug) makes it clear.

Okay.

	--yliu

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 00/13] vhost-user multiple queues enabling
  2015-10-09  5:45 [PATCH v6 00/13] vhost-user multiple queues enabling Yuanhan Liu
                   ` (14 preceding siblings ...)
  2015-10-19 13:16 ` Flavio Leitner
@ 2015-10-20 10:39 ` Marcel Apfelbaum
  2015-10-21  2:05   ` Yuanhan Liu
  15 siblings, 1 reply; 37+ messages in thread
From: Marcel Apfelbaum @ 2015-10-20 10:39 UTC (permalink / raw)
  To: Yuanhan Liu, dev; +Cc: Michael S. Tsirkin

On 10/09/2015 08:45 AM, Yuanhan Liu wrote:
> This patch set enables vhost-user multiple queues.
>
> Overview
> ========
>
> It depends on some QEMU patches that has already been merged to upstream.
> Those qemu patches introduce some new vhost-user messages, for vhost-user
> mq enabling negotiation. Here is the main negotiation steps (Qemu
> as master, and DPDK vhost-user as slave):
>
> - Master queries features by VHOST_USER_GET_FEATURES from slave
>
> - Check if VHOST_USER_F_PROTOCOL_FEATURES exist. If not, mq is not
>    supported. (check patch 1 for why VHOST_USER_F_PROTOCOL_FEATURES
>    is introduced)
>
> - Master then sends another command, VHOST_USER_GET_QUEUE_NUM, for
>    querying how many queues the slave supports.
>
>    Master will compare the result with the requested queue number.
>    Qemu exits if the former is smaller.
>
> - Master then tries to initiate all queue pairs by sending some vhost
>    user commands, including VHOST_USER_SET_VRING_CALL, which will
>    trigger the slave to do related vring setup, such as vring allocation.
>
>
> Till now, all necessary initiation and negotiation are done. And master
> could send another message, VHOST_USER_SET_VRING_ENABLE, to enable/disable
> a specific queue dynamically later.
>
>
> Patchset
> ========
>
> Patch 1-6 are all prepare works for enabling mq; they are all atomic
> changes, with "do not breaking anything" beared in mind while making
> them.
>
> Patch 7 acutally enables mq feature, by setting two key feature flags.
>
> Patch 8 handles VHOST_USER_SET_VRING_ENABLE message, which is for enabling
> disabling a specific virt queue pair, and there is only one queue pair is
> enabled by default.
>
> Patch 9-12 is for demostrating the mq feature.
>
> Patch 13 udpates the doc release note.
>
>
> Testing
> =======
>
> Host side
> ----------
>
> - # Start vhost-switch
>
>    sudo mount -t hugetlbfs nodev /mnt/huge
>    sudo modprobe uio
>    sudo insmod $RTE_SDK/$RTE_TARGET/kmod/igb_uio.ko
>
>    sudo $RTE_SDK/tools/dpdk_nic_bind.py --bind igb_uio 0000:08:00.0
>
>    sudo $RTE_SDK/examples/vhost/build/vhost-switch -c 0xf0 -n 4     \
>         --huge-dir /mnt/huge --socket-mem 2048,0 -- -p 1 --vm2vm 0  \
>         --dev-basename usvhost --rxq 2
>
>    # Above common generates a usvhost socket file at PWD. You could also
>    # specify "--stats 1" option to enable stats dumping.
>
>
>
> - # start qemu
>
>
>    sudo sudo mount -t hugetlbfs nodev $HOME/hugetlbfs
>    $QEMU_DIR/x86_64-softmmu/qemu-system-x86_64 -machine accel=kvm -m 4G \
>          -object memory-backend-file,id=mem,size=4G,mem-path=$HOME/hugetlbfs,share=on \
> 	-numa node,memdev=mem -chardev socket,id=chr0,path=/path/to/usvhost \
> 	-netdev vhost-user,id=net0,chardev=chr0,vhostforce,queues=2     \
> 	-device virtio-net-pci,netdev=net0,mq=on,vectors=6,mac=52:54:00:12:34:58,csum=off,gso=off,guest_tso4=off,guest_tso6=off,guest_ecn=off \
> 	-hda $HOME/iso/fc-22-x86_64.img -smp 10 -cpu core2duo,+sse3,+sse4.1,+sse4.2
>
>
> Guest side
> ----------
>
>     modprobe uio
>     insmod $RTE_SDK/$RTE_TARGET/kmod/igb_uio.ko
>     echo 1024 > /sys/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages
>     ./tools/dpdk_nic_bind.py --bind igb_uio 00:03.0
>
>     $RTE_SDK/$RTE_TARGET/app/testpmd -c 1f -n 4 -- --rxq=2 --txq=2 \
>          --nb-cores=4 -i --disable-hw-vlan --txqflags 0xf00
>
>     > set fwd mac
>     > start tx_first
>
>
> After those setups, you then could use packet generator for packet tx/rx testing.
>
>
>
> Test with OVS
> =============
>
> Marcel also created a simple yet quite clear test guide with OVS at:
>
>     http://wiki.qemu.org/Features/vhost-user-ovs-dpdk
>
> BTW, Marcel, would you please complete the page on mq testing?

Done.

http://wiki.qemu.org/Features/vhost-user-ovs-dpdk#Enable_multi-queue

Please let me know if I missed anything,
Marcel


>
>
> ---
> Changchun Ouyang (7):
>    vhost: rxtx: prepare work for multiple queue support
>    virtio: read virtio_net_config correctly
>    vhost: add VHOST_USER_SET_VRING_ENABLE message
>    vhost: add API bind a virtq to a specific core
>    ixgbe: support VMDq RSS in non-SRIOV environment
>    examples/vhost: demonstrate the usage of vhost mq feature
>    examples/vhost: add per queue stats
>
> Yuanhan Liu (6):
>    vhost-user: add protocol features support
>    vhost-user: add VHOST_USER_GET_QUEUE_NUM message
>    vhost: vring queue setup for multiple queue support
>    vhost-user: handle VHOST_USER_RESET_OWNER correctly
>    vhost-user: enable vhost-user multiple queue
>    doc: update release note for vhost-user mq support
>
>   doc/guides/rel_notes/release_2_2.rst          |   5 +
>   drivers/net/ixgbe/ixgbe_rxtx.c                |  86 +++++-
>   drivers/net/virtio/virtio_ethdev.c            |  16 +-
>   examples/vhost/main.c                         | 420 +++++++++++++++++---------
>   examples/vhost/main.h                         |   3 +-
>   lib/librte_ether/rte_ethdev.c                 |  11 +
>   lib/librte_vhost/rte_vhost_version.map        |   7 +
>   lib/librte_vhost/rte_virtio_net.h             |  38 ++-
>   lib/librte_vhost/vhost_rxtx.c                 |  56 +++-
>   lib/librte_vhost/vhost_user/vhost-net-user.c  |  27 +-
>   lib/librte_vhost/vhost_user/vhost-net-user.h  |   4 +
>   lib/librte_vhost/vhost_user/virtio-net-user.c |  83 +++--
>   lib/librte_vhost/vhost_user/virtio-net-user.h |  10 +
>   lib/librte_vhost/virtio-net.c                 | 181 +++++++----
>   14 files changed, 692 insertions(+), 255 deletions(-)
>

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 00/13] vhost-user multiple queues enabling
  2015-10-20 10:39 ` Marcel Apfelbaum
@ 2015-10-21  2:05   ` Yuanhan Liu
  0 siblings, 0 replies; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-21  2:05 UTC (permalink / raw)
  To: Marcel Apfelbaum; +Cc: dev, Michael S. Tsirkin


> >Test with OVS
> >=============
> >
> >Marcel also created a simple yet quite clear test guide with OVS at:
> >
> >    http://wiki.qemu.org/Features/vhost-user-ovs-dpdk
> >
> >BTW, Marcel, would you please complete the page on mq testing?
> 
> Done.
> 
> http://wiki.qemu.org/Features/vhost-user-ovs-dpdk#Enable_multi-queue
> 
> Please let me know if I missed anything,

Nope, that's perfect. Thanks for the effort.

	--yliu

> 
> 
> >
> >
> >---
> >Changchun Ouyang (7):
> >   vhost: rxtx: prepare work for multiple queue support
> >   virtio: read virtio_net_config correctly
> >   vhost: add VHOST_USER_SET_VRING_ENABLE message
> >   vhost: add API bind a virtq to a specific core
> >   ixgbe: support VMDq RSS in non-SRIOV environment
> >   examples/vhost: demonstrate the usage of vhost mq feature
> >   examples/vhost: add per queue stats
> >
> >Yuanhan Liu (6):
> >   vhost-user: add protocol features support
> >   vhost-user: add VHOST_USER_GET_QUEUE_NUM message
> >   vhost: vring queue setup for multiple queue support
> >   vhost-user: handle VHOST_USER_RESET_OWNER correctly
> >   vhost-user: enable vhost-user multiple queue
> >   doc: update release note for vhost-user mq support
> >
> >  doc/guides/rel_notes/release_2_2.rst          |   5 +
> >  drivers/net/ixgbe/ixgbe_rxtx.c                |  86 +++++-
> >  drivers/net/virtio/virtio_ethdev.c            |  16 +-
> >  examples/vhost/main.c                         | 420 +++++++++++++++++---------
> >  examples/vhost/main.h                         |   3 +-
> >  lib/librte_ether/rte_ethdev.c                 |  11 +
> >  lib/librte_vhost/rte_vhost_version.map        |   7 +
> >  lib/librte_vhost/rte_virtio_net.h             |  38 ++-
> >  lib/librte_vhost/vhost_rxtx.c                 |  56 +++-
> >  lib/librte_vhost/vhost_user/vhost-net-user.c  |  27 +-
> >  lib/librte_vhost/vhost_user/vhost-net-user.h  |   4 +
> >  lib/librte_vhost/vhost_user/virtio-net-user.c |  83 +++--
> >  lib/librte_vhost/vhost_user/virtio-net-user.h |  10 +
> >  lib/librte_vhost/virtio-net.c                 | 181 +++++++----
> >  14 files changed, 692 insertions(+), 255 deletions(-)
> >

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 10/13] ixgbe: support VMDq RSS in non-SRIOV environment
  2015-10-09  5:46 ` [PATCH v6 10/13] ixgbe: support VMDq RSS in non-SRIOV environment Yuanhan Liu
  2015-10-20  7:46   ` Thomas Monjalon
@ 2015-10-21 10:05   ` Ananyev, Konstantin
  2015-10-21 13:01     ` Yuanhan Liu
  1 sibling, 1 reply; 37+ messages in thread
From: Ananyev, Konstantin @ 2015-10-21 10:05 UTC (permalink / raw)
  To: Yuanhan Liu, dev; +Cc: marcel, Michael S. Tsirkin

Hi 

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Yuanhan Liu
> Sent: Friday, October 09, 2015 6:46 AM
> To: dev@dpdk.org
> Cc: Michael S. Tsirkin; marcel@redhat.com
> Subject: [dpdk-dev] [PATCH v6 10/13] ixgbe: support VMDq RSS in non-SRIOV environment
> 
> From: Changchun Ouyang <changchun.ouyang@intel.com>
> 
> In non-SRIOV environment, VMDq RSS could be enabled by MRQC register.
> In theory, the queue number per pool could be 2 or 4, but only 2 queues
> are available due to HW limitation, the same limit also exists in Linux
> ixgbe driver.
> 
> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> ---
>  drivers/net/ixgbe/ixgbe_rxtx.c | 86 +++++++++++++++++++++++++++++++++++-------
>  lib/librte_ether/rte_ethdev.c  | 11 ++++++
>  2 files changed, 84 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index a598a72..e502fe8 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -3445,16 +3445,16 @@ void ixgbe_configure_dcb(struct rte_eth_dev *dev)
>  	return;
>  }
> 
> -/*
> - * VMDq only support for 10 GbE NIC.
> +/**
> + * Config pool for VMDq on 10 GbE NIC.
>   */
>  static void
> -ixgbe_vmdq_rx_hw_configure(struct rte_eth_dev *dev)
> +ixgbe_vmdq_pool_configure(struct rte_eth_dev *dev)
>  {
>  	struct rte_eth_vmdq_rx_conf *cfg;
>  	struct ixgbe_hw *hw;
>  	enum rte_eth_nb_pools num_pools;
> -	uint32_t mrqc, vt_ctl, vlanctrl;
> +	uint32_t vt_ctl, vlanctrl;
>  	uint32_t vmolr = 0;
>  	int i;
> 
> @@ -3463,12 +3463,6 @@ ixgbe_vmdq_rx_hw_configure(struct rte_eth_dev *dev)
>  	cfg = &dev->data->dev_conf.rx_adv_conf.vmdq_rx_conf;
>  	num_pools = cfg->nb_queue_pools;
> 
> -	ixgbe_rss_disable(dev);
> -
> -	/* MRQC: enable vmdq */
> -	mrqc = IXGBE_MRQC_VMDQEN;
> -	IXGBE_WRITE_REG(hw, IXGBE_MRQC, mrqc);
> -
>  	/* PFVTCTL: turn on virtualisation and set the default pool */
>  	vt_ctl = IXGBE_VT_CTL_VT_ENABLE | IXGBE_VT_CTL_REPLEN;
>  	if (cfg->enable_default_pool)
> @@ -3534,7 +3528,29 @@ ixgbe_vmdq_rx_hw_configure(struct rte_eth_dev *dev)
>  	IXGBE_WRITE_FLUSH(hw);
>  }
> 
> -/*
> +/**
> + * VMDq only support for 10 GbE NIC.
> + */
> +static void
> +ixgbe_vmdq_rx_hw_configure(struct rte_eth_dev *dev)
> +{
> +	struct ixgbe_hw *hw;
> +	uint32_t mrqc;
> +
> +	PMD_INIT_FUNC_TRACE();
> +	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +
> +	ixgbe_rss_disable(dev);
> +
> +	/* MRQC: enable vmdq */
> +	mrqc = IXGBE_MRQC_VMDQEN;
> +	IXGBE_WRITE_REG(hw, IXGBE_MRQC, mrqc);
> +	IXGBE_WRITE_FLUSH(hw);
> +
> +	ixgbe_vmdq_pool_configure(dev);
> +}
> +
> +/**
>   * ixgbe_dcb_config_tx_hw_config - Configure general VMDq TX parameters
>   * @hw: pointer to hardware structure
>   */
> @@ -3639,6 +3655,41 @@ ixgbe_config_vf_rss(struct rte_eth_dev *dev)
>  }
> 
>  static int
> +ixgbe_config_vmdq_rss(struct rte_eth_dev *dev)
> +{
> +	struct ixgbe_hw *hw;
> +	uint32_t mrqc;
> +
> +	ixgbe_rss_configure(dev);
> +
> +	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +
> +	/* MRQC: enable VMDQ RSS */
> +	mrqc = IXGBE_READ_REG(hw, IXGBE_MRQC);
> +	mrqc &= ~IXGBE_MRQC_MRQE_MASK;
> +
> +	switch (RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool) {
> +	case 2:
> +		mrqc |= IXGBE_MRQC_VMDQRSS64EN;
> +		break;
> +
> +	case 4:
> +		mrqc |= IXGBE_MRQC_VMDQRSS32EN;
> +		break;
> +
> +	default:
> +		PMD_INIT_LOG(ERR, "Invalid pool number in non-IOV mode with VMDQ RSS");
> +		return -EINVAL;
> +	}
> +
> +	IXGBE_WRITE_REG(hw, IXGBE_MRQC, mrqc);
> +
> +	ixgbe_vmdq_pool_configure(dev);
> +
> +	return 0;
> +}

So ixgbe_config_vmdq_rss() checks nb_q_per_pool value, and might return an error if the value is invalid.
Though this return value seems just ignored by ixgbe_dev_mq_rx_configure() below.
Probably, it is better to move nb_q_per_pool value checking into rte_eth_dev_check_mq_mode(),
as is done for other modes?
I know it is no ideal, as it probably should be HW specific check,
but seems anyway better than just ignoring the error.

Konstantin

> +
> +static int
>  ixgbe_config_vf_default(struct rte_eth_dev *dev)
>  {
>  	struct ixgbe_hw *hw =
> @@ -3694,6 +3745,10 @@ ixgbe_dev_mq_rx_configure(struct rte_eth_dev *dev)
>  				ixgbe_vmdq_rx_hw_configure(dev);
>  				break;
> 
> +			case ETH_MQ_RX_VMDQ_RSS:
> +				ixgbe_config_vmdq_rss(dev);
> +				break;
> +
>  			case ETH_MQ_RX_NONE:
>  				/* if mq_mode is none, disable rss mode.*/
>  			default: ixgbe_rss_disable(dev);
> @@ -4186,6 +4241,8 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
> 
>  	/* Setup RX queues */
>  	for (i = 0; i < dev->data->nb_rx_queues; i++) {
> +		uint32_t psrtype = 0;
> +
>  		rxq = dev->data->rx_queues[i];
> 
>  		/*
> @@ -4213,12 +4270,10 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>  		if (rx_conf->header_split) {
>  			if (hw->mac.type == ixgbe_mac_82599EB) {
>  				/* Must setup the PSRTYPE register */
> -				uint32_t psrtype;
>  				psrtype = IXGBE_PSRTYPE_TCPHDR |
>  					IXGBE_PSRTYPE_UDPHDR   |
>  					IXGBE_PSRTYPE_IPV4HDR  |
>  					IXGBE_PSRTYPE_IPV6HDR;
> -				IXGBE_WRITE_REG(hw, IXGBE_PSRTYPE(rxq->reg_idx), psrtype);
>  			}
>  			srrctl = ((rx_conf->split_hdr_size <<
>  				IXGBE_SRRCTL_BSIZEHDRSIZE_SHIFT) &
> @@ -4228,6 +4283,11 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>  #endif
>  			srrctl = IXGBE_SRRCTL_DESCTYPE_ADV_ONEBUF;
> 
> +		/* Set RQPL for VMDQ RSS according to max Rx queue */
> +		psrtype |= (RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool >> 1) <<
> +			IXGBE_PSRTYPE_RQPL_SHIFT;
> +		IXGBE_WRITE_REG(hw, IXGBE_PSRTYPE(rxq->reg_idx), psrtype);
> +
>  		/* Set if packets are dropped when no descriptors available */
>  		if (rxq->drop_en)
>  			srrctl |= IXGBE_SRRCTL_DROP_EN;
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index f593f6e..fe9dc5c 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -1067,6 +1067,17 @@ rte_eth_dev_check_mq_mode(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>  				return -EINVAL;
>  			}
>  		}
> +
> +		if (dev_conf->rxmode.mq_mode == ETH_MQ_RX_VMDQ_RSS) {
> +			uint32_t nb_queue_pools =
> +				dev_conf->rx_adv_conf.vmdq_rx_conf.nb_queue_pools;
> +			struct rte_eth_dev_info dev_info;
> +
> +			rte_eth_dev_info_get(port_id, &dev_info);
> +			dev->data->dev_conf.rxmode.mq_mode = ETH_MQ_RX_VMDQ_RSS;
> +			RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool =
> +				dev_info.vmdq_queue_num / nb_queue_pools;
> +		}
>  	}
>  	return 0;
>  }
> --
> 1.9.0

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 10/13] ixgbe: support VMDq RSS in non-SRIOV environment
  2015-10-21 10:05   ` Ananyev, Konstantin
@ 2015-10-21 13:01     ` Yuanhan Liu
  2015-10-21 13:03       ` Ananyev, Konstantin
  0 siblings, 1 reply; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-21 13:01 UTC (permalink / raw)
  To: Ananyev, Konstantin; +Cc: dev, marcel, Michael S. Tsirkin

> >  static int
> > +ixgbe_config_vmdq_rss(struct rte_eth_dev *dev)
> > +{
> > +	struct ixgbe_hw *hw;
> > +	uint32_t mrqc;
> > +
> > +	ixgbe_rss_configure(dev);
> > +
> > +	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> > +
> > +	/* MRQC: enable VMDQ RSS */
> > +	mrqc = IXGBE_READ_REG(hw, IXGBE_MRQC);
> > +	mrqc &= ~IXGBE_MRQC_MRQE_MASK;
> > +
> > +	switch (RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool) {
> > +	case 2:
> > +		mrqc |= IXGBE_MRQC_VMDQRSS64EN;
> > +		break;
> > +
> > +	case 4:
> > +		mrqc |= IXGBE_MRQC_VMDQRSS32EN;
> > +		break;
> > +
> > +	default:
> > +		PMD_INIT_LOG(ERR, "Invalid pool number in non-IOV mode with VMDQ RSS");
> > +		return -EINVAL;
> > +	}
> > +
> > +	IXGBE_WRITE_REG(hw, IXGBE_MRQC, mrqc);
> > +
> > +	ixgbe_vmdq_pool_configure(dev);
> > +
> > +	return 0;
> > +}
> 
> So ixgbe_config_vmdq_rss() checks nb_q_per_pool value, and might return an error if the value is invalid.
> Though this return value seems just ignored by ixgbe_dev_mq_rx_configure() below.
> Probably, it is better to move nb_q_per_pool value checking into rte_eth_dev_check_mq_mode(),
> as is done for other modes?
> I know it is no ideal, as it probably should be HW specific check,
> but seems anyway better than just ignoring the error.
> 

Hi,

Thanks for the review. However, as you might have seen, I dropped this
patch in the lastest version. This patch is for using the NIC VMDq
feature to link with VM multiple queue to demonstrate the multiple
queue feature. However, this introduces too much limitation. So,
I dropped it.

However, despite of above usage, if you think it's necessary to add
such ability (support VMDq RSS in non-SRIOV), I could update this patch
based on your comments, and sent it out as a standalone patch. Otherwise,
I'll simply drop it.

	--yliu

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 10/13] ixgbe: support VMDq RSS in non-SRIOV environment
  2015-10-21 13:01     ` Yuanhan Liu
@ 2015-10-21 13:03       ` Ananyev, Konstantin
  2015-10-21 13:45         ` Yuanhan Liu
  0 siblings, 1 reply; 37+ messages in thread
From: Ananyev, Konstantin @ 2015-10-21 13:03 UTC (permalink / raw)
  To: Yuanhan Liu; +Cc: dev, marcel, Michael S. Tsirkin

Hi Yliu,

> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Wednesday, October 21, 2015 2:01 PM
> To: Ananyev, Konstantin
> Cc: dev@dpdk.org; Michael S. Tsirkin; marcel@redhat.com
> Subject: Re: [dpdk-dev] [PATCH v6 10/13] ixgbe: support VMDq RSS in non-SRIOV environment
> 
> > >  static int
> > > +ixgbe_config_vmdq_rss(struct rte_eth_dev *dev)
> > > +{
> > > +	struct ixgbe_hw *hw;
> > > +	uint32_t mrqc;
> > > +
> > > +	ixgbe_rss_configure(dev);
> > > +
> > > +	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> > > +
> > > +	/* MRQC: enable VMDQ RSS */
> > > +	mrqc = IXGBE_READ_REG(hw, IXGBE_MRQC);
> > > +	mrqc &= ~IXGBE_MRQC_MRQE_MASK;
> > > +
> > > +	switch (RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool) {
> > > +	case 2:
> > > +		mrqc |= IXGBE_MRQC_VMDQRSS64EN;
> > > +		break;
> > > +
> > > +	case 4:
> > > +		mrqc |= IXGBE_MRQC_VMDQRSS32EN;
> > > +		break;
> > > +
> > > +	default:
> > > +		PMD_INIT_LOG(ERR, "Invalid pool number in non-IOV mode with VMDQ RSS");
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	IXGBE_WRITE_REG(hw, IXGBE_MRQC, mrqc);
> > > +
> > > +	ixgbe_vmdq_pool_configure(dev);
> > > +
> > > +	return 0;
> > > +}
> >
> > So ixgbe_config_vmdq_rss() checks nb_q_per_pool value, and might return an error if the value is invalid.
> > Though this return value seems just ignored by ixgbe_dev_mq_rx_configure() below.
> > Probably, it is better to move nb_q_per_pool value checking into rte_eth_dev_check_mq_mode(),
> > as is done for other modes?
> > I know it is no ideal, as it probably should be HW specific check,
> > but seems anyway better than just ignoring the error.
> >
> 
> Hi,
> 
> Thanks for the review. However, as you might have seen, I dropped this
> patch in the lastest version. This patch is for using the NIC VMDq
> feature to link with VM multiple queue to demonstrate the multiple
> queue feature. However, this introduces too much limitation. So,
> I dropped it.

Ah sorry, didn't notice it - too many patches are flying around these days.

> 
> However, despite of above usage, if you think it's necessary to add
> such ability (support VMDq RSS in non-SRIOV), I could update this patch
> based on your comments, and sent it out as a standalone patch. Otherwise,
> I'll simply drop it.

That's ok by me.
Konstantin

> 
> 	--yliu

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v6 10/13] ixgbe: support VMDq RSS in non-SRIOV environment
  2015-10-21 13:03       ` Ananyev, Konstantin
@ 2015-10-21 13:45         ` Yuanhan Liu
  0 siblings, 0 replies; 37+ messages in thread
From: Yuanhan Liu @ 2015-10-21 13:45 UTC (permalink / raw)
  To: Ananyev, Konstantin; +Cc: dev, marcel, Michael S. Tsirkin

> > 
> > Hi,
> > 
> > Thanks for the review. However, as you might have seen, I dropped this
> > patch in the lastest version. This patch is for using the NIC VMDq
> > feature to link with VM multiple queue to demonstrate the multiple
> > queue feature. However, this introduces too much limitation. So,
> > I dropped it.
> 
> Ah sorry, didn't notice it - too many patches are flying around these days.

That's true :)

	--yliu
> 
> > 
> > However, despite of above usage, if you think it's necessary to add
> > such ability (support VMDq RSS in non-SRIOV), I could update this patch
> > based on your comments, and sent it out as a standalone patch. Otherwise,
> > I'll simply drop it.
> 
> That's ok by me.
> Konstantin
> 
> > 
> > 	--yliu

^ permalink raw reply	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2015-10-21 13:44 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-09  5:45 [PATCH v6 00/13] vhost-user multiple queues enabling Yuanhan Liu
2015-10-09  5:46 ` [PATCH v6 01/13] vhost-user: add protocol features support Yuanhan Liu
2015-10-09  5:46 ` [PATCH v6 02/13] vhost-user: add VHOST_USER_GET_QUEUE_NUM message Yuanhan Liu
2015-10-09  5:46 ` [PATCH v6 03/13] vhost: vring queue setup for multiple queue support Yuanhan Liu
2015-10-20  6:49   ` Thomas Monjalon
2015-10-20  6:56     ` Yuanhan Liu
2015-10-09  5:46 ` [PATCH v6 04/13] vhost: rxtx: prepare work " Yuanhan Liu
2015-10-20  6:57   ` Thomas Monjalon
2015-10-20  7:10     ` Yuanhan Liu
2015-10-20  7:24       ` Thomas Monjalon
2015-10-09  5:46 ` [PATCH v6 05/13] vhost-user: handle VHOST_USER_RESET_OWNER correctly Yuanhan Liu
2015-10-20  7:03   ` Thomas Monjalon
2015-10-20  7:14     ` Yuanhan Liu
2015-10-09  5:46 ` [PATCH v6 06/13] virtio: read virtio_net_config correctly Yuanhan Liu
2015-10-20  7:07   ` Thomas Monjalon
2015-10-20  7:23     ` Yuanhan Liu
2015-10-20  7:27       ` Thomas Monjalon
2015-10-20  8:00         ` Yuanhan Liu
2015-10-09  5:46 ` [PATCH v6 07/13] vhost-user: enable vhost-user multiple queue Yuanhan Liu
2015-10-09  5:46 ` [PATCH v6 08/13] vhost: add VHOST_USER_SET_VRING_ENABLE message Yuanhan Liu
2015-10-09  5:46 ` [PATCH v6 09/13] vhost: add API bind a virtq to a specific core Yuanhan Liu
2015-10-20  6:16   ` Xie, Huawei
2015-10-20  6:25     ` Yuanhan Liu
2015-10-20  7:17       ` Thomas Monjalon
2015-10-09  5:46 ` [PATCH v6 10/13] ixgbe: support VMDq RSS in non-SRIOV environment Yuanhan Liu
2015-10-20  7:46   ` Thomas Monjalon
2015-10-21 10:05   ` Ananyev, Konstantin
2015-10-21 13:01     ` Yuanhan Liu
2015-10-21 13:03       ` Ananyev, Konstantin
2015-10-21 13:45         ` Yuanhan Liu
2015-10-09  5:46 ` [PATCH v6 11/13] examples/vhost: demonstrate the usage of vhost mq feature Yuanhan Liu
2015-10-09  5:46 ` [PATCH v6 12/13] examples/vhost: add per queue stats Yuanhan Liu
2015-10-09  5:46 ` [PATCH v6 13/13] doc: update release note for vhost-user mq support Yuanhan Liu
2015-10-09 13:45 ` [PATCH v6 00/13] vhost-user multiple queues enabling Marcel Apfelbaum
2015-10-19 13:16 ` Flavio Leitner
2015-10-20 10:39 ` Marcel Apfelbaum
2015-10-21  2:05   ` Yuanhan Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.