On 6/24/20 2:26 PM, Maxime Coquelin wrote:
> 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.coque...@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
>
Acked-by: Adrián Moreno <amore...@redhat.com>
--
Adrián Moreno
