Thread (59 messages) 59 messages, 4 authors, 2020-07-17

Re: [dpdk-dev] [PATCH v1 1/4] vhost: support host notifier queue configuration

From: Matan Azrad <hidden>
Date: 2020-06-19 13:29:09


From: Maxime Coquelin:
On 6/18/20 6:28 PM, Matan Azrad wrote:
quoted
As an arrangement to per queue operations in the vDPA device it is
needed to change the next experimental API:

The API ``rte_vhost_host_notifier_ctrl`` was changed to be per queue
instead of per device.

A `qid` parameter was added to the API arguments list.

Setting the parameter to the value VHOST_QUEUE_ALL will configure the
host notifier to all the device queues as done before this patch.

Signed-off-by: Matan Azrad <redacted>
---
 doc/guides/rel_notes/release_20_08.rst |  2 ++
 drivers/vdpa/ifc/ifcvf_vdpa.c          |  6 +++---
 drivers/vdpa/mlx5/mlx5_vdpa.c          |  5 +++--
 lib/librte_vhost/rte_vdpa.h            |  8 ++++++--
 lib/librte_vhost/rte_vhost.h           |  2 ++
 lib/librte_vhost/vhost.h               |  3 ---
 lib/librte_vhost/vhost_user.c          | 18 ++++++++++++++----
 7 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/doc/guides/rel_notes/release_20_08.rst
b/doc/guides/rel_notes/release_20_08.rst
index ba16d3b..9732959 100644
--- a/doc/guides/rel_notes/release_20_08.rst
+++ b/doc/guides/rel_notes/release_20_08.rst
@@ -111,6 +111,8 @@ API Changes
    Also, make sure to start the actual text at the margin.
=========================================================
quoted
+* vhost: The API of ``rte_vhost_host_notifier_ctrl`` was changed to
+be per
+  queue and not per device, a qid parameter was added to the arguments
list.
quoted
 ABI Changes
 -----------
diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c
b/drivers/vdpa/ifc/ifcvf_vdpa.c index ec97178..336837a 100644
--- a/drivers/vdpa/ifc/ifcvf_vdpa.c
+++ b/drivers/vdpa/ifc/ifcvf_vdpa.c
@@ -839,7 +839,7 @@ struct internal_list {
 	vdpa_ifcvf_stop(internal);
 	vdpa_disable_vfio_intr(internal);

-	ret = rte_vhost_host_notifier_ctrl(vid, false);
+	ret = rte_vhost_host_notifier_ctrl(vid, VHOST_QUEUE_ALL, false);
 	if (ret && ret != -ENOTSUP)
 		goto error;
@@ -858,7 +858,7 @@ struct internal_list {
 	if (ret)
 		goto stop_vf;

-	rte_vhost_host_notifier_ctrl(vid, true);
+	rte_vhost_host_notifier_ctrl(vid, VHOST_QUEUE_ALL, true);

 	internal->sw_fallback_running = true;
@@ -893,7 +893,7 @@ struct internal_list {
 	rte_atomic32_set(&internal->dev_attached, 1);
 	update_datapath(internal);

-	if (rte_vhost_host_notifier_ctrl(vid, true) != 0)
+	if (rte_vhost_host_notifier_ctrl(vid, VHOST_QUEUE_ALL, true) != 0)
 		DRV_LOG(NOTICE, "vDPA (%d): software relay is used.", did);

 	return 0;
diff --git a/drivers/vdpa/mlx5/mlx5_vdpa.c
b/drivers/vdpa/mlx5/mlx5_vdpa.c index 9e758b6..8ea1300 100644
--- a/drivers/vdpa/mlx5/mlx5_vdpa.c
+++ b/drivers/vdpa/mlx5/mlx5_vdpa.c
@@ -147,7 +147,8 @@
 	int ret;

 	if (priv->direct_notifier) {
-		ret = rte_vhost_host_notifier_ctrl(priv->vid, false);
+		ret = rte_vhost_host_notifier_ctrl(priv->vid,
VHOST_QUEUE_ALL,
quoted
+						   false);
 		if (ret != 0) {
 			DRV_LOG(INFO, "Direct HW notifier FD cannot be "
 				"destroyed for device %d: %d.", priv->vid,
ret); @@ -155,7 +156,7
quoted
@@
 		}
 		priv->direct_notifier = 0;
 	}
-	ret = rte_vhost_host_notifier_ctrl(priv->vid, true);
+	ret = rte_vhost_host_notifier_ctrl(priv->vid, VHOST_QUEUE_ALL,
+true);
 	if (ret != 0)
 		DRV_LOG(INFO, "Direct HW notifier FD cannot be configured
for"
quoted
 			" device %d: %d.", priv->vid, ret); diff --git
a/lib/librte_vhost/rte_vdpa.h b/lib/librte_vhost/rte_vdpa.h index
ecb3d91..2db536c 100644
--- a/lib/librte_vhost/rte_vdpa.h
+++ b/lib/librte_vhost/rte_vdpa.h
@@ -202,22 +202,26 @@ struct rte_vdpa_device *  int
rte_vdpa_get_device_num(void);

+#define VHOST_QUEUE_ALL VHOST_MAX_VRING
+
 /**
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice
  *
- * Enable/Disable host notifier mapping for a vdpa port.
+ * Enable/Disable host notifier mapping for a vdpa queue.
  *
  * @param vid
  *  vhost device id
  * @param enable
  *  true for host notifier map, false for host notifier unmap
+ * @param qid
+ *  vhost queue id, VHOST_QUEUE_ALL to configure all the device
+ queues
I would prefer two APIs that passing a special ID that means all queues:

rte_vhost_host_notifier_ctrl(int vid, uint16_t qid, bool enable);
rte_vhost_host_notifier_ctrl_all(int vid, bool enable);

I think it is clearer for the user of the API.
Or if you think an extra API is overkill, just let the driver loop on all the
queues.
We have a lot of options here with pros and cons.
I took the rte_eth_dev_callback_register style.

It is less intrusive with minimum code change.  

I'm not sure what is the clearest option but the current suggestion is well defined and 
allows to configure all the queues too.

Let me know what you prefer....
quoted
  * @return
  *  0 on success, -1 on failure
  */
 __rte_experimental
 int
-rte_vhost_host_notifier_ctrl(int vid, bool enable);
+rte_vhost_host_notifier_ctrl(int vid, uint16_t qid, bool enable);

 /**
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help