[dpdk-dev] [PATCH v2 07/14] vhost: replace device ID in applications

[Maxime Coquelin <maxime.coquelin@redhat.com>]

This patch replaces the use of vDPA device ID with
vDPA device pointer. The goals is to remove the vDPA
device ID to avoid condusion with the Vhost ID.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 examples/vdpa/main.c                   | 118 +++++++++++++------------
 lib/librte_vhost/rte_vdpa.h            |  44 ++++++---
 lib/librte_vhost/rte_vhost_version.map |   3 +-
 lib/librte_vhost/vdpa.c                |  64 +++++++-------
 4 files changed, 123 insertions(+), 106 deletions(-)

diff --git a/examples/vdpa/main.c b/examples/vdpa/main.c
index 6a719e32d8..54f4940b31 100644
--- a/examples/vdpa/main.c
+++ b/examples/vdpa/main.c
@@ -27,7 +27,7 @@
 
 struct vdpa_port {
 	char ifname[MAX_PATH_LEN];
-	int did;
+	struct rte_vdpa_device *dev;
 	int vid;
 	uint64_t flags;
 	int stats_n;
@@ -105,16 +105,23 @@ static int
 new_device(int vid)
 {
 	char ifname[MAX_PATH_LEN];
+	struct rte_device *dev;
 	int i;
 
 	rte_vhost_get_ifname(vid, ifname, sizeof(ifname));
 	for (i = 0; i < MAX_VDPA_SAMPLE_PORTS; i++) {
-		if (strncmp(ifname, vports[i].ifname, MAX_PATH_LEN) == 0) {
-			printf("\nnew port %s, did: %d\n",
-					ifname, vports[i].did);
-			vports[i].vid = vid;
-			break;
+		if (strncmp(ifname, vports[i].ifname, MAX_PATH_LEN))
+			continue;
+
+		dev = rte_vdpa_get_rte_device(vports[i].dev);
+		if (!dev) {
+			RTE_LOG(ERR, VDPA,
+				"Failed to get generic device for port %d\n", i);
+			continue;
 		}
+		printf("\nnew port %s, device : %s\n", ifname, dev->name);
+		vports[i].vid = vid;
+		break;
 	}
 
 	if (i >= MAX_VDPA_SAMPLE_PORTS)
@@ -126,16 +133,24 @@ new_device(int vid)
 static void
 destroy_device(int vid)
 {
+	struct rte_device *dev;
 	char ifname[MAX_PATH_LEN];
 	int i;
 
 	rte_vhost_get_ifname(vid, ifname, sizeof(ifname));
 	for (i = 0; i < MAX_VDPA_SAMPLE_PORTS; i++) {
-		if (strcmp(ifname, vports[i].ifname) == 0) {
-			printf("\ndestroy port %s, did: %d\n",
-					ifname, vports[i].did);
-			break;
+		if (strncmp(ifname, vports[i].ifname, MAX_PATH_LEN))
+			continue;
+
+		dev = rte_vdpa_get_rte_device(vports[i].dev);
+		if (!dev) {
+			RTE_LOG(ERR, VDPA,
+				"Failed to get generic device for port %d\n", i);
+			continue;
 		}
+
+		printf("\ndestroy port %s, device: %s\n", ifname, dev->name);
+		break;
 	}
 }
 
@@ -149,8 +164,6 @@ start_vdpa(struct vdpa_port *vport)
 {
 	int ret;
 	char *socket_path = vport->ifname;
-	struct rte_vdpa_device *vdev;
-	int did = vport->did;
 
 	if (client_mode)
 		vport->flags |= RTE_VHOST_USER_CLIENT;
@@ -174,13 +187,7 @@ start_vdpa(struct vdpa_port *vport)
 			"register driver ops failed: %s\n",
 			socket_path);
 
-	vdev = rte_vdpa_get_device(did);
-	if (!vdev)
-		rte_exit(EXIT_FAILURE,
-			"vDPA device retrieval failed: %p\n",
-			vdev);
-
-	ret = rte_vhost_driver_attach_vdpa_device(socket_path, vdev);
+	ret = rte_vhost_driver_attach_vdpa_device(socket_path, vport->dev);
 	if (ret != 0)
 		rte_exit(EXIT_FAILURE,
 			"attach vdpa device failed: %s\n",
@@ -283,35 +290,31 @@ static void cmd_list_vdpa_devices_parsed(
 		struct cmdline *cl,
 		__rte_unused void *data)
 {
-	int did;
 	uint32_t queue_num;
 	uint64_t features;
 	struct rte_vdpa_device *vdev;
 	struct rte_device *dev;
 	struct rte_dev_iterator dev_iter;
 
-	cmdline_printf(cl, "device id\tdevice name\tqueue num\tsupported features\n");
+	cmdline_printf(cl, "device name\tqueue num\tsupported features\n");
 	RTE_DEV_FOREACH(dev, "class=vdpa", &dev_iter) {
-		did = rte_vdpa_find_device_id_by_name(dev->name);
-		if (did < 0)
-			continue;
-		vdev = rte_vdpa_get_device(did);
+		vdev = rte_vdpa_find_device_by_name(dev->name);
 		if (!vdev)
 			continue;
 		if (vdev->ops->get_queue_num(vdev, &queue_num) < 0) {
 			RTE_LOG(ERR, VDPA,
 				"failed to get vdpa queue number "
-				"for device id %d.\n", did);
+				"for device %s.\n", dev->name);
 			continue;
 		}
 		if (vdev->ops->get_features(vdev, &features) < 0) {
 			RTE_LOG(ERR, VDPA,
 				"failed to get vdpa features "
-				"for device id %d.\n", did);
+				"for device %s.\n", dev->name);
 			continue;
 		}
-		cmdline_printf(cl, "%d\t\t%s\t\t%" PRIu32 "\t\t0x%" PRIx64 "\n",
-			did, dev->name, queue_num, features);
+		cmdline_printf(cl, "%s\t\t%" PRIu32 "\t\t0x%" PRIx64 "\n",
+			dev->name, queue_num, features);
 	}
 }
 
@@ -339,18 +342,18 @@ static void cmd_create_vdpa_port_parsed(void *parsed_result,
 		struct cmdline *cl,
 		__rte_unused void *data)
 {
-	int did;
+	struct rte_vdpa_device *dev;
 	struct cmd_create_result *res = parsed_result;
 
 	rte_strscpy(vports[devcnt].ifname, res->socket_path, MAX_PATH_LEN);
-	did = rte_vdpa_find_device_id_by_name(res->bdf);
-	if (did < 0) {
+	dev = rte_vdpa_find_device_by_name(res->bdf);
+	if (dev == NULL) {
 		cmdline_printf(cl, "Unable to find vdpa device id for %s.\n",
 				res->bdf);
 		return;
 	}
 
-	vports[devcnt].did = did;
+	vports[devcnt].dev = dev;
 
 	if (start_vdpa(&vports[devcnt]) == 0)
 		devcnt++;
@@ -378,7 +381,7 @@ cmdline_parse_inst_t cmd_create_vdpa_port = {
 /* *** STATS *** */
 struct cmd_stats_result {
 	cmdline_fixed_string_t stats;
-	uint16_t did;
+	cmdline_fixed_string_t bdf;
 	uint16_t qid;
 };
 
@@ -386,25 +389,24 @@ static void cmd_device_stats_parsed(void *parsed_result, struct cmdline *cl,
 				    __rte_unused void *data)
 {
 	struct cmd_stats_result *res = parsed_result;
-	struct rte_vdpa_device *vdev = rte_vdpa_get_device(res->did);
+	struct rte_vdpa_device *vdev = rte_vdpa_find_device_by_name(res->bdf);
 	struct vdpa_port *vport = NULL;
 	uint32_t first, last;
 	int i;
 
 	if (!vdev) {
-		RTE_LOG(ERR, VDPA, "Invalid device id %" PRIu16 ".\n",
-			res->did);
+		RTE_LOG(ERR, VDPA, "Invalid device: %s.\n",
+			res->bdf);
 		return;
 	}
 	for (i = 0; i < RTE_MIN(MAX_VDPA_SAMPLE_PORTS, dev_total); i++) {
-		if (vports[i].did == res->did) {
+		if (vports[i].dev == vdev) {
 			vport = &vports[i];
 			break;
 		}
 	}
 	if (!vport) {
-		RTE_LOG(ERR, VDPA, "Device id %" PRIu16 " was not created.\n",
-			res->did);
+		RTE_LOG(ERR, VDPA, "Device %s was not created.\n", res->bdf);
 		return;
 	}
 	if (res->qid == 0xFFFF) {
@@ -412,7 +414,7 @@ static void cmd_device_stats_parsed(void *parsed_result, struct cmdline *cl,
 		last = rte_vhost_get_vring_num(vport->vid);
 		if (last == 0) {
 			RTE_LOG(ERR, VDPA, "Failed to get num of actual virtqs"
-				" for device id %d.\n", (int)res->did);
+				" for device %s.\n", res->bdf);
 			return;
 		}
 		last--;
@@ -421,10 +423,10 @@ static void cmd_device_stats_parsed(void *parsed_result, struct cmdline *cl,
 		last = res->qid;
 	}
 	if (!vport->stats_names) {
-		vport->stats_n = rte_vdpa_get_stats_names(vport->did, NULL, 0);
+		vport->stats_n = rte_vdpa_get_stats_names(vport->dev, NULL, 0);
 		if (vport->stats_n <= 0) {
 			RTE_LOG(ERR, VDPA, "Failed to get names number of "
-				"device %d stats.\n", (int)res->did);
+				"device %s stats.\n", res->bdf);
 			return;
 		}
 		vport->stats_names = rte_zmalloc(NULL,
@@ -432,26 +434,26 @@ static void cmd_device_stats_parsed(void *parsed_result, struct cmdline *cl,
 							vport->stats_n, 0);
 		if (!vport->stats_names) {
 			RTE_LOG(ERR, VDPA, "Failed to allocate memory for stat"
-				" names of device %d.\n", (int)res->did);
+				" names of device %s.\n", res->bdf);
 			return;
 		}
-		i = rte_vdpa_get_stats_names(vport->did, vport->stats_names,
+		i = rte_vdpa_get_stats_names(vport->dev, vport->stats_names,
 						vport->stats_n);
 		if (vport->stats_n != i) {
-			RTE_LOG(ERR, VDPA, "Failed to get names of device %d "
-				"stats.\n", (int)res->did);
+			RTE_LOG(ERR, VDPA, "Failed to get names of device %s "
+				"stats.\n", res->bdf);
 			return;
 		}
 		vport->stats = (struct rte_vdpa_stat *)
 					(vport->stats_names + vport->stats_n);
 	}
-	cmdline_printf(cl, "\nDevice %d:\n", (int)res->did);
+	cmdline_printf(cl, "\nDevice %s:\n", res->bdf);
 	for (; first <= last; first++) {
 		memset(vport->stats, 0, sizeof(*vport->stats) * vport->stats_n);
-		if (rte_vdpa_get_stats(vport->did, (int)first, vport->stats,
+		if (rte_vdpa_get_stats(vport->dev, (int)first, vport->stats,
 					vport->stats_n) <= 0) {
 			RTE_LOG(ERR, VDPA, "Failed to get vdpa queue statistics"
-				" for device id %d qid %d.\n", (int)res->did,
+				" for device %s qid %d.\n", res->bdf,
 				(int)first);
 			return;
 		}
@@ -467,8 +469,8 @@ static void cmd_device_stats_parsed(void *parsed_result, struct cmdline *cl,
 
 cmdline_parse_token_string_t cmd_device_stats_ =
 	TOKEN_STRING_INITIALIZER(struct cmd_stats_result, stats, "stats");
-cmdline_parse_token_num_t cmd_device_id =
-	TOKEN_NUM_INITIALIZER(struct cmd_stats_result, did, UINT32);
+cmdline_parse_token_string_t cmd_device_bdf =
+	TOKEN_STRING_INITIALIZER(struct cmd_stats_result, bdf, NULL);
 cmdline_parse_token_num_t cmd_queue_id =
 	TOKEN_NUM_INITIALIZER(struct cmd_stats_result, qid, UINT32);
 
@@ -478,7 +480,7 @@ cmdline_parse_inst_t cmd_device_stats = {
 	.help_str = "stats: show device statistics",
 	.tokens = {
 		(void *)&cmd_device_stats_,
-		(void *)&cmd_device_id,
+		(void *)&cmd_device_bdf,
 		(void *)&cmd_queue_id,
 		NULL,
 	},
@@ -522,9 +524,9 @@ int
 main(int argc, char *argv[])
 {
 	char ch;
-	int did;
 	int ret;
 	struct cmdline *cl;
+	struct rte_vdpa_device *vdev;
 	struct rte_device *dev;
 	struct rte_dev_iterator dev_iter;
 
@@ -553,12 +555,12 @@ main(int argc, char *argv[])
 		cmdline_stdin_exit(cl);
 	} else {
 		RTE_DEV_FOREACH(dev, "class=vdpa", &dev_iter) {
-			did = rte_vdpa_find_device_id_by_name(dev->name);
-			if (did < 0) {
-				rte_panic("Failed to find device id for %s\n",
+			vdev = rte_vdpa_find_device_by_name(dev->name);
+			if (vdev == NULL) {
+				rte_panic("Failed to find vDPA dev for %s\n",
 						dev->name);
 			}
-			vports[devcnt].did = did;
+			vports[devcnt].dev = vdev;
 			snprintf(vports[devcnt].ifname, MAX_PATH_LEN, "%s%d",
 					iface, devcnt);
 
diff --git a/lib/librte_vhost/rte_vdpa.h b/lib/librte_vhost/rte_vdpa.h
index 109cf83b8f..549b0a2cb0 100644
--- a/lib/librte_vhost/rte_vdpa.h
+++ b/lib/librte_vhost/rte_vdpa.h
@@ -153,11 +153,26 @@ rte_vdpa_unregister_device(struct rte_vdpa_device *);
  * @param name
  *  the vdpa device name
  * @return
- *  device id on success, -1 on failure
+ *  vDPA device pointer on success, NULL on failure
  */
 __rte_experimental
-int
-rte_vdpa_find_device_id_by_name(const char *name);
+struct rte_vdpa_device *
+rte_vdpa_find_device_by_name(const char *name);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Get the generic device from the vdpa device
+ *
+ * @param vdpa_dev
+ *  the vdpa device pointer
+ * @return
+ *  generic device pointer on success, NULL on failure
+ */
+__rte_experimental
+struct rte_device *
+rte_vdpa_get_rte_device(struct rte_vdpa_device *vdpa_dev);
 
 /**
  * @warning
@@ -252,8 +267,8 @@ rte_vdpa_relay_vring_used(int vid, uint16_t qid, void *vring_m);
  * And the array index is same with id field of 'struct rte_vdpa_stat':
  * stats[i].id == i
  *
- * @param did
- *  device id
+ * @param dev
+ *  vDPA device pointer
  * @param stats_names
  *   array of at least size elements to be filled.
  *   If set to NULL, the function returns the required number of elements.
@@ -265,8 +280,9 @@ rte_vdpa_relay_vring_used(int vid, uint16_t qid, void *vring_m);
  */
 __rte_experimental
 int
-rte_vdpa_get_stats_names(int did, struct rte_vdpa_stat_name *stats_names,
-			 unsigned int size);
+rte_vdpa_get_stats_names(struct rte_vdpa_device *dev,
+		struct rte_vdpa_stat_name *stats_names,
+		unsigned int size);
 
 /**
  * @warning
@@ -280,8 +296,8 @@ rte_vdpa_get_stats_names(int did, struct rte_vdpa_stat_name *stats_names,
  * And the array index is same with id field of 'struct rte_vdpa_stat':
  * stats[i].id == i
  *
- * @param did
- *  device id
+ * @param dev
+ *  vDPA device pointer
  * @param qid
  *  queue id
  * @param stats
@@ -295,16 +311,16 @@ rte_vdpa_get_stats_names(int did, struct rte_vdpa_stat_name *stats_names,
  */
 __rte_experimental
 int
-rte_vdpa_get_stats(int did, uint16_t qid, struct rte_vdpa_stat *stats,
-		   unsigned int n);
+rte_vdpa_get_stats(struct rte_vdpa_device *dev, uint16_t qid,
+		struct rte_vdpa_stat *stats, unsigned int n);
 /**
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice
  *
  * Reset statistics of a vDPA device.
  *
- * @param did
- *  device id
+ * @param dev
+ *  vDPA device pointer
  * @param qid
  *  queue id
  * @return
@@ -312,5 +328,5 @@ rte_vdpa_get_stats(int did, uint16_t qid, struct rte_vdpa_stat *stats,
  */
 __rte_experimental
 int
-rte_vdpa_reset_stats(int did, uint16_t qid);
+rte_vdpa_reset_stats(struct rte_vdpa_device *dev, uint16_t qid);
 #endif /* _RTE_VDPA_H_ */
diff --git a/lib/librte_vhost/rte_vhost_version.map b/lib/librte_vhost/rte_vhost_version.map
index 60d67445d7..ba55f6f421 100644
--- a/lib/librte_vhost/rte_vhost_version.map
+++ b/lib/librte_vhost/rte_vhost_version.map
@@ -69,5 +69,6 @@ EXPERIMENTAL {
 	rte_vhost_get_vhost_ring_inflight;
 	rte_vhost_get_vring_base_from_inflight;
 	rte_vhost_slave_config_change;
-	rte_vdpa_find_device_id_by_name;
+	rte_vdpa_find_device_by_name;
+	rte_vdpa_get_rte_device;
 };
diff --git a/lib/librte_vhost/vdpa.c b/lib/librte_vhost/vdpa.c
index ed20082f0f..91bc483774 100644
--- a/lib/librte_vhost/vdpa.c
+++ b/lib/librte_vhost/vdpa.c
@@ -40,14 +40,14 @@ rte_vdpa_find_device_id(struct rte_vdpa_device *dev)
 	return -1;
 }
 
-int
-rte_vdpa_find_device_id_by_name(const char *name)
+struct rte_vdpa_device *
+rte_vdpa_find_device_by_name(const char *name)
 {
 	struct rte_vdpa_device *dev;
 	int i;
 
 	if (name == NULL)
-		return -1;
+		return NULL;
 
 	for (i = 0; i < MAX_VHOST_DEVICE; ++i) {
 		dev = &vdpa_devices[i];
@@ -55,10 +55,19 @@ rte_vdpa_find_device_id_by_name(const char *name)
 			continue;
 
 		if (strcmp(dev->device->name, name) == 0)
-			return i;
+			return dev;
 	}
 
-	return -1;
+	return NULL;
+}
+
+struct rte_device *
+rte_vdpa_get_rte_device(struct rte_vdpa_device *vdpa_dev)
+{
+	if (vdpa_dev == NULL)
+		return NULL;
+
+	return vdpa_dev->device;
 }
 
 struct rte_vdpa_device *
@@ -228,50 +237,39 @@ rte_vdpa_relay_vring_used(int vid, uint16_t qid, void *vring_m)
 }
 
 int
-rte_vdpa_get_stats_names(int did, struct rte_vdpa_stat_name *stats_names,
-			 unsigned int size)
+rte_vdpa_get_stats_names(struct rte_vdpa_device *dev,
+		struct rte_vdpa_stat_name *stats_names,
+		unsigned int size)
 {
-	struct rte_vdpa_device *vdpa_dev;
-
-	vdpa_dev = rte_vdpa_get_device(did);
-	if (!vdpa_dev)
-		return -ENODEV;
+	if (!dev)
+		return -EINVAL;
 
-	RTE_FUNC_PTR_OR_ERR_RET(vdpa_dev->ops->get_stats_names, -ENOTSUP);
+	RTE_FUNC_PTR_OR_ERR_RET(dev->ops->get_stats_names, -ENOTSUP);
 
-	return vdpa_dev->ops->get_stats_names(vdpa_dev, stats_names, size);
+	return dev->ops->get_stats_names(dev, stats_names, size);
 }
 
 int
-rte_vdpa_get_stats(int did, uint16_t qid, struct rte_vdpa_stat *stats,
-		   unsigned int n)
+rte_vdpa_get_stats(struct rte_vdpa_device *dev, uint16_t qid,
+		struct rte_vdpa_stat *stats, unsigned int n)
 {
-	struct rte_vdpa_device *vdpa_dev;
-
-	vdpa_dev = rte_vdpa_get_device(did);
-	if (!vdpa_dev)
-		return -ENODEV;
-
-	if (!stats || !n)
+	if (!dev || !stats || !n)
 		return -EINVAL;
 
-	RTE_FUNC_PTR_OR_ERR_RET(vdpa_dev->ops->get_stats, -ENOTSUP);
+	RTE_FUNC_PTR_OR_ERR_RET(dev->ops->get_stats, -ENOTSUP);
 
-	return vdpa_dev->ops->get_stats(vdpa_dev, qid, stats, n);
+	return dev->ops->get_stats(dev, qid, stats, n);
 }
 
 int
-rte_vdpa_reset_stats(int did, uint16_t qid)
+rte_vdpa_reset_stats(struct rte_vdpa_device *dev, uint16_t qid)
 {
-	struct rte_vdpa_device *vdpa_dev;
-
-	vdpa_dev = rte_vdpa_get_device(did);
-	if (!vdpa_dev)
-		return -ENODEV;
+	if (!dev)
+		return -EINVAL;
 
-	RTE_FUNC_PTR_OR_ERR_RET(vdpa_dev->ops->reset_stats, -ENOTSUP);
+	RTE_FUNC_PTR_OR_ERR_RET(dev->ops->reset_stats, -ENOTSUP);
 
-	return vdpa_dev->ops->reset_stats(vdpa_dev, qid);
+	return dev->ops->reset_stats(dev, qid);
 }
 
 static uint16_t
-- 
2.26.2