[dpdk-dev,v4,2/5] vhost: support selective datapath

Message ID 20180310100148.12030-3-zhihong.wang@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

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

Commit Message

Zhihong Wang March 10, 2018, 10:01 a.m. UTC
  This patch set introduces support for selective datapath in DPDK vhost-user
lib. vDPA stands for vhost Data Path Acceleration. The idea is to support
virtio ring compatible devices to serve virtio driver directly to enable
datapath acceleration.

A set of device ops is defined for device specific operations:

     a. queue_num_get: Called to get supported queue number of the device.

     b. feature_get: Called to get supported features of the device.

     c. protocol_feature_get: Called to get supported protocol features of
        the device.

     d. dev_conf: Called to configure the actual device when the virtio
        device becomes ready.

     e. dev_close: Called to close the actual device when the virtio device
        is stopped.

     f. vring_state_set: Called to change the state of the vring in the
        actual device when vring state changes.

     g. feature_set: Called to set the negotiated features to device.

     h. migration_done: Called to allow the device to response to RARP
        sending.

     i. get_vfio_group_fd: Called to get the VFIO group fd of the device.

     j. get_vfio_device_fd: Called to get the VFIO device fd of the device.

     k. get_notify_area: Called to get the notify area info of the queue.

Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
---
Changes in v4:

 1. Remove the "engine" concept in the lib.

---
Changes in v2:

 1. Add VFIO related vDPA device ops.

 lib/librte_vhost/Makefile              |  4 +-
 lib/librte_vhost/rte_vdpa.h            | 94 +++++++++++++++++++++++++++++++++
 lib/librte_vhost/rte_vhost_version.map |  6 +++
 lib/librte_vhost/vdpa.c                | 96 ++++++++++++++++++++++++++++++++++
 4 files changed, 198 insertions(+), 2 deletions(-)
 create mode 100644 lib/librte_vhost/rte_vdpa.h
 create mode 100644 lib/librte_vhost/vdpa.c
  

Comments

