[dpdk-dev,v3,3/5] vhost: add apis for datapath configuration

Message ID 20180227101342.18521-4-zhihong.wang@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Maxime Coquelin
Headers

Checks

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

Commit Message

Zhihong Wang Feb. 27, 2018, 10:13 a.m. UTC
  This patch adds APIs for datapath configuration. The eid and did of the
vhost-user socket can be configured to identify the actual device.

When the default software datapath is used, eid and did are set to -1.
When alternative datapath is used, eid and did are set by app to specify
which device to use. Each vhost-user socket can have only 1 connection in
this case.

Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
---
 lib/librte_vhost/rte_vhost.h           | 70 ++++++++++++++++++++++++++++++++++
 lib/librte_vhost/rte_vhost_version.map |  6 +++
 lib/librte_vhost/socket.c              | 65 +++++++++++++++++++++++++++++++
 lib/librte_vhost/vhost.c               | 50 ++++++++++++++++++++++++
 lib/librte_vhost/vhost.h               | 10 +++++
 5 files changed, 201 insertions(+)
  

Comments

Maxime Coquelin March 21, 2018, 9:08 p.m. UTC | #1
On 02/27/2018 11:13 AM, Zhihong Wang wrote:
> This patch adds APIs for datapath configuration. The eid and did of the
> vhost-user socket can be configured to identify the actual device.
> 
> When the default software datapath is used, eid and did are set to -1.
> When alternative datapath is used, eid and did are set by app to specify
> which device to use. Each vhost-user socket can have only 1 connection in
> this case.
> 
> Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
> ---
>   lib/librte_vhost/rte_vhost.h           | 70 ++++++++++++++++++++++++++++++++++
>   lib/librte_vhost/rte_vhost_version.map |  6 +++
>   lib/librte_vhost/socket.c              | 65 +++++++++++++++++++++++++++++++
>   lib/librte_vhost/vhost.c               | 50 ++++++++++++++++++++++++
>   lib/librte_vhost/vhost.h               | 10 +++++
>   5 files changed, 201 insertions(+)
> 

Isn't the notion of EID & DID Intel specifics?
At vhost API level, shouldn't we only care of the offload device ID?
  
Zhihong Wang March 22, 2018, 8:22 a.m. UTC | #2
> -----Original Message-----

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

> Sent: Thursday, March 22, 2018 5:08 AM

> 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 v3 3/5] vhost: add apis for datapath configuration

> 

> 

> 

> On 02/27/2018 11:13 AM, Zhihong Wang wrote:

> > This patch adds APIs for datapath configuration. The eid and did of the

> > vhost-user socket can be configured to identify the actual device.

> >

> > When the default software datapath is used, eid and did are set to -1.

> > When alternative datapath is used, eid and did are set by app to specify

> > which device to use. Each vhost-user socket can have only 1 connection in

> > this case.

> >

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

> > ---

> >   lib/librte_vhost/rte_vhost.h           | 70

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

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

> >   lib/librte_vhost/socket.c              | 65

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

> >   lib/librte_vhost/vhost.c               | 50 ++++++++++++++++++++++++

> >   lib/librte_vhost/vhost.h               | 10 +++++

> >   5 files changed, 201 insertions(+)

> >

> 

> Isn't the notion of EID & DID Intel specifics?

> At vhost API level, shouldn't we only care of the offload device ID?


It's not vendor specific: Engine id refers to an engine which is a device
on a bus, the engine could have multiple queue pairs or virtual functions.
The driver can manage them to present multiple vhost ports with vDPA to
application, so logically the concept of device id exists.

In a lot of acceleration cases, application needs to be able to choose the
exact port to use instead of letting the driver to decide (because it does
make a difference), therefore it's necessary to expose the device id here.
  
Maxime Coquelin March 22, 2018, 2:18 p.m. UTC | #3
Hi,

On 03/22/2018 09:22 AM, Wang, Zhihong wrote:
> 
> 
>> -----Original Message-----
>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>> Sent: Thursday, March 22, 2018 5:08 AM
>> 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 v3 3/5] vhost: add apis for datapath configuration
>>
>>
>>
>> On 02/27/2018 11:13 AM, Zhihong Wang wrote:
>>> This patch adds APIs for datapath configuration. The eid and did of the
>>> vhost-user socket can be configured to identify the actual device.
>>>
>>> When the default software datapath is used, eid and did are set to -1.
>>> When alternative datapath is used, eid and did are set by app to specify
>>> which device to use. Each vhost-user socket can have only 1 connection in
>>> this case.
>>>
>>> Signed-off-by: Zhihong Wang <zhihong.wang@intel.com>
>>> ---
>>>    lib/librte_vhost/rte_vhost.h           | 70
>> ++++++++++++++++++++++++++++++++++
>>>    lib/librte_vhost/rte_vhost_version.map |  6 +++
>>>    lib/librte_vhost/socket.c              | 65
>> +++++++++++++++++++++++++++++++
>>>    lib/librte_vhost/vhost.c               | 50 ++++++++++++++++++++++++
>>>    lib/librte_vhost/vhost.h               | 10 +++++
>>>    5 files changed, 201 insertions(+)
>>>
>>
>> Isn't the notion of EID & DID Intel specifics?
>> At vhost API level, shouldn't we only care of the offload device ID?
> 
> It's not vendor specific: Engine id refers to an engine which is a device
> on a bus, the engine could have multiple queue pairs or virtual functions.
> The driver can manage them to present multiple vhost ports with vDPA to
> application, so logically the concept of device id exists.
> 
> In a lot of acceleration cases, application needs to be able to choose the
> exact port to use instead of letting the driver to decide (because it does
> make a difference), therefore it's necessary to expose the device id here.

Yes, but if I understood correctly with the IFCVF driver, we could pass 
directly the virtual function to the vhost-user lib, no need to specify
the engine. We would just need to register one device per VF, but that 
looks like the right think to do looking at how IFCVF manages MAC
addresses and link status for example.

Thanks,
Maxime
  
Zhihong Wang March 23, 2018, 10:35 a.m. UTC | #4
Hi Maxime,

> -----Original Message-----

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

> Sent: Thursday, March 22, 2018 10:19 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 v3 3/5] vhost: add apis for datapath configuration

> 

> Hi,

> 

> On 03/22/2018 09:22 AM, Wang, Zhihong wrote:

> >

> >

> >> -----Original Message-----

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

> >> Sent: Thursday, March 22, 2018 5:08 AM

> >> 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 v3 3/5] vhost: add apis for datapath configuration

> >>

> >>

> >>

> >> On 02/27/2018 11:13 AM, Zhihong Wang wrote:

> >>> This patch adds APIs for datapath configuration. The eid and did of the

> >>> vhost-user socket can be configured to identify the actual device.

> >>>

> >>> When the default software datapath is used, eid and did are set to -1.

> >>> When alternative datapath is used, eid and did are set by app to specify

> >>> which device to use. Each vhost-user socket can have only 1 connection

> in

> >>> this case.

> >>>

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

> >>> ---

> >>>    lib/librte_vhost/rte_vhost.h           | 70

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

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

> >>>    lib/librte_vhost/socket.c              | 65

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

> >>>    lib/librte_vhost/vhost.c               | 50 ++++++++++++++++++++++++

> >>>    lib/librte_vhost/vhost.h               | 10 +++++

> >>>    5 files changed, 201 insertions(+)

> >>>

> >>

> >> Isn't the notion of EID & DID Intel specifics?

> >> At vhost API level, shouldn't we only care of the offload device ID?

> >

> > It's not vendor specific: Engine id refers to an engine which is a device

> > on a bus, the engine could have multiple queue pairs or virtual functions.

> > The driver can manage them to present multiple vhost ports with vDPA to

> > application, so logically the concept of device id exists.

> >

> > In a lot of acceleration cases, application needs to be able to choose the

> > exact port to use instead of letting the driver to decide (because it does

> > make a difference), therefore it's necessary to expose the device id here.

> 

> Yes, but if I understood correctly with the IFCVF driver, we could pass

> directly the virtual function to the vhost-user lib, no need to specify

> the engine. We would just need to register one device per VF, but that

> looks like the right think to do looking at how IFCVF manages MAC

> addresses and link status for example.


The lib is for generic designs. An engine could also be an AFU device [1]
which has multiple virtio ring compatible queue pairs that can serve
different VMs independently, instead of multiple virtual functions. In
this case, we need eid to index the AFU device, and did to index the queue
pair(s).

[1] http://dpdk.org/ml/archives/dev/2018-March/093343.html (struct rte_afu_device)

Thanks
-Zhihong

> 

> Thanks,

> Maxime
  

Patch

diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
index 671ea5053..7aa57ca87 100644
--- a/lib/librte_vhost/rte_vhost.h
+++ b/lib/librte_vhost/rte_vhost.h
@@ -200,6 +200,54 @@  int rte_vhost_driver_register(const char *path, uint64_t flags);
 int rte_vhost_driver_unregister(const char *path);
 
 /**
+ * Set the engine id, enforce single connection per socket
+ *
+ * @param path
+ *  The vhost-user socket file path
+ * @param eid
+ *  Engine id
+ * @return
+ *  0 on success, -1 on failure
+ */
+int __rte_experimental
+rte_vhost_driver_set_vdpa_eid(const char *path, int eid);
+
+/**
+ * Set the device id, enforce single connection per socket
+ *
+ * @param path
+ *  The vhost-user socket file path
+ * @param did
+ *  Device id
+ * @return
+ *  0 on success, -1 on failure
+ */
+int __rte_experimental
+rte_vhost_driver_set_vdpa_did(const char *path, int did);
+
+/**
+ * Get the engine id
+ *
+ * @param path
+ *  The vhost-user socket file path
+ * @return
+ *  Engine id, -1 on failure
+ */
+int __rte_experimental
+rte_vhost_driver_get_vdpa_eid(const char *path);
+
+/**
+ * Get the device id
+ *
+ * @param path
+ *  The vhost-user socket file path
+ * @return
+ *  Device id, -1 on failure
+ */
+int __rte_experimental
+rte_vhost_driver_get_vdpa_did(const char *path);
+
+/**
  * Set the feature bits the vhost-user driver supports.
  *
  * @param path
@@ -464,6 +512,28 @@  int rte_vhost_vring_call(int vid, uint16_t vring_idx);
  */
 uint32_t rte_vhost_rx_queue_count(int vid, uint16_t qid);
 
+/**
+ * Get vdpa engine id for vhost device.
+ *
+ * @param vid
+ *  vhost device ID
+ * @return
+ *  engine id
+ */
+int __rte_experimental
+rte_vhost_get_vdpa_eid(int vid);
+
+/**
+ * Get vdpa device id for vhost device.
+ *
+ * @param vid
+ *  vhost device ID
+ * @return
+ *  device id
+ */
+int __rte_experimental
+rte_vhost_get_vdpa_did(int vid);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_vhost/rte_vhost_version.map b/lib/librte_vhost/rte_vhost_version.map
index 57a3edd01..c505596c5 100644
--- a/lib/librte_vhost/rte_vhost_version.map
+++ b/lib/librte_vhost/rte_vhost_version.map
@@ -66,4 +66,10 @@  EXPERIMENTAL {
 	rte_vdpa_find_engine_id;
 	rte_vdpa_info_query;
 	rte_vdpa_register_driver;
+	rte_vhost_driver_set_vdpa_eid;
+	rte_vhost_driver_set_vdpa_did;
+	rte_vhost_driver_get_vdpa_eid;
+	rte_vhost_driver_get_vdpa_did;
+	rte_vhost_get_vdpa_eid;
+	rte_vhost_get_vdpa_did;
 } DPDK_18.02;
diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index cfc31e179..8551eb58c 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -52,6 +52,13 @@  struct vhost_user_socket {
 	uint64_t supported_features;
 	uint64_t features;
 
+	/* engine and device id to identify a certain port on a specific
+	 * backend, both are set to -1 for sw. when used, one socket can
+	 * have 1 connection only.
+	 */
+	int eid;
+	int did;
+
 	struct vhost_device_ops const *notify_ops;
 };
 
@@ -545,6 +552,64 @@  find_vhost_user_socket(const char *path)
 }
 
 int