Maxime Coquelin March 31, 2018, 6:10 a.m. UTC | #1
On 03/10/2018 11:01 AM, Zhihong Wang wrote:
> This patch set introduces support for selective datapath in DPDK vhost-user
> lib. vDPA stands for vhost Data Path Acceleration. The idea is to support
> virtio ring compatible devices to serve virtio driver directly to enable
> datapath acceleration.
> 
> A set of device ops is defined for device specific operations:
> 
>       a. queue_num_get: Called to get supported queue number of the device.
> 
>       b. feature_get: Called to get supported features of the device.
> 
>       c. protocol_feature_get: Called to get supported protocol features of
>          the device.
> 
>       d. dev_conf: Called to configure the actual device when the virtio
>          device becomes ready.
> 
>       e. dev_close: Called to close the actual device when the virtio device
>          is stopped.
> 
>       f. vring_state_set: Called to change the state of the vring in the
>          actual device when vring state changes.
> 
>       g. feature_set: Called to set the negotiated features to device.
> 
>       h. migration_done: Called to allow the device to response to RARP
>          sending.
> 
>       i. get_vfio_group_fd: Called to get the VFIO group fd of the device.
> 
>       j. get_vfio_device_fd: Called to get the VFIO device fd of the device.
> 
>       k. get_notify_area: Called to get the notify area info of the queue.
> 
> Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
> ---
> Changes in v4:
> 
>   1. Remove the "engine" concept in the lib.
> 
> ---
> Changes in v2:
> 
>   1. Add VFIO related vDPA device ops.
> 
>   lib/librte_vhost/Makefile              |  4 +-
>   lib/librte_vhost/rte_vdpa.h            | 94 +++++++++++++++++++++++++++++++++
>   lib/librte_vhost/rte_vhost_version.map |  6 +++
>   lib/librte_vhost/vdpa.c                | 96 ++++++++++++++++++++++++++++++++++
>   4 files changed, 198 insertions(+), 2 deletions(-)
>   create mode 100644 lib/librte_vhost/rte_vdpa.h
>   create mode 100644 lib/librte_vhost/vdpa.c
> 
> diff --git a/lib/librte_vhost/Makefile b/lib/librte_vhost/Makefile
> index 5d6c6abae..37044ac03 100644
> --- a/lib/librte_vhost/Makefile
> +++ b/lib/librte_vhost/Makefile
> @@ -22,9 +22,9 @@ LDLIBS += -lrte_eal -lrte_mempool -lrte_mbuf -lrte_ethdev -lrte_net
>   
>   # all source are stored in SRCS-y
>   SRCS-$(CONFIG_RTE_LIBRTE_VHOST) := fd_man.c iotlb.c socket.c vhost.c \
> -					vhost_user.c virtio_net.c
> +					vhost_user.c virtio_net.c vdpa.c
>   
>   # install includes
> -SYMLINK-$(CONFIG_RTE_LIBRTE_VHOST)-include += rte_vhost.h
> +SYMLINK-$(CONFIG_RTE_LIBRTE_VHOST)-include += rte_vhost.h rte_vdpa.h
>   
>   include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/lib/librte_vhost/rte_vdpa.h b/lib/librte_vhost/rte_vdpa.h
> new file mode 100644
> index 000000000..a4bbbd93d
> --- /dev/null
> +++ b/lib/librte_vhost/rte_vdpa.h
> @@ -0,0 +1,94 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2018 Intel Corporation
> + */
> +
> +#ifndef _RTE_VDPA_H_
> +#define _RTE_VDPA_H_
> +
> +/**
> + * @file
> + *
> + * Device specific vhost lib
> + */
> +
> +#include <rte_pci.h>
> +#include "rte_vhost.h"
> +
> +#define MAX_VDPA_NAME_LEN 128
> +
> +enum vdpa_addr_type {
> +	PCI_ADDR,
> +	VDPA_ADDR_MAX
> +};
> +
> +struct rte_vdpa_dev_addr {
> +	enum vdpa_addr_type type;
> +	union {
> +		uint8_t __dummy[64];
> +		struct rte_pci_addr pci_addr;
> +	};
> +};
> +
> +/* Get capabilities of this device */
> +typedef int (*vdpa_dev_queue_num_get_t)(int did, uint32_t *queue_num);
> +typedef int (*vdpa_dev_feature_get_t)(int did, uint64_t *features);
> +
> +/* Driver configure/close the device */
> +typedef int (*vdpa_dev_conf_t)(int vid);
> +typedef int (*vdpa_dev_close_t)(int vid);
> +
> +/* Enable/disable this vring */
> +typedef int (*vdpa_vring_state_set_t)(int vid, int vring, int state);
> +
> +/* Set features when changed */
> +typedef int (*vdpa_feature_set_t)(int vid);
> +
> +/* Destination operations when migration done */
> +typedef int (*vdpa_migration_done_t)(int vid);
> +
> +/* Get the vfio group fd */
> +typedef int (*vdpa_get_vfio_group_fd_t)(int vid);
> +
> +/* Get the vfio device fd */
> +typedef int (*vdpa_get_vfio_device_fd_t)(int vid);
> +
> +/* Get the notify area info of the queue */
> +typedef int (*vdpa_get_notify_area_t)(int vid, int qid, uint64_t *offset,
> +		uint64_t *size);
> +/* Device ops */
> +struct rte_vdpa_dev_ops {
> +	vdpa_dev_queue_num_get_t  queue_num_get;
> +	vdpa_dev_feature_get_t    feature_get;
> +	vdpa_dev_feature_get_t    protocol_feature_get;
> +	vdpa_dev_conf_t           dev_conf;
> +	vdpa_dev_close_t          dev_close;
> +	vdpa_vring_state_set_t    vring_state_set;
> +	vdpa_feature_set_t        feature_set;
> +	vdpa_migration_done_t     migration_done;
> +	vdpa_get_vfio_group_fd_t  get_vfio_group_fd;
> +	vdpa_get_vfio_device_fd_t get_vfio_device_fd;
> +	vdpa_get_notify_area_t    get_notify_area;

Maybe you could reserve some room here to avoid breaking the ABI in the
future if we need to add some optional ops.

> +};
> +
> +struct rte_vdpa_device {
> +	struct rte_vdpa_dev_addr addr;
> +	struct rte_vdpa_dev_ops *ops;
> +} __rte_cache_aligned;
> +
> +extern struct rte_vdpa_device *vdpa_devices[];
> +extern uint32_t vdpa_device_num;
> +
> +/* Register a vdpa device, return did if successful, -1 on failure */
> +int __rte_experimental
> +rte_vdpa_register_device(struct rte_vdpa_dev_addr *addr,
> +		struct rte_vdpa_dev_ops *ops);
> +
> +/* Unregister a vdpa device, return -1 on failure */
> +int __rte_experimental
> +rte_vdpa_unregister_device(int did);
> +
> +/* Find did of a vdpa device, return -1 on failure */
> +int __rte_experimental
> +rte_vdpa_find_device_id(struct rte_vdpa_dev_addr *addr);
> +
> +#endif /* _RTE_VDPA_H_ */
> diff --git a/lib/librte_vhost/rte_vhost_version.map b/lib/librte_vhost/rte_vhost_version.map
> index df0103129..7bcffb490 100644
> --- a/lib/librte_vhost/rte_vhost_version.map
> +++ b/lib/librte_vhost/rte_vhost_version.map
> @@ -59,3 +59,9 @@ DPDK_18.02 {
>   	rte_vhost_vring_call;
>   
>   } DPDK_17.08;
> +
> +EXPERIMENTAL {
> +	rte_vdpa_register_device;
> +	rte_vdpa_unregister_device;
> +	rte_vdpa_find_device_id;

I think you need also to declare the new structs here,
not only the new functions.

> +} DPDK_18.02;
> diff --git a/lib/librte_vhost/vdpa.c b/lib/librte_vhost/vdpa.c
> new file mode 100644
> index 000000000..0c950d45f
> --- /dev/null
> +++ b/lib/librte_vhost/vdpa.c
> @@ -0,0 +1,96 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2018 Intel Corporation
> + */
> +
> +/**
> + * @file
> + *
> + * Device specific vhost lib
> + */
> +
> +#include <stdbool.h>
> +
> +#include <rte_malloc.h>
> +#include "rte_vdpa.h"
> +#include "vhost.h"
> +
> +struct rte_vdpa_device *vdpa_devices[MAX_VHOST_DEVICE];
> +uint32_t vdpa_device_num;
> +
> +static int is_same_vdpa_dev_addr(struct rte_vdpa_dev_addr *a,
> +		struct rte_vdpa_dev_addr *b)
> +{

Given the boolean nature of the function name, I would return 1 if same
device, 0 if different.

> +	int ret = 0;
> +
> +	if (a->type != b->type)
> +		return -1;
> +
> +	switch (a->type) {
> +	case PCI_ADDR:
> +		if (a->pci_addr.domain != b->pci_addr.domain ||
> +				a->pci_addr.bus != b->pci_addr.bus ||
> +				a->pci_addr.devid != b->pci_addr.devid ||
> +				a->pci_addr.function != b->pci_addr.function)
> +			ret = -1;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +int rte_vdpa_register_device(struct rte_vdpa_dev_addr *addr,
> +		struct rte_vdpa_dev_ops *ops)
> +{
> +	struct rte_vdpa_device *dev;
> +	char device_name[MAX_VDPA_NAME_LEN];
> +	int i;
> +
> +	if (vdpa_device_num >= MAX_VHOST_DEVICE)
> +		return -1;
> +
> +	for (i = 0; i < MAX_VHOST_DEVICE; i++) {
> +		if (vdpa_devices[i] == NULL)
> +			break;
You might want to check same device isn't being registering a second
time, and return an error in that case.

This is not a blocker though, and can be done in a dedicated patch.

> +	}
> +
> +	sprintf(device_name, "vdpa-dev-%d", i);
> +	dev = rte_zmalloc(device_name, sizeof(struct rte_vdpa_device),
> +			RTE_CACHE_LINE_SIZE);
> +	if (!dev)
> +		return -1;
> +
> +	memcpy(&dev->addr, addr, sizeof(struct rte_vdpa_dev_addr));
> +	dev->ops = ops;
> +	vdpa_devices[i] = dev;
> +	vdpa_device_num++;
> +
> +	return i;
> +}
> +
> +int rte_vdpa_unregister_device(int did)
> +{
> +	if (did < 0 || did >= MAX_VHOST_DEVICE || vdpa_devices[did] == NULL)
> +		return -1;
> +
> +	rte_free(vdpa_devices[did]);
> +	vdpa_devices[did] = NULL;
> +	vdpa_device_num--;
> +
> +	return did;
> +}
> +
> +int rte_vdpa_find_device_id(struct rte_vdpa_dev_addr *addr)
> +{
> +	struct rte_vdpa_device *dev;
> +	int i;
> +
> +	for (i = 0; i < MAX_VHOST_DEVICE; ++i) {
> +		dev = vdpa_devices[i];
> +		if (dev && is_same_vdpa_dev_addr(&dev->addr, addr) == 0)
> +			return i;
> +	}
> +
> +	return -1;
> +}
>
  
Maxime Coquelin March 31, 2018, 7:38 a.m. UTC | #2
On 03/10/2018 11:01 AM, Zhihong Wang wrote:
> +		uint64_t *size);
> +/* Device ops */
> +struct rte_vdpa_dev_ops {
> +	vdpa_dev_queue_num_get_t  queue_num_get;
> +	vdpa_dev_feature_get_t    feature_get;
> +	vdpa_dev_feature_get_t    protocol_feature_get;

I would prefer them to be named as in Vhost-user spec:

get_queue_num
get_features
get_protocol_features

Thanks,
Maxime
  
Zhihong Wang April 2, 2018, 1:58 a.m. UTC | #3
> -----Original Message-----

> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]

> Sent: Saturday, March 31, 2018 2:10 PM

> To: Wang, Zhihong <zhihong.wang@intel.com>; dev@dpdk.org

> Cc: Tan, Jianfeng <jianfeng.tan@intel.com>; Bie, Tiwei

> <tiwei.bie@intel.com>; yliu@fridaylinux.org; Liang, Cunming

> <cunming.liang@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>; Daly,

> Dan <dan.daly@intel.com>

> Subject: Re: [PATCH v4 2/5] vhost: support selective datapath

> 

> 

> 

> On 03/10/2018 11:01 AM, Zhihong Wang wrote:

> > This patch set introduces support for selective datapath in DPDK vhost-user

> > lib. vDPA stands for vhost Data Path Acceleration. The idea is to support

> > virtio ring compatible devices to serve virtio driver directly to enable

> > datapath acceleration.

> >

> > A set of device ops is defined for device specific operations:

> >

> >       a. queue_num_get: Called to get supported queue number of the

> device.

> >

> >       b. feature_get: Called to get supported features of the device.

> >

> >       c. protocol_feature_get: Called to get supported protocol features of

> >          the device.

> >

> >       d. dev_conf: Called to configure the actual device when the virtio

> >          device becomes ready.

> >

> >       e. dev_close: Called to close the actual device when the virtio device

> >          is stopped.

> >

> >       f. vring_state_set: Called to change the state of the vring in the

> >          actual device when vring state changes.

> >

> >       g. feature_set: Called to set the negotiated features to device.

> >

> >       h. migration_done: Called to allow the device to response to RARP

> >          sending.

> >

> >       i. get_vfio_group_fd: Called to get the VFIO group fd of the device.

> >

> >       j. get_vfio_device_fd: Called to get the VFIO device fd of the device.

> >

> >       k. get_notify_area: Called to get the notify area info of the queue.

> >

> > Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>

> > ---

> > Changes in v4:

> >

> >   1. Remove the "engine" concept in the lib.

> >

> > ---

> > Changes in v2:

> >

> >   1. Add VFIO related vDPA device ops.

> >

> >   lib/librte_vhost/Makefile              |  4 +-

> >   lib/librte_vhost/rte_vdpa.h            | 94

> +++++++++++++++++++++++++++++++++

> >   lib/librte_vhost/rte_vhost_version.map |  6 +++

> >   lib/librte_vhost/vdpa.c                | 96

> ++++++++++++++++++++++++++++++++++

> >   4 files changed, 198 insertions(+), 2 deletions(-)

> >   create mode 100644 lib/librte_vhost/rte_vdpa.h

> >   create mode 100644 lib/librte_vhost/vdpa.c

> >

> > diff --git a/lib/librte_vhost/Makefile b/lib/librte_vhost/Makefile

> > index 5d6c6abae..37044ac03 100644

> > --- a/lib/librte_vhost/Makefile

> > +++ b/lib/librte_vhost/Makefile

> > @@ -22,9 +22,9 @@ LDLIBS += -lrte_eal -lrte_mempool -lrte_mbuf -

> lrte_ethdev -lrte_net

> >

> >   # all source are stored in SRCS-y

> >   SRCS-$(CONFIG_RTE_LIBRTE_VHOST) := fd_man.c iotlb.c socket.c vhost.c

> \

> > -					vhost_user.c virtio_net.c

> > +					vhost_user.c virtio_net.c vdpa.c

> >

> >   # install includes

> > -SYMLINK-$(CONFIG_RTE_LIBRTE_VHOST)-include += rte_vhost.h

> > +SYMLINK-$(CONFIG_RTE_LIBRTE_VHOST)-include += rte_vhost.h

> rte_vdpa.h

> >

> >   include $(RTE_SDK)/mk/rte.lib.mk

> > diff --git a/lib/librte_vhost/rte_vdpa.h b/lib/librte_vhost/rte_vdpa.h

> > new file mode 100644

> > index 000000000..a4bbbd93d

> > --- /dev/null

> > +++ b/lib/librte_vhost/rte_vdpa.h

> > @@ -0,0 +1,94 @@

> > +/* SPDX-License-Identifier: BSD-3-Clause

> > + * Copyright(c) 2018 Intel Corporation

> > + */

> > +

> > +#ifndef _RTE_VDPA_H_

> > +#define _RTE_VDPA_H_

> > +

> > +/**

> > + * @file

> > + *

> > + * Device specific vhost lib

> > + */

> > +

> > +#include <rte_pci.h>

> > +#include "rte_vhost.h"

> > +

> > +#define MAX_VDPA_NAME_LEN 128

> > +

> > +enum vdpa_addr_type {

> > +	PCI_ADDR,

> > +	VDPA_ADDR_MAX

> > +};

> > +

> > +struct rte_vdpa_dev_addr {

> > +	enum vdpa_addr_type type;

> > +	union {

> > +		uint8_t __dummy[64];

> > +		struct rte_pci_addr pci_addr;

> > +	};

> > +};

> > +

> > +/* Get capabilities of this device */

> > +typedef int (*vdpa_dev_queue_num_get_t)(int did, uint32_t

> *queue_num);

> > +typedef int (*vdpa_dev_feature_get_t)(int did, uint64_t *features);

> > +

> > +/* Driver configure/close the device */

> > +typedef int (*vdpa_dev_conf_t)(int vid);

> > +typedef int (*vdpa_dev_close_t)(int vid);

> > +

> > +/* Enable/disable this vring */

> > +typedef int (*vdpa_vring_state_set_t)(int vid, int vring, int state);

> > +

> > +/* Set features when changed */

> > +typedef int (*vdpa_feature_set_t)(int vid);

> > +

> > +/* Destination operations when migration done */

> > +typedef int (*vdpa_migration_done_t)(int vid);

> > +

> > +/* Get the vfio group fd */

> > +typedef int (*vdpa_get_vfio_group_fd_t)(int vid);

> > +

> > +/* Get the vfio device fd */

> > +typedef int (*vdpa_get_vfio_device_fd_t)(int vid);

> > +

> > +/* Get the notify area info of the queue */

> > +typedef int (*vdpa_get_notify_area_t)(int vid, int qid, uint64_t *offset,

> > +		uint64_t *size);

> > +/* Device ops */

> > +struct rte_vdpa_dev_ops {

> > +	vdpa_dev_queue_num_get_t  queue_num_get;

> > +	vdpa_dev_feature_get_t    feature_get;

> > +	vdpa_dev_feature_get_t    protocol_feature_get;

> > +	vdpa_dev_conf_t           dev_conf;

> > +	vdpa_dev_close_t          dev_close;

> > +	vdpa_vring_state_set_t    vring_state_set;

> > +	vdpa_feature_set_t        feature_set;

> > +	vdpa_migration_done_t     migration_done;

> > +	vdpa_get_vfio_group_fd_t  get_vfio_group_fd;

> > +	vdpa_get_vfio_device_fd_t get_vfio_device_fd;

> > +	vdpa_get_notify_area_t    get_notify_area;

> 

> Maybe you could reserve some room here to avoid breaking the ABI in the

> future if we need to add some optional ops.


Good suggestion.

> 

> > +};

> > +

> > +struct rte_vdpa_device {

> > +	struct rte_vdpa_dev_addr addr;

> > +	struct rte_vdpa_dev_ops *ops;

> > +} __rte_cache_aligned;

> > +

> > +extern struct rte_vdpa_device *vdpa_devices[];

> > +extern uint32_t vdpa_device_num;

> > +

> > +/* Register a vdpa device, return did if successful, -1 on failure */

> > +int __rte_experimental

> > +rte_vdpa_register_device(struct rte_vdpa_dev_addr *addr,

> > +		struct rte_vdpa_dev_ops *ops);

> > +

> > +/* Unregister a vdpa device, return -1 on failure */

> > +int __rte_experimental

> > +rte_vdpa_unregister_device(int did);

> > +

> > +/* Find did of a vdpa device, return -1 on failure */

> > +int __rte_experimental

> > +rte_vdpa_find_device_id(struct rte_vdpa_dev_addr *addr);

> > +

> > +#endif /* _RTE_VDPA_H_ */

> > diff --git a/lib/librte_vhost/rte_vhost_version.map

> b/lib/librte_vhost/rte_vhost_version.map

> > index df0103129..7bcffb490 100644

> > --- a/lib/librte_vhost/rte_vhost_version.map

> > +++ b/lib/librte_vhost/rte_vhost_version.map

> > @@ -59,3 +59,9 @@ DPDK_18.02 {

> >   	rte_vhost_vring_call;

> >

> >   } DPDK_17.08;

> > +

> > +EXPERIMENTAL {

> > +	rte_vdpa_register_device;

> > +	rte_vdpa_unregister_device;

> > +	rte_vdpa_find_device_id;

> 

> I think you need also to declare the new structs here,

> not only the new functions.


Ok.

> 

> > +} DPDK_18.02;

> > diff --git a/lib/librte_vhost/vdpa.c b/lib/librte_vhost/vdpa.c

> > new file mode 100644

> > index 000000000..0c950d45f

> > --- /dev/null

> > +++ b/lib/librte_vhost/vdpa.c

> > @@ -0,0 +1,96 @@

> > +/* SPDX-License-Identifier: BSD-3-Clause

> > + * Copyright(c) 2018 Intel Corporation

> > + */

> > +

> > +/**

> > + * @file

> > + *

> > + * Device specific vhost lib

> > + */

> > +

> > +#include <stdbool.h>

> > +

> > +#include <rte_malloc.h>

> > +#include "rte_vdpa.h"

> > +#include "vhost.h"

> > +

> > +struct rte_vdpa_device *vdpa_devices[MAX_VHOST_DEVICE];

> > +uint32_t vdpa_device_num;

> > +

> > +static int is_same_vdpa_dev_addr(struct rte_vdpa_dev_addr *a,

> > +		struct rte_vdpa_dev_addr *b)

> > +{

> 

> Given the boolean nature of the function name, I would return 1 if same

> device, 0 if different.


Ok, will use bool.

> 

> > +	int ret = 0;

> > +

> > +	if (a->type != b->type)

> > +		return -1;

> > +

> > +	switch (a->type) {

> > +	case PCI_ADDR:

> > +		if (a->pci_addr.domain != b->pci_addr.domain ||

> > +				a->pci_addr.bus != b->pci_addr.bus ||

> > +				a->pci_addr.devid != b->pci_addr.devid ||

> > +				a->pci_addr.function != b->pci_addr.function)

> > +			ret = -1;

> > +		break;

> > +	default:

> > +		break;

> > +	}

> > +

> > +	return ret;

> > +}

> > +

> > +int rte_vdpa_register_device(struct rte_vdpa_dev_addr *addr,

> > +		struct rte_vdpa_dev_ops *ops)

> > +{

> > +	struct rte_vdpa_device *dev;

> > +	char device_name[MAX_VDPA_NAME_LEN];

> > +	int i;

> > +

> > +	if (vdpa_device_num >= MAX_VHOST_DEVICE)

> > +		return -1;

> > +

> > +	for (i = 0; i < MAX_VHOST_DEVICE; i++) {

> > +		if (vdpa_devices[i] == NULL)

> > +			break;

> You might want to check same device isn't being registering a second

> time, and return an error in that case.


Will do.

Thanks
-Zhihong

> 

> This is not a blocker though, and can be done in a dedicated patch.

> 

> > +	}

> > +

> > +	sprintf(device_name, "vdpa-dev-%d", i);

> > +	dev = rte_zmalloc(device_name, sizeof(struct rte_vdpa_device),

> > +			RTE_CACHE_LINE_SIZE);

> > +	if (!dev)

> > +		return -1;

> > +

> > +	memcpy(&dev->addr, addr, sizeof(struct rte_vdpa_dev_addr));

> > +	dev->ops = ops;

> > +	vdpa_devices[i] = dev;

> > +	vdpa_device_num++;

> > +

> > +	return i;

> > +}

> > +

> > +int rte_vdpa_unregister_device(int did)

> > +{

> > +	if (did < 0 || did >= MAX_VHOST_DEVICE || vdpa_devices[did] ==

> NULL)

> > +		return -1;

> > +

> > +	rte_free(vdpa_devices[did]);

> > +	vdpa_devices[did] = NULL;

> > +	vdpa_device_num--;

> > +

> > +	return did;

> > +}

> > +

> > +int rte_vdpa_find_device_id(struct rte_vdpa_dev_addr *addr)

> > +{

> > +	struct rte_vdpa_device *dev;

> > +	int i;

> > +

> > +	for (i = 0; i < MAX_VHOST_DEVICE; ++i) {

> > +		dev = vdpa_devices[i];

> > +		if (dev && is_same_vdpa_dev_addr(&dev->addr, addr) == 0)

> > +			return i;

> > +	}

> > +

> > +	return -1;

> > +}

> >
  
Zhihong Wang April 2, 2018, 2:03 a.m. UTC | #4
> -----Original Message-----

> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]

> Sent: Saturday, March 31, 2018 3:38 PM

> To: Wang, Zhihong <zhihong.wang@intel.com>; dev@dpdk.org

> Cc: Tan, Jianfeng <jianfeng.tan@intel.com>; Bie, Tiwei

> <tiwei.bie@intel.com>; yliu@fridaylinux.org; Liang, Cunming

> <cunming.liang@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>; Daly,

> Dan <dan.daly@intel.com>

> Subject: Re: [PATCH v4 2/5] vhost: support selective datapath

> 

> 

> 

> On 03/10/2018 11:01 AM, Zhihong Wang wrote:

> > +		uint64_t *size);

> > +/* Device ops */

> > +struct rte_vdpa_dev_ops {

> > +	vdpa_dev_queue_num_get_t  queue_num_get;

> > +	vdpa_dev_feature_get_t    feature_get;

> > +	vdpa_dev_feature_get_t    protocol_feature_get;

> 

> I would prefer them to be named as in Vhost-user spec:

> 

> get_queue_num

> get_features

> get_protocol_features


Ok. Will change them.

> 

> Thanks,

> Maxime
  

Patch

diff --git a/lib/librte_vhost/Makefile b/lib/librte_vhost/Makefile
index 5d6c6abae..37044ac03 100644
--- a/lib/librte_vhost/Makefile
+++ b/lib/librte_vhost/Makefile
@@ -22,9 +22,9 @@  LDLIBS += -lrte_eal -lrte_mempool -lrte_mbuf -lrte_ethdev -lrte_net
 
 # all source are stored in SRCS-y
 SRCS-$(CONFIG_RTE_LIBRTE_VHOST) := fd_man.c iotlb.c socket.c vhost.c \
-					vhost_user.c virtio_net.c
+					vhost_user.c virtio_net.c vdpa.c
 
 # install includes
-SYMLINK-$(CONFIG_RTE_LIBRTE_VHOST)-include += rte_vhost.h
+SYMLINK-$(CONFIG_RTE_LIBRTE_VHOST)-include += rte_vhost.h rte_vdpa.h
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_vhost/rte_vdpa.h b/lib/librte_vhost/rte_vdpa.h
new file mode 100644
index 000000000..a4bbbd93d
--- /dev/null
+++ b/lib/librte_vhost/rte_vdpa.h
@@ -0,0 +1,94 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation
+ */
+
+#ifndef _RTE_VDPA_H_
+#define _RTE_VDPA_H_
+
+/**
+ * @file
+ *
+ * Device specific vhost lib
+ */
+
+#include <rte_pci.h>
+#include "rte_vhost.h"
+
+#define MAX_VDPA_NAME_LEN 128
+
+enum vdpa_addr_type {
+	PCI_ADDR,
+	VDPA_ADDR_MAX
+};
+
+struct rte_vdpa_dev_addr {
+	enum vdpa_addr_type type;
+	union {
+		uint8_t __dummy[64];
+		struct rte_pci_addr pci_addr;
+	};
+};
+
+/* Get capabilities of this device */
+typedef int (*vdpa_dev_queue_num_get_t)(int did, uint32_t *queue_num);
+typedef int (*vdpa_dev_feature_get_t)(int did, uint64_t *features);
+
+/* Driver configure/close the device */
+typedef int (*vdpa_dev_conf_t)(int vid);
+typedef int (*vdpa_dev_close_t)(int vid);
+
+/* Enable/disable this vring */
+typedef int (*vdpa_vring_state_set_t)(int vid, int vring, int state);
+
+/* Set features when changed */
+typedef int (*vdpa_feature_set_t)(int vid);
+
+/* Destination operations when migration done */
+typedef int (*vdpa_migration_done_t)(int vid);
+
+/* Get the vfio group fd */
+typedef int (*vdpa_get_vfio_group_fd_t)(int vid);
+
+/* Get the vfio device fd */
+typedef int (*vdpa_get_vfio_device_fd_t)(int vid);
+
+/* Get the notify area info of the queue */
+typedef int (*vdpa_get_notify_area_t)(int vid, int qid, uint64_t *offset,
+		uint64_t *size);
+/* Device ops */
+struct rte_vdpa_dev_ops {
+	vdpa_dev_queue_num_get_t  queue_num_get;
+	vdpa_dev_feature_get_t    feature_get;
+	vdpa_dev_feature_get_t    protocol_feature_get;
+	vdpa_dev_conf_t           dev_conf;
+	vdpa_dev_close_t          dev_close;
+	vdpa_vring_state_set_t    vring_state_set;
+	vdpa_feature_set_t        feature_set;
+	vdpa_migration_done_t     migration_done;
+	vdpa_get_vfio_group_fd_t  get_vfio_group_fd;
+	vdpa_get_vfio_device_fd_t get_vfio_device_fd;
+	vdpa_get_notify_area_t    get_notify_area;
+};
+
+struct rte_vdpa_device {
+	struct rte_vdpa_dev_addr addr;
+	struct rte_vdpa_dev_ops *ops;
+} __rte_cache_aligned;
+
+extern struct rte_vdpa_device *vdpa_devices[];
+extern uint32_t vdpa_device_num;
+
+/* Register a vdpa device, return did if successful, -1 on failure */
+int __rte_experimental
+rte_vdpa_register_device(struct rte_vdpa_dev_addr *addr,
+		struct rte_vdpa_dev_ops *ops);
+
+/* Unregister a vdpa device, return -1 on failure */
+int __rte_experimental
+rte_vdpa_unregister_device(int did);
+
+/* Find did of a vdpa device, return -1 on failure */
+int __rte_experimental
+rte_vdpa_find_device_id(struct rte_vdpa_dev_addr *addr);
+
+#endif /* _RTE_VDPA_H_ */
diff --git a/lib/librte_vhost/rte_vhost_version.map b/lib/librte_vhost/rte_vhost_version.map
index df0103129..7bcffb490 100644
--- a/lib/librte_vhost/rte_vhost_version.map
+++ b/lib/librte_vhost/rte_vhost_version.map
@@ -59,3 +59,9 @@  DPDK_18.02 {
 	rte_vhost_vring_call;
 
 } DPDK_17.08;
+
+EXPERIMENTAL {
+	rte_vdpa_register_device;
+	rte_vdpa_unregister_device;
+	rte_vdpa_find_device_id;
+} DPDK_18.02;
diff --git a/lib/librte_vhost/vdpa.c b/lib/librte_vhost/vdpa.c
new file mode 100644
index 000000000..0c950d45f
--- /dev/null
+++ b/lib/librte_vhost/vdpa.c
@@ -0,0 +1,96 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation
+ */
+
+/**
+ * @file
+ *
+ * Device specific vhost lib
+ */
+
+#include <stdbool.h>
+
+#include <rte_malloc.h>
+#include "rte_vdpa.h"
+#include "vhost.h"
+
+struct rte_vdpa_device *vdpa_devices[MAX_VHOST_DEVICE];
+uint32_t vdpa_device_num;
+
+static int is_same_vdpa_dev_addr(struct rte_vdpa_dev_addr *a,
+		struct rte_vdpa_dev_addr *b)
+{
+	int ret = 0;
+
+	if (a->type != b->type)
+		return -1;
+
+	switch (a->type) {
+	case PCI_ADDR:
+		if (a->pci_addr.domain != b->pci_addr.domain ||
+				a->pci_addr.bus != b->pci_addr.bus ||
+				a->pci_addr.devid != b->pci_addr.devid ||
+				a->pci_addr.function != b->pci_addr.function)
+			ret = -1;
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+int rte_vdpa_register_device(struct rte_vdpa_dev_addr *addr,
+		struct rte_vdpa_dev_ops *ops)
+{
+	struct rte_vdpa_device *dev;
+	char device_name[MAX_VDPA_NAME_LEN];
+	int i;
+
+	if (vdpa_device_num >= MAX_VHOST_DEVICE)
+		return -1;
+
+	for (i = 0; i < MAX_VHOST_DEVICE; i++) {
+		if (vdpa_devices[i] == NULL)
+			break;
+	}
+
+	sprintf(device_name, "vdpa-dev-%d", i);
+	dev = rte_zmalloc(device_name, sizeof(struct rte_vdpa_device),
+			RTE_CACHE_LINE_SIZE);
+	if (!dev)
+		return -1;
+
+	memcpy(&dev->addr, addr, sizeof(struct rte_vdpa_dev_addr));
+	dev->ops = ops;
+	vdpa_devices[i] = dev;
+	vdpa_device_num++;
+
+	return i;
+}
+
+int rte_vdpa_unregister_device(int did)
+{
+	if (did < 0 || did >= MAX_VHOST_DEVICE || vdpa_devices[did] == NULL)
+		return -1;
+
+	rte_free(vdpa_devices[did]);
+	vdpa_devices[did] = NULL;
+	vdpa_device_num--;
+
+	return did;
+}
+
+int rte_vdpa_find_device_id(struct rte_vdpa_dev_addr *addr)
+{
+	struct rte_vdpa_device *dev;
+	int i;
+
+	for (i = 0; i < MAX_VHOST_DEVICE; ++i) {
+		dev = vdpa_devices[i];
+		if (dev && is_same_vdpa_dev_addr(&dev->addr, addr) == 0)
+			return i;
+	}
+
+	return -1;
+}