+rte_vhost_driver_set_vdpa_eid(const char *path, int eid)
+{
+	struct vhost_user_socket *vsocket;
+
+	pthread_mutex_lock(&vhost_user.mutex);
+	vsocket = find_vhost_user_socket(path);
+	if (vsocket)
+		vsocket->eid = eid;
+	pthread_mutex_unlock(&vhost_user.mutex);
+
+	return vsocket ? 0 : -1;
+}
+
+int
+rte_vhost_driver_set_vdpa_did(const char *path, int did)
+{
+	struct vhost_user_socket *vsocket;
+
+	pthread_mutex_lock(&vhost_user.mutex);
+	vsocket = find_vhost_user_socket(path);
+	if (vsocket)
+		vsocket->did = did;
+	pthread_mutex_unlock(&vhost_user.mutex);
+
+	return vsocket ? 0 : -1;
+}
+
+int
+rte_vhost_driver_get_vdpa_eid(const char *path)
+{
+	struct vhost_user_socket *vsocket;
+	int eid = -1;
+
+	pthread_mutex_lock(&vhost_user.mutex);
+	vsocket = find_vhost_user_socket(path);
+	if (vsocket)
+		eid = vsocket->eid;
+	pthread_mutex_unlock(&vhost_user.mutex);
+
+	return eid;
+}
+
+int
+rte_vhost_driver_get_vdpa_did(const char *path)
+{
+	struct vhost_user_socket *vsocket;
+	int did = -1;
+
+	pthread_mutex_lock(&vhost_user.mutex);
+	vsocket = find_vhost_user_socket(path);
+	if (vsocket)
+		did = vsocket->did;
+	pthread_mutex_unlock(&vhost_user.mutex);
+
+	return did;
+}
+
+int
 rte_vhost_driver_disable_features(const char *path, uint64_t features)
 {
 	struct vhost_user_socket *vsocket;
diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index f6f12a03b..45cf90f99 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -283,6 +283,8 @@  vhost_new_device(void)
 	dev->vid = i;
 	dev->flags = VIRTIO_DEV_BUILTIN_VIRTIO_NET;
 	dev->slave_req_fd = -1;
+	dev->eid = -1;
+	dev->did = -1;
 
 	return i;
 }
@@ -311,6 +313,34 @@  vhost_destroy_device(int vid)
 }
 
 void
+vhost_set_vdpa_eid(int vid, int eid)
+{
+	struct virtio_net *dev = get_device(vid);
+
+	if (eid < 0 || eid >= MAX_VDPA_ENGINE_NUM || vdpa_engines[eid] == NULL)
+		return;
+
+	if (dev == NULL)
+		return;
+
+	dev->eid = eid;
+}
+
+void
+vhost_set_vdpa_did(int vid, int did)
+{
+	struct virtio_net *dev = get_device(vid);
+
+	if (did < 0)
+		return;
+
+	if (dev == NULL)
+		return;
+
+	dev->did = did;
+}
+
+void
 vhost_set_ifname(int vid, const char *if_name, unsigned int if_len)
 {
 	struct virtio_net *dev;
@@ -614,3 +644,23 @@  rte_vhost_rx_queue_count(int vid, uint16_t qid)
 
 	return *((volatile uint16_t *)&vq->avail->idx) - vq->last_avail_idx;
 }
+
+int rte_vhost_get_vdpa_eid(int vid)
+{
+	struct virtio_net *dev = get_device(vid);
+
+	if (dev == NULL)
+		return -1;
+
+	return dev->eid;
+}
+
+int rte_vhost_get_vdpa_did(int vid)
+{
+	struct virtio_net *dev = get_device(vid);
+
+	if (dev == NULL)
+		return -1;
+
+	return dev->did;
+}
diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 2e28e4026..e06d789fa 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -19,6 +19,7 @@ 
 #include <rte_rwlock.h>
 
 #include "rte_vhost.h"
+#include "rte_vdpa.h"
 
 /* Used to indicate that the device is running on a data core */
 #define VIRTIO_DEV_RUNNING 1
@@ -239,6 +240,12 @@  struct virtio_net {
 	struct guest_page       *guest_pages;
 
 	int			slave_req_fd;
+
+	/* engine and device id to identify a certain port on a specific
+	 * backend, both are set to -1 for sw.
+	 */
+	int			eid;
+	int			did;
 } __rte_cache_aligned;
 
 
@@ -365,6 +372,9 @@  void free_vq(struct vhost_virtqueue *vq);
 
 int alloc_vring_queue(struct virtio_net *dev, uint32_t vring_idx);
 
+void vhost_set_vdpa_eid(int vid, int eid);
+void vhost_set_vdpa_did(int vid, int did);
+
 void vhost_set_ifname(int, const char *if_name, unsigned int if_len);
 void vhost_enable_dequeue_zero_copy(int vid);
 void vhost_set_builtin_virtio_net(int vid, bool enable